[PATCH RFC can-next 0/3] Introduce diag support for CAN

[PATCH RFC can-next 0/3] Introduce diag support for CAN

[Filippo Storniolo]

In the current state of the art, it is not possible for userspace to use
tool like ss(8) to query open AF_CAN sockets.

This patch series adds the netlink can_diag interface for AF_CAN. Userspace
is  now able to send a netlink request to request information about open
AF_CAN sockets that are retrieved by the can-diag.ko module.

Patch 1 is a fix that is mandatory to the correct behaviour of the diag
module: the field sk_protocol is not assigned during `can_create()` instead
of being assigned only in case the CAN socket belong to the j1939 transport
protocol. This is needed to the correct filtering of the can diag module.

Patch 2 introduces a list per network namespace containing open CAN sockets.
The list is protected by a mutex.
CAN sockets are added in `can_create()` to the list and removed from it
in the `release()` calls defined for each transport protocol.

Patch 3 is the core patch that introduces the userspace structures to query
and receive netlink messages for CAN sockets and the actual CAN diag module.
The latter will use the userspace structures defined in <linux/can_diag.h>
to filter and retrieve open CAN sockets.

In the current implementation, every CAN sockets in the list is
returned to the userspace, however a filtering could be implemented:
Userspace could possibly request only bound CAN sockets or CAN sockets
belonging to a specific transport protocol (raw, iso-tp, j1939, bcm)
and so on.

Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
Davide Caratti (1):
      af_can: ensure sk_protocol is always set on socket creation

Filippo Storniolo (2):
      af_can: store socket pointers in struct netns_can
      can: add can diag interface

 MAINTAINERS                   |   1 +
 include/linux/can/core.h      |   9 +++
 include/net/netns/can.h       |   6 ++
 include/uapi/linux/can_diag.h |  43 ++++++++++++
 net/can/Kconfig               |  10 +++
 net/can/Makefile              |   2 +
 net/can/af_can.c              |  38 +++++++++++
 net/can/bcm.c                 |   2 +
 net/can/can-diag.c            | 153 ++++++++++++++++++++++++++++++++++++++++++
 net/can/isotp.c               |   2 +
 net/can/j1939/socket.c        |   3 +-
 net/can/raw.c                 |   2 +
 12 files changed, 270 insertions(+), 1 deletion(-)
---
base-commit: f1359c240191e686614847905fc861cbda480b47
change-id: 20260401-feat-can-diag-b082b4f75b27

Best regards,
-- 
Filippo Storniolo <fstornio@redhat.com>

[PATCH RFC can-next 1/3] af_can: ensure sk_protocol is always set on socket creation

[Filippo Storniolo]

From: Davide Caratti <dcaratti@redhat.com>

Currently, only j1939 assigns a value to 'sk_protocol'. As a prerequisite
for the implementation of CAN sockets diagnostics, AF_CAN sockets need an
easy way to determine which protocol is on top of an existing socket.

POC test using can-tests:

 | # perf probe -m can_j1939 --add "j1939_sk_recvmsg sock->sk->sk_protocol"
 | # perf record -e probe:j1939_sk_recvmsg -aR -- ./j1939/tst-j1939-ac
 | [...]
 | # perf script
 | tst-j1939-ac  5807 [002] 322767.312599: probe:j1939_sk_recvmsg: (ffffffffc0b29a14) sk_protocol=0x7
 |  ^^^ 0x7, that's CAN_J1939

 | # perf probe -m can_raw --add "raw_recvmsg sock->sk->sk_protocol"
 | # perf record -e probe:raw_recvmsg -aR ./netlayer/tst-rcv-own-msgs vcan0
 | # perf script
 | [...]
 | tst-rcv-own-msg  5816 [001] 323173.651122: probe:raw_recvmsg: (ffffffffc0b20154) sk_protocol=0x1
 |  ^^^ 0x1, that's CAN_RAW. It was 0x0 on unpatched kernel

Storing the "protocol" value in can_create() fixes the problem. This also
fixes AF_CAN support for the following system call:

 | getsockopt(..., SOL_SOCKET, SO_PROTOCOL, &proto, sizeof(proto))

that was assigning 0 to 'proto' since the earliest kernel versions.

Fixes: 0d66548a10cb ("[CAN]: Add PF_CAN core module")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/can/af_can.c       | 1 +
 net/can/j1939/socket.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 7bc86b176b4de32dddeab5a42a420d62434fcd7f..65af259469854fc19cda0d7dd01c7b4da2ec0f4c 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -162,6 +162,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
 	}
 
 	sock_init_data(sock, sk);
+	sk->sk_protocol = protocol;
 	sk->sk_destruct = can_sock_destruct;
 
 	if (sk->sk_prot->init)
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 0502b030d23851652be252f1342861332ce97367..f07394d44cdc97a09a4f7b0769b493b31ad6f4e3 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -419,7 +419,6 @@ static int j1939_sk_init(struct sock *sk)
 	/* j1939_sk_sock_destruct() depends on SOCK_RCU_FREE flag */
 	sock_set_flag(sk, SOCK_RCU_FREE);
 	sk->sk_destruct = j1939_sk_sock_destruct;
-	sk->sk_protocol = CAN_J1939;
 
 	return 0;
 }

-- 
2.51.0

[PATCH RFC can-next 2/3] af_can: store socket pointers in struct netns_can

[Filippo Storniolo]

AF_CAN sockets need to be stored in the netns_can structure
in order to be retrieved by the CAN diagnostic module when
a netlink request message is issued by the userspace.

On socket creation (`can_create()`), add the pointer to the
new socket to `netns_can::sk_list`. During socket release
(`isotp_release()`, `raw_release()`, `j1939_release()`,
`bcm_release()`), remove the corresponding pointer from
this list.

Since this is a prerequisite of the CAN diagnostic module,
deletes and insert operations are conditioned by
IS_ENABLED(CONFIG_CAN_DIAG).

Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
 include/linux/can/core.h |  9 +++++++++
 include/net/netns/can.h  |  6 ++++++
 net/can/af_can.c         | 37 +++++++++++++++++++++++++++++++++++++
 net/can/bcm.c            |  2 ++
 net/can/isotp.c          |  2 ++
 net/can/j1939/socket.c   |  2 ++
 net/can/raw.c            |  2 ++
 7 files changed, 60 insertions(+)

diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 3287232e3cad92b8514bdad996fbaa02c8fe53fe..0c658ee4aab286e48eb5815640fedbd98a90b92a 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -61,4 +61,13 @@ extern int can_send(struct sk_buff *skb, int loop);
 void can_set_skb_uid(struct sk_buff *skb);
 void can_sock_destruct(struct sock *sk);
 
+/* function prototypes for the CAN diag module */
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+void can_add_sock_sklist(struct sock *sk);
+void can_remove_sock_sklist(struct sock *sk);
+#else
+#define can_add_sock_sklist(sk)
+#define can_remove_sock_sklist(sk)
+#endif
+
 #endif /* !_CAN_CORE_H */
diff --git a/include/net/netns/can.h b/include/net/netns/can.h
index 48b79f7e6236d9b4cf4bd57655f89e161faff05d..bcafff5e66697faf065b4a6b4ab009ce0c62dee7 100644
--- a/include/net/netns/can.h
+++ b/include/net/netns/can.h
@@ -36,6 +36,12 @@ struct netns_can {
 
 	/* CAN GW per-net gateway jobs */
 	struct hlist_head cgw_list;
+
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	/* CAN diag support */
+	struct mutex		sklist_lock;
+	struct hlist_head	sklist;
+#endif
 };
 
 #endif /* __NETNS_CAN_H__ */
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 65af259469854fc19cda0d7dd01c7b4da2ec0f4c..8e1be301fa0ca9dba4cbf1bbd27bfc62ddb19f18 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -111,6 +111,32 @@ static inline void can_put_proto(const struct can_proto *cp)
 	module_put(cp->prot->owner);
 }
 
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+void can_add_sock_sklist(struct sock *sk)
+{
+	struct net *net;
+
+	net = sock_net(sk);
+
+	mutex_lock(&net->can.sklist_lock);
+	sk_add_node(sk, &net->can.sklist);
+	mutex_unlock(&net->can.sklist_lock);
+}
+EXPORT_SYMBOL(can_add_sock_sklist);
+
+void can_remove_sock_sklist(struct sock *sk)
+{
+	struct net *net;
+
+	net = sock_net(sk);
+
+	mutex_lock(&net->can.sklist_lock);
+	sk_del_node_init(sk);
+	mutex_unlock(&net->can.sklist_lock);
+}
+EXPORT_SYMBOL(can_remove_sock_sklist);
+#endif
+
 static int can_create(struct net *net, struct socket *sock, int protocol,
 		      int kern)
 {
@@ -174,6 +200,8 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
 		sock_put(sk);
 		sock->sk = NULL;
 	} else {
+		can_add_sock_sklist(sk);
+
 		sock_prot_inuse_add(net, sk->sk_prot, 1);
 	}
 
@@ -821,6 +849,11 @@ static int can_pernet_init(struct net *net)
 		can_init_proc(net);
 	}
 
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	mutex_init(&net->can.sklist_lock);
+	INIT_HLIST_HEAD(&net->can.sklist);
+#endif
+
 	return 0;
 
  out_free_pkg_stats:
@@ -842,6 +875,10 @@ static void can_pernet_exit(struct net *net)
 	kfree(net->can.rx_alldev_list);
 	kfree(net->can.pkg_stats);
 	kfree(net->can.rcv_lists_stats);
+
+#if IS_ENABLED(CONFIG_CAN_DIAG)
+	WARN_ON_ONCE(!hlist_empty(&net->can.sklist));
+#endif
 }
 
 /* af_can module init/exit functions */
diff --git a/net/can/bcm.c b/net/can/bcm.c
index fd9fa072881e22ced725fa77dd096dea07fb73a6..20c2b9b82c42ca920b803510d16ecfbbe6bd414a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1595,6 +1595,8 @@ static int bcm_release(struct socket *sock)
 	if (!sk)
 		return 0;
 
+	can_remove_sock_sklist(sk);
+
 	net = sock_net(sk);
 	bo = bcm_sk(sk);
 
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 2770f43f4951884658d54ac90bd1e0ae21c24102..0fe00b3e99cd6484843923480a10f7bb58c67bb5 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1200,6 +1200,8 @@ static int isotp_release(struct socket *sock)
 	so = isotp_sk(sk);
 	net = sock_net(sk);
 
+	can_remove_sock_sklist(sk);
+
 	/* wait for complete transmission of current pdu */
 	while (wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE) == 0 &&
 	       cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SHUTDOWN) != ISOTP_IDLE)
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index f07394d44cdc97a09a4f7b0769b493b31ad6f4e3..ed0afe319033ba13e6bc2480962c7d16ad7f7fa1 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -640,6 +640,8 @@ static int j1939_sk_release(struct socket *sock)
 	if (!sk)
 		return 0;
 
+	can_remove_sock_sklist(sk);
+
 	lock_sock(sk);
 	jsk = j1939_sk(sk);
 
diff --git a/net/can/raw.c b/net/can/raw.c
index eee244ffc31ecc0e1cc1aae29cd1d13a4e6b54ca..5c4168860e48f7caea3378607f44b9b498fcff87 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -407,6 +407,8 @@ static int raw_release(struct socket *sock)
 	ro = raw_sk(sk);
 	net = sock_net(sk);
 
