Re: [dpdk-dev] [PATCH] net: fix unneeded replacement of 0 by ffff for TCP checksum

[Olivier Matz <olivier.matz@6wind.com>]

On Fri, Jul 10, 2020 at 02:55:51PM +0800, Hongzhi Guo wrote:
> Per RFC768:
> If the computed checksum is zero, it is transmitted as all ones.
> An all zero transmitted checksum value means that the transmitter
> generated no checksum.
> 
> RFC793 for TCP has no such special treatment for the checksum of zero.
> 
> Fixes: 6006818cfb26 ("net: new checksum functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hongzhi Guo <guohongzhi1@huawei.com>
> ---
> v2:
> * Fixed commit tile
> * Fixed the API comment
> ---
> ---
>  lib/librte_net/rte_ip.h | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index 292f63fd7..d03c77120 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -325,7 +325,7 @@ rte_ipv4_phdr_cksum(const struct rte_ipv4_hdr *ipv4_hdr, uint64_t ol_flags)
>   *   The pointer to the beginning of the L4 header.
>   * @return
>   *   The complemented checksum to set in the IP packet
> - *   or 0 on error
> + *   or 0 if the IP length is invalid in the header.
>   */
>  static inline uint16_t
>  rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
> @@ -344,7 +344,13 @@ rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
>  
>  	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
>  	cksum = (~cksum) & 0xffff;
> -	if (cksum == 0)
> +	/*
> +	 *Per RFC768:
> +	 *If the computed checksum is zero for udp,
> +	 *it is transmitted as all ones.
> +	 *(the equivalent in one's complement arithmetic).
> +	 */

There should be a space after the '*', and maybe it could be on
less lines.

Thomas, maybe you can do it when applying?


> +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
>  		cksum = 0xffff;
>  
>  	return (uint16_t)cksum;
> @@ -438,7 +444,13 @@ rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
>  
>  	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
>  	cksum = (~cksum) & 0xffff;
> -	if (cksum == 0)
> +	/*
> +	 *Per RFC768:
> +	 *If the computed checksum is zero for udp,
> +	 *it is transmitted as all ones.
> +	 *(the equivalent in one's complement arithmetic).
> +	 */

Same here

> +	if (cksum == 0 && ipv6_hdr->proto == IPPROTO_UDP)
>  		cksum = 0xffff;
>  
>  	return (uint16_t)cksum;
> -- 
> 2.21.0.windows.1
> 
> 

Acked-by: Olivier Matz <olivier.matz@6wind.com>