> 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. > > /Bruce
