> -----Original Message-----
> From: Alan Elder <alan.el...@microsoft.com>
> Sent: Thursday, February 8, 2024 7:09 AM
> To: Long Li <lon...@microsoft.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3] net/netvsc: fix parsing of VLAN metadata
>
> The previous code incorrectly parsed the VLAN ID and priority.
> If the 16-bits of VLAN ID and priority/CFI on the wire was 0123456789ABCDEF
> the code parsed it as 456789ABCDEF3012. There were macros defined to handle
> this conversion but they were not used.
>
> This fix takes an approach similar to the Linux netvsc driver and defines an
> explicit structure to use for parsing.
>
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Cc: sthem...@microsoft.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Alan Elder <alan.el...@microsoft.com>
> ---
> .mailmap | 1 +
> drivers/net/netvsc/hn_rxtx.c | 23 +++++++++++++++++------
> drivers/net/netvsc/ndis.h | 23 +++++++++++++----------
> 3 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/.mailmap b/.mailmap
> index a0756974e2..eca02318d6 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -33,6 +33,7 @@ Alain Leon <xer...@gmail.com> Alan Brady
> <alan.br...@intel.com> Alan Carew <alan.ca...@intel.com> Alan Dewar
> <alan.de...@att.com> <ade...@brocade.com>
> +Alan Elder <alan.el...@microsoft.com>
> Alan Liu <zaoxing...@gmail.com>
> Alan Winkowski <wa...@marvell.com>
> Alejandro Lucero <alejandro.luc...@netronome.com> diff --git
> a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index
> e4f5015aa3..e3b9899f1e 100644
> --- a/drivers/net/netvsc/hn_rxtx.c
> +++ b/drivers/net/netvsc/hn_rxtx.c
> @@ -42,8 +42,13 @@
> #define HN_TXD_CACHE_SIZE 32 /* per cpu tx_descriptor pool cache */
> #define HN_RXQ_EVENT_DEFAULT 2048
>
> +#define HN_VLAN_PRIO_MASK 0xe000 /* Priority Code Point */
> +#define HN_VLAN_PRIO_SHIFT 13
> +#define HN_VLAN_CFI_MASK 0x1000 /* Canonical Format Indicator / Drop
> Eligible Indicator */
> +#define HN_VLAN_VID_MASK 0x0fff /* VLAN Identifier */
> +
> struct hn_rxinfo {
> - uint32_t vlan_info;
> + struct ndis_pkt_vlan_info vlan_info;
> uint32_t csum_info;
> uint32_t hash_info;
> uint32_t hash_value;
> @@ -477,7 +482,7 @@ hn_rndis_rxinfo(const void *info_data, unsigned int
> info_dlen,
> case NDIS_PKTINFO_TYPE_VLAN:
> if (unlikely(dlen < NDIS_VLAN_INFO_SIZE))
> return -EINVAL;
> - info->vlan_info = *((const uint32_t *)data);
> + info->vlan_info = *((const struct ndis_pkt_vlan_info
> *)data);
> mask |= HN_RXINFO_VLAN;
> break;
>
> @@ -611,8 +616,10 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct
> hn_rx_bufinfo *rxb,
> RTE_PTYPE_L3_MASK |
> RTE_PTYPE_L4_MASK);
>
> - if (info->vlan_info != HN_NDIS_VLAN_INFO_INVALID) {
> - m->vlan_tci = info->vlan_info;
> + if (info->vlan_info.value != HN_NDIS_VLAN_INFO_INVALID) {
> + m->vlan_tci = info->vlan_info.vlanid |
> + (info->vlan_info.pri << HN_VLAN_PRIO_SHIFT) |
> + (info->vlan_info.cfi ? HN_VLAN_CFI_MASK : 0);
> m->ol_flags |= RTE_MBUF_F_RX_VLAN_STRIPPED |
> RTE_MBUF_F_RX_VLAN;
>
> /* NDIS always strips tag, put it back if necessary */ @@ -669,7
> +676,7 @@ static void hn_rndis_rx_data(struct hn_rx_queue *rxq,
> unsigned int pktinfo_off, pktinfo_len;
> const struct rndis_packet_msg *pkt = data;
> struct hn_rxinfo info = {
> - .vlan_info = HN_NDIS_VLAN_INFO_INVALID,
> + .vlan_info.value = HN_NDIS_VLAN_INFO_INVALID,
> .csum_info = HN_NDIS_RXCSUM_INFO_INVALID,
> .hash_info = HN_NDIS_HASH_INFO_INVALID,
> };
> @@ -1332,7 +1339,11 @@ static void hn_encap(struct rndis_packet_msg *pkt,
> if (m->ol_flags & RTE_MBUF_F_TX_VLAN) {
> pi_data = hn_rndis_pktinfo_append(pkt, NDIS_VLAN_INFO_SIZE,
> NDIS_PKTINFO_TYPE_VLAN);
> - *pi_data = m->vlan_tci;
> + struct ndis_pkt_vlan_info *vlan = (struct ndis_pkt_vlan_info
> *)pi_data;
> + vlan->value = 0;
> + vlan->vlanid = (m->vlan_tci & HN_VLAN_VID_MASK);
> + vlan->cfi = (!!(m->vlan_tci & HN_VLAN_CFI_MASK));
> + vlan->pri = ((m->vlan_tci & HN_VLAN_PRIO_MASK) >>
> +HN_VLAN_PRIO_SHIFT);
Thanks Alan.
Can you remove the extra "()" as suggested by Stephen? The rest of the patch
looks good.
Please include maintainers of of dpdk-next-net tree: (from "MAINTAINERS")
Next-net Tree
M: Ferruh Yigit <ferruh.yi...@amd.com>
M: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Long