netdev - Re: [PATCH v2 net-next] net-gro: Prepare GRO stack for the upcoming tunneling support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 12 Dec 2013 11:45:29 +0800
From:	Jerry Chu <hkchu@...gle.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Eric Dumazet <edumazet@...gle.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Ben Hutchings <bhutchings@...arflare.com>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next] net-gro: Prepare GRO stack for the upcoming
 tunneling support

On Thu, Dec 12, 2013 at 3:37 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Wed, 2013-12-11 at 11:04 -0800, H.K. Jerry Chu wrote:
>
>> -int tcp_gro_complete(struct sk_buff *skb)
>> +int __tcp_gro_complete(struct sk_buff *skb, struct tcphdr *th)
>>  {
>> -     struct tcphdr *th = tcp_hdr(skb);
>> -
>> -     skb->csum_start = skb_transport_header(skb) - skb->head;
>> +     skb->csum_start = (unsigned char *)th - skb->head;
>>       skb->csum_offset = offsetof(struct tcphdr, check);
>>       skb->ip_summed = CHECKSUM_PARTIAL;
>>
>> @@ -251,6 +250,12 @@ int tcp_gro_complete(struct sk_buff *skb)
>>
>>       return 0;
>>  }
>> +EXPORT_SYMBOL(__tcp_gro_complete);
>> +
>> +int tcp_gro_complete(struct sk_buff *skb)
>> +{
>> +     return __tcp_gro_complete(skb,  tcp_hdr(skb));
>> +}
>
>
> I have no idea why you kept this change.
>
> As we said, the transport offset is set, so it looks like a lot of your
> changes can be removed to ease the review

Yes I know but you were the one who suggested not setting the
transport header a while back, and seemed to prefer avoiding the use
of saved headers in skb.

Then you discovered the transport header must be set by the GRO
stack. That leaves the question should I still try to avoid using headers
saved in skb in case you may attempt to get rid of all the header setting
in GRO completely in the future. I decided last night to use the new
way (nhoff). But obviously the other way (to simply use
skb_transport_header to the extent possible) can be easily done and
will avoid much of the code change too. I'll send out v3 soon.

>
> tcp_gro_receive() sets the transport header, so the xxx_complete()
> handlers can rely on it.
>
> The only part that you could keep is the following optim :
>
> - skb->csum_start = skb_transport_header(skb) - skb->head;
> + skb->csum_start = (unsigned char *)th - skb->head;
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html