* Re: [PATCH bpf-next] bpftool: disable bpfilter kernel config checks
2023-01-25 2:55 [PATCH bpf-next] bpftool: disable bpfilter kernel config checks Chethan Suresh
@ 2023-01-25 10:33 ` Quentin Monnet
2023-01-25 17:57 ` Daniel Borkmann
2023-01-27 11:17 ` Quentin Deslandes
2023-02-01 15:21 ` Quentin Deslandes
2 siblings, 1 reply; 5+ messages in thread
From: Quentin Monnet @ 2023-01-25 10:33 UTC (permalink / raw)
To: Chethan Suresh, bpf; +Cc: Kenta Tada, Quentin Deslandes
2023-01-25 08:25 UTC+0530 ~ Chethan Suresh <chethan.suresh@sony.com>
> We've experienced similar issues about bpfilter like below:
> https://github.com/moby/moby/issues/43755
> https://lore.kernel.org/bpf/CAADnVQJ5MxGkq=ng214aYoH-NmZ1gjoS=ZTY1eU-Fag4RwZjdg@mail.gmail.com/
>
> Considering the current development status of bpfilter,
> disable bpfilter kernel config checks in bpftool feature.
> For production system, we should disable both
> CONFIG_BPFILTER and CONFIG_BPFILTER_UMH for now.
> Or can be enabled as some tools depend on bpfilter.
>
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
> tools/bpf/bpftool/feature.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 36cf0f1517c9..c6087bbc6613 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -426,10 +426,6 @@ static void probe_kernel_image_config(const char *define_prefix)
> { "CONFIG_BPF_STREAM_PARSER", },
> /* xt_bpf module for passing BPF programs to netfilter */
> { "CONFIG_NETFILTER_XT_MATCH_BPF", },
> - /* bpfilter back-end for iptables */
> - { "CONFIG_BPFILTER", },
> - /* bpftilter module with "user mode helper" */
> - { "CONFIG_BPFILTER_UMH", },
>
> /* test_bpf module for BPF tests */
> { "CONFIG_TEST_BPF", },
Hi,
I don't understand. The feature probe simply looks for the kconfig
option in the kconfig file. What are you hoping to achieve by removing
this check? How is it going to help with your issues?
Quentin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpftool: disable bpfilter kernel config checks
2023-01-25 10:33 ` Quentin Monnet
@ 2023-01-25 17:57 ` Daniel Borkmann
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2023-01-25 17:57 UTC (permalink / raw)
To: Quentin Monnet, Chethan Suresh, bpf; +Cc: Kenta Tada, Quentin Deslandes
On 1/25/23 11:33 AM, Quentin Monnet wrote:
> 2023-01-25 08:25 UTC+0530 ~ Chethan Suresh <chethan.suresh@sony.com>
>> We've experienced similar issues about bpfilter like below:
>> https://github.com/moby/moby/issues/43755
>> https://lore.kernel.org/bpf/CAADnVQJ5MxGkq=ng214aYoH-NmZ1gjoS=ZTY1eU-Fag4RwZjdg@mail.gmail.com/
>>
>> Considering the current development status of bpfilter,
>> disable bpfilter kernel config checks in bpftool feature.
>> For production system, we should disable both
>> CONFIG_BPFILTER and CONFIG_BPFILTER_UMH for now.
>> Or can be enabled as some tools depend on bpfilter.
>>
>> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
>> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
>> ---
>> tools/bpf/bpftool/feature.c | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>> index 36cf0f1517c9..c6087bbc6613 100644
>> --- a/tools/bpf/bpftool/feature.c
>> +++ b/tools/bpf/bpftool/feature.c
>> @@ -426,10 +426,6 @@ static void probe_kernel_image_config(const char *define_prefix)
>> { "CONFIG_BPF_STREAM_PARSER", },
>> /* xt_bpf module for passing BPF programs to netfilter */
>> { "CONFIG_NETFILTER_XT_MATCH_BPF", },
>> - /* bpfilter back-end for iptables */
>> - { "CONFIG_BPFILTER", },
>> - /* bpftilter module with "user mode helper" */
>> - { "CONFIG_BPFILTER_UMH", },
Right, for bpftool this change is rather moot. Maybe until the work from
QuentinD materializes, the BPFILTER should just be built with `depends on
COMPILE_TEST` so that this doesn't negatively affect users as reported in
above links.
>>
>> /* test_bpf module for BPF tests */
>> { "CONFIG_TEST_BPF", },
>
> Hi,
> I don't understand. The feature probe simply looks for the kconfig
> option in the kconfig file. What are you hoping to achieve by removing
> this check? How is it going to help with your issues?
>
> Quentin
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: disable bpfilter kernel config checks
2023-01-25 2:55 [PATCH bpf-next] bpftool: disable bpfilter kernel config checks Chethan Suresh
2023-01-25 10:33 ` Quentin Monnet
@ 2023-01-27 11:17 ` Quentin Deslandes
2023-02-01 15:21 ` Quentin Deslandes
2 siblings, 0 replies; 5+ messages in thread
From: Quentin Deslandes @ 2023-01-27 11:17 UTC (permalink / raw)
To: Chethan Suresh; +Cc: quentin, bpf, Kenta Tada
On Wed, Jan 25, 2023 at 08:25:16AM +0530, Chethan Suresh wrote:
> We've experienced similar issues about bpfilter like below:
> https://github.com/moby/moby/issues/43755
> https://lore.kernel.org/bpf/CAADnVQJ5MxGkq=ng214aYoH-NmZ1gjoS=ZTY1eU-Fag4RwZjdg@mail.gmail.com/
>
> Considering the current development status of bpfilter,
> disable bpfilter kernel config checks in bpftool feature.
> For production system, we should disable both
> CONFIG_BPFILTER and CONFIG_BPFILTER_UMH for now.
> Or can be enabled as some tools depend on bpfilter.
>
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
> tools/bpf/bpftool/feature.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 36cf0f1517c9..c6087bbc6613 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -426,10 +426,6 @@ static void probe_kernel_image_config(const char *define_prefix)
> { "CONFIG_BPF_STREAM_PARSER", },
> /* xt_bpf module for passing BPF programs to netfilter */
> { "CONFIG_NETFILTER_XT_MATCH_BPF", },
> - /* bpfilter back-end for iptables */
> - { "CONFIG_BPFILTER", },
> - /* bpftilter module with "user mode helper" */
> - { "CONFIG_BPFILTER_UMH", },
>
> /* test_bpf module for BPF tests */
> { "CONFIG_TEST_BPF", },
> --
> 2.17.1
>
While I don't think this check is effectively needed in bpftool
regarding bpfilter's current state, I don't see how it's related to the
issues you're linking.
The GitHub issue you're linking is due to CONFIG_BPFILTER being enabled
on a kernel, with no related to bpftool.
Regards,
Quentin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next] bpftool: disable bpfilter kernel config checks
2023-01-25 2:55 [PATCH bpf-next] bpftool: disable bpfilter kernel config checks Chethan Suresh
2023-01-25 10:33 ` Quentin Monnet
2023-01-27 11:17 ` Quentin Deslandes
@ 2023-02-01 15:21 ` Quentin Deslandes
2 siblings, 0 replies; 5+ messages in thread
From: Quentin Deslandes @ 2023-02-01 15:21 UTC (permalink / raw)
To: Chethan Suresh, quentin, bpf; +Cc: Kenta Tada, qde
On 25/01/2023 03:55, Chethan Suresh wrote:
> We've experienced similar issues about bpfilter like below:
> https://github.com/moby/moby/issues/43755
I've been looking into this issue a bit more, it seems the author has
CONFIG_BPFILTER enabled, which shouldn't be. I've answered to the thread
to clarify the situation.
Regarding why CONFIG_BPFILTER was enabled, it seems linuxkit [1]
project's default configurations for multiple kernel verisons had it
enabled, for some reason. This was fixed [2] a few month ago for
*some* of the configurations, I've published a PR [3] for the remaining
configuration.
It's been approved but not merged yet. It's unclear why those
configurations had CONFIG_BPFILTER enabled in the first place, but it's
definitely a mistake.
[1]: https://github.com/linuxkit/linuxkit
[2]: https://github.com/linuxkit/linuxkit/pull/3701
[3]: https://github.com/linuxkit/linuxkit/pull/3904
> https://lore.kernel.org/bpf/CAADnVQJ5MxGkq=ng214aYoH-NmZ1gjoS=ZTY1eU-Fag4RwZjdg@mail.gmail.com/
>
> Considering the current development status of bpfilter,
> disable bpfilter kernel config checks in bpftool feature.
> For production system, we should disable both
> CONFIG_BPFILTER and CONFIG_BPFILTER_UMH for now.
> Or can be enabled as some tools depend on bpfilter.
>
> Signed-off-by: Chethan Suresh <chethan.suresh@sony.com>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
> tools/bpf/bpftool/feature.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 36cf0f1517c9..c6087bbc6613 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -426,10 +426,6 @@ static void probe_kernel_image_config(const char *define_prefix)
> { "CONFIG_BPF_STREAM_PARSER", },
> /* xt_bpf module for passing BPF programs to netfilter */
> { "CONFIG_NETFILTER_XT_MATCH_BPF", },
> - /* bpfilter back-end for iptables */
> - { "CONFIG_BPFILTER", },
> - /* bpftilter module with "user mode helper" */
> - { "CONFIG_BPFILTER_UMH", },
>
> /* test_bpf module for BPF tests */
> { "CONFIG_TEST_BPF", },
Regards,
Quentin
^ permalink raw reply [flat|nested] 5+ messages in thread