On Sun, Mar 5, 2023 at 7:58 AM Konstantin Ananyev <konstantin.v.anan...@yandex.ru> wrote: > > 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: sta...@dpdk.org > >>> > >>> Signed-off-by: Chengwen Feng <fengcheng...@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.
We have been operating without the application being aware of the underlying functionality for some time now. Each step here is an improvement. I think it is okay to keep it simple and update them separately. Thanks Ajit > > > > > >> > >>> > >>> - 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 > >> > >> . > >> >
smime.p7s
Description: S/MIME Cryptographic Signature
