* [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
@ 2016-01-15 16:21 Eric Dumazet
2016-01-15 16:43 ` Tom Herbert
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2016-01-15 16:21 UTC (permalink / raw)
To: David Miller, Pablo Neira Ayuso, Patrick McHardy
Cc: netfilter-devel, netdev, Herbert Xu, Tom Herbert
From: Eric Dumazet <edumazet@google.com>
In case MSS option is added in TCP options, skb length increases by 4.
IPv6 needs to update skb->csum if skb has CHECKSUM_COMPLETE,
otherwise kernel complains loudly in netdev_rx_csum_fault() with a
stack dump.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/netfilter/xt_TCPMSS.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index b7c43def0dc6..e118397254af 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -228,7 +228,7 @@ tcpmss_tg6(struct sk_buff *skb, const struct xt_action_param *par)
{
struct ipv6hdr *ipv6h = ipv6_hdr(skb);
u8 nexthdr;
- __be16 frag_off;
+ __be16 frag_off, oldlen, newlen;
int tcphoff;
int ret;
@@ -244,7 +244,12 @@ tcpmss_tg6(struct sk_buff *skb, const struct xt_action_param *par)
return NF_DROP;
if (ret > 0) {
ipv6h = ipv6_hdr(skb);
- ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) + ret);
+ oldlen = ipv6h->payload_len;
+ newlen = htons(ntohs(oldlen) + ret);
+ if (skb->ip_summed == CHECKSUM_COMPLETE)
+ skb->csum = csum_add(csum_sub(skb->csum, oldlen),
+ newlen);
+ ipv6h->payload_len = newlen;
}
return XT_CONTINUE;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
2016-01-15 16:21 [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6() Eric Dumazet
@ 2016-01-15 16:43 ` Tom Herbert
2016-01-15 17:18 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Tom Herbert @ 2016-01-15 16:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Pablo Neira Ayuso, Patrick McHardy,
netfilter-devel, netdev, Herbert Xu
On Fri, Jan 15, 2016 at 8:21 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In case MSS option is added in TCP options, skb length increases by 4.
> IPv6 needs to update skb->csum if skb has CHECKSUM_COMPLETE,
> otherwise kernel complains loudly in netdev_rx_csum_fault() with a
> stack dump.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/netfilter/xt_TCPMSS.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
> index b7c43def0dc6..e118397254af 100644
> --- a/net/netfilter/xt_TCPMSS.c
> +++ b/net/netfilter/xt_TCPMSS.c
> @@ -228,7 +228,7 @@ tcpmss_tg6(struct sk_buff *skb, const struct xt_action_param *par)
> {
> struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> u8 nexthdr;
> - __be16 frag_off;
> + __be16 frag_off, oldlen, newlen;
> int tcphoff;
> int ret;
>
> @@ -244,7 +244,12 @@ tcpmss_tg6(struct sk_buff *skb, const struct xt_action_param *par)
> return NF_DROP;
> if (ret > 0) {
> ipv6h = ipv6_hdr(skb);
> - ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) + ret);
> + oldlen = ipv6h->payload_len;
> + newlen = htons(ntohs(oldlen) + ret);
> + if (skb->ip_summed == CHECKSUM_COMPLETE)
> + skb->csum = csum_add(csum_sub(skb->csum, oldlen),
> + newlen);
> + ipv6h->payload_len = newlen;
Probably should have a utility function do this. Maybe something like
update_hdr_field(skb, old, new)? Could return new value so above could
just be:
ipv6h->payload_len = update_hdr_field(skb, ipv6h->payload_len,
htons(ntohs(oldlen) + ret);
> }
> return XT_CONTINUE;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
2016-01-15 16:43 ` Tom Herbert
@ 2016-01-15 17:18 ` Eric Dumazet
2016-01-15 17:25 ` Tom Herbert
2016-01-15 19:04 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2016-01-15 17:18 UTC (permalink / raw)
To: Tom Herbert
Cc: David Miller, Pablo Neira Ayuso, Patrick McHardy,
netfilter-devel, netdev, Herbert Xu
On Fri, 2016-01-15 at 08:43 -0800, Tom Herbert wrote:
> Probably should have a utility function do this. Maybe something like
> update_hdr_field(skb, old, new)? Could return new value so above could
> just be:
>
> ipv6h->payload_len = update_hdr_field(skb, ipv6h->payload_len,
> htons(ntohs(oldlen) + ret);
Well, oldlen would still need to be ipv6h->payload_len
Not sure a helper will be nice for a stable submission.
Probably better for net-next ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
2016-01-15 17:18 ` Eric Dumazet
@ 2016-01-15 17:25 ` Tom Herbert
2016-01-15 19:04 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Tom Herbert @ 2016-01-15 17:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Pablo Neira Ayuso, Patrick McHardy,
netfilter-devel, netdev, Herbert Xu
On Fri, Jan 15, 2016 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-01-15 at 08:43 -0800, Tom Herbert wrote:
>
>> Probably should have a utility function do this. Maybe something like
>> update_hdr_field(skb, old, new)? Could return new value so above could
>> just be:
>>
>> ipv6h->payload_len = update_hdr_field(skb, ipv6h->payload_len,
>> htons(ntohs(oldlen) + ret);
>
> Well, oldlen would still need to be ipv6h->payload_len
>
That comes from the second argument.
> Not sure a helper will be nice for a stable submission.
>
> Probably better for net-next ?
>
I think this would apply to ipv6: update skb->csum when CE mark is
propagated also, maybe others? Seems safer to implement the logic only
in one place if possible... No big deal though.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
2016-01-15 17:18 ` Eric Dumazet
2016-01-15 17:25 ` Tom Herbert
@ 2016-01-15 19:04 ` David Miller
2016-01-15 20:04 ` Pablo Neira Ayuso
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2016-01-15 19:04 UTC (permalink / raw)
To: eric.dumazet; +Cc: tom, pablo, kaber, netfilter-devel, netdev, herbert
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 15 Jan 2016 09:18:44 -0800
> On Fri, 2016-01-15 at 08:43 -0800, Tom Herbert wrote:
>
>> Probably should have a utility function do this. Maybe something like
>> update_hdr_field(skb, old, new)? Could return new value so above could
>> just be:
>>
>> ipv6h->payload_len = update_hdr_field(skb, ipv6h->payload_len,
>> htons(ntohs(oldlen) + ret);
>
> Well, oldlen would still need to be ipv6h->payload_len
>
> Not sure a helper will be nice for a stable submission.
>
> Probably better for net-next ?
Agreed. We already have those csum_replace{2,4}() helpers in net/checksum.h
which we could augment with ones that can handle length changes too.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6()
2016-01-15 19:04 ` David Miller
@ 2016-01-15 20:04 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-01-15 20:04 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, tom, kaber, netfilter-devel, netdev, herbert
On Fri, Jan 15, 2016 at 02:04:49PM -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 15 Jan 2016 09:18:44 -0800
>
> > On Fri, 2016-01-15 at 08:43 -0800, Tom Herbert wrote:
> >
> >> Probably should have a utility function do this. Maybe something like
> >> update_hdr_field(skb, old, new)? Could return new value so above could
> >> just be:
> >>
> >> ipv6h->payload_len = update_hdr_field(skb, ipv6h->payload_len,
> >> htons(ntohs(oldlen) + ret);
> >
> > Well, oldlen would still need to be ipv6h->payload_len
> >
> > Not sure a helper will be nice for a stable submission.
> >
> > Probably better for net-next ?
>
> Agreed. We already have those csum_replace{2,4}() helpers in net/checksum.h
> which we could augment with ones that can handle length changes too.
I'm going to place this into the nf tree, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-15 20:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15 16:21 [PATCH net] netfilter: xt_TCPMSS: handle CHECKSUM_COMPLETE in tcpmss_tg6() Eric Dumazet
2016-01-15 16:43 ` Tom Herbert
2016-01-15 17:18 ` Eric Dumazet
2016-01-15 17:25 ` Tom Herbert
2016-01-15 19:04 ` David Miller
2016-01-15 20:04 ` Pablo Neira Ayuso
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.