[PATCH net-next] net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

[PATCH net-next] net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

[Faizan Ali]

Add a new socket option MCTP_OPT_ROUTE_SRCADDR that allows applications
to query which local EID the kernel would use as the source address
when sending to a given destination EID.

Applications such as PLDM need to advertise a local EID as the event
receiver address to remote endpoints. Previously this required a manual
multi-step lookup via the mctp tool (route show + addr show).
The kernel routing table is the authoritative
source for this mapping, so expose it directly via a socket option.

The caller fills in net and daddr in struct mctp_route_srcaddr before
calling getsockopt(SOL_MCTP, MCTP_OPT_ROUTE_SRCADDR); the kernel
performs the same route lookup used for actual packet output and
returns the resolved local EID in saddr.

Verified with the included kunit tests, and manually validated on
hardware that the resolved source EID matches mctp route show / addr
show output for both reachable and unreachable destinations.

Link: https://github.com/CodeConstruct/mctp/issues/147
Signed-off-by: Faizan Ali <faizana@nvidia.com>
---
 include/uapi/linux/mctp.h | 13 +++++++++
 net/mctp/af_mctp.c        | 31 ++++++++++++++++++++
 net/mctp/test/sock-test.c | 61 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+)

diff --git a/include/uapi/linux/mctp.h b/include/uapi/linux/mctp.h
index 19ad12a0c..7c3d4a936 100644
--- a/include/uapi/linux/mctp.h
+++ b/include/uapi/linux/mctp.h
@@ -55,6 +55,19 @@ struct mctp_fq_addr {
 #define MCTP_TAG_PREALLOC	0x10
 
 #define MCTP_OPT_ADDR_EXT	1
+#define MCTP_OPT_ROUTE_SRCADDR	2
+
+/* Query structure for MCTP_OPT_ROUTE_SRCADDR getsockopt.
+ *
+ * Caller fills in @net and @daddr before calling getsockopt.
+ * Performs a route lookup and returns the local source EID in @saddr.
+ */
+struct mctp_route_srcaddr {
+	unsigned int	net;
+	mctp_eid_t	daddr;
+	mctp_eid_t	saddr;
+	__u8		__pad[2];
+};
 
 #define SIOCMCTPALLOCTAG	(SIOCPROTOPRIVATE + 0)
 #define SIOCMCTPDROPTAG		(SIOCPROTOPRIVATE + 1)
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index 8af5e2b3c..5c6077e03 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -425,6 +425,37 @@ static int mctp_getsockopt(struct socket *sock, int level, int optname,
 		return 0;
 	}
 
+	if (optname == MCTP_OPT_ROUTE_SRCADDR) {
+		struct mctp_route_srcaddr rsa;
+		struct mctp_dst dst;
+		unsigned int net;
+		int rc;
+
+		if (len != sizeof(rsa))
+			return -EINVAL;
+
+		if (copy_from_iter(&rsa, len, &opt->iter_in) != len)
+			return -EFAULT;
+
+		net = rsa.net;
+		if (net == MCTP_NET_ANY)
+			net = mctp_default_net(sock_net(sock->sk));
+
+		rc = mctp_route_lookup(sock_net(sock->sk), net, rsa.daddr, &dst);
+		if (rc)
+			return rc;
+
+		rsa.saddr = dst.saddr;
+		mctp_dst_release(&dst);
+
+		if (rsa.saddr == MCTP_ADDR_NULL)
+			return -EADDRNOTAVAIL;
+
+		if (copy_to_iter(&rsa, len, &opt->iter_out) != len)
+			return -EFAULT;
+		return 0;
+	}
+
 	return -ENOPROTOOPT;
 }
 
diff --git a/net/mctp/test/sock-test.c b/net/mctp/test/sock-test.c
index b0942deb5..b3ebc504b 100644
--- a/net/mctp/test/sock-test.c
+++ b/net/mctp/test/sock-test.c
@@ -379,10 +379,71 @@ static void mctp_test_assumptions(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, mctp_default_net(&init_net), 1);
 }
 
