Re: [dpdk-dev] [PATCH RFC 11/11] ixgbe/mbuf: add TSO support

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Konstantin,

On 05/15/2014 06:30 PM, Ananyev, Konstantin wrote:
> With the current DPDK implementation the upper code would still be different for TCP checksum (without segmentation) and TCP segmentation:
> different flags in mbuf, with TSO you need to setup l4_len and mss fields inside mbuf, with just checksum - you don't.

You are right on this point.

> Plus, as I said, it is a bit confusing to calculate PSD csum once in the stack and then re-calculate in PMD.
> Again - unnecessary slowdown.
> So why not just have get_ipv4_psd_sum() and get_ipv4_psd_tso_sum() inside testpmd/csumonly.c and call them accordingly?

Yes, recalculating the pseudo-header checksum without the ip_len
is a slow down. This slow down should however be compared to the
operation in progress. When you do TSO, you are generally transmitting
a large TCP packet (several KB), and the cost of the TCP stack is
probably much higher than fixing the checksum. But that's not the
main argument: my idea was to choose the proper API that will
reduce the slow down for most cases.

Let's take the case of a future vnic pmd driver supporting an emulation
of TSO. In this case, the calculation of the pseudo header is also an
unnecessary slowdown.

Also, some other hardware I've seen don't need to calculate a different
pseudo header checksum when doing TSO.

Last argument, the way Linux works is the same that what I've
implemented. See in linux ixgbe driver [1] at line 6461, there is a
call to csum_tcpudp_magic() which reprocesses the checksum without
the ip_len.

On the other hand, that's true that today ixgbe is the only hardware
supporting TSO in DPDK. The pragmatic approach could be to choose the
API that gives the best performance with what we have (the ixgbe
driver). I'm ok with this approach if we accept to reconsider the API
(and maybe modifying it) when another PMD supporting TSO will be
implemented.

> About potential future problem with NICs that implement TX checksum/segmentation offloads in a different manner - yeh that's true...
> I think at the final point all that logic could be hidden inside some function at rte_ethdev level, something like:  rte_eth_dev_prep_tx(portid, mbuf[], num).

I don't see the real difference between:

   rte_eth_dev_prep_tx(portid, mbuf[], num)
   rte_eth_dev_tx(portid, mbuf[], num)

and:

   rte_eth_dev_tx(portid, mbuf[], num) /* the tx does the cksum job */

And the second is faster because there is only one pointer dereference.

> So,  based on mbuf TX offload flags and device type, it would do necessary modifications inside the packet.
> But that's future discussion, I suppose.

To me, it's not an option to fill that the network stack fills the
mbuf differently depending on the device type. For instance, when doing
ethernet bonding or bridging, the stack may not know which physical
device will be used at the end. So the API to enable TSO on a packet
has to be the same whatever the device.

If fixing the checksum in the PMD is an unnecessary slowdown, forcing
the network stack to check what has to be filled in the mbuf depending
on the device type also has a cost.

> For now, I still think we need to keep pseudo checksum calculations out of PMD code.

To me, there are 2 options:

1/ Update patch to calculate the pseudo header without the ip_len when
    doing TSO. In this case the API is mapped on ixgbe behavior,
    probably providing the best performance today. If another PMD comes
    in the future, this API may change to something more generic.

2/ Try to define a generic API today, accepting that the first driver
    that supports TSO is a bit slower, but reducing the risks of changing
    the API for TSO in the future.

I'm fine with both options.

Regards,
Olivier

[1] 
http://lxr.free-electrons.com/source/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?v=3.14#L6434