Indeed, the vlan insertion could be a costly operation. We should probably do it only if the user specifically asks to have the vlan tag in the packet. Otherwise, af_packet PMD users might pay a price in terms of performance for something they didn't ask for.
I was thinking of avoiding having to change the application in order to re-insert the vlan tag. Doing this operation inside the PMD driver seemed like a good fit. Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is guarded by a check to hv->vlan_strip if (!hv->vlan_strip && rte_vlan_insert(&m)) { hv->vlan_strip seems to be initialized in hn_dev_configure() in the following way hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP); while 'hv' seems to be stored in rte_eth_dev->data->dev_private I am thinking of doing something similar for the af_packet PMD. The 'pmd_internals' structure could potentially hold a field, say vlan_strip', which could be initialized if the application enables the DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads This way, I'm thinking that the application could potentially control the effect of vlan stripping for the af_packet PMD, in an uniform way, similar to other PMDs. Would this be considered an acceptable solution ? On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yi...@intel.com> wrote: > On 8/20/2021 1:46 PM, Tudor Cornea wrote: > > The af_packet pmd driver binds to a raw socket and allows > > sending and receiving of packets through the kernel. > > > > Since commit bcc6d47903 [1], the kernel strips the vlan tags early in > > __netif_receive_skb_core(), so we receive untagged packets while > > running with the af_packet pmd. > > > > Luckily for us, the skb vlan-related fields are still populated from the > > stripped vlan tags, so we end up having all the information > > that we need in the mbuf. > > > > We would like to have the the vlan tag inside the mbuf. > > Let's take a shot at it by trying to reinsert the stripped vlan tag. > > > > PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED' > flags, so application can be aware of the vlan tag and can consume it. > > Inserting the vlan tag back to packet is costly, what is the motivation to > do so? > > > As a side note, something similar was done for the netvsc pmd. > > > > [1] > https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2 > > > > Signed-off-by: Tudor Cornea <tudor.cor...@gmail.com> > > --- > > drivers/net/af_packet/rte_eth_af_packet.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c > b/drivers/net/af_packet/rte_eth_af_packet.c > > index b73b211..d116583 100644 > > --- a/drivers/net/af_packet/rte_eth_af_packet.c > > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > > @@ -148,6 +148,10 @@ eth_af_packet_rx(void *queue, struct rte_mbuf > **bufs, uint16_t nb_pkts) > > if (ppd->tp_status & TP_STATUS_VLAN_VALID) { > > mbuf->vlan_tci = ppd->tp_vlan_tci; > > mbuf->ol_flags |= (PKT_RX_VLAN | > PKT_RX_VLAN_STRIPPED); > > + > > + /* the kernel always strips the vlan tag, try to > reinsert it */ > > + if (rte_vlan_insert(&mbuf)) > > + PMD_LOG(ERR, "Failed to reinsert vlan > tag"); > > } > > > > /* release incoming frame and advance ring buffer */ > > > >