On Mon, Jul 1, 2019 at 9:45 AM <nusid...@redhat.com> wrote: > > From: Numan Siddique <nusid...@redhat.com> > > This patch handles sending GARPs for > > - router port IPs of a distributed router port > > - router port IPs of a router port which belongs to gateway router > (with the option - redirect-chassis set in Logical_Router.options) > > Signed-off-by: Numan Siddique <nusid...@redhat.com>
I tested the patches on my setup and the code looks good to me. A bit unrelated to this change, at least for me the ovn_port_update_sbrec is getting quite hard to follow. Maybe we can consider refactoring it a bit in the future so different functionalities are handled by smaller functions (i.e., router-port, switch-port connected to vif, switch-port connected to router). Acked-by: Dumitru Ceara <dce...@redhat.com> > --- > ovn/northd/ovn-northd.c | 44 ++++++++++++++++---- > tests/ovn.at | 89 +++++++++++++++++++++++++++++++---------- > 2 files changed, 105 insertions(+), 28 deletions(-) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index e0af234f8..e8cbc3534 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1983,9 +1983,23 @@ get_nat_addresses(const struct ovn_port *op, size_t *n) > } > } else { > /* Centralized NAT rule, either on gateway router or distributed > - * router. */ > - ds_put_format(&c_addresses, " %s", nat->external_ip); > - central_ip_address = true; > + * router. > + * Check if external_ip is same as router ip. If so, then there > + * is no need to add this to the nat_addresses. The router IPs > + * will be added separately. */ > + bool is_router_ip = false; > + for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) { > + if (!strcmp(nat->external_ip, > + op->lrp_networks.ipv4_addrs[j].addr_s)) { > + is_router_ip = true; > + break; > + } > + } > + > + if (!is_router_ip) { > + ds_put_format(&c_addresses, " %s", nat->external_ip); > + central_ip_address = true; > + } > } > } > > @@ -2531,13 +2545,26 @@ ovn_port_update_sbrec(struct northd_context *ctx, > * - op->peer has 'reside-on-gateway-chassis' set and the > * the logical router datapath has distributed router port. > * > + * - op->peer is distributed gateway router port. > + * > + * - op->peer's router is a gateway router and op has a localnet > + * port. > + * > * Note: Port_Binding.nat_addresses column is also used for > * sending the GARPs for the router port IPs. > * */ > + bool add_router_port_garp = false; > if (op->peer && op->peer->nbrp && op->peer->od->l3dgw_port && > op->peer->od->l3redirect_port && > - smap_get_bool(&op->peer->nbrp->options, > - "reside-on-redirect-chassis", false)) { > + (smap_get_bool(&op->peer->nbrp->options, > + "reside-on-redirect-chassis", false) || > + op->peer == op->peer->od->l3dgw_port)) { > + add_router_port_garp = true; > + } else if (chassis && op->od->localnet_port) { > + add_router_port_garp = true; > + } > + > + if (add_router_port_garp) { > struct ds garp_info = DS_EMPTY_INITIALIZER; > ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s); > for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs; > @@ -2545,8 +2572,11 @@ ovn_port_update_sbrec(struct northd_context *ctx, > ds_put_format(&garp_info, " %s", > > op->peer->lrp_networks.ipv4_addrs[i].addr_s); > } > - ds_put_format(&garp_info, " is_chassis_resident(%s)", > - op->peer->od->l3redirect_port->json_key); > + > + if (op->peer->od->l3redirect_port) { > + ds_put_format(&garp_info, " is_chassis_resident(%s)", > + op->peer->od->l3redirect_port->json_key); > + } > > sbrec_port_binding_update_nat_addresses_addvalue( > op->sb, ds_cstr(&garp_info)); > diff --git a/tests/ovn.at b/tests/ovn.at > index ea627e128..2e266d94a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -6730,6 +6730,9 @@ 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 until the patch ports are created in hv1 to connect br-int to br-eth0 > +OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln_port" | wc -l`]) > > # Wait for packet to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50]) > @@ -6737,10 +6740,11 @@ trim_zeros() { > sed 's/\(00\)\{1,\}$//' > } > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | > trim_zeros > packets > -expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002" > +expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80001000000000000c0a80001" > echo $expected > expout > +expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002" > +echo $expected >> expout > AT_CHECK([sort packets], [0], [expout]) > -cat packets > > OVN_CLEANUP([hv1]) > > @@ -6786,7 +6790,15 @@ 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 until the patch ports are created to connect br-int to br-eth0 > +OVS_WAIT_UNTIL([test 1 = `ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln_port" | wc -l`]) > > +ovn-sbctl list port_binding lrp0-rp > +echo "*****" > +ovn-nbctl list logical_switch_port lrp0-rp > +ovn-nbctl list logical_router_port lrp0 > +ovn-nbctl show > # Wait for packet to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50]) > trim_zeros() { > @@ -6800,7 +6812,6 @@ echo $expected >> expout > > expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80003000000000000c0a80003" > echo $expected >> expout > AT_CHECK([sort packets], [0], [expout]) > -cat packets > > OVN_CLEANUP([hv1]) > > @@ -8564,7 +8575,8 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 > dst_ip=`ip_to_hex 172 16 1 3` > > expected=${dst_mac}${src_mac}08004500001c000000003f110100${src_ip}${dst_ip}0035111100080000 > echo $expected > ext1-vif1.expected > - > + > exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > + echo $exp_gw_ip_garp >> ext1-vif1.expected > as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 > > if test $backup_vswitchd_dead != 1; then > @@ -8580,7 +8592,10 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq > 1 > > OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" > $active_gw/br-phys_n1-tx.pcap > packets > - AT_CHECK([grep $expected packets | sort], [0], [expout]) > + cat packets | grep $expected > exp > + cat packets | grep $exp_gw_ip_garp >> exp > + AT_CHECK([cat exp], [0], [expout]) > + rm -f expout > if test $backup_vswitchd_dead != 1; then > # Check for backup gw only if vswitchd is alive > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" > $backup_gw/br-phys_n1-tx.pcap > packets > @@ -8812,6 +8827,8 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` -eq 1 > dst_ip=`ip_to_hex 172 16 1 3` > > expected=${dst_mac}${src_mac}08004500001c000000003e110200${src_ip}${dst_ip}0035111100080000 > echo $expected > ext1-vif1.expected > + > exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > + echo $exp_gw_ip_garp >> ext1-vif1.expected > > as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 > as $backup_gw reset_pcap_file br-phys_n1 $backup_gw/br-phys_n1 > @@ -8820,11 +8837,12 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` > -eq 1 > # Resend packet from foo1 to outside1 > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > - sleep 1 > - > OVN_CHECK_PACKETS([ext1/vif1-tx.pcap], [ext1-vif1.expected]) > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" > $active_gw/br-phys_n1-tx.pcap > packets > - AT_CHECK([grep $expected packets | sort], [0], [expout]) > + cat packets | grep $expected > exp > + cat packets | grep $exp_gw_ip_garp >> exp > + AT_CHECK([cat exp], [0], [expout]) > + > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" > $backup_gw/br-phys_n1-tx.pcap > packets > AT_CHECK([grep $expected packets | sort], [0], []) > } > @@ -9062,13 +9080,17 @@ OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], > [hv3-vif1.expected]) > # Now add bridge-mappings on hv2, which should make everything work > as hv2 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > > -# Allow some time for ovn-northd and ovn-controller to catch up. > -# XXX This should be more systematic. > -sleep 2 > +# Wait until the patch ports are created in hv2 to connect br-int to br-phys > +OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln-alice" | wc -l`]) > > # ARP for router IP address from outside1 > test_arp 3 1 f00000010204 $outside_ip $rtr_ip 000002010203 > > +# hv3-vif1.expected should also have the gw router port garp packet. > +exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > +echo $exp_gw_ip_garp >> hv3-vif1.expected > + > # Now check the packets actually received against the ones expected. > OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [hv3-vif1.expected]) > > @@ -9224,7 +9246,7 @@ ovn_attach n1 br-phys 192.168.0.3 > 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]) > +AT_CHECK([ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1]) > > # Allow some time for ovn-northd and ovn-controller to catch up. > # XXX This should be more systematic. > @@ -9237,6 +9259,10 @@ OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [packets]) > # Add bridge-mapping on hv2 > AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > > +# Wait until the patch ports are created in hv2 to connect br-int to br-phys > +OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln_port" | wc -l`]) > + > # Wait for packets to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) > trim_zeros() { > @@ -9277,6 +9303,10 @@ ovs-vsctl -- add-port br-int hv3-vif2 -- \ > # Add bridge-mapping on hv3 > AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=physnet1:br-phys]) > > +# Wait until the patch ports are created in hv3 to connect br-int to br-phys > +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln_port" | wc -l`]) > + > # Re-add nat-addresses option > ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" > > @@ -9287,12 +9317,14 @@ trim_zeros() { > } > > $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | > trim_zeros > packets > -expected="fffffffffffff0000000000308060001080006040001f00000000003c0a80003000000000000c0a80003" > -echo $expected >> expout > -expected="fffffffffffff0000000000408060001080006040001f00000000004c0a80004000000000000c0a80004" > -echo $expected >> expout > -AT_CHECK([sort packets], [0], [expout]) > -sort packets | cat > +garp_1="fffffffffffff0000000000308060001080006040001f00000000003c0a80003000000000000c0a80003" > +echo $garp_1 > expout > +garp_2="fffffffffffff0000000000408060001080006040001f00000000004c0a80004000000000000c0a80004" > +echo $garp_2 >> expout > + > +cat packets | grep $garp_1 | head -1 > exp > +cat packets | grep $garp_2 | head -1 >> exp > +AT_CHECK([cat exp], [0], [expout]) > > OVN_CLEANUP([hv1],[hv2],[hv3]) > > @@ -10763,16 +10795,31 @@ AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], > []) > AT_CHECK([ovn-nbctl set Logical_Switch_Port ln_port tag=2014]) > > # wait for earlier changes to take effect > -AT_CHECK([ovn-nbctl --timeout=3 --wait=hv sync], [0], [ignore]) > +OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-ofctl dump-flows br-int table=65 | \ > +grep "actions=mod_vlan_vid:2014" | wc -l` > +]) > + > +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=65 | \ > +grep "actions=mod_vlan_vid:2014" | wc -l` > +]) > > # update nat-addresses option > -ovn-nbctl --wait=hv lsp-set-options lrp0-rp router-port=lrp0 > -ovn-nbctl --wait=hv lsp-set-options lrp0-rp router-port=lrp0 > nat-addresses="router" > +ovn-nbctl --wait=hv clear logical_switch_port lrp0-rp options > + > +#Wait until the Port_Binding.nat_addresses is cleared. > +OVS_WAIT_UNTIL([test 0 = `ovn-sbctl --bare --columns nat_addresses find > port_binding \ > +logical_port=lrp0-rp | grep is_chassis | wc -l`]) > > 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 > > +ovn-nbctl --wait=hv lsp-set-options lrp0-rp router-port=lrp0 > nat-addresses="router" > + > +#Wait until the Port_Binding.nat_addresses is set. > +OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns nat_addresses find > port_binding \ > +logical_port=lrp0-rp | grep is_chassis | wc -l`]) > + > # Wait for packets to be received. > OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) > trim_zeros() { > -- > 2.21.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev