Re: [PATCH 3/3] net/mlx5: rebuild flows on updating RETA

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

On Fri, Mar 17, 2017 at 10:14:56AM -0700, Yongseok Koh wrote:
> Hi Nelio,
> 
> On Fri, Mar 17, 2017 at 02:11:43AM -0700, Nélio Laranjeiro wrote:
> > On Thu, Mar 16, 2017 at 03:40:56PM -0700, Yongseok Koh wrote:
> > > Currently mlx5_dev_rss_reta_update() just updates tables in the host,
> > > therefore it isn't immediately effective until restarting the device by
> > > calling mlx5_dev_stop()/mlx5_dev_start() to update the changes in the
> > > device side. This patch adds rebuilding the device-specific datastructure
> > > and applying it to the device right away.
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > >  drivers/net/mlx5/mlx5_rss.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
> > > index 0702f1a63..30e59faa5 100644
> > > --- a/drivers/net/mlx5/mlx5_rss.c
> > > +++ b/drivers/net/mlx5/mlx5_rss.c
> > > @@ -357,8 +357,11 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
> > >        int ret;
> > >        struct priv *priv = dev->data->dev_private;
> > > 
> > > +     mlx5_dev_stop(dev);
> > >        priv_lock(priv);
> > >        ret = priv_dev_rss_reta_update(priv, reta_conf, reta_size);
> > >        priv_unlock(priv);
> > > +     if (!ret)
> > > +             ret = (unsigned int)mlx5_dev_start(dev);
> > >        return -ret;
> > >  }
> > > --
> > > 2.11.0
> > >
> > 
> > Hi Yongseok,
> > 
> > I don't understand why you need the cast for the returned value of
> > mlx5_dev_start() as it already returns an int and your final variable is
> > also an int.
> 
> For some reason, in mlx5 PMD code, priv_* calls return positive error numbers
> and corresponding mlx5_* APIs return negative values by adding (-) sign the the
> return value from priv_* calls.

This is due to the IOCTL calls which returns only positives errno, we
decided to keep only positives errors internally and negate them in when
they are returned to DPDK.

> To follow this tacit rule in the existing code, I casted the return value to
> unsigned.

I see more this cast as something confusing than helping, knowing that
it could be avoided with something like:

 if (ret)
 	return -ret;
 return mlx5_dev_start(dev);

Regards,

-- 
Nélio Laranjeiro
6WIND