[ovs-dev] [PATCH v4] ofproto-dpif-xlate: Don't reinstall removed XC_LEARN rule

[Mike Pattrick]

When the a revalidator thread is updating statistics for an XC_LEARN
xcache entry in xlate_push_stats_entry it uses ofproto_flow_mod_learn.
The revalidator will update stats for rules even if they are in a
removed state or marked as invisible. However, ofproto_flow_mod_learn
will detect if a flow has been removed and re-add it in that case. This
can result in an old learn action replacing the new learn action that
had replaced it in the first place.

This change adds a new force parameter to ofproto_flow_mod_learn
allowing the caller to specify an action to take if temp_rule is
removed. If force is set to false and the rule has been replaced in the
classifier with a more recent rule, then ofproto_flow_mod_learn will
just return.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2213892
Signed-off-by: Mike Pattrick 
---
 v2: Added additional checks if rule is removed
 v3: v2 patch was corrupted in transit
 v4: Added check against dpif flow stats
---
 ofproto/ofproto-dpif-xlate-cache.c |  2 +-
 ofproto/ofproto-dpif-xlate.c   |  2 +-
 ofproto/ofproto-dpif.c |  2 +-
 ofproto/ofproto-provider.h |  6 ++--
 ofproto/ofproto.c  | 51 +-
 5 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate-cache.c 
b/ofproto/ofproto-dpif-xlate-cache.c
index 9224ee2e6..2e1fcb3a6 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -125,7 +125,7 @@ xlate_push_stats_entry(struct xc_entry *entry,
 case XC_LEARN: {
 enum ofperr error;
 error = ofproto_flow_mod_learn(entry->learn.ofm, true,
-   entry->learn.limit, NULL);
+   entry->learn.limit, NULL, stats->used);
 if (error) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 VLOG_WARN_RL(, "xcache LEARN action execution failed.");
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 47ea0f47e..3a68cef4a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5701,7 +5701,7 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct 
ofpact_learn *learn)
 bool success = true;
 if (ctx->xin->allow_side_effects) {
 error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL,
-   learn->limit, );
+   learn->limit, , 0);
 } else if (learn->limit) {
 if (!ofm->temp_rule
 || ofm->temp_rule->state != RULE_INSERTED) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e22ca757a..1efd9fc91 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4880,7 +4880,7 @@ packet_xlate(struct ofproto *ofproto_, struct 
ofproto_packet_out *opo)
 if (entry->type == XC_LEARN) {
 struct ofproto_flow_mod *ofm = entry->learn.ofm;

-error = ofproto_flow_mod_learn_refresh(ofm);
+error = ofproto_flow_mod_learn_refresh(ofm, 0);
 if (error) {
 goto error_out;
 }
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 143ded690..dc5c1a60d 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -2027,9 +2027,11 @@ enum ofperr ofproto_flow_mod_init_for_learn(struct 
ofproto *,
 struct ofproto_flow_mod *)
 OVS_EXCLUDED(ofproto_mutex);
 enum ofperr ofproto_flow_mod_learn(struct ofproto_flow_mod *, bool keep_ref,
-   unsigned limit, bool *below_limit)
+   unsigned limit, bool *below_limit,
+   long long int stats_used)
 OVS_EXCLUDED(ofproto_mutex);
-enum ofperr ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm);
+enum ofperr ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm,
+   long long int stats_used);
 enum ofperr ofproto_flow_mod_learn_start(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dbf4958bc..a86d6a58e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5472,7 +5472,8 @@ ofproto_flow_mod_init_for_learn(struct ofproto *ofproto,
 }

 enum ofperr
-ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm)
+ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod *ofm,
+   long long int stats_used)
 {
 enum ofperr error = 0;

@@ -5494,8 +5495,37 @@ ofproto_flow_mod_learn_refresh(struct ofproto_flow_mod 
*ofm)
 if (rule->state == RULE_REMOVED) {
 struct cls_rule cr;

-cls_rule_clone(, >cr);
 ovs_mutex_lock(>mutex);
+

[ovs-dev] OVN 23.09 soft freeze is tomorrow

[Mark Michelson]

Hi everyone,

As I announced a few weeks ago, the soft freeze date for OVN 23.09 is 
tomorrow, 18 August, 2023. This is the final date that patches for new 
features may be submitted for inclusion in OVN 23.09.


The OVN team will spend the next two weeks attempting to get those 
patches merged. Then on 1 September, we will create the branch for OVN 
23.09. At that point, no new features will be merged into the branch 
[1]. Then on 15 September, we will release 23.09.0.


Thanks,
Mark Michelson


[1] Unless there is an exception for a really good reason.

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

Re: [ovs-dev] [PATCH ovn 0/5] Add port group incremental processing in northd.

[Mark Michelson]

Hi Dumitru,

I had a look through the series, and it looks good to me.

Acked-by: Mark Michelson 

On 8/10/23 08:44, Dumitru Ceara wrote:

This series cleans up, factors out and then adds incremental processing
support to the code that processes port group updates in ovn-northd.

Some performance results are shared in the commit message of the last
patch in this series.

Dumitru Ceara (5):
   ovn-util: Factor out struct sorted_addresses into sorted_array.
   northd: Merge port group related structures.
   northd: Move port group processing to its separate module.
   northd: Move port group processing to its own I-P node.
   northd: Add incremental processing for NB port groups.


  lib/stopwatch-names.h|   1 +
  northd/automake.mk   |   2 +
  northd/en-lflow.c|  21 +-
  northd/en-lflow.h|   1 +
  northd/en-northd.c   |   4 -
  northd/en-port-group.c   | 636 +++
  northd/en-port-group.h   | 114 +++
  northd/inc-proc-northd.c |  22 +-
  northd/northd.c  | 317 +--
  northd/northd.h  |  12 +-
  northd/ovn-northd.c  |   4 +
  tests/ovn-northd.at  | 254 +++-
  12 files changed, 1118 insertions(+), 270 deletions(-)
  create mode 100644 northd/en-port-group.c
  create mode 100644 northd/en-port-group.h



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

Re: [ovs-dev] [PATCH ovn v3] northd: Fix incorrect warning logs when handling port binding changes.

[Numan Siddique]

On Thu, Aug 17, 2023 at 9:42 PM Han Zhou  wrote:
>
> On Thu, Aug 17, 2023 at 12:05 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > When changes to port bindings corresponding to router ports are
> > handled by northd engine node, incorrect warning logs (like below)
> > are logged.
> >
> > 
> > northd|WARN|A port-binding for lrp0 is created but the LSP is not found.
> > 
> >
> > Fix these warnings.
> >
> > Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding
> in "northd" node.")
> >
> > CC: Han Zhou 
> > Signed-off-by: Numan Siddique 
> > ---
> >
> > v2 -> v3
> > --
> >   * Checked if type of the 'pb' and return false if its a router port
> > before doing a lookup in the ls_ports map.
> >
> > v1 -> v2
> > ---
> >   * v1 was not returning false in northd_handle_sb_port_binding_changes()
> > for router port - port bindings because of which IPv6 PD system test
> > was failing.  Fixed it in v2.
> >
> >
> >  controller/binding.c |  75 
> >  controller/binding.h |  20 +
> >  lib/ovn-util.c   | 101 +++
> >  lib/ovn-util.h   |  21 +
> >  northd/northd.c  |   7 +++
> >  5 files changed, 130 insertions(+), 94 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 3ac0c35df..a521f2828 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct
> binding_lport *);
> >  static bool binding_lport_update_port_sec(
> >  struct binding_lport *, const struct sbrec_port_binding *);
> >
> > -static char *get_lport_type_str(enum en_lport_type lport_type);
> >  static bool ovs_iface_matches_lport_iface_id_ver(
> >  const struct ovsrec_interface *,
> >  const struct sbrec_port_binding *);
> > @@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct
> local_binding_data *lbinding_data,
> >  free(nodes);
> >  }
> >
> > -static bool
> > -is_lport_vif(const struct sbrec_port_binding *pb)
> > -{
> > -return !pb->type[0];
> > -}
> > -
> > -enum en_lport_type
> > -get_lport_type(const struct sbrec_port_binding *pb)
> > -{
> > -if (is_lport_vif(pb)) {
> > -if (pb->parent_port && pb->parent_port[0]) {
> > -return LP_CONTAINER;
> > -}
> > -return LP_VIF;
> > -} else if (!strcmp(pb->type, "patch")) {
> > -return LP_PATCH;
> > -} else if (!strcmp(pb->type, "chassisredirect")) {
> > -return LP_CHASSISREDIRECT;
> > -} else if (!strcmp(pb->type, "l3gateway")) {
> > -return LP_L3GATEWAY;
> > -} else if (!strcmp(pb->type, "localnet")) {
> > -return LP_LOCALNET;
> > -} else if (!strcmp(pb->type, "localport")) {
> > -return LP_LOCALPORT;
> > -} else if (!strcmp(pb->type, "l2gateway")) {
> > -return LP_L2GATEWAY;
> > -} else if (!strcmp(pb->type, "virtual")) {
> > -return LP_VIRTUAL;
> > -} else if (!strcmp(pb->type, "external")) {
> > -return LP_EXTERNAL;
> > -} else if (!strcmp(pb->type, "remote")) {
> > -return LP_REMOTE;
> > -} else if (!strcmp(pb->type, "vtep")) {
> > -return LP_VTEP;
> > -}
> > -
> > -return LP_UNKNOWN;
> > -}
> > -
> > -static char *
> > -get_lport_type_str(enum en_lport_type lport_type)
> > -{
> > -switch (lport_type) {
> > -case LP_VIF:
> > -return "VIF";
> > -case LP_CONTAINER:
> > -return "CONTAINER";
> > -case LP_VIRTUAL:
> > -return "VIRTUAL";
> > -case LP_PATCH:
> > -return "PATCH";
> > -case LP_CHASSISREDIRECT:
> > -return "CHASSISREDIRECT";
> > -case LP_L3GATEWAY:
> > -return "L3GATEWAY";
> > -case LP_LOCALNET:
> > -return "PATCH";
> > -case LP_LOCALPORT:
> > -return "LOCALPORT";
> > -case LP_L2GATEWAY:
> > -return "L2GATEWAY";
> > -case LP_EXTERNAL:
> > -return "EXTERNAL";
> > -case LP_REMOTE:
> > -return "REMOTE";
> > -case LP_VTEP:
> > -return "VTEP";
> > -case LP_UNKNOWN:
> > -return "UNKNOWN";
> > -}
> > -
> > -OVS_NOT_REACHED();
> > -}
> > -
> >  void
> >  set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
> >  const struct sbrec_chassis *chassis_rec,
> > diff --git a/controller/binding.h b/controller/binding.h
> > index 235e5860d..24bc84079 100644
> > --- a/controller/binding.h
> > +++ b/controller/binding.h
> > @@ -22,6 +22,7 @@
> >  #include "openvswitch/hmap.h"
> >  #include "openvswitch/uuid.h"
> >  #include "openvswitch/list.h"
> > +# include "lib/ovn-util.h"
> >  #include "sset.h"
> >  #include "lport.h"
> >
> > @@ -217,25 +218,6 @@ void port_binding_set_down(const struct
> sbrec_chassis *chassis_rec,
> > const char *iface_id,
> > const struct uuid *pb_uuid);
> >
> > -/* Corresponds to each 

[ovs-dev] [PATCH v2] bond: Always revalidate unbalanced bonds when active member changes

[Mike Pattrick]

Currently a bond will not always revalidate when an active member
changes. This can result in counter-intuitive behaviors like the fact
that using ovs-appctl bond/set-active-member will cause the bond to
revalidate but changing other_config:bond-primary will not trigger a
revalidate in the bond.

When revalidation is not set but the active member changes in an
unbalanced bond, OVS may send traffic out of previously active member
instead of the new active member.

This change will always mark unbalanced bonds for revalidation if the
active member changes.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2214979
Signed-off-by: Mike Pattrick 
---
v2: Added a test
---
 ofproto/bond.c  |  8 +--
 tests/system-traffic.at | 50 +
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index cfdf44f85..fb108d30a 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -193,6 +193,7 @@ static void bond_update_post_recirc_rules__(struct bond *, 
bool force)
 static bool bond_is_falling_back_to_ab(const struct bond *);
 static void bond_add_lb_output_buckets(const struct bond *);
 static void bond_del_lb_output_buckets(const struct bond *);
+static bool bond_is_balanced(const struct bond *bond) OVS_REQ_RDLOCK(rwlock);
 
 
 /* Attempts to parse 's' as the name of a bond balancing mode.  If successful,
@@ -552,11 +553,15 @@ bond_find_member_by_mac(const struct bond *bond, const 
struct eth_addr mac)
 
 static void
 bond_active_member_changed(struct bond *bond)
+OVS_REQ_WRLOCK(rwlock)
 {
 if (bond->active_member) {
 struct eth_addr mac;
 netdev_get_etheraddr(bond->active_member->netdev, );
 bond->active_member_mac = mac;
+if (!bond_is_balanced(bond)) {
+bond->bond_revalidate = true;
+}
 } else {
 bond->active_member_mac = eth_addr_zero;
 }
@@ -1124,7 +1129,7 @@ bond_get_recirc_id_and_hash_basis(struct bond *bond, 
uint32_t *recirc_id,
 /* Rebalancing. */
 
 static bool
