Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

Hi Daria,

please see my comments below and answer on them or apply fix and send
2nd version of the patch. You can do that by adding -v2 flag to git
format-patch command.

Please also sent the new version in response to this  email. You can
do that by adding --in-reply-to 'msgid' to git send-email. The message
ID can be read from the raw version of the email.

Thanks,
Michal

2018-06-25 15:40 GMT+02:00 Daria Kolistratova <daria.kolistratova@intel.com>:
> When  he number of rx queues is 0
Please fix the typo (' he' -> 'the').
Please also add information that it happens when the application is
also requesting ETH_MQ_RX_RSS_FLAG in the
rte_dev->data->dev_conf.rxmode.mq_mode.

> (what can be when application does not receive)
> failed with SIGFPE.
> Fixed adding zero check before division.
>
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 drivers/net/ena/ena_ethdev.c
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> old mode 100644
> new mode 100755
> index 9ae73e331..89004c903
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>         }
>
>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
> -               val = i % nb_rx_queues;
> +               if (nb_rx_queues != 0)
> +                       val = i % nb_rx_queues;
> +               else
> +                       val = 0;
> +
This change is not needed if you are adding the below change. This
function should not be called if the nb_rx_queues == 0, so there is no
need to perform additional check.

>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>                                                        ENA_IO_RXQ_IDX(val));
>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
> @@ -1052,7 +1056,7 @@ static int ena_start(struct rte_eth_dev *dev)
>                 return rc;
>
>         if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
> -           ETH_MQ_RX_RSS_FLAG) {
> +           ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
>                 rc = ena_rss_init_default(adapter);
>                 if (rc)
>                         return rc;
> --
> 2.14.4
>

[dpdk-dev] [PATCH v2 1/1] ena: fix SIGFPE with 0 rx queues

[Ferruh Yigit]

From: Daria Kolistratova <daria.kolistratova@intel.com>

When the number of rx queues is 0
(what can be when application does not receive)
failed with SIGFPE. It happens when the application
is also requesting ETH_MQ_RX_RSS_FLAG in the
rte_dev->data->dev_conf.rxmode.mq_mode.
Fixed adding zero rx queues check.

Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 9ae73e331..3249b9d92 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1052,7 +1052,7 @@ static int ena_start(struct rte_eth_dev *dev)
 		return rc;
 
 	if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
-	    ETH_MQ_RX_RSS_FLAG) {
+	    ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
 		rc = ena_rss_init_default(adapter);
 		if (rc)
 			return rc;
-- 
2.14.4

Re: [dpdk-dev] [PATCH v2 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

2018-06-26 13:07 GMT+02:00 Daria Kolistratova <daria.kolistratova@intel.com>:
> When the number of rx queues is 0
> (what can be when application does not receive)
> failed with SIGFPE. It happens when the application
> is also requesting ETH_MQ_RX_RSS_FLAG in the
> rte_dev->data->dev_conf.rxmode.mq_mode.
> Fixed adding zero rx queues check.
>
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 9ae73e331..3249b9d92 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1052,7 +1052,7 @@ static int ena_start(struct rte_eth_dev *dev)
>                 return rc;
>
>         if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
> -           ETH_MQ_RX_RSS_FLAG) {
> +           ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
>                 rc = ena_rss_init_default(adapter);
>                 if (rc)
>                         return rc;
> --
> 2.14.4
>

Re: [dpdk-dev] [PATCH v2 1/1] ena: fix SIGFPE with 0 rx queues

[Ferruh Yigit]

On 6/26/2018 6:38 PM, Ferruh Yigit wrote:
> From: Daria Kolistratova <daria.kolistratova@intel.com>

I re-send the Daria's patch, somehow it didn't appeared in mail list and patchwork.

> 
> When the number of rx queues is 0
> (what can be when application does not receive)
> failed with SIGFPE. It happens when the application
> is also requesting ETH_MQ_RX_RSS_FLAG in the
> rte_dev->data->dev_conf.rxmode.mq_mode.
> Fixed adding zero rx queues check.
> 
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 9ae73e331..3249b9d92 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1052,7 +1052,7 @@ static int ena_start(struct rte_eth_dev *dev)
>  		return rc;
>  
>  	if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
> -	    ETH_MQ_RX_RSS_FLAG) {
> +	    ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
>  		rc = ena_rss_init_default(adapter);
>  		if (rc)
>  			return rc;
> 

Re: [dpdk-dev] [PATCH v2 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

2018-06-26 18:53 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
> On 6/26/2018 6:38 PM, Ferruh Yigit wrote:
>> From: Daria Kolistratova <daria.kolistratova@intel.com>
>
> I re-send the Daria's patch, somehow it didn't appeared in mail list and patchwork.
>
>>
>> When the number of rx queues is 0
>> (what can be when application does not receive)
>> failed with SIGFPE. It happens when the application
>> is also requesting ETH_MQ_RX_RSS_FLAG in the
>> rte_dev->data->dev_conf.rxmode.mq_mode.
>> Fixed adding zero rx queues check.
>>
>> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
>> ---
>>  drivers/net/ena/ena_ethdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
>> index 9ae73e331..3249b9d92 100644
>> --- a/drivers/net/ena/ena_ethdev.c
>> +++ b/drivers/net/ena/ena_ethdev.c
>> @@ -1052,7 +1052,7 @@ static int ena_start(struct rte_eth_dev *dev)
>>               return rc;
>>
>>       if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
>> -         ETH_MQ_RX_RSS_FLAG) {
>> +         ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
>>               rc = ena_rss_init_default(adapter);
>>               if (rc)
>>                       return rc;
>>
>

Re: [dpdk-dev] [PATCH v2 1/1] ena: fix SIGFPE with 0 rx queues

[Ferruh Yigit]

On 6/27/2018 9:36 AM, Michał Krawczyk wrote:
> 2018-06-26 18:53 GMT+02:00 Ferruh Yigit <ferruh.yigit@intel.com>:
>> On 6/26/2018 6:38 PM, Ferruh Yigit wrote:
>>> From: Daria Kolistratova <daria.kolistratova@intel.com>
>>
>> I re-send the Daria's patch, somehow it didn't appeared in mail list and patchwork.
>>
>>>
>>> When the number of rx queues is 0
>>> (what can be when application does not receive)
>>> failed with SIGFPE. It happens when the application
>>> is also requesting ETH_MQ_RX_RSS_FLAG in the
>>> rte_dev->data->dev_conf.rxmode.mq_mode.
>>> Fixed adding zero rx queues check.
>>>
>>> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
>
> Acked-by: Michal Krawczyk <mk@semihalf.com>

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

Hi Daria,

>From what you reported on bugzilla, it looks like this patch won't fix
the issue. Have you tried to execute the application with the changes
I suggested in the earlier email thread?

Thanks,
Michal


czw., 21.06.2018, 17:34 użytkownik Daria Kolistratova
<daria.kolistratova@intel.com> napisał:
>
> When  he number of rx queues is 0
> (what can be when application does not receive)
> failed with SIGFPE.
> Fixed adding zero check before division.
>
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 9ae73e331..76c483921 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>         }
>
>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
> -               val = i % nb_rx_queues;
> +               if (nb_rx_queues != 0)
> +                       val = i % nb_rx_queues;
> +               else
> +                       val = 0;
> +
>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>                                                        ENA_IO_RXQ_IDX(val));
>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
> --
> 2.14.4
>

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Kolistratova, Daria]

