* [PATCH] tunnel: eliminate recursion field @ 2009-09-23 20:17 Eric Dumazet 2009-09-23 20:28 ` Eric Dumazet 2009-09-25 21:51 ` Herbert Xu 0 siblings, 2 replies; 4+ messages in thread From: Eric Dumazet @ 2009-09-23 20:17 UTC (permalink / raw) To: David S. Miller; +Cc: Linux Netdev List It seems recursion field from "struct ip_tunnel" is not anymore needed. recursion prevention is done at the upper level (in dev_queue_xmit()), since we use HARD_TX_LOCK protection for tunnels. This avoids a cache line ping pong on "struct ip_tunnel" : This structure should be now mostly read on xmit and receive paths. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/ipip.h | 1 - net/ipv4/ip_gre.c | 13 +------------ net/ipv4/ipip.c | 8 -------- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/include/net/ipip.h b/include/net/ipip.h index 5d3036f..76e3ea6 100644 --- a/include/net/ipip.h +++ b/include/net/ipip.h @@ -12,7 +12,6 @@ struct ip_tunnel struct ip_tunnel *next; struct net_device *dev; - int recursion; /* Depth of hard_start_xmit recursion */ int err_count; /* Number of arrived ICMP errors */ unsigned long err_time; /* Time when the last ICMP error arrived */ diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index d9645c9..41ada99 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -66,10 +66,7 @@ solution, but it supposes maintaing new variable in ALL skb, even if no tunneling is used. - Current solution: t->recursion lock breaks dead loops. It looks - like dev->tbusy flag, but I preferred new variable, because - the semantics is different. One day, when hard_start_xmit - will be multithreaded we will have to use skb->encapsulation. + Current solution: HARD_TX_LOCK lock breaks dead loops. @@ -678,11 +675,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev __be32 dst; int mtu; - if (tunnel->recursion++) { - stats->collisions++; - goto tx_error; - } - if (dev->type == ARPHRD_ETHER) IPCB(skb)->flags = 0; @@ -820,7 +812,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev ip_rt_put(rt); stats->tx_dropped++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } if (skb->sk) @@ -888,7 +879,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev nf_reset(skb); IPTUNNEL_XMIT(); - tunnel->recursion--; return NETDEV_TX_OK; tx_error_icmp: @@ -897,7 +887,6 @@ tx_error_icmp: tx_error: stats->tx_errors++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index 62548cb..08ccd34 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -402,11 +402,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) __be32 dst = tiph->daddr; int mtu; - if (tunnel->recursion++) { - stats->collisions++; - goto tx_error; - } - if (skb->protocol != htons(ETH_P_IP)) goto tx_error; @@ -485,7 +480,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) ip_rt_put(rt); stats->tx_dropped++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } if (skb->sk) @@ -523,7 +517,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) nf_reset(skb); IPTUNNEL_XMIT(); - tunnel->recursion--; return NETDEV_TX_OK; tx_error_icmp: @@ -531,7 +524,6 @@ tx_error_icmp: tx_error: stats->tx_errors++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tunnel: eliminate recursion field 2009-09-23 20:17 [PATCH] tunnel: eliminate recursion field Eric Dumazet @ 2009-09-23 20:28 ` Eric Dumazet 2009-09-24 22:47 ` David Miller 2009-09-25 21:51 ` Herbert Xu 1 sibling, 1 reply; 4+ messages in thread From: Eric Dumazet @ 2009-09-23 20:28 UTC (permalink / raw) To: David S. Miller; +Cc: Linux Netdev List Eric Dumazet a écrit : > It seems recursion field from "struct ip_tunnel" is not anymore needed. > recursion prevention is done at the upper level (in dev_queue_xmit()), > since we use HARD_TX_LOCK protection for tunnels. > > This avoids a cache line ping pong on "struct ip_tunnel" : This structure > should be now mostly read on xmit and receive paths. Oops I forgot ipv6 tunnels, silly me, here is an updated version. Thanks [PATCH] tunnel: eliminate recursion field It seems recursion field from "struct ip_tunnel" is not anymore needed. recursion prevention is done at the upper level (in dev_queue_xmit()), since we use HARD_TX_LOCK protection for tunnels. This avoids a cache line ping pong on "struct ip_tunnel" : This structure should be now mostly read on xmit and receive paths. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/ipip.h | 1 - net/ipv4/ip_gre.c | 13 +------------ net/ipv4/ipip.c | 8 -------- net/ipv6/ip6_tunnel.c | 7 ------- net/ipv6/sit.c | 8 -------- 5 files changed, 1 insertion(+), 36 deletions(-) diff --git a/include/net/ipip.h b/include/net/ipip.h index 5d3036f..76e3ea6 100644 --- a/include/net/ipip.h +++ b/include/net/ipip.h @@ -12,7 +12,6 @@ struct ip_tunnel struct ip_tunnel *next; struct net_device *dev; - int recursion; /* Depth of hard_start_xmit recursion */ int err_count; /* Number of arrived ICMP errors */ unsigned long err_time; /* Time when the last ICMP error arrived */ diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index d9645c9..41ada99 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -66,10 +66,7 @@ solution, but it supposes maintaing new variable in ALL skb, even if no tunneling is used. - Current solution: t->recursion lock breaks dead loops. It looks - like dev->tbusy flag, but I preferred new variable, because - the semantics is different. One day, when hard_start_xmit - will be multithreaded we will have to use skb->encapsulation. + Current solution: HARD_TX_LOCK lock breaks dead loops. @@ -678,11 +675,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev __be32 dst; int mtu; - if (tunnel->recursion++) { - stats->collisions++; - goto tx_error; - } - if (dev->type == ARPHRD_ETHER) IPCB(skb)->flags = 0; @@ -820,7 +812,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev ip_rt_put(rt); stats->tx_dropped++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } if (skb->sk) @@ -888,7 +879,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev nf_reset(skb); IPTUNNEL_XMIT(); - tunnel->recursion--; return NETDEV_TX_OK; tx_error_icmp: @@ -897,7 +887,6 @@ tx_error_icmp: tx_error: stats->tx_errors++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index 62548cb..08ccd34 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -402,11 +402,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) __be32 dst = tiph->daddr; int mtu; - if (tunnel->recursion++) { - stats->collisions++; - goto tx_error; - } - if (skb->protocol != htons(ETH_P_IP)) goto tx_error; @@ -485,7 +480,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) ip_rt_put(rt); stats->tx_dropped++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } if (skb->sk) @@ -523,7 +517,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) nf_reset(skb); IPTUNNEL_XMIT(); - tunnel->recursion--; return NETDEV_TX_OK; tx_error_icmp: @@ -531,7 +524,6 @@ tx_error_icmp: tx_error: stats->tx_errors++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index 7d25bbe..c595bbe 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1043,11 +1043,6 @@ ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev) struct net_device_stats *stats = &t->dev->stats; int ret; - if (t->recursion++) { - stats->collisions++; - goto tx_err; - } - switch (skb->protocol) { case htons(ETH_P_IP): ret = ip4ip6_tnl_xmit(skb, dev); @@ -1062,14 +1057,12 @@ ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev) if (ret < 0) goto tx_err; - t->recursion--; return NETDEV_TX_OK; tx_err: stats->tx_errors++; stats->tx_dropped++; kfree_skb(skb); - t->recursion--; return NETDEV_TX_OK; } diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 0ae4f64..fcb5396 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -626,11 +626,6 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb, struct in6_addr *addr6; int addr_type; - if (tunnel->recursion++) { - stats->collisions++; - goto tx_error; - } - if (skb->protocol != htons(ETH_P_IPV6)) goto tx_error; @@ -753,7 +748,6 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb, ip_rt_put(rt); stats->tx_dropped++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } if (skb->sk) @@ -794,7 +788,6 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb, nf_reset(skb); IPTUNNEL_XMIT(); - tunnel->recursion--; return NETDEV_TX_OK; tx_error_icmp: @@ -802,7 +795,6 @@ tx_error_icmp: tx_error: stats->tx_errors++; dev_kfree_skb(skb); - tunnel->recursion--; return NETDEV_TX_OK; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tunnel: eliminate recursion field 2009-09-23 20:28 ` Eric Dumazet @ 2009-09-24 22:47 ` David Miller 0 siblings, 0 replies; 4+ messages in thread From: David Miller @ 2009-09-24 22:47 UTC (permalink / raw) To: eric.dumazet; +Cc: netdev From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 23 Sep 2009 22:28:33 +0200 > Eric Dumazet a écrit : >> It seems recursion field from "struct ip_tunnel" is not anymore needed. >> recursion prevention is done at the upper level (in dev_queue_xmit()), >> since we use HARD_TX_LOCK protection for tunnels. >> >> This avoids a cache line ping pong on "struct ip_tunnel" : This structure >> should be now mostly read on xmit and receive paths. > > Oops I forgot ipv6 tunnels, silly me, here is an updated version. > > Thanks > > [PATCH] tunnel: eliminate recursion field > > It seems recursion field from "struct ip_tunnel" is not anymore needed. > recursion prevention is done at the upper level (in dev_queue_xmit()), > since we use HARD_TX_LOCK protection for tunnels. > > This avoids a cache line ping pong on "struct ip_tunnel" : This structure > should be now mostly read on xmit and receive paths. > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tunnel: eliminate recursion field 2009-09-23 20:17 [PATCH] tunnel: eliminate recursion field Eric Dumazet 2009-09-23 20:28 ` Eric Dumazet @ 2009-09-25 21:51 ` Herbert Xu 1 sibling, 0 replies; 4+ messages in thread From: Herbert Xu @ 2009-09-25 21:51 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev Eric Dumazet <eric.dumazet@gmail.com> wrote: > It seems recursion field from "struct ip_tunnel" is not anymore needed. > recursion prevention is done at the upper level (in dev_queue_xmit()), > since we use HARD_TX_LOCK protection for tunnels. What if we get async processing in between, e.g., nf_queue? In that case we could end up with a packet that went round and around until it exceeded the MTU. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-09-25 21:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-09-23 20:17 [PATCH] tunnel: eliminate recursion field Eric Dumazet 2009-09-23 20:28 ` Eric Dumazet 2009-09-24 22:47 ` David Miller 2009-09-25 21:51 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).