Re: [ovs-dev] [PATCH v6 ovn] northd: Add bfd, static_routes and route_policies nodes to I-P engine.

[Han Zhou]

On Wed, Jul 10, 2024 at 11:24 AM Lorenzo Bianconi
 wrote:
>
> Introduce bfd, static_routes and route_policies nodes to northd I-P
> engine to track bfd connections and northd static_route/policy_route
> changes.
>
> Acked-by: Numan Siddique 
> Reported-at: https://issues.redhat.com/browse/FDP-600
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v5:
> - remove bfd_mutex
> Changes since v4:
> - introduce bfd_nb_sync node and ger rid of bfd_consumer routine
> Changes since v3:
> - fix bfd_northd_change_handler logic
> - fix route_policies_northd_change_handler logic
> - fix static_routes_northd_change_handler logic
> - add unit-tests
> - cosmetics
> Changes since v2:
> - add incremental processing routines
> - split bfd_consumer node in static_routes and route_policies nodes
> Changes since v1:
> - add incremental processing logic for bfd_consumer node to avoid a full
>   recompute
> ---
>  northd/en-lflow.c|  25 +-
>  northd/en-northd.c   | 215 ++
>  northd/en-northd.h   |  23 ++
>  northd/inc-proc-northd.c |  37 ++-
>  northd/northd.c  | 592 +++
>  northd/northd.h  |  63 -
>  tests/ovn-northd.at  |  56 
>  7 files changed, 808 insertions(+), 203 deletions(-)
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index c4b927fb8..3dba5034b 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -41,6 +41,11 @@ 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 bfd_data *bfd_data = engine_get_input_data("bfd", node);
> +struct static_routes_data *static_routes_data =
> +engine_get_input_data("static_routes", node);
> +struct route_policies_data *route_policies_data =
> +engine_get_input_data("route_policies", node);
>  struct port_group_data *pg_data =
>  engine_get_input_data("port_group", node);
>  struct sync_meters_data *sync_meters_data =
> @@ -50,10 +55,6 @@ lflow_get_input_data(struct engine_node *node,
>  struct ed_type_ls_stateful *ls_stateful_data =
>  engine_get_input_data("ls_stateful", node);
>
> -lflow_input->nbrec_bfd_table =
> -EN_OVSDB_GET(engine_get_input("NB_bfd", node));
> -lflow_input->sbrec_bfd_table =
> -EN_OVSDB_GET(engine_get_input("SB_bfd", node));
>  lflow_input->sbrec_logical_flow_table =
>  EN_OVSDB_GET(engine_get_input("SB_logical_flow", node));
>  lflow_input->sbrec_multicast_group_table =
> @@ -78,7 +79,10 @@ lflow_get_input_data(struct engine_node *node,
>  lflow_input->meter_groups = _meters_data->meter_groups;
>  lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
>  lflow_input->svc_monitor_map = _data->svc_monitor_map;
> -lflow_input->bfd_connections = NULL;
> +lflow_input->bfd_connections = _data->bfd_connections;
> +lflow_input->parsed_routes = _routes_data->parsed_routes;
> +lflow_input->route_tables = _routes_data->route_tables;
> +lflow_input->route_policies = _policies_data->route_policies;
>
>  struct ed_type_global_config *global_config =
>  engine_get_input_data("global_config", node);
> @@ -95,25 +99,14 @@ void en_lflow_run(struct engine_node *node, void *data)
>  struct lflow_input lflow_input;
>  lflow_get_input_data(node, _input);
>
> -struct hmap bfd_connections = HMAP_INITIALIZER(_connections);
> -lflow_input.bfd_connections = _connections;
> -
>  stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
>
>  struct lflow_data *lflow_data = data;
>  lflow_table_clear(lflow_data->lflow_table);
>  lflow_reset_northd_refs(_input);
>
> -build_bfd_table(eng_ctx->ovnsb_idl_txn,
> -lflow_input.nbrec_bfd_table,
> -lflow_input.sbrec_bfd_table,
> -lflow_input.lr_ports,
> -_connections);
>  build_lflows(eng_ctx->ovnsb_idl_txn, _input,
>   lflow_data->lflow_table);
> -bfd_cleanup_connections(lflow_input.nbrec_bfd_table,
> -_connections);
> -hmap_destroy(_connections);
>  stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
>
>  engine_set_node_state(node, EN_UPDATED);
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 4479b4aff..7595c6f5f 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -236,6 +236,162 @@ northd_global_config_handler(struct engine_node *node, 
> void *data OVS_UNUSED)
>  return true;
>  }
>
> +bool
> +route_policies_northd_change_handler(struct engine_node *node,
> + void *data OVS_UNUSED)
> +{
> +struct northd_data *northd_data = engine_get_input_data("northd", node);
> +if (!northd_has_tracked_data(_data->trk_data)) {
> +return false;
> +}
> +
> +/* This node uses the below data 

Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to branch-3.4.

[Ales Musil]

On Wed, Jul 17, 2024 at 6:04 PM Ilya Maximets  wrote:

> Updating to OVS 3.4 to get enhanced sample action and other latest
> features and fixes in general.
>
> Unixctl library was changed to provide JSON-formatted output, so
> clients has to be adjusted.  Clients also need to handle extra line
> breaks at the end, since those are no longer prvided by the server.
>
> We could define a common reply_to_string() function for both dbctl
> and appctl, but these programs beahve differently in regerads to exit
> codes and we'll need to do more refactoring to handle the difference.
>
> Will move to v3.4.0 once it is available.
>
> Signed-off-by: Ilya Maximets 
> ---
>  controller/physical.c  |  4 ++--
>  lib/actions.c  |  6 +++---
>  ovs|  2 +-
>  utilities/ovn-appctl.c | 42 --
>  utilities/ovn-dbctl.c  | 40 +---
>  5 files changed, 75 insertions(+), 19 deletions(-)
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 22756810f..0255a1bbd 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -889,8 +889,8 @@ put_drop(const struct physical_debug *debug, uint8_t
> table_id,
>  struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
>  os->probability = UINT16_MAX;
>  os->collector_set_id = debug->collector_set_id;
> -os->obs_domain_id = (debug->obs_domain_id << 24);
> -os->obs_point_id = table_id;
> +os->obs_domain_imm = (debug->obs_domain_id << 24);
> +os->obs_point_imm = table_id;
>  os->sampling_port = OFPP_NONE;
>  }
>  }
> diff --git a/lib/actions.c b/lib/actions.c
> index e8cc0994d..37676ef81 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -4546,13 +4546,13 @@ encode_SAMPLE(const struct ovnact_sample *sample,
>  os = ofpact_put_SAMPLE(ofpacts);
>  os->probability = sample->probability;
>  os->collector_set_id = sample->collector_set_id;
> -os->obs_domain_id =
> +os->obs_domain_imm =
>  (sample->obs_domain_id << 24) | (ep->dp_key & 0xFF);
>
>  if (sample->use_cookie) {
> -os->obs_point_id = ep->lflow_uuid.parts[0];
> +os->obs_point_imm = ep->lflow_uuid.parts[0];
>  } else {
> -os->obs_point_id = sample->obs_point_id;
> +os->obs_point_imm = sample->obs_point_id;
>  }
>  os->sampling_port = OFPP_NONE;
>  }
> diff --git a/ovs b/ovs
> index bf1b16364..0aa14d912 16
> --- a/ovs
> +++ b/ovs
> @@ -1 +1 @@
> -Subproject commit bf1b16364b3f01b0ff5f2f6e76842e666226a17b
> +Subproject commit 0aa14d912d9a29d07ebc727007a1f21e3639eea5
> diff --git a/utilities/ovn-appctl.c b/utilities/ovn-appctl.c
> index 059492972..dff7d1295 100644
> --- a/utilities/ovn-appctl.c
> +++ b/utilities/ovn-appctl.c
> @@ -27,6 +27,7 @@
>  #include "lib/ovn-dirs.h"
>  #include "lib/ovn-util.h"
>  #include "openvswitch/dynamic-string.h"
> +#include "openvswitch/json.h"
>  #include "jsonrpc.h"
>  #include "process.h"
>  #include "timeval.h"
> @@ -37,14 +38,16 @@
>  static void usage(void);
>  static const char *parse_command_line(int argc, char *argv[]);
>  static struct jsonrpc *connect_to_target(const char *target);
> +static char *reply_to_string(struct json *);
>
>  int
>  main(int argc, char *argv[])
>  {
> -char *cmd_result, *cmd_error;
> +struct json *cmd_result, *cmd_error;
>  struct jsonrpc *client;
>  char *cmd, **cmd_argv;
>  const char *target;
> +char *msg = NULL;
>  int cmd_argc;
>  int error;
>
> @@ -66,19 +69,23 @@ main(int argc, char *argv[])
>
>  if (cmd_error) {
>  jsonrpc_close(client);
> -fputs(cmd_error, stderr);
> +msg = reply_to_string(cmd_error);
> +fputs(msg, stderr);
> +free(msg);
>  ovs_error(0, "%s: server returned an error", target);
> -free(cmd_error);
> +json_destroy(cmd_error);
>  error ? exit(error) : exit(2);
>  } else if (cmd_result) {
> -fputs(cmd_result, stdout);
> +msg = reply_to_string(cmd_result);
> +fputs(msg, stdout);
> +free(msg);
>  } else {
>  OVS_NOT_REACHED();
>  }
>
>  jsonrpc_close(client);
> -free(cmd_result);
> -free(cmd_error);
> +json_destroy(cmd_result);
> +json_destroy(cmd_error);
>  return 0;
>  }
>
> @@ -239,3 +246,26 @@ connect_to_target(const char *target)
>  return client;
>  }
>
> +/* The caller is responsible for freeing the returned string, with
> free(), when
> + * it is no longer needed. */
> +static char *
> +reply_to_string(struct json *reply)
> +{
> +ovs_assert(reply);
> +
> +if (reply->type != JSON_STRING) {
> +ovs_error(0, "Unexpected reply type in JSON rpc reply: %s",
> +  json_type_to_string(reply->type));
> +exit(2);
> +}
> +
> +struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +ds_put_cstr(, json_string(reply));
> +
> +if (ds_last() != EOF && ds_last() 

[ovs-dev] [PATCH net-next] openvswitch: switch to per-action label counting in conntrack

[Xin Long]

Similar to commit 70f06c115bcc ("sched: act_ct: switch to per-action
label counting"), we should also switch to per-action label counting
in openvswitch conntrack, as Florian suggested.

The difference is that nf_connlabels_get() is called unconditionally
when creating an ct action in ovs_ct_copy_action(). As with these
flows:

  table=0,ip,actions=ct(commit,table=1)
  table=1,ip,actions=ct(commit,exec(set_field:0xac->ct_label),table=2)

it needs to make sure the label ext is created in the 1st flow before
the ct is committed in ovs_ct_commit(). Otherwise, the warning in
nf_ct_ext_add() when creating the label ext in the 2nd flow will
be triggered:

   WARN_ON(nf_ct_is_confirmed(ct));

Signed-off-by: Xin Long 
---
 net/openvswitch/conntrack.c | 28 +++-
 net/openvswitch/datapath.h  |  3 ---
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 8eb1d644b741..2cc38faab682 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1368,11 +1368,8 @@ bool ovs_ct_verify(struct net *net, enum ovs_key_attr 
attr)
attr == OVS_KEY_ATTR_CT_MARK)
return true;
if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) &&
-   attr == OVS_KEY_ATTR_CT_LABELS) {
-   struct ovs_net *ovs_net = net_generic(net, ovs_net_id);
-
-   return ovs_net->xt_label;
-   }
+   attr == OVS_KEY_ATTR_CT_LABELS)
+   return true;
 
return false;
 }
