> -----Original Message-----
> From: Long Li <lon...@microsoft.com>
> Sent: Saturday, February 10, 2024 2:48 AM
> To: Wei Hu <w...@microsoft.com>; ferruh.yi...@amd.com;
> andrew.rybche...@oktetlabs.ru; Thomas Monjalon
> <tho...@monjalon.net>; Alan Elder <alan.el...@microsoft.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH 1/1] net/mana: add vlan tagging support
>
> > + if (oob->rx_vlan_tag_present) {
> > + mbuf->ol_flags |=
> > + RTE_MBUF_F_RX_VLAN |
> > RTE_MBUF_F_RX_VLAN_STRIPPED;
> > + mbuf->vlan_tci = oob->rx_vlan_id;
> > + }
> > +
>
> Netvsc has the following code for dealing with vlan on RX mbufs (in
> hn_rxtx.c):
> /* NDIS always strips tag, put it back if necessary */
> if (!hv->vlan_strip && rte_vlan_insert(&m)) {
>
> It seems we should do the same?
Not sure if we want to do the same. Two reasons.
1. Searching the netvsc source, I don't see a place that it set hv->vlan_strip
to false. It means
!hv->vlan_string is always false, and rte_vlan_insert(&m) never run.
2. Usually vlan_strip can be set to true or false if the hardware supports this
feature. In the mana
case, the hardware strips off the vlan tag anyway. There is no way to tell the
mana hardware to
keep the tag. Adding the tag back by software not only slows things down, but
it also complicates the
code and test. Not sure if there is any real application needs it.
I am open to add it. But in my opinion, we don't need it. Let me know what you
think.
>
> > pkts[pkt_received++] = mbuf;
> > rxq->stats.packets++;
> > rxq->stats.bytes += mbuf->data_len; diff --git
> > a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c index
> > 58c4a1d976..f075fcb0f5 100644
> > --- a/drivers/net/mana/tx.c
> > +++ b/drivers/net/mana/tx.c
> > @@ -180,6 +180,15 @@ get_vsq_frame_num(uint32_t vsq)
> > return v.vsq_frame;
> > }
> >
> > +#define VLAN_PRIO_MASK 0xe000 /* Priority Code Point */
> > +#define VLAN_PRIO_SHIFT 13
> > +#define VLAN_CFI_MASK 0x1000 /* Canonical Format Indicator
> > / Drop Eligible Indicator */
> > +#define VLAN_VID_MASK 0x0fff /* VLAN Identifier */
> > +
> > +#define mana_mbuf_vlan_tag_get_id(m) ((m)->vlan_tci &
> > VLAN_VID_MASK)
> > +#define mana_mbuf_vlan_tag_get_cfi(m) (!!((m)->vlan_tci &
> > VLAN_CFI_MASK))
> > +#define mana_mbuf_vlan_tag_get_prio(m) (((m)->vlan_tci &
> > VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT)
> > +
>
> Those definitions look like those in @Alan Elder's patch for netvsc. Can we
> consolidate some of those definitions into a common place?
>
> Maybe in "lib/net/rte_ether.h"?
>
Ok. Will add it to rte_ether.h.
Thanks,
Wei
> Thanks,
>
> Long
