* [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
@ 2010-05-30 0:26 Changli Gao
2010-05-30 12:43 ` jamal
0 siblings, 1 reply; 7+ messages in thread
From: Changli Gao @ 2010-05-30 0:26 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, linux-kernel, Changli Gao
fix the wrong checksum when addr isn't in old_addr/mask
For TCP and UDP packets, when addr isn't in old_addr/mask we don't do SNAT or
DNAT, and we should not update layer 4 checksum.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/sched/act_nat.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index d885ba3..5709494 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -159,6 +159,9 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
iph->daddr = new_addr;
csum_replace4(&iph->check, addr, new_addr);
+ } else if ((iph->frag_off & htons(IP_OFFSET)) ||
+ iph->protocol != IPPROTO_ICMP) {
+ goto out;
}
ihl = iph->ihl * 4;
@@ -247,6 +250,7 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
break;
}
+out:
return action;
drop:
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 0:26 [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask Changli Gao
@ 2010-05-30 12:43 ` jamal
2010-05-30 12:58 ` Herbert Xu
0 siblings, 1 reply; 7+ messages in thread
From: jamal @ 2010-05-30 12:43 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev, Herbert Xu
Copying Herbert, taking linux-kernel off...
On Sun, 2010-05-30 at 08:26 +0800, Changli Gao wrote:
> fix the wrong checksum when addr isn't in old_addr/mask
>
> For TCP and UDP packets, when addr isn't in old_addr/mask we don't do SNAT or
> DNAT, and we should not update layer 4 checksum.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/sched/act_nat.c | 4 ++++
> 1 file changed, 4 insertions(+)
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index d885ba3..5709494 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -159,6 +159,9 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
> iph->daddr = new_addr;
>
> csum_replace4(&iph->check, addr, new_addr);
> + } else if ((iph->frag_off & htons(IP_OFFSET)) ||
> + iph->protocol != IPPROTO_ICMP) {
> + goto out;
> }
>
> ihl = iph->ihl * 4;
> @@ -247,6 +250,7 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
> break;
> }
>
> +out:
> return action;
>
> drop:
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 12:43 ` jamal
@ 2010-05-30 12:58 ` Herbert Xu
2010-05-30 13:33 ` Changli Gao
2010-06-02 13:52 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Herbert Xu @ 2010-05-30 12:58 UTC (permalink / raw)
To: jamal; +Cc: Changli Gao, David S. Miller, netdev
On Sun, May 30, 2010 at 08:43:41AM -0400, jamal wrote:
>
> Copying Herbert, taking linux-kernel off...
Thanks Jamal.
> On Sun, 2010-05-30 at 08:26 +0800, Changli Gao wrote:
> > fix the wrong checksum when addr isn't in old_addr/mask
> >
> > For TCP and UDP packets, when addr isn't in old_addr/mask we don't do SNAT or
> > DNAT, and we should not update layer 4 checksum.
> >
> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > ----
> > net/sched/act_nat.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> > diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> > index d885ba3..5709494 100644
> > --- a/net/sched/act_nat.c
> > +++ b/net/sched/act_nat.c
> > @@ -159,6 +159,9 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
> > iph->daddr = new_addr;
> >
> > csum_replace4(&iph->check, addr, new_addr);
> > + } else if ((iph->frag_off & htons(IP_OFFSET)) ||
> > + iph->protocol != IPPROTO_ICMP) {
> > + goto out;
> > }
Yes the patch is correct.
However, the fact that you need this patch means that your act_nat
setup isn't perfect. Ideally all the unNATed packets should be
filtered out before you hit act_nat.
Thanks,
--
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] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 12:58 ` Herbert Xu
@ 2010-05-30 13:33 ` Changli Gao
2010-05-30 14:11 ` Changli Gao
2010-05-30 21:53 ` Herbert Xu
2010-06-02 13:52 ` David Miller
1 sibling, 2 replies; 7+ messages in thread
From: Changli Gao @ 2010-05-30 13:33 UTC (permalink / raw)
To: Herbert Xu; +Cc: jamal, David S. Miller, netdev
On Sun, May 30, 2010 at 8:58 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Sun, May 30, 2010 at 08:43:41AM -0400, jamal wrote:
>>
>> Copying Herbert, taking linux-kernel off...
>
> Thanks Jamal.
>
>> On Sun, 2010-05-30 at 08:26 +0800, Changli Gao wrote:
>> > fix the wrong checksum when addr isn't in old_addr/mask
>> >
>> > For TCP and UDP packets, when addr isn't in old_addr/mask we don't do SNAT or
>> > DNAT, and we should not update layer 4 checksum.
>> >
>> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> > ----
>> > net/sched/act_nat.c | 4 ++++
>> > 1 file changed, 4 insertions(+)
>> > diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
>> > index d885ba3..5709494 100644
>> > --- a/net/sched/act_nat.c
>> > +++ b/net/sched/act_nat.c
>> > @@ -159,6 +159,9 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
>> > iph->daddr = new_addr;
>> >
>> > csum_replace4(&iph->check, addr, new_addr);
>> > + } else if ((iph->frag_off & htons(IP_OFFSET)) ||
>> > + iph->protocol != IPPROTO_ICMP) {
>> > + goto out;
>> > }
>
> Yes the patch is correct.
>
> However, the fact that you need this patch means that your act_nat
> setup isn't perfect. Ideally all the unNATed packets should be
> filtered out before you hit act_nat.
Thinking about this topologic:
client -> DNAT -> router -> server.
DNAT is used to map a public IP to server's private IP. If a
DEST_UNREACH ICMP packet is sent out by router, in order to handle
this ICMP packet correctly, I have to pass it to act_nat.c. How can I
filter out the other packets? By inspecting the inner IP destination
address of this ICMP packet? Maybe I can use u32 with complicate
parameters.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 13:33 ` Changli Gao
@ 2010-05-30 14:11 ` Changli Gao
2010-05-30 21:53 ` Herbert Xu
1 sibling, 0 replies; 7+ messages in thread
From: Changli Gao @ 2010-05-30 14:11 UTC (permalink / raw)
To: Herbert Xu; +Cc: jamal, David S. Miller, netdev
On Sun, May 30, 2010 at 9:33 PM, Changli Gao <xiaosuo@gmail.com> wrote:
> On Sun, May 30, 2010 at 8:58 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>
>> Yes the patch is correct.
>>
>> However, the fact that you need this patch means that your act_nat
>> setup isn't perfect. Ideally all the unNATed packets should be
>> filtered out before you hit act_nat.
>
> Thinking about this topologic:
>
> client -> DNAT -> router -> server.
>
> DNAT is used to map a public IP to server's private IP. If a
> DEST_UNREACH ICMP packet is sent out by router, in order to handle
> this ICMP packet correctly, I have to pass it to act_nat.c. How can I
> filter out the other packets? By inspecting the inner IP destination
> address of this ICMP packet? Maybe I can use u32 with complicate
> parameters.
>
Oh, I can pass all the ICMP packets, and the packets to the public IP
and the packets from the private IP.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 13:33 ` Changli Gao
2010-05-30 14:11 ` Changli Gao
@ 2010-05-30 21:53 ` Herbert Xu
1 sibling, 0 replies; 7+ messages in thread
From: Herbert Xu @ 2010-05-30 21:53 UTC (permalink / raw)
To: Changli Gao; +Cc: jamal, David S. Miller, netdev
On Sun, May 30, 2010 at 09:33:22PM +0800, Changli Gao wrote:
>
> Thinking about this topologic:
>
> client -> DNAT -> router -> server.
>
> DNAT is used to map a public IP to server's private IP. If a
> DEST_UNREACH ICMP packet is sent out by router, in order to handle
> this ICMP packet correctly, I have to pass it to act_nat.c. How can I
> filter out the other packets? By inspecting the inner IP destination
> address of this ICMP packet? Maybe I can use u32 with complicate
> parameters.
You should filter out all the non-ICMP packets, just like you
do here.
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] 7+ messages in thread
* Re: [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask
2010-05-30 12:58 ` Herbert Xu
2010-05-30 13:33 ` Changli Gao
@ 2010-06-02 13:52 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2010-06-02 13:52 UTC (permalink / raw)
To: herbert; +Cc: hadi, xiaosuo, netdev
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 30 May 2010 22:58:11 +1000
> Yes the patch is correct.
>
> However, the fact that you need this patch means that your act_nat
> setup isn't perfect. Ideally all the unNATed packets should be
> filtered out before you hit act_nat.
Regardless of whether said configuration is optimal, we should
handle this case correctly.
So I have applied this patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-06-02 13:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-30 0:26 [PATCH v2] act_nat: fix the wrong checksum when addr isn't in old_addr/mask Changli Gao
2010-05-30 12:43 ` jamal
2010-05-30 12:58 ` Herbert Xu
2010-05-30 13:33 ` Changli Gao
2010-05-30 14:11 ` Changli Gao
2010-05-30 21:53 ` Herbert Xu
2010-06-02 13:52 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.