On Thu, Apr 8, 2021 at 9:53 AM Olivier Matz <olivier.m...@6wind.com> wrote: > > On Thu, Apr 01, 2021 at 11:52:40AM +0200, David Marchand wrote: > > Tx offload flags are of the application responsibility. > > Leave the mbuf alone and check for TSO where needed. > > > > Signed-off-by: David Marchand <david.march...@redhat.com>
Self nack on this patch. > > Maybe the problem being solved should be better described in the commit > log. Is it a problem (other than cosmetic) to touch a mbuf in the Tx > function of a driver, where we could expect that the mbuf is owned by > the driver? > > The only problem I can think about is in case we transmit a direct mbuf > whose refcnt is increased, but I wonder how much this is really > supported: for instance, several drivers add vlans using > rte_vlan_insert() in their Tx path. This was my initial thought, as I suspected issues with applications which keep a refcount on the mbuf. But after more discussions offlist, it is hard to find a usecase for this. The gso library already touches the ol_flags from the input mbuf and this is documented in its API. Plus, my patch also has an issue. This library gives back an array of direct/indirect mbufs pointing at the data from the original mbuf. Those mbufs are populated with the original mbuf ol_flags but nothing has been done on the checksum in this data. The net/tap driver relies on this feature: mbuf_in is marked with PKT_TX_TCP_CKSUM so that the generated mbufs given back by the rte_gso library has this offload flag set and PKT_TX_TCP_SEG is not present anymore. Later in the net/tap tx handler, PKT_TX_TCP_CKSUM presence triggers tcp checksum computation. So this patch breaks the tso support in net/tap. Just for the record. As far as rte_vlan_insert() is concerned, it ensures that the mbuf is not shared. https://git.dpdk.org/dpdk/tree/lib/net/rte_ether.h#n352 -- David Marchand
