Messages in this thread Patch in this message | | | From | Vincent Mailhol <> | Subject | [PATCH v4] can: netlink: report the CAN controller mode supported flags | Date | Tue, 26 Oct 2021 21:16:51 +0900 |
| |
This patch introduces a method for the user to check both the supported and the static capabilities. The proposed method reuses the existing struct can_ctrlmode and thus do not need a new IFLA_CAN_* entry.
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 can_ctrlmode::mask 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".
The union is tagged as packed to prevent any ABI from adding padding. In fact, any padding added after the union would change the offset of can_ctrlmode::flags within the structure and thus break the UAPI backward compatibility. References:
- ISO/IEC 9899-1999, section 6.7.2.1 "Structure and union specifiers", clause 15: "There may be unnamed padding at the end of a structure or union."
- The -mstructure-size-boundary=64 ARM option in GCC: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
- A similar issue which occurred on struct can_frame: https://lore.kernel.org/linux-can/212c8bc3-89f9-9c33-ed1b-b50ac04e7532@hartkopp.net
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
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- Hi Marc,
Because you already added the series to the testing branch of linux-can-next, I am only resending the last patch. Please let me know if you prefer me to resend the full series.
** Changelog **
v3 -> v4:
- Tag the union in struct can_ctrlmode as packed.
- Remove patch 1, 2 and 3 from the series because those were already added to the testing branch of linux-can-next (and no changes occured on those first three patches).
v2 -> v3:
- Make can_set_static_ctrlmode() return an error and adjust the drivers which use this helper function accordingly.
v1 -> v2:
- Add a first patch to replace can_priv::ctrlmode_static by the inline function can_get_static_ctrlmode()
- Add a second patch to reorder the fields of struct can_priv for better packing (save eight bytes on x86_64 \o/)
- Rewrite the comments of the third patch "can: netlink: report the CAN controller mode supported flags" (no changes on the code itself).
--- 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 26c336808be5..32e1eb63ee7d 100644 --- a/drivers/net/can/dev/netlink.c +++ b/drivers/net/can/dev/netlink.c @@ -475,7 +475,10 @@ static int can_tdc_fill_info(struct sk_buff *skb, 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 75b85c60efb2..091bb78b0406 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 */ + } __attribute__((packed)); /* Prevent ABI from adding padding */ __u32 flags; }; -- 2.32.0
| |