ip_frag:
Subsystems (3)
PATCH 1/1 ip_frag:
When sending fragmentation expiration ICMP V4/V6 messages, we can avoid touching device refcount, thanks to RCU
--- net/ipv4/ip_fragment.c | 6 +++--- net/ipv6/reassembly.c | 13 ++++++------- 2 files changed, 9 insertions(+), 10 deletions(-)
net/ipv4/ip_fragment.c +3 -3
@@ -206,10 +206,10 @@ static void ip_expire(unsigned long arg)struct sk_buff *head = qp->q.fragments;/* Send an ICMP "Fragment Reassembly Timeout" message. */ -if ((head->dev =dev_get_by_index (net, qp->iif)) !=NULL ) { +rcu_read_lock (); +if ((head->dev =dev_get_by_index_rcu (net, qp->iif)) !=NULL )icmp_send (head,ICMP_TIME_EXCEEDED ,ICMP_EXC_FRAGTIME ,0 ); -dev_put (head->dev); - } +rcu_read_unlock (); } out:spin_unlock (&qp->q.lock);
net/ipv6/reassembly.c +6 -7
@@ -208,18 +208,17 @@ static void ip6_frag_expire(unsigned long data)fq_kill (fq); net = container_of(fq->q.net, struct net, ipv6.frags); - dev =dev_get_by_index (net, fq->iif); +rcu_read_lock (); + dev =dev_get_by_index_rcu (net, fq->iif);if (!dev) - goto out; + goto out_rcu_unlock; -rcu_read_lock ();IP6_INC_STATS_BH (net,__in6_dev_get (dev),IPSTATS_MIB_REASMTIMEOUT );IP6_INC_STATS_BH (net,__in6_dev_get (dev),IPSTATS_MIB_REASMFAILS ); -rcu_read_unlock ();/* Don't send error if the first segment did not arrive. */ if (!(fq->q.last_in &INET_FRAG_FIRST_IN ) || !fq->q.fragments) - goto out; + goto out_rcu_unlock;/* But use as source device on which LAST ARRIVED
@@ -228,9 +227,9 @@ static void ip6_frag_expire(unsigned long data)*/ fq->q.fragments->dev = dev;icmpv6_send (fq->q.fragments,ICMPV6_TIME_EXCEED ,ICMPV6_EXC_FRAGTIME ,0 , dev); +out_rcu_unlock: +rcu_read_unlock (); out: -if (dev) -dev_put (dev);spin_unlock (&fq->q.lock);fq_put (fq); }
Quoted 7 lines · Eric Dumazet
Signed-off-by Eric Dumazet <[email protected]>net/ipv4/ip_fragment.c +0
-0@@ -206,10 +206,10 @@ static void ip_expire(unsigned long arg)struct sk_buff *head = qp->q.fragments;
net/ipv4/ip_fragment.c static void ip_expire(unsigned long arg) · line 213
Quoted 10 lines
net/ipv4/ip_fragment.c +3
-3/* Send an ICMP "Fragment Reassembly Timeout" message. */ -if ((head->dev =dev_get_by_index (net, qp->iif)) !=NULL ) { +rcu_read_lock (); +if ((head->dev =dev_get_by_index_rcu (net, qp->iif)) !=NULL )icmp_send (head,ICMP_TIME_EXCEEDED ,ICMP_EXC_FRAGTIME ,0 ); -dev_put (head->dev); - } +rcu_read_unlock (); }
Hi Eric. If you're going to do more of these, could you please convert them to the more standard kernel style as well?
var = func(foo);
if (var) {
etc...Quoted 8 lines · Joe Perches
Hi Eric. If you're going to do more of these, could you please convert them to the more standard kernel style as well?
var = func(foo); if (var) { etc...
Well, rule of thumb is : dont mix cleanups with functional changes :)
But yes, I suspect David wont shout too much if I do one trivial cleanup on the fly :)
Anyway my patch title was incomplete...
Thanks
[PATCH net-next-2.6] ip_frag: dont touch device refcount
When sending fragmentation expiration ICMP V4/V6 messages, we can avoid touching device refcount, thanks to RCU
--- net/ipv4/ip_fragment.c | 7 ++++--- net/ipv6/reassembly.c | 13 ++++++------- 2 files changed, 10 insertions(+), 10 deletions(-)
net/ipv4/ip_fragment.c +4 -3
@@ -206,10 +206,11 @@ static void ip_expire(unsigned long arg)struct sk_buff *head = qp->q.fragments;/* Send an ICMP "Fragment Reassembly Timeout" message. */ -if ((head->dev =dev_get_by_index (net, qp->iif)) !=NULL ) { +rcu_read_lock (); + head->dev =dev_get_by_index_rcu (net, qp->iif); +if (head->dev)icmp_send (head,ICMP_TIME_EXCEEDED ,ICMP_EXC_FRAGTIME ,0 ); -dev_put (head->dev); - } +rcu_read_unlock (); } out:spin_unlock (&qp->q.lock);
net/ipv6/reassembly.c +6 -7
@@ -208,18 +208,17 @@ static void ip6_frag_expire(unsigned long data)fq_kill (fq); net = container_of(fq->q.net, struct net, ipv6.frags); - dev =dev_get_by_index (net, fq->iif); +rcu_read_lock (); + dev =dev_get_by_index_rcu (net, fq->iif);if (!dev) - goto out; + goto out_rcu_unlock; -rcu_read_lock ();IP6_INC_STATS_BH (net,__in6_dev_get (dev),IPSTATS_MIB_REASMTIMEOUT );IP6_INC_STATS_BH (net,__in6_dev_get (dev),IPSTATS_MIB_REASMFAILS ); -rcu_read_unlock ();/* Don't send error if the first segment did not arrive. */ if (!(fq->q.last_in &INET_FRAG_FIRST_IN ) || !fq->q.fragments) - goto out; + goto out_rcu_unlock;/* But use as source device on which LAST ARRIVED
@@ -228,9 +227,9 @@ static void ip6_frag_expire(unsigned long data)*/ fq->q.fragments->dev = dev;icmpv6_send (fq->q.fragments,ICMPV6_TIME_EXCEED ,ICMPV6_EXC_FRAGTIME ,0 , dev); +out_rcu_unlock: +rcu_read_unlock (); out: -if (dev) -dev_put (dev);spin_unlock (&fq->q.lock);fq_put (fq); }
From: Eric Dumazet <[email protected]>
Date: Wed, 04 Nov 2009 23:11:49 +0100
Quoted 6 lines
[PATCH net-next-2.6]
ip_frag: dont touch device refcountWhen sending fragmentation expiration ICMP V4/V6 messages, we can avoid touching device refcount, thanks to RCU
Signed-off-by Eric Dumazet <[email protected]>
Applied to net-next-2.6
Although I was worried a little bit initially about the ipv4 case, as we're assigning the device pointer to "skb->dev" and I thought it might be troublesome for that reference to escape.
But that seems ok, because icmp_send() doesn't do anything interesting with it's 'skb_in' arg other than look at the attached route, the packet contents, and things like that.
Thanks.