[PATCH resend] netfilter: place in source hash after SNAT is done

[PATCH resend] netfilter: place in source hash after SNAT is done

[Changli Gao]

If SNAT isn't done, the wrong info maybe got by the other cts.

As the filter table is after DNAT table, the packets dropped in filter
table also bother bysource hash table.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/ipv4/netfilter/nf_nat_core.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index c04787c..51ce55a 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -221,7 +221,14 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
 	   manips not an issue.  */
 	if (maniptype == IP_NAT_MANIP_SRC &&
 	    !(range->flags & IP_NAT_RANGE_PROTO_RANDOM)) {
-		if (find_appropriate_src(net, zone, orig_tuple, tuple, range)) {
+		/* try the original tuple first */
+		if (in_range(orig_tuple, range)) {
+			if (!nf_nat_used_tuple(orig_tuple, ct)) {
+				*tuple = *orig_tuple;
+				return;
+			}
+		} else if (find_appropriate_src(net, zone, orig_tuple, tuple,
+			   range)) {
 			pr_debug("get_unique_tuple: Found current src map\n");
 			if (!nf_nat_used_tuple(tuple, ct))
 				return;
@@ -266,7 +273,6 @@ nf_nat_setup_info(struct nf_conn *ct,
 	struct net *net = nf_ct_net(ct);
 	struct nf_conntrack_tuple curr_tuple, new_tuple;
 	struct nf_conn_nat *nat;
-	int have_to_hash = !(ct->status & IPS_NAT_DONE_MASK);
 
 	/* nat helper or nfctnetlink also setup binding */
 	nat = nfct_nat(ct);
@@ -306,8 +312,7 @@ nf_nat_setup_info(struct nf_conn *ct,
 			ct->status |= IPS_DST_NAT;
 	}
 
-	/* Place in source hash if this is the first time. */
-	if (have_to_hash) {
+	if (maniptype == IP_NAT_MANIP_SRC) {
 		unsigned int srchash;
 
 		srchash = hash_by_src(net, nf_ct_zone(ct),
@@ -532,7 +537,7 @@ static void nf_nat_cleanup_conntrack(struct nf_conn *ct)
 	if (nat == NULL || nat->ct == NULL)
 		return;
 
-	NF_CT_ASSERT(nat->ct->status & IPS_NAT_DONE_MASK);
+	NF_CT_ASSERT(nat->ct->status & IPS_SRC_NAT_DONE);
 
 	spin_lock_bh(&nf_nat_lock);
 	hlist_del_rcu(&nat->bysource);
@@ -545,11 +550,10 @@ static void nf_nat_move_storage(void *new, void *old)
 	struct nf_conn_nat *old_nat = old;
 	struct nf_conn *ct = old_nat->ct;
 
-	if (!ct || !(ct->status & IPS_NAT_DONE_MASK))
+	if (!ct || !(ct->status & IPS_SRC_NAT_DONE))
 		return;
 
 	spin_lock_bh(&nf_nat_lock);
-	new_nat->ct = ct;
 	hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
 	spin_unlock_bh(&nf_nat_lock);
 }

[PATCH resend] netfilter: make rcu read section smaller

[Changli Gao]

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/ipv4/netfilter/nf_nat_core.c |   38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index c04787c..7300611 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -85,7 +85,7 @@ in_range(const struct nf_conntrack_tuple *tuple,
 	 const struct nf_nat_range *range)
 {
 	const struct nf_nat_protocol *proto;
-	int ret = 0;
+	int ret = 1;
 
 	/* If we are supposed to map IPs, then we must be in the
 	   range specified, otherwise let this drag us onto a new src IP. */
@@ -95,13 +95,14 @@ in_range(const struct nf_conntrack_tuple *tuple,
 			return 0;
 	}
 
-	rcu_read_lock();
-	proto = __nf_nat_proto_find(tuple->dst.protonum);
-	if (!(range->flags & IP_NAT_RANGE_PROTO_SPECIFIED) ||
-	    proto->in_range(tuple, IP_NAT_MANIP_SRC,
-			    &range->min, &range->max))
-		ret = 1;
-	rcu_read_unlock();
+	if (range->flags & IP_NAT_RANGE_PROTO_SPECIFIED) {
+		rcu_read_lock();
+		proto = __nf_nat_proto_find(tuple->dst.protonum);
+		if (!proto->in_range(tuple, IP_NAT_MANIP_SRC, &range->min,
+				     &range->max))
+			ret = 0;
+		rcu_read_unlock();
+	}
 
 	return ret;
 }
@@ -235,22 +236,21 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
 
 	/* 3) The per-protocol part of the manip is made to map into
 	   the range to make a unique tuple. */
+	if (!(range->flags & (IP_NAT_RANGE_PROTO_RANDOM |
+			      IP_NAT_RANGE_PROTO_SPECIFIED)) &&
+	    !nf_nat_used_tuple(tuple, ct))
+		return;
 
 	rcu_read_lock();
 	proto = __nf_nat_proto_find(orig_tuple->dst.protonum);
 
 	/* Only bother mapping if it's not already in range and unique */
-	if (!(range->flags & IP_NAT_RANGE_PROTO_RANDOM)) {
-		if (range->flags & IP_NAT_RANGE_PROTO_SPECIFIED) {
-			if (proto->in_range(tuple, maniptype, &range->min,
-					    &range->max) &&
-			    (range->min.all == range->max.all ||
-			     !nf_nat_used_tuple(tuple, ct)))
-				goto out;
-		} else if (!nf_nat_used_tuple(tuple, ct)) {
-			goto out;
-		}
-	}
+	if ((range->flags & (IP_NAT_RANGE_PROTO_RANDOM |
+			     IP_NAT_RANGE_PROTO_SPECIFIED)) ==
+	    IP_NAT_RANGE_PROTO_SPECIFIED &&
+	    proto->in_range(tuple, maniptype, &range->min, &range->max) &&
+	    (range->min.all == range->max.all || !nf_nat_used_tuple(tuple, ct)))
+		goto out;
 
 	/* Last change: get protocol to try to obtain unique tuple. */
 	proto->unique_tuple(tuple, range, maniptype, ct);

Re: [PATCH resend] netfilter: place in source hash after SNAT is done

[Patrick McHardy]

On 15.01.2011 12:28, Changli Gao wrote:
> If SNAT isn't done, the wrong info maybe got by the other cts.
> 
> As the filter table is after DNAT table, the packets dropped in filter
> table also bother bysource hash table.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
>  net/ipv4/netfilter/nf_nat_core.c |   18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
> index c04787c..51ce55a 100644
> --- a/net/ipv4/netfilter/nf_nat_core.c
> +++ b/net/ipv4/netfilter/nf_nat_core.c
> @@ -221,7 +221,14 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
>  	   manips not an issue.  */
>  	if (maniptype == IP_NAT_MANIP_SRC &&
>  	    !(range->flags & IP_NAT_RANGE_PROTO_RANDOM)) {
> -		if (find_appropriate_src(net, zone, orig_tuple, tuple, range)) {
> +		/* try the original tuple first */

This doesn't seem to be related to the hashing change. Please describe
the intention behind this change.

> +		if (in_range(orig_tuple, range)) {
> +			if (!nf_nat_used_tuple(orig_tuple, ct)) {
> +				*tuple = *orig_tuple;
> +				return;
> +			}
> +		} else if (find_appropriate_src(net, zone, orig_tuple, tuple,
> +			   range)) {
>  			pr_debug("get_unique_tuple: Found current src map\n");
>  			if (!nf_nat_used_tuple(tuple, ct))
>  				return;

Re: [PATCH resend] netfilter: place in source hash after SNAT is done

[Changli Gao]

On Tue, Jan 18, 2011 at 10:17 PM, Patrick McHardy <kaber@trash.net> wrote:
>>  net/ipv4/netfilter/nf_nat_core.c |   18 +++++++++++-------
>>  1 file changed, 11 insertions(+), 7 deletions(-)
>> diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
>> index c04787c..51ce55a 100644
>> --- a/net/ipv4/netfilter/nf_nat_core.c
>> +++ b/net/ipv4/netfilter/nf_nat_core.c
>> @@ -221,7 +221,14 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
>>          manips not an issue.  */
>>       if (maniptype == IP_NAT_MANIP_SRC &&
>>           !(range->flags & IP_NAT_RANGE_PROTO_RANDOM)) {
>> -             if (find_appropriate_src(net, zone, orig_tuple, tuple, range)) {
>> +             /* try the original tuple first */
>
> This doesn't seem to be related to the hashing change. Please describe
> the intention behind this change.

Currently, we add the ct at the head of the corresponding bucket of
the source hash table after DNAT is done, so when we do SNAT, the
original ct will be tried first. This change is used to keep this
behavior.


>
>> +             if (in_range(orig_tuple, range)) {
>> +                     if (!nf_nat_used_tuple(orig_tuple, ct)) {
>> +                             *tuple = *orig_tuple;
>> +                             return;
>> +                     }
>> +             } else if (find_appropriate_src(net, zone, orig_tuple, tuple,
>> +                        range)) {
>>                       pr_debug("get_unique_tuple: Found current src map\n");
>>                       if (!nf_nat_used_tuple(tuple, ct))
>>                               return;
>
>
>



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] netfilter: place in source hash after SNAT is done

[Patrick McHardy]

Am 19.01.2011 01:03, schrieb Changli Gao:
> On Tue, Jan 18, 2011 at 10:17 PM, Patrick McHardy <kaber@trash.net> wrote:
>>>  net/ipv4/netfilter/nf_nat_core.c |   18 +++++++++++-------
>>>  1 file changed, 11 insertions(+), 7 deletions(-)
>>> diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
>>> index c04787c..51ce55a 100644
>>> --- a/net/ipv4/netfilter/nf_nat_core.c
>>> +++ b/net/ipv4/netfilter/nf_nat_core.c
>>> @@ -221,7 +221,14 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
>>>          manips not an issue.  */
>>>       if (maniptype == IP_NAT_MANIP_SRC &&
>>>           !(range->flags & IP_NAT_RANGE_PROTO_RANDOM)) {
>>> -             if (find_appropriate_src(net, zone, orig_tuple, tuple, range)) {
>>> +             /* try the original tuple first */
>>
>> This doesn't seem to be related to the hashing change. Please describe
>> the intention behind this change.
> 
> Currently, we add the ct at the head of the corresponding bucket of
> the source hash table after DNAT is done, so when we do SNAT, the
> original ct will be tried first. This change is used to keep this
> behavior.

Thanks for the explanation, applied.