@@ -1381,6 +1378,7 @@ int ovs_ct_copy_action(struct net *net, const struct 
nlattr *attr,
   const struct sw_flow_key *key,
   struct sw_flow_actions **sfa,  bool log)
 {
+   unsigned int n_bits = sizeof(struct ovs_key_ct_labels) * BITS_PER_BYTE;
struct ovs_conntrack_info ct_info;
const char *helper = NULL;
u16 family;
@@ -1409,6 +1407,12 @@ int ovs_ct_copy_action(struct net *net, const struct 
nlattr *attr,
return -ENOMEM;
}
 
+   if (nf_connlabels_get(net, n_bits - 1)) {
+   nf_ct_tmpl_free(ct_info.ct);
+   OVS_NLERR(log, "Failed to set connlabel length");
+   return -EOPNOTSUPP;
+   }
+
if (ct_info.timeout[0]) {
if (nf_ct_set_timeout(net, ct_info.ct, family, key->ip.proto,
  ct_info.timeout))
@@ -1577,6 +1581,7 @@ static void __ovs_ct_free_action(struct 
ovs_conntrack_info *ct_info)
if (ct_info->ct) {
if (ct_info->timeout[0])
nf_ct_destroy_timeout(ct_info->ct);
+   nf_connlabels_put(nf_ct_net(ct_info->ct));
nf_ct_tmpl_free(ct_info->ct);
}
 }
@@ -2002,17 +2007,9 @@ struct genl_family dp_ct_limit_genl_family 
__ro_after_init = {
 
 int ovs_ct_init(struct net *net)
 {
-   unsigned int n_bits = sizeof(struct ovs_key_ct_labels) * BITS_PER_BYTE;
+#ifIS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
struct ovs_net *ovs_net = net_generic(net, ovs_net_id);
 
-   if (nf_connlabels_get(net, n_bits - 1)) {
-   ovs_net->xt_label = false;
-   OVS_NLERR(true, "Failed to set connlabel length");
-   } else {
-   ovs_net->xt_label = true;
-   }
-
-#ifIS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
return ovs_ct_limit_init(net, ovs_net);
 #else
return 0;
@@ -2026,7 +2023,4 @@ void ovs_ct_exit(struct net *net)
 #ifIS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
ovs_ct_limit_exit(net, ovs_net);
 #endif
-
-   if (ovs_net->xt_label)
-   nf_connlabels_put(net);
 }
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 9ca6231ea647..365b9bb7f546 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -160,9 +160,6 @@ struct ovs_net {
 #ifIS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
struct ovs_ct_limit_info *ct_limit_info;
 #endif
-
-   /* Module reference for configuring conntrack. */
-   bool xt_label;
 };
 
 /**
-- 
2.43.0

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

Re: [ovs-dev] [PATCH] AUTHORS: Add Vipul Ashri.

[Ilya Maximets]

On 7/17/24 15:24, Simon Horman wrote:
> Add Vipul Ashri to AUTHORS file.
> 
> Signed-off-by: Simon Horman 
> ---
>  AUTHORS.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/AUTHORS.rst b/AUTHORS.rst
> index 155e484360d9..28dcce4eaf79 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -760,6 +760,7 @@ Tytus Kurek tytus.ku...@pega.com
>  Valentin Budvalen...@hackaserver.com
>  Vasiliy Tolstov v.tols...@selfip.ru
>  Vinllen Chencvinl...@gmail.com
> +Vipul Ashri vipul.as...@ericsson.com
>  Vishal Swarankarvishal.swarn...@gmail.com
>  Vjekoslav Brajkovic bal...@cs.washington.edu
>  Voravit T.  vora...@kth.se

Thanks, Simon!  I was applying some other patches and picked
this one up as well for a good measure.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] match: Fix false-positive snprintf size warning.

[Ilya Maximets]

On 7/17/24 16:18, Mike Pattrick wrote:
> On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>>
>> GCC 14.1.1 of Fedora 41 thinks that 'i' can be in a full range and
>> so 8 bytes is not enough to print it.
>>
>>  lib/match.c: In function 'match_format':
>>  lib/match.c:1631:45:
>>  error: '%d' directive output may be truncated writing
>> between 1 and 11 bytes into a region of size 8
>>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>>| ^~
>>  lib/match.c:1631:44:
>>note: directive argument in the range [-2147483646, 1]
>>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>>|^~~~
>>  lib/match.c:1631:13:
>>note: 'snprintf' output between 2 and 12 bytes into
>>  a destination of size 8
>>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>>| ^~~
>>
>> In practice that value can't be larger than 2, but it's not a
>> performance critical code, so let's just increase the size to
>> a maximum 12.
>>
>> Signed-off-by: Ilya Maximets 
> 
> 
> Acked-by: Mike Pattrick 
> 

Thanks!  Applied and backported down to 2.17.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] util: Add non-NULL format assertion to xvasprintf.

[Ilya Maximets]

On 7/17/24 15:40, Mike Pattrick wrote:
> On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>>
>> For some reason GCC 14.1.1 on Fedora 41 assumes that format can
>> be NULL and emits a warning:
>>
>>  lib/util.c: In function 'xvasprintf':
>>  lib/util.c:229:14: error: null format string
>>229 | needed = vsnprintf(NULL, 0, format, args);
>>|  ^~~~
>>
>> I didn't find any users where this can be true.  Adding an
>> assertion to silence the warning.  In the worst case we'll
>> find out where it is being called incorrectly.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Acked-by: Mike Pattrick 
> 

Thanks, Mike!  Applied and backported down to 2.17.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] ofproto-dpif-xlate: Initialize observe_offset for sample actions.

[Ilya Maximets]

On 7/17/24 15:38, Mike Pattrick wrote:
> On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>>
>> For some reason gcc 14.1.1 from Fedora 41 thinks that the variable
>> may end up not initialized:
>>
>>  ofproto/ofproto-dpif-xlate.c: In function 'compose_sample_action':
>>  ofproto/ofproto-dpif-xlate.c:3465:40:
>>   error: 'observe_offset' may be used uninitialized
>>   3465 | ctx->xout->last_observe_offset = observe_offset;
>>| ~~~^~~~
>>  ofproto/ofproto-dpif-xlate.c:3418:12:
>>   note: 'observe_offset' was declared here
>>   3418 | size_t observe_offset;
>>|^~
>>
>> We have an assertion in the code to ensure that at least one of
>> the actions is present (userspace or psample), so the variable
>> should actually be always initialized.
>>
>> Initialize explicitly just to silence the warning.
>>
>> Fixes: 516569d31fbf ("ofproto: xlate: Make sampled drops explicit.")
>> Signed-off-by: Ilya Maximets 
>> ---
> 
> Acked-by: Mike Pattrick 
> 

Thanks, Mike!  Applied to main and 3.4.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] docs: Define Read the Docs configuration for Sphinx HTML parameters.

[Ilya Maximets]

On 7/17/24 13:55, Simon Horman wrote:
> On Wed, Jul 17, 2024 at 12:55:02PM +0200, Ilya Maximets wrote:
>> Read the Docs was always mangling the conf.py during the build to
>> inject custom domains configured in the project settings and some
>> other stuff.  But they will stop doing that soon [1].
>>
>> Adding recommended changes to the config to get this info from the
>> environment.
>>
>> [1] https://about.readthedocs.com/blog/2024/07/addons-by-default/
>>
>> Signed-off-by: Ilya Maximets 
> 
> Acked-by: Simon Horman 
> 

Thanks, Simon!

Applied and backported down to 3.3, since that's the version
we're currently building 'stable' documentation from.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Remove misleading wc NULL check in packet mirror.

[Ilya Maximets]

On 7/16/24 15:12, Mike Pattrick wrote:
> On Tue, Jul 16, 2024 at 8:47 AM Ilya Maximets  wrote:
>>
>> 'wc' can't be NULL there and if it can we'd already crash a few lines
>> before setting up vlan flags.
>>
>> The check is misleading as it makes people to assume that wc can be
>> NULL.  And it makes Coverity think the same:
>>
>>   CID 1596572: (#1 of 1): Dereference after null check (FORWARD_NULL)
>>   25. var_deref_op: Dereferencing null pointer ctx->wc.
>>
>>   14. var_compare_op: Comparing ctx->wc to null implies that ctx->wc
>>   might be null
>>
>> Remove the check.
>>
>> Fixes: 3b1882261c8b ("ofproto-dpif-mirror: Add support for pre-selection 
>> filter.")
>> Signed-off-by: Ilya Maximets 
>> ---
> 
> It looks like you're right, it can't be null.
> 
> Acked-by: Mike Pattrick 
> 

Thanks, Mike!  Applied and backported to 3.4.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] flow: Fix unaligned access to the ND target in miniflow_extract.

[Ilya Maximets]

On 7/16/24 15:34, Simon Horman wrote:
> On Tue, Jul 16, 2024 at 01:45:53PM +0200, Ales Musil wrote:
>> The data in the buffer are aligned to 2 bytes, however
>> 'struct in6_addr' is aligned to 4 bytes. Use the 2 bytes aligned
>> equivalent 'union ovs_16aligned_in6_addr' instead. This was caught
>> by one of the OVN tests:
>>
>> lib/flow.c:1133:25: runtime error: load of misaligned address
>> 0x5149cc92 for type 'const struct in6_addr *', which requires
>> 4 byte alignment
>> 0x5149cc92: note: pointer points here
>>  00 00  00 00 10 00 00 00 00 00  00 00 00 00 00 00 00 00
>>   ^
>> 0 0x8255b2 in miniflow_extract /workspace/ovn/ovs/lib/flow.c:1133:25
>> 1 0x81d921 in flow_extract /workspace/ovn/ovs/lib/flow.c:671:5
>> 2 0xa966d4 in ofp_packet_to_string /workspace/ovn/ovs/lib/ofp-print.c:82:5
>> 3 0xa76de2 in ofputil_packet_in_private_format 
>> /workspace/ovn/ovs/lib/ofp-packet.c:1037:24
>> 4 0xa99817 in ofp_print_packet_in /workspace/ovn/ovs/lib/ofp-print.c:132:9
>> 5 0xa97f46 in ofp_to_string__ /workspace/ovn/ovs/lib/ofp-print.c
>> 6 0xa97f46 in ofp_to_string /workspace/ovn/ovs/lib/ofp-print.c:1264:21
>> 7 0xc338f4 in do_send /workspace/ovn/ovs/lib/vconn.c:687:19
>> 8 0xb7f678 in try_send /workspace/ovn/ovs/lib/rconn.c:1128:14
>> 9 0xb7d725 in rconn_send__ /workspace/ovn/ovs/lib/rconn.c:760:13
>> 10 0xb7d8e7 in rconn_send_with_limit /workspace/ovn/ovs/lib/rconn.c:816:17
>> 11 0x6f70de in do_send_packet_ins 
>> /workspace/ovn/ovs/ofproto/connmgr.c:1697:13
>> 12 0x6f691f in connmgr_send_async_msg 
>> /workspace/ovn/ovs/ofproto/connmgr.c:1682:9
>> 13 0x5c2d23 in run /workspace/ovn/ovs/ofproto/ofproto-dpif.c:1877:13
>> 14 0x56737f in ofproto_run /workspace/ovn/ovs/ofproto/ofproto.c:1906:13
>> 15 0x50d4fc in bridge_run__ /workspace/ovn/ovs/vswitchd/bridge.c:3287:9
>> 16 0x50a764 in bridge_run /workspace/ovn/ovs/vswitchd/bridge.c:3346:5
>> 17 0x53eed7 in main /workspace/ovn/ovs/vswitchd/ovs-vswitchd.c:130:9
>>
>> Signed-off-by: Ales Musil 
> 
> Acked-by: Simon Horman 
> 

Thanks, Ales and Simon!
Applied and backported down to 2.17.

It is a legit issue, but for some reason I was not able to reproduce
the runtime exception with different compilers/versions/flags.  Could
you share some more details on how it was triggered?

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofp-actions: Fix reporting observation point bits instead of domain.

[Ilya Maximets]

On 7/16/24 15:40, Simon Horman wrote:
> On Tue, Jul 16, 2024 at 12:48:06PM +0200, Ilya Maximets wrote:
>> Found by Coverity:
>>
>>   CID 397544:  Incorrect expression  (COPY_PASTE_ERROR)
>>   "obs_point_src" in "(*os).obs_point_src.n_bits" looks
>>   like a copy-paste error.
>>
>> Also adding a test case to cover this situation.
>>
>> Fixes: 1aa9e137fe36 ("ofp-actions: Load data from fields in sample action.")
>> Signed-off-by: Ilya Maximets 
> 
> Acked-by: Simon Horman 
> 

Thanks!  Applied to main and branch-3.4.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v1] northd: Add ECMP symmetric replies for egress.

[Ilya Maximets]

On 7/16/24 23:16, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> When there are ECMP symmetric static routes configured, OVN selects
> one of the next hop for the traffic originated from within the
> cluster.  For the subsequent packets to the same destination,
> OVN may select a different next hop (which is fine).  But there can
> be certain usecases, where the next hop entity can be stateful and
> selecting the same next hop is desirable.
> 
> This patch address this usecase in the following way
> 
>1.  For the first packet originating from the OVN logical port
>VIF, OVN selects a next hop 'A' and forwards the traffic to
>it.
> 
>2.  When the reply traffic is received (either from next hop 'A'
>or any other next hop), it commits the connection in the
>DNAT zone of the logical router and saves the state in
>ct_label.ecmp_reply_eth and ct_label.ecmp_reply_port.
>Note that we already support this for the traffic
>originating from an ECMP route [1].  We are now extending
>the same for the traffic originating from the cluster towards
>the ECMP route.
> 
> 3. For the subsequent packets from the cluster, we select the
>next hop eth address and the port from the saved conntrack
>state.  This is straightforward as we anyway send the packet
>to the DNAT zone of the logical router.

Hi, Numan.  Thanks for the change!

It seems like you missed the update for system tests.  A few ECMP
system tests are failing in CI.

Also, not sure how big of a problem that is, but we may still spray
out uni-directional traffic.  For example, any UDP tunnels may still
be re-shuffled.  Do we need a solution for those?

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Update QEMU Git URLs.

[Ilya Maximets]

On 7/17/24 15:18, Simon Horman wrote:
> The current QEMU Git URLs appear to time out.
> Update it with a new links on gitlab.com, which according
> to the link below is currently the canonical repository for QEMU.
> 
> https://www.qemu.org/contribute/
> 
> Signed-off-by: Simon Horman 
> ---
>  Documentation/topics/dpdk/vhost-user.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index d9d87aa08727..5845d02352f8 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -312,7 +312,7 @@ predictable migration time. Mostly used as a second phase 
> after the normal
>  
>  More information can be found in QEMU `docs`_.
>  
> -.. _`docs`: 
> https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/migration.rst
> +.. _`docs`: 
> https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/postcopy.rst?ref_type=heads
>  

Maybe its better to point to the website instead?
e.g. https://www.qemu.org/docs/master/devel/migration/postcopy.html

>  Post-copy support may be enabled via a global config value
>  ``vhost-postcopy-support``. Setting this to ``true`` enables Post-copy 
> support
> @@ -487,7 +487,7 @@ Sample XML
>
>  
>  
> -.. _QEMU documentation: 
> http://git.qemu-project.org/?p=qemu.git;a=blob;f=docs/specs/vhost-user.txt;h=7890d7169;hb=HEAD
> +.. _QEMU documentation: 
> https://gitlab.com/qemu-project/qemu/-/blob/master/docs/system/devices/vhost-user.rst
>  

This link seems to not be the one.  The vhost-user.txt was
moved and renamed, so now it is available at:
  https://www.qemu.org/docs/master/interop/vhost-user.html

In the source tree it is in docs/interop/vhost-user.rst.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.

[Simon Horman]

On Wed, Jul 17, 2024 at 02:50:22PM +0100, Simon Horman wrote:
> On Fri, Jul 12, 2024 at 05:14:47PM +0200, David Marchand wrote:
> > On Fri, Jul 12, 2024 at 5:03 PM Eric Garver  wrote:
> > >
> > > IIRC, the out-of-tree modules have been removed since 3.0. I think this
> > > probe could be removed entirely.
> > 
> > I was wondering too if this code should be dropped, but at least from
> > a backport pov, this fix seems to make sense.
> >
> > > At any rate, giving my ACK for this patch.
> > >
> > > Acked-by: Eric Garver 
> > 
> > Thanks.
> 
> Thanks David and Eric,
> 
> Applied to main.
> 
> - dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
>   https://github.com/openvswitch/ovs/commit/03cd668e05c2
> 
> As a follow-up I'll work on backporting.

I have now pushed the following backports.

Backport to branch-3.4
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/86f7db6c1f08

Backport to branch-3.3
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/d2119feb0159

Backport to branch-3.2
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/6fd78d522c25

Backport to branch-3.1
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/d97c8a730a05

Backport to branch-3.0
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/56e545aeed19

Backport to branch-2.17
- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/f3d5ce5fad74
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.

[Simon Horman]

On Wed, Jul 17, 2024 at 07:19:36PM +0200, Ilya Maximets wrote:
> On 7/17/24 15:50, Simon Horman wrote:
> > On Fri, Jul 12, 2024 at 05:14:47PM +0200, David Marchand wrote:
> >> On Fri, Jul 12, 2024 at 5:03 PM Eric Garver  wrote:
> >>>
> >>> IIRC, the out-of-tree modules have been removed since 3.0. I think this
> >>> probe could be removed entirely.
> >>
> >> I was wondering too if this code should be dropped, but at least from
> >> a backport pov, this fix seems to make sense.
> >>
> >>> At any rate, giving my ACK for this patch.
> >>>
> >>> Acked-by: Eric Garver 
> >>
> >> Thanks.
> > 
> > Thanks David and Eric,
> > 
> > Applied to main.
> > 
> > - dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
> >   https://github.com/openvswitch/ovs/commit/03cd668e05c2
> > 
> > As a follow-up I'll work on backporting.
> > 
> > Please do consider following-up on removing the check entirely.
> 
> I think, we should keep the code as long as there is still one
> supported version (2.17) that technically ships the modules.
> We'll need to do a larger cleanup after 3.5 release when 2.17
> will go EoL.

Sure, that sounds reasonable to me.
I wonder if we can start making a list of things to clean up.

> Also, I wonder which interface windows tunnels are using...
> 
> Best regards, Ilya Maximets.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.

[Ilya Maximets]

On 7/17/24 15:50, Simon Horman wrote:
> On Fri, Jul 12, 2024 at 05:14:47PM +0200, David Marchand wrote:
>> On Fri, Jul 12, 2024 at 5:03 PM Eric Garver  wrote:
>>>
>>> IIRC, the out-of-tree modules have been removed since 3.0. I think this
>>> probe could be removed entirely.
>>
>> I was wondering too if this code should be dropped, but at least from
>> a backport pov, this fix seems to make sense.
>>
>>> At any rate, giving my ACK for this patch.
>>>
>>> Acked-by: Eric Garver 
>>
>> Thanks.
> 
> Thanks David and Eric,
> 
> Applied to main.
> 
> - dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
>   https://github.com/openvswitch/ovs/commit/03cd668e05c2
> 
> As a follow-up I'll work on backporting.
> 
> Please do consider following-up on removing the check entirely.

I think, we should keep the code as long as there is still one
supported version (2.17) that technically ships the modules.
We'll need to do a larger cleanup after 3.5 release when 2.17
will go EoL.

Also, I wonder which interface windows tunnels are using...

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpctl: Fix netdev reference leak in "show" command.

[Simon Horman]

On Wed, Jul 17, 2024 at 01:31:41PM +0100, Simon Horman wrote:
> On Tue, Jul 16, 2024 at 01:57:36PM +0530, Vipul Ashri wrote:
> > This specific Netdev leak is causing us stale VHU entries, where it
> > is showing false limit reaching maximum and preventing us to create
> > new entries for us. This leak can impact other nics also.
> > 
> > Steps to reproduce,
> > While running a test with a continous VM creation/deletion using an
> > orchestration script with-in cloud environment. In parallel we have
> > some monitoring script calling ovs-appctl dpctl/show stats commands
> > every minute.
> > 
> > Root-cause analysis,
> > During VHU port delete, one of netdev references were not reduced to
> > 0 as show_dpif call has not given-up the reference back or doing bad
> > cleanup. This pending deference preventing VHU deletion sequence, this
> > is found to be one of corner case inside dpctl code which results in
> > leaking up netdev which ultimately results in stale VHU entry. After
> > fixing this problematic cleanup, issue is not seen.
> > 
> > Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> > dpif-netdev")
> > Signed-off-by: Vipul Ashri 
> 
> Thanks,
> 
> Applied with David's tag from v1:
> 
> - dpctl: Fix netdev reference leak in "show" command.
>   https://github.com/openvswitch/ovs/commit/3985fa03b5c9
> 
> I will follow-up by working on backports.

I have now also applied the following backports.

Backport to branch-3.4
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/12f9e14104e8

Backport to branch-3.3
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/9c439c23e688

Backport to branch-3.2
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/8c1eca08fa55

Backport to branch-3.1
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/be777868541c

Backport to branch-3.0
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/e538108f8c64

Backport to branch-2.17
- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/d7fa3c3ff60b
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] ovs: Bump submodule to branch-3.4.

[Ilya Maximets]

Updating to OVS 3.4 to get enhanced sample action and other latest
features and fixes in general.

Unixctl library was changed to provide JSON-formatted output, so
clients has to be adjusted.  Clients also need to handle extra line
breaks at the end, since those are no longer prvided by the server.

We could define a common reply_to_string() function for both dbctl
and appctl, but these programs beahve differently in regerads to exit
codes and we'll need to do more refactoring to handle the difference.

Will move to v3.4.0 once it is available.

Signed-off-by: Ilya Maximets 
---
 controller/physical.c  |  4 ++--
 lib/actions.c  |  6 +++---
 ovs|  2 +-
 utilities/ovn-appctl.c | 42 --
 utilities/ovn-dbctl.c  | 40 +---
 5 files changed, 75 insertions(+), 19 deletions(-)

diff --git a/controller/physical.c b/controller/physical.c
index 22756810f..0255a1bbd 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -889,8 +889,8 @@ put_drop(const struct physical_debug *debug, uint8_t 
table_id,
 struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
 os->probability = UINT16_MAX;
 os->collector_set_id = debug->collector_set_id;
-os->obs_domain_id = (debug->obs_domain_id << 24);
-os->obs_point_id = table_id;
+os->obs_domain_imm = (debug->obs_domain_id << 24);
+os->obs_point_imm = table_id;
 os->sampling_port = OFPP_NONE;
 }
 }
diff --git a/lib/actions.c b/lib/actions.c
index e8cc0994d..37676ef81 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -4546,13 +4546,13 @@ encode_SAMPLE(const struct ovnact_sample *sample,
 os = ofpact_put_SAMPLE(ofpacts);
 os->probability = sample->probability;
 os->collector_set_id = sample->collector_set_id;
-os->obs_domain_id =
+os->obs_domain_imm =
 (sample->obs_domain_id << 24) | (ep->dp_key & 0xFF);
 
 if (sample->use_cookie) {
-os->obs_point_id = ep->lflow_uuid.parts[0];
+os->obs_point_imm = ep->lflow_uuid.parts[0];
 } else {
-os->obs_point_id = sample->obs_point_id;
+os->obs_point_imm = sample->obs_point_id;
 }
 os->sampling_port = OFPP_NONE;
 }
diff --git a/ovs b/ovs
index bf1b16364..0aa14d912 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit bf1b16364b3f01b0ff5f2f6e76842e666226a17b
+Subproject commit 0aa14d912d9a29d07ebc727007a1f21e3639eea5
diff --git a/utilities/ovn-appctl.c b/utilities/ovn-appctl.c
index 059492972..dff7d1295 100644
--- a/utilities/ovn-appctl.c
+++ b/utilities/ovn-appctl.c
@@ -27,6 +27,7 @@
 #include "lib/ovn-dirs.h"
 #include "lib/ovn-util.h"
 #include "openvswitch/dynamic-string.h"
+#include "openvswitch/json.h"
 #include "jsonrpc.h"
 #include "process.h"
 #include "timeval.h"
@@ -37,14 +38,16 @@
 static void usage(void);
 static const char *parse_command_line(int argc, char *argv[]);
 static struct jsonrpc *connect_to_target(const char *target);
+static char *reply_to_string(struct json *);
 
 int
 main(int argc, char *argv[])
 {
-char *cmd_result, *cmd_error;
+struct json *cmd_result, *cmd_error;
 struct jsonrpc *client;
 char *cmd, **cmd_argv;
 const char *target;
+char *msg = NULL;
 int cmd_argc;
 int error;
 
@@ -66,19 +69,23 @@ main(int argc, char *argv[])
 
 if (cmd_error) {
 jsonrpc_close(client);
-fputs(cmd_error, stderr);
+msg = reply_to_string(cmd_error);
+fputs(msg, stderr);
+free(msg);
 ovs_error(0, "%s: server returned an error", target);
-free(cmd_error);
+json_destroy(cmd_error);
 error ? exit(error) : exit(2);
 } else if (cmd_result) {
-fputs(cmd_result, stdout);
+msg = reply_to_string(cmd_result);
+fputs(msg, stdout);
+free(msg);
 } else {
 OVS_NOT_REACHED();
 }
 
 jsonrpc_close(client);
-free(cmd_result);
-free(cmd_error);
+json_destroy(cmd_result);
+json_destroy(cmd_error);
 return 0;
 }
 
@@ -239,3 +246,26 @@ connect_to_target(const char *target)
 return client;
 }
 
+/* The caller is responsible for freeing the returned string, with free(), when
+ * it is no longer needed. */
+static char *
+reply_to_string(struct json *reply)
+{
+ovs_assert(reply);
+
+if (reply->type != JSON_STRING) {
+ovs_error(0, "Unexpected reply type in JSON rpc reply: %s",
+  json_type_to_string(reply->type));
+exit(2);
+}
+
+struct ds ds = DS_EMPTY_INITIALIZER;
+
+ds_put_cstr(, json_string(reply));
+
+if (ds_last() != EOF && ds_last() != '\n') {
+ds_put_char(, '\n');
+}
+
+return ds_steal_cstr();
+}
diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
index 92be27b2c..4024261ef 100644
--- a/utilities/ovn-dbctl.c
+++ b/utilities/ovn-dbctl.c
@@ -1181,6 +1181,28 @@ server_loop(const struct ovn_dbctl_options 
*dbctl_options,
 

Re: [ovs-dev] [ovs-build] |fail| pw1960612 [ovs-dev, 2/2] Prepare for post-3.4.0 (3.4.90).

[Phelan, Michael]

> -Original Message-
> From: Ilya Maximets 
> Sent: Wednesday, July 17, 2024 10:43 AM
> To: Phelan, Michael 
> Cc: i.maxim...@ovn.org; ovs-dev 
> Subject: Re: [ovs-build] |fail| pw1960612 [ovs-dev,2/2] Prepare for post-
> 3.4.0 (3.4.90).
> 
> On 7/17/24 10:36, Phelan, Michael wrote:
> >
> >> -Original Message-
> >> From: Ilya Maximets 
> >> Sent: Tuesday, July 16, 2024 12:25 PM
> >> To: Phelan, Michael 
> >> Cc: i.maxim...@ovn.org; ovs-dev 
> >> Subject: Re: [ovs-build] |fail| pw1960612 [ovs-dev,2/2] Prepare for
> >> post-
> >> 3.4.0 (3.4.90).
> >>
> >> On 7/15/24 22:23, ovs_jenk...@intel.com wrote:
> >>> Test-Label: intel-ovs-compilation
> >>> Test-Status: fail
> >>> http://patchwork.ozlabs.org/api/patches/1960612/
> >>>
> >>> AVX-512_compilation: failed
> >>> DPLCS Test: fail
> >>> DPIF Test: fail
> >>> MFEX Test: fail
> >>> Actions Test: fail
> >>> Errors in DPCLS test:
> >>> make check-dpdk
> >>> make  all-am
> >>> make[1]: Entering directory '/root/ovs-dev'
> >>> make[1]: Leaving directory '/root/ovs-dev'
> >>> set /bin/bash './tests/system-dpdk-testsuite' -C tests
> >>> AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests:ipsec::'; \ "$@"
> >>> -j1 || (test X'' = Xyes && "$@" --recheck) ##
> >>> -- ## ## openvswitch 3.4.90 test suite.
> >>> ## ## -- ##
> >>>
> >>> OVS-DPDK unit tests
> >>>
> >>>   1: OVS-DPDK - EAL init ok
> >>>   2: OVS-DPDK - add standard DPDK port   ok
> >>>   3: OVS-DPDK - add vhost-user-client port   ok
> >>>   4: OVS-DPDK - ping vhost-user portsFAILED 
> >>> (ovs-macros.at:242)
> >>>   5: OVS-DPDK - ping vhost-user-client ports FAILED (ovs-
> macros.at:242)
> >>>   6: OVS-DPDK - Ingress policing create delete phy port ok
> >>>   7: OVS-DPDK - Ingress policing create delete vport port ok
> >>>   8: OVS-DPDK - Ingress policing no policing rateok
> >>>   9: OVS-DPDK - Ingress policing no policing burst   ok
> >>>  10: OVS-DPDK - QoS create delete phy port   ok
> >>>  11: OVS-DPDK - QoS create delete vport port ok
> >>>  12: OVS-DPDK - QoS no cir   ok
> >>>  13: OVS-DPDK - QoS no cbs   ok
> >>>  14: OVS-DPDK - MTU increase phy portok
> >>>  15: OVS-DPDK - MTU decrease phy portok
> >>>  16: OVS-DPDK - MTU increase vport port  FAILED (ovs-
> macros.at:242)
> >>>  17: OVS-DPDK - MTU decrease vport port  FAILED (ovs-
> >> macros.at:242)
> >>>  18: OVS-DPDK - MTU upper bound phy port ok
> >>>  19: OVS-DPDK - MTU lower bound phy port ok
> >>>  20: OVS-DPDK - MTU upper bound vport port   FAILED (ovs-
> >> macros.at:242)
> >>>  21: OVS-DPDK - MTU lower bound vport port   FAILED (ovs-
> >> macros.at:242)
> >>>  22: OVS-DPDK - user configured mempool  ok
> >>
> >> Hi, Michael.  Could you, please, check the reason why these tests are
> failing?
> >> The logs are truncated a little too much, so it's hard to tell what went
> wrong.
> > Hi Ilya,
> > The output in the logs is:
> >
> > ./system-dpdk.at:109: ovs-vsctl add-br br10 -- set bridge br10
> > datapath_type=netdev
> > ./system-dpdk.at:110: ovs-vsctl add-port br10 dpdkvhostuser0 -- set
> > Interface dpdkvhostuser0 type=dpdkvhostuser
> > stderr:
> > stdout:
> > system-dpdk.at:110: waiting until grep "VHOST_CONFIG:
> ($OVS_RUNDIR/dpdkvhostuser0) vhost-user server: socket created" ovs-
> vswitchd.log...
> > 2024-07-17T08:32:51.860Z|00062|dpdk|INFO|VHOST_CONFIG:
> > (/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0)
> > vhost-user server: socket created, fd: 95
> > system-dpdk.at:110: wait succeeded immediately
> > system-dpdk.at:110: waiting until grep "Socket
> $OVS_RUNDIR/dpdkvhostuser0 created for vhost-user port dpdkvhostuser0"
> ovs-vswitchd.log...
> > 2024-07-17T08:32:51.860Z|00063|netdev_dpdk|INFO|Socket
> > /root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0
> > created for vhost-user port dpdkvhostuser0
> > system-dpdk.at:110: wait succeeded immediately
> > system-dpdk.at:110: waiting until grep "VHOST_CONFIG:
> ($OVS_RUNDIR/dpdkvhostuser0) binding succeeded" ovs-vswitchd.log...
> > 2024-07-17T08:32:51.861Z|00064|dpdk|INFO|VHOST_CONFIG:
> > (/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0)
> > binding succeeded
> > system-dpdk.at:110: wait succeeded immediately
> > ./system-dpdk.at:111: ovs-vsctl show
> > stdout:
> > 45be2775-e3c2-45bc-9d5e-2eb502748c78
> > Bridge br10
> > datapath_type: netdev
> > Port br10
> > Interface br10
> > type: internal
> > Port dpdkvhostuser0
> > Interface dpdkvhostuser0
> > type: dpdkvhostuser
> > Cannot remove namespace file "/run/netns/ns1": No such file or
> > directory
> > ./system-dpdk.at:114: ip netns add ns1 || return 77
> > net.netfilter.nf_conntrack_helper 

Re: [ovs-dev] [PATCH 3/3] match: Fix false-positive snprintf size warning.

[Mike Pattrick]

On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>
> GCC 14.1.1 of Fedora 41 thinks that 'i' can be in a full range and
> so 8 bytes is not enough to print it.
>
>  lib/match.c: In function 'match_format':
>  lib/match.c:1631:45:
>  error: '%d' directive output may be truncated writing
> between 1 and 11 bytes into a region of size 8
>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>| ^~
>  lib/match.c:1631:44:
>note: directive argument in the range [-2147483646, 1]
>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>|^~~~
>  lib/match.c:1631:13:
>note: 'snprintf' output between 2 and 12 bytes into
>  a destination of size 8
>   1631 | snprintf(str_i, sizeof(str_i), "%d", i);
>| ^~~
>
> In practice that value can't be larger than 2, but it's not a
> performance critical code, so let's just increase the size to
> a maximum 12.
>
> Signed-off-by: Ilya Maximets 


Acked-by: Mike Pattrick 

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

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.

[Simon Horman]

On Fri, Jul 12, 2024 at 05:14:47PM +0200, David Marchand wrote:
> On Fri, Jul 12, 2024 at 5:03 PM Eric Garver  wrote:
> >
> > IIRC, the out-of-tree modules have been removed since 3.0. I think this
> > probe could be removed entirely.
> 
> I was wondering too if this code should be dropped, but at least from
> a backport pov, this fix seems to make sense.
>
> > At any rate, giving my ACK for this patch.
> >
> > Acked-by: Eric Garver 
> 
> Thanks.

Thanks David and Eric,

Applied to main.

- dpif-netlink-rtnl: Fix netdev leak in out-of-tree tunnels probe.
  https://github.com/openvswitch/ovs/commit/03cd668e05c2

As a follow-up I'll work on backporting.

Please do consider following-up on removing the check entirely.

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

Re: [ovs-dev] [PATCH v3 0/3] Fix redundant mirror with tunnel options.

[Simon Horman]

On Wed, Jul 17, 2024 at 12:54:31PM +0100, Simon Horman wrote:
> On Mon, Jul 15, 2024 at 11:48:17AM -0400, Mike Pattrick wrote:
> > On Fri, Oct 13, 2023 at 6:59 AM Eelco Chaudron  wrote:
> > >
> > >
> > >
> > > On 9 Oct 2023, at 14:05, Roi Dayan wrote:
> > >
> > > > Hi,
> > > >
> > > > The first commit removing the redundant mirror when using tunnel 
> > > > options.
> > > > The second is a test to verify it stays like this and doesn't break 
> > > > again.
> > > > The third commit is just updating prefixes of tests to match the file 
> > > > their in.
> > > >
> > > > Thanks,
> > > > Roi
> > >
> > > I’ve applied the series to master. Once again, thanks for the patch.
> > 
> > Hello,
> > 
> > Is it possible to apply this bugfix to 3.1 and 3.2 as well?
> 
> Thanks Mike,
> 
> I will look into making this so.

I have backported:
* [PATCH 1/3] ofproto-dpif-upcall: Fix redundant mirror on geneve tunnel 
options.
* [PATCH 2/3] tests/tunnel.at: Add geneve options mirror test.

But not:
* [PATCH 3/3] tests: Update some tests title prefix print.

I didn't backport 3/3 because it doesn't really seem to be a fix.

Backport to branch-3.2:
- tests/tunnel.at: Add geneve options mirror test.
  https://github.com/openvswitch/ovs/commit/d5d77951d9c9
- ofproto-dpif-upcall: Fix redundant mirror on geneve tunnel options.
  https://github.com/openvswitch/ovs/commit/a4f3079effbc

Backport to branch-3.1:
- tests/tunnel.at: Add geneve options mirror test.
  https://github.com/openvswitch/ovs/commit/9587dcd9063d
- ofproto-dpif-upcall: Fix redundant mirror on geneve tunnel options.
  https://github.com/openvswitch/ovs/commit/b7d325714beb
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Update QEMU Git URLs.

[0-day Robot]

Bleep bloop.  Greetings Simon Horman, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 111 characters long (recommended limit is 79)
#26 FILE: Documentation/topics/dpdk/vhost-user.rst:315:
.. _`docs`: 
https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/postcopy.rst?ref_type=heads

WARNING: Line is 109 characters long (recommended limit is 79)
#35 FILE: Documentation/topics/dpdk/vhost-user.rst:490:
.. _QEMU documentation: 
https://gitlab.com/qemu-project/qemu/-/blob/master/docs/system/devices/vhost-user.rst

Lines checked: 42, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] util: Add non-NULL format assertion to xvasprintf.

[Mike Pattrick]

On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>
> For some reason GCC 14.1.1 on Fedora 41 assumes that format can
> be NULL and emits a warning:
>
>  lib/util.c: In function 'xvasprintf':
>  lib/util.c:229:14: error: null format string
>229 | needed = vsnprintf(NULL, 0, format, args);
>|  ^~~~
>
> I didn't find any users where this can be true.  Adding an
> assertion to silence the warning.  In the worst case we'll
> find out where it is being called incorrectly.
>
> Signed-off-by: Ilya Maximets 

Acked-by: Mike Pattrick 

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

Re: [ovs-dev] [PATCH 1/3] ofproto-dpif-xlate: Initialize observe_offset for sample actions.

[Mike Pattrick]

On Wed, Jul 17, 2024 at 8:01 AM Ilya Maximets  wrote:
>
> For some reason gcc 14.1.1 from Fedora 41 thinks that the variable
> may end up not initialized:
>
>  ofproto/ofproto-dpif-xlate.c: In function 'compose_sample_action':
>  ofproto/ofproto-dpif-xlate.c:3465:40:
>   error: 'observe_offset' may be used uninitialized
>   3465 | ctx->xout->last_observe_offset = observe_offset;
>| ~~~^~~~
>  ofproto/ofproto-dpif-xlate.c:3418:12:
>   note: 'observe_offset' was declared here
>   3418 | size_t observe_offset;
>|^~
>
> We have an assertion in the code to ensure that at least one of
> the actions is present (userspace or psample), so the variable
> should actually be always initialized.
>
> Initialize explicitly just to silence the warning.
>
> Fixes: 516569d31fbf ("ofproto: xlate: Make sampled drops explicit.")
> Signed-off-by: Ilya Maximets 
> ---

Acked-by: Mike Pattrick 

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

[ovs-dev] [PATCH] AUTHORS: Add Vipul Ashri.

[Simon Horman]

Add Vipul Ashri to AUTHORS file.

Signed-off-by: Simon Horman 
---
 AUTHORS.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 155e484360d9..28dcce4eaf79 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -760,6 +760,7 @@ Tytus Kurek tytus.ku...@pega.com
 Valentin Budvalen...@hackaserver.com
 Vasiliy Tolstov v.tols...@selfip.ru
 Vinllen Chencvinl...@gmail.com
+Vipul Ashri vipul.as...@ericsson.com
 Vishal Swarankarvishal.swarn...@gmail.com
 Vjekoslav Brajkovic bal...@cs.washington.edu
 Voravit T.  vora...@kth.se

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

Re: [ovs-dev] [PATCH v1 1/1] datapath-windows: Merge split dis-continuous net-buf.

[Wilson Peng via dev]

Hi, Ilya,

The patch is adding a fix for the current IPv4 traffic not supporting
non-contiguous
header issues on WIndows2019 and Windows2022.  Actually there are several
places
in ovs-windows still using NdisGetDataBuffer directly. We would replace
them with the
mentioned helper function like OvsGetTcp or OvsGetPacketBytes in other
patches after
The needed testing is done. Not sure if this kind of strategy is accepted?

Regards
Wilson

On Wed, Jul 17, 2024 at 8:10 PM Ilya Maximets  wrote:

> On 7/17/24 13:42, Wilson Peng via dev wrote:
> > From: Wilson Peng 
> >
> > NdisGetDataBuffer() is called without providing a buffer to copy packet
> > data in case it is not contiguous.  So, it fails in some scenarios
> > where the packet is handled by the general network stack before OVS
> > and headers become split in multiple buffers.
> >
> > In the fix it will supply the stack buffer to copy packet data when
> > call NdisGetDataBuffer().
> >
> > In the conntrack Action process, it will do OvsPartialCopyNBL firstly
> > with the size of layers l7offsets. If the header is split the header
> > will be merged to one continuous buffer.
> >
> > But IPV6 traffic is not handed in this case.
> >
> > Reported-at: https://github.com/openvswitch/ovs-issues/issues/323
> > Signed-off-by: Wilson Peng 
> > ---
> >  datapath-windows/ovsext/Actions.c|  7 +++
> >  datapath-windows/ovsext/BufferMgmt.c | 16 +---
> >  datapath-windows/ovsext/Conntrack.c  |  4 +++-
> >  datapath-windows/ovsext/IpFragment.c |  8 ++--
> >  4 files changed, 29 insertions(+), 6 deletions(-)
>
> Hi, Wilson.  Thanks for the patch!
>
> Though I wonder why are we using NdisGetDataBuffer directly at all?
> We have helper functions OvsGetTcp or OvsGetPacketBytes and these
> should be able to handle non-contiguous memory.  I tried to use them
> in my previous attempt to fix this issue here:
>
> https://patchwork.ozlabs.org/project/openvswitch/patch/20240606105348.2354265-1-i.maxim...@ovn.org/
> Though I definitely missed a few places and so the patch doesn't work
> in its current form.
>
> Best regards, Ilya Maximets.
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Documentation: Update QEMU Git URLs.

[Simon Horman]

The current QEMU Git URLs appear to time out.
Update it with a new links on gitlab.com, which according
to the link below is currently the canonical repository for QEMU.

https://www.qemu.org/contribute/

Signed-off-by: Simon Horman 
---
 Documentation/topics/dpdk/vhost-user.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/topics/dpdk/vhost-user.rst 
b/Documentation/topics/dpdk/vhost-user.rst
index d9d87aa08727..5845d02352f8 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -312,7 +312,7 @@ predictable migration time. Mostly used as a second phase 
after the normal
 
 More information can be found in QEMU `docs`_.
 
-.. _`docs`: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/migration.rst
+.. _`docs`: 
https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/postcopy.rst?ref_type=heads
 
 Post-copy support may be enabled via a global config value
 ``vhost-postcopy-support``. Setting this to ``true`` enables Post-copy support
@@ -487,7 +487,7 @@ Sample XML
   
 
 
-.. _QEMU documentation: 
http://git.qemu-project.org/?p=qemu.git;a=blob;f=docs/specs/vhost-user.txt;h=7890d7169;hb=HEAD
+.. _QEMU documentation: 
https://gitlab.com/qemu-project/qemu/-/blob/master/docs/system/devices/vhost-user.rst
 
 Jumbo Frames
 

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

Re: [ovs-dev] [PATCH v2] dpctl: Fix netdev reference leak in "show" command.

[Simon Horman]

On Tue, Jul 16, 2024 at 01:57:36PM +0530, Vipul Ashri wrote:
> This specific Netdev leak is causing us stale VHU entries, where it
> is showing false limit reaching maximum and preventing us to create
> new entries for us. This leak can impact other nics also.
> 
> Steps to reproduce,
> While running a test with a continous VM creation/deletion using an
> orchestration script with-in cloud environment. In parallel we have
> some monitoring script calling ovs-appctl dpctl/show stats commands
> every minute.
> 
> Root-cause analysis,
> During VHU port delete, one of netdev references were not reduced to
> 0 as show_dpif call has not given-up the reference back or doing bad
> cleanup. This pending deference preventing VHU deletion sequence, this
> is found to be one of corner case inside dpctl code which results in
> leaking up netdev which ultimately results in stale VHU entry. After
> fixing this problematic cleanup, issue is not seen.
> 
> Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> dpif-netdev")
> Signed-off-by: Vipul Ashri 

Thanks,

Applied with David's tag from v1:

- dpctl: Fix netdev reference leak in "show" command.
  https://github.com/openvswitch/ovs/commit/3985fa03b5c9

I will follow-up by working on backports.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] datapath-windows: Merge split dis-continuous net-buf.

[Ilya Maximets]

On 7/17/24 13:42, Wilson Peng via dev wrote:
> From: Wilson Peng 
> 
> NdisGetDataBuffer() is called without providing a buffer to copy packet
> data in case it is not contiguous.  So, it fails in some scenarios
> where the packet is handled by the general network stack before OVS
> and headers become split in multiple buffers.
> 
> In the fix it will supply the stack buffer to copy packet data when
> call NdisGetDataBuffer().
> 
> In the conntrack Action process, it will do OvsPartialCopyNBL firstly
> with the size of layers l7offsets. If the header is split the header
> will be merged to one continuous buffer.
> 
> But IPV6 traffic is not handed in this case.
> 
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/323
> Signed-off-by: Wilson Peng 
> ---
>  datapath-windows/ovsext/Actions.c|  7 +++
>  datapath-windows/ovsext/BufferMgmt.c | 16 +---
>  datapath-windows/ovsext/Conntrack.c  |  4 +++-
>  datapath-windows/ovsext/IpFragment.c |  8 ++--
>  4 files changed, 29 insertions(+), 6 deletions(-)

Hi, Wilson.  Thanks for the patch!

Though I wonder why are we using NdisGetDataBuffer directly at all?
We have helper functions OvsGetTcp or OvsGetPacketBytes and these
should be able to handle non-contiguous memory.  I tried to use them
in my previous attempt to fix this issue here:
  
https://patchwork.ozlabs.org/project/openvswitch/patch/20240606105348.2354265-1-i.maxim...@ovn.org/
Though I definitely missed a few places and so the patch doesn't work
in its current form.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 3/3] match: Fix false-positive snprintf size warning.

[Ilya Maximets]

GCC 14.1.1 of Fedora 41 thinks that 'i' can be in a full range and
so 8 bytes is not enough to print it.

 lib/match.c: In function 'match_format':
 lib/match.c:1631:45:
 error: '%d' directive output may be truncated writing
between 1 and 11 bytes into a region of size 8
  1631 | snprintf(str_i, sizeof(str_i), "%d", i);
   | ^~
 lib/match.c:1631:44:
   note: directive argument in the range [-2147483646, 1]
  1631 | snprintf(str_i, sizeof(str_i), "%d", i);
   |^~~~
 lib/match.c:1631:13:
   note: 'snprintf' output between 2 and 12 bytes into
 a destination of size 8
  1631 | snprintf(str_i, sizeof(str_i), "%d", i);
   | ^~~

In practice that value can't be larger than 2, but it's not a
performance critical code, so let's just increase the size to
a maximum 12.

Signed-off-by: Ilya Maximets 
---
 lib/match.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/match.c b/lib/match.c
