On 7/2/2024 3:31 AM, Przemek Kitszel wrote:

On 7/1/24 21:55, Tom Herbert wrote:

quoted

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)

quoted

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

quoted

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)

We will be adding support for GCO "Checksum 16 with pseudo Headers" to
the ice driver. It will be off by default.

quoted

   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)