[ovs-dev] [PATCH v5] userspace: Add TCP Segmentation Offload support

Ben Pfaff blp at ovn.org
Tue Jan 21 21:35:39 UTC 2020


On Sat, Jan 18, 2020 at 12:08:06AM +0100, Ilya Maximets wrote:
> On 18.01.2020 00:03, Stokes, Ian wrote:
> > Thanks all for review/testing, pushed to master.
> 
> OK, thanks Ian.
> 
> @Ben, even though this patch already merged, I'd ask you to take a look
> at the code in case you'll spot some issues especially in non-DPDK related
> parts.

I found the name dp_packet_hwol_is_ipv4(), and similar, confusing.  The
name suggested to me "test whether the packet is IPv4" not "test whether
the packet has an offloaded IPv4 checksum".  I guess the "hwol" is
offload related but...  I like the name dp_packet_hwol_tx_l4_checksum()
much more, it makes it obvious at a glance that it's a
checksum-offloading check.

In the case where we actually receive a 64 kB packet, I think that this
code is going to be relatively inefficient.  If I'm reading the code
correctly (I did it quickly), then this is what happens:

        - The first 1500 bytes of the packet land in the first
          dp_packet.

        - The remaining 64000ish bytes land in the second dp_packet.

        - Then we expand the first dp_packet to the needed size and copy
          the remaining 64000 bytes into it.

An alternative would be:

        - Set up the first dp_packet as currently.

        - Set up the second dp_packet so that the bytes are received
          into it starting at offset (mtu + headroom).

        - If more than mtu bytes are received, then copy those bytes
          into the headroom of the second dp_packet and return it to the
          caller instead of the first dp_packet.

The advantage is that we do a 1500-byte copy instead of a 64000-byte
copy.  The disadvantage is that we waste any memory leftover in the
second dp_packet, e.g. 32 kB if it's only a 32 kB packet instead of 64
kB.  Also we need slightly more sophisticated dp_packet allocation (we
will need to replenish the supply of aux_bufs).

Thanks,

Ben.


More information about the dev mailing list