index 0b9dc4278..9b7e06e0c 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1618,7 +1618,7 @@ match_format(const struct match *match,
 ds_put_char(s, ',');
 }
 for (i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
-char str_i[8];
+char str_i[12];
 
 if (!wc->masks.vlans[i].tci) {
 break;
-- 
2.45.2

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

[ovs-dev] [PATCH 2/3] util: Add non-NULL format assertion to xvasprintf.

[Ilya Maximets]

For some reason GCC 14.1.1 on Fedora 41 assumes that format can
be NULL and emits a warning:

 lib/util.c: In function 'xvasprintf':
 lib/util.c:229:14: error: null format string
   229 | needed = vsnprintf(NULL, 0, format, args);
   |  ^~~~

I didn't find any users where this can be true.  Adding an
assertion to silence the warning.  In the worst case we'll
find out where it is being called incorrectly.

Signed-off-by: Ilya Maximets 
---
 lib/util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/util.c b/lib/util.c
index 5253921b2..bdd6408b2 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -225,6 +225,8 @@ xvasprintf(const char *format, va_list args)
 size_t needed;
 char *s;
 
+ovs_assert(format);
+
 va_copy(args2, args);
 needed = vsnprintf(NULL, 0, format, args);
 
-- 
2.45.2

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

[ovs-dev] [PATCH 1/3] ofproto-dpif-xlate: Initialize observe_offset for sample actions.

[Ilya Maximets]

For some reason gcc 14.1.1 from Fedora 41 thinks that the variable
may end up not initialized:

 ofproto/ofproto-dpif-xlate.c: In function 'compose_sample_action':
 ofproto/ofproto-dpif-xlate.c:3465:40:
  error: 'observe_offset' may be used uninitialized
  3465 | ctx->xout->last_observe_offset = observe_offset;
   | ~~~^~~~
 ofproto/ofproto-dpif-xlate.c:3418:12:
  note: 'observe_offset' was declared here
  3418 | size_t observe_offset;
   |^~

We have an assertion in the code to ensure that at least one of
the actions is present (userspace or psample), so the variable
should actually be always initialized.

Initialize explicitly just to silence the warning.

Fixes: 516569d31fbf ("ofproto: xlate: Make sampled drops explicit.")
Signed-off-by: Ilya Maximets 
---
 ofproto/ofproto-dpif-xlate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 02567a961..850597b3a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3423,8 +3423,8 @@ compose_sample_action(struct xlate_ctx *ctx,
  * insert a meter action before the user space action.  */
 struct ofproto *ofproto = >xin->ofproto->up;
 uint32_t meter_id = ofproto->slowpath_meter_id;
+size_t observe_offset = UINT32_MAX;
 size_t cookie_offset = 0;
-size_t observe_offset;
 
 /* The meter action is only used to throttle userspace actions.
  * If they are not needed and the sampling rate is 100%, avoid generating
-- 
2.45.2

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

[ovs-dev] [PATCH 0/3] Fix some GCC 14 false-positive build warnings.

[Ilya Maximets]

GCC 14.1.1 on Fedora 41 fails to build reporting some warnings when
building with '-fsanitize=address,undefined'.

Ilya Maximets (3):
  ofproto-dpif-xlate: Initialize observe_offset for sample actions.
  util: Add non-NULL format assertion to xvasprintf.
  match: Fix false-positive snprintf size warning.

 lib/match.c  | 2 +-
 lib/util.c   | 2 ++
 ofproto/ofproto-dpif-xlate.c | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.45.2

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

Re: [ovs-dev] [PATCH] docs: Define Read the Docs configuration for Sphinx HTML parameters.

[Simon Horman]

On Wed, Jul 17, 2024 at 12:55:02PM +0200, Ilya Maximets wrote:
> Read the Docs was always mangling the conf.py during the build to
> inject custom domains configured in the project settings and some
> other stuff.  But they will stop doing that soon [1].
> 
> Adding recommended changes to the config to get this info from the
> environment.
> 
> [1] https://about.readthedocs.com/blog/2024/07/addons-by-default/
> 
> Signed-off-by: Ilya Maximets 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH v3 0/3] Fix redundant mirror with tunnel options.

[Simon Horman]

On Mon, Jul 15, 2024 at 11:48:17AM -0400, Mike Pattrick wrote:
> On Fri, Oct 13, 2023 at 6:59 AM Eelco Chaudron  wrote:
> >
> >
> >
> > On 9 Oct 2023, at 14:05, Roi Dayan wrote:
> >
> > > Hi,
> > >
> > > The first commit removing the redundant mirror when using tunnel options.
> > > The second is a test to verify it stays like this and doesn't break again.
> > > The third commit is just updating prefixes of tests to match the file 
> > > their in.
> > >
> > > Thanks,
> > > Roi
> >
> > I’ve applied the series to master. Once again, thanks for the patch.
> 
> Hello,
> 
> Is it possible to apply this bugfix to 3.1 and 3.2 as well?

Thanks Mike,

I will look into making this so.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1 1/1] datapath-windows: Merge split dis-continuous net-buf.

[Wilson Peng via dev]

From: Wilson Peng 

NdisGetDataBuffer() is called without providing a buffer to copy packet
data in case it is not contiguous.  So, it fails in some scenarios
where the packet is handled by the general network stack before OVS
and headers become split in multiple buffers.

In the fix it will supply the stack buffer to copy packet data when
call NdisGetDataBuffer().

In the conntrack Action process, it will do OvsPartialCopyNBL firstly
with the size of layers l7offsets. If the header is split the header
will be merged to one continuous buffer.

But IPV6 traffic is not handed in this case.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/323
Signed-off-by: Wilson Peng 
---
 datapath-windows/ovsext/Actions.c|  7 +++
 datapath-windows/ovsext/BufferMgmt.c | 16 +---
 datapath-windows/ovsext/Conntrack.c  |  4 +++-
 datapath-windows/ovsext/IpFragment.c |  8 ++--
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 97029b0f4..4bc2a6b65 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2414,6 +2414,13 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 
 PNET_BUFFER_LIST oldNbl = ovsFwdCtx.curNbl;
+PUINT8 bufferStart = NULL;
+
+bufferStart = OvsGetHeaderBySize(, layers->l7Offset);
+if (!bufferStart) {
+ dropReason = L"OVS-Netbuf reallocated failed";
+ goto dropit;
+}
 status = OvsExecuteConntrackAction(, key,
(const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index 5c52757a0..40faaefd6 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -1110,12 +1110,22 @@ GetIpHeaderInfo(PNET_BUFFER_LIST curNbl,
 IPHdr *ipHdr;
 IPv6Hdr *ipv6Hdr;
 PNET_BUFFER curNb;
+CHAR tempBuf[MAX_IPV4_HLEN];
 
 curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
 ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
-eth = (EthHdr *)NdisGetDataBuffer(curNb,
-  hdrInfo->l4Offset,
-  NULL, 1, 0);
+
+if (hdrInfo->isIPv6) {
+eth = (EthHdr *)NdisGetDataBuffer(curNb,
+  hdrInfo->l4Offset,
+  NULL, 1, 0);
+} else {
+NdisZeroMemory(tempBuf, MAX_IPV4_HLEN);
+eth = (EthHdr *)NdisGetDataBuffer(curNb,
+  hdrInfo->l4Offset,
+  (PVOID)tempBuf, 1, 0);
+}
+
 if (eth == NULL) {
 return NDIS_STATUS_INVALID_PACKET;
 }
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 39ba5cc10..1a1794fc1 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -683,6 +683,7 @@ OvsGetTcpHeader(PNET_BUFFER_LIST nbl,
 TCPHdr *tcp;
 VOID *dest = storage;
 uint16_t ipv6ExtLength = 0;
+CHAR tempBuf[MAX_IPV4_HLEN];
 
 if (layers->isIPv6) {
 ipv6Hdr = NdisGetDataBuffer(NET_BUFFER_LIST_FIRST_NB(nbl),
@@ -701,9 +702,10 @@ OvsGetTcpHeader(PNET_BUFFER_LIST nbl,
 return storage;
 }
 } else {
+NdisZeroMemory(tempBuf, MAX_IPV4_HLEN);
 ipHdr = NdisGetDataBuffer(NET_BUFFER_LIST_FIRST_NB(nbl),
   layers->l4Offset + sizeof(TCPHdr),
-  NULL, 1 /*no align*/, 0);
+  (PVOID)tempBuf, 1 /*no align*/, 0);
 if (ipHdr == NULL) {
 return NULL;
 }
diff --git a/datapath-windows/ovsext/IpFragment.c 
b/datapath-windows/ovsext/IpFragment.c
index afb8e50d6..4e119d82a 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -153,12 +153,14 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
 PNET_BUFFER_LIST newNbl = NULL;
 UINT16 ipHdrLen, packetHeader;
 UINT32 packetLen;
+CHAR tempBuf[MAX_IPV4_HLEN];
 
 curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl);
 ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
+NdisZeroMemory(tempBuf, MAX_IPV4_HLEN);
 eth = (EthHdr*)NdisGetDataBuffer(curNb, layers->l4Offset,
- NULL, 1, 0);
+ (PVOID)tempBuf, 1, 0);
 if (eth == NULL) {
 return NDIS_STATUS_INVALID_PACKET;
 }
@@ -253,12 +255,14 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
 POVS_IPFRAG_ENTRY entry;
 POVS_FRAGMENT_LIST fragStorage;
 LOCK_STATE_EX htLockState;
+CHAR tempBuf[MAX_IPV4_HLEN];
 
 curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl);
 ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
+

[ovs-dev] [PATCH] docs: Define Read the Docs configuration for Sphinx HTML parameters.

[Ilya Maximets]

Read the Docs was always mangling the conf.py during the build to
inject custom domains configured in the project settings and some
other stuff.  But they will stop doing that soon [1].

Adding recommended changes to the config to get this info from the
environment.

[1] https://about.readthedocs.com/blog/2024/07/addons-by-default/

Signed-off-by: Ilya Maximets 
---
 Documentation/conf.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..2364405ad 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -12,6 +12,7 @@
 # All configuration values have a default; values that are commented out
 # serve to show the default.
 
+import os
 import string
 import sys
 
@@ -108,6 +109,13 @@ html_logo = '_static/logo.png'
 # so a file named "default.css" will overwrite the builtin "default.css".
 html_static_path = ['_static']
 
+# Define the canonical URL for our domain configured on Read the Docs.
+html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "")
+
+# Tell Jinja2 templates the build is running on Read the Docs.
+html_context = {}
+if os.environ.get("READTHEDOCS", "") == "True":
+html_context["READTHEDOCS"] = True
 
 # -- Options for manual page output ---
 
-- 
2.45.2

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

[ovs-dev] [PATCH ovn] northd: Add change handler for FDB updates.

[Naveen Yerramneni]

This change avoids northd full recompute for FDB updates.

Signed-off-by: Naveen Yerramneni 
---
 northd/en-northd.c   | 11 +++
 northd/en-northd.h   |  1 +
 northd/inc-proc-northd.c |  2 +-
 northd/northd.c  |  2 +-
 northd/northd.h  |  2 ++
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/northd/en-northd.c b/northd/en-northd.c
index 4479b4aff..4f84a8717 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -259,3 +259,14 @@ en_northd_clear_tracked_data(void *data_)
 struct northd_data *data = data_;
 destroy_northd_data_tracked_changes(data);
 }
+
+bool
+sb_fdb_change_handler(struct engine_node *node, void *data)
+{
+struct northd_data *nd = data;
+const struct sbrec_fdb_table *sbrec_fdb_table =
+EN_OVSDB_GET(engine_get_input("SB_fdb", node));
+cleanup_stale_fdb_entries(sbrec_fdb_table, >ls_datapaths.datapaths);
+engine_set_node_state(node, EN_UPDATED);
+return true;
+}
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 9b7bda32a..9c722e401 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -19,5 +19,6 @@ bool northd_nb_logical_switch_handler(struct engine_node *, 
void *data);
 bool northd_nb_logical_router_handler(struct engine_node *, void *data);
 bool northd_sb_port_binding_handler(struct engine_node *, void *data);
 bool northd_lb_data_handler(struct engine_node *, void *data);
+bool sb_fdb_change_handler(struct engine_node *node, void *data);
 
 #endif /* EN_NORTHD_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index d56e9783a..213e6d88a 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -189,7 +189,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
 engine_add_input(_northd, _sb_ha_chassis_group, NULL);
 engine_add_input(_northd, _sb_ip_multicast, NULL);
 engine_add_input(_northd, _sb_service_monitor, NULL);
-engine_add_input(_northd, _sb_fdb, NULL);
+engine_add_input(_northd, _sb_fdb, sb_fdb_change_handler);
 engine_add_input(_northd, _sb_static_mac_binding, NULL);
 engine_add_input(_northd, _sb_chassis_template_var, NULL);
 engine_add_input(_northd, _global_config,
diff --git a/northd/northd.c b/northd/northd.c
index 6898daa00..447d4fd19 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3270,7 +3270,7 @@ cleanup_sb_ha_chassis_groups(
 }
 }
 
-static void
+void
 cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table,
   struct hmap *ls_datapaths)
 {
diff --git a/northd/northd.h b/northd/northd.h
index d4a8d75ab..e224a073e 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -731,6 +731,8 @@ bool sync_pbs_for_northd_changed_ovn_ports(
 struct tracked_ovn_ports *,
 const struct lr_stateful_table *);
 
+void cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table,
+  struct hmap *ls_datapaths);
 static inline bool
 northd_has_tracked_data(struct northd_tracked_data *trk_nd_changes) {
 return trk_nd_changes->type != NORTHD_TRACKED_NONE;
-- 
2.36.6

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

[ovs-dev] Fwd: ovn-controller generates some non-expected openflows to forward packets to tunnel ports in table 37 when a localnet port existed on the datapath

[Jun Gu]

Hi team
  The version is below:
OVN version: branch-21.09
OVS version: branch-2.16.2
  we encounter a issue that ovn-controller generates some non-expected 
openflows to forward packets to tunnel ports in table 37 when a localnet 
port existed on the datapath. The related information is below:

  - Related datapath and ports.
```
switch d9954742-5f9a-440d-8f4a-e9cba08ff5b3 
(neutron-102c1351-65a6-4640-8310-c3cd446a5b69) (aka JSNX_APP_VLAN121)
port b4decafe-1948-4214-b021-393953bf94c3 (aka 
sc-arm-lnbp-clbatch-13_JSNX_APP_VLAN121_d36c91ab)

addresses: ["fa:16:3e:32:ca:7e 32.12.121.150"]
port 87e5ac12-80b8-4477-af59-dfb0fc0a43ff (aka 
sc-arm-lnbp-pfservice-6_JSNX_APP_VLAN121_109c4c56)

addresses: ["fa:16:3e:e7:58:37 32.12.121.103"]
port ad722de8-20ca-4f52-97bf-df29f9512d6e
type: localport
addresses: ["fa:16:3e:3d:96:cd 32.12.121.10"]
...
port 067b57c8-2e50-4a86-a255-31e799a24ac5 (aka 
sc-arm-lnbp-taebatch-5_JSNX_APP_VLAN121_a029cd93)

addresses: ["fa:16:3e:e5:0e:10 32.12.121.151"]
port d2479952-a2e2-4c94-b5d9-5086f5a177ee (aka 
sc-arm-lnbp-mars-3_JSNX_APP_VLAN121_319305dc)

addresses: ["fa:16:3e:7c:4e:35 32.12.121.194"]
port 64ddd42f-6a70-4060-8782-bd5cad7c4983 (aka 
sc-arm-lnbp-clbatch-8_JSNX_APP_VLAN121_7a4ca62f)

addresses: ["fa:16:3e:9c:56:aa 32.12.121.213"]
port provnet-6612e9ff-5364-4788-9b27-0a8c6053bec3
type: localnet
tag: 121
addresses: ["unknown"]
port f004de9d-5e52-43c7-9094-364a07ac420f (aka 
sc-arm-lnbp-influxdb-3_JSNX_APP_VLAN121_7527d6dc)

addresses: ["fa:16:3e:96:6b:45 32.12.121.231"]
```
  - Related openflows.
```
 cookie=0xd9b58719, duration=3803793.502s, table=37, n_packets=0, 
n_bytes=0, idle_age=65535, priority=100,reg15=0x37,metadata=0xd 
actions=set_field:0xd/0xff->tun_id,set_field:0x37->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:13
 cookie=0x997eb173, duration=3803793.502s, table=37, n_packets=0, 
n_bytes=0, idle_age=65535, priority=100,reg15=0x8005,metadata=0xd 
actions=set_field:0x2->reg15,resubmit(,39),set_field:0x8005->reg15,set_field:0xd/0xff->tun_id,set_field:0x8005->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:300,output:14,output:138,output:137,output:299,output:140,output:304,output:303,output:142,output:1,output:302,output:139,output:301,output:13,output:11,output:10,resubmit(,38)
 cookie=0xce52adfe, duration=3803793.502s, table=37, n_packets=2475804, 
n_bytes=162274574, idle_age=0, priority=100,reg15=0x8000,metadata=0xd 
actions=set_field:0x2->reg15,resubmit(,39),set_field:0x8000->reg15,set_field:0xd/0xff->tun_id,set_field:0x8000->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:300,output:14,output:138,output:137,output:299,output:140,output:304,output:303,output:142,output:1,output:302,output:139,output:301,output:13,output:11,output:10,resubmit(,38)
 cookie=0xb9dee9ea, duration=3803793.502s, table=37, n_packets=0, 
n_bytes=0, idle_age=65535, priority=100,reg15=0x8003,metadata=0xd 
actions=set_field:0xd/0xff->tun_id,set_field:0x8003->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:300,output:14,output:138,output:137,output:299,output:140,output:304,output:303,output:142,output:1,output:302,output:139,output:301,output:13,output:11,output:10,resubmit(,38)

```
  The datapath tunnel_key is 0xd, its localnet port tunnel_key is 0x1 
and its localport port tunnel_key is 0x2. From openflows related 0xd 
datapath, the localnet port has not been acquired to generate the table 
37 non-expected openflows. And other tables can identify the localnet 
port, so other tables which generate openflows are correct. For example, 
a openflow from table 38

```
 cookie=0xce52adfe, duration=3803727.055s, table=38, n_packets=2475787, 
n_bytes=162273290, idle_age=0, priority=100,reg15=0x8000,metadata=0xd 
actions=set_field:0x1->reg15,resubmit(,39),set_field:0x70->reg13,set_field:0x3->reg15,resubmit(,39),set_field:0x72->reg13,set_field:0x26->reg13,set_field:0x29->reg15,resubmit(,39),set_field:0x39->reg13,set_field:0x33->reg15,resubmit(,39),set_field:0x8000->reg15

```
  Based on the above information, we analyze the related ovn-controller 
codes. Only one condition that the localnet port is not queried by 
get_localnet_port function, the issue will occur.
  However, a deeper analysis of the code did not yield more useful 
information.





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

Re: [ovs-dev] [ovs-build] |fail| pw1960612 [ovs-dev, 2/2] Prepare for post-3.4.0 (3.4.90).

[Ilya Maximets]

On 7/17/24 10:36, Phelan, Michael wrote:
> 
>> -Original Message-
>> From: Ilya Maximets 
>> Sent: Tuesday, July 16, 2024 12:25 PM
>> To: Phelan, Michael 
>> Cc: i.maxim...@ovn.org; ovs-dev 
>> Subject: Re: [ovs-build] |fail| pw1960612 [ovs-dev,2/2] Prepare for post-
>> 3.4.0 (3.4.90).
>>
>> On 7/15/24 22:23, ovs_jenk...@intel.com wrote:
>>> Test-Label: intel-ovs-compilation
>>> Test-Status: fail
>>> http://patchwork.ozlabs.org/api/patches/1960612/
>>>
>>> AVX-512_compilation: failed
>>> DPLCS Test: fail
>>> DPIF Test: fail
>>> MFEX Test: fail
>>> Actions Test: fail
>>> Errors in DPCLS test:
>>> make check-dpdk
>>> make  all-am
>>> make[1]: Entering directory '/root/ovs-dev'
>>> make[1]: Leaving directory '/root/ovs-dev'
>>> set /bin/bash './tests/system-dpdk-testsuite' -C tests
>>> AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests:ipsec::'; \ "$@"
>>> -j1 || (test X'' = Xyes && "$@" --recheck) ##
>>> -- ## ## openvswitch 3.4.90 test suite. ##
>>> ## -- ##
>>>
>>> OVS-DPDK unit tests
>>>
>>>   1: OVS-DPDK - EAL init ok
>>>   2: OVS-DPDK - add standard DPDK port   ok
>>>   3: OVS-DPDK - add vhost-user-client port   ok
>>>   4: OVS-DPDK - ping vhost-user portsFAILED 
>>> (ovs-macros.at:242)
>>>   5: OVS-DPDK - ping vhost-user-client ports FAILED 
>>> (ovs-macros.at:242)
>>>   6: OVS-DPDK - Ingress policing create delete phy port ok
>>>   7: OVS-DPDK - Ingress policing create delete vport port ok
>>>   8: OVS-DPDK - Ingress policing no policing rateok
>>>   9: OVS-DPDK - Ingress policing no policing burst   ok
>>>  10: OVS-DPDK - QoS create delete phy port   ok
>>>  11: OVS-DPDK - QoS create delete vport port ok
>>>  12: OVS-DPDK - QoS no cir   ok
>>>  13: OVS-DPDK - QoS no cbs   ok
>>>  14: OVS-DPDK - MTU increase phy portok
>>>  15: OVS-DPDK - MTU decrease phy portok
>>>  16: OVS-DPDK - MTU increase vport port  FAILED 
>>> (ovs-macros.at:242)
>>>  17: OVS-DPDK - MTU decrease vport port  FAILED (ovs-
>> macros.at:242)
>>>  18: OVS-DPDK - MTU upper bound phy port ok
>>>  19: OVS-DPDK - MTU lower bound phy port ok
>>>  20: OVS-DPDK - MTU upper bound vport port   FAILED (ovs-
>> macros.at:242)
>>>  21: OVS-DPDK - MTU lower bound vport port   FAILED (ovs-
>> macros.at:242)
>>>  22: OVS-DPDK - user configured mempool  ok
>>
>> Hi, Michael.  Could you, please, check the reason why these tests are 
>> failing?
>> The logs are truncated a little too much, so it's hard to tell what went 
>> wrong.
> Hi Ilya,
> The output in the logs is:
> 
> ./system-dpdk.at:109: ovs-vsctl add-br br10 -- set bridge br10 
> datapath_type=netdev
> ./system-dpdk.at:110: ovs-vsctl add-port br10 dpdkvhostuser0 -- set Interface 
> dpdkvhostuser0 type=dpdkvhostuser
> stderr:
> stdout:
> system-dpdk.at:110: waiting until grep "VHOST_CONFIG: 
> ($OVS_RUNDIR/dpdkvhostuser0) vhost-user server: socket created" 
> ovs-vswitchd.log...
> 2024-07-17T08:32:51.860Z|00062|dpdk|INFO|VHOST_CONFIG: 
> (/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0) vhost-user 
> server: socket created, fd: 95
> system-dpdk.at:110: wait succeeded immediately
> system-dpdk.at:110: waiting until grep "Socket $OVS_RUNDIR/dpdkvhostuser0 
> created for vhost-user port dpdkvhostuser0" ovs-vswitchd.log...
> 2024-07-17T08:32:51.860Z|00063|netdev_dpdk|INFO|Socket 
> /root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0 created for 
> vhost-user port dpdkvhostuser0
> system-dpdk.at:110: wait succeeded immediately
> system-dpdk.at:110: waiting until grep "VHOST_CONFIG: 
> ($OVS_RUNDIR/dpdkvhostuser0) binding succeeded" ovs-vswitchd.log...
> 2024-07-17T08:32:51.861Z|00064|dpdk|INFO|VHOST_CONFIG: 
> (/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0) binding 
> succeeded
> system-dpdk.at:110: wait succeeded immediately
> ./system-dpdk.at:111: ovs-vsctl show
> stdout:
> 45be2775-e3c2-45bc-9d5e-2eb502748c78
> Bridge br10
> datapath_type: netdev
> Port br10
> Interface br10
> type: internal
> Port dpdkvhostuser0
> Interface dpdkvhostuser0
> type: dpdkvhostuser
> Cannot remove namespace file "/run/netns/ns1": No such file or directory
> ./system-dpdk.at:114: ip netns add ns1 || return 77
> net.netfilter.nf_conntrack_helper = 0
> Cannot remove namespace file "/run/netns/ns2": No such file or directory
> ./system-dpdk.at:114: ip netns add ns2 || return 77
> net.netfilter.nf_conntrack_helper = 0
> ./system-dpdk.at:117: ip link add tap1 type veth peer name ovs-tap1 || return 
> 77
> ./system-dpdk.at:117: ethtool -K tap1 tx off
> stderr:
> stdout:
> Actual changes:
> tx-checksumming: off
>   tx-checksum-ip-generic: off
>   tx-checksum-sctp: off
> 

Re: [ovs-dev] [PATCH ovn] northd: Allow Load balancer to be attached to router with multiple GW ports

[0-day Robot]

References:  <20240717090334.3957-1-priyankar.j...@nutanix.com>
 

Bleep bloop.  Greetings Priyankar Jain, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject, ': ', is over 70 characters, i.e., 75.
WARNING: The subject summary should end with a dot.
Subject: northd: Allow Load balancer to be attached to router with multiple GW 
ports
Lines checked: 352, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] northd: Allow Load balancer to be attached to router with multiple GW ports

[Priyankar Jain]

This commit removes the restriction of support LB for router with only
<= 1 Distributed gateway ports.
Added datapath and logical flows validation cases.

Signed-off-by: Priyankar Jain 
---
 northd/en-lr-stateful.c |  12 ---
 northd/northd.c |  12 +--
 tests/ovn-northd.at |  86 +
 tests/ovn.at| 167 
 4 files changed, 260 insertions(+), 17 deletions(-)

diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
index baf1bd2f8..f09691af6 100644
--- a/northd/en-lr-stateful.c
+++ b/northd/en-lr-stateful.c
@@ -516,18 +516,6 @@ lr_stateful_record_create(struct lr_stateful_table *table,
 
 table->array[od->index] = lr_stateful_rec;
 
-/* Load balancers are not supported (yet) if a logical router has multiple
- * distributed gateway port.  Log a warning. */
-if (lr_stateful_rec->has_lb_vip && lr_has_multiple_gw_ports(od)) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(, "Load-balancers are configured on logical "
- "router %s, which has %"PRIuSIZE" distributed "
- "gateway ports. Load-balancer is not supported "
- "yet when there is more than one distributed "
- "gateway port on the router.",
- od->nbr->name, od->n_l3dgw_ports);
-}
-
 return lr_stateful_rec;
 }
 
diff --git a/northd/northd.c b/northd/northd.c
index 6898daa00..e6f53f361 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11026,10 +11026,9 @@ static void
 build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx,
  enum lrouter_nat_lb_flow_type type,
  struct ovn_datapath *od,
