Re: ip link valid options checking

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi Stefan,

On 09.07.21 17:07, Stefan Mätje wrote:
> Am Freitag, den 09.07.2021, 14:00 +0200 schrieb Vincent MAILHOL:

> 
> Unfortunately the netlink kernel interface gives access only to the
> IFLA_CAN_CTRLMODE data which boils down to an access of the netdev_priv(dev)-
>> priv->ctrlmode flags set in the driver (see also in a Linux tree under
> drivers/net/can/dev/netlink.c). But the "ctrlmode" flags represent only the
> current setup. So you can see if CAN-FD mode is currently enabled.
> 
> But I think the thread opener wants to know in advance if the kernel gives us
> the information what modes a certain driver supports. That is encoded in
> netdev_priv(dev)->priv->ctrlmode_supported and netdev_priv(dev)->priv-
>> ctrlmode_static. But for these flags there is no netlink interface to
> interrogate that settings at the moment.
> 
> So you can't see in advance if a CAN driver would support for instance triple-
> sampling or the CAN_CTRLMODE_CC_LEN8_DLC mode. To know it you must try to set
> such option atm. I think.

Yes, but the settings of priv->ctrlmode_static may lead into problems 
too as there might be a fixed setting that can not be altered.

As you already pointed out the netlink interface only provides the 
current setting of priv->ctrlmode.

Btw. the struct can_ctrlmode has two u32 elements, so we could use the 
mask element to pass the priv->ctrlmode_supported value to the user space:

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index e38c2566aff4..91c6ae06a576 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -259,11 +259,12 @@ static size_t can_get_size(const struct net_device 
*dev)
  }

  static int can_fill_info(struct sk_buff *skb, const struct net_device 
*dev)
  {
         struct can_priv *priv = netdev_priv(dev);
-       struct can_ctrlmode cm = {.flags = priv->ctrlmode};
+       struct can_ctrlmode cm = {.flags = priv->ctrlmode,
+               .mask = priv->ctrlmode_supported};
         struct can_berr_counter bec = { };
         enum can_state state = priv->state;

         if (priv->do_get_state)
                 priv->do_get_state(dev, &state);

Additionally we could also make the mask element in struct can_ctrlmode 
a union with a 'supported' element ...

But this is, what would be needed here, right?

Best regards,
Oliver