Re: [PATCH net-next v2 0/7] drivers: Fix drivers doing TX csum offload with EH

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 7/1/24 21:55, Tom Herbert wrote:
> Several NICs would seem to support protocol specific TX checksum offload
> and allow for cases where an IPv6 packet contains extension headers.
> When deciding whether to offload a packet, ipv6_skip_exthdr is called
> to skip extension headers. The problem is that if a packet contains an
> IPv6 Routing Header then protocol specific checksum offload can't work,
> the destination IP address in the IPv6 header is not the same one that
> is used in the pseudo header for TCP or UDP. The correct address is
> derived from the last segment in the routing list (which itself might
> be obfuscated so that a device could even read it).

feels like there is a missing "not" after "could" - with it added, reads
fine (not a request to change, just being verbose about assumptions)

> 
> This patch set adds a new function ipv6_skip_exthdr_no_rthdr to be
> called in lieu of ipv6_skip_exthdr. If a routing header is present in
> a packet then ipv6_skip_exthdr_no_rthdr returns a value less than
> zero, this is an indication to the driver that TX checksum offload
> is not viable and it should call skb_checksum_help instead of
> offloading the checksum.
> 
> The i40e, iavf, ice, idpf, hinic, and fm10k are updated accordingly
> to call ipv6_skip_exthdr_no_rthdr.
> 
> Testing: The code compiles, but is otherwise untested due to lack of
> NIC hardware. It would be appreciated if someone with access to the
> hardware could test.

we could test intel ones (except fm10k) via @Tony's tree

> 
> v2: Fixed uninitialized variable in exthdrs_core.c
> 
> Tom Herbert (7):
>    ipv6: Add ipv6_skip_exthdr_no_rthdr
>    i40e: Don't do TX csum offload with routing header present
>    iavf: Don't do TX csum offload with routing header present
>    ice: Don't do TX csum offload with routing header present

sidenote:
our HW is supporting (among others) a GCO check-summing mode described
as: "Checksum 16bit (TCP/UDP) with no pseudo Header", but we have not
yet provided patches for that, and I don't even know if this mode
will be used (CC @Paul)

>    idpf: Don't do TX csum offload with routing header present
>    hinic: Don't do TX csum offload with routing header present
>    fm10k: Don't do TX csum offload with routing header present
> 
>   drivers/net/ethernet/huawei/hinic/hinic_tx.c  | 23 +++++++++++----
>   drivers/net/ethernet/intel/fm10k/fm10k_main.c |  9 ++++--
>   drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 22 ++++++---------
>   drivers/net/ethernet/intel/iavf/iavf_txrx.c   | 20 ++++++-------
>   drivers/net/ethernet/intel/ice/ice_txrx.c     | 22 ++++++---------
>   .../ethernet/intel/idpf/idpf_singleq_txrx.c   | 28 +++++++++----------
>   include/net/ipv6.h                            | 17 +++++++++--
>   net/ipv6/exthdrs_core.c                       | 25 ++++++++++++-----
>   8 files changed, 98 insertions(+), 68 deletions(-)
> 

I have reviewed the patches and they conform to commit message/intent,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
(for the series)