On Wed, Mar 25, 2026 at 10:36:56AM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:[email protected]] > > Sent: Wednesday, 25 March 2026 10.13 > > > > On Wed, Mar 25, 2026 at 08:41:39AM +0100, Morten Brørup wrote: > > > > From: Stephen Hemminger [mailto:[email protected]] > > > > Sent: Tuesday, 24 March 2026 18.12 > > > > > > > > On Mon, 16 Mar 2026 16:55:29 +0100 > > > > Morten Brørup <[email protected]> wrote: > > > > > > > > > > > > > > > > This is an example of something I previously flagged. Like with > > > > real > > > > > > hardware, I think the PMD should be inserting the VLAN tag into > > the > > > > > > packet > > > > > > as part of the Tx function, not the prepare function. > > > > > > > > > > Agree with Bruce on this. > > > > > For simple stuff like VLAN offload, applications should not be > > > > required to call tx_prep first. > > > > > > > > > > However, the Tx function is supposed to not modify the packets; > > > > relevant when refcnt > 1. > > > > > > > > > > Instead of modifying the packet data to insert/strip the VLAN > > tag, > > > > > perhaps the driver can split the write/read operation into > > multiple > > > > write/read operations: > > > > > 1. the Ethernet header > > > > > 2. the VLAN tag > > > > > 3. the remaining packet data > > > > > > > > > > I haven't really followed the pcap driver, so maybe my suggestion > > > > doesn't make sense. > > > > > > > > The prepare code and VLAN was copied from virtio. > > > > I assume virtio is widely used already. > > > > > > OK, that makes it harder to object to. > > > > > > > Yes, but I also believe that the topic was not previously discussed and > > that the virtio driver may be wrong in how it behaves. > > > > I still think, for consistency with HW drivers, SW drivers should do > > the > > tagging in the Tx function. > > +1 > > > > > I also think that we should provide a DPDK-lib level helper function > > (be it in > > ethdev or elsewhere) for doing this sort of thing for all drivers. That > > way > > we can put in the necessary copying of packets with refcnt > 1 and have > > it > > apply globally. > > If an application clones packets instead of copying them, it is probably for > performance reasons. > If the drivers start copying those clones, it may defeat the performance > purpose. > > <brainstorming> > Maybe segmentation can be used instead of copying the full packet: > Make the "copy" packet of two (or more) segments, where the header is copied > into a new mbuf (where the VLAN tag is added), and the remaining part of the > packet uses an indirect mbuf referring to the "original" packet at the offset > after the header. > </brainstorming> > > Furthermore... > If drivers start copying packets in the Tx function, the Tx queue should have > its own mbuf pool to allocate these mbufs from. > Drivers should not steal mbufs from the pools used by the packets being > transmitted. > E.g. if a segmented packet has a small mbuf for the first few bytes, followed > by a large mbuf (from another pool) for the remaining bytes. > Or if the "original" mbuf comes from a mempool allocated on different CPU > socket, the "copy" would too. > Yes, agree on using a chain of buffers for Tx in this case. Not bothered much either way about the separate mempool.
/Bruce
