| Message ID | 1471367661-1344-1-git-send-email-csvejend@us.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
On 16 August 2016 at 10:14, Chandra S Vejendla <csvejend@us.ibm.com> wrote: > In cases where a DNAT IP is moved to a new router or the SNAT IP is reused > with a new mac address, the NAT IPs become unreachable because the external > switches/routers have stale ARP entries. This commit > aims to fix the problem by sending GARPs for NAT IPs via locanet. There are > two parts to this patch. > > [1] Adding the datapath of the l3 gateway port to local datapaths in > ovn-controller. This will result in creation of patch ports between > br-int and the physical bridge (that provides connectivity to local network > via localnet port) and will enable gateway router to have external > connectivity > > [2] A new options key "nat-addresses" is added to the logical switch port > of > type router, the logical switch that has this port is the one that provides > connectivity to local network via localnet port. The value for the key > "nat-addresses" is the MAC address of the port followed by a list of > SNAT & DNAT IPs. When ovn-controller sees a new IP in nat-addrress option, > it sends a GARP message for the IP via the localnet port. nat-addresses > option is added to the logical switch port of type router and not to the > logical router port, because the logical switch datapath has the localnet > port. Adding nat-addresses option to the router port will involve more > changes to get to the local net port. > > Signed-off-by: Chandra Sekhar Vejendla <csvejend@us.ibm.com> > Acked-by: Ryan Moats <rmoats@us.ibm.com> > Thanks for working on this fix and the unit test. I applied this to master and 2.6. > --- > v5->v6: > Fixed style check error > Fixed some typos > Eloborated the commit meesage > v4->v5: > Fixed memory leak in ovn-northd.c by freeing laddrs > ovn/controller/binding.c | 9 ++- > ovn/controller/ovn-controller.8.xml | 20 +++++- > ovn/controller/patch.c | 8 ++- > ovn/controller/physical.c | 6 ++ > ovn/controller/pinctrl.c | 134 ++++++++++++++++++++++++++++++ > +----- > ovn/lib/ovn-util.c | 6 +- > ovn/lib/ovn-util.h | 2 +- > ovn/northd/ovn-northd.c | 14 ++++ > ovn/ovn-nb.xml | 10 +++ > ovn/ovn-sb.xml | 10 +++ > tests/ovn.at | 49 +++++++++++++ > 11 files changed, 239 insertions(+), 29 deletions(-) > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index 3073727..bd73da8 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -232,8 +232,13 @@ consider_local_datapath(struct controller_ctx *ctx, > sset_add(all_lports, binding_rec->logical_port); > add_local_datapath(local_datapaths, binding_rec); > } > - } else if (chassis_rec && binding_rec->chassis == chassis_rec > - && strcmp(binding_rec->type, "l3gateway")) { > + } else if (!strcmp(binding_rec->type, "l3gateway")) { > + const char *chassis = smap_get(&binding_rec->options, > + "l3gateway-chassis"); > + if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) { > + add_local_datapath(local_datapaths, binding_rec); > + } > + } else if (chassis_rec && binding_rec->chassis == chassis_rec) { > if (ctx->ovnsb_idl_txn) { > VLOG_INFO("Releasing lport %s from this chassis.", > binding_rec->logical_port); > diff --git a/ovn/controller/ovn-controller.8.xml > b/ovn/controller/ovn-controller.8.xml > index 345d104..559031f 100644 > --- a/ovn/controller/ovn-controller.8.xml > +++ b/ovn/controller/ovn-controller.8.xml > @@ -232,7 +232,7 @@ > <code>ovn-controller</code> to connect the integration bridge > and > another bridge to implement a <code>l2gateway</code> logical > port. > Its value is the name of the logical port with <code>type</code> > - set to <code>l3gateway</code> that the port implements. See > + set to <code>l2gateway</code> that the port implements. See > <code>external_ids:ovn-bridge-mappings</code>, above, for more > information. > </p> > @@ -246,7 +246,23 @@ > </dd> > > <dt> > - <code>external_ids:ovn-logical-patch-port</code> in the > + <code>external-ids:ovn-l3gateway-port</code> in the > <code>Port</code> > + table > + </dt> > + > + <dd> > + <p> > + This key identifies a patch port as one created by > + <code>ovn-controller</code> to implement a > <code>l3gateway</code> > + logical port. Its value is the name of the logical port with > type > + set to <code>l3gateway</code>. This patch port is similar to > + the OVN logical patch port, except that <code>l3gateway</code> > + port can only be bound to a paticular chassis. > + </p> > + </dd> > + > + <dt> > + <code>external-ids:ovn-logical-patch-port</code> in the > <code>Port</code> table > </dt> > > diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c > index 62d68d9..02bf450 100644 > --- a/ovn/controller/patch.c > +++ b/ovn/controller/patch.c > @@ -345,12 +345,14 @@ add_logical_patch_ports(struct controller_ctx *ctx, > > const struct sbrec_port_binding *binding; > SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) { > + const char *patch_port_id = "ovn-logical-patch-port"; > bool local_port = false; > if (!strcmp(binding->type, "l3gateway")) { > const char *chassis = smap_get(&binding->options, > "l3gateway-chassis"); > if (chassis && !strcmp(local_chassis_id, chassis)) { > local_port = true; > + patch_port_id = "ovn-l3gateway-port"; > } > } > > @@ -363,7 +365,7 @@ add_logical_patch_ports(struct controller_ctx *ctx, > > char *src_name = patch_port_name(local, peer); > char *dst_name = patch_port_name(peer, local); > - create_patch_port(ctx, "ovn-logical-patch-port", local, > + create_patch_port(ctx, patch_port_id, local, > br_int, src_name, br_int, dst_name, > existing_ports); > free(dst_name); > @@ -394,6 +396,7 @@ patch_run(struct controller_ctx *ctx, const struct > ovsrec_bridge *br_int, > OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) { > if (smap_get(&port->external_ids, "ovn-localnet-port") > || smap_get(&port->external_ids, "ovn-l2gateway-port") > + || smap_get(&port->external_ids, "ovn-l3gateway-port") > || smap_get(&port->external_ids, "ovn-logical-patch-port")) { > shash_add(&existing_ports, port->name, port); > } > @@ -402,7 +405,8 @@ patch_run(struct controller_ctx *ctx, const struct > ovsrec_bridge *br_int, > /* Create in the database any patch ports that should exist. Remove > from > * 'existing_ports' any patch ports that do exist in the database and > * should be there. */ > - add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, > chassis_id); > + add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, > + chassis_id); > add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports, > patched_datapaths); > > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > index 0bd228c..23e3e2c 100644 > --- a/ovn/controller/physical.c > +++ b/ovn/controller/physical.c > @@ -709,6 +709,8 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > "ovn-localnet-port"); > const char *l2gateway = smap_get(&port_rec->external_ids, > "ovn-l2gateway-port"); > + const char *l3gateway = smap_get(&port_rec->external_ids, > + "ovn-l3gateway-port"); > const char *logpatch = smap_get(&port_rec->external_ids, > "ovn-logical-patch-port"); > > @@ -735,6 +737,10 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > /* L2 gateway patch ports can be handled just like VIFs. > */ > simap_put(&new_localvif_to_ofport, l2gateway, ofport); > break; > + } else if (is_patch && l3gateway) { > + /* L3 gateway patch ports can be handled just like VIFs. > */ > + simap_put(&new_localvif_to_ofport, l3gateway, ofport); > + break; > } else if (is_patch && logpatch) { > /* Logical patch ports can be handled just like VIFs. */ > simap_put(&new_localvif_to_ofport, logpatch, ofport); > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > index 8a759cd..8bbc42d 100644 > --- a/ovn/controller/pinctrl.c > +++ b/ovn/controller/pinctrl.c > @@ -1010,10 +1010,24 @@ destroy_send_garps(void) > shash_destroy_free_data(&send_garp_data); > } > > +static void > +add_garp(const char *name, ofp_port_t ofport, > + const struct eth_addr ea, ovs_be32 ip) > +{ > + 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; > + 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, struct hmap > *local_datapaths) > + struct simap *localnet_ofports, struct hmap > *local_datapaths, > + struct shash *nat_addresses) > { > /* Find the localnet ofport to send this GARP. */ > struct local_datapath *ld > @@ -1025,9 +1039,32 @@ send_garp_update(const struct sbrec_port_binding > *binding_rec, > ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports, > > ld->localnet_port->logical_port)); > > - /* Update GARP if it exists. */ > - struct garp_data *garp = shash_find_data(&send_garp_data, > - binding_rec->logical_port); > + volatile struct garp_data *garp = NULL; > + /* Update GARP for NAT IP if it exists. */ > + if (!strcmp(binding_rec->type, "l3gateway")) { > + struct lport_addresses *laddrs = NULL; > + laddrs = shash_find_data(nat_addresses, > binding_rec->logical_port); > + if (!laddrs) { > + return; > + } > + int i; > + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > + char *name = xasprintf("%s-%s", binding_rec->logical_port, > + laddrs->ipv4_addrs[i].addr_s); > + garp = shash_find_data(&send_garp_data, name); > + if (garp) { > + garp->ofport = ofport; > + } else { > + add_garp(name, ofport, laddrs->ea, > laddrs->ipv4_addrs[i].addr); > + } > + free(name); > + } > + destroy_lport_addresses(laddrs); > + return; > + } > + > + /* Update GARP for vif if it exists. */ > + garp = shash_find_data(&send_garp_data, binding_rec->logical_port); > if (garp) { > garp->ofport = ofport; > return; > @@ -1042,13 +1079,8 @@ send_garp_update(const struct sbrec_port_binding > *binding_rec, > continue; > } > > - struct garp_data *garp = xmalloc(sizeof *garp); > - garp->ea = laddrs.ea; > - garp->ipv4 = laddrs.ipv4_addrs[0].addr; > - garp->announce_time = time_msec() + 1000; > - garp->backoff = 1; > - garp->ofport = ofport; > - shash_add(&send_garp_data, binding_rec->logical_port, garp); > + add_garp(binding_rec->logical_port, ofport, > + laddrs.ea, laddrs.ipv4_addrs[0].addr); > > destroy_lport_addresses(&laddrs); > break; > @@ -1107,14 +1139,15 @@ send_garp(struct garp_data *garp, long long int > current_time) > return garp->announce_time; > } > > -/* Get localnet vifs, and ofport for localnet patch ports. */ > +/* Get localnet vifs, local l3gw ports and ofport for localnet patch > ports. */ > static void > -get_localnet_vifs(const struct ovsrec_bridge *br_int, > +get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int, > const char *this_chassis_id, > const struct lport_index *lports, > struct hmap *local_datapaths, > struct sset *localnet_vifs, > - struct simap *localnet_ofports) > + struct simap *localnet_ofports, > + struct sset *local_l3gw_ports) > { > for (int i = 0; i < br_int->n_ports; i++) { > const struct ovsrec_port *port_rec = br_int->ports[i]; > @@ -1128,6 +1161,8 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int, > } > const char *localnet = smap_get(&port_rec->external_ids, > "ovn-localnet-port"); > + const char *l3_gateway_port = smap_get(&port_rec->external_ids, > + "ovn-l3gateway-port"); > 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) { > @@ -1141,6 +1176,10 @@ get_localnet_vifs(const struct ovsrec_bridge > *br_int, > simap_put(localnet_ofports, localnet, ofport); > continue; > } > + if (l3_gateway_port) { > + sset_add(local_l3gw_ports, l3_gateway_port); > + continue; > + } > const char *iface_id = smap_get(&iface_rec->external_ids, > "iface-id"); > if (!iface_id) { > @@ -1162,6 +1201,41 @@ get_localnet_vifs(const struct ovsrec_bridge > *br_int, > } > > static void > +get_nat_addresses_and_keys(struct sset *nat_address_keys, > + struct sset *local_l3gw_ports, > + const struct lport_index *lports, > + struct shash *nat_addresses) > +{ > + const char *gw_port; > + SSET_FOR_EACH(gw_port, local_l3gw_ports) { > + const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, > + > gw_port); > + if (!pb) { > + continue; > + } > + const char *nat_addresses_options = smap_get(&pb->options, > + "nat-addresses"); > + if (!nat_addresses_options) { > + continue; > + } > + > + struct lport_addresses *laddrs = xmalloc(sizeof *laddrs); > + if (!extract_lsp_addresses(nat_addresses_options, laddrs)) { > + free(laddrs); > + continue; > + } > + int i; > + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { > + char *name = xasprintf("%s-%s", pb->logical_port, > + laddrs->ipv4_addrs[i].addr_s); > + sset_add(nat_address_keys, name); > + free(name); > + } > + shash_add(nat_addresses, pb->logical_port, laddrs); > + } > +} > + > +static void > send_garp_wait(void) > { > poll_timer_wait_until(send_garp_time); > @@ -1173,15 +1247,23 @@ send_garp_run(const struct ovsrec_bridge *br_int, > const char *chassis_id, > struct hmap *local_datapaths) > { > 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; > > - get_localnet_vifs(br_int, chassis_id, lports, local_datapaths, > - &localnet_vifs, &localnet_ofports); > + shash_init(&nat_addresses); > > - /* For deleted ports, remove from send_garp_data. */ > + get_localnet_vifs_l3gwports(br_int, chassis_id, lports, > local_datapaths, > + &localnet_vifs, &localnet_ofports, > &local_l3gw_ports); > + > + get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports, > + &nat_addresses); > + /* For deleted ports and deleted nat ips, remove from send_garp_data. > */ > struct shash_node *iter, *next; > SHASH_FOR_EACH_SAFE (iter, next, &send_garp_data) { > - if (!sset_contains(&localnet_vifs, iter->name)) { > + if (!sset_contains(&localnet_vifs, iter->name) && > + !sset_contains(&nat_ip_keys, iter->name)) { > send_garp_delete(iter->name); > } > } > @@ -1192,7 +1274,19 @@ send_garp_run(const struct ovsrec_bridge *br_int, > const char *chassis_id, > const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, > > iface_id); > if (pb) { > - send_garp_update(pb, &localnet_ofports, local_datapaths); > + send_garp_update(pb, &localnet_ofports, local_datapaths, > + &nat_addresses); > + } > + } > + > + /* Update send_garp_data for nat-addresses. */ > + const char *gw_port; > + SSET_FOR_EACH (gw_port, &local_l3gw_ports) { > + const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, > + gw_port); > + if (pb) { > + send_garp_update(pb, &localnet_ofports, local_datapaths, > + &nat_addresses); > } > } > > @@ -1206,7 +1300,9 @@ send_garp_run(const struct ovsrec_bridge *br_int, > const char *chassis_id, > } > } > sset_destroy(&localnet_vifs); > + sset_destroy(&local_l3gw_ports); > simap_destroy(&localnet_ofports); > + shash_destroy_free_data(&nat_addresses); > } > > static void > diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c > index ff0d204..94feadb 100644 > --- a/ovn/lib/ovn-util.c > +++ b/ovn/lib/ovn-util.c > @@ -71,13 +71,13 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, > struct in6_addr addr, > * > * The caller must call destroy_lport_addresses(). */ > bool > -extract_lsp_addresses(char *address, struct lport_addresses *laddrs) > +extract_lsp_addresses(const char *address, struct lport_addresses *laddrs) > { > memset(laddrs, 0, sizeof *laddrs); > > - char *buf = address; > + const char *buf = address; > int buf_index = 0; > - char *buf_end = buf + strlen(address); > + const char *buf_end = buf + strlen(address); > if (!ovs_scan_len(buf, &buf_index, ETH_ADDR_SCAN_FMT, > ETH_ADDR_SCAN_ARGS(laddrs->ea))) { > laddrs->ea = eth_addr_zero; > diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h > index c2ac471..22bb06d 100644 > --- a/ovn/lib/ovn-util.h > +++ b/ovn/lib/ovn-util.h > @@ -53,7 +53,7 @@ struct lport_addresses { > }; > > > -bool extract_lsp_addresses(char *address, struct lport_addresses *); > +bool extract_lsp_addresses(const char *address, struct lport_addresses *); > bool extract_lrp_networks(const struct nbrec_logical_router_port *, > struct lport_addresses *); > void destroy_lport_addresses(struct lport_addresses *); > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 8901f7b..30dfa6c 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1195,6 +1195,20 @@ ovn_port_update_sbrec(const struct ovn_port *op) > if (chassis) { > smap_add(&new, "l3gateway-chassis", chassis); > } > + > + const char *nat_addresses = smap_get(&op->nbsp->options, > + "nat-addresses"); > + if (nat_addresses) { > + struct lport_addresses laddrs; > + if (!extract_lsp_addresses(nat_addresses, &laddrs)) { > + static struct vlog_rate_limit rl = > + VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Error extracting nat-addresses."); > + } else { > + smap_add(&new, "nat-addresses", nat_addresses); > + destroy_lport_addresses(&laddrs); > + } > + } > sbrec_port_binding_set_options(op->sb, &new); > smap_destroy(&new); > } > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml > index 8fb5244..5719e74 100644 > --- a/ovn/ovn-nb.xml > +++ b/ovn/ovn-nb.xml > @@ -225,6 +225,16 @@ > table="Logical_Router_Port"/> to which this logical switch port > is > connected. > </column> > + > + <column name="options" key="nat-addresses"> > + MAC address of the <code>router-port</code> followed by a list > of > + SNAT and DNAT IP addresses. This is used to send gratuitous > ARPs for > + SNAT and DNAT IP addresses via <code>localnet</code> and is > valid for > + only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 > 158.36.44.22 > + 158.36.44.24</code>. This would result in generation of > gratuitous > + ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC > + address of 80:fa:5b:06:72:b7. > + </column> > </group> > > <group title="Options for localnet ports"> > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml > index 1da309c..e119249 100644 > --- a/ovn/ovn-sb.xml > +++ b/ovn/ovn-sb.xml > @@ -1756,6 +1756,16 @@ tcp.flags = RST; > <column name="options" key="l3gateway-chassis"> > The <code>chassis</code> in which the port resides. > </column> > + > + <column name="options" key="nat-addresses"> > + MAC address of the <code>l3gateway</code> port followed by a > list of > + SNAT and DNAT IP addresses. This is used to send gratuitous > ARPs for > + SNAT and DNAT IP addresses via <code>localnet</code> and is > valid for > + only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 > 158.36.44.22 > + 158.36.44.24</code>. This would result in generation of > gratuitous > + ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC > + address of 80:fa:5b:06:72:b7. > + </column> > </group> > > <group title="Localnet Options"> > diff --git a/tests/ovn.at b/tests/ovn.at > index 6232214..216bb07 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -4916,3 +4916,52 @@ cat packets > OVN_CLEANUP([hv1]) > > AT_CLEANUP > + > +AT_SETUP([ovn -- send gratuitous arp for nat ips in localnet]) > +AT_KEYWORDS([ovn]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +ovn_start > +# Create logical switch > +ovn-nbctl ls-add ls0 > +# Create gateway router > +ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > +# Add router port to gateway router > +ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.1/24 > +ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \ > + type=router options:router-port=lrp0-rp addresses='"f0:00:00:00:00:01" > ' > +# Add nat-address option > +ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 > nat-addresses="f0:00:00:00:00:01 192.168.0.2" > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl \ > + -- add-br br-phys \ > + -- add-br br-eth0 > + > +ovn_attach n1 br-phys 192.168.0.1 > + > +AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappin > gs=physnet1:br-eth0]) > +AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif > options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif- > rx.pcap]) > + > +# Create a localnet port. > +AT_CHECK([ovn-nbctl lsp-add ls0 ln_port]) > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > + > + > +# Wait for packet to be received. > +OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50]) > +trim_zeros() { > + sed 's/\(00\)\{1,\}$//' > +} > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | > trim_zeros > packets > +expected="fffffffffffff0000000000108060001080006040001f0000 > 0000001c0a80002000000000000c0a80002" > +echo $expected > expout > +AT_CHECK([sort packets], [0], [expout]) > +cat packets > + > +OVN_CLEANUP([hv1]) > + > +AT_CLEANUP > -- > 2.6.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 3073727..bd73da8 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -232,8 +232,13 @@ consider_local_datapath(struct controller_ctx *ctx, sset_add(all_lports, binding_rec->logical_port); add_local_datapath(local_datapaths, binding_rec); } - } else if (chassis_rec && binding_rec->chassis == chassis_rec - && strcmp(binding_rec->type, "l3gateway")) { + } else if (!strcmp(binding_rec->type, "l3gateway")) { + const char *chassis = smap_get(&binding_rec->options, + "l3gateway-chassis"); + if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) { + add_local_datapath(local_datapaths, binding_rec); + } + } else if (chassis_rec && binding_rec->chassis == chassis_rec) { if (ctx->ovnsb_idl_txn) { VLOG_INFO("Releasing lport %s from this chassis.", binding_rec->logical_port); diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml index 345d104..559031f 100644 --- a/ovn/controller/ovn-controller.8.xml +++ b/ovn/controller/ovn-controller.8.xml @@ -232,7 +232,7 @@ <code>ovn-controller</code> to connect the integration bridge and another bridge to implement a <code>l2gateway</code> logical port. Its value is the name of the logical port with <code>type</code> - set to <code>l3gateway</code> that the port implements. See + set to <code>l2gateway</code> that the port implements. See <code>external_ids:ovn-bridge-mappings</code>, above, for more information. </p> @@ -246,7 +246,23 @@ </dd> <dt> - <code>external_ids:ovn-logical-patch-port</code> in the + <code>external-ids:ovn-l3gateway-port</code> in the <code>Port</code> + table + </dt> + + <dd> + <p> + This key identifies a patch port as one created by + <code>ovn-controller</code> to implement a <code>l3gateway</code> + logical port. Its value is the name of the logical port with type + set to <code>l3gateway</code>. This patch port is similar to + the OVN logical patch port, except that <code>l3gateway</code> + port can only be bound to a paticular chassis. + </p> + </dd> + + <dt> + <code>external-ids:ovn-logical-patch-port</code> in the <code>Port</code> table </dt> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c index 62d68d9..02bf450 100644 --- a/ovn/controller/patch.c +++ b/ovn/controller/patch.c @@ -345,12 +345,14 @@ add_logical_patch_ports(struct controller_ctx *ctx, const struct sbrec_port_binding *binding; SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) { + const char *patch_port_id = "ovn-logical-patch-port"; bool local_port = false; if (!strcmp(binding->type, "l3gateway")) { const char *chassis = smap_get(&binding->options, "l3gateway-chassis"); if (chassis && !strcmp(local_chassis_id, chassis)) { local_port = true; + patch_port_id = "ovn-l3gateway-port"; } } @@ -363,7 +365,7 @@ add_logical_patch_ports(struct controller_ctx *ctx, char *src_name = patch_port_name(local, peer); char *dst_name = patch_port_name(peer, local); - create_patch_port(ctx, "ovn-logical-patch-port", local, + create_patch_port(ctx, patch_port_id, local, br_int, src_name, br_int, dst_name, existing_ports); free(dst_name); @@ -394,6 +396,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) { if (smap_get(&port->external_ids, "ovn-localnet-port") || smap_get(&port->external_ids, "ovn-l2gateway-port") + || smap_get(&port->external_ids, "ovn-l3gateway-port") || smap_get(&port->external_ids, "ovn-logical-patch-port")) { shash_add(&existing_ports, port->name, port); } @@ -402,7 +405,8 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, /* Create in the database any patch ports that should exist. Remove from * 'existing_ports' any patch ports that do exist in the database and * should be there. */ - add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, chassis_id); + add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, + chassis_id); add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports, patched_datapaths); diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index 0bd228c..23e3e2c 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -709,6 +709,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, "ovn-localnet-port"); const char *l2gateway = smap_get(&port_rec->external_ids, "ovn-l2gateway-port"); + const char *l3gateway = smap_get(&port_rec->external_ids, + "ovn-l3gateway-port"); const char *logpatch = smap_get(&port_rec->external_ids, "ovn-logical-patch-port"); @@ -735,6 +737,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* L2 gateway patch ports can be handled just like VIFs. */ simap_put(&new_localvif_to_ofport, l2gateway, ofport); break; + } else if (is_patch && l3gateway) { + /* L3 gateway patch ports can be handled just like VIFs. */ + simap_put(&new_localvif_to_ofport, l3gateway, ofport); + break; } else if (is_patch && logpatch) { /* Logical patch ports can be handled just like VIFs. */ simap_put(&new_localvif_to_ofport, logpatch, ofport); diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 8a759cd..8bbc42d 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -1010,10 +1010,24 @@ destroy_send_garps(void) shash_destroy_free_data(&send_garp_data); } +static void +add_garp(const char *name, ofp_port_t ofport, + const struct eth_addr ea, ovs_be32 ip) +{ + 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; + 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, struct hmap *local_datapaths) + struct simap *localnet_ofports, struct hmap *local_datapaths, + struct shash *nat_addresses) { /* Find the localnet ofport to send this GARP. */ struct local_datapath *ld @@ -1025,9 +1039,32 @@ send_garp_update(const struct sbrec_port_binding *binding_rec, ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports, ld->localnet_port->logical_port)); - /* Update GARP if it exists. */ - struct garp_data *garp = shash_find_data(&send_garp_data, - binding_rec->logical_port); + volatile struct garp_data *garp = NULL; + /* Update GARP for NAT IP if it exists. */ + if (!strcmp(binding_rec->type, "l3gateway")) { + struct lport_addresses *laddrs = NULL; + laddrs = shash_find_data(nat_addresses, binding_rec->logical_port); + if (!laddrs) { + return; + } + int i; + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { + char *name = xasprintf("%s-%s", binding_rec->logical_port, + laddrs->ipv4_addrs[i].addr_s); + garp = shash_find_data(&send_garp_data, name); + if (garp) { + garp->ofport = ofport; + } else { + add_garp(name, ofport, laddrs->ea, laddrs->ipv4_addrs[i].addr); + } + free(name); + } + destroy_lport_addresses(laddrs); + return; + } + + /* Update GARP for vif if it exists. */ + garp = shash_find_data(&send_garp_data, binding_rec->logical_port); if (garp) { garp->ofport = ofport; return; @@ -1042,13 +1079,8 @@ send_garp_update(const struct sbrec_port_binding *binding_rec, continue; } - struct garp_data *garp = xmalloc(sizeof *garp); - garp->ea = laddrs.ea; - garp->ipv4 = laddrs.ipv4_addrs[0].addr; - garp->announce_time = time_msec() + 1000; - garp->backoff = 1; - garp->ofport = ofport; - shash_add(&send_garp_data, binding_rec->logical_port, garp); + add_garp(binding_rec->logical_port, ofport, + laddrs.ea, laddrs.ipv4_addrs[0].addr); destroy_lport_addresses(&laddrs); break; @@ -1107,14 +1139,15 @@ send_garp(struct garp_data *garp, long long int current_time) return garp->announce_time; } -/* Get localnet vifs, and ofport for localnet patch ports. */ +/* Get localnet vifs, local l3gw ports and ofport for localnet patch ports. */ static void -get_localnet_vifs(const struct ovsrec_bridge *br_int, +get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int, const char *this_chassis_id, const struct lport_index *lports, struct hmap *local_datapaths, struct sset *localnet_vifs, - struct simap *localnet_ofports) + struct simap *localnet_ofports, + struct sset *local_l3gw_ports) { for (int i = 0; i < br_int->n_ports; i++) { const struct ovsrec_port *port_rec = br_int->ports[i]; @@ -1128,6 +1161,8 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int, } const char *localnet = smap_get(&port_rec->external_ids, "ovn-localnet-port"); + const char *l3_gateway_port = smap_get(&port_rec->external_ids, + "ovn-l3gateway-port"); 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) { @@ -1141,6 +1176,10 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int, simap_put(localnet_ofports, localnet, ofport); continue; } + if (l3_gateway_port) { + sset_add(local_l3gw_ports, l3_gateway_port); + continue; + } const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); if (!iface_id) { @@ -1162,6 +1201,41 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int, } static void +get_nat_addresses_and_keys(struct sset *nat_address_keys, + struct sset *local_l3gw_ports, + const struct lport_index *lports, + struct shash *nat_addresses) +{ + const char *gw_port; + SSET_FOR_EACH(gw_port, local_l3gw_ports) { + const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, + gw_port); + if (!pb) { + continue; + } + const char *nat_addresses_options = smap_get(&pb->options, + "nat-addresses"); + if (!nat_addresses_options) { + continue; + } + + struct lport_addresses *laddrs = xmalloc(sizeof *laddrs); + if (!extract_lsp_addresses(nat_addresses_options, laddrs)) { + free(laddrs); + continue; + } + int i; + for (i = 0; i < laddrs->n_ipv4_addrs; i++) { + char *name = xasprintf("%s-%s", pb->logical_port, + laddrs->ipv4_addrs[i].addr_s); + sset_add(nat_address_keys, name); + free(name); + } + shash_add(nat_addresses, pb->logical_port, laddrs); + } +} + +static void send_garp_wait(void) { poll_timer_wait_until(send_garp_time); @@ -1173,15 +1247,23 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id, struct hmap *local_datapaths) { 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; - get_localnet_vifs(br_int, chassis_id, lports, local_datapaths, - &localnet_vifs, &localnet_ofports); + shash_init(&nat_addresses); - /* For deleted ports, remove from send_garp_data. */ + get_localnet_vifs_l3gwports(br_int, chassis_id, lports, local_datapaths, + &localnet_vifs, &localnet_ofports, &local_l3gw_ports); + + get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports, + &nat_addresses); + /* For deleted ports and deleted nat ips, remove from send_garp_data. */ struct shash_node *iter, *next; SHASH_FOR_EACH_SAFE (iter, next, &send_garp_data) { - if (!sset_contains(&localnet_vifs, iter->name)) { + if (!sset_contains(&localnet_vifs, iter->name) && + !sset_contains(&nat_ip_keys, iter->name)) { send_garp_delete(iter->name); } } @@ -1192,7 +1274,19 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id, const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, iface_id); if (pb) { - send_garp_update(pb, &localnet_ofports, local_datapaths); + send_garp_update(pb, &localnet_ofports, local_datapaths, + &nat_addresses); + } + } + + /* Update send_garp_data for nat-addresses. */ + const char *gw_port; + SSET_FOR_EACH (gw_port, &local_l3gw_ports) { + const struct sbrec_port_binding *pb = lport_lookup_by_name(lports, + gw_port); + if (pb) { + send_garp_update(pb, &localnet_ofports, local_datapaths, + &nat_addresses); } } @@ -1206,7 +1300,9 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id, } } sset_destroy(&localnet_vifs); + sset_destroy(&local_l3gw_ports); simap_destroy(&localnet_ofports); + shash_destroy_free_data(&nat_addresses); } static void diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c index ff0d204..94feadb 100644 --- a/ovn/lib/ovn-util.c +++ b/ovn/lib/ovn-util.c @@ -71,13 +71,13 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr, * * The caller must call destroy_lport_addresses(). */ bool -extract_lsp_addresses(char *address, struct lport_addresses *laddrs) +extract_lsp_addresses(const char *address, struct lport_addresses *laddrs) { memset(laddrs, 0, sizeof *laddrs); - char *buf = address; + const char *buf = address; int buf_index = 0; - char *buf_end = buf + strlen(address); + const char *buf_end = buf + strlen(address); if (!ovs_scan_len(buf, &buf_index, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(laddrs->ea))) { laddrs->ea = eth_addr_zero; diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h index c2ac471..22bb06d 100644 --- a/ovn/lib/ovn-util.h +++ b/ovn/lib/ovn-util.h @@ -53,7 +53,7 @@ struct lport_addresses { }; -bool extract_lsp_addresses(char *address, struct lport_addresses *); +bool extract_lsp_addresses(const char *address, struct lport_addresses *); bool extract_lrp_networks(const struct nbrec_logical_router_port *, struct lport_addresses *); void destroy_lport_addresses(struct lport_addresses *); diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 8901f7b..30dfa6c 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1195,6 +1195,20 @@ ovn_port_update_sbrec(const struct ovn_port *op) if (chassis) { smap_add(&new, "l3gateway-chassis", chassis); } + + const char *nat_addresses = smap_get(&op->nbsp->options, + "nat-addresses"); + if (nat_addresses) { + struct lport_addresses laddrs; + if (!extract_lsp_addresses(nat_addresses, &laddrs)) { + static struct vlog_rate_limit rl = + VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "Error extracting nat-addresses."); + } else { + smap_add(&new, "nat-addresses", nat_addresses); + destroy_lport_addresses(&laddrs); + } + } sbrec_port_binding_set_options(op->sb, &new); smap_destroy(&new); } diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index 8fb5244..5719e74 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -225,6 +225,16 @@ table="Logical_Router_Port"/> to which this logical switch port is connected. </column> + + <column name="options" key="nat-addresses"> + MAC address of the <code>router-port</code> followed by a list of + SNAT and DNAT IP addresses. This is used to send gratuitous ARPs for + SNAT and DNAT IP addresses via <code>localnet</code> and is valid for + only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 158.36.44.22 + 158.36.44.24</code>. This would result in generation of gratuitous + ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC + address of 80:fa:5b:06:72:b7. + </column> </group> <group title="Options for localnet ports"> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 1da309c..e119249 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -1756,6 +1756,16 @@ tcp.flags = RST; <column name="options" key="l3gateway-chassis"> The <code>chassis</code> in which the port resides. </column> + + <column name="options" key="nat-addresses"> + MAC address of the <code>l3gateway</code> port followed by a list of + SNAT and DNAT IP addresses. This is used to send gratuitous ARPs for + SNAT and DNAT IP addresses via <code>localnet</code> and is valid for + only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 158.36.44.22 + 158.36.44.24</code>. This would result in generation of gratuitous + ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC + address of 80:fa:5b:06:72:b7. + </column> </group> <group title="Localnet Options"> diff --git a/tests/ovn.at b/tests/ovn.at index 6232214..216bb07 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -4916,3 +4916,52 @@ cat packets OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([ovn -- send gratuitous arp for nat ips in localnet]) +AT_KEYWORDS([ovn]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +ovn_start +# Create logical switch +ovn-nbctl ls-add ls0 +# Create gateway router +ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 +# Add router port to gateway router +ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.1/24 +ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \ + type=router options:router-port=lrp0-rp addresses='"f0:00:00:00:00:01"' +# Add nat-address option +ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="f0:00:00:00:00:01 192.168.0.2" + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl \ + -- add-br br-phys \ + -- add-br br-eth0 + +ovn_attach n1 br-phys 192.168.0.1 + +AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=physnet1:br-eth0]) +AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap]) + +# Create a localnet port. +AT_CHECK([ovn-nbctl lsp-add ls0 ln_port]) +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) + + +# Wait for packet to be received. +OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50]) +trim_zeros() { + sed 's/\(00\)\{1,\}$//' +} +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros > packets +expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002" +echo $expected > expout +AT_CHECK([sort packets], [0], [expout]) +cat packets + +OVN_CLEANUP([hv1]) + +AT_CLEANUP