Re: [PATCH 4/5] net/bnxt: use fp ops setup function

[fengchengwen <fengchengwen@huawei.com>]

On 2023/3/3 8:01, Konstantin Ananyev wrote:
> 02/03/2023 12:30, Konstantin Ananyev пишет:
>>
>>> Use rte_eth_fp_ops_setup() instead of directly manipulating
>>> rte_eth_fp_ops variable.
>>>
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>> ---
>>>   drivers/net/bnxt/bnxt_cpr.c    | 5 +----
>>>   drivers/net/bnxt/bnxt_ethdev.c | 5 +----
>>>   2 files changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
>>> index 3950840600..a3f33c24c3 100644
>>> --- a/drivers/net/bnxt/bnxt_cpr.c
>>> +++ b/drivers/net/bnxt/bnxt_cpr.c
>>> @@ -416,10 +416,7 @@ void bnxt_stop_rxtx(struct rte_eth_dev *eth_dev)
>>>       eth_dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
>>>       eth_dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
>>
>> I am not that familiar with bnxt driver, but shouldn't we set here
>> other optional fp_ops (descripto_status, etc.) to some dummy values OR to null values?
> 
> After another thought - wouldn't it be better just to call fp_ops_reset() here?

The fp_ops_reset was targeting who violate invocation, so it contain an error log and stack-dump.

It's not suitable for this error recovering scenario.

I've also considered expansion (e.g. add extra parameter for fp_ops_reset), but there are also
other callbacks (e.g. rx_queue_count) should adjust, and make all not simple but complicated.

> 
>>  
>>>
>>> -    rte_eth_fp_ops[eth_dev->data->port_id].rx_pkt_burst =
>>> -        eth_dev->rx_pkt_burst;
>>> -    rte_eth_fp_ops[eth_dev->data->port_id].tx_pkt_burst =
>>> -        eth_dev->tx_pkt_burst;
>>> +    rte_eth_fp_ops_setup(eth_dev);
>>>       rte_mb();
>>>
>>>       /* Allow time for threads to exit the real burst functions. */
>>> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
>>> index 4083a69d02..d6064ceea4 100644
>>> --- a/drivers/net/bnxt/bnxt_ethdev.c
>>> +++ b/drivers/net/bnxt/bnxt_ethdev.c
>>> @@ -4374,10 +4374,7 @@ static void bnxt_dev_recover(void *arg)
>>>       if (rc)
>>>           goto err_start;
>>>
>>> -    rte_eth_fp_ops[bp->eth_dev->data->port_id].rx_pkt_burst =
>>> -        bp->eth_dev->rx_pkt_burst;
>>> -    rte_eth_fp_ops[bp->eth_dev->data->port_id].tx_pkt_burst =
>>> -        bp->eth_dev->tx_pkt_burst;
>>> +    rte_eth_fp_ops_setup(bp->eth_dev);
>>>       rte_mb();
>>>
>>>       PMD_DRV_LOG(INFO, "Port: %u Recovered from FW reset\n",
>>> -- 
>>> 2.17.1
>>
> 
> .