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

[Jerry Chu <hkchu@google.com>]

On Thu, Dec 12, 2013 at 3:37 AM, Eric Dumazet <eric.dumazet@gmail.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;
>
>
>