| Message ID | b1f969b14815ab65134c079565c37357dc74c945.1685966596.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] controller: make garp_max_timeout configurable | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Mon, Jun 5, 2023 at 5:54 PM Lorenzo Bianconi <lorenzo.bianconi@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 <lorenzo.bianconi@redhat.com> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
> On Mon, Jun 5, 2023 at 5:54 PM Lorenzo Bianconi > <lorenzo.bianconi@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 <lorenzo.bianconi@redhat.com> > > --- > > 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. Hi Numan, thx for the review. Ack I will fix them. Regards, Lorenzo > > 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 > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
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);
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 <lorenzo.bianconi@redhat.com> --- controller/ovn-controller.c | 5 ++++- controller/pinctrl.c | 41 ++++++++++++++++++++++++++++++------- controller/pinctrl.h | 4 +++- 3 files changed, 41 insertions(+), 9 deletions(-)