On Tue, Jan 07, 2025 at 04:48:51PM -0800, Martin KaFai Lau wrote:
> On 1/6/25 8:17 PM, D. Wythe wrote:
> >+static int send_cmd(int fd, __u16 nlmsg_type, __u32 nlmsg_pid, __u16 nlmsg_flags,
> >+ __u8 genl_cmd, __u16 nla_type,
> >+ while ((r = sendto(fd, buf, buflen, 0, (struct sockaddr *) &nladdr,
> >+ sizeof(nladdr))) < buflen) {
> >+ if (r > 0) {
> >+ buf += r;
> >+ buflen -= r;
> >+ } else if (errno != EAGAIN)
> >+ return -1;
> >+ }
>
> The "}" indentation is off.
>
> I was wondering if it missed a "}" for the while loop. Turns out the
> "else if" does not have braces while the "if" has. I would add
> braces to the "else if" also to avoid confusion like this.
>
Take it. I fix change it in next version.
> >+ return 0;
> >+}
> >+
> >+static bool get_smc_nl_family_id(void)
> >+{
> >+ struct sockaddr_nl nl_src;
> >+ struct msgtemplate msg;
> >+ ret = send_cmd(fd, smc_nl_family_id, pid,
> >+ NLM_F_REQUEST | NLM_F_ACK, op, SMC_NLA_EID_TABLE_ENTRY,
> >+ (void *)test_ueid, sizeof(test_ueid));
>
> Same. Indentation is off.
Take it. Thanks for pointing it out.
>
> >+ if (!ASSERT_EQ(ret, 0, "ueid cmd"))
> >+ goto fail;
> >+
> >+ nstoken = open_netns(TEST_NS);
>
> Instead of make_netns and then immediately open_netns, try
> netns_new(TEST_NS, true) from the test_progs.c.
Got it, I'll try it in next version.
>
> >+ if (!ASSERT_OK_PTR(nstoken, "open net namespace"))
> >+ goto fail_open_netns;
> >+
> >+ if (!ASSERT_OK(system("ip addr add 127.0.1.0/8 dev lo"), "add server node"))
> >+ goto fail_ip;
> >+
> >+ if (!ASSERT_OK(system("ip addr add 127.0.2.0/8 dev lo"), "server via risk path"))
> >+ close_netns(nstoken);
> >+ return false;
> >+}
> >+
> >+ /* Configure ip strat */
> >+ block_link(map_fd, CLIENT_IP, SERVER_IP_VIA_RISK_PATH);
> >+ block_link(map_fd, SERVER_IP, SERVER_IP);
> >+ close(map_fd);
>
> No need to close(map-fd) here. bpf_smc__destroy(skel) will do it.
Got it. Many thanks.
>
> It seems the new selftest fails also. not always though which is concerning.
>
This might not be a random failure, but rather related to s390x, which
carries a seid by default, which may affect my action of deleting ueid.
I am requesting IBM folks to help me analyze this issue since i have no
s390x machine.
Anyway, I will solve it in the next version.
Best wishes,
D. Wythe
> pw-bot: cr
>
> >+
> >+ /* should go with smc */
> >+ run_link(CLIENT_IP, SERVER_IP, SERVICE_1);
> >+ /* should go with smc fallback */
> >+ run_link(SERVER_IP, SERVER_IP, SERVICE_2);
> >+
> >+ ASSERT_EQ(skel->bss->smc_cnt, 2, "smc count");
> >+ ASSERT_EQ(skel->bss->fallback_cnt, 1, "fallback count");
> >+
> >+ /* should go with smc */
> >+ run_link(CLIENT_IP, SERVER_IP, SERVICE_2);
> >+
> >+ ASSERT_EQ(skel->bss->smc_cnt, 3, "smc count");
> >+ ASSERT_EQ(skel->bss->fallback_cnt, 1, "fallback count");
> >+
> >+ /* should go with smc fallback */
> >+ run_link(CLIENT_IP, SERVER_IP_VIA_RISK_PATH, SERVICE_3);
> >+
> >+ ASSERT_EQ(skel->bss->smc_cnt, 4, "smc count");
> >+ ASSERT_EQ(skel->bss->fallback_cnt, 2, "fallback count");
> >+
> >+fail:
> >+ bpf_smc__destroy(skel);
> >+}
>