+	can_remove_sock_sklist(sk);
+
 	spin_lock(&raw_notifier_lock);
 	while (raw_busy_notifier == ro) {
 		spin_unlock(&raw_notifier_lock);

-- 
2.51.0

[PATCH RFC can-next 3/3] can: add can diag interface

[Filippo Storniolo]

Add the can_diag interface for querying sockets from userspace.
ss(8) tool can use this interface to list open sockets.

The userspace ABI is defined in <linux/can_diag.h> and includes
netlink request and response structs.  The request queries open
can sockets and the response contains socket information fields
including the interface index for bound sockets, inode number,
transport protocol etc.

Support can be added later by extending can_diag_dump().

Suggested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
---
 MAINTAINERS                   |   1 +
 include/uapi/linux/can_diag.h |  43 ++++++++++++
 net/can/Kconfig               |  10 +++
 net/can/Makefile              |   2 +
 net/can/can-diag.c            | 153 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 209 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2ffd9d37d57c0db59e602eeffc2e2f09b613d9..f338ef2380a634a671d06f27bb4dac6f45f4d2a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5730,6 +5730,7 @@ F:	include/linux/can/skb.h
 F:	include/net/can.h
 F:	include/net/netns/can.h
 F:	include/uapi/linux/can.h
+F:	include/uapi/linux/can_diag.h
 F:	include/uapi/linux/can/bcm.h
 F:	include/uapi/linux/can/gw.h
 F:	include/uapi/linux/can/isotp.h
diff --git a/include/uapi/linux/can_diag.h b/include/uapi/linux/can_diag.h
new file mode 100644
index 0000000000000000000000000000000000000000..e63d79f1ab3803a5778407e07d485732a112745a
--- /dev/null
+++ b/include/uapi/linux/can_diag.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _UAPI__CAN_DIAG_H__
+#define _UAPI__CAN_DIAG_H__
+
+#include <linux/types.h>
+#include <linux/can.h>
+
+/* Request */
+struct can_diag_req {
+	__u8	sdiag_family;	/* must be AF_CAN */
+	__u8	sdiag_protocol; /* for future filtering of transport protocols */
+	__u16	pad;
+	__u32	cdiag_states;
+	__u32	cdiag_ino;
+	__u32	cdiag_show;
+	__u32	cdiag_cookie[2];
+};
+
+enum {
+	CAN_DIAG_UNSPEC,
+	CAN_DIAG_UID,
+
+	__CAN_DIAG_MAX,
+};
+
+#define CAN_DIAG_MAX (__CAN_DIAG_MAX - 1)
+
+/* Response */
+struct can_diag_msg {
+	__u8	cdiag_family;	/* AF_CAN */
+	__u8	cdiag_state;
+	__u16	cdiag_protocol;
+	__u16	cdiag_type;
+	__u16	pad16;
+	__u32	cdiag_ino;
+	canid_t cdiag_tx_id; /* meaningful only for ISO-TP */
+	canid_t cdiag_rx_id; /* meaningful only for ISO-TP */
+	__s32	cdiag_ifindex;
+	__u32	cdiag_cookie[2];
+};
+
+#endif /* _UAPI__CAN_DIAG_H__ */
diff --git a/net/can/Kconfig b/net/can/Kconfig
index abbb4be7ad2152c66abd2e28dbbd5282ec45d065..ed210d62da7c28798d2f6f1967739e1621093cbf 100644
--- a/net/can/Kconfig
+++ b/net/can/Kconfig
@@ -70,4 +70,14 @@ config CAN_ISOTP
 	  as needed e.g. for vehicle diagnosis (UDS, ISO 14229) or IP-over-CAN
 	  traffic.
 
+config CAN_DIAG
+	tristate "CAN socket monitoring interface"
+	depends on CAN
+	default m
+	help
+	  Support for CAN socket monitoring interface used by the ss tool.
+	  If unsure, say Y.
+
+	  Enable this module so userspace applications can query open sockets.
+
 endif
diff --git a/net/can/Makefile b/net/can/Makefile
index 58f2c31c1ef377ef8c5385211e7182d42ac2225a..76a3247aa97c982b864914f7aaa3a5b0e96a2e82 100644
--- a/net/can/Makefile
+++ b/net/can/Makefile
@@ -20,3 +20,5 @@ obj-$(CONFIG_CAN_J1939)	+= j1939/
 
 obj-$(CONFIG_CAN_ISOTP)	+= can-isotp.o
 can-isotp-y		:= isotp.o
+
+obj-$(CONFIG_CAN_DIAG) += can-diag.o
diff --git a/net/can/can-diag.c b/net/can/can-diag.c
new file mode 100644
index 0000000000000000000000000000000000000000..77e1fe66e4a6513ff640d0c19c688dcd2e3970ee
--- /dev/null
+++ b/net/can/can-diag.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *
+ * Copyright (C) 2026 Red Hat
+ * Author: Filippo Storniolo <fstornio@redhat.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/netlink.h>
+#include <linux/sock_diag.h>
+#include <linux/can.h>
+#include <linux/can_diag.h>
+#include <net/net_namespace.h>
+#include <net/sock.h>
+#include <linux/netdevice.h>
+#include <linux/percpu.h>
+#include <linux/user_namespace.h>
+
+static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
+			struct can_diag_req *req,
+			struct user_namespace *user_ns,
+			u32 portid, u32 seq, u32 flags, int sk_ino)
+{
+	struct sockaddr_can can_addr;
+	struct can_diag_msg *rep;
+	struct nlmsghdr *nlh;
+	uid_t uid;
+	int err;
+
+	nlh = nlmsg_put(skb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep), flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	rep = nlmsg_data(nlh);
+	memset(rep, 0, sizeof(struct can_diag_msg));
+
+	rep->cdiag_family = AF_CAN;
+	rep->cdiag_type = sk->sk_type;
+	rep->cdiag_ino = sk_ino;
+	rep->cdiag_protocol = sk->sk_protocol;
+	rep->cdiag_state = READ_ONCE(sk->sk_state);
+	sock_diag_save_cookie(sk, rep->cdiag_cookie);
+
+	uid = from_kuid_munged(user_ns, sk_uid(sk));
+	err = nla_put(skb, CAN_DIAG_UID, sizeof(uid_t), &uid);
+	if (err < 0) {
+		nlmsg_cancel(skb, nlh);
+		return err;
+	}
+
+	memset(&can_addr, 0, sizeof(can_addr));
+
+	err = kernel_getsockname(sk->sk_socket, (struct sockaddr *)&can_addr);
+	if (err < 0) {
+		/* Some protocols (e.g. CAN_BCM) do not implement kernel_getsockname().
+		 * No error returned because the netlink message is still valid.
+		 */
+		if (err == -EOPNOTSUPP)
+			goto exit_no_err;
+
+		nlmsg_cancel(skb, nlh);
+		return err;
+	}
+
+	rep->cdiag_ifindex = can_addr.can_ifindex;
+
+	if (sk->sk_protocol == CAN_ISOTP) {
+		rep->cdiag_tx_id = can_addr.can_addr.tp.tx_id;
+		rep->cdiag_rx_id = can_addr.can_addr.tp.rx_id;
+	}
+
+exit_no_err:
+	nlmsg_end(skb, nlh);
+	return 0;
+}
+
+static int can_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	int num = 0, s_num = cb->args[0];
+	struct can_diag_req *req;
+	struct net *net;
+	struct sock *sk;
+
+	net = sock_net(skb->sk);
+	req = nlmsg_data(cb->nlh);
+
+	mutex_lock(&net->can.sklist_lock);
+	sk_for_each(sk, &net->can.sklist) {
+		if (num < s_num)
+			goto next;
+
+		if (sk_diag_fill(sk, skb, req,
+				 sk_user_ns(NETLINK_CB(cb->skb).sk),
+				 NETLINK_CB(cb->skb).portid,
+				 cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				 sock_i_ino(sk)) < 0)
+			goto done;
+next:
+		num++;
+	}
+done:
+	mutex_unlock(&net->can.sklist_lock);
+	cb->args[0] = num;
+
+	return skb->len;
+}
+
+static int can_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
+{
+	int hdrlen = sizeof(struct can_diag_req);
+	struct net *net = sock_net(skb->sk);
+	struct can_diag_req *req;
+
+	if (nlmsg_len(h) < hdrlen)
+		return -EINVAL;
+
+	req = nlmsg_data(h);
+	if (req->sdiag_protocol)
+		return -EINVAL;
+
+	if (h->nlmsg_flags & NLM_F_DUMP) {
+		struct netlink_dump_control c = {
+			.dump = can_diag_dump
+		};
+		return netlink_dump_start(net->diag_nlsk, skb, h, &c);
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static const struct sock_diag_handler can_diag_handler = {
+	.owner = THIS_MODULE,
+	.family = AF_CAN,
+	.dump = can_diag_handler_dump,
+};
+
+static int __init can_diag_init(void)
+{
+	pr_info("can: diagnostic module\n");
+	return sock_diag_register(&can_diag_handler);
+}
+
+static void __exit can_diag_exit(void)
+{
+	sock_diag_unregister(&can_diag_handler);
+}
+
+module_init(can_diag_init);
+module_exit(can_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("CAN socket monitoring via SOCK_DIAG");

-- 
2.51.0

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Oliver Hartkopp]

Hello Filippo,

many thanks for your patches.

I'm not yet convinced we would need this kind of interface as many 
features in ss(8) are only relevant for IP and not really for CAN.

Btw. having an overview over PIDs and open sockets might be a nice 
informational feature.

Some remarks at first sight:

On 02.04.26 12:54, Filippo Storniolo wrote:
> Add the can_diag interface for querying sockets from userspace.
> ss(8) tool can use this interface to list open sockets.
> 
> The userspace ABI is defined in <linux/can_diag.h> and includes

I would suggest <linux/can/diag.h> to match the other CAN netlayer 
definitions and not bloat the include/linux directory once more.

> netlink request and response structs.  The request queries open
> can sockets and the response contains socket information fields
> including the interface index for bound sockets, inode number,
> transport protocol etc.
> 
> Support can be added later by extending can_diag_dump().
> 
> Suggested-by: Davide Caratti <dcaratti@redhat.com>
> Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
> ---
>   MAINTAINERS                   |   1 +
>   include/uapi/linux/can_diag.h |  43 ++++++++++++

include/uapi/linux/can/diag.h

>   net/can/Kconfig               |  10 +++
>   net/can/Makefile              |   2 +
>   net/can/can-diag.c            | 153 ++++++++++++++++++++++++++++++++++++++++++

net/can/diag.c

for the same reason.

>   5 files changed, 209 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7a2ffd9d37d57c0db59e602eeffc2e2f09b613d9..f338ef2380a634a671d06f27bb4dac6f45f4d2a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5730,6 +5730,7 @@ F:	include/linux/can/skb.h
>   F:	include/net/can.h
>   F:	include/net/netns/can.h
>   F:	include/uapi/linux/can.h
> +F:	include/uapi/linux/can_diag.h
>   F:	include/uapi/linux/can/bcm.h
>   F:	include/uapi/linux/can/gw.h
>   F:	include/uapi/linux/can/isotp.h
> diff --git a/include/uapi/linux/can_diag.h b/include/uapi/linux/can_diag.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..e63d79f1ab3803a5778407e07d485732a112745a
> --- /dev/null
> +++ b/include/uapi/linux/can_diag.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +
> +#ifndef _UAPI__CAN_DIAG_H__

Why not using _UAPI_CAN_DIAG_H_ here?

> +#define _UAPI__CAN_DIAG_H__
> +
> +#include <linux/types.h>
> +#include <linux/can.h>
> +
> +/* Request */
> +struct can_diag_req {
> +	__u8	sdiag_family;	/* must be AF_CAN */
> +	__u8	sdiag_protocol; /* for future filtering of transport protocols */
> +	__u16	pad;
> +	__u32	cdiag_states;
> +	__u32	cdiag_ino;
> +	__u32	cdiag_show;
> +	__u32	cdiag_cookie[2];
> +};
> +
> +enum {
> +	CAN_DIAG_UNSPEC,
> +	CAN_DIAG_UID,
> +
> +	__CAN_DIAG_MAX,
> +};
> +
> +#define CAN_DIAG_MAX (__CAN_DIAG_MAX - 1)
> +
> +/* Response */
> +struct can_diag_msg {
> +	__u8	cdiag_family;	/* AF_CAN */
> +	__u8	cdiag_state;
> +	__u16	cdiag_protocol;
> +	__u16	cdiag_type;
> +	__u16	pad16;
> +	__u32	cdiag_ino;
> +	canid_t cdiag_tx_id; /* meaningful only for ISO-TP */
> +	canid_t cdiag_rx_id; /* meaningful only for ISO-TP */

What about the J1939 addressing here which is part of the struct 
sockaddr_can too?

> +	__s32	cdiag_ifindex;
> +	__u32	cdiag_cookie[2];
> +};
> +
> +#endif /* _UAPI__CAN_DIAG_H__ */
> diff --git a/net/can/Kconfig b/net/can/Kconfig
> index abbb4be7ad2152c66abd2e28dbbd5282ec45d065..ed210d62da7c28798d2f6f1967739e1621093cbf 100644
> --- a/net/can/Kconfig
> +++ b/net/can/Kconfig
> @@ -70,4 +70,14 @@ config CAN_ISOTP
>   	  as needed e.g. for vehicle diagnosis (UDS, ISO 14229) or IP-over-CAN
>   	  traffic.
>   
> +config CAN_DIAG
> +	tristate "CAN socket monitoring interface"
> +	depends on CAN
> +	default m
> +	help
> +	  Support for CAN socket monitoring interface used by the ss tool.
> +	  If unsure, say Y.
> +
> +	  Enable this module so userspace applications can query open sockets.
> +
>   endif
> diff --git a/net/can/Makefile b/net/can/Makefile
> index 58f2c31c1ef377ef8c5385211e7182d42ac2225a..76a3247aa97c982b864914f7aaa3a5b0e96a2e82 100644
> --- a/net/can/Makefile
> +++ b/net/can/Makefile
> @@ -20,3 +20,5 @@ obj-$(CONFIG_CAN_J1939)	+= j1939/
>   
>   obj-$(CONFIG_CAN_ISOTP)	+= can-isotp.o
>   can-isotp-y		:= isotp.o
> +
> +obj-$(CONFIG_CAN_DIAG) += can-diag.o

can-diag.ko is a good name indeed.

Best regards,
Oliver

> diff --git a/net/can/can-diag.c b/net/can/can-diag.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..77e1fe66e4a6513ff640d0c19c688dcd2e3970ee
> --- /dev/null
> +++ b/net/can/can-diag.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *
> + * Copyright (C) 2026 Red Hat
> + * Author: Filippo Storniolo <fstornio@redhat.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/net.h>
> +#include <net/netlink.h>
> +#include <linux/sock_diag.h>
> +#include <linux/can.h>
> +#include <linux/can_diag.h>
> +#include <net/net_namespace.h>
> +#include <net/sock.h>
> +#include <linux/netdevice.h>
> +#include <linux/percpu.h>
> +#include <linux/user_namespace.h>
> +
> +static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
> +			struct can_diag_req *req,
> +			struct user_namespace *user_ns,
> +			u32 portid, u32 seq, u32 flags, int sk_ino)
> +{
> +	struct sockaddr_can can_addr;
> +	struct can_diag_msg *rep;
> +	struct nlmsghdr *nlh;
> +	uid_t uid;
> +	int err;
> +
> +	nlh = nlmsg_put(skb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep), flags);
> +	if (!nlh)
> +		return -EMSGSIZE;
> +
> +	rep = nlmsg_data(nlh);
> +	memset(rep, 0, sizeof(struct can_diag_msg));
> +
> +	rep->cdiag_family = AF_CAN;
> +	rep->cdiag_type = sk->sk_type;
> +	rep->cdiag_ino = sk_ino;
> +	rep->cdiag_protocol = sk->sk_protocol;
> +	rep->cdiag_state = READ_ONCE(sk->sk_state);
> +	sock_diag_save_cookie(sk, rep->cdiag_cookie);
> +
> +	uid = from_kuid_munged(user_ns, sk_uid(sk));
> +	err = nla_put(skb, CAN_DIAG_UID, sizeof(uid_t), &uid);
> +	if (err < 0) {
> +		nlmsg_cancel(skb, nlh);
> +		return err;
> +	}
> +
> +	memset(&can_addr, 0, sizeof(can_addr));
> +
> +	err = kernel_getsockname(sk->sk_socket, (struct sockaddr *)&can_addr);
> +	if (err < 0) {
> +		/* Some protocols (e.g. CAN_BCM) do not implement kernel_getsockname().
> +		 * No error returned because the netlink message is still valid.
> +		 */
> +		if (err == -EOPNOTSUPP)
> +			goto exit_no_err;
> +
> +		nlmsg_cancel(skb, nlh);
> +		return err;
> +	}
> +
> +	rep->cdiag_ifindex = can_addr.can_ifindex;
> +
> +	if (sk->sk_protocol == CAN_ISOTP) {
> +		rep->cdiag_tx_id = can_addr.can_addr.tp.tx_id;
> +		rep->cdiag_rx_id = can_addr.can_addr.tp.rx_id;
> +	}
> +
> +exit_no_err:
> +	nlmsg_end(skb, nlh);
> +	return 0;
> +}
> +
> +static int can_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> +	int num = 0, s_num = cb->args[0];
> +	struct can_diag_req *req;
> +	struct net *net;
> +	struct sock *sk;
> +
> +	net = sock_net(skb->sk);
> +	req = nlmsg_data(cb->nlh);
> +
> +	mutex_lock(&net->can.sklist_lock);
> +	sk_for_each(sk, &net->can.sklist) {
> +		if (num < s_num)
> +			goto next;
> +
> +		if (sk_diag_fill(sk, skb, req,
> +				 sk_user_ns(NETLINK_CB(cb->skb).sk),
> +				 NETLINK_CB(cb->skb).portid,
> +				 cb->nlh->nlmsg_seq, NLM_F_MULTI,
> +				 sock_i_ino(sk)) < 0)
> +			goto done;
> +next:
> +		num++;
> +	}
> +done:
> +	mutex_unlock(&net->can.sklist_lock);
> +	cb->args[0] = num;
> +
> +	return skb->len;
> +}
> +
> +static int can_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
> +{
> +	int hdrlen = sizeof(struct can_diag_req);
> +	struct net *net = sock_net(skb->sk);
> +	struct can_diag_req *req;
> +
> +	if (nlmsg_len(h) < hdrlen)
> +		return -EINVAL;
> +
> +	req = nlmsg_data(h);
> +	if (req->sdiag_protocol)
> +		return -EINVAL;
> +
> +	if (h->nlmsg_flags & NLM_F_DUMP) {
> +		struct netlink_dump_control c = {
> +			.dump = can_diag_dump
> +		};
> +		return netlink_dump_start(net->diag_nlsk, skb, h, &c);
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct sock_diag_handler can_diag_handler = {
> +	.owner = THIS_MODULE,
> +	.family = AF_CAN,
> +	.dump = can_diag_handler_dump,
> +};
> +
> +static int __init can_diag_init(void)
> +{
> +	pr_info("can: diagnostic module\n");
> +	return sock_diag_register(&can_diag_handler);
> +}
> +
> +static void __exit can_diag_exit(void)
> +{
> +	sock_diag_unregister(&can_diag_handler);
> +}
> +
> +module_init(can_diag_init);
> +module_exit(can_diag_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN socket monitoring via SOCK_DIAG");
> 

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Davide Caratti]

On Thu, Apr 02, 2026 at 03:07:53PM +0200, Oliver Hartkopp wrote:
> Hello Filippo,
> 
> many thanks for your patches.
> 
> I'm not yet convinced we would need this kind of interface as many features
> in ss(8) are only relevant for IP and not really for CAN.
> 
> Btw. having an overview over PIDs and open sockets might be a nice
> informational feature.


hi, 

thanks for reading!
(an unsolicited answer to the above sentence, just because of the
"Suggested-by:" tag in patch 3/3 :) )

Enumerating open sockets with {uid, pid, interface, protocol} looked to me
a good-enough reason for adding support for ss(8): this is done also by
other non-IP socket families, such as AF_XDP [1] and AF_PACKET for the same
purpose. An alternative would be a tool like tcpstates [2], but it needs to
add tracepoints as well; the diag module (at the expense of serializing
'can_create()' with 'proto.release()' with a mutex) looked a cheaper
solution.

[1] speaking of this, I wonder whether we should pack some of the info we
have now in 'rep', e.g. the result of 'getname()', into a dedicated
attribute. That would save from returning zeros for modules that have stub
getname(), like can_bcm. WDYT?
[2] https://github.com/iovisor/bcc/blob/master/tools/tcpstates.py

-- 
davide

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Oliver Hartkopp]

