Skip to contentSkip to conversation

net/netvsc: misc fixes

PATCH In review Stephen Hemminger 2 participants 14 messages Dec 14, 2018 – Dec 18, 2018 1 message not in the archive Network drivers

lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover net/netvsc: misc fixes

Minor updates to netvsc driver.

Stephen Hemminger (3):
net/netvsc: support receive without vlan strip net/netvsc: cleanup transmit descriptor pool net/netvsc: not finding VF should not cause failure

 drivers/net/netvsc/hn_ethdev.c |  6 +++++-
 drivers/net/netvsc/hn_rxtx.c   | 19 +++++++++++++++++++
 drivers/net/netvsc/hn_var.h    |  6 ++++--
 3 files changed, 28 insertions(+), 3 deletions(-)
Signature · 1 line
2.19.2
author
#1 lore raw

PATCH 1/3 net/netvsc: support receive without vlan strip

In some cases, VLAN stripping is not desireable. If necessary re-insert stripped vlan tag.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c | 2 ++
 drivers/net/netvsc/hn_rxtx.c   | 8 ++++++++
 drivers/net/netvsc/hn_var.h    | 5 +++--
 3 files changed, 13 insertions(+), 2 deletions(-)
drivers/net/netvsc/hn_ethdev.c +2 -0
index b330bf3d7255..d7ac0c7320b0 100644
@@ -378,6 +378,8 @@ static int hn_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
+
 	err = hn_rndis_conf_offload(hv, txmode->offloads,
 				    rxmode->offloads);
 	if (err) {
drivers/net/netvsc/hn_rxtx.c +8 -0
index f4a36641b6c3..622a83983e10 100644
@@ -501,6 +501,14 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	if (info->vlan_info != HN_NDIS_VLAN_INFO_INVALID) {
 		m->vlan_tci = info->vlan_info;
 		m->ol_flags |= PKT_RX_VLAN_STRIPPED | PKT_RX_VLAN;
+
+		/* NDIS always strips tag, put it back if necessary */
+		if (!hv->vlan_strip && rte_vlan_insert(&m)) {
+			PMD_DRV_LOG(DEBUG, "vlan insert failed");
+			++rxq->stats.errors;
+			rte_pktmbuf_free(m);
+			return;
+		}
 	}
 
 	if (info->csum_info != HN_NDIS_RXCSUM_INFO_INVALID) {
drivers/net/netvsc/hn_var.h +3 -2
index e1072c7cfa55..cd173f6af942 100644
@@ -97,8 +97,9 @@ struct hn_data {
 	struct rte_eth_dev *vf_dev;		/* Subordinate device */
 	rte_spinlock_t  vf_lock;
 	uint16_t	port_id;
-	bool		closed;
-	bool		vf_present;
+	uint8_t		closed;
+	uint8_t		vf_present;
+	uint8_t		vlan_strip;
 	uint32_t	link_status;
 	uint32_t	link_speed;
Signature · 1 line
2.19.2
#2 lore raw

Re: [PATCH 1/3] net/netvsc: support receive without vlan strip

Quoted 4 lines · Stephen Hemminger

In some cases, VLAN stripping is not desireable. If necessary re-insert stripped vlan tag.

Signed-off-by Stephen Hemminger <[email protected]>

<...>

drivers/net/netvsc/hn_rxtx.c static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, · line 506

Quoted 7 lines
drivers/net/netvsc/hn_rxtx.c +3 -0
@@ -501,6 +501,14 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	if (info->vlan_info != HN_NDIS_VLAN_INFO_INVALID) {
 		m->vlan_tci = info->vlan_info;
 		m->ol_flags |= PKT_RX_VLAN_STRIPPED | PKT_RX_VLAN;
+
+		/* NDIS always strips tag, put it back if necessary */
+		if (!hv->vlan_strip && rte_vlan_insert(&m)) {

Should 'PKT_RX_VLAN_STRIPPED' flag removed when vlan inserted back?

author
#3 lore raw

Re: [PATCH 1/3] net/netvsc: support receive without vlan strip

Quoted 8 lines · On Fri, 14 Dec 2018 16:09:33 +0000 Ferruh Yigit <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger

In some cases, VLAN stripping is not desireable. If necessary re-insert stripped vlan tag.

Signed-off-by Stephen Hemminger <[email protected]>

<...>

Quoted 9 lines

drivers/net/netvsc/hn_rxtx.c static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, · line 506

Quoted 7 lines
drivers/net/netvsc/hn_rxtx.c +3 -0
@@ -501,6 +501,14 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	if (info->vlan_info != HN_NDIS_VLAN_INFO_INVALID) {
 		m->vlan_tci = info->vlan_info;
 		m->ol_flags |= PKT_RX_VLAN_STRIPPED | PKT_RX_VLAN;
+
+		/* NDIS always strips tag, put it back if necessary */
+		if (!hv->vlan_strip && rte_vlan_insert(&m)) {

Should 'PKT_RX_VLAN_STRIPPED' flag removed when vlan inserted back?

This is already done by rte_vlan_insert()

#4 lore raw

Re: [PATCH 1/3] net/netvsc: support receive without vlan strip

Quoted 21 lines · Stephen Hemminger
Quoted 8 lines · On Fri, 14 Dec 2018 16:09:33 +0000 Ferruh Yigit <[email protected]> wrote:
Quoted 4 lines · Stephen Hemminger

In some cases, VLAN stripping is not desireable. If necessary re-insert stripped vlan tag.

Signed-off-by Stephen Hemminger <[email protected]>

<...>

Quoted 9 lines

drivers/net/netvsc/hn_rxtx.c static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb, · line 506

Quoted 7 lines
drivers/net/netvsc/hn_rxtx.c +3 -0
@@ -501,6 +501,14 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
 	if (info->vlan_info != HN_NDIS_VLAN_INFO_INVALID) {
 		m->vlan_tci = info->vlan_info;
 		m->ol_flags |= PKT_RX_VLAN_STRIPPED | PKT_RX_VLAN;
+
+		/* NDIS always strips tag, put it back if necessary */
+		if (!hv->vlan_strip && rte_vlan_insert(&m)) {

Should 'PKT_RX_VLAN_STRIPPED' flag removed when vlan inserted back?

Quoted 1 line

This is already done by rte_vlan_insert()

Yes it does, so it is OK.

author
#5 lore raw

PATCH 2/3 net/netvsc: cleanup transmit descriptor pool

In reply to Stephen Hemminger

On device close or startup errors, the transmit descriptor pool was being left behind.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c |  2 ++
 drivers/net/netvsc/hn_rxtx.c   | 11 +++++++++++
 drivers/net/netvsc/hn_var.h    |  1 +
 3 files changed, 14 insertions(+)
drivers/net/netvsc/hn_ethdev.c +2 -0
index d7ac0c7320b0..de872212d3f3 100644
@@ -796,6 +796,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 failed:
 	PMD_INIT_LOG(NOTICE, "device init failed");
 
+	hn_tx_pool_uninit(eth_dev);
 	hn_detach(hv);
 	return err;
 }

@@ -818,6 +819,7 @@ eth_hn_dev_uninit(struct rte_eth_dev *eth_dev)
 	eth_dev->rx_pkt_burst = NULL;
 
 	hn_detach(hv);
+	hn_tx_pool_uninit(eth_dev);
 	rte_vmbus_chan_close(hv->primary->chan);
 	rte_free(hv->primary);
 	rte_eth_dev_owner_delete(hv->owner.id);
drivers/net/netvsc/hn_rxtx.c +11 -0
index 622a83983e10..6197118b01ee 100644
@@ -199,6 +199,17 @@ hn_tx_pool_init(struct rte_eth_dev *dev)
 	return 0;
 }
 
+void
+hn_tx_pool_uninit(struct rte_eth_dev *dev)
+{
+	struct hn_data *hv = dev->data->dev_private;
+
+	if (hv->tx_pool) {
+		rte_mempool_free(hv->tx_pool);
+		hv->tx_pool = NULL;
+	}
+}
+
 static void hn_reset_txagg(struct hn_tx_queue *txq)
 {
 	txq->agg_szleft = txq->agg_szmax;
drivers/net/netvsc/hn_var.h +1 -0
index cd173f6af942..7f3266c451fb 100644
@@ -150,6 +150,7 @@ uint16_t hn_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		      uint16_t nb_pkts);
 
 int	hn_tx_pool_init(struct rte_eth_dev *dev);
+void	hn_tx_pool_uninit(struct rte_eth_dev *dev);
 int	hn_dev_link_update(struct rte_eth_dev *dev, int wait);
 int	hn_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 			      uint16_t nb_desc, unsigned int socket_id,
Signature · 1 line
2.19.2
#6 lore raw

Re: [PATCH 2/3] net/netvsc: cleanup transmit descriptor pool

Quoted 2 lines · Stephen Hemminger

On device close or startup errors, the transmit descriptor pool was being left behind.

So this is fixing possible memory leak, should this be an "fix" commit, so that it can be backported to stable trees?

Quoted 2 lines
Signed-off-by Stephen Hemminger <[email protected]>

<...>

#7 lore raw

Re: [PATCH 2/3] net/netvsc: cleanup transmit descriptor pool

Quoted 6 lines · Ferruh Yigit
Quoted 2 lines · Stephen Hemminger

On device close or startup errors, the transmit descriptor pool was being left behind.

So this is fixing possible memory leak, should this be an "fix" commit, so that it can be backported to stable trees?

Added:

    Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
    Cc: [email protected]
Quoted 6 lines
Quoted 3 lines
Signed-off-by Stephen Hemminger <[email protected]>

<...>

author
#8 lore raw

PATCH 3/3 net/netvsc: not finding VF should not cause failure

In reply to Stephen Hemminger

It is possible that the VF device exists but DPDK doesn't know about it. This could happen if device was blacklisted or more likely the necessary device (Mellanox) was not part of the DPDK configuration.

In either case, the right thing to do is just keep working but only with the slower para-virtual device.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/netvsc/hn_ethdev.c +1 -1
index de872212d3f3..1f7a7e66a51b 100644
@@ -788,7 +788,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 
 		err = hn_vf_add(eth_dev, hv);
 		if (err)
-			goto failed;
+			hv->vf_present = 0;
 	}
 
 	return 0;
Signature · 1 line
2.19.2
#9 lore raw

Re: [PATCH 3/3] net/netvsc: not finding VF should not cause failure

Quoted 7 lines · Stephen Hemminger

It is possible that the VF device exists but DPDK doesn't know about it. This could happen if device was blacklisted or more likely the necessary device (Mellanox) was not part of the DPDK configuration.

In either case, the right thing to do is just keep working but only with the slower para-virtual device.

Same question for this one, is this something that should be backported? Is it intentionally left out from backporting?

Just a reminder, for backport, a patch needs a few markers,
- fix patch with fixes line
- Cc: [email protected] line

Quoted 20 lines
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/netvsc/hn_ethdev.c +1 -1
index de872212d3f3..1f7a7e66a51b 100644
@@ -788,7 +788,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 
 		err = hn_vf_add(eth_dev, hv);
 		if (err)
-			goto failed;
+			hv->vf_present = 0;
 	}
 
 	return 0;

A message that is not in the archive (sent off-list or not yet received): DM5PR21MB0698C89DE76C533F41190931CCBC0@DM5PR21MB0698.namprd21.prod.outlook.com

#11 lore raw

Re: [PATCH 3/3] net/netvsc: not finding VF should not cause failure

Quoted 1 line · Stephen Hemminger

This can go to 18.11 stable

I will add fixes and stable tags while merging, please provide them with commit to ensure the backport of the patches to the stable trees.

Quoted 45 lines

-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Monday, December 17, 2018 2:49 AM
To: Stephen Hemminger <[email protected]>; [email protected]
Cc: Stephen Hemminger <[email protected]>; Kevin Traynor <[email protected]>
Subject: Re: [dpdk-dev] [PATCH 3/3] net/netvsc: not finding VF should not cause failure

Quoted 7 lines · Stephen Hemminger

It is possible that the VF device exists but DPDK doesn't know about it. This could happen if device was blacklisted or more likely the necessary device (Mellanox) was not part of the DPDK configuration.

In either case, the right thing to do is just keep working but only with the slower para-virtual device.

Same question for this one, is this something that should be backported? Is it intentionally left out from backporting?

Just a reminder, for backport, a patch needs a few markers,
- fix patch with fixes line
- Cc: [email protected] line

Quoted 20 lines
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/netvsc/hn_ethdev.c +1 -1
index de872212d3f3..1f7a7e66a51b 100644
@@ -788,7 +788,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 
 		err = hn_vf_add(eth_dev, hv);
 		if (err)
-			goto failed;
+			hv->vf_present = 0;
 	}
 
 	return 0;
#12 lore raw

Re: [PATCH 3/3] net/netvsc: not finding VF should not cause failure

Quoted 5 lines · Ferruh Yigit
Quoted 1 line · Stephen Hemminger

This can go to 18.11 stable

I will add fixes and stable tags while merging, please provide them with commit to ensure the backport of the patches to the stable trees.

Added:

    Fixes: dc7680e8597c ("net/netvsc: support integrated VF")
    Cc: [email protected]
Quoted 47 lines
Quoted 46 lines

-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Monday, December 17, 2018 2:49 AM
To: Stephen Hemminger <[email protected]>; [email protected]
Cc: Stephen Hemminger <[email protected]>; Kevin Traynor <[email protected]>
Subject: Re: [dpdk-dev] [PATCH 3/3] net/netvsc: not finding VF should not cause failure

Quoted 7 lines · Stephen Hemminger

It is possible that the VF device exists but DPDK doesn't know about it. This could happen if device was blacklisted or more likely the necessary device (Mellanox) was not part of the DPDK configuration.

In either case, the right thing to do is just keep working but only with the slower para-virtual device.

Same question for this one, is this something that should be backported? Is it intentionally left out from backporting?

Just a reminder, for backport, a patch needs a few markers,
- fix patch with fixes line
- Cc: [email protected] line

Quoted 20 lines
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/netvsc/hn_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/netvsc/hn_ethdev.c +1 -1
index de872212d3f3..1f7a7e66a51b 100644
@@ -788,7 +788,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
 
 		err = hn_vf_add(eth_dev, hv);
 		if (err)
-			goto failed;
+			hv->vf_present = 0;
 	}
 
 	return 0;
#13 lore raw
In reply to Stephen Hemminger
Quoted 6 lines · Stephen Hemminger

Minor updates to netvsc driver.

Stephen Hemminger (3):
net/netvsc: support receive without vlan strip net/netvsc: cleanup transmit descriptor pool net/netvsc: not finding VF should not cause failure

Series applied to dpdk-next-net/master, thanks.

(Commit titles converted to fix and Fixes line & stable tag added, please confirm them at next-net)

author
#14 lore raw
Quoted 8 lines · On Mon, 17 Dec 2018 22:13:19 +0000 Ferruh Yigit <[email protected]> wrote:
Quoted 6 lines · Stephen Hemminger

Minor updates to netvsc driver.

Stephen Hemminger (3):
net/netvsc: support receive without vlan strip net/netvsc: cleanup transmit descriptor pool net/netvsc: not finding VF should not cause failure

Quoted 4 lines

Series applied to dpdk-next-net/master, thanks.

(Commit titles converted to fix and Fixes line & stable tag added, please confirm them at next-net)

Thanks.