On Wed, Jul 5, 2017 at 9:45 AM, Miguel Angel Ajo <majop...@redhat.com> wrote: > From: Venkata Anil Kommaddi <vkomm...@redhat.com> > > This patch extends gratuitous ARP support for NAT addresses so that it > applies to centralized NAT rules on a HA router. > Gratuitous ARP packets for centralized NAT rules on a HA router > are only generated on the active gateway chassis. > > Signed-off-by: Anil Venkata <vkomm...@redhat.com> > --- > ovn/controller/ovn-controller.c | 3 +- > ovn/controller/pinctrl.c | 45 ++++++++++++--- > ovn/controller/pinctrl.h | 5 +- > tests/ovn.at | 121 > ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 165 insertions(+), 9 deletions(-) > > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c > index 1418a15..0360c4c 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -655,7 +655,8 @@ main(int argc, char *argv[]) > enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int, > &pending_ct_zones); > > - pinctrl_run(&ctx, &lports, br_int, chassis, &local_datapaths); > + pinctrl_run(&ctx, &lports, br_int, chassis, &chassis_index, > + &local_datapaths, &active_tunnels); > update_ct_zones(&local_lports, &local_datapaths, &ct_zones, > ct_zone_bitmap, &pending_ct_zones); > if (ctx.ovs_idl_txn) { > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > index 660233a..b7bcee6 100644 > --- a/ovn/controller/pinctrl.c > +++ b/ovn/controller/pinctrl.c > @@ -23,6 +23,7 @@ > #include "dirs.h" > #include "dp-packet.h" > #include "flow.h" > +#include "gchassis.h" > #include "lport.h" > #include "nx-match.h" > #include "ovn-controller.h" > @@ -72,7 +73,9 @@ static void send_garp_wait(void); > static void send_garp_run(const struct ovsrec_bridge *, > const struct sbrec_chassis *, > const struct lport_index *lports, > - struct hmap *local_datapaths); > + const struct chassis_index *chassis_index, > + struct hmap *local_datapaths, > + struct sset *active_tunnels); > static void pinctrl_handle_nd_na(const struct flow *ip_flow, > const struct match *md, > struct ofpbuf *userdata); > @@ -1016,7 +1019,9 @@ void > pinctrl_run(struct controller_ctx *ctx, const struct lport_index *lports, > const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *chassis, > - struct hmap *local_datapaths) > + const struct chassis_index *chassis_index, > + struct hmap *local_datapaths, > + struct sset *active_tunnels) > { > char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name); > if (strcmp(target, rconn_get_target(swconn))) { > @@ -1051,7 +1056,8 @@ pinctrl_run(struct controller_ctx *ctx, const struct > lport_index *lports, > } > > run_put_mac_bindings(ctx, lports); > - send_garp_run(br_int, chassis, lports, local_datapaths); > + send_garp_run(br_int, chassis, lports, chassis_index, local_datapaths, > + active_tunnels); > } > > void > @@ -1490,11 +1496,26 @@ get_localnet_vifs_l3gwports(const struct > ovsrec_bridge *br_int, > static bool > pinctrl_is_chassis_resident(const struct lport_index *lports, > const struct sbrec_chassis *chassis, > + const struct chassis_index *chassis_index, > + struct sset *active_tunnels, > const char *port_name) > { > const struct sbrec_port_binding *pb > = lport_lookup_by_name(lports, port_name); > - return pb && pb->chassis && pb->chassis == chassis; > + if (!pb || !pb->chassis) { > + return false; > + } > + if (strcmp(pb->type, "chassisredirect")) { > + return pb->chassis == chassis; > + } else { > + struct ovs_list *gateway_chassis = > + gateway_chassis_get_ordered(pb, chassis_index); > + bool active = gateway_chassis_is_active(gateway_chassis, > + chassis, > + active_tunnels); > + gateway_chassis_destroy(gateway_chassis); > + return active;
This seems to be the core of this patch, but why is this change necessary? If we update the port binding chassis column to always be the active gateway_chassis, the original code would have the same result, right? What am I missing? > + } > } > > /* Extracts the mac, IPv4 and IPv6 addresses, and logical port from > @@ -1569,13 +1590,16 @@ consider_nat_address(const char *nat_address, > struct sset *nat_address_keys, > const struct lport_index *lports, > const struct sbrec_chassis *chassis, > + const struct chassis_index *chassis_index, > + struct sset *active_tunnels, > struct shash *nat_addresses) > { > struct lport_addresses *laddrs = xmalloc(sizeof *laddrs); > char *lport = NULL; > if (!extract_addresses_with_port(nat_address, laddrs, &lport) > || (!lport && !strcmp(pb->type, "patch")) > - || (lport && !pinctrl_is_chassis_resident(lports, chassis, lport))) { > + || (lport && !pinctrl_is_chassis_resident( > + lports, chassis, chassis_index, active_tunnels, lport))) { > destroy_lport_addresses(laddrs); > free(laddrs); > free(lport); > @@ -1598,6 +1622,8 @@ get_nat_addresses_and_keys(struct sset > *nat_address_keys, > struct sset *local_l3gw_ports, > const struct lport_index *lports, > const struct sbrec_chassis *chassis, > + const struct chassis_index *chassis_index, > + struct sset *active_tunnels, > struct shash *nat_addresses) > { > const char *gw_port; > @@ -1612,6 +1638,7 @@ get_nat_addresses_and_keys(struct sset > *nat_address_keys, > for (int i = 0; i < pb->n_nat_addresses; i++) { > consider_nat_address(pb->nat_addresses[i], pb, > nat_address_keys, lports, chassis, > + chassis_index, active_tunnels, > nat_addresses); > } > } else { > @@ -1622,6 +1649,7 @@ get_nat_addresses_and_keys(struct sset > *nat_address_keys, > if (nat_addresses_options) { > consider_nat_address(nat_addresses_options, pb, > nat_address_keys, lports, chassis, > + chassis_index, active_tunnels, > nat_addresses); > } > } > @@ -1638,7 +1666,9 @@ static void > send_garp_run(const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *chassis, > const struct lport_index *lports, > - struct hmap *local_datapaths) > + const struct chassis_index *chassis_index, > + struct hmap *local_datapaths, > + struct sset *active_tunnels) > { > struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs); > struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports); > @@ -1652,7 +1682,8 @@ send_garp_run(const struct ovsrec_bridge *br_int, > &localnet_vifs, &localnet_ofports, &local_l3gw_ports); > > get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports, > - chassis, &nat_addresses); > + chassis, chassis_index, active_tunnels, > + &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) { > diff --git a/ovn/controller/pinctrl.h b/ovn/controller/pinctrl.h > index 230580b..913c170 100644 > --- a/ovn/controller/pinctrl.h > +++ b/ovn/controller/pinctrl.h > @@ -19,8 +19,10 @@ > > #include <stdint.h> > > +#include "lib/sset.h" > #include "openvswitch/meta-flow.h" > > +struct chassis_index; > struct controller_ctx; > struct hmap; > struct lport_index; > @@ -30,7 +32,8 @@ struct sbrec_chassis; > void pinctrl_init(void); > void pinctrl_run(struct controller_ctx *, const struct lport_index *, > const struct ovsrec_bridge *, const struct sbrec_chassis *, > - struct hmap *local_datapaths); > + const struct chassis_index *, struct hmap *local_datapaths, > + struct sset *active_tunnels); > void pinctrl_wait(struct controller_ctx *); > void pinctrl_destroy(void); > > diff --git a/tests/ovn.at b/tests/ovn.at > index 398afee..4282328 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -8000,3 +8000,124 @@ OVN_CLEANUP([gw1],[gw2],[hv1],[hv2]) > > AT_CLEANUP > > +AT_SETUP([ovn -- send gratuitous ARP for NAT rules on HA distributed router]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +ovn_start > +ovn-nbctl ls-add ls0 > +ovn-nbctl ls-add ls1 > +ovn-nbctl create Logical_Router name=lr0 > +ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.100/24 > + > +ovn-nbctl --id=@gc0 create Gateway_Chassis \ > + name=outside_gw1 chassis_name=hv2 priority=10 -- \ > + --id=@gc1 create Gateway_Chassis \ > + name=outside_gw2 chassis_name=hv3 priority=1 -- \ > + set Logical_Router_Port lrp0 'gateway_chassis=[@gc0,@gc1]' > + > +ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \ > + type=router options:router-port=lrp0 addresses="router" > +ovn-nbctl lrp-add lr0 lrp1 f0:00:00:00:00:02 10.0.0.1/24 > +ovn-nbctl lsp-add ls1 lrp1-rp -- set Logical_Switch_Port lrp1-rp \ > + type=router options:router-port=lrp1 addresses="router" > + > +# Add NAT rules > +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 192.168.0.100 10.0.0.0/24]) > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +AT_CHECK([ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > +AT_CHECK([ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif > options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap]) > + > +sim_add hv2 > +as hv2 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.2 > +AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > + > +sim_add hv3 > +as hv3 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.3 > +AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > + > +# 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 earlier changes to take effect > +AT_CHECK([ovn-nbctl --timeout=3 --wait=sb sync], [0], [ignore]) > + > +reset_pcap_file() { > + local iface=$1 > + local pcap_file=$2 > + ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ > +options:rxq_pcap=dummy-rx.pcap > + rm -f ${pcap_file}*.pcap > + ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ > +options:rxq_pcap=${pcap_file}-rx.pcap > +} > + > +as hv1 reset_pcap_file snoopvif hv1/snoopvif > +as hv2 reset_pcap_file br-phys_n1 hv2/br-phys_n1 > +as hv3 reset_pcap_file br-phys_n1 hv3/br-phys_n1 > +# add nat-addresses option > +ovn-nbctl --wait=sb lsp-set-options lrp0-rp router-port=lrp0 > nat-addresses="router" > + > +# Wait for packets to be received through hv2. > +OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) > +trim_zeros() { > + sed 's/\(00\)\{1,\}$//' > +} > + > +garp="fffffffffffff0000000000108060001080006040001f00000000001c0a80064000000000000c0a80064" > +echo $garp >> expout > +echo $garp >> expout > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | > trim_zeros > hv1_snoop_tx > +echo "packets on hv1-snoopvif:" > +cat hv1_snoop_tx > +AT_CHECK([sort hv1_snoop_tx], [0], [expout]) > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-phys_n1-tx.pcap | > trim_zeros > hv2_br_phys_tx > +echo "packets on hv2 br-phys tx" > +cat hv2_br_phys_tx > +AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], [expout]) > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | > trim_zeros > hv3_br_phys_tx > +echo "packets on hv3 br-phys tx" > +cat hv3_br_phys_tx > +AT_CHECK([grep $garp hv3_br_phys_tx | sort], [0], []) > + > + > +# at this point, we invert the priority of the gw chassis between hv2 and hv3 > + > +ovn-nbctl --wait=hv \ > + --id=@gc0 create Gateway_Chassis \ > + name=outside_gw1 chassis_name=hv2 priority=1 -- \ > + --id=@gc1 create Gateway_Chassis \ > + name=outside_gw2 chassis_name=hv3 priority=10 -- \ > + set Logical_Router_Port lrp0 'gateway_chassis=[@gc0,@gc1]' > + > + > +as hv1 reset_pcap_file snoopvif hv1/snoopvif > +as hv2 reset_pcap_file br-phys_n1 hv2/br-phys_n1 > +as hv3 reset_pcap_file br-phys_n1 hv3/br-phys_n1 > + > +# Wait for packets to be received. > +OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) > +trim_zeros() { > + sed 's/\(00\)\{1,\}$//' > +} > + > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | > trim_zeros > hv1_snoopvif_tx > +AT_CHECK([sort hv1_snoopvif_tx], [0], [expout]) > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | > trim_zeros > hv3_br_phys_tx > +AT_CHECK([grep $garp hv3_br_phys_tx | sort], [0], [expout]) > +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-phys_n1-tx.pcap | > trim_zeros > hv2_br_phys_tx > +AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], []) > +OVN_CLEANUP([hv1],[hv2],[hv3]) > + > +AT_CLEANUP > + > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev -- Russell Bryant _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev