On Wed, May 4, 2016 at 7:42 PM, Alexander Duyck
<alexander.du...@gmail.com> wrote:
> On Wed, May 4, 2016 at 6:02 PM, Tom Herbert <t...@herbertland.com> wrote:
>> When RCO is in effect we want to ensure that the outer checksum is
>> properly offloaded. Don't set skb->encapsulation in this case to
>> ensure that checksum offload is later considered for hw_features
>> instead of hw_enc_features.
>>
>> Signed-off-by: Tom Herbert <t...@herbertland.com>
>> ---
>> net/ipv4/udp_offload.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index b556ef6..4eedec6 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -94,11 +94,13 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct
>> sk_buff *skb,
>> do {
>> unsigned int len;
>>
>> - if (remcsum)
>> + if (remcsum) {
>> skb->ip_summed = CHECKSUM_NONE;
>> -
>> - /* Set up inner headers if we are offloading inner checksum
>> */
>> - if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> + skb->encapsulation = 0;
>> + } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> + /* Set up inner headers if we are offloading inner
>> + * checksum
>> + */
>> skb_reset_inner_headers(skb);
>> skb->encapsulation = 1;
>> }
>
>
> Why are you wasting cycles clearing a value that should have already
> been cleared?
>
> We set skb->encapsulation to 0 for the incoming skb before we segment
> it. As such all of the segments we get should also not have it set.
> It seems like you are just wasting cycles writing it again even though
> it isn't written.
>
I believe this is needed because skb->encapsulation could have been
set at a lower inner GRO, like if we were encapsulating GRE in UDP...
Tom
> - Alex