+ struct ovn_port *dgp,
  struct lflow_ref *lflow_ref)
 {
-struct ovn_port *dgp = od->l3dgw_ports[0];
-
 const char *undnat_action;
 
 switch (type) {
@@ -11060,7 +11059,7 @@ build_distr_lrouter_nat_flows_for_lb(struct 
lrouter_nat_lb_flows_ctx *ctx,
 
 if (ctx->lb_vip->n_backends || !ctx->lb_vip->empty_backend_rej) {
 ds_put_format(ctx->new_match, " && is_chassis_resident(%s)",
-  od->l3dgw_ports[0]->cr_port->json_key);
+  dgp->cr_port->json_key);
 }
 
 ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio,
@@ -11263,8 +11262,11 @@ build_lrouter_nat_flows_for_lb(
 if (!od->n_l3dgw_ports) {
 bitmap_set1(gw_dp_bitmap[type], index);
 } else {
-build_distr_lrouter_nat_flows_for_lb(, type, od,
- lb_dps->lflow_ref);
+for (int i = 0; i < od->n_l3dgw_ports; i++) {
+struct ovn_port *dgp = od->l3dgw_ports[i];
+build_distr_lrouter_nat_flows_for_lb(, type, od, dgp,
+ lb_dps->lflow_ref);
+}
 }
 
 if (lb->affinity_timeout) {
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index a389d1988..5be48f49e 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12721,3 +12721,89 @@ AT_CHECK([ovn-sbctl dump-flows lr | grep lr_in_dnat | 
ovn_strip_lflows], [0], [d
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ovn-northd -- LB on Lr with multiple gw ports])
+AT_KEYWORDS([lb-multiple-l3dgw-ports])
+ovn_start
+
+# Logical network:
+# 1 Logical Router, 3 bridged Logical Switches,
+# 1 gateway chassis attached to each corresponding LRP.
+# LB added attached to DR
+#
+#| S1 (gw1)
+#|
+#  ls    DR -- S3 (gw3)
+# (20.0.0.0/24)  |
+#| S2 (gw2)
+#
+# Validate basic LB logical flows.
+
+check ovn-sbctl chassis-add gw1 geneve 127.0.0.1
+check ovn-sbctl chassis-add gw2 geneve 128.0.0.1
+check ovn-sbctl chassis-add gw3 geneve 129.0.0.1
+
+check ovn-nbctl lr-add DR
+check ovn-nbctl lrp-add DR DR-S1 02:ac:10:01:00:01 172.16.1.1/24
+check ovn-nbctl lrp-add DR DR-S2 03:ac:10:01:00:01 172.16.2.1/24
+check ovn-nbctl lrp-add DR DR-S3 04:ac:10:01:00:01 172.16.3.1/24
+check ovn-nbctl lrp-add DR DR-ls 05:ac:10:01:00:01 20.0.0.1/24
+
+check ovn-nbctl ls-add S1
+check ovn-nbctl lsp-add S1 S1-DR
+check ovn-nbctl lsp-set-type S1-DR router
+check ovn-nbctl lsp-set-addresses S1-DR router
+check ovn-nbctl --wait=sb lsp-set-options S1-DR router-port=DR-S1
+
+check ovn-nbctl ls-add S2
+check ovn-nbctl lsp-add S2 S2-DR
+check ovn-nbctl lsp-set-type S2-DR router
+check ovn-nbctl lsp-set-addresses S2-DR router
+check ovn-nbctl --wait=sb lsp-set-options S2-DR router-port=DR-S2
+
+check ovn-nbctl ls-add S3
+check ovn-nbctl lsp-add S3 S3-DR
+check ovn-nbctl lsp-set-type S3-DR router
+check ovn-nbctl lsp-set-addresses S3-DR router
+check ovn-nbctl --wait=sb lsp-set-options S3-DR router-port=DR-S3
+
+check ovn-nbctl ls-add  ls
+check ovn-nbctl 

Re: [ovs-dev] [ovs-build] |fail| pw1960612 [ovs-dev, 2/2] Prepare for post-3.4.0 (3.4.90).

[Phelan, Michael]

> -Original Message-
> From: Ilya Maximets 
> Sent: Tuesday, July 16, 2024 12:25 PM
> To: Phelan, Michael 
> Cc: i.maxim...@ovn.org; ovs-dev 
> Subject: Re: [ovs-build] |fail| pw1960612 [ovs-dev,2/2] Prepare for post-
> 3.4.0 (3.4.90).
> 
> On 7/15/24 22:23, ovs_jenk...@intel.com wrote:
> > Test-Label: intel-ovs-compilation
> > Test-Status: fail
> > http://patchwork.ozlabs.org/api/patches/1960612/
> >
> > AVX-512_compilation: failed
> > DPLCS Test: fail
> > DPIF Test: fail
> > MFEX Test: fail
> > Actions Test: fail
> > Errors in DPCLS test:
> > make check-dpdk
> > make  all-am
> > make[1]: Entering directory '/root/ovs-dev'
> > make[1]: Leaving directory '/root/ovs-dev'
> > set /bin/bash './tests/system-dpdk-testsuite' -C tests
> > AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests:ipsec::'; \ "$@"
> > -j1 || (test X'' = Xyes && "$@" --recheck) ##
> > -- ## ## openvswitch 3.4.90 test suite. ##
> > ## -- ##
> >
> > OVS-DPDK unit tests
> >
> >   1: OVS-DPDK - EAL init ok
> >   2: OVS-DPDK - add standard DPDK port   ok
> >   3: OVS-DPDK - add vhost-user-client port   ok
> >   4: OVS-DPDK - ping vhost-user portsFAILED 
> > (ovs-macros.at:242)
> >   5: OVS-DPDK - ping vhost-user-client ports FAILED 
> > (ovs-macros.at:242)
> >   6: OVS-DPDK - Ingress policing create delete phy port ok
> >   7: OVS-DPDK - Ingress policing create delete vport port ok
> >   8: OVS-DPDK - Ingress policing no policing rateok
> >   9: OVS-DPDK - Ingress policing no policing burst   ok
> >  10: OVS-DPDK - QoS create delete phy port   ok
> >  11: OVS-DPDK - QoS create delete vport port ok
> >  12: OVS-DPDK - QoS no cir   ok
> >  13: OVS-DPDK - QoS no cbs   ok
> >  14: OVS-DPDK - MTU increase phy portok
> >  15: OVS-DPDK - MTU decrease phy portok
> >  16: OVS-DPDK - MTU increase vport port  FAILED 
> > (ovs-macros.at:242)
> >  17: OVS-DPDK - MTU decrease vport port  FAILED (ovs-
> macros.at:242)
> >  18: OVS-DPDK - MTU upper bound phy port ok
> >  19: OVS-DPDK - MTU lower bound phy port ok
> >  20: OVS-DPDK - MTU upper bound vport port   FAILED (ovs-
> macros.at:242)
> >  21: OVS-DPDK - MTU lower bound vport port   FAILED (ovs-
> macros.at:242)
> >  22: OVS-DPDK - user configured mempool  ok
> 
> Hi, Michael.  Could you, please, check the reason why these tests are failing?
> The logs are truncated a little too much, so it's hard to tell what went 
> wrong.
Hi Ilya,
The output in the logs is:

./system-dpdk.at:109: ovs-vsctl add-br br10 -- set bridge br10 
datapath_type=netdev
./system-dpdk.at:110: ovs-vsctl add-port br10 dpdkvhostuser0 -- set Interface 
dpdkvhostuser0 type=dpdkvhostuser
stderr:
stdout:
system-dpdk.at:110: waiting until grep "VHOST_CONFIG: 
($OVS_RUNDIR/dpdkvhostuser0) vhost-user server: socket created" 
ovs-vswitchd.log...
2024-07-17T08:32:51.860Z|00062|dpdk|INFO|VHOST_CONFIG: 
(/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0) vhost-user 
server: socket created, fd: 95
system-dpdk.at:110: wait succeeded immediately
system-dpdk.at:110: waiting until grep "Socket $OVS_RUNDIR/dpdkvhostuser0 
created for vhost-user port dpdkvhostuser0" ovs-vswitchd.log...
2024-07-17T08:32:51.860Z|00063|netdev_dpdk|INFO|Socket 
/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0 created for 
vhost-user port dpdkvhostuser0
system-dpdk.at:110: wait succeeded immediately
system-dpdk.at:110: waiting until grep "VHOST_CONFIG: 
($OVS_RUNDIR/dpdkvhostuser0) binding succeeded" ovs-vswitchd.log...
2024-07-17T08:32:51.861Z|00064|dpdk|INFO|VHOST_CONFIG: 
(/root/ovs-dev/tests/system-dpdk-testsuite.dir/004/dpdkvhostuser0) binding 
succeeded
system-dpdk.at:110: wait succeeded immediately
./system-dpdk.at:111: ovs-vsctl show
stdout:
45be2775-e3c2-45bc-9d5e-2eb502748c78
Bridge br10
datapath_type: netdev
Port br10
Interface br10
type: internal
Port dpdkvhostuser0
Interface dpdkvhostuser0
type: dpdkvhostuser
Cannot remove namespace file "/run/netns/ns1": No such file or directory
./system-dpdk.at:114: ip netns add ns1 || return 77
net.netfilter.nf_conntrack_helper = 0
Cannot remove namespace file "/run/netns/ns2": No such file or directory
./system-dpdk.at:114: ip netns add ns2 || return 77
net.netfilter.nf_conntrack_helper = 0
./system-dpdk.at:117: ip link add tap1 type veth peer name ovs-tap1 || return 77
./system-dpdk.at:117: ethtool -K tap1 tx off
stderr:
stdout:
Actual changes:
tx-checksumming: off
tx-checksum-ip-generic: off
tx-checksum-sctp: off
tcp-segmentation-offload: off
tx-tcp-segmentation: off [requested on]
tx-tcp-ecn-segmentation: off [requested on]
tx-tcp-mangleid-segmentation: off 

Re: [ovs-dev] [PATCH v2 1/1] ofproto-dpif-upcall: Avoid stale ukeys leaks.

[Roi Dayan via dev]

On 10/07/2024 18:35, Eelco Chaudron wrote:
> 
> 
> On 9 Jul 2024, at 23:38, Ilya Maximets wrote:
> 
>> On 7/3/24 12:28, Roi Dayan via dev wrote:
>>>
>>>
>>> On 03/07/2024 13:16, Eelco Chaudron wrote:


 On 3 Jul 2024, at 9:33, Roi Dayan wrote:

> It is observed in some environments that there are much more ukeys than
> actual DP flows. For example:
>
> $ ovs-appctl upcall/show
> system@ovs-system:
> flows : (current 7) (avg 6) (max 117) (limit 2125)
> offloaded flows : 525
> dump duration : 1063ms
> ufid enabled : true
>
> 23: (keys 3612)
> 24: (keys 3625)
> 25: (keys 3485)
>
> The revalidator threads are busy revalidating the stale ukeys leading to
> high CPU and long dump duration.
>
> This patch adds checks in the sweep phase for such ukeys and move them
> to DELETE so that they can be cleared eventually.

 Thank Roi for the patch update, one small issue below. Let’s also discuss 
 a bit more a potential testcase before sending a v3.

 Cheers,

 Eelco


>>>
>>> I also replied in v0 but replying here so we can continue the discussion 
>>> here in v2.
>>>
>>> I did this for testing this case:
>>>
>>> - create bridge with 2 veth ports. configure ips.
>>> - ping between the ports to have tc rules.
>>> - repeated few times: clear the tc rules like this: tc filter del dev veth1 
>>> ingress and also on the 2nd port.
>>> - set max-idle to 1 and remove it to cause a flush of the rules.
>>> - create another set of veth ports. add/del veth4 from the bridge a few 
>>> times to cause a sweep.
>>> - before the fix: ovs-appctl upcall/show will show ukeys.
>>> - after the fix upcall/show will show 0 ukeys.
>>>
>>>
>>> what do you think?
>>> I think if there is a cleaner way to do a sweep with purge=false as just
>>> will just skip seq mismatch check and just set every ukey to delete.
>>
>> One problem here is that flow dumps are inherently racy.
>> We can dump the same flow multiple times as well as not
>> receive some flows that are actually in the datapath.
>> So, we can't just remove ukeys if we missed a flow once
>> or twice.  If we didn't see the flow in the dumps for the
>> max-idle interval, that might be a better indicator that
>> something is wrong.
>>
>> One more problem here however is that ukey statistics can
>> keep increasing if there is some traffic that matches.
>> Since the datapath flow doesn't exist, all this traffic
>> will go to userspace updating the ukey stats, but the
>> flow will never be installed again.  And we will not be
>> able to fix that in this patch since the stats will grow.
> 
> Yes, I noticed this when trying to add a quick testcase for this. The entries 
> were used, so it would never hit the code path.
> 
> 
>> We need a way to tell that we haven't seen this flow in
>> the dumps for a long time and stats are not a good indicator
>> in this case.
> 
> Agreed, I made a rough testcase and a patch that does this. Please take a 
> look and use whatever you need for your patch. I can also try to work on this 
> if you want, but it will be another three weeks as I’m about to go on PTO.
> 
> Here are the patches (diffs):
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 83609ec62..8b449537d 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -57,6 +57,7 @@ COVERAGE_DEFINE(dumped_inconsistent_flow);
>  COVERAGE_DEFINE(dumped_new_flow);
>  COVERAGE_DEFINE(handler_duplicate_upcall);
>  COVERAGE_DEFINE(revalidate_missed_dp_flow);
> +COVERAGE_DEFINE(revalidate_missing_dp_flow);
>  COVERAGE_DEFINE(ukey_dp_change);
>  COVERAGE_DEFINE(ukey_invalid_stat_reset);
>  COVERAGE_DEFINE(ukey_replace_contention);
> @@ -278,6 +279,7 @@ enum flow_del_reason {
>  FDR_BAD_ODP_FIT,/* Bad ODP flow fit. */
>  FDR_FLOW_IDLE,  /* Flow idle timeout. */
>  FDR_FLOW_LIMIT, /* Kill all flows condition reached. */
> +FDR_FLOW_MISSING_DP,/* Flow is missing from the datapath. */
>  FDR_FLOW_WILDCARDED,/* Flow needs a narrower wildcard mask. */
>  FDR_NO_OFPROTO, /* Bridge not found. */
>  FDR_PURGE,  /* User requested flow deletion. */
> @@ -315,6 +317,7 @@ struct udpif_key {
>  struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
>  const char *dp_layer OVS_GUARDED; /* Last known dp_layer. */
>  long long int created OVS_GUARDED;/* Estimate of creation time. 
> */
> +long long int last_dumped OVS_GUARDED;/* Flow last dump time. */
>  uint64_t dump_seq OVS_GUARDED;/* Tracks udpif->dump_seq. */
>  uint64_t reval_seq OVS_GUARDED;   /* Tracks udpif->reval_seq. */
>  enum ukey_state state OVS_GUARDED;/* Tracks ukey lifetime. */
> @@ -1819,6 +1822,7 @@ ukey_create__(const struct nlattr *key, size_t key_len,
>  ukey->state_thread = ovsthread_id_self();
>   

Re: [ovs-dev] RFC OVN: fabric integration

[Felix Huettner via dev]

Hi everyone,

thanks a lot for that work that was already put into this.
I just want to share my thoughs below as i'm looking for a similar thing
at the moment.

On Thu, Jul 04, 2024 at 11:39:36AM +0200, Frode Nordahl wrote:
> > I tend to agree with Ilya here, clarity for the operator as for what
> > the system is actually doing becomes even more important when we are
> > integrating with external systems (the ToRs). The operator would
> > expect to be able to map configuration and status observed on one
> > system to configuration and status observed in another system.
> >
> > Another issue is that I see no way for magically mapping a single
> > localnet port into multiple chassis resident LRPs which would be
> > required for configurations with multiple NICs that do not use bonds.

>From my understanding users will need to configure multiple things that
are specific to each node anyway. They will need at least a peering IP
for each node (or one per nic per node) and the bgp sessions for each of
these. This also means that the users will need to do configuration on
each of their nodes directly (at least for BGP).

Maybe we can also use this for caring about the detailed lrp
configuration. Since this is relevant only for the respective node and
not for the outside we could put that config to the local ovsdb and just
leave it for ovn-controller to handle. This might include splitting the
lrp up into multiple ones.

This way one could have a single LR and LS handling all off the external
connections on the northbound side while splitting this up on the actual
chassis. This might be easier to understand and to extend (like adding
new nodes). This might look something like this when viewed from
northbound:

+---+ +---+
|Project1 LS| |Project2 LS|
++--+ +-+-+
 |  |
++--+ +-+-+
|Project1 LR| |Project2 LR|
++--+ +-+-+
 |  |
++--+-+
|"provider network" LS|
+--+--+
   |
+--+--+
|magic bgp router |
+--+--+
   |
+--+--+
|physical network LS  |
+-+

where the "provider network" LS is the provider network from OpenStack
side but without localnet port (as Frode wrote below).
The magic bgp router would have one lrp to the provider network side.
The magic bgp router would also have one other lrp to the phyiscal
network side. Maybe we add something like replicated_chassis_group to LRPs
that contains all chassis the lrp should run on (so active-active while
HA_Chassis_Group is active-passive).
I am unsure if the phyiscal network LS is actually helpful, or if it would
be easier to just put the network_name directly on the LRP.

>From my perspecitve this gives us a nicer separation between the target
architecture the user defines in the northbound and the per node
implementation details that are then in the local ovsdb.

> >
> > Presumably the goal with your proposal is to find a low-touch way to
> > make existing CMSs with overlay-based OVN configuration, such as
> > OpenStack, work in the new topology.
> >
> > We're also interested in minimizing the development effort on the CMS
> > side, so the tardy response to this thread is due to us spending a few
> > days exploring options.
> >
> >
> > I'll describe one approach that from observation mostly works below:
> >
> > With the outset of a classic OpenStack OVN configuration:
> > * Single provider LS
> > * Per project LSs with distributed LRs that have gateway chassis set
> > and NAT rules configured
> > * Instances scattered across three nodes
> >
> > We did the following steps to morph it into a configuration with per
> > node gateway routers and local entry/exit of traffic to/from
> > instances:
> > * Apply Numan's overlay provider network patch [0] and set
> > other_config:overlay_provider_network=true on provider LS
> > * Remove the provider network localnet port
> > * Create per chassis/NIC LS with localnet ports
> > * Create per chassis GWR and attach it to NIC LS as well as provider network
> >
> > We have this handy script to do most of it [1].
> 
> One thing I forgot to mention is that for simplicity the script uses a
> lot of IPv4 addresses. In a final solution I would propose we use IPv6
> LLAs also for routing between the internal OVN L3 constructs to avoid
> this.

/me goes back to fixing the v4-over-v6 routing patch :)

> 
> --
> Frode Nordahl
> 
> > With that we can have an outside observer sending traffic via external
> > GWR IP destined to an instance FIP local to that chassis and have the
> > traffic enter/exit locally.
> >
> > The part that does not work in this setup is correct identification of
> > the return path due to the project LR having a single global default
> > route, so it only works for a single chassis at a time.
> >
> >
> > Perhaps we could solve this with a per chassis routing table and/or
> 

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Shibir Basak]

Hi Ales,

Configuring “unknown” addresses will remove the port security even if IP/MAC is 
configured on the port or when OVN is acting as DHCP server for the subnet.
We would like to avoid it.

Thanks,
Shibir

From: Ales Musil 
Date: Tuesday, 16 July 2024 at 3:48 PM
To: Shibir Basak 
Cc: d...@openvswitch.org 
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
Yes, that's why I'm trying to find out if that would work for you. If I 
remember correctly libvirt/qemu is supposed to send gARP on its own when the 
migration finishes. On Tue, Jul 16, 2024 at 12: 13 PM Shibir Basak 
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd
Yes, that's why I'm trying to find out if that would work for you. If I 
remember correctly libvirt/qemu is supposed to send gARP on its own when the 
migration finishes.

On Tue, Jul 16, 2024 at 12:13 PM Shibir Basak 
mailto:shibir.ba...@nutanix.com>> wrote:
Yes, I think OVN doesn’t send rarp and garp unless the VIF has MAC and IP 
addresses configured respectively.

Are you suggesting to configure the IP/MAC once we feel the VM is ready to 
avoid the problem?

Thanks,
Shibir

From: Ales Musil mailto:amu...@redhat.com>>
Date: Tuesday, 16 July 2024 at 1:10 PM
To: Shibir Basak mailto:shibir.ba...@nutanix.com>>
Cc: d...@openvswitch.org 
mailto:d...@openvswitch.org>>
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
I fully understand the problem, however when I update your test into the 
snippet below it starts to fail because we don't send any garp/rarp for ports 
with unknown address, does that make sense? diff --git a/tests/ovn. at [ovn. 
at] b/tests/ovn. at
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd
I fully understand the problem, however when I update your test into the 
snippet below it starts to fail because we don't send any garp/rarp for ports 
with unknown address, does that make sense?

diff --git a/tests/ovn.at 
[ovn.at]
 b/tests/ovn.at 
[ovn.at]
index 185ba4a21..618d7bd72 100644
--- a/tests/ovn.at 
[ovn.at]
+++ b/tests/ovn.at 
[ovn.at]
@@ -38426,3 +38426,58 @@ OVN_CLEANUP([hv1],[hv2])

 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Disabling RARP/GARP announcements])
+ovn_start
+
+# In this test case we create 1 switch and bring up 4 VIFs on it.
+# Two VIFs will be assigned MAC addresses only (i.e. without ips)
+# and two VIFs will be assigned IP addresses along with MAC addresses.
+# VIFs with IPs are supposed to send GARPs and VIFs with only MAC
+# addresses are supposed to send RARP. However, we test the lsp
+# option disable_garp_rarp, which when set to true for an lsp does
+# not send the GARP/RARP announcements.
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 ln1 "" 101
+ovn-nbctl lsp-set-addresses ln1 unknown
+ovn-nbctl lsp-set-type ln1 localnet
+ovn-nbctl lsp-set-options ln1 network_name=phys
+
+ovn-nbctl lsp-add ls1 lp11
+ovn-nbctl lsp-set-addresses lp11 unknown
+
+ovn-nbctl lsp-add ls1 lp13
+ovn-nbctl lsp-set-addresses lp13 unknown
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif 
options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+ovs-vsctl add-port br-int vif11 -- \
+set Interface vif11 external-ids:iface-id=lp11
+
+ovs-vsctl add-port br-int vif13 -- \
+set Interface vif13 external-ids:iface-id=lp13
+
+wait_for_ports_up
+ovn-nbctl --wait=sb sync
+
+# RARP packet for lp11
+echo 
"f011816580350001080006040003f011f011"
 > expected
+# GARP packet for lp13
+echo 
"f013816508060001080006040001f013c0a80103c0a80103"
 >> expected
+OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])

On Tue, Jul 16, 2024 

[ovs-dev] [PATCH ovn v1] northd: Add ECMP symmetric replies for egress.

[numans]

From: Numan Siddique 

When there are ECMP symmetric static routes configured, OVN selects
one of the next hop for the traffic originated from within the
cluster.  For the subsequent packets to the same destination,
OVN may select a different next hop (which is fine).  But there can
be certain usecases, where the next hop entity can be stateful and
selecting the same next hop is desirable.

This patch address this usecase in the following way

   1.  For the first packet originating from the OVN logical port
   VIF, OVN selects a next hop 'A' and forwards the traffic to
   it.

   2.  When the reply traffic is received (either from next hop 'A'
   or any other next hop), it commits the connection in the
   DNAT zone of the logical router and saves the state in
   ct_label.ecmp_reply_eth and ct_label.ecmp_reply_port.
   Note that we already support this for the traffic
   originating from an ECMP route [1].  We are now extending
   the same for the traffic originating from the cluster towards
   the ECMP route.

3. For the subsequent packets from the cluster, we select the
   next hop eth address and the port from the saved conntrack
   state.  This is straightforward as we anyway send the packet
   to the DNAT zone of the logical router.

Example: If a logical router lr0 is configured with the below
EMCP static routes

ovn-nbctl lr-route-list lr0
IPv4 Routes
Route Table :
0.0.0.0/0172.20.0.1 dst-ip ecmp
0.0.0.0/0172.20.0.2 dst-ip ecmp 
ecmp-symmetric-reply

Before this patch, we were adding the below logical flows in the router
pipeline for the ECMP route handling:

-
table=10(lr_in_ecmp_stateful), priority=100  , match=(inport == "lr0-public" && 
ip4.src == 0.0.0.0/0 && !ct.rpl && (ct.new || ct.est)), action=(ct_commit { 
ct_label.ecmp_reply_eth = eth.src;  ct_mark.ecmp_reply_port = 3;}; next;)
table=14(lr_in_ip_routing   ), priority=10300, match=(ct.rpl && 
ct_mark.ecmp_reply_port == 3 && reg7 == 0 && ip4.dst == 0.0.0.0/0), 
action=(ip.ttl--; flags.loopback = 1; eth.src = 00:11:22:00:ff:01; reg1 = 
172.20.0.100; outport = "lr0-public"; next;)
table=16(lr_in_policy   ), priority=65535, match=(ct.rpl && 
ct_mark.ecmp_reply_port == 3), action=(next;)
table=20(lr_in_arp_resolve  ), priority=200  , match=(ct.rpl && 
ct_mark.ecmp_reply_port == 3), action=(push(xxreg1); xxreg1 = ct_label; eth.dst 
= xxreg1[32..79]; pop(xxreg1); next;)
-

After this patch, we add the below logical flows:

--
table=10(lr_in_ecmp_stateful), priority=100  , match=(inport == "lr0-public" && 
ip4.src == 0.0.0.0/0 && (ct.new || ct.est)), action=(ct_commit { 
ct_label.ecmp_reply_eth = eth.src;  ct_mark.ecmp_reply_port = 3;}; next;)
table=14(lr_in_ip_routing   ), priority=10300, match=(ct_mark.ecmp_reply_port 
== 3 && reg7 == 0 && ip4.dst == 0.0.0.0/0), action=(ip.ttl--; flags.loopback = 
1; eth.src = 00:11:22:00:ff:01; reg1 = 172.20.0.100; outport = "lr0-public"; 
next;)
table=16(lr_in_policy   ), priority=65535, match=(ct_mark.ecmp_reply_port 
== 3), action=(next;)
table=20(lr_in_arp_resolve  ), priority=200  , match=(ct_mark.ecmp_reply_port 
== 3), action=(push(xxreg1); xxreg1 = ct_label; eth.dst = xxreg1[32..79]; 
pop(xxreg1); next;)
--

[1] - 4fdca656857d ("Add ECMP symmetric replies.")
Reported-at: https://issues.redhat.com/browse/FDP-628
Signed-off-by: Numan Siddique 
---
 northd/northd.c |  4 ++--
 tests/ovn-northd.at | 26 +-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 6898daa00d..3227c093cb 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10706,7 +10706,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table 
*lflows,
  * NOTE: we purposely are not clearing match before this
  * ds_put_cstr() call. The previous contents are needed.
  */
-ds_put_cstr(, " && !ct.rpl && (ct.new || ct.est)");
+ds_put_cstr(, " && (ct.new || ct.est)");
 ds_put_format(,
 "ct_commit { ct_label.ecmp_reply_eth = eth.src; "
 " %s = %" PRId64 ";}; "
@@ -10721,7 +10721,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table 
*lflows,
  * for where to route the packet.
  */
 ds_put_format(_reply,
-  "ct.rpl && %s == %"PRId64,
+  "%s == %"PRId64,
   ct_ecmp_reply_port_match, out_port->sb->tunnel_key);
 ds_clear();
 ds_put_format(, "%s && %s", ds_cstr(_reply),
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index a389d19886..98a5006cb5 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -6722,8 +6722,14 @@ check ovn-nbctl --wait=sb --ecmp-symmetric-reply 
lr-route-add lr0 1.0.0.1 192.16
 
 ovn-sbctl dump-flows lr0 > lr0flows
 
-AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
+AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], 

[ovs-dev] [Patch ovn v6] Text respresntations for drop sampling.

[Jacob Tanenbaum]

Created a new column in the southbound database to hardcode a human readable
description for flows. This first use is describing why the flow is dropping 
packets.
The new column is called flow_desc and will create southbound database entries 
like this

_uuid   : 20f1897b-477e-47ae-a32c-c546d83ec097
actions : 
"sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); /* 
drop */"
controller_meter: []
external_ids: {source="northd.c:8721", stage-name=ls_in_l2_unknown}
flow_desc   : "No L2 destination"
logical_datapath: []
logical_dp_group: ee3c3db5-98a2-4f34-8a84-409deae140a7
match   : "outport == \"none\""
pipeline: ingress
priority: 50
table_id: 27
tags: {}
hash: 0

future work includes entering more flow_desc for more flows and adding
flow_desc to the actions as a comment.

Signed-off-by: Jacob Tanenbaum 
Suggested-by: Dumitru Ceara 
Reported-at: https://issues.redhat.com/browse/FDP-307

---

v1: initial version
v2: correct commit message
make the flow_desc a char*
correct a typo in the ovn-sb.xml
correct the test
v3: rebase issue with NEWS file
v4: remove options:debug_drop_domain_id="1" from testing as we
do not depend on it
v5: introduce string wrapper
increment ovs-sb.ovsschema version
correct the testing
added descriptions to more dropped packets
v6: v5 was not the correct branch, this is...
added descriptions to more dropped packets
changed the names of the macros to be more descriptive
---
 NEWS|  2 ++
 lib/ovn-util.h  | 26 +++
 northd/lflow-mgr.c  | 25 +++
 northd/lflow-mgr.h  | 27 +++-
 northd/northd.c | 50 -
 ovn-sb.ovsschema|  8 +---
 ovn-sb.xml  |  5 +
 tests/ovn-northd.at | 15 ++
 8 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/NEWS b/NEWS
index 3e392ff08..0039b04be 100644
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,8 @@ Post v24.03.0
 ability to disable "VXLAN mode" to extend available tunnel IDs space for
 datapaths from 4095 to 16711680.  For more details see man ovn-nb(5) for
 mentioned option.
+  - Added a new column in the southbound database "flow_desc" to provide
+human readable context to flows.
 
 OVN v24.03.0 - 01 Mar 2024
 --
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index f75b821b6..2da5cd086 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -466,6 +466,32 @@ void sorted_array_apply_diff(const struct sorted_array *a1,
 bool add),
  const void *arg);
 
+/* A wrapper that holds strings */
+struct string_wrapper
+{
+char *str;
+bool owns_string;
+};
+
+#define EMPTY_STRING_WRAPPER (struct string_wrapper){NULL, false}
+
+static inline struct string_wrapper
+string_wrapper_create(char *str, bool take_ownership)
+{
+return (struct string_wrapper) {
+.str = str,
+.owns_string = take_ownership,
+};
+}
+
+static inline void
+string_wrapper_destroy(struct string_wrapper *s)
+{
+if (s->owns_string) {
+free(s->str);
+}
+}
+
 /* Utilities around properly handling exit command. */
 struct ovn_exit_args {
 struct unixctl_conn **conns;
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index b2c60b5de..c81d9afcd 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -25,6 +25,7 @@
 #include "debug.h"
 #include "lflow-mgr.h"
 #include "lib/ovn-parallel-hmap.h"
+#include "lib/ovn-util.h"
 
 VLOG_DEFINE_THIS_MODULE(lflow_mgr);
 
@@ -36,7 +37,7 @@ static void ovn_lflow_init(struct ovn_lflow *, struct 
ovn_datapath *od,
uint16_t priority, char *match,
char *actions, char *io_port,
char *ctrl_meter, char *stage_hint,
-   const char *where);
+   const char *where, struct string_wrapper flow_desc);
 static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows,
 enum ovn_stage stage,
 uint16_t priority, const char *match,
@@ -52,7 +53,7 @@ static struct ovn_lflow *do_ovn_lflow_add(
 const char *actions, const char *io_port,
 const char *ctrl_meter,
 const struct ovsdb_idl_row *stage_hint,
-const char *where);
+const char *where, struct string_wrapper flow_desc);
 
 
 static struct ovs_mutex *lflow_hash_lock(const struct hmap *lflow_table,
@@ -173,6 +174,7 @@ struct ovn_lflow {
   * 'dpg_bitmap'. */
 struct ovn_dp_group *dpg;/* Link to unique Sb datapath group. */
 const char *where;
+struct string_wrapper flow_desc;
 
 struct uuid sb_uuid; /* SB DB row uuid, specified by northd. 

Re: [ovs-dev] [PATCH v2] dpctl: Fix netdev reference leak in "show" command.

[Simon Horman]

On Tue, Jul 16, 2024 at 01:57:36PM +0530, Vipul Ashri wrote:
> This specific Netdev leak is causing us stale VHU entries, where it
> is showing false limit reaching maximum and preventing us to create
> new entries for us. This leak can impact other nics also.
> 
> Steps to reproduce,
> While running a test with a continous VM creation/deletion using an
> orchestration script with-in cloud environment. In parallel we have
> some monitoring script calling ovs-appctl dpctl/show stats commands
> every minute.
> 
> Root-cause analysis,
> During VHU port delete, one of netdev references were not reduced to
> 0 as show_dpif call has not given-up the reference back or doing bad
> cleanup. This pending deference preventing VHU deletion sequence, this
> is found to be one of corner case inside dpctl code which results in
> leaking up netdev which ultimately results in stale VHU entry. After
> fixing this problematic cleanup, issue is not seen.
> 
> Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> dpif-netdev")
> Signed-off-by: Vipul Ashri 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] ofp-actions: Fix reporting observation point bits instead of domain.

[Simon Horman]

On Tue, Jul 16, 2024 at 12:48:06PM +0200, Ilya Maximets wrote:
> Found by Coverity:
> 
>   CID 397544:  Incorrect expression  (COPY_PASTE_ERROR)
>   "obs_point_src" in "(*os).obs_point_src.n_bits" looks
>   like a copy-paste error.
> 
> Also adding a test case to cover this situation.
> 
> Fixes: 1aa9e137fe36 ("ofp-actions: Load data from fields in sample action.")
> Signed-off-by: Ilya Maximets 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] flow: Fix unaligned access to the ND target in miniflow_extract.

[Simon Horman]

On Tue, Jul 16, 2024 at 01:45:53PM +0200, Ales Musil wrote:
> The data in the buffer are aligned to 2 bytes, however
> 'struct in6_addr' is aligned to 4 bytes. Use the 2 bytes aligned
> equivalent 'union ovs_16aligned_in6_addr' instead. This was caught
> by one of the OVN tests:
> 
> lib/flow.c:1133:25: runtime error: load of misaligned address
> 0x5149cc92 for type 'const struct in6_addr *', which requires
> 4 byte alignment
> 0x5149cc92: note: pointer points here
>  00 00  00 00 10 00 00 00 00 00  00 00 00 00 00 00 00 00
>   ^
> 0 0x8255b2 in miniflow_extract /workspace/ovn/ovs/lib/flow.c:1133:25
> 1 0x81d921 in flow_extract /workspace/ovn/ovs/lib/flow.c:671:5
> 2 0xa966d4 in ofp_packet_to_string /workspace/ovn/ovs/lib/ofp-print.c:82:5
> 3 0xa76de2 in ofputil_packet_in_private_format 
> /workspace/ovn/ovs/lib/ofp-packet.c:1037:24
> 4 0xa99817 in ofp_print_packet_in /workspace/ovn/ovs/lib/ofp-print.c:132:9
> 5 0xa97f46 in ofp_to_string__ /workspace/ovn/ovs/lib/ofp-print.c
> 6 0xa97f46 in ofp_to_string /workspace/ovn/ovs/lib/ofp-print.c:1264:21
> 7 0xc338f4 in do_send /workspace/ovn/ovs/lib/vconn.c:687:19
> 8 0xb7f678 in try_send /workspace/ovn/ovs/lib/rconn.c:1128:14
> 9 0xb7d725 in rconn_send__ /workspace/ovn/ovs/lib/rconn.c:760:13
> 10 0xb7d8e7 in rconn_send_with_limit /workspace/ovn/ovs/lib/rconn.c:816:17
> 11 0x6f70de in do_send_packet_ins /workspace/ovn/ovs/ofproto/connmgr.c:1697:13
> 12 0x6f691f in connmgr_send_async_msg 
> /workspace/ovn/ovs/ofproto/connmgr.c:1682:9
> 13 0x5c2d23 in run /workspace/ovn/ovs/ofproto/ofproto-dpif.c:1877:13
> 14 0x56737f in ofproto_run /workspace/ovn/ovs/ofproto/ofproto.c:1906:13
> 15 0x50d4fc in bridge_run__ /workspace/ovn/ovs/vswitchd/bridge.c:3287:9
> 16 0x50a764 in bridge_run /workspace/ovn/ovs/vswitchd/bridge.c:3346:5
> 17 0x53eed7 in main /workspace/ovn/ovs/vswitchd/ovs-vswitchd.c:130:9
> 
> Signed-off-by: Ales Musil 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Remove misleading wc NULL check in packet mirror.

[Mike Pattrick]

On Tue, Jul 16, 2024 at 8:47 AM Ilya Maximets  wrote:
>
> 'wc' can't be NULL there and if it can we'd already crash a few lines
> before setting up vlan flags.
>
> The check is misleading as it makes people to assume that wc can be
> NULL.  And it makes Coverity think the same:
>
>   CID 1596572: (#1 of 1): Dereference after null check (FORWARD_NULL)
>   25. var_deref_op: Dereferencing null pointer ctx->wc.
>
>   14. var_compare_op: Comparing ctx->wc to null implies that ctx->wc
>   might be null
>
> Remove the check.
>
> Fixes: 3b1882261c8b ("ofproto-dpif-mirror: Add support for pre-selection 
> filter.")
> Signed-off-by: Ilya Maximets 
> ---

It looks like you're right, it can't be null.

Acked-by: Mike Pattrick 

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

[ovs-dev] [PATCH] ofproto-dpif-xlate: Remove misleading wc NULL check in packet mirror.

[Ilya Maximets]

'wc' can't be NULL there and if it can we'd already crash a few lines
before setting up vlan flags.

The check is misleading as it makes people to assume that wc can be
NULL.  And it makes Coverity think the same:

  CID 1596572: (#1 of 1): Dereference after null check (FORWARD_NULL)
  25. var_deref_op: Dereferencing null pointer ctx->wc.

  14. var_compare_op: Comparing ctx->wc to null implies that ctx->wc
  might be null

Remove the check.

Fixes: 3b1882261c8b ("ofproto-dpif-mirror: Add support for pre-selection 
filter.")
Signed-off-by: Ilya Maximets 
---
 ofproto/ofproto-dpif-xlate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index be2c70721..02567a961 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2315,7 +2315,7 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle 
*xbundle,
 }
 
 /* After the VLAN check, apply a flow mask if a filter is specified. */
-if (ctx->wc && mc.filter_flow) {
+if (mc.filter_flow) {
 flow_wildcards_union_with_minimask(ctx->wc, mc.filter_mask);
 if (!OVS_UNLIKELY(
  miniflow_equal_flow_in_minimask(mc.filter_flow,
-- 
2.45.2

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

[ovs-dev] [PATCH ovn] controller: Remove OvS iface type check in I-P processing.

[Ales Musil]

The handler for OvS interface in runtime data was checking interface
type before proceeding with the I-P processing. The type is not
necessary because the main thing that is interesting for OVN is
the iface-id, if the interface doesn't have any it won't be processed
regardless of the type. The processing would happen anyway, however
it would be more costly because it would lead to full recompute
of the whole runtime data node.

Reported-at: https://github.com/ovn-org/ovn/issues/174
Reported-at: https://issues.redhat.com/browse/FDP-255
Signed-off-by: Ales Musil 
---
 controller/binding.c| 25 ++---
 tests/ovn-controller.at | 48 +
 2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index b7e7f4874..da1f8afcf 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2505,17 +2505,6 @@ consider_iface_release(const struct ovsrec_interface 
*iface_rec,
 return true;
 }
 
-static bool
-is_iface_vif(const struct ovsrec_interface *iface_rec)
-{
-if (iface_rec->type && iface_rec->type[0] &&
-strcmp(iface_rec->type, "internal")) {
-return false;
-}
-
-return true;
-}
-
 static bool
 is_iface_in_int_bridge(const struct ovsrec_interface *iface,
const struct ovsrec_bridge *br_int)
@@ -2630,17 +2619,17 @@ binding_handle_ovs_interface_changes(struct 
binding_ctx_in *b_ctx_in,
 const struct ovsrec_interface *iface_rec;
 OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec,
  b_ctx_in->iface_table) {
-if (!is_iface_vif(iface_rec)) {
-/* Right now we are not handling ovs_interface changes of
- * other types. This can be enhanced to handle of
- * types - patch and tunnel. */
+const char *iface_id = smap_get(_rec->external_ids, "iface-id");
+const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids,
+iface_rec->name);
+if (!iface_id && !old_iface_id) {
+/* Right now we are not handling ovs_interface changes if the
+ * interface doesn't have iface-id or didn't have it
+ * previously. */
 handled = false;
 break;
 }
 
-const char *iface_id = smap_get(_rec->external_ids, "iface-id");
-const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids,
-iface_rec->name);
 const char *cleared_iface_id = NULL;
 if (!ovsrec_interface_is_deleted(iface_rec)) {
 int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 2cb86dc98..be913676b 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -3127,3 +3127,51 @@ OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1235: connected' 
hv1/ovn-controller.log])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+AT_SETUP([ovn-controller - I-P different port types])
+AT_KEYWORDS([ovn])
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.20
+
+check ovn-nbctl ls-add ls0
+check ovn-nbctl lsp-add ls0 vif
+
+ovn-appctl inc-engine/clear-stats
+
+ovs-vsctl -- add-port br-int vif -- \
+set Interface vif external-ids:iface-id=vif
+wait_row_count Port_Binding 1 logical_port="vif" up=true
+
+ovs-vsctl del-port br-int vif
+wait_row_count Port_Binding 1 logical_port="vif" up=false
+
+ovs-vsctl add-port br-int vif -- \
+set Interface vif type=dummy -- \
+set Interface vif external-ids:iface-id=vif
+wait_row_count Port_Binding 1 logical_port="vif" up=true
+
+ovs-vsctl del-port br-int vif
+wait_row_count Port_Binding 1 logical_port="vif" up=false
+
+ovs-vsctl add-port br-int vif -- \
+set Interface vif type=geneve -- \
+set Interface vif options:remote_ip=1.1.1.1 external-ids:iface-id=vif
+wait_row_count Port_Binding 1 logical_port="vif" up=true
+
+ovs-vsctl del-port br-int vif
+wait_row_count Port_Binding 1 logical_port="vif" up=false
+
+AT_CHECK([ovn-appctl inc-engine/show-stats runtime_data |\
+  sed "s/- compute:\s\+[[0-9]]\+/- compute: ??/"], [0], [dnl
+Node: runtime_data
+- recompute:0
+- compute: ??
+- cancel:   0
+])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
-- 
2.45.2

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

[ovs-dev] [PATCH] flow: Fix unaligned access to the ND target in miniflow_extract.

[Ales Musil]

The data in the buffer are aligned to 2 bytes, however
'struct in6_addr' is aligned to 4 bytes. Use the 2 bytes aligned
equivalent 'union ovs_16aligned_in6_addr' instead. This was caught
by one of the OVN tests:

lib/flow.c:1133:25: runtime error: load of misaligned address
0x5149cc92 for type 'const struct in6_addr *', which requires
4 byte alignment
0x5149cc92: note: pointer points here
 00 00  00 00 10 00 00 00 00 00  00 00 00 00 00 00 00 00
  ^
0 0x8255b2 in miniflow_extract /workspace/ovn/ovs/lib/flow.c:1133:25
1 0x81d921 in flow_extract /workspace/ovn/ovs/lib/flow.c:671:5
2 0xa966d4 in ofp_packet_to_string /workspace/ovn/ovs/lib/ofp-print.c:82:5
3 0xa76de2 in ofputil_packet_in_private_format 
/workspace/ovn/ovs/lib/ofp-packet.c:1037:24
4 0xa99817 in ofp_print_packet_in /workspace/ovn/ovs/lib/ofp-print.c:132:9
5 0xa97f46 in ofp_to_string__ /workspace/ovn/ovs/lib/ofp-print.c
6 0xa97f46 in ofp_to_string /workspace/ovn/ovs/lib/ofp-print.c:1264:21
7 0xc338f4 in do_send /workspace/ovn/ovs/lib/vconn.c:687:19
8 0xb7f678 in try_send /workspace/ovn/ovs/lib/rconn.c:1128:14
9 0xb7d725 in rconn_send__ /workspace/ovn/ovs/lib/rconn.c:760:13
10 0xb7d8e7 in rconn_send_with_limit /workspace/ovn/ovs/lib/rconn.c:816:17
11 0x6f70de in do_send_packet_ins /workspace/ovn/ovs/ofproto/connmgr.c:1697:13
12 0x6f691f in connmgr_send_async_msg 
/workspace/ovn/ovs/ofproto/connmgr.c:1682:9
13 0x5c2d23 in run /workspace/ovn/ovs/ofproto/ofproto-dpif.c:1877:13
14 0x56737f in ofproto_run /workspace/ovn/ovs/ofproto/ofproto.c:1906:13
15 0x50d4fc in bridge_run__ /workspace/ovn/ovs/vswitchd/bridge.c:3287:9
16 0x50a764 in bridge_run /workspace/ovn/ovs/vswitchd/bridge.c:3346:5
17 0x53eed7 in main /workspace/ovn/ovs/vswitchd/ovs-vswitchd.c:130:9

Signed-off-by: Ales Musil 
---
 lib/flow.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index dc5fb328d..9be437524 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -408,7 +408,8 @@ parse_ethertype(const void **datap, size_t *sizep)
 static inline bool
 parse_icmpv6(const void **datap, size_t *sizep,
  const struct icmp6_data_header *icmp6,
- ovs_be32 *rso_flags, const struct in6_addr **nd_target,
+ ovs_be32 *rso_flags,
+ const union ovs_16aligned_in6_addr **nd_target,
  struct eth_addr arp_buf[2], uint8_t *opt_type)
 {
 if (icmp6->icmp6_base.icmp6_code != 0 ||
@@ -1117,7 +1118,7 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 }
 } else if (OVS_LIKELY(nw_proto == IPPROTO_ICMPV6)) {
 if (OVS_LIKELY(size >= sizeof(struct icmp6_data_header))) {
-const struct in6_addr *nd_target;
+const union ovs_16aligned_in6_addr *nd_target;
 struct eth_addr arp_buf[2];
 /* This will populate whether we received Option 1
  * or Option 2. */
-- 
2.45.2

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

Re: [ovs-dev] [ovs-build] |fail| pw1960612 [ovs-dev, 2/2] Prepare for post-3.4.0 (3.4.90).

[Ilya Maximets]

On 7/15/24 22:23, ovs_jenk...@intel.com wrote:
> Test-Label: intel-ovs-compilation
> Test-Status: fail 
> http://patchwork.ozlabs.org/api/patches/1960612/ 
> 
> AVX-512_compilation: failed 
> DPLCS Test: fail 
> DPIF Test: fail 
> MFEX Test: fail 
> Actions Test: fail
> Errors in DPCLS test: 
> make check-dpdk
> make  all-am
> make[1]: Entering directory '/root/ovs-dev'
> make[1]: Leaving directory '/root/ovs-dev'
> set /bin/bash './tests/system-dpdk-testsuite' -C tests  
> AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests:ipsec::'; \
> "$@"  -j1 || (test X'' = Xyes && "$@" --recheck)
> ## -- ##
> ## openvswitch 3.4.90 test suite. ##
> ## -- ##
> 
> OVS-DPDK unit tests
> 
>   1: OVS-DPDK - EAL init ok
>   2: OVS-DPDK - add standard DPDK port   ok
>   3: OVS-DPDK - add vhost-user-client port   ok
>   4: OVS-DPDK - ping vhost-user portsFAILED 
> (ovs-macros.at:242)
>   5: OVS-DPDK - ping vhost-user-client ports FAILED 
> (ovs-macros.at:242)
>   6: OVS-DPDK - Ingress policing create delete phy port ok
>   7: OVS-DPDK - Ingress policing create delete vport port ok
>   8: OVS-DPDK - Ingress policing no policing rateok
>   9: OVS-DPDK - Ingress policing no policing burst   ok
>  10: OVS-DPDK - QoS create delete phy port   ok
>  11: OVS-DPDK - QoS create delete vport port ok
>  12: OVS-DPDK - QoS no cir   ok
>  13: OVS-DPDK - QoS no cbs   ok
>  14: OVS-DPDK - MTU increase phy portok
>  15: OVS-DPDK - MTU decrease phy portok
>  16: OVS-DPDK - MTU increase vport port  FAILED 
> (ovs-macros.at:242)
>  17: OVS-DPDK - MTU decrease vport port  FAILED 
> (ovs-macros.at:242)
>  18: OVS-DPDK - MTU upper bound phy port ok
>  19: OVS-DPDK - MTU lower bound phy port ok
>  20: OVS-DPDK - MTU upper bound vport port   FAILED 
> (ovs-macros.at:242)
>  21: OVS-DPDK - MTU lower bound vport port   FAILED 
> (ovs-macros.at:242)
>  22: OVS-DPDK - user configured mempool  ok

Hi, Michael.  Could you, please, check the reason why these tests are failing?
The logs are truncated a little too much, so it's hard to tell what went wrong.

> 2024-07-15T19:14:11Z|7|dpdk|INFO|Using DPDK 23.11.0

I remember that there were some vhost issues in 23.11.0, we may need to upgrade
to 23.11.1 here.  We use it in GitHub Actions as well.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v5 2/2] pinctrl: Handle arp/nd for other address families.

[Felix Huettner via dev]

> 
> I pinged 172.17.0.50 from the logical port sw0p1 which has IP
> 11.0.0.3.  Since there is a dnat_and_snat entry for this IP,  the
> logical flow in table 23 (shown above) is hit and it corrupts xxreg0
> due to the action reg1 = 172.16.1.110.
> 
> In table 24, the priority 200 flow is never matched because xxreg0 is
> no longer 3001::b (since it is now 3001:0:ac10:16e::b) and the
> priority 100 flow  is hit with the action=(nd_ns { nd.target = xxreg0;
> output; }; output;)
> 
> And that's why the IPv6 Neigh Solicitation packet has the wrong nd.target.
> 
> I'm afraid your approach doesn't work when  a router has NAT entries
> configured.  We need to find an alternate solution to address your use
> case.
> 
> Thanks
> Numan
> 
Thanks a lot for that detailed information.
I'll try to rework it.

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

[ovs-dev] [PATCH] ofp-actions: Fix reporting observation point bits instead of domain.

[Ilya Maximets]

Found by Coverity:

  CID 397544:  Incorrect expression  (COPY_PASTE_ERROR)
  "obs_point_src" in "(*os).obs_point_src.n_bits" looks
  like a copy-paste error.

Also adding a test case to cover this situation.

Fixes: 1aa9e137fe36 ("ofp-actions: Load data from fields in sample action.")
Signed-off-by: Ilya Maximets 
---
 lib/ofp-actions.c| 2 +-
 tests/ofp-actions.at | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 2a1f5c3c4..fe6a17b6d 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6489,7 +6489,7 @@ parse_SAMPLE(char *arg, const struct ofpact_parse_params 
*pp)
 if (os->obs_domain_src.n_bits > 32) {
 return xasprintf("size of obs_domain_id field (%d) "
  "exceeds maximum (32)",
- os->obs_point_src.n_bits);
+ os->obs_domain_src.n_bits);
 }
 }
 } else if (!strcmp(key, "obs_point_id")) {
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index 86aec12e8..8a0504b3c 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -1127,6 +1127,8 @@ bad_action 'unroll_xlate' "UNROLL is an internal action 
that shouldn't be used v
 # sample
 bad_action 'sample(probability=0)' 'invalid probability value "0"'
 bad_action 'sample(sampling_port=asdf)' 'asdf: unknown port'
+bad_action 'sample(probability=12345,obs_domain_id=NXM_NX_CT_LABEL[[5..40]])' \
+'size of obs_domain_id field (36) exceeds maximum (32)'
 bad_action 'sample(probability=12345,obs_point_id=NXM_NX_CT_LABEL[[0..32]])' \
 'size of obs_point_id field (33) exceeds maximum (32)'
 bad_action 'sample(foo=bar)' 'invalid key "foo" in "sample" argument'
-- 
2.45.2

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

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Ales Musil]

Yes, that's why I'm trying to find out if that would work for you. If I
remember correctly libvirt/qemu is supposed to send gARP on its own when
the migration finishes.

On Tue, Jul 16, 2024 at 12:13 PM Shibir Basak 
wrote:

> Yes, I think OVN doesn’t send rarp and garp unless the VIF has MAC and IP
> addresses configured respectively.
>
>
>
> Are you suggesting to configure the IP/MAC once we feel the VM is ready to
> avoid the problem?
>

>
> Thanks,
>
> Shibir
>
>
>
> *From: *Ales Musil 
> *Date: *Tuesday, 16 July 2024 at 1:10 PM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> disable_garp_rarp.
>
> I fully understand the problem, however when I update your test into the
> snippet below it starts to fail because we don't send any garp/rarp for
> ports with unknown address, does that make sense? diff --git a/tests/ovn. at
> [ovn. at] b/tests/ovn. at
>
> ZjQcmQRYFpfptBannerStart
>
> *CAUTION: External Email *
>
>
>
> ZjQcmQRYFpfptBannerEnd
>
> I fully understand the problem, however when I update your test into the
> snippet below it starts to fail because we don't send any garp/rarp for
> ports with unknown address, does that make sense?
>
> diff --git a/tests/ovn.at [ovn.at]
> 
> b/tests/ovn.at [ovn.at]
> 
> index 185ba4a21..618d7bd72 100644
> --- a/tests/ovn.at [ovn.at]
> 
> +++ b/tests/ovn.at [ovn.at]
> 
> @@ -38426,3 +38426,58 @@ OVN_CLEANUP([hv1],[hv2])
>
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([Disabling RARP/GARP announcements])
> +ovn_start
> +
> +# In this test case we create 1 switch and bring up 4 VIFs on it.
> +# Two VIFs will be assigned MAC addresses only (i.e. without ips)
> +# and two VIFs will be assigned IP addresses along with MAC addresses.
> +# VIFs with IPs are supposed to send GARPs and VIFs with only MAC
> +# addresses are supposed to send RARP. However, we test the lsp
> +# option disable_garp_rarp, which when set to true for an lsp does
> +# not send the GARP/RARP announcements.
> +
> +ovn-nbctl ls-add ls1
> +ovn-nbctl lsp-add ls1 ln1 "" 101
> +ovn-nbctl lsp-set-addresses ln1 unknown
> +ovn-nbctl lsp-set-type ln1 localnet
> +ovn-nbctl lsp-set-options ln1 network_name=phys
> +
> +ovn-nbctl lsp-add ls1 lp11
> +ovn-nbctl lsp-set-addresses lp11 unknown
> +
> +ovn-nbctl lsp-add ls1 lp13
> +ovn-nbctl lsp-set-addresses lp13 unknown
> +
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +AT_CHECK([ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif
> options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
> +
> +ovs-vsctl add-port br-int vif11 -- \
> +set Interface vif11 external-ids:iface-id=lp11
> +
> +ovs-vsctl add-port br-int vif13 -- \
> +set Interface vif13 external-ids:iface-id=lp13
> +
> +wait_for_ports_up
> +ovn-nbctl --wait=sb sync
> +
> +# RARP packet for lp11
> +echo
> "f011816580350001080006040003f011f011"
> > expected
> +# GARP packet for lp13
> +echo
> "f013816508060001080006040001f013c0a80103c0a80103"
> >> expected
> +OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected])
> +
> +OVN_CLEANUP([hv1])
> +
> +AT_CLEANUP
> +])
>
>
>
> On Tue, Jul 16, 2024 at 8:43 AM Shibir Basak 
> wrote:
>
> Hi Ales,
>
>
>
> The problem is with the rarp/garp sent by ovn-controller before the VM (on
> AZ2) is ready.
>
> This leads to traffic being switched to the VM on AZ2 resulting in traffic
> loss.
>
>
>
> Even with the vif set to "unknown" address on the migrating VM, we are
> unable to avoid this.
>
>
>
> Let me know if I am missing something obvious.
>
>
>
> Thanks,
>
> Shibir
>
>
>
> *From: *Ales Musil 
> *Date: *Tuesday, 16 July 2024 at 11:45 AM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> 

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Shibir Basak]

Yes, I think OVN doesn’t send rarp and garp unless the VIF has MAC and IP 
addresses configured respectively.

Are you suggesting to configure the IP/MAC once we feel the VM is ready to 
avoid the problem?

Thanks,
Shibir

From: Ales Musil 
Date: Tuesday, 16 July 2024 at 1:10 PM
To: Shibir Basak 
Cc: d...@openvswitch.org 
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
I fully understand the problem, however when I update your test into the 
snippet below it starts to fail because we don't send any garp/rarp for ports 
with unknown address, does that make sense? diff --git a/tests/ovn. at [ovn. 
at] b/tests/ovn. at
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd
I fully understand the problem, however when I update your test into the 
snippet below it starts to fail because we don't send any garp/rarp for ports 
with unknown address, does that make sense?

diff --git a/tests/ovn.at 
[ovn.at]
 b/tests/ovn.at 
[ovn.at]
index 185ba4a21..618d7bd72 100644
--- a/tests/ovn.at 
[ovn.at]
+++ b/tests/ovn.at 
[ovn.at]
@@ -38426,3 +38426,58 @@ OVN_CLEANUP([hv1],[hv2])

 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Disabling RARP/GARP announcements])
