Re: [dpdk-dev] [PATCH v1 1/2] net/mlx5: change eth device reference for secondary process

["Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>]

On Fri, Aug 25, 2017 at 07:15:50AM +0000, Xueming(Steven) Li wrote:
> Nelio, thanks, comments inline.
>[...]
> > >  static int
> > > -priv_set_link(struct priv *priv, int up)
> > > +mlx5_dev_set_link(struct rte_eth_dev *dev, int up)
> > >  {
> > > -	struct rte_eth_dev *dev = priv->dev;
> > > +	struct priv *priv = dev->data->dev_private;
> > >  	int err;
> > >
> > 
> > This function should lock/unclock priv.
> This is a static function, caller function do the lock/unlock. 
> Is there naming convention here? Mlx5_* is outpost interfaces that normally
> require lock/unlock priv?

Yes there is a naming convention following the patterns:
 - priv_...(struct *priv priv, ...): no locks inside.
 - priv_dev_...(struct *priv priv, struct rte_eth_dev *dev, ...): no locks
   inside.
 - mlx5_...(struct rte_eth_dev *dev, ...): should lock any access to struct priv
   and to priv_*().

> > >  	if (up) {
> > >  		err = priv_set_flags(priv, ~IFF_UP, IFF_UP);
> > >  		if (err)
> > >  			return err;
> > > -		priv_select_tx_function(priv);
> > > -		priv_select_rx_function(priv);
> > > +		mlx5_dev_select_tx_function(dev);
> > > +		mlx5_dev_select_rx_function(dev);
> > 
> > This also induce that those function mlx5_dev_select_rx/tx_function() should
> > be renamed to:
> >  priv_dev_select_rx/tx_function(struct *priv, struct rte_eth_dev *dev, ...)
> > 
> > this will avoid the multiple lock/unlocks inside the functions.
> So priv_* are lock-free functions?

priv_*() assume the lock have been done by the caller.

Hope it helps.

Thanks,

-- 
Nélio Laranjeiro
6WIND