Re: [PATCH bpf v2 2/2] selftests/bpf: Add protocol check test for bpf_sk_assign_tcp_reqsk()

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 3/27/26 1:33 AM, Kuniyuki Iwashima wrote:
[...]
>>          for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
>> @@ -145,6 +145,82 @@ void test_tcp_custom_syncookie(void)
>>
>>   destroy_skel:
>>          system("tc qdisc del dev lo clsact");
>> +       test_tcp_custom_syncookie__destroy(skel);
>> +}
>>
>> +/* Test: bpf_sk_assign_tcp_reqsk() should reject non-TCP skb.
>> + *
>> + * Send a UDP packet through TC ingress where a BPF program calls
>> + * bpf_sk_assign_tcp_reqsk() on it. The kfunc should return an error
>> + * because the skb carries UDP, not TCP.
>> + */
>> +void test_tcp_custom_syncookie_protocol_check(void)
>> +{
>> +       int tcp_server = -1, udp_server = -1, udp_client = -1;
>> +       struct test_tcp_custom_syncookie *skel;
>> +       struct sockaddr_in udp_addr;
>> +       char buf[32] = "test";
>> +       int udp_port, ret;
>> +
>> +       if (setup_netns())
>> +               return;
>> +
>> +       skel = test_tcp_custom_syncookie__open_and_load();
>> +       if (!ASSERT_OK_PTR(skel, "open_and_load"))
>> +               return;
>> +
>> +       /* Create a TCP listener so the BPF can find a LISTEN socket */
>> +       tcp_server = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", 0, 0);
> Can you add IPv6 test as well ?
> You can reuse test_tcp_custom_syncookie_case[].
>
>
>> +       if (!ASSERT_NEQ(tcp_server, -1, "start tcp_server"))
>> +               goto destroy_skel;
>> +
>> +       /* Create a UDP server to receive the packet as synchronization */
>> +       udp_server = start_server(AF_INET, SOCK_DGRAM, "127.0.0.1", 0, 0);
> You can specify the port to get_socket_local_port(tcp_server),
>
>
>> +       if (!ASSERT_NEQ(udp_server, -1, "start udp_server"))
>> +               goto close_tcp;
>> +
>> +       skel->bss->tcp_listener_port = ntohs(get_socket_local_port(tcp_server));
>> +       udp_port = ntohs(get_socket_local_port(udp_server));
>> +       skel->bss->udp_test_port = udp_port;
> then the 3 lines above will be unnecessary,
>
>
>> +
>> +       ret = bpf_program__fd(skel->progs.tcp_custom_syncookie_badproto);
>> +       if (setup_tc(ret))
>> +               goto close_udp_server;
>> +
>> +       udp_client = socket(AF_INET, SOCK_DGRAM, 0);
>> +       if (!ASSERT_NEQ(udp_client, -1, "udp socket"))
>> +               goto cleanup_tc;
>> +
>> +       memset(&udp_addr, 0, sizeof(udp_addr));
>> +       udp_addr.sin_family = AF_INET;
>> +       udp_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
>> +       udp_addr.sin_port = htons(udp_port);
> and you can reuse get_socket_local_port(tcp_server) here too.
>
>
>> +
>> [...]
>> +       iph = (struct iphdr *)(eth + 1);
>> +       if (iph + 1 > data_end)
>> +               return TC_ACT_OK;
>> +
>> +       if (iph->protocol != IPPROTO_UDP)
>> +               return TC_ACT_OK;
>> +
>> +       udp = (struct udphdr *)(iph + 1);
>> +       if (udp + 1 > data_end)
>> +               return TC_ACT_OK;
>> +
>> +       if (bpf_ntohs(udp->dest) != udp_test_port)
>> +               return TC_ACT_OK;
> You don't need to worry about other program sending UDP
> packets in this netns created by unshare().
>
>
>> +
>> +       udp_intercepted = true;
>> +
>> +       tuple.ipv4.saddr = iph->saddr;
>> +       tuple.ipv4.daddr = iph->daddr;
>> +       tuple.ipv4.sport = udp->source;
>> +       tuple.ipv4.dport = bpf_htons(tcp_listener_port);
> and you can simply reuse dport here too.
>

Thanks for the review. I'll update the code to address all these points.