[dpdk-dev] [PATCH 0/3] net/netvsc: misc fixes

[dpdk-dev] [PATCH 0/3] net/netvsc: misc fixes

[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

 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(-)

-- 
2.19.2

[dpdk-dev] [PATCH 1/3] net/netvsc: support receive without vlan strip

[Stephen Hemminger]

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

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 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(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index b330bf3d7255..d7ac0c7320b0 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -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) {
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index f4a36641b6c3..622a83983e10 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -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) {
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index e1072c7cfa55..cd173f6af942 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -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;
 
-- 
2.19.2

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

[Ferruh Yigit]

On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> In some cases, VLAN stripping is not desireable. If necessary
> re-insert stripped vlan tag.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>

<...>

> @@ -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?

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

[Stephen Hemminger]

On Fri, 14 Dec 2018 16:09:33 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> > In some cases, VLAN stripping is not desireable. If necessary
> > re-insert stripped vlan tag.
> > 
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>  
> 
> <...>
> 
> > @@ -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()

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

[Ferruh Yigit]

On 12/14/2018 4:28 PM, Stephen Hemminger wrote:
> On Fri, 14 Dec 2018 16:09:33 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
>>> In some cases, VLAN stripping is not desireable. If necessary
>>> re-insert stripped vlan tag.
>>>
>>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>  
>>
>> <...>
>>
>>> @@ -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()

Yes it does, so it is OK.

[dpdk-dev] [PATCH 2/3] net/netvsc: cleanup transmit descriptor pool

[Stephen Hemminger]

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

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 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(+)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index d7ac0c7320b0..de872212d3f3 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -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);
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 622a83983e10..6197118b01ee 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -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;
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index cd173f6af942..7f3266c451fb 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -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,
-- 
2.19.2

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

[Ferruh Yigit]

On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> 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?

> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>

<...>

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

[Ferruh Yigit]

On 12/17/2018 10:46 AM, Ferruh Yigit wrote:
> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
>> 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: stable@dpdk.org

> 
>>
>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> 
> <...>
> 

[dpdk-dev] [PATCH 3/3] net/netvsc: not finding VF should not cause failure

[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 <sthemmin@microsoft.com>
---
 drivers/net/netvsc/hn_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index de872212d3f3..1f7a7e66a51b 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -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;
-- 
2.19.2

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

[Ferruh Yigit]

On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> 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: stable@dpdk.org line

> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  drivers/net/netvsc/hn_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index de872212d3f3..1f7a7e66a51b 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -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;
> 

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

[Ferruh Yigit]

On 12/17/2018 7:59 PM, Stephen Hemminger wrote:
> 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.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> Sent: Monday, December 17, 2018 2:49 AM
> To: Stephen Hemminger <stephen@networkplumber.org>; dev@dpdk.org
> Cc: Stephen Hemminger <sthemmin@microsoft.com>; Kevin Traynor <ktraynor@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH 3/3] net/netvsc: not finding VF should not cause failure
> 
> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
>> 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: stable@dpdk.org line
> 
>>
>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>> ---
>>  drivers/net/netvsc/hn_ethdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
>> index de872212d3f3..1f7a7e66a51b 100644
>> --- a/drivers/net/netvsc/hn_ethdev.c
>> +++ b/drivers/net/netvsc/hn_ethdev.c
>> @@ -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;
>>
> 

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

[Ferruh Yigit]

On 12/17/2018 10:11 PM, Ferruh Yigit wrote:
> On 12/17/2018 7:59 PM, Stephen Hemminger wrote:
>> 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: stable@dpdk.org

> 
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com> 
>> Sent: Monday, December 17, 2018 2:49 AM
>> To: Stephen Hemminger <stephen@networkplumber.org>; dev@dpdk.org
>> Cc: Stephen Hemminger <sthemmin@microsoft.com>; Kevin Traynor <ktraynor@redhat.com>
>> Subject: Re: [dpdk-dev] [PATCH 3/3] net/netvsc: not finding VF should not cause failure
>>
>> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
>>> 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: stable@dpdk.org line
>>
>>>
>>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>>> ---
>>>  drivers/net/netvsc/hn_ethdev.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
>>> index de872212d3f3..1f7a7e66a51b 100644
>>> --- a/drivers/net/netvsc/hn_ethdev.c
>>> +++ b/drivers/net/netvsc/hn_ethdev.c
>>> @@ -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;
>>>
>>
> 

Re: [dpdk-dev] [PATCH 0/3] net/netvsc: misc fixes

[Ferruh Yigit]

On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> 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)

Re: [dpdk-dev] [PATCH 0/3] net/netvsc: misc fixes

[Stephen Hemminger]

On Mon, 17 Dec 2018 22:13:19 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 12/14/2018 1:26 AM, Stephen Hemminger wrote:
> > 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)

Thanks.