When two gateways fight in ha mode to bind the router port,
there were cases in which one of the gateway almost always
immediately reclaimed the router port, as the grace time was
honored by the other gw. This is also the case since [1] for
the highest priority gw.
In such a case, the garp_rarp module did not detect the change in
the router port binding (as the port bound to a different chassis
in idl was already rebound in runtime_data, potentially during a
recompute.
Hence it was not resetting the garp "count", potentially resulting
in no new garps.
Also, with a fight time of 500 msec there is not enough time for gw
to send garp, so increase the grace time to two seconds.
[1] controller: Do not postpone claim for highest priority chassis.
Fixes: 8ab8570e21ff ("pinctrl: Fix missing garp.")
Reported-at: https://issues.redhat.com/browse/FDP-1418
Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
Signed-off-by: Ales Musil <amu...@redhat.com>
(cherry picked from commit 195331f5a84901f0f27b77a4f2f13c004a9397f8)
---
v2: - Added missing cherry-pick and Acked-by/Signed-off-by from origin commit.
---
controller/binding.c | 2 +-
controller/if-status.c | 26 ++++++++++
controller/if-status.h | 2 +
controller/ovn-controller.c | 3 +-
controller/pinctrl.c | 97 +++++++++++++++++++++++++++++--------
controller/pinctrl.h | 4 +-
6 files changed, 112 insertions(+), 22 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index c22f26b66..a526fb62b 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -49,7 +49,7 @@ VLOG_DEFINE_THIS_MODULE(binding);
#define OVN_QOS_TYPE "linux-htb"
-#define CLAIM_TIME_THRESHOLD_MS 500
+#define CLAIM_TIME_THRESHOLD_MS 2000
struct claimed_port {
long long int last_claimed;
diff --git a/controller/if-status.c b/controller/if-status.c
index 9b0f2bcdf..659fe0305 100644
--- a/controller/if-status.c
+++ b/controller/if-status.c
@@ -206,6 +206,10 @@ struct if_status_mgr {
/* All local interfaces, stored per state. */
struct hmapx ifaces_per_state[OIF_MAX];
+ /* All chassisredirect ports bound to different chassis in idl and bound in
+ * this loop. */
+ struct sset claimed_cr;
+
/* Registered ofctrl seqno type for port_binding flow installation. */
size_t iface_seq_type_pb_cfg;
@@ -245,6 +249,7 @@ if_status_mgr_create(void)
hmapx_init(&mgr->ifaces_per_state[i]);
}
shash_init(&mgr->ifaces);
+ sset_init(&mgr->claimed_cr);
shash_init(&mgr->ovn_uninstall_hash);
return mgr;
}
@@ -259,6 +264,8 @@ if_status_mgr_clear(struct if_status_mgr *mgr)
}
ovs_assert(shash_is_empty(&mgr->ifaces));
+ sset_clear(&mgr->claimed_cr);
+
SHASH_FOR_EACH_SAFE (node, &mgr->ovn_uninstall_hash) {
ovn_uninstall_hash_destroy(mgr, node);
}
@@ -274,6 +281,7 @@ if_status_mgr_destroy(struct if_status_mgr *mgr)
{
if_status_mgr_clear(mgr);
shash_destroy(&mgr->ifaces);
+ sset_destroy(&mgr->claimed_cr);
shash_destroy(&mgr->ovn_uninstall_hash);
for (size_t i = 0; i < ARRAY_SIZE(mgr->ifaces_per_state); i++) {
hmapx_destroy(&mgr->ifaces_per_state[i]);
@@ -313,6 +321,11 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr,
memcpy(&iface->parent_pb_uuid, &parent_pb->header_.uuid,
sizeof(iface->pb_uuid));
}
+ if (pb->chassis && pb->chassis != chassis_rec &&
+ !strcmp(pb->type, "chassisredirect")) {
+ sset_add(&mgr->claimed_cr, pb->logical_port);
+ }
+
if (!sb_readonly) {
if (bind_type == CAN_BIND_AS_MAIN) {
set_pb_chassis_in_sbrec(pb, chassis_rec, true);
@@ -346,6 +359,18 @@ if_status_mgr_iface_is_present(struct if_status_mgr *mgr,
const char *iface_id)
return !!shash_find_data(&mgr->ifaces, iface_id);
}
+bool
+if_status_reclaimed(struct if_status_mgr *mgr, const char *iface_id)
+{
+ return sset_find(&mgr->claimed_cr, iface_id);
+}
+
+struct sset *
+get_claimed_cr(struct if_status_mgr *mgr)
+{
+ return &mgr->claimed_cr;
+}
+
void
if_status_mgr_release_iface(struct if_status_mgr *mgr, const char *iface_id)
{
@@ -707,6 +732,7 @@ if_status_mgr_run(struct if_status_mgr *mgr,
if_status_mgr_update_bindings(mgr, binding_data, chassis_rec,
iface_table, pb_table,
sb_readonly, ovs_readonly);
+ sset_clear(&mgr->claimed_cr);
}
static void
diff --git a/controller/if-status.h b/controller/if-status.h
index d4f972355..d15ca3008 100644
--- a/controller/if-status.h
+++ b/controller/if-status.h
@@ -67,5 +67,7 @@ bool if_status_mgr_iface_update(const struct if_status_mgr
*mgr,
const struct ovsrec_interface *iface_rec);
bool if_status_is_port_claimed(const struct if_status_mgr *mgr,
const char *iface_id);
+bool if_status_reclaimed(struct if_status_mgr *mgr, const char *iface_id);
+struct sset * get_claimed_cr(struct if_status_mgr *mgr);
# endif /* controller/if-status.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index bb7d6c3b2..4a0f7084e 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -6450,7 +6450,8 @@ main(int argc, char *argv[])
&runtime_data->local_active_ports_ras,
ovsrec_open_vswitch_table_get(
ovs_idl_loop.idl),
- ovnsb_idl_loop.cur_cfg);
+ ovnsb_idl_loop.cur_cfg,
+ if_mgr);
stopwatch_stop(PINCTRL_RUN_STOPWATCH_NAME,
time_msec());
mirror_run(ovs_idl_txn,
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index db88b486a..e1fd5895c 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -242,7 +242,8 @@ static void send_garp_rarp_prepare(
const struct ovsrec_bridge *,
const struct sbrec_chassis *,
const struct hmap *local_datapaths,
- const struct ovsrec_open_vswitch_table *ovs_table)
+ const struct ovsrec_open_vswitch_table *ovs_table,
+ struct if_status_mgr *mgr)
OVS_REQUIRES(pinctrl_mutex);
static void send_garp_rarp_run(struct rconn *swconn,
long long int *send_garp_rarp_time)
@@ -4086,7 +4087,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct shash *local_active_ports_ipv6_pd,
const struct shash *local_active_ports_ras,
const struct ovsrec_open_vswitch_table *ovs_table,
- int64_t cur_cfg)
+ int64_t cur_cfg,
+ struct if_status_mgr *mgr)
{
ovs_mutex_lock(&pinctrl_mutex);
run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
@@ -4097,7 +4099,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
send_garp_rarp_prepare(ovnsb_idl_txn, sbrec_port_binding_by_datapath,
sbrec_port_binding_by_name,
sbrec_mac_binding_by_lport_ip, ecmp_nh_table,
- br_int, chassis, local_datapaths, ovs_table);
+ br_int, chassis, local_datapaths, ovs_table,
+ mgr);
prepare_ipv6_ras(local_active_ports_ras, sbrec_port_binding_by_name);
prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name,
local_active_ports_ipv6_pd, chassis,
@@ -4989,6 +4992,7 @@ wait_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn)
* their port-mac and ARP tables.
*/
struct garp_rarp_data {
+ struct cmap_node cmap_node;
struct eth_addr ea; /* Ethernet address of port. */
ovs_be32 ipv4; /* Ipv4 address of port. */
long long int announce_time; /* Next announcement in ms. */
@@ -4996,11 +5000,17 @@ struct garp_rarp_data {
* announcement (in msecs). */
uint32_t dp_key; /* Datapath used to output this GARP. */
uint32_t port_key; /* Port to inject the GARP into. */
+ char *logical_port; /* Name of the cr logical_port, if any */
};
/* Contains GARPs/RARPs to be sent. Protected by pinctrl_mutex*/
static struct shash send_garp_rarp_data;
+struct laddrs_port {
+ struct lport_addresses laddrs;
+ char *lport;
+};
+
static void
init_send_garps_rarps(void)
{
@@ -5010,13 +5020,51 @@ init_send_garps_rarps(void)
static void
destroy_send_garps_rarps(void)
{
+ struct shash_node *iter;
+ SHASH_FOR_EACH (iter, &send_garp_rarp_data) {
+ struct garp_rarp_data *garp_rarp = iter->data;
+ free(garp_rarp->logical_port);
+ }
shash_destroy_free_data(&send_garp_rarp_data);
}
+static void
+garp_rarp_node_reset_timers(const char *logical_port)
+{
+ struct shash_node *iter;
+ bool changed = false;
+ SHASH_FOR_EACH (iter, &send_garp_rarp_data) {
+ struct garp_rarp_data *garp_rarp_data = iter->data;
+ if (garp_rarp_data->logical_port &&
+ !strcmp(garp_rarp_data->logical_port, logical_port)) {
+ changed = true;
+ garp_rarp_data->announce_time = time_msec() + 1000;
+ garp_rarp_data->backoff = 1000;
+ }
+ }
+ if (changed) {
+ notify_pinctrl_handler();
+ }
+}
+
+static void
+reset_timers_for_claimed_cr(struct if_status_mgr *mgr)
+{
+ struct sset *claimed_cr = get_claimed_cr(mgr);
+ const char *cr_logical_port;
+ SSET_FOR_EACH_SAFE (cr_logical_port, claimed_cr) {
+ garp_rarp_node_reset_timers(cr_logical_port);
+ sset_delete(claimed_cr, SSET_NODE_FROM_NAME(cr_logical_port));
+ }
+
+}
+
+
/* Runs with in the main ovn-controller thread context. */
static void
add_garp_rarp(const char *name, const struct eth_addr ea, ovs_be32 ip,
- uint32_t dp_key, uint32_t port_key)
+ uint32_t dp_key, uint32_t port_key,
+ const char *logical_port)
{
struct garp_rarp_data *garp_rarp = xmalloc(sizeof *garp_rarp);
garp_rarp->ea = ea;
@@ -5025,6 +5073,7 @@ add_garp_rarp(const char *name, const struct eth_addr ea,
ovs_be32 ip,
garp_rarp->backoff = 1000; /* msec. */
garp_rarp->dp_key = dp_key;
garp_rarp->port_key = port_key;
+ garp_rarp->logical_port = nullable_xstrdup(logical_port);
shash_add(&send_garp_rarp_data, name, garp_rarp);
/* Notify pinctrl_handler so that it can wakeup and process
@@ -5055,9 +5104,10 @@ send_garp_rarp_update(struct ovsdb_idl_txn
*ovnsb_idl_txn,
* distributed gateway ports. */
if (!strcmp(binding_rec->type, "l3gateway")
|| !strcmp(binding_rec->type, "patch")) {
- struct lport_addresses *laddrs = NULL;
- while ((laddrs = shash_find_and_delete(nat_addresses,
- binding_rec->logical_port))) {
+ struct laddrs_port *laddrs_port = NULL;
+ while ((laddrs_port = shash_find_and_delete(nat_addresses,
+ binding_rec->logical_port))) {
+ struct lport_addresses *laddrs = &laddrs_port->laddrs;
int i;
for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
char *name = xasprintf("%s-%s", binding_rec->logical_port,
@@ -5076,7 +5126,8 @@ send_garp_rarp_update(struct ovsdb_idl_txn *ovnsb_idl_txn,
add_garp_rarp(name, laddrs->ea,
laddrs->ipv4_addrs[i].addr,
binding_rec->datapath->tunnel_key,
- binding_rec->tunnel_key);
+ binding_rec->tunnel_key,
+ laddrs_port->lport);
send_garp_locally(ovnsb_idl_txn,
sbrec_mac_binding_by_lport_ip,
local_datapaths, binding_rec, laddrs->ea,
@@ -5105,12 +5156,14 @@ send_garp_rarp_update(struct ovsdb_idl_txn
*ovnsb_idl_txn,
} else {
add_garp_rarp(name, laddrs->ea,
0, binding_rec->datapath->tunnel_key,
- binding_rec->tunnel_key);
+ binding_rec->tunnel_key,
+ laddrs_port->lport);
}
free(name);
}
destroy_lport_addresses(laddrs);
- free(laddrs);
+ free(laddrs_port->lport);
+ free(laddrs_port);
}
return;
}
@@ -5146,7 +5199,7 @@ send_garp_rarp_update(struct ovsdb_idl_txn *ovnsb_idl_txn,
add_garp_rarp(binding_rec->logical_port,
laddrs.ea, ip,
binding_rec->datapath->tunnel_key,
- binding_rec->tunnel_key);
+ binding_rec->tunnel_key, NULL);
if (ip) {
send_garp_locally(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
local_datapaths, binding_rec, laddrs.ea, ip);
@@ -5306,6 +5359,7 @@ send_garp_rarp_delete(const char *lport)
{
struct garp_rarp_data *garp_rarp = shash_find_and_delete
(&send_garp_rarp_data, lport);
+ free(garp_rarp->logical_port);
free(garp_rarp);
notify_pinctrl_handler();
}
@@ -6589,7 +6643,9 @@ consider_nat_address(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
const struct sbrec_chassis *chassis,
struct shash *nat_addresses)
{
- struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
+ struct laddrs_port *laddrs_port = xmalloc(sizeof *laddrs_port);
+ struct lport_addresses *laddrs = &laddrs_port->laddrs;
+
char *lport = NULL;
const struct sbrec_port_binding *cr_pb = NULL;
bool rc = extract_addresses_with_port(nat_address, laddrs, &lport);
@@ -6600,11 +6656,10 @@ consider_nat_address(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
|| (!lport && !strcmp(pb->type, "patch"))
|| (lport && (!cr_pb || (cr_pb->chassis != chassis)))) {
destroy_lport_addresses(laddrs);
- free(laddrs);
free(lport);
+ free(laddrs_port);
return;
}
- free(lport);
int i;
for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
@@ -6618,7 +6673,8 @@ consider_nat_address(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
sset_add(nat_address_keys, name);
free(name);
}
- shash_add(nat_addresses, pb->logical_port, laddrs);
+ laddrs_port->lport = lport;
+ shash_add(nat_addresses, pb->logical_port, laddrs_port);
}
static void
@@ -6761,7 +6817,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
*ovnsb_idl_txn,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis,
const struct hmap *local_datapaths,
- const struct ovsrec_open_vswitch_table *ovs_table)
+ const struct ovsrec_open_vswitch_table *ovs_table,
+ struct if_status_mgr *mgr)
OVS_REQUIRES(pinctrl_mutex)
{
struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs);
@@ -6789,6 +6846,7 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
*ovnsb_idl_txn,
shash_init(&nat_addresses);
+ reset_timers_for_claimed_cr(mgr);
get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath,
sbrec_port_binding_by_name,
br_int, chassis, local_datapaths,
@@ -6850,10 +6908,11 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
*ovnsb_idl_txn,
sset_destroy(&local_l3gw_ports);
SHASH_FOR_EACH_SAFE (iter, &nat_addresses) {
- struct lport_addresses *laddrs = iter->data;
- destroy_lport_addresses(laddrs);
+ struct laddrs_port *laddrs_port = iter->data;
+ destroy_lport_addresses(&laddrs_port->laddrs);
shash_delete(&nat_addresses, iter);
- free(laddrs);
+ free(laddrs_port->lport);
+ free(laddrs_port);
}
shash_destroy(&nat_addresses);
diff --git a/controller/pinctrl.h b/controller/pinctrl.h
index d3d5d8831..f2aed9b5e 100644
--- a/controller/pinctrl.h
+++ b/controller/pinctrl.h
@@ -22,6 +22,7 @@
#include "lib/sset.h"
#include "openvswitch/list.h"
#include "openvswitch/meta-flow.h"
+#include "if-status.h"
struct hmap;
struct shash;
@@ -62,7 +63,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct shash *local_active_ports_ipv6_pd,
const struct shash *local_active_ports_ras,
const struct ovsrec_open_vswitch_table *ovs_table,
- int64_t cur_cfg);
+ int64_t cur_cfg,
+ struct if_status_mgr *mgr);
void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
void pinctrl_destroy(void);
--
2.47.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
