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

[Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>]

03/03/2023 01:38, fengchengwen пишет:
> On 2023/3/2 20:30, Konstantin Ananyev wrote:
>>
>>> 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?
> 
> I checked the bnxt PMD code, the other fp_ops (rx_queue_count/rx_descriptor_status/tx_descriptor_status)
> both add following logic at the beginning of function:
> 
> 	rc = is_bnxt_in_error(bp);
> 	if (rc)
> 		return rc;
> 
> So I think it okey to keep it.

I still think it is much safer/cleaner to update all fp_ops in one go
(use fp_ops_reset()) here.
But as you believe it would work either way, I'll leave it to bnxt
maintainers to decide.


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