On Tue, Jun 30, 2020 at 03:30:04PM +0900, Masahiro Yamada wrote: > On Mon, Jun 8, 2020 at 8:56 PM Michal Kubecek <mkubecek@xxxxxxx> wrote: > > > > I just noticed that this patch (now in mainline as commit 8a2cc0505cc4) > > drops the test if CONFIG_BPFILTER_UMH is "y" so that -static is now > > passed to the linker even if bpfilter_umh is built as a module which > > wasn't the case in v5.7. > > > > This is not mentioned in the commit message and the comment still says > > "*builtin* bpfilter_umh should be linked with -static" so this change > > doesn't seem to be intentional. Did I miss something? > > I was away for a while from this because I saw long discussion in > "net/bpfilter: Remove this broken and apparently unmaintained" > > > Please let me resume this topic now. > > > The original behavior of linking umh was like this: > - If CONFIG_BPFILTER_UMH=y, bpfilter_umh was linked with -static > - If CONFIG_BPFILTER_UMH=m, bpfilter_umh was linked without -static > > > > Restoring the original behavior will add more complexity because > now we have CONFIG_CC_CAN_LINK and CONFIG_CC_CAN_LINK_STATIC > since commit b1183b6dca3e0d5 > > If CONFIG_BPFILTER_UMH=y, we need to check CONFIG_CC_CAN_LINK_STATIC. > If CONFIG_BPFILTER_UMH=m, we need to check CONFIG_CC_CAN_LINK. > This would make the Kconfig dependency logic too complicated. > > > To make it simpler, I'd like to suggest two options. > > > > Idea 1: > > Always use -static irrespective of whether > CONFIG_BPFILTER_UMH is y or m. > > Add two more lines to clarify this > in the comment in net/bpfilter/Makefile: > > # builtin bpfilter_umh should be linked with -static > # since rootfs isn't mounted at the time of __init > # function is called and do_execv won't find elf interpreter. > # Static linking is not required when bpfilter is modular, but > # we always pass -static to keep the 'depends on' in Kconfig simple. I wouldn't be very happy with this solution as that would mean adding an extra build dependency which we don't really need. We might even consider disabling CONFIG_BPFILTER_UMH instead. > Idea 2: > > Allow umh to become only modular, > and drop -static flag entirely. > > If you look at net/bpfilter/Kconfig, > BPFILTER_UMH already has 'default m'. > So, I assume the most expected use-case > is modular. > > My suggestion is to replace 'default m' with 'depends on m'. > > config BPFILTER_UMH > tristate "bpfilter kernel module with user mode helper" > depends on CC_CAN_LINK > depends on m > > Then BPFILTER_UMH will be restricted to either m or n. > Link umh dynamically because we can expect rootfs > is already mounted for the module case. This wouldn't be a problem for me or openSUSE kernels as we already have CONFIG_BPFILTER_UMH=m. But I can't speak for others, I'm not sure if there are some use cases requiring CONFIG_BPFILTER_UMH=y. Michal