-bond_is_balanced(const struct bond *bond) OVS_REQ_RDLOCK(rwlock)
+bond_is_balanced(const struct bond *bond)
 {
 return bond->rebalance_interval
 && (bond->balance == BM_SLB || bond->balance == BM_TCP)
@@ -1728,7 +1733,6 @@ bond_unixctl_set_active_member(struct unixctl_conn *conn,
 }
 
 if (bond->active_member != member) {
-bond->bond_revalidate = true;
 bond->active_member = member;
 VLOG_INFO("bond %s: active member is now %s",
   bond->name, member->name);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 945037ec0..a51725012 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -291,6 +291,56 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 
10.1.1.2 | FORMAT_PING
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over active-backup bond])
+OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])])
+
+AT_CHECK([ovs-ofctl add-flow br0 "priority=1 actions=drop"])
+AT_CHECK([ovs-ofctl add-flow br1 "priority=1 actions=drop"])
+AT_CHECK([ovs-ofctl add-flow br0 "ip,icmp actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br1 "ip,icmp actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br0 "arp actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br1 "arp actions=normal"])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24")
+on_exit 'ip link del link0a'
+on_exit 'ip link del link0b'
+AT_CHECK([ip link add link0a type veth peer name link1a])
+AT_CHECK([ip link add link0b type veth peer name link1b])
+
+AT_CHECK([ip link set dev link0a up])
+AT_CHECK([ip link set dev link1a up])
+AT_CHECK([ip link set dev link0b up])
+AT_CHECK([ip link set dev link1b up])
+
+AT_CHECK([ovs-vsctl add-bond br0 bond0 link0a link0b bond_mode=active-backup])
+AT_CHECK([ovs-vsctl add-bond br1 bond1 link1a link1b bond_mode=active-backup])
+
+dnl Set primary
+AT_CHECK([ovs-vsctl set port bond0 other_config:bond-primary=link0a -- \
+set port bond1 other_config:bond-primary=link1a])
+
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.1.2])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Change primary
+AT_CHECK([ovs-vsctl set port bond0 other_config:bond-primary=link0b -- \
+set port bond1 other_config:bond-primary=link1b])
+
+sleep 1
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 6 -i 0.5 -w 6 10.1.1.2 | FORMAT_PING], 
[0], [dnl
+6 packets transmitted, 6 received, 0% packet loss, time 0ms
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - ping over vxlan tunnel])
 OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_VXLAN()
-- 
2.39.3

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

Re: [ovs-dev] [PATCH] bond: Always revalidate unbalanced bonds when active member changes

[Mike Pattrick]

On Wed, Aug 16, 2023 at 4:18 PM Ilya Maximets  wrote:
>
> On 8/9/23 19:00, Mike Pattrick wrote:
> > Currently a bond will not always revalidate when an active member
> > changes. This can result in counter-intuitive behaviors like the fact
> > that using ovs-appctl bond/set-active-member will cause the bond to
> > revalidate but changing other_config:bond-primary will not trigger a
> > revalidate in the bond.
>
> Uhm, is that true?  If primary changes in the database,
> bond_reconfigure() should return 'true' and the bundle_set
> function should set need_revalidate = REV_RECONFIGURE.
> Is that not happening?
>
> Can we have a test for this case?

I'll resubmit with a test.

-M

>
> Best regards, Ilya Maximets.
>
> >
> > When revalidation is not set but the active member changes in an
> > unbalanced bond, OVS may send traffic out of previously active member
> > instead of the new active member.
> >
> > This change will always mark unbalanced bonds for revalidation if the
> > active member changes.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2214979
> > Signed-off-by: Mike Pattrick 
> > ---
> >  ofproto/bond.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/ofproto/bond.c b/ofproto/bond.c
> > index cfdf44f85..fb108d30a 100644
> > --- a/ofproto/bond.c
> > +++ b/ofproto/bond.c
> > @@ -193,6 +193,7 @@ static void bond_update_post_recirc_rules__(struct bond 
> > *, bool force)
> >  static bool bond_is_falling_back_to_ab(const struct bond *);
> >  static void bond_add_lb_output_buckets(const struct bond *);
> >  static void bond_del_lb_output_buckets(const struct bond *);
> > +static bool bond_is_balanced(const struct bond *bond) 
> > OVS_REQ_RDLOCK(rwlock);
> >
> >
> >  /* Attempts to parse 's' as the name of a bond balancing mode.  If 
> > successful,
> > @@ -552,11 +553,15 @@ bond_find_member_by_mac(const struct bond *bond, 
> > const struct eth_addr mac)
> >
> >  static void
> >  bond_active_member_changed(struct bond *bond)
> > +OVS_REQ_WRLOCK(rwlock)
> >  {
> >  if (bond->active_member) {
> >  struct eth_addr mac;
> >  netdev_get_etheraddr(bond->active_member->netdev, );
> >  bond->active_member_mac = mac;
> > +if (!bond_is_balanced(bond)) {
> > +bond->bond_revalidate = true;
> > +}
> >  } else {
> >  bond->active_member_mac = eth_addr_zero;
> >  }
> > @@ -1124,7 +1129,7 @@ bond_get_recirc_id_and_hash_basis(struct bond *bond, 
> > uint32_t *recirc_id,
> >  /* Rebalancing. */
> >
> >  static bool
> > -bond_is_balanced(const struct bond *bond) OVS_REQ_RDLOCK(rwlock)
> > +bond_is_balanced(const struct bond *bond)
> >  {
> >  return bond->rebalance_interval
> >  && (bond->balance == BM_SLB || bond->balance == BM_TCP)
> > @@ -1728,7 +1733,6 @@ bond_unixctl_set_active_member(struct unixctl_conn 
> > *conn,
> >  }
> >
> >  if (bond->active_member != member) {
> > -bond->bond_revalidate = true;
> >  bond->active_member = member;
> >  VLOG_INFO("bond %s: active member is now %s",
> >bond->name, member->name);
>

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

Re: [ovs-dev] [PATCH ovn] northd.c: Incremental processing for first/last switch port change.

[Han Zhou]

On Thu, Aug 17, 2023 at 2:13 AM Ales Musil  wrote:
>
>
>
> On Wed, Aug 16, 2023 at 2:56 AM Han Zhou  wrote:
>>
>> Commit 1eb09838 fixed an incremental processing problem by falling back
>> to recompute. The problem was handling VIF changes for a LS that
>> has router ports when:
>> - adding the first switch port to the LS, or
>> - deleting the last switch port from the LS
>> because there are router port related lflows that depends on the
>> condition whether there are only router ports on the LS or not.
>>
>> Since such scenario is common, this patch further improves it to avoid
>> falling back to recompute.
>>
>> Signed-off-by: Han Zhou 
>
>
> Hi Han,
>
> there is one small nit down below.
>>
>> ---
>>  northd/northd.c | 71 +
>>  northd/northd.h |  1 +
>>  tests/ovn-northd.at |  6 ++--
>>  3 files changed, 45 insertions(+), 33 deletions(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 9a12a94ae25d..a87298a026de 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -5120,21 +5120,8 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
>>  op->visited = false;
>>  }
>>
>> -/* Check if the logical switch has only router ports before
this change.
>> - * If so, fall back to recompute.
>> - * lflow engine node while building the lflows checks if the
logical switch
>> - * has any router ports and depending on that it adds different
flows.
>> - * See build_lswitch_rport_arp_req_flow() for more details.
>> - * Note: We can definitely handle this scenario incrementally
in the
>> - * northd engine node and fall back to recompute in lflow
engine node
>> - * and even handle this incrementally in lflow node.  Until we
do that
>> - * resort to full recompute of northd node.
>> - */
>> -bool only_rports = (od->n_router_ports
>> -&& (od->n_router_ports ==
hmap_count(>ports)));
>> -if (only_rports) {
>> -goto fail;
>> -}
>> +ls_change->had_only_router_ports = (od->n_router_ports
>> +&& (od->n_router_ports == hmap_count(>ports)));
>>
>>  /* Compare the individual ports in the old and new Logical
Switches */
>>  for (size_t j = 0; j < changed_ls->n_ports; ++j) {
>> @@ -5217,22 +5204,6 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
>>  }
>>  }
>>
>> -/* Check if the logical switch has only router ports after this
change.
>> - * If so, fall back to recompute.
>> - * lflow engine node while building the lflows checks if the
logical switch
>> - * has any router ports and depending on that it adds different
flows.
>> - * See build_lswitch_rport_arp_req_flow() for more details.
>> - * Note: We can definitely handle this scenario incrementally
in the
>> - * northd engine node and fall back to recompute in lflow
engine node
>> - * and even handle this incrementally in lflow node.  Until we
do that
>> - * resort to full recompute of northd node.
>> - */
>> -only_rports = (od->n_router_ports
>> -   && (od->n_router_ports ==
hmap_count(>ports)));
>> -if (only_rports) {
>> -goto fail_clean_deleted;
>> -}
>> -
>>  if (!ovs_list_is_empty(_change->added_ports) ||
>>  !ovs_list_is_empty(_change->updated_ports) ||
>>  !ovs_list_is_empty(_change->deleted_ports)) {
>> @@ -16550,6 +16521,44 @@ bool lflow_handle_northd_ls_changes(struct
ovsdb_idl_txn *ovnsb_txn,
>>lfrn->lflow);
>>  }
>>  }
>> +
>> +bool ls_has_only_router_ports = (ls_change->od->n_router_ports
&&
>> + (ls_change->od->n_router_ports
==
>> +
 hmap_count(_change->od->ports)));
