[ovs-dev,V2,07/11] datapath: Kbuild: Add kcompat.h header to front of NOSTDINC
diff mbox series

Message ID 1582738882-15842-8-git-send-email-gvrose8192@gmail.com
State Superseded
Headers show
Series
  • Add Linux kernel datapath support up to 5.5
Related show

Commit Message

Greg Rose Feb. 26, 2020, 5:41 p.m. UTC
Since this commit in the Linux upstream kernel:
'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")'
The openvswitch kernel module fails to build against the upstream
Linux kernel. The cause of the build failure is that the include of the
KBUILD_EXTMOD variable was dropped in Makefile.modfinal when
it was split out from Makefile.modpost.  Our Kbuild was setting
the ccflags-y variable to include our kcompat.h header as the
first header file.  The Linux kernel maintainer has said that
it is incorrect to rely on the ccflags-y variable for the modfinal
phase of the build so that is why KBUILD_EXTMOD is not included.

We fix this by breaking a different Linux kernel make rule.  We
add '-include $(builddir)/kcompat.h' to the front of the NOSTDINC
variable setting in our Kbuild makefile.

As noted already in the comment for the NOSTDINC setting:
\# These include directories have to go before -I$(KSRC)/include.
\# NOSTDINC_FLAGS just happens to be a variable that goes in the
\# right place, even though it's conceptually incorrect.

So we continue the misuse of the NOSTDINC variable to fix this
issue as well.

The assumption of the Linux kernel maintainers is that any
local, out-of-tree build include files can be added to the end
of the command line. In our case that is wrong of course, but
there is nothing we can do about it that I know of other than using
some utility like unifdef to strip out offending chunks of our
compatibility layer code before invocation of Makefile.modfinal.
That is a big change that would take a lot of work to implement.

We could ask the Linux kernel maintainers to provide some
way for out-of-tree kernel modules to include their own header
files first in a proper manner. I consider that to be a very
low probability of success but something we could ask about.

For now we cheat and take the easy way out.

Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
---
 datapath/linux/Kbuild.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Yi-Hung Wei Feb. 28, 2020, 12:39 a.m. UTC | #1
On Wed, Feb 26, 2020 at 9:42 AM Greg Rose <gvrose8192@gmail.com> wrote:
>
> Since this commit in the Linux upstream kernel:
> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")'
> The openvswitch kernel module fails to build against the upstream
> Linux kernel. The cause of the build failure is that the include of the
> KBUILD_EXTMOD variable was dropped in Makefile.modfinal when
> it was split out from Makefile.modpost.  Our Kbuild was setting
> the ccflags-y variable to include our kcompat.h header as the
> first header file.  The Linux kernel maintainer has said that
> it is incorrect to rely on the ccflags-y variable for the modfinal
> phase of the build so that is why KBUILD_EXTMOD is not included.
>
> We fix this by breaking a different Linux kernel make rule.  We
> add '-include $(builddir)/kcompat.h' to the front of the NOSTDINC
> variable setting in our Kbuild makefile.
>
> As noted already in the comment for the NOSTDINC setting:
> \# These include directories have to go before -I$(KSRC)/include.
> \# NOSTDINC_FLAGS just happens to be a variable that goes in the
> \# right place, even though it's conceptually incorrect.
>
> So we continue the misuse of the NOSTDINC variable to fix this
> issue as well.
>
> The assumption of the Linux kernel maintainers is that any
> local, out-of-tree build include files can be added to the end
> of the command line. In our case that is wrong of course, but
> there is nothing we can do about it that I know of other than using
> some utility like unifdef to strip out offending chunks of our
> compatibility layer code before invocation of Makefile.modfinal.
> That is a big change that would take a lot of work to implement.
>
> We could ask the Linux kernel maintainers to provide some
> way for out-of-tree kernel modules to include their own header
> files first in a proper manner. I consider that to be a very
> low probability of success but something we could ask about.
>
> For now we cheat and take the easy way out.
>
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> ---

Thanks for this patch.  It must be a lot of effort to figure this out.

Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
Greg Rose Feb. 28, 2020, 5:07 p.m. UTC | #2
On 2/27/2020 4:39 PM, Yi-Hung Wei wrote:
> On Wed, Feb 26, 2020 at 9:42 AM Greg Rose <gvrose8192@gmail.com> wrote:
>> Since this commit in the Linux upstream kernel:
>> 'commit 9b9a3f20cbe0 ("kbuild: split final module linking out into Makefile.modfinal")'
>> The openvswitch kernel module fails to build against the upstream
>> Linux kernel. The cause of the build failure is that the include of the
>> KBUILD_EXTMOD variable was dropped in Makefile.modfinal when
>> it was split out from Makefile.modpost.  Our Kbuild was setting
>> the ccflags-y variable to include our kcompat.h header as the
>> first header file.  The Linux kernel maintainer has said that
>> it is incorrect to rely on the ccflags-y variable for the modfinal
>> phase of the build so that is why KBUILD_EXTMOD is not included.
>>
>> We fix this by breaking a different Linux kernel make rule.  We
>> add '-include $(builddir)/kcompat.h' to the front of the NOSTDINC
>> variable setting in our Kbuild makefile.
>>
>> As noted already in the comment for the NOSTDINC setting:
>> \# These include directories have to go before -I$(KSRC)/include.
>> \# NOSTDINC_FLAGS just happens to be a variable that goes in the
>> \# right place, even though it's conceptually incorrect.
>>
>> So we continue the misuse of the NOSTDINC variable to fix this
>> issue as well.
>>
>> The assumption of the Linux kernel maintainers is that any
>> local, out-of-tree build include files can be added to the end
>> of the command line. In our case that is wrong of course, but
>> there is nothing we can do about it that I know of other than using
>> some utility like unifdef to strip out offending chunks of our
>> compatibility layer code before invocation of Makefile.modfinal.
>> That is a big change that would take a lot of work to implement.
>>
>> We could ask the Linux kernel maintainers to provide some
>> way for out-of-tree kernel modules to include their own header
>> files first in a proper manner. I consider that to be a very
>> low probability of success but something we could ask about.
>>
>> For now we cheat and take the easy way out.
>>
>> Reported-by: David Ahern <dsahern@gmail.com>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>> ---
> Thanks for this patch.  It must be a lot of effort to figure this out.
>
> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>

Haha - yes, and then after figuring out the problem there was all
the going back and forth about the solution.  :)

I don't like this one bit but it's probably the best solution for now.

Thanks,

- Greg

Patch
diff mbox series

diff --git a/datapath/linux/Kbuild.in b/datapath/linux/Kbuild.in
index 9e3259f..395b0cb 100644
--- a/datapath/linux/Kbuild.in
+++ b/datapath/linux/Kbuild.in
@@ -16,7 +16,7 @@  ccflags-y += -include $(builddir)/kcompat.h
 # These include directories have to go before -I$(KSRC)/include.
 # NOSTDINC_FLAGS just happens to be a variable that goes in the
 # right place, even though it's conceptually incorrect.
-NOSTDINC_FLAGS += -I$(top_srcdir)/include -I$(srcdir)/compat -I$(srcdir)/compat/include
+NOSTDINC_FLAGS += -include $(builddir)/kcompat.h -I$(top_srcdir)/include -I$(srcdir)/compat -I$(srcdir)/compat/include
 
 obj-m := $(subst _,-,$(patsubst %,%.o,$(build_modules)))