RE: [PATCH v6 02/27] net/nfp: fix the promiscuous mode control functions

[Chaoyong He <chaoyong.he@corigine.com>]

> On 10/21/2022 9:01 AM, Chaoyong He wrote:
> > The original functions of promiscuous mode can't process the
> > representor port rightly, revise the logic to do that.
> >
> > Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > ---
> >   drivers/net/nfp/flower/nfp_flower_representor.c | 26
> +++++++++++++++++++++++--
> >   1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
> > b/drivers/net/nfp/flower/nfp_flower_representor.c
> > index 0e60f50..cecdf46 100644
> > --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> > +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> > @@ -415,6 +415,9 @@
> >   static int
> >   nfp_flower_repr_promiscuous_enable(struct rte_eth_dev *dev)
> >   {
> > +	int ret;
> > +	uint32_t update;
> > +	uint32_t new_ctrl;
> >   	struct nfp_net_hw *pf_hw;
> >   	struct nfp_flower_representor *repr;
> >
> > @@ -431,12 +434,23 @@
> >   		return 0;
> >   	}
> >
> > -	return nfp_net_promisc_enable(pf_hw->eth_dev);
> > +	new_ctrl = pf_hw->ctrl | NFP_NET_CFG_CTRL_PROMISC;
> > +	update = NFP_NET_CFG_UPDATE_GEN;
> > +	ret = nfp_net_reconfig(pf_hw, new_ctrl, update);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	pf_hw->ctrl = new_ctrl;
> > +
> 
> With this change 'nfp_net_promisc_enable()' seems not used at all in the
> driver.
> 
Actually, 'nfp_net_promisc_enable()' is still be used by the VF and when the NIC
using CoreNIC firmware app.
In the `static const struct eth_dev_ops nfp_netvf_eth_dev_ops[]` of file 'nfp_ethdev_vf.c'
and `static const struct eth_dev_ops nfp_net_eth_dev_ops[]` of file 'nfp_ethdev.c'.

> And the difference between above code and 'nfp_net_promisc_enable()' is
> that capability check is removed, so it looks like hack for flower, instead why
> not fix the issue in the 'nfp_net_promisc_enable()'?
> 
> Same comment for 'nfp_net_promisc_disable()'.
> 
Yes, it's a good idea to fix this issue in the 'nfp_ent_promisc_enable()' and 'nfp_net_promisc_disable()'.
I will send a v7 patch for this one, thanks.

> > +	return 0;
> >   }
> >
> >   static int
> >   nfp_flower_repr_promiscuous_disable(struct rte_eth_dev *dev)
> >   {
> > +	int ret;
> > +	uint32_t update;
> > +	uint32_t new_ctrl;
> >   	struct nfp_net_hw *pf_hw;
> >   	struct nfp_flower_representor *repr;
> >
> > @@ -448,7 +462,15 @@
> >   		return 0;
> >   	}
> >
> > -	return nfp_net_promisc_disable(pf_hw->eth_dev);
> > +	new_ctrl = pf_hw->ctrl & ~NFP_NET_CFG_CTRL_PROMISC;
> > +	update = NFP_NET_CFG_UPDATE_GEN;
> > +	ret = nfp_net_reconfig(pf_hw, new_ctrl, update);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	pf_hw->ctrl = new_ctrl;
> > +
> > +	return 0;
> >   }
> >
> >   static int