Messages in this thread | | | Date | Sun, 5 Apr 2020 03:36:18 -0700 | From | Matthew Wilcox <> | Subject | Re: [PATCH net] skbuff.h: Improve the checksum related comments |
| |
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@infradead.org>
| |