On Wed, Apr 7, 2021 at 6:40 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote:
>
> On 3/31/2021 9:53 AM, Leyi Rong wrote:
> > Fill up dev->tx_pkt_prepare to i40e_pkt_prepare when on vector and simple
> > data path selection, as the sanity check is needed ideally.
> >
> > Signed-off-by: Leyi Rong <leyi.r...@intel.com>
> > ---
> > drivers/net/i40e/i40e_rxtx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > index 61cb204be2..b3d7765e3b 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -3412,7 +3412,7 @@ i40e_set_tx_function(struct rte_eth_dev *dev)
> > PMD_INIT_LOG(DEBUG, "Simple tx finally be used.");
> > dev->tx_pkt_burst = i40e_xmit_pkts_simple;
> > }
> > - dev->tx_pkt_prepare = NULL;
> > + dev->tx_pkt_prepare = i40e_prep_pkts;
> > } else {
> > PMD_INIT_LOG(DEBUG, "Xmit tx finally be used.");
> > dev->tx_pkt_burst = i40e_xmit_pkts;
> >
>
> It seems prepare function is doing some sanity checks before handing packets
> to
> the HW.
> So with this change all Tx paths calls the same Tx prepare function, if so why
> not set the function pointer outside of the if block, instead of setting it in
> both legs of the if/else? This clarifies that Tx prepare used always.
As I said earlier, what is the point of setting it in ethdev.c too?
It would be more consistent to have this in a single location.
drivers/net/i40e/i40e_ethdev.c: dev->tx_pkt_burst = i40e_xmit_pkts;
drivers/net/i40e/i40e_ethdev.c: dev->tx_pkt_prepare = i40e_prep_pkts;
drivers/net/i40e/i40e_ethdev_vf.c: eth_dev->tx_pkt_burst = &i40e_xmit_pkts;
drivers/net/i40e/i40e_rxtx.c: if (dev->tx_pkt_burst ==
i40e_xmit_pkts_vec_avx512) {
drivers/net/i40e/i40e_rxtx.c: if (dev->tx_pkt_burst ==
i40e_xmit_pkts_vec_avx2 ||
drivers/net/i40e/i40e_rxtx.c: dev->tx_pkt_burst ==
i40e_xmit_pkts_vec) {
drivers/net/i40e/i40e_rxtx.c:
dev->tx_pkt_burst = i40e_xmit_pkts_vec_avx512;
drivers/net/i40e/i40e_rxtx.c:
dev->tx_pkt_burst = use_avx2 ?
drivers/net/i40e/i40e_rxtx.c: dev->tx_pkt_burst =
i40e_xmit_pkts_simple;
drivers/net/i40e/i40e_rxtx.c: dev->tx_pkt_prepare = NULL;
drivers/net/i40e/i40e_rxtx.c: dev->tx_pkt_burst = i40e_xmit_pkts;
drivers/net/i40e/i40e_rxtx.c: dev->tx_pkt_prepare = i40e_prep_pkts;
drivers/net/i40e/i40e_rxtx.c: eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
drivers/net/i40e/i40e_vf_representor.c: ethdev->tx_pkt_burst =
i40e_vf_representor_tx_burst;
--
David Marchand
