Skip to contentSkip to conversation

nat: remove obsolete rcu_read_unlock call

PATCH Applied Ulrich Weber 4 participants 8 messages Sep 20, 2012 – Sep 21, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nat: remove obsolete rcu_read_unlock call

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

---
 net/netfilter/nf_nat_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
net/netfilter/nf_nat_core.c +1 -3
index 1816ad3..125d832 100644
@@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone,
 				       &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 			result->dst = tuple->dst;
 
-			if (in_range(l3proto, l4proto, result, range)) {
-				rcu_read_unlock();
+			if (in_range(l3proto, l4proto, result, range))
 				return 1;
-			}
 		}
 	}
 	return 0;
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 14 lines · Ulrich Weber

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

---
 net/netfilter/nf_nat_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
net/netfilter/nf_nat_core.c +0 -0
index 1816ad3..125d832 100644
@@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone,
 				       &ct->tuplehash[IP_CT_DIR_REPLY].tuple);

net/netfilter/nf_nat_core.c find_appropriate_src(struct net *net, u16 zone, · line 208

Quoted 10 lines
net/netfilter/nf_nat_core.c +1 -3
 			result->dst = tuple->dst;
 
-			if (in_range(l3proto, l4proto, result, range)) {
-				rcu_read_unlock();
+			if (in_range(l3proto, l4proto, result, range))
 				return 1;
-			}
 		}
 	}
 	return 0;

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next)

How old is this bug ?

#2 lore raw
Quoted 1 line · Eric Dumazet

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next) How old is this bug ?

Bug was introduced with c7232c9979cba684c50b64c513c4a83c9aa70563

Cheers
Ulrich

Signature · 8 lines
Ulrich Weber | [email protected] | Senior Software Engineer
Astaro - a Sophos company | Amalienbadstr 41 | 76227 Karlsruhe | Germany
Phone +49-721-25516-0 | Fax –200 | www.astaro.com

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw
Quoted 5 lines · Ulrich Weber
Quoted 2 lines · Eric Dumazet

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next) How old is this bug ?

Bug was introduced with c7232c9979cba684c50b64c513c4a83c9aa70563

I've added this to the patch description, it's good for the record.

Thanks.

#4 lore raw
In reply to Eric Dumazet
Quoted 31 lines · On Thu, 20 Sep 2012 16:14:45 +0200 Eric Dumazet <[email protected]> wrote:
Quoted 14 lines · Ulrich Weber

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

---
 net/netfilter/nf_nat_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
net/netfilter/nf_nat_core.c +0 -0
index 1816ad3..125d832 100644
@@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone,
 				       &ct->tuplehash[IP_CT_DIR_REPLY].tuple);

net/netfilter/nf_nat_core.c find_appropriate_src(struct net *net, u16 zone, · line 208

Quoted 10 lines
net/netfilter/nf_nat_core.c +1 -3
 			result->dst = tuple->dst;
 
-			if (in_range(l3proto, l4proto, result, range)) {
-				rcu_read_unlock();
+			if (in_range(l3proto, l4proto, result, range))
 				return 1;
-			}
 		}
 	}
 	return 0;

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next)

Quoted 4 lines

How old is this bug ?

Will validate the fix tomorrow when back home.

#5 lore raw
In reply to Eric Dumazet
Quoted 31 lines · On Thu, 20 Sep 2012 16:14:45 +0200 Eric Dumazet <[email protected]> wrote:
Quoted 14 lines · Ulrich Weber

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

---
 net/netfilter/nf_nat_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
net/netfilter/nf_nat_core.c +0 -0
index 1816ad3..125d832 100644
@@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone,
 				       &ct->tuplehash[IP_CT_DIR_REPLY].tuple);

net/netfilter/nf_nat_core.c find_appropriate_src(struct net *net, u16 zone, · line 208

Quoted 10 lines
net/netfilter/nf_nat_core.c +1 -3
 			result->dst = tuple->dst;
 
-			if (in_range(l3proto, l4proto, result, range)) {
-				rcu_read_unlock();
+			if (in_range(l3proto, l4proto, result, range))
 				return 1;
-			}
 		}
 	}
 	return 0;

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next)

Quoted 4 lines

How old is this bug ?

This fixes the lockdep splat I saw when staring KVM vm's with net-next and lockdep enabled

#6 lore raw
Quoted 34 lines · Stephen Hemminger
Quoted 31 lines · On Thu, 20 Sep 2012 16:14:45 +0200 Eric Dumazet <[email protected]> wrote:
Quoted 14 lines · Ulrich Weber

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

---
 net/netfilter/nf_nat_core.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
net/netfilter/nf_nat_core.c +0 -0
index 1816ad3..125d832 100644
@@ -201,10 +201,8 @@ find_appropriate_src(struct net *net, u16 zone,
 				       &ct->tuplehash[IP_CT_DIR_REPLY].tuple);

net/netfilter/nf_nat_core.c find_appropriate_src(struct net *net, u16 zone, · line 208

Quoted 10 lines
net/netfilter/nf_nat_core.c +1 -3
 			result->dst = tuple->dst;
 
-			if (in_range(l3proto, l4proto, result, range)) {
-				rcu_read_unlock();
+			if (in_range(l3proto, l4proto, result, range))
 				return 1;
-			}
 		}
 	}
 	return 0;

This could explain the rcu_read_unlock() imbalance Stephen had (lockdep splat with net-next)

Quoted 6 lines
Quoted 4 lines

How old is this bug ?

This fixes the lockdep splat I saw when staring KVM vm's with net-next and lockdep enabled

It's in my tree already.

I'll pass it to David asap. Thanks for the report.

maintainer Applied
#7 lore raw
In reply to Ulrich Weber
Quoted 2 lines · Ulrich Weber

hlist walk in find_appropriate_src() is not protected anymore by rcu_read_lock(), so rcu_read_unlock() is unnecessary if in_range() matches.

Applied, thanks Ulrich.