diff mbox series

[3/3] bpfilter: do not (ab)use host-program build rule

Message ID 1528477930-7342-4-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series net: bpfilter: clean-up build rules | expand

Commit Message

Masahiro Yamada June 8, 2018, 5:12 p.m. UTC
It is an ugly hack to overwrite $(HOSTCC) with $(CC) to reuse the
build rules from scripts/Makefile.host.  It should not be tedious
to write a build rule for its own.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 net/bpfilter/Makefile                   | 17 +++++++++++------
 net/bpfilter/{main.c => bpfilter_umh.c} |  0
 2 files changed, 11 insertions(+), 6 deletions(-)
 rename net/bpfilter/{main.c => bpfilter_umh.c} (100%)

-- 
2.7.4

Comments

Alexei Starovoitov June 8, 2018, 8:52 p.m. UTC | #1
On Sat, Jun 09, 2018 at 02:12:10AM +0900, Masahiro Yamada wrote:
> It is an ugly hack to overwrite $(HOSTCC) with $(CC) to reuse the

> build rules from scripts/Makefile.host.  It should not be tedious

> to write a build rule for its own.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

> 

>  net/bpfilter/Makefile                   | 17 +++++++++++------

>  net/bpfilter/{main.c => bpfilter_umh.c} |  0

>  2 files changed, 11 insertions(+), 6 deletions(-)

>  rename net/bpfilter/{main.c => bpfilter_umh.c} (100%)

> 

> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile

> index 39c6980..6571b30 100644

> --- a/net/bpfilter/Makefile

> +++ b/net/bpfilter/Makefile

> @@ -3,18 +3,23 @@

>  # Makefile for the Linux BPFILTER layer.

>  #

>  

> -hostprogs-y := bpfilter_umh

> -bpfilter_umh-objs := main.o

> -HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi

> -HOSTCC := $(CC)


that is a hack indeed. I don't like it either, but see below.

> -

>  ifeq ($(CONFIG_BPFILTER_UMH), y)

>  # builtin bpfilter_umh should be compiled with -static

>  # since rootfs isn't mounted at the time of __init

>  # function is called and do_execv won't find elf interpreter

> -HOSTLDFLAGS += -static

> +STATIC := -static

>  endif

>  

> +quiet_cmd_cc_user = CC      $@

> +      cmd_cc_user = $(CC) -Wall -Wmissing-prototypes -O2 -std=gnu89 \

> +		    -I$(srctree) -I$(srctree)/tools/include/ \

> +		    -I$(srctree)/tools/include/uapi $(STATIC) -o $@ $<

> +

> +$(obj)/bpfilter_umh: $(src)/bpfilter_umh.c FORCE

> +	$(call if_changed,cc_user)


Does this scale?
Please see two top patches here:
https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/log/?h=ipt_bpf
that add more meat to bpfilter and a lot more files.
Recompiling all of them at once isn't nice either.
This Makefile needs different .c -> .o rules for bpfilter_kern.c files
that get compiled into kernel module and for the rest of umh files:
main.c ctor.c init.c gen.c etc
that need to be compiled .c -> .o differently.
I don't see how to do it without ugly hacks in Makefile.
In that sense HOSTCC = CC hack looked the least ugly to me that's
why I went with it.
Better ideas?
Masahiro Yamada June 9, 2018, 3:13 p.m. UTC | #2
2018-06-09 5:52 GMT+09:00 Alexei Starovoitov <alexei.starovoitov@gmail.com>:
> On Sat, Jun 09, 2018 at 02:12:10AM +0900, Masahiro Yamada wrote:

>> It is an ugly hack to overwrite $(HOSTCC) with $(CC) to reuse the

>> build rules from scripts/Makefile.host.  It should not be tedious

>> to write a build rule for its own.

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> ---

>>

>>  net/bpfilter/Makefile                   | 17 +++++++++++------

>>  net/bpfilter/{main.c => bpfilter_umh.c} |  0

>>  2 files changed, 11 insertions(+), 6 deletions(-)

>>  rename net/bpfilter/{main.c => bpfilter_umh.c} (100%)

>>

>> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile

>> index 39c6980..6571b30 100644

>> --- a/net/bpfilter/Makefile

>> +++ b/net/bpfilter/Makefile

>> @@ -3,18 +3,23 @@

>>  # Makefile for the Linux BPFILTER layer.

>>  #

>>

>> -hostprogs-y := bpfilter_umh

>> -bpfilter_umh-objs := main.o

>> -HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi

>> -HOSTCC := $(CC)

>

> that is a hack indeed. I don't like it either, but see below.

>

>> -

>>  ifeq ($(CONFIG_BPFILTER_UMH), y)

>>  # builtin bpfilter_umh should be compiled with -static

>>  # since rootfs isn't mounted at the time of __init

>>  # function is called and do_execv won't find elf interpreter

>> -HOSTLDFLAGS += -static

>> +STATIC := -static

>>  endif

>>

>> +quiet_cmd_cc_user = CC      $@

>> +      cmd_cc_user = $(CC) -Wall -Wmissing-prototypes -O2 -std=gnu89 \

>> +                 -I$(srctree) -I$(srctree)/tools/include/ \

>> +                 -I$(srctree)/tools/include/uapi $(STATIC) -o $@ $<

>> +

>> +$(obj)/bpfilter_umh: $(src)/bpfilter_umh.c FORCE

>> +     $(call if_changed,cc_user)

>

> Does this scale?

> Please see two top patches here:

> https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/log/?h=ipt_bpf

> that add more meat to bpfilter and a lot more files.


Oh, I just thought main.c was the only user-program file.

> Recompiling all of them at once isn't nice either.


Indeed.

It should be able to compile .c -> .o separately.

> This Makefile needs different .c -> .o rules for bpfilter_kern.c files

> that get compiled into kernel module and for the rest of umh files:

> main.c ctor.c init.c gen.c etc

> that need to be compiled .c -> .o differently.

> I don't see how to do it without ugly hacks in Makefile.

> In that sense HOSTCC = CC hack looked the least ugly to me that's

> why I went with it.

> Better ideas?



I will think about it after your patches land.

I will drop this for now.


-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index 39c6980..6571b30 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile
@@ -3,18 +3,23 @@ 
 # Makefile for the Linux BPFILTER layer.
 #
 
-hostprogs-y := bpfilter_umh
-bpfilter_umh-objs := main.o
-HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi
-HOSTCC := $(CC)
-
 ifeq ($(CONFIG_BPFILTER_UMH), y)
 # builtin bpfilter_umh should be compiled with -static
 # since rootfs isn't mounted at the time of __init
 # function is called and do_execv won't find elf interpreter
-HOSTLDFLAGS += -static
+STATIC := -static
 endif
 
+quiet_cmd_cc_user = CC      $@
+      cmd_cc_user = $(CC) -Wall -Wmissing-prototypes -O2 -std=gnu89 \
+		    -I$(srctree) -I$(srctree)/tools/include/ \
+		    -I$(srctree)/tools/include/uapi $(STATIC) -o $@ $<
+
+$(obj)/bpfilter_umh: $(src)/bpfilter_umh.c FORCE
+	$(call if_changed,cc_user)
+
+targets += bpfilter_umh
+
 $(obj)/bpfilter_umh_blob.o: $(obj)/bpfilter_umh
 
 obj-$(CONFIG_BPFILTER_UMH) += bpfilter.o
diff --git a/net/bpfilter/main.c b/net/bpfilter/bpfilter_umh.c
similarity index 100%
rename from net/bpfilter/main.c
rename to net/bpfilter/bpfilter_umh.c