Re: [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.

Kuniyuki Iwashima <[email protected]>
Newsgroups dev.linux.lists.sashiko,org.kernel.vger.bpf
Message-ID <CAAVpQUAbUnN_bBfguit7GO1VkQFwO3Oj-1adra820r3RpjO=kw@mail.gmail.com>
On Mon, May 4, 2026 at 9:14 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Newly added code uses manual error checks instead of the mandated modern ASSERT_* macros.
> --
>
> commit 1a574056fa08ab8c66dcd9b6ee5f0739ff6721b7
> Author: Kuniyuki Iwashima <[email protected]>
>
> selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
>
> This commit adds a test to cover bpf_tcp_sock() for wrong socket types,
> specifically tricking it with IPPROTO_TCP on an AF_INET RAW socket. It
> also fixes a missing free() in the test.
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> > index 53637431ec5d..5fd33ad2eaaf 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
>
> [ ... ]
>
> > @@ -211,6 +211,22 @@ static int getsetsockopt(void)
> >       }
> >       ASSERT_EQ(optlen, 8, "Unexpected NETLINK_LIST_MEMBERSHIPS value");
> >
> > +     /* Trick bpf_tcp_sock() with IPPROTO_TCP */
> > +     close(fd);
> > +     fd = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
> > +     if (fd < 0) {
> > +             log_err("Failed to create RAW socket");
> > +             goto err;
> > +     }
>
> This isn't a bug, but should this new code use the modern ASSERT_OK_FD()
> macro instead of manual fd checks and log_err()?

I don't mix different styles in the same file, also such
cleanup should be done in bpf-next.


>
> > +
> > +     optlen = 20;
> > +     errno = 0;
> > +     err = setsockopt(fd, SOL_TCP, TCP_SAVED_SYN, &buf, optlen);
> > +     if (!err) {
> > +             log_err("Unexpected setsockopt(TCP_SAVED_SYN)");
> > +             goto err;
> > +     }
>
> Could we use ASSERT_ERR() for this error check as well?

Same.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.