+ovn_start
+
+# In this test case we create 1 switch and bring up 4 VIFs on it.
+# Two VIFs will be assigned MAC addresses only (i.e. without ips)
+# and two VIFs will be assigned IP addresses along with MAC addresses.
+# VIFs with IPs are supposed to send GARPs and VIFs with only MAC
+# addresses are supposed to send RARP. However, we test the lsp
+# option disable_garp_rarp, which when set to true for an lsp does
+# not send the GARP/RARP announcements.
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 ln1 "" 101
+ovn-nbctl lsp-set-addresses ln1 unknown
+ovn-nbctl lsp-set-type ln1 localnet
+ovn-nbctl lsp-set-options ln1 network_name=phys
+
+ovn-nbctl lsp-add ls1 lp11
+ovn-nbctl lsp-set-addresses lp11 unknown
+
+ovn-nbctl lsp-add ls1 lp13
+ovn-nbctl lsp-set-addresses lp13 unknown
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif 
options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+ovs-vsctl add-port br-int vif11 -- \
+set Interface vif11 external-ids:iface-id=lp11
+
+ovs-vsctl add-port br-int vif13 -- \
+set Interface vif13 external-ids:iface-id=lp13
+
+wait_for_ports_up
+ovn-nbctl --wait=sb sync
+
+# RARP packet for lp11
+echo 
"f011816580350001080006040003f011f011"
 > expected
+# GARP packet for lp13
+echo 
"f013816508060001080006040001f013c0a80103c0a80103"
 >> expected
+OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])

On Tue, Jul 16, 2024 at 8:43 AM Shibir Basak 
mailto:shibir.ba...@nutanix.com>> wrote:
Hi Ales,

The problem is with the rarp/garp sent by ovn-controller before the VM (on AZ2) 
is ready.
This leads to traffic being switched to the VM on AZ2 resulting in traffic loss.

Even with the vif set to "unknown" address on the migrating VM, we are unable 
to avoid this.

Let me know if I am missing something obvious.

Thanks,
Shibir

From: Ales Musil mailto:amu...@redhat.com>>
Date: Tuesday, 16 July 2024 at 11:45 AM
To: Shibir Basak mailto:shibir.ba...@nutanix.com>>
Cc: d...@openvswitch.org 
mailto:d...@openvswitch.org>>
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
On Wed, Jul 10, 2024 at 1: 06 PM Shibir Basak  
wrote: Hi, Yes, that’s right. Let’s say, the VM is getting copied to the 
destination AZ or say it is not yet ready to process any traffic, yet. In such 
cases we may
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd


On Wed, Jul 10, 2024 at 1:06 PM Shibir Basak 

[ovs-dev] [PATCH v2] dpctl: Fix netdev reference leak in "show" command.

[Vipul Ashri via dev]

This specific Netdev leak is causing us stale VHU entries, where it
is showing false limit reaching maximum and preventing us to create
new entries for us. This leak can impact other nics also.

Steps to reproduce,
While running a test with a continous VM creation/deletion using an
orchestration script with-in cloud environment. In parallel we have
some monitoring script calling ovs-appctl dpctl/show stats commands
every minute.

Root-cause analysis,
During VHU port delete, one of netdev references were not reduced to
0 as show_dpif call has not given-up the reference back or doing bad
cleanup. This pending deference preventing VHU deletion sequence, this
is found to be one of corner case inside dpctl code which results in
leaking up netdev which ultimately results in stale VHU entry. After
fixing this problematic cleanup, issue is not seen.

Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
dpif-netdev")
Signed-off-by: Vipul Ashri 
---
 lib/dpctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index a70df5342..f764cf164 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -738,8 +738,8 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
 continue;
 }
 error = netdev_get_stats(netdev, );
+netdev_close(netdev);
 if (!error) {
-netdev_close(netdev);
 print_stat(dpctl_p, "RX packets:", s.rx_packets);
 print_stat(dpctl_p, " errors:", s.rx_errors);
 print_stat(dpctl_p, " dropped:", s.rx_dropped);
-- 
2.34.1.windows.1

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

[ovs-dev] [PATCH] dpctl: Netdev leak causes stale VHU & reaching limit.

[Vipul Ashri via dev]

> On Mon, Jul 15, 2024 at 02:55:29PM +0100, Simon Horman wrote:
> > On Thu, Jul 11, 2024 at 08:20:28PM +, Vipul Ashri via dev wrote:
> > >
> > > While running a test with a continous VM creation/deletion using an
> > > orchestration script with-in cloud environment. In parallel we have
> > > some monitoring script calling ovs-appctl dpctl/show stats commands
> > > every minute.
> > >
> > > During VHU port delete, one of netdev references were not reduced to
> > > 0 as show_dpif call has not given-up the reference back or doing bad
> > > cleanup. This pending deference preventing VHU deletion sequence, this
> > > is found to be one of corner case inside dpctl code which results in
> > > leaking up netdev which ultimately results in stale VHU entry. After
> > > fixing this problematic cleanup, issue is not seen.
> > >
> > > Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> > > dpif-netdev")
> > > Signed-off-by: Vipul Ashri 
> >
> > Acked-by: Simon Horman 
>
> I now see this is a duplicate of an earlier posting [1] (why?).

Sending twice was mistake from my end, You can ignore this time.

>
> [1] 
> https://patchwork.ozlabs.org/project/openvswitch/patch/pawpr07mb9877859e07fbad793eefd01d90...@pawpr07mb9877.eurprd07.prod.outlook.com/
>
> In that thread David Marchard (CCed) said:
>
> "The issue seems generic as any netdev for which a call to
>  netdev_get_stats failed would trigger the same leak.
>  So I would update the commit title with something more generic like:
>  dpctl: Fix netdev reference leak in "show" command.

Let me send a v2 patch addressing this @David Marchand comment and one more
comment suggested by @Simon Horman about changing netdev_close() place.

>
> "This can probably be changed when applying.
>  In any case, the fix lgtm.
>
> "Reviewed-by: David Marchand 
>
> Which all seems reasonable to me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Ales Musil]

I fully understand the problem, however when I update your test into the
snippet below it starts to fail because we don't send any garp/rarp for
ports with unknown address, does that make sense?

diff --git a/tests/ovn.at b/tests/ovn.at
index 185ba4a21..618d7bd72 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -38426,3 +38426,58 @@ OVN_CLEANUP([hv1],[hv2])

 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Disabling RARP/GARP announcements])
+ovn_start
+
+# In this test case we create 1 switch and bring up 4 VIFs on it.
+# Two VIFs will be assigned MAC addresses only (i.e. without ips)
+# and two VIFs will be assigned IP addresses along with MAC addresses.
+# VIFs with IPs are supposed to send GARPs and VIFs with only MAC
+# addresses are supposed to send RARP. However, we test the lsp
+# option disable_garp_rarp, which when set to true for an lsp does
+# not send the GARP/RARP announcements.
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 ln1 "" 101
+ovn-nbctl lsp-set-addresses ln1 unknown
+ovn-nbctl lsp-set-type ln1 localnet
+ovn-nbctl lsp-set-options ln1 network_name=phys
+
+ovn-nbctl lsp-add ls1 lp11
+ovn-nbctl lsp-set-addresses lp11 unknown
+
+ovn-nbctl lsp-add ls1 lp13
+ovn-nbctl lsp-set-addresses lp13 unknown
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif
options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+ovs-vsctl add-port br-int vif11 -- \
+set Interface vif11 external-ids:iface-id=lp11
+
+ovs-vsctl add-port br-int vif13 -- \
+set Interface vif13 external-ids:iface-id=lp13
+
+wait_for_ports_up
+ovn-nbctl --wait=sb sync
+
+# RARP packet for lp11
+echo
"f011816580350001080006040003f011f011"
> expected
+# GARP packet for lp13
+echo
"f013816508060001080006040001f013c0a80103c0a80103"
>> expected
+OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])

