[PATCH] ip6tables: use skb->len for accounting

[PATCH] ip6tables: use skb->len for accounting

[Changli Gao]

ip6tables: use skb->len for accounting

ipv6_hdr(skb)->payload_len is ZERO and can't be used for accounting, if the
payload is a Jumbo Payload specified in RFC2675.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/ipv6/netfilter/ip6_tables.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index dc41d6d..33113c1 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -387,9 +387,7 @@ ip6t_do_table(struct sk_buff *skb,
 				goto no_match;
 		}
 
-		ADD_COUNTER(e->counters,
-			    ntohs(ipv6_hdr(skb)->payload_len) +
-			    sizeof(struct ipv6hdr), 1);
+		ADD_COUNTER(e->counters, skb->len, 1);
 
 		t = ip6t_get_target_c(e);
 		IP_NF_ASSERT(t->u.kernel.target);

Re: [PATCH] ip6tables: use skb->len for accounting

[Jan Engelhardt]

On Friday 2010-07-23 05:31, Changli Gao wrote:

>ip6tables: use skb->len for accounting
>
>ipv6_hdr(skb)->payload_len is ZERO and can't be used for accounting, if the
>payload is a Jumbo Payload specified in RFC2675.
>
>Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>----
> net/ipv6/netfilter/ip6_tables.c |    4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
>index dc41d6d..33113c1 100644
>--- a/net/ipv6/netfilter/ip6_tables.c
>+++ b/net/ipv6/netfilter/ip6_tables.c
>@@ -387,9 +387,7 @@ ip6t_do_table(struct sk_buff *skb,
> 				goto no_match;
> 		}
> 
>-		ADD_COUNTER(e->counters,
>-			    ntohs(ipv6_hdr(skb)->payload_len) +
>-			    sizeof(struct ipv6hdr), 1);
>+		ADD_COUNTER(e->counters, skb->len, 1);
> 
> 		t = ip6t_get_target_c(e);
> 		IP_NF_ASSERT(t->u.kernel.target);
>--

I wonder how this fares with trailing padding or data, like, when
you have a standard v4/v6 packet created in a raw socket, and append
a bunch of \0s to it.

Re: [PATCH] ip6tables: use skb->len for accounting

[Changli Gao]

On Fri, Jul 23, 2010 at 2:16 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>
>
> I wonder how this fares with trailing padding or data, like, when
> you have a standard v4/v6 packet created in a raw socket, and append
> a bunch of \0s to it.
>
>

For the packets received, ip_rcv, ipv6_rcv and bridge all call
pskb_trim_rcsum before feeding them to netfilter. The raw packets are
sent via dev_queue_xmit(), and they don't pass through the output path
of netfilter. One case, maybe the queued packets mangled "wrongly" in
userspace are reinjected, however, we can't prevent a user from
changing the payload_len wrongly.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: [PATCH] ip6tables: use skb->len for accounting

[Patrick McHardy]

On 23.07.2010 08:38, Changli Gao wrote:
> On Fri, Jul 23, 2010 at 2:16 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>>
>>
>> I wonder how this fares with trailing padding or data, like, when
>> you have a standard v4/v6 packet created in a raw socket, and append
>> a bunch of \0s to it.
>>
>>
> 
> For the packets received, ip_rcv, ipv6_rcv and bridge all call
> pskb_trim_rcsum before feeding them to netfilter. The raw packets are
> sent via dev_queue_xmit(), and they don't pass through the output path
> of netfilter.

That's not true, raw packets also pass through netfilter. However
I agree that this patch makes sense to properly deal with jumbo
frames, but you should also update xt_length for consistency.

Re: [PATCH] ip6tables: use skb->len for accounting

[Changli Gao]

On Fri, Jul 23, 2010 at 7:48 PM, Patrick McHardy <kaber@trash.net> wrote:
> On 23.07.2010 08:38, Changli Gao wrote:
>>
>> For the packets received, ip_rcv, ipv6_rcv and bridge all call
>> pskb_trim_rcsum before feeding them to netfilter. The raw packets are
>> sent via dev_queue_xmit(), and they don't pass through the output path
>> of netfilter.
>
> That's not true, raw packets also pass through netfilter. However
> I agree that this patch makes sense to properly deal with jumbo
> frames, but you should also update xt_length for consistency.
>

In order to support jumbo frames, we have to change the type of min
and max in xt_length_info to u32. I'll change xt_match.revision to 1,
do I need to support the current revision 0 at the same time?

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: [PATCH] ip6tables: use skb->len for accounting

[Patrick McHardy]

On 23.07.2010 15:05, Changli Gao wrote:
> On Fri, Jul 23, 2010 at 7:48 PM, Patrick McHardy <kaber@trash.net> wrote:
>> On 23.07.2010 08:38, Changli Gao wrote:
>>>
>>> For the packets received, ip_rcv, ipv6_rcv and bridge all call
>>> pskb_trim_rcsum before feeding them to netfilter. The raw packets are
>>> sent via dev_queue_xmit(), and they don't pass through the output path
>>> of netfilter.
>>
>> That's not true, raw packets also pass through netfilter. However
>> I agree that this patch makes sense to properly deal with jumbo
>> frames, but you should also update xt_length for consistency.
>>
> 
> In order to support jumbo frames, we have to change the type of min
> and max in xt_length_info to u32.

I see. In that case I'll apply your skb->len patches as they are,
so please make the xt_length change in a new patch.

> I'll change xt_match.revision to 1,
> do I need to support the current revision 0 at the same time?

You should simply keep the revision 0 code as it is. Revision 1,
using skb->len, can be used for both IPv4 and IPv6.

Re: [PATCH] ip6tables: use skb->len for accounting

[Patrick McHardy]

On 23.07.2010 05:31, Changli Gao wrote:
> ip6tables: use skb->len for accounting
> 
> ipv6_hdr(skb)->payload_len is ZERO and can't be used for accounting, if the
> payload is a Jumbo Payload specified in RFC2675.

Applied, thanks.

Re: [PATCH] ip6tables: use skb->len for accounting

[Jan Engelhardt]

On Friday 2010-07-23 15:05, Changli Gao wrote:

>On Fri, Jul 23, 2010 at 7:48 PM, Patrick McHardy <kaber@trash.net> wrote:
>> On 23.07.2010 08:38, Changli Gao wrote:
>>>
>>> For the packets received, ip_rcv, ipv6_rcv and bridge all call
>>> pskb_trim_rcsum before feeding them to netfilter. The raw packets are
>>> sent via dev_queue_xmit(), and they don't pass through the output path
>>> of netfilter.
>>
>> That's not true, raw packets also pass through netfilter. However
>> I agree that this patch makes sense to properly deal with jumbo
>> frames, but you should also update xt_length for consistency.
>>
>
>In order to support jumbo frames, we have to change the type of min
>and max in xt_length_info to u32. I'll change xt_match.revision to 1,
>do I need to support the current revision 0 at the same time?

This is all already handled by xt_length.2. But alas, merge has been 
declined so far.