| Message ID | 20181204181435.76755-1-dalvarez@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] ovn-controller: Inject GARPs to logical switch pipeline to update neighbors | expand |
On Tue, Dec 4, 2018 at 10:14 AM Daniel Alvarez <dalvarez@redhat.com> wrote: > > 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@redhat.com> > --- > > v1->v2 > Fix VLAN tests to account for the GARPs received by the local pipeline. > Remove localnet_ofports parameter as it's not used anymore. > > ovn/controller/pinctrl.c | 80 +++++++++---------------- > tests/ovn.at | 124 +++++++++++++++++++++++++++++++++------ > 2 files changed, 135 insertions(+), 69 deletions(-) > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > index 56539a891..3cd2ad718 100644 > --- a/ovn/controller/pinctrl.c > +++ b/ovn/controller/pinctrl.c > @@ -2019,8 +2019,8 @@ struct garp_data { > ovs_be32 ipv4; /* Ipv4 address of port. */ > long long int announce_time; /* Next announcement in ms. */ > 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 +2043,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 +2077,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 +2096,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 +2110,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 +2141,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), > @@ -2194,7 +2184,6 @@ get_localnet_vifs_l3gwports( > const struct sbrec_chassis *chassis, > const struct hmap *local_datapaths, > struct sset *localnet_vifs, > - struct simap *localnet_ofports, > struct sset *local_l3gw_ports) > { > for (int i = 0; i < br_int->n_ports; i++) { > @@ -2209,20 +2198,14 @@ get_localnet_vifs_l3gwports( > } > const char *localnet = smap_get(&port_rec->external_ids, > "ovn-localnet-port"); > + if (localnet) { > + continue; > + } > for (int j = 0; j < port_rec->n_interfaces; j++) { > const struct ovsrec_interface *iface_rec = port_rec->interfaces[j]; > if (!iface_rec->n_ofport) { > continue; > } > - /* Get localnet port with its ofport. */ > - if (localnet) { > - int64_t ofport = iface_rec->ofport[0]; > - if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) { > - continue; > - } > - simap_put(localnet_ofports, localnet, ofport); > - continue; > - } > /* Get localnet vif. */ > const char *iface_id = smap_get(&iface_rec->external_ids, > "iface-id"); > @@ -2458,7 +2441,6 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, > 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 simap localnet_ofports = SIMAP_INITIALIZER(&localnet_ofports); > struct shash nat_addresses; > > shash_init(&nat_addresses); > @@ -2466,8 +2448,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, > get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath, > sbrec_port_binding_by_name, > br_int, chassis, local_datapaths, > - &localnet_vifs, &localnet_ofports, > - &local_l3gw_ports); > + &localnet_vifs, &local_l3gw_ports); > > get_nat_addresses_and_keys(sbrec_chassis_by_name, > sbrec_port_binding_by_name, > @@ -2489,8 +2470,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 +2480,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); > } > } > > @@ -2516,7 +2495,6 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, > } > sset_destroy(&localnet_vifs); > sset_destroy(&local_l3gw_ports); > - simap_destroy(&localnet_ofports); > > SHASH_FOR_EACH_SAFE (iter, next, &nat_addresses) { > struct lport_addresses *laddrs = iter->data; > diff --git a/tests/ovn.at b/tests/ovn.at > index 2db3f675a..975229af7 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -8761,21 +8761,7 @@ src_mac="f00000010203" > dst_mac="000001010203" > packet=${foo_mac}${foo1_mac}08004500001c0000000040110000${foo1_ip}${dst_ip}0035111100080000 > > -as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > -sleep 2 > - > -# ARP request packet for nexthop_ip to expect at outside1 > -arp_request=ffffffffffff${gw_mac}08060001080006040001${gw_mac}${gw_ip}000000000000${nexthop_ip} > -echo $arp_request >> hv3-vif1.expected > -cat hv3-vif1.expected > expout > -$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/vif1-tx.pcap | grep ${nexthop_ip} | uniq > hv3-vif1 > -AT_CHECK([sort hv3-vif1], [0], [expout]) > - > -# Send ARP reply from outside1 back to the router > -reply_mac="f00000010204" > -arp_reply=${gw_mac}${nexthop_mac}08060001080006040002${nexthop_mac}${nexthop_ip}${gw_mac}${gw_ip} > - > -as hv3 ovs-appctl netdev-dummy/receive hv3-vif1 $arp_reply > +# Wait for GARPs announcing gw IP to arrive > OVS_WAIT_UNTIL([ > test `as hv2 ovs-ofctl dump-flows br-int | grep table=66 | \ > grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 > @@ -8806,15 +8792,17 @@ options:rxq_pcap=${pcap_file}-rx.pcap > as hv1 reset_pcap_file br-ex_n2 hv1/br-ex_n2 > as hv3 reset_pcap_file hv3-vif1 hv3/vif1 > sleep 2 > +# Take note of how many packets arrived on the VLAN switch before generating > +# further traffic > +n_packets=`as hv1 ovs-ofctl dump-flows br-int table=65 | grep "priority=100,reg15=0x1,metadata=0x2" | grep actions=clone | sed 's/.*n_packets=\([[0-9]]\+\),.*/\1/'` > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > sleep 2 > > # On hv1, the packet should not go from vlan switch pipleline to router > -# pipleine > +# pipeline > as hv1 ovs-ofctl dump-flows br-int > - > AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep "priority=100,reg15=0x1,metadata=0x2" \ > -| grep actions=clone | grep -v n_packets=0 | wc -l], [0], [[0 > +| grep actions=clone | grep -v n_packets=$n_packets | wc -l], [0], [[0 > ]]) > > # On hv1, table 32 check that no packet goes via the tunnel port > @@ -11727,3 +11715,103 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > > OVN_CLEANUP([hv1],[hv2]) > AT_CLEANUP > + > +AT_SETUP([ovn -- neighbor update on same HV]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +ovn_start > + > +# Logical network: > +# A public switch (pub) with a localnet port connected to two LRs (lr0 and lr1) > +# each with a distributed gateway port. > +# Two VMs: lp0 on sw0 connected to lr0 > +# lp1 on sw1 connected to lr1 > +# > +# This test adds a floating IP to each VM so when they are bound to the same > +# hypervisor, it checks that the GARP sent by ovn-controller causes the > +# MAC_Binding entries to be updated properly on each logical router. > +# It will also capture packets on the physical interface to make sure that the > +# GARPs have been sent out to the external network as well. > + > +# Create logical switches > +ovn-nbctl ls-add sw0 > +ovn-nbctl ls-add sw1 > +ovn-nbctl ls-add pub > + > +# Created localnet port on public switch > +ovn-nbctl lsp-add pub ln-pub > +ovn-nbctl lsp-set-type ln-pub localnet > +ovn-nbctl lsp-set-addresses ln-pub unknown > +ovn-nbctl lsp-set-options ln-pub network_name=phys > + > +# Create logical routers and connect them to public switch > +ovn-nbctl create Logical_Router name=lr0 > +ovn-nbctl create Logical_Router name=lr1 > + > +ovn-nbctl lrp-add lr0 lr0-pub f0:00:00:00:00:01 172.24.4.220/24 > +ovn-nbctl lsp-add pub pub-lr0 -- set Logical_Switch_Port pub-lr0 \ > + type=router options:router-port=lr0-pub options:nat-addresses="router" addresses="router" > +ovn-nbctl lrp-add lr1 lr1-pub f0:00:00:00:01:01 172.24.4.221/24 > +ovn-nbctl lsp-add pub pub-lr1 -- set Logical_Switch_Port pub-lr1 \ > + type=router options:router-port=lr1-pub options:nat-addresses="router" addresses="router" > + > +ovn-nbctl lrp-set-gateway-chassis lr0-pub hv1 10 > +ovn-nbctl lrp-set-gateway-chassis lr1-pub hv1 10 > + > +# Connect sw0 and sw1 to lr0 and lr1 > +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 > +ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 type=router \ > + options:router-port=lr0-sw0 addresses="router" > +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:00:00:ff:02 20.0.0.254/24 > +ovn-nbctl lsp-add sw1 sw1-lr1 -- set Logical_Switch_Port sw1-lr1 type=router \ > + options:router-port=lr1-sw1 addresses="router" > + > + > +# Add SNAT rules > +ovn-nbctl lr-nat-add lr0 snat 172.24.4.220 10.0.0.0/24 > +ovn-nbctl lr-nat-add lr1 snat 172.24.4.221 20.0.0.0/24 > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 172.24.4.1 > +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > + > +ovs-vsctl add-port br-int vif0 -- set Interface vif0 external-ids:iface-id=lp0 > +ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1 > + > +ovn-nbctl lsp-add sw0 lp0 > +ovn-nbctl lsp-add sw1 lp1 > +ovn-nbctl lsp-set-addresses lp0 "50:54:00:00:00:01 10.0.0.10" > +ovn-nbctl lsp-set-addresses lp1 "50:54:00:00:00:02 20.0.0.10" > + > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp0` = xup]) > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xup]) > + > +# Create two floating IPs, one for each VIF > +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10 > +ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10 > + > +# Check that the MAC_Binding entries have been properly created > +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr0-pub" ip="172.24.4.200" | wc -l` -gt 0]) > +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr1-pub" ip="172.24.4.100" | wc -l` -gt 0]) > + > +# Check that the GARPs went also to the external physical network > +# Wait until at least 4 packets have arrived and copy them to a separate file as > +# more GARPs are expected in the capture in order to avoid race conditions. > +OVS_WAIT_UNTIL([test `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | wc -l` -gt 4]) > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | head -n4 > hv1/br-phys-tx4.pcap > + > +# GARP for lp0 172.24.4.100 on lr0-pub MAC (f0:00:00:00:00:01) > +echo "fffffffffffff0000000000108060001080006040001f00000000001ac180464000000000000ac180464" > expout > +# GARP for 172.24.4.220 on lr0-pub (f0:00:00:00:00:01) > +echo "fffffffffffff0000000000108060001080006040001f00000000001ac1804dc000000000000ac1804dc" >> expout > +# GARP for lp1 172.24.4.200 on lr1-pub MAC (f0:00:00:00:01:01) > +echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804c8000000000000ac1804c8" >> expout > +# GARP for 172.24.4.221 on lr1-pub (f0:00:00:00:01:01) > +echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd000000000000ac1804dd" >> expout > +AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout]) > +#OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > -- > 2.17.2 (Apple Git-113) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks for the quick update. Acked-by: Han Zhou <hzhou8@ebay.com>
On Wed, Dec 5, 2018 at 5:03 AM Han Zhou <zhouhan@gmail.com> wrote: > On Tue, Dec 4, 2018 at 10:14 AM Daniel Alvarez <dalvarez@redhat.com> > wrote: > > > > 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@redhat.com> > > --- > > > > v1->v2 > > Fix VLAN tests to account for the GARPs received by the local pipeline. > > Remove localnet_ofports parameter as it's not used anymore. > > > > ovn/controller/pinctrl.c | 80 +++++++++---------------- > > tests/ovn.at | 124 +++++++++++++++++++++++++++++++++------ > > 2 files changed, 135 insertions(+), 69 deletions(-) > > > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > > index 56539a891..3cd2ad718 100644 > > --- a/ovn/controller/pinctrl.c > > +++ b/ovn/controller/pinctrl.c > > @@ -2019,8 +2019,8 @@ struct garp_data { > > ovs_be32 ipv4; /* Ipv4 address of port. */ > > long long int announce_time; /* Next announcement in ms. */ > > 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 +2043,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 +2077,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 +2096,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 +2110,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 +2141,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), > > @@ -2194,7 +2184,6 @@ get_localnet_vifs_l3gwports( > > const struct sbrec_chassis *chassis, > > const struct hmap *local_datapaths, > > struct sset *localnet_vifs, > > - struct simap *localnet_ofports, > > struct sset *local_l3gw_ports) > > { > > for (int i = 0; i < br_int->n_ports; i++) { > > @@ -2209,20 +2198,14 @@ get_localnet_vifs_l3gwports( > > } > > const char *localnet = smap_get(&port_rec->external_ids, > > "ovn-localnet-port"); > > + if (localnet) { > > + continue; > > + } > > for (int j = 0; j < port_rec->n_interfaces; j++) { > > const struct ovsrec_interface *iface_rec = > port_rec->interfaces[j]; > > if (!iface_rec->n_ofport) { > > continue; > > } > > - /* Get localnet port with its ofport. */ > > - if (localnet) { > > - int64_t ofport = iface_rec->ofport[0]; > > - if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) { > > - continue; > > - } > > - simap_put(localnet_ofports, localnet, ofport); > > - continue; > > - } > > /* Get localnet vif. */ > > const char *iface_id = smap_get(&iface_rec->external_ids, > > "iface-id"); > > @@ -2458,7 +2441,6 @@ send_garp_run(struct ovsdb_idl_index > *sbrec_chassis_by_name, > > 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 simap localnet_ofports = > SIMAP_INITIALIZER(&localnet_ofports); > > struct shash nat_addresses; > > > > shash_init(&nat_addresses); > > @@ -2466,8 +2448,7 @@ send_garp_run(struct ovsdb_idl_index > *sbrec_chassis_by_name, > > get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath, > > sbrec_port_binding_by_name, > > br_int, chassis, local_datapaths, > > - &localnet_vifs, &localnet_ofports, > > - &local_l3gw_ports); > > + &localnet_vifs, &local_l3gw_ports); > > > > get_nat_addresses_and_keys(sbrec_chassis_by_name, > > sbrec_port_binding_by_name, > > @@ -2489,8 +2470,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 +2480,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); > > } > > } > > > > @@ -2516,7 +2495,6 @@ send_garp_run(struct ovsdb_idl_index > *sbrec_chassis_by_name, > > } > > sset_destroy(&localnet_vifs); > > sset_destroy(&local_l3gw_ports); > > - simap_destroy(&localnet_ofports); > > > > SHASH_FOR_EACH_SAFE (iter, next, &nat_addresses) { > > struct lport_addresses *laddrs = iter->data; > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 2db3f675a..975229af7 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -8761,21 +8761,7 @@ src_mac="f00000010203" > > dst_mac="000001010203" > > > > packet=${foo_mac}${foo1_mac}08004500001c0000000040110000${foo1_ip}${dst_ip}0035111100080000 > > > > -as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > -sleep 2 > > - > > -# ARP request packet for nexthop_ip to expect at outside1 > > > > -arp_request=ffffffffffff${gw_mac}08060001080006040001${gw_mac}${gw_ip}000000000000${nexthop_ip} > > -echo $arp_request >> hv3-vif1.expected > > -cat hv3-vif1.expected > expout > > -$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/vif1-tx.pcap | grep > ${nexthop_ip} | uniq > hv3-vif1 > > -AT_CHECK([sort hv3-vif1], [0], [expout]) > > - > > -# Send ARP reply from outside1 back to the router > > -reply_mac="f00000010204" > > > > -arp_reply=${gw_mac}${nexthop_mac}08060001080006040002${nexthop_mac}${nexthop_ip}${gw_mac}${gw_ip} > > - > > -as hv3 ovs-appctl netdev-dummy/receive hv3-vif1 $arp_reply > > +# Wait for GARPs announcing gw IP to arrive > > OVS_WAIT_UNTIL([ > > test `as hv2 ovs-ofctl dump-flows br-int | grep table=66 | \ > > grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 > > @@ -8806,15 +8792,17 @@ options:rxq_pcap=${pcap_file}-rx.pcap > > as hv1 reset_pcap_file br-ex_n2 hv1/br-ex_n2 > > as hv3 reset_pcap_file hv3-vif1 hv3/vif1 > > sleep 2 > > +# Take note of how many packets arrived on the VLAN switch before > generating > > +# further traffic > > +n_packets=`as hv1 ovs-ofctl dump-flows br-int table=65 | grep > "priority=100,reg15=0x1,metadata=0x2" | grep actions=clone | sed > 's/.*n_packets=\([[0-9]]\+\),.*/\1/'` > > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > sleep 2 > > > > # On hv1, the packet should not go from vlan switch pipleline to router > > -# pipleine > > +# pipeline > > as hv1 ovs-ofctl dump-flows br-int > > - > > AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep > "priority=100,reg15=0x1,metadata=0x2" \ > > -| grep actions=clone | grep -v n_packets=0 | wc -l], [0], [[0 > > +| grep actions=clone | grep -v n_packets=$n_packets | wc -l], [0], [[0 > > ]]) > > > > # On hv1, table 32 check that no packet goes via the tunnel port > > @@ -11727,3 +11715,103 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], > [expected]) > > > > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > > + > > +AT_SETUP([ovn -- neighbor update on same HV]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > > +ovn_start > > + > > +# Logical network: > > +# A public switch (pub) with a localnet port connected to two LRs (lr0 > and lr1) > > +# each with a distributed gateway port. > > +# Two VMs: lp0 on sw0 connected to lr0 > > +# lp1 on sw1 connected to lr1 > > +# > > +# This test adds a floating IP to each VM so when they are bound to the > same > > +# hypervisor, it checks that the GARP sent by ovn-controller causes the > > +# MAC_Binding entries to be updated properly on each logical router. > > +# It will also capture packets on the physical interface to make sure > that the > > +# GARPs have been sent out to the external network as well. > > + > > +# Create logical switches > > +ovn-nbctl ls-add sw0 > > +ovn-nbctl ls-add sw1 > > +ovn-nbctl ls-add pub > > + > > +# Created localnet port on public switch > > +ovn-nbctl lsp-add pub ln-pub > > +ovn-nbctl lsp-set-type ln-pub localnet > > +ovn-nbctl lsp-set-addresses ln-pub unknown > > +ovn-nbctl lsp-set-options ln-pub network_name=phys > > + > > +# Create logical routers and connect them to public switch > > +ovn-nbctl create Logical_Router name=lr0 > > +ovn-nbctl create Logical_Router name=lr1 > > + > > +ovn-nbctl lrp-add lr0 lr0-pub f0:00:00:00:00:01 172.24.4.220/24 > > +ovn-nbctl lsp-add pub pub-lr0 -- set Logical_Switch_Port pub-lr0 \ > > + type=router options:router-port=lr0-pub > options:nat-addresses="router" addresses="router" > > +ovn-nbctl lrp-add lr1 lr1-pub f0:00:00:00:01:01 172.24.4.221/24 > > +ovn-nbctl lsp-add pub pub-lr1 -- set Logical_Switch_Port pub-lr1 \ > > + type=router options:router-port=lr1-pub > options:nat-addresses="router" addresses="router" > > + > > +ovn-nbctl lrp-set-gateway-chassis lr0-pub hv1 10 > > +ovn-nbctl lrp-set-gateway-chassis lr1-pub hv1 10 > > + > > +# Connect sw0 and sw1 to lr0 and lr1 > > +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 > > +ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 > type=router \ > > + options:router-port=lr0-sw0 addresses="router" > > +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:00:00:ff:02 20.0.0.254/24 > > +ovn-nbctl lsp-add sw1 sw1-lr1 -- set Logical_Switch_Port sw1-lr1 > type=router \ > > + options:router-port=lr1-sw1 addresses="router" > > + > > + > > +# Add SNAT rules > > +ovn-nbctl lr-nat-add lr0 snat 172.24.4.220 10.0.0.0/24 > > +ovn-nbctl lr-nat-add lr1 snat 172.24.4.221 20.0.0.0/24 > > + > > +net_add n1 > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 172.24.4.1 > > +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > > + > > +ovs-vsctl add-port br-int vif0 -- set Interface vif0 > external-ids:iface-id=lp0 > > +ovs-vsctl add-port br-int vif1 -- set Interface vif1 > external-ids:iface-id=lp1 > > + > > +ovn-nbctl lsp-add sw0 lp0 > > +ovn-nbctl lsp-add sw1 lp1 > > +ovn-nbctl lsp-set-addresses lp0 "50:54:00:00:00:01 10.0.0.10" > > +ovn-nbctl lsp-set-addresses lp1 "50:54:00:00:00:02 20.0.0.10" > > + > > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp0` = xup]) > > +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xup]) > > + > > +# Create two floating IPs, one for each VIF > > +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10 > > +ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10 > > + > > +# Check that the MAC_Binding entries have been properly created > > +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr0-pub" > ip="172.24.4.200" | wc -l` -gt 0]) > > +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr1-pub" > ip="172.24.4.100" | wc -l` -gt 0]) > > + > > +# Check that the GARPs went also to the external physical network > > +# Wait until at least 4 packets have arrived and copy them to a separate > file as > > +# more GARPs are expected in the capture in order to avoid race > conditions. > > +OVS_WAIT_UNTIL([test `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" > hv1/br-phys-tx.pcap | wc -l` -gt 4]) > > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | head > -n4 > hv1/br-phys-tx4.pcap > > + > > +# GARP for lp0 172.24.4.100 on lr0-pub MAC (f0:00:00:00:00:01) > > +echo > > "fffffffffffff0000000000108060001080006040001f00000000001ac180464000000000000ac180464" > > expout > > +# GARP for 172.24.4.220 on lr0-pub (f0:00:00:00:00:01) > > +echo > > "fffffffffffff0000000000108060001080006040001f00000000001ac1804dc000000000000ac1804dc" > >> expout > > +# GARP for lp1 172.24.4.200 on lr1-pub MAC (f0:00:00:00:01:01) > > +echo > > "fffffffffffff0000000010108060001080006040001f00000000101ac1804c8000000000000ac1804c8" > >> expout > > +# GARP for 172.24.4.221 on lr1-pub (f0:00:00:00:01:01) > > +echo > > "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd000000000000ac1804dd" > >> expout > > +AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout]) > > +#OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > -- > > 2.17.2 (Apple Git-113) > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks for the quick update. > > Acked-by: Han Zhou <hzhou8@ebay.com> > Acked-by: Numan Siddique <nusiddiq@redhat.com> Numan > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Dec 04, 2018 at 07:14:35PM +0100, Daniel Alvarez wrote: > 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@redhat.com> > --- > > v1->v2 > Fix VLAN tests to account for the GARPs received by the local pipeline. > Remove localnet_ofports parameter as it's not used anymore. Thanks! I applied this to master.
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 56539a891..3cd2ad718 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -2019,8 +2019,8 @@ struct garp_data { ovs_be32 ipv4; /* Ipv4 address of port. */ long long int announce_time; /* Next announcement in ms. */ 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 +2043,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 +2077,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 +2096,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 +2110,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 +2141,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), @@ -2194,7 +2184,6 @@ get_localnet_vifs_l3gwports( const struct sbrec_chassis *chassis, const struct hmap *local_datapaths, struct sset *localnet_vifs, - struct simap *localnet_ofports, struct sset *local_l3gw_ports) { for (int i = 0; i < br_int->n_ports; i++) { @@ -2209,20 +2198,14 @@ get_localnet_vifs_l3gwports( } const char *localnet = smap_get(&port_rec->external_ids, "ovn-localnet-port"); + if (localnet) { + continue; + } for (int j = 0; j < port_rec->n_interfaces; j++) { const struct ovsrec_interface *iface_rec = port_rec->interfaces[j]; if (!iface_rec->n_ofport) { continue; } - /* Get localnet port with its ofport. */ - if (localnet) { - int64_t ofport = iface_rec->ofport[0]; - if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) { - continue; - } - simap_put(localnet_ofports, localnet, ofport); - continue; - } /* Get localnet vif. */ const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); @@ -2458,7 +2441,6 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, 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 simap localnet_ofports = SIMAP_INITIALIZER(&localnet_ofports); struct shash nat_addresses; shash_init(&nat_addresses); @@ -2466,8 +2448,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, br_int, chassis, local_datapaths, - &localnet_vifs, &localnet_ofports, - &local_l3gw_ports); + &localnet_vifs, &local_l3gw_ports); get_nat_addresses_and_keys(sbrec_chassis_by_name, sbrec_port_binding_by_name, @@ -2489,8 +2470,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 +2480,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); } } @@ -2516,7 +2495,6 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name, } sset_destroy(&localnet_vifs); sset_destroy(&local_l3gw_ports); - simap_destroy(&localnet_ofports); SHASH_FOR_EACH_SAFE (iter, next, &nat_addresses) { struct lport_addresses *laddrs = iter->data; diff --git a/tests/ovn.at b/tests/ovn.at index 2db3f675a..975229af7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8761,21 +8761,7 @@ src_mac="f00000010203" dst_mac="000001010203" packet=${foo_mac}${foo1_mac}08004500001c0000000040110000${foo1_ip}${dst_ip}0035111100080000 -as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet -sleep 2 - -# ARP request packet for nexthop_ip to expect at outside1 -arp_request=ffffffffffff${gw_mac}08060001080006040001${gw_mac}${gw_ip}000000000000${nexthop_ip} -echo $arp_request >> hv3-vif1.expected -cat hv3-vif1.expected > expout -$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/vif1-tx.pcap | grep ${nexthop_ip} | uniq > hv3-vif1 -AT_CHECK([sort hv3-vif1], [0], [expout]) - -# Send ARP reply from outside1 back to the router -reply_mac="f00000010204" -arp_reply=${gw_mac}${nexthop_mac}08060001080006040002${nexthop_mac}${nexthop_ip}${gw_mac}${gw_ip} - -as hv3 ovs-appctl netdev-dummy/receive hv3-vif1 $arp_reply +# Wait for GARPs announcing gw IP to arrive OVS_WAIT_UNTIL([ test `as hv2 ovs-ofctl dump-flows br-int | grep table=66 | \ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 @@ -8806,15 +8792,17 @@ options:rxq_pcap=${pcap_file}-rx.pcap as hv1 reset_pcap_file br-ex_n2 hv1/br-ex_n2 as hv3 reset_pcap_file hv3-vif1 hv3/vif1 sleep 2 +# Take note of how many packets arrived on the VLAN switch before generating +# further traffic +n_packets=`as hv1 ovs-ofctl dump-flows br-int table=65 | grep "priority=100,reg15=0x1,metadata=0x2" | grep actions=clone | sed 's/.*n_packets=\([[0-9]]\+\),.*/\1/'` as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet sleep 2 # On hv1, the packet should not go from vlan switch pipleline to router -# pipleine +# pipeline as hv1 ovs-ofctl dump-flows br-int - AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep "priority=100,reg15=0x1,metadata=0x2" \ -| grep actions=clone | grep -v n_packets=0 | wc -l], [0], [[0 +| grep actions=clone | grep -v n_packets=$n_packets | wc -l], [0], [[0 ]]) # On hv1, table 32 check that no packet goes via the tunnel port @@ -11727,3 +11715,103 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP + +AT_SETUP([ovn -- neighbor update on same HV]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +ovn_start + +# Logical network: +# A public switch (pub) with a localnet port connected to two LRs (lr0 and lr1) +# each with a distributed gateway port. +# Two VMs: lp0 on sw0 connected to lr0 +# lp1 on sw1 connected to lr1 +# +# This test adds a floating IP to each VM so when they are bound to the same +# hypervisor, it checks that the GARP sent by ovn-controller causes the +# MAC_Binding entries to be updated properly on each logical router. +# It will also capture packets on the physical interface to make sure that the +# GARPs have been sent out to the external network as well. + +# Create logical switches +ovn-nbctl ls-add sw0 +ovn-nbctl ls-add sw1 +ovn-nbctl ls-add pub + +# Created localnet port on public switch +ovn-nbctl lsp-add pub ln-pub +ovn-nbctl lsp-set-type ln-pub localnet +ovn-nbctl lsp-set-addresses ln-pub unknown +ovn-nbctl lsp-set-options ln-pub network_name=phys + +# Create logical routers and connect them to public switch +ovn-nbctl create Logical_Router name=lr0 +ovn-nbctl create Logical_Router name=lr1 + +ovn-nbctl lrp-add lr0 lr0-pub f0:00:00:00:00:01 172.24.4.220/24 +ovn-nbctl lsp-add pub pub-lr0 -- set Logical_Switch_Port pub-lr0 \ + type=router options:router-port=lr0-pub options:nat-addresses="router" addresses="router" +ovn-nbctl lrp-add lr1 lr1-pub f0:00:00:00:01:01 172.24.4.221/24 +ovn-nbctl lsp-add pub pub-lr1 -- set Logical_Switch_Port pub-lr1 \ + type=router options:router-port=lr1-pub options:nat-addresses="router" addresses="router" + +ovn-nbctl lrp-set-gateway-chassis lr0-pub hv1 10 +ovn-nbctl lrp-set-gateway-chassis lr1-pub hv1 10 + +# Connect sw0 and sw1 to lr0 and lr1 +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 +ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 type=router \ + options:router-port=lr0-sw0 addresses="router" +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:00:00:ff:02 20.0.0.254/24 +ovn-nbctl lsp-add sw1 sw1-lr1 -- set Logical_Switch_Port sw1-lr1 type=router \ + options:router-port=lr1-sw1 addresses="router" + + +# Add SNAT rules +ovn-nbctl lr-nat-add lr0 snat 172.24.4.220 10.0.0.0/24 +ovn-nbctl lr-nat-add lr1 snat 172.24.4.221 20.0.0.0/24 + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 172.24.4.1 +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys + +ovs-vsctl add-port br-int vif0 -- set Interface vif0 external-ids:iface-id=lp0 +ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1 + +ovn-nbctl lsp-add sw0 lp0 +ovn-nbctl lsp-add sw1 lp1 +ovn-nbctl lsp-set-addresses lp0 "50:54:00:00:00:01 10.0.0.10" +ovn-nbctl lsp-set-addresses lp1 "50:54:00:00:00:02 20.0.0.10" + +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp0` = xup]) +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xup]) + +# Create two floating IPs, one for each VIF +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10 +ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10 + +# Check that the MAC_Binding entries have been properly created +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr0-pub" ip="172.24.4.200" | wc -l` -gt 0]) +OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr1-pub" ip="172.24.4.100" | wc -l` -gt 0]) + +# Check that the GARPs went also to the external physical network +# Wait until at least 4 packets have arrived and copy them to a separate file as +# more GARPs are expected in the capture in order to avoid race conditions. +OVS_WAIT_UNTIL([test `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | wc -l` -gt 4]) +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | head -n4 > hv1/br-phys-tx4.pcap + +# GARP for lp0 172.24.4.100 on lr0-pub MAC (f0:00:00:00:00:01) +echo "fffffffffffff0000000000108060001080006040001f00000000001ac180464000000000000ac180464" > expout +# GARP for 172.24.4.220 on lr0-pub (f0:00:00:00:00:01) +echo "fffffffffffff0000000000108060001080006040001f00000000001ac1804dc000000000000ac1804dc" >> expout +# GARP for lp1 172.24.4.200 on lr1-pub MAC (f0:00:00:00:01:01) +echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804c8000000000000ac1804c8" >> expout +# GARP for 172.24.4.221 on lr1-pub (f0:00:00:00:01:01) +echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd000000000000ac1804dd" >> expout +AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout]) +#OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP
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@redhat.com> --- v1->v2 Fix VLAN tests to account for the GARPs received by the local pipeline. Remove localnet_ofports parameter as it's not used anymore. ovn/controller/pinctrl.c | 80 +++++++++---------------- tests/ovn.at | 124 +++++++++++++++++++++++++++++++++------ 2 files changed, 135 insertions(+), 69 deletions(-) -- 2.17.2 (Apple Git-113)