Hi Davide,

On 02.04.26 18:24, Davide Caratti wrote:
> On Thu, Apr 02, 2026 at 03:07:53PM +0200, Oliver Hartkopp wrote:
>> Hello Filippo,
>>
>> many thanks for your patches.
>>
>> I'm not yet convinced we would need this kind of interface as many features
>> in ss(8) are only relevant for IP and not really for CAN.
>>
>> Btw. having an overview over PIDs and open sockets might be a nice
>> informational feature.
> 
> 
> hi,
> 
> thanks for reading!
> (an unsolicited answer to the above sentence, just because of the
> "Suggested-by:" tag in patch 3/3 :) )
> 
> Enumerating open sockets with {uid, pid, interface, protocol} looked to me
> a good-enough reason for adding support for ss(8): this is done also by
> other non-IP socket families, such as AF_XDP [1] and AF_PACKET for the same
> purpose. An alternative would be a tool like tcpstates [2], but it needs to
> add tracepoints as well; the diag module (at the expense of serializing
> 'can_create()' with 'proto.release()' with a mutex) looked a cheaper
> solution.
> 
> [1] speaking of this, I wonder whether we should pack some of the info we
> have now in 'rep', e.g. the result of 'getname()', into a dedicated
> attribute. That would save from returning zeros for modules that have stub
> getname(), like can_bcm. WDYT?

