All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
To: Marc Kleine-Budde <mkl@pengutronix.de>, linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Subject: [PATCH v4] can: netlink: report the CAN controller mode supported flags
Date: Tue, 26 Oct 2021 21:16:51 +0900	[thread overview]
Message-ID: <20211026121651.1814251-1-mailhol.vincent@wanadoo.fr> (raw)

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


             reply	other threads:[~2021-10-26 12:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26 12:16 Vincent Mailhol [this message]
2021-10-29 12:44 ` [PATCH v4] can: netlink: report the CAN controller mode supported flags Marc Kleine-Budde
2021-10-29 16:14   ` Vincent MAILHOL
2021-10-29 12:46 ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211026121651.1814251-1-mailhol.vincent@wanadoo.fr \
    --to=mailhol.vincent@wanadoo.fr \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.