On Mon, Jun 5, 2023 at 5:54 PM Lorenzo Bianconi
<[email protected]> 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 <[email protected]>
> ---
> controller/ovn-controller.c | 5 ++++-
> controller/pinctrl.c | 41 ++++++++++++++++++++++++++++++-------
> controller/pinctrl.h | 4 +++-
> 3 files changed, 41 insertions(+), 9 deletions(-)
Hi Lorenzo,
Sorry for the late reviews. Can you please add a test case for this ?
Also you need to update the relevant documentation for adding the new config.
Thanks
Numan
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 3a81a13fb..1b1a8ba1f 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1028,6 +1028,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> * calls are after the "non-track" calls. */
> ovsdb_idl_add_table(ovs_idl, &ovsrec_table_open_vswitch);
> ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_other_config);
> + ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
> ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_bridges);
> ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_datapaths);
> ovsdb_idl_add_table(ovs_idl, &ovsrec_table_interface);
> @@ -5339,7 +5340,9 @@ main(int argc, char *argv[])
> &runtime_data->local_datapaths,
> &runtime_data->active_tunnels,
>
> &runtime_data->local_active_ports_ipv6_pd,
> - &runtime_data->local_active_ports_ras);
> + &runtime_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 c396ad4c2..389c5f6a9 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);
>
> @@ -226,7 +227,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)
> @@ -3491,7 +3493,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct hmap *local_datapaths,
> const struct sset *active_tunnels,
> const struct shash *local_active_ports_ipv6_pd,
> - const struct shash *local_active_ports_ras)
> + const struct shash *local_active_ports_ras,
> + const struct ovsrec_open_vswitch_table *ovs_table)
> {
> ovs_mutex_lock(&pinctrl_mutex);
> run_put_mac_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
> @@ -3502,7 +3505,7 @@ 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, br_int, chassis,
> - local_datapaths, active_tunnels);
> + local_datapaths, active_tunnels, ovs_table);
> 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,
> @@ -4423,7 +4426,8 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> const struct hmap *local_datapaths,
> const struct sbrec_port_binding *binding_rec,
> - struct shash *nat_addresses)
> + struct shash *nat_addresses,
> + long long int garp_max_timeout)
> {
> volatile struct garp_rarp_data *garp_rarp = NULL;
>
> @@ -4449,6 +4453,9 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> if (garp_rarp) {
> garp_rarp->dp_key = binding_rec->datapath->tunnel_key;
> garp_rarp->port_key = binding_rec->tunnel_key;
> + if (garp_max_timeout != garp_rarp_max_timeout) {
> + garp_rarp->announce_time = time_msec() + 1000;
> + }
> } else {
> add_garp_rarp(name, laddrs->ea,
> laddrs->ipv4_addrs[i].addr,
> @@ -4473,6 +4480,9 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> if (garp_rarp) {
> garp_rarp->dp_key =
> binding_rec->datapath->tunnel_key;
> garp_rarp->port_key = binding_rec->tunnel_key;
> + if (garp_max_timeout != garp_rarp_max_timeout) {
> + garp_rarp->announce_time = time_msec() + 1000;
> + }
> } else {
> add_garp_rarp(name, laddrs->ea,
> 0, binding_rec->datapath->tunnel_key,
> @@ -4492,6 +4502,9 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> if (garp_rarp) {
> garp_rarp->dp_key = binding_rec->datapath->tunnel_key;
> garp_rarp->port_key = binding_rec->tunnel_key;
> + if (garp_max_timeout != garp_rarp_max_timeout) {
> + garp_rarp->announce_time = time_msec() + 1000;
> + }
> return;
> }
>
> @@ -4581,6 +4594,8 @@ send_garp_rarp(struct rconn *swconn, struct
> garp_rarp_data *garp_rarp,
> if (garp_rarp->backoff < 16) {
> garp_rarp->backoff *= 2;
> garp_rarp->announce_time = current_time + garp_rarp->backoff * 1000;
> + } else if (garp_rarp_max_timeout != LLONG_MAX) {
> + garp_rarp->announce_time = current_time + garp_rarp_max_timeout;
> } else {
> garp_rarp->announce_time = LLONG_MAX;
> }
> @@ -5880,13 +5895,21 @@ 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 sset *active_tunnels)
> + const struct sset *active_tunnels,
> + const struct ovsrec_open_vswitch_table *ovs_table)
> OVS_REQUIRES(pinctrl_mutex)
> {
> struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs);
> struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports);
> struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys);
> struct shash nat_addresses;
> + unsigned long long garp_max_timeout = LLONG_MAX;
> + const struct ovsrec_open_vswitch *cfg =
> + ovsrec_open_vswitch_table_first(ovs_table);
> + if (cfg) {
> + garp_max_timeout = smap_get_ullong(
> + &cfg->external_ids, "garp-max-timeout", LLONG_MAX);
> + }
>
> shash_init(&nat_addresses);
>
> @@ -5917,7 +5940,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> if (pb) {
> send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, pb, &nat_addresses);
> + local_datapaths, pb, &nat_addresses,
> + garp_max_timeout);
> }
> }
>
> @@ -5928,7 +5952,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
> if (pb) {
> send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, pb, &nat_addresses);
> + local_datapaths, pb, &nat_addresses,
> + garp_max_timeout);
> }
> }
>
> @@ -5946,6 +5971,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> shash_destroy(&nat_addresses);
>
> sset_destroy(&nat_ip_keys);
> +
> + garp_rarp_max_timeout = garp_max_timeout;
> }
>
> static bool
> diff --git a/controller/pinctrl.h b/controller/pinctrl.h
> index 279a49fbc..23343f097 100644
> --- a/controller/pinctrl.h
> +++ b/controller/pinctrl.h
> @@ -30,6 +30,7 @@ struct ovsdb_idl;
> struct ovsdb_idl_index;
> struct ovsdb_idl_txn;
> struct ovsrec_bridge;
> +struct ovsrec_open_vswitch_table;
> struct sbrec_chassis;
> struct sbrec_dns_table;
> struct sbrec_controller_event_table;
> @@ -57,7 +58,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct hmap *local_datapaths,
> const struct sset *active_tunnels,
> const struct shash *local_active_ports_ipv6_pd,
> - const struct shash *local_active_ports_ras);
> + const struct shash *local_active_ports_ras,
> + const struct ovsrec_open_vswitch_table *ovs_table);
> void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
> void pinctrl_destroy(void);
> void pinctrl_set_br_int_name(const char *br_int_name);
> --
> 2.40.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