There's generally no address information like IP-address or port numbers 
in CAN. It is all about CAN frames containing some data. And a CAN frame 
is defined by a CAN Identifier and a CAN bus (CAN interface) it is sent on.

Either can-raw and can-bcm simply work on this CAN ID / CAN Bus kind of 
"addressing". They use a CAN ID filter infrastructure in af_can.c to 
subscribe for CAN IDs received on a (specific/all) CAN interface(s).
But e.g. a CAN_RAW socket can have 200 filters you probably do not want 
to provide via ss(8). The filter lists can be seen in 
/proc/net/can/rcvlist_*

ISO 15765-2 and J1939 define higher layer protocols on the ISO level 2 
CAN bus. And only those two protocols can show something valuable that 
might look like a getname() content.

Best regards,
Oliver

> [2] https://github.com/iovisor/bcc/blob/master/tools/tcpstates.py
> 

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Filippo Storniolo]

Hi Oliver,
thank you for the review.

On Thu, Apr 02, 2026 at 03:07:53PM +0200, Oliver Hartkopp wrote:
>Hello Filippo,
>
>many thanks for your patches.
>
>I'm not yet convinced we would need this kind of interface as many
>features in ss(8) are only relevant for IP and not really for CAN.
>
>Btw. having an overview over PIDs and open sockets might be a nice
>informational feature.
>
>Some remarks at first sight:
>
>On 02.04.26 12:54, Filippo Storniolo wrote:
>>Add the can_diag interface for querying sockets from userspace.
>>ss(8) tool can use this interface to list open sockets.
>>
>>The userspace ABI is defined in <linux/can_diag.h> and includes
>
>I would suggest <linux/can/diag.h> to match the other CAN netlayer
>definitions and not bloat the include/linux directory once more.
>

