| Message ID | 20210517225308.720677-1-me@ubique.spb.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpfilter | expand |
> On May 17, 2021, at 3:52 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > The patchset is based on the patches from David S. Miller [1] and Daniel > Borkmann [2]. > > The main goal of the patchset is to prepare bpfilter for iptables' > configuration blob parsing and code generation. > > The patchset introduces data structures and code for matches, targets, rules > and tables. > > It seems inconvenient to continue to use the same blob internally in bpfilter > in parts other than the blob parsing. That is why a superstructure with native > types is introduced. It provides a more convenient way to iterate over the blob > and limit the crazy structs widespread in the bpfilter code. > [...] > > > 1. https://lore.kernel.org/patchwork/patch/902785/ [1] used bpfilter_ prefix on struct definitions, like "struct bpfilter_target" I think we should do the same in this version. (Or were there discussions on removing the prefix?). Thanks, Song [...]
On Thu, May 20, 2021 at 04:54:45AM +0000, Song Liu wrote: > > > > On May 17, 2021, at 3:52 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > > > The patchset is based on the patches from David S. Miller [1] and Daniel > > Borkmann [2]. > > > > The main goal of the patchset is to prepare bpfilter for iptables' > > configuration blob parsing and code generation. > > > > The patchset introduces data structures and code for matches, targets, rules > > and tables. > > > > It seems inconvenient to continue to use the same blob internally in bpfilter > > in parts other than the blob parsing. That is why a superstructure with native > > types is introduced. It provides a more convenient way to iterate over the blob > > and limit the crazy structs widespread in the bpfilter code. > > > > [...] > > > > > > > 1. https://lore.kernel.org/patchwork/patch/902785/ > > [1] used bpfilter_ prefix on struct definitions, like "struct bpfilter_target" > I think we should do the same in this version. (Or were there discussions on > removing the prefix?). There were no discussions about it. As those structs are private to bpfilter I assumed that it is safe to save some characters. I will add the prefix to all internal structs in the next iteration. > > Thanks, > Song > > [...] >
On Thu, May 20, 2021 at 12:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > On Thu, May 20, 2021 at 04:54:45AM +0000, Song Liu wrote: > > > > > > > On May 17, 2021, at 3:52 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > > > > > The patchset is based on the patches from David S. Miller [1] and Daniel > > > Borkmann [2]. > > > > > > The main goal of the patchset is to prepare bpfilter for iptables' > > > configuration blob parsing and code generation. > > > > > > The patchset introduces data structures and code for matches, targets, rules > > > and tables. > > > > > > It seems inconvenient to continue to use the same blob internally in bpfilter > > > in parts other than the blob parsing. That is why a superstructure with native > > > types is introduced. It provides a more convenient way to iterate over the blob > > > and limit the crazy structs widespread in the bpfilter code. > > > > > > > [...] > > > > > > > > > > > 1. https://lore.kernel.org/patchwork/patch/902785/ > > > > [1] used bpfilter_ prefix on struct definitions, like "struct bpfilter_target" > > I think we should do the same in this version. (Or were there discussions on > > removing the prefix?). > > There were no discussions about it. > As those structs are private to bpfilter I assumed that it is > safe to save some characters. > I will add the prefix to all internal structs in the next > iteration. For internal types it's ok to skip the prefix otherwise it's too verbose. In libbpf we skip 'bpf_' prefix in such cases.
> On May 20, 2021, at 9:55 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, May 20, 2021 at 12:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote: >> >> On Thu, May 20, 2021 at 04:54:45AM +0000, Song Liu wrote: >>> >>> >>>> On May 17, 2021, at 3:52 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote: >>>> >>>> The patchset is based on the patches from David S. Miller [1] and Daniel >>>> Borkmann [2]. >>>> >>>> The main goal of the patchset is to prepare bpfilter for iptables' >>>> configuration blob parsing and code generation. >>>> >>>> The patchset introduces data structures and code for matches, targets, rules >>>> and tables. >>>> >>>> It seems inconvenient to continue to use the same blob internally in bpfilter >>>> in parts other than the blob parsing. That is why a superstructure with native >>>> types is introduced. It provides a more convenient way to iterate over the blob >>>> and limit the crazy structs widespread in the bpfilter code. >>>> >>> >>> [...] >>> >>>> >>>> >>>> 1. https://lore.kernel.org/patchwork/patch/902785/ >>> >>> [1] used bpfilter_ prefix on struct definitions, like "struct bpfilter_target" >>> I think we should do the same in this version. (Or were there discussions on >>> removing the prefix?). >> >> There were no discussions about it. >> As those structs are private to bpfilter I assumed that it is >> safe to save some characters. >> I will add the prefix to all internal structs in the next >> iteration. > > For internal types it's ok to skip the prefix otherwise it's too verbose. > In libbpf we skip 'bpf_' prefix in such cases. Do we have plan to put some of this logic in a library? If that is the case, the effort now may save some pain in the future. Thanks, Song
On Thu, May 20, 2021 at 05:56:30PM +0000, Song Liu wrote: > > > > On May 20, 2021, at 9:55 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > > > On Thu, May 20, 2021 at 12:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > >> > >> On Thu, May 20, 2021 at 04:54:45AM +0000, Song Liu wrote: > >>> > >>> > >>>> On May 17, 2021, at 3:52 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > >>>> > >>>> The patchset is based on the patches from David S. Miller [1] and Daniel > >>>> Borkmann [2]. > >>>> > >>>> The main goal of the patchset is to prepare bpfilter for iptables' > >>>> configuration blob parsing and code generation. > >>>> > >>>> The patchset introduces data structures and code for matches, targets, rules > >>>> and tables. > >>>> > >>>> It seems inconvenient to continue to use the same blob internally in bpfilter > >>>> in parts other than the blob parsing. That is why a superstructure with native > >>>> types is introduced. It provides a more convenient way to iterate over the blob > >>>> and limit the crazy structs widespread in the bpfilter code. > >>>> > >>> > >>> [...] > >>> > >>>> > >>>> > >>>> 1. https://lore.kernel.org/patchwork/patch/902785/ > >>> > >>> [1] used bpfilter_ prefix on struct definitions, like "struct bpfilter_target" > >>> I think we should do the same in this version. (Or were there discussions on > >>> removing the prefix?). > >> > >> There were no discussions about it. > >> As those structs are private to bpfilter I assumed that it is > >> safe to save some characters. > >> I will add the prefix to all internal structs in the next > >> iteration. > > > > For internal types it's ok to skip the prefix otherwise it's too verbose. > > In libbpf we skip 'bpf_' prefix in such cases. > > Do we have plan to put some of this logic in a library? If that is the case, the > effort now may save some pain in the future. I cannot imagine a case when we need this logic in a library. Even if we eventually need it as these definitions are private to bpfilter - amount of pain should be minimal. > > Thanks, > Song >