DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup
@ 2019-10-01 15:44 Andrew Rybchenko
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx " Andrew Rybchenko
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-01 15:44 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

Deferred start Rx queue is not supported by the driver.

Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 929aa4cbd..9c8c617f7 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -929,6 +929,11 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (rx_conf->rx_deferred_start) {
+		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
+		return -EINVAL;
+	}
+
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx queue setup
  2019-10-01 15:44 [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Andrew Rybchenko
@ 2019-10-01 15:44 ` Andrew Rybchenko
  2019-10-08  5:35   ` Tiwei Bie
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-01 15:44 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

Deferred start Tx queue is not supported by the driver.

Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 9c8c617f7..af3f350a8 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1057,6 +1057,11 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (tx_conf->tx_deferred_start) {
+		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
+		return -EINVAL;
+	}
+
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-01 15:44 [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Andrew Rybchenko
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx " Andrew Rybchenko
@ 2019-10-01 15:44 ` Andrew Rybchenko
  2019-10-08  5:46   ` Tiwei Bie
  2019-10-01 16:29 ` [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Kevin Traynor
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-01 15:44 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

This driver supports none of DCB, RSS or VMDQ modes,
therefore must check and return error if configured incorrectly.

Fixes: c1f86306a026 ("virtio: add new driver")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 7261109dd..0af4fc392 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_LOG(DEBUG, "configure");
 	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
 
+	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
+		PMD_DRV_LOG(ERR,
+			"Unsupported Rx multi queue mode %d",
+			rxmode->mq_mode);
+		return -EINVAL;
+	}
+
 	if (dev->data->dev_conf.intr_conf.rxq) {
 		ret = virtio_init_device(dev, hw->req_guest_features);
 		if (ret < 0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup
  2019-10-01 15:44 [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Andrew Rybchenko
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx " Andrew Rybchenko
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
@ 2019-10-01 16:29 ` Kevin Traynor
  2019-10-01 16:39   ` Andrew Rybchenko
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
  3 siblings, 1 reply; 26+ messages in thread
From: Kevin Traynor @ 2019-10-01 16:29 UTC (permalink / raw)
  To: Andrew Rybchenko, Maxime Coquelin, Tiwei Bie, Zhihong Wang
  Cc: dev, Dilshod Urazov, stable

Hi,

On 01/10/2019 16:44, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> Deferred start Rx queue is not supported by the driver.
> 
> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 929aa4cbd..9c8c617f7 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -929,6 +929,11 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  

const struct rte_eth_rxconf *rx_conf __rte_unused,
				     ^^^^^^^^^^^^
The unused can be removed now

>  	PMD_INIT_FUNC_TRACE();
>  
> +	if (rx_conf->rx_deferred_start) {
> +		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
> +		return -EINVAL;
> +	}
> +
>  	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
>  		nb_desc = vq->vq_nentries;
>  	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup
  2019-10-01 16:29 ` [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Kevin Traynor
@ 2019-10-01 16:39   ` Andrew Rybchenko
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-01 16:39 UTC (permalink / raw)
  To: Kevin Traynor, Maxime Coquelin, Tiwei Bie, Zhihong Wang
  Cc: dev, Dilshod Urazov, stable

Hi Kevin,

On 10/1/19 7:29 PM, Kevin Traynor wrote:
> Hi,
>
> On 01/10/2019 16:44, Andrew Rybchenko wrote:
>> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>
>> Deferred start Rx queue is not supported by the driver.
>>
>> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   drivers/net/virtio/virtio_rxtx.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>> index 929aa4cbd..9c8c617f7 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -929,6 +929,11 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>   
> const struct rte_eth_rxconf *rx_conf __rte_unused,
> 				     ^^^^^^^^^^^^
> The unused can be removed now

Thanks, will fix in v2.

>>   	PMD_INIT_FUNC_TRACE();
>>   
>> +	if (rx_conf->rx_deferred_start) {
>> +		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
>>   		nb_desc = vq->vq_nentries;
>>   	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx queue setup
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx " Andrew Rybchenko
@ 2019-10-08  5:35   ` Tiwei Bie
  0 siblings, 0 replies; 26+ messages in thread
From: Tiwei Bie @ 2019-10-08  5:35 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On Tue, Oct 01, 2019 at 04:44:30PM +0100, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> Deferred start Tx queue is not supported by the driver.
> 
> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-01 15:44 ` [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
@ 2019-10-08  5:46   ` Tiwei Bie
  2019-10-09  8:04     ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Tiwei Bie @ 2019-10-08  5:46 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> This driver supports none of DCB, RSS or VMDQ modes,
> therefore must check and return error if configured incorrectly.
> 
> Fixes: c1f86306a026 ("virtio: add new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 7261109dd..0af4fc392 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  	PMD_INIT_LOG(DEBUG, "configure");
>  	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
>  
> +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
> +		PMD_DRV_LOG(ERR,
> +			"Unsupported Rx multi queue mode %d",
> +			rxmode->mq_mode);
> +		return -EINVAL;
> +	}

We need similar checks for Tx as well.

Thanks,
Tiwei

> +
>  	if (dev->data->dev_conf.intr_conf.rxq) {
>  		ret = virtio_init_device(dev, hw->req_guest_features);
>  		if (ret < 0)
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-08  5:46   ` Tiwei Bie
@ 2019-10-09  8:04     ` Andrew Rybchenko
  2019-10-09  8:43       ` Tiwei Bie
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09  8:04 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On 10/8/19 8:46 AM, Tiwei Bie wrote:
> On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
>> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>
>> This driver supports none of DCB, RSS or VMDQ modes,
>> therefore must check and return error if configured incorrectly.
>>
>> Fixes: c1f86306a026 ("virtio: add new driver")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 7261109dd..0af4fc392 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>   	PMD_INIT_LOG(DEBUG, "configure");
>>   	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
>>   
>> +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
>> +		PMD_DRV_LOG(ERR,
>> +			"Unsupported Rx multi queue mode %d",
>> +			rxmode->mq_mode);
>> +		return -EINVAL;
>> +	}
> We need similar checks for Tx as well.

OK, I'll add.

However, I'm not 100% sure about RSS. Yes, I know that virtio has
no RSS configuration support, but it looks possible to have multi queue
in vhost-net case.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09  8:04     ` Andrew Rybchenko
@ 2019-10-09  8:43       ` Tiwei Bie
  2019-10-09  9:00         ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Tiwei Bie @ 2019-10-09  8:43 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On Wed, Oct 09, 2019 at 11:04:38AM +0300, Andrew Rybchenko wrote:
> On 10/8/19 8:46 AM, Tiwei Bie wrote:
> > On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
> > > From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> > > 
> > > This driver supports none of DCB, RSS or VMDQ modes,
> > > therefore must check and return error if configured incorrectly.
> > > 
> > > Fixes: c1f86306a026 ("virtio: add new driver")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> > > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > ---
> > >   drivers/net/virtio/virtio_ethdev.c | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > index 7261109dd..0af4fc392 100644
> > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> > >   	PMD_INIT_LOG(DEBUG, "configure");
> > >   	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
> > > +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
> > > +		PMD_DRV_LOG(ERR,
> > > +			"Unsupported Rx multi queue mode %d",
> > > +			rxmode->mq_mode);
> > > +		return -EINVAL;
> > > +	}
> > We need similar checks for Tx as well.
> 
> OK, I'll add.
> 
> However, I'm not 100% sure about RSS. Yes, I know that virtio has
> no RSS configuration support, but it looks possible to have multi queue
> in vhost-net case.

Yeah, it's possible to have MQ in virtio.
The RSS support in virtio is still WIP.
https://github.com/oasis-tcs/virtio-spec/issues/48


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09  8:43       ` Tiwei Bie
@ 2019-10-09  9:00         ` Andrew Rybchenko
  2019-10-09 10:41           ` Tiwei Bie
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09  9:00 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On 10/9/19 11:43 AM, Tiwei Bie wrote:
> On Wed, Oct 09, 2019 at 11:04:38AM +0300, Andrew Rybchenko wrote:
>> On 10/8/19 8:46 AM, Tiwei Bie wrote:
>>> On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
>>>> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>>>
>>>> This driver supports none of DCB, RSS or VMDQ modes,
>>>> therefore must check and return error if configured incorrectly.
>>>>
>>>> Fixes: c1f86306a026 ("virtio: add new driver")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> ---
>>>>    drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>>> index 7261109dd..0af4fc392 100644
>>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>>> @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>>>    	PMD_INIT_LOG(DEBUG, "configure");
>>>>    	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
>>>> +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
>>>> +		PMD_DRV_LOG(ERR,
>>>> +			"Unsupported Rx multi queue mode %d",
>>>> +			rxmode->mq_mode);
>>>> +		return -EINVAL;
>>>> +	}
>>> We need similar checks for Tx as well.
>> OK, I'll add.
>>
>> However, I'm not 100% sure about RSS. Yes, I know that virtio has
>> no RSS configuration support, but it looks possible to have multi queue
>> in vhost-net case.
> Yeah, it's possible to have MQ in virtio.
> The RSS support in virtio is still WIP.
> https://github.com/oasis-tcs/virtio-spec/issues/48

Thanks for the link. So, may be ETH_MQ_RX_RSS should be accepted
as well, but attempts to configure RSS rejected?
Yes, it is a bit strange to accept RSS with empty rss_hf etc, but
at least it is exactly what net/virtio can do.

And one more thought...
It looks like if more than one Rx queue is configured, mq_mode must
be ETH_MQ_RX_RSS and must not be ETH_MQ_RX_NONE.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09  9:00         ` Andrew Rybchenko
@ 2019-10-09 10:41           ` Tiwei Bie
  2019-10-09 11:24             ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Tiwei Bie @ 2019-10-09 10:41 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

On Wed, Oct 09, 2019 at 12:00:28PM +0300, Andrew Rybchenko wrote:
> On 10/9/19 11:43 AM, Tiwei Bie wrote:
> > On Wed, Oct 09, 2019 at 11:04:38AM +0300, Andrew Rybchenko wrote:
> > > On 10/8/19 8:46 AM, Tiwei Bie wrote:
> > > > On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
> > > > > From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> > > > > 
> > > > > This driver supports none of DCB, RSS or VMDQ modes,
> > > > > therefore must check and return error if configured incorrectly.
> > > > > 
> > > > > Fixes: c1f86306a026 ("virtio: add new driver")
> > > > > Cc: stable@dpdk.org
> > > > > 
> > > > > Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> > > > > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > > > ---
> > > > >    drivers/net/virtio/virtio_ethdev.c | 7 +++++++
> > > > >    1 file changed, 7 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > > > index 7261109dd..0af4fc392 100644
> > > > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > > > @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> > > > >    	PMD_INIT_LOG(DEBUG, "configure");
> > > > >    	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
> > > > > +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
> > > > > +		PMD_DRV_LOG(ERR,
> > > > > +			"Unsupported Rx multi queue mode %d",
> > > > > +			rxmode->mq_mode);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > We need similar checks for Tx as well.
> > > OK, I'll add.
> > > 
> > > However, I'm not 100% sure about RSS. Yes, I know that virtio has
> > > no RSS configuration support, but it looks possible to have multi queue
> > > in vhost-net case.
> > Yeah, it's possible to have MQ in virtio.
> > The RSS support in virtio is still WIP.
> > https://github.com/oasis-tcs/virtio-spec/issues/48
> 
> Thanks for the link. So, may be ETH_MQ_RX_RSS should be accepted
> as well, but attempts to configure RSS rejected?
> Yes, it is a bit strange to accept RSS with empty rss_hf etc, but
> at least it is exactly what net/virtio can do.
> 
> And one more thought...
> It looks like if more than one Rx queue is configured, mq_mode must
> be ETH_MQ_RX_RSS and must not be ETH_MQ_RX_NONE.

Some apps in DPDK will set mq_mode to ETH_MQ_RX_NONE while
enabling multiple queue pairs, e.g.:

https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L137
https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L181-L182
https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L3938-L3948

Testpmd will also set mq_mode to ETH_MQ_RX_NONE when multiple
Rx queues are enabled but rss_hf is empty:

https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2935-L2938
https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2945-L2948
(the flow_type_rss_offloads reported by virtio-PMD is zero)

My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
no method is enforced on how to route packets to MQs.
It looks that ETH_MQ_RX_NONE is the best fit for virtio currently.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09 10:41           ` Tiwei Bie
@ 2019-10-09 11:24             ` Andrew Rybchenko
  2019-10-10  7:42               ` Thomas Monjalon
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09 11:24 UTC (permalink / raw)
  To: Tiwei Bie, Ferruh Yigit, Thomas Monjalon
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov, stable

@Thomas, @Ferruh, please see small question below.

On 10/9/19 1:41 PM, Tiwei Bie wrote:
> On Wed, Oct 09, 2019 at 12:00:28PM +0300, Andrew Rybchenko wrote:
>> On 10/9/19 11:43 AM, Tiwei Bie wrote:
>>> On Wed, Oct 09, 2019 at 11:04:38AM +0300, Andrew Rybchenko wrote:
>>>> On 10/8/19 8:46 AM, Tiwei Bie wrote:
>>>>> On Tue, Oct 01, 2019 at 04:44:31PM +0100, Andrew Rybchenko wrote:
>>>>>> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>>>>>
>>>>>> This driver supports none of DCB, RSS or VMDQ modes,
>>>>>> therefore must check and return error if configured incorrectly.
>>>>>>
>>>>>> Fixes: c1f86306a026 ("virtio: add new driver")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>> ---
>>>>>>     drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>>>>>     1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>>>>> index 7261109dd..0af4fc392 100644
>>>>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>>>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>>>>> @@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>>>>>>     	PMD_INIT_LOG(DEBUG, "configure");
>>>>>>     	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
>>>>>> +	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
>>>>>> +		PMD_DRV_LOG(ERR,
>>>>>> +			"Unsupported Rx multi queue mode %d",
>>>>>> +			rxmode->mq_mode);
>>>>>> +		return -EINVAL;
>>>>>> +	}
>>>>> We need similar checks for Tx as well.
>>>> OK, I'll add.
>>>>
>>>> However, I'm not 100% sure about RSS. Yes, I know that virtio has
>>>> no RSS configuration support, but it looks possible to have multi queue
>>>> in vhost-net case.
>>> Yeah, it's possible to have MQ in virtio.
>>> The RSS support in virtio is still WIP.
>>> https://github.com/oasis-tcs/virtio-spec/issues/48
>> Thanks for the link. So, may be ETH_MQ_RX_RSS should be accepted
>> as well, but attempts to configure RSS rejected?
>> Yes, it is a bit strange to accept RSS with empty rss_hf etc, but
>> at least it is exactly what net/virtio can do.
>>
>> And one more thought...
>> It looks like if more than one Rx queue is configured, mq_mode must
>> be ETH_MQ_RX_RSS and must not be ETH_MQ_RX_NONE.
> Some apps in DPDK will set mq_mode to ETH_MQ_RX_NONE while
> enabling multiple queue pairs, e.g.:
>
> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L137
> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L181-L182
> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L3938-L3948
>
> Testpmd will also set mq_mode to ETH_MQ_RX_NONE when multiple
> Rx queues are enabled but rss_hf is empty:
>
> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2935-L2938
> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2945-L2948
> (the flow_type_rss_offloads reported by virtio-PMD is zero)
>
> My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
> no method is enforced on how to route packets to MQs.

I'm not sure. It is definitely a place to be improved in
ethdev documentation. Thomas, Ferruh, what do you think?
Is it really a definition of ETH_MQ_RX_NONE?

> It looks that ETH_MQ_RX_NONE is the best fit for virtio currently.

Anyway I agree and I think it is better to go this way in the patch.

Thanks,
Andrew.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup
  2019-10-01 15:44 [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Andrew Rybchenko
                   ` (2 preceding siblings ...)
  2019-10-01 16:29 ` [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Kevin Traynor
@ 2019-10-09 12:32 ` Andrew Rybchenko
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx " Andrew Rybchenko
                     ` (3 more replies)
  3 siblings, 4 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09 12:32 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

Deferred start Rx queue is not supported by the driver.

Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_rxtx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 929aa4cbd..a8cd80100 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -919,7 +919,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			uint16_t queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id __rte_unused,
-			const struct rte_eth_rxconf *rx_conf __rte_unused,
+			const struct rte_eth_rxconf *rx_conf,
 			struct rte_mempool *mp)
 {
 	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
@@ -929,6 +929,11 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (rx_conf->rx_deferred_start) {
+		PMD_INIT_LOG(ERR, "Rx deferred start is not supported");
+		return -EINVAL;
+	}
+
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx queue setup
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
@ 2019-10-09 12:32   ` Andrew Rybchenko
  2019-10-24  9:44     ` Maxime Coquelin
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09 12:32 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

Deferred start Tx queue is not supported by the driver.

Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/net/virtio/virtio_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index a8cd80100..f29e2154b 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1057,6 +1057,11 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (tx_conf->tx_deferred_start) {
+		PMD_INIT_LOG(ERR, "Tx deferred start is not supported");
+		return -EINVAL;
+	}
+
 	if (nb_desc == 0 || nb_desc > vq->vq_nentries)
 		nb_desc = vq->vq_nentries;
 	vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx " Andrew Rybchenko
@ 2019-10-09 12:32   ` Andrew Rybchenko
  2019-10-24  9:44     ` Maxime Coquelin
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx " Andrew Rybchenko
  2019-10-24  9:44   ` [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup Maxime Coquelin
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09 12:32 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable

From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>

This driver supports none of DCB, RSS or VMDQ modes, therefore must
check and return error if configured incorrectly.

Virtio can distribute Rx packets across multi-queue, but there is
no controls (algorithm, redirection table, hash function) except
number of Rx queues and ETH_MQ_RX_NONE is the best fit meaning
no method is enforced on how to route packets to MQs.

Fixes: c1f86306a026 ("virtio: add new driver")
Cc: stable@dpdk.org

Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 7261109dd..0af4fc392 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2071,6 +2071,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_LOG(DEBUG, "configure");
 	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
 
+	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
+		PMD_DRV_LOG(ERR,
+			"Unsupported Rx multi queue mode %d",
+			rxmode->mq_mode);
+		return -EINVAL;
+	}
+
 	if (dev->data->dev_conf.intr_conf.rxq) {
 		ret = virtio_init_device(dev, hw->req_guest_features);
 		if (ret < 0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx multi queue modes
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx " Andrew Rybchenko
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
@ 2019-10-09 12:32   ` Andrew Rybchenko
  2019-10-24  9:44     ` Maxime Coquelin
  2019-10-24  9:44   ` [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup Maxime Coquelin
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-09 12:32 UTC (permalink / raw)
  To: Maxime Coquelin, Tiwei Bie, Zhihong Wang; +Cc: dev, stable

This driver supports none of DCB or VMDQ modes, therefore must
check and return error if configured incorrectly.

Fixes: c1f86306a026 ("virtio: add new driver")
Cc: stable@dpdk.org

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 0af4fc392..7d3db4d73 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2078,6 +2078,13 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		return -EINVAL;
 	}
 
+	if (txmode->mq_mode != ETH_MQ_TX_NONE) {
+		PMD_DRV_LOG(ERR,
+			"Unsupported Tx multi queue mode %d",
+			txmode->mq_mode);
+		return -EINVAL;
+	}
+
 	if (dev->data->dev_conf.intr_conf.rxq) {
 		ret = virtio_init_device(dev, hw->req_guest_features);
 		if (ret < 0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09 11:24             ` Andrew Rybchenko
@ 2019-10-10  7:42               ` Thomas Monjalon
  2019-10-10  8:13                 ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Thomas Monjalon @ 2019-10-10  7:42 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Ferruh Yigit
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov

09/10/2019 13:24, Andrew Rybchenko:
> On 10/9/19 1:41 PM, Tiwei Bie wrote:
> > On Wed, Oct 09, 2019 at 12:00:28PM +0300, Andrew Rybchenko wrote:
> >> And one more thought...
> >> It looks like if more than one Rx queue is configured, mq_mode must
> >> be ETH_MQ_RX_RSS and must not be ETH_MQ_RX_NONE.
> > Some apps in DPDK will set mq_mode to ETH_MQ_RX_NONE while
> > enabling multiple queue pairs, e.g.:
> >
> > https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L137
> > https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L181-L182
> > https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L3938-L3948
> >
> > Testpmd will also set mq_mode to ETH_MQ_RX_NONE when multiple
> > Rx queues are enabled but rss_hf is empty:
> >
> > https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2935-L2938
> > https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2945-L2948
> > (the flow_type_rss_offloads reported by virtio-PMD is zero)
> >
> > My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
> > no method is enforced on how to route packets to MQs.
> 
> I'm not sure. It is definitely a place to be improved in
> ethdev documentation. Thomas, Ferruh, what do you think?
> Is it really a definition of ETH_MQ_RX_NONE?

I think it means everything go to queue 0.
The comment says no DCB, RSS or VMDQ.
It looks like the "NONE" value has been abused for some custom steering.
We have two options:
	- document NONE as a possible case of custom steering
	- add a new CUSTOM value




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-10  7:42               ` Thomas Monjalon
@ 2019-10-10  8:13                 ` Andrew Rybchenko
  2019-10-10  8:23                   ` David Marchand
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-10  8:13 UTC (permalink / raw)
  To: Thomas Monjalon, Tiwei Bie, Ferruh Yigit
  Cc: Maxime Coquelin, Zhihong Wang, dev, Dilshod Urazov

On 10/10/19 10:42 AM, Thomas Monjalon wrote:
> 09/10/2019 13:24, Andrew Rybchenko:
>> On 10/9/19 1:41 PM, Tiwei Bie wrote:
>>> On Wed, Oct 09, 2019 at 12:00:28PM +0300, Andrew Rybchenko wrote:
>>>> And one more thought...
>>>> It looks like if more than one Rx queue is configured, mq_mode must
>>>> be ETH_MQ_RX_RSS and must not be ETH_MQ_RX_NONE.
>>> Some apps in DPDK will set mq_mode to ETH_MQ_RX_NONE while
>>> enabling multiple queue pairs, e.g.:
>>>
>>> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L137
>>> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L181-L182
>>> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test/test_link_bonding.c#L3938-L3948
>>>
>>> Testpmd will also set mq_mode to ETH_MQ_RX_NONE when multiple
>>> Rx queues are enabled but rss_hf is empty:
>>>
>>> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2935-L2938
>>> https://github.com/DPDK/dpdk/blob/5b5bb56532fd5dea5d6260c0a0e405c0e32da2a0/app/test-pmd/testpmd.c#L2945-L2948
>>> (the flow_type_rss_offloads reported by virtio-PMD is zero)
>>>
>>> My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
>>> no method is enforced on how to route packets to MQs.
>> I'm not sure. It is definitely a place to be improved in
>> ethdev documentation. Thomas, Ferruh, what do you think?
>> Is it really a definition of ETH_MQ_RX_NONE?
> I think it means everything go to queue 0.

I understand it this way as well.

> The comment says no DCB, RSS or VMDQ.
> It looks like the "NONE" value has been abused for some custom steering.
> We have two options:
> 	- document NONE as a possible case of custom steering
> 	- add a new CUSTOM value

I'd prefer to say that ETH_MQ_RX_RSS with rss_hf equal to 0 means
unspecified/unknown steering. If application just want to spread
traffic across many Rx queues, it is natural choice to say that
it want RSS, but do not care about spreading algorithm etc.
It allows driver use recommended defaults if rss_hf is controllable,
or just spread in virtio case.

Yes, it means that testpmd should be fixed for this specific case
to be able to enable RSS with rss_hf equal to 0.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-10  8:13                 ` Andrew Rybchenko
@ 2019-10-10  8:23                   ` David Marchand
  2019-10-10  8:27                     ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: David Marchand @ 2019-10-10  8:23 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Thomas Monjalon, Tiwei Bie, Ferruh Yigit, Maxime Coquelin,
	Zhihong Wang, dev, Dilshod Urazov

On Thu, Oct 10, 2019 at 10:14 AM Andrew Rybchenko
<arybchenko@solarflare.com> wrote:
>
> On 10/10/19 10:42 AM, Thomas Monjalon wrote:
> > 09/10/2019 13:24, Andrew Rybchenko:
> >> On 10/9/19 1:41 PM, Tiwei Bie wrote:
> >>> My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
> >>> no method is enforced on how to route packets to MQs.
> >> I'm not sure. It is definitely a place to be improved in
> >> ethdev documentation. Thomas, Ferruh, what do you think?
> >> Is it really a definition of ETH_MQ_RX_NONE?
> > I think it means everything go to queue 0.
>
> I understand it this way as well.
>
> > The comment says no DCB, RSS or VMDQ.
> > It looks like the "NONE" value has been abused for some custom steering.
> > We have two options:
> >       - document NONE as a possible case of custom steering
> >       - add a new CUSTOM value
>
> I'd prefer to say that ETH_MQ_RX_RSS with rss_hf equal to 0 means
> unspecified/unknown steering. If application just want to spread
> traffic across many Rx queues, it is natural choice to say that
> it want RSS, but do not care about spreading algorithm etc.
> It allows driver use recommended defaults if rss_hf is controllable,
> or just spread in virtio case.

RSS is about maintaining affinity of a "flow" (as in packets sharing
the same l3/l4 tuples) to a specific queue.
Here, we can have packets from a same flow on any queue depending on
what happened on the vhost side.

I prefer we describe this behavior as something else than RSS.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-10  8:23                   ` David Marchand
@ 2019-10-10  8:27                     ` Andrew Rybchenko
  2019-10-10  9:10                       ` Tiwei Bie
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2019-10-10  8:27 UTC (permalink / raw)
  To: David Marchand
  Cc: Thomas Monjalon, Tiwei Bie, Ferruh Yigit, Maxime Coquelin,
	Zhihong Wang, dev, Dilshod Urazov

On 10/10/19 11:23 AM, David Marchand wrote:
> On Thu, Oct 10, 2019 at 10:14 AM Andrew Rybchenko
> <arybchenko@solarflare.com> wrote:
>> On 10/10/19 10:42 AM, Thomas Monjalon wrote:
>>> 09/10/2019 13:24, Andrew Rybchenko:
>>>> On 10/9/19 1:41 PM, Tiwei Bie wrote:
>>>>> My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
>>>>> no method is enforced on how to route packets to MQs.
>>>> I'm not sure. It is definitely a place to be improved in
>>>> ethdev documentation. Thomas, Ferruh, what do you think?
>>>> Is it really a definition of ETH_MQ_RX_NONE?
>>> I think it means everything go to queue 0.
>> I understand it this way as well.
>>
>>> The comment says no DCB, RSS or VMDQ.
>>> It looks like the "NONE" value has been abused for some custom steering.
>>> We have two options:
>>>        - document NONE as a possible case of custom steering
>>>        - add a new CUSTOM value
>> I'd prefer to say that ETH_MQ_RX_RSS with rss_hf equal to 0 means
>> unspecified/unknown steering. If application just want to spread
>> traffic across many Rx queues, it is natural choice to say that
>> it want RSS, but do not care about spreading algorithm etc.
>> It allows driver use recommended defaults if rss_hf is controllable,
>> or just spread in virtio case.
> RSS is about maintaining affinity of a "flow" (as in packets sharing
> the same l3/l4 tuples) to a specific queue.
> Here, we can have packets from a same flow on any queue depending on
> what happened on the vhost side.

Interesting. I'd like to know a bit more about it. I didn't know that
it is so unstable. Could someone who knows the topic well add
a bit more information about it.

> I prefer we describe this behavior as something else than RSS.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes
  2019-10-10  8:27                     ` Andrew Rybchenko
@ 2019-10-10  9:10                       ` Tiwei Bie
  0 siblings, 0 replies; 26+ messages in thread
From: Tiwei Bie @ 2019-10-10  9:10 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: David Marchand, Thomas Monjalon, Ferruh Yigit, Maxime Coquelin,
	Zhihong Wang, dev, Dilshod Urazov

On Thu, Oct 10, 2019 at 11:27:53AM +0300, Andrew Rybchenko wrote:
> On 10/10/19 11:23 AM, David Marchand wrote:
> > On Thu, Oct 10, 2019 at 10:14 AM Andrew Rybchenko
> > <arybchenko@solarflare.com> wrote:
> > > On 10/10/19 10:42 AM, Thomas Monjalon wrote:
> > > > 09/10/2019 13:24, Andrew Rybchenko:
> > > > > On 10/9/19 1:41 PM, Tiwei Bie wrote:
> > > > > > My understanding is that, setting mq_mode to ETH_MQ_RX_NONE means
> > > > > > no method is enforced on how to route packets to MQs.
> > > > > I'm not sure. It is definitely a place to be improved in
> > > > > ethdev documentation. Thomas, Ferruh, what do you think?
> > > > > Is it really a definition of ETH_MQ_RX_NONE?
> > > > I think it means everything go to queue 0.
> > > I understand it this way as well.
> > > 
> > > > The comment says no DCB, RSS or VMDQ.
> > > > It looks like the "NONE" value has been abused for some custom steering.
> > > > We have two options:
> > > >        - document NONE as a possible case of custom steering
> > > >        - add a new CUSTOM value
> > > I'd prefer to say that ETH_MQ_RX_RSS with rss_hf equal to 0 means
> > > unspecified/unknown steering. If application just want to spread
> > > traffic across many Rx queues, it is natural choice to say that
> > > it want RSS, but do not care about spreading algorithm etc.
> > > It allows driver use recommended defaults if rss_hf is controllable,
> > > or just spread in virtio case.
> > RSS is about maintaining affinity of a "flow" (as in packets sharing
> > the same l3/l4 tuples) to a specific queue.
> > Here, we can have packets from a same flow on any queue depending on
> > what happened on the vhost side.
> 
> Interesting. I'd like to know a bit more about it. I didn't know that
> it is so unstable. Could someone who knows the topic well add
> a bit more information about it.

The spec defined automatic steering [1], but it's not very strict
for the device. So it depends on the implementation.

[1] https://github.com/oasis-tcs/virtio-spec/commit/f7020384521e5938cdd1351270b32b143f070d00


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup
  2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
                     ` (2 preceding siblings ...)
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx " Andrew Rybchenko
@ 2019-10-24  9:44   ` Maxime Coquelin
  2019-10-24 10:19     ` Maxime Coquelin
  3 siblings, 1 reply; 26+ messages in thread
From: Maxime Coquelin @ 2019-10-24  9:44 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable



On 10/9/19 2:32 PM, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> Deferred start Rx queue is not supported by the driver.
> 
> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx queue setup
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx " Andrew Rybchenko
@ 2019-10-24  9:44     ` Maxime Coquelin
  0 siblings, 0 replies; 26+ messages in thread
From: Maxime Coquelin @ 2019-10-24  9:44 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable



On 10/9/19 2:32 PM, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> Deferred start Tx queue is not supported by the driver.
> 
> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>  drivers/net/virtio/virtio_rxtx.c | 5 +++++
>  1 file changed, 5 insertions(+)


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
@ 2019-10-24  9:44     ` Maxime Coquelin
  0 siblings, 0 replies; 26+ messages in thread
From: Maxime Coquelin @ 2019-10-24  9:44 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable



On 10/9/19 2:32 PM, Andrew Rybchenko wrote:
> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> 
> This driver supports none of DCB, RSS or VMDQ modes, therefore must
> check and return error if configured incorrectly.
> 
> Virtio can distribute Rx packets across multi-queue, but there is
> no controls (algorithm, redirection table, hash function) except
> number of Rx queues and ETH_MQ_RX_NONE is the best fit meaning
> no method is enforced on how to route packets to MQs.
> 
> Fixes: c1f86306a026 ("virtio: add new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx multi queue modes
  2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx " Andrew Rybchenko
@ 2019-10-24  9:44     ` Maxime Coquelin
  0 siblings, 0 replies; 26+ messages in thread
From: Maxime Coquelin @ 2019-10-24  9:44 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Zhihong Wang; +Cc: dev, stable



On 10/9/19 2:32 PM, Andrew Rybchenko wrote:
> This driver supports none of DCB or VMDQ modes, therefore must
> check and return error if configured incorrectly.
> 
> Fixes: c1f86306a026 ("virtio: add new driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup
  2019-10-24  9:44   ` [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup Maxime Coquelin
@ 2019-10-24 10:19     ` Maxime Coquelin
  0 siblings, 0 replies; 26+ messages in thread
From: Maxime Coquelin @ 2019-10-24 10:19 UTC (permalink / raw)
  To: Andrew Rybchenko, Tiwei Bie, Zhihong Wang; +Cc: dev, Dilshod Urazov, stable



On 10/24/19 11:44 AM, Maxime Coquelin wrote:
> 
> 
> On 10/9/19 2:32 PM, Andrew Rybchenko wrote:
>> From: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>>
>> Deferred start Rx queue is not supported by the driver.
>>
>> Fixes: 0748be2cf9a2 ("ethdev: queue start and stop")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dilshod Urazov <Dilshod.Urazov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  drivers/net/virtio/virtio_rxtx.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime
> 


Series applied to dpdk-next-virtio/master.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2019-10-24 10:19 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-01 15:44 [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Andrew Rybchenko
2019-10-01 15:44 ` [dpdk-dev] [PATCH 2/3] net/virtio: reject deferred start Tx " Andrew Rybchenko
2019-10-08  5:35   ` Tiwei Bie
2019-10-01 15:44 ` [dpdk-dev] [PATCH 3/3] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
2019-10-08  5:46   ` Tiwei Bie
2019-10-09  8:04     ` Andrew Rybchenko
2019-10-09  8:43       ` Tiwei Bie
2019-10-09  9:00         ` Andrew Rybchenko
2019-10-09 10:41           ` Tiwei Bie
2019-10-09 11:24             ` Andrew Rybchenko
2019-10-10  7:42               ` Thomas Monjalon
2019-10-10  8:13                 ` Andrew Rybchenko
2019-10-10  8:23                   ` David Marchand
2019-10-10  8:27                     ` Andrew Rybchenko
2019-10-10  9:10                       ` Tiwei Bie
2019-10-01 16:29 ` [dpdk-dev] [PATCH 1/3] net/virtio: reject deferred start Rx queue setup Kevin Traynor
2019-10-01 16:39   ` Andrew Rybchenko
2019-10-09 12:32 ` [dpdk-dev] [PATCH v2 1/4] " Andrew Rybchenko
2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 2/4] net/virtio: reject deferred start Tx " Andrew Rybchenko
2019-10-24  9:44     ` Maxime Coquelin
2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 3/4] net/virtio: reject unsupported Rx multi queue modes Andrew Rybchenko
2019-10-24  9:44     ` Maxime Coquelin
2019-10-09 12:32   ` [dpdk-dev] [PATCH v2 4/4] net/virtio: reject unsupported Tx " Andrew Rybchenko
2019-10-24  9:44     ` Maxime Coquelin
2019-10-24  9:44   ` [dpdk-dev] [PATCH v2 1/4] net/virtio: reject deferred start Rx queue setup Maxime Coquelin
2019-10-24 10:19     ` Maxime Coquelin

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