[ovs-dev] [PATCH v3 ovn 3/4] ovn-northd: Refactor parsing of *_force_snat_ip.
Dumitru Ceara
dceara at redhat.com
Mon Sep 28 07:53:48 UTC 2020
On 9/22/20 8:11 AM, Numan Siddique wrote:
>
>
> On Mon, Sep 21, 2020 at 12:32 PM Han Zhou <hzhou at ovn.org
> <mailto:hzhou at ovn.org>> wrote:
>
> On Thu, Sep 17, 2020 at 5:51 AM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
> >
> > Avoid reparsing the *_force_snat_ip addresses for every logical router
> port.
> > These addresses are defined once for the whole router.
> >
>
> The commit message is a little misleading. Originally it wasn't
> reparsing
> for every logical router port. It was per-router. This patch avoids one
> extra parsing.
>
> In addition, another minor comment is that the function name
> "empty_lport_addresses" may be "lport_addresses_is_empty" to follow the
> naming convention of a lot of xxx_is_empty() function names in OVN/OVS.
>
> With these addressed:
> Acked-by: Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>>
>
>
>
> Hi Dumitru,
>
> The compilation fails for clang with the below messages
>
> *****
>
> libtool: link: clang -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare
> -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum
> -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes
> -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers
> -Wthread-safety -fno-strict-aliasing -Wswitch-bool
> -Wlogical-not-parentheses -Wsizeof-array-argument -Wshift-negative-value
> -Qunused-arguments -Wshadow -Wno-null-pointer-arithmetic
> -Warray-bounds-pointer-arithmetic -Werror -Werror -g -O2 -o ic/ovn-ic
> ic/ovn-ic.o lib/.libs/libovn.a
> /home/nusiddiq/work/ovs_ovn/ovs/ovsdb/.libs/libovsdb.a
> /home/nusiddiq/work/ovs_ovn/ovs/lib/.libs/libopenvswitch.a -lssl
> -lcrypto -lcap-ng
> /home/nusiddiq/work/ovs_ovn/ovs/lib/.libs/libopenvswitchavx512.a
> -lpthread -lrt -lm -lunbound
> ../northd/ovn-northd.c:681:40: error: address of
> 'od->dnat_force_snat_addrs.n_ipv4_addrs' will always evaluate to 'true'
> [-Werror,-Wpointer-bool-conversion]
> if (&od->dnat_force_snat_addrs.n_ipv4_addrs) {
> ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> ../northd/ovn-northd.c:685:40: error: address of
> 'od->dnat_force_snat_addrs.n_ipv6_addrs' will always evaluate to 'true'
> [-Werror,-Wpointer-bool-conversion]
> if (&od->dnat_force_snat_addrs.n_ipv6_addrs) {
> ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> *******
>
> I think there is no need for '&'.
>
> Thanks
> Numan
>
Hi Numan,
You're right, this is a very nasty copy/paste error. I'll fix it in a
new revision. I'll also try to add a test to spot this kind of issues
earlier.
Regards,
Dumitru
More information about the dev
mailing list