netdev - Re: [PATCH net-next] net/mlx4_en: optimizes get_fixed_ipv6

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 2 May 2018 17:18:34 +0300
From:   Tariq Toukan <tariqt@...lanox.com>
To:     Saeed Mahameed <saeedm@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "eric.dumazet@...il.com" <eric.dumazet@...il.com>
Subject: Re: [PATCH net-next] net/mlx4_en: optimizes get_fixed_ipv6_csum()



On 27/04/2018 1:56 AM, Saeed Mahameed wrote:
> On Thu, 2018-04-19 at 08:49 -0700, Eric Dumazet wrote:
>> While trying to support CHECKSUM_COMPLETE for IPV6 fragments,
>> I had to experiments various hacks in get_fixed_ipv6_csum().
>> I must admit I could not find how to implement this :/
>>
>> However, get_fixed_ipv6_csum() does a lot of redundant operations,
>> calling csum_partial() twice.
>>
>> First csum_partial() computes the checksum of saddr and daddr,
>> put in @csum_pseudo_hdr. Undone later in the second csum_partial()
>> computed on whole ipv6 header.
>>
>> Then nexthdr is added once, added a second time, then substracted.
>>
>> payload_len is added once, then substracted.
>>
>> Really all this can be reduced to two add_csum(), to add back 6 bytes
>> that were removed by mlx4 when providing hw_checksum in RX
>> descriptor.
>>
>> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>> Cc: Saeed Mahameed <saeedm@...lanox.com>
>> Cc: Tariq Toukan <tariqt@...lanox.com>
>> ---
>> Note: This patch, like other mlx4 patches can definitely wait
>> Tariq approval, thanks !
>>
> 
> LGTM,
> 
> Reviewed-by: Saeed Mahameed <saeedm@...lanox.com>
> 

Acked-by: Tariq Toukan <tariqt@...lanox.com>

Thanks Eric.

>>   drivers/net/ethernet/mellanox/mlx4/en_rx.c | 21 ++++++++----------
>> ---
>>   1 file changed, 8 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> index
>> 5c613c6663da51a4ae792eeb4d8956b54655786b..38c56fb6e5f5970f245dd56c38e
>> 1fc63a9349a07 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> @@ -593,30 +593,25 @@ static int get_fixed_ipv4_csum(__wsum
>> hw_checksum, struct sk_buff *skb,
>>   }
>>   
>>   #if IS_ENABLED(CONFIG_IPV6)
>> -/* In IPv6 packets, besides subtracting the pseudo header checksum,
>> - * we also compute/add the IP header checksum which
>> - * is not added by the HW.
>> +/* In IPv6 packets, hw_checksum lacks 6 bytes from IPv6 header:
>> + * 4 first bytes : priority, version, flow_lbl
>> + * and 2 additional bytes : nexthdr, hop_limit.
>>    */
>>   static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff
>> *skb,
>>   			       struct ipv6hdr *ipv6h)
>>   {
>>   	__u8 nexthdr = ipv6h->nexthdr;
>> -	__wsum csum_pseudo_hdr = 0;
>> +	__wsum temp;
>>   
>>   	if (unlikely(nexthdr == IPPROTO_FRAGMENT ||
>>   		     nexthdr == IPPROTO_HOPOPTS ||
>>   		     nexthdr == IPPROTO_SCTP))
>>   		return -1;
>> -	hw_checksum = csum_add(hw_checksum, (__force
>> __wsum)htons(nexthdr));
>>   
>> -	csum_pseudo_hdr = csum_partial(&ipv6h->saddr,
>> -				       sizeof(ipv6h->saddr) +
>> sizeof(ipv6h->daddr), 0);
>> -	csum_pseudo_hdr = csum_add(csum_pseudo_hdr, (__force
>> __wsum)ipv6h->payload_len);
>> -	csum_pseudo_hdr = csum_add(csum_pseudo_hdr,
>> -				   (__force __wsum)htons(nexthdr));
>> -
>> -	skb->csum = csum_sub(hw_checksum, csum_pseudo_hdr);
>> -	skb->csum = csum_add(skb->csum, csum_partial(ipv6h,
>> sizeof(struct ipv6hdr), 0));
>> +	/* priority, version, flow_lbl */
>> +	temp = csum_add(hw_checksum, *(__wsum *)ipv6h);
>> +	/* nexthdr and hop_limit */
>> +	skb->csum = csum_add(temp, (__force __wsum)*(__be16
>> *)&ipv6h->nexthdr);
>>   	return 0;
>>   }
>>   #endif