ip link valid options checking

ip link valid options checking

[Joshua Quesenberry]

Good Afternoon,

Do any of you know a way to check what options are valid when setting up a
CAN device with the ip link commands, either from bash or through C++? I'm
working on a piece of code that will get used on systems with varying CAN
drivers and for instance one of them doesn't support CAN-FD, so how would I
be able to know that setting the fd flag or dbitrate value would fail before
calling the setup command and seeing it fail?

Thanks,

Josh Q

Re: ip link valid options checking

[Vincent MAILHOL]

On Wed. 7 Jul 2021 at 19:45, Joshua Quesenberry <engnfrc@gmail.com> wrote:
> Good Afternoon,
>
> Do any of you know a way to check what options are valid when setting up a
> CAN device with the ip link commands, either from bash or through C++? I'm
> working on a piece of code that will get used on systems with varying CAN
> drivers and for instance one of them doesn't support CAN-FD, so how would I
> be able to know that setting the fd flag or dbitrate value would fail before
> calling the setup command and seeing it fail?

I would suggest to use:
| ip --details link show can0

You can then parse the results to check if the data bitrate
information is present. If so, it means that FD is supported.
Also, the output might be easier to parse if formatted in json:
| ip --details --json --pretty link show can0

Alternatively, instead of using the command line, you might
prefer to directly use the kernel netlink interface to directly
retrieve the different modes supported by the controller.
You can refer to iproute2 source code for the how-to:
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/iplink_can.c#n282


Yours sincerely,
Vincent

Re: ip link valid options checking

[Stefan Mätje]

Am Freitag, den 09.07.2021, 14:00 +0200 schrieb Vincent MAILHOL:
> On Wed. 7 Jul 2021 at 19:45, Joshua Quesenberry <engnfrc@gmail.com> wrote:
> > Good Afternoon,
> > 
> > Do any of you know a way to check what options are valid when setting up a
> > CAN device with the ip link commands, either from bash or through C++? I'm
> > working on a piece of code that will get used on systems with varying CAN
> > drivers and for instance one of them doesn't support CAN-FD, so how would I
> > be able to know that setting the fd flag or dbitrate value would fail before
> > calling the setup command and seeing it fail?
> 
> I would suggest to use:
> > ip --details link show can0
> 
> You can then parse the results to check if the data bitrate
> information is present. If so, it means that FD is supported.
> Also, the output might be easier to parse if formatted in json:
> > ip --details --json --pretty link show can0
> 
> Alternatively, instead of using the command line, you might
> prefer to directly use the kernel netlink interface to directly
> retrieve the different modes supported by the controller.
> You can refer to iproute2 source code for the how-to:
> 
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/iplink_can.c#n282
> 

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.

Best regards,
    Stefan

Re: ip link valid options checking

[Oliver Hartkopp]

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

Re: ip link valid options checking

[Stefan Mätje]

Am Samstag, den 10.07.2021, 12:19 +0200 schrieb Oliver Hartkopp:
> 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?

It looks like a step in the right direction. But I want to add two points to 
the discussion.

1) I think the flags member of struct can_ctrlmode should contain the 
ctrlmode_static flags in any case, because the ctrlmode_static flags also describe
a part of the current setting. I. e. the changed lines in your patch should read then:

+       struct can_ctrlmode cm = {.flags = priv->ctrlmode | priv->ctrlmode_static,
+               .mask = priv->ctrlmode_supported};

I believe this did not show up as an error till now because ctrlmode_static
is not used by any driver atm.


Then an application can possibly do something like this to get the needed 
information (pseudo code):

app_decode_ctrlmode (struct can_ctrlmode *cm) {
	u32 app_ctrlmode,  app_ctrlmode_supported, app_ctrlmode_static;

	app_ctrlmode = cm->flags;			/* current setting */
	app_ctrlmode_supported = cm->mask; 		/* changeable settings */
	app_ctrlmode_static = cm->flags & ~cm->mask;	/* fixed settings */
}


2) I don't know how an application like the "ip" tool could discriminate whether 
it talks to a kernel with the new interface (mask element contains 
ctrlmode_supported) or to a kernel with the old interface. This is because on 
an old kernel the mask element contains uninitialized data from the kernel 
stack as the mask element is not set till now.

I don't see how this decision could be made by the "ip" tool.

Best regards,
    Stefan