2021-10-03 04:46:07

by Vincent MAILHOL

[permalink] [raw]
Subject: [PATCH v1] can: netlink: report the CAN controller mode supported flags

This patch introduces a method for the user to check both the
supported and the static capabilities.

Currently, the CAN netlink interface provides no easy ways to check
the capabilities of a given controller. The only method from the
command line is to try each CAN_CTRLMODE_ individually to check
whether the netlink interface returns an -EOPNOTSUPP error or not
(alternatively, one may find it easier to directly check the source
code of the driver instead...)

It appears that, currently, the struct can_ctrlmode::mask field is
only used in one direction: from the userland to the kernel. So we can
just reuse this field in the other direction (from the kernel to
userland). But, because the semantic is different, we use a union to
give this field a proper name: supported.

Below table explains how the two fields can_ctrlmode::supported and
can_ctrlmode::flags, when masked with any of the CAN_CTRLMODE_* bit
flags, allow us to identify both the supported and the static
capabilities:

supported & flags & Controller capabilities
CAN_CTRLMODE_* CAN_CTRLMODE_*
------------------------------------------------------------------------
false false Feature not supported (always disabled)
false true Static feature (always enabled)
true false Feature supported but disabled
true true Feature supported and enabled

N.B.: This patch relies on the fact that a given CAN_CTRLMODE_*
feature can not be set for both can_priv::ctrlmode_supported and
can_priv::ctrlmode_static at the same time. c.f. comments in struct
can_priv [1]. Else, there would be no way to distinguish which
features were statically enabled.

[1] https://elixir.bootlin.com/linux/v5.14/source/include/linux/can/dev.h#61

Signed-off-by: Vincent Mailhol <[email protected]>
---
I will send a iproute2-next patch right after to illustrate the idea.
---
drivers/net/can/dev/netlink.c | 5 ++++-
include/uapi/linux/can/netlink.h | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 80425636049d..480818edccc1 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -264,7 +264,10 @@ 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 = {
+ .supported = priv->ctrlmode_supported,
+ .flags = priv->ctrlmode
+ };
struct can_berr_counter bec = { };
enum can_state state = priv->state;

diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index f730d443b918..2847ed0dcac3 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -88,7 +88,10 @@ struct can_berr_counter {
* CAN controller mode
*/
struct can_ctrlmode {
- __u32 mask;
+ union {
+ __u32 mask; /* Userland to kernel */
+ __u32 supported; /* Kernel to userland */
+ };
__u32 flags;
};

--
2.32.0


2021-10-09 05:39:43

by Vincent MAILHOL

[permalink] [raw]
Subject: Re: [PATCH v1] can: netlink: report the CAN controller mode supported flags

On Sun. 3 Oct 2021 at 13:40, Vincent Mailhol <[email protected]> wrote:
> This patch introduces a method for the user to check both the
> supported and the static capabilities.
>
> Currently, the CAN netlink interface provides no easy ways to check
> the capabilities of a given controller. The only method from the
> command line is to try each CAN_CTRLMODE_ individually to check
> whether the netlink interface returns an -EOPNOTSUPP error or not
> (alternatively, one may find it easier to directly check the source
> code of the driver instead...)
>
> It appears that, currently, the struct can_ctrlmode::mask field is
> only used in one direction: from the userland to the kernel. So we can
> just reuse this field in the other direction (from the kernel to
> userland). But, because the semantic is different, we use a union to
> give this field a proper name: supported.
>
> Below table explains how the two fields can_ctrlmode::supported and
> can_ctrlmode::flags, when masked with any of the CAN_CTRLMODE_* bit
> flags, allow us to identify both the supported and the static
> capabilities:
>
> supported & flags & Controller capabilities
> CAN_CTRLMODE_* CAN_CTRLMODE_*
> ------------------------------------------------------------------------
> false false Feature not supported (always disabled)
> false true Static feature (always enabled)
> true false Feature supported but disabled
> true true Feature supported and enabled
>
> N.B.: This patch relies on the fact that a given CAN_CTRLMODE_*
> feature can not be set for both can_priv::ctrlmode_supported and
> can_priv::ctrlmode_static at the same time. c.f. comments in struct
> can_priv [1]. Else, there would be no way to distinguish which
> features were statically enabled.

Actually, can_priv::ctrlmode_static can be derived from the other
ctrlmode fields. I will send a v2 in which I will add a patch to
replace that field with an inline function.

Yours sincerely,
Vincent Mailhol