| Message ID | 1580161806-8037-1-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | [ovs-dev] kbuild: Include external modules compile flags | expand |
Bleep bloop. Greetings Greg Rose, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: fatal: sha1 information is lacking or useless (scripts/Makefile.modfinal). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 kbuild: Include external modules compile flags The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: > > Since this commit: > 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")' > at least one out-of-tree external kernel module build fails > during the modfinal make phase because Makefile.modfinal does > not include the ccflags-y variable from the exernal module's Kbuild. ccflags-y is passed only for compiling C files in that directory. It is not used for compiling *.mod.c This is true for both in-kernel and external modules. So, ccflags-y is not a good choice for passing such flags that should be globally effective. Maybe, KCFLAGS should work. module: $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname -r)/build modules > Make sure to include the external kernel module's Kbuild so that the > necessary command line flags from the external module are set. > > Reported-by: David Ahern <dsahern@gmail.com> > CC: Masahiro Yamada <yamada.masahiro@socionext.com> > Signed-off-by: Greg Rose <gvrose8192@gmail.com> > --- > scripts/Makefile.modfinal | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 411c1e60..a645ba6 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -21,6 +21,10 @@ __modfinal: $(modules) > modname = $(notdir $(@:.mod.o=)) > part-of-module = y > > +# Include the module's Makefile to find KBUILD_EXTRA_SYMBOLS > +include $(if $(wildcard $(KBUILD_EXTMOD)/Kbuild), \ > + $(KBUILD_EXTMOD)/Kbuild) > + > quiet_cmd_cc_o_c = CC [M] $@ > cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< > > -- > 1.8.3.1 >
On 1/27/2020 7:35 PM, Masahiro Yamada wrote: > On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: >> Since this commit: >> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")' >> at least one out-of-tree external kernel module build fails >> during the modfinal make phase because Makefile.modfinal does >> not include the ccflags-y variable from the exernal module's Kbuild. > ccflags-y is passed only for compiling C files in that directory. > > It is not used for compiling *.mod.c > This is true for both in-kernel and external modules. > > So, ccflags-y is not a good choice > for passing such flags that should be globally effective. > > > Maybe, KCFLAGS should work. > > > module: > $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname -r)/build modules > Thanks, I'll see if I can get that to work. - Greg > >> Make sure to include the external kernel module's Kbuild so that the >> necessary command line flags from the external module are set. >> >> Reported-by: David Ahern <dsahern@gmail.com> >> CC: Masahiro Yamada <yamada.masahiro@socionext.com> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >> --- >> scripts/Makefile.modfinal | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal >> index 411c1e60..a645ba6 100644 >> --- a/scripts/Makefile.modfinal >> +++ b/scripts/Makefile.modfinal >> @@ -21,6 +21,10 @@ __modfinal: $(modules) >> modname = $(notdir $(@:.mod.o=)) >> part-of-module = y >> >> +# Include the module's Makefile to find KBUILD_EXTRA_SYMBOLS >> +include $(if $(wildcard $(KBUILD_EXTMOD)/Kbuild), \ >> + $(KBUILD_EXTMOD)/Kbuild) >> + >> quiet_cmd_cc_o_c = CC [M] $@ >> cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< >> >> -- >> 1.8.3.1 >> >
On 1/28/2020 7:37 AM, Gregory Rose wrote: > On 1/27/2020 7:35 PM, Masahiro Yamada wrote: >> On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: >>> Since this commit: >>> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into >>> Makefile.modfinal")' >>> at least one out-of-tree external kernel module build fails >>> during the modfinal make phase because Makefile.modfinal does >>> not include the ccflags-y variable from the exernal module's Kbuild. >> ccflags-y is passed only for compiling C files in that directory. >> >> It is not used for compiling *.mod.c >> This is true for both in-kernel and external modules. >> >> So, ccflags-y is not a good choice >> for passing such flags that should be globally effective. >> >> >> Maybe, KCFLAGS should work. >> >> >> module: >> $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname >> -r)/build modules >> Hi Masahiro, I'm unable to get that to work. KCFLAGS does not seem to be used in Makefile.modfinal. [snip] >>> --- a/scripts/Makefile.modfinal >>> +++ b/scripts/Makefile.modfinal >>> @@ -21,6 +21,10 @@ __modfinal: $(modules) >>> modname = $(notdir $(@:.mod.o=)) >>> part-of-module = y >>> >>> +# Include the module's Makefile to find KBUILD_EXTRA_SYMBOLS >>> +include $(if $(wildcard $(KBUILD_EXTMOD)/Kbuild), \ >>> + $(KBUILD_EXTMOD)/Kbuild) >>> + I continue to wonder why this it is so bad to include the external module's Kbuild. It used to be included in Makefile.modpost and did no harm, and in fact was what made our external build work at all in the past. Without the ability to define our local kernel module build environment during the modfinal make I see no way forward. That said, I'm no expert on the Linux kernel Makefile interdependencies. If you have some other idea we could try I'm game. Thanks, - Greg
On Thu, Jan 30, 2020 at 3:09 AM Gregory Rose <gvrose8192@gmail.com> wrote: > > > On 1/28/2020 7:37 AM, Gregory Rose wrote: > > On 1/27/2020 7:35 PM, Masahiro Yamada wrote: > >> On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: > >>> Since this commit: > >>> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into > >>> Makefile.modfinal")' > >>> at least one out-of-tree external kernel module build fails > >>> during the modfinal make phase because Makefile.modfinal does > >>> not include the ccflags-y variable from the exernal module's Kbuild. > >> ccflags-y is passed only for compiling C files in that directory. > >> > >> It is not used for compiling *.mod.c > >> This is true for both in-kernel and external modules. > >> > >> So, ccflags-y is not a good choice > >> for passing such flags that should be globally effective. > >> > >> > >> Maybe, KCFLAGS should work. > >> > >> > >> module: > >> $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname > >> -r)/build modules > >> > > Hi Masahiro, > > I'm unable to get that to work. KCFLAGS does not seem to be used in > Makefile.modfinal. I quickly tested it, and confirmed KCFLAGS works for external modules, too. Makefile.modfinal includes scripts/Makefile.lib So, c_flags contains $(KCFLAGS) c_flags -> KBUILD_CFLAGS -> KCFLAGS > [snip] > >>> --- a/scripts/Makefile.modfinal > >>> +++ b/scripts/Makefile.modfinal > >>> @@ -21,6 +21,10 @@ __modfinal: $(modules) > >>> modname = $(notdir $(@:.mod.o=)) > >>> part-of-module = y > >>> > >>> +# Include the module's Makefile to find KBUILD_EXTRA_SYMBOLS > >>> +include $(if $(wildcard $(KBUILD_EXTMOD)/Kbuild), \ > >>> + $(KBUILD_EXTMOD)/Kbuild) > >>> + > > I continue to wonder why this it is so bad to include the external > module's Kbuild. Because it is not correct behavior. > It used to be included in Makefile.modpost and did no harm, and in fact > was what > made our external build work at all in the past. Without the ability to > define our > local kernel module build environment during the modfinal make I see no > way forward. As I said, ccflags-y is not intended to be passed to *.mod.c Upstream modules never rely on it. External module should not rely on it either. > > That said, I'm no expert on the Linux kernel Makefile > interdependencies. If you > have some other idea we could try I'm game.
On 2/1/2020 10:07 PM, Masahiro Yamada wrote: > On Thu, Jan 30, 2020 at 3:09 AM Gregory Rose <gvrose8192@gmail.com> wrote: >> >> On 1/28/2020 7:37 AM, Gregory Rose wrote: >>> On 1/27/2020 7:35 PM, Masahiro Yamada wrote: >>>> On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: >>>>> Since this commit: >>>>> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into >>>>> Makefile.modfinal")' >>>>> at least one out-of-tree external kernel module build fails >>>>> during the modfinal make phase because Makefile.modfinal does >>>>> not include the ccflags-y variable from the exernal module's Kbuild. >>>> ccflags-y is passed only for compiling C files in that directory. >>>> >>>> It is not used for compiling *.mod.c >>>> This is true for both in-kernel and external modules. >>>> >>>> So, ccflags-y is not a good choice >>>> for passing such flags that should be globally effective. >>>> >>>> >>>> Maybe, KCFLAGS should work. >>>> >>>> >>>> module: >>>> $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname >>>> -r)/build modules >>>> >> Hi Masahiro, >> >> I'm unable to get that to work. KCFLAGS does not seem to be used in >> Makefile.modfinal. > > I quickly tested it, and confirmed > KCFLAGS works for external modules, too. > > > Makefile.modfinal includes scripts/Makefile.lib > > > So, c_flags contains $(KCFLAGS) > > c_flags -> KBUILD_CFLAGS -> KCFLAGS > Hi Masahiro, I must have missed something then - again, my unfamiliarity with the Linux Makefiles is probably tripping me up. I'll dig around and see if I can get that working. Thanks for your help, - Greg
On 2/1/2020 10:07 PM, Masahiro Yamada wrote: > On Thu, Jan 30, 2020 at 3:09 AM Gregory Rose <gvrose8192@gmail.com> wrote: >> >> On 1/28/2020 7:37 AM, Gregory Rose wrote: >>> On 1/27/2020 7:35 PM, Masahiro Yamada wrote: >>>> On Tue, Jan 28, 2020 at 6:50 AM Greg Rose <gvrose8192@gmail.com> wrote: >>>>> Since this commit: >>>>> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into >>>>> Makefile.modfinal")' >>>>> at least one out-of-tree external kernel module build fails >>>>> during the modfinal make phase because Makefile.modfinal does >>>>> not include the ccflags-y variable from the exernal module's Kbuild. >>>> ccflags-y is passed only for compiling C files in that directory. >>>> >>>> It is not used for compiling *.mod.c >>>> This is true for both in-kernel and external modules. >>>> >>>> So, ccflags-y is not a good choice >>>> for passing such flags that should be globally effective. >>>> >>>> >>>> Maybe, KCFLAGS should work. >>>> >>>> >>>> module: >>>> $(MAKE) KCFLAGS=... M=$(PWD) -C /lib/modules/$(uname >>>> -r)/build modules >>>> >> Hi Masahiro, >> >> I'm unable to get that to work. KCFLAGS does not seem to be used in >> Makefile.modfinal. > > I quickly tested it, and confirmed > KCFLAGS works for external modules, too. > > > Makefile.modfinal includes scripts/Makefile.lib > > > So, c_flags contains $(KCFLAGS) > > c_flags -> KBUILD_CFLAGS -> KCFLAGS > > Hi Masahiro, I found a way to make this work on the openvswitch out of tree kernel module. KCFLAGS doesn't work because it is added at the end of the gcc command line and we need to add a '-include <file>' directive so that the include file comes *before* all other include files. I fix this by inserting the '-include <file>' command line option to the beginning of our own Kbuild NOSTDINC variable. Thanks for your help in debugging this. - Greg
On 2/5/20 3:48 PM, Gregory Rose wrote: > > I found a way to make this work on the openvswitch out of tree kernel > module. KCFLAGS > doesn't work because it is added at the end of the gcc command line and > we need to > add a '-include <file>' directive so that the include file comes > *before* all other include > files. I fix this by inserting the '-include <file>' command line > option to the beginning > of our own Kbuild NOSTDINC variable. Hi Greg: Thanks for chasing this down. Are you going to commit the change to ovs master? Thanks, David
On 2/21/2020 9:43 AM, David Ahern wrote: > On 2/5/20 3:48 PM, Gregory Rose wrote: >> I found a way to make this work on the openvswitch out of tree kernel >> module. KCFLAGS >> doesn't work because it is added at the end of the gcc command line and >> we need to >> add a '-include <file>' directive so that the include file comes >> *before* all other include >> files. I fix this by inserting the '-include <file>' command line >> option to the beginning >> of our own Kbuild NOSTDINC variable. > Hi Greg: > > Thanks for chasing this down. Are you going to commit the change to ovs > master? Hi David, I have a patch set ready to go to add support for kernels all the way up to 5.5 however OVS is broken on kernels from 5.2 on up. All of our netlink nested messages are failing with -EINVAL return from nla_parse_nested(). I am looking into why this is occurring . There were some changes around use of the netlink policy and I think the issues I'm seeing might be related. Specifically the policy field from the genl_ops structure was removed and I think that might be the problem. When I have a resolution for that issue I'll be posting my patches. I'll keep you in the loop. Thanks, - Greg > > Thanks, > David >
On 2/21/2020 10:07 AM, Gregory Rose wrote: > > On 2/21/2020 9:43 AM, David Ahern wrote: >> On 2/5/20 3:48 PM, Gregory Rose wrote: >>> I found a way to make this work on the openvswitch out of tree kernel >>> module. KCFLAGS >>> doesn't work because it is added at the end of the gcc command line and >>> we need to >>> add a '-include <file>' directive so that the include file comes >>> *before* all other include >>> files. I fix this by inserting the '-include <file>' command line >>> option to the beginning >>> of our own Kbuild NOSTDINC variable. >> Hi Greg: >> >> Thanks for chasing this down. Are you going to commit the change to ovs >> master? > > Hi David, > > I have a patch set ready to go to add support for kernels all the way > up to 5.5 > however OVS is broken on kernels from 5.2 on up. All of our netlink > nested > messages are failing with -EINVAL return from nla_parse_nested(). I am > looking into why this is occurring . There were some changes around > use of the netlink policy and I think the issues I'm seeing might be > related. > Specifically the policy field from the genl_ops structure was removed and > I think that might be the problem. > > When I have a resolution for that issue I'll be posting my patches. > I'll keep > you in the loop. > David, I need to do some further backporting of these patches: 56738f4 netlink: add strict parsing for future attributes 3de6440 netlink: re-add parse/validate functions in strict mode 8cb0817 netlink: make validation more configurable for future strictness 6f455f5 netlink: add NLA_MIN_LEN f6ad55a Merge branch 'nla_nest_start' I already backported some of that code in my patch series, just to get it to compile for one thing, but I've obviously missed something. Just FYI. - Greg
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 411c1e60..a645ba6 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -21,6 +21,10 @@ __modfinal: $(modules) modname = $(notdir $(@:.mod.o=)) part-of-module = y +# Include the module's Makefile to find KBUILD_EXTRA_SYMBOLS +include $(if $(wildcard $(KBUILD_EXTMOD)/Kbuild), \ + $(KBUILD_EXTMOD)/Kbuild) + quiet_cmd_cc_o_c = CC [M] $@ cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
Since this commit: 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")' at least one out-of-tree external kernel module build fails during the modfinal make phase because Makefile.modfinal does not include the ccflags-y variable from the exernal module's Kbuild. Make sure to include the external kernel module's Kbuild so that the necessary command line flags from the external module are set. Reported-by: David Ahern <dsahern@gmail.com> CC: Masahiro Yamada <yamada.masahiro@socionext.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> --- scripts/Makefile.modfinal | 4 ++++ 1 file changed, 4 insertions(+)