Subject: Re: [EXTERNAL] Re: [PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver

From: Simon Horman <[email-masked]>
Date: Tue, 12 Nov 2024 14:12:23 +0000

On Mon, Nov 11, 2024 at 05:29:53AM +0000, Shinas Rasheed wrote:
> Hi Simon,
> 
> On Thu, Nov 07, 2024 at 04:16:37AM -0800, Shinas Rasheed wrote:
> >> These APIs are needed to support applicaitons that use netlink to get VF
> >> information from a PF driver.
> >> 
> >> Signed-off-by: Shinas Rasheed <mailto:[email-masked]>
> >
> >...
> >
> >> +static int octep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan, u8 qos, __be16 vlan_proto)
> >> +{
> >> +	struct octep_device *oct = netdev_priv(dev);
> >> +
> >> +	dev_err(&oct->pdev->dev, "Setting VF VLAN not supported\n");>
> >> +	return 0;
> >> +}
> >
> >Hi Shinas,
> >
> >Given that the operation is not supported I think it would
> >be more appropriate to return -EOPNOTSUPP. And moreover, given
> >that this is a noop, I think it would be yet more appropriate
> >not to implement it at all and let the core treat it as not supported.
> >
> >Likewise for other NDOs implemented as noops in this patch.
> >
> >...
> 
> I think the problem was for some userspace programs and operators, sometimes returning -EOPNOTSUPP is a no-go. I think the idea was at least if the user saw these messages, they would know to
> set it in some other way, and also not have the operator stop just because setting these values failed. Though I understand that’s counter-intuitive, but sometimes it lets operators work and go ahead. What do you think so?

Hi Shinas,

I think it would be good to provide more detail of such use-cases:
my understanding is that not implementing the operations would
be the go-to solution if they are not supported by the driver.

> Thanks for the comments!