>> +
>> +if (ls_change->had_only_router_ports !=
ls_has_only_router_ports) {
>> +/* There are lflows related to router ports that depends on
whether
>> + * there are switch ports on the logical switch (see
>> + * build_lswitch_rport_arp_req_flow() for more details).
Since this
>> + * dependency changed, we need to regenerate lflows for
each router
>> + * port on this logical switch. */
>> +for (size_t i = 0; i < ls_change->od->n_router_ports; i++) {
>> +op = ls_change->od->router_ports[i];
>> +
>> +/* Delete old lflows. */
>> +if (!delete_lflow_for_lsp(op, "affected router",
>> +
 lflow_input->sbrec_logical_flow_table,
>> +  lflows)) {
>> +return false;
>> +}
>> +
>> +/* Generate new lflows. */
>> +struct ds match = DS_EMPTY_INITIALIZER;
>> +struct ds actions = 

Re: [ovs-dev] [PATCH ovn v3] northd: Fix incorrect warning logs when handling port binding changes.

[Han Zhou]

On Thu, Aug 17, 2023 at 12:05 AM  wrote:
>
> From: Numan Siddique 
>
> When changes to port bindings corresponding to router ports are
> handled by northd engine node, incorrect warning logs (like below)
> are logged.
>
> 
> northd|WARN|A port-binding for lrp0 is created but the LSP is not found.
> 
>
> Fix these warnings.
>
> Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding
in "northd" node.")
>
> CC: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---
>
> v2 -> v3
> --
>   * Checked if type of the 'pb' and return false if its a router port
> before doing a lookup in the ls_ports map.
>
> v1 -> v2
> ---
>   * v1 was not returning false in northd_handle_sb_port_binding_changes()
> for router port - port bindings because of which IPv6 PD system test
> was failing.  Fixed it in v2.
>
>
>  controller/binding.c |  75 
>  controller/binding.h |  20 +
>  lib/ovn-util.c   | 101 +++
>  lib/ovn-util.h   |  21 +
>  northd/northd.c  |   7 +++
>  5 files changed, 130 insertions(+), 94 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 3ac0c35df..a521f2828 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct
binding_lport *);
>  static bool binding_lport_update_port_sec(
>  struct binding_lport *, const struct sbrec_port_binding *);
>
> -static char *get_lport_type_str(enum en_lport_type lport_type);
>  static bool ovs_iface_matches_lport_iface_id_ver(
>  const struct ovsrec_interface *,
>  const struct sbrec_port_binding *);
> @@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct
local_binding_data *lbinding_data,
>  free(nodes);
>  }
>
> -static bool
> -is_lport_vif(const struct sbrec_port_binding *pb)
> -{
> -return !pb->type[0];
> -}
> -
> -enum en_lport_type
> -get_lport_type(const struct sbrec_port_binding *pb)
> -{
> -if (is_lport_vif(pb)) {
> -if (pb->parent_port && pb->parent_port[0]) {
> -return LP_CONTAINER;
> -}
> -return LP_VIF;
> -} else if (!strcmp(pb->type, "patch")) {
> -return LP_PATCH;
> -} else if (!strcmp(pb->type, "chassisredirect")) {
> -return LP_CHASSISREDIRECT;
> -} else if (!strcmp(pb->type, "l3gateway")) {
> -return LP_L3GATEWAY;
> -} else if (!strcmp(pb->type, "localnet")) {
> -return LP_LOCALNET;
> -} else if (!strcmp(pb->type, "localport")) {
> -return LP_LOCALPORT;
> -} else if (!strcmp(pb->type, "l2gateway")) {
> -return LP_L2GATEWAY;
> -} else if (!strcmp(pb->type, "virtual")) {
> -return LP_VIRTUAL;
> -} else if (!strcmp(pb->type, "external")) {
> -return LP_EXTERNAL;
> -} else if (!strcmp(pb->type, "remote")) {
> -return LP_REMOTE;
> -} else if (!strcmp(pb->type, "vtep")) {
> -return LP_VTEP;
> -}
> -
> -return LP_UNKNOWN;
> -}
> -
> -static char *
> -get_lport_type_str(enum en_lport_type lport_type)
> -{
> -switch (lport_type) {
> -case LP_VIF:
> -return "VIF";
> -case LP_CONTAINER:
> -return "CONTAINER";
> -case LP_VIRTUAL:
> -return "VIRTUAL";
> -case LP_PATCH:
> -return "PATCH";
> -case LP_CHASSISREDIRECT:
> -return "CHASSISREDIRECT";
> -case LP_L3GATEWAY:
> -return "L3GATEWAY";
> -case LP_LOCALNET:
> -return "PATCH";
> -case LP_LOCALPORT:
> -return "LOCALPORT";
> -case LP_L2GATEWAY:
> -return "L2GATEWAY";
> -case LP_EXTERNAL:
> -return "EXTERNAL";
> -case LP_REMOTE:
> -return "REMOTE";
> -case LP_VTEP:
> -return "VTEP";
> -case LP_UNKNOWN:
> -return "UNKNOWN";
> -}
> -
> -OVS_NOT_REACHED();
> -}
> -
>  void
>  set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
>  const struct sbrec_chassis *chassis_rec,
> diff --git a/controller/binding.h b/controller/binding.h
> index 235e5860d..24bc84079 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -22,6 +22,7 @@
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/uuid.h"
>  #include "openvswitch/list.h"
> +# include "lib/ovn-util.h"
>  #include "sset.h"
>  #include "lport.h"
>
> @@ -217,25 +218,6 @@ void port_binding_set_down(const struct
sbrec_chassis *chassis_rec,
> const char *iface_id,
> const struct uuid *pb_uuid);
>
> -/* Corresponds to each Port_Binding.type. */
> -enum en_lport_type {
> -LP_UNKNOWN,
> -LP_VIF,
> -LP_CONTAINER,
> -LP_PATCH,
> -LP_L3GATEWAY,
> -LP_LOCALNET,
> -LP_LOCALPORT,
> -LP_L2GATEWAY,
> -LP_VTEP,
> -LP_CHASSISREDIRECT,
> -LP_VIRTUAL,
> -LP_EXTERNAL,
> -LP_REMOTE
> -};
> -
> -enum en_lport_type get_lport_type(const struct 

Re: [ovs-dev] [PATCH branch-3.2 0/2] Release patches for v3.2.0.

[Ilya Maximets]

On 8/17/23 17:16, Eelco Chaudron wrote:
> 
> 
> On 17 Aug 2023, at 15:25, Ilya Maximets wrote:
> 
>> According to our release process, it's time to release v3.2.0!
>>
>> Ilya Maximets (2):
>>   Set release date for 3.2.0.
>>   Prepare for 3.2.1.
> 
> 
> Ack on the series!
> 
> Acked-by: Eelco Chaudron 
> 

Thanks, Simon and Eelco!

Applied and tagged.  The first patch also applied to master.
Will work on updating the website and sending an announcement.

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

Re: [ovs-dev] [PATCH ovn] northd: Add a separate I-P node for handling meters.

[Dumitru Ceara]

On 8/17/23 12:43, Ales Musil wrote:
> On Wed, Aug 16, 2023 at 10:42 PM Dumitru Ceara  wrote:
> 
>> This is beneficial in a few ways:
>> - first, it reduces the number of different types of data the northd
>>   I-P node has to process.
>> - it turns out the northd I-P node (whose recompute is rather costly)
>>   doesn't really depend on meters or ACLs.
>> - prepares the ground for a pure I-P implementation for ACLs, with a
>>   simple/clear dependency between NB.ACL and the lflow I-P node.
>>
>> From a meters synchronization perspective this commit doesn't change any
>> of the existing behavior and logic.  It mostly just moves the meters
>> related code out of northd.c and into en-meters.c.
>>
>> Signed-off-by: Dumitru Ceara 
>>
> ---
>>
> 
> Hi Dumitru,
> 

Hi Ales,

> I have a couple of suggestions.
> 

Thanks for your review!

> 
>>  lib/stopwatch-names.h|   1 +
>>  northd/automake.mk   |   2 +
>>  northd/en-lflow.c|   5 +-
>>  northd/en-meters.c   | 295 +++
>>  northd/en-meters.h   |  44 ++
>>  northd/en-northd.c   |   6 -
>>  northd/inc-proc-northd.c |  14 +-
>>  northd/northd.c  | 249 ++---
>>  northd/northd.h  |   4 -
>>  northd/ovn-northd.c  |   3 +
>>  tests/ovn-northd.at  |  36 +
>>  11 files changed, 404 insertions(+), 255 deletions(-)
>>  create mode 100644 northd/en-meters.c
>>  create mode 100644 northd/en-meters.h
>>
>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
>> index de6fca4ccc..98535fff5a 100644
>> --- a/lib/stopwatch-names.h
>> +++ b/lib/stopwatch-names.h
>> @@ -30,5 +30,6 @@
>>  #define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
>>  #define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
>>  #define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
>> +#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>>
>>  #endif
>> diff --git a/northd/automake.mk b/northd/automake.mk
>> index b17f1fdb54..f52766de0c 100644
>> --- a/northd/automake.mk
>> +++ b/northd/automake.mk
>> @@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
>> northd/en-northd.h \
>> northd/en-lflow.c \
>> northd/en-lflow.h \
>> +   northd/en-meters.c \
>> +   northd/en-meters.h \
>> northd/en-northd-output.c \
>> northd/en-northd-output.h \
>> northd/en-sync-sb.c \
>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>> index 28ab1c67fb..0886d4c5ca 100644
>> --- a/northd/en-lflow.c
>> +++ b/northd/en-lflow.c
>> @@ -20,6 +20,7 @@
>>
>>  #include "en-lflow.h"
>>  #include "en-northd.h"
>> +#include "en-meters.h"
>>
>>  #include "lib/inc-proc-eng.h"
>>  #include "northd.h"
>> @@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
>>   struct lflow_input *lflow_input)
>>  {
>>  struct northd_data *northd_data = engine_get_input_data("northd",
>> node);
>> +struct sync_meters_data *sync_meters_data =
>> +engine_get_input_data("sync_meters", node);
>>  lflow_input->nbrec_bfd_table =
>>  EN_OVSDB_GET(engine_get_input("NB_bfd", node));
>>  lflow_input->sbrec_bfd_table =
>> @@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
>>  lflow_input->ls_ports = _data->ls_ports;
>>  lflow_input->lr_ports = _data->lr_ports;
>>  lflow_input->port_groups = _data->port_groups;
>> -lflow_input->meter_groups = _data->meter_groups;
>> +lflow_input->meter_groups = _meters_data->meter_groups;
>>  lflow_input->lbs = _data->lbs;
>>  lflow_input->features = _data->features;
>>  lflow_input->ovn_internal_version_changed =
>> diff --git a/northd/en-meters.c b/northd/en-meters.c
>> new file mode 100644
>> index 00..3d3b22368f
>> --- /dev/null
>> +++ b/northd/en-meters.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * Copyright (c) 2023, Red Hat, Inc.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include 
>> +
>> +#include "openvswitch/vlog.h"
>> +#include "stopwatch.h"
>> +
>> +#include "en-meters.h"
>> +#include "lib/stopwatch-names.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(en_meters);
>> +
>> +static void build_meter_groups(struct shash *meter_group,
>> +   const struct nbrec_meter_table *);
>> +static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
>> +const struct nbrec_meter_table *,
>> +const struct nbrec_acl_table *,

[ovs-dev] [PATCH ovn v2 2/2] northd: Add a separate I-P node for handling meters.

[Dumitru Ceara]

This is beneficial in a few ways:
- first, it reduces the number of different types of data the northd
  I-P node has to process.
- it turns out the northd I-P node (whose recompute is rather costly)
  doesn't really depend on meters or ACLs.
- prepares the ground for a pure I-P implementation for ACLs, with a
  simple/clear dependency between NB.ACL and the lflow I-P node.

>From a meters synchronization perspective this commit doesn't change any
of the existing behavior and logic.  It mostly just moves the meters
related code out of northd.c and into en-meters.c.

Signed-off-by: Dumitru Ceara 
---
 lib/stopwatch-names.h|1 
 northd/automake.mk   |2 
 northd/en-lflow.c|5 +
 northd/en-meters.c   |  281 ++
 northd/en-meters.h   |   44 +++
 northd/en-northd.c   |6 -
 northd/inc-proc-northd.c |   14 ++
 northd/northd.c  |  235 +-
 northd/northd.h  |4 -
 northd/ovn-northd.c  |3 
 tests/ovn-northd.at  |   36 ++
 11 files changed, 390 insertions(+), 241 deletions(-)
 create mode 100644 northd/en-meters.c
 create mode 100644 northd/en-meters.h

diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index de6fca4ccc..98535fff5a 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
 #define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
 #define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
 #define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
+#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
 
 #endif
diff --git a/northd/automake.mk b/northd/automake.mk
index b17f1fdb54..f52766de0c 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
northd/en-northd.h \
northd/en-lflow.c \
northd/en-lflow.h \
+   northd/en-meters.c \
+   northd/en-meters.h \
northd/en-northd-output.c \
northd/en-northd-output.h \
northd/en-sync-sb.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 28ab1c67fb..0886d4c5ca 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -20,6 +20,7 @@
 
 #include "en-lflow.h"
 #include "en-northd.h"
+#include "en-meters.h"
 
 #include "lib/inc-proc-eng.h"
 #include "northd.h"
@@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
  struct lflow_input *lflow_input)
 {
 struct northd_data *northd_data = engine_get_input_data("northd", node);
+struct sync_meters_data *sync_meters_data =
+engine_get_input_data("sync_meters", node);
 lflow_input->nbrec_bfd_table =
 EN_OVSDB_GET(engine_get_input("NB_bfd", node));
 lflow_input->sbrec_bfd_table =
@@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
 lflow_input->ls_ports = _data->ls_ports;
 lflow_input->lr_ports = _data->lr_ports;
 lflow_input->port_groups = _data->port_groups;
-lflow_input->meter_groups = _data->meter_groups;
+lflow_input->meter_groups = _meters_data->meter_groups;
 lflow_input->lbs = _data->lbs;
 lflow_input->features = _data->features;
 lflow_input->ovn_internal_version_changed =
diff --git a/northd/en-meters.c b/northd/en-meters.c
new file mode 100644
index 00..aabd002b62
--- /dev/null
+++ b/northd/en-meters.c
@@ -0,0 +1,281 @@
+/*
+ * Copyright (c) 2023, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include "openvswitch/vlog.h"
+#include "stopwatch.h"
+
+#include "en-meters.h"
+#include "lib/stopwatch-names.h"
+
+VLOG_DEFINE_THIS_MODULE(en_meters);
+
+static void build_meter_groups(struct shash *meter_group,
+   const struct nbrec_meter_table *);
+static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
+const struct nbrec_meter_table *,
+const struct nbrec_acl_table *,
+const struct sbrec_meter_table *,
+struct shash *meter_groups);
+
+void
+*en_sync_meters_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+struct sync_meters_data *data = xmalloc(sizeof *data);
+
+*data = (struct sync_meters_data) {
+.meter_groups = SHASH_INITIALIZER(>meter_groups),
+};
+return data;
+}
+
+void
+en_sync_meters_cleanup(void *data_)
+{
+struct sync_meters_data *data = data_;
+
+

[ovs-dev] [PATCH ovn v2 1/2] northd: Refactor meter code to avoid duplication.

[Dumitru Ceara]

Use the band_cmp() function instead of duplicating code.  Also simplify
some of the band related code.

Suggested-by: Ales Musil 
Signed-off-by: Dumitru Ceara 
---
 northd/northd.c |   22 --
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 9a12a94ae2..13f2ae0565 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -16621,9 +16621,9 @@ band_cmp(const void *band1_, const void *band2_)
 const struct band_entry *band2p = band2_;
 
 if (band1p->rate != band2p->rate) {
-return band1p->rate > band2p->rate ? -1 : 1;
+return band1p->rate - band2p->rate;
 } else if (band1p->burst_size != band2p->burst_size) {
-return band1p->burst_size > band2p->burst_size ? -1 : 1;
+return band1p->burst_size - band2p->burst_size;
 } else {
 return strcmp(band1p->action, band2p->action);
 }
@@ -16637,17 +16637,6 @@ bands_need_update(const struct nbrec_meter *nb_meter,
 return true;
 }
 
-/* A single band is the most common scenario, so speed up that
- * check. */
-if (nb_meter->n_bands == 1) {
-struct nbrec_meter_band *nb_band = nb_meter->bands[0];
-struct sbrec_meter_band *sb_band = sb_meter->bands[0];
-
-return !(nb_band->rate == sb_band->rate
- && nb_band->burst_size == sb_band->burst_size
- && !strcmp(sb_band->action, nb_band->action));
-}
-
 /* Place the Northbound entries in sorted order. */
 struct band_entry *nb_bands;
 nb_bands = xmalloc(sizeof *nb_bands * nb_meter->n_bands);
@@ -16674,15 +16663,12 @@ bands_need_update(const struct nbrec_meter *nb_meter,
 
 bool need_update = false;
 for (size_t i = 0; i < nb_meter->n_bands; i++) {
-if (nb_bands[i].rate != sb_bands[i].rate
-|| nb_bands[i].burst_size != sb_bands[i].burst_size
-|| strcmp(nb_bands[i].action, sb_bands[i].action)) {
+if (band_cmp(_bands[i], _bands[i])) {
 need_update = true;
-goto done;
+break;
 }
 }
 
-done:
 free(nb_bands);
 free(sb_bands);
 

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

[ovs-dev] [PATCH ovn v2 0/2] northd: Meter incremental processing.

[Dumitru Ceara]

Changes in V2:
- Refactored a bit the meter code and added that as first patch.

Dumitru Ceara (2):
  northd: Refactor meter code to avoid duplication.
  northd: Add a separate I-P node for handling meters.


 lib/stopwatch-names.h|   1 +
 northd/automake.mk   |   2 +
 northd/en-lflow.c|   5 +-
 northd/en-meters.c   | 281 +++
 northd/en-meters.h   |  44 ++
 northd/en-northd.c   |   6 -
 northd/inc-proc-northd.c |  14 +-
 northd/northd.c  | 235 ++--
 northd/northd.h  |   4 -
 northd/ovn-northd.c  |   3 +
 tests/ovn-northd.at  |  36 +
 11 files changed, 390 insertions(+), 241 deletions(-)
 create mode 100644 northd/en-meters.c
 create mode 100644 northd/en-meters.h

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

Re: [ovs-dev] [PATCH branch-3.2 0/2] Release patches for v3.2.0.

[Eelco Chaudron]

On 17 Aug 2023, at 15:25, Ilya Maximets wrote:

> According to our release process, it's time to release v3.2.0!
>
> Ilya Maximets (2):
>   Set release date for 3.2.0.
>   Prepare for 3.2.1.


Ack on the series!

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] [PATCH branch-3.2 1/2] Set release date for 3.2.0.

[Ilya Maximets]

On 8/17/23 15:36, 0-day Robot wrote:
> Bleep bloop.  Greetings Ilya Maximets, 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.
> 
> 
> git-am:
> error: Failed to merge in the changes.
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> Patch failed at 0001 Set release date for 3.2.0.
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 

Looks like the bot tried applying to a wrong branch.

Best regards, Ilya Maximets.

> 
> Patch skipped due to previous failure.
> 
> 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 ovn 2/2] ovn-controller: Assume well-known tables are in the SB schema.

[Vladislav Odintsov]

Thanks Dumitru for the efforts!

> On 15 Aug 2023, at 13:39, Dumitru Ceara  wrote:
> 
> On 8/14/23 00:24, Han Zhou wrote:
>> On Fri, Aug 11, 2023 at 5:25 AM Dumitru Ceara  wrote:
>>> 
>>> It's safe to assume that tables that existed in the previous LTS branch
>>> first release (currently 22.03.0) can be monitored directly.  Do so and
>>> only "optionally" monitor the ones that have been added since.
>>> 
>>> This way we avoid the need for the IDL to expose an API to change the
>>> default condition for monitored tables.  It also avoids complex code in
>>> ovn-controller because we'd otherwise have to explicitly re-initialize
>>> conditions to a non-default (false) value after every SB reconnect.
>>> 
>>> NOTE: In order to make sure that pre-existing L3 and L2 gateways are not
>>> initially considered "non-local" we explicitly request for all port
>>> bindings of this type to be monitored in the startup stage (before we
>>> got the initial contents of the database and our chassis record).
>>> 
>> 
>> The commit subject and message had a good introduction about the solution
>> but it would be better to at least add a brief description of the problem
>> (about the memory spike) before the first paragraph.
>> It would also be good to add a Fixes tag for commit 1b0dbde.
> 
> Ack, I did that now.
> 
>> Otherwise it looks good to me!
>> 
>> Acked-by: Han Zhou mailto:hz...@ovn.org>>
>> 
> 
> Thanks, applied to main and backported to all branches down to 22.03.
> 
> Regards,
> Dumitru


Regards,
Vladislav Odintsov

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

Re: [ovs-dev] [PATCH branch-3.2 1/2] Set release date for 3.2.0.

[0-day Robot]

Bleep bloop.  Greetings Ilya Maximets, 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.


git-am:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Set release date for 3.2.0.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

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 branch-3.2 2/2] Prepare for 3.2.1.

