From: Long Li <longli@microsoft.com>
To: Alan Elder <alan.elder@microsoft.com>,
Ferruh Yigit <ferruh.yigit@amd.com>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: "dev@dpdk.org" <dev@dpdk.org>, stephen <stephen@networkplumber.org>
Subject: RE: [PATCH v3] net/netvsc: fix parsing of VLAN metadata
Date: Fri, 9 Feb 2024 01:18:41 +0000 [thread overview]
Message-ID: <PH7PR21MB3263EFEA76459C959F2EA954CE4B2@PH7PR21MB3263.namprd21.prod.outlook.com> (raw)
In-Reply-To: <PA4PR83MB0526CCCB791C5510CE053A9497442@PA4PR83MB0526.EURPRD83.prod.outlook.com>
> -----Original Message-----
> From: Alan Elder <alan.elder@microsoft.com>
> Sent: Thursday, February 8, 2024 7:09 AM
> To: Long Li <longli@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: sthemmin@microsoft.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Alan Elder <alan.elder@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 <xerebz@gmail.com> Alan Brady
> <alan.brady@intel.com> Alan Carew <alan.carew@intel.com> Alan Dewar
> <alan.dewar@att.com> <adewar@brocade.com>
> +Alan Elder <alan.elder@microsoft.com>
> Alan Liu <zaoxingliu@gmail.com>
> Alan Winkowski <walan@marvell.com>
> Alejandro Lucero <alejandro.lucero@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.yigit@amd.com>
M: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Long
next prev parent reply other threads:[~2024-02-09 1:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-08 15:09 Alan Elder
2024-02-09 1:18 ` Long Li [this message]
2024-02-09 15:50 ` [PATCH v4] " Alan Elder
2024-02-14 22:17 ` Long Li
2024-02-15 11:46 ` Ferruh Yigit
2024-02-15 18:12 ` [PATCH v5] " Alan Elder
2024-02-15 18:25 ` Stephen Hemminger
2024-02-16 9:43 ` [PATCH v6] " Alan Elder
2024-02-16 11:39 ` Ferruh Yigit
2024-02-19 9:31 ` [PATCH v7 0/2] " Alan Elder
2024-02-19 9:31 ` [PATCH v7 1/2] lib/net: " Alan Elder
2024-02-19 11:12 ` Ferruh Yigit
2024-02-19 11:14 ` Ferruh Yigit
2024-02-19 9:31 ` [PATCH v7 2/2] net/netvsc: " Alan Elder
2024-02-19 11:12 ` Ferruh Yigit
2024-02-19 9:34 ` [EXTERNAL] Re: [PATCH v6] " Alan Elder
2024-02-16 11:33 ` [PATCH v5] " Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PH7PR21MB3263EFEA76459C959F2EA954CE4B2@PH7PR21MB3263.namprd21.prod.outlook.com \
--to=longli@microsoft.com \
--cc=alan.elder@microsoft.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).