| Message ID | 20211003044049.568441-1-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1] can: netlink: report the CAN controller mode supported flags | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject |
On Sun. 3 Oct 2021 at 13:40, Vincent Mailhol <mailhol.vincent@wanadoo.fr> 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
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; };
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 <mailhol.vincent@wanadoo.fr> --- 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(-)