Re: 19.11.11 patches review and test

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

On Wed, Dec 15, 2021 at 3:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 12/15/2021 1:17 PM, Christian Ehrhardt wrote:
> > On Tue, Dec 14, 2021 at 3:52 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >> On 12/14/2021 2:46 PM, Christian Ehrhardt wrote:
> >>> On Tue, Dec 14, 2021 at 2:58 PM Christian Ehrhardt
> >>> <christian.ehrhardt@canonical.com> wrote:
> >>>>
> >>>> On Tue, Dec 14, 2021 at 2:10 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>>>>
> >>>>> On 12/14/2021 11:39 AM, Christian Ehrhardt wrote:
> >>>>>> On Tue, Dec 14, 2021 at 11:13 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>>>>>>
> >>>>>>> On 12/14/2021 7:44 AM, Christian Ehrhardt wrote:
> >>>>>>>> On Tue, Dec 14, 2021 at 6:49 AM Kalesh Anakkur Purayil
> >>>>>>>> <kalesh-anakkur.purayil@broadcom.com> wrote:
> >>>>>>>>
> >>>>>>>> [snip]
> >>>>>>>>
> >>>>>>>>>>> [Kalesh] Yes, i am seeing the same error. I used make command to build dpdk, not meson.
> >>>>>>>>>>> The back ported commit you mentioned takes care of meson build only I think.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I see, make build is failing, and yes the fix is only for the meson.
> >>>>>>>>>> I will check the make build and will send a fix for it.
> >>>>>>>>>
> >>>>>>>>> [Kalesh]: looks like the below changes fixes the issue. I tried only on SLES15 SP3 and not on other SLES flavors.
> >>>>>>>>>
> >>>>>>>>> diff --git a/kernel/linux/kni/Makefile b/kernel/linux/kni/Makefile
> >>>>>>>>> index 595bac2..bf0efab 100644
> >>>>>>>>> --- a/kernel/linux/kni/Makefile
> >>>>>>>>> +++ b/kernel/linux/kni/Makefile
> >>>>>>>>> @@ -16,6 +16,16 @@ MODULE_CFLAGS += -I$(RTE_OUTPUT)/include
> >>>>>>>>>      MODULE_CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
> >>>>>>>>>      MODULE_CFLAGS += -Wall -Werror
> >>>>>>>>>
> >>>>>>>>> +#
> >>>>>>>>> +# Use explicit 'source' folder for header path. In SUSE 'source' is not linked to 'build' folder.
> >>>>>>>>> +#
> >>>>>>>>> +ifdef CONFIG_SUSE_KERNEL
> >>>>>>>>> +   KSRC = /lib/modules/$(shell uname -r)/source
> >>>>>>>>> +   ifneq ($(shell grep -A 1 "ndo_tx_timeout" $(KSRC)/include/linux/netdevice.h | grep -o txqueue),)
> >>>>>>>>> +      MODULE_CFLAGS += -DHAVE_TX_TIMEOUT_TXQUEUE
> >>>>>>>>> +   endif
> >>>>>>>>> +endif
> >>>>>>>>
> >>>>>>>> Back in the day we tried various "is Suse and kernel version x.y"
> >>>>>>>> approaches, but they failed as there was no clear version throughout
> >>>>>>>> all of the Suse streams (leap, tumbleweed, sles) that worked well for
> >>>>>>>> all.
> >>>>>>>> This change here follows the upstream approach of "just check if it is there".
> >>>>>>>>
> >>>>>>>> I've applied this to 19.11 and did test builds across various distributions:
> >>>>>>>> 1. no non-suse build changed
> >>>>>>>> 2. suse builds stayed as-is or improved
> >>>>>>>> Formerly failing:
> >>>>>>>>        openSUSE_Factory_ARM aarch64
> >>>>>>>>        SLE_15  x86_64 -> now working
> >>>>>>>>        openSUSE_Leap_15.3 x86_64 -> now working
> >>>>>>>>        openSUSE_Tumbleweed  x86_64 -> still failing
> >>>>>>>> Formerly working:
> >>>>>>>>        SLE_12_SP4 x86_64 ppc64le -> still fine
> >>>>>>>>        openSUSE_Factory_ARM armv7l  -> still fine
> >>>>>>>>        openSUSE_Leap_15.2 x86_64  -> still fine
> >>>>>>>>
> >>>>>>>
> >>>>>>> Thanks Kalesh for the fix, and thanks Christian for testing.
> >>>>>>>
> >>>>>>> I was expecting this approach will fix all builds, after patch only
> >>>>>>> 'openSUSE_Tumbleweed' is failing, right? I will check it.
> >>>>>>
> >>>>>> As just discussed on IRC, yes and the log for that is at
> >>>>>> https://build.opensuse.org/package/live_build_log/home:cpaelzer:branches:home:bluca:dpdk/dpdk-19.11/openSUSE_Tumbleweed/x86_64
> >>>>>>
> >>>>>> It also is affected by an issue around  -Werror=implicit-fallthrough,
> >>>>>> so even with KNI fixed it likely is going to fail.
> >>>>>>
> >>>>>>> And I think you need the fix as a patch anyway, @Kalesh are you
> >>>>>>> planning to send the patch?
> >>>>>>
> >>>>>> I don't need it, as I have already grabbed and preliminary added it:
> >>>>>> https://github.com/cpaelzer/dpdk-stable-queue/commit/d43fa3e198c08a3a76d70f4565b31ad3ab5f29c4
> >>>>>>
> >>>>>> But surely, once/If you come up with a full patch that also includes
> >>>>>> tumbleweed I can replace it with yours.
> >>>>>>
> >>>>>
> >>>>> 'tumbleweed' error is odd, it complains about macro being redefined,
> >>>>> not sure why only in this platform we are getting an error.
> >>>>>
> >>>>> Macro is only defined in one place, but indeed header file included
> >>>>> multiple times, one direct and one indirect, so macro defined multiple
> >>>>> times but without value, so it should be OK and it is OK for other
> >>>>> platforms, it is defined as:
> >>>>> #define HAVE_TX_TIMEOUT_TXQUEUE
> >>>>>
> >>>>> Another option is that macro is defined in some other header file,
> >>>>> although I think that is very unlikely, can you please test with
> >>>>> below change to rule out that option:
> >>>>
> >>>> I'm testing that and will let you know in a bit ...
> >>>
> >>> Hi Ferruh,
> >>> with your change the build now works.
> >>> So indeed the symbol might have been defined elsewhere.
> >>>
> >>
> >> Interesting, this is self note to prefix 'RTE_' future macros.
> >
> > While generally an interesting Idea I do not know what I saw yesterday.
> > I have rebuilt it three times today and must say that other than I
> > said before - it does not work with RTE_*.
> >
>
> This was more expected result :)
>
> Is there a way to debug that environment?