On Tue, Jul 16, 2024 at 8:43 AM Shibir Basak 
wrote:

> Hi Ales,
>
>
>
> The problem is with the rarp/garp sent by ovn-controller before the VM (on
> AZ2) is ready.
>
> This leads to traffic being switched to the VM on AZ2 resulting in traffic
> loss.
>
>
>
> Even with the vif set to "unknown" address on the migrating VM, we are
> unable to avoid this.
>
>
>
> Let me know if I am missing something obvious.
>
>
>
> Thanks,
>
> Shibir
>
>
>
> *From: *Ales Musil 
> *Date: *Tuesday, 16 July 2024 at 11:45 AM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> disable_garp_rarp.
>
> On Wed, Jul 10, 2024 at 1: 06 PM Shibir Basak 
> wrote: Hi, Yes, that’s right. Let’s say, the VM is getting copied to the
> destination AZ or say it is not yet ready to process any traffic, yet. In
> such cases we may
>
> ZjQcmQRYFpfptBannerStart
>
> *CAUTION: External Email *
>
>
>
> ZjQcmQRYFpfptBannerEnd
>
>
>
>
>
> On Wed, Jul 10, 2024 at 1:06 PM Shibir Basak 
> wrote:
>
> Hi,
>
>
>
> Yes, that’s right.
>
>
> Let’s say, the VM is getting copied to the destination AZ or say it is not
> yet ready to process any traffic, yet.
>
> In such cases we may want OVN to avoid sending RARP/GARP early and lead to
> traffic outage.
>
>
>
> Thanks,
>
> Shibir
>
>
>
>
>
> Hi,
>
> sorry for the late reply,  would it work if you'll use an "unknown"
> address initially on the second interface and change it when the VM is done
> with migration?
>
>
>
> *From: *Ales Musil 
> *Date: *Wednesday, 10 July 2024 at 4:20 PM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> disable_garp_rarp.
>
> On Wed, Jul 10, 2024 at 12: 26 PM Shibir Basak 
> wrote: Hi Ales, The use case we are trying to address is not limited to
> migration within a local AZ. Here, the VM can also be migrated across AZs
> and it is supposed
>
> ZjQcmQRYFpfptBannerStart
>
> *CAUTION: External Email *
>
>
>
> ZjQcmQRYFpfptBannerEnd
>
>
>
>
>
> On Wed, Jul 10, 2024 at 12:26 PM Shibir Basak 
> wrote:
>
> Hi Ales,
>
>
>
> The use case we are trying to address is not limited to migration within a
> local AZ.
>
> Here, the VM can also be migrated across AZs and it is supposed to retain
> the same MAC & IP addresses.
>
> Hence, requested-chassis along with LSP option "activation-strategy" is
> not applicable for such a case.
>
>
>
> Thanks,
>
> Shibir
>
>
>
>
>
> Hi,
>
>
>
> so to make sure I understand the use case, you are migrating
>
> across AZ, and to make it faster you are creating an LSP with
>
> the same IP + MAC on the second AZ in advance. But because
>
> the VM is not there yet you don't want to announce that to the
>
> ToR switch is that assumption correct?
>
>
>
>
>
> *From: *Ales Musil 
> *Date: *Wednesday, 10 July 2024 at 

[ovs-dev] [Patch ovn v2 1/1] northd: BGP port mirroring.

[Martin Kalcok]

This change adds a 'bgp-mirror' option to LRP that allows
mirroring of BGP control plane traffic to an arbitrary LSP
in its peer LS.

The option expects a string with a LSP name. When set,
any traffic entering LS that's destined for any of the
LRP's IP addresses (including IPv6 LLA) is redirected
to the LSP specified in the option's value.

This enables external BGP daemons to listen on an interface
bound to a LSP and effectively act as if they were listening
on (and speaking from) LRP's IP address.

Signed-off-by: Martin Kalcok 
---
 northd/northd.c | 87 +
 northd/ovn-northd.8.xml | 23 +++
 ovn-nb.xml  | 14 +++
 tests/ovn-northd.at | 45 +
 tests/system-ovn.at | 86 
 5 files changed, 255 insertions(+)

diff --git a/northd/northd.c b/northd/northd.c
index 4353df07d..e07bf68cc 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -13048,6 +13048,92 @@ build_arp_resolve_flows_for_lrp(struct ovn_port *op,
 }
 }
 
+static void
+build_bgp_redirect_rule__(
+const char *s_addr, const char *bgp_port_name, bool is_ipv6,
+struct ovn_port *ls_peer, struct lflow_table *lflows,
+struct ds *match, struct ds *actions)
+{
+int ip_ver = is_ipv6 ? 6 : 4;
+/* Redirect packets in the input pipeline destined for LR's IP to
+ * the port specified in 'bgp-mirror' option.
+ */
+ds_clear(match);
+ds_clear(actions);
+ds_put_format(match, "ip%d.dst == %s && tcp.dst == 179", ip_ver, s_addr);
+ds_put_format(actions, "outport = \"%s\"; output;", bgp_port_name);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_L2_LKUP, 100,
+  ds_cstr(match),
+  ds_cstr(actions),
+  ls_peer->lflow_ref);
+
+
+/* Drop any traffic originating from 'bgp-mirror' port that does
+ * not originate from BGP daemon port. This blocks unnecessary
+ * traffic like ARP broadcasts or IPv6 router solicitation packets
+ * from the dummy 'bgp-mirror' port.
+ */
+ds_clear(match);
+ds_put_format(match, "inport == \"%s\"", bgp_port_name);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_CHECK_PORT_SEC, 80,
+  ds_cstr(match),
+  REGBIT_PORT_SEC_DROP " = 1; next;",
+  ls_peer->lflow_ref);
+
+ds_put_format(match,
+  " && ip%d.src == %s && tcp.src == 179",
+  ip_ver,
+  s_addr);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_CHECK_PORT_SEC, 81,
+  ds_cstr(match),
+  REGBIT_PORT_SEC_DROP " = check_in_port_sec(); next;",
+  ls_peer->lflow_ref);
+}
+
+static void
+build_lrouter_bgp_redirect(
+struct ovn_port *op, struct lflow_table *lflows,
+struct ds *match, struct ds *actions)
+{
+/* LRP has to have a peer.*/
+if (op->peer == NULL) {
+return;
+}
+/* LRP has to have NB record.*/
+if (op->nbrp == NULL) {
+return;
+}
+
+/* Proceed only for LRPs that have 'bgp-mirror' option set. Value of this
+ * option is the name of LSP to which a BGP traffic will be mirrored.
+ */
+const char *bgp_port = smap_get(>nbrp->options, "bgp-mirror");
+if (bgp_port == NULL) {
+return;
+}
+
+if (op->cr_port != NULL) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "Option 'bgp-mirror' is not supported on"
+  " Distributed Gateway Port '%s'", op->key);
+return;
+}
+
+/* Mirror traffic destined for LRP's IPs and default BGP port
+ * to the port defined in 'bgp-mirror' option.
+ */
+for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+const char *ip_s = op->lrp_networks.ipv4_addrs[i].addr_s;
+build_bgp_redirect_rule__(ip_s, bgp_port, false,  op->peer, lflows,
+  match, actions);
+}
+for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
+const char *ip_s = op->lrp_networks.ipv6_addrs[i].addr_s;
+build_bgp_redirect_rule__(ip_s, bgp_port, true, op->peer, lflows,
+  match, actions);
+}
+}
+
 /* This function adds ARP resolve flows related to a LSP. */
 static void
 build_arp_resolve_flows_for_lsp(
@@ -16003,6 +16089,7 @@ build_lswitch_and_lrouter_iterate_by_lrp(struct 
ovn_port *op,
 lsi->meter_groups, op->lflow_ref);
 build_lrouter_icmp_packet_toobig_admin_flows(op, lsi->lflows, >match,
  >actions, op->lflow_ref);
+build_lrouter_bgp_redirect(op, lsi->lflows, >match, >actions);
 }
 
 static void *
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b06b09ac5..1bf9d2dc0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -284,6 +284,21 @@
 

[ovs-dev] [Patch ovn v2 0/1] Add LRP option for mirroring of BGP traffic.

[Martin Kalcok]

This patch introduces a new Logical Router Port option
called 'bgp-mirror' that enables mirroring of BGP control plane
traffic to an arbitrary port. More details directly in the commit message.

Although not directly dependent, this change is intended to go
hand-in-hand with a series posted by fnordhal[0].

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2024-July/415795.html

Martin Kalcok (1):
  northd: BGP port mirroring.

 northd/northd.c | 87 +
 northd/ovn-northd.8.xml | 23 +++
 ovn-nb.xml  | 14 +++
 tests/ovn-northd.at | 45 +
 tests/system-ovn.at | 86 
 5 files changed, 255 insertions(+)

-- 
2.40.1

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

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Shibir Basak]

Hi Ales,

The problem is with the rarp/garp sent by ovn-controller before the VM (on AZ2) 
is ready.
This leads to traffic being switched to the VM on AZ2 resulting in traffic loss.

Even with the vif set to "unknown" address on the migrating VM, we are unable 
to avoid this.

Let me know if I am missing something obvious.

Thanks,
Shibir

From: Ales Musil 
Date: Tuesday, 16 July 2024 at 11:45 AM
To: Shibir Basak 
Cc: d...@openvswitch.org 
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
On Wed, Jul 10, 2024 at 1: 06 PM Shibir Basak  
wrote: Hi, Yes, that’s right. Let’s say, the VM is getting copied to the 
destination AZ or say it is not yet ready to process any traffic, yet. In such 
cases we may
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd


On Wed, Jul 10, 2024 at 1:06 PM Shibir Basak 
mailto:shibir.ba...@nutanix.com>> wrote:
Hi,

Yes, that’s right.

Let’s say, the VM is getting copied to the destination AZ or say it is not yet 
ready to process any traffic, yet.
In such cases we may want OVN to avoid sending RARP/GARP early and lead to 
traffic outage.

Thanks,
Shibir


Hi,
sorry for the late reply,  would it work if you'll use an "unknown" address 
initially on the second interface and change it when the VM is done with 
migration?

From: Ales Musil mailto:amu...@redhat.com>>
Date: Wednesday, 10 July 2024 at 4:20 PM
To: Shibir Basak mailto:shibir.ba...@nutanix.com>>
Cc: d...@openvswitch.org 
mailto:d...@openvswitch.org>>
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
On Wed, Jul 10, 2024 at 12: 26 PM Shibir Basak  
wrote: Hi Ales, The use case we are trying to address is not limited to 
migration within a local AZ. Here, the VM can also be migrated across AZs and 
it is supposed
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd


On Wed, Jul 10, 2024 at 12:26 PM Shibir Basak 
mailto:shibir.ba...@nutanix.com>> wrote:
Hi Ales,

The use case we are trying to address is not limited to migration within a 
local AZ.
Here, the VM can also be migrated across AZs and it is supposed to retain the 
same MAC & IP addresses.
Hence, requested-chassis along with LSP option "activation-strategy" is not 
applicable for such a case.

Thanks,
Shibir


Hi,

so to make sure I understand the use case, you are migrating
across AZ, and to make it faster you are creating an LSP with
the same IP + MAC on the second AZ in advance. But because
the VM is not there yet you don't want to announce that to the
ToR switch is that assumption correct?


From: Ales Musil mailto:amu...@redhat.com>>
Date: Wednesday, 10 July 2024 at 12:40 PM
To: Shibir Basak mailto:shibir.ba...@nutanix.com>>
Cc: d...@openvswitch.org 
mailto:d...@openvswitch.org>>
Subject: Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.
On Tue, Jul 9, 2024 at 7: 57 AM Shibir Basak  
wrote: If the lsp option 'disable_garp_rarp' is set to true, GARP and RARP 
announcements are not sent by ovn when a VIF port is created on a bridged 
logical
ZjQcmQRYFpfptBannerStart
CAUTION: External Email

ZjQcmQRYFpfptBannerEnd


On Tue, Jul 9, 2024 at 7:57 AM Shibir Basak 
mailto:shibir.ba...@nutanix.com>> wrote:
If the lsp option 'disable_garp_rarp' is set to true,
GARP and RARP announcements are not sent by ovn when a
VIF port is created on a bridged logical switch.

Usecase

This option is useful during VM migration to let the
hypervisor/VM send the RARP/GARP once VM is ready to
process the packets post migration. This helps to reduce
downtime during VM migration.

Signed-off-by: Shibir Basak 
mailto:shibir.ba...@nutanix.com>>
Acked-by: Naveen Yerramneni 
mailto:naveen.yerramn...@nutanix.com>>

Hello,

I would like to avoid yet another config option, have you considered
using "activation-strategy" LSP option along with setting multiple
requested-chassis during the migration process? This should ensure
that the downtime is actually as low as possible. See presentation
from Ihar about this specific option [0].

---
 controller/pinctrl.c |  4 +--
 ovn-nb.xml   |  7 +
 tests/ovn.at 
[ovn.at]
 | 68 
 3 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 9a2f3f5b3..800c85d21 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6618,7 +6618,7 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
 SSET_FOR_EACH (iface_id, _vifs) {
 const struct sbrec_port_binding *pb = lport_lookup_by_name(
 sbrec_port_binding_by_name, iface_id);
-if (pb) {
+if (pb && 

Re: [ovs-dev] [PATCH ovn] controller: Add lsp option disable_garp_rarp.

[Ales Musil]

On Wed, Jul 10, 2024 at 1:06 PM Shibir Basak 
wrote:

> Hi,
>
>
>
> Yes, that’s right.
>
>
> Let’s say, the VM is getting copied to the destination AZ or say it is not
> yet ready to process any traffic, yet.
>
> In such cases we may want OVN to avoid sending RARP/GARP early and lead to
> traffic outage.
>
>
>
> Thanks,
>
> Shibir
>


Hi,
sorry for the late reply,  would it work if you'll use an "unknown" address
initially on the second interface and change it when the VM is done with
migration?



> *From: *Ales Musil 
> *Date: *Wednesday, 10 July 2024 at 4:20 PM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> disable_garp_rarp.
>
> On Wed, Jul 10, 2024 at 12: 26 PM Shibir Basak 
> wrote: Hi Ales, The use case we are trying to address is not limited to
> migration within a local AZ. Here, the VM can also be migrated across AZs
> and it is supposed
>
> ZjQcmQRYFpfptBannerStart
>
> *CAUTION: External Email *
>
>
>
> ZjQcmQRYFpfptBannerEnd
>
>
>
>
>
> On Wed, Jul 10, 2024 at 12:26 PM Shibir Basak 
> wrote:
>
> Hi Ales,
>
>
>
> The use case we are trying to address is not limited to migration within a
> local AZ.
>
> Here, the VM can also be migrated across AZs and it is supposed to retain
> the same MAC & IP addresses.
>
> Hence, requested-chassis along with LSP option "activation-strategy" is
> not applicable for such a case.
>
>
>
> Thanks,
>
> Shibir
>
>
>
>
>
> Hi,
>
>
>
> so to make sure I understand the use case, you are migrating
>
> across AZ, and to make it faster you are creating an LSP with
>
> the same IP + MAC on the second AZ in advance. But because
>
> the VM is not there yet you don't want to announce that to the
>
> ToR switch is that assumption correct?
>
>
>
>
>
> *From: *Ales Musil 
> *Date: *Wednesday, 10 July 2024 at 12:40 PM
> *To: *Shibir Basak 
> *Cc: *d...@openvswitch.org 
> *Subject: *Re: [ovs-dev] [PATCH ovn] controller: Add lsp option
> disable_garp_rarp.
>
> On Tue, Jul 9, 2024 at 7: 57 AM Shibir Basak 
> wrote: If the lsp option 'disable_garp_rarp' is set to true, GARP and RARP
> announcements are not sent by ovn when a VIF port is created on a bridged
> logical
>
> ZjQcmQRYFpfptBannerStart
>
> *CAUTION: External Email *
>
>
>
> ZjQcmQRYFpfptBannerEnd
>
>
>
>
>
> On Tue, Jul 9, 2024 at 7:57 AM Shibir Basak 
> wrote:
>
> If the lsp option 'disable_garp_rarp' is set to true,
> GARP and RARP announcements are not sent by ovn when a
> VIF port is created on a bridged logical switch.
>
> Usecase
> 
> This option is useful during VM migration to let the
> hypervisor/VM send the RARP/GARP once VM is ready to
> process the packets post migration. This helps to reduce
> downtime during VM migration.
>
> Signed-off-by: Shibir Basak 
> Acked-by: Naveen Yerramneni 
>
>
>
> Hello,
>
> I would like to avoid yet another config option, have you considered
> using "activation-strategy" LSP option along with setting multiple
> requested-chassis during the migration process? This should ensure
> that the downtime is actually as low as possible. See presentation
> from Ihar about this specific option [0].
>
>
>
> ---
>  controller/pinctrl.c |  4 +--
>  ovn-nb.xml   |  7 +
>  tests/ovn.at [ovn.at]
> 
>| 68 
>  3 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 9a2f3f5b3..800c85d21 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6618,7 +6618,7 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>  SSET_FOR_EACH (iface_id, _vifs) {
>  const struct sbrec_port_binding *pb = lport_lookup_by_name(
>  sbrec_port_binding_by_name, iface_id);
> -if (pb) {
> +if (pb && !smap_get_bool(>options, "disable_garp_rarp",
> false)) {
>  send_garp_rarp_update(ovnsb_idl_txn,
>sbrec_mac_binding_by_lport_ip,
>local_datapaths, pb, _addresses,
> @@ -6631,7 +6631,7 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>  SSET_FOR_EACH (gw_port, _l3gw_ports) {
>  const struct sbrec_port_binding *pb
>  = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
> -if (pb) {
> +if (pb && !smap_get_bool(>options, "disable_garp_rarp",
> false)) {
>  send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
>local_datapaths, pb, _addresses,
>garp_max_timeout, garp_continuous);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 9552534f6..3a0f0e34d 100644
> --- a/ovn-nb.xml
> +++ 

Re: [ovs-dev] [PATCH 0/2] Patches to branch for 3.4.

[Ilya Maximets]

On 7/15/24 13:23, Ilya Maximets wrote:
> The plan is to branch by the end of today.  There is still a couple of
> patch sets on the list that can / will be applied before that.
> 
> Ilya Maximets (2):
>   Prepare for 3.4.0.
>   Prepare for post-3.4.0 (3.4.90).
> 
>  Documentation/faq/releases.rst |  1 +
>  NEWS   |  6 +-
>  configure.ac   |  2 +-
>  debian/changelog   | 10 --
>  debian/rules   |  4 ++--
>  5 files changed, 17 insertions(+), 6 deletions(-)
> 


Thanks, Mike and Jakob for review!  Applied and branched.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] OVS has now branched for the 3.4 release.

[Ilya Maximets]

Hi, everyone.

branch-3.4 was just created.  Please, test it and report issues!

Official release date, according to our release process, should be
on Thursday, August 15th (1 month from now).

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ilya Maximets]

On 7/8/24 20:27, 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.
> 
> 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
> v10:
>  - Changed rcu memory order
>  - Updated documentation to refer to packets instead of flows
>  - Updated comments
> ---
>  Documentation/ref/ovs-tcpdump.8.rst |   8 +-
>  NEWS|   6 +
>  lib/flow.h  |   9 ++
>  ofproto/ofproto-dpif-mirror.c   | 106 +-
>  ofproto/ofproto-dpif-mirror.h   |   8 +-
>  ofproto/ofproto-dpif-xlate.c|  15 ++-
>  ofproto/ofproto-dpif.c  |  12 +-
>  ofproto/ofproto.h   |   3 +
>  tests/ofproto-dpif.at   | 167 
>  utilities/ovs-tcpdump.in|  13 ++-
>  vswitchd/bridge.c   |  13 ++-
>  vswitchd/vswitch.ovsschema  |   7 +-
>  vswitchd/vswitch.xml|  16 +++
>  13 files changed, 368 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/ref/ovs-tcpdump.8.rst 
> b/Documentation/ref/ovs-tcpdump.8.rst
> index b9f8cdf6f..e7bd5e9e4 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 packets 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 e0359b759..cbe777a71 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,6 +16,12 @@ Post-v3.3.0
> per interface 'options:dpdk-lsc-interrupt' to 'false'.
> - Python:
>   * Added custom transaction support to the Idl via add_op().
> +   - 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 packets
> +   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..a14f85843 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 *) 

[ovs-dev] [PATCH ovn] tests: Add system tests for MAC_Binding.

[Naveen Yerramneni]

1. Test to validate traffic that needs ARP resolution
when SB is disconnected.
2. Test to validate MAC binding buffer limit when SB
is disconnected.

Signed-off-by: Naveen Yerramneni 
---
 tests/system-ovn.at | 223 
 1 file changed, 223 insertions(+)

diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index c24ede7c5..5f8778a97 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -13027,3 +13027,226 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port 
patch-.*/d
 /connection dropped.*/d"])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([SB Disconnect - MAC_Binding])
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_BR([br-int])
+ADD_BR([br-ext])
+
+SB_PATH="unix:$ovs_base/ovn-sb/ovn-sb.sock"
+ovs-ofctl add-flow br-ext action=normal
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+-- set Open_vSwitch . external-ids:system-id=hv1 \
+-- set Open_vSwitch . external-ids:ovn-remote=$SB_PATH \
+-- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+-- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+-- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+ADD_NAMESPACES(sw01)
+ADD_VETH(sw01, sw01, br-int, "192.168.1.10/24", "f0:00:00:01:02:03", \
+"192.168.1.1")
+ADD_NAMESPACES(server)
+ADD_VETH(s1, server, br-ext, "172.16.1.1/24", "f0:00:00:01:02:05", \
+ "172.16.1.254")
+NS_CHECK_EXEC([server], [ip addr add 172.16.1.2 dev s1])
+check ovn-nbctl lr-add R1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add sw-ext
+
+check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
+check ovn-nbctl lrp-add R1 rp-ext 00:00:02:01:02:03 172.16.1.254/16
+
+check ovn-nbctl lrp-set-gateway-chassis rp-ext hv1
+
+check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
+type=router options:router-port=rp-sw0 \
+-- lsp-set-addresses sw0-rp router
+
+check ovn-nbctl set Logical_Switch sw0 other_config:dhcp_relay_port=sw0-rp
+
+check ovn-nbctl lsp-add sw-ext ext-rp -- set Logical_Switch_Port ext-rp \
+type=router options:router-port=rp-ext \
+-- lsp-set-addresses ext-rp router
+check ovn-nbctl lsp-add sw-ext lnet \
+-- lsp-set-addresses lnet unknown \
+-- lsp-set-type lnet localnet \
+-- lsp-set-options lnet network_name=phynet
+
+check ovn-nbctl lsp-add sw0 sw01 \
+-- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.10"
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-bridge-mappings=phynet:br-ext])
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-remote-probe-interval=100])
+
+OVN_POPULATE_ARP
+
+check ovn-nbctl --wait=hv sync
+
+AS_BOX([Verify ARP resolution is working when SB is in connected state])
+
+AT_CHECK([ovn-appctl connection-status], [0], [dnl
+connected
+])
+
+NS_CHECK_EXEC([server], [tcpdump -l -nvv -i s1  icmp > pkt.pcap 2>tcpdump_err 
&])
+OVS_WAIT_UNTIL([grep "listening" tcpdump_err])
+on_exit 'kill $(pidof tcpdump)'
+
+NS_CHECK_EXEC([sw01], [ping -q -c 2 -i 0.2 -w 2 172.16.1.1 | FORMAT_PING],
+[0], [dnl
+2 packets transmitted, 2 received, 0% packet loss, time 0ms
+])
+
+OVS_WAIT_UNTIL([
+total_pkts=$(cat pkt.pcap | wc -l)
+test ${total_pkts} -ge 2
+])
+
+AS_BOX([Disconnect SB and test ping that generates new ARP])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-remote=tcp:127.0.0.1:1234])
+
+sleep 1
+AT_CHECK([ovn-appctl connection-status], [0], [dnl
+not connected
+])
+
+NS_CHECK_EXEC([sw01], [ping -q -c 2 -i 0.5 -w 1 172.16.1.2 | FORMAT_PING | sed 
's/.* packets/n packets/'],
+[0], [dnl
+n packets transmitted, 0 received, 100% packet loss, time 0ms
+])
+
+ovn-sbctl list fdb
+AS_BOX([Test traffic for which ARP entry existed in MAC_Binding table])
+NS_CHECK_EXEC([sw01], [ping -q -c 2 -i 0.2 -w 2 172.16.1.1 | FORMAT_PING],
+[0], [dnl
+2 packets transmitted, 2 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-remote=$SB_PATH])
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
+/failed to query port patch-.*/d
+/.*terminating with signal 15.*/d"])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([SB Disconnect - MAC_Binding buffer limit])
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_BR([br-int])
+ADD_BR([br-ext])
+
+SB_PATH="unix:$ovs_base/ovn-sb/ovn-sb.sock"
+ovs-ofctl add-flow br-ext action=normal
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+-- set Open_vSwitch . external-ids:system-id=hv1 \
+-- set Open_vSwitch . external-ids:ovn-remote=$SB_PATH \
+-- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+-- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+

Re: [ovs-dev] [Patch ovn 1/2] test macros: Ensure ADD_VETH macro sets IPv6 LLA.

[martin . kalcok]

This change actually broke a bunch of other tests, which I did not
expect. I'll resubmit v2 without it.

Martin.

On Mon, 2024-07-15 at 19:19 +0200, Martin Kalcok wrote:
> Test macro ADD_VET takes an optional argument to specify
> MAC address for the interface. However this address is
> set only after the interface is brought UP, causing it
> to keep IPv6 Link Local Address based on its old MAC.
> 
> Toggling the interface down/up after a new MAC address
> is set fixes this issue.
> 
> Signed-off-by: Martin Kalcok 
> ---
>  tests/system-common-macros.at | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/system-common-macros.at b/tests/system-common-
> macros.at
> index 691c271a3..e7dee9d28 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -69,6 +69,9 @@ m4_define([ADD_VETH],
>    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])
> +    # Toggle the interface down/up to get an IPv6 LLA that
> matches its MAC addr.
> +    NS_CHECK_EXEC([$2], [ip link set down $1])
> +    NS_CHECK_EXEC([$2], [ip link set up $1])
>    fi
>    if test -n "$6"; then
>  NS_CHECK_EXEC([$2], [ip route add $6 dev $1])

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

Re: [ovs-dev] [PATCH v6] ofproto: Add ofproto/detrace command to map UFIDs to OpenFlow.

[Ilya Maximets]

On 7/12/24 15:47, Dumitru Ceara wrote:
> It improves the debugging experience if we can easily get a list of
> OpenFlow rules and groups that contribute to the creation of a datapath
> flow.
> 
> The suggested workflow is:
> a. dump datapath flows (along with UUIDs), this also prints the core IDs
> (PMD IDs) when applicable.
> 
>   $ ovs-appctl dpctl/dump-flows -m
>   flow-dump from pmd on cpu core: 7
>   ufid:7460db8f..., recirc_id(0), 
> 
> b. dump related OpenFlow rules and groups:
>   $ ovs-appctl ofproto/detrace ufid:7460db8f... pmd=7
>   cookie=0x12345678, table=0 
> priority=100,ip,in_port=2,nw_dst=10.0.0.2,actions=resubmit(,1)
>   cookie=0x0, table=1 priority=200,actions=group:1
>   group_id=1,bucket=bucket_id:0,actions=ct(commit,table=2,nat(dst=20.0.0.2))
>   cookie=0x0, table=2 actions=output:1
> 
> The new command only shows rules and groups attached to ukeys that are
> in states UKEY_VISIBLE or UKEY_OPERATIONAL.  That should be fine as all
> other ukeys should not be relevant for the use case presented above.
> 
> This commit tries to mimic the output format of the ovs-ofctl
> dump-flows/dump-groups commands.
> 
> Signed-off-by: Dumitru Ceara 
> ---
> V6:
> - Removed group_dpif_format().
> - Cleaned up test.
> V5:
> - Addressed Ilya's comments:
>   - I addressed all from here
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-June/415144.html
> except the one about adding -m/-mm.  It felt OK to always dump
> statistics.
> V4:
> - Addressed Mike's comment:
>   - use predictable port IDs.
> V3:
> - Addressed Mike's comments:
>   - use ds_put_char()
>   - make the test less flaky
> V2:
> - Addressed Adrian's comments:
>   - check return value of populate_xcache()
>   - use flow_stats_ds() (renamed to ofproto_rule_stats_ds()) instead of
> custom printing
>   - move ukey->state check to caller
>   - handle case when group bucket is not available
>   - update test case to cover the point above
> ---
>  NEWS   |   2 +
>  include/openvswitch/ofp-group.h|  14 +++
>  lib/ofp-group.c| 131 +
>  ofproto/ofproto-dpif-upcall.c  |  51 +++
>  ofproto/ofproto-dpif-xlate-cache.c |  34 
>  ofproto/ofproto-dpif-xlate-cache.h |   2 +
>  ofproto/ofproto-provider.h |   2 +
>  ofproto/ofproto.c  |  11 +--
>  tests/ofproto-dpif.at  |  56 
>  tests/ofproto-macros.at|  14 ++-
>  10 files changed, 257 insertions(+), 60 deletions(-)

Thanks, Dumitru!  Applied.

Best regards, Ilya Maximets.

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

[ovs-dev] [Patch ovn 2/2] northd: BGP port mirroring.

[Martin Kalcok]

This change adds a 'bgp-mirror' option to LRP that allows
mirroring of BGP control plane traffic to an arbitrary LSP
in its peer LS.

