[4/8] net/bnxt: allow Tx only or Rx only configs in PMD

Message ID 20220615145703.6613-5-kalesh-anakkur.purayil@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series bnxt PMD fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Kalesh A P June 15, 2022, 2:56 p.m. UTC
  From: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>

Currently, we fail the init/probe of pmd if eth_dev->data->nb_tx_queues
or eth_dev->data->nb_rx_queues is 0. We are removing this check.

Fixes: daef48efe5e5 ("net/bnxt: support set MTU")
Cc: stable@dpdk.org

Signed-off-by: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
  

Comments

Ferruh Yigit June 16, 2022, 5:03 p.m. UTC | #1
On 6/15/2022 3:56 PM, Kalesh A P wrote:

> 
> From: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>
> 
> Currently, we fail the init/probe of pmd if eth_dev->data->nb_tx_queues
> or eth_dev->data->nb_rx_queues is 0. We are removing this check.
> 

Is there a valid usecase for Rx only or Tx only config?
I assume testpmd doesn't support it, how are you testing this?

> Fixes: daef48efe5e5 ("net/bnxt: support set MTU")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> ---
>   drivers/net/bnxt/bnxt_ethdev.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 34f2149..8181e1f 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -723,7 +723,7 @@ static int bnxt_alloc_prev_ring_stats(struct bnxt *bp)
>                                               sizeof(struct bnxt_ring_stats) *
>                                               bp->tx_cp_nr_rings,
>                                               0);
> -       if (bp->prev_tx_ring_stats == NULL)
> +       if (bp->tx_cp_nr_rings > 0 && bp->prev_tx_ring_stats == NULL)
>                  goto error;
> 
>          return 0;
> @@ -1567,11 +1567,6 @@ int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
>          int vlan_mask = 0;
>          int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
> 
> -       if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
> -               PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
> -               return -EINVAL;
> -       }
> -
>          if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
>                  PMD_DRV_LOG(ERR,
>                              "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
> --
> 2.10.1
>
  
Kalesh A P June 21, 2022, 4:46 a.m. UTC | #2
Hi Damo,

Could you please respond to Ferruh's question?

Regards,
Kalesh

On Thu, Jun 16, 2022 at 10:33 PM Ferruh Yigit <ferruh.yigit@xilinx.com>
wrote:

> On 6/15/2022 3:56 PM, Kalesh A P wrote:
>
> >
> > From: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>
> >
> > Currently, we fail the init/probe of pmd if eth_dev->data->nb_tx_queues
> > or eth_dev->data->nb_rx_queues is 0. We are removing this check.
> >
>
> Is there a valid usecase for Rx only or Tx only config?
> I assume testpmd doesn't support it, how are you testing this?
>
> > Fixes: daef48efe5e5 ("net/bnxt: support set MTU")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Damodharam Ammepalli <damodharam.ammepall@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > ---
> >   drivers/net/bnxt/bnxt_ethdev.c | 7 +------
> >   1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> > index 34f2149..8181e1f 100644
> > --- a/drivers/net/bnxt/bnxt_ethdev.c
> > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> > @@ -723,7 +723,7 @@ static int bnxt_alloc_prev_ring_stats(struct bnxt
> *bp)
> >                                               sizeof(struct
> bnxt_ring_stats) *
> >                                               bp->tx_cp_nr_rings,
> >                                               0);
> > -       if (bp->prev_tx_ring_stats == NULL)
> > +       if (bp->tx_cp_nr_rings > 0 && bp->prev_tx_ring_stats == NULL)
> >                  goto error;
> >
> >          return 0;
> > @@ -1567,11 +1567,6 @@ int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
> >          int vlan_mask = 0;
> >          int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
> >
> > -       if (!eth_dev->data->nb_tx_queues ||
> !eth_dev->data->nb_rx_queues) {
> > -               PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
> > -               return -EINVAL;
> > -       }
> > -
> >          if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
> >                  PMD_DRV_LOG(ERR,
> >                              "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS
> %d\n",
> > --
> > 2.10.1
> >
>
>
  
Damodharam Ammepalli June 21, 2022, 4:54 a.m. UTC | #3
Hi Ferruh,

Please see my inline responses [Damo];

 

Thanks

Damo

 

From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> 
Sent: Monday, June 20, 2022 9:47 PM
To: Ferruh Yigit <ferruh.yigit@xilinx.com>; Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Cc: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>; dpdk-dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 4/8] net/bnxt: allow Tx only or Rx only configs in PMD

 

Hi Damo,

 

Could you please respond to Ferruh's question?

 

Regards,

Kalesh

 

On Thu, Jun 16, 2022 at 10:33 PM Ferruh Yigit <ferruh.yigit@xilinx.com <mailto:ferruh.yigit@xilinx.com> > wrote:

On 6/15/2022 3:56 PM, Kalesh A P wrote:

> 
> From: Damodharam Ammepalli <damodharam.ammepall@broadcom.com <mailto:damodharam.ammepall@broadcom.com> >
> 
> Currently, we fail the init/probe of pmd if eth_dev->data->nb_tx_queues
> or eth_dev->data->nb_rx_queues is 0. We are removing this check.
> 

Is there a valid usecase for Rx only or Tx only config?
I assume testpmd doesn't support it, how are you testing this?
[Damo]:

Yes. There is a valid use case. We are trying to address a

customer request, to deploy an application in Rxonly mode.

This is the sample testpmd command we used in our unit tests.

./build/app/dpdk-testpmd -c 0xff  -n 4 --log-level="pmd.",7

--socket-mem 0,1024  -- --forward-mode=rxonly --txq=0 -i

 

We can update the commit headline to indicate Rx only configuration to

prevent misunderstanding.

Please let me know and we will address accordingly.


> Fixes: daef48efe5e5 ("net/bnxt: support set MTU")
> Cc: stable@dpdk.org <mailto:stable@dpdk.org> 
> 
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepall@broadcom.com <mailto:damodharam.ammepall@broadcom.com> >
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com <mailto:ajit.khaparde@broadcom.com> >
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com <mailto:somnath.kotur@broadcom.com> >
> ---
>   drivers/net/bnxt/bnxt_ethdev.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 34f2149..8181e1f 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -723,7 +723,7 @@ static int bnxt_alloc_prev_ring_stats(struct bnxt *bp)
>                                               sizeof(struct bnxt_ring_stats) *
>                                               bp->tx_cp_nr_rings,
>                                               0);
> -       if (bp->prev_tx_ring_stats == NULL)
> +       if (bp->tx_cp_nr_rings > 0 && bp->prev_tx_ring_stats == NULL)
>                  goto error;
> 
>          return 0;
> @@ -1567,11 +1567,6 @@ int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
>          int vlan_mask = 0;
>          int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
> 
> -       if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
> -               PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
> -               return -EINVAL;
> -       }
> -
>          if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
>                  PMD_DRV_LOG(ERR,
>                              "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
> --
> 2.10.1
>
  
Ferruh Yigit June 21, 2022, 7:57 a.m. UTC | #4
On 6/21/2022 5:54 AM, Damodharam Ammepalli wrote:
> Hi Ferruh,
> 
> Please see my inline responses [Damo];
> 
> Thanks
> 
> Damo
> 
> *From:*Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> *Sent:* Monday, June 20, 2022 9:47 PM
> *To:* Ferruh Yigit <ferruh.yigit@xilinx.com>; Damodharam Ammepalli 
> <damodharam.ammepalli@broadcom.com>
> *Cc:* Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>; dpdk-dev 
> <dev@dpdk.org>
> *Subject:* Re: [dpdk-dev] [PATCH 4/8] net/bnxt: allow Tx only or Rx only 
> configs in PMD
> 
> Hi Damo,
> 
> Could you please respond to Ferruh's question?
> 
> Regards,
> 
> Kalesh
> 
> On Thu, Jun 16, 2022 at 10:33 PM Ferruh Yigit <ferruh.yigit@xilinx.com 
> <mailto:ferruh.yigit@xilinx.com>> wrote:
> 
>     On 6/15/2022 3:56 PM, Kalesh A P wrote:
> 
>      >
>      > From: Damodharam Ammepalli <damodharam.ammepall@broadcom.com
>     <mailto:damodharam.ammepall@broadcom.com>>
>      >
>      > Currently, we fail the init/probe of pmd if
>     eth_dev->data->nb_tx_queues
>      > or eth_dev->data->nb_rx_queues is 0. We are removing this check.
>      >
> 
>     Is there a valid usecase for Rx only or Tx only config?
>     I assume testpmd doesn't support it, how are you testing this?
>     [Damo]:
> 
>     Yes. There is a valid use case. We are trying to address a
> 
>     customer request, to deploy an application in Rxonly mode.
> 

OK, I just want to confirm this is a valid usecase.

>     This is the sample testpmd command we used in our unit tests.
> 
>     ./build/app/dpdk-testpmd -c 0xff  -n 4 --log-level="pmd.",7
> 
>     --socket-mem 0,1024  -- --forward-mode=rxonly --txq=0 -i
> 

Got it, testpmd 'rxonly', and possibly 'txonly', mode is working.

>     We can update the commit headline to indicate Rx only configuration to
> 
>     prevent misunderstanding.
> 
>     Please let me know and we will address accordingly.
> 

I think it is OK as it is, thanks for clarification.

> 
>      > Fixes: daef48efe5e5 ("net/bnxt: support set MTU")
>      > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
>      >
>      > Signed-off-by: Damodharam Ammepalli
>     <damodharam.ammepall@broadcom.com
>     <mailto:damodharam.ammepall@broadcom.com>>
>      > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
>     <mailto:ajit.khaparde@broadcom.com>>
>      > Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com
>     <mailto:somnath.kotur@broadcom.com>>
>      > ---
>      >   drivers/net/bnxt/bnxt_ethdev.c | 7 +------
>      >   1 file changed, 1 insertion(+), 6 deletions(-)
>      >
>      > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
>     b/drivers/net/bnxt/bnxt_ethdev.c
>      > index 34f2149..8181e1f 100644
>      > --- a/drivers/net/bnxt/bnxt_ethdev.c
>      > +++ b/drivers/net/bnxt/bnxt_ethdev.c
>      > @@ -723,7 +723,7 @@ static int bnxt_alloc_prev_ring_stats(struct
>     bnxt *bp)
>      >                                               sizeof(struct
>     bnxt_ring_stats) *
>      >                                               bp->tx_cp_nr_rings,
>      >                                               0);
>      > -       if (bp->prev_tx_ring_stats == NULL)
>      > +       if (bp->tx_cp_nr_rings > 0 && bp->prev_tx_ring_stats == NULL)
>      >                  goto error;
>      >
>      >          return 0;
>      > @@ -1567,11 +1567,6 @@ int bnxt_dev_start_op(struct rte_eth_dev
>     *eth_dev)
>      >          int vlan_mask = 0;
>      >          int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
>      >
>      > -       if (!eth_dev->data->nb_tx_queues ||
>     !eth_dev->data->nb_rx_queues) {
>      > -               PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
>      > -               return -EINVAL;
>      > -       }
>      > -
>      >          if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
>      >                  PMD_DRV_LOG(ERR,
>      >                              "RxQ cnt %d >
>     RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",
>      > --
>      > 2.10.1
>      >
> 
> 
> -- 
> 
> Regards,
> 
> Kalesh A P
> 
> 
> This electronic communication and the information and any files 
> transmitted with it, or attached to it, are confidential and are 
> intended solely for the use of the individual or entity to whom it is 
> addressed and may contain information that is confidential, legally 
> privileged, protected by privacy laws, or otherwise restricted from 
> disclosure to anyone else. If you are not the intended recipient or the 
> person responsible for delivering the e-mail to the intended recipient, 
> you are hereby notified that any use, copying, distributing, 
> dissemination, forwarding, printing, or copying of this e-mail is 
> strictly prohibited. If you received this e-mail in error, please return 
> the e-mail to the sender, delete it from your computer, and destroy any 
> printed copy of it.
  

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 34f2149..8181e1f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -723,7 +723,7 @@  static int bnxt_alloc_prev_ring_stats(struct bnxt *bp)
 					     sizeof(struct bnxt_ring_stats) *
 					     bp->tx_cp_nr_rings,
 					     0);
-	if (bp->prev_tx_ring_stats == NULL)
+	if (bp->tx_cp_nr_rings > 0 && bp->prev_tx_ring_stats == NULL)
 		goto error;
 
 	return 0;
@@ -1567,11 +1567,6 @@  int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	int vlan_mask = 0;
 	int rc, retry_cnt = BNXT_IF_CHANGE_RETRY_COUNT;
 
-	if (!eth_dev->data->nb_tx_queues || !eth_dev->data->nb_rx_queues) {
-		PMD_DRV_LOG(ERR, "Queues are not configured yet!\n");
-		return -EINVAL;
-	}
-
 	if (bp->rx_cp_nr_rings > RTE_ETHDEV_QUEUE_STAT_CNTRS)
 		PMD_DRV_LOG(ERR,
 			    "RxQ cnt %d > RTE_ETHDEV_QUEUE_STAT_CNTRS %d\n",