[0-day Robot]

Bleep bloop.  Greetings Ilya Maximets, 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.


Patch skipped due to previous failure.

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 branch-3.2 0/2] Release patches for v3.2.0.

[Simon Horman]

On Thu, Aug 17, 2023 at 03:25:03PM +0200, Ilya Maximets wrote:
> According to our release process, it's time to release v3.2.0!
> 
> Ilya Maximets (2):
>   Set release date for 3.2.0.
>   Prepare for 3.2.1.
> 
>  NEWS | 5 -
>  configure.ac | 2 +-
>  debian/changelog | 8 +++-
>  3 files changed, 12 insertions(+), 3 deletions(-)

Acked-by: Simon Horman 

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

[ovs-dev] [PATCH branch-3.2 2/2] Prepare for 3.2.1.

[Ilya Maximets]

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

diff --git a/NEWS b/NEWS
index a3a5c2e4a..790cab495 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+v3.2.1 - xx xxx 
+
+
 v3.2.0 - 17 Aug 2023
 
- OVSDB:
diff --git a/configure.ac b/configure.ac
index 320509c5f..3c5225346 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(openvswitch, 3.2.0, b...@openvswitch.org)
+AC_INIT(openvswitch, 3.2.1, 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 8757e5cb2..385fe9716 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+openvswitch (3.2.1-1) unstable; urgency=low
+   [ Open vSwitch team ]
+   * New upstream version
+
+ -- Open vSwitch team   Thu, 17 Aug 2023 15:20:36 +0200
+
 openvswitch (3.2.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.40.1

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

[ovs-dev] [PATCH branch-3.2 0/2] Release patches for v3.2.0.

[Ilya Maximets]

According to our release process, it's time to release v3.2.0!

Ilya Maximets (2):
  Set release date for 3.2.0.
  Prepare for 3.2.1.

 NEWS | 5 -
 configure.ac | 2 +-
 debian/changelog | 8 +++-
 3 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.40.1

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

[ovs-dev] [PATCH branch-3.2 1/2] Set release date for 3.2.0.

[Ilya Maximets]

Signed-off-by: Ilya Maximets 
---
 NEWS | 2 +-
 debian/changelog | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 1438f9f8d..a3a5c2e4a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-v3.2.0 - xx xxx 
+v3.2.0 - 17 Aug 2023
 
- OVSDB:
  * Changed format in which ovsdb schema conversion operations are stored in
diff --git a/debian/changelog b/debian/changelog
index 294b4a5db..8757e5cb2 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,7 +2,7 @@ openvswitch (3.2.0-1) unstable; urgency=low
 
* New upstream version
 
- -- Open vSwitch team   Mon, 17 Jul 2023 14:40:00 +0100
+ -- Open vSwitch team   Thu, 17 Aug 2023 15:20:36 +0200
 
 openvswitch (3.1.0-1) unstable; urgency=low
 
-- 
2.40.1

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

Re: [ovs-dev] [PATCH ovn 3/3] m4: Update ax_func_posix_memalign to the latest version.

[Dumitru Ceara]

On 8/17/23 12:03, Ilya Maximets wrote:
> On 8/17/23 09:31, Dumitru Ceara wrote:
>> On 8/17/23 00:26, Ilya Maximets wrote:
>>> This fixes the obsolescence warning for AC_TRY_RUN with autoconf 2.70+:
>>>
>>>   $ ./boot.sh
>>>   configure.ac:141: warning: The macro `AC_TRY_RUN' is obsolete.
>>>   configure.ac:141: You should run autoupdate.
>>>   ./lib/autoconf/general.m4:2997: AC_TRY_RUN is expanded from...
>>>   lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
>>>   lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
>>>   ./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
>>>   ./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
>>>   m4/ax_func_posix_memalign.m4:27: AX_FUNC_POSIX_MEMALIGN is expanded 
>>> from...
>>>   configure.ac:141: the top level
>>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  m4/ax_func_posix_memalign.m4 | 8 
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/m4/ax_func_posix_memalign.m4 b/m4/ax_func_posix_memalign.m4
>>> index bd60adcbc..2442ceca7 100644
>>> --- a/m4/ax_func_posix_memalign.m4
>>> +++ b/m4/ax_func_posix_memalign.m4
>>> @@ -1,5 +1,5 @@
>>>  # 
>>> ===
>>> -#  http://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
>>> +#  
>>> https://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
>>>  # 
>>> ===
>>>  #
>>>  # SYNOPSIS
>>> @@ -22,12 +22,12 @@
>>>  #   and this notice are preserved. This file is offered as-is, without any
>>>  #   warranty.
>>>  
>>> -#serial 7
>>> +#serial 9
>>>  
>>>  AC_DEFUN([AX_FUNC_POSIX_MEMALIGN],
>>>  [AC_CACHE_CHECK([for working posix_memalign],
>>>[ax_cv_func_posix_memalign_works],
>>> -  [AC_TRY_RUN([
>>> +  [AC_RUN_IFELSE([AC_LANG_SOURCE([[
>>>  #include 
>>>  
>>>  int
>>> @@ -39,7 +39,7 @@ main ()
>>> * the size word. */
>>>exit (posix_memalign (, sizeof(void *), 123) != 0);
>>>  }
>>> -],
>>> +]])],
>>>  [ax_cv_func_posix_memalign_works=yes],
>>>  [ax_cv_func_posix_memalign_works=no],
>>>  [ax_cv_func_posix_memalign_works=no])])
>>
>> Hi Ilya,
>>
>> This looks correct but I we don't use HAVE_POSIX_MEMALIGN anywhere.
>> OVS libs do but OVN doesn't do that directly.
>>
>> Shouldn't we just remove this all together instead?  What do you think?
> 
> You're probably right and we could remove the macro, but I didn't
> test that.  Was just fixing a problem at hands.
> 

It was easy to remove but..

> The issue however might arise in the future if the check will
> ever creep out from OVS'es .c files to its internal (non-public)
> headers that OVN use.  In this case, if OVN will not check, it
> will silently start using a slow allocation method.
> 

.. indeed OVN uses internal OVS stuff so it's probably safer to keep the
check.  I pushed it as is.

Thanks!

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

Re: [ovs-dev] [PATCH ovn 0/3] m4: Fix autoconf 2.70+ warnings.

[Dumitru Ceara]

On 8/17/23 11:31, Ales Musil wrote:
> On Thu, Aug 17, 2023 at 12:26 AM Ilya Maximets  wrote:
> 
>> autoconf 2.70+ complais about these obsolete macros.
>> Replacements are available in our baseline 2.63, so
>> use them instead.
>>
>> It also complains about AC_PROG_CC_C99, but I'm not
>> sure if we want to or how to correctly replace it.
>> Suggested AC_PROG_CC enables C11 by default and that
>> might not be a desired behavior.
>>
>> This patch set is similar to what I did for OVS about
>> a year ago.
>>
>> Ilya Maximets (3):
>>   m4: Replace obsolete AC_HELP_STRING with AS_HELP_STRING.
>>   m4: Replace obsolete AC_ERROR with AC_MSG_ERROR.
>>   m4: Update ax_func_posix_memalign to the latest version.
>>
>>  acinclude.m4 | 23 +--
>>  m4/ax_func_posix_memalign.m4 |  8 
>>  m4/ovn.m4| 16 
>>  3 files changed, 25 insertions(+), 22 deletions(-)
>>
>> --
>> 2.40.1
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> The whole series looks good to me, thanks.
> 
> Acked-by: Ales Musil 
> 

Ah, I apologize, Ales, I just pushed the series and missed adding your
ack.  Thanks for reviewing it though and thanks, Ilya, for the patches!

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] northd: Add a separate I-P node for handling meters.

[Ales Musil]

On Wed, Aug 16, 2023 at 10:42 PM Dumitru Ceara  wrote:

> This is beneficial in a few ways:
> - first, it reduces the number of different types of data the northd
>   I-P node has to process.
> - it turns out the northd I-P node (whose recompute is rather costly)
>   doesn't really depend on meters or ACLs.
> - prepares the ground for a pure I-P implementation for ACLs, with a
>   simple/clear dependency between NB.ACL and the lflow I-P node.
>
> From a meters synchronization perspective this commit doesn't change any
> of the existing behavior and logic.  It mostly just moves the meters
> related code out of northd.c and into en-meters.c.
>
> Signed-off-by: Dumitru Ceara 
>
---
>

Hi Dumitru,

I have a couple of suggestions.


>  lib/stopwatch-names.h|   1 +
>  northd/automake.mk   |   2 +
>  northd/en-lflow.c|   5 +-
>  northd/en-meters.c   | 295 +++
>  northd/en-meters.h   |  44 ++
>  northd/en-northd.c   |   6 -
>  northd/inc-proc-northd.c |  14 +-
>  northd/northd.c  | 249 ++---
>  northd/northd.h  |   4 -
>  northd/ovn-northd.c  |   3 +
>  tests/ovn-northd.at  |  36 +
>  11 files changed, 404 insertions(+), 255 deletions(-)
>  create mode 100644 northd/en-meters.c
>  create mode 100644 northd/en-meters.h
>
> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> index de6fca4ccc..98535fff5a 100644
> --- a/lib/stopwatch-names.h
> +++ b/lib/stopwatch-names.h
> @@ -30,5 +30,6 @@
>  #define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
>  #define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
>  #define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
> +#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>
>  #endif
> diff --git a/northd/automake.mk b/northd/automake.mk
> index b17f1fdb54..f52766de0c 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -12,6 +12,8 @@ northd_ovn_northd_SOURCES = \
> northd/en-northd.h \
> northd/en-lflow.c \
> northd/en-lflow.h \
> +   northd/en-meters.c \
> +   northd/en-meters.h \
> northd/en-northd-output.c \
> northd/en-northd-output.h \
> northd/en-sync-sb.c \
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 28ab1c67fb..0886d4c5ca 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -20,6 +20,7 @@
>
>  #include "en-lflow.h"
>  #include "en-northd.h"
> +#include "en-meters.h"
>
>  #include "lib/inc-proc-eng.h"
>  #include "northd.h"
> @@ -35,6 +36,8 @@ lflow_get_input_data(struct engine_node *node,
>   struct lflow_input *lflow_input)
>  {
>  struct northd_data *northd_data = engine_get_input_data("northd",
> node);
> +struct sync_meters_data *sync_meters_data =
> +engine_get_input_data("sync_meters", node);
>  lflow_input->nbrec_bfd_table =
>  EN_OVSDB_GET(engine_get_input("NB_bfd", node));
>  lflow_input->sbrec_bfd_table =
> @@ -56,7 +59,7 @@ lflow_get_input_data(struct engine_node *node,
>  lflow_input->ls_ports = _data->ls_ports;
>  lflow_input->lr_ports = _data->lr_ports;
>  lflow_input->port_groups = _data->port_groups;
> -lflow_input->meter_groups = _data->meter_groups;
> +lflow_input->meter_groups = _meters_data->meter_groups;
>  lflow_input->lbs = _data->lbs;
>  lflow_input->features = _data->features;
>  lflow_input->ovn_internal_version_changed =
> diff --git a/northd/en-meters.c b/northd/en-meters.c
> new file mode 100644
> index 00..3d3b22368f
> --- /dev/null
> +++ b/northd/en-meters.c
> @@ -0,0 +1,295 @@
> +/*
> + * Copyright (c) 2023, Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include 
> +
> +#include "openvswitch/vlog.h"
> +#include "stopwatch.h"
> +
> +#include "en-meters.h"
> +#include "lib/stopwatch-names.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_meters);
> +
> +static void build_meter_groups(struct shash *meter_group,
> +   const struct nbrec_meter_table *);
> +static void sync_meters(struct ovsdb_idl_txn *ovnsb_txn,
> +const struct nbrec_meter_table *,
> +const struct nbrec_acl_table *,
> +const struct sbrec_meter_table *,
> +struct shash *meter_groups);
> +
> +void
> +*en_sync_meters_init(struct engine_node *node OVS_UNUSED,
> + 

Re: [ovs-dev] [PATCH ovn 3/3] m4: Update ax_func_posix_memalign to the latest version.

[Ilya Maximets]

On 8/17/23 09:31, Dumitru Ceara wrote:
> On 8/17/23 00:26, Ilya Maximets wrote:
>> This fixes the obsolescence warning for AC_TRY_RUN with autoconf 2.70+:
>>
>>   $ ./boot.sh
>>   configure.ac:141: warning: The macro `AC_TRY_RUN' is obsolete.
>>   configure.ac:141: You should run autoupdate.
>>   ./lib/autoconf/general.m4:2997: AC_TRY_RUN is expanded from...
>>   lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
>>   lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
>>   ./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
>>   ./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
>>   m4/ax_func_posix_memalign.m4:27: AX_FUNC_POSIX_MEMALIGN is expanded from...
>>   configure.ac:141: the top level
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  m4/ax_func_posix_memalign.m4 | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/m4/ax_func_posix_memalign.m4 b/m4/ax_func_posix_memalign.m4
>> index bd60adcbc..2442ceca7 100644
>> --- a/m4/ax_func_posix_memalign.m4
>> +++ b/m4/ax_func_posix_memalign.m4
>> @@ -1,5 +1,5 @@
>>  # 
>> ===
>> -#  http://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
>> +#  https://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
>>  # 
>> ===
>>  #
>>  # SYNOPSIS
>> @@ -22,12 +22,12 @@
>>  #   and this notice are preserved. This file is offered as-is, without any
>>  #   warranty.
>>  
>> -#serial 7
>> +#serial 9
>>  
>>  AC_DEFUN([AX_FUNC_POSIX_MEMALIGN],
>>  [AC_CACHE_CHECK([for working posix_memalign],
>>[ax_cv_func_posix_memalign_works],
>> -  [AC_TRY_RUN([
>> +  [AC_RUN_IFELSE([AC_LANG_SOURCE([[
>>  #include 
>>  
>>  int
>> @@ -39,7 +39,7 @@ main ()
>> * the size word. */
>>exit (posix_memalign (, sizeof(void *), 123) != 0);
>>  }
>> -],
>> +]])],
>>  [ax_cv_func_posix_memalign_works=yes],
>>  [ax_cv_func_posix_memalign_works=no],
>>  [ax_cv_func_posix_memalign_works=no])])
> 
> Hi Ilya,
> 
> This looks correct but I we don't use HAVE_POSIX_MEMALIGN anywhere.
> OVS libs do but OVN doesn't do that directly.
> 
> Shouldn't we just remove this all together instead?  What do you think?

You're probably right and we could remove the macro, but I didn't
test that.  Was just fixing a problem at hands.

The issue however might arise in the future if the check will
ever creep out from OVS'es .c files to its internal (non-public)
headers that OVN use.  In this case, if OVN will not check, it
will silently start using a slow allocation method.

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

Re: [ovs-dev] [PATCH ovn 0/3] m4: Fix autoconf 2.70+ warnings.

[Ales Musil]

On Thu, Aug 17, 2023 at 12:26 AM Ilya Maximets  wrote:

> autoconf 2.70+ complais about these obsolete macros.
> Replacements are available in our baseline 2.63, so
> use them instead.
>
> It also complains about AC_PROG_CC_C99, but I'm not
> sure if we want to or how to correctly replace it.
> Suggested AC_PROG_CC enables C11 by default and that
> might not be a desired behavior.
>
> This patch set is similar to what I did for OVS about
> a year ago.
>
> Ilya Maximets (3):
>   m4: Replace obsolete AC_HELP_STRING with AS_HELP_STRING.
>   m4: Replace obsolete AC_ERROR with AC_MSG_ERROR.
>   m4: Update ax_func_posix_memalign to the latest version.
>
>  acinclude.m4 | 23 +--
>  m4/ax_func_posix_memalign.m4 |  8 
>  m4/ovn.m4| 16 
>  3 files changed, 25 insertions(+), 22 deletions(-)
>
> --
> 2.40.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
The whole series looks good to me, thanks.

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

Re: [ovs-dev] [PATCH ovn] northd.c: Incremental processing for first/last switch port change.

[Ales Musil]

On Wed, Aug 16, 2023 at 2:56 AM Han Zhou  wrote:

> Commit 1eb09838 fixed an incremental processing problem by falling back
> to recompute. The problem was handling VIF changes for a LS that
> has router ports when:
> - adding the first switch port to the LS, or
> - deleting the last switch port from the LS
> because there are router port related lflows that depends on the
> condition whether there are only router ports on the LS or not.
>
> Since such scenario is common, this patch further improves it to avoid
> falling back to recompute.
>
> Signed-off-by: Han Zhou 
>

Hi Han,

there is one small nit down below.

> ---
>  northd/northd.c | 71 +
>  northd/northd.h |  1 +
>  tests/ovn-northd.at |  6 ++--
>  3 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 9a12a94ae25d..a87298a026de 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5120,21 +5120,8 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>  op->visited = false;
>  }
>
> -/* Check if the logical switch has only router ports before this
> change.
> - * If so, fall back to recompute.
> - * lflow engine node while building the lflows checks if the
> logical switch
> - * has any router ports and depending on that it adds different
> flows.
> - * See build_lswitch_rport_arp_req_flow() for more details.
> - * Note: We can definitely handle this scenario incrementally in
> the
> - * northd engine node and fall back to recompute in lflow engine
> node
> - * and even handle this incrementally in lflow node.  Until we do
> that
> - * resort to full recompute of northd node.
> - */
> -bool only_rports = (od->n_router_ports
> -&& (od->n_router_ports ==
> hmap_count(>ports)));
> -if (only_rports) {
> -goto fail;
> -}
> +ls_change->had_only_router_ports = (od->n_router_ports
> +&& (od->n_router_ports == hmap_count(>ports)));
>
>  /* Compare the individual ports in the old and new Logical
> Switches */
>  for (size_t j = 0; j < changed_ls->n_ports; ++j) {
> @@ -5217,22 +5204,6 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>  }
>  }
>
> -/* Check if the logical switch has only router ports after this
> change.
> - * If so, fall back to recompute.
> - * lflow engine node while building the lflows checks if the
> logical switch
> - * has any router ports and depending on that it adds different
> flows.
> - * See build_lswitch_rport_arp_req_flow() for more details.
> - * Note: We can definitely handle this scenario incrementally in
> the
> - * northd engine node and fall back to recompute in lflow engine
> node
> - * and even handle this incrementally in lflow node.  Until we do
> that
> - * resort to full recompute of northd node.
> - */
> -only_rports = (od->n_router_ports
> -   && (od->n_router_ports == hmap_count(>ports)));
> -if (only_rports) {
> -goto fail_clean_deleted;
> -}
> -
>  if (!ovs_list_is_empty(_change->added_ports) ||
>  !ovs_list_is_empty(_change->updated_ports) ||
>  !ovs_list_is_empty(_change->deleted_ports)) {
> @@ -16550,6 +16521,44 @@ bool lflow_handle_northd_ls_changes(struct
> ovsdb_idl_txn *ovnsb_txn,
>lfrn->lflow);
>  }
>  }
> +
> +bool ls_has_only_router_ports = (ls_change->od->n_router_ports &&
> + (ls_change->od->n_router_ports ==
> +
> hmap_count(_change->od->ports)));
> +
> +if (ls_change->had_only_router_ports != ls_has_only_router_ports)
> {
> +/* There are lflows related to router ports that depends on
> whether
> + * there are switch ports on the logical switch (see
> + * build_lswitch_rport_arp_req_flow() for more details).
> Since this
> + * dependency changed, we need to regenerate lflows for each
> router
> + * port on this logical switch. */
> +for (size_t i = 0; i < ls_change->od->n_router_ports; i++) {
> +op = ls_change->od->router_ports[i];
> +
> +/* Delete old lflows. */
> +if (!delete_lflow_for_lsp(op, "affected router",
> +
> lflow_input->sbrec_logical_flow_table,
> +  lflows)) {
> +return false;
> +}
> +
> +/* Generate new lflows. */
> +struct ds match = DS_EMPTY_INITIALIZER;
> +struct ds actions = DS_EMPTY_INITIALIZER;
> +build_lswitch_and_lrouter_iterate_by_lsp(op,
> +lflow_input->ls_ports, 

Re: [ovs-dev] [PATCH v2 ovn] controller: make garp_max_timeout configurable

[Ales Musil]

On Thu, Jul 20, 2023 at 5:02 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> When using VLAN backed networks and OVN routers leveraging the
> 'ovn-chassis-mac-mappings' option for east-west traffic, the eth.src field
> is
> replaced by the chassis mac address in order to not expose the router mac
> address from different nodes and confuse the TOR switch. However doing so
> the TOR switch is not able to learn the port/mac bindings for routed E/W
> traffic and it is force to always flood it. Fix this issue adding the
> capability to configure a given timeout for garp sent by ovn-controller
> and not disable it after the exponential backoff in order to keep
> refreshing the entries in TOR swtich fdb table.
> More into about the issue can be found here [0].
>
> [0]
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779
>
> Signed-off-by: Lorenzo Bianconi 
>

Hi Lorenzo,

I have a couple of comments below.

---
> Changes since v1:
> - add uni-test
> - add documentation
> ---
>  controller/ovn-controller.8.xml | 12 
>  controller/ovn-controller.c |  5 +++-
>  controller/pinctrl.c| 52 -
>  controller/pinctrl.h|  4 ++-
>  tests/ovn.at| 17 +++
>  5 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/controller/ovn-controller.8.xml
> b/controller/ovn-controller.8.xml
> index 0883d8da9..f2893f7ee 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -365,6 +365,18 @@
>  heplful to pin source outer IP for the tunnel when multiple
> interfaces
>  are used on the host for overlay traffic.
>
> +  external_ids:garp-max-timeout
>

The name is very confusing, it doesn't limit the value as specified, but
instead it's just timeout. I would suggest dropping the "max" from the name
or treat it as "max" and let the backoff do the work, capping it at the
specified value.


> +  
> +When used, this configuration value specifies the maximum timeout
> +(in seconds) between two consecutive GARP packets sent by
> +ovn-controller.
> +ovn-controller by default sends just 4 GARP packets
> +with an exponential backoff timeout.
> +Setting external_ids:garp-max-timeout allows to
> +continue sending GARPs with a given timeout.
> +Setting external_ids:garp-max-timeout to 0 will
> +reset the default behaviour.
> +  
>  
>
>  
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 236974f4f..b6d044bd0 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1040,6 +1040,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>   * calls are after the "non-track" calls. */
>  ovsdb_idl_add_table(ovs_idl, _table_open_vswitch);
>  ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_other_config);
> +ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_external_ids);
>

Why do we need this condition? We have other config values in this table
that are acted upon during the ovn-controller.


>  ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_bridges);
>  ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_datapaths);
>  ovsdb_idl_add_table(ovs_idl, _table_interface);
> @@ -5363,7 +5364,9 @@ main(int argc, char *argv[])
>  _data->local_datapaths,
>  _data->active_tunnels,
>
>  _data->local_active_ports_ipv6_pd,
> -
> _data->local_active_ports_ras);
> +_data->local_active_ports_ras,
> +ovsrec_open_vswitch_table_get(
> +ovs_idl_loop.idl));
>  stopwatch_stop(PINCTRL_RUN_STOPWATCH_NAME,
> time_msec());
>  mirror_run(ovs_idl_txn,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index bed90fe0b..9e724d259 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -165,6 +165,7 @@ VLOG_DEFINE_THIS_MODULE(pinctrl);
>  static struct ovs_mutex pinctrl_mutex = OVS_MUTEX_INITIALIZER;
>  static struct seq *pinctrl_handler_seq;
>  static struct seq *pinctrl_main_seq;
> +static long long int garp_rarp_max_timeout = LLONG_MAX;
>
>  static void *pinctrl_handler(void *arg);
>
> @@ -227,7 +228,8 @@ static void send_garp_rarp_prepare(
>  const struct ovsrec_bridge *,
>  const struct sbrec_chassis *,
>  const struct hmap *local_datapaths,
> -const struct sset *active_tunnels)
> +const struct sset *active_tunnels,
> +const struct ovsrec_open_vswitch_table *ovs_table)
>  OVS_REQUIRES(pinctrl_mutex);
>  static void send_garp_rarp_run(struct rconn *swconn,
> long long int *send_garp_rarp_time)
> 

