netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tom Herbert <tom@herbertland.com>
To: "Rustad, Mark D" <mark.d.rustad@intel.com>
Cc: "Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	"nhorman@redhat.com" <nhorman@redhat.com>,
	"sassmann@redhat.com" <sassmann@redhat.com>,
	"jogreene@redhat.com" <jogreene@redhat.com>
Subject: Re: [net-next 05/19] ixgbe: Add support for UDP-encapsulated tx checksum offload
Date: Wed, 2 Sep 2015 10:38:17 -0700	[thread overview]
Message-ID: <CALx6S36T4tzB5aq3bbKq4DMEumvC4q2YhGscn19se0mGXGe5cQ@mail.gmail.com> (raw)
In-Reply-To: <F63CFE64-29E1-484B-B19B-269FAAA9C54B@intel.com>

On Wed, Sep 2, 2015 at 9:46 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:
>> On Sep 1, 2015, at 8:17 PM, Tom Herbert <tom@herbertland.com> wrote:
>>
>> I suspect this is not UDP-encapsulation specific, will it work with
>> TCP/IP/IP, TCP/IP/GRE etc.?
>
> It could do more, but this is what has been tested up to this point.
>
Well, please test the those other encapsulations too! It's nice and
all if they get the benefit, but it's really bad news if these changes
were to screw them up (i.e. you don't want users of the GRE, IPIP to
find out that they're now broken).

>> Isn't there anyway the ixgbe could just be made to NETIF_HW_CSUM? That
>> would be so much more straightforward and support nearly all use cases
>> without needing to jump through all these hoops.
>
> Well, the description says:
>
>  ---
> Note: NETIF_F_HW_CSUM is a superset of NETIF_F_IP_CSUM + NETIF_F_IPV6_CSUM.
> It means that device can fill TCP/UDP-like checksum anywhere in the packets
> whatever headers there might be.
>  ---
>
> The device can't do whatever, wherever. There is always a limit to the offset to the inner headers that can be handled, for instance.
>
If the device does NETIF_F_HW_CSUM then inner/outer headers are
irrelevant at least in the non-GSO case. All the device needs to do is
compute the checksum from start and write the answer at the given
offset. No protocol awareness needed in the device, no need to parse
headers on transmit.

I have the same complaint that ixgbe requires a bunch of driver logic
to offload VXLAN checksum unnecessary instead of just providing
CHECKSUM_COMPLETE which would work with any encapsulation protocol,
require no encapsulation awareness in the device, and should be a much
simpler driver implementation.

So my input to NIC vendors will continue to be they provide general
protocol agnostic solutions and *stop* perpetuating these narrow
protocol specific and unnecessarily complicated solutions. If you
don't believe me, see the similar longstanding comments in skbuff.h
about NIC capabilities and checksums and what choices vendors make.

Tom

> --
> Mark Rustad, Networking Division, Intel Corporation
>

  reply	other threads:[~2015-09-02 17:38 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-02  1:13 [net-next 00/19][pull request] Intel Wired LAN Driver Updates 2015-09-01 Jeff Kirsher
2015-09-02  1:13 ` [net-next 01/19] i40e: Set defport behavior for the Main VSI when in promiscuous mode Jeff Kirsher
2015-09-02  1:13 ` [net-next 02/19] ixgbe: add new function to check for management presence Jeff Kirsher
2015-09-02  1:13 ` [net-next 03/19] ixgbe: Assign set_phy_power dynamically where needed Jeff Kirsher
2015-09-02  1:13 ` [net-next 04/19] ixgbe: Check whether FDIRCMD writes actually complete Jeff Kirsher
2015-09-02  1:13 ` [net-next 05/19] ixgbe: Add support for UDP-encapsulated tx checksum offload Jeff Kirsher
2015-09-02  3:17   ` Tom Herbert
2015-09-02 16:46     ` Rustad, Mark D
2015-09-02 17:38       ` Tom Herbert [this message]
2015-09-02 20:56         ` Or Gerlitz
2015-09-02 21:07         ` Or Gerlitz
2015-09-02 22:34           ` Tom Herbert
2015-09-02 23:21       ` Tom Herbert
2015-09-03  0:21         ` Rustad, Mark D
2015-09-02  1:13 ` [net-next 06/19] ixgbe: Add support for VXLAN RX offloads Jeff Kirsher
2015-09-02  3:31   ` Tom Herbert
2015-09-02 16:53     ` Rustad, Mark D
2015-09-02  1:13 ` [net-next 07/19] ixgbe: Add support for entering low power link up state Jeff Kirsher
2015-09-02  1:13 ` [net-next 08/19] ixgbe: add get_bus_info method for X550 Jeff Kirsher
2015-09-02  1:13 ` [net-next 09/19] ixgbe: add new bus type for intergrated I/O interface (IOSF) Jeff Kirsher
2015-09-02  1:13 ` [net-next 10/19] ixgbe: Remove unused PCI bus types Jeff Kirsher
2015-09-02  1:13 ` [net-next 11/19] ixgbe: use kzalloc for allocating one thing Jeff Kirsher
2015-09-02  1:13 ` [net-next 12/19] ixgbe: Remove second instance of lan_id variable Jeff Kirsher
2015-09-02  1:13 ` [net-next 13/19] ixgbe: cleanup to use cached mask value Jeff Kirsher
2015-09-02  1:13 ` [net-next 14/19] ixgbe: Avoid needless PHY access on copper phys Jeff Kirsher
2015-09-02  1:13 ` [net-next 15/19] ixgbe: support for ethtool set_rxfh Jeff Kirsher
2015-09-02  1:14 ` [net-next 16/19] ixgbe: fix bounds checking in ixgbe_setup_tc for 82598 Jeff Kirsher
2015-09-02  1:14 ` [net-next 17/19] ixgbe: Add support for reporting 2.5G link speed Jeff Kirsher
2015-09-02  1:14 ` [net-next 18/19] ixgbe: Remove bimodal SR-IOV disabling Jeff Kirsher
2015-09-02  1:14 ` [net-next 19/19] ixgbe: Resolve "initialized field overwritten" warnings Jeff Kirsher
2015-09-02  3:19 ` [net-next 00/19][pull request] Intel Wired LAN Driver Updates 2015-09-01 David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALx6S36T4tzB5aq3bbKq4DMEumvC4q2YhGscn19se0mGXGe5cQ@mail.gmail.com \
    --to=tom@herbertland.com \
    --cc=davem@davemloft.net \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jogreene@redhat.com \
    --cc=mark.d.rustad@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@redhat.com \
    --cc=sassmann@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).