+static void mctp_test_sockopt_init(sockopt_t *opt, struct kvec *vec,
+				   void *buf, size_t len)
+{
+	vec->iov_base = buf;
+	vec->iov_len = len;
+	iov_iter_kvec(&opt->iter_in, ITER_SOURCE, vec, 1, len);
+	iov_iter_kvec(&opt->iter_out, ITER_DEST, vec, 1, len);
+	opt->optlen = len;
+}
+
+static void mctp_test_getsockopt_route_srcaddr(struct kunit *test)
+{
+	struct mctp_route_srcaddr rsa = {
+		.net = MCTP_INITIAL_DEFAULT_NET,
+		.daddr = 9,
+	};
+	struct mctp_test_route *rt;
+	struct mctp_test_dev *dev;
+	struct socket *sock;
+	struct kvec vec;
+	sockopt_t opt;
+	int rc;
+
+	__mctp_sock_test_init(test, &dev, &rt, &sock);
+
+	/* Query the source EID for destination EID 9; the device has
+	 * local EID 8, so the route lookup should return saddr=8.
+	 */
+	mctp_test_sockopt_init(&opt, &vec, &rsa, sizeof(rsa));
+	rc = mctp_getsockopt(sock, SOL_MCTP, MCTP_OPT_ROUTE_SRCADDR, &opt);
+	KUNIT_EXPECT_EQ(test, rc, 0);
+	KUNIT_EXPECT_EQ(test, (int)rsa.saddr, 8);
+	KUNIT_EXPECT_EQ(test, opt.optlen, (int)sizeof(rsa));
+
+	__mctp_sock_test_fini(test, dev, rt, sock);
+}
+
+static void mctp_test_getsockopt_route_srcaddr_no_route(struct kunit *test)
+{
+	struct mctp_route_srcaddr rsa = {
+		.net = MCTP_INITIAL_DEFAULT_NET,
+		.daddr = 99,  /* no route for this EID */
+	};
+	struct mctp_test_route *rt;
+	struct mctp_test_dev *dev;
+	struct socket *sock;
+	struct kvec vec;
+	sockopt_t opt;
+	int rc;
+
+	__mctp_sock_test_init(test, &dev, &rt, &sock);
+
+	mctp_test_sockopt_init(&opt, &vec, &rsa, sizeof(rsa));
+	rc = mctp_getsockopt(sock, SOL_MCTP, MCTP_OPT_ROUTE_SRCADDR, &opt);
+	KUNIT_EXPECT_EQ(test, rc, -EHOSTUNREACH);
+
+	__mctp_sock_test_fini(test, dev, rt, sock);
+}
+
 static struct kunit_case mctp_test_cases[] = {
 	KUNIT_CASE(mctp_test_assumptions),
 	KUNIT_CASE(mctp_test_sock_sendmsg_extaddr),
 	KUNIT_CASE(mctp_test_sock_recvmsg_extaddr),
+	KUNIT_CASE(mctp_test_getsockopt_route_srcaddr),
+	KUNIT_CASE(mctp_test_getsockopt_route_srcaddr_no_route),
 	KUNIT_CASE_PARAM(mctp_test_bind_conflicts, mctp_bind_pair_gen_params),
 	KUNIT_CASE(mctp_test_bind_invalid),
 	{}
-- 
2.43.0

Re: [PATCH net-next] net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

[Jeremy Kerr]

Hi Faizan,

> Add a new socket option MCTP_OPT_ROUTE_SRCADDR that allows applications
> to query which local EID the kernel would use as the source address
> when sending to a given destination EID.
> 
> Applications such as PLDM need to advertise a local EID as the event
> receiver address to remote endpoints. Previously this required a manual
> multi-step lookup via the mctp tool (route show + addr show).
> The kernel routing table is the authoritative
> source for this mapping, so expose it directly via a socket option.

As I had asked earlier, can you elaborate on why this is needed over
choosing any local address? Is there a routing topology where this
would not work?

I'm not against the idea, we just need a fairly solidy justification
for adding user ABI that cannot be changed in future.

Cheers,


Jeremy

Re: [PATCH net-next] net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

[Faizan Ali]

Hello Jeremy,

Sorry, I missed following up on this earlier - continuing our
conversation from the earlier thread.

> As I had asked earlier, can you elaborate on why this is needed over
> choosing any local address? Is there a routing topology where this
> would not work?
>
> I'm not against the idea, we just need a fairly solidy justification
> for adding user ABI that cannot be changed in future.

In theory, yes - any incoming message destined to an active local EID
should reach an application bound to that message type. That said, I
still find these problems with simply picking any local EID:

1. It requires a snapshot of all available local EIDs first -
   information the kernel already has internally via the routing
   table, but which applications can today only get via ad-hoc mctp
   route/addr correlation.

2. Not every local EID is reachable from every peer, even on the same
   network. Example from our hardware:

     BMC --USB(EID 8)--> SMA(EID 20) --I3C--> GPU(EID 30)
     BMC also has a separate mctpi2c0 (EID 9), unrelated to the SMA.

   Reaching the GPU is a gateway route (30 -> via 20 -> via 8), which
   mctp_route_lookup() already resolves correctly today for
   sendmsg(). If PLDM instead picks "any" EID and gets 9, the GPU's
   event notifications go out via I3C to the SMA - which has no
   knowledge of EID 9 at all (it's on an unrelated bus). The packet is
   undeliverable at the SMA itself, one hop before it would even reach
   the BMC.

   I acknowledge this is implementation-specific, and additional
   route provisioning on the bridge could help - but that still
   requires out-of-band configuration to stay in sync with every
   local EID. Separately, how device firmware handles Set Event
   Receiver packets carrying an EID different from the one it saw
   during Set Endpoint ID discovery is also implementation-defined.

3. A picked EID doesn't stay valid - interface teardown (hot unplug)
   removes its local EID from the available routes, and Set Event
   Receiver is a one-shot registration with no way to detect that
   drift later. Further events would then be silently dropped by the
   kernel, since it has no route for the removed EID, and would never
   reach the application.

The kernel already walks this resolution (including gateway chains)
correctly for every sendmsg() - this just exposes that same lookup,
rather than requiring applications to parse route/addr output and
re-derive it themselves.

Regards,
Faizan

Re: [PATCH net-next] net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In `mctp_getsockopt()` (net/mctp/af_mctp.c:451-452) a
  *successful* route lookup whose resolved source EID is MCTP_ADDR_NULL…
- [Low] The patch adds a permanent MCTP socket ABI but adds no
  documentation to Documentation/networking/mctp.rst (verified: grep for…

--

net: mctp: add MCTP_OPT_ROUTE_SRCADDR getsockopt

This adds a new SOL_MCTP socket option that takes a destination EID and
network in struct mctp_route_srcaddr and returns the local EID the kernel
would use as the source address. The implementation reuses
mctp_route_lookup() and new kunit cases cover the resolvable and
no-route paths.

Link: https://github.com/CodeConstruct/mctp/issues/147

A couple of questions below about the semantics of the new option.

> diff --git a/include/uapi/linux/mctp.h b/include/uapi/linux/mctp.h
> index 19ad12a0cd4b..7c3d4a936861 100644
> --- a/include/uapi/linux/mctp.h
> +++ b/include/uapi/linux/mctp.h
> @@ -55,6 +55,19 @@ struct mctp_fq_addr {
>  #define MCTP_TAG_PREALLOC	0x10
>  
>  #define MCTP_OPT_ADDR_EXT	1
> +#define MCTP_OPT_ROUTE_SRCADDR	2
> +
> +/* Query structure for MCTP_OPT_ROUTE_SRCADDR getsockopt.
> + *
> + * Caller fills in @net and @daddr before calling getsockopt.
> + * Performs a route lookup and returns the local source EID in @saddr.
> + */
> +struct mctp_route_srcaddr {
> +	unsigned int	net;
> +	mctp_eid_t	daddr;
> +	mctp_eid_t	saddr;
> +	__u8		__pad[2];
> +};

[Severity: Low]
Should this new option also be described in
Documentation/networking/mctp.rst?  A grep for sockopt or MCTP_OPT in
that file returns no matches, while the same file documents the other
userspace-visible MCTP interfaces in detail, including
ioctl(SIOCMCTPALLOCTAG) and ioctl(SIOCMCTPDROPTAG) along with their
"other fields must be zero" rules.

The four-line comment here is the only in-tree description, and it leaves
out several behaviours that the implementation has:

- optval is used as both input and output buffer
- optlen must be exactly sizeof(struct mctp_route_srcaddr)
- net == MCTP_NET_ANY is translated to mctp_default_net()
- the resolved net is not written back into @net
- -EHOSTUNREACH when no route exists
- -EADDRNOTAVAIL when the route resolves with no local EID

On the MCTP_NET_ANY case in particular: since @net is left as 0 on
return, how is a caller on a multi-net system supposed to know which
network the returned EID belongs to?  That is the same ambiguity the
deprecation note on struct mctp_ioc_tag_ctl warns about.  Would writing
the resolved net back into rsa.net be reasonable here?

> diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
> index 8af5e2b3c8d1..5c6077e0367e 100644
> --- a/net/mctp/af_mctp.c
> +++ b/net/mctp/af_mctp.c
> @@ -425,6 +425,37 @@ static int mctp_getsockopt(struct socket *sock, int level, int optname,
>  		return 0;
>  	}
>  
> +	if (optname == MCTP_OPT_ROUTE_SRCADDR) {
> +		struct mctp_route_srcaddr rsa;
> +		struct mctp_dst dst;
> +		unsigned int net;
> +		int rc;
> +
> +		if (len != sizeof(rsa))
> +			return -EINVAL;
> +
> +		if (copy_from_iter(&rsa, len, &opt->iter_in) != len)
> +			return -EFAULT;
> +
> +		net = rsa.net;
> +		if (net == MCTP_NET_ANY)
> +			net = mctp_default_net(sock_net(sock->sk));
> +
> +		rc = mctp_route_lookup(sock_net(sock->sk), net, rsa.daddr, &dst);
> +		if (rc)
> +			return rc;
> +
> +		rsa.saddr = dst.saddr;
> +		mctp_dst_release(&dst);
> +
> +		if (rsa.saddr == MCTP_ADDR_NULL)
> +			return -EADDRNOTAVAIL;

[Severity: Medium]
Is this -EADDRNOTAVAIL consistent with what the output path does for the
same lookup result?

mctp_route_lookup() in net/mctp/route.c deliberately accepts
saddr == MCTP_ADDR_NULL for a direct route, and only rejects it once a
gateway has been traversed:

	if (rt->dst_type == MCTP_ROUTE_DIRECT) {
		mctp_eid_t saddr;
		...
		saddr = mctp_dev_saddr(rt->dev);

		/* cannot do gateway-ed routes without a src  */
		if (saddr == MCTP_ADDR_NULL && depth != 0) {
			mctp_dev_put(rt->dev);
			break;
		}

		if (dst)
			mctp_dst_from_route(dst, daddr, saddr, mtu, rt);

So for a direct route at depth 0 the lookup succeeds with dst.saddr == 0,
and mctp_local_output() then uses it without complaint:

	hdr->dest = daddr;
	hdr->src = dst->saddr;

That state is exercised today by the existing kunit case
mctp_test_route_output_direct_no_eids, where transmit over a direct route
with no local EID is expected to succeed.

So if a direct route exists to daddr on a device with no local address
assigned, sendmsg() transmits with src EID 0 while this new option
reports -EADDRNOTAVAIL.  Does that conflict with the commit message
claim:

    the kernel performs the same route lookup used for actual packet
    output and returns the resolved local EID in saddr

If returning an error for the unassigned-local-EID case is intentional,
could the commit message and the uapi comment say so, and could a kunit
case be added for it?  Neither of the two new cases covers a successful
lookup that yields MCTP_ADDR_NULL.

> +
> +		if (copy_to_iter(&rsa, len, &opt->iter_out) != len)
> +			return -EFAULT;
> +		return 0;
> +	}
> +
>  	return -ENOPROTOOPT;
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921035118.1649687-1-faizana%40nvidia.com