Hi Ankur, Can you please resend patches setting the proper subject prefix .. like "PATCH v2 1/4 ovn"
The ovsrobot will apply these patches to the new ovn repo if "ovn" tag is present with in the subject prefix. Eg. - https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/361271.html Thanks Numan On Wed, Jul 31, 2019 at 11:37 AM Ankur Sharma <ankur.sha...@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.sha...@nutanix.com> > --- > controller/physical.c | 19 ++- > controller/pinctrl.c | 37 +++--- > controller/pinctrl.h | 5 + > tests/ovn.at | 313 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 354 insertions(+), 20 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index 5c2f74e..aa06b3f 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -38,6 +38,7 @@ > #include "lib/ovn-sb-idl.h" > #include "lib/ovn-util.h" > #include "physical.h" > +#include "pinctrl.h" > #include "openvswitch/shash.h" > #include "simap.h" > #include "smap.h" > @@ -228,9 +229,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 +274,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 (pinctrl_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 +801,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 ebbb52a..046820f 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -441,6 +441,25 @@ pinctrl_init(void) > &pinctrl); > } > > +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); > + } > +} > + > static ovs_be32 > queue_msg(struct rconn *swconn, struct ofpbuf *msg) > { > @@ -3731,24 +3750,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 ..] > diff --git a/controller/pinctrl.h b/controller/pinctrl.h > index 80da28d..0fa9ba3 100644 > --- a/controller/pinctrl.h > +++ b/controller/pinctrl.h > @@ -47,5 +47,10 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sset *active_tunnels); > void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > void pinctrl_destroy(void); > +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); > > #endif /* controller/pinctrl.h */ > diff --git a/tests/ovn.at b/tests/ovn.at > index 23159c8..6d9a08a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -29,6 +29,12 @@ 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__ "$1" "$2" > + echo "received_text=$rcv_text" > + sed -i '/ffffffffffff/d' $rcv_text > + AT_CHECK([sort $rcv_text], [0], [expout])]) > + > AT_BANNER([OVN components]) > > AT_SETUP([ovn -- lexer]) > @@ -14700,3 +14706,310 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected]) > > OVN_CLEANUP([hv1], [hv2]) > 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" "$@" > +} > + > +# 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 > + > +# 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 > +sleep 2 > + > +test_arp vif-north f0f000000011 $sip $tip 000001010207 > + > +sleep 1 > + > +# 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]) > +AT_CHECK([grep 000001010207 hv3/br-phys_n1-tx.packets | wc -l], [0], [[1 > +]]) > + > +# 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 > 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