I created the file can_diag.h because I saw that every other
diag header (e.g. inet, packet, vm_sock, unix, etc.) are all
located under /include/uapi/linux.
However, on the other hand, most of these do not have a directory
under /include/uapi/linux as can already has it.

>>netlink request and response structs.  The request queries open
>>can sockets and the response contains socket information fields
>>including the interface index for bound sockets, inode number,
>>transport protocol etc.
>>
>>Support can be added later by extending can_diag_dump().
>>
>>Suggested-by: Davide Caratti <dcaratti@redhat.com>
>>Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
>>---
>>  MAINTAINERS                   |   1 +
>>  include/uapi/linux/can_diag.h |  43 ++++++++++++
>
>include/uapi/linux/can/diag.h
>

If needed, this will be changed accordingly.

>>  net/can/Kconfig               |  10 +++
>>  net/can/Makefile              |   2 +
>>  net/can/can-diag.c            | 153 ++++++++++++++++++++++++++++++++++++++++++
>
>net/can/diag.c
>
>for the same reason.
>

Agree, I will change the file from can_diag.c to diag.c

>>  5 files changed, 209 insertions(+)
>>
>>diff --git a/MAINTAINERS b/MAINTAINERS
>>index 7a2ffd9d37d57c0db59e602eeffc2e2f09b613d9..f338ef2380a634a671d06f27bb4dac6f45f4d2a4 100644
>>--- a/MAINTAINERS
>>+++ b/MAINTAINERS
>>@@ -5730,6 +5730,7 @@ F:      include/linux/can/skb.h
>>  F:  include/net/can.h
>>  F:  include/net/netns/can.h
>>  F:  include/uapi/linux/can.h
>>+F:   include/uapi/linux/can_diag.h
>>  F:  include/uapi/linux/can/bcm.h
>>  F:  include/uapi/linux/can/gw.h
>>  F:  include/uapi/linux/can/isotp.h
>>diff --git a/include/uapi/linux/can_diag.h b/include/uapi/linux/can_diag.h
>>new file mode 100644
>>index 0000000000000000000000000000000000000000..e63d79f1ab3803a5778407e07d485732a112745a
>>--- /dev/null
>>+++ b/include/uapi/linux/can_diag.h
>>@@ -0,0 +1,43 @@
>>+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>>+
>>+#ifndef _UAPI__CAN_DIAG_H__
>
>Why not using _UAPI_CAN_DIAG_H_ here?
>

No particular reason, we can change it to _UAPI_CAN_DIAG_H_