Hi Michal,

Yes, it won't fix the issue, but will return error instead of fail. I tried to execute with changes from the earlier email thread and it works. Should I resend patch with these changes? 

Best regards,
Kolistratova Daria


-----Original Message-----
From: Michał Krawczyk [mailto:mk@semihalf.com] 
Sent: Monday, June 25, 2018 11:02 AM
To: Kolistratova, Daria <daria.kolistratova@intel.com>
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Marcin Wojtas <mw@semihalf.com>; Tzalik, Guy <gtzalik@amazon.com>; Schmeilin, Evgeny <evgenys@amazon.com>; dev@dpdk.org
Subject: Re: [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

Hi Daria,

From what you reported on bugzilla, it looks like this patch won't fix the issue. Have you tried to execute the application with the changes I suggested in the earlier email thread?

Thanks,
Michal


czw., 21.06.2018, 17:34 użytkownik Daria Kolistratova <daria.kolistratova@intel.com> napisał:
>
> When  he number of rx queues is 0
> (what can be when application does not receive) failed with SIGFPE.
> Fixed adding zero check before division.
>
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c 
> b/drivers/net/ena/ena_ethdev.c index 9ae73e331..76c483921 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>         }
>
>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
> -               val = i % nb_rx_queues;
> +               if (nb_rx_queues != 0)
> +                       val = i % nb_rx_queues;
> +               else
> +                       val = 0;
> +
>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>                                                        ENA_IO_RXQ_IDX(val));
>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
> --
> 2.14.4
>

--------------------------------------------------------------------
Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

Daria,

Yes, please. I will ack the patch once it will be resent.

Thanks,
Michal

