> From: Bruce Richardson [mailto:[email protected]] > Sent: Monday, 16 March 2026 17.06 > > On Mon, Mar 16, 2026 at 04:55:29PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:[email protected]] > > > Sent: Monday, 16 March 2026 16.34 > > > > > > On Tue, Mar 10, 2026 at 09:10:03AM -0700, Stephen Hemminger wrote: > > > > When the source port has VLAN strip enabled, captured packets > have > > > > the VLAN tag in mbuf metadata (vlan_tci) but not in the packet > data. > > > > Similarly, TX captures with pending VLAN insert have the tag only > > > > in metadata. The resulting pcap files contain untagged packets. > > > > > > > > Convert RX_VLAN_STRIPPED metadata to TX_VLAN offload requests on > > > > dequeued mbufs and call rte_eth_tx_prepare() before > > > rte_eth_tx_burst() > > > > so the pcap vdev inserts the tag into the packet data. > > > > > > > > > > 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. > > > > Reading the doc on tx_burst it can modify the packets (obviously > enough) if > refcnt is one, so only the edge case of refcnt > 1 needs to be worried > about. The other thing worth noting is that there is already an > exception > for bonding driver - if we need to, I suppose we can look to make a > more > general exception for a set of virtual drivers under specific > circumstances. I'd be ok with documenting that "the following drivers > modify > packets for VLAN insertion..." or something like that.
I'm opposed to API contract exceptions for virtual drivers. They should be just as well behaved as physical device drivers. Otherwise, applications need to behave differently, depending on which driver they are using. I cannot remember the reason for the exception in the bonding driver, but optimally it should be treated as a bug and fixed. > > > 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. > > > > > > Let's say an application adds 1 to the mbufs' refcnt before each Tx, > so the mbufs still exist after Tx. > > Then, the application captures/mirrors the packets consumed by the > driver, and logs/drops the packets the driver was unable to consume. > > If the capture/mirror is filtering by VLAN ID, modifying the packets > in the driver's Tx function is bad. > > > Worth investigating. We already have copies for scattered packets, I > think, > so doing some copying for VLAN insertion if refcnt > 1 wouldn't be the > end > of the world. +1 > > /Bruce
