[1/1] net/mana: add vlan tagging support
Checks
Commit Message
For tx path, use LONG_PACKET_FORMAT if vlan tag is present. For rx,
extract vlan id from oob, put into mbuf and set the vlan flags in
mbuf.
Also add myself to the maintainers list for vmbus, mana and netvsc.
Signed-off-by: Wei Hu <weh@microsoft.com>
---
MAINTAINERS | 3 +++
drivers/net/mana/mana.h | 2 ++
drivers/net/mana/rx.c | 6 ++++++
drivers/net/mana/tx.c | 30 +++++++++++++++++++++++++++---
4 files changed, 38 insertions(+), 3 deletions(-)
Comments
On 2/9/2024 8:52 AM, Wei Hu wrote:
> For tx path, use LONG_PACKET_FORMAT if vlan tag is present. For rx,
> extract vlan id from oob, put into mbuf and set the vlan flags in
> mbuf.
>
> Also add myself to the maintainers list for vmbus, mana and netvsc.
>
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
> MAINTAINERS | 3 +++
> drivers/net/mana/mana.h | 2 ++
> drivers/net/mana/rx.c | 6 ++++++
> drivers/net/mana/tx.c | 30 +++++++++++++++++++++++++++---
> 4 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5fb3a73f84..9983d013a6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -608,6 +608,7 @@ F: app/test/test_vdev.c
>
> VMBUS bus driver
> M: Long Li <longli@microsoft.com>
> +M: Wei Hu <weh@microsoft.com>
> F: drivers/bus/vmbus/
>
>
> @@ -867,6 +868,7 @@ F: doc/guides/nics/features/mlx5.ini
>
> Microsoft mana
> M: Long Li <longli@microsoft.com>
> +M: Wei Hu <weh@microsoft.com>
> F: drivers/net/mana/
> F: doc/guides/nics/mana.rst
> F: doc/guides/nics/features/mana.ini
> @@ -878,6 +880,7 @@ F: doc/guides/nics/vdev_netvsc.rst
>
> Microsoft Hyper-V netvsc
> M: Long Li <longli@microsoft.com>
> +M: Wei Hu <weh@microsoft.com>
> F: drivers/net/netvsc/
> F: doc/guides/nics/netvsc.rst
> F: doc/guides/nics/features/netvsc.ini
>
Hi Wei,
Can you please separate the maintainer file update to its own patch?
> + 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?
> 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"?
Thanks,
Long
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Saturday, February 10, 2024 12:06 AM
> To: Wei Hu <weh@microsoft.com>; andrew.rybchenko@oktetlabs.ru; Thomas
> Monjalon <thomas@monjalon.net>; Long Li <longli@microsoft.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 1/1] net/mana: add vlan tagging support
>
> On 2/9/2024 8:52 AM, Wei Hu wrote:
> > For tx path, use LONG_PACKET_FORMAT if vlan tag is present. For rx,
> > extract vlan id from oob, put into mbuf and set the vlan flags in
> > mbuf.
> >
> > Also add myself to the maintainers list for vmbus, mana and netvsc.
> >
> > Signed-off-by: Wei Hu <weh@microsoft.com>
> > ---
> > MAINTAINERS | 3 +++
> > drivers/net/mana/mana.h | 2 ++
> > drivers/net/mana/rx.c | 6 ++++++
> > drivers/net/mana/tx.c | 30 +++++++++++++++++++++++++++---
> > 4 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index 5fb3a73f84..9983d013a6
> > 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -608,6 +608,7 @@ F: app/test/test_vdev.c
> >
> > VMBUS bus driver
> > M: Long Li <longli@microsoft.com>
> > +M: Wei Hu <weh@microsoft.com>
> > F: drivers/bus/vmbus/
> >
> >
> > @@ -867,6 +868,7 @@ F: doc/guides/nics/features/mlx5.ini
> >
> > Microsoft mana
> > M: Long Li <longli@microsoft.com>
> > +M: Wei Hu <weh@microsoft.com>
> > F: drivers/net/mana/
> > F: doc/guides/nics/mana.rst
> > F: doc/guides/nics/features/mana.ini
> > @@ -878,6 +880,7 @@ F: doc/guides/nics/vdev_netvsc.rst
> >
> > Microsoft Hyper-V netvsc
> > M: Long Li <longli@microsoft.com>
> > +M: Wei Hu <weh@microsoft.com>
> > F: drivers/net/netvsc/
> > F: doc/guides/nics/netvsc.rst
> > F: doc/guides/nics/features/netvsc.ini
> >
>
> Hi Wei,
>
> Can you please separate the maintainer file update to its own patch?
Sure. I will remove the maintainer file update and send it out later separately.
Thanks,
Wei
> -----Original Message-----
> From: Long Li <longli@microsoft.com>
> Sent: Saturday, February 10, 2024 2:48 AM
> To: Wei Hu <weh@microsoft.com>; ferruh.yigit@amd.com;
> andrew.rybchenko@oktetlabs.ru; Thomas Monjalon
> <thomas@monjalon.net>; Alan Elder <alan.elder@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
> 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.
It's set here:
drivers/net/netvsc/hn_ethdev.c: hv->vlan_strip = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
>
> 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.
If MANA always strips the tag, you need to add the tag back if needed.
>
> 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.
@Alan has sent a patch for some helper marcos for parsing vlan_tci, please use those helper marcos in
https://git.dpdk.org/next/dpdk-next-net/commit/?id=f9c66ea9b5bfd15f314f6d101f156a312121e7e1
Thanks,
Long
@@ -608,6 +608,7 @@ F: app/test/test_vdev.c
VMBUS bus driver
M: Long Li <longli@microsoft.com>
+M: Wei Hu <weh@microsoft.com>
F: drivers/bus/vmbus/
@@ -867,6 +868,7 @@ F: doc/guides/nics/features/mlx5.ini
Microsoft mana
M: Long Li <longli@microsoft.com>
+M: Wei Hu <weh@microsoft.com>
F: drivers/net/mana/
F: doc/guides/nics/mana.rst
F: doc/guides/nics/features/mana.ini
@@ -878,6 +880,7 @@ F: doc/guides/nics/vdev_netvsc.rst
Microsoft Hyper-V netvsc
M: Long Li <longli@microsoft.com>
+M: Wei Hu <weh@microsoft.com>
F: drivers/net/netvsc/
F: doc/guides/nics/netvsc.rst
F: doc/guides/nics/features/netvsc.ini
@@ -21,10 +21,12 @@ struct mana_shared_data {
#define MANA_MAX_MAC_ADDR 1
#define MANA_DEV_RX_OFFLOAD_SUPPORT ( \
+ RTE_ETH_RX_OFFLOAD_VLAN_STRIP | \
RTE_ETH_RX_OFFLOAD_CHECKSUM | \
RTE_ETH_RX_OFFLOAD_RSS_HASH)
#define MANA_DEV_TX_OFFLOAD_SUPPORT ( \
+ RTE_ETH_TX_OFFLOAD_VLAN_INSERT | \
RTE_ETH_TX_OFFLOAD_MULTI_SEGS | \
RTE_ETH_TX_OFFLOAD_IPV4_CKSUM | \
RTE_ETH_TX_OFFLOAD_TCP_CKSUM | \
@@ -517,6 +517,12 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
mbuf->hash.rss = oob->packet_info[pkt_idx].packet_hash;
}
+ 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;
+ }
+
pkts[pkt_received++] = mbuf;
rxq->stats.packets++;
rxq->stats.bytes += mbuf->data_len;
@@ -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)
+
uint16_t
mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
{
@@ -254,7 +263,18 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
}
/* Fill in the oob */
- tx_oob.short_oob.packet_format = SHORT_PACKET_FORMAT;
+ if (m_pkt->ol_flags & RTE_MBUF_F_TX_VLAN) {
+ tx_oob.short_oob.packet_format = LONG_PACKET_FORMAT;
+ tx_oob.long_oob.inject_vlan_prior_tag = 1;
+ tx_oob.long_oob.priority_code_point =
+ mana_mbuf_vlan_tag_get_prio(m_pkt);
+ tx_oob.long_oob.drop_eligible_indicator =
+ mana_mbuf_vlan_tag_get_cfi(m_pkt);
+ tx_oob.long_oob.vlan_identifier =
+ mana_mbuf_vlan_tag_get_id(m_pkt);
+ } else {
+ tx_oob.short_oob.packet_format = SHORT_PACKET_FORMAT;
+ }
tx_oob.short_oob.tx_is_outer_ipv4 =
m_pkt->ol_flags & RTE_MBUF_F_TX_IPV4 ? 1 : 0;
tx_oob.short_oob.tx_is_outer_ipv6 =
@@ -409,8 +429,12 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
work_req.sgl = sgl.gdma_sgl;
work_req.num_sgl_elements = m_pkt->nb_segs;
- work_req.inline_oob_size_in_bytes =
- sizeof(struct transmit_short_oob_v2);
+ if (tx_oob.short_oob.packet_format == SHORT_PACKET_FORMAT)
+ work_req.inline_oob_size_in_bytes =
+ sizeof(struct transmit_short_oob_v2);
+ else
+ work_req.inline_oob_size_in_bytes =
+ sizeof(struct transmit_oob_v2);
work_req.inline_oob_data = &tx_oob;
work_req.flags = 0;
work_req.client_data_unit = NOT_USING_CLIENT_DATA_UNIT;