Re: [EXTERNAL] Re: [PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver
From: Simon Horman
Date: Tue Nov 12 2024 - 09:12:45 EST
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:srasheed@xxxxxxxxxxx>
> >
> >...
> >
> >> +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!