| Message ID | 20231114124713.87976-1-odivlad@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [ovs-dev,branch-2.17] datapath-linux: Fix kmod build with recent CentOS Stream kernels | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/intel-ovs-compilation | success | test: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Bleep bloop. Greetings Vladislav Odintsov, 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. checkpatch: WARNING: The subject summary should end with a dot. Subject: datapath-linux: Fix kmod build with recent CentOS Stream kernels Lines checked: 55, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 11/14/23 13:47, Vladislav Odintsov wrote: > With kernel 4.18.0-477.x.y there was a compilation error: > > /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/mm.h:13:21: error: redefinition of 'kvmalloc' > static inline void *kvmalloc(size_t size, gfp_t flags) > ^~~~~~~~ > In file included from ./include/linux/crypto.h:24, > from ./include/crypto/hash.h:16, > from ./include/linux/uio.h:16, > from ./include/linux/socket.h:8, > from ./include/linux/skbuff.h:23, > from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/skbuff.h:17, > from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/datapath/linux/actions.c:21: > ./include/linux/slab.h:740:21: note: previous definition of 'kvmalloc' was here > > This was introduced by upstream kernel commit [0]. > This patch fixes this error adding a new path to searched functions to lookup. > > 0: https://lore.kernel.org/all/20211105203507.gvGl5bZmw%25akpm@linux-foundation.org/ Hi, Vladislav. The out-of-tree kernel module is deprecated and support is capped at kernel version 5.8. The patch you specified was introduced in 5.16, so I think it falls into kernels newer than 5.8 category. It is nominally 4.18, but RHEL/CentOS/etc kernels are heavily modified and the 4.18 number doesn't really mean anything useful. We can't continue adapting this module to all the feature backports made by distributions. Hope that makes sense. OTOH, I'm curious why are you still using the OOT module with CentOS Stream? The openvswitch module there should be in much better shape, so I'd recommend just using it instead. Best regards, Ilya Maximets.
Hi Ilya, Thanks for the review. regards, Vladislav Odintsov > On 15 Nov 2023, at 00:17, Ilya Maximets <i.maximets@ovn.org> wrote: > > On 11/14/23 13:47, Vladislav Odintsov wrote: >> With kernel 4.18.0-477.x.y there was a compilation error: >> >> /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/mm.h:13:21: error: redefinition of 'kvmalloc' >> static inline void *kvmalloc(size_t size, gfp_t flags) >> ^~~~~~~~ >> In file included from ./include/linux/crypto.h:24, >> from ./include/crypto/hash.h:16, >> from ./include/linux/uio.h:16, >> from ./include/linux/socket.h:8, >> from ./include/linux/skbuff.h:23, >> from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/skbuff.h:17, >> from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/datapath/linux/actions.c:21: >> ./include/linux/slab.h:740:21: note: previous definition of 'kvmalloc' was here >> >> This was introduced by upstream kernel commit [0]. >> This patch fixes this error adding a new path to searched functions to lookup. >> >> 0: https://lore.kernel.org/all/20211105203507.gvGl5bZmw%25akpm@linux-foundation.org/ > > Hi, Vladislav. > > The out-of-tree kernel module is deprecated and support is capped > at kernel version 5.8. > > The patch you specified was introduced in 5.16, so I think it > falls into kernels newer than 5.8 category. It is nominally > 4.18, but RHEL/CentOS/etc kernels are heavily modified and the > 4.18 number doesn't really mean anything useful. We can't > continue adapting this module to all the feature backports made > by distributions. > > Hope that makes sense. Yeah, I know it’s deprecated. I just had to do this work internally and decided to share the results with community. Especially while 2.17 branch is an LTS. For now it works with latest CentOS 8 stream/Rocky Linux 8 kernel, but also requires a trivial patch for ovs-manage-kmod.sh script, which I also can send as a follow-up. I see this as a “best effort” support level ;) > > OTOH, I'm curious why are you still using the OOT module with > CentOS Stream? The openvswitch module there should be in much > better shape, so I'd recommend just using it instead. That’s true. The reason — lack of support for STT in kernel. Some our deployments still rely on this protocol. At the same time OVN also offers usage of STT. Hope I’ve described my intention :) > > Best regards, Ilya Maximets.
diff --git a/acinclude.m4 b/acinclude.m4 index c981f90bc..bffbd1cef 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -1179,9 +1179,15 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/linux/overflow.h], [struct_size], [OVS_DEFINE([HAVE_STRUCT_SIZE])]) OVS_GREP_IFELSE([$KSRC/include/linux/mm.h], [kvmalloc_array], - [OVS_DEFINE([HAVE_KVMALLOC_ARRAY])]) + [OVS_DEFINE([HAVE_KVMALLOC_ARRAY])], + [OVS_GREP_IFELSE([$KSRC/include/linux/slab.h], + [kvmalloc_array], + [OVS_DEFINE([HAVE_KVMALLOC_ARRAY])])]) OVS_GREP_IFELSE([$KSRC/include/linux/mm.h], [kvmalloc_node], - [OVS_DEFINE([HAVE_KVMALLOC_NODE])]) + [OVS_DEFINE([HAVE_KVMALLOC_NODE])], + [OVS_GREP_IFELSE([$KSRC/include/linux/slab.h], + [kvmalloc_array], + [OVS_DEFINE([HAVE_KVMALLOC_ARRAY])])]) OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_l3proto.h], [nf_conntrack_l3proto], [OVS_DEFINE([HAVE_NF_CONNTRACK_L3PROATO_H])])
With kernel 4.18.0-477.x.y there was a compilation error: /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/mm.h:13:21: error: redefinition of 'kvmalloc' static inline void *kvmalloc(size_t size, gfp_t flags) ^~~~~~~~ In file included from ./include/linux/crypto.h:24, from ./include/crypto/hash.h:16, from ./include/linux/uio.h:16, from ./include/linux/socket.h:8, from ./include/linux/skbuff.h:23, from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/../datapath/linux/compat/include/linux/skbuff.h:17, from /builddir/build/BUILD/openvswitch-2.17.3/_4.18.0-477.10.1.el8_8.x86_64/datapath/linux/actions.c:21: ./include/linux/slab.h:740:21: note: previous definition of 'kvmalloc' was here This was introduced by upstream kernel commit [0]. This patch fixes this error adding a new path to searched functions to lookup. 0: https://lore.kernel.org/all/20211105203507.gvGl5bZmw%25akpm@linux-foundation.org/ Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> --- acinclude.m4 | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)