Re: [ovs-dev] [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions
On 9/28/22 19:13, Kees Cook wrote:
On Wed, Sep 28, 2022 at 09:26:15AM +0200, Geert Uytterhoeven wrote:
Hi Kees,
On Fri, Sep 23, 2022 at 10:35 PM Kees Cook wrote:
The __malloc attribute should not be applied to "realloc" functions, as
the returned pointer may alias the storage of the prior pointer. Instead
of splitting __malloc from __alloc_size, which would be a huge amount of
churn, just create __realloc_size for the few cases where it is needed.
Additionally removes the conditional test for __alloc_size__, which is
always defined now.
Cc: Christoph Lameter
Cc: Pekka Enberg
Cc: David Rientjes
Cc: Joonsoo Kim
Cc: Andrew Morton
Cc: Vlastimil Babka
Cc: Roman Gushchin
Cc: Hyeonggon Yoo <42.hye...@gmail.com>
Cc: Marco Elver
Cc: linux...@kvack.org
Signed-off-by: Kees Cook
Thanks for your patch, which is now commit 63caa04ec60583b1 ("slab:
Remove __malloc attribute from realloc functions") in next-20220927.
nore...@ellerman.id.au reported all gcc8-based builds to fail
(e.g. [1], more at [2]):
In file included from :
./include/linux/percpu.h: In function ‘__alloc_reserved_percpu’:
././include/linux/compiler_types.h:279:30: error: expected
declaration specifiers before ‘__alloc_size__’
#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
^~
./include/linux/percpu.h:120:74: note: in expansion of macro ‘__alloc_size’
[...]
It's building fine with e.g. gcc-9 (which is my usual m68k cross-compiler).
Reverting this commit on next-20220927 fixes the issue.
[1] http://kisskb.ellerman.id.au/kisskb/buildresult/14803908/
[2]
http://kisskb.ellerman.id.au/kisskb/head/1bd8b75fe6adeaa89d02968bdd811ffe708cf839/
Eek! Thanks for letting me know. I'm confused about this --
__alloc_size__ wasn't optional in compiler_attributes.h -- but obviously
I broke something! I'll go figure this out.
Even in latest next I can see at the end of include/linux/compiler-gcc.h
/*
* Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
* attribute) do not work, and must be disabled.
*/
#if GCC_VERSION < 90100
#undef __alloc_size__
#endif
-Kees
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] northd: Use separate SNAT for already-DNATted traffic.
On Tue, Sep 27, 2022 at 3:02 PM Mark Michelson wrote:
>
> Commit 4deac4509abbedd6ffaecf27eed01ddefccea40a introduced functionality
> in ovn-northd to use a single zone for SNAT and DNAT when possible. It
> accounts for certain situations, such as hairpinned traffic, where we
> still need a separate SNAT and DNAT zone.
>
> However, one situation it did not account for was when traffic traverses
> a logical router and is DNATted as a result of a load balancer, then
> when the traffic egresses the router, it needs to be SNATted. In this
> situation, we try to use the same CT zone for the SNAT as for the load
> balancer DNAT, which does not work.
>
> This commit fixes the issue by setting the DNAT_LOCAL bit in the initial
> stage of the egress pipeline if the packet was dnatted during the
> ingress pipeline. This ensures that when the SNAT stage is reached, a
> separate CT zone is used for SNAT.
>
> Signed-off-by: Mark Michelson
Acked-by: Numan Siddique
Numan
> ---
> v2 -> v3:
> * Rebased on top of current main branch
> * Fixed checkpatch issues from v2.
> * Accounted for the ct_label -> ct_mark change.
> * All tests are now passing (locally)
> ---
> northd/northd.c | 26 -
> northd/ovn-northd.8.xml | 16 ++
> tests/ovn-northd.at | 7 ++-
> tests/system-ovn.at | 117
> 4 files changed, 163 insertions(+), 3 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 84440a47f..5eac7fa51 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13633,7 +13633,8 @@ static void
> build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows,
> const struct hmap *ports, struct ds *match,
> struct ds *actions,
> -const struct shash *meter_groups)
> +const struct shash *meter_groups,
> +bool ct_lb_mark)
> {
> if (!od->nbr) {
> return;
> @@ -13827,6 +13828,26 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
> *od, struct hmap *lflows,
> }
> }
>
> +if (od->nbr->n_nat) {
> +ds_clear(match);
> +const char *ct_natted = ct_lb_mark ?
> +"ct_mark.natted" :
> +"ct_label.natted";
> +ds_put_format(match, "ip && %s == 1", ct_natted);
> +/* This flow is unique since it is in the egress pipeline but checks
> + * the value of ct_label.natted, which would have been set in the
> + * ingress pipeline. If a change is ever introduced that clears or
> + * otherwise invalidates the ct_label between the ingress and egress
> + * pipelines, then an alternative will need to be devised.
> + */
> +ds_clear(actions);
> +ds_put_cstr(actions, REGBIT_DST_NAT_IP_LOCAL" = 1; next;");
> +ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_CHECK_DNAT_LOCAL,
> +50, ds_cstr(match), ds_cstr(actions),
> +>nbr->header_);
> +
> +}
> +
> /* Handle force SNAT options set in the gateway router. */
> if (od->is_gw_router) {
> if (dnat_force_snat_ip) {
> @@ -13925,7 +13946,8 @@ build_lswitch_and_lrouter_iterate_by_od(struct
> ovn_datapath *od,
> build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows);
> build_lrouter_arp_nd_for_datapath(od, lsi->lflows, lsi->meter_groups);
> build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->ports, >match,
> ->actions, lsi->meter_groups);
> +>actions, lsi->meter_groups,
> +lsi->features->ct_no_masked_label);
> }
>
> /* Helper function to combine all lflow generation which is iterated by port.
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index dae961c87..177b6341d 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -4392,6 +4392,22 @@ nd_ns {
>
>
>
> +
> + This table also installs a priority-50 logical flow for each logical
> + router that has NATs configured on it. The flow has match
> + ip ct_label.natted == 1 and action
> + REGBIT_DST_NAT_IP_LOCAL = 1; next;. This is intended
> + to ensure that traffic that was DNATted locally will use a separate
> + conntrack zone for SNAT if SNAT is required later in the egress
> + pipeline. Note that this flow checks the value of
> + ct_label.natted, which is set in the ingress pipeline.
> + This means that ovn-northd assumes that this value is carried over
> + from the ingress pipeline to the egress pipeline and is not altered
> + or cleared. If conntrack label values are ever changed to be cleared
> + between the ingress and egress pipelines, then the match conditions
> + of this
Re: [ovs-dev] [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions
Hi Kees,
On Fri, Sep 23, 2022 at 10:35 PM Kees Cook wrote:
> The __malloc attribute should not be applied to "realloc" functions, as
> the returned pointer may alias the storage of the prior pointer. Instead
> of splitting __malloc from __alloc_size, which would be a huge amount of
> churn, just create __realloc_size for the few cases where it is needed.
>
> Additionally removes the conditional test for __alloc_size__, which is
> always defined now.
>
> Cc: Christoph Lameter
> Cc: Pekka Enberg
> Cc: David Rientjes
> Cc: Joonsoo Kim
> Cc: Andrew Morton
> Cc: Vlastimil Babka
> Cc: Roman Gushchin
> Cc: Hyeonggon Yoo <42.hye...@gmail.com>
> Cc: Marco Elver
> Cc: linux...@kvack.org
> Signed-off-by: Kees Cook
Thanks for your patch, which is now commit 63caa04ec60583b1 ("slab:
Remove __malloc attribute from realloc functions") in next-20220927.
nore...@ellerman.id.au reported all gcc8-based builds to fail
(e.g. [1], more at [2]):
In file included from :
./include/linux/percpu.h: In function ‘__alloc_reserved_percpu’:
././include/linux/compiler_types.h:279:30: error: expected
declaration specifiers before ‘__alloc_size__’
#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
^~
./include/linux/percpu.h:120:74: note: in expansion of macro ‘__alloc_size’
[...]
It's building fine with e.g. gcc-9 (which is my usual m68k cross-compiler).
Reverting this commit on next-20220927 fixes the issue.
[1] http://kisskb.ellerman.id.au/kisskb/buildresult/14803908/
[2]
http://kisskb.ellerman.id.au/kisskb/head/1bd8b75fe6adeaa89d02968bdd811ffe708cf839/
> ---
> include/linux/compiler_types.h | 13 +
> include/linux/slab.h | 12 ++--
> mm/slab_common.c | 4 ++--
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 4f2a819fd60a..f141a6f6b9f6 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,15 +271,12 @@ struct ftrace_likely_data {
>
> /*
> * Any place that could be marked with the "alloc_size" attribute is also
> - * a place to be marked with the "malloc" attribute. Do this as part of the
> - * __alloc_size macro to avoid redundant attributes and to avoid missing a
> - * __malloc marking.
> + * a place to be marked with the "malloc" attribute, except those that may
> + * be performing a _reallocation_, as that may alias the existing pointer.
> + * For these, use __realloc_size().
> */
> -#ifdef __alloc_size__
> -# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
> -#else
> -# define __alloc_size(x, ...) __malloc
> -#endif
> +#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
> +#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__)
>
> #ifndef asm_volatile_goto
> #define asm_volatile_goto(x...) asm goto(x)
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 0fefdf528e0d..41bd036e7551 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -184,7 +184,7 @@ int kmem_cache_shrink(struct kmem_cache *s);
> /*
> * Common kmalloc functions provided by all allocators
> */
> -void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
> __alloc_size(2);
> +void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
> __realloc_size(2);
> void kfree(const void *objp);
> void kfree_sensitive(const void *objp);
> size_t __ksize(const void *objp);
> @@ -647,10 +647,10 @@ static inline __alloc_size(1, 2) void
> *kmalloc_array(size_t n, size_t size, gfp_
> * @new_size: new size of a single member of the array
> * @flags: the type of memory to allocate (see kmalloc)
> */
> -static inline __alloc_size(2, 3) void * __must_check krealloc_array(void *p,
> - size_t
> new_n,
> - size_t
> new_size,
> - gfp_t
> flags)
> +static inline __realloc_size(2, 3) void * __must_check krealloc_array(void
> *p,
> + size_t
> new_n,
> + size_t
> new_size,
> + gfp_t
> flags)
> {
> size_t bytes;
>
> @@ -774,7 +774,7 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n,
> size_t size, gfp_t fla
> }
>
> extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t
> flags)
> - __alloc_size(3);
> + __realloc_size(3);
> extern void kvfree(const void *addr);
> extern void kvfree_sensitive(const void *addr, size_t len);
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
>
Re: [ovs-dev] [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions
On Wed, Sep 28, 2022 at 09:26:15AM +0200, Geert Uytterhoeven wrote:
> Hi Kees,
>
> On Fri, Sep 23, 2022 at 10:35 PM Kees Cook wrote:
> > The __malloc attribute should not be applied to "realloc" functions, as
> > the returned pointer may alias the storage of the prior pointer. Instead
> > of splitting __malloc from __alloc_size, which would be a huge amount of
> > churn, just create __realloc_size for the few cases where it is needed.
> >
> > Additionally removes the conditional test for __alloc_size__, which is
> > always defined now.
> >
> > Cc: Christoph Lameter
> > Cc: Pekka Enberg
> > Cc: David Rientjes
> > Cc: Joonsoo Kim
> > Cc: Andrew Morton
> > Cc: Vlastimil Babka
> > Cc: Roman Gushchin
> > Cc: Hyeonggon Yoo <42.hye...@gmail.com>
> > Cc: Marco Elver
> > Cc: linux...@kvack.org
> > Signed-off-by: Kees Cook
>
> Thanks for your patch, which is now commit 63caa04ec60583b1 ("slab:
> Remove __malloc attribute from realloc functions") in next-20220927.
>
> nore...@ellerman.id.au reported all gcc8-based builds to fail
> (e.g. [1], more at [2]):
>
> In file included from :
> ./include/linux/percpu.h: In function ‘__alloc_reserved_percpu’:
> ././include/linux/compiler_types.h:279:30: error: expected
> declaration specifiers before ‘__alloc_size__’
> #define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
> ^~
> ./include/linux/percpu.h:120:74: note: in expansion of macro
> ‘__alloc_size’
> [...]
>
> It's building fine with e.g. gcc-9 (which is my usual m68k cross-compiler).
> Reverting this commit on next-20220927 fixes the issue.
>
> [1] http://kisskb.ellerman.id.au/kisskb/buildresult/14803908/
> [2]
> http://kisskb.ellerman.id.au/kisskb/head/1bd8b75fe6adeaa89d02968bdd811ffe708cf839/
Eek! Thanks for letting me know. I'm confused about this --
__alloc_size__ wasn't optional in compiler_attributes.h -- but obviously
I broke something! I'll go figure this out.
-Kees
--
Kees Cook
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-build] |fail| pw1682469 [ovs-dev, dpdk-latest] netdev-dpdk: Report device bus specific information.
Hello Michael, On Mon, Sep 26, 2022 at 12:07 PM wrote: > > Test-Label: intel-ovs-compilation > Test-Status: fail > http://patchwork.ozlabs.org/api/patches/1682469/ > > AVX-512_compilation: failed > DPLCS Test: fail > DPIF Test: fail > MFEX Test: fail > Errors in DPCLS test: > git-pw patch apply 1682469 > Starting new HTTPS connection (1): patchwork.ozlabs.org > Starting new HTTPS connection (1): patchwork.ozlabs.org > Failed to apply patch: > fatal: Dirty index: cannot apply patches (dirty: .ci/linux-build.sh > .cirrus.yml .github/workflows/build-and-test.yml AUTHORS.rst > Documentation/automake.mk Documentation/faq/general.rst > Documentation/faq/releases.rst Documentation/howto/qos.rst > Documentation/howto/sflow.rst Documentation/howto/tunneling.rst > Documentation/howto/vlan.rst Documentation/intro/install/general.rst > Documentation/intro/install/index.rst > Documentation/intro/install/xenserver.rst Documentation/ref/ovs-appctl.8.rst > Documentation/ref/ovs-ctl.8.rst Documentation/topics/bonding.rst > Documentation/topics/integration.rst Documentation/tutorials/index.rst > Makefile.am NEWS README.rst acinclude.m4 build-aux/initial-tab-allowed-files > configure.ac debian/changelog debian/copyright.in debian/rules > include/openvswitch/dynamic-string.h lib/dpif-netdev-lookup.c > lib/dpif-netdev-private-dpif.c lib/dpif-netdev-private-extract.c > lib/dpif-netdev-private-extract.h lib/dpif-netlink.c lib/dynamic-string.c > lib/hmapx.c lib/libopenvswitch.pc.in lib/libsflow.pc.in lib/netdev-linux.c > lib/netdev-offload-tc.c lib/netdev-offload.h lib/netdev.c > lib/netlink-conntrack.c lib/ovs-numa.c lib/ovs-numa.h lib/ovs-thread.c > lib/ovs-thread.h lib/packets.c lib/smap.c lib/sset.c lib/tc.c > m4/ax_func_posix_memalign.m4 m4/openvswitch.m4 ofproto/libofproto.pc.in > ofproto/ofproto-dpif-xlate.c ovsdb/libovsdb.pc.in ovsdb/raft.c > ovsdb/transaction.c python/ovs/_json.c python/ovs/db/idl.py > python/ovs/socket_util.py python/setup.py rhel/openvswitch-fedora.spec.in > rhel/openvswitch.spec.in tests/MockXenAPI.py tests/automake.mk > tests/checkpatch.at tests/interface-reconfigure.at tests/ofproto-dpif.at > tests/ovs-macros.at tests/ovs-xapi-sync.at tests/system-dpdk.at > tests/system-ipsec.at tests/system-kmod-macros.at > tests/system-layer3-tunnels.at tests/system-offloads-traffic.at > tests/system-traffic.at tests/system-userspace-macros.at tests/test-ovsdb.c > tests/test-ovsdb.py tests/testsuite.at tests/tunnel-push-pop.at > tests/tunnel.at utilities/bugtool/ovs-bugtool.in utilities/checkpatch.py > utilities/ovs-ctl.in utilities/ovs-save utilities/ovs-testcontroller.8.in > vswitchd/automake.mk vswitchd/bridge.c vswitchd/vswitch.xml > vswitchd/xenserver.c vswitchd/xenserver.h xenserver/.gitignore > xenserver/GPLv2 xenserver/LICENSE xenserver/README.rst xenserver/automake.mk > xenserver/etc_init.d_openvswitch xenserver/etc_init.d_openvswitch-xapi-update > xenserver/etc_logrotate.d_openvswitch xenserver/etc_profile.d_openvswitch.sh > xenserver/etc_xapi.d_plugins_openvswitch-cfg-update > xenserver/etc_xensource_scripts_vif xenserver/openvswitch-xen.spec.in > xenserver/opt_xensource_libexec_InterfaceReconfigure.py > xenserver/opt_xensource_libexec_InterfaceReconfigureBridge.py > xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py > xenserver/opt_xensource_libexec_interface-reconfigure > xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py > xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > xenserver/usr_share_openvswitch_scripts_sysconfig.template) > .github/workflows/build-and-test.yml: needs merge There seems to be something broken here, could you look into it? > > > Errors in DPIF test: > make -j CFLAGS=-g -O0 -march=native > make all-am > make[1]: Entering directory '/root/ovs-dev' > (echo '/* -*- mode: c; buffer-read-only: t -*- */' && sed < ./lib/dirs.c.in \ > -e 's,[@]srcdir[@],.,g' \ > -e 's,[@]LOGDIR[@],"/usr/local/var/log/openvswitch",g' \ > -e 's,[@]RUNDIR[@],"/usr/local/var/run/openvswitch",g' \ > -e 's,[@]DBDIR[@],"/usr/local/etc/openvswitch",g' \ > -e 's,[@]bindir[@],"/usr/local/bin",g' \ > -e 's,[@]sysconfdir[@],"/usr/local/etc",g' \ > -e 's,[@]pkgdatadir[@],"/usr/local/share/openvswitch",g') \ > > lib/dirs.c.tmp && \ > mv -f lib/dirs.c.tmp lib/dirs.c > /bin/bash ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I.-I > ./include -I ./include -I ./lib -I ./lib-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 > -fno-strict-aliasing -Wswitch-bool -Wlogical-not-parentheses > -Wsizeof-array-argument -Wbool-compare -Wshift-negative-value > -Wduplicated-cond -Wshadow -Wmultistatement-macros -Wcast-align=strict > -mssse3 -include rte_config.h
Re: [ovs-dev] [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions
On 9/28/22 09:26, Geert Uytterhoeven wrote:
Hi Kees,
On Fri, Sep 23, 2022 at 10:35 PM Kees Cook wrote:
The __malloc attribute should not be applied to "realloc" functions, as
the returned pointer may alias the storage of the prior pointer. Instead
of splitting __malloc from __alloc_size, which would be a huge amount of
churn, just create __realloc_size for the few cases where it is needed.
Additionally removes the conditional test for __alloc_size__, which is
always defined now.
Cc: Christoph Lameter
Cc: Pekka Enberg
Cc: David Rientjes
Cc: Joonsoo Kim
Cc: Andrew Morton
Cc: Vlastimil Babka
Cc: Roman Gushchin
Cc: Hyeonggon Yoo <42.hye...@gmail.com>
Cc: Marco Elver
Cc: linux...@kvack.org
Signed-off-by: Kees Cook
Thanks for your patch, which is now commit 63caa04ec60583b1 ("slab:
Remove __malloc attribute from realloc functions") in next-20220927.
nore...@ellerman.id.au reported all gcc8-based builds to fail
(e.g. [1], more at [2]):
In file included from :
./include/linux/percpu.h: In function ‘__alloc_reserved_percpu’:
././include/linux/compiler_types.h:279:30: error: expected
declaration specifiers before ‘__alloc_size__’
#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
^~
./include/linux/percpu.h:120:74: note: in expansion of macro ‘__alloc_size’
[...]
It's building fine with e.g. gcc-9 (which is my usual m68k cross-compiler).
Reverting this commit on next-20220927 fixes the issue.
So IIUC it was wrong to remove the #ifdefs?
[1] http://kisskb.ellerman.id.au/kisskb/buildresult/14803908/
[2]
http://kisskb.ellerman.id.au/kisskb/head/1bd8b75fe6adeaa89d02968bdd811ffe708cf839/
---
include/linux/compiler_types.h | 13 +
include/linux/slab.h | 12 ++--
mm/slab_common.c | 4 ++--
3 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 4f2a819fd60a..f141a6f6b9f6 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -271,15 +271,12 @@ struct ftrace_likely_data {
/*
* Any place that could be marked with the "alloc_size" attribute is also
- * a place to be marked with the "malloc" attribute. Do this as part of the
- * __alloc_size macro to avoid redundant attributes and to avoid missing a
- * __malloc marking.
+ * a place to be marked with the "malloc" attribute, except those that may
+ * be performing a _reallocation_, as that may alias the existing pointer.
+ * For these, use __realloc_size().
*/
-#ifdef __alloc_size__
-# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
-#else
-# define __alloc_size(x, ...) __malloc
-#endif
+#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
+#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__)
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0fefdf528e0d..41bd036e7551 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -184,7 +184,7 @@ int kmem_cache_shrink(struct kmem_cache *s);
/*
* Common kmalloc functions provided by all allocators
*/
-void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
__alloc_size(2);
+void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
__realloc_size(2);
void kfree(const void *objp);
void kfree_sensitive(const void *objp);
size_t __ksize(const void *objp);
@@ -647,10 +647,10 @@ static inline __alloc_size(1, 2) void
*kmalloc_array(size_t n, size_t size, gfp_
* @new_size: new size of a single member of the array
* @flags: the type of memory to allocate (see kmalloc)
*/
-static inline __alloc_size(2, 3) void * __must_check krealloc_array(void *p,
- size_t
new_n,
- size_t
new_size,
- gfp_t flags)
+static inline __realloc_size(2, 3) void * __must_check krealloc_array(void *p,
+ size_t
new_n,
+ size_t
new_size,
+ gfp_t
flags)
{
size_t bytes;
@@ -774,7 +774,7 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n,
size_t size, gfp_t fla
}
extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t
flags)
- __alloc_size(3);
+ __realloc_size(3);
extern void kvfree(const void *addr);
extern void kvfree_sensitive(const void *addr, size_t len);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 17996649cfe3..457671ace7eb 100644
--- a/mm/slab_common.c
+++
[ovs-dev] [PATCH v2 ovn] northd: do not centralize FIP traffic if redirect-type is set to fixed
Keep FIP traffic distributed and do not centralize it even if the
CMS sets redirect-type option to bridged for distributed gateway port.
Tested-by: Jianlin Shi
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2007120
Signed-off-by: Lorenzo Bianconi
---
Changes since v1:
- add unit-test
- fix typos
---
northd/northd.c | 29 +
northd/northd.h | 2 ++
northd/ovn-northd.8.xml | 17 +
tests/ovn-northd.at | 37 +
4 files changed, 85 insertions(+)
diff --git a/northd/northd.c b/northd/northd.c
index 84440a47f..a60467963 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2616,6 +2616,13 @@ join_logical_ports(struct northd_input *input_data,
op->od = od;
ovs_list_push_back(>port_list, >dp_node);
+if (!od->redirect_bridged) {
+const char *redirect_type =
+smap_get(>options, "redirect-type");
+od->redirect_bridged =
+redirect_type && !strcasecmp(redirect_type, "bridged");
+}
+
if (op->nbrp->ha_chassis_group ||
op->nbrp->n_gateway_chassis) {
/* Additional "derived" ovn_port crp represents the
@@ -13731,6 +13738,28 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
*od, struct hmap *lflows,
100, ds_cstr(match),
ds_cstr(actions),
>header_);
+if (od->redirect_bridged && distributed) {
+ds_clear(match);
+ds_put_format(
+match,
+"outport == %s && ip%s.src == %s "
+"&& is_chassis_resident(\"%s\")",
+od->l3dgw_ports[0]->json_key,
+is_v6 ? "6" : "4", nat->logical_ip,
+nat->logical_port);
+ds_clear(actions);
+if (is_v6) {
+ds_put_cstr(actions,
+"get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;");
+} else {
+ds_put_cstr(actions,
+"get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;");
+}
+ovn_lflow_add_with_hint(lflows, od,
+S_ROUTER_IN_ARP_RESOLVE, 90,
+ds_cstr(match), ds_cstr(actions),
+>header_);
+}
sset_add(_entries, nat->external_ip);
}
}
diff --git a/northd/northd.h b/northd/northd.h
index aa9a3ae6e..60601803f 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -229,6 +229,8 @@ struct ovn_datapath {
size_t n_nat_entries;
bool has_distributed_nat;
+/* router datapath has a logical port with redirect-type set to bridged. */
+bool redirect_bridged;
/* Set of nat external ips on the router. */
struct sset external_ips;
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index d9e9a7345..009a380bd 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -4053,6 +4053,23 @@ outport = P
+
+
+ If the router datapath runs a port with redirect-type
+ set to bridged, for each distributed NAT rule with IP
+ A in the
+ column
+ and logical port P in the
+ column
+ of table, a priority-90 flow
+ with the match outport == Q ip.src ===
+ A is_chassis_resident(P),
+ where Q is the distributed logical router port and
+ action get_arp(outport, reg0); next; for IPv4 and
+ get_nd(outport, xxreg0); next; for IPv6.
+
+
+
Traffic with IP destination an address owned by the router should be
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 093e01c6d..a210fc575 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -7861,3 +7861,40 @@ check_column "" sb:load_balancer datapaths name=lb0
AT_CLEANUP
])
+
+AT_SETUP([check fip flows with redirect-type bridged])
+AT_KEYWORDS([fip-redirect-type-bridged])
+ovn_start
+
+ovn-nbctl lr-add R1
+ovn-nbctl lrp-add R1 R1-S0 02:ac:10:01:00:01 10.0.0.1/24 1000::a/64
+ovn-nbctl lrp-add R1 R1-PUB 02:ac:20:01:01:01 172.16.0.1/24 3000::a/64
+ovn-nbctl lrp-set-gateway-chassis R1-PUB hv1 20
+
+ovn-nbctl ls-add S0
+ovn-nbctl lsp-add S0 S0-R1
+ovn-nbctl lsp-set-type S0-R1 router
+ovn-nbctl lsp-set-addresses S0-R1 02:ac:10:01:00:01
+ovn-nbctl lsp-set-options S0-R1 router-port=R1-S0
+ovn-nbctl lsp-add S0 S0-P0
+ovn-nbctl lsp-set-addresses S0-P0 "50:54:00:00:00:03 10.0.0.3 1000::3"
+
+ovn-nbctl lr-nat-add R1
[ovs-dev] [PATCH ovn v2] ovs: Bump submodule to newer version and add related test
Specifically for:
02f31a1262fc ("ovsdb-idl: Preserve references for rows deleted in same IDL run
as their insertion.")
A OVN test case reproducing the bug (issue when port_binding is added/deleted
within the
same IDL) is also added.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2126450
Signed-off-by: Xavier Simonart
---
v2: updated ovs commit ID
---
ovs | 2 +-
tests/ovn.at | 29 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/ovs b/ovs
index 6f24c2bc7..5a686267d 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 6f24c2bc769afde0a390ce344de1a7d9c592e5a6
+Subproject commit 5a686267d36c5c4229ec801a9616ceb60740fbe3
diff --git a/tests/ovn.at b/tests/ovn.at
index 07f72dc31..739c09934 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32946,3 +32946,32 @@ check ovn-nbctl --wait=hv sync
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller: batch add port and delete port in same IDL])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl add-port br-int p1
+
+ovs-vsctl set interface p1 external-ids:iface-id=sw0-port1
+check ovn-nbctl --wait=hv sync
+ovn-appctl debug/pause
+OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) =
"xpaused"])
+
+ovn-nbctl ls-add sw0 -- lsp-add sw0 sw0-port1 -- lsp-set-addresses sw0-port1
"50:54:00:00:00:01 192.168.0.2"
+ovn-nbctl lsp-del sw0-port1
+
+ovn-appctl debug/resume
+check ovn-nbctl --wait=hv sync
+
+ovn-nbctl ls-del sw0
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovs: Bump submodule to newer version and add related test
Specifically for:
360f5b0e197d ("ovsdb-idl: Preserve references for rows deleted in same IDL as
their insertion.")
A OVN test case reproducing the bug (issue when port_binding is added/deleted
within the
same IDL) is also added.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2126450
Signed-off-by: Xavier Simonart
---
ovs | 2 +-
tests/ovn.at | 29 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/ovs b/ovs
index 6f24c2bc7..360f5b0e1 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 6f24c2bc769afde0a390ce344de1a7d9c592e5a6
+Subproject commit 360f5b0e197d49a4de5811a45e020b000b230d20
diff --git a/tests/ovn.at b/tests/ovn.at
index 07f72dc31..739c09934 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32946,3 +32946,32 @@ check ovn-nbctl --wait=hv sync
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller: batch add port and delete port in same IDL])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl add-port br-int p1
+
+ovs-vsctl set interface p1 external-ids:iface-id=sw0-port1
+check ovn-nbctl --wait=hv sync
+ovn-appctl debug/pause
+OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) =
"xpaused"])
+
+ovn-nbctl ls-add sw0 -- lsp-add sw0 sw0-port1 -- lsp-set-addresses sw0-port1
"50:54:00:00:00:01 192.168.0.2"
+ovn-nbctl lsp-del sw0-port1
+
+ovn-appctl debug/resume
+check ovn-nbctl --wait=hv sync
+
+ovn-nbctl ls-del sw0
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [OVN v7] OVN - Add Support for Remote Port Mirroring
Bleep bloop. Greetings Abhiram R N, 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: Line lacks whitespace around operator #2029 FILE: utilities/ovn-nbctl.c:275: mirror-add NAME TYPE INDEX FILTER IP\n\ WARNING: Line lacks whitespace around operator #2038 FILE: utilities/ovn-nbctl.c:284: mirror-del [NAME] remove mirrors\n\ WARNING: Line lacks whitespace around operator #2039 FILE: utilities/ovn-nbctl.c:285: mirror-list print mirrors\n\ WARNING: Line lacks whitespace around operator #2048 FILE: utilities/ovn-nbctl.c:327: lsp-attach-mirror PORT MIRROR attach source PORT to MIRROR\n\ WARNING: Line lacks whitespace around operator #2049 FILE: utilities/ovn-nbctl.c:328: lsp-detach-mirror PORT MIRROR detach source PORT from MIRROR\n\ Lines checked: 2447, Warnings: 5, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [OVN v7] OVN - Add Support for Remote Port Mirroring
Added changes in ovn-nbctl, ovn-sbctl, northd and in ovn-controller.
While Mirror creation just creates the mirror, the lsp-attach-mirror
triggers the sequence to create Mirror in OVS DB on compute node.
OVS already supports Port Mirroring.
Note: This is targeted to mirror to destinations anywhere outside the
cluster where the analyser resides and it need not be an OVN node.
Example commands are as below:
Mirror creation
ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.2
Attach a logical port to the mirror.
ovn-nbctl lsp-attach-mirror sw0-port1 mirror1
Detach a source from Mirror
ovn-nbctl lsp-detach-mirror sw0-port1 mirror1
Mirror deletion
ovn-nbctl mirror-del mirror1
Co-authored-by: Veda Barrenkala
Signed-off-by: Veda Barrenkala
Signed-off-by: Abhiram R N
---
V6 --> V7: Addressed review comments from V6 by Numan
i) Did suggested changes in mirror.c and ovn-controller.c
ii) Handled the use-case and added to ovn.at test
iii) Indentation correction in ovn-nbctl.c
Files modified (V6 --> v7):
Code --> mirror.c, ovn-controller.c, ovn-nbctl.c
Test --> ovn.at
controller/automake.mk | 4 +-
controller/mirror.c | 537
controller/mirror.h | 53
controller/ovn-controller.c | 266 --
northd/en-northd.c | 4 +
northd/inc-proc-northd.c| 4 +
northd/northd.c | 172
northd/northd.h | 2 +
ovn-nb.ovsschema| 31 ++-
ovn-nb.xml | 63 +
ovn-sb.ovsschema| 26 +-
ovn-sb.xml | 50
tests/ovn-nbctl.at | 116
tests/ovn-northd.at | 102 +++
tests/ovn.at| 231
utilities/ovn-nbctl.c | 357
utilities/ovn-sbctl.c | 4 +
17 files changed, 1994 insertions(+), 28 deletions(-)
create mode 100644 controller/mirror.c
create mode 100644 controller/mirror.h
diff --git a/controller/automake.mk b/controller/automake.mk
index c2ab1bbe6..334672b4d 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -41,7 +41,9 @@ controller_ovn_controller_SOURCES = \
controller/ovsport.h \
controller/ovsport.c \
controller/vif-plug.h \
- controller/vif-plug.c
+ controller/vif-plug.c \
+ controller/mirror.h \
+ controller/mirror.c
controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
man_MANS += controller/ovn-controller.8
diff --git a/controller/mirror.c b/controller/mirror.c
new file mode 100644
index 0..37f8b2e9f
--- /dev/null
+++ b/controller/mirror.c
@@ -0,0 +1,537 @@
+/* Copyright (c) 2022 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include
+
+/* library headers */
+#include "lib/sset.h"
+#include "lib/util.h"
+
+/* OVS includes. */
+#include "lib/vswitch-idl.h"
+#include "openvswitch/vlog.h"
+
+/* OVN includes. */
+#include "binding.h"
+#include "lib/ovn-sb-idl.h"
+#include "mirror.h"
+
+VLOG_DEFINE_THIS_MODULE(port_mirror);
+
+/* Static function declarations */
+
+static const struct ovsrec_port *
+get_port_for_iface(const struct ovsrec_interface *iface,
+ const struct ovsrec_bridge *br_int)
+{
+for (size_t i = 0; i < br_int->n_ports; i++) {
+const struct ovsrec_port *p = br_int->ports[i];
+for (size_t j = 0; j < p->n_interfaces; j++) {
+if (!strcmp(iface->name, p->interfaces[j]->name)) {
+return p;
+}
+}
+}
+return NULL;
+}
+
+static bool
+mirror_create(const struct sbrec_port_binding *pb,
+ struct port_mirror_ctx *pm_ctx)
+{
+const struct ovsrec_mirror *mirror = NULL;
+
+if (pb->n_up && !pb->up[0]) {
+return true;
+}
+
+if (pb->chassis != pm_ctx->chassis_rec) {
+return true;
+}
+
+if (!pm_ctx->ovs_idl_txn) {
+return false;
+}
+
+
+VLOG_INFO("Mirror rule(s) present for %s ", pb->logical_port);
+/* Loop through the mirror rules */
+for (size_t i =0; i < pb->n_mirror_rules; i++) {
+/* check if the mirror already exists in OVS DB */
+bool create_mirror = true;
+OVSREC_MIRROR_TABLE_FOR_EACH (mirror, pm_ctx->mirror_table) {
+if (!strcmp(pb->mirror_rules[i]->name, mirror->name)) {
+/* Mirror with same name already exists
+
Re: [ovs-dev] [RFC PATCH ovn 0/5] Add OVN component templates.
On 9/23/22 18:18, Han Zhou wrote: > On Fri, Sep 23, 2022 at 8:10 AM Dumitru Ceara wrote: >> >> On 9/22/22 19:55, Han Zhou wrote: >>> On Thu, Sep 22, 2022 at 10:38 AM Han Zhou wrote: On Thu, Sep 22, 2022 at 1:00 AM Dumitru Ceara > wrote: > > Hi Han, > > On 9/21/22 23:06, Han Zhou wrote: >> Thanks Dumitru for this promising optimization! >> > > Thanks for checking it out! > >> On Thu, Aug 11, 2022 at 1:03 AM Dumitru Ceara >>> wrote: >>> >>> On 8/10/22 19:54, Mark Michelson wrote: Hi Dumitru, >>> >>> Hi Mark, >>> I read the patch series, and I think the idea of chassis-specific variables is a good idea to reduce the number of DB records for >>> certain things. Aside from load balancers, I suspect this could have a >>> positive impact for other structures as well. >>> >>> Thanks for taking a look! Yes, I think this might be applicable to >>> other structures too. >>> >> >> I think it is a good idea to make it more generic, but for my >>> understanding >> this template concept is for anything that's "node/chassis" specific, >>> and >> supposed to be instantiated at chassis level. Probably we should name >>> the >> DB tables as something like chassis_template_var. >> > > If we decide to go for a simple "chassis" column (instead of a generic > "match" column) then naming the table Chassis_Template_Var makes sense > indeed. > Rather than criticizing the individual lines of code, I'll focus >>> instead on some higher-level questions/ideas. >>> >>> Sure, thanks! :) >>> First, one question I had was what happens when a template variable >>> name is used in a load balancer, but there is no appropriate value to substitute? For instance, what if a load balancer applies to >>> chassis-3, but you only have template variables for chassis-1 and chassis-2? >>> This might be addressed in the code but I didn't notice if it was. >>> >>> There are actually two things to consider here: >>> >>> 1. there might be a logical flow that uses a template variable: in >>> this >>> case if template expansion/instantiation fails we currently leave > the >>> token untouched (e.g., '^variable' stays '^variable'). That will >>> cause >>> the flow action/match parsing to fail and currently logs a warning. >>> The >>> flow itself is skipped, as it should be. We probably need to avoid >>> logging a warning though. >>> >>> 2. like you pointed out, there might be a load balancer using >>> templates >>> in its backends/vips: if some of those templates cannot be >>> instantiated >>> locally the backend/vip where they're added is skipped. Unless I >>> missed >>> something, the code should already do that. >>> Second, it seems like template variables are a natural extension of existing concepts like address sets and port groups. In those > cases, they were an unconditional collection of IP addresses or ports. For >>> >>> You're right to some extent template variables are similar to port >>> groups. The southbound database port group table splits the >>> northbound >>> port group per datapath though not per chassis like template >>> variables. >>> template variables, they're a collection of untyped values with the condition of only applying on certain Chassis. I wonder if this >>> could all be reconciled with a single table that uses untyped values with user-specified conditions. Right now template variables have a >>> "Chassis" column, but maybe this could be replaced with a broader > "condition", "when", or "match" column. To get something integrated quickly, > this column could just accept the syntax of "chassis.name == " or "chassis.uuid == " to allow for chassis-specific application >>> of the values. With this foundation, we could eventually allow unconditional application of the value, or more complex conditions >>> (e.g. only apply to logical switch ports that are connected to a router >>> with a distributed gateway port). Doing this, we could deprecate address >>> sets and port groups eventually in favor of template variables. >>> >>> This sounds like a good idea to me. I wasn't too happy with the >>> "chassis" string column of the Template_Var table anyway. A generic >>> condition field makes more sense. >>> >> If it is chassis-specific template, a column "chassis" seems to be >> straightforward. With a "match" column it is another burden of > parsing > > I have a generic implementation (with a "predicate" column) almost > ready > for review. I agree it's a bit more work to parse
Re: [ovs-dev] [PATCH ovn] northd: do not centralize FIP traffic if redirect-type is set to fixed
On Sep 20, Mark Michelson wrote:
> Hi Lorenzo,
Hi Mark,
thx for reviewing the patch.
>
> I have a couple of comments below. In addition, please add a test for this
> scenario to the testsuite.
>
> On 9/19/22 09:22, Lorenzo Bianconi wrote:
> > Keep FIP traffic distributed and do not centralize it even if the
> > CMS sets redirect-type option to bridged for distributed gateway port.
> >
> > Tested-by: Jianlin Shi
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2007120
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > northd/northd.c | 29 +
> > northd/northd.h | 2 ++
> > northd/ovn-northd.8.xml | 27 +++
> > 3 files changed, 58 insertions(+)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 84440a47f..5c1ddf5c2 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -2616,6 +2616,13 @@ join_logical_ports(struct northd_input *input_data,
> > op->od = od;
> > ovs_list_push_back(>port_list, >dp_node);
> > +const char *redirect_type = smap_get(>options,
> > + "redirect-type");
> > +if (!od->redirect_bridged && redirect_type &&
> > +!strcasecmp(redirect_type, "bridged")) {
> > +od->redirect_bridged = true;
> > +}
>
> Nit-picky optimization: Only call smap_get() if od->redirect_bridge is
> false.
ack, I will fix it in v2.
>
> > +
> > if (op->nbrp->ha_chassis_group ||
> > op->nbrp->n_gateway_chassis) {
> > /* Additional "derived" ovn_port crp represents the
> > @@ -13731,6 +13738,28 @@ build_lrouter_nat_defrag_and_lb(struct
> > ovn_datapath *od, struct hmap *lflows,
> > 100, ds_cstr(match),
> > ds_cstr(actions),
> > >header_);
> > +if (od->redirect_bridged && distributed) {
> > +ds_clear(match);
> > +ds_put_format(
> > +match,
> > +"outport == %s && ip%s.src == %s "
> > +"&& is_chassis_resident(\"%s\")",
> > +od->l3dgw_ports[0]->json_key,
> > +is_v6 ? "6" : "4", nat->logical_ip,
> > +nat->logical_port);
> > +ds_clear(actions);
> > +if (is_v6) {
> > +ds_put_cstr(actions,
> > +"get_arp(outport, " REG_NEXT_HOP_IPV4 ");
> > next;");
> > +} else {
> > +ds_put_cstr(actions,
> > +"get_nd(outport, " REG_NEXT_HOP_IPV6 ");
> > next;");
> > +}
>
> It looks like the logic of the if-else statement above is reversed.
> Shouldn't the is_v6 case use get_nd() instead of get_arp()?
ack, I will fix it in v2.
>
> > +ovn_lflow_add_with_hint(lflows, od,
> > +S_ROUTER_IN_ARP_RESOLVE, 90,
> > +ds_cstr(match),
> > ds_cstr(actions),
> > +>header_);
> > +}
> > sset_add(_entries, nat->external_ip);
> > }
> > }
> > diff --git a/northd/northd.h b/northd/northd.h
> > index aa9a3ae6e..60601803f 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -229,6 +229,8 @@ struct ovn_datapath {
> > size_t n_nat_entries;
> > bool has_distributed_nat;
> > +/* router datapath has a logical port with redirect-type set to
> > bridged. */
> > +bool redirect_bridged;
> > /* Set of nat external ips on the router. */
> > struct sset external_ips;
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index dae961c87..1d5a46a0d 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -4053,6 +4053,33 @@ outport = P
> >
> >
> > +
> > +
> > + If the router datapath runs a port with
> > redirect-type
> > + set to bridged, for each distributed NAT rule with
> > IP
> > + A in the
> > + column
> > + and logical port P in the
> > +
> > column
> > + of table, a priority-90
> > flow
> > + with the match outport === Q ip.src
> > ===
>
> Shouldn't "===" be "=="?
ack, I will fix it in v2.
>
> > + A is_chassis_resident(P),
> > + where Q is the distributed logical router port and
> > + action get_arp(outport, reg0); next; for IPv4 and
> > + get_nd(outport, xxreg0); next; for IPv6.
> > +
> > +
> > +
> > + A priority-0 logical
Re: [ovs-dev] [PATCH] vconn: Allow ECONNREFUSED in refuse connection test
On 27 Sep 2022, at 18:04, Mike Pattrick wrote: > The "tcp vconn - refuse connection" test may fail due to a Connection > Refused error. The network stack returns ECONNREFUSED on a reset > connection in SYN_SENT state and EPIPE or ECONNRESET in all other > cases. > > 2022-09-19T17:45:48Z|1|socket_util|INFO|0:127.0.0.1: listening on > port 34189 > 2022-09-19T17:45:48Z|2|poll_loop|DBG|wakeup due to [POLLOUT][ > POLLERR][POLLHUP] on fd 4 (127.0.0.1:47140<->) at ../lib/stream-fd. > c:153 > test-vconn: unexpected vconn_connect() return value 111 (Connection > refused) > ../../tests/vconn.at:21: exit code was 1, expected 0 > 530. vconn.at:21: 530. tcp vconn - refuse connection (vconn.at:21): > FAILED (vconn.at:21) > > This was observed from a CI system, and isn't a common case. > > Signed-off-by: Mike Pattrick Changes look fine to me. Ran the basic check, no problems were found. Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: allow sample when no in_port
On 27 Sep 2022, at 17:32, Adrian Moreno wrote: > OVN can (and indeed does) set in_port to OFPP_NONE during > the pipeline evaluation. If a sample action follows, it > will be incorrectly skipped. > > Per-flow sampling version of: > f0a9000ca ofproto: Fix ipfix not always sampling on egress. I did some poking around to make sure this will not mess up the cookie and pid values, but it all look good. Did basic testing, and passes here. Acked-by: Eelco Chaudron Cheers, Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.03] github: ovn-kubernetes: Update go, kube and libovsdb versions.
On 9/26/22 12:59, Dumitru Ceara wrote:
> With this they'll match the current upstream ovn-kubernetes code.
>
> (cherry picked from commit 4a5e20ee58cd012eb52a94ee1c97fe225e4e91f2)
> Signed-off-by: Dumitru Ceara
> ---
> Backporting this patch to the LTS too. Otherwise we can't run ovnkube
> tests there.
> ---
To make the ovn-kubernetes CI jobs green again I backported:
- to branch-22.06:
bcc7338a24d4 ("ci: ovn-kubernetes: Align CI jobs with recent ovn-kubernetes
upstream.")
- to branch-22.03:
bcc7338a24d4 ("ci: ovn-kubernetes: Align CI jobs with recent ovn-kubernetes
upstream.")
4a5e20ee58cd ("github: ovn-kubernetes: Update go, kube and libovsdb
versions.")
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] controller: Fix first ping from lsp to external through snat failing
On 9/27/22 17:16, Xavier Simonart wrote:
> Fixes: b89b96e1a16 ("fix potential segmentation violation when removing
> ports")
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2130045
>
> Signed-off-by: Xavier Simonart
> ---
Thanks for the quick fix Xavier! I applied this to the main branch and
backported it all the way down to branch-22.03.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
