netdev - Re: [PATCH v1 bpf-next 10/10] selftests/bpf: Add tests for bucket resume logic in established sockets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <oqnwzaemu6agvwqt6vgcjygklzhcvxghbzzi7x65dqapzsjsxh@rif4xe655t4u>
Date: Wed, 28 May 2025 11:32:06 -0700
From: Jordan Rife <jordan@...fe.io>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Daniel Borkmann <daniel@...earbox.net>, 
	Willem de Bruijn <willemdebruijn.kernel@...il.com>, Kuniyuki Iwashima <kuniyu@...zon.com>, 
	Alexei Starovoitov <alexei.starovoitov@...il.com>, netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v1 bpf-next 10/10] selftests/bpf: Add tests for bucket
 resume logic in established sockets

> > +
> > +		close(iter_fd);
> > +
> > +		if (!exists)
> > +			break;
> > +
> > +		usleep(100 * us_per_ms);
> 
> Instead of retrying with the bpf_iter_tcp to confirm the sk is gone from the
> ehash table, I think the bpf_sock_destroy() can help here.

Sure, I will explore this. I was a little worried about having a sleep
here, since it may introduce some flakiness into CI at some point, so
it would be good to have something more deterministic.

> 
> > +	}
> > +
> > +	return !exists;
> > +}
> > +
> >   static int get_seen_count(int fd, struct sock_count counts[], int n)
> >   {
> >   	__u64 cookie = socket_cookie(fd);
> > @@ -241,6 +279,43 @@ static void remove_seen(int family, int sock_type, const char *addr, __u16 port,
> >   			       counts_len);
> >   }
> > +static void remove_seen_established(int family, int sock_type, const char *addr,
> > +				    __u16 port, int *listen_socks,
> > +				    int listen_socks_len, int *established_socks,
> > +				    int established_socks_len,
> > +				    struct sock_count *counts, int counts_len,
> > +				    struct bpf_link *link, int iter_fd)
> > +{
> > +	int close_idx;
> > +
> > +	/* Iterate through all listening sockets. */
> > +	read_n(iter_fd, listen_socks_len, counts, counts_len);
> > +
> > +	/* Make sure we saw all listening sockets exactly once. */
> > +	check_n_were_seen_once(listen_socks, listen_socks_len, listen_socks_len,
> > +			       counts, counts_len);
> > +
> > +	/* Leave one established socket. */
> > +	read_n(iter_fd, established_socks_len - 1, counts, counts_len);
> 
> hmm... In the "SEC("iter/tcp") int iter_tcp_soreuse(...)" bpf prog, there is
> a "sk->sk_state != TCP_LISTEN" check and the established sk should have been
> skipped. Does it have an existing bug? I suspect it is missing a "()" around
> "sk->sk_family == AF_INET6 ? !ipv6_addr_loopback(...) : ...".

Agh, yeah it looks like it's the "()". Adding these around the ternary
operation leads to the expected result with all established sockets
being skipped. I will fix that in the next spin.

Jordan