Re: [PATCH bpf-next v6 5/5] bpf/selftests: add selftest for bpf_smc_ops

["D. Wythe" <alibuda@linux.alibaba.com >]

On Fri, Jan 17, 2025 at 04:37:04PM -0800, Martin KaFai Lau wrote:
> On 1/15/25 11:44 PM, D. Wythe wrote:
> >This tests introduces a tiny smc_ops for filtering SMC connections based on
> >IP pairs, and also adds a realistic topology model to verify this ops.
> >
> >Also, we can only use SMC loopback under CI test, so an
> >additional configuration needs to be enabled.
> >
> >Follow the steps below to run this test.
> >
> >make -C tools/testing/selftests/bpf
> >cd tools/testing/selftests/bpf
> >sudo ./test_progs -t smc
> >
> >Results shows:
> >Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
> >
> >Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> >---
> >  tools/testing/selftests/bpf/config            |   4 +
> >  .../selftests/bpf/prog_tests/test_bpf_smc.c   | 397 ++++++++++++++++++
> >  tools/testing/selftests/bpf/progs/bpf_smc.c   | 117 ++++++
> >  3 files changed, 518 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_smc.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/bpf_smc.c
> >
> >diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> >index c378d5d07e02..fac2f2a9d02f 100644
> >--- a/tools/testing/selftests/bpf/config
> >+++ b/tools/testing/selftests/bpf/config
> >@@ -113,3 +113,7 @@ CONFIG_XDP_SOCKETS=y
> >  CONFIG_XFRM_INTERFACE=y
> >  CONFIG_TCP_CONG_DCTCP=y
> >  CONFIG_TCP_CONG_BBR=y
> >+CONFIG_INFINIBAND=y
> >+CONFIG_SMC=y
> >+CONFIG_SMC_OPS=y
> >+CONFIG_SMC_LO=y
> >\ No newline at end of file
> >+	int fd, ret;
> >+	pid_t pid;
> >+
> >+	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
> >+	if (!ASSERT_GT(fd, 0, "nl_family socket"))
> 
> Should be _GE. or just use ASSERT_OK_FD.
> 
Take it.
> >+	if (!ASSERT_GE(ret, 0, "nl_family bind"))
> 
> nit. ASSERT_OK.
> 
> >+	if (!ASSERT_EQ(ret, 0, "nl_family query"))
> 
> ASSERT_OK.
> 
> >+	if (!ASSERT_GT(fd, 0, "ueid socket"))
> 
> ASSERT_OK_FD
> 
> >+		return false;
> >+	ret = bind(fd, (struct sockaddr *) &nl_src, sizeof(nl_src));
> >+	if (!ASSERT_GE(ret, 0, "ueid bind"))
> 
> ASSERT_OK
> 
> >+		goto fail;
> >+	ret = send_cmd(fd, smc_nl_family_id, pid,
> >+		       (void *)test_ueid, sizeof(test_ueid));
> >+	if (!ASSERT_EQ(ret, 0, "ueid cmd"))
> 
> ASSERT_OK
> 

The parts of the assert macro have been all fixed, thanks for your
suggestion.

> >+		goto fail;
> >+
> >+int BPF_PROG(bpf_smc_switch_to_fallback, struct smc_sock___local *smc)
> >+{
> >+	/* only count from one side (client) */
> >+	if (smc && !BPF_CORE_READ(smc, listen_smc))
> 
> It should not need BPF_CORE_READ. smc can be directly read like the
> above sock->sk->...

Got it. I'll fix it in next version.

Thanks,
D. Wythe

> 
> >+		fallback_cnt++;
> >+	return 0;
> >+}