[v3] net/iavf: fix mbuf release API selection
Checks
Commit Message
When AVX2 is forcibly selected and outer checksum
offload is configured, the basic Tx path will be
selected. Also, the txq mbuf release API is incorrectly
set to iavf_tx_queue_release_mbufs_sse. This causes
coredump.
This commit selects release_txq_mbufs to releasing
txq mbufs when selecting the basic Tx path.
Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
Cc: stable@dpdk.org
Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
drivers/net/iavf/iavf_rxtx.c | 1 +
1 file changed, 1 insertion(+)
Comments
> -----Original Message-----
> From: Kaiwen Deng <kaiwenx.deng@intel.com>
> Sent: Thursday, November 2, 2023 12:43 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: [PATCH v3] net/iavf: fix mbuf release API selection
>
> When AVX2 is forcibly selected and outer checksum offload is configured, the
> basic Tx path will be selected. Also, the txq mbuf release API is incorrectly set
> to iavf_tx_queue_release_mbufs_sse. This causes coredump.
>
> This commit selects release_txq_mbufs to releasing txq mbufs when selecting
> the basic Tx path.
>
> Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> Cc: stable@dpdk.org
>
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> ---
> drivers/net/iavf/iavf_rxtx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> 610912f635..a16e03d88c 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> PMD_DRV_LOG(DEBUG,
> "AVX2 does not support outer
> checksum offload, using Basic Tx (port %d).",
> dev->data->port_id);
> + return;
This make the execution routing not consistent between avx2 and avx512.
I think it will be a better solution if we reset the use_avx2 flag here, and use this flag to decide if need to overwrite the release function later.
> } else {
> dev->tx_pkt_burst =
> iavf_xmit_pkts_vec_avx2_offload;
> dev->tx_pkt_prepare = iavf_prep_pkts;
> --
> 2.34.1
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, November 8, 2023 5:33 PM
> To: Deng, KaiwenX <kaiwenx.deng@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
>
>
>
> > -----Original Message-----
> > From: Kaiwen Deng <kaiwenx.deng@intel.com>
> > Sent: Thursday, November 2, 2023 12:43 PM
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>
> > Subject: [PATCH v3] net/iavf: fix mbuf release API selection
> >
> > When AVX2 is forcibly selected and outer checksum offload is
> > configured, the basic Tx path will be selected. Also, the txq mbuf
> > release API is incorrectly set to iavf_tx_queue_release_mbufs_sse. This
> causes coredump.
> >
> > This commit selects release_txq_mbufs to releasing txq mbufs when
> > selecting the basic Tx path.
> >
> > Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> > ---
> > drivers/net/iavf/iavf_rxtx.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > b/drivers/net/iavf/iavf_rxtx.c index 610912f635..a16e03d88c 100644
> > --- a/drivers/net/iavf/iavf_rxtx.c
> > +++ b/drivers/net/iavf/iavf_rxtx.c
> > @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> > PMD_DRV_LOG(DEBUG,
> > "AVX2 does not support outer
> > checksum offload, using Basic Tx (port %d).",
> > dev->data->port_id);
> > + return;
>
> This make the execution routing not consistent between avx2 and avx512.
> I think it will be a better solution if we reset the use_avx2 flag here, and use
> this flag to decide if need to overwrite the release function later.
Or you can just "goto normal" and removing function call assignment.
>
> > } else {
> > dev->tx_pkt_burst =
> > iavf_xmit_pkts_vec_avx2_offload;
> > dev->tx_pkt_prepare = iavf_prep_pkts;
> > --
> > 2.34.1
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Wednesday, November 8, 2023 8:03 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Deng, KaiwenX
> <kaiwenx.deng@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; Deng, KaiwenX <kaiwenx.deng@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zeng,
> ZhichaoX <zhichaox.zeng@intel.com>
> Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
>
>
>
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Wednesday, November 8, 2023 5:33 PM
> > To: Deng, KaiwenX <kaiwenx.deng@intel.com>; dev@dpdk.org
> > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > <zhichaox.zeng@intel.com>
> > Subject: RE: [PATCH v3] net/iavf: fix mbuf release API selection
> >
> >
> >
> > > -----Original Message-----
> > > From: Kaiwen Deng <kaiwenx.deng@intel.com>
> > > Sent: Thursday, November 2, 2023 12:43 PM
> > > To: dev@dpdk.org
> > > Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou,
> > > YidingX <yidingx.zhou@intel.com>; Deng, KaiwenX
> > > <kaiwenx.deng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > > Xing, Beilei <beilei.xing@intel.com>; Zeng, ZhichaoX
> > > <zhichaox.zeng@intel.com>
> > > Subject: [PATCH v3] net/iavf: fix mbuf release API selection
> > >
> > > When AVX2 is forcibly selected and outer checksum offload is
> > > configured, the basic Tx path will be selected. Also, the txq mbuf
> > > release API is incorrectly set to iavf_tx_queue_release_mbufs_sse.
> > > This
> > causes coredump.
> > >
> > > This commit selects release_txq_mbufs to releasing txq mbufs when
> > > selecting the basic Tx path.
> > >
> > > Fixes: 22f1e7608ebc ("net/iavf: fix AVX2 Tx selection")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> > > ---
> > > drivers/net/iavf/iavf_rxtx.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/iavf/iavf_rxtx.c
> > > b/drivers/net/iavf/iavf_rxtx.c index 610912f635..a16e03d88c 100644
> > > --- a/drivers/net/iavf/iavf_rxtx.c
> > > +++ b/drivers/net/iavf/iavf_rxtx.c
> > > @@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
> > > PMD_DRV_LOG(DEBUG,
> > > "AVX2 does not support outer
> > > checksum offload, using Basic Tx (port %d).",
> > > dev->data->port_id);
> > > + return;
> >
> > This make the execution routing not consistent between avx2 and avx512.
> > I think it will be a better solution if we reset the use_avx2 flag
> > here, and use this flag to decide if need to overwrite the release function
> later.
>
> Or you can just "goto normal" and removing function call assignment.
I will prepare a new patch.
Thanks.
>
> >
> > > } else {
> > > dev->tx_pkt_burst =
> > > iavf_xmit_pkts_vec_avx2_offload;
> > > dev->tx_pkt_prepare = iavf_prep_pkts;
> > > --
> > > 2.34.1
@@ -4022,6 +4022,7 @@ iavf_set_tx_function(struct rte_eth_dev *dev)
PMD_DRV_LOG(DEBUG,
"AVX2 does not support outer checksum offload, using Basic Tx (port %d).",
dev->data->port_id);
+ return;
} else {
dev->tx_pkt_burst = iavf_xmit_pkts_vec_avx2_offload;
dev->tx_pkt_prepare = iavf_prep_pkts;