netdev - Re: [PATCH v1 bpf-next 05/10] bpf: tcp: Avoid socket skips and repeats during iteration

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 for Android: free password hash cracker in your pocket

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

Message-ID: <mfrii5tn6wj4bskpoewzsmqmhyc47s5343qrhpq7sdotr54zoe@3kpggsqe4cxi>
Date: Fri, 23 May 2025 18:24:52 -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 05/10] bpf: tcp: Avoid socket skips and
 repeats during iteration

> > +static struct sock *bpf_iter_tcp_resume_listening(struct seq_file *seq)
> > +{
> > +	struct inet_hashinfo *hinfo = seq_file_net(seq)->ipv4.tcp_death_row.hashinfo;
> > +	struct bpf_tcp_iter_state *iter = seq->private;
> > +	struct tcp_iter_state *st = &iter->state;
> > +	unsigned int find_cookie = iter->cur_sk;
> > +	unsigned int end_cookie = iter->end_sk;
> > +	int resume_bucket = st->bucket;
> > +	struct sock *sk;
> > +
> > +	sk = listening_get_first(seq);
> 
> Since it does not advance the sk->bucket++ now, it will still scan until the
> first seq_sk_match()?

Yeah, true. It will waste some time in the current bucket to find the
first match even if iter->cur_sk == iter->end_sk.

> Does it make sense to advance the st->bucket++ in the bpf_iter_tcp_seq_next
> and bpf_iter_tcp_seq_stop?

It seems like this should work. If you're on the last listening bucket
and do st->bucket++ on stop/next, the next call to listening_get_first()
will just return NULL then fall through to the established buckets in
bpf_iter_tcp_resume(). Will need to think through the edge cases a bit
more...

> > +static struct sock *bpf_iter_tcp_resume(struct seq_file *seq)
> > +{
> > +	struct inet_hashinfo *hinfo = seq_file_net(seq)->ipv4.tcp_death_row.hashinfo;
> > +	struct bpf_tcp_iter_state *iter = seq->private;
> > +	struct tcp_iter_state *st = &iter->state;
> > +	struct sock *sk = NULL;
> > +
> > +	switch (st->state) {
> > +	case TCP_SEQ_STATE_LISTENING:
> > +		if (st->bucket > hinfo->lhash2_mask)
> 
> Understood that this is borrowed from the existing tcp_seek_last_pos().
> 
> I wonder if this case would ever be hit. If it is not, may be avoid adding
> it to this new resume function?

Yeah, I think we should be able to get rid of this.
bpf_iter_tcp_resume_listening and bpf_iter_tcp_resume_established should
just fall through and return NULL anyway in these cases.

Jordan