Re: [ovs-dev] [PATCH ovn 3/3] m4: Update ax_func_posix_memalign to the latest version.

[Dumitru Ceara]

On 8/17/23 00:26, Ilya Maximets wrote:
> This fixes the obsolescence warning for AC_TRY_RUN with autoconf 2.70+:
> 
>   $ ./boot.sh
>   configure.ac:141: warning: The macro `AC_TRY_RUN' is obsolete.
>   configure.ac:141: You should run autoupdate.
>   ./lib/autoconf/general.m4:2997: AC_TRY_RUN is expanded from...
>   lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
>   lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
>   ./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
>   ./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
>   m4/ax_func_posix_memalign.m4:27: AX_FUNC_POSIX_MEMALIGN is expanded from...
>   configure.ac:141: the top level
> 
> Signed-off-by: Ilya Maximets 
> ---
>  m4/ax_func_posix_memalign.m4 | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/m4/ax_func_posix_memalign.m4 b/m4/ax_func_posix_memalign.m4
> index bd60adcbc..2442ceca7 100644
> --- a/m4/ax_func_posix_memalign.m4
> +++ b/m4/ax_func_posix_memalign.m4
> @@ -1,5 +1,5 @@
>  # ===
> -#  http://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
> +#  https://www.gnu.org/software/autoconf-archive/ax_func_posix_memalign.html
>  # ===
>  #
>  # SYNOPSIS
> @@ -22,12 +22,12 @@
>  #   and this notice are preserved. This file is offered as-is, without any
>  #   warranty.
>  
> -#serial 7
> +#serial 9
>  
>  AC_DEFUN([AX_FUNC_POSIX_MEMALIGN],
>  [AC_CACHE_CHECK([for working posix_memalign],
>[ax_cv_func_posix_memalign_works],
> -  [AC_TRY_RUN([
> +  [AC_RUN_IFELSE([AC_LANG_SOURCE([[
>  #include 
>  
>  int
> @@ -39,7 +39,7 @@ main ()
> * the size word. */
>exit (posix_memalign (, sizeof(void *), 123) != 0);
>  }
> -],
> +]])],
>  [ax_cv_func_posix_memalign_works=yes],
>  [ax_cv_func_posix_memalign_works=no],
>  [ax_cv_func_posix_memalign_works=no])])

Hi Ilya,

This looks correct but I we don't use HAVE_POSIX_MEMALIGN anywhere.
OVS libs do but OVN doesn't do that directly.

Shouldn't we just remove this all together instead?  What do you think?

Thanks,
Dumitru

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

[ovs-dev] [PATCH ovn v3] northd: Fix incorrect warning logs when handling port binding changes.

[numans]

From: Numan Siddique 

When changes to port bindings corresponding to router ports are
handled by northd engine node, incorrect warning logs (like below)
are logged.


northd|WARN|A port-binding for lrp0 is created but the LSP is not found.


Fix these warnings.

Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding in 
"northd" node.")

CC: Han Zhou 
Signed-off-by: Numan Siddique 
---

v2 -> v3
--
  * Checked if type of the 'pb' and return false if its a router port
before doing a lookup in the ls_ports map.

v1 -> v2
---
  * v1 was not returning false in northd_handle_sb_port_binding_changes()
for router port - port bindings because of which IPv6 PD system test
was failing.  Fixed it in v2.


 controller/binding.c |  75 
 controller/binding.h |  20 +
 lib/ovn-util.c   | 101 +++
 lib/ovn-util.h   |  21 +
 northd/northd.c  |   7 +++
 5 files changed, 130 insertions(+), 94 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 3ac0c35df..a521f2828 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct 
binding_lport *);
 static bool binding_lport_update_port_sec(
 struct binding_lport *, const struct sbrec_port_binding *);
 
-static char *get_lport_type_str(enum en_lport_type lport_type);
 static bool ovs_iface_matches_lport_iface_id_ver(
 const struct ovsrec_interface *,
 const struct sbrec_port_binding *);
@@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct local_binding_data 
*lbinding_data,
 free(nodes);
 }
 
-static bool
-is_lport_vif(const struct sbrec_port_binding *pb)
-{
-return !pb->type[0];
-}
-
-enum en_lport_type
-get_lport_type(const struct sbrec_port_binding *pb)
-{
-if (is_lport_vif(pb)) {
-if (pb->parent_port && pb->parent_port[0]) {
-return LP_CONTAINER;
-}
-return LP_VIF;
-} else if (!strcmp(pb->type, "patch")) {
-return LP_PATCH;
-} else if (!strcmp(pb->type, "chassisredirect")) {
-return LP_CHASSISREDIRECT;
-} else if (!strcmp(pb->type, "l3gateway")) {
-return LP_L3GATEWAY;
-} else if (!strcmp(pb->type, "localnet")) {
-return LP_LOCALNET;
-} else if (!strcmp(pb->type, "localport")) {
-return LP_LOCALPORT;
-} else if (!strcmp(pb->type, "l2gateway")) {
-return LP_L2GATEWAY;
-} else if (!strcmp(pb->type, "virtual")) {
-return LP_VIRTUAL;
-} else if (!strcmp(pb->type, "external")) {
-return LP_EXTERNAL;
-} else if (!strcmp(pb->type, "remote")) {
-return LP_REMOTE;
-} else if (!strcmp(pb->type, "vtep")) {
-return LP_VTEP;
-}
-
-return LP_UNKNOWN;
-}
-
-static char *
-get_lport_type_str(enum en_lport_type lport_type)
-{
-switch (lport_type) {
-case LP_VIF:
-return "VIF";
-case LP_CONTAINER:
-return "CONTAINER";
-case LP_VIRTUAL:
-return "VIRTUAL";
-case LP_PATCH:
-return "PATCH";
-case LP_CHASSISREDIRECT:
-return "CHASSISREDIRECT";
-case LP_L3GATEWAY:
-return "L3GATEWAY";
-case LP_LOCALNET:
-return "PATCH";
-case LP_LOCALPORT:
-return "LOCALPORT";
-case LP_L2GATEWAY:
-return "L2GATEWAY";
-case LP_EXTERNAL:
-return "EXTERNAL";
-case LP_REMOTE:
-return "REMOTE";
-case LP_VTEP:
-return "VTEP";
-case LP_UNKNOWN:
-return "UNKNOWN";
-}
-
-OVS_NOT_REACHED();
-}
-
 void
 set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
 const struct sbrec_chassis *chassis_rec,
diff --git a/controller/binding.h b/controller/binding.h
index 235e5860d..24bc84079 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -22,6 +22,7 @@
 #include "openvswitch/hmap.h"
 #include "openvswitch/uuid.h"
 #include "openvswitch/list.h"
+# include "lib/ovn-util.h"
 #include "sset.h"
 #include "lport.h"
 
@@ -217,25 +218,6 @@ void port_binding_set_down(const struct sbrec_chassis 
*chassis_rec,
const char *iface_id,
const struct uuid *pb_uuid);
 
-/* Corresponds to each Port_Binding.type. */
-enum en_lport_type {
-LP_UNKNOWN,
-LP_VIF,
-LP_CONTAINER,
-LP_PATCH,
-LP_L3GATEWAY,
-LP_LOCALNET,
-LP_LOCALPORT,
-LP_L2GATEWAY,
-LP_VTEP,
-LP_CHASSISREDIRECT,
-LP_VIRTUAL,
-LP_EXTERNAL,
-LP_REMOTE
-};
-
-enum en_lport_type get_lport_type(const struct sbrec_port_binding *);
-
 /* This structure represents a logical port (or port binding)
  * which is associated with 'struct local_binding'.
  *
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 080ad4c0c..3a237b7fe 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -1168,3 +1168,104 @@ encode_fqdn_string(const char *fqdn, size_t *len)
 
 return encoded;

Re: [ovs-dev] [PATCH ovn v2] northd: Fix incorrect warning logs when handling port binding changes.

[Numan Siddique]

On Thu, Aug 17, 2023 at 11:47 AM Han Zhou  wrote:
>
> On Wed, Aug 16, 2023 at 1:27 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > When changes to port bindings corresponding to router ports are
> > handled by northd engine node, incorrect warning logs (like below)
> > are logged.
> >
> > 
> > northd|WARN|A port-binding for lrp0 is created but the LSP is not found.
> > 
> >
> > Fix these warnings.
> >
> > Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding
> in "northd" node.")
> >
> > CC: Han Zhou 
> > Signed-off-by: Numan Siddique 
> > ---
> >
> > v1 -> v2
> > ---
> >   * v1 was not returning false in northd_handle_sb_port_binding_changes()
> > for router port - port bindings because of which IPv6 PD system test
> > was failing.  Fixed it in v2.
> >
> >  controller/binding.c |  75 
> >  controller/binding.h |  20 +
> >  lib/ovn-util.c   | 101 +++
> >  lib/ovn-util.h   |  21 +
> >  northd/northd.c  |   7 +++
> >  5 files changed, 130 insertions(+), 94 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 3ac0c35df..a521f2828 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct
> binding_lport *);
> >  static bool binding_lport_update_port_sec(
> >  struct binding_lport *, const struct sbrec_port_binding *);
> >
> > -static char *get_lport_type_str(enum en_lport_type lport_type);
> >  static bool ovs_iface_matches_lport_iface_id_ver(
> >  const struct ovsrec_interface *,
> >  const struct sbrec_port_binding *);
> > @@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct
> local_binding_data *lbinding_data,
> >  free(nodes);
> >  }
> >
> > -static bool
> > -is_lport_vif(const struct sbrec_port_binding *pb)
> > -{
> > -return !pb->type[0];
> > -}
> > -
> > -enum en_lport_type
> > -get_lport_type(const struct sbrec_port_binding *pb)
> > -{
> > -if (is_lport_vif(pb)) {
> > -if (pb->parent_port && pb->parent_port[0]) {
> > -return LP_CONTAINER;
> > -}
> > -return LP_VIF;
> > -} else if (!strcmp(pb->type, "patch")) {
> > -return LP_PATCH;
> > -} else if (!strcmp(pb->type, "chassisredirect")) {
> > -return LP_CHASSISREDIRECT;
> > -} else if (!strcmp(pb->type, "l3gateway")) {
> > -return LP_L3GATEWAY;
> > -} else if (!strcmp(pb->type, "localnet")) {
> > -return LP_LOCALNET;
> > -} else if (!strcmp(pb->type, "localport")) {
> > -return LP_LOCALPORT;
> > -} else if (!strcmp(pb->type, "l2gateway")) {
> > -return LP_L2GATEWAY;
> > -} else if (!strcmp(pb->type, "virtual")) {
> > -return LP_VIRTUAL;
> > -} else if (!strcmp(pb->type, "external")) {
> > -return LP_EXTERNAL;
> > -} else if (!strcmp(pb->type, "remote")) {
> > -return LP_REMOTE;
> > -} else if (!strcmp(pb->type, "vtep")) {
> > -return LP_VTEP;
> > -}
> > -
> > -return LP_UNKNOWN;
> > -}
> > -
> > -static char *
> > -get_lport_type_str(enum en_lport_type lport_type)
> > -{
> > -switch (lport_type) {
> > -case LP_VIF:
> > -return "VIF";
> > -case LP_CONTAINER:
> > -return "CONTAINER";
> > -case LP_VIRTUAL:
> > -return "VIRTUAL";
> > -case LP_PATCH:
> > -return "PATCH";
> > -case LP_CHASSISREDIRECT:
> > -return "CHASSISREDIRECT";
> > -case LP_L3GATEWAY:
> > -return "L3GATEWAY";
> > -case LP_LOCALNET:
> > -return "PATCH";
> > -case LP_LOCALPORT:
> > -return "LOCALPORT";
> > -case LP_L2GATEWAY:
> > -return "L2GATEWAY";
> > -case LP_EXTERNAL:
> > -return "EXTERNAL";
> > -case LP_REMOTE:
> > -return "REMOTE";
> > -case LP_VTEP:
> > -return "VTEP";
> > -case LP_UNKNOWN:
> > -return "UNKNOWN";
> > -}
> > -
> > -OVS_NOT_REACHED();
> > -}
> > -
> >  void
> >  set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
> >  const struct sbrec_chassis *chassis_rec,
> > diff --git a/controller/binding.h b/controller/binding.h
> > index 235e5860d..24bc84079 100644
> > --- a/controller/binding.h
> > +++ b/controller/binding.h
> > @@ -22,6 +22,7 @@
> >  #include "openvswitch/hmap.h"
> >  #include "openvswitch/uuid.h"
> >  #include "openvswitch/list.h"
> > +# include "lib/ovn-util.h"
> >  #include "sset.h"
> >  #include "lport.h"
> >
> > @@ -217,25 +218,6 @@ void port_binding_set_down(const struct
> sbrec_chassis *chassis_rec,
> > const char *iface_id,
> > const struct uuid *pb_uuid);
> >
> > -/* Corresponds to each Port_Binding.type. */
> > -enum en_lport_type {
> > -LP_UNKNOWN,
> > -LP_VIF,
> > -LP_CONTAINER,
> > -LP_PATCH,
> > -LP_L3GATEWAY,
> > -LP_LOCALNET,

Re: [ovs-dev] [PATCH v2] doc: Fix description of max_len for controller action.

[Simon Horman]

On Wed, Aug 16, 2023 at 05:30:59PM -0700, Antonin Bas wrote:
> From: Antonin Bas 
> 
> From: Antonin Bas 
> 
> Since Open vSwitch 2.7, the max_len option has no effect, and the full
> packet is always sent to controllers. This was confirmed with both the
> kernel and netdev datapaths.
> 
> Reported-by: Antonin Bas 
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/295
> Signed-off-by: Antonin Bas 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH ovn v2] northd: Fix incorrect warning logs when handling port binding changes.

[Han Zhou]

On Wed, Aug 16, 2023 at 1:27 AM  wrote:
>
> From: Numan Siddique 
>
> When changes to port bindings corresponding to router ports are
> handled by northd engine node, incorrect warning logs (like below)
> are logged.
>
> 
> northd|WARN|A port-binding for lrp0 is created but the LSP is not found.
> 
>
> Fix these warnings.
>
> Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding
in "northd" node.")
>
> CC: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---
>
> v1 -> v2
> ---
>   * v1 was not returning false in northd_handle_sb_port_binding_changes()
> for router port - port bindings because of which IPv6 PD system test
> was failing.  Fixed it in v2.
>
>  controller/binding.c |  75 
>  controller/binding.h |  20 +
>  lib/ovn-util.c   | 101 +++
>  lib/ovn-util.h   |  21 +
>  northd/northd.c  |   7 +++
>  5 files changed, 130 insertions(+), 94 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 3ac0c35df..a521f2828 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct
binding_lport *);
>  static bool binding_lport_update_port_sec(
>  struct binding_lport *, const struct sbrec_port_binding *);
>
> -static char *get_lport_type_str(enum en_lport_type lport_type);
>  static bool ovs_iface_matches_lport_iface_id_ver(
>  const struct ovsrec_interface *,
>  const struct sbrec_port_binding *);
> @@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct
local_binding_data *lbinding_data,
>  free(nodes);
>  }
>
> -static bool
> -is_lport_vif(const struct sbrec_port_binding *pb)
> -{
> -return !pb->type[0];
> -}
> -
> -enum en_lport_type
> -get_lport_type(const struct sbrec_port_binding *pb)
> -{
> -if (is_lport_vif(pb)) {
> -if (pb->parent_port && pb->parent_port[0]) {
> -return LP_CONTAINER;
> -}
> -return LP_VIF;
> -} else if (!strcmp(pb->type, "patch")) {
> -return LP_PATCH;
> -} else if (!strcmp(pb->type, "chassisredirect")) {
> -return LP_CHASSISREDIRECT;
> -} else if (!strcmp(pb->type, "l3gateway")) {
> -return LP_L3GATEWAY;
> -} else if (!strcmp(pb->type, "localnet")) {
> -return LP_LOCALNET;
> -} else if (!strcmp(pb->type, "localport")) {
> -return LP_LOCALPORT;
> -} else if (!strcmp(pb->type, "l2gateway")) {
> -return LP_L2GATEWAY;
> -} else if (!strcmp(pb->type, "virtual")) {
> -return LP_VIRTUAL;
> -} else if (!strcmp(pb->type, "external")) {
> -return LP_EXTERNAL;
> -} else if (!strcmp(pb->type, "remote")) {
> -return LP_REMOTE;
> -} else if (!strcmp(pb->type, "vtep")) {
> -return LP_VTEP;
> -}
> -
> -return LP_UNKNOWN;
> -}
> -
> -static char *
> -get_lport_type_str(enum en_lport_type lport_type)
> -{
> -switch (lport_type) {
> -case LP_VIF:
> -return "VIF";
> -case LP_CONTAINER:
> -return "CONTAINER";
> -case LP_VIRTUAL:
> -return "VIRTUAL";
> -case LP_PATCH:
> -return "PATCH";
> -case LP_CHASSISREDIRECT:
> -return "CHASSISREDIRECT";
> -case LP_L3GATEWAY:
> -return "L3GATEWAY";
> -case LP_LOCALNET:
> -return "PATCH";
> -case LP_LOCALPORT:
> -return "LOCALPORT";
> -case LP_L2GATEWAY:
> -return "L2GATEWAY";
> -case LP_EXTERNAL:
> -return "EXTERNAL";
> -case LP_REMOTE:
> -return "REMOTE";
> -case LP_VTEP:
> -return "VTEP";
> -case LP_UNKNOWN:
> -return "UNKNOWN";
> -}
> -
> -OVS_NOT_REACHED();
> -}
> -
>  void
>  set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb,
>  const struct sbrec_chassis *chassis_rec,
> diff --git a/controller/binding.h b/controller/binding.h
> index 235e5860d..24bc84079 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -22,6 +22,7 @@
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/uuid.h"
>  #include "openvswitch/list.h"
> +# include "lib/ovn-util.h"
>  #include "sset.h"
>  #include "lport.h"
>
> @@ -217,25 +218,6 @@ void port_binding_set_down(const struct
sbrec_chassis *chassis_rec,
> const char *iface_id,
> const struct uuid *pb_uuid);
>
> -/* Corresponds to each Port_Binding.type. */
> -enum en_lport_type {
> -LP_UNKNOWN,
> -LP_VIF,
> -LP_CONTAINER,
> -LP_PATCH,
> -LP_L3GATEWAY,
> -LP_LOCALNET,
> -LP_LOCALPORT,
> -LP_L2GATEWAY,
> -LP_VTEP,
> -LP_CHASSISREDIRECT,
> -LP_VIRTUAL,
> -LP_EXTERNAL,
> -LP_REMOTE
> -};
> -
> -enum en_lport_type get_lport_type(const struct sbrec_port_binding *);
> -
>  /* This structure represents a logical port (or port binding)
>   * which is associated with 'struct local_binding'.
>   *
> diff