The option expects a string with a LSP name. When set,
any traffic entering LS that's destined for any of the
LRP's IP addresses (including IPv6 LLA) is redirected
to the LSP specified in the option's value.

This enables external BGP daemons to listen on an interface
bound to a LSP and effectively act as if they were listening
on (and speaking from) LRP's IP address.

Signed-off-by: Martin Kalcok 
---
 northd/northd.c | 87 +
 northd/ovn-northd.8.xml | 23 +++
 ovn-nb.xml  | 14 +++
 tests/ovn-northd.at | 45 +
 tests/system-ovn.at | 82 ++
 5 files changed, 251 insertions(+)

diff --git a/northd/northd.c b/northd/northd.c
index 4353df07d..e07bf68cc 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -13048,6 +13048,92 @@ build_arp_resolve_flows_for_lrp(struct ovn_port *op,
 }
 }
 
+static void
+build_bgp_redirect_rule__(
+const char *s_addr, const char *bgp_port_name, bool is_ipv6,
+struct ovn_port *ls_peer, struct lflow_table *lflows,
+struct ds *match, struct ds *actions)
+{
+int ip_ver = is_ipv6 ? 6 : 4;
+/* Redirect packets in the input pipeline destined for LR's IP to
+ * the port specified in 'bgp-mirror' option.
+ */
+ds_clear(match);
+ds_clear(actions);
+ds_put_format(match, "ip%d.dst == %s && tcp.dst == 179", ip_ver, s_addr);
+ds_put_format(actions, "outport = \"%s\"; output;", bgp_port_name);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_L2_LKUP, 100,
+  ds_cstr(match),
+  ds_cstr(actions),
+  ls_peer->lflow_ref);
+
+
+/* Drop any traffic originating from 'bgp-mirror' port that does
+ * not originate from BGP daemon port. This blocks unnecessary
+ * traffic like ARP broadcasts or IPv6 router solicitation packets
+ * from the dummy 'bgp-mirror' port.
+ */
+ds_clear(match);
+ds_put_format(match, "inport == \"%s\"", bgp_port_name);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_CHECK_PORT_SEC, 80,
+  ds_cstr(match),
+  REGBIT_PORT_SEC_DROP " = 1; next;",
+  ls_peer->lflow_ref);
+
+ds_put_format(match,
+  " && ip%d.src == %s && tcp.src == 179",
+  ip_ver,
+  s_addr);
+ovn_lflow_add(lflows, ls_peer->od, S_SWITCH_IN_CHECK_PORT_SEC, 81,
+  ds_cstr(match),
+  REGBIT_PORT_SEC_DROP " = check_in_port_sec(); next;",
+  ls_peer->lflow_ref);
+}
+
+static void
+build_lrouter_bgp_redirect(
+struct ovn_port *op, struct lflow_table *lflows,
+struct ds *match, struct ds *actions)
+{
+/* LRP has to have a peer.*/
+if (op->peer == NULL) {
+return;
+}
+/* LRP has to have NB record.*/
+if (op->nbrp == NULL) {
+return;
+}
+
+/* Proceed only for LRPs that have 'bgp-mirror' option set. Value of this
+ * option is the name of LSP to which a BGP traffic will be mirrored.
+ */
+const char *bgp_port = smap_get(>nbrp->options, "bgp-mirror");
+if (bgp_port == NULL) {
+return;
+}
+
+if (op->cr_port != NULL) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "Option 'bgp-mirror' is not supported on"
+  " Distributed Gateway Port '%s'", op->key);
+return;
+}
+
+/* Mirror traffic destined for LRP's IPs and default BGP port
+ * to the port defined in 'bgp-mirror' option.
+ */
+for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+const char *ip_s = op->lrp_networks.ipv4_addrs[i].addr_s;
+build_bgp_redirect_rule__(ip_s, bgp_port, false,  op->peer, lflows,
+  match, actions);
+}
+for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
+const char *ip_s = op->lrp_networks.ipv6_addrs[i].addr_s;
+build_bgp_redirect_rule__(ip_s, bgp_port, true, op->peer, lflows,
+  match, actions);
+}
+}
+
 /* This function adds ARP resolve flows related to a LSP. */
 static void
 build_arp_resolve_flows_for_lsp(
@@ -16003,6 +16089,7 @@ build_lswitch_and_lrouter_iterate_by_lrp(struct 
ovn_port *op,
 lsi->meter_groups, op->lflow_ref);
 build_lrouter_icmp_packet_toobig_admin_flows(op, lsi->lflows, >match,
  >actions, op->lflow_ref);
+build_lrouter_bgp_redirect(op, lsi->lflows, >match, >actions);
 }
 
 static void *
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b06b09ac5..1bf9d2dc0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -284,6 +284,21 @@
 dropped 

[ovs-dev] [Patch ovn 0/2] Add LRP option for mirroring of BGP traffic.

[Martin Kalcok]

This series of patches introduces new Logical Router Port option
called 'bgp-mirror' that enables mirroring of BGP control plane
traffic to an arbitrary port. More info in commit message 2/2.

Although not directly dependant, this change is intended go
hand-in-hand with a series posted by fnordhal[0].

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2024-July/415795.html

Martin Kalcok (2):
  test macros: Ensure ADD_VETH macro sets IPv6 LLA.
  northd: BGP port mirroring.

 northd/northd.c   | 87 +++
 northd/ovn-northd.8.xml   | 23 +
 ovn-nb.xml| 14 ++
 tests/ovn-northd.at   | 45 ++
 tests/system-common-macros.at |  3 ++
 tests/system-ovn.at   | 82 +
 6 files changed, 254 insertions(+)

-- 
2.40.1

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

[ovs-dev] [Patch ovn 1/2] test macros: Ensure ADD_VETH macro sets IPv6 LLA.

[Martin Kalcok]

Test macro ADD_VET takes an optional argument to specify
MAC address for the interface. However this address is
set only after the interface is brought UP, causing it
to keep IPv6 Link Local Address based on its old MAC.

Toggling the interface down/up after a new MAC address
is set fixes this issue.

Signed-off-by: Martin Kalcok 
---
 tests/system-common-macros.at | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 691c271a3..e7dee9d28 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -69,6 +69,9 @@ m4_define([ADD_VETH],
   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])
+# Toggle the interface down/up to get an IPv6 LLA that matches its MAC 
addr.
+NS_CHECK_EXEC([$2], [ip link set down $1])
+NS_CHECK_EXEC([$2], [ip link set up $1])
   fi
   if test -n "$6"; then
 NS_CHECK_EXEC([$2], [ip route add $6 dev $1])
-- 
2.40.1

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

Re: [ovs-dev] [PATCH v3 0/3] Fix redundant mirror with tunnel options.

[Mike Pattrick]

On Fri, Oct 13, 2023 at 6:59 AM Eelco Chaudron  wrote:
>
>
>
> On 9 Oct 2023, at 14:05, Roi Dayan wrote:
>
> > Hi,
> >
> > The first commit removing the redundant mirror when using tunnel options.
> > The second is a test to verify it stays like this and doesn't break again.
> > The third commit is just updating prefixes of tests to match the file their 
> > in.
> >
> > Thanks,
> > Roi
>
> I’ve applied the series to master. Once again, thanks for the patch.

Hello,

Is it possible to apply this bugfix to 3.1 and 3.2 as well?


Thank you,
Mike

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

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

Re: [ovs-dev] [PATCH ovn 5/5] controller: Introduce route-exchange module.

[0-day Robot]

Bleep bloop.  Greetings Frode Nordahl, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Comment with 'xxx' marker
#336 FILE: controller/route-exchange.c:86:
/* XXX: nat_addresses can also contain the LRP IP without any

Lines checked: 527, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v9] rhel: Make the version, displayed to the user, customizable.

[Ilya Maximets]

On 7/15/24 10:48, Timothy Redaelli wrote:
> On Fri, 12 Jul 2024 18:14:28 +0200
> Ilya Maximets  wrote:
> 
>> On 7/10/24 13:06, Timothy Redaelli wrote:
>>> Since on CentOS/RHEL the builds are based on stable branches and not on
>>> tags for debugging purpose it's better to have the downstream version as
>>> version so it's easier to know which commits are included in a build.
>>>
>>> This commit adds --with-version-suffix as ./configure option in
>>> order to set an OVS version suffix that should be shown to the user via
>>> ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
>>> utilities.
>>>
>>> --with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to 
>>> have
>>> the version be aligned with the downstream one.
>>>
>>> Signed-off-by: Timothy Redaelli 
>>> ---
>>> v1 -> v2: Use --with-version-suffix= and add version to other utilies
>>>   (as requested by Ilya).
>>>
>>> v2 -> v3: Add versioning to python utilities and python library itself
>>>   (as suggested by Aaron).
>>>
>>> v3 -> v4: Remove versioning to python library itself to avoid breaking
>>>   PEP440 (as requested by Ilya). Versioning is still used in
>>>   python utilities.
>>>
>>> v4 -> v5: Re-add versioning to python library itself, but don't use it on
>>>   setup.py (to avoid breaking PEP440). This will permit to have the
>>>   custom version as ovs.version.VERSION (in case somebody uses it) 
>>> and,
>>>   so, also in python/ovs/unixctl/server.py (as suggested by Ilya).
>>>
>>> v5 -> v6: Fix some setup.py leftovers and change the test as a loop
>>>   (as suggested by Ilya).
>>>
>>> v6 -> v7: Rebase with last master (it should pass CI now)
>>>
>>> v7 -> v8: Be sure python-sdist depends on python/setup.py and run flake8 on
>>>   setup.py.template instead of setup.py (as suggested by Ilya).
>>>   Fix commit summary to make checkpatch.py happy
>>>
>>> v8 -> v9: Fix make distcheck when --with-version-suffix is specified (as
>>>   repoted by Ilya). I know AM_DISTCHECK_CONFIGURE_FLAGS is
>>>   discouraged upstream, but it's also used by GNU m4 for a similar
>>>   scenario so I guess it's ok to use it (see
>>>   
>>> https://www.gnu.org/software/automake/manual/html_node/Checking-the-Distribution.html)
>>>   Restore the loop in setup.py.template (as reported by Ilya)
>>>   Use version suffixalso for libraries (aka test-lib) (as reported 
>>> by Ilya)
>>>   Fix a typo in configure (as reported by Ilya)
>>> ---
>>>  Makefile.am|  3 +++
>>>  acinclude.m4   | 13 
>>>  configure.ac   |  1 +
>>>  include/openvswitch/version.h.in   |  2 +-
>>>  lib/ovsdb-error.c  |  2 +-
>>>  lib/util.c |  8 +---
>>>  ovsdb/ovsdb-server.c   |  3 ++-
>>>  python/.gitignore  |  1 +
>>>  python/automake.mk | 22 +---
>>>  python/{setup.py => setup.py.template} | 28 +-
>>>  rhel/openvswitch-fedora.spec.in|  1 +
>>>  utilities/ovs-dpctl-top.in |  2 +-
>>>  utilities/ovs-lib.in   |  2 +-
>>>  utilities/ovs-parse-backtrace.in   |  2 +-
>>>  utilities/ovs-pcap.in  |  2 +-
>>>  utilities/ovs-pki.in   |  2 +-
>>>  utilities/ovs-tcpdump.in   |  4 ++--
>>>  utilities/ovs-tcpundump.in |  2 +-
>>>  utilities/ovs-vlan-test.in |  2 +-
>>>  vswitchd/bridge.c  |  3 ++-
>>>  20 files changed, 64 insertions(+), 41 deletions(-)
>>>  rename python/{setup.py => setup.py.template} (87%)
>>>



>>> @@ -147,6 +146,15 @@ $(srcdir)/python/ovs/dirs.py: 
>>> python/ovs/dirs.py.template
>>>  EXTRA_DIST += python/ovs/dirs.py.template
>>>  CLEANFILES += python/ovs/dirs.py
>>>  
>>> +ALL_LOCAL += $(srcdir)/python/setup.py
>>> +$(srcdir)/python/setup.py: python/setup.py.template
>>> +   $(AM_V_GEN)sed \
>>> +   -e 's,[@]VERSION[@],$(VERSION),g' \
>>> +   < $? > $@.tmp && \
>>> +   mv $@.tmp $@
>>
>> This target needs a dependency on config.status the same as version.py.
>> Otherwise, if we change the version number in configure.ac, the setup.py
>> is not getting re-generated.
>>
>> I think, we need something like this:
>>
>> diff --git a/python/automake.mk b/python/automake.mk
>> index 1e7563156..d0523870d 100644
>> --- a/python/automake.mk
>> +++ b/python/automake.mk
>> @@ -147,11 +147,11 @@ EXTRA_DIST += python/ovs/dirs.py.template
>>  CLEANFILES += python/ovs/dirs.py
>>  
>>  ALL_LOCAL += $(srcdir)/python/setup.py
>> -$(srcdir)/python/setup.py: python/setup.py.template
>> +$(srcdir)/python/setup.py: python/setup.py.template config.status
>> $(AM_V_GEN)sed \
>> -e 's,[@]VERSION[@],$(VERSION),g' \
>> -   < $? > $@.tmp && \

[ovs-dev] [PATCH ovn 3/5] controller: Introduce route-exchange-netlink module.

[Frode Nordahl]

We want to export host routes to resources such as NAT addresses
and LB VIPs to routing protocol suite software running on the same
system.

Such routing protocol software conveniently interfaces with
netlink, and has support for interacting with non-standard routing
tables through the use of VRFs [0].

Introduce route-exchange-netlink module which implements interface
for maintaining VRFs and host routes through Netlink.

0: https://docs.kernel.org/networking/vrf.html

Signed-off-by: Frode Nordahl 
---
 configure.ac|   2 +
 controller/automake.mk  |   5 +-
 controller/route-exchange-netlink-private.h | 243 +++
 controller/route-exchange-netlink.c | 244 
 controller/route-exchange-netlink.h |  37 +++
 m4/ovn.m4   |  25 ++
 6 files changed, 555 insertions(+), 1 deletion(-)
 create mode 100644 controller/route-exchange-netlink-private.h
 create mode 100644 controller/route-exchange-netlink.c
 create mode 100644 controller/route-exchange-netlink.h

diff --git a/configure.ac b/configure.ac
index 6a6b0db6a..6f0f485c4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -87,6 +87,8 @@ OVS_CHECK_WIN32
 OVS_CHECK_VISUAL_STUDIO_DDK
 OVN_CHECK_COVERAGE
 OVS_CHECK_NDEBUG
+OVS_CHECK_NETLINK
+OVS_CHECK_LINUX_NETLINK
 OVS_CHECK_OPENSSL
 OVN_CHECK_LOGDIR
 OVN_CHECK_PYTHON3
diff --git a/controller/automake.mk b/controller/automake.mk
index ed93cfb3c..70facdd72 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -49,7 +49,10 @@ controller_ovn_controller_SOURCES = \
controller/statctrl.h \
controller/statctrl.c \
controller/ct-zone.h \
-   controller/ct-zone.c
+   controller/ct-zone.c \
+   controller/route-exchange-netlink.h \
+   controller/route-exchange-netlink-private.h \
+   controller/route-exchange-netlink.c
 
 controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
 man_MANS += controller/ovn-controller.8
diff --git a/controller/route-exchange-netlink-private.h 
b/controller/route-exchange-netlink-private.h
new file mode 100644
index 0..4c2559895
--- /dev/null
+++ b/controller/route-exchange-netlink-private.h
@@ -0,0 +1,243 @@
+/*
+ * Copyright (c) 2024 Canonical, Ltd.
+ * Copyright (c) 2011, 2012, 2013, 2014, 2017 Nicira, 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.
+ */
+
+#ifndef ROUTE_EXCHANGE_NETLINK_PRIVATE_H
+#define ROUTE_EXCHANGE_NETLINK_PRIVATE_H 1
+
+/*
+ * NOTE(fnordahl): The below code is stolen directly from OVS lib/route-table.c
+ * with the addition of inlining of function definitions for practical reasons
+ * and modifications:
+ *
+ * struct route_data:
+ *
+ * - Add rta_table_id.
+ *
+ * route_table_parse():
+ *
+ * - Consider non-standard routing tables and store the table_id.
+ *
+ * route_table_dump_one_table():
+ *
+ * - Use uint32_t for table id and pass it to kernel using thee RTA_TABLE
+ *   attribute to allow use of table IDs greater than 256.
+ * - Use callback with argument instead of hard coded call to static function
+ *   route_table_handle_msg().
+ *
+ * Ideally we would upstream those changes along with export of interesting
+ * data structures and functions to OVS, but in the interest of time we vendor
+ * the code here for now.
+ *
+ * BEGIN VENDORED CODE FROM OVS lib/route-table.c
+ */
+struct route_data {
+/* Copied from struct rtmsg. */
+unsigned char rtm_dst_len;
+bool local;
+
+/* Extracted from Netlink attributes. */
+struct in6_addr rta_dst; /* 0 if missing. */
+struct in6_addr rta_prefsrc; /* 0 if missing. */
+struct in6_addr rta_gw;
+char ifname[IFNAMSIZ]; /* Interface name. */
+uint32_t mark;
+uint32_t rta_table_id; /* 0 if missing. */
+};
+
+/* A digested version of a route message sent down by the kernel to indicate
+ * that a route has changed. */
+struct route_table_msg {
+bool relevant;/* Should this message be processed? */
+int nlmsg_type;   /* e.g. RTM_NEWROUTE, RTM_DELROUTE. */
+struct route_data rd; /* Data parsed from this message. */
+};
+
+/* Return RTNLGRP_IPV4_ROUTE or RTNLGRP_IPV6_ROUTE on success, 0 on parse
+ * error. */
+static inline int
+route_table_parse(struct ofpbuf *buf, struct route_table_msg *change)
+{
+bool parsed, ipv4 = false;
+
+static const struct nl_policy policy[] = {
+[RTA_DST] = { .type = NL_A_U32, .optional = true 

[ovs-dev] [PATCH ovn 2/5] controller: Move LB by dp helpers to lb module.

[Frode Nordahl]

In a subsequent patch we need these functions in the
route-exchange module.

Signed-off-by: Frode Nordahl 
---
 controller/lb.c | 119 +
 controller/lb.h |  19 ++
 controller/ovn-controller.c | 128 
 3 files changed, 138 insertions(+), 128 deletions(-)

diff --git a/controller/lb.c b/controller/lb.c
index 8f9f20ed5..8dd748d64 100644
--- a/controller/lb.c
+++ b/controller/lb.c
@@ -23,6 +23,7 @@
 /* OVN includes */
 #include "lb.h"
 #include "lib/ovn-sb-idl.h"
+#include "local_data.h"
 #include "ovn/lex.h"
 
 VLOG_DEFINE_THIS_MODULE(controller_lb);
@@ -144,3 +145,121 @@ ovn_controller_lb_find(const struct hmap 
*ovn_controller_lbs,
 return NULL;
 }
 
+static struct load_balancers_by_dp *
+load_balancers_by_dp_create(struct hmap *lbs,
+const struct sbrec_datapath_binding *dp)
+{
+struct load_balancers_by_dp *lbs_by_dp = xzalloc(sizeof *lbs_by_dp);
+
+lbs_by_dp->dp = dp;
+hmap_insert(lbs, _by_dp->node, hash_uint64(dp->tunnel_key));
+return lbs_by_dp;
+}
+
+static void
+load_balancers_by_dp_destroy(struct load_balancers_by_dp *lbs_by_dp)
+{
+if (!lbs_by_dp) {
+return;
+}
+
+free(lbs_by_dp->dp_lbs);
+free(lbs_by_dp);
+}
+
+struct load_balancers_by_dp *
+load_balancers_by_dp_find(struct hmap *lbs,
+  const struct sbrec_datapath_binding *dp)
+{
+uint32_t hash = hash_uint64(dp->tunnel_key);
+struct load_balancers_by_dp *lbs_by_dp;
+
+HMAP_FOR_EACH_WITH_HASH (lbs_by_dp, node, hash, lbs) {
+if (lbs_by_dp->dp == dp) {
+return lbs_by_dp;
+}
+}
+return NULL;
+}
+
+static void
+load_balancers_by_dp_add_one(const struct hmap *local_datapaths,
+ const struct sbrec_datapath_binding *datapath,
+ const struct sbrec_load_balancer *lb,
+ struct hmap *lbs)
+{
+struct local_datapath *ldp =
+get_local_datapath(local_datapaths, datapath->tunnel_key);
+
+if (!ldp) {
+return;
+}
+
+struct load_balancers_by_dp *lbs_by_dp =
+load_balancers_by_dp_find(lbs, ldp->datapath);
+if (!lbs_by_dp) {
+lbs_by_dp = load_balancers_by_dp_create(lbs, ldp->datapath);
+}
+
+if (lbs_by_dp->n_dp_lbs == lbs_by_dp->n_allocated_dp_lbs) {
+lbs_by_dp->dp_lbs = x2nrealloc(lbs_by_dp->dp_lbs,
+   _by_dp->n_allocated_dp_lbs,
+   sizeof *lbs_by_dp->dp_lbs);
+}
+lbs_by_dp->dp_lbs[lbs_by_dp->n_dp_lbs++] = lb;
+}
+
+/* Builds and returns a hmap of 'load_balancers_by_dp', one record for each
+ * local datapath.
+ */
+struct hmap *
+load_balancers_by_dp_init(const struct hmap *local_datapaths,
+  const struct sbrec_load_balancer_table *lb_table)
+{
+struct hmap *lbs = xmalloc(sizeof *lbs);
+hmap_init(lbs);
+
+const struct sbrec_load_balancer *lb;
+SBREC_LOAD_BALANCER_TABLE_FOR_EACH (lb, lb_table) {
+for (size_t i = 0; i < lb->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->datapaths[i], lb, lbs);
+}
+/* datapath_group column is deprecated. */
+for (size_t i = 0; lb->datapath_group
+   && i < lb->datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->datapath_group->datapaths[i],
+ lb, lbs);
+}
+for (size_t i = 0; lb->ls_datapath_group
+   && i < lb->ls_datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->ls_datapath_group->datapaths[i],
+ lb, lbs);
+}
+for (size_t i = 0; lb->lr_datapath_group
+   && i < lb->lr_datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->lr_datapath_group->datapaths[i],
+ lb, lbs);
+}
+}
+return lbs;
+}
+
+void
+load_balancers_by_dp_cleanup(struct hmap *lbs)
+{
+if (!lbs) {
+return;
+}
+
+struct load_balancers_by_dp *lbs_by_dp;
+
+HMAP_FOR_EACH_POP (lbs_by_dp, node, lbs) {
+load_balancers_by_dp_destroy(lbs_by_dp);
+}
+hmap_destroy(lbs);
+free(lbs);
+}
diff --git a/controller/lb.h b/controller/lb.h
index 84d51c332..86e6e611b 100644
--- a/controller/lb.h
+++ b/controller/lb.h
@@ -20,6 +20,7 @@
 #include "lib/lb.h"
 
 struct sbrec_load_balancer;
+struct sbrec_load_balancer_table;
 
 struct ovn_controller_lb {
 struct hmap_node hmap_node;
@@ -41,6 +42,15 @@ struct ovn_controller_lb {
  

[ovs-dev] [PATCH ovn 1/5] controller: Move address with port parser to lib.

[Frode Nordahl]

We will need parsing of Port_Binding->nat_addresses in other
parts of the ovn-controller in subsequent patches.

Move extract_addresses_with_port() to the lib/ ovn-util module.

Signed-off-by: Frode Nordahl 
---
 controller/pinctrl.c | 67 
 lib/ovn-util.c   | 66 +++
 lib/ovn-util.h   |  4 +++
 3 files changed, 70 insertions(+), 67 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 6a4299b82..708240e24 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6377,73 +6377,6 @@ get_localnet_vifs_l3gwports(
 sbrec_port_binding_index_destroy_row(target);
 }
 
-
-/* Extracts the mac, IPv4 and IPv6 addresses, and logical port from
- * 'addresses' which should be of the format 'MAC [IP1 IP2 ..]
- * [is_chassis_resident("LPORT_NAME")]', where IPn should be a valid IPv4
- * or IPv6 address, and stores them in the 'ipv4_addrs' and 'ipv6_addrs'
- * fields of 'laddrs'.  The logical port name is stored in 'lport'.
- *
- * Returns true if at least 'MAC' is found in 'address', false otherwise.
- *
- * The caller must call destroy_lport_addresses() and free(*lport). */
-static bool
-extract_addresses_with_port(const char *addresses,
-struct lport_addresses *laddrs,
-char **lport)
-{
-int ofs;
-if (!extract_addresses(addresses, laddrs, )) {
-return false;
-} else if (!addresses[ofs]) {
-return true;
-}
-
-struct lexer lexer;
-lexer_init(, addresses + ofs);
-lexer_get();
-
-if (lexer.error || lexer.token.type != LEX_T_ID
-|| !lexer_match_id(, "is_chassis_resident")) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_INFO_RL(, "invalid syntax '%s' in address", addresses);
-lexer_destroy();
-return true;
-}
-
-if (!lexer_match(, LEX_T_LPAREN)) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_INFO_RL(, "Syntax error: expecting '(' after "
-  "'is_chassis_resident' in address '%s'", addresses);
-lexer_destroy();
-return false;
-}
-
-if (lexer.token.type != LEX_T_STRING) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_INFO_RL(,
-"Syntax error: expecting quoted string after "
-"'is_chassis_resident' in address '%s'", addresses);
-lexer_destroy();
-return false;
-}
-
-*lport = xstrdup(lexer.token.s);
-
-lexer_get();
-if (!lexer_match(, LEX_T_RPAREN)) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_INFO_RL(, "Syntax error: expecting ')' after quoted string in "
-  "'is_chassis_resident()' in address '%s'",
-  addresses);
-lexer_destroy();
-return false;
-}
-
-lexer_destroy();
-return true;
-}
-
 static void
 consider_nat_address(struct ovsdb_idl_index *sbrec_port_binding_by_name,
  const char *nat_address,
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 58e941193..c1557f42d 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -1314,3 +1314,69 @@ ovn_update_swconn_at(struct rconn *swconn, const char 
*target,
 
 return notify;
 }
+
+/* Extracts the mac, IPv4 and IPv6 addresses, and logical port from
+ * 'addresses' which should be of the format 'MAC [IP1 IP2 ..]
+ * [is_chassis_resident("LPORT_NAME")]', where IPn should be a valid IPv4
+ * or IPv6 address, and stores them in the 'ipv4_addrs' and 'ipv6_addrs'
+ * fields of 'laddrs'.  The logical port name is stored in 'lport'.
+ *
+ * Returns true if at least 'MAC' is found in 'address', false otherwise.
+ *
+ * The caller must call destroy_lport_addresses() and free(*lport). */
+bool
+extract_addresses_with_port(const char *addresses,
+struct lport_addresses *laddrs,
+char **lport)
+{
+int ofs;
+if (!extract_addresses(addresses, laddrs, )) {
+return false;
+} else if (!addresses[ofs]) {
+return true;
+}
+
+struct lexer lexer;
+lexer_init(, addresses + ofs);
+lexer_get();
+
+if (lexer.error || lexer.token.type != LEX_T_ID
+|| !lexer_match_id(, "is_chassis_resident")) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+VLOG_INFO_RL(, "invalid syntax '%s' in address", addresses);
+lexer_destroy();
+return true;
+}
+
+if (!lexer_match(, LEX_T_LPAREN)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+VLOG_INFO_RL(, "Syntax error: expecting '(' after "
+  "'is_chassis_resident' in address '%s'", addresses);
+lexer_destroy();
+return false;
+}
+
+if (lexer.token.type != LEX_T_STRING) {
+static struct 

[ovs-dev] [PATCH ovn 5/5] controller: Introduce route-exchange module.

[Frode Nordahl]

Introduce route-exchange module that depending on Logical Router
Port options maintains a VRF in the system for redistribution of
host routes to NAT addresses and LB VIPs attached to local gateway
router datapaths.

The route-exchange module requires input from both runtime_data
and lb_data engine nodes.  Consequently it needs its own I-P
engine node.

Signed-off-by: Frode Nordahl 
---
 controller/automake.mk  |   4 +-
 controller/ovn-controller.c | 159 +
 controller/route-exchange.c | 229 
 controller/route-exchange.h |  38 ++
 4 files changed, 429 insertions(+), 1 deletion(-)
 create mode 100644 controller/route-exchange.c
 create mode 100644 controller/route-exchange.h

diff --git a/controller/automake.mk b/controller/automake.mk
index 70facdd72..a824aee28 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -52,7 +52,9 @@ controller_ovn_controller_SOURCES = \
controller/ct-zone.c \
controller/route-exchange-netlink.h \
controller/route-exchange-netlink-private.h \
-   controller/route-exchange-netlink.c
+   controller/route-exchange-netlink.c \
+   controller/route-exchange.h \
+   controller/route-exchange.c
 
 controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
 man_MANS += controller/ovn-controller.8
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index f46edd22d..52deae439 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -87,6 +87,7 @@
 #include "statctrl.h"
 #include "lib/dns-resolve.h"
 #include "ct-zone.h"
+#include "route-exchange.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -4572,6 +4573,14 @@ controller_output_mac_cache_handler(struct engine_node 
*node,
 return true;
 }
 
+static bool
+controller_output_route_exchange_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+engine_set_node_state(node, EN_UPDATED);
+return true;
+}
+
 /* Handles sbrec_chassis changes.
  * If a new chassis is added or removed return false, so that
  * flows are recomputed.  For any updates, there is no need for
@@ -4595,6 +4604,142 @@ pflow_lflow_output_sb_chassis_handler(struct 
engine_node *node,
 return true;
 }
 
+struct ed_type_route_exchange {
+};
+
+static void
+en_route_exchange_run(struct engine_node *node,
+  void *data OVS_UNUSED)
+{
+const struct ovsrec_open_vswitch_table *ovs_table =
+EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
+const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ovs_assert(chassis_id);
+
+struct ovsdb_idl_index *sbrec_chassis_by_name =
+engine_ovsdb_node_get_index(
+engine_get_input("SB_chassis", node),
+"name");
+const struct sbrec_chassis *chassis
+= chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
+ovs_assert(chassis);
+
+struct ovsdb_idl_index *sbrec_port_binding_by_name =
+engine_ovsdb_node_get_index(
+engine_get_input("SB_port_binding", node),
+"name");
+struct ed_type_runtime_data *rt_data =
+engine_get_input_data("runtime_data", node);
+
+const struct sbrec_load_balancer_table *lb_table =
+EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
+struct ed_type_lb_data *lb_data =
+engine_get_input_data("lb_data", node);
+
+struct route_exchange_ctx_in r_ctx_in = {
+.sbrec_port_binding_by_name = sbrec_port_binding_by_name,
+.lb_table = lb_table,
+.chassis_rec = chassis,
+.active_tunnels = _data->active_tunnels,
+.local_datapaths = _data->local_datapaths,
+.local_lbs = _data->local_lbs,
+};
+
+route_exchange_run(_ctx_in);
+
+engine_set_node_state(node, EN_UPDATED);
+}
+
+
+static void *
+en_route_exchange_init(struct engine_node *node OVS_UNUSED,
+   struct engine_arg *arg OVS_UNUSED)
+{
+return NULL;
+}
+
+static void
+en_route_exchange_cleanup(void *data OVS_UNUSED)
+{
+}
+
+static bool
+route_exchange_runtime_data_handler(struct engine_node *node,
+void *data OVS_UNUSED)
+{
+struct ed_type_runtime_data *rt_data =
+engine_get_input_data("runtime_data", node);
+
+if (!rt_data->tracked) {
+return false;
+}
+
+struct tracked_datapath *tdp;
+HMAP_FOR_EACH (tdp, node, _data->tracked_dp_bindings) {
+struct shash_node *shash_node;
+SHASH_FOR_EACH (shash_node, >lports) {
+struct tracked_lport *lport = shash_node->data;
+if (route_exchange_relevant_port(lport->pb)) {
+/* Until we get I-P support for route exchange we need to
+ * request recompute. */
+return false;
+}
+}
+}
+
+return true;
+}
+
+static bool
+route_exchange_lb_data_handler(struct 

[ovs-dev] [PATCH ovn 4/5] northd: Add options for host route redistribution.

[Frode Nordahl]

Add three new options for Logical Router Ports that control
host route redistribution.

Signed-off-by: Frode Nordahl 
---
 northd/northd.c | 11 +++
 ovn-nb.xml  | 39 +++
 2 files changed, 50 insertions(+)

diff --git a/northd/northd.c b/northd/northd.c
index 6898daa00..c76501549 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3939,6 +3939,17 @@ sync_pb_for_lrp(struct ovn_port *op,
 }
 if (chassis_name) {
 smap_add(, "l3gateway-chassis", chassis_name);
+if (smap_get_bool(>nbrp->options, "maintain-vrf", false)) {
+smap_add(, "maintain-vrf", "true");
+}
+if (smap_get_bool(>nbrp->options,
+  "redistribute-nat", false)) {
+smap_add(, "redistribute-nat", "true");
+}
+if (smap_get_bool(>nbrp->options,
+  "redistribute-lb-vips", false)) {
+smap_add(, "redistribute-lb-vips", "true");
+}
 }
 }
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 9552534f6..97ac98e96 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -3451,6 +3451,45 @@ or
option.
 
   
+
+  
+
+  Redistribute host routes to Load Balancer VIPs local to this chassis.
+
+  This option currently only works on gateway routers (routers that
+  have 
+  set).
+
+  
+
+  
+
+  Redistribute host routes to dnat_and_snat NAT records
+  associated with logical ports local to this chassis.
+
+  This option currently only works on gateway routers (routers that
+  have 
+  set).
+
+  
+
+  
+
+  Maintain a VRF in the system for exporting host routes to NAT
+  addresses (see 
+  option) and Load Balancer VIPs (see  option).
+
+  Route table ID to use can be influenced by setting the
+   option.  If not
+  set, ovn-northd will allocate one.
+
+  This option currently only works on gateway routers (routers that
+  have 
+  set).
+
+  
+
 
 
 
