Re: [ovs-dev] [PATCH ovn v3] controller: Track individual address set constants.

[Han Zhou]

On Tue, Apr 30, 2024 at 9:56 AM Ales Musil  wrote:
>
> Instead of tracking address set per struct expr_constant_set track it
> per individual struct expr_constant. This allows more fine grained
> control for I-P processing of address sets in controller. It helps with
> scenarios like matching on two address sets in one expression e.g.
> "ip4.src == {$as1, $as2}". This allows any addition or removal of
> individual adress from the set to be incrementally processed instead
> of reprocessing all the flows.
>
> This unfortunately doesn't help with the following flows:
> "ip4.src == $as1 && ip4.dst == $as2"
> "ip4.src == $as1 || ip4.dst == $as2"
>
> The memory impact should be minimal as there is only increase of 8 bytes
> per the struct expr_constant.
>
> Reported-at: https://issues.redhat.com/browse/FDP-509
> Signed-off-by: Ales Musil 
> ---
> v3: Rebase on top of current main.
> Address comments from Han:
> - Adjust the comment for "lflow_handle_addr_set_update" to include
remaning corner cases.
> - Make sure that the flows are consistent between I-P and recompute.
> v2: Rebase on top of current main.
> Adjust the comment for I-P optimization.
> ---
>  controller/lflow.c  |  7 ++-
>  include/ovn/actions.h   |  2 +-
>  include/ovn/expr.h  | 46 ++-
>  lib/actions.c   | 20 -
>  lib/expr.c  | 99 +
>  tests/ovn-controller.at | 79 +---
>  6 files changed, 153 insertions(+), 100 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 760ec0b41..06e839cbe 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -278,7 +278,7 @@ lflow_handle_changed_flows(struct lflow_ctx_in
*l_ctx_in,
>  }
>
>  static bool
> -as_info_from_expr_const(const char *as_name, const union expr_constant
*c,
> +as_info_from_expr_const(const char *as_name, const struct expr_constant
*c,
>  struct addrset_info *as_info)
>  {
>  as_info->name = as_name;
> @@ -647,9 +647,8 @@ as_update_can_be_handled(const char *as_name, struct
addr_set_diff *as_diff,
>   *expressions/constants, usually because of disjunctions between
>   *sub-expressions/constants, e.g.:
>   *
> + *  ip.src == $as1 && ip.dst == $as2
>   *  ip.src == $as1 || ip.dst == $as2
> - *  ip.src == {$as1, $as2}
> - *  ip.src == {$as1, ip1}
>   *
>   *All these could have been split into separate lflows.

Hi Ales, thanks for v3.

I checked again and wondered why you mentioned that "ip.src == $as1 &&
ip.dst == $as2" is not supported. This expression would generate
conjunctions, which works with I-P before your change and still works. Did
I miss anything?

In addition, since the constraints are relaxed after your change, I'd also
update the above comments a little more, something like:

   *  - The sub expression of the address set is combined with other
sub-
   *expressions/constants on different fields, e.g.:


   *




   *  ip.src == $as1 || ip.dst == $as2

   *

   *This could have been split into separate lflows.


What do you think?

Thanks,
Han

>   *
> @@ -714,7 +713,7 @@ lflow_handle_addr_set_update(const char *as_name,
>  if (as_diff->deleted) {
>  struct addrset_info as_info;
>  for (size_t i = 0; i < as_diff->deleted->n_values; i++) {
> -union expr_constant *c = _diff->deleted->values[i];
> +struct expr_constant *c = _diff->deleted->values[i];
>  if (!as_info_from_expr_const(as_name, c, _info)) {
>  continue;
>  }
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index ae0864fdd..88cf4de79 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -241,7 +241,7 @@ struct ovnact_next {
>  struct ovnact_load {
>  struct ovnact ovnact;
>  struct expr_field dst;
> -union expr_constant imm;
> +struct expr_constant imm;
>  };
>
>  /* OVNACT_MOVE, OVNACT_EXCHANGE. */
> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> index c48f82398..e54edb5bf 100644
> --- a/include/ovn/expr.h
> +++ b/include/ovn/expr.h
> @@ -368,7 +368,7 @@ bool expr_relop_from_token(enum lex_type type, enum
expr_relop *relop);
>  struct expr {
>  struct ovs_list node;   /* In parent EXPR_T_AND or EXPR_T_OR if
any. */
>  enum expr_type type;/* Expression type. */
> -char *as_name;  /* Address set name. Null if it is not an
> +const char *as_name;/* Address set name. Null if it is not an
> address set. */
>
>  union {
> @@ -505,40 +505,42 @@ enum expr_constant_type {
>  };
>
>  /* A string or integer constant (one must know which from context). */
> -union expr_constant {
> -/* Integer constant.
> - *
> - * The width of a constant isn't always clear, e.g. if you write "1",
> - * 

[ovs-dev] [PATCH v9 1/2] ofproto-dpif-mirror: Reduce number of function parameters.

[Mike Pattrick]

Previously the mirror_set() and mirror_get() functions took a large
number of parameters, which was inefficient and difficult to read and
extend. This patch moves most of the parameters into a struct.

Signed-off-by: Mike Pattrick 
Acked-by: Simon Horman 
Acked-by: Eelco Chaudron 
Signed-off-by: Mike Pattrick 
---
 ofproto/ofproto-dpif-mirror.c | 60 ++-
 ofproto/ofproto-dpif-mirror.h | 40 ++-
 ofproto/ofproto-dpif-xlate.c  | 29 -
 ofproto/ofproto-dpif.c| 23 +++---
 4 files changed, 88 insertions(+), 64 deletions(-)

diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..4967ecc9a 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -207,19 +207,22 @@ mirror_bundle_dst(struct mbridge *mbridge, struct 
ofbundle *ofbundle)
 }
 
 int
-mirror_set(struct mbridge *mbridge, void *aux, const char *name,
-   struct ofbundle **srcs, size_t n_srcs,
-   struct ofbundle **dsts, size_t n_dsts,
-   unsigned long *src_vlans, struct ofbundle *out_bundle,
-   uint16_t snaplen,
-   uint16_t out_vlan)
+mirror_set(struct mbridge *mbridge, void *aux,
+   const struct ofproto_mirror_settings *ms,
+   const struct mirror_bundles *mb)
 {
 struct mbundle *mbundle, *out;
 mirror_mask_t mirror_bit;
 struct mirror *mirror;
 struct hmapx srcs_map;  /* Contains "struct ofbundle *"s. */
 struct hmapx dsts_map;  /* Contains "struct ofbundle *"s. */
+uint16_t out_vlan;
 
+if (!ms || !mbridge) {
+return EINVAL;
+}
+
+out_vlan = ms->out_vlan;
 mirror = mirror_lookup(mbridge, aux);
 if (!mirror) {
 int idx;
@@ -227,7 +230,7 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 idx = mirror_scan(mbridge);
 if (idx < 0) {
 VLOG_WARN("maximum of %d port mirrors reached, cannot create %s",
-  MAX_MIRRORS, name);
+  MAX_MIRRORS, ms->name);
 return EFBIG;
 }
 
@@ -242,8 +245,8 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 unsigned long *vlans = ovsrcu_get(unsigned long *, >vlans);
 
 /* Get the new configuration. */
-if (out_bundle) {
-out = mbundle_lookup(mbridge, out_bundle);
+if (mb->out_bundle) {
+out = mbundle_lookup(mbridge, mb->out_bundle);
 if (!out) {
 mirror_destroy(mbridge, mirror->aux);
 return EINVAL;
@@ -252,16 +255,16 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 } else {
 out = NULL;
 }
-mbundle_lookup_multiple(mbridge, srcs, n_srcs, _map);
-mbundle_lookup_multiple(mbridge, dsts, n_dsts, _map);
+mbundle_lookup_multiple(mbridge, mb->srcs, mb->n_srcs, _map);
+mbundle_lookup_multiple(mbridge, mb->dsts, mb->n_dsts, _map);
 
 /* If the configuration has not changed, do nothing. */
 if (hmapx_equals(_map, >srcs)
 && hmapx_equals(_map, >dsts)
-&& vlan_bitmap_equal(vlans, src_vlans)
+&& vlan_bitmap_equal(vlans, ms->src_vlans)
 && mirror->out == out
 && mirror->out_vlan == out_vlan
-&& mirror->snaplen == snaplen)
+&& mirror->snaplen == ms->snaplen)
 {
 hmapx_destroy(_map);
 hmapx_destroy(_map);
@@ -275,15 +278,15 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 hmapx_swap(_map, >dsts);
 hmapx_destroy(_map);
 
-if (vlans || src_vlans) {
+if (vlans || ms->src_vlans) {
 ovsrcu_postpone(free, vlans);
-vlans = vlan_bitmap_clone(src_vlans);
+vlans = vlan_bitmap_clone(ms->src_vlans);
 ovsrcu_set(>vlans, vlans);
 }
 
 mirror->out = out;
 mirror->out_vlan = out_vlan;
-mirror->snaplen = snaplen;
+mirror->snaplen = ms->snaplen;
 
 /* Update mbundles. */
 mirror_bit = MIRROR_MASK_C(1) << mirror->idx;
@@ -406,23 +409,22 @@ mirror_update_stats(struct mbridge *mbridge, 
mirror_mask_t mirrors,
 /* Retrieves the mirror numbered 'index' in 'mbridge'.  Returns true if such a
  * mirror exists, false otherwise.
  *
- * If successful, '*vlans' receives the mirror's VLAN membership information,
+ * If successful 'mc->vlans' receives the mirror's VLAN membership information,
  * either a null pointer if the mirror includes all VLANs or a 4096-bit bitmap
  * in which a 1-bit indicates that the mirror includes a particular VLAN,
- * '*dup_mirrors' receives a bitmap of mirrors whose output duplicates mirror
- * 'index', '*out' receives the output ofbundle (if any), and '*out_vlan'
- * receives the output VLAN (if any).
+ * 'mc->dup_mirrors' receives a bitmap of mirrors whose output duplicates
+ * mirror 'index', 'mc->out' receives the output ofbundle (if any),
+ * and 'mc->out_vlan' receives the output VLAN (if any).
  *
  * Everything returned here is assumed to be RCU 

[ovs-dev] [PATCH v9 2/2] ofproto-dpif-mirror: Add support for pre-selection filter.

[Mike Pattrick]

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.

Signed-off-by: Mike Pattrick 
---
v8:
 - Corrected code from v7 related to sequence and in_port. Mirrors
   reject filters with an in_port set as this could cause confusion.
 - Combined ovsrcu pointers into a new struct, minimatch wasn't used
   because the minimatch_* functions didn't fit the usage here.
 - Added a test to check for modifying filters when partially
   overlapping flows already exist.
 - Corrected documentation.
v9:
 - Explicitly cleared mirror_config.filter* when not set
---
 Documentation/ref/ovs-tcpdump.8.rst |   8 +-
 NEWS|   6 +
 lib/flow.h  |   9 ++
 ofproto/ofproto-dpif-mirror.c   | 104 +-
 ofproto/ofproto-dpif-mirror.h   |   9 +-
 ofproto/ofproto-dpif-xlate.c|  15 ++-
 ofproto/ofproto-dpif.c  |  12 +-
 ofproto/ofproto.h   |   3 +
 tests/ofproto-dpif.at   | 165 
 utilities/ovs-tcpdump.in|  13 ++-
 vswitchd/bridge.c   |  13 ++-
 vswitchd/vswitch.ovsschema  |   7 +-
 vswitchd/vswitch.xml|  16 +++
 13 files changed, 365 insertions(+), 15 deletions(-)

diff --git a/Documentation/ref/ovs-tcpdump.8.rst 
b/Documentation/ref/ovs-tcpdump.8.rst
index b9f8cdf6f..e21e61211 100644
--- a/Documentation/ref/ovs-tcpdump.8.rst
+++ b/Documentation/ref/ovs-tcpdump.8.rst
@@ -61,8 +61,14 @@ Options
 
   If specified, mirror all ports (optional).
 
+* ``--filter ``
+
+  If specified, only mirror flows that match the provided OpenFlow filter.
+  The available fields are documented in ``ovs-fields(7)``.
+
 See Also
 
 
 ``ovs-appctl(8)``, ``ovs-vswitchd(8)``, ``ovs-pcap(1)``,
-``ovs-tcpundump(1)``, ``tcpdump(8)``, ``wireshark(8)``.
+``ovs-fields(7)``, ``ovs-tcpundump(1)``, ``tcpdump(8)``,
+``wireshark(8)``.
diff --git a/NEWS b/NEWS
index b92cec532..f3a4bf076 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,12 @@ Post-v3.3.0
- The primary development branch has been renamed from 'master' to 'main'.
  The OVS tree remains hosted on GitHub.
  https://github.com/openvswitch/ovs.git
+   - ovs-vsctl:
+ * Added a new filter column in the Mirror table which can be used to
+   apply filters to mirror ports.
+   - ovs-tcpdump:
+ * Added command line parameter --filter to enable filtering the flows
+   that are captured by tcpdump.
 
 
 v3.3.0 - 16 Feb 2024
diff --git a/lib/flow.h b/lib/flow.h
index 75a9be3c1..60ec4b0d7 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -939,6 +939,15 @@ 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 4967ecc9a..6d89d13a5 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"
 
@@ -48,6 +49,11 @@ struct mbundle {
 mirror_mask_t mirror_out;   /* Mirrors that output to this mbundle. */
 };
 
+struct filtermask {
+struct miniflow *flow;
+struct minimask *mask;
+};
+
 struct mirror {
 struct mbridge *mbridge;/* Owning ofproto. */
 size_t idx; /* In ofproto's "mirrors" array. */
@@ -57,6 +63,10 @@ struct mirror {
 struct hmapx srcs;  /* Contains "struct mbundle*"s. */
 struct hmapx dsts;  /* Contains "struct mbundle*"s. */
 
+/* Filter criteria. */
+OVSRCU_TYPE(struct filtermask *) filter_mask;
+char *filter_str;
+
 /* This is accessed by handler threads assuming RCU protection (see
  * mirror_get()), but can be manipulated by mirror_set() without any
  * explicit synchronization. */
@@ -83,6 +93,23 @@ static void mbundle_lookup_multiple(const struct mbridge *, 
struct ofbundle **,
 static int mirror_scan(struct mbridge *);
 static void 

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

[Finn, Emma]

> -Original Message-
> From: Eelco Chaudron 
> Sent: Wednesday, May 1, 2024 1:52 PM
> To: Simon Horman 
> Cc: Finn, Emma ; Stokes, Ian ;
> sunil.pa...@intel.com; Van Haaren, Harry ;
> d...@openvswitch.org; Flavio Leitner ; Ilya Maximets
> 
> Subject: Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header
> modification on ip fragments.
> 
> 
> 
> On 1 May 2024, at 14:39, Simon Horman wrote:
> 
> > On Tue, Apr 30, 2024 at 02:42:45PM +0200, Eelco Chaudron wrote:
> >> Greetings, Intel team!
> >>
> >> The self-test conducted as part of this patch has revealed an issue with 
> >> the
> AVX512 checksumming code. Since it was agreed upon that your team would
> maintain this code upon its inclusion, could you please review the problem
> and provide a patch?
> >>
> >> Details on the problem can be found in this mail link:
> >>
> >>  https://mail.openvswitch.org/pipermail/ovs-build/2024-
> April/038590.html
> >
> > Thanks Eelco,
> >
> > In light of the above, could you clarify your plans for this patch?
> 
> I hope Intel keeps their promise and will have a patch out soon, so we can
> apply this patch.
> 
> If not, I guess I can send out a patch to disable pedit acceleration as it’s
> broken, and then apply this patch.
> 
> Intel are you looking into this?
> 
> Cheers,
> 
> Eelco

Hi Folks,

I'll look into this and try reproduce locally myself.
Will reach out when I have an update. 

Thanks, 
Emma  


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

[Eelco Chaudron]

On 1 May 2024, at 14:39, Simon Horman wrote:

> On Tue, Apr 30, 2024 at 02:42:45PM +0200, Eelco Chaudron wrote:
>> Greetings, Intel team!
>>
>> The self-test conducted as part of this patch has revealed an issue with the 
>> AVX512 checksumming code. Since it was agreed upon that your team would 
>> maintain this code upon its inclusion, could you please review the problem 
>> and provide a patch?
>>
>> Details on the problem can be found in this mail link:
>>
>>  https://mail.openvswitch.org/pipermail/ovs-build/2024-April/038590.html
>
> Thanks Eelco,
>
> In light of the above, could you clarify your plans for this patch?

I hope Intel keeps their promise and will have a patch out soon, so we can 
apply this patch.

If not, I guess I can send out a patch to disable pedit acceleration as it’s 
broken, and then apply this patch.

Intel are you looking into this?

Cheers,

Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [Patch] ovsdb-client: Add missing arg to help for 'dump'.

[Simon Horman]

On Wed, May 01, 2024 at 01:10:43PM +0200, Martin Kalcok wrote:
> Help text for 'ovsdb-client dump' does not mention that it's capable
> of dumping specific table's contents if user supplies table's name
> as a third positional argument.
> 
> Signed-off-by: Martin Kalcok 

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC] route-table: Add support for v4 via v6 route.

[Simon Horman]

On Wed, May 01, 2024 at 11:59:02AM +0100, Simon Horman wrote:
> On Wed, May 01, 2024 at 11:57:46AM +0100, Simon Horman wrote:
> > On Fri, Apr 26, 2024 at 10:16:31PM -0700, William Tu via dev wrote:
> > > Add route-table that support ipv4 dst via ipv6. BGP unnumbered is 
> > > mechanism
> > > that allows BGP to establish peering sessions without the need to 
> > > explicitly
> > > configure IP addresses on the interfaces involved in the peering. Without
> > > using IP address assignments, it uses link-local IPv6 addresses of the
> > > directly connected neighbors for peering purposes. For example, BGP
> > > might install the following route:
> > > $ ip route get 100.87.18.3
> > >  100.87.18.3 via inet6 fe80::920a:84ff:fe9e:9570 dev br-phy src 
> > > 100.87.18.6
> > > 
> > > Currently OVS can only support either all-ipv4 or all-ipv6, the patch
> > > adds support for such use case.
> > > 
> > > Reported-at: 
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-January/052908.html
> > > Signed-off-by: William Tu 
> > 
> > Acked-by: Simon Horman 
> 
> Just to clarify: this patch looks good to me.
> But I suggest resubmit it as a non-RFC when you are ready.

The comments above were supposed to be responses to v2.
Oops.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

[Simon Horman]

On Mon, Apr 29, 2024 at 04:48:41PM +0200, Eelco Chaudron wrote:
> While offloading header modifications to TC, OVS is using {TCA_PEDIT} +
> {TCA_CSUM} combination as that it the only way to represent header
> rewrite.  However, {TCA_CSUM} is unable to calculate L4 checksums for
> IP fragments.
> 
> Since TC already applies fragmentation bit masking, this patch simply
> needs to prevent these packets from being processed through TC.
> 
> Signed-off-by: Eelco Chaudron 

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

[Simon Horman]

On Tue, Apr 30, 2024 at 02:42:45PM +0200, Eelco Chaudron wrote:
> Greetings, Intel team!
> 
> The self-test conducted as part of this patch has revealed an issue with the 
> AVX512 checksumming code. Since it was agreed upon that your team would 
> maintain this code upon its inclusion, could you please review the problem 
> and provide a patch?
> 
> Details on the problem can be found in this mail link:
> 
>   https://mail.openvswitch.org/pipermail/ovs-build/2024-April/038590.html

Thanks Eelco,

In light of the above, could you clarify your plans for this patch?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ofproto-dpif-rid: Fix duplicate entries.

[Simon Horman]

On Tue, Apr 30, 2024 at 05:36:09PM +0800, wushao...@chinatelecom.cn wrote:
> From: Shaohua Wu 
> 
> In scenarios with multiple PMDs, there may be
> simultaneous requests for recirc_id from multiple
> PMD threads.In recirc_alloc_id_ctx, we first check
> if there is a duplicate entry in the metadata_map
> for the same frozen_state field. If successful,
> we directly retrieve the recirc_id. If unsuccessful,
> we create a new recirc_node and insert it into
> id_map and metadata_map. There is no locking mechanism
> to prevent the possibility of two threads with the same
> state simultaneously inserting, meaning their IDs are
> different, but their frozen_states are the same.

Hi Shaohua Wu,

It seems that recirc_alloc_id_ctx() already contains
logic to check for an existing entry found
by recirc_ref_equal(). Is the problem that this is
not inside the critical section protected by mutex?.

If so:
1. The check in recirc_alloc_id_ctx should be removed.
2. What are the implications for recirc_alloc_id__(),
   the other caller of recirc_alloc_id__()?
   Does it also have the problem you describe?
   Although I don't think there are any correctness issues,
   could the logic added to recirc_alloc_id__() have some
   impact on recirc_alloc_id(), such as performance?

> 
> trace log:
> static struct recirc_id_node *
> recirc_alloc_id__(const struct frozen_state *state, uint32_t hash)
> {
> ovs_assert(state->action_set_len <= state->ofpacts_len);
> 
> struct recirc_id_node *node = xzalloc(sizeof *node);
> 
> node->hash = hash;
> ovs_refcount_init(>refcount);
> node->is_hotupgrade = false;
> frozen_state_clone(CONST_CAST(struct frozen_state *, >state), 
> state);
> struct recirc_id_node *hash_recirc_node = recirc_find_equal(>state, 
> state);
> if (hash_recirc_node) {
> VLOG_INFO("wsh:hash equal:hash_recirc_node %p id:%u, hash_recirc_node 
> hash:%u,node %p hash:%u\n",
>hash_recirc_node， hash_recirc_node->id, 
> hash_recirc_node->hash, node, node->hash);
> }
> ovs_mutex_lock();
> ...
> }
> 
> Log recording:
> 2024-04-27T12:28:47.973Z|6|ofproto_dpif_rid(pmd-c08/id:13)|INFO|wsh:hash 
> equal:
> hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 
> 0x7fb2900194d0 hash:3224122528
> 2024-04-27T12:28:47.973Z|9|ofproto_dpif_rid(pmd-c02/id:15)|INFO|wsh:hash 
> equal:
> hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 
> 0x7fb288025270 hash:3224122528
> 2024-04-27T12:28:47.973Z|6|ofproto_dpif_rid(pmd-c03/id:14)|INFO|wsh:hash 
> equal:
> hash_recirc_node 0x7fb29c0276a0 id:27,hash_recirc_node hash:3224122528,node 
> 0x7fb29401d4e0 hash:3224122528
> 2024-04-27T12:28:47.973Z|4|ofproto_dpif_rid(pmd-c09/id:12)|INFO|node->id:28,hash:4019648042,table_id:75
> 2024-04-27T12:28:47.973Z|7|ofproto_dpif_rid(pmd-c08/id:13)|INFO|wsh:hash 
> equal:
> hash_recirc_node 0x7fb29c028d40 id:28,hash_recirc_node hash:4019648042,node 
> 0x7fb29001ac30 hash:4019648042
> 2024-04-27T12:28:48.065Z|5|ofproto_dpif_rid(pmd-c09/id:12)|INFO|node->id:29,hash:3800776147,table_id:30
> 2024-04-27T12:28:48.101Z|7|ofproto_dpif_rid(pmd-c03/id:14)|INFO|node->id:30,hash:1580334976,table_id:75

nit: blank line before Signed-off-by line please.

> Signed-off-by: Shaohua Wu 
> 
> ---
> v1->v2:modify log recording , add trace code.

If you post a v3, please do so in a new thread.
Thanks!

...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] GitHub: Add Coverity scan as a daily GitHub action.

[Eelco Chaudron]

On 29 Apr 2024, at 14:59, Ilya Maximets wrote:

> On 4/16/24 09:44, Eelco Chaudron wrote:
>> This patch adds a daily Coverity run for the OVS main branch
>> to the GitHub actions. The result of the runs can be found here:
>>
>>  https://scan.coverity.com/projects/openvswitch
>>
>> Before applying, we need to add the following two actions secrets
>> to the GitHub openvswitch project:
>>
>> - COVERITY_SCAN_TOKEN; The secret token from the project page
>> - COVERITY_SCAN_EMAIL; The maintainer's email alias
>>
>> Signed-off-by: Eelco Chaudron 
>> ---
>>  .github/workflows/coverity.yml | 131 +
>>  Makefile.am|   1 +
>>  README.rst |   2 +
>>  3 files changed, 134 insertions(+)
>>  create mode 100644 .github/workflows/coverity.yml
>
> Thanks, Eelco for bringing this up.  Though, continuing the
> offline discussion, I'm not sure having coverity scans as
> a GitHub action has any benefits for OVS community.
>
> A few issues:
>
> 1. Reports are not available to people outside of the OVS
>Coverity project.  Adding everyone there doesn't seem
>reasonable.  Also a potential security concern.

I do not see a security concern, as anyone can set up the coverity scan on his 
fork of the project and has all the insights. I do agree that we should not try 
to add everybody to the Coverity project, and this might be more for 
maintainers (who can opt-in to get emails on new issues).

> 2. Workflow cannot be used in forks due to organization secrets.
>(IIUC, current implementation will just fail once a day
>annoying people who didn't disable that workflow.)

I guess this can be fixed, we could check for the variables. This way if 
someone wants to add Coverity scans to his fork, all they need to do is add the 
secrets.

> 3. Necessity to have secrets in the organization.  We currently
>don't have any, and from a security standpoint not having
>any secrets is always better.

Is having the secrets in a 3rd party located robot more secure?

> 4. A decent amount of code duplication for this workflow.
>(Reusable workflows maybe?)

Yes, I can take a look at composition in GitHub actions, as there is no real 
‘include’ like feature.

> In its current form, I think, the same functionality can be
> provided by the 0-day robot that could submit sources once
> a day for the scan.  And it could also send some emails with
> build details if necessary (again, there may be some security
> concerns in case coverity discovers a genuine security issue).

Guess results are only available in the GUI and through an email, don’t think 
you can query new items through an API.

@Aaron do you know how this is handled for DPDK?

> With the total amount of code in the project, IIUC, we could
> submit up to 21 builds a week with no more than 3 per day
> (Coverity claims that it checked just under 300K lines in OVS).
> So, technically, we could create a bit more complex logic
> for 0-day bot to run a check per patch set (probably, not per
> patch though).  In most cases we would fit within the limit
> with the current patch rate.  Maybe reserving one run a week
> for the main branch.  Such logic would be harder to implement
> with GHA.
>
> All in all, using GHA just for scheduling of a job that wider
> community can't take advantage of seems a little unjustified.
>
> Thoughts?

I think it would benefit the maintainers to start with, and if implemented 
right people who fork should not see any side effects. In addition, it also 
enables them to integrate Coverity for their own fork if they want to.

Others?

//Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] GitHub: Add Coverity scan as a daily GitHub action.

[Eelco Chaudron]

On 26 Apr 2024, at 10:57, Simon Horman wrote:

> On Tue, Apr 16, 2024 at 09:44:52AM +0200, Eelco Chaudron wrote:
>> This patch adds a daily Coverity run for the OVS main branch
>> to the GitHub actions. The result of the runs can be found here:
>>
>>  https://scan.coverity.com/projects/openvswitch
>>
>> Before applying, we need to add the following two actions secrets
>> to the GitHub openvswitch project:
>>
>> - COVERITY_SCAN_TOKEN; The secret token from the project page
>> - COVERITY_SCAN_EMAIL; The maintainer's email alias
>>
>> Signed-off-by: Eelco Chaudron 
>
> Hi Eelco,
>
> I'm fine with this patch in it's current form.
>
> Acked-by: Simon Horman 
>
> But I do have a few questions.
>
> 1. Would it be useful to provide instructions to
>allow people to become involved in addressing warnings?
>
>It seems that a signup to scan.coverity.com is required
>to see information on warnings.

I think my idea was for now to only enable this for maintainers, which I think 
is how they do it for DPDK also (or people who contribute to a specific area a 
lot). Trying to maintain a list of all user interested so they can access might 
be too much (but we could try if we really want to).

The goal of the patch was, that if they wanted to, they could put in the 
secrets and it would run on their fork.

> 2. Is there any plan to address warnings?

If someone is interested in the work that would be nice. I’ll try if I get any 
time, but no commitments right now.

At least we should find new issues and can ask patch submitters to look at 
their introduced issue.

> 3. See below.
>
>> ---
>>  .github/workflows/coverity.yml | 131 +
>>  Makefile.am|   1 +
>>  README.rst |   2 +
>>  3 files changed, 134 insertions(+)
>>  create mode 100644 .github/workflows/coverity.yml
>>
>> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
>> new file mode 100644
>> index 0..ae28920de
>> --- /dev/null
>> +++ b/.github/workflows/coverity.yml
>> @@ -0,0 +1,131 @@
>> +name: Coverity scan
>> +on:
>> +  schedule:
>> +- cron: '0 0 * * *'
>> +
>> +env:
>> +  python_default: 3.12
>> +
>> +jobs:
>> +  build-dpdk:
>> +env:
>> +  dependencies: gcc libbpf-dev libnuma-dev libpcap-dev ninja-build 
>> pkgconf
>> +  CC: gcc
>> +  DPDK_GIT: https://dpdk.org/git/dpdk
>> +  DPDK_VER: 23.11
>> +name: dpdk gcc
>> +outputs:
>> +  dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
>> +runs-on: ubuntu-22.04
>> +timeout-minutes: 30
>> +
>> +steps:
>> +- name: checkout
>> +  uses: actions/checkout@v4
>> +
>> +- name: update PATH
>> +  run: |
>> +echo "$HOME/bin">> $GITHUB_PATH
>> +echo "$HOME/.local/bin" >> $GITHUB_PATH
>> +
>> +- name: create ci signature file for the dpdk cache key
>> +  # This will collect most of DPDK related lines, so hash will be 
>> different
>> +  # if something changed in a way we're building DPDK including 
>> DPDK_VER.
>> +  # This also allows us to use cache from any branch as long as version
>> +  # and a way we're building DPDK stays the same.
>> +  run: |
>> +cat .ci/dpdk-* > dpdk-ci-signature
>> +grep -rwE 'DPDK_GIT|DPDK_VER' .github/ >> dpdk-ci-signature
>> +if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
>> +git ls-remote --heads $DPDK_GIT $DPDK_VER >> dpdk-ci-signature
>> +fi
>> +cat dpdk-ci-signature
>> +
>> +- name: generate ci DPDK key
>> +  id: gen_dpdk_key
>> +  env:
>> +ci_key: ${{ hashFiles('dpdk-ci-signature') }}
>> +  run: echo 'key=dpdk-${{ env.ci_key }}' >> $GITHUB_OUTPUT
>
> The above two steps seem both complex and somewhat different to
> the implementation in build-and-test.yml.
>
> Would it be worth introducing some sort of helper to generate
> the key? Perhaps a script that lives in .ci/

It looks the same in my tree, as I copied it from that file :) But I think as 
Ilya mentioned, we might be able to use some GitHub action composition, but 
need to play with this and make it work from within the same repository.

>> +
>> +- name: cache
>> +  id: dpdk_cache
>> +  uses: actions/cache@v4
>> +  with:
>> +path: dpdk-dir
>> +key: ${{ steps.gen_dpdk_key.outputs.key }}
>
> ...

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Update Pacemaker main page link.

[Eelco Chaudron]

On 1 May 2024, at 12:54, Simon Horman wrote:

> Update link to pacemaker main page as the existing link is broken.
> Also, use HTTPS.
>
> Broken link flagged by make check-docs
>
> Signed-off-by: Simon Horman 
> ---

Thanks for fixing the link.

Acked-by: Eelco Chaudron 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5] ci: Add clang-analyze to GitHub actions.

[Eelco Chaudron]

On 5 Apr 2024, at 12:14, Ilya Maximets wrote:

> On 1/11/24 00:08, Eelco Chaudron wrote:
>> This patch identifies new static analysis issues during a GitHub action
>> run and reports them. The process involves analyzing the changes introduced
>> in the current commit and comparing them to those in the preceding commit.
>>
>> However, there are two cases when the GitHub push action runner does not
>> provide enough details to determine the preceding commit. These cases are
>> a new branch or a forced push. The strategy for these exceptions is to
>> find the first common commit on any upstream branch, and use that.
>>
>> An example error output might look like this:
>>
>>   error level: +0 -0 no changes
>>   warning level: +2 +0
>> New issue "deadcode.DeadStores Value stored to 'remote' is never read" 
>> (1 occurrence)
>>  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
>> New issue "unix.Malloc Potential leak of memory pointed to by 'remote'" 
>> (1 occurrence)
>>  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
>>   note level: +0 -0 no changes
>>   all levels: +2 +0
>>
>> Signed-off-by: Eelco Chaudron 
>> ---
>>
>> changes in v2:
>>   - When it's a new branch, it compares it to the HEAD of the default branch.
>>
>> changes in v3:
>>   - Include the clang version as part of the cache
>>   - Change the way it looks for the 'default' branch so it will work
>> for patch branches.
>>   - Also compare to the base branch for forced commits.
>>
>> changes in v4:
>>   - No longer look for a default branch, but consume all patches
>> from the current author.
>>
>> changes in v5:
>>   - Addressed Ilya's comments.
>>   - Checkout upstream branch and find common point to base delta on.
>>
>>  .ci/linux-build.sh   |  30 +++
>>  .ci/linux-prepare.sh |   2 +-
>>  .github/workflows/build-and-test.yml | 113 +++
>>  3 files changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index 90581c10b..4589a8ba2 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -50,6 +50,31 @@ function build_ovs()
>>  make ${JOBS}
>>  }
>>
>> +function clang_analyze()
>> +{
>> +[ -d "./base-clang-analyzer-results" ] && cache_build=false \
>> +   || cache_build=true
>> +if [ "$cache_build" = true ]; then
>> +# If this is a cache build, proceed to the base branch's directory.
>> +pushd base_ovs_main
>> +fi;
>> +
>> +configure_ovs $OPTS
>> +
>> +make clean
>> +scan-build -o ./clang-analyzer-results -sarif --use-cc=${CC} make 
>> ${JOBS}
>> +
>> +if [ "$cache_build" = true ]; then
>> +# Move results, so it will be picked up by the cache.
>> +mv ./clang-analyzer-results ../base-clang-analyzer-results
>> +popd
>> +else
>> +# Only do the compare on the none cache builds.
>> +sarif --check note diff ./base-clang-analyzer-results \
>> +./clang-analyzer-results
>> +fi;
>> +}
>> +
>>  if [ "$DEB_PACKAGE" ]; then
>>  ./boot.sh && ./configure --with-dpdk=$DPDK && make debian
>>  mk-build-deps --install --root-cmd sudo --remove debian/control
>> @@ -117,6 +142,11 @@ fi
>>
>>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>
>> +if [ "$CLANG_ANALYZE" ]; then
>> +clang_analyze
>> +exit 0
>> +fi
>> +
>>  if [ "$TESTSUITE" = 'test' ]; then
>>  # 'distcheck' will reconfigure with required options.
>>  # Now we only need to prepare the Makefile without sparse-wrapped CC.
>> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
>> index c28b6819a..5028bdc44 100755
>> --- a/.ci/linux-prepare.sh
>> +++ b/.ci/linux-prepare.sh
>> @@ -23,7 +23,7 @@ cd ..
>>  # https://github.com/pypa/pip/issues/10655
>>  pip3 install --disable-pip-version-check --user wheel
>>  pip3 install --disable-pip-version-check --user \
>> -flake8 'hacking>=3.0' netaddr pyparsing sphinx setuptools
>> +flake8 'hacking>=3.0' netaddr pyparsing sarif-tools sphinx setuptools
>>
>>  # Install python test dependencies
>>  pip3 install -r python/test_requirements.txt
>> diff --git a/.github/workflows/build-and-test.yml 
>> b/.github/workflows/build-and-test.yml
>> index 710757693..f5858fdbe 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -254,6 +254,119 @@ jobs:
>>  name: logs-linux-${{ join(matrix.*, '-') }}
>>  path: logs.tgz
>>
>> +  build-clang-analyze:
>> +needs: build-dpdk
>> +env:
>> +  dependencies: |
>> +automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
>> +libunbound-dev libunwind-dev libssl-dev libtool llvm-dev
>> +  CC:   clang
>> +  DPDK: dpdk
>> +  CLANG_ANALYZE: true
>> +name: clang-analyze
>> +runs-on: ubuntu-22.04
>> +timeout-minutes: 30
>> +
>> +steps:
>> +- name: checkout
>> +  

[ovs-dev] [Patch] ovsdb-client: Add missing arg to help for 'dump'.

[Martin Kalcok]

Help text for 'ovsdb-client dump' does not mention that it's capable
of dumping specific table's contents if user supplies table's name
as a third positional argument.

Signed-off-by: Martin Kalcok 
---
 ovsdb/ovsdb-client.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 7249805ba..cf2ecfd08 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -451,8 +451,9 @@ usage(void)
"wait until DATABASE reaches STATE "
"(\"added\" or \"connected\" or \"removed\")\n"
"in DATBASE on SERVER.\n"
-   "\n  dump [SERVER] [DATABASE]\n"
-   "dump contents of DATABASE on SERVER to stdout\n"
+   "\n  dump [SERVER] [DATABASE] [TABLE]\n"
+   "dump contents of TABLE (or all tables) in DATABASE on SERVER\n"
+   "to stdout\n"
"\n  backup [SERVER] [DATABASE] > SNAPSHOT\n"
"dump database contents in the form of a database file\n"
"\n  [--force] restore [SERVER] [DATABASE] < SNAPSHOT\n"
-- 
2.40.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/2] conntrack: Key connections by zone.

[Simon Horman]

On Wed, Apr 24, 2024 at 02:44:54PM +0200, Felix Huettner via dev wrote:
> Currently conntrack uses a single large cmap for all connections stored.
> This cmap contains all connections for all conntrack zones which are
> completely separate from each other. By separating each zone to its own
> cmap we can significantly optimize the performance when using multiple
> zones.
> 
> The change fixes a similar issue as [1] where slow conntrack zone flush
> operations significantly slow down OVN router failover. The difference is
> just that this fix is used whith dpdk, while [1] was when using the ovs
> kernel module.
> 
> As we now need to store more cmap's the memory usage of struct conntrack
> increases by 524280 bytes. Additionally we need 65535 cmaps with 128
> bytes each. This leads to a total memory increase of around 10MB.
> 
> Running "./ovstest test-conntrack benchmark 4 33554432 32 1" shows no
> real difference in the multithreading behaviour against a single zone.
> 
> Running the new "./ovstest test-conntrack benchmark-zones" show
> significant speedups as shown below. The values for "ct execute" are for
> acting on the complete zone with all its entries in total (so in the
> first case adding 10,000 new conntrack entries). All tests are run 1000
> times.
> 
> When running with 1,000 zones with 10,000 entries each we see the
> following results (all in microseconds):
> "./ovstest test-conntrack benchmark-zones 1 1000 1000"
> 
>  +--++-+-+
>  |  Min |   Max  |  95%ile |   Avg   |
> ++--++-+-+
> | ct execute (commit)|  || | |
> |with commit | 2266 |   3505 | 2707.06 | 2592.06 |
> | without commit | 2411 |  12730 | 4432.50 | 2736.78 |
> ++--++-+-+
> | ct execute (no commit) |  || | |
> |with commit |  699 |   1238 |  886.15 |  722.67 |
> | without commit |  700 |   3377 | 1934.42 |  803.53 |
> ++--++-+-+
> | flush full zone|  || | |
> |with commit |  619 |   1122 |  901.36 |  679.15 |
> | without commit |  618 | 105078 |   64591 | 2886.46 |
> ++--++-+-+
> | flush empty zone   |  || | |
> |with commit |0 |  5 |1.00 |0.64 |
> | without commit |   54 |  87469 |   64520 | 2172.25 |
> ++--++-+-+
> 
> When running with 10,000 zones with 1,000 entries each we see the
> following results (all in microseconds):
> "./ovstest test-conntrack benchmark-zones 1000 1 1000"
> 
>  +--++-+-+
>  |  Min |   Max  |  95%ile |   Avg   |
> ++--++-+-+
> | ct execute (commit)|  || | |
> |with commit |  215 |287 |  231.88 |  222.30 |
> | without commit |  214 |   1692 |  569.18 |  285.83 |
> ++--++-+-+
> | ct execute (no commit) |  || | |
> |with commit |   68 | 97 |   74.69 |   70.09 |
> | without commit |   68 |300 |  158.40 |   82.06 |
> ++--++-+-+
> | flush full zone|  || | |
> |with commit |   47 |211 |   56.34 |   50.34 |
> | without commit |   48 |  96330 |   63392 |   63923 |
> ++--++-+-+
> | flush empty zone   |  || | |
> |with commit |0 |  1 |1.00 |0.44 |
> | without commit |3 | 109728 |   63923 | 3629.44 |
> ++--++-+-+
> 
> Comparing the averages we see:
> * a moderate performance improvement for conntrack_execute with or
>   without commiting of around 6% to 23%
> * a significant performance improvement for flushing a full zone of
>   around 75% to 99%
> * an even more significant improvement for flushing empty zones since we
>   no longer need to check any unrelated connections

Very nice numbers indeed.

> [1] 9ec849e8aa869b646c372fac552ae2609a4b5f66
> 
> Signed-off-by: Felix Huettner 

Acked-by: Simon Horman 

...

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/2] test-conntrack: Add per zone benchmark tool.

[Simon Horman]

On Wed, Apr 24, 2024 at 02:44:47PM +0200, Felix Huettner via dev wrote:
> The current test-conntrack benchmark command runs with multiple threads
> against a single conntrack zone. We now add a new benchmark-zones
> command that allows us to check the performance between multiple zones.
> 
> We in there test the following scenarios for one zone while other zones
> also contain entries:
> 1. Flushing a single full zone
> 2. Flushing a single empty zone
> 3. Commiting new conntrack entries against a single zone
> 4. Running conntrack_execute without commit against the entries of a
>single zone
> 
> Signed-off-by: Felix Huettner 
> ---
> v1->v2: fix formatting

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC] route-table: Add support for v4 via v6 route.

[Simon Horman]

On Wed, May 01, 2024 at 11:57:46AM +0100, Simon Horman wrote:
> On Fri, Apr 26, 2024 at 10:16:31PM -0700, William Tu via dev wrote:
> > Add route-table that support ipv4 dst via ipv6. BGP unnumbered is mechanism
> > that allows BGP to establish peering sessions without the need to explicitly
> > configure IP addresses on the interfaces involved in the peering. Without
> > using IP address assignments, it uses link-local IPv6 addresses of the
> > directly connected neighbors for peering purposes. For example, BGP
> > might install the following route:
> > $ ip route get 100.87.18.3
> >  100.87.18.3 via inet6 fe80::920a:84ff:fe9e:9570 dev br-phy src 100.87.18.6
> > 
> > Currently OVS can only support either all-ipv4 or all-ipv6, the patch
> > adds support for such use case.
> > 
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-January/052908.html
> > Signed-off-by: William Tu 
> 
> Acked-by: Simon Horman 

Just to clarify: this patch looks good to me.
But I suggest resubmit it as a non-RFC when you are ready.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC] route-table: Add support for v4 via v6 route.

[Simon Horman]

On Fri, Apr 26, 2024 at 10:16:31PM -0700, William Tu via dev wrote:
> Add route-table that support ipv4 dst via ipv6. BGP unnumbered is mechanism
> that allows BGP to establish peering sessions without the need to explicitly
> configure IP addresses on the interfaces involved in the peering. Without
> using IP address assignments, it uses link-local IPv6 addresses of the
> directly connected neighbors for peering purposes. For example, BGP
> might install the following route:
> $ ip route get 100.87.18.3
>  100.87.18.3 via inet6 fe80::920a:84ff:fe9e:9570 dev br-phy src 100.87.18.6
> 
> Currently OVS can only support either all-ipv4 or all-ipv6, the patch
> adds support for such use case.
> 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2024-January/052908.html
> Signed-off-by: William Tu 

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Documentation: Update Pacemaker main page link.

[Simon Horman]

Update link to pacemaker main page as the existing link is broken.
Also, use HTTPS.

Broken link flagged by make check-docs

Signed-off-by: Simon Horman 
---
 Documentation/topics/integration.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/topics/integration.rst 
b/Documentation/topics/integration.rst
index ee83f8d43902..0f40baae741c 100644
--- a/Documentation/topics/integration.rst
+++ b/Documentation/topics/integration.rst
@@ -191,7 +191,7 @@ contents. At all times, the data can be transacted only 
from the active server.
 When the active server dies for some reason, entire OVN operations will be
 stalled.
 
-`Pacemaker `__ is a cluster resource
+`Pacemaker `__ is a cluster resource
 manager which can manage a defined set of resource across a set of clustered
 nodes. Pacemaker manages the resource with the help of the resource agents.
 One among the resource agent is `OCF

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] socket: Don't fail when AF_UNIX connect() returns EAGAIN.

[Simon Horman]

On Fri, Apr 26, 2024 at 10:42:36PM +, Ihar Hrachyshka wrote:
> POSIX defines EINPROGRESS as the return value for non-blocking connect()
> [1]. But in Linux, AF_UNIX connect() returns EAGAIN instead of
> EINPROGRESS. (but only for AF_UNIX sockets!) [2]
> 
> Both cases should be handled the same way - by returning the `fd` and
> letting the caller to complete connection asynchronously.
> 
> [1]: 
> https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/functions/connect.html
> [2]: see `connect(2)` on Linux for details
> 
> Signed-off-by: Ihar Hrachyshka 

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] docs: Document manual cluster recovery procedure.

[Simon Horman]

On Fri, Apr 26, 2024 at 04:54:48PM +, Ihar Hrachyshka wrote:
> Remove the notion of cluster/leave --force since it was never
> implemented. Instead of these instructions, document how a broken
> cluster can be re-initialized with the old database contents.
> 
> Signed-off-by: Ihar Hrachyshka 
> 
> ---
> 
> v1: initial version.
> v2: remove --force mentioned in ovsdb-server(1).
> v3: multiple language and markup changes suggested by Ilya.

Thanks for the updates Ihar, this version looks good to me.

Acked-by: Simon Horman 

...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev