[ovs-dev] [PATCH ovn branch-20.09 1/2] pinctrl: Directly update MAC_Bindings created by self originated GARPs.

Numan Siddique numans at ovn.org
Wed Nov 4 12:14:24 UTC 2020


On Wed, Nov 4, 2020 at 4:22 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> OVN uses GARPs to announce changes to locally owned NAT addresses.  This is
> OK when updating upstream router caches but is unnecessary for updating OVN
> logical router MAC_Bindings.
>
> ovn-controller already has the information required for directly
> updating/inserting the MAC_Bindings that would be created by neighbor
> routers.
>
> This also has the advantage that GARPs don't necessarily need to be flooded
> in the complete L2 domain of the switch and that router patch ports can be
> skipped.  An upcoming commit will take advantage of this.
>
> Suggested-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Fixes: 81e928526b8a ("ovn-controller: Inject GARPs to logical switch pipeline to update neighbors")
> Acked-by: Mark Michelson <mmichels at redhat.com>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>

Thanks. Applied both the patches to branch-20.09.

Numan

> (cherry-picked from master commit a2b88dc5136507e727e4bcdc4bf6fde559f519a9)
> ---
>  controller/pinctrl.c |  105 ++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 85 insertions(+), 20 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 14303b0..2cb7839 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -200,8 +200,10 @@ static void init_send_garps_rarps(void);
>  static void destroy_send_garps_rarps(void);
>  static void send_garp_rarp_wait(long long int send_garp_rarp_time);
>  static void send_garp_rarp_prepare(
> +    struct ovsdb_idl_txn *ovnsb_idl_txn,
>      struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>      struct ovsdb_idl_index *sbrec_port_binding_by_name,
> +    struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>      const struct ovsrec_bridge *,
>      const struct sbrec_chassis *,
>      const struct hmap *local_datapaths,
> @@ -3009,8 +3011,9 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                           sbrec_mac_binding_by_lport_ip);
>      run_put_vport_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key,
>                             sbrec_port_binding_by_key, chassis);
> -    send_garp_rarp_prepare(sbrec_port_binding_by_datapath,
> -                           sbrec_port_binding_by_name, br_int, chassis,
> +    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);
>      prepare_ipv6_ras(local_datapaths);
>      prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name,
> @@ -3701,6 +3704,64 @@ mac_binding_lookup(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>      return retval;
>  }
>
> +/* Update or add an IP-MAC binding for 'logical_port'. */
> +static void
> +mac_binding_add(struct ovsdb_idl_txn *ovnsb_idl_txn,
> +                struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> +                const char *logical_port,
> +                const struct sbrec_datapath_binding *dp,
> +                struct eth_addr ea, const char *ip)
> +{
> +    /* Convert ethernet argument to string form for database. */
> +    char mac_string[ETH_ADDR_STRLEN + 1];
> +    snprintf(mac_string, sizeof mac_string, ETH_ADDR_FMT, ETH_ADDR_ARGS(ea));
> +
> +    const struct sbrec_mac_binding *b =
> +        mac_binding_lookup(sbrec_mac_binding_by_lport_ip, logical_port, ip);
> +    if (!b) {
> +        b = sbrec_mac_binding_insert(ovnsb_idl_txn);
> +        sbrec_mac_binding_set_logical_port(b, logical_port);
> +        sbrec_mac_binding_set_ip(b, ip);
> +        sbrec_mac_binding_set_mac(b, mac_string);
> +        sbrec_mac_binding_set_datapath(b, dp);
> +    } else if (strcmp(b->mac, mac_string)) {
> +        sbrec_mac_binding_set_mac(b, mac_string);
> +    }
> +}
> +
> +/* Simulate the effect of a GARP on local datapaths, i.e., create MAC_Bindings
> + * on peer router datapaths.
> + */
> +static void
> +send_garp_locally(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 *in_pb,
> +                  struct eth_addr ea, ovs_be32 ip)
> +{
> +    const struct local_datapath *ldp =
> +        get_local_datapath(local_datapaths, in_pb->datapath->tunnel_key);
> +
> +    ovs_assert(ldp);
> +    for (size_t i = 0; i < ldp->n_peer_ports; i++) {
> +        const struct sbrec_port_binding *local = ldp->peer_ports[i].local;
> +        const struct sbrec_port_binding *remote = ldp->peer_ports[i].remote;
> +
> +        /* Skip "ingress" port. */
> +        if (local == in_pb) {
> +            continue;
> +        }
> +
> +        struct ds ip_s = DS_EMPTY_INITIALIZER;
> +
> +        ip_format_masked(ip, OVS_BE32_MAX, &ip_s);
> +        mac_binding_add(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> +                        remote->logical_port, remote->datapath,
> +                        ea, ds_cstr(&ip_s));
> +        ds_destroy(&ip_s);
> +    }
> +}
> +
>  static void
>  run_put_mac_binding(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                      struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> @@ -3727,20 +3788,8 @@ run_put_mac_binding(struct ovsdb_idl_txn *ovnsb_idl_txn,
>
>      struct ds ip_s = DS_EMPTY_INITIALIZER;
>      ipv6_format_mapped(&pmb->ip_key, &ip_s);
> -
> -    /* Update or add an IP-MAC binding for this logical port. */
> -    const struct sbrec_mac_binding *b =
> -        mac_binding_lookup(sbrec_mac_binding_by_lport_ip, pb->logical_port,
> -                           ds_cstr(&ip_s));
> -    if (!b) {
> -        b = sbrec_mac_binding_insert(ovnsb_idl_txn);
> -        sbrec_mac_binding_set_logical_port(b, pb->logical_port);
> -        sbrec_mac_binding_set_ip(b, ds_cstr(&ip_s));
> -        sbrec_mac_binding_set_mac(b, mac_string);
> -        sbrec_mac_binding_set_datapath(b, pb->datapath);
> -    } else if (strcmp(b->mac, mac_string)) {
> -        sbrec_mac_binding_set_mac(b, mac_string);
> -    }
> +    mac_binding_add(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> +                    pb->logical_port, pb->datapath, pmb->mac, ds_cstr(&ip_s));
>      ds_destroy(&ip_s);
>  }
>
> @@ -3882,7 +3931,10 @@ add_garp_rarp(const char *name, const struct eth_addr ea, ovs_be32 ip,
>
>  /* Add or update a vif for which GARPs need to be announced. */
>  static void
> -send_garp_rarp_update(const struct sbrec_port_binding *binding_rec,
> +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)
>  {
>      volatile struct garp_rarp_data *garp_rarp = NULL;
> @@ -3908,6 +3960,11 @@ send_garp_rarp_update(const struct sbrec_port_binding *binding_rec,
>                                    laddrs->ipv4_addrs[i].addr,
>                                    binding_rec->datapath->tunnel_key,
>                                    binding_rec->tunnel_key);
> +                    send_garp_locally(ovnsb_idl_txn,
> +                                      sbrec_mac_binding_by_lport_ip,
> +                                      local_datapaths, binding_rec, laddrs->ea,
> +                                      laddrs->ipv4_addrs[i].addr);
> +
>                  }
>                  free(name);
>              }
> @@ -3943,6 +4000,10 @@ send_garp_rarp_update(const struct sbrec_port_binding *binding_rec,
>                        laddrs.ea, ip,
>                        binding_rec->datapath->tunnel_key,
>                        binding_rec->tunnel_key);
> +        if (ip) {
> +            send_garp_locally(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> +                              local_datapaths, binding_rec, laddrs.ea, ip);
> +        }
>
>          destroy_lport_addresses(&laddrs);
>          break;
> @@ -5219,8 +5280,10 @@ send_garp_rarp_run(struct rconn *swconn, long long int *send_garp_rarp_time)
>  /* Called by pinctrl_run(). Runs with in the main ovn-controller
>   * thread context. */
>  static void
> -send_garp_rarp_prepare(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> +send_garp_rarp_prepare(struct ovsdb_idl_txn *ovnsb_idl_txn,
> +                       struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>                         struct ovsdb_idl_index *sbrec_port_binding_by_name,
> +                       struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>                         const struct ovsrec_bridge *br_int,
>                         const struct sbrec_chassis *chassis,
>                         const struct hmap *local_datapaths,
> @@ -5259,7 +5322,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>          const struct sbrec_port_binding *pb = lport_lookup_by_name(
>              sbrec_port_binding_by_name, iface_id);
>          if (pb) {
> -            send_garp_rarp_update(pb, &nat_addresses);
> +            send_garp_rarp_update(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> +                                  local_datapaths, pb, &nat_addresses);
>          }
>      }
>
> @@ -5269,7 +5333,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>          const struct sbrec_port_binding *pb
>              = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
>          if (pb) {
> -            send_garp_rarp_update(pb, &nat_addresses);
> +            send_garp_rarp_update(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> +                                  local_datapaths, pb, &nat_addresses);
>          }
>      }
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list