Re: [PATCH] ethdev: introduce generic dummy packet burst function

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 10 Feb 2022 13:58:43 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 2/8/2022 7:44 PM, Ferruh Yigit wrote:
> > diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
> > index aecdc5a68abb..bbe05bb837e0 100644
> > --- a/drivers/net/mlx5/linux/mlx5_os.c
> > +++ b/drivers/net/mlx5/linux/mlx5_os.c
> > @@ -1623,8 +1623,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> >   	DRV_LOG(DEBUG, "port %u MTU is %u", eth_dev->data->port_id,
> >   		priv->mtu);
> >   	/* Initialize burst functions to prevent crashes before link-up. */
> > -	eth_dev->rx_pkt_burst = removed_rx_burst;
> > -	eth_dev->tx_pkt_burst = removed_tx_burst;
> > +	eth_dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
> > +	eth_dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
> >   	eth_dev->dev_ops = &mlx5_dev_ops;
> >   	eth_dev->rx_descriptor_status = mlx5_rx_descriptor_status;
> >   	eth_dev->tx_descriptor_status = mlx5_tx_descriptor_status;
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> > index 67eda41a60a5..5571e9067787 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -1559,8 +1559,8 @@ mlx5_dev_close(struct rte_eth_dev *dev)
> >   	mlx5_action_handle_flush(dev);
> >   	mlx5_flow_meter_flush(dev, NULL);
> >   	/* Prevent crashes when queues are still in use. */
> > -	dev->rx_pkt_burst = removed_rx_burst;
> > -	dev->tx_pkt_burst = removed_tx_burst;
> > +	dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
> > +	dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
> >   	rte_wmb();
> >   	/* Disable datapath on secondary process. */
> >   	mlx5_mp_os_req_stop_rxtx(dev);
> > diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
> > index f388fcc31395..11ea935d72f0 100644
> > --- a/drivers/net/mlx5/mlx5_rx.c
> > +++ b/drivers/net/mlx5/mlx5_rx.c
> > @@ -252,7 +252,7 @@ mlx5_rx_queue_count(void *rx_queue)
> >   	dev = &rte_eth_devices[rxq->port_id];
> >   
> >   	if (dev->rx_pkt_burst == NULL ||
> > -	    dev->rx_pkt_burst == removed_rx_burst) {
> > +	    dev->rx_pkt_burst == rte_eth_pkt_burst_dummy) {
> >   		rte_errno = ENOTSUP;
> >   		return -rte_errno;
> >   	}  
> 
> Thinking twice I am not sure if above change works.
> 
> Since function is in the header file, and the .c file that assign
> the 'dev->rx_pkt_burst' and the .c file that check function pointer
> are different, these two different instance of same function may
> have different addresses and above check may fail when it should match.
> 
> I guess solution is move the function to a .c file and export it
> internally.
> I was thinking to add ethdev_driver.c file, perhaps this can be
> a motivation to start that file.
> Thomas, Andrew, what do you think about ethdev_driver.c file?


Right putting it the header file ends up with multiple copies of same
code compiled into each driver.