diff mbox series

[ovs-dev,dpdk-latest] ci: Remove -Wno-cast-align from CI

Message ID 20211205073442.16397-1-elibr@nvidia.com
State Accepted
Headers show
Series [ovs-dev,dpdk-latest] ci: Remove -Wno-cast-align from CI | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eli Britstein Dec. 5, 2021, 7:34 a.m. UTC
Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
Remove ignoring them if they occur.

GitHub actions:
v1: https://github.com/elibritstein/OVS/actions/runs/1540651133

[1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
[2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
[3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 .ci/linux-build.sh   | 4 ----
 utilities/ovs-dev.py | 1 -
 2 files changed, 5 deletions(-)

Comments

Eelco Chaudron Jan. 4, 2022, 2:56 p.m. UTC | #1
On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:

> Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
> Remove ignoring them if they occur.
>
> GitHub actions:
> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
>
> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")


Changes look fine to me, maybe you can include the fixes tags to mention the commits added these?!

Acked-by: Eelco Chaudron <echaudro@redhat.com>


> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---
>  .ci/linux-build.sh   | 4 ----
>  utilities/ovs-dev.py | 1 -
>  2 files changed, 5 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index e20cc6ad0..65578880b 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -226,10 +226,6 @@ if [ "$DPDK" ]; then
>          DPDK_VER="20.11.1"
>      fi
>      install_dpdk $DPDK_VER
> -    if [ "$CC" = "clang" ]; then
> -        # Disregard cast alignment errors until DPDK is fixed
> -        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
> -    fi
>      if [ -n "$DPDK_EXPERIMENTAL" ]; then
>          CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -DALLOW_EXPERIMENTAL_API"
>      fi
> diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
> index c45788acd..534c5e7f1 100755
> --- a/utilities/ovs-dev.py
> +++ b/utilities/ovs-dev.py
> @@ -90,7 +90,6 @@ def conf():
>
>      if options.with_dpdk:
>          configure.append("--with-dpdk=" + options.with_dpdk)
> -        cflags += " -Wno-cast-align -Wno-bad-function-cast"  # DPDK warnings.

Guess this script only works with older kernels due to always including the --with-linux= option.

>      if options.optimize is None:
>          options.optimize = 0
> -- 
> 2.28.0.2311.g225365fb51
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Eli Britstein Jan. 4, 2022, 3:44 p.m. UTC | #2
On 1/4/2022 4:56 PM, Eelco Chaudron wrote:
> External email: Use caution opening links or attachments
>
>
> On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:
>
>> Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
>> Remove ignoring them if they occur.
>>
>> GitHub actions:
>> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
>>
>> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
>> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
>> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")
>
> Changes look fine to me, maybe you can include the fixes tags to mention the commits added these?!

It was inevitable from day 1, since DPDK always had those issues (until 
now), so it was not a "bug" to fix now.

For utilities/ovs-dev.py, the "-W" is there from the first commit that 
added dpdk support:

25dfecf88742 ("ovs-dev.py: Add support for dpdk builds.")

For .ci/linux-build.sh, it's added in this commit:

ecc3c395b5a6 ("travis: Fix DPDK build and treat bad-function-cast 
warning as non-error")

See in its commit message:

     Due to incorrect casts in the DPDK headers, we have to disable
     bad-function-cast and cast-align warnings as being treated as errors
     for now.

>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>
>
>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>> ---
>>   .ci/linux-build.sh   | 4 ----
>>   utilities/ovs-dev.py | 1 -
>>   2 files changed, 5 deletions(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index e20cc6ad0..65578880b 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -226,10 +226,6 @@ if [ "$DPDK" ]; then
>>           DPDK_VER="20.11.1"
>>       fi
>>       install_dpdk $DPDK_VER
>> -    if [ "$CC" = "clang" ]; then
>> -        # Disregard cast alignment errors until DPDK is fixed
>> -        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
>> -    fi
>>       if [ -n "$DPDK_EXPERIMENTAL" ]; then
>>           CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -DALLOW_EXPERIMENTAL_API"
>>       fi
>> diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
>> index c45788acd..534c5e7f1 100755
>> --- a/utilities/ovs-dev.py
>> +++ b/utilities/ovs-dev.py
>> @@ -90,7 +90,6 @@ def conf():
>>
>>       if options.with_dpdk:
>>           configure.append("--with-dpdk=" + options.with_dpdk)
>> -        cflags += " -Wno-cast-align -Wno-bad-function-cast"  # DPDK warnings.
> Guess this script only works with older kernels due to always including the --with-linux= option.
>
>>       if options.optimize is None:
>>           options.optimize = 0
>> --
>> 2.28.0.2311.g225365fb51
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Eelco Chaudron Jan. 4, 2022, 3:53 p.m. UTC | #3
On 4 Jan 2022, at 16:44, Eli Britstein wrote:

> On 1/4/2022 4:56 PM, Eelco Chaudron wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:
>>
>>> Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
>>> Remove ignoring them if they occur.
>>>
>>> GitHub actions:
>>> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
>>>
>>> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
>>> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
>>> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")
>>
>> Changes look fine to me, maybe you can include the fixes tags to mention the commits added these?!
>
> It was inevitable from day 1, since DPDK always had those issues (until now), so it was not a "bug" to fix now.
>
> For utilities/ovs-dev.py, the "-W" is there from the first commit that added dpdk support:
>
> 25dfecf88742 ("ovs-dev.py: Add support for dpdk builds.")
>
> For .ci/linux-build.sh, it's added in this commit:
>
> ecc3c395b5a6 ("travis: Fix DPDK build and treat bad-function-cast warning as non-error")
>
> See in its commit message:
>
>     Due to incorrect casts in the DPDK headers, we have to disable
>     bad-function-cast and cast-align warnings as being treated as errors
>     for now.

Thanks for finding out the commit ids. Guess Ilya (or whichever maintainer commits this patch) can add them if they feel the need!

Cheers,

Eelco

>>
>> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>>
>>
>>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>>> ---
>>>   .ci/linux-build.sh   | 4 ----
>>>   utilities/ovs-dev.py | 1 -
>>>   2 files changed, 5 deletions(-)
>>>
>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>> index e20cc6ad0..65578880b 100755
>>> --- a/.ci/linux-build.sh
>>> +++ b/.ci/linux-build.sh
>>> @@ -226,10 +226,6 @@ if [ "$DPDK" ]; then
>>>           DPDK_VER="20.11.1"
>>>       fi
>>>       install_dpdk $DPDK_VER
>>> -    if [ "$CC" = "clang" ]; then
>>> -        # Disregard cast alignment errors until DPDK is fixed
>>> -        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
>>> -    fi
>>>       if [ -n "$DPDK_EXPERIMENTAL" ]; then
>>>           CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -DALLOW_EXPERIMENTAL_API"
>>>       fi
>>> diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
>>> index c45788acd..534c5e7f1 100755
>>> --- a/utilities/ovs-dev.py
>>> +++ b/utilities/ovs-dev.py
>>> @@ -90,7 +90,6 @@ def conf():
>>>
>>>       if options.with_dpdk:
>>>           configure.append("--with-dpdk=" + options.with_dpdk)
>>> -        cflags += " -Wno-cast-align -Wno-bad-function-cast"  # DPDK warnings.
>> Guess this script only works with older kernels due to always including the --with-linux= option.
>>
>>>       if options.optimize is None:
>>>           options.optimize = 0
>>> --
>>> 2.28.0.2311.g225365fb51
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Jan. 4, 2022, 7:55 p.m. UTC | #4
On 1/4/22 16:53, Eelco Chaudron wrote:
> 
> 
> On 4 Jan 2022, at 16:44, Eli Britstein wrote:
> 
>> On 1/4/2022 4:56 PM, Eelco Chaudron wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:
>>>
>>>> Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
>>>> Remove ignoring them if they occur.
>>>>
>>>> GitHub actions:
>>>> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
>>>>
>>>> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
>>>> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
>>>> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")
>>>
>>> Changes look fine to me, maybe you can include the fixes tags to mention the commits added these?!
>>
>> It was inevitable from day 1, since DPDK always had those issues (until now), so it was not a "bug" to fix now.
>>
>> For utilities/ovs-dev.py, the "-W" is there from the first commit that added dpdk support:
>>
>> 25dfecf88742 ("ovs-dev.py: Add support for dpdk builds.")
>>
>> For .ci/linux-build.sh, it's added in this commit:
>>
>> ecc3c395b5a6 ("travis: Fix DPDK build and treat bad-function-cast warning as non-error")
>>
>> See in its commit message:
>>
>>     Due to incorrect casts in the DPDK headers, we have to disable
>>     bad-function-cast and cast-align warnings as being treated as errors
>>     for now.
> 
> Thanks for finding out the commit ids. Guess Ilya (or whichever maintainer commits this patch) can add them if they feel the need!
> 
> Cheers,
> 
> Eelco
> 
>>>
>>> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>>>
>>>
>>>> Signed-off-by: Eli Britstein <elibr@nvidia.com>

Thanks, Eli and Eelco!

Since we're already on 21.11, I rebased this patch and applied
directly to master.

Speaking of cast-align, it would be great to have something like
this on the DPDK side:

diff --git a/config/meson.build b/config/meson.build
index 805d5d51d0..4c47b2d63c 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -251,6 +251,7 @@ warning_flags = [
         '-Wextra',
 
         # additional warnings in alphabetical order
+        '-Wcast-align',
         '-Wcast-qual',
         '-Wdeprecated',
         '-Wformat',
---

But I don't know if that will cause any problems.

Best regards, Ilya Maximets.

>>>> ---
>>>>   .ci/linux-build.sh   | 4 ----
>>>>   utilities/ovs-dev.py | 1 -
>>>>   2 files changed, 5 deletions(-)
>>>>
>>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>>> index e20cc6ad0..65578880b 100755
>>>> --- a/.ci/linux-build.sh
>>>> +++ b/.ci/linux-build.sh
>>>> @@ -226,10 +226,6 @@ if [ "$DPDK" ]; then
>>>>           DPDK_VER="20.11.1"
>>>>       fi
>>>>       install_dpdk $DPDK_VER
>>>> -    if [ "$CC" = "clang" ]; then
>>>> -        # Disregard cast alignment errors until DPDK is fixed
>>>> -        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
>>>> -    fi
>>>>       if [ -n "$DPDK_EXPERIMENTAL" ]; then
>>>>           CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -DALLOW_EXPERIMENTAL_API"
>>>>       fi
>>>> diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
>>>> index c45788acd..534c5e7f1 100755
>>>> --- a/utilities/ovs-dev.py
>>>> +++ b/utilities/ovs-dev.py
>>>> @@ -90,7 +90,6 @@ def conf():
>>>>
>>>>       if options.with_dpdk:
>>>>           configure.append("--with-dpdk=" + options.with_dpdk)
>>>> -        cflags += " -Wno-cast-align -Wno-bad-function-cast"  # DPDK warnings.
>>> Guess this script only works with older kernels due to always including the --with-linux= option.

Yeah.  I'm actually not sure if anyone uses this script.

>>>
>>>>       if options.optimize is None:
>>>>           options.optimize = 0
>>>> --
>>>> 2.28.0.2311.g225365fb51
>>>>
David Marchand Jan. 6, 2022, 10:41 a.m. UTC | #5
On Tue, Jan 4, 2022 at 8:55 PM Ilya Maximets <i.maximets@ovn.org> wrote:
> On 1/4/22 16:53, Eelco Chaudron wrote:
> > On 4 Jan 2022, at 16:44, Eli Britstein wrote:
> >> On 1/4/2022 4:56 PM, Eelco Chaudron wrote:
> >>> On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:
> >>>
> >>>> Following [1]-[3] in DPDK, there are no more such warnings from DPDK.
> >>>> Remove ignoring them if they occur.
> >>>>
> >>>> GitHub actions:
> >>>> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
> >>>>
> >>>> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert function")
> >>>> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset macro")
> >>>> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy functions")
> >>>

[snip]

> Thanks, Eli and Eelco!
>
> Since we're already on 21.11, I rebased this patch and applied
> directly to master.
>
> Speaking of cast-align, it would be great to have something like
> this on the DPDK side:
>
> diff --git a/config/meson.build b/config/meson.build
> index 805d5d51d0..4c47b2d63c 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -251,6 +251,7 @@ warning_flags = [
>          '-Wextra',
>
>          # additional warnings in alphabetical order
> +        '-Wcast-align',
>          '-Wcast-qual',
>          '-Wdeprecated',
>          '-Wformat',
> ---
>
> But I don't know if that will cause any problems.

This is something that DPDK had in the past, when building with make + gcc.
It did not get to meson because of clang.
https://git.dpdk.org/dpdk/commit?id=524a0d5d66b9a54242623aab9ace9ce4a7ec1347

On DPDK side, we would have to analyze/fix issues with clang, or
enable this check only for gcc.
Copied some people in case they have more input on this topic.
Richardson, Bruce Jan. 7, 2022, 9:17 a.m. UTC | #6
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, January 6, 2022 10:42 AM
> To: Ilya Maximets <i.maximets@ovn.org>
> Cc: Eelco Chaudron <echaudro@redhat.com>; Eli Britstein
> <elibr@nvidia.com>; ovs dev <dev@openvswitch.org>; Richardson, Bruce
> <bruce.richardson@intel.com>; Luca Boccassi <bluca@debian.org>; Thomas
> Monjalon <thomas@monjalon.net>
> Subject: Re: [ovs-dev] [PATCH dpdk-latest] ci: Remove -Wno-cast-align from
> CI
> 
> On Tue, Jan 4, 2022 at 8:55 PM Ilya Maximets <i.maximets@ovn.org> wrote:
> > On 1/4/22 16:53, Eelco Chaudron wrote:
> > > On 4 Jan 2022, at 16:44, Eli Britstein wrote:
> > >> On 1/4/2022 4:56 PM, Eelco Chaudron wrote:
> > >>> On 5 Dec 2021, at 8:34, Eli Britstein via dev wrote:
> > >>>
> > >>>> Following [1]-[3] in DPDK, there are no more such warnings from
> DPDK.
> > >>>> Remove ignoring them if they occur.
> > >>>>
> > >>>> GitHub actions:
> > >>>> v1: https://github.com/elibritstein/OVS/actions/runs/1540651133
> > >>>>
> > >>>> [1] a3f8d0587188 ("net: avoid cast-align warning in VLAN insert
> function")
> > >>>> [2] da0333c8790b ("mbuf: avoid cast-align warning in data offset
> macro")
> > >>>> [3] 6de430b7079e ("eal/x86: avoid cast-align warning in memcpy
> functions")
> > >>>
> 
> [snip]
> 
> > Thanks, Eli and Eelco!
> >
> > Since we're already on 21.11, I rebased this patch and applied
> > directly to master.
> >
> > Speaking of cast-align, it would be great to have something like
> > this on the DPDK side:
> >
> > diff --git a/config/meson.build b/config/meson.build
> > index 805d5d51d0..4c47b2d63c 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -251,6 +251,7 @@ warning_flags = [
> >          '-Wextra',
> >
> >          # additional warnings in alphabetical order
> > +        '-Wcast-align',
> >          '-Wcast-qual',
> >          '-Wdeprecated',
> >          '-Wformat',
> > ---
> >
> > But I don't know if that will cause any problems.
> 
> This is something that DPDK had in the past, when building with make +
> gcc.
> It did not get to meson because of clang.
> https://git.dpdk.org/dpdk/commit?id=524a0d5d66b9a54242623aab9ace9ce4a7ec13
> 47
> 
> On DPDK side, we would have to analyze/fix issues with clang, or
> enable this check only for gcc.
> Copied some people in case they have more input on this topic.
> 

It would be nice to get the issues fixed and the flag added to DPDK builds. In the absence of having those fixed, my personal preference is *not* to enable for gcc only, on the basis that I don't want us to start having different lists of warning flags to enable for different compilers. I prefer the scheme of having a list of flags and then enabling them based on availability rather than on compiler or compiler version.

/Bruce
diff mbox series

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e20cc6ad0..65578880b 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -226,10 +226,6 @@  if [ "$DPDK" ]; then
         DPDK_VER="20.11.1"
     fi
     install_dpdk $DPDK_VER
-    if [ "$CC" = "clang" ]; then
-        # Disregard cast alignment errors until DPDK is fixed
-        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
-    fi
     if [ -n "$DPDK_EXPERIMENTAL" ]; then
         CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -DALLOW_EXPERIMENTAL_API"
     fi
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index c45788acd..534c5e7f1 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -90,7 +90,6 @@  def conf():
 
     if options.with_dpdk:
         configure.append("--with-dpdk=" + options.with_dpdk)
-        cflags += " -Wno-cast-align -Wno-bad-function-cast"  # DPDK warnings.
 
     if options.optimize is None:
         options.optimize = 0