Re: [PATCH v20 25/25] app/pdump: preserve VLAN tags in captured packets

Stephen Hemminger
2026-03-25T17:01:01Z

On Wed, 25 Mar 2026 09:12:38 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Mar 25, 2026 at 08:41:39AM +0100, Morten Brørup wrote:
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Tuesday, 24 March 2026 18.12
> > >
> > > On Mon, 16 Mar 2026 16:55:29 +0100
> > > Morten Brørup <mb@smartsharesystems.com> 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.
>
> 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.
>
> /Bruce

Virtio also requires tx_prepare if doing any form of segmentation offload.

If this is not ok, it should be put somewhere in the docs and virtio fixed.