>>+#define _UAPI__CAN_DIAG_H__
>>+
>>+#include <linux/types.h>
>>+#include <linux/can.h>
>>+
>>+/* Request */
>>+struct can_diag_req {
>>+     __u8    sdiag_family;   /* must be AF_CAN */
>>+     __u8    sdiag_protocol; /* for future filtering of transport protocols */
>>+     __u16   pad;
>>+     __u32   cdiag_states;
>>+     __u32   cdiag_ino;
>>+     __u32   cdiag_show;
>>+     __u32   cdiag_cookie[2];
>>+};
>>+
>>+enum {
>>+     CAN_DIAG_UNSPEC,
>>+     CAN_DIAG_UID,
>>+
>>+     __CAN_DIAG_MAX,
>>+};
>>+
>>+#define CAN_DIAG_MAX (__CAN_DIAG_MAX - 1)
>>+
>>+/* Response */
>>+struct can_diag_msg {
>>+     __u8    cdiag_family;   /* AF_CAN */
>>+     __u8    cdiag_state;
>>+     __u16   cdiag_protocol;
>>+     __u16   cdiag_type;
>>+     __u16   pad16;
>>+     __u32   cdiag_ino;
>>+     canid_t cdiag_tx_id; /* meaningful only for ISO-TP */
>>+     canid_t cdiag_rx_id; /* meaningful only for ISO-TP */
>
>What about the J1939 addressing here which is part of the struct
>sockaddr_can too?
>

Right, I missed this during implementation. I guess we can either
extend the structure so that it contains also the addressing
information of J1939 or add a new attribyte in the netlink message,
similar to what it has been done for the uid.

IMHO, I think the second option is better, especially when
we dump information that are valid only for specific protocols.
Otherwise, we would have a bigger netlink message for every socket,
even if they do not need some fields.

However, we can evaluate pros and cons and refine it.

>>+     __s32   cdiag_ifindex;
>>+     __u32   cdiag_cookie[2];
>>+};
>>+
>>+#endif /* _UAPI__CAN_DIAG_H__ */

Regards,
Filippo

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Oliver Hartkopp]

On 02.04.26 20:13, Filippo Storniolo wrote:
> Hi Oliver,
> thank you for the review.
> 
> On Thu, Apr 02, 2026 at 03:07:53PM +0200, Oliver Hartkopp wrote:
>> Hello Filippo,
>>
>> many thanks for your patches.
>>
>> I'm not yet convinced we would need this kind of interface as many
>> features in ss(8) are only relevant for IP and not really for CAN.
>>
>> Btw. having an overview over PIDs and open sockets might be a nice
>> informational feature.
>>
>> Some remarks at first sight:
>>
>> On 02.04.26 12:54, Filippo Storniolo wrote:
>>> Add the can_diag interface for querying sockets from userspace.
>>> ss(8) tool can use this interface to list open sockets.
>>>
>>> The userspace ABI is defined in <linux/can_diag.h> and includes
>>
>> I would suggest <linux/can/diag.h> to match the other CAN netlayer
>> definitions and not bloat the include/linux directory once more.
>>
> 
> I created the file can_diag.h because I saw that every other
> diag header (e.g. inet, packet, vm_sock, unix, etc.) are all
> located under /include/uapi/linux.
> However, on the other hand, most of these do not have a directory
> under /include/uapi/linux as can already has it.

Thanks

> 
>>> netlink request and response structs.  The request queries open
>>> can sockets and the response contains socket information fields
>>> including the interface index for bound sockets, inode number,
>>> transport protocol etc.
>>>
>>> Support can be added later by extending can_diag_dump().
>>>
>>> Suggested-by: Davide Caratti <dcaratti@redhat.com>
>>> Signed-off-by: Filippo Storniolo <fstornio@redhat.com>
>>> ---
>>>  MAINTAINERS                   |   1 +
>>>  include/uapi/linux/can_diag.h |  43 ++++++++++++
>>
>> include/uapi/linux/can/diag.h
>>
> 
> If needed, this will be changed accordingly.
> 
>>>  net/can/Kconfig               |  10 +++
>>>  net/can/Makefile              |   2 +
>>>  net/can/can-diag.c            | 153 ++++++++++++++++++++++++++++++++ 
>>> ++++++++++
>>
>> net/can/diag.c
>>
>> for the same reason.
>>
> 
> Agree, I will change the file from can_diag.c to diag.c

Thanks!

>>>  5 files changed, 209 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 
>>> 7a2ffd9d37d57c0db59e602eeffc2e2f09b613d9..f338ef2380a634a671d06f27bb4dac6f45f4d2a4 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -5730,6 +5730,7 @@ F:      include/linux/can/skb.h
>>>  F:  include/net/can.h
>>>  F:  include/net/netns/can.h
>>>  F:  include/uapi/linux/can.h
>>> +F:   include/uapi/linux/can_diag.h
>>>  F:  include/uapi/linux/can/bcm.h
>>>  F:  include/uapi/linux/can/gw.h
>>>  F:  include/uapi/linux/can/isotp.h
>>> diff --git a/include/uapi/linux/can_diag.h b/include/uapi/linux/ 
>>> can_diag.h
>>> new file mode 100644
>>> index 
>>> 0000000000000000000000000000000000000000..e63d79f1ab3803a5778407e07d485732a112745a
>>> --- /dev/null
>>> +++ b/include/uapi/linux/can_diag.h
>>> @@ -0,0 +1,43 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>>> +
>>> +#ifndef _UAPI__CAN_DIAG_H__
>>
>> Why not using _UAPI_CAN_DIAG_H_ here?
>>
> 
> No particular reason, we can change it to _UAPI_CAN_DIAG_H_
> 

Thanks!

