DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>,
	"Abhishek Marathe" <Abhishek.Marathe@microsoft.com>,
	Akhil Goyal <akhil.goyal@nxp.com>,
	 Ali Alnubani <alialnu@nvidia.com>,
	David Christensen <drc@linux.vnet.ibm.com>,
	Hariprasad Govindharajan <hariprasad.govindharajan@intel.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	Ian Stokes <ian.stokes@intel.com>,
	Jerin Jacob <jerinj@marvell.com>,
	John McNamara <john.mcnamara@intel.com>,
	Ju-Hyoung Lee <juhlee@microsoft.com>,
	Kevin Traynor <ktraynor@redhat.com>,
	Luca Boccassi <bluca@debian.org>, Pei Zhang <pezhang@redhat.com>,
	Raslan Darawsheh <rasland@nvidia.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	<benjamin.walker@intel.com>, dpdk stable <stable@dpdk.org>,
	dpdk-dev <dev@dpdk.org>, <pingx.yu@intel.com>,
	<qian.q.xu@intel.com>, <yuan.peng@intel.com>,
	<zhaoyan.chen@intel.com>
Subject: Re: 19.11.11 patches review and test
Date: Tue, 14 Dec 2021 13:10:08 +0000	[thread overview]
Message-ID: <9f445304-f5d6-49a7-1f0f-65db8725d1ca@intel.com> (raw)
In-Reply-To: <CAATJJ0+6WnkGOX9=r=AumsmXqqyiTgn8C9Q5yPNu+O9Je13hPw@mail.gmail.com>

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:

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]
>>>
>>
> 
> 


  parent reply	other threads:[~2021-12-14 13:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 14:43 christian.ehrhardt
2021-12-13  4:14 ` Kalesh Anakkur Purayil
2021-12-13 11:06   ` Ferruh Yigit
2021-12-13 12:39     ` Christian Ehrhardt
2021-12-13 12:58       ` Ferruh Yigit
2021-12-13 13:32         ` Christian Ehrhardt
2021-12-13 13:49           ` Kalesh Anakkur Purayil
2021-12-13 14:07             ` Ferruh Yigit
2021-12-14  5:49               ` Kalesh Anakkur Purayil
2021-12-14  7:44                 ` Christian Ehrhardt
2021-12-14 10:13                   ` Ferruh Yigit
2021-12-14 11:39                     ` Christian Ehrhardt
2021-12-14 12:08                       ` Ferruh Yigit
2021-12-14 13:57                         ` Christian Ehrhardt
2021-12-14 13:10                       ` Ferruh Yigit [this message]
2021-12-14 13:58                         ` Christian Ehrhardt
2021-12-14 14:46                           ` Christian Ehrhardt
2021-12-14 14:52                             ` Ferruh Yigit
2021-12-15 13:17                               ` Christian Ehrhardt
2021-12-15 14:44                                 ` Ferruh Yigit
2021-12-16  7:21                                   ` Christian Ehrhardt
2021-12-14  7:14 ` Jiang, YuX
2021-12-14  7:45   ` Christian Ehrhardt
2021-12-14  8:05     ` Christian Ehrhardt
2021-12-15 11:04   ` Jiang, YuX
2021-12-15 13:16     ` Christian Ehrhardt
2021-12-17  7:05     ` Jiang, YuX
2021-12-17  7:47       ` Christian Ehrhardt
2021-12-14 13:51 ` Pei Zhang
2021-12-14 13:52   ` Christian Ehrhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9f445304-f5d6-49a7-1f0f-65db8725d1ca@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=Abhishek.Marathe@microsoft.com \
    --cc=akhil.goyal@nxp.com \
    --cc=alialnu@nvidia.com \
    --cc=benjamin.walker@intel.com \
    --cc=bluca@debian.org \
    --cc=christian.ehrhardt@canonical.com \
    --cc=dev@dpdk.org \
    --cc=drc@linux.vnet.ibm.com \
    --cc=hariprasad.govindharajan@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=ian.stokes@intel.com \
    --cc=jerinj@marvell.com \
    --cc=john.mcnamara@intel.com \
    --cc=juhlee@microsoft.com \
    --cc=kalesh-anakkur.purayil@broadcom.com \
    --cc=ktraynor@redhat.com \
    --cc=pezhang@redhat.com \
    --cc=pingx.yu@intel.com \
    --cc=qian.q.xu@intel.com \
    --cc=rasland@nvidia.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=yuan.peng@intel.com \
    --cc=zhaoyan.chen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).