bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Cong Wang <xiyou.wangcong@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: Jakub Sitnicki <jakub@cloudflare.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>,
	duanxiongchun@bytedance.com,
	Dongdong Wang <wangdongdong.6@bytedance.com>,
	jiang.wang@bytedance.com, Cong Wang <cong.wang@bytedance.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Lorenz Bauer <lmb@cloudflare.com>
Subject: Re: [Patch bpf-next 02/19] skmsg: get rid of struct sk_psock_parser
Date: Mon, 8 Feb 2021 16:19:10 -0800	[thread overview]
Message-ID: <CAM_iQpWK8V4bzv4pK0v45FRb5aG1F=uSN3hrv-Sw92SrttJhQg@mail.gmail.com> (raw)
In-Reply-To: <6020f8b616d4_cc8682087d@john-XPS-13-9370.notmuch>

On Mon, Feb 8, 2021 at 12:39 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Jakub Sitnicki wrote:
> > On Wed, Feb 03, 2021 at 05:16 AM CET, Cong Wang wrote:
> > > From: Cong Wang <cong.wang@bytedance.com>
> > >
> > > struct sk_psock_parser is embedded in sk_psock, it is
> > > unnecessary as skb verdict also uses ->saved_data_ready.
> > > We can simply fold these fields into sk_psock.
> > >
> > > Cc: John Fastabend <john.fastabend@gmail.com>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Jakub Sitnicki <jakub@cloudflare.com>
> > > Cc: Lorenz Bauer <lmb@cloudflare.com>
> > > Signed-off-by: Cong Wang <cong.wang@bytedance.com>
> > > ---
> >
> > This one looks like a candidate for splitting out of the series, as it
> > stands on its own, to make the itself series smaller.
> >
> > Also, it seems that we always have:
> >
> >   parser.enabled/bpf_running == (saved_data_ready != NULL)
> >
> > Maybe parser.enabled can be turned into a predicate function.

Yeah, this looks cleaner.

> >
> > [...]
>
> Agree. To speed this along consider breaking it into three
> series.
>
>  - couple cleanup things: this patch, config option, etc.
>
>  - udp changes
>
>  - af_unix changes.
>
> Although if you really think udp changes and af_unix need to go
> together that is fine imo. I think the basic rule is to try and avoid
> getting patch counts above 10 or so if at all possible.
>
> At least this patch, the renaming patch, and the config patch
> can get pulled out into their own series so we can get those
> merged and out of the way.

Sounds good. Since each patch is already separated, there is no
extra burden on my side to split the whole patchset as suggested
above, except adjusting the cover letters.

Thanks.

  reply	other threads:[~2021-02-09  0:20 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-03  4:16 [Patch bpf-next 00/19] sock_map: add non-TCP and cross-protocol support Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 01/19] bpf: rename BPF_STREAM_PARSER to BPF_SOCK_MAP Cong Wang
2021-02-05 10:32   ` Jakub Sitnicki
2021-02-09  1:40     ` Cong Wang
2021-02-08  8:21   ` John Fastabend
2021-02-08  9:50     ` Lorenz Bauer
2021-02-09  1:45     ` Cong Wang
2021-02-09  6:48       ` John Fastabend
2021-02-03  4:16 ` [Patch bpf-next 02/19] skmsg: get rid of struct sk_psock_parser Cong Wang
2021-02-05 11:25   ` Jakub Sitnicki
2021-02-08  8:39     ` John Fastabend
2021-02-09  0:19       ` Cong Wang [this message]
2021-02-03  4:16 ` [Patch bpf-next 03/19] skmsg: use skb ext instead of TCP_SKB_CB Cong Wang
2021-02-05 22:09   ` Jakub Sitnicki
2021-02-08 18:56     ` Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 04/19] sock_map: rename skb_parser and skb_verdict Cong Wang
2021-02-08  8:27   ` John Fastabend
2021-02-03  4:16 ` [Patch bpf-next 05/19] sock_map: introduce BPF_SK_SKB_VERDICT Cong Wang
2021-02-08  8:31   ` John Fastabend
2021-02-03  4:16 ` [Patch bpf-next 06/19] sock: introduce sk_prot->update_proto() Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 07/19] udp: implement ->sendmsg_locked() Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 08/19] udp: implement ->read_sock() for sockmap Cong Wang
2021-02-08  9:48   ` Lorenz Bauer
2021-02-09  1:35     ` Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 09/19] udp: add ->read_sock() and ->sendmsg_locked() to ipv6 Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 10/19] af_unix: implement ->sendmsg_locked for dgram socket Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 11/19] af_unix: implement ->read_sock() for sockmap Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 12/19] af_unix: implement ->update_proto() Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 13/19] af_unix: set TCP_ESTABLISHED for datagram sockets too Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 14/19] skmsg: extract __tcp_bpf_recvmsg() and tcp_bpf_wait_data() Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 15/19] udp: implement udp_bpf_recvmsg() for sockmap Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 16/19] af_unix: implement unix_dgram_bpf_recvmsg() Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 17/19] sock_map: update sock type checks Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 18/19] selftests/bpf: add test cases for unix and udp sockmap Cong Wang
2021-02-05 10:53   ` Jakub Sitnicki
2021-02-08 18:43     ` Cong Wang
2021-02-03  4:16 ` [Patch bpf-next 19/19] selftests/bpf: add test case for redirection between udp and unix Cong Wang
2021-02-03 17:48 ` [Patch bpf-next 00/19] sock_map: add non-TCP and cross-protocol support Alexei Starovoitov
2021-02-03 19:22   ` Cong Wang
2021-02-03 20:29     ` John Fastabend

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAM_iQpWK8V4bzv4pK0v45FRb5aG1F=uSN3hrv-Sw92SrttJhQg@mail.gmail.com' \
    --to=xiyou.wangcong@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=cong.wang@bytedance.com \
    --cc=daniel@iogearbox.net \
    --cc=duanxiongchun@bytedance.com \
    --cc=jakub@cloudflare.com \
    --cc=jiang.wang@bytedance.com \
    --cc=john.fastabend@gmail.com \
    --cc=lmb@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=wangdongdong.6@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).