Re: [Patch bpf-next 02/19] skmsg: get rid of struct sk_psock_parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 8, 2021 at 12:39 AM John Fastabend <john.fastabend@xxxxxxxxx> wrote:
>
> Jakub Sitnicki wrote:
> > On Wed, Feb 03, 2021 at 05:16 AM CET, Cong Wang wrote:
> > > From: Cong Wang <cong.wang@xxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> > > Cc: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> > > Cc: Lorenz Bauer <lmb@xxxxxxxxxxxxxx>
> > > Signed-off-by: Cong Wang <cong.wang@xxxxxxxxxxxxx>
> > > ---
> >
> > 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.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux