Re: [ovs-dev] [PATCH ovn v5 09/14] northd: Use objdep mgr for lport to lflow references.
On Tue, 22 Aug, 2023, 6:11 am Han Zhou, wrote: > On Fri, Aug 18, 2023 at 4:35 AM Numan Siddique wrote: > > > > On Fri, Aug 18, 2023 at 2:33 PM Numan Siddique wrote: > > > > > > On Fri, Aug 18, 2023 at 11:38 AM Han Zhou wrote: > > > > > > > > On Wed, Aug 9, 2023 at 9:38 AM wrote: > > > > > > > > > > From: Numan Siddique > > > > > > > > > Thanks Numan for the enhancement. > > > > > > > > > Instead of maintaing the lport to lflow references using > > > > > 'struct lflow_ref_list', this patch makes use of the existing > > > > > objdep APIs. Since objdep_mgr is not thread safe, an instance > > > > > of objdep_mgr is maintained for each 'ovn_port'. > > > > > > > > > > Once we add the thread safe support to objdep APIs we can move > > > > > the objdep_mgr to 'lflow' engine date. > > > > > > > > > > Using objdep APIs does come with a cost in memory. We now > > > > > maintain an additional hmap of ovn_lflow's to look up using > > > > > uuid. But this will be useful to handle datapath and load > > > > > balancer changes in the lflow engine node. > > > > > > > > > > > > > I understand that the 'struct lflow_ref_list' may not satisfy the > needs of > > > > more complex I-P, but I wonder if objdep_mgr (without modification) > is the > > > > solution. It seems to me more complex at least for this patch without > > > > adding obvious benefits. Since I am still reviewing, maybe I missed > some > > > > points which would show the value from the rest of the patches, and > please > > > > forgive me for my slow review. > > > > > > Sure. No worries. > > > > > > Can you please continue reviewing from the latest v6 ? > > > > > > http://patchwork.ozlabs.org/project/ovn/list/?series=369384 > > > > > > I think we can discuss more once you go through all the patches in the > > > series (or until patch 14). > > > I think that would give a better idea of how it is used and we can > > > discuss if it's worth using obj dep mgr. > > > > > > Also I think the patches P1 - P8 make one set and can be considered > > > first. I submitted all these patches > > > as one series mainly because > > >- all the patches can be applied easily > > >- the soft freeze was near. > > > > > > The patches can be found here too - > > > https://github.com/numansiddique/ovn/tree/northd_ip_lb_ip_v6 > > > > > > Thanks > > > Numan > > > > > > > > > > > While I am still reviewing the patches, I did a performance test with > > > > https://github.com/hzhou8/ovn-test-script for the 200 chassis x 50 > lsp > > > > scale of simulated ovn-k8s topology. > > > > > > > > - Before this patch, a recompute takes 1293ms > > > > - After this patch, a recompute takes 1536ms, which is 20% more time. > > > > - I also did the same test with all the rest of the patches of this > series, > > > > which takes 1690ms. > > > > > > > > I did some testing using the OVN dbs of the ovn-heater density heavy > run. > > A triggered forced recompute with the present main of ovn-northd takes > > around 4389ms > > where as with the patch 9 of this series, it is taking around 4838ms. > > > > I hacked the 'engine_compute_log_timeout_msec' [1] value from 500ms to > > 50ms so that > > inc-eng can log the engine nodes taking more than 50ms in the recompute. > > > > With main > > --- > > 2023-08-18T11:28:20.714Z|00117|inc_proc_eng|INFO|User triggered force > recompute. > > 2023-08-18T11:28:23.189Z|00118|inc_proc_eng|INFO|node: northd, > > recompute (forced) took 2475ms > > 2023-08-18T11:28:25.102Z|00119|inc_proc_eng|INFO|node: lflow, > > recompute (forced) took 1887ms > > 2023-08-18T11:28:25.102Z|00120|timeval|WARN|Unreasonably long 4389ms > > poll interval (4373ms user, 3ms system) > > > > > > And with patch 9 > > - > > 2023-08-18T11:28:10.558Z|00220|inc_proc_eng|INFO|User triggered force > recompute. > > 2023-08-18T11:28:10.883Z|00221|inc_proc_eng|INFO|node: lb_data, > > recompute (forced) took 325ms > > 2023-08-18T11:28:13.163Z|00222|inc_proc_eng|INFO|node: northd, > > recompute (forced) took 2280ms > > 2023-08-18T11:28:13.431Z|00223|inc_proc_eng|INFO|node: sync_to_sb_lb, > > recompute (forced) took 241ms > > 2023-08-18T11:28:15.396Z|00224|inc_proc_eng|INFO|node: lflow, > > recompute (forced) took 1956ms > > 2023-08-18T11:28:15.397Z|00225|timeval|WARN|Unreasonably long 4838ms > > poll interval (4819ms user, 3ms system) > > > > > > As you can see the lflow recompute time is not very significant > > (1887 ms vs 1956 ms) > > > > It is the lb_data engine node which is taking 325ms more. But if you > > see the lb_data engine code, the lb_data handlers never return false. > > So the lb_data full engine recompute cost can be ignored as it will be > > triggered when engine aborts or when the user triggers a recompute. > > > > I don't think the lflow engine recompute cost due to objdep is > > significant if lflow recomputes are less. > > > > My point is that the performance degradation ~20% is caused by just patch > 9, because I did the same test on patch 8 v.s.
[ovs-dev] [PATCH ovn] Use correct nw_ttl=255 to match against legit NAs
RFC 4861 (Neighbor Discovery for IP version 6) requires that Hop Limit
is set to 255, but the flows we generated for 135 and 136 erroneously
used 225.
Fixes: 8cab00bdb581 ("ovn-controller: Add OF rules for port security.")
Signed-off-by: Ihar Hrachyshka
---
controller/lflow.c | 6 +++---
tests/ovn.at | 34 +-
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index bc5f73279..f70080e8e 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -2535,10 +2535,10 @@ build_in_port_sec_default_flows(const struct
sbrec_port_binding *pb,
* investigation.
*
* Eg. If there are below OF rules in the same table
- * (1) priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=135,
+ * (1) priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=135,
* icmp_code=0,nd_sll=fa:16:3e:94:05:98
* actions=load:0->NXM_NX_REG10[12]
- * (2) priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=135,
+ * (2) priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=135,
* icmp_code=0 actions=load:1->NXM_NX_REG10[12]
*
* An IPv6 NS packet with nd_sll = fa:16:3e:94:05:98 is matching on the
@@ -2823,7 +2823,7 @@ build_in_port_sec_nd_flows(const struct
sbrec_port_binding *pb,
reset_match_for_port_sec_flows(pb, MFF_LOG_INPORT, m);
match_set_dl_type(m, htons(ETH_TYPE_IPV6));
match_set_nw_proto(m, IPPROTO_ICMPV6);
-match_set_nw_ttl(m, 225);
+match_set_nw_ttl(m, 255);
match_set_icmp_type(m, 135);
match_set_icmp_code(m, 0);
diff --git a/tests/ovn.at b/tests/ovn.at
index bb5cbf0b9..7c61a2d5b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -34139,10 +34139,10 @@ echo " table=74,
priority=80,arp,reg14=0x$sw0p1_key,metadata=0x1 actions=load:0x
table=74,
priority=80,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=135
actions=load:0->NXM_NX_REG10[[12]]
table=74,
priority=80,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=136
actions=load:0x1->NXM_NX_REG10[[12]]
table=74,
priority=90,arp,reg14=0x$sw0p1_key,metadata=0x1,dl_src=00:00:00:00:00:03,arp_sha=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]" > hv1_t74_flows.expected
+ table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
+ table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]
+ table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
+ table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]" > hv1_t74_flows.expected
check_port_sec_offlows hv1 74
@@ -34176,12 +34176,12 @@ echo " table=74,
priority=80,arp,reg14=0x$sw0p1_key,metadata=0x1 actions=load:0x
table=74,
priority=80,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=136
actions=load:0x1->NXM_NX_REG10[[12]]
table=74,
priority=90,arp,reg14=0x$sw0p1_key,metadata=0x1,dl_src=00:00:00:00:00:03,arp_spa=10.0.0.3,arp_sha=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]
table=74,
priority=90,arp,reg14=0x$sw0p1_key,metadata=0x1,dl_src=00:00:00:00:00:13,arp_spa=10.0.0.13,arp_sha=00:00:00:00:00:13
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:03
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:13
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00
actions=load:0->NXM_NX_REG10[[12]]
- table=74,
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:03
Re: [ovs-dev] [PATCH v2] conntrack: Remove nat_conn introducing key directionality.
On 8/23/23 14:53, Paolo Valerio wrote:
> From: hepeng
>
> The patch avoids the extra allocation for nat_conn.
> Currently, when doing NAT, the userspace conntrack will use an extra
> conn for the two directions in a flow. However, each conn has actually
> the two keys for both orig and rev directions. This patch introduces a
> key_node[CT_DIRS] member as per Aaron's suggestion in the conn which
> consists of a key, direction, and a cmap_node for hash lookup so
> addressing the feedback received by the original patch [0].
>
> [0]
> https://patchwork.ozlabs.org/project/openvswitch/patch/20201129033255.64647-2-hepeng.0...@bytedance.com/
>
> Signed-off-by: Peng He
> Co-authored-by: Paolo Valerio
> Signed-off-by: Paolo Valerio
> ---
> v2:
> - use enum value instead of bool (Aaron).
> - s/conn_for_expectation/conn_for_exp/ in process_ftp_ctl_v6()
> to avoid long line.
> - removed CT_CONN_TYPE_* reference in two comments.
> ---
> lib/conntrack-private.h | 19 +--
> lib/conntrack-tp.c |6 +
> lib/conntrack.c | 350
> +++
> 3 files changed, 155 insertions(+), 220 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index bb326868e..3fd5fccd3 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -49,6 +49,12 @@ struct ct_endpoint {
> * hashing in ct_endpoint_hash_add(). */
> BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(union ct_addr) + 4);
>
> +enum key_dir {
> +CT_DIR_FWD = 0,
> +CT_DIR_REV,
> +CT_DIRS,
> +};
> +
> /* Changes to this structure need to be reflected in conn_key_hash()
> * and conn_key_cmp(). */
> struct conn_key {
> @@ -112,20 +118,18 @@ enum ct_timeout {
>
> #define N_EXP_LISTS 100
>
> -enum OVS_PACKED_ENUM ct_conn_type {
> -CT_CONN_TYPE_DEFAULT,
> -CT_CONN_TYPE_UN_NAT,
> +struct conn_key_node {
> +enum key_dir dir;
> +struct conn_key key;
> +struct cmap_node cm_node;
> };
This structure and the whole business of adding the connection
to cmap twice with different hashes is bothering me, but I really
don't have a better solution for this, so let it be... :)
Just to refresh my memory, we do that because original and reply
tuples can be completely different due to NAT, so the hashing
being symmetric doesn't help in this case, right?
>
> struct conn {
> /* Immutable data. */
> -struct conn_key key;
> -struct conn_key rev_key;
> +struct conn_key_node key_node[CT_DIRS];
> struct conn_key parent_key; /* Only used for orig_tuple support. */
> -struct cmap_node cm_node;
> uint16_t nat_action;
> char *alg;
> -struct conn *nat_conn; /* The NAT 'conn' context, if there is one. */
> atomic_flag reclaimed; /* False during the lifetime of the connection,
> * True as soon as a thread has started freeing
> * its memory. */
> @@ -150,7 +154,6 @@ struct conn {
>
> /* Immutable data. */
> bool alg_related; /* True if alg data connection. */
> -enum ct_conn_type conn_type;
>
> uint32_t tp_id; /* Timeout policy ID. */
> };
> diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
> index 89cb2704a..2149fdc73 100644
> --- a/lib/conntrack-tp.c
> +++ b/lib/conntrack-tp.c
> @@ -253,7 +253,8 @@ conn_update_expiration(struct conntrack *ct, struct conn
> *conn,
> }
> VLOG_DBG_RL(, "Update timeout %s zone=%u with policy id=%d "
> "val=%u sec.",
> -ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
> +ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
> +conn->tp_id, val);
>
> atomic_store_relaxed(>expiration, now + val * 1000);
> }
> @@ -273,7 +274,8 @@ conn_init_expiration(struct conntrack *ct, struct conn
> *conn,
> }
>
> VLOG_DBG_RL(, "Init timeout %s zone=%u with policy id=%d val=%u sec.",
> -ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
> +ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
> +conn->tp_id, val);
>
> conn->expiration = now + val * 1000;
> }
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 5f1176d33..f75f9a8f1 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -113,8 +113,7 @@ static void set_label(struct dp_packet *, struct conn *,
> static void *clean_thread_main(void *f_);
>
> static bool
> -nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
> - struct conn *nat_conn,
> +nat_get_unique_tuple(struct conntrack *ct, struct conn *conn,
> const struct nat_action_info_t *nat_info);
>
> static uint8_t
> @@ -208,7 +207,7 @@ static alg_helper alg_helpers[] = {
> #define ALG_WC_SRC_PORT 0
>
> /* If the total number of connections goes above this value, no new
> connections
> - * are accepted; this is for CT_CONN_TYPE_DEFAULT connections.
Re: [ovs-dev] [PATCH v2] ofproto-dpif-mirror: Add support for pre-selection filter
On Fri, Aug 25, 2023 at 10:28 AM Adrian Moreno wrote:
>
>
>
> On 7/18/23 21:38, Mike Pattrick wrote:
> > Currently a bridge mirror will collect all packets and tools like
> > ovs-tcpdump can apply additional filters after they have already been
> > duplicated by vswitchd. This can result in inefficient collection.
> >
> > This patch adds support to apply pre-selection to bridge mirrors, which
> > can limit which packets are mirrored based on flow metadata. This
> > significantly improves overall vswitchd performance during mirroring if
> > only a subset of traffic is required.
> >
> > I benchmarked this with two setups. A netlink based test with two veth
> > pairs connected to a single bridge, and a netdev based test involving a
> > mix of DPDK nics, and netdev-linux interfaces. Both tests involved
> > saturating the link with iperf3 and then sending an icmp ping every
> > second. I then measured the throughput on the link with no mirroring,
> > icmp pre-selected mirroring, and full mirroring. The results, below,
> > indicate a significant reduction to impact of mirroring when only a
> > subset of the traffic on a port is selected for collection.
> >
> > Test No Mirror | No Filter | Filter | No Filter | Filter |
> > +---+---+---+---+--+
> > netlink | 39.0 Gbps | 36.1 Gbps | 38.2 Gbps | 7%|2%|
> > netdev | 7.39 Gbps | 4.95 Gbps | 6.24 Gbps |33%| 15%|
> >
> > The ratios above are the percent reduction in total throughput when
> > mirroring is used either with or without a filter.
> >
>
> Looks really promising!
>
> > Signed-off-by: Mike Pattrick
> > ---
> > Documentation/ref/ovs-tcpdump.8.rst | 4 ++
> > NEWS| 2 +
> > lib/flow.h | 11 ++
> > ofproto/ofproto-dpif-mirror.c | 60 +++--
> > ofproto/ofproto-dpif-mirror.h | 9 -
> > ofproto/ofproto-dpif-xlate.c| 6 ++-
> > ofproto/ofproto-dpif.c | 2 +-
> > ofproto/ofproto.h | 3 ++
> > tests/ofproto-dpif.at | 43 +
> > utilities/ovs-tcpdump.in| 13 ++-
> > vswitchd/bridge.c | 8
> > vswitchd/vswitch.ovsschema | 4 +-
> > vswitchd/vswitch.xml| 6 +++
> > 13 files changed, 160 insertions(+), 11 deletions(-)
> >
> > diff --git a/Documentation/ref/ovs-tcpdump.8.rst
> > b/Documentation/ref/ovs-tcpdump.8.rst
> > index b9f8cdf6f..9163c8a5e 100644
> > --- a/Documentation/ref/ovs-tcpdump.8.rst
> > +++ b/Documentation/ref/ovs-tcpdump.8.rst
> > @@ -61,6 +61,10 @@ Options
> >
> > If specified, mirror all ports (optional).
> >
> > +* ``--filter ``
> > +
> > + If specified, only mirror flows that match the provided filter.
> > +
> > See Also
> >
> >
> > diff --git a/NEWS b/NEWS
> > index 7a852427e..26797ca56 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> > Post-v3.2.0
> >
> > + - ovs-tcpdump:
> > + * Added new --filter parameter to apply pre-selection on mirrored
> > flows.
> >
>
> Maybe also comment on the new column in the Mirror table of the OVSDB?
>
> >
> > v3.2.0 - xx xxx
> > diff --git a/lib/flow.h b/lib/flow.h
> > index a9d026e1c..c2e67dfc5 100644
> > --- a/lib/flow.h
> > +++ b/lib/flow.h
> > @@ -939,6 +939,17 @@ flow_union_with_miniflow(struct flow *dst, const
> > struct miniflow *src)
> > flow_union_with_miniflow_subset(dst, src, src->map);
> > }
> >
> > +/* Perform a bitwise OR of minimask 'src' mask data with the equivalent
> > + * fields in 'dst', storing the result in 'dst'. */
> > +static inline void
> > +flow_wildcards_union_with_minimask(struct flow_wildcards *dst,
> > + const struct minimask *src)
> > +{
> > +flow_union_with_miniflow_subset(>masks,
> > +>masks,
> > +src->masks.map);
> > +}
> > +
> > static inline bool is_ct_valid(const struct flow *flow,
> > const struct flow_wildcards *mask,
> > struct flow_wildcards *wc)
> > diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
> > index 343b75f0e..e4174a564 100644
> > --- a/ofproto/ofproto-dpif-mirror.c
> > +++ b/ofproto/ofproto-dpif-mirror.c
> > @@ -21,6 +21,7 @@
> > #include "cmap.h"
> > #include "hmapx.h"
> > #include "ofproto.h"
> > +#include "ofproto-dpif-trace.h"
> > #include "vlan-bitmap.h"
> > #include "openvswitch/vlog.h"
> >
> > @@ -57,6 +58,10 @@ struct mirror {
> > struct hmapx srcs; /* Contains "struct mbundle*"s. */
> > struct hmapx dsts; /* Contains "struct mbundle*"s. */
> >
> > +/* Filter criteria. */
> > +struct miniflow *filter;
> > +struct minimask *mask;
> > +
> > /* This is accessed by handler threads
Re: [ovs-dev] [PATCH ovn branch-22.03 0/2] Release patches for v22.03.3.
On 8/29/23 17:30, Mark Michelson wrote: > > Mark Michelson (2): > Set release date for 22.03.3. > Prepare for 22.03.4. > > NEWS | 7 ++- > configure.ac | 2 +- > debian/changelog | 8 +++- > 3 files changed, 14 insertions(+), 3 deletions(-) > I didn't test the builds, but these patches seem to be correct: Acked-by: Ilya Maximets ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.09 1/2] Set release date for 22.09.2.
On 8/29/23 17:30, Mark Michelson wrote: > Signed-off-by: Mark Michelson > --- > NEWS | 4 +++- > debian/changelog | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index d5170786b..1758bf633 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -OVN v22.09.2 - xx xxx > +OVN v22.09.2 - 29 Aug 2023 > -- > + - Bug fixes This one is a bit over-indented. The rest seems fine. For the set: Acked-by: Ilya Maximets >- Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast > Listener Discovery protocols, regardless of ACLs defined. >- Send ICMP Fragmentation Needed packets back to offending ports when > @@ -8,6 +9,7 @@ OVN v22.09.2 - xx xxx > physical network via a localnet port, in which case multichassis ports > may > have an effective MTU different from regular ports and hence may need > this > mechanism to maintain connectivity with other peers in the network. > + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. > > OVN v22.09.1 - 20 Dec 2022 > -- > diff --git a/debian/changelog b/debian/changelog > index 08cc66fc0..e7ec2b762 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ OVN (22.09.2-1) unstable; urgency=low > [ OVN team ] > * New upstream version > > - -- OVN team Tue, 20 Dec 2022 13:53:56 -0500 > + -- OVN team Tue, 29 Aug 2023 11:30:11 -0400 > > OVN (22.09.1-1) unstable; urgency=low > [ OVN team ] ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.12 1/2] Set release date for 22.12.1.
On 8/29/23 17:30, Mark Michelson wrote: > Signed-off-by: Mark Michelson > --- > NEWS | 4 +++- > debian/changelog | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 01baa2be6..4efa1d1fe 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -OVN v22.12.1 - xx xxx > +OVN v22.12.1 - 29 Aug 2023 > -- > + - Bug fixes This one is a bit over-indented. The rest seems fine. For the set: Acked-by: Ilya Maximets >- Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast > Listener Discovery protocols, regardless of ACLs defined. >- Send ICMP Fragmentation Needed packets back to offending ports when > @@ -8,6 +9,7 @@ OVN v22.12.1 - xx xxx > physical network via a localnet port, in which case multichassis ports > may > have an effective MTU different from regular ports and hence may need > this > mechanism to maintain connectivity with other peers in the network. > + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. > > OVN v22.12.0 - 16 Dec 2022 > -- > diff --git a/debian/changelog b/debian/changelog > index d658774f6..b58fbaef7 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ OVN (22.12.1-1) unstable; urgency=low > [ OVN team ] > * New upstream version > > - -- OVN team Fri, 16 Dec 2022 09:52:44 -0500 > + -- OVN team Tue, 29 Aug 2023 11:30:03 -0400 > > ovn (22.12.0-1) unstable; urgency=low > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-23.03 1/2] Set release date for 23.03.1.
On 8/29/23 17:30, Mark Michelson wrote: > Signed-off-by: Mark Michelson > --- > NEWS | 4 +++- > debian/changelog | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 47d259150..1c75eeeae 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -OVN v23.03.1 - xx xxx > +OVN v23.03.1 - 29 Aug 2023 > -- > + - Bug fixes This one is a bit over-indented. The rest seems fine. For the set: Acked-by: Ilya Maximets >- CT entries are not flushed by default anymore whenever a load balancer > backend is removed. A new, per-LB, option 'ct_flush' can be used to > restore the previous behavior. Disabled by default. > @@ -15,6 +16,7 @@ OVN v23.03.1 - xx xxx > Existing sessions might get re-hashed to a different ECMP path when > OVN detects the algorithm support in the datapath during an upgrade > or restart of ovn-controller. > + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. > > OVN v23.03.0 - 03 Mar 2023 > -- > diff --git a/debian/changelog b/debian/changelog > index 02a9953ba..4c744105e 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ OVN (23.03.1-1) unstable; urgency=low > [ OVN team ] > * New upstream version > > - -- OVN team Fri, 03 Mar 2023 10:40:37 -0500 > + -- OVN team Tue, 29 Aug 2023 11:29:51 -0400 > > ovn (23.03.0-1) unstable; urgency=low > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-23.06 0/2] Release patches for v23.06.1.
On 8/29/23 17:30, Mark Michelson wrote: > > Mark Michelson (2): > Set release date for 23.06.1. > Prepare for 23.06.2. > > NEWS | 7 ++- > configure.ac | 2 +- > debian/changelog | 8 +++- > 3 files changed, 14 insertions(+), 3 deletions(-) > I didn't test the builds, but these patches seems correct: Acked-by: Ilya Maximets ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 7/7] system-dpdk: Run traffic tests.
Integrate system-traffic.at tests as part of check-dpdk. Some tests that can't work with the userspace datapath are skipped by overriding some OVS_CHECK_* macros. ADD_VETH is implemented with a tap interface in the kernel, directly polled by OVS with the net/tap DPDK driver. ADD_VETH_IGNORE_LOGS is provided to filter warning logs that may be emitted (like for example, lack of DPDK multiprocess feature which OVS does not care about). This driver expects an equal number of rxq and txq. On the other hand, OVS spawns one PMD thread per numa node, and OVS reserves one txq for non PMD thread usage. This means that a net/tap port n_rxq must be set to numa_count + 1. As we don't care about performance in those unit tests, simply fake running them on a mono core, mono numa using --dummy-numa. Using --dummy-numa, an additional error log must be waived since ovs_numa_thread_getaffinity_dump() is unavailable. Signed-off-by: David Marchand --- Changes since v2: - added ADD_VETH_IGNORE_LOGS and moved ignored error logs to OVS_TRAFFIC_VSWITCHD_STOP, - added --no-pci to DPDK options to avoid failing the tests when running in a vm with a virtio-net device, - faked a mono numa/mono core so that OVS requests at max 2 txqs on the net/tap port, --- tests/system-dpdk-macros.at| 93 ++ tests/system-dpdk-testsuite.at | 2 + tests/system-dpdk.at | 7 +-- 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at index f29b3c283f..9f37286817 100644 --- a/tests/system-dpdk-macros.at +++ b/tests/system-dpdk-macros.at @@ -124,3 +124,96 @@ m4_define([OVS_DPDK_STOP_TESTPMD], [AT_CHECK([kill `cat testpmd.pid`]) OVS_WAIT([kill -0 `cat testpmd.pid`], [kill -9 `cat testpmd.pid`]) ]) + + +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]) +# +# Creates a database and starts ovsdb-server, starts ovs-vswitchd +# connected to that database, calls ovs-vsctl to create a bridge named +# br0 with predictable settings, passing 'vsctl-args' as additional +# commands to ovs-vsctl. If 'vsctl-args' causes ovs-vsctl to provide +# output (e.g. because it includes "create" commands) then 'vsctl-output' +# specifies the expected output after filtering through uuidfilt. +# 'dbinit-aux-args' are passed as additional commands to 'ovs-vsctl init' +# before starting ovs-vswitchd. +m4_define([OVS_TRAFFIC_VSWITCHD_START], + [ + OVS_DPDK_PRE_CHECK() + OVS_WAIT_WHILE([ip link show ovs-netdev]) + # For functional tests: + # - no need for DPDK PCI probing, + # - fake one core only, on one NUMA so that net/tap port works, + OVS_DPDK_START([--no-pci], [--disable-system --dummy-numa 0], [$3]) + dnl Add bridges, ports, etc. + OVS_WAIT_WHILE([ip link show br0]) + AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) +]) + + +# OVS_TRAFFIC_VSWITCHD_STOP([ALLOWLIST], [extra_cmds]) +# +# Gracefully stops ovs-vswitchd and ovsdb-server, checking their log files +# for messages with severity WARN or higher and signaling an error if any +# is present. The optional ALLOWLIST may contain shell-quoted "sed" +# commands to delete any warnings that are actually expected, e.g.: +# +# OVS_TRAFFIC_VSWITCHD_STOP(["/expected error/d"]) +# +# 'extra_cmds' are shell commands to be executed after OVS_VSWITCHD_STOP() is +# invoked. They can be used to perform additional cleanups such as name space +# removal. +m4_define([OVS_TRAFFIC_VSWITCHD_STOP], + [OVS_DPDK_STOP_VSWITCHD([$1";"ADD_VETH_IGNORE_LOGS";dnl +/Thread getaffinity failed. Using core #0/d +/does not exist. The Open vSwitch kernel module is probably not loaded./d"]) + AT_CHECK([:; $2]) +]) + + +m4_define([OVS_CHECK_8021AD], +[AT_SKIP_IF([:])]) + + +m4_define([OVS_CHECK_TC_QDISC], +[AT_SKIP_IF([:])]) + + +m4_define([OVS_CHECK_TCPDUMP], +[AT_SKIP_IF([:])]) + + +# Fake a veth by creating a tap on kernel side and plug it in OVS using the +# net/tap DPDK driver. +m4_define([ADD_VETH], +[ AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \ +set interface ovs-$1 external-ids:iface-id="$1" -- \ +set interface ovs-$1 type=dpdk -- \ +set interface ovs-$1 options:n_rxq=2 -- \ +set interface ovs-$1 options:dpdk-devargs=net_tap$1,iface=$1]) + OVS_WAIT_UNTIL([ip link show dev $1 | grep -qw LOWER_UP]) + AT_CHECK([ip link set $1 netns $2]) + NS_CHECK_EXEC([$2], [ip addr add $4 dev $1 $7]) + NS_CHECK_EXEC([$2], [ip link set dev $1 up]) + if test -n "$5"; then +NS_CHECK_EXEC([$2], [ip link set dev $1 address $5]) + fi + if test -n "$6"; then +NS_CHECK_EXEC([$2], [ip route add default via $6]) + fi +] +) + + +# Using a net/tap port with ADD_VETH can trigger some warning logs that can be +# filtered with the following macro: +m4_define([ADD_VETH_IGNORE_LOGS], +["dnl +/tap_nl_dump_ext_ack():
[ovs-dev] [PATCH v3 6/7] system-dpdk: Refactor OVS daemons helpers.
Align system-dpdk existing helpers to other common OVS helpers so they
can accept some optional arguments.
Introduce a OVS_DPDK_STOP_VSWITCHD wrapper around OVS_VSWITCHD_STOP to
catch dpdk related logs in a centralised fashion.
Signed-off-by: David Marchand
Acked-by: Eelco Chaudron
---
tests/system-dpdk-macros.at | 21 -
tests/system-dpdk.at| 159 +++-
2 files changed, 83 insertions(+), 97 deletions(-)
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 9a6685fafd..f29b3c283f 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -36,12 +36,13 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
#
m4_define([OVS_DPDK_START],
[dnl start ovs dpdk
- OVS_DPDK_START_OVSDB()
+ OVS_DPDK_START_OVSDB($3)
dnl Enable DPDK functionality
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
other_config:dpdk-init=true])
- OVS_DPDK_START_VSWITCHD($1)
+ OVS_DPDK_START_VSWITCHD([$1], [$2])
])
+
# OVS_DPDK_START_OVSDB()
#
# Create an empty database and start ovsdb-server.
@@ -60,9 +61,10 @@ m4_define([OVS_DPDK_START_OVSDB],
AT_CAPTURE_FILE([ovsdb-server.log])
dnl Initialize database.
- AT_CHECK([ovs-vsctl --no-wait init])
+ AT_CHECK([ovs-vsctl --no-wait init $1])
])
+
# OVS_DPDK_START_VSWITCHD()
#
# Add special configuration for dpdk-init. Start ovs-vswitchd.
@@ -72,12 +74,23 @@ m4_define([OVS_DPDK_START_VSWITCHD],
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
other_config:dpdk-extra="--log-level=pmd.*:error $1"])
dnl Start ovs-vswitchd.
- AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn
-vofproto_dpif -vunixctl], [0], [stdout], [stderr])
+ AT_CHECK([ovs-vswitchd $2 --detach --no-chdir --pidfile --log-file -vvconn
-vofproto_dpif -vunixctl], [0], [stdout], [stderr])
AT_CAPTURE_FILE([ovs-vswitchd.log])
on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
])
+m4_define([OVS_DPDK_STOP_VSWITCHD],
+ [OVS_VSWITCHD_STOP([dnl
+$1";/does not exist. The Open vSwitch kernel module is probably not loaded./d
+/does not support MTU configuration,/d
+/EAL: No \(available\|free\) .*hugepages reported/d
+/Failed to enable flow control/d
+/Rx checksum offload is not supported on/d
+/TELEMETRY: No legacy callbacks, legacy socket not created/d"])
+])
+
+
# OVS_DPDK_CHECK_TESTPMD()
#
# Check dpdk-testpmd availability.
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 270587e2c0..e8a04d1d86 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -3,15 +3,6 @@ m4_define([CONFIGURE_VETH_OFFLOADS],
AT_BANNER([OVS-DPDK unit tests])
-m4_define([SYSTEM_DPDK_ALLOWED_LOGS],[
-\@does not exist. The Open vSwitch kernel module is probably not loaded.@d
-\@does not support MTU configuration,@d
-\@EAL: No \(available\|free\) .*hugepages reported@d
-\@Failed to enable flow control@d
-\@Rx checksum offload is not supported on@d
-\@TELEMETRY: No legacy callbacks, legacy socket not created@d
-])
-
dnl CHECK_MEMPOOL_PARAM([mtu], [numa], [+line])
dnl
dnl Waits for logs to indicate that the user has configured a mempool
@@ -36,7 +27,7 @@ OVS_DPDK_START([--no-pci])
AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [],
[stdout])
AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
-OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
+OVS_DPDK_STOP_VSWITCHD
AT_CLEANUP
dnl --
@@ -58,7 +49,7 @@ sleep 2
dnl Clean up
AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
-OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
+OVS_DPDK_STOP_VSWITCHD
AT_CLEANUP
dnl --
@@ -84,9 +75,8 @@ AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0)
reconnecting..." ov
dnl Clean up
AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout],
[stderr])
-OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
-\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file
or directory@d
-])")
+OVS_DPDK_STOP_VSWITCHD(["dnl
+/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or
directory/d"])
AT_CLEANUP
dnl --
@@ -150,12 +140,11 @@ OVS_WAIT_UNTIL([grep "vHost Device
'$OVS_RUNDIR/dpdkvhostuser0' has been removed
dnl Clean up
AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuser0], [], [stdout], [stderr])
-OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
-\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostuser0) recvmsg failed@d
-\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostuser0) failed to connect: No such file
or directory@d
-\@dpdkvhostuser ports are considered deprecated; please migrate to
dpdkvhostuserclient ports.@d
-\@failed to enumerate system datapaths: No such file or directory@d
-])")
[ovs-dev] [PATCH v3 5/7] tests: Define a macro to skip tc/tcpdump relying tests.
Some unit tests expect that a OVS port has an associated netdevice on which they can hook tc or tcpdump. This will not be possible when testing the userspace datapath with DPDK. Introduce two helpers (which will be overriden in system-dpdk tests) and use them in the existing tests. Signed-off-by: David Marchand Acked-by: Eelco Chaudron Acked-by: Aaron Conole --- tests/system-common-macros.at| 12 tests/system-offloads-traffic.at | 6 +++--- tests/system-traffic.at | 32 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0077a8609c..37bc510eaa 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -297,6 +297,18 @@ m4_define([OVS_START_L7], # m4_define([OFPROTO_CLEAR_DURATION_IDLE], [[sed -e 's/duration=.*s,/duration=,/g' -e 's/idle_age=[0-9]*,/idle_age=,/g']]) +# OVS_CHECK_TC_QDISC() +# +# Macro to skip tests when tc qdisc can't be applied on a OVS port. +m4_define([OVS_CHECK_TC_QDISC], +[AT_SKIP_IF([test $HAVE_TC = no])]) + +# OVS_CHECK_TCPDUMP() +# +# Macro to skip tests when tcpdump can't be applied on a OVS port. +m4_define([OVS_CHECK_TCPDUMP], +[AT_SKIP_IF([test $HAVE_TCPDUMP = no])]) + # OVS_CHECK_TUNNEL_TSO() # # Macro to be used in general tunneling tests that could be also diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 7215e36e2d..270314bfd6 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -20,7 +20,7 @@ m4_define([OVS_CHECK_ACTIONS], [ m4_define([CHECK_TC_INGRESS_PPS], [ -AT_SKIP_IF([test $HAVE_TC = "no"]) +OVS_CHECK_TC_QDISC() AT_CHECK([ip link add ovs_tc_pps0 type veth peer name ovs_tc_pps1 dnl || exit 77]) on_exit 'ip link del ovs_tc_pps0' @@ -95,7 +95,7 @@ AT_CLEANUP AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - offloads disabled]) AT_KEYWORDS([ingress_policing]) -AT_SKIP_IF([test $HAVE_TC = "no"]) +OVS_CHECK_TC_QDISC() OVS_TRAFFIC_VSWITCHD_START() AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=false]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) @@ -118,7 +118,7 @@ AT_CLEANUP AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - offloads enabled]) AT_KEYWORDS([ingress_policing]) -AT_SKIP_IF([test $HAVE_TC = "no"]) +OVS_CHECK_TC_QDISC() OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 808c492a22..dc7fa861ef 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -856,7 +856,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([datapath - slow_action on geneve6 tunnel]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_CHECK_TUNNEL_TSO() OVS_CHECK_GENEVE_UDP6ZEROCSUM() @@ -2114,7 +2114,7 @@ AT_CLEANUP AT_BANNER([MPLS]) AT_SETUP([mpls - encap header dp-support]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_SKIP_IF([! ovs-appctl dpif/show-dp-features br0 2>&1 | grep "MPLS Label add: Yes" >/dev/null]) @@ -2147,7 +2147,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([mpls - encap header slow-path]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_CHECK(ovs-appctl dpif/set-dp-features br0 add_mpls false) @@ -2179,7 +2179,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([mpls_mc - encap header dp-support]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_SKIP_IF([! ovs-appctl dpif/show-dp-features br0 2>&1 | grep "MPLS Label add: Yes" >/dev/null]) @@ -2212,7 +2212,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([mpls_mc - encap header slow-path]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_CHECK(ovs-appctl dpif/set-dp-features br0 add_mpls false) @@ -2244,7 +2244,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([mpls - decap header dp-support]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_SKIP_IF([! ovs-appctl dpif/show-dp-features br0 2>&1 | grep "MPLS Label add: Yes" >/dev/null]) @@ -2282,7 +2282,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([mpls - decap header slow-path]) -AT_SKIP_IF([test $HAVE_TCPDUMP = no]) +OVS_CHECK_TCPDUMP() OVS_TRAFFIC_VSWITCHD_START() AT_CHECK(ovs-appctl dpif/set-dp-features br0 add_mpls false) @@ -2321,7 +2321,7 @@ AT_CLEANUP AT_BANNER([QoS]) AT_SETUP([QoS - basic configuration]) -AT_SKIP_IF([test $HAVE_TC = no]) +OVS_CHECK_TC_QDISC() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2355,7 +2355,7 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP
[ovs-dev] [PATCH v3 4/7] ci: Run DPDK tests in GitHub Actions.
Let's enhance our coverage in the CI and run DPDK system tests.
A few DPDK drivers are enabled in DPDK compilation.
Put DPDK build in $PATH for dpdk-testpmd to be available.
sudo drops PATH= updates and -E alone does not seem to preserve this
variable.
Pass PATH=$PATH when running the tests, as a workaround.
Since those tests are run as root, the collection of logs is updated
accordingly.
In GHA, only two cores are available but some test rely on testpmd using
three lcores.
Add a DPDK_EAL_OPTIONS environment variable and use it to map all
testpmd lcores to core 1 (and leave core 0 alone for OVS main and PMD
threads).
Signed-off-by: David Marchand
Acked-by: Aaron Conole
Acked-by: Eelco Chaudron
---
Changes since v1:
- rebased after DPDK build has been moved out of linux-build.sh,
- restored running "normal" checks in the DPDK jobs,
---
.ci/dpdk-build.sh| 8 +---
.ci/linux-build.sh | 15 ++-
.github/workflows/build-and-test.yml | 7 ---
tests/system-dpdk-macros.at | 2 +-
4 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
index 02dcefef61..a754809d3d 100755
--- a/.ci/dpdk-build.sh
+++ b/.ci/dpdk-build.sh
@@ -35,9 +35,11 @@ function build_dpdk()
DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
# OVS compilation and "normal" unit tests (run in the CI) do not depend on
-# any DPDK driver being present.
-# We can disable all drivers to save compilation time.
-DPDK_OPTS="$DPDK_OPTS -Ddisable_drivers=*/*"
+# any DPDK driver.
+# check-dpdk unit tests requires testpmd and some net/ driver.
+# We can disable all drivers but them, in order to save compilation time.
+DPDK_OPTS="$DPDK_OPTS -Denable_apps=test-pmd"
+DPDK_OPTS="$DPDK_OPTS -Denable_drivers=net/null,net/tap,net/virtio"
# Install DPDK using prefix.
DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 8227a57487..aa2ecc5050 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -22,6 +22,9 @@ function install_dpdk()
# Export the following path for pkg-config to find the .pc file.
export PKG_CONFIG_PATH=$DPDK_LIB/pkgconfig/:$PKG_CONFIG_PATH
+# Expose dpdk binaries.
+export PATH=$(pwd)/dpdk-dir/build/bin:$PATH
+
if [ ! -f "${VERSION_FILE}" ]; then
echo "Could not find DPDK in $(pwd)/dpdk-dir"
return 1
@@ -113,7 +116,7 @@ fi
OPTS="${EXTRA_OPTS} ${OPTS} $*"
-if [ "$TESTSUITE" ]; then
+if [ "$TESTSUITE" = 'test' ]; then
# 'distcheck' will reconfigure with required options.
# Now we only need to prepare the Makefile without sparse-wrapped CC.
configure_ovs
@@ -123,6 +126,16 @@ if [ "$TESTSUITE" ]; then
TESTSUITEFLAGS=-j4 RECHECK=yes
else
build_ovs
+for testsuite in $TESTSUITE; do
+run_as_root=
+if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
+sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
+[ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
+export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
+run_as_root="sudo -E PATH=$PATH"
+fi
+$run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
+done
fi
exit 0
diff --git a/.github/workflows/build-and-test.yml
b/.github/workflows/build-and-test.yml
index bc5494e863..4f62efb7c3 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -123,10 +123,10 @@ jobs:
opts: --enable-shared
- compiler: gcc
-testsuite:test
+testsuite:check check-dpdk
dpdk: dpdk
- compiler: clang
-testsuite:test
+testsuite:check check-dpdk
dpdk: dpdk
- compiler: gcc
@@ -213,7 +213,8 @@ jobs:
mkdir logs
cp config.log ./logs/
cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
-tar -czvf logs.tgz logs/
+sudo cp -r ./tests/*testsuite.* ./logs/ || true
+sudo tar -czvf logs.tgz logs/
- name: upload logs on failure
if: failure() || cancelled()
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index c149b9ce70..9a6685fafd 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -92,7 +92,7 @@ m4_define([OVS_DPDK_CHECK_TESTPMD],
# Start dpdk-testpmd in background.
#
m4_define([OVS_DPDK_START_TESTPMD],
- [eal_options="--in-memory --single-file-segments --no-pci"
+ [eal_options="$DPDK_EAL_OPTIONS --in-memory --single-file-segments --no-pci"
options="$1"
[ "$options" != "${options%% -- *}" ] || options="$options -- "
eal_options="$eal_options ${options%% -- *}"
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 3/7] system-dpdk: Don't require hugetlbfs.
dpdk-testpmd does not need hugetlbfs backing as we don't require
multiprocess support in OVS unit tests.
Plus, there is no need for explicitly reserving more memory than what
testpmd would actually need at runtime.
Switch to --in-memory, use dynamic allocations and remove the then
unneeded check on hugetlbfs presence.
Signed-off-by: David Marchand
Acked-by: Aaron Conole
Acked-by: Eelco Chaudron
---
tests/system-dpdk-macros.at | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index e1e91f2972..c149b9ce70 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -7,9 +7,6 @@ m4_define([OVS_DPDK_PRE_CHECK],
[dnl Check Hugepages
AT_CHECK([cat /proc/meminfo], [], [stdout])
AT_SKIP_IF([grep -E 'HugePages_Free: *0' stdout], [], [stdout])
- AT_CHECK([mount], [], [stdout])
- AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], [])
-
])
@@ -95,9 +92,7 @@ m4_define([OVS_DPDK_CHECK_TESTPMD],
# Start dpdk-testpmd in background.
#
m4_define([OVS_DPDK_START_TESTPMD],
- [AT_CHECK([lscpu], [], [stdout])
- AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3))
{printf "512,"}; print "512"}' > NUMA_NODE])
- eal_options="--socket-mem="$(cat NUMA_NODE)" --file-prefix page0
--single-file-segments --no-pci"
+ [eal_options="--in-memory --single-file-segments --no-pci"
options="$1"
[ "$options" != "${options%% -- *}" ] || options="$options -- "
eal_options="$eal_options ${options%% -- *}"
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 2/7] system-dpdk: Introduce helpers for testpmd.
Rather than copy/paste everywhere, introduce helpers to control
testpmd runs.
Rely on --stats-period (which outputs port stats every n seconds) so that
testpmd keeps running without expecting any user input.
Signed-off-by: David Marchand
Acked-by: Aaron Conole
Acked-by: Eelco Chaudron
---
Changes since v1:
- fixed OVS_DPDK_START_TESTPMD passed arguments evaluation:: $@ -> $1,
---
tests/system-dpdk-macros.at | 37 +
tests/system-dpdk.at| 103 +---
2 files changed, 61 insertions(+), 79 deletions(-)
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 3920f08a5e..e1e91f2972 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -79,3 +79,40 @@ m4_define([OVS_DPDK_START_VSWITCHD],
AT_CAPTURE_FILE([ovs-vswitchd.log])
on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
])
+
+
+# OVS_DPDK_CHECK_TESTPMD()
+#
+# Check dpdk-testpmd availability.
+#
+m4_define([OVS_DPDK_CHECK_TESTPMD],
+ [AT_SKIP_IF([! which dpdk-testpmd >/dev/null 2>/dev/null])
+])
+
+
+# OVS_DPDK_START_TESTPMD()
+#
+# Start dpdk-testpmd in background.
+#
+m4_define([OVS_DPDK_START_TESTPMD],
+ [AT_CHECK([lscpu], [], [stdout])
+ AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3))
{printf "512,"}; print "512"}' > NUMA_NODE])
+ eal_options="--socket-mem="$(cat NUMA_NODE)" --file-prefix page0
--single-file-segments --no-pci"
+ options="$1"
+ [ "$options" != "${options%% -- *}" ] || options="$options -- "
+ eal_options="$eal_options ${options%% -- *}"
+ testpmd_options="-a --stats-period 2 ${options#* -- }"
+ dpdk-testpmd $eal_options -- $testpmd_options >testpmd.log 2>&1 & \
+ echo $! > testpmd.pid
+ on_exit "kill -9 `cat testpmd.pid`"
+])
+
+
+# OVS_DPDK_STOP_TESTPMD()
+#
+# Stop background dpdk-testpmd.
+#
+m4_define([OVS_DPDK_STOP_TESTPMD],
+ [AT_CHECK([kill `cat testpmd.pid`])
+ OVS_WAIT([kill -0 `cat testpmd.pid`], [kill -9 `cat testpmd.pid`])
+])
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 0f58e85742..270587e2c0 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -97,13 +97,9 @@ dnl Ping vhost-user port
AT_SETUP([OVS-DPDK - ping vhost-user ports])
AT_KEYWORDS([dpdk])
OVS_DPDK_PRE_CHECK()
-AT_SKIP_IF([! which dpdk-testpmd >/dev/null 2>/dev/null])
+OVS_DPDK_CHECK_TESTPMD()
OVS_DPDK_START([--no-pci])
-dnl Find number of sockets
-AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3))
{printf "512,"}; print "512"}' > NUMA_NODE])
-
dnl Add userspace bridge and attach it to OVS
AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuser0 -- set Interface
dpdkvhostuser0 \
@@ -125,12 +121,8 @@ ADD_NAMESPACES(ns1, ns2)
dnl Add veth device
ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
-dnl Execute testpmd in background
-on_exit "pkill -f -x -9 'tail -f /dev/null'"
-tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
- --vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostuser0" \
- --vdev="net_tap0,iface=tap0" --file-prefix page0 \
- --single-file-segments -- -a
>$OVS_RUNDIR/testpmd-dpdkvhostuser0.log 2>&1 &
+OVS_DPDK_START_TESTPMD([--vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostuser0"
\
+--vdev="net_tap0,iface=tap0"])
OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log])
OVS_WAIT_UNTIL([ip link show dev tap0 | grep -qw LOWER_UP])
@@ -151,8 +143,7 @@ AT_CHECK([ip netns exec ns2 ip link show], [], [stdout],
[stderr])
AT_CHECK([ip netns exec ns1 ping -c 4 -I tap0 172.31.110.12], [], [stdout],
[stderr])
-dnl Clean up the testpmd now
-pkill -f -x -9 'tail -f /dev/null'
+OVS_DPDK_STOP_TESTPMD()
dnl Wait for vhost-user handling the socket disconnect.
OVS_WAIT_UNTIL([grep "vHost Device '$OVS_RUNDIR/dpdkvhostuser0' has been
removed" ovs-vswitchd.log])
@@ -173,13 +164,9 @@ dnl Ping vhost-user-client port
AT_SETUP([OVS-DPDK - ping vhost-user-client ports])
AT_KEYWORDS([dpdk])
OVS_DPDK_PRE_CHECK()
-AT_SKIP_IF([! which dpdk-testpmd >/dev/null 2>/dev/null])
+OVS_DPDK_CHECK_TESTPMD()
OVS_DPDK_START([--no-pci])
-dnl Find number of sockets
-AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3))
{printf "512,"}; print "512"}' > NUMA_NODE])
-
dnl Add userspace bridge and attach it to OVS
AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface \
@@ -200,13 +187,8 @@ ADD_NAMESPACES(ns1, ns2)
dnl Add veth device
ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
-dnl Execute testpmd in background
-on_exit "pkill -f -x -9 'tail -f /dev/null'"
-tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
-
[ovs-dev] [PATCH v3 1/7] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
As reported by Ales when doing some OVN integration tests with OVS 3.2,
net/tap has broken L4 checksum offloads.
Fixes are pending on DPDK side.
Until they get in a LTS release used by OVS, disable those Tx offloads.
Acked-by: Eelco Chaudron
Signed-off-by: David Marchand
---
Changes since v1:
- added this patch as part of the DPDK tests update series,
- restored TSO feature,
- added comment in code,
- changed log level,
---
lib/netdev-dpdk.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2f5a133184..55700250df 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1312,6 +1312,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
dev->hw_ol_features &= ~NETDEV_RX_HW_SCATTER;
}
+if (!strcmp(info.driver_name, "net_tap")) {
+/* FIXME: L4 checksum offloading is broken in DPDK net/tap driver.
+ * This workaround can be removed once the fix makes it to a DPDK
+ * LTS release used by OVS. */
+VLOG_INFO("%s: disabled Tx L4 checksum offloads for a net/tap port.",
+ netdev_get_name(>up));
+info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_UDP_CKSUM;
+info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
+}
+
if (info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM) {
dev->hw_ol_features |= NETDEV_TX_IPV4_CKSUM_OFFLOAD;
} else {
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] northd, controller: Add CoPP for SVC monitor
Bleep bloop. Greetings Mark Michelson, 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: ERROR: Committer Mark Michelson needs to sign off. Lines checked: 131, Warnings: 0, Errors: 1 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] [PATCH ovn branch-22.03 1/2] Set release date for 22.03.3.
Signed-off-by: Mark Michelson --- NEWS | 4 +++- debian/changelog | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 55b73b5f8..9dc5ae9ce 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,9 @@ -OVN v22.03.3 - xx xxx +OVN v22.03.3 - 29 Aug 2023 -- + - Bug fixes - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast Listener Discovery protocols, regardless of ACLs defined. + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. OVN v22.03.2 - 20 Dec 2022 -- diff --git a/debian/changelog b/debian/changelog index 1ed1b5a91..5a8185a26 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (22.03.3-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Tue, 20 Dec 2022 13:53:49 -0500 + -- OVN team Tue, 29 Aug 2023 11:30:16 -0400 OVN (22.03.2-1) unstable; urgency=low [ OVN team ] -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.03 2/2] Prepare for 22.03.4.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 9dc5ae9ce..1dfded4ba 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.03.4 - xx xxx +-- + OVN v22.03.3 - 29 Aug 2023 -- - Bug fixes diff --git a/configure.ac b/configure.ac index a99446683..f6317743a 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.03.3, b...@openvswitch.org) +AC_INIT(ovn, 22.03.4, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 5a8185a26..84f0b450a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.03.4-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Tue, 29 Aug 2023 11:30:16 -0400 + OVN (22.03.3-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.03 0/2] Release patches for v22.03.3.
Mark Michelson (2): Set release date for 22.03.3. Prepare for 22.03.4. NEWS | 7 ++- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.09 2/2] Prepare for 22.09.3.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 1758bf633..bbe636cd9 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.09.3 - xx xxx +-- + OVN v22.09.2 - 29 Aug 2023 -- - Bug fixes diff --git a/configure.ac b/configure.ac index 408184649..11e93326a 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.09.2, b...@openvswitch.org) +AC_INIT(ovn, 22.09.3, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index e7ec2b762..0a26210f5 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.09.3-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Tue, 29 Aug 2023 11:30:11 -0400 + OVN (22.09.2-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.09 0/2] Release patches for v22.09.2.
Mark Michelson (2): Set release date for 22.09.2. Prepare for 22.09.3. NEWS | 7 ++- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.09 1/2] Set release date for 22.09.2.
Signed-off-by: Mark Michelson --- NEWS | 4 +++- debian/changelog | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index d5170786b..1758bf633 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ -OVN v22.09.2 - xx xxx +OVN v22.09.2 - 29 Aug 2023 -- + - Bug fixes - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast Listener Discovery protocols, regardless of ACLs defined. - Send ICMP Fragmentation Needed packets back to offending ports when @@ -8,6 +9,7 @@ OVN v22.09.2 - xx xxx physical network via a localnet port, in which case multichassis ports may have an effective MTU different from regular ports and hence may need this mechanism to maintain connectivity with other peers in the network. + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. OVN v22.09.1 - 20 Dec 2022 -- diff --git a/debian/changelog b/debian/changelog index 08cc66fc0..e7ec2b762 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (22.09.2-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Tue, 20 Dec 2022 13:53:56 -0500 + -- OVN team Tue, 29 Aug 2023 11:30:11 -0400 OVN (22.09.1-1) unstable; urgency=low [ OVN team ] -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.12 1/2] Set release date for 22.12.1.
Signed-off-by: Mark Michelson --- NEWS | 4 +++- debian/changelog | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 01baa2be6..4efa1d1fe 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ -OVN v22.12.1 - xx xxx +OVN v22.12.1 - 29 Aug 2023 -- + - Bug fixes - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast Listener Discovery protocols, regardless of ACLs defined. - Send ICMP Fragmentation Needed packets back to offending ports when @@ -8,6 +9,7 @@ OVN v22.12.1 - xx xxx physical network via a localnet port, in which case multichassis ports may have an effective MTU different from regular ports and hence may need this mechanism to maintain connectivity with other peers in the network. + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. OVN v22.12.0 - 16 Dec 2022 -- diff --git a/debian/changelog b/debian/changelog index d658774f6..b58fbaef7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (22.12.1-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Fri, 16 Dec 2022 09:52:44 -0500 + -- OVN team Tue, 29 Aug 2023 11:30:03 -0400 ovn (22.12.0-1) unstable; urgency=low -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.12 2/2] Prepare for 22.12.2.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 4efa1d1fe..a8fdfd387 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.12.2 - xx xxx +-- + OVN v22.12.1 - 29 Aug 2023 -- - Bug fixes diff --git a/configure.ac b/configure.ac index 357758e0c..a42df536e 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.12.1, b...@openvswitch.org) +AC_INIT(ovn, 22.12.2, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index b58fbaef7..6fdfa5c2c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.12.2-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Tue, 29 Aug 2023 11:30:03 -0400 + OVN (22.12.1-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.12 0/2] Release patches for v22.12.1.
Mark Michelson (2): Set release date for 22.12.1. Prepare for 22.12.2. NEWS | 7 ++- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.03 0/2] Release patches for v23.03.1.
Mark Michelson (2): Set release date for 23.03.1. Prepare for 23.03.2. NEWS | 7 ++- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.03 2/2] Prepare for 23.03.2.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 1c75eeeae..6c680afbb 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v23.03.2 - xx xxx +-- + OVN v23.03.1 - 29 Aug 2023 -- - Bug fixes diff --git a/configure.ac b/configure.ac index 0ba9e8d7e..e4133e888 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 23.03.1, b...@openvswitch.org) +AC_INIT(ovn, 23.03.2, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 4c744105e..15c14a7bd 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (23.03.2-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Tue, 29 Aug 2023 11:29:51 -0400 + OVN (23.03.1-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.03 1/2] Set release date for 23.03.1.
Signed-off-by: Mark Michelson --- NEWS | 4 +++- debian/changelog | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 47d259150..1c75eeeae 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ -OVN v23.03.1 - xx xxx +OVN v23.03.1 - 29 Aug 2023 -- + - Bug fixes - CT entries are not flushed by default anymore whenever a load balancer backend is removed. A new, per-LB, option 'ct_flush' can be used to restore the previous behavior. Disabled by default. @@ -15,6 +16,7 @@ OVN v23.03.1 - xx xxx Existing sessions might get re-hashed to a different ECMP path when OVN detects the algorithm support in the datapath during an upgrade or restart of ovn-controller. + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. OVN v23.03.0 - 03 Mar 2023 -- diff --git a/debian/changelog b/debian/changelog index 02a9953ba..4c744105e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (23.03.1-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Fri, 03 Mar 2023 10:40:37 -0500 + -- OVN team Tue, 29 Aug 2023 11:29:51 -0400 ovn (23.03.0-1) unstable; urgency=low -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.06 2/2] Prepare for 23.06.2.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ef000ef7b..0056bb175 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v23.06.2 - xx xxx +-- + OVN v23.06.1 - 29 Aug 2023 -- - Bug fixes diff --git a/configure.ac b/configure.ac index fc4ba024a..3ed093ebe 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 23.06.1, b...@openvswitch.org) +AC_INIT(ovn, 23.06.2, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index e1d505a7e..e147d113c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (23.06.2-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Tue, 29 Aug 2023 11:28:59 -0400 + OVN (23.06.1-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.06 1/2] Set release date for 23.06.1.
Signed-off-by: Mark Michelson --- NEWS | 4 +++- debian/changelog | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index d17c5d508..ef000ef7b 100644 --- a/NEWS +++ b/NEWS @@ -1,9 +1,11 @@ -OVN v23.06.1 - xx xxx +OVN v23.06.1 - 29 Aug 2023 -- + - Bug fixes - ECMP routes use L4_SYM dp-hash by default if the datapath supports it. Existing sessions might get re-hashed to a different ECMP path when OVN detects the algorithm support in the datapath during an upgrade or restart of ovn-controller. + - Add CoPP for the svc_monitor_mac. This addresses CVE-2023-3153. OVN v23.06.0 - 01 Jun 2023 -- diff --git a/debian/changelog b/debian/changelog index 124462da2..e1d505a7e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (23.06.1-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Thu, 01 Jun 2023 18:30:40 -0400 + -- OVN team Tue, 29 Aug 2023 11:28:59 -0400 ovn (23.06.0-1) unstable; urgency=low -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-23.06 0/2] Release patches for v23.06.1.
Mark Michelson (2): Set release date for 23.06.1. Prepare for 23.06.2. NEWS | 7 ++- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 14 insertions(+), 3 deletions(-) -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] northd, controller: Add CoPP for SVC monitor
Thanks Ales,
Acked-by: Mark Michelson
I pushed the change to main and all branches back to 22.03.
On 8/29/23 11:24, Mark Michelson wrote:
From: Ales Musil
The SVC monitor was exposed without any limitation.
Add CoPP for the SVC monitor flow, which adds a way
for CMSs to limit the traffic that this flow accepts.
Signed-off-by: Ales Musil
---
lib/copp.c | 1 +
lib/copp.h | 1 +
northd/northd.c | 8 +---
ovn-nb.xml | 4
tests/ovn-northd.at | 2 +-
tests/system-ovn.at | 20 +++-
6 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/lib/copp.c b/lib/copp.c
index 603e3f5bf..11dd9029d 100644
--- a/lib/copp.c
+++ b/lib/copp.c
@@ -38,6 +38,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = {
[COPP_ND_RA_OPTS]= "nd-ra-opts",
[COPP_TCP_RESET] = "tcp-reset",
[COPP_REJECT]= "reject",
+[COPP_SVC_MONITOR] = "svc-monitor",
[COPP_BFD] = "bfd",
};
diff --git a/lib/copp.h b/lib/copp.h
index f03004aa6..b99737220 100644
--- a/lib/copp.h
+++ b/lib/copp.h
@@ -37,6 +37,7 @@ enum copp_proto {
COPP_TCP_RESET,
COPP_BFD,
COPP_REJECT,
+COPP_SVC_MONITOR,
COPP_PROTO_MAX,
COPP_PROTO_INVALID = COPP_PROTO_MAX,
};
diff --git a/northd/northd.c b/northd/northd.c
index 8519617de..b43a67b87 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9804,9 +9804,11 @@ build_lswitch_destination_lookup_bmcast(struct
ovn_datapath *od,
{
ovs_assert(od->nbs);
-ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
- "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
- "handle_svc_check(inport);");
+ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
+ "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
+ "handle_svc_check(inport);",
+ copp_meter_get(COPP_SVC_MONITOR, od->nbs->copp,
+ meter_groups));
struct mcast_switch_info *mcast_sw_info = >mcast_info.sw;
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4fbf4f7e5..b7ddd50c5 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -514,6 +514,10 @@
Rate limiting meter for packets that trigger a reject action
+
+ Rate limiting meter for packets that are arriving to service
+ monitor MAC address.
+
See External IDs at the beginning of this document.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index aa59754c1..5c2b78f2f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3655,7 +3655,7 @@ AT_CHECK([ovn-sbctl list logical_flow | grep
trigger_event -A 2 | grep -q meter0
# let's try to add an usupported protocol "dhcp"
AT_CHECK([ovn-nbctl --wait=hv copp-add copp5 dhcp meter1],[1],[],[dnl
-ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve,
dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp,
nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject.
+ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve,
dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp,
nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject, svc-monitor.
])
#Let's try to add a valid protocol to an unknown datapath
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 55c5ddc19..59d0cb2a0 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -7469,6 +7469,23 @@ OVS_WAIT_UNTIL([
])
kill $(pidof tcpdump)
+check ovn-nbctl set nb_global . options:svc_monitor_mac="33:33:33:33:33:33"
+check ovn-nbctl meter-add svc-meter drop 1 pktps 0
+check ovn-nbctl --wait=hv copp-add copp4 svc-monitor svc-meter
+check ovn-nbctl --wait=hv ls-copp-add copp4 sw0
+check ovn-appctl -t ovn-controller vlog/set vconn:dbg
+AT_CHECK([ovn-nbctl copp-list copp4], [0], [dnl
+svc-monitor: svc-meter
+])
+
+ip netns exec sw01 scapy -H <<-EOF
+p = Ether(dst="33:33:33:33:33:33", src="f0:00:00:01:02:03") /\
+IP(dst="192.168.1.100", src="192.168.1.2") / TCP(dport=1234, sport=1234)
+sendp(p, iface='sw01', loop=0, verbose=0, count=20)
+EOF
+
+OVS_WAIT_UNTIL([test "1" = "$(grep -c "dl_dst=33:33:33:33:33:33"
ovn-controller.log)"])
+
OVS_APP_EXIT_AND_WAIT([ovn-controller])
as ovn-sb
@@ -7482,7 +7499,8 @@ OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
as
OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
-/.*terminating with signal 15.*/d"])
+/.*terminating with signal 15.*/d
+/.*Service monitor not found/d"])
AT_CLEANUP
])
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] northd, controller: Add CoPP for SVC monitor
From: Ales Musil
The SVC monitor was exposed without any limitation.
Add CoPP for the SVC monitor flow, which adds a way
for CMSs to limit the traffic that this flow accepts.
Signed-off-by: Ales Musil
---
lib/copp.c | 1 +
lib/copp.h | 1 +
northd/northd.c | 8 +---
ovn-nb.xml | 4
tests/ovn-northd.at | 2 +-
tests/system-ovn.at | 20 +++-
6 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/lib/copp.c b/lib/copp.c
index 603e3f5bf..11dd9029d 100644
--- a/lib/copp.c
+++ b/lib/copp.c
@@ -38,6 +38,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = {
[COPP_ND_RA_OPTS]= "nd-ra-opts",
[COPP_TCP_RESET] = "tcp-reset",
[COPP_REJECT]= "reject",
+[COPP_SVC_MONITOR] = "svc-monitor",
[COPP_BFD] = "bfd",
};
diff --git a/lib/copp.h b/lib/copp.h
index f03004aa6..b99737220 100644
--- a/lib/copp.h
+++ b/lib/copp.h
@@ -37,6 +37,7 @@ enum copp_proto {
COPP_TCP_RESET,
COPP_BFD,
COPP_REJECT,
+COPP_SVC_MONITOR,
COPP_PROTO_MAX,
COPP_PROTO_INVALID = COPP_PROTO_MAX,
};
diff --git a/northd/northd.c b/northd/northd.c
index 8519617de..b43a67b87 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9804,9 +9804,11 @@ build_lswitch_destination_lookup_bmcast(struct
ovn_datapath *od,
{
ovs_assert(od->nbs);
-ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
- "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
- "handle_svc_check(inport);");
+ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
+ "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
+ "handle_svc_check(inport);",
+ copp_meter_get(COPP_SVC_MONITOR, od->nbs->copp,
+ meter_groups));
struct mcast_switch_info *mcast_sw_info = >mcast_info.sw;
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4fbf4f7e5..b7ddd50c5 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -514,6 +514,10 @@
Rate limiting meter for packets that trigger a reject action
+
+ Rate limiting meter for packets that are arriving to service
+ monitor MAC address.
+
See External IDs at the beginning of this document.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index aa59754c1..5c2b78f2f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3655,7 +3655,7 @@ AT_CHECK([ovn-sbctl list logical_flow | grep
trigger_event -A 2 | grep -q meter0
# let's try to add an usupported protocol "dhcp"
AT_CHECK([ovn-nbctl --wait=hv copp-add copp5 dhcp meter1],[1],[],[dnl
-ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve,
dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp,
nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject.
+ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve,
dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp,
nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject, svc-monitor.
])
#Let's try to add a valid protocol to an unknown datapath
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 55c5ddc19..59d0cb2a0 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -7469,6 +7469,23 @@ OVS_WAIT_UNTIL([
])
kill $(pidof tcpdump)
+check ovn-nbctl set nb_global . options:svc_monitor_mac="33:33:33:33:33:33"
+check ovn-nbctl meter-add svc-meter drop 1 pktps 0
+check ovn-nbctl --wait=hv copp-add copp4 svc-monitor svc-meter
+check ovn-nbctl --wait=hv ls-copp-add copp4 sw0
+check ovn-appctl -t ovn-controller vlog/set vconn:dbg
+AT_CHECK([ovn-nbctl copp-list copp4], [0], [dnl
+svc-monitor: svc-meter
+])
+
+ip netns exec sw01 scapy -H <<-EOF
+p = Ether(dst="33:33:33:33:33:33", src="f0:00:00:01:02:03") /\
+IP(dst="192.168.1.100", src="192.168.1.2") / TCP(dport=1234, sport=1234)
+sendp(p, iface='sw01', loop=0, verbose=0, count=20)
+EOF
+
+OVS_WAIT_UNTIL([test "1" = "$(grep -c "dl_dst=33:33:33:33:33:33"
ovn-controller.log)"])
+
OVS_APP_EXIT_AND_WAIT([ovn-controller])
as ovn-sb
@@ -7482,7 +7499,8 @@ OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
as
OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
-/.*terminating with signal 15.*/d"])
+/.*terminating with signal 15.*/d
+/.*Service monitor not found/d"])
AT_CLEANUP
])
--
2.40.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-23.03] northd: Fix recompute of referenced chassis in HA chassis groups.
Thank you Ilya, I applied this patch to 23.03 and all branches back to
22.03.
On 8/25/23 14:03, Ilya Maximets wrote:
Current implementation of 'ref_chassis' recompute is horribly slow.
For each port binding it finds LR group, then it adds a chassis of the
current port binding to each HA chassis group of this LR group.
The number of ports is much greater than number of chassis. And the
number of distinct LR groups is even smaller than a number of chassis
in a typical setup. Chances are, the setup only has one LR group.
So, northd today is trying to add the same chassis to the same
HA chassis group over and over again.
In an OpenStack setup with ~20 chassis, 40K ports and 5K routers we
end up with 33+ million calls to add_to_ha_ref_chassis_info() that
also performs a linear search for duplicates leading to about 270
million operations, while we only have 5K Sb HA chassis groups with
15 ref_chassis in each. I.e. only 5000 * 15 = 75.000 operations out
of 270 million are actually useful.
In practice that leads to 80 second recompute:
inc_proc_eng|INFO|node: sync_from_sb, recompute (forced) took 80074ms
Fix that by accumulating unique chassis per unique LR groups first
and then updating ref_chassis with these unique values avoiding any
unnecessary calls. Result is ~1000x performance improvement:
inc_proc_eng|DBG|node: sync_from_sb, recompute (forced) took 75ms
Considering this as a performance bug, because it makes OVN practically
unusable in certain (default?) OpenStack configurations at scale.
Fixes: b82d351976a1 ("ovn: Add generic HA chassis group")
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2023-August/052614.html
Reported-by: Roberto Bartzen Acosta
Acked-by: Mark Michelson
Signed-off-by: Ilya Maximets
---
cherry-picked from commit 4023d6a5fa573021a6218ac49796f3f7688227d7
This version is also applicable all the way down to 22.03.
northd/northd.c | 119 +++-
1 file changed, 76 insertions(+), 43 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 4d3646d10..7b91a94cb 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -857,6 +857,8 @@ struct lrouter_group {
int n_router_dps;
/* Set of ha_chassis_groups which are associated with the router dps. */
struct sset ha_chassis_groups;
+/* Temporary storage for chassis references while computing HA groups. */
+struct hmapx tmp_ha_chassis;
};
static struct ovn_datapath *
@@ -7874,6 +7876,7 @@ build_lrouter_groups(struct hmap *ports, struct ovs_list
*lr_list)
od->lr_group->router_dps[0] = od;
od->lr_group->n_router_dps = 1;
sset_init(>lr_group->ha_chassis_groups);
+hmapx_init(>lr_group->tmp_ha_chassis);
build_lrouter_groups__(ports, od);
}
}
@@ -15783,6 +15786,7 @@ destroy_datapaths_and_ports(struct hmap *datapaths,
struct hmap *ports,
free(lr_group->router_dps);
sset_destroy(_group->ha_chassis_groups);
+hmapx_destroy(_group->tmp_ha_chassis);
free(lr_group);
}
}
@@ -16319,38 +16323,27 @@ ovnnb_db_run(struct northd_input *input_data,
smap_destroy();
}
-/* Stores the list of chassis which references an ha_chassis_group.
+/* Stores the set of chassis which references an ha_chassis_group.
*/
struct ha_ref_chassis_info {
const struct sbrec_ha_chassis_group *ha_chassis_group;
-struct sbrec_chassis **ref_chassis;
-size_t n_ref_chassis;
-size_t free_slots;
+struct hmapx ref_chassis;
};
static void
add_to_ha_ref_chassis_info(struct ha_ref_chassis_info *ref_ch_info,
- const struct sbrec_chassis *chassis)
+ const struct hmapx *chassis)
{
-for (size_t j = 0; j < ref_ch_info->n_ref_chassis; j++) {
-if (ref_ch_info->ref_chassis[j] == chassis) {
- return;
-}
-}
+if (!hmapx_count(_ch_info->ref_chassis)) {
+hmapx_destroy(_ch_info->ref_chassis);
+hmapx_clone(_ch_info->ref_chassis, chassis);
+} else {
+struct hmapx_node *node;
-/* Allocate space for 3 chassis at a time. */
-if (!ref_ch_info->free_slots) {
-ref_ch_info->ref_chassis =
-xrealloc(ref_ch_info->ref_chassis,
- sizeof *ref_ch_info->ref_chassis *
- (ref_ch_info->n_ref_chassis + 3));
-ref_ch_info->free_slots = 3;
+HMAPX_FOR_EACH (node, chassis) {
+hmapx_add(_ch_info->ref_chassis, node->data);
+}
}
-
-ref_ch_info->ref_chassis[ref_ch_info->n_ref_chassis] =
-CONST_CAST(struct sbrec_chassis *, chassis);
-ref_ch_info->n_ref_chassis++;
-ref_ch_info->free_slots--;
}
struct ha_chassis_group_node {
@@ -16379,9 +16372,20 @@ update_sb_ha_group_ref_chassis(struct northd_input
*input_data,
struct shash_node *node;
Re: [ovs-dev] [PATCH ovn 2/2] ovn-ic: support learning routes in same AZ
Thank you for the feedback,
the route learning feature needs to be explicitly enabled [1]. This is
currently a global setting, so if we want to limit this to individual
routers i would rather make this a general ovn-ic feature than to
include this here.
Thanks
Felix
[1]
https://docs.ovn.org/en/latest/tutorials/ovn-interconnection.html#route-advertisement
On Wed, Aug 02, 2023 at 03:13:01PM -0400, Mark Michelson wrote:
> Looks good to me.
>
> Acked-by: Mark Michelson
>
> Having said that, I'd like for some OVN devs that are more familiar with the
> ins and outs of ovn-ic to make the determination if this is behavior that we
> want or not. For non-IC OVN, routers learn routes to their neighbor routers
> automatically, unless the "dynamic_neigh_routers" option is enabled. For
> ovn-ic, do we have a way to ensure that routers do not learn neighbor routes
> if they do not want to?
>
> On 8/1/23 03:22, maximkorezkij via dev wrote:
> > when connecting multiple logical routers to a transit switch per az then
> > previously the routers in the same az would not learn each others
> > routes while the routers in the others az would learn all of them.
> >
> > As this is confusing and would require each user to have additional
> > logical that configures static routing within each az.
> >
> > Signed-off-by: Maxim Korezkij
> > Signed-off-by: Felix Huettner
> > ---
> > ic/ovn-ic.c | 52 -
> > tests/ovn-ic.at | 2 ++
> > 2 files changed, 40 insertions(+), 14 deletions(-)
> >
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index 49850954f..cea14d008 100644
> > --- a/ic/ovn-ic.c
> > +++ b/ic/ovn-ic.c
> > @@ -861,6 +861,8 @@ struct ic_route_info {
> > const char *origin;
> > const char *route_table;
> >
> > +const struct nbrec_logical_router *nb_lr;
> > +
> > /* Either nb_route or nb_lrp is set and the other one must be NULL.
> >* - For a route that is learned from IC-SB, or a static route that is
> >* generated from a route that is configured in NB, the "nb_route"
> > @@ -937,7 +939,8 @@ parse_route(const char *s_prefix, const char *s_nexthop,
> > /* Return false if can't be added due to bad format. */
> > static bool
> > add_to_routes_learned(struct hmap *routes_learned,
> > - const struct nbrec_logical_router_static_route
> > *nb_route)
> > + const struct nbrec_logical_router_static_route
> > *nb_route,
> > + const struct nbrec_logical_router *nb_lr)
> > {
> > struct in6_addr prefix, nexthop;
> > unsigned int plen;
> > @@ -959,6 +962,7 @@ add_to_routes_learned(struct hmap *routes_learned,
> > ic_route->nb_route = nb_route;
> > ic_route->origin = origin;
> > ic_route->route_table = nb_route->route_table;
> > +ic_route->nb_lr = nb_lr;
> > hmap_insert(routes_learned, _route->node,
> > ic_route_hash(, plen, , origin,
> > nb_route->route_table));
> > @@ -1099,7 +1103,8 @@ add_to_routes_ad(struct hmap *routes_ad, const struct
> > in6_addr prefix,
> >unsigned int plen, const struct in6_addr nexthop,
> >const char *origin, const char *route_table,
> >const struct nbrec_logical_router_port *nb_lrp,
> > - const struct nbrec_logical_router_static_route *nb_route)
> > + const struct nbrec_logical_router_static_route *nb_route,
> > + const struct nbrec_logical_router *nb_lr)
> > {
> > if (route_table == NULL) {
> > route_table = "";
> > @@ -1117,6 +1122,7 @@ add_to_routes_ad(struct hmap *routes_ad, const struct
> > in6_addr prefix,
> > ic_route->origin = origin;
> > ic_route->route_table = route_table;
> > ic_route->nb_lrp = nb_lrp;
> > +ic_route->nb_lr = nb_lr;
> > hmap_insert(routes_ad, _route->node, hash);
> > } else {
> > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > @@ -1130,6 +1136,7 @@ static void
> > add_static_to_routes_ad(
> > struct hmap *routes_ad,
> > const struct nbrec_logical_router_static_route *nb_route,
> > +const struct nbrec_logical_router *nb_lr,
> > const struct lport_addresses *nexthop_addresses,
> > const struct smap *nb_options)
> > {
> > @@ -1172,14 +1179,15 @@ add_static_to_routes_ad(
> > }
> >
> > add_to_routes_ad(routes_ad, prefix, plen, nexthop,
> > ROUTE_ORIGIN_STATIC,
> > - nb_route->route_table, NULL, nb_route);
> > + nb_route->route_table, NULL, nb_route, nb_lr);
> > }
> >
> > static void
> > add_network_to_routes_ad(struct hmap *routes_ad, const char *network,
> >const struct nbrec_logical_router_port *nb_lrp,
> >const struct lport_addresses *nexthop_addresses,
> > -
Re: [ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
On Fri, Aug 25, 2023 at 3:46 PM Ilya Maximets wrote:
>
> On 8/24/23 17:19, David Marchand wrote:
> > As reported by Ales when doing some OVN integration tests with OVS 3.2,
> > net/tap has broken L4 checksum offloads.
> >
> > Fixes are pending on DPDK side.
> > Until they get in a LTS release used by OVS, disable those Tx offloads.
> >
> > Signed-off-by: David Marchand
> > ---
> > lib/netdev-dpdk.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 8f1361e21f..fc7225cba1 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1312,6 +1312,14 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> > dev->hw_ol_features &= ~NETDEV_RX_HW_SCATTER;
> > }
> >
> > +if (!strcmp(info.driver_name, "net_tap")) {
> > +VLOG_WARN("%s: disabled Tx L4 checksum offloads for a net/tap
> > port.",
> > + netdev_get_name(>up));
>
> Maybe convert this one to INFO? I'm not sure we need to warn users every
> time the tap interface is reconfigured. It's not a high performance port
> anyway.
>
> Also, would be nice to have an XXX/FIXME comment here explaining the
> situation, so we do not forget to remove this hack in the future.
Ok.
>
> > +info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_UDP_CKSUM;
> > +info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
> > +info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_TCP_TSO;
>
> Did someone test this with userspace-tso enabled? I mean, do we need to
> backport this to 3.1 as well? Or even maybe 2.17 ?
I had a try with TSO enabled (virtio-net -> vhost-user -> ovs -> tap).
Without the patch, a tcp connection does not get established, since
TCP checksums are KO and we don't get to TSO packets.
I then tested by only filtering TCP/UDP checksum offloading.
Keeping TSO feature for net/tap works fine and TSO'd packets have
valid checksums...
Which (kind of) makes sense, when you look at the bug in the net/tap
driver: it was expecting l4_len to compute l4 checksum, and OVS sets
it when for TSO is requested.
I can update the patch and only filter RTE_ETH_TX_OFFLOAD_UDP_CKSUM
and RTE_ETH_TX_OFFLOAD_TCP_CKSUM.
As for earlier OVS versions, the TSO feature is experimental.
The code changed a bit when compared to 3.2.
I am not sure it is worth spending time fixing in a corner case like net/tap.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: offload scapy transformations to a separate daemon
On 8/25/23 18:50, Ihar Hrachyshka wrote:
> The daemon life cycle spans over the whole test case life time, which
> significantly speeds up test cases that rely on fmt_pkt for packet byte
> representations.
>
> The speed-up comes from the fact that we no longer start a python
> environment with all scapy modules imported on any fmt_pkt invocation;
> but only on the first call to fmt_pkt.
>
> The daemon is not started for test cases that don't trigger fmt_pkt.
> (The server is started lazily as part of fmt_pkt call.)
>
> For example, without the daemon, all tests that use fmt_pkt took the
> following on my vagrant box:
>
> real15m27.117s
> user23m55.023s
> sys 4m35.469s
>
> With the daemon, the same set of tests run in:
>
> real4m34.092s
> user3m20.231s
> sys 1m10.886s
>
> We may want to make the daemon global, so that it's invoked once per
> test suite run. But I haven't figured out, yet, how to run a trap to
> clean up the deamon and its socket and pid files on suite exit (and not
> on test case exit.)
Hi, Ihar. Not a full review, but a few comments inline.
>
> Signed-off-by: Ihar Hrachyshka
> ---
> tests/automake.mk | 1 +
> tests/ovn-macros.at | 27 ++--
> tests/scapy_server.py | 100 ++
> 3 files changed, 124 insertions(+), 4 deletions(-)
> create mode 100755 tests/scapy_server.py
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index eea0d00f4..0f7f1bd9a 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -309,6 +309,7 @@ tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
> CHECK_PYFILES = \
> tests/test-l7.py \
> tests/uuidfilt.py \
> + tests/scapy_server.py \
> tests/test-tcp-rst.py \
> tests/check_acl_log.py
>
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 13d5dc3d4..8b12f8e55 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -816,6 +816,15 @@ ovn_trace_client() {
> ovs-appctl -t $target trace "$@" | tee trace | sed '/^# /d'
> }
>
> +wait_unix_socket() {
> +local socketfile=$1
> +for i in `seq 100`; do
> +nc -zU "$socketfile" && return 0
> +sleep 0.1
> +done
> +return 1
> +}
> +
> # Receives a string with scapy python code that represents a packet.
> # Returns a hex-string that contains bytes that reflect the packet symbolic
> # description.
> @@ -833,10 +842,20 @@ ovn_trace_client() {
> # ovs-appctl netdev-dummy/receive $vif $packet
> #
> fmt_pkt() {
> -echo "from scapy.all import *; \
> - import binascii; \
> - out = binascii.hexlify(raw($1)); \
> - print(out.decode())" | $PYTHON3
> +sockfile=$ovs_base/scapy.sock
> +if [[ ! -e $sockfile ]]; then
> +start_scapy_server > /dev/null
> +wait_unix_socket $sockfile
This should probbaly be OVS_WAIT_UNTIL macro that tests for
a socket file to exist.
> +fi
> +echo "$(echo $1 | nc -U $sockfile)"
> +}
> +
> +start_scapy_server() {
> +pidfile=$ovs_base/scapy.pid
> +sockfile=$ovs_base/scapy.sock
> +"$top_srcdir"/tests/scapy_server.py --pidfile=$pidfile
> --sockfile=$sockfile
> +on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`"
> +on_exit "test -e \"$sockfile\" && rm \"$sockfile\""
> }
>
> sleep_sb() {
> diff --git a/tests/scapy_server.py b/tests/scapy_server.py
> new file mode 100755
> index 0..a417d42c9
> --- /dev/null
> +++ b/tests/scapy_server.py
> @@ -0,0 +1,100 @@
> +#!/usr/bin/env python3
> +
> +import argparse
> +import atexit
> +import os
> +import socket
> +import sys
> +import threading
> +
> +import binascii
> +from scapy.all import * # noqa: F401,F403
> +from scapy.all import raw
> +
> +
> +_MAX_PATTERN_LEN = 1024 * 32
> +
> +
> +def write_pidfile(pidfile):
> +pid = str(os.getpid())
> +with open(pidfile, 'w') as f:
> +f.write(pid)
> +
> +
> +def remove_pidfile(pidfile):
> +os.remove(pidfile)
> +
> +
> +def check_pidfile(pidfile):
> +if os.path.exists(pidfile):
> +with open(pidfile, 'r') as f:
> +pid = f.read().strip()
> +try:
> +pid = int(pid)
> +if os.path.exists(f"/proc/{pid}"):
> +print("Scapy server is already running:", pid)
> +sys.exit(1)
> +except ValueError:
> +sys.exit(1)
> +
> +
> +def fork():
> +try:
> +pid = os.fork()
> +if pid > 0:
> +sys.exit(0)
> +except OSError as e:
> +print("Fork failed:", e)
> +sys.exit(1)
> +
> +
> +def daemonize(pidfile):
> +fork()
> +check_pidfile(pidfile)
> +write_pidfile(pidfile)
> +atexit.register(remove_pidfile, pidfile)
> +
> +
> +def process(data):
> +try:
> +return binascii.hexlify(raw(eval(data)))
> +except Exception:
> +return ""
> +
> +
> +def handle_client(client_socket):
> +data = client_socket.recv(_MAX_PATTERN_LEN)
> +
Re: [ovs-dev] [PATCH ovn v2 2/2] northd: Add a separate I-P node for handling meters.
On 8/29/23 04:49, Dumitru Ceara wrote:
On 8/21/23 19:33, Mark Michelson wrote:
Hi Dumitru,
I have one finding in the test code, but other than that it looks good.
Hi Mark,
Thanks for the review!
On 8/17/23 11:25, Dumitru Ceara wrote:
This is beneficial in a few ways:
- first, it reduces the number of different types of data the northd
I-P node has to process.
- it turns out the northd I-P node (whose recompute is rather costly)
doesn't really depend on meters or ACLs.
- prepares the ground for a pure I-P implementation for ACLs, with a
simple/clear dependency between NB.ACL and the lflow I-P node.
From a meters synchronization perspective this commit doesn't change any
of the existing behavior and logic. It mostly just moves the meters
related code out of northd.c and into en-meters.c.
Signed-off-by: Dumitru Ceara
---
lib/stopwatch-names.h | 1
northd/automake.mk | 2
northd/en-lflow.c | 5 +
northd/en-meters.c | 281
++
northd/en-meters.h | 44 +++
northd/en-northd.c | 6 -
northd/inc-proc-northd.c | 14 ++
northd/northd.c | 235 +-
northd/northd.h | 4 -
northd/ovn-northd.c | 3
tests/ovn-northd.at | 36 ++
11 files changed, 390 insertions(+), 241 deletions(-)
create mode 100644 northd/en-meters.c
create mode 100644 northd/en-meters.h
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index de6fca4ccc..98535fff5a 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
#define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
#define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
+#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
#endif
diff --git a/northd/automake.mk b/northd/automake.mk
index b17f1fdb54..f52766de0c 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
northd/en-northd.h \
northd/en-lflow.c \
northd/en-lflow.h \
+ northd/en-meters.c \
+ northd/en-meters.h \
northd/en-northd-output.c \
northd/en-northd-output.h \
northd/en-sync-sb.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 28ab1c67fb..0886d4c5ca 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -20,6 +20,7 @@
#include "en-lflow.h"
#include "en-northd.h"
+#include "en-meters.h"
#include "lib/inc-proc-eng.h"
#include "northd.h"
@@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
struct lflow_input *lflow_input)
{
struct northd_data *northd_data =
engine_get_input_data("northd", node);
+ struct sync_meters_data *sync_meters_data =
+ engine_get_input_data("sync_meters", node);
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
lflow_input->sbrec_bfd_table =
@@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->ls_ports = _data->ls_ports;
lflow_input->lr_ports = _data->lr_ports;
lflow_input->port_groups = _data->port_groups;
- lflow_input->meter_groups = _data->meter_groups;
+ lflow_input->meter_groups = _meters_data->meter_groups;
lflow_input->lbs = _data->lbs;
lflow_input->features = _data->features;
lflow_input->ovn_internal_version_changed =
diff --git a/northd/en-meters.c b/northd/en-meters.c
new file mode 100644
index 00..aabd002b62
--- /dev/null
+++ b/northd/en-meters.c
@@ -0,0 +1,281 @@
+/*
+ * Copyright (c) 2023, 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 "openvswitch/vlog.h"
+#include "stopwatch.h"
+
+#include "en-meters.h"
+#include "lib/stopwatch-names.h"
+
+VLOG_DEFINE_THIS_MODULE(en_meters);
+
+static void build_meter_groups(struct shash *meter_group,
+ const struct nbrec_meter_table *);
+static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
+ const struct nbrec_meter_table *,
+ const struct nbrec_acl_table *,
+ const struct sbrec_meter_table *,
+ struct shash *meter_groups);
+
+void
+*en_sync_meters_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg
Re: [ovs-dev] [PATCH v3 ovn] controller: make garp_max_timeout configurable
On Thu, Aug 24, 2023 at 6:36 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:
> When using VLAN backed networks and OVN routers leveraging the
> 'ovn-chassis-mac-mappings' option for east-west traffic, the eth.src field
> is
> replaced by the chassis mac address in order to not expose the router mac
> address from different nodes and confuse the TOR switch. However doing so
> the TOR switch is not able to learn the port/mac bindings for routed E/W
> traffic and it is force to always flood it. Fix this issue adding the
> capability to configure a given timeout for garp sent by ovn-controller
> and not disable it after the exponential backoff in order to keep
> refreshing the entries in TOR swtich fdb table.
> More into about the issue can be found here [0].
>
> [0]
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779
>
> Signed-off-by: Lorenzo Bianconi
> ---
>
Hi Lorenzo,
I have a couple more comments, sorry for not including those in the
previous review.
> Changes since v2:
> - cap exponential backoff timeout to garp_max_timeout
>
> Changes since v1:
> - add uni-test
> - add documentation
> ---
> controller/ovn-controller.8.xml | 11 ++
> controller/ovn-controller.c | 4 +-
> controller/pinctrl.c| 70 +++--
> controller/pinctrl.h| 4 +-
> tests/ovn.at| 16
> 5 files changed, 91 insertions(+), 14 deletions(-)
>
> diff --git a/controller/ovn-controller.8.xml
> b/controller/ovn-controller.8.xml
> index 0883d8da9..831df4481 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -365,6 +365,17 @@
> heplful to pin source outer IP for the tunnel when multiple
> interfaces
> are used on the host for overlay traffic.
>
> + external_ids:garp-max-timeout
>
nit: We should specify the unit in the name e.g. "garp-max-timeout-s".
> +
> +When used, this configuration value specifies the maximum timeout
> +(in seconds) between two consecutive GARP packets sent by
> +ovn-controller.
> +ovn-controller by default sends just 4 GARP packets
> +with an exponential backoff timeout.
> +Setting external_ids:garp-max-timeout allows to
> +cap for the exponential backoff used by
> ovn-controller
> +to send GARPs packets.
> +
>
>
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0c975dc5e..1ae2f6cbe 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -5447,7 +5447,9 @@ main(int argc, char *argv[])
> _data->local_datapaths,
> _data->active_tunnels,
>
> _data->local_active_ports_ipv6_pd,
> -
> _data->local_active_ports_ras);
> +_data->local_active_ports_ras,
> +ovsrec_open_vswitch_table_get(
> +ovs_idl_loop.idl));
> stopwatch_stop(PINCTRL_RUN_STOPWATCH_NAME,
> time_msec());
> mirror_run(ovs_idl_txn,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index bed90fe0b..850c53c58 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -165,6 +165,7 @@ VLOG_DEFINE_THIS_MODULE(pinctrl);
> static struct ovs_mutex pinctrl_mutex = OVS_MUTEX_INITIALIZER;
> static struct seq *pinctrl_handler_seq;
> static struct seq *pinctrl_main_seq;
> +static long long int garp_rarp_max_timeout = LLONG_MAX;
>
By setting the default value to 16000 (maybe even making that #define so it
is really clear) and introducing something like "static bool
garp_rarp_continuous = false;" we can simplify the code below
significantly.
>
> static void *pinctrl_handler(void *arg);
>
> @@ -227,7 +228,8 @@ static void send_garp_rarp_prepare(
> const struct ovsrec_bridge *,
> const struct sbrec_chassis *,
> const struct hmap *local_datapaths,
> -const struct sset *active_tunnels)
> +const struct sset *active_tunnels,
> +const struct ovsrec_open_vswitch_table *ovs_table)
> OVS_REQUIRES(pinctrl_mutex);
> static void send_garp_rarp_run(struct rconn *swconn,
> long long int *send_garp_rarp_time)
> @@ -3492,7 +3494,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct hmap *local_datapaths,
> const struct sset *active_tunnels,
> const struct shash *local_active_ports_ipv6_pd,
> -const struct shash *local_active_ports_ras)
> +const struct shash *local_active_ports_ras,
> +const struct ovsrec_open_vswitch_table *ovs_table)
> {
> ovs_mutex_lock(_mutex);
> run_put_mac_bindings(ovnsb_idl_txn,
Re: [ovs-dev] [PATCH ovn v2 2/2] northd: Add a separate I-P node for handling meters.
On 8/21/23 19:33, Mark Michelson wrote:
> Hi Dumitru,
>
> I have one finding in the test code, but other than that it looks good.
>
Hi Mark,
Thanks for the review!
> On 8/17/23 11:25, Dumitru Ceara wrote:
>> This is beneficial in a few ways:
>> - first, it reduces the number of different types of data the northd
>> I-P node has to process.
>> - it turns out the northd I-P node (whose recompute is rather costly)
>> doesn't really depend on meters or ACLs.
>> - prepares the ground for a pure I-P implementation for ACLs, with a
>> simple/clear dependency between NB.ACL and the lflow I-P node.
>>
>> From a meters synchronization perspective this commit doesn't change any
>> of the existing behavior and logic. It mostly just moves the meters
>> related code out of northd.c and into en-meters.c.
>>
>> Signed-off-by: Dumitru Ceara
>> ---
>> lib/stopwatch-names.h | 1
>> northd/automake.mk | 2
>> northd/en-lflow.c | 5 +
>> northd/en-meters.c | 281
>> ++
>> northd/en-meters.h | 44 +++
>> northd/en-northd.c | 6 -
>> northd/inc-proc-northd.c | 14 ++
>> northd/northd.c | 235 +-
>> northd/northd.h | 4 -
>> northd/ovn-northd.c | 3
>> tests/ovn-northd.at | 36 ++
>> 11 files changed, 390 insertions(+), 241 deletions(-)
>> create mode 100644 northd/en-meters.c
>> create mode 100644 northd/en-meters.h
>>
>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
>> index de6fca4ccc..98535fff5a 100644
>> --- a/lib/stopwatch-names.h
>> +++ b/lib/stopwatch-names.h
>> @@ -30,5 +30,6 @@
>> #define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
>> #define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
>> #define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
>> +#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>> #endif
>> diff --git a/northd/automake.mk b/northd/automake.mk
>> index b17f1fdb54..f52766de0c 100644
>> --- a/northd/automake.mk
>> +++ b/northd/automake.mk
>> @@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
>> northd/en-northd.h \
>> northd/en-lflow.c \
>> northd/en-lflow.h \
>> + northd/en-meters.c \
>> + northd/en-meters.h \
>> northd/en-northd-output.c \
>> northd/en-northd-output.h \
>> northd/en-sync-sb.c \
>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>> index 28ab1c67fb..0886d4c5ca 100644
>> --- a/northd/en-lflow.c
>> +++ b/northd/en-lflow.c
>> @@ -20,6 +20,7 @@
>> #include "en-lflow.h"
>> #include "en-northd.h"
>> +#include "en-meters.h"
>> #include "lib/inc-proc-eng.h"
>> #include "northd.h"
>> @@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
>> struct lflow_input *lflow_input)
>> {
>> struct northd_data *northd_data =
>> engine_get_input_data("northd", node);
>> + struct sync_meters_data *sync_meters_data =
>> + engine_get_input_data("sync_meters", node);
>> lflow_input->nbrec_bfd_table =
>> EN_OVSDB_GET(engine_get_input("NB_bfd", node));
>> lflow_input->sbrec_bfd_table =
>> @@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
>> lflow_input->ls_ports = _data->ls_ports;
>> lflow_input->lr_ports = _data->lr_ports;
>> lflow_input->port_groups = _data->port_groups;
>> - lflow_input->meter_groups = _data->meter_groups;
>> + lflow_input->meter_groups = _meters_data->meter_groups;
>> lflow_input->lbs = _data->lbs;
>> lflow_input->features = _data->features;
>> lflow_input->ovn_internal_version_changed =
>> diff --git a/northd/en-meters.c b/northd/en-meters.c
>> new file mode 100644
>> index 00..aabd002b62
>> --- /dev/null
>> +++ b/northd/en-meters.c
>> @@ -0,0 +1,281 @@
>> +/*
>> + * Copyright (c) 2023, 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 "openvswitch/vlog.h"
>> +#include "stopwatch.h"
>> +
>> +#include "en-meters.h"
>> +#include "lib/stopwatch-names.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(en_meters);
>> +
>> +static void build_meter_groups(struct shash *meter_group,
>> + const struct nbrec_meter_table *);
>> +static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
>> + const struct nbrec_meter_table *,
>> +
[ovs-dev] [PATCH ovn v2 2/2] ofctrl: Prevent conjunction duplication
During ofctrl_add_or_append_flow we are able to combine
two flows with same match but different conjunctions.
However, the function didn't check if the conjunctions already
exist in the installed flow, which could result in conjunction
duplication and the flow would grow infinitely e.g.
actions=conjunction(1,1/2), conjunction(1,1/2)
Make sure that we add only conjunctions that are not present
in the already existing flow.
Reported-at: https://bugzilla.redhat.com/2175928
Signed-off-by: Ales Musil
---
controller/ofctrl.c | 56 -
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 9de8a145f..e39cef7aa 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1273,6 +1273,37 @@ ofctrl_add_flow_metered(struct ovn_desired_flow_table
*desired_flows,
meter_id, as_info, true);
}
+struct ofpact_ref {
+struct hmap_node hmap_node;
+struct ofpact *ofpact;
+};
+
+static struct ofpact_ref *
+ofpact_ref_find(const struct hmap *refs, const struct ofpact *ofpact)
+{
+uint32_t hash = hash_bytes(ofpact, ofpact->len, 0);
+
+struct ofpact_ref *ref;
+HMAP_FOR_EACH_WITH_HASH (ref, hmap_node, hash, refs) {
+if (ofpacts_equal(ref->ofpact, ref->ofpact->len,
+ ofpact, ofpact->len)) {
+return ref;
+}
+}
+
+return NULL;
+}
+
+static void
+ofpact_refs_destroy(struct hmap *refs)
+{
+struct ofpact_ref *ref;
+HMAP_FOR_EACH_POP (ref, hmap_node, refs) {
+free(ref);
+}
+hmap_destroy(refs);
+}
+
/* Either add a new flow, or append actions on an existing flow. If the
* flow existed, a new link will also be created between the new sb_uuid
* and the existing flow. */
@@ -1292,6 +1323,21 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table
*desired_flows,
meter_id);
existing = desired_flow_lookup_conjunctive(desired_flows, >flow);
if (existing) {
+struct hmap existing_conj = HMAP_INITIALIZER(_conj);
+
+struct ofpact *ofpact;
+OFPACT_FOR_EACH (ofpact, existing->flow.ofpacts,
+ existing->flow.ofpacts_len) {
+if (ofpact->type != OFPACT_CONJUNCTION) {
+continue;
+}
+
+struct ofpact_ref *ref = xmalloc(sizeof *ref);
+ref->ofpact = ofpact;
+uint32_t hash = hash_bytes(ofpact, ofpact->len, 0);
+hmap_insert(_conj, >hmap_node, hash);
+}
+
/* There's already a flow with this particular match and action
* 'conjunction'. Append the action to that flow rather than
* adding a new flow.
@@ -1301,7 +1347,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table
*desired_flows,
ofpbuf_use_stub(, compound_stub, sizeof(compound_stub));
ofpbuf_put(, existing->flow.ofpacts,
existing->flow.ofpacts_len);
-ofpbuf_put(, f->flow.ofpacts, f->flow.ofpacts_len);
+
+OFPACT_FOR_EACH (ofpact, f->flow.ofpacts, f->flow.ofpacts_len) {
+if (ofpact->type != OFPACT_CONJUNCTION ||
+!ofpact_ref_find(_conj, ofpact)) {
+ofpbuf_put(, ofpact, OFPACT_ALIGN(ofpact->len));
+}
+}
+
+ofpact_refs_destroy(_conj);
mem_stats.desired_flow_usage -= desired_flow_size(existing);
free(existing->flow.ofpacts);
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 1/2] ofctrl: Do not try to program long flows
If the flow size is bigger than UINT16_MAX it doesn't
fit into openflow message. Programming of such flow will
fail which results in disconnect of ofctrl. After reconnect
we program everything from scratch, in case the long flow still
remains the cycle continues. This causes the node to be almost
unusable as there will be massive traffic disruptions.
To prevent that check if the flow is within the allowed size.
If not log the flow that would trigger this problem and do not program
it. This isn't a self-healing process, but the chance of this happening
are very slim. Also, any flow that is bigger than allowed size is OVN
bug, and it should be fixed.
Reported-at: https://bugzilla.redhat.com/1955167
Signed-off-by: Ales Musil
---
v2: Fix the formatting error.
---
controller/ofctrl.c | 40
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 64a444ff6..9de8a145f 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1769,6 +1769,16 @@ ovn_flow_log(const struct ovn_flow *f, const char
*action)
}
}
+static void
+ovn_flow_log_size_err(const struct ovn_flow *f)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+
+char *s = ovn_flow_to_string(f);
+VLOG_ERR_RL(, "The FLOW_MOD message is too big: %s", s);
+free(s);
+}
+
static void
ovn_flow_uninit(struct ovn_flow *f)
{
@@ -1888,15 +1898,27 @@ encode_bundle_add(struct ofpbuf *msg, struct
ofputil_bundle_ctrl_msg *bc)
return ofputil_encode_bundle_add(OFP15_VERSION, );
}
-static void
+static bool
add_flow_mod(struct ofputil_flow_mod *fm,
struct ofputil_bundle_ctrl_msg *bc,
struct ovs_list *msgs)
{
struct ofpbuf *msg = encode_flow_mod(fm);
struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
+
+uint32_t flow_mod_len = msg->size;
+uint32_t bundle_len = bundle_msg->size;
+
ofpbuf_delete(msg);
+
+if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
+ofpbuf_delete(bundle_msg);
+
+return false;
+}
+
ovs_list_push_back(msgs, _msg->list_node);
+return true;
}
/* group_table. */
@@ -2235,7 +2257,10 @@ installed_flow_add(struct ovn_flow *d,
.new_cookie = htonll(d->cookie),
.command = OFPFC_ADD,
};
-add_flow_mod(, bc, msgs);
+
+if (!add_flow_mod(, bc, msgs)) {
+ovn_flow_log_size_err(d);
+}
}
static void
@@ -2259,7 +2284,7 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow *d,
/* Use OFPFC_ADD so that cookie can be updated. */
fm.command = OFPFC_ADD;
}
-add_flow_mod(, bc, msgs);
+bool result = add_flow_mod(, bc, msgs);
/* Replace 'i''s actions and cookie by 'd''s. */
mem_stats.installed_flow_usage -= i->ofpacts_len - d->ofpacts_len;
@@ -2267,6 +2292,10 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow
*d,
i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len);
i->ofpacts_len = d->ofpacts_len;
i->cookie = d->cookie;
+
+if (!result) {
+ovn_flow_log_size_err(i);
+}
}
static void
@@ -2280,7 +2309,10 @@ installed_flow_del(struct ovn_flow *i,
.table_id = i->table_id,
.command = OFPFC_DELETE_STRICT,
};
-add_flow_mod(, bc, msgs);
+
+if (!add_flow_mod(, bc, msgs)) {
+ovn_flow_log_size_err(i);
+}
}
static void
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 1/2] ofctrl: Do not try to program long flows
On Tue, Aug 29, 2023 at 10:42 AM Ales Musil wrote:
> If the flow size is bigger than UINT16_MAX it doesn't
> fit into openflow message. Programming of such flow will
> fail which results in disconnect of ofctrl. After reconnect
> we program everything from scratch, in case the long flow still
> remains the cycle continues. This causes the node to be almost
> unusable as there will be massive traffic disruptions.
>
> To prevent that check if the flow is within the allowed size.
> If not log the flow that would trigger this problem and do not program
> it. This isn't a self-healing process, but the chance of this happening
> are very slim. Also, any flow that is bigger than allowed size is OVN
> bug, and it should be fixed.
>
> Reported-at: https://bugzilla.redhat.com/1955167
> Signed-off-by: Ales Musil
> ---
> controller/ofctrl.c | 74 -
> 1 file changed, 53 insertions(+), 21 deletions(-)
>
> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> index 64a444ff6..bb42d474f 100644
> --- a/controller/ofctrl.c
> +++ b/controller/ofctrl.c
> @@ -1769,6 +1769,16 @@ ovn_flow_log(const struct ovn_flow *f, const char
> *action)
> }
> }
>
> +static void
> +ovn_flow_log_size_err(const struct ovn_flow *f)
> +{
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +
> +char *s = ovn_flow_to_string(f);
> +VLOG_ERR_RL(, "The FLOW_MOD message is too big: %s", s);
> +free(s);
> +}
> +
> static void
> ovn_flow_uninit(struct ovn_flow *f)
> {
> @@ -1888,15 +1898,27 @@ encode_bundle_add(struct ofpbuf *msg, struct
> ofputil_bundle_ctrl_msg *bc)
> return ofputil_encode_bundle_add(OFP15_VERSION, );
> }
>
> -static void
> +static bool
> add_flow_mod(struct ofputil_flow_mod *fm,
> struct ofputil_bundle_ctrl_msg *bc,
> struct ovs_list *msgs)
> {
> struct ofpbuf *msg = encode_flow_mod(fm);
> struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
> +
> +uint32_t flow_mod_len = msg->size;
> +uint32_t bundle_len = bundle_msg->size;
> +
> ofpbuf_delete(msg);
> +
> +if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
> +ofpbuf_delete(bundle_msg);
> +
> +return false;
> +}
> +
> ovs_list_push_back(msgs, _msg->list_node);
> +return true;
> }
>
> /* group_table. */
> @@ -2227,15 +2249,18 @@ installed_flow_add(struct ovn_flow *d,
> {
> /* Send flow_mod to add flow. */
> struct ofputil_flow_mod fm = {
> -.match = d->match,
> -.priority = d->priority,
> -.table_id = d->table_id,
> -.ofpacts = d->ofpacts,
> -.ofpacts_len = d->ofpacts_len,
> -.new_cookie = htonll(d->cookie),
> -.command = OFPFC_ADD,
> +.match = d->match,
> +.priority = d->priority,
> +.table_id = d->table_id,
> +.ofpacts = d->ofpacts,
> +.ofpacts_len = d->ofpacts_len,
> +.new_cookie = htonll(d->cookie),
> +.command = OFPFC_ADD,
> };
> -add_flow_mod(, bc, msgs);
> +
> +if (!add_flow_mod(, bc, msgs)) {
> +ovn_flow_log_size_err(d);
> +}
> }
>
> static void
> @@ -2245,12 +2270,12 @@ installed_flow_mod(struct ovn_flow *i, struct
> ovn_flow *d,
> {
> /* Update actions in installed flow. */
> struct ofputil_flow_mod fm = {
> -.match = i->match,
> -.priority = i->priority,
> -.table_id = i->table_id,
> -.ofpacts = d->ofpacts,
> -.ofpacts_len = d->ofpacts_len,
> -.command = OFPFC_MODIFY_STRICT,
> +.match = i->match,
> +.priority = i->priority,
> +.table_id = i->table_id,
> +.ofpacts = d->ofpacts,
> +.ofpacts_len = d->ofpacts_len,
> +.command = OFPFC_MODIFY_STRICT,
> };
> /* Update cookie if it is changed. */
> if (i->cookie != d->cookie) {
> @@ -2259,7 +2284,7 @@ installed_flow_mod(struct ovn_flow *i, struct
> ovn_flow *d,
> /* Use OFPFC_ADD so that cookie can be updated. */
> fm.command = OFPFC_ADD;
> }
> -add_flow_mod(, bc, msgs);
> +bool result = add_flow_mod(, bc, msgs);
>
> /* Replace 'i''s actions and cookie by 'd''s. */
> mem_stats.installed_flow_usage -= i->ofpacts_len - d->ofpacts_len;
> @@ -2267,6 +2292,10 @@ installed_flow_mod(struct ovn_flow *i, struct
> ovn_flow *d,
> i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len);
> i->ofpacts_len = d->ofpacts_len;
> i->cookie = d->cookie;
> +
> +if (!result) {
> +ovn_flow_log_size_err(i);
> +}
> }
>
> static void
> @@ -2275,12 +2304,15 @@ installed_flow_del(struct ovn_flow *i,
> struct ovs_list *msgs)
> {
> struct ofputil_flow_mod fm = {
> -.match = i->match,
> -.priority = i->priority,
> -.table_id = i->table_id,
> -.command = OFPFC_DELETE_STRICT,
> +.match = i->match,
> +
[ovs-dev] [PATCH ovn 2/2] ofctrl: Prevent conjunction duplication
During ofctrl_add_or_append_flow we are able to combine
two flows with same match but different conjunctions.
However, the function didn't check if the conjunctions already
exist in the installed flow, which could result in conjunction
duplication and the flow would grow infinitely e.g.
actions=conjunction(1,1/2), conjunction(1,1/2)
Make sure that we add only conjunctions that are not present
in the already existing flow.
Reported-at: https://bugzilla.redhat.com/2175928
Signed-off-by: Ales Musil
---
controller/ofctrl.c | 56 -
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index bb42d474f..e4c99117f 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1273,6 +1273,37 @@ ofctrl_add_flow_metered(struct ovn_desired_flow_table
*desired_flows,
meter_id, as_info, true);
}
+struct ofpact_ref {
+struct hmap_node hmap_node;
+struct ofpact *ofpact;
+};
+
+static struct ofpact_ref *
+ofpact_ref_find(const struct hmap *refs, const struct ofpact *ofpact)
+{
+uint32_t hash = hash_bytes(ofpact, ofpact->len, 0);
+
+struct ofpact_ref *ref;
+HMAP_FOR_EACH_WITH_HASH (ref, hmap_node, hash, refs) {
+if (ofpacts_equal(ref->ofpact, ref->ofpact->len,
+ ofpact, ofpact->len)) {
+return ref;
+}
+}
+
+return NULL;
+}
+
+static void
+ofpact_refs_destroy(struct hmap *refs)
+{
+struct ofpact_ref *ref;
+HMAP_FOR_EACH_POP (ref, hmap_node, refs) {
+free(ref);
+}
+hmap_destroy(refs);
+}
+
/* Either add a new flow, or append actions on an existing flow. If the
* flow existed, a new link will also be created between the new sb_uuid
* and the existing flow. */
@@ -1292,6 +1323,21 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table
*desired_flows,
meter_id);
existing = desired_flow_lookup_conjunctive(desired_flows, >flow);
if (existing) {
+struct hmap existing_conj = HMAP_INITIALIZER(_conj);
+
+struct ofpact *ofpact;
+OFPACT_FOR_EACH (ofpact, existing->flow.ofpacts,
+ existing->flow.ofpacts_len) {
+if (ofpact->type != OFPACT_CONJUNCTION) {
+continue;
+}
+
+struct ofpact_ref *ref = xmalloc(sizeof *ref);
+ref->ofpact = ofpact;
+uint32_t hash = hash_bytes(ofpact, ofpact->len, 0);
+hmap_insert(_conj, >hmap_node, hash);
+}
+
/* There's already a flow with this particular match and action
* 'conjunction'. Append the action to that flow rather than
* adding a new flow.
@@ -1301,7 +1347,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table
*desired_flows,
ofpbuf_use_stub(, compound_stub, sizeof(compound_stub));
ofpbuf_put(, existing->flow.ofpacts,
existing->flow.ofpacts_len);
-ofpbuf_put(, f->flow.ofpacts, f->flow.ofpacts_len);
+
+OFPACT_FOR_EACH (ofpact, f->flow.ofpacts, f->flow.ofpacts_len) {
+if (ofpact->type != OFPACT_CONJUNCTION ||
+!ofpact_ref_find(_conj, ofpact)) {
+ofpbuf_put(, ofpact, OFPACT_ALIGN(ofpact->len));
+}
+}
+
+ofpact_refs_destroy(_conj);
mem_stats.desired_flow_usage -= desired_flow_size(existing);
free(existing->flow.ofpacts);
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 1/2] ofctrl: Do not try to program long flows
If the flow size is bigger than UINT16_MAX it doesn't
fit into openflow message. Programming of such flow will
fail which results in disconnect of ofctrl. After reconnect
we program everything from scratch, in case the long flow still
remains the cycle continues. This causes the node to be almost
unusable as there will be massive traffic disruptions.
To prevent that check if the flow is within the allowed size.
If not log the flow that would trigger this problem and do not program
it. This isn't a self-healing process, but the chance of this happening
are very slim. Also, any flow that is bigger than allowed size is OVN
bug, and it should be fixed.
Reported-at: https://bugzilla.redhat.com/1955167
Signed-off-by: Ales Musil
---
controller/ofctrl.c | 74 -
1 file changed, 53 insertions(+), 21 deletions(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 64a444ff6..bb42d474f 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1769,6 +1769,16 @@ ovn_flow_log(const struct ovn_flow *f, const char
*action)
}
}
+static void
+ovn_flow_log_size_err(const struct ovn_flow *f)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+
+char *s = ovn_flow_to_string(f);
+VLOG_ERR_RL(, "The FLOW_MOD message is too big: %s", s);
+free(s);
+}
+
static void
ovn_flow_uninit(struct ovn_flow *f)
{
@@ -1888,15 +1898,27 @@ encode_bundle_add(struct ofpbuf *msg, struct
ofputil_bundle_ctrl_msg *bc)
return ofputil_encode_bundle_add(OFP15_VERSION, );
}
-static void
+static bool
add_flow_mod(struct ofputil_flow_mod *fm,
struct ofputil_bundle_ctrl_msg *bc,
struct ovs_list *msgs)
{
struct ofpbuf *msg = encode_flow_mod(fm);
struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
+
+uint32_t flow_mod_len = msg->size;
+uint32_t bundle_len = bundle_msg->size;
+
ofpbuf_delete(msg);
+
+if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
+ofpbuf_delete(bundle_msg);
+
+return false;
+}
+
ovs_list_push_back(msgs, _msg->list_node);
+return true;
}
/* group_table. */
@@ -2227,15 +2249,18 @@ installed_flow_add(struct ovn_flow *d,
{
/* Send flow_mod to add flow. */
struct ofputil_flow_mod fm = {
-.match = d->match,
-.priority = d->priority,
-.table_id = d->table_id,
-.ofpacts = d->ofpacts,
-.ofpacts_len = d->ofpacts_len,
-.new_cookie = htonll(d->cookie),
-.command = OFPFC_ADD,
+.match = d->match,
+.priority = d->priority,
+.table_id = d->table_id,
+.ofpacts = d->ofpacts,
+.ofpacts_len = d->ofpacts_len,
+.new_cookie = htonll(d->cookie),
+.command = OFPFC_ADD,
};
-add_flow_mod(, bc, msgs);
+
+if (!add_flow_mod(, bc, msgs)) {
+ovn_flow_log_size_err(d);
+}
}
static void
@@ -2245,12 +2270,12 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow
*d,
{
/* Update actions in installed flow. */
struct ofputil_flow_mod fm = {
-.match = i->match,
-.priority = i->priority,
-.table_id = i->table_id,
-.ofpacts = d->ofpacts,
-.ofpacts_len = d->ofpacts_len,
-.command = OFPFC_MODIFY_STRICT,
+.match = i->match,
+.priority = i->priority,
+.table_id = i->table_id,
+.ofpacts = d->ofpacts,
+.ofpacts_len = d->ofpacts_len,
+.command = OFPFC_MODIFY_STRICT,
};
/* Update cookie if it is changed. */
if (i->cookie != d->cookie) {
@@ -2259,7 +2284,7 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow *d,
/* Use OFPFC_ADD so that cookie can be updated. */
fm.command = OFPFC_ADD;
}
-add_flow_mod(, bc, msgs);
+bool result = add_flow_mod(, bc, msgs);
/* Replace 'i''s actions and cookie by 'd''s. */
mem_stats.installed_flow_usage -= i->ofpacts_len - d->ofpacts_len;
@@ -2267,6 +2292,10 @@ installed_flow_mod(struct ovn_flow *i, struct ovn_flow
*d,
i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len);
i->ofpacts_len = d->ofpacts_len;
i->cookie = d->cookie;
+
+if (!result) {
+ovn_flow_log_size_err(i);
+}
}
static void
@@ -2275,12 +2304,15 @@ installed_flow_del(struct ovn_flow *i,
struct ovs_list *msgs)
{
struct ofputil_flow_mod fm = {
-.match = i->match,
-.priority = i->priority,
-.table_id = i->table_id,
-.command = OFPFC_DELETE_STRICT,
+.match = i->match,
+.priority = i->priority,
+.table_id = i->table_id,
+.command = OFPFC_DELETE_STRICT,
};
-add_flow_mod(, bc, msgs);
+
+if (!add_flow_mod(, bc, msgs)) {
+ovn_flow_log_size_err(i);
+}
}
static void
--
2.41.0
___
dev mailing list
Re: [ovs-dev] [PATCH v3 2/2] netdev-tc-offload: Add IPIP/GRE protocols to offload in ip rewrite
>This looks strange to me. Was it intentional?No. So sorry for this typo.BTW, >v4 patches are sent. Any suggestions about the commit message of 2/2? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