-- 
2.45.2

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

[ovs-dev] OVN 24.09 Soft Freeze 19 July, 2024

[Mark Michelson]

Hi everyone,

This Friday, 19 July, 2024, is the scheduled soft freeze for OVN 24.09. 
As a reminder, this is the deadline to submit new feature patches for 
OVN 24.09. Please submit your new feature patches for review before that 
date. Bug fix patches can be submitted at any time.


We will create the 24.09 branch two weeks after the soft freeze date on 
2 August, 2024, and we plan to release 24.09.0 four weeks after the 
branch is created on 6 September, 2024.


Thank you,
Mark Michelson

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

Re: [ovs-dev] [PATCH] dpctl: Netdev leak causes stale VHU & reaching limit.

[Simon Horman]

On Fri, Jul 12, 2024 at 09:51:18AM +0200, David Marchand wrote:
> On Thu, Jul 11, 2024 at 10:18 PM Vipul Ashri via dev
>  wrote:
> >
> >
> > While running a test with a continous VM creation/deletion using an
> > orchestration script with-in cloud environment. In parallel we have
> > some monitoring script calling ovs-appctl dpctl/show stats commands
> > every minute.
> >
> > During VHU port delete, one of netdev references were not reduced to
> > 0 as show_dpif call has not given-up the reference back or doing bad
> > cleanup. This pending deference preventing VHU deletion sequence, this
> > is found to be one of corner case inside dpctl code which results in
> > leaking up netdev which ultimately results in stale VHU entry. After
> > fixing this problematic cleanup, issue is not seen.
> >
> > Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> > dpif-netdev")
> > Signed-off-by: Vipul Ashri 
> 
> The issue seems generic as any netdev for which a call to
> netdev_get_stats failed would trigger the same leak.
> So I would update the commit title with something more generic like:
> dpctl: Fix netdev reference leak in "show" command.
> 
> This can probably be changed when applying.
> In any case, the fix lgtm.
> 
> Reviewed-by: David Marchand 

Thanks David,

That sounds find to me.

There was another posting of this patch [1].
I'll plan to apply that one in a day or so
unless someone says otherwise.

[1] 
https://patchwork.ozlabs.org/project/openvswitch/patch/pawpr07mb9877c192d7e7712c72a403ee90...@pawpr07mb9877.eurprd07.prod.outlook.com/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpctl: Netdev leak causes stale VHU & reaching limit.

[Simon Horman]

On Mon, Jul 15, 2024 at 02:55:29PM +0100, Simon Horman wrote:
> On Thu, Jul 11, 2024 at 08:20:28PM +, Vipul Ashri via dev wrote:
> > 
> > While running a test with a continous VM creation/deletion using an
> > orchestration script with-in cloud environment. In parallel we have
> > some monitoring script calling ovs-appctl dpctl/show stats commands
> > every minute.
> > 
> > During VHU port delete, one of netdev references were not reduced to
> > 0 as show_dpif call has not given-up the reference back or doing bad
> > cleanup. This pending deference preventing VHU deletion sequence, this
> > is found to be one of corner case inside dpctl code which results in
> > leaking up netdev which ultimately results in stale VHU entry. After
> > fixing this problematic cleanup, issue is not seen.
> > 
> > Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> > dpif-netdev")
> > Signed-off-by: Vipul Ashri 
> 
> Acked-by: Simon Horman 

I now see this is a duplicate of an earlier posting [1] (why?).

[1] 
https://patchwork.ozlabs.org/project/openvswitch/patch/pawpr07mb9877859e07fbad793eefd01d90...@pawpr07mb9877.eurprd07.prod.outlook.com/

In that thread David Marchard (CCed) said:

"The issue seems generic as any netdev for which a call to
 netdev_get_stats failed would trigger the same leak.
 So I would update the commit title with something more generic like:
 dpctl: Fix netdev reference leak in "show" command.

"This can probably be changed when applying.
 In any case, the fix lgtm.

"Reviewed-by: David Marchand 

Which all seems reasonable to me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] tests: Do not use check in VXLAN mode disable test.

[Ihar Hrachyshka]

Thanks. (Confirmed the other test case that covers tunnel exhaustion
scenarios already uses `eval`.)

Reviewed-By: Ihar Hrachyshka 

On Mon, Jul 15, 2024 at 5:17 AM Ales Musil  wrote:

> The test tries to setup 4097 LSs via single ovn-nbctl command,
> this is fine however it might lead to "Unreasonably long Xms poll
> interval" on slower systems thus failing the test. Remove the check
> as the test still checks if all switches were created right under
> so it should be completely fine and this should avoid faliures
> on slower systems.
>
> Fixes: 7e99500e60bf ("northd: Add support for disabling vxlan mode.")
> Signed-off-by: Ales Musil 
> ---
>  tests/ovn-northd.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index a389d1988..49a145e7d 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2887,7 +2887,7 @@ for i in {1..4097..1}; do
>  cmd="${cmd} -- ls-add lsw-${i}"
>  done
>
> -check $cmd
> +eval $cmd
>
>  check_row_count nb:Logical_Switch 4097
>  wait_row_count sb:Datapath_Binding 4095
> --
> 2.45.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpctl: Netdev leak causes stale VHU & reaching limit.

[Simon Horman]

On Thu, Jul 11, 2024 at 08:20:28PM +, Vipul Ashri via dev wrote:
> 
> While running a test with a continous VM creation/deletion using an
> orchestration script with-in cloud environment. In parallel we have
> some monitoring script calling ovs-appctl dpctl/show stats commands
> every minute.
> 
> During VHU port delete, one of netdev references were not reduced to
> 0 as show_dpif call has not given-up the reference back or doing bad
> cleanup. This pending deference preventing VHU deletion sequence, this
> is found to be one of corner case inside dpctl code which results in
> leaking up netdev which ultimately results in stale VHU entry. After
> fixing this problematic cleanup, issue is not seen.
> 
> Fixes: fceef2095222 ("dpctl: add ovs-appctl dpctl/* commands to talk to 
> dpif-netdev")
> Signed-off-by: Vipul Ashri 

Acked-by: Simon Horman 

> ---
>  lib/dpctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index a70df5342..81e9ca0e9 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -739,7 +739,6 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
>  }
>  error = netdev_get_stats(netdev, );

FWIIW, i think netdev_close() could go here.

>  if (!error) {
> -netdev_close(netdev);
>  print_stat(dpctl_p, "RX packets:", s.rx_packets);
>  print_stat(dpctl_p, " errors:", s.rx_errors);
>  print_stat(dpctl_p, " dropped:", s.rx_dropped);
> @@ -770,6 +769,7 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
>  dpctl_print(dpctl_p, ", could not retrieve stats (%s)",
>  ovs_strerror(error));
>  }
> +netdev_close(netdev);
>  }
>  dpif_port_destroy(_port);
>  }

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

Re: [ovs-dev] Intel CI not running?

[Phelan, Michael]

> -Original Message-
> From: Ilya Maximets 
> Sent: Friday, July 12, 2024 2:47 PM
> To: Phelan, Michael 
> Cc: i.maxim...@ovn.org; ovs-dev ; Aaron Conole
> ; Chaudron, Eelco 
> Subject: Re: Intel CI not running?
> 
> On 7/10/24 18:14, Ilya Maximets wrote:
> > On 7/10/24 17:41, Phelan, Michael wrote:
> >>
> >>> -Original Message-
> >>> From: Ilya Maximets 
> >>> Sent: Monday, July 8, 2024 4:41 PM
> >>> To: Phelan, Michael 
> >>> Cc: i.maxim...@ovn.org; ovs-dev ; Aaron
> >>> Conole ; Chaudron, Eelco
> 
> >>> Subject: Re: Intel CI not running?
> >>>
> >>> On 7/8/24 11:02, Phelan, Michael wrote:
> > -Original Message-
> > From: Ilya Maximets 
> > Sent: Thursday, July 4, 2024 9:14 PM
> > To: Phelan, Michael 
> > Cc: i.maxim...@ovn.org; ovs-dev ; Aaron
> > Conole ; Chaudron, Eelco
> 
> > Subject: Re: Intel CI not running?
> >
> > On 7/4/24 20:46, Ilya Maximets wrote:
> >> On 7/4/24 13:04, Phelan, Michael wrote:
> >>> Hi Ilya,
> >>> The CI got stuck running make check on a patch, I have solved
> >>> the issue now so reports should be back to normal now.
> >>
> >> Thanks for checking!  Though I see only two reports were sent out
> >> in the past 7 hours with a few hour interval between them.
> >> Did it get stuck again?
> >
> > Got another report.  Looks like we're getting reports at rate of
> > one per 3.5 hours.  That doesn't sound right.
> 
>  We have added make check to the job parameters so that has
>  increased the
> >>> duration of the testing.
> >>>
> >>> 'make check' in GitHub CI takes about 7 minutes.  And machines there
> >>> are not very fast.  It shouldn't take hours.
> >>>
> >>> In GitHub actions we're running it with TESTSUITEFLAGS='-j4'
> >>> RECHECK=yes
> >>>
> >>
> >> I've added these flags, so that should speed up the tests.
> 
> If you have more cores, you may also increase the -j4 to -j.
> This may increase the rate of flakiness, but RECHECK should handle those.
> 
> 
>  It seems like the test number 98: barrier module from make check is
>  a bit
> >>> finicky and stalls occasionally also.
> >>>
> >>> Hmm. I've never seen this test getting stuck.  Could you try to
> >>> reproduce this by running './tests/ovstest test-barrier -v'
> >>> manually?  Would be also great to know where exactly it is getting stuck,
> e.g. by running under gdb.
> >>
> >> I haven't been able to reproduce the issue manually with this command but
> it is consistently getting stuck when running on CI which is strange.
> >>
> >> Any other suggestions on how to debug this?
> >
> > You could try to attach gdb to a running process that is stuck to see
> > where it is waiting and what it is waiting for.
The test does not get stuck every time, I took the CI offline to take a look 
but its back up now. If the test gets stuck again I will try to debug.

> 
> In addition we're also receiving a reports with all tests skipped for some
> reason:
>   https://mail.openvswitch.org/pipermail/ovs-build/2024-July/040128.html

I think this was from some resources not being cleaned up after a unit test, 
this issue should be resolved now.

Thanks,
Michael.
> 
> >
> >>
> >> Kind regards,
> >> Michael.
> >>
> >>>
> >>> Best regards, Ilya Maximets.
> >>>
> 
> >
> >>
> >> Best regards, Ilya Maximets.
> >>
> >>>
> >>> Thanks,
> >>> Michael.
>  -Original Message-
>  From: Ilya Maximets 
>  Sent: Thursday, July 4, 2024 10:47 AM
>  To: Phelan, Michael 
>  Cc: i.maxim...@ovn.org; ovs-dev ;
>  Aaron Conole ; Chaudron, Eelco
> > 
>  Subject: Intel CI not running?
> 
>  Hi, Michael!  We seem to not get reports from Intel CI since
>  some time on Monday.  The last report was:
> 
> 
>  https://mail.openvswitch.org/pipermail/ovs-build/2024-
> July/039755.
>  ht
>  ml
> 
>  Could you, please, check?
> 
>  Best regards, Ilya Maximets.
> >>
> 
> >>
> >

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

Re: [ovs-dev] [PATCH 0/2] Patches to branch for 3.4.

[Jakob Meng]

On 15.07.24 13:23, Ilya Maximets wrote:
> The plan is to branch by the end of today.  There is still a couple of
> patch sets on the list that can / will be applied before that.
>
> Ilya Maximets (2):
>   Prepare for 3.4.0.
>   Prepare for post-3.4.0 (3.4.90).
>
>  Documentation/faq/releases.rst |  1 +
>  NEWS   |  6 +-
>  configure.ac   |  2 +-
>  debian/changelog   | 10 --
>  debian/rules   |  4 ++--
>  5 files changed, 17 insertions(+), 6 deletions(-)
>

Except for the Conntrack Helper Persistence version, the changes match the 
version bump to 3.2 back in the day. Ack on the series…

Acked-by: Jakob Meng 

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

Re: [ovs-dev] [PATCH 2/2] Prepare for post-3.4.0 (3.4.90).

[Mike Pattrick]

On Mon, Jul 15, 2024 at 7:24 AM Ilya Maximets  wrote:
>
> Signed-off-by: Ilya Maximets 

Acked-by: Mike Pattrick 

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

Re: [ovs-dev] [PATCH 1/2] Prepare for 3.4.0.

[Mike Pattrick]

On Mon, Jul 15, 2024 at 7:24 AM Ilya Maximets  wrote:
>
> Signed-off-by: Ilya Maximets 

Acked-by: Mike Pattrick 

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

Re: [ovs-dev] [PATCH ovn] northd: Fix logical router load-balancer nat rules when using DGP.

[Roberto Bartzen Acosta via dev]

Em qui., 30 de mai. de 2024 às 11:57, Roberto Bartzen Acosta <
roberto.aco...@luizalabs.com> escreveu:

>
>
> Em qui., 2 de mai. de 2024 às 10:11, Numan Siddique 
> escreveu:
>
>> On Fri, Mar 22, 2024 at 7:51 AM Roberto Bartzen Acosta
>>  wrote:
>> >
>> > Hi Mark,
>> >
>> > Thanks for your feedback.
>> >
>> > Em seg., 18 de mar. de 2024 às 11:53, Mark Michelson <
>> mmich...@redhat.com>
>> > escreveu:
>> >
>> > > Hi Roberto,
>> > >
>> > > I have some concerns about this patch. Let's use the test case you
>> added
>> > > as an example network. Let's bind the vms and DGPs to hypervisors:
>> > >
>> > > * vm1 and lr1-ts1 are bound to hypervisor hv1
>> > > * vm2 and lr1-ts2 are bound to hypervisor hv2
>> > >
>> > > Now imagine a packet arrives on lr1-ts1. The packet gets load balanced
>> > > and sent to vm2. In this case, since lr1-ts1 is on hv1, the
>> ct_lb_mark()
>> > > action runs there, creating a conntrack entry on hv1. However, the
>> > > packet eventually is tunneled to hv2 so that it can be output to vm2.
>> > >
>> > > Now vm2 replies to the packet. Is there anything that ensures that the
>> > > reply from vm2 gets sent to hv1 for the egress pipeline of lr1? If so,
>> > > then this should work fine since the packet will be unDNATted as
>> > > expected on hv1. However, if the packet runs all of lr1's pipelines on
>> > > hv2, then there is no conntrack entry present, and the attempt to
>> unDNAT
>> > > will not succeed. The packet will either be dropped because of invalid
>> > > CT, or the packet will have an incorrect source IP and port. Either
>> way,
>> > > this isn't what is desired.
>>
>
Hi @Mark Michelson  , I solved the problem that you
have pointed out about CT with multiple chassis hosting the DGP in v2 of
this path. Can you please take a look and provide some feedback?

Thanks,
Roberto



> > >
>> >
>> > yep, you're right! that makes sense.
>> > If the packet comes back with a chassis that does not have the related
>> LB
>> > contrack entry corresponding to the initial request, this will trigger a
>> > miss in the pipeline.
>> >
>> > I tried to disable ct tracking for lb by configuring the parameter on
>> the
>> > router, but I still wasn't successful. E.g.:
>> > options:lb_force_snat_ip="172.16.200.201"
>> >
>> > Even changing the lb_force snat ip on the router, or creating a SNAT
>> > "stateless" rule, I still see this action in the output pipeline in the
>> > highest priority table (table=1).
>> >
>> >   table=1 (lr_out_undnat  ), priority=120  , match=(ip4 &&
>> ((ip4.src ==
>> > 10.195.185.2 && tcp.src == 8000) || (ip4.src == 10.195.185.3 && tcp.src
>> ==
>> > 8000) || (ip4.src == 10.195.185.4 && tcp.src == 8000) || (ip4.src ==
>> > 10.195.185.5 && tcp.src == 8000) || (ip4.src == 10.195.185.6 && tcp.src
>> ==
>> > 8000)) && outport == "incus-net40-lr-lrp-01" &&
>> > is_chassis_resident("cr-incus-net40-lr-lrp-01")),
>> action=(ct_dnat_in_czone;)
>> >   table=1 (lr_out_undnat  ), priority=120  , match=(ip4 &&
>> ((ip4.src ==
>> > 10.195.185.2 && tcp.src == 8000) || (ip4.src == 10.195.185.3 && tcp.src
>> ==
>> > 8000) || (ip4.src == 10.195.185.4 && tcp.src == 8000) || (ip4.src ==
>> > 10.195.185.5 && tcp.src == 8000) || (ip4.src == 10.195.185.6 && tcp.src
>> ==
>> > 8000)) && outport == "incus-net40-lr-lrp-02" &&
>> > is_chassis_resident("cr-incus-net40-lr-lrp-02")),
>> action=(ct_dnat_in_czone;)
>> >   table=1 (lr_out_undnat  ), priority=120  , match=(ip4 &&
>> ((ip4.src ==
>> > 10.195.185.2 && tcp.src == 8000) || (ip4.src == 10.195.185.3 && tcp.src
>> ==
>> > 8000) || (ip4.src == 10.195.185.4 && tcp.src == 8000) || (ip4.src ==
>> > 10.195.185.5 && tcp.src == 8000) || (ip4.src == 10.195.185.6 && tcp.src
>> ==
>> > 8000)) && outport == "incus-net40-lr-lrp-03" &&
>> > is_chassis_resident("cr-incus-net40-lr-lrp-03")),
>> action=(ct_dnat_in_czone;)
>> >   table=1 (lr_out_undnat  ), priority=120  , match=(ip4 &&
>> ((ip4.src ==
>> > 10.195.185.2 && tcp.src == 8000) || (ip4.src == 10.195.185.3 && tcp.src
>> ==
>> > 8000) || (ip4.src == 10.195.185.4 && tcp.src == 8000) || (ip4.src ==
>> > 10.195.185.5 && tcp.src == 8000) || (ip4.src == 10.195.185.6 && tcp.src
>> ==
>> > 8000)) && outport == "incus-net40-lr-lrp-04" &&
>> > is_chassis_resident("cr-incus-net40-lr-lrp-04")),
>> action=(ct_dnat_in_czone;)
>> >   table=1 (lr_out_undnat  ), priority=120  , match=(ip4 &&
>> ((ip4.src ==
>> > 10.195.185.2 && tcp.src == 8000) || (ip4.src == 10.195.185.3 && tcp.src
>> ==
>> > 8000) || (ip4.src == 10.195.185.4 && tcp.src == 8000) || (ip4.src ==
>> > 10.195.185.5 && tcp.src == 8000) || (ip4.src == 10.195.185.6 && tcp.src
>> ==
>> > 8000)) && outport == "incus-net40-lr-lrp-ext" &&
>> > is_chassis_resident("cr-incus-net40-lr-lrp-ext")),
>> > action=(ct_dnat_in_czone;)
>> >
>> >
>> > Considering that the return when using multiple DGPs is probably
>> associated
>> > with ECMP, any idea on how to change the rules so that the LB output
>> rules
>> > are 'stateless' (not ct dependent) in this 

Re: [ovs-dev] [PATCH] selftests/net: fix gro.c compilation failure due to non-existent opt_ipproto_off

[Greg Kroah-Hartman]

On Fri, Jul 12, 2024 at 05:01:38PM -0700, John Hubbard wrote:
> On 7/12/24 4:51 PM, John Hubbard wrote:
> > Linux 6.6 does not have an opt_ipproto_off variable in gro.c at all (it
> > was added in later kernel versions), so attempting to initialize one
> > breaks the build.
> 
> This is the first time I've tried to fix something in linux-stable, and
> I'm not sure that I've made it completely clear. This is only for
> linux-6.6.y in linux-stable.

This worked just fine, now queued up, thanks.

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

[ovs-dev] Patch "[PATCH] selftests/net: fix gro.c compilation failure due to non-existent opt_ipproto_off" has been added to the 6.6-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

[PATCH] selftests/net: fix gro.c compilation failure due to non-existent 
opt_ipproto_off

to the 6.6-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 
selftests-net-fix-gro.c-compilation-failure-due-to-non-existent-opt_ipproto_off.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From jhubb...@nvidia.com  Mon Jul 15 14:10:26 2024
From: John Hubbard 
Date: Fri, 12 Jul 2024 16:51:50 -0700
Subject: [PATCH] selftests/net: fix gro.c compilation failure due to 
non-existent opt_ipproto_off
To: Greg Kroah-Hartman 
Cc: "Shuah Khan" , "David S . Miller" , 
"Eric Dumazet" , "Jakub Kicinski" , 
"Paolo Abeni" , "Steffen Klassert" 
, "Herbert Xu" , 
"Andreas Färber" , "Manivannan Sadhasivam" 
, "Matthieu Baerts" , 
"Mat Martineau" , "Geliang Tang" , 
"Pravin B Shelar" , "Willem de Bruijn" 
, "Alexander Mikhalitsyn" 
, zhujun2 , "Petr 
Machata" , "Ido Schimmel" , "Hangbin Liu" 
, "Nikolay Aleksandrov" , "Benjamin 
Poirier" , "Sebastian Andrzej Siewior" 
, "Dmitry Safonov" <0x7f454...@gmail.com>, 
net...@vger.kernel.org, linux
 -arm-ker...@lists.infradead.org, linux-acti...@lists.infradead.org, 
mp...@lists.linux.dev, d...@openvswitch.org, linux-kselft...@vger.kernel.org, 
LKML , l...@lists.linux.dev, "John Hubbard" 
, sta...@vger.kernel.org, "Ignat Korchagin" 

Message-ID: <20240712235150.99175-1-jhubb...@nvidia.com>

From: John Hubbard 

Linux 6.6 does not have an opt_ipproto_off variable in gro.c at all (it
was added in later kernel versions), so attempting to initialize one
breaks the build.

Fixes: c80d53c484e8 ("selftests/net: fix uninitialized variables")
Cc:  # 6.6
Reported-by: Ignat Korchagin 
Closes: 
https://lore.kernel.org/all/8b1717db-8c4a-47ee-b28c-170b630c4...@cloudflare.com/#t
Signed-off-by: John Hubbard 
Signed-off-by: Greg Kroah-Hartman 
---
 tools/testing/selftests/net/gro.c |3 ---
 1 file changed, 3 deletions(-)

--- a/tools/testing/selftests/net/gro.c
+++ b/tools/testing/selftests/net/gro.c
@@ -113,9 +113,6 @@ static void setup_sock_filter(int fd)
next_off = offsetof(struct ipv6hdr, nexthdr);
ipproto_off = ETH_HLEN + next_off;
 
-   /* Overridden later if exthdrs are used: */
-   opt_ipproto_off = ipproto_off;
-
if (strcmp(testname, "ip") == 0) {
if (proto == PF_INET)
optlen = sizeof(struct ip_timestamp);


Patches currently in stable-queue which might be from jhubb...@nvidia.com are

queue-6.6/selftests-net-fix-gro.c-compilation-failure-due-to-non-existent-opt_ipproto_off.patch
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] Prepare for post-3.4.0 (3.4.90).

[Ilya Maximets]

Signed-off-by: Ilya Maximets 
---
 NEWS | 4 
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 507d91721..4ca9ae692 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Post-v3.4.0
+
+
+
 v3.4.0 - xx xxx 
 
- Option '--mlockall' now only locks memory pages on fault, if possible.
diff --git a/configure.ac b/configure.ac
index 5b3857773..d867c6c28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 3.4.0, b...@openvswitch.org)
+AC_INIT(openvswitch, 3.4.90, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([vswitchd/ovs-vswitchd.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index 929b8ecf4..3bc24aa70 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (3.4.90-1) unstable; urgency=low
+
+   * New upstream version
+
+ -- Open vSwitch team   Mon, 15 Jul 2024 13:00:01 +0100
+
 openvswitch (3.4.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.45.2

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

[ovs-dev] [PATCH 0/2] Patches to branch for 3.4.

[Ilya Maximets]

The plan is to branch by the end of today.  There is still a couple of
patch sets on the list that can / will be applied before that.

Ilya Maximets (2):
  Prepare for 3.4.0.
  Prepare for post-3.4.0 (3.4.90).

 Documentation/faq/releases.rst |  1 +
 NEWS   |  6 +-
 configure.ac   |  2 +-
 debian/changelog   | 10 --
 debian/rules   |  4 ++--
 5 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.45.2

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

[ovs-dev] [PATCH 1/2] Prepare for 3.4.0.

[Ilya Maximets]

Signed-off-by: Ilya Maximets 
---
 Documentation/faq/releases.rst | 1 +
 NEWS   | 2 +-
 configure.ac   | 2 +-
 debian/changelog   | 4 ++--
 debian/rules   | 4 ++--
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 70219d717..9fbee90ed 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -221,6 +221,7 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 3.1.x22.11.5
 3.2.x22.11.5
 3.3.x23.11.1
+3.4.x23.11.1
  
 
 Q: Are all the DPDK releases that OVS versions work with maintained?
diff --git a/NEWS b/NEWS
index 10e08fbac..507d91721 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-Post-v3.3.0
+v3.4.0 - xx xxx 
 
- Option '--mlockall' now only locks memory pages on fault, if possible.
  This also makes it compatible with vHost Post-copy Live Migration.
diff --git a/configure.ac b/configure.ac
index dd6553fea..5b3857773 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 3.3.90, b...@openvswitch.org)
+AC_INIT(openvswitch, 3.4.0, b...@openvswitch.org)
 AC_CONFIG_SRCDIR([vswitchd/ovs-vswitchd.c])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/debian/changelog b/debian/changelog
index 614c46ef9..929b8ecf4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-openvswitch (3.3.90-1) unstable; urgency=low
+openvswitch (3.4.0-1) unstable; urgency=low
 
* New upstream version
 
- -- Open vSwitch team   Wed, 17 Jan 2024 13:00:01 +0100
+ -- Open vSwitch team   Mon, 15 Jul 2024 13:00:00 +0100
 
 openvswitch (3.3.0-1) unstable; urgency=low
 
diff --git a/debian/rules b/debian/rules
index 075b04162..b6f905f3c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -134,8 +134,8 @@ override_dh_python3:
 # Helper target for creating snapshots from upstream git
 DATE=$(shell date +%Y%m%d)
 # Upstream branch to track
-BRANCH=branch-3.3
-VERSION=3.3.0
+BRANCH=branch-3.4
+VERSION=3.4.0
 
 get-orig-snapshot:
rm -Rf openvswitch-upstream
-- 
2.45.2

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