Not that I'd know of.
Get a local VM of the same would be my way :-/

@Luca - you came up with this CI platform for the LTS builds, do you
know of a way to debug in-place?

> > Actually even worse than before, with RTE_.. even opensuse_leap15.3
> > and SLES15 fail again :-/
> >
> >>> https://build.opensuse.org/package/live_build_log/home:cpaelzer:branches:home:bluca:dpdk/dpdk-19.11/openSUSE_Tumbleweed/x86_64
> >>>
> >>> It still fails later with the "-Werror=implicit-fallthrough=" but that
> >>> is a different problem
> >>> => https://bugs.dpdk.org/show_bug.cgi?id=907
> >>>
> >>
> >> Yep, this is igb_uio error, I assigned the bug to myself and will look at it.
> >>
> >>> Ferruh - are you ok if I merge your suggestion with the backport I got
> >>> from Kalesh?
> >>>
> >>
> >> Sure.
> >> But would you mind sending the final patch to the stable mail list as record?
> >> Or I can do the same if you prefer?
> >>
> >>>>> diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h
> >>>>> index 664785674ff1..71846419f437 100644
> >>>>> --- a/kernel/linux/kni/compat.h
> >>>>> +++ b/kernel/linux/kni/compat.h
> >>>>> @@ -135,7 +135,7 @@
> >>>>>            (defined(RHEL_RELEASE_CODE) && \
> >>>>>             RHEL_RELEASE_VERSION(8, 3) <= RHEL_RELEASE_CODE) || \
> >>>>>             (defined(CONFIG_SUSE_KERNEL) && defined(HAVE_ARG_TX_QUEUE))
> >>>>> -#define HAVE_TX_TIMEOUT_TXQUEUE
> >>>>> +#define RTE_HAVE_TX_TIMEOUT_TXQUEUE
> >>>>>     #endif
> >>>>>
> >>>>>     #if KERNEL_VERSION(5, 9, 0) > LINUX_VERSION_CODE
> >>>>> diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
> >>>>> index c8bad5f197ca..7397de4659b2 100644
> >>>>> --- a/kernel/linux/kni/kni_net.c
> >>>>> +++ b/kernel/linux/kni/kni_net.c
> >>>>> @@ -623,7 +623,7 @@ kni_net_rx(struct kni_dev *kni)
> >>>>>     /*
> >>>>>      * Deal with a transmit timeout.
> >>>>>      */
> >>>>> -#ifdef HAVE_TX_TIMEOUT_TXQUEUE
> >>>>> +#ifdef RTE_HAVE_TX_TIMEOUT_TXQUEUE
> >>>>>     static void
> >>>>>     kni_net_tx_timeout(struct net_device *dev, unsigned int txqueue)
> >>>>>     #else
> >>>>>
> >>>>>
> >>>>>>>> Past fixes always "inverted" the result, by fixing some but breaking others.
> >>>>>>>> This new patch works in "not breaking any formerly working build" but
> >>>>>>>> at the same time fixing a few builds.
> >>>>>>>> Therefore -> applied & thanks!
> >>>>>>>>
> >>>>>>>> I'll likely tag -rc2 before the end of the week.
> >>>>>>>> The good thing is that (so far) we have:
> >>>>>>>> 1. a non functional change
> >>>>>>>> 2. a change fixing clang-13 builds (TBH only one of many needed clang13 issues)
> >>>>>>>> 3. a change fixing sles15SP3 builds
> >>>>>>>>
> >>>>>>>> Due to those, no current ongoing tests will have to be restarted.
> >>>>>>>> Whoever was able to build, can continue the current tests.
> >>>>>>>> Whoever was blocked by SLES15SP3 or clang-13 had no tests other than a
> >>>>>>>> failing build and can work with -rc2 then.
> >>>>>>>> I'll explain the same in the mail about -rc2.
> >>>>>>>>
> >>>>>>>>>      -include /etc/lsb-release
> >>>>>>>>>
> >>>>>>>>>      ifeq ($(DISTRIB_ID),Ubuntu)
> >>>>>>>>>
> >>>>>>>>> Regards,
> >>>>>>>>> Kalesh
> >>>>>>>>
> >>>>>>>> [snip]
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>> --
> >>>> Christian Ehrhardt
> >>>> Staff Engineer, Ubuntu Server
> >>>> Canonical Ltd
> >>>
> >>>
> >>>
> >>
> >
> >
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd