Message-ID: <734b02a3-1941-49df-a0da-ec14310d41e4@ghiti.fr>
Date: Mon, 11 Aug 2025 13:35:12 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Matt Johnston <matt@...econstruct.com.au>,
 Jeremy Kerr <jk@...econstruct.com.au>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Alexandre Ghiti <alex@...ti.fr>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next 7/7] net: mctp: Add bind lookup test

Hi Matt,

On 7/3/25 11:11, Matt Johnston wrote:
> Test the preference order of bound socket matches with a series of test
> packets.
>
> Signed-off-by: Matt Johnston <matt@...econstruct.com.au>
> ---
>   net/mctp/test/route-test.c | 184 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 182 insertions(+), 2 deletions(-)
>
> diff --git a/net/mctp/test/route-test.c b/net/mctp/test/route-test.c
> index fa33d44399f14fd019c82fd5182b65b3457825e2..af62b3378c3e9b2d2be30bc32c1ac34df2a4a78a 100644
> --- a/net/mctp/test/route-test.c
> +++ b/net/mctp/test/route-test.c
> @@ -1245,6 +1245,9 @@ struct mctp_test_bind_setup {
>   	bool have_peer;
>   	mctp_eid_t peer_addr;
>   	int peer_net;
> +
> +	/* optional name. Used for comparison in "lookup" tests */
> +	const char *name;
>   };
>   
>   static const struct mctp_test_bind_setup bind_addrany_netdefault_type1 = {
> @@ -1384,11 +1387,17 @@ static void mctp_test_bind_invalid(struct kunit *test)
>   	int rc;
>   
>   	/* bind() fails if the bind() vs connect() networks mismatch. */
> -	const struct mctp_test_bind_setup bind_connect_net_mismatch = {
> +	const struct mctp_test_bind_setup bind_connect_net_mismatch1 = {
>   		.bind_addr = MCTP_ADDR_ANY, .bind_net = 1, .bind_type = 1,
>   		.have_peer = true, .peer_addr = 9, .peer_net = 2,
>   	};
> -	mctp_test_bind_run(test, &bind_connect_net_mismatch, &rc, &sock);
> +	const struct mctp_test_bind_setup bind_connect_net_mismatch5 = {
> +		.bind_addr = MCTP_ADDR_ANY, .bind_net = 5, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 9, .peer_net = 2,
> +	};
> +
> +	mctp_test_bind_run(test, &bind_connect_net_mismatch1, &rc, &sock);
> +	mctp_test_bind_run(test, &bind_connect_net_mismatch5, &rc, &sock);
>   	KUNIT_EXPECT_EQ(test, -rc, EINVAL);
>   	sock_release(sock);
>   }
> @@ -1436,6 +1445,176 @@ static void mctp_test_bind_conflicts(struct kunit *test)
>   	KUNIT_EXPECT_EQ(test, -bind_errno, pair->error);
>   }
>   
> +struct mctp_bind_lookup_test {
> +	/* header of incoming message */
> +	struct mctp_hdr hdr;
> +	u8 ty;
> +	/* mctp network of incoming interface (smctp_network) */
> +	unsigned int net;
> +
> +	/* expected socket, matches .name in lookup_binds, or NULL for dropped */
> +	const char *expect;
> +};
> +
> +/* Single-packet TO-set message */
> +#define LK(src, dst) RX_HDR(1, (src), (dst), FL_S | FL_E | FL_TO)
> +
> +/* Input message test cases for bind lookup tests.
> + *
> + * 10 and 11 are local EIDs.
> + * 20 and 21 are remote EIDs.
> + */
> +static const struct mctp_bind_lookup_test mctp_bind_lookup_tests[] = {
> +	/* have both local-eid and remote-eid binds, remote eid is preferenced */
> +	{ .hdr = LK(20, 10),  .ty = 1, .net = 1, .expect = "remote20" },
> +
> +	{ .hdr = LK(20, 255), .ty = 1, .net = 1, .expect = "remote20" },
> +	{ .hdr = LK(20, 0),   .ty = 1, .net = 1, .expect = "remote20" },
> +	{ .hdr = LK(0, 255),  .ty = 1, .net = 1, .expect = "any" },
> +	{ .hdr = LK(0, 11),   .ty = 1, .net = 1, .expect = "any" },
> +	{ .hdr = LK(0, 0),    .ty = 1, .net = 1, .expect = "any" },
> +	{ .hdr = LK(0, 10),   .ty = 1, .net = 1, .expect = "local10" },
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 1, .expect = "local10" },
> +	{ .hdr = LK(21, 11),  .ty = 1, .net = 1, .expect = "remote21local11" },
> +
> +	/* both src and dest set to eid=99. unusual, but accepted
> +	 * by MCTP stack currently.
> +	 */
> +	{ .hdr = LK(99, 99),  .ty = 1, .net = 1, .expect = "any" },
> +
> +	/* unbound smctp_type */
> +	{ .hdr = LK(20, 10),  .ty = 3, .net = 1, .expect = NULL },
> +
> +	/* smctp_network tests */
> +
> +	{ .hdr = LK(0, 0),    .ty = 1, .net = 7, .expect = "any" },
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 2, .expect = "any" },
> +
> +	/* remote EID 20 matches, but MCTP_NET_ANY in "remote20" resolved to net=1,
> +	 * so lookup doesn't match "remote20"
> +	 */
> +	{ .hdr = LK(20, 10),  .ty = 1, .net = 3, .expect = "any" },
> +
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 3, .expect = "remote21net3" },
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 4, .expect = "remote21net4" },
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 5, .expect = "remote21net5" },
> +
> +	{ .hdr = LK(21, 10),  .ty = 1, .net = 5, .expect = "remote21net5" },
> +
> +	{ .hdr = LK(99, 10),  .ty = 1, .net = 8, .expect = "local10net8" },
> +
> +	{ .hdr = LK(99, 10),  .ty = 1, .net = 9, .expect = "anynet9" },
> +	{ .hdr = LK(0, 0),    .ty = 1, .net = 9, .expect = "anynet9" },
> +	{ .hdr = LK(99, 99),  .ty = 1, .net = 9, .expect = "anynet9" },
> +	{ .hdr = LK(20, 10),  .ty = 1, .net = 9, .expect = "anynet9" },
> +};
> +
> +/* Binds to create during the lookup tests */
> +static const struct mctp_test_bind_setup lookup_binds[] = {
> +	/* any address and net, type 1 */
> +	{ .name = "any", .bind_addr = MCTP_ADDR_ANY, .bind_net = MCTP_NET_ANY, .bind_type = 1, },
> +	/* local eid 10, net 1 (resolved from MCTP_NET_ANY) */
> +	{ .name = "local10", .bind_addr = 10, .bind_net = MCTP_NET_ANY, .bind_type = 1, },
> +	/* local eid 10, net 8 */
> +	{ .name = "local10net8", .bind_addr = 10, .bind_net = 8, .bind_type = 1, },
> +	/* any EID, net 9 */
> +	{ .name = "anynet9", .bind_addr = MCTP_ADDR_ANY, .bind_net = 9, .bind_type = 1, },
> +
> +	/* remote eid 20, net 1, any local eid */
> +	{ .name = "remote20", .bind_addr = MCTP_ADDR_ANY, .bind_net = MCTP_NET_ANY, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 20, .peer_net = MCTP_NET_ANY, },
> +
> +	/* remote eid 20, net 1, local eid 11 */
> +	{ .name = "remote21local11", .bind_addr = 11, .bind_net = MCTP_NET_ANY, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 21, .peer_net = MCTP_NET_ANY, },
> +
> +	/* remote eid 21, specific net=3 for connect() */
> +	{ .name = "remote21net3", .bind_addr = MCTP_ADDR_ANY,
> +		.bind_net = MCTP_NET_ANY, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 21, .peer_net = 3, },
> +
> +	/* remote eid 21, net 4 for bind, specific net=4 for connect() */
> +	{ .name = "remote21net4", .bind_addr = MCTP_ADDR_ANY, .bind_net = 4, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 21, .peer_net = 4, },
> +
> +	/* remote eid 21, net 5 for bind, specific net=5 for connect() */
> +	{ .name = "remote21net5", .bind_addr = MCTP_ADDR_ANY, .bind_net = 5, .bind_type = 1,
> +		.have_peer = true, .peer_addr = 21, .peer_net = 5, },
> +};
> +
> +static void mctp_bind_lookup_desc(const struct mctp_bind_lookup_test *t, char *desc)
> +{
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "{src %d dst %d ty %d net %d expect %s}",
> +		 t->hdr.src, t->hdr.dest, t->ty, t->net, t->expect);
> +}
> +
> +KUNIT_ARRAY_PARAM(mctp_bind_lookup, mctp_bind_lookup_tests, mctp_bind_lookup_desc);
> +
> +static void mctp_test_bind_lookup(struct kunit *test)
> +{
> +	const struct mctp_bind_lookup_test *rx;
> +	struct socket *socks[ARRAY_SIZE(lookup_binds)];
> +	struct sk_buff *skb_pkt = NULL, *skb_sock = NULL;
> +	struct mctp_test_route *rt;
> +	struct mctp_test_dev *dev;
> +	struct socket *sock_ty0, *sock_expect = NULL;
> +	int rc;
> +
> +	rx = test->param_value;
> +
> +	__mctp_route_test_init(test, &dev, &rt, &sock_ty0, rx->net);
> +	/* Create all binds */
> +	for (size_t i = 0; i < ARRAY_SIZE(lookup_binds); i++) {
> +		mctp_test_bind_run(test, &lookup_binds[i],
> +				   &rc, &socks[i]);
> +		KUNIT_ASSERT_EQ(test, rc, 0);
> +
> +		/* Record the expected receive socket */
> +		if (rx->expect && strcmp(rx->expect, lookup_binds[i].name) == 0) {
> +			KUNIT_ASSERT_NULL(test, sock_expect);
> +			sock_expect = socks[i];
> +		}
> +	}
> +	KUNIT_ASSERT_EQ(test, !!sock_expect, !!rx->expect);
> +
> +	/* Create test message */
> +	skb_pkt = mctp_test_create_skb_data(&rx->hdr, &rx->ty);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, skb_pkt);
> +	mctp_test_skb_set_dev(skb_pkt, dev);
> +
> +	rc = mctp_route_input(&rt->rt, skb_pkt);
> +	if (rx->expect) {
> +		/* Test the message is received on the expected socket */
> +		KUNIT_EXPECT_EQ(test, rc, 0);
> +		skb_sock = skb_recv_datagram(sock_expect->sk, MSG_DONTWAIT, &rc);
> +		if (!skb_sock) {
> +			/* Find which socket received it instead */
> +			for (size_t i = 0; i < ARRAY_SIZE(lookup_binds); i++) {
> +				skb_sock = skb_recv_datagram(socks[i]->sk, MSG_DONTWAIT, &rc);
> +				if (skb_sock) {
> +					KUNIT_FAIL(test,
> +						   "received on incorrect socket '%s', expect '%s'",
> +						   lookup_binds[i].name, rx->expect);
> +					goto cleanup;
> +				}
> +			}
> +			KUNIT_FAIL(test, "no message received");
> +		}
> +	} else {
> +		KUNIT_EXPECT_NE(test, rc, 0);
> +	}
> +
> +cleanup:
> +	kfree_skb(skb_sock);
> +	kfree_skb(skb_pkt);
> +
> +	/* Drop all binds */
> +	for (size_t i = 0; i < ARRAY_SIZE(lookup_binds); i++)
> +		sock_release(socks[i]);
> +
> +	__mctp_route_test_fini(test, dev, rt, sock_ty0);
> +}
> +
>   static void mctp_test_assumptions(struct kunit *test)
>   {
>   	/* check assumption of default net from bind_addr8_net1_type1 */
> @@ -1461,6 +1640,7 @@ static struct kunit_case mctp_test_cases[] = {
>   	KUNIT_CASE(mctp_test_route_input_cloned_frag),
>   	KUNIT_CASE_PARAM(mctp_test_bind_conflicts, mctp_bind_pair_gen_params),
>   	KUNIT_CASE(mctp_test_bind_invalid),
> +	KUNIT_CASE_PARAM(mctp_test_bind_lookup, mctp_bind_lookup_gen_params),
>   
>   	{ /* terminator */ },
>   };
>
I'm hitting different issues with this test.

With a simple riscv defconfig, I get the following warning a bit after 
the test succeeds:

[    5.991567] WARNING: CPU: 1 PID: 0 at net/netlink/af_netlink.c:402 
netlink_sock_destruct+0x3a/0x6a
[    5.992039] Modules linked in:
[    5.992384] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: 
G                 N  6.17.0-rc1-00005-g1692753ec9e8-dirty #40 NONE
[    5.992502] Tainted: [N]=TEST
[    5.992521] Hardware name: riscv-virtio,qemu (DT)
[    5.992604] epc : netlink_sock_destruct+0x3a/0x6a
[    5.992643]  ra : netlink_sock_destruct+0x18/0x6a
[    5.992669] epc : ffffffff809bdf26 ra : ffffffff809bdf04 sp : 
ff2000000000bd90
[    5.992685]  gp : ffffffff81a1b458 tp : ff600000802ce000 t0 : 
0000000000000040
[    5.992700]  t1 : 0000000000000000 t2 : 0000000080150012 s0 : 
ff2000000000bdb0
[    5.992715]  s1 : ff600000825a0800 a0 : ff600000825a08a8 a1 : 
0000000000000068
[    5.992729]  a2 : 0000000000000008 a3 : 000000000000000a a4 : 
0000000000000000
[    5.992743]  a5 : ffffffffffffce00 a6 : ffffffffffffffff a7 : 
ffffffff81a20810
[    5.992757]  s2 : ff600000825a0800 s3 : ffffffff81a7c040 s4 : 
ff2000000000be60
[    5.992770]  s5 : 0000000000000000 s6 : 000000000000000a s7 : 
0000000000000000
[    5.992784]  s8 : ff600003fff1e580 s9 : 0000000000000001 s10: 
ffffffff81890a80
[    5.992798]  s11: ff600000825a0c28 t3 : 0000000000000002 t4 : 
0000000000000402
[    5.992811]  t5 : ff60000080069168 t6 : ff60000080069170
[    5.992824] status: 0000000200000120 badaddr: ffffffff809bdf26 cause: 
0000000000000003
[    5.992925] [<ffffffff809bdf26>] netlink_sock_destruct+0x3a/0x6a
[    5.993015] [<ffffffff809369b8>] __sk_destruct+0x22/0x1aa
[    5.993034] [<ffffffff809385b0>] sk_destruct+0x46/0x50
[    5.993049] [<ffffffff8093860a>] __sk_free+0x50/0xc6
[    5.993062] [<ffffffff809386aa>] sk_free+0x2a/0x46
[    5.993077] [<ffffffff809bdb08>] deferred_put_nlk_sk+0x42/0x62
[    5.993089] [<ffffffff800a5b80>] rcu_core+0x1ba/0x5c8
[    5.993106] [<ffffffff800a5f9a>] rcu_core_si+0xc/0x14
[    5.993119] [<ffffffff8002d8de>] handle_softirqs+0x110/0x2e8
[    5.993133] [<ffffffff8002dbd2>] __irq_exit_rcu+0xd0/0xfa
[    5.993144] [<ffffffff8002ddbe>] irq_exit_rcu+0xc/0x14
[    5.993156] [<ffffffff80b438e0>] handle_riscv_irq+0x64/0x74
[    5.993177] [<ffffffff80b4f88a>] call_on_irq_stack+0x32/0x40

The following diff fixes it (completely unsure this is the right fix 
though):

diff --git a/net/mctp/test/route-test.c b/net/mctp/test/route-test.c
index fb6b46a952cb4..03f2f76c97f75 100644
--- a/net/mctp/test/route-test.c
+++ b/net/mctp/test/route-test.c
@@ -1585,7 +1585,6 @@ static void mctp_test_bind_lookup(struct kunit *test)
         }

  cleanup:
-       kfree_skb(skb_sock);
         kfree_skb(skb_pkt);

         /* Drop all binds */


But then with a more consequent config attached, even with the previous 
fix, I'm hitting:

[  137.494285] kernel BUG at mm/slub.c:563!
[  137.494365] Kernel BUG [#2]
[  137.494388] Modules linked in:
[  137.494449] CPU: 4 UID: 0 PID: 4699 Comm: kunit_try_catch Tainted: 
G      D W        N  6.17.0-rc1+ #16 PREEMPT(voluntary)
[  137.494500] Tainted: [D]=DIE, [W]=WARN, [N]=TEST
[  137.494514] Hardware name: riscv-virtio,qemu (DT)
[  137.494534] epc : kmem_cache_free+0x334/0x39e
[  137.494588]  ra : kmem_cache_free+0x76/0x39e
[  137.494617] epc : ffffffff802e8ec4 ra : ffffffff802e8c06 sp : 
ff2000000b6cbb60
[  137.494637]  gp : ffffffff82ac7b70 tp : ff60000087cc1f00 t0 : 
ff60000087ba6820
[  137.494657]  t1 : 0000000000000000 t2 : 0000000000000000 s0 : 
ff2000000b6cbbb0
[  137.494676]  s1 : ffffffff80c34db8 a0 : ff1c0000021ee900 a1 : 
ff60000087ba6680
[  137.494695]  a2 : ff60000087ba6680 a3 : ffffffff82e4a858 a4 : 
ff600003fff21d80
[  137.494715]  a5 : ff60000087ba67e0 a6 : 000000000002dc04 a7 : 
0000000000000000
[  137.494733]  s2 : ff60000087ba6680 s3 : ff600000802cc900 s4 : 
ff1c0000021ee900
[  137.494754]  s5 : 0000000000000002 s6 : ff600000833d9b00 s7 : 
ff60000087b55a80
[  137.494772]  s8 : 0000000000000000 s9 : ff2000000b6cbd18 s10: 
ffffffff81e29a90
[  137.494791]  s11: 000000000000060e t3 : 0000000000000000 t4 : 
0000000000000000
[  137.494810]  t5 : 0000000000000000 t6 : ff60000087ba6685
[  137.494826] status: 0000000200000120 badaddr: ffffffff802e8ec4 cause: 
0000000000000003
[  137.494849] [<ffffffff802e8ec4>] kmem_cache_free+0x334/0x39e
[  137.494883] [<ffffffff80c34db8>] skb_free_head+0xca/0x112
[  137.494922] [<ffffffff80c37bba>] skb_release_data+0xe2/0x112
[  137.494952] [<ffffffff80c3a202>] sk_skb_reason_drop+0x40/0x124
[  137.494984] [<ffffffff80e57afe>] mctp_test_bind_lookup+0x15e/0x3f4
[  137.495019] [<ffffffff80623454>] kunit_try_run_case+0x4c/0x13e
[  137.495051] [<ffffffff80625a10>] 
kunit_generic_run_threadfn_adapter+0x1a/0x34
[  137.495080] [<ffffffff80064a9c>] kthread+0xea/0x1c2
[  137.495111] [<ffffffff8001b42e>] ret_from_fork_kernel+0x10/0x162
[  137.495139] [<ffffffff80e9d192>] ret_from_fork_kernel_asm+0x16/0x18
[  137.495199] Code: 80a3 00e7 5097 ffd5 80e7 9880 9002 b715 ec56 e85a 
(9002) 9b61
[  137.495760] ---[ end trace 0000000000000000 ]---

Let me know if I can do anything to help.

Thanks,

Alex

View attachment "ubuntu_defconfig" of type "text/plain" (288716 bytes)

Powered by blists - more mailing lists