| Message ID | 20240212194928.3498672-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] physical: Don't reset encap ID across pipelines. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Feb 12, 2024 at 2:49 PM Han Zhou <hzhou@ovn.org> wrote: > > The MFF_LOG_ENCAP_ID register was defined to save the encap ID and avoid > changing across pipelines, but in the function > load_logical_ingress_metadata it was reset unconditionally. Because of > this, the encap selection doesn't work when traffic traverses L3 > boundaries. This patch fixes it by ensuring the register is loaded only > in the flows of table 0, which is where packets from VIFs enter the > pipeline for the first time. > > Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs selection.") > Signed-off-by: Han Zhou <hzhou@ovn.org> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > controller/physical.c | 28 ++++++++------- > tests/ovn.at | 83 ++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 98 insertions(+), 13 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index c32642d2c69b..7ef259da44b1 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -73,7 +73,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > const struct zone_ids *zone_ids, > size_t n_encap_ips, > const char **encap_ips, > - struct ofpbuf *ofpacts_p); > + struct ofpbuf *ofpacts_p, > + bool load_encap_id); > static int64_t get_vxlan_port_key(int64_t port_key); > > /* UUID to identify OF flows not associated with ovsdb rows. */ > @@ -689,7 +690,7 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones, > ofpact_put_STRIP_VLAN(ofpacts_p); > } > load_logical_ingress_metadata(localnet_port, &zone_ids, 0, NULL, > - ofpacts_p); > + ofpacts_p, true); > replace_mac = ofpact_put_SET_ETH_SRC(ofpacts_p); > replace_mac->mac = router_port_mac; > > @@ -1047,7 +1048,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > const struct zone_ids *zone_ids, > size_t n_encap_ips, > const char **encap_ips, > - struct ofpbuf *ofpacts_p) > + struct ofpbuf *ofpacts_p, > + bool load_encap_id) > { > put_zones_ofpacts(zone_ids, ofpacts_p); > > @@ -1057,13 +1059,15 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, ofpacts_p); > put_load(port_key, MFF_LOG_INPORT, 0, 32, ofpacts_p); > > - /* Default encap_id 0. */ > - size_t encap_id = 0; > - if (encap_ips && binding->encap) { > - encap_id = encap_ip_to_id(n_encap_ips, encap_ips, > - binding->encap->ip); > + if (load_encap_id) { > + /* Default encap_id 0. */ > + size_t encap_id = 0; > + if (encap_ips && binding->encap) { > + encap_id = encap_ip_to_id(n_encap_ips, encap_ips, > + binding->encap->ip); > + } > + put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); > } > - put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); > } > > static const struct sbrec_port_binding * > @@ -1108,7 +1112,7 @@ setup_rarp_activation_strategy(const struct sbrec_port_binding *binding, > match_set_dl_type(&match, htons(ETH_TYPE_RARP)); > match_set_in_port(&match, ofport); > > - load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts); > + load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts, true); > > encode_controller_op(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, > NX_CTLR_NO_METER, &ofpacts); > @@ -1522,7 +1526,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > put_load(0, MFF_LOG_CT_ZONE, 0, 16, ofpacts_p); > struct zone_ids peer_zones = get_zone_ids(peer, ct_zones); > load_logical_ingress_metadata(peer, &peer_zones, n_encap_ips, > - encap_ips, ofpacts_p); > + encap_ips, ofpacts_p, false); > put_load(0, MFF_LOG_FLAGS, 0, 32, ofpacts_p); > put_load(0, MFF_LOG_OUTPORT, 0, 32, ofpacts_p); > for (int i = 0; i < MFF_N_LOG_REGS; i++) { > @@ -1739,7 +1743,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > uint32_t ofpacts_orig_size = ofpacts_p->size; > > load_logical_ingress_metadata(binding, &zone_ids, n_encap_ips, > - encap_ips, ofpacts_p); > + encap_ips, ofpacts_p, true); > > if (!strcmp(binding->type, "localport")) { > /* mark the packet as incoming from a localport */ > diff --git a/tests/ovn.at b/tests/ovn.at > index 902dd3793b92..30748c96e1c6 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -30426,7 +30426,7 @@ AT_CLEANUP > > > OVN_FOR_EACH_NORTHD([ > -AT_SETUP([multiple encap ips selection based on VIF's encap_ip]) > +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L2]) > AT_SKIP_IF([test $HAVE_SCAPY = no]) > ovn_start > net_add n1 > @@ -30521,6 +30521,87 @@ AT_CLEANUP > ]) > > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L3]) > +AT_SKIP_IF([test $HAVE_SCAPY = no]) > +ovn_start > +net_add n1 > + > +ovn-nbctl lr-add lr > + > +# 2 HVs, each with 2 encap-ips, and each hosting a separate LS, connected by a LR. > +# lspXY is the Yth LSP on hvX, with encap-ip 192.168.0.XY, with ip 10.0.X.Y > +for i in 1 2; do > + sim_add hv$i > + as hv$i > + ovs-vsctl add-br br-phys-$j > + ovn_attach n1 br-phys-$j 192.168.0.${i}1 > + ovs-vsctl set open . external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2 > + > + check ovn-nbctl ls-add ls$i -- \ > + lrp-add lr lrp-ls$i f0:00:00:00:88:0$i 10.0.$i.88/24 -- \ > + lsp-add ls$i lsp-lr-$i -- lsp-set-type lsp-lr-$i router -- \ > + lsp-set-addresses lsp-lr-$i router -- \ > + lsp-set-options lsp-lr-$i router-port=lrp-ls$i > + > + for j in 1 2; do > + ovs-vsctl add-port br-int vif$i$j -- set Interface vif$i$j \ > + external_ids:iface-id=lsp$i$j \ > + external_ids:encap-ip=192.168.0.$i$j \ > + options:tx_pcap=hv$i/vif$i$j-tx.pcap options:rxq_pcap=hv$i/vif$i$j-rx.pcap > + ovn-nbctl lsp-add ls$i lsp$i$j -- lsp-set-addresses lsp$i$j "f0:00:00:00:00:$i$j 10.0.$i.$j" > + > + done > +done > + > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +vif_to_hv() { > + case $1 in dnl ( > + vif[[1]]?) echo hv1 ;; dnl ( > + vif[[2]]?) echo hv2 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +vif_to_ip() { > + vif=$1 > + echo 10.0.${vif:3:1}.${vif:4:1} > +} > + > +# check_packet_tunnel SRC_VIF DST_VIF > +# Verify that a packet from SRC_VIF to DST_VIF goes through the corresponding > +# tunnel that matches the VIFs' encap_ip configurations. > +check_packet_tunnel() { > + local src=$1 dst=$2 > + local src_mac=f0:00:00:00:00:$src > + local dst_mac=f0:00:00:00:88:01 # lrp-ls1's MAC > + local src_ip=$(vif_to_ip vif$src) > + local dst_ip=$(vif_to_ip vif$dst) > + local local_encap_ip=192.168.0.$src > + local remote_encap_ip=192.168.0.$dst > + local packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/ \ > + IP(dst='${dst_ip}', src='${src_ip}')/ \ > + ICMP(type=8)") > + hv=`vif_to_hv vif$src` > + as $hv > + echo "vif$src -> vif$dst should go through tunnel $local_encap_ip -> $remote_encap_ip" > + tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip) > + AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src $packet | grep "output:" | awk -F ':' '{ print $2 }') == $tunnel_ofport]) > +} > + > +for i in 1 2; do > + for j in 1 2; do > + check_packet_tunnel 1$i 2$j > + done > +done > + > +OVN_CLEANUP([hv1],[hv2]) > +AT_CLEANUP > +]) > + > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([Load Balancer LS hairpin OF flows]) > ovn_start > -- > 2.38.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Fri, Feb 16, 2024 at 2:50 PM Numan Siddique <numans@ovn.org> wrote: > > On Mon, Feb 12, 2024 at 2:49 PM Han Zhou <hzhou@ovn.org> wrote: > > > > The MFF_LOG_ENCAP_ID register was defined to save the encap ID and avoid > > changing across pipelines, but in the function > > load_logical_ingress_metadata it was reset unconditionally. Because of > > this, the encap selection doesn't work when traffic traverses L3 > > boundaries. This patch fixes it by ensuring the register is loaded only > > in the flows of table 0, which is where packets from VIFs enter the > > pipeline for the first time. > > > > Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs selection.") > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > Acked-by: Numan Siddique <numans@ovn.org> > > Numan > Thanks Numan. Applied to main and backported to branch-24.03. Han > > --- > > controller/physical.c | 28 ++++++++------- > > tests/ovn.at | 83 ++++++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 98 insertions(+), 13 deletions(-) > > > > diff --git a/controller/physical.c b/controller/physical.c > > index c32642d2c69b..7ef259da44b1 100644 > > --- a/controller/physical.c > > +++ b/controller/physical.c > > @@ -73,7 +73,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > > const struct zone_ids *zone_ids, > > size_t n_encap_ips, > > const char **encap_ips, > > - struct ofpbuf *ofpacts_p); > > + struct ofpbuf *ofpacts_p, > > + bool load_encap_id); > > static int64_t get_vxlan_port_key(int64_t port_key); > > > > /* UUID to identify OF flows not associated with ovsdb rows. */ > > @@ -689,7 +690,7 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones, > > ofpact_put_STRIP_VLAN(ofpacts_p); > > } > > load_logical_ingress_metadata(localnet_port, &zone_ids, 0, NULL, > > - ofpacts_p); > > + ofpacts_p, true); > > replace_mac = ofpact_put_SET_ETH_SRC(ofpacts_p); > > replace_mac->mac = router_port_mac; > > > > @@ -1047,7 +1048,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > > const struct zone_ids *zone_ids, > > size_t n_encap_ips, > > const char **encap_ips, > > - struct ofpbuf *ofpacts_p) > > + struct ofpbuf *ofpacts_p, > > + bool load_encap_id) > > { > > put_zones_ofpacts(zone_ids, ofpacts_p); > > > > @@ -1057,13 +1059,15 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, > > put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, ofpacts_p); > > put_load(port_key, MFF_LOG_INPORT, 0, 32, ofpacts_p); > > > > - /* Default encap_id 0. */ > > - size_t encap_id = 0; > > - if (encap_ips && binding->encap) { > > - encap_id = encap_ip_to_id(n_encap_ips, encap_ips, > > - binding->encap->ip); > > + if (load_encap_id) { > > + /* Default encap_id 0. */ > > + size_t encap_id = 0; > > + if (encap_ips && binding->encap) { > > + encap_id = encap_ip_to_id(n_encap_ips, encap_ips, > > + binding->encap->ip); > > + } > > + put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); > > } > > - put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); > > } > > > > static const struct sbrec_port_binding * > > @@ -1108,7 +1112,7 @@ setup_rarp_activation_strategy(const struct sbrec_port_binding *binding, > > match_set_dl_type(&match, htons(ETH_TYPE_RARP)); > > match_set_in_port(&match, ofport); > > > > - load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts); > > + load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts, true); > > > > encode_controller_op(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, > > NX_CTLR_NO_METER, &ofpacts); > > @@ -1522,7 +1526,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > put_load(0, MFF_LOG_CT_ZONE, 0, 16, ofpacts_p); > > struct zone_ids peer_zones = get_zone_ids(peer, ct_zones); > > load_logical_ingress_metadata(peer, &peer_zones, n_encap_ips, > > - encap_ips, ofpacts_p); > > + encap_ips, ofpacts_p, false); > > put_load(0, MFF_LOG_FLAGS, 0, 32, ofpacts_p); > > put_load(0, MFF_LOG_OUTPORT, 0, 32, ofpacts_p); > > for (int i = 0; i < MFF_N_LOG_REGS; i++) { > > @@ -1739,7 +1743,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > uint32_t ofpacts_orig_size = ofpacts_p->size; > > > > load_logical_ingress_metadata(binding, &zone_ids, n_encap_ips, > > - encap_ips, ofpacts_p); > > + encap_ips, ofpacts_p, true); > > > > if (!strcmp(binding->type, "localport")) { > > /* mark the packet as incoming from a localport */ > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 902dd3793b92..30748c96e1c6 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -30426,7 +30426,7 @@ AT_CLEANUP > > > > > > OVN_FOR_EACH_NORTHD([ > > -AT_SETUP([multiple encap ips selection based on VIF's encap_ip]) > > +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L2]) > > AT_SKIP_IF([test $HAVE_SCAPY = no]) > > ovn_start > > net_add n1 > > @@ -30521,6 +30521,87 @@ AT_CLEANUP > > ]) > > > > > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L3]) > > +AT_SKIP_IF([test $HAVE_SCAPY = no]) > > +ovn_start > > +net_add n1 > > + > > +ovn-nbctl lr-add lr > > + > > +# 2 HVs, each with 2 encap-ips, and each hosting a separate LS, connected by a LR. > > +# lspXY is the Yth LSP on hvX, with encap-ip 192.168.0.XY, with ip 10.0.X.Y > > +for i in 1 2; do > > + sim_add hv$i > > + as hv$i > > + ovs-vsctl add-br br-phys-$j > > + ovn_attach n1 br-phys-$j 192.168.0.${i}1 > > + ovs-vsctl set open . external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2 > > + > > + check ovn-nbctl ls-add ls$i -- \ > > + lrp-add lr lrp-ls$i f0:00:00:00:88:0$i 10.0.$i.88/24 -- \ > > + lsp-add ls$i lsp-lr-$i -- lsp-set-type lsp-lr-$i router -- \ > > + lsp-set-addresses lsp-lr-$i router -- \ > > + lsp-set-options lsp-lr-$i router-port=lrp-ls$i > > + > > + for j in 1 2; do > > + ovs-vsctl add-port br-int vif$i$j -- set Interface vif$i$j \ > > + external_ids:iface-id=lsp$i$j \ > > + external_ids:encap-ip=192.168.0.$i$j \ > > + options:tx_pcap=hv$i/vif$i$j-tx.pcap options:rxq_pcap=hv$i/vif$i$j-rx.pcap > > + ovn-nbctl lsp-add ls$i lsp$i$j -- lsp-set-addresses lsp$i$j "f0:00:00:00:00:$i$j 10.0.$i.$j" > > + > > + done > > +done > > + > > +wait_for_ports_up > > +check ovn-nbctl --wait=hv sync > > + > > +vif_to_hv() { > > + case $1 in dnl ( > > + vif[[1]]?) echo hv1 ;; dnl ( > > + vif[[2]]?) echo hv2 ;; dnl ( > > + *) AT_FAIL_IF([:]) ;; > > + esac > > +} > > + > > +vif_to_ip() { > > + vif=$1 > > + echo 10.0.${vif:3:1}.${vif:4:1} > > +} > > + > > +# check_packet_tunnel SRC_VIF DST_VIF > > +# Verify that a packet from SRC_VIF to DST_VIF goes through the corresponding > > +# tunnel that matches the VIFs' encap_ip configurations. > > +check_packet_tunnel() { > > + local src=$1 dst=$2 > > + local src_mac=f0:00:00:00:00:$src > > + local dst_mac=f0:00:00:00:88:01 # lrp-ls1's MAC > > + local src_ip=$(vif_to_ip vif$src) > > + local dst_ip=$(vif_to_ip vif$dst) > > + local local_encap_ip=192.168.0.$src > > + local remote_encap_ip=192.168.0.$dst > > + local packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/ \ > > + IP(dst='${dst_ip}', src='${src_ip}')/ \ > > + ICMP(type=8)") > > + hv=`vif_to_hv vif$src` > > + as $hv > > + echo "vif$src -> vif$dst should go through tunnel $local_encap_ip -> $remote_encap_ip" > > + tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip) > > + AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src $packet | grep "output:" | awk -F ':' '{ print $2 }') == $tunnel_ofport]) > > +} > > + > > +for i in 1 2; do > > + for j in 1 2; do > > + check_packet_tunnel 1$i 2$j > > + done > > +done > > + > > +OVN_CLEANUP([hv1],[hv2]) > > +AT_CLEANUP > > +]) > > + > > + > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([Load Balancer LS hairpin OF flows]) > > ovn_start > > -- > > 2.38.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/controller/physical.c b/controller/physical.c index c32642d2c69b..7ef259da44b1 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -73,7 +73,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, const struct zone_ids *zone_ids, size_t n_encap_ips, const char **encap_ips, - struct ofpbuf *ofpacts_p); + struct ofpbuf *ofpacts_p, + bool load_encap_id); static int64_t get_vxlan_port_key(int64_t port_key); /* UUID to identify OF flows not associated with ovsdb rows. */ @@ -689,7 +690,7 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones, ofpact_put_STRIP_VLAN(ofpacts_p); } load_logical_ingress_metadata(localnet_port, &zone_ids, 0, NULL, - ofpacts_p); + ofpacts_p, true); replace_mac = ofpact_put_SET_ETH_SRC(ofpacts_p); replace_mac->mac = router_port_mac; @@ -1047,7 +1048,8 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, const struct zone_ids *zone_ids, size_t n_encap_ips, const char **encap_ips, - struct ofpbuf *ofpacts_p) + struct ofpbuf *ofpacts_p, + bool load_encap_id) { put_zones_ofpacts(zone_ids, ofpacts_p); @@ -1057,13 +1059,15 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, ofpacts_p); put_load(port_key, MFF_LOG_INPORT, 0, 32, ofpacts_p); - /* Default encap_id 0. */ - size_t encap_id = 0; - if (encap_ips && binding->encap) { - encap_id = encap_ip_to_id(n_encap_ips, encap_ips, - binding->encap->ip); + if (load_encap_id) { + /* Default encap_id 0. */ + size_t encap_id = 0; + if (encap_ips && binding->encap) { + encap_id = encap_ip_to_id(n_encap_ips, encap_ips, + binding->encap->ip); + } + put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); } - put_load(encap_id, MFF_LOG_ENCAP_ID, 16, 16, ofpacts_p); } static const struct sbrec_port_binding * @@ -1108,7 +1112,7 @@ setup_rarp_activation_strategy(const struct sbrec_port_binding *binding, match_set_dl_type(&match, htons(ETH_TYPE_RARP)); match_set_in_port(&match, ofport); - load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts); + load_logical_ingress_metadata(binding, zone_ids, 0, NULL, &ofpacts, true); encode_controller_op(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, NX_CTLR_NO_METER, &ofpacts); @@ -1522,7 +1526,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, put_load(0, MFF_LOG_CT_ZONE, 0, 16, ofpacts_p); struct zone_ids peer_zones = get_zone_ids(peer, ct_zones); load_logical_ingress_metadata(peer, &peer_zones, n_encap_ips, - encap_ips, ofpacts_p); + encap_ips, ofpacts_p, false); put_load(0, MFF_LOG_FLAGS, 0, 32, ofpacts_p); put_load(0, MFF_LOG_OUTPORT, 0, 32, ofpacts_p); for (int i = 0; i < MFF_N_LOG_REGS; i++) { @@ -1739,7 +1743,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, uint32_t ofpacts_orig_size = ofpacts_p->size; load_logical_ingress_metadata(binding, &zone_ids, n_encap_ips, - encap_ips, ofpacts_p); + encap_ips, ofpacts_p, true); if (!strcmp(binding->type, "localport")) { /* mark the packet as incoming from a localport */ diff --git a/tests/ovn.at b/tests/ovn.at index 902dd3793b92..30748c96e1c6 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -30426,7 +30426,7 @@ AT_CLEANUP OVN_FOR_EACH_NORTHD([ -AT_SETUP([multiple encap ips selection based on VIF's encap_ip]) +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L2]) AT_SKIP_IF([test $HAVE_SCAPY = no]) ovn_start net_add n1 @@ -30521,6 +30521,87 @@ AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([multiple encap ips selection based on VIF's encap_ip - L3]) +AT_SKIP_IF([test $HAVE_SCAPY = no]) +ovn_start +net_add n1 + +ovn-nbctl lr-add lr + +# 2 HVs, each with 2 encap-ips, and each hosting a separate LS, connected by a LR. +# lspXY is the Yth LSP on hvX, with encap-ip 192.168.0.XY, with ip 10.0.X.Y +for i in 1 2; do + sim_add hv$i + as hv$i + ovs-vsctl add-br br-phys-$j + ovn_attach n1 br-phys-$j 192.168.0.${i}1 + ovs-vsctl set open . external_ids:ovn-encap-ip=192.168.0.${i}1,192.168.0.${i}2 + + check ovn-nbctl ls-add ls$i -- \ + lrp-add lr lrp-ls$i f0:00:00:00:88:0$i 10.0.$i.88/24 -- \ + lsp-add ls$i lsp-lr-$i -- lsp-set-type lsp-lr-$i router -- \ + lsp-set-addresses lsp-lr-$i router -- \ + lsp-set-options lsp-lr-$i router-port=lrp-ls$i + + for j in 1 2; do + ovs-vsctl add-port br-int vif$i$j -- set Interface vif$i$j \ + external_ids:iface-id=lsp$i$j \ + external_ids:encap-ip=192.168.0.$i$j \ + options:tx_pcap=hv$i/vif$i$j-tx.pcap options:rxq_pcap=hv$i/vif$i$j-rx.pcap + ovn-nbctl lsp-add ls$i lsp$i$j -- lsp-set-addresses lsp$i$j "f0:00:00:00:00:$i$j 10.0.$i.$j" + + done +done + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +vif_to_hv() { + case $1 in dnl ( + vif[[1]]?) echo hv1 ;; dnl ( + vif[[2]]?) echo hv2 ;; dnl ( + *) AT_FAIL_IF([:]) ;; + esac +} + +vif_to_ip() { + vif=$1 + echo 10.0.${vif:3:1}.${vif:4:1} +} + +# check_packet_tunnel SRC_VIF DST_VIF +# Verify that a packet from SRC_VIF to DST_VIF goes through the corresponding +# tunnel that matches the VIFs' encap_ip configurations. +check_packet_tunnel() { + local src=$1 dst=$2 + local src_mac=f0:00:00:00:00:$src + local dst_mac=f0:00:00:00:88:01 # lrp-ls1's MAC + local src_ip=$(vif_to_ip vif$src) + local dst_ip=$(vif_to_ip vif$dst) + local local_encap_ip=192.168.0.$src + local remote_encap_ip=192.168.0.$dst + local packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/ \ + IP(dst='${dst_ip}', src='${src_ip}')/ \ + ICMP(type=8)") + hv=`vif_to_hv vif$src` + as $hv + echo "vif$src -> vif$dst should go through tunnel $local_encap_ip -> $remote_encap_ip" + tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip) + AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src $packet | grep "output:" | awk -F ':' '{ print $2 }') == $tunnel_ofport]) +} + +for i in 1 2; do + for j in 1 2; do + check_packet_tunnel 1$i 2$j + done +done + +OVN_CLEANUP([hv1],[hv2]) +AT_CLEANUP +]) + + OVN_FOR_EACH_NORTHD([ AT_SETUP([Load Balancer LS hairpin OF flows]) ovn_start
The MFF_LOG_ENCAP_ID register was defined to save the encap ID and avoid changing across pipelines, but in the function load_logical_ingress_metadata it was reset unconditionally. Because of this, the encap selection doesn't work when traffic traverses L3 boundaries. This patch fixes it by ensuring the register is loaded only in the flows of table 0, which is where packets from VIFs enter the pipeline for the first time. Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs selection.") Signed-off-by: Han Zhou <hzhou@ovn.org> --- controller/physical.c | 28 ++++++++------- tests/ovn.at | 83 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 13 deletions(-)