* [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).