[ovs-dev] [RFC] ovn-controller: Inject GARPs to logical switch pipeline to update neighbours

Daniel Alvarez dalvarez at redhat.com
Fri Nov 30 15:22:22 UTC 2018


Prior to this patch, GARPs announcing NAT addresses or new VIFs
were sent out to localnet ofport through an output action.
This can lead to problems since local datapaths won't get those
GARPs and ovn-controller won't update MAC_Binding entries (as
upstream switch will not send back the GARP to this port hence
other logical routers won't update their neighbours).

This patch is changing the behavior so that GARPs get injected
to OVN pipeline of the external switch. This way, they'll get
broadcasted to local pipelines and also sent out to the external
network through the localnet port.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047604.html
Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
---
 ovn/controller/pinctrl.c | 61 +++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 35 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 56539a891..b3b887cd0 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -2021,6 +2021,8 @@ struct garp_data {
     int backoff;                 /* Backoff for the next announcement. */
     ofp_port_t ofport;           /* ofport used to output this GARP. */
     int tag;                     /* VLAN tag of this GARP packet, or -1. */
+    uint32_t dp_key;             /* Datapath used to output this GARP. */
+    uint32_t port_key;           /* Port to inject the GARP into. */
 };

 /* Contains GARPs to be sent. */
@@ -2043,37 +2045,24 @@ destroy_send_garps(void)
 }

 static void
-add_garp(const char *name, ofp_port_t ofport, int tag,
-         const struct eth_addr ea, ovs_be32 ip)
+add_garp(const char *name, const struct eth_addr ea, ovs_be32 ip,
+         uint32_t dp_key, uint32_t port_key)
 {
     struct garp_data *garp = xmalloc(sizeof *garp);
     garp->ea = ea;
     garp->ipv4 = ip;
     garp->announce_time = time_msec() + 1000;
     garp->backoff = 1;
-    garp->ofport = ofport;
-    garp->tag = tag;
+    garp->dp_key = dp_key;
+    garp->port_key = port_key;
     shash_add(&send_garp_data, name, garp);
 }

 /* Add or update a vif for which GARPs need to be announced. */
 static void
 send_garp_update(const struct sbrec_port_binding *binding_rec,
-                 struct simap *localnet_ofports,
-                 const struct hmap *local_datapaths,
                  struct shash *nat_addresses)
 {
-    /* Find the localnet ofport to send this GARP. */
-    struct local_datapath *ld
-        = get_local_datapath(local_datapaths,
-                             binding_rec->datapath->tunnel_key);
-    if (!ld || !ld->localnet_port) {
-        return;
-    }
-    ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
-                                             ld->localnet_port->logical_port));
-    int tag = ld->localnet_port->n_tag ? *ld->localnet_port->tag : -1;
-
     volatile struct garp_data *garp = NULL;
     /* Update GARP for NAT IP if it exists.  Consider port bindings with type
      * "l3gateway" for logical switch ports attached to gateway routers, and
@@ -2090,11 +2079,13 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
                                                 laddrs->ipv4_addrs[i].addr_s);
                 garp = shash_find_data(&send_garp_data, name);
                 if (garp) {
-                    garp->ofport = ofport;
-                    garp->tag = tag;
+                    garp->dp_key = binding_rec->datapath->tunnel_key;
+                    garp->port_key = binding_rec->tunnel_key;
                 } else {
-                    add_garp(name, ofport, tag, laddrs->ea,
-                             laddrs->ipv4_addrs[i].addr);
+                    add_garp(name, laddrs->ea,
+                             laddrs->ipv4_addrs[i].addr,
+                             binding_rec->datapath->tunnel_key,
+                             binding_rec->tunnel_key);
                 }
                 free(name);
             }
@@ -2107,7 +2098,8 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
     /* Update GARP for vif if it exists. */
     garp = shash_find_data(&send_garp_data, binding_rec->logical_port);
     if (garp) {
-        garp->ofport = ofport;
+        garp->dp_key = binding_rec->datapath->tunnel_key;
+        garp->port_key = binding_rec->tunnel_key;
         return;
     }

@@ -2120,8 +2112,9 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
             continue;
         }

-        add_garp(binding_rec->logical_port, ofport, tag,
-                 laddrs.ea, laddrs.ipv4_addrs[0].addr);
+        add_garp(binding_rec->logical_port,
+                 laddrs.ea, laddrs.ipv4_addrs[0].addr,
+                 binding_rec->datapath->tunnel_key, binding_rec->tunnel_key);

         destroy_lport_addresses(&laddrs);
         break;
@@ -2150,16 +2143,15 @@ send_garp(struct garp_data *garp, long long int current_time)
     compose_arp(&packet, ARP_OP_REQUEST, garp->ea, eth_addr_zero,
                 true, garp->ipv4, garp->ipv4);

-    /* Compose a GARP request packet's vlan if exist. */
-    if (garp->tag >= 0) {
-        eth_push_vlan(&packet, htons(ETH_TYPE_VLAN), htons(garp->tag));
-    }
-
-    /* Compose actions.  The garp request is output on localnet ofport. */
+    /* Inject GARP request. */
     uint64_t ofpacts_stub[4096 / 8];
     struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
     enum ofp_version version = rconn_get_version(swconn);
-    ofpact_put_OUTPUT(&ofpacts)->port = garp->ofport;
+    put_load(garp->dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
+    put_load(garp->port_key, MFF_LOG_INPORT, 0, 32, &ofpacts);
+    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
+    resubmit->in_port = OFPP_CONTROLLER;
+    resubmit->table_id = OFTABLE_LOG_INGRESS_PIPELINE;

     struct ofputil_packet_out po = {
         .packet = dp_packet_data(&packet),
@@ -2171,6 +2163,7 @@ send_garp(struct garp_data *garp, long long int current_time)
     match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
     enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
     queue_msg(ofputil_encode_packet_out(&po, proto));
+
     dp_packet_uninit(&packet);
     ofpbuf_uninit(&ofpacts);

@@ -2489,8 +2482,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
         const struct sbrec_port_binding *pb = lport_lookup_by_name(
             sbrec_port_binding_by_name, iface_id);
         if (pb) {
-            send_garp_update(pb, &localnet_ofports, local_datapaths,
-                             &nat_addresses);
+            send_garp_update(pb, &nat_addresses);
         }
     }

@@ -2500,8 +2492,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
         const struct sbrec_port_binding *pb
             = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
         if (pb) {
-            send_garp_update(pb, &localnet_ofports, local_datapaths,
-                             &nat_addresses);
+            send_garp_update(pb, &nat_addresses);
         }
     }

--



More information about the dev mailing list