> > +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
