Re: [PATCH net] skbuff.h: Improve the checksum related comments

[Matthew Wilcox]

On Sun, Apr 05, 2020 at 12:17:43AM -0700, Dexuan Cui wrote:
>   * CHECKSUM_COMPLETE:
>   *
> - *   This is the most generic way. The device supplied checksum of the _whole_
> - *   packet as seen by netif_rx() and fills out in skb->csum. Meaning, the
> + *   This is the most generic way. The device supplies checksum of the _whole_
> + *   packet as seen by netif_rx() and fills out in skb->csum. This means the

I think both 'supplies' and 'supplied' are correct in this sentence.  The
nuances are slightly different, but the meaning is the same in this instance.

You missed a mistake in the second line though, it should be either 'fills
out' or 'fills in'.  I think we tend to prefer 'fills in'.

>   * CHECKSUM_COMPLETE:
>   *   Not used in checksum output. If a driver observes a packet with this value
> - *   set in skbuff, if should treat as CHECKSUM_NONE being set.
> + *   set in skbuff, the driver should treat it as CHECKSUM_NONE being set.

I would go with "it should treat the packet as if CHECKSUM_NONE were set."

> @@ -211,7 +211,7 @@
>   * is implied by the SKB_GSO_* flags in gso_type. Most obviously, if the
>   * gso_type is SKB_GSO_TCPV4 or SKB_GSO_TCPV6, TCP checksum offload as
>   * part of the GSO operation is implied. If a checksum is being offloaded
> - * with GSO then ip_summed is CHECKSUM_PARTIAL, csum_start and csum_offset
> + * with GSO then ip_summed is CHECKSUM_PARTIAL AND csum_start and csum_offset
>   * are set to refer to the outermost checksum being offload (two offloaded
>   * checksums are possible with UDP encapsulation).

Why the capitalisation of 'AND'?

Thanks for the improvements,

Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>