[ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
Dumitru Ceara
dceara at redhat.com
Tue Jul 2 09:00:42 UTC 2019
On Mon, Jul 1, 2019 at 9:45 AM <nusiddiq at redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq at 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 <nusiddiq at 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 <dceara at 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list