> 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.
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
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