2018-06-25 13:05 GMT+02:00 Kolistratova, Daria <daria.kolistratova@intel.com>:
> Hi Michal,
>
> Yes, it won't fix the issue, but will return error instead of fail. I tried to execute with changes from the earlier email thread and it works. Should I resend patch with these changes?
>
> Best regards,
> Kolistratova Daria
>
>
> -----Original Message-----
> From: Michał Krawczyk [mailto:mk@semihalf.com]
> Sent: Monday, June 25, 2018 11:02 AM
> To: Kolistratova, Daria <daria.kolistratova@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Marcin Wojtas <mw@semihalf.com>; Tzalik, Guy <gtzalik@amazon.com>; Schmeilin, Evgeny <evgenys@amazon.com>; dev@dpdk.org
> Subject: Re: [PATCH 1/1] ena: fix SIGFPE with 0 rx queues
>
> Hi Daria,
>
> From what you reported on bugzilla, it looks like this patch won't fix the issue. Have you tried to execute the application with the changes I suggested in the earlier email thread?
>
> Thanks,
> Michal
>
>
> czw., 21.06.2018, 17:34 użytkownik Daria Kolistratova <daria.kolistratova@intel.com> napisał:
>>
>> When  he number of rx queues is 0
>> (what can be when application does not receive) failed with SIGFPE.
>> Fixed adding zero check before division.
>>
>> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
>> ---
>>  drivers/net/ena/ena_ethdev.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ena/ena_ethdev.c
>> b/drivers/net/ena/ena_ethdev.c index 9ae73e331..76c483921 100644
>> --- a/drivers/net/ena/ena_ethdev.c
>> +++ b/drivers/net/ena/ena_ethdev.c
>> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>>         }
>>
>>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
>> -               val = i % nb_rx_queues;
>> +               if (nb_rx_queues != 0)
>> +                       val = i % nb_rx_queues;
>> +               else
>> +                       val = 0;
>> +
>>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>>                                                        ENA_IO_RXQ_IDX(val));
>>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
>> --
>> 2.14.4
>>
>
> --------------------------------------------------------------------
> Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> Russian Federation
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Marcin Wojtas]

+ Ferruh and Michal

2018-06-20 18:32 GMT+02:00 Daria Kolistratova <daria.kolistratova@intel.com>:
> When  he number of rx queues is 0
> (what can be when application does not receive)
> failed with SIGFPE.
> Fixed adding zero check before division.
>
> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 9ae73e331..76c483921 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>         }
>
>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
> -               val = i % nb_rx_queues;
> +               if (nb_rx_queues != 0)
> +                       val = i % nb_rx_queues;
> +               else
> +                       val = 0;
> +
>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>                                                        ENA_IO_RXQ_IDX(val));
>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
> --
> 2.14.4
>

Re: [dpdk-dev] [PATCH 1/1] ena: fix SIGFPE with 0 rx queues

[Michał Krawczyk]

Hi Daria,

I couldn't find the patch in the patchwork or the mailing list, so
sorry for late response.

2018-06-21 12:14 GMT+02:00 Marcin Wojtas <mw@semihalf.com>:
> + Ferruh and Michal
>
> 2018-06-20 18:32 GMT+02:00 Daria Kolistratova <daria.kolistratova@intel.com>:
>> When  he number of rx queues is 0

Typo (missing 't' letter)

>> (what can be when application does not receive)

To be precise, the issue appears only when the application is
requesting ETH_MQ_RX_RSS_FLAG in the
rte_dev->data->dev_conf.rxmode.mq_mode.

>> failed with SIGFPE.
>> Fixed adding zero check before division.
>>
>> Signed-off-by: Daria Kolistratova <daria.kolistratova@intel.com>
>> ---
>>  drivers/net/ena/ena_ethdev.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
>> index 9ae73e331..76c483921 100644
>> --- a/drivers/net/ena/ena_ethdev.c
>> +++ b/drivers/net/ena/ena_ethdev.c
>> @@ -684,7 +684,11 @@ static int ena_rss_init_default(struct ena_adapter *adapter)
>>         }
>>
>>         for (i = 0; i < ENA_RX_RSS_TABLE_SIZE; i++) {
>> -               val = i % nb_rx_queues;
>> +               if (nb_rx_queues != 0)
>> +                       val = i % nb_rx_queues;
>> +               else
>> +                       val = 0;
>> +

We can skip the RSS configuration at all if the nb_rx_queues is 0. I
suggest to modify the patch like this:

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 8701328ba..5ba90f58a 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1056,7 +1056,7 @@ static int ena_start(struct rte_eth_dev *dev)
                return rc;

        if (adapter->rte_dev->data->dev_conf.rxmode.mq_mode &
-           ETH_MQ_RX_RSS_FLAG) {
+           ETH_MQ_RX_RSS_FLAG && adapter->rte_dev->data->nb_rx_queues > 0) {
                rc = ena_rss_init_default(adapter);
                if (rc)
                        return rc;

>>                 rc = ena_com_indirect_table_fill_entry(ena_dev, i,
>>                                                        ENA_IO_RXQ_IDX(val));
>>                 if (unlikely(rc && (rc != ENA_COM_UNSUPPORTED))) {
>> --
>> 2.14.4
>>