[ovs-dev] [PATCH v6 1/4 ovn] OVN: Do not replace router port mac on gateway chassis.

Numan Siddique nusiddiq at redhat.com
Sat Aug 24 18:37:57 UTC 2019


On Sat, Aug 17, 2019 at 6:23 AM Ankur Sharma <ankur.sharma at nutanix.com>
wrote:

> With 795d7f24ce0e2ed5454e193a059451d237289542 we have added
> support for E-W routing on vlan backed networks by replacing
> router port macs with chassis macs.
>
> This replacement of router port mac need NOT be done on
> gateway chassis for following reasons:
>
> a. For N-S traffic, gateway chassis will respond to ARP
> for the router port (to which it is attached) and
> traffic will be using router port mac as destination mac.
>
> b. Chassis redirect port is a centralized version of distributed
> router port, hence we need not replace its mac with chassis mac
> on the resident chassis.
>
> This patch addresses the same.
>
> Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>
>

Hi Ankur,

I applied this patch of the series to master with a small change.

diff --git a/tests/ovn.at b/tests/ovn.at
index 062d43314..c5281a09d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -31,7 +31,6 @@ m4_define([OVN_CHECK_PACKETS],

 m4_define([OVN_CHECK_PACKETS_REMOVE_BROADCAST],
   [ovn_check_packets__ () {
-   echo
    echo "checking packets in $1 against $2:"
    rcv_pcap=$1
    exp_text=$2


Thanks
Numan


> ---
>  controller/lport.c    |  20 ++++
>  controller/lport.h    |   6 +
>  controller/physical.c |  18 ++-
>  controller/pinctrl.c  |  20 +---
>  tests/ovn.at          | 321
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 364 insertions(+), 21 deletions(-)
>
> diff --git a/controller/lport.c b/controller/lport.c
> index 792c825..478fcfd 100644
> --- a/controller/lport.c
> +++ b/controller/lport.c
> @@ -17,6 +17,7 @@
>
>  #include "lib/sset.h"
>  #include "lport.h"
> +#include "ha-chassis.h"
>  #include "hash.h"
>  #include "openvswitch/vlog.h"
>  #include "lib/ovn-sb-idl.h"
> @@ -64,6 +65,25 @@ lport_lookup_by_key(struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
>      return retval;
>  }
>
> +bool
> +lport_is_chassis_resident(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> +                          const struct sbrec_chassis *chassis,
> +                          const struct sset *active_tunnels,
> +                          const char *port_name)
> +{
> +    const struct sbrec_port_binding *pb
> +        = lport_lookup_by_name(sbrec_port_binding_by_name, port_name);
> +    if (!pb || !pb->chassis) {
> +        return false;
> +    }
> +    if (strcmp(pb->type, "chassisredirect")) {
> +        return pb->chassis == chassis;
> +    } else {
> +        return ha_chassis_group_is_active(pb->ha_chassis_group,
> +                                          active_tunnels, chassis);
> +    }
> +}
> +
>  const struct sbrec_datapath_binding *
>  datapath_lookup_by_key(struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
>                         uint64_t dp_key)
> diff --git a/controller/lport.h b/controller/lport.h
> index 2d4bb71..345efc1 100644
> --- a/controller/lport.h
> +++ b/controller/lport.h
> @@ -23,6 +23,7 @@ struct sbrec_chassis;
>  struct sbrec_datapath_binding;
>  struct sbrec_multicast_group;
>  struct sbrec_port_binding;
> +struct sset;
>
>
>  /* Database indexes.
> @@ -48,5 +49,10 @@ const struct sbrec_datapath_binding
> *datapath_lookup_by_key(
>  const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name(
>      struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
>      const struct sbrec_datapath_binding *, const char *name);
> +bool
> +lport_is_chassis_resident(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> +                          const struct sbrec_chassis *chassis,
> +                          const struct sset *active_tunnels,
> +                          const char *port_name);
>
>  #endif /* controller/lport.h */
> diff --git a/controller/physical.c b/controller/physical.c
> index a05962b..5068785 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -228,9 +228,12 @@ get_zone_ids(const struct sbrec_port_binding *binding,
>  }
>
>  static void
> -put_replace_router_port_mac_flows(const struct
> +put_replace_router_port_mac_flows(struct ovsdb_idl_index
> +                                  *sbrec_port_binding_by_name,
> +                                  const struct
>                                    sbrec_port_binding *localnet_port,
>                                    const struct sbrec_chassis *chassis,
> +                                  const struct sset *active_tunnels,
>                                    const struct hmap *local_datapaths,
>                                    struct ofpbuf *ofpacts_p,
>                                    ofp_port_t ofport,
> @@ -270,6 +273,16 @@ put_replace_router_port_mac_flows(const struct
>          struct eth_addr router_port_mac;
>          struct match match;
>          struct ofpact_mac *replace_mac;
> +        char *cr_peer_name = xasprintf("cr-%s",
> rport_binding->logical_port);
> +        if (lport_is_chassis_resident(sbrec_port_binding_by_name,
> +                                      chassis, active_tunnels,
> +                                      cr_peer_name)) {
> +            /* If a router port's chassisredirect port is
> +             * resident on this chassis, then we need not do mac replace.
> */
> +            free(cr_peer_name);
> +            continue;
> +        }
> +        free(cr_peer_name);
>
>          /* Table 65, priority 150.
>           * =======================
> @@ -787,7 +800,8 @@ consider_port_binding(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
>                          &match, ofpacts_p, &binding->header_.uuid);
>
>          if (!strcmp(binding->type, "localnet")) {
> -            put_replace_router_port_mac_flows(binding, chassis,
> +            put_replace_router_port_mac_flows(sbrec_port_binding_by_name,
> +                                              binding, chassis,
> active_tunnels,
>                                                local_datapaths, ofpacts_p,
>                                                ofport, flow_table);
>          }
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index f27718f..e8abe0b 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -3755,24 +3755,6 @@ get_localnet_vifs_l3gwports(
>      sbrec_port_binding_index_destroy_row(target);
>  }
>
> -static bool
> -pinctrl_is_chassis_resident(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> -                            const struct sbrec_chassis *chassis,
> -                            const struct sset *active_tunnels,
> -                            const char *port_name)
> -{
> -    const struct sbrec_port_binding *pb
> -        = lport_lookup_by_name(sbrec_port_binding_by_name, port_name);
> -    if (!pb || !pb->chassis) {
> -        return false;
> -    }
> -    if (strcmp(pb->type, "chassisredirect")) {
> -        return pb->chassis == chassis;
> -    } else {
> -        return ha_chassis_group_is_active(pb->ha_chassis_group,
> -                                          active_tunnels, chassis);
> -    }
> -}
>
>  /* Extracts the mac, IPv4 and IPv6 addresses, and logical port from
>   * 'addresses' which should be of the format 'MAC [IP1 IP2 ..]
> @@ -3853,7 +3835,7 @@ consider_nat_address(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
>      char *lport = NULL;
>      if (!extract_addresses_with_port(nat_address, laddrs, &lport)
>          || (!lport && !strcmp(pb->type, "patch"))
> -        || (lport && !pinctrl_is_chassis_resident(
> +        || (lport && !lport_is_chassis_resident(
>                  sbrec_port_binding_by_name, chassis,
>                  active_tunnels, lport))) {
>          destroy_lport_addresses(laddrs);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 71eb390..045cec3 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -29,6 +29,23 @@ m4_define([OVN_CHECK_PACKETS],
>    [ovn_check_packets__ "$1" "$2"
>     AT_CHECK([sort $rcv_text], [0], [expout])])
>
> +m4_define([OVN_CHECK_PACKETS_REMOVE_BROADCAST],
> +  [ovn_check_packets__ () {
> +   echo
> +   echo "checking packets in $1 against $2:"
> +   rcv_pcap=$1
> +   exp_text=$2
> +   exp_n=`wc -l < "$exp_text"`
> +   OVS_WAIT_UNTIL(
> +     [$PYTHON "$top_srcdir/ovs/utilities/ovs-pcap.in" $rcv_pcap >
> $rcv_text
> +      sed -i '/ffffffffffff/d' $rcv_text
> +      rcv_n=`wc -l < "$rcv_text"`
> +      echo "rcv_n=$rcv_n exp_n=$exp_n"
> +      test $rcv_n -ge $exp_n])
> +   sort $exp_text > expout
> + }
> +])
> +
>  AT_BANNER([OVN components])
>
>  AT_SETUP([ovn -- lexer])
> @@ -15009,3 +15026,307 @@ on_exit 'kill $(cat ovn-nbctl.pid)'
>  AT_CHECK([ovn-nbctl -u $sockfile show])
>
>  AT_CLEANUP
> +
> +
> +AT_SETUP([ovn -- 2 HVs, 2 lports/HV, localnet ports, DVR N-S ARP
> handling])
> +ovn_start
> +
> +# In this test cases we create 3 switches, all connected to same
> +# physical network (through br-phys on each HV). LS1 and LS2 have
> +# 1 VIF each. Each HV has 1 VIF port. The first digit
> +# of VIF port name indicates the hypervisor it is bound to, e.g.
> +# lp23 means VIF 3 on hv2.
> +#
> +# All the switches are connected to a logical router "router".
> +#
> +# Each switch's VLAN tag and their logical switch ports are:
> +#   - ls1:
> +#       - tagged with VLAN 101
> +#       - ports: lp11
> +#   - ls2:
> +#       - tagged with VLAN 201
> +#       - ports: lp22
> +#   - ls-underlay:
> +#       - tagged with VLAN 1000
> +# Note: a localnet port is created for each switch to connect to
> +# physical network.
> +
> +for i in 1 2; do
> +    ls_name=ls$i
> +    ovn-nbctl ls-add $ls_name
> +    ln_port_name=ln$i
> +    if test $i -eq 1; then
> +        ovn-nbctl lsp-add $ls_name $ln_port_name "" 101
> +    elif test $i -eq 2; then
> +        ovn-nbctl lsp-add $ls_name $ln_port_name "" 201
> +    fi
> +    ovn-nbctl lsp-set-addresses $ln_port_name unknown
> +    ovn-nbctl lsp-set-type $ln_port_name localnet
> +    ovn-nbctl lsp-set-options $ln_port_name network_name=phys
> +done
> +
> +# lsp_to_ls LSP
> +#
> +# Prints the name of the logical switch that contains LSP.
> +lsp_to_ls () {
> +    case $1 in dnl (
> +        lp?[[11]]) echo ls1 ;; dnl (
> +        lp?[[12]]) echo ls2 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +vif_to_hv () {
> +    case $1 in dnl (
> +        vif[[1]]?) echo hv1 ;; dnl (
> +        vif[[2]]?) echo hv2 ;; dnl (
> +        vif?[[north]]?) echo hv4 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +ip_to_hex() {
> +       printf "%02x%02x%02x%02x" "$@"
> +}
> +
> +net_add n1
> +for i in 1 2; do
> +    sim_add hv$i
> +    as hv$i
> +    ovs-vsctl add-br br-phys
> +    ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +    ovs-vsctl set open .
> external-ids:ovn-chassis-mac-mappings="phys:aa:bb:cc:dd:ee:$i$i"
> +    ovn_attach n1 br-phys 192.168.0.$i
> +
> +    ovs-vsctl add-port br-int vif$i$i -- \
> +        set Interface vif$i$i external-ids:iface-id=lp$i$i \
> +                              options:tx_pcap=hv$i/vif$i$i-tx.pcap \
> +                              options:rxq_pcap=hv$i/vif$i$i-rx.pcap \
> +                              ofport-request=$i$i
> +
> +    lsp_name=lp$i$i
> +    ls_name=$(lsp_to_ls $lsp_name)
> +
> +    ovn-nbctl lsp-add $ls_name $lsp_name
> +    ovn-nbctl lsp-set-addresses $lsp_name "f0:00:00:00:00:$i$i
> 192.168.$i.$i"
> +    ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:$i$i
> +
> +    OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> +
> +done
> +
> +ovn-nbctl ls-add ls-underlay
> +ovn-nbctl lsp-add ls-underlay ln3 "" 1000
> +ovn-nbctl lsp-set-addresses ln3 unknown
> +ovn-nbctl lsp-set-type ln3 localnet
> +ovn-nbctl lsp-set-options ln3 network_name=phys
> +
> +ovn-nbctl ls-add ls-north
> +ovn-nbctl lsp-add ls-north ln4 "" 1000
> +ovn-nbctl lsp-set-addresses ln4 unknown
> +ovn-nbctl lsp-set-type ln4 localnet
> +ovn-nbctl lsp-set-options ln4 network_name=phys
> +
> +# Add a VM on ls-north
> +ovn-nbctl lsp-add ls-north lp-north
> +ovn-nbctl lsp-set-addresses lp-north "f0:f0:00:00:00:11 172.31.0.10"
> +ovn-nbctl lsp-set-port-security lp-north f0:f0:00:00:00:11
> +
> +# Add 3rd hypervisor
> +sim_add hv3
> +as hv3 ovs-vsctl add-br br-phys
> +as hv3 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +as hv3 ovs-vsctl set open .
> external-ids:ovn-chassis-mac-mappings="phys:aa:bb:cc:dd:ee:33"
> +as hv3 ovn_attach n1 br-phys 192.168.0.3
> +
> +# Add 4th hypervisor
> +sim_add hv4
> +as hv4 ovs-vsctl add-br br-phys
> +as hv4 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +as hv4 ovs-vsctl set open .
> external-ids:ovn-chassis-mac-mappings="phys:aa:bb:cc:dd:ee:44"
> +as hv4 ovn_attach n1 br-phys 192.168.0.4
> +
> +as hv4 ovs-vsctl add-port br-int vif-north -- \
> +        set Interface vif-north external-ids:iface-id=lp-north \
> +                              options:tx_pcap=hv4/vif-north-tx.pcap \
> +                              options:rxq_pcap=hv4/vif-north-rx.pcap \
> +                              ofport-request=44
> +
> +ovn-nbctl lr-add router
> +ovn-nbctl lrp-add router router-to-ls1 00:00:01:01:02:03 192.168.1.3/24
> +ovn-nbctl <http://192.168.1.3/24+ovn-nbctl> lrp-add router router-to-ls2
> 00:00:01:01:02:05 192.168.2.3/24
> +ovn-nbctl <http://192.168.2.3/24+ovn-nbctl> lrp-add router
> router-to-underlay 00:00:01:01:02:07 172.31.0.1/24
> +
> +ovn-nbctl lsp-add ls1 ls1-to-router -- set Logical_Switch_Port
> ls1-to-router type=router \
> +          options:router-port=router-to-ls1 -- lsp-set-addresses
> ls1-to-router router
> +ovn-nbctl lsp-add ls2 ls2-to-router -- set Logical_Switch_Port
> ls2-to-router type=router \
> +          options:router-port=router-to-ls2 -- lsp-set-addresses
> ls2-to-router router
> +ovn-nbctl lsp-add ls-underlay underlay-to-router -- set
> Logical_Switch_Port \
> +                              underlay-to-router type=router \
> +                              options:router-port=router-to-underlay \
> +                              -- lsp-set-addresses underlay-to-router
> router
> +
> +
> +OVN_POPULATE_ARP
> +
> +# lsp_to_ls LSP
> +#
> +# Prints the name of the logical switch that contains LSP.
> +lsp_to_ls () {
> +    case $1 in dnl (
> +        lp?[[11]]) echo ls1 ;; dnl (
> +        lp?[[12]]) echo ls2 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +vif_to_ls () {
> +    case $1 in dnl (
> +        vif?[[11]]) echo ls1 ;; dnl (
> +        vif?[[12]]) echo ls2 ;; dnl (
> +        vif-north) echo ls-north ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +hv_to_num () {
> +    case $1 in dnl (
> +        hv1) echo 1 ;; dnl (
> +        hv2) echo 2 ;; dnl (
> +        hv3) echo 3 ;; dnl (
> +        hv4) echo 4 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +vif_to_num () {
> +    case $1 in dnl (
> +        vif22) echo 22 ;; dnl (
> +        vif21) echo 21 ;; dnl (
> +        vif11) echo 11 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +vif_to_hv () {
> +    case $1 in dnl (
> +        vif[[1]]?) echo hv1 ;; dnl (
> +        vif[[2]]?) echo hv2 ;; dnl (
> +        vif-north) echo hv4 ;; dnl (
> +        *) AT_FAIL_IF([:]) ;;
> +    esac
> +}
> +
> +vif_to_lrp () {
> +    echo router-to-`vif_to_ls $1`
> +}
> +
> +ip_to_hex() {
> +       printf "%02x%02x%02x%02x" "$@"
> +}
> +
> +# test_arp INPORT SHA SPA TPA [REPLY_HA]
> +#
> +# Causes a packet to be received on INPORT.  The packet is an ARP
> +# request with SHA, SPA, and TPA as specified.  If REPLY_HA is provided,
> then
> +# it should be the hardware address of the target to expect to receive in
> an
> +# ARP reply; otherwise no reply is expected.
> +#
> +# INPORT is an logical switch port number, e.g. 11 for vif11.
> +# SHA and REPLY_HA are each 12 hex digits.
> +# SPA and TPA are each 8 hex digits.
> +test_arp() {
> +    local inport=$1 sha=$2 spa=$3 tpa=$4 reply_ha=$5
> +    local
> request=ffffffffffff${sha}08060001080006040001${sha}${spa}ffffffffffff${tpa}
> +    hv=`vif_to_hv $inport`
> +    as $hv ovs-appctl netdev-dummy/receive $inport $request
> +
> +    if test X$reply_ha = X; then
> +        # Expect to receive the broadcast ARP on the other logical switch
> ports
> +        # if no reply is expected.
> +        local i j
> +        for i in 1 2 3; do
> +            for j in 1 2 3; do
> +                if test $i$j != $inport; then
> +                    echo $request >> $i$j.expected
> +                fi
> +            done
> +        done
> +    else
> +        # Expect to receive the reply, if any.
> +        local
> reply=${sha}${reply_ha}08060001080006040002${reply_ha}${tpa}${sha}${spa}
> +        local
> reply_vid=${sha}${reply_ha}810003e808060001080006040002${reply_ha}${tpa}${sha}${spa}
> +        echo $reply_vid >> ${inport}_vid.expected
> +        echo $reply >> $inport.expected
> +    fi
> +}
> +
> +sip=`ip_to_hex 172 31 0 10`
> +tip=`ip_to_hex 172 31 0 1`
> +
> +# Set a hypervisor as gateway chassis, for router port 172.31.0.1
> +ovn-nbctl lrp-set-gateway-chassis router-to-underlay hv3
> +ovn-nbctl --wait=sb sync
> +
> +# Dump a bunch of info helpful for debugging if there's a failure.
> +
> +echo "------ OVN dump ------"
> +ovn-nbctl show
> +ovn-sbctl show
> +ovn-sbctl list port_binding
> +ovn-sbctl list mac_binding
> +
> +echo "------ hv1 dump ------"
> +as hv1 ovs-vsctl show
> +as hv1 ovs-vsctl list Open_Vswitch
> +
> +echo "------ hv2 dump ------"
> +as hv2 ovs-vsctl show
> +as hv2 ovs-vsctl list Open_Vswitch
> +
> +echo "------ hv3 dump ------"
> +as hv3 ovs-vsctl show
> +as hv3 ovs-vsctl list Open_Vswitch
> +
> +echo "------ hv4 dump ------"
> +as hv4 ovs-vsctl show
> +as hv4 ovs-vsctl list Open_Vswitch
> +
> +OVS_WAIT_UNTIL([test x`ovn-sbctl --bare --columns chassis find
> port_binding  logical_port=cr-router-to-underlay | wc -l` = x1])
> +
> +test_arp vif-north f0f000000011 $sip $tip 000001010207
> +
> +# Confirm that vif-north gets a single ARP reply
> +OVN_CHECK_PACKETS_REMOVE_BROADCAST([hv4/vif-north-tx.pcap],
> [vif-north.expected])
> +
> +# Confirm that only redirect chassis allowed arp resolution.
> +OVN_CHECK_PACKETS_REMOVE_BROADCAST([hv3/br-phys_n1-tx.pcap],
> [vif-north_vid.expected])
> +
> +# Confirm that other OVN chassis did not generate ARP reply.
> +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys_n1-tx.pcap >
> hv1/br-phys_n1-tx.packets
> +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-phys_n1-tx.pcap >
> hv2/br-phys_n1-tx.packets
> +
> +AT_CHECK([grep 000001010207 hv1/br-phys_n1-tx.packets | wc -l], [0], [[0
> +]])
> +AT_CHECK([grep 000001010207 hv2/br-phys_n1-tx.packets | wc -l], [0], [[0
> +]])
> +
> +echo "----------- Post Traffic hv1 dump -----------"
> +as hv1 ovs-ofctl -O OpenFlow13 dump-flows br-int
> +as hv1 ovs-appctl fdb/show br-phys
> +
> +echo "----------- Post Traffic hv2 dump -----------"
> +as hv2 ovs-ofctl -O OpenFlow13 dump-flows br-int
> +as hv2 ovs-appctl fdb/show br-phys
> +
> +echo "----------- Post Traffic hv3 dump -----------"
> +as hv3 ovs-ofctl -O OpenFlow13 dump-flows br-int
> +as hv3 ovs-appctl fdb/show br-phys
> +
> +echo "----------- Post Traffic hv4 dump -----------"
> +as hv4 ovs-ofctl -O OpenFlow13 dump-flows br-int
> +as hv4 ovs-appctl fdb/show br-phys
> +
> +OVN_CLEANUP([hv1],[hv2],[hv3],[hv4])
> +
> +AT_CLEANUP
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list