>>> +#define _UAPI__CAN_DIAG_H__
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/can.h>
>>> +
>>> +/* Request */
>>> +struct can_diag_req {
>>> +     __u8    sdiag_family;   /* must be AF_CAN */
>>> +     __u8    sdiag_protocol; /* for future filtering of transport 
>>> protocols */
>>> +     __u16   pad;
>>> +     __u32   cdiag_states;
>>> +     __u32   cdiag_ino;
>>> +     __u32   cdiag_show;
>>> +     __u32   cdiag_cookie[2];
>>> +};
>>> +
>>> +enum {
>>> +     CAN_DIAG_UNSPEC,
>>> +     CAN_DIAG_UID,
>>> +
>>> +     __CAN_DIAG_MAX,
>>> +};
>>> +
>>> +#define CAN_DIAG_MAX (__CAN_DIAG_MAX - 1)
>>> +
>>> +/* Response */
>>> +struct can_diag_msg {
>>> +     __u8    cdiag_family;   /* AF_CAN */
>>> +     __u8    cdiag_state;
>>> +     __u16   cdiag_protocol;
>>> +     __u16   cdiag_type;
>>> +     __u16   pad16;
>>> +     __u32   cdiag_ino;
>>> +     canid_t cdiag_tx_id; /* meaningful only for ISO-TP */
>>> +     canid_t cdiag_rx_id; /* meaningful only for ISO-TP */
>>
>> What about the J1939 addressing here which is part of the struct
>> sockaddr_can too?
>>
> 
> Right, I missed this during implementation. I guess we can either
> extend the structure so that it contains also the addressing
> information of J1939 or add a new attribyte in the netlink message,
> similar to what it has been done for the uid.
> 
> IMHO, I think the second option is better, especially when
> we dump information that are valid only for specific protocols.
> Otherwise, we would have a bigger netlink message for every socket,
> even if they do not need some fields.
> 
> However, we can evaluate pros and cons and refine it.

In sockaddr_can the ISO-TP and J1939 address information is placed in an 
union as they can not be valid at the same time and the cdiag_protocol 
also provides the information what kind of content we would have read, 
right?

Best regards,
Oliver

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Filippo Storniolo]

On Wed, Apr 08, 2026 at 06:54:31PM +0200, Oliver Hartkopp wrote:

Hi Oliver,

>>>>+/* Response */
>>>>+struct can_diag_msg {
>>>>+     __u8    cdiag_family;   /* AF_CAN */
>>>>+     __u8    cdiag_state;
>>>>+     __u16   cdiag_protocol;
>>>>+     __u16   cdiag_type;
>>>>+     __u16   pad16;
>>>>+     __u32   cdiag_ino;
>>>>+     canid_t cdiag_tx_id; /* meaningful only for ISO-TP */
>>>>+     canid_t cdiag_rx_id; /* meaningful only for ISO-TP */
>>>
>>>What about the J1939 addressing here which is part of the struct
>>>sockaddr_can too?
>>>
>>
>>Right, I missed this during implementation. I guess we can either
>>extend the structure so that it contains also the addressing
>>information of J1939 or add a new attribyte in the netlink message,
>>similar to what it has been done for the uid.
>>
>>IMHO, I think the second option is better, especially when
>>we dump information that are valid only for specific protocols.
>>Otherwise, we would have a bigger netlink message for every socket,
>>even if they do not need some fields.
>>
>>However, we can evaluate pros and cons and refine it.
>
>In sockaddr_can the ISO-TP and J1939 address information is placed in 
>an union as they can not be valid at the same time and the 
>cdiag_protocol also provides the information what kind of content we 
>would have read, right?
>

Yes, you are right. To be more specific, my idea is to have the
can_diag_msg struct like the following:

/* Response */
struct can_diag_msg {
	__u8	cdiag_family;	/* AF_CAN */
	__u8	cdiag_state;
	__u16	cdiag_protocol;
	__u16	cdiag_type;
	__u16	pad16;
	__u32	cdiag_ino;
	__s32	cdiag_ifindex;
	__u32	cdiag_cookie[2];
};

and then add the following structures in the uapi:

struct can_diag_isotp_id {
	canid_t tx_id;
	canid_t rx_id;
};

struct can_diag_j1939_id {
	__u64	name;
	__u32	pgn;
	__u8	addr;
};

By defining two new attributes for j1939 and isotp,
we can fill the netlink response like this:

struct can_diag_isotp_id isotp_id; 
isotp_id.tx_id = can_addr.can_addr.tp.tx_id;
isotp_id.rx_id = can_addr.can_addr.tp.rx_id;

rep->cdiag_rx_id = can_addr.can_addr.tp.rx_id;
if (sk->sk_protocol == CAN_ISOTP) {
	nla_put(skb, CAN_DIAG_ISOTP_ID, sizeof(can_diag_isotp_id), &isotp_id);
}

and same for j1939.

This is useful because we don't have any un-needed fields
in the can_diag_msg structure, but we can still provide to the
userspace information relevant only for specific protocols.

Cheers,
Filippo

Re: [PATCH RFC can-next 3/3] can: add can diag interface

[Oliver Hartkopp]

On 13.04.26 20:04, Filippo Storniolo wrote:
> On Wed, Apr 08, 2026 at 06:54:31PM +0200, Oliver Hartkopp wrote:

>>
>> In sockaddr_can the ISO-TP and J1939 address information is placed in 
>> an union as they can not be valid at the same time and the 
>> cdiag_protocol also provides the information what kind of content we 
>> would have read, right?
>>
> 
> Yes, you are right. To be more specific, my idea is to have the
> can_diag_msg struct like the following:
> 
> /* Response */
> struct can_diag_msg {
>      __u8    cdiag_family;    /* AF_CAN */
>      __u8    cdiag_state;
>      __u16    cdiag_protocol;
>      __u16    cdiag_type;
>      __u16    pad16;
>      __u32    cdiag_ino;
>      __s32    cdiag_ifindex;
>      __u32    cdiag_cookie[2];
> };
> 
> and then add the following structures in the uapi:
> 
> struct can_diag_isotp_id {
>      canid_t tx_id;
>      canid_t rx_id;
> };
> 
> struct can_diag_j1939_id {
>      __u64    name;
>      __u32    pgn;
>      __u8    addr;
> };
> 
> By defining two new attributes for j1939 and isotp,
> we can fill the netlink response like this:
> 
> struct can_diag_isotp_id isotp_id; isotp_id.tx_id = 
> can_addr.can_addr.tp.tx_id;
> isotp_id.rx_id = can_addr.can_addr.tp.rx_id;
> 
> rep->cdiag_rx_id = can_addr.can_addr.tp.rx_id;
> if (sk->sk_protocol == CAN_ISOTP) {
>      nla_put(skb, CAN_DIAG_ISOTP_ID, sizeof(can_diag_isotp_id), &isotp_id);
> }
> 
> and same for j1939.
> 
> This is useful because we don't have any un-needed fields
> in the can_diag_msg structure, but we can still provide to the
> userspace information relevant only for specific protocols.

Ok, got it.

Btw. either for ISO-TP and for J1939 these values are address definitions.

So the structs should be named

struct can_diag_isotp_addr {
      canid_t tx_id;
      canid_t rx_id;
};

struct can_diag_j1939_addr {
      __u64    name;
      __u32    pgn;
      __u8    addr;
};

@Oleksij: Does that fit for you too?

Best regards,
Oliver