| Message ID | 20240815165222.1144250-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] Reply only for the multicast ND solicitations. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 8/15/24 18:52, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > field. These flows when translated to datapath flows also match on > ip6.dst, which means a separate datapath flow per destination IP > address. This may cause significant performance issues in some > setups (particularly ovs-dpdk telco deployments). > > This patch addresses this issue by matching on eth.mcast6 so that > datapath flows for normal IPv6 traffic doesn't have to match on > ip6.dst. IPv6 NS packets are generally multicast. A new logical > match "nd_ns_mcast" is added for this purpose. > > After this patch, We no longer respond to IPv6 NS unicast packets. > Let the target reply to it, so that the sender has the ability to > monitor the targe liveness via the unicast ND solicitations. > This behavior now matches the IPv4 ARP responder flows. Note that > after the commit [1] which was recently added we now only respond > to IPv4 ARP broadcast packets. > > A recent patch [2] from Ilya partially addressed the same datapath > flow explosion issue by matching on eth.mcast6 for MLD packets. > With this patch, we now address the datapath flow explosion issue > for IPv6 traffic provided 2 conditions are met: > a. All the logical ports of a logical switch are not configured > with port security. > b. The logical switch port of type router if configured > with "arp_proxy" option doesn't include any IPv6 address(es). > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches for MLD and IGMP.") > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > missing from ovn-sb.xml and this patch adds it. > > Reported-at: https://issues.redhat.com/browse/FDP-728 > Reported-by: Mike Pattrick <mkp@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > > v2 -> v3 > ------- The CI failures look unrelated to the change to me: https://github.com/ovsrobot/ovn/actions/runs/10407451042/job/28823295402 https://github.com/ovsrobot/ovn/actions/runs/10407451041/job/28822927349 Recheck-request: github-robot
On Fri, Aug 16, 2024, 5:38 AM Dumitru Ceara <dceara@redhat.com> wrote: > On 8/15/24 18:52, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > > field. These flows when translated to datapath flows also match on > > ip6.dst, which means a separate datapath flow per destination IP > > address. This may cause significant performance issues in some > > setups (particularly ovs-dpdk telco deployments). > > > > This patch addresses this issue by matching on eth.mcast6 so that > > datapath flows for normal IPv6 traffic doesn't have to match on > > ip6.dst. IPv6 NS packets are generally multicast. A new logical > > match "nd_ns_mcast" is added for this purpose. > > > > After this patch, We no longer respond to IPv6 NS unicast packets. > > Let the target reply to it, so that the sender has the ability to > > monitor the targe liveness via the unicast ND solicitations. > > This behavior now matches the IPv4 ARP responder flows. Note that > > after the commit [1] which was recently added we now only respond > > to IPv4 ARP broadcast packets. > > > > A recent patch [2] from Ilya partially addressed the same datapath > > flow explosion issue by matching on eth.mcast6 for MLD packets. > > With this patch, we now address the datapath flow explosion issue > > for IPv6 traffic provided 2 conditions are met: > > a. All the logical ports of a logical switch are not configured > > with port security. > > b. The logical switch port of type router if configured > > with "arp_proxy" option doesn't include any IPv6 address(es). > > > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches for > MLD and IGMP.") > > > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > > missing from ovn-sb.xml and this patch adds it. > > > > Reported-at: https://issues.redhat.com/browse/FDP-728 > > Reported-by: Mike Pattrick <mkp@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > > > v2 -> v3 > > ------- > > The CI failures look unrelated to the change to me: > > https://github.com/ovsrobot/ovn/actions/runs/10407451042/job/28823295402 > https://github.com/ovsrobot/ovn/actions/runs/10407451041/job/28822927349 > > Recheck-request: github-robot > FYI - The CI has passed on my GitHub repo - https://github.com/numansiddique/ovn/commit/27e40a0ed759c0e535c1d13e319082137a24e52f Thanks Numan > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 8/15/24 18:52, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > field. These flows when translated to datapath flows also match on > ip6.dst, which means a separate datapath flow per destination IP > address. This may cause significant performance issues in some > setups (particularly ovs-dpdk telco deployments). > > This patch addresses this issue by matching on eth.mcast6 so that > datapath flows for normal IPv6 traffic doesn't have to match on > ip6.dst. IPv6 NS packets are generally multicast. A new logical > match "nd_ns_mcast" is added for this purpose. > > After this patch, We no longer respond to IPv6 NS unicast packets. > Let the target reply to it, so that the sender has the ability to > monitor the targe liveness via the unicast ND solicitations. > This behavior now matches the IPv4 ARP responder flows. Note that > after the commit [1] which was recently added we now only respond > to IPv4 ARP broadcast packets. > > A recent patch [2] from Ilya partially addressed the same datapath > flow explosion issue by matching on eth.mcast6 for MLD packets. > With this patch, we now address the datapath flow explosion issue > for IPv6 traffic provided 2 conditions are met: > a. All the logical ports of a logical switch are not configured > with port security. > b. The logical switch port of type router if configured > with "arp_proxy" option doesn't include any IPv6 address(es). > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches for MLD and IGMP.") > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > missing from ovn-sb.xml and this patch adds it. > > Reported-at: https://issues.redhat.com/browse/FDP-728 > Reported-by: Mike Pattrick <mkp@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- Hi Numan, The change looks good to me with the exception of some minor comments I had on the test, please see below. > > v2 -> v3 > ------- > - Removed the system test as it was flaky and intead added the test in > ovn.at and used ofproto/trace as suggested by Ilya to check the > megaflows. > > v1 -> v2 > ------- > - Addressed review comments from Ilya. > > > lib/logical-fields.c | 3 + > northd/northd.c | 21 ++++--- > ovn-sb.xml | 3 + > tests/ovn-northd.at | 4 +- > tests/ovn.at | 144 ++++++++++++++++++++++++++++++++++++++++++- > 5 files changed, 163 insertions(+), 12 deletions(-) > > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > index 2c9d3c61bf..5a8b53f2b6 100644 > --- a/lib/logical-fields.c > +++ b/lib/logical-fields.c > @@ -293,6 +293,9 @@ ovn_init_symtab(struct shash *symtab) > "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_ns", > "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255"); > + expr_symtab_add_predicate(symtab, "nd_ns_mcast", > + "ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && " > + "ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_na", > "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_rs", > diff --git a/northd/northd.c b/northd/northd.c > index c4824aae3b..c9ae1e958f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -9633,16 +9633,21 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > op->lflow_ref); > } > > - /* For ND solicitations, we need to listen for both the > - * unicast IPv6 address and its all-nodes multicast address, > - * but always respond with the unicast IPv6 address. */ > + /* For ND solicitations: > + * - Reply only for the all-nodes multicast address(es) of the > + * logical port IPv6 address(es). > + * > + * - Do not reply for unicast ND solicitations. Let the target > + * reply it, so that the sender has the ability to monitor > + * the targe liveness via the unicast ND solicitations. > + */ > for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) { > ds_clear(match); > - ds_put_format(match, > - "nd_ns && ip6.dst == {%s, %s} && nd.target == %s", > - op->lsp_addrs[i].ipv6_addrs[j].addr_s, > - op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > - op->lsp_addrs[i].ipv6_addrs[j].addr_s); > + ds_put_format( > + match, > + "nd_ns_mcast && ip6.dst == %s && nd.target == %s", > + op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > + op->lsp_addrs[i].ipv6_addrs[j].addr_s); > > ds_clear(actions); > ds_put_format(actions, > diff --git a/ovn-sb.xml b/ovn-sb.xml > index de0bd636fd..c11296d7c4 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -1146,6 +1146,7 @@ > <ul> > <li><code>eth.bcast</code> expands to <code>eth.dst == ff:ff:ff:ff:ff:ff</code></li> > <li><code>eth.mcast</code> expands to <code>eth.dst[40]</code></li> > + <li><code>eth.mcastv6</code> expands to <code>eth.dst[32..47] == 0x3333</code></li> > <li><code>vlan.present</code> expands to <code>vlan.tci[12]</code></li> > <li><code>ip4</code> expands to <code>eth.type == 0x800</code></li> > <li><code>ip4.src_mcast</code> expands to > @@ -1161,8 +1162,10 @@ > <li><code>ip.first_frag</code> expands to <code>ip.is_frag && !ip.later_frag</code></li> > <li><code>arp</code> expands to <code>eth.type == 0x806</code></li> > <li><code>rarp</code> expands to <code>eth.type == 0x8035</code></li> > + <li><code>ip6.mcast</code> expands to <code>eth.mcastv6 && ip6.dst[120..127] == 0xff</code></li> > <li><code>nd</code> expands to <code>icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255</code></li> > <li><code>nd_ns</code> expands to <code>icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255</code></li> > + <li><code>nd_ns_mcast</code> expands to <code>ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255</code></li> > <li><code>nd_na</code> expands to <code>icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255</code></li> > <li><code>nd_rs</code> expands to <code>icmp6.type == 133 && > icmp6.code == 0 && ip.ttl == 255</code></li> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 3c49c0d43b..93ccbce6b0 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -9503,9 +9503,9 @@ AT_CAPTURE_FILE([S1flows]) > AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | ovn_strip_lflows], [0], [dnl > table=??(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) > table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff && inport == "S1-vm1"), action=(next;) > - table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) > + table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns_mcast && ip6.dst == ff02::1:ff00:10 && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) > table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) > - table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) > + table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns_mcast && ip6.dst == ff02::1:ff00:10 && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) > ]) > > #Set the disable_arp_nd_rsp option and verify the flow > diff --git a/tests/ovn.at b/tests/ovn.at > index a1d689e849..b8cb831b80 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -8617,7 +8617,7 @@ done > # Complete Neighbor Solicitation packet and Neighbor Advertisement packet > # vif1 -> NS -> vif2. vif1 <- NA <- ovn-controller. > # vif2 will not receive NS packet, since ovn-controller will reply for it. > -ns_packet=3333ffa1f9aefa163e94059886dd6000000000203afffd81ce49a9480000f8163efffe940598fd81ce49a9480000f8163efffea1f9ae8700e01160000000fd81ce49a9480000f8163efffea1f9ae0101fa163e940598 > +ns_packet=3333ffa1f9aefa163e94059886dd6000000000203afffd81ce49a9480000f8163efffe940598ff0200000000000000000001ffa1f9ae8700e01160000000fd81ce49a9480000f8163efffea1f9ae0101fa163e940598 > na_packet=fa163e940598fa163ea1f9ae86dd6000000000203afffd81ce49a9480000f8163efffea1f9aefd81ce49a9480000f8163efffe9405988800e9ed60000000fd81ce49a9480000f8163efffea1f9ae0201fa163ea1f9ae > > as hv1 ovs-appctl netdev-dummy/receive vif1 $ns_packet > @@ -14417,7 +14417,7 @@ test_ns_na() { > local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 > > packet=$(fmt_pkt " > - Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') / > + Ether(dst='33:33:ff:ff:ff:ff', src='${src_mac}') / > IPv6(src='${src_ip}', dst='ff02::1:ff00:2') / > ICMPv6ND_NS(tgt='${dst_ip}') > ") > @@ -38756,3 +38756,143 @@ OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > ]) > + > +dnl This test checks that the megaflows translated by ovs-vswitchd > +dnl don't match on IPv6 source and destination addresses for > +dnl simple switching. > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([IPv6 switching - megaflow check for IPv6 src/dst matches]) > +ovn_start > + > +check ovn-nbctl ls-add sw0 > + > +check ovn-nbctl lsp-add sw0 vm0 > +check ovn-nbctl lsp-set-addresses vm0 "f0:00:0f:01:02:03 10.0.0.3 1000::3" > + > +check ovn-nbctl lsp-add sw0 vm1 > +check ovn-nbctl lsp-set-addresses vm1 "f0:00:0f:01:02:04 10.0.0.4 1000::4" > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl lrp-add lr0 lr0-sw0 fa:16:3e:00:00:01 10.0.0.1 1000::1/64 > +check ovn-nbctl lsp-add sw0 sw0-lr0 > +check ovn-nbctl lsp-set-type sw0-lr0 router > +check ovn-nbctl lsp-set-addresses sw0-lr0 router > +check ovn-nbctl --wait=hv lsp-set-options sw0-lr0 router-port=lr0-sw0 > + > +net_add n1 > +sim_add hv > +as hv > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +check ovs-vsctl add-port br-int vif1 -- set Interface vif1 \ > +external-ids:iface-id=vm0 options:tx_pcap=hv/vif1-tx.pcap \ > +options:rxq_pcap=hv/vif1-rx.pcap ofport-request=1 > +check ovs-vsctl add-port br-int vif2 -- set Interface vif2 \ > +external-ids:iface-id=vm1 options:tx_pcap=hv/vif2-tx.pcap \ > +options:rxq_pcap=hv/vif2-rx.pcap ofport-request=2 Nit: I'd indent this a bit differently, e.g.: check ovs-vsctl add-port br-int vif1 -- \ set Interface vif1 external-ids:iface-id=vm0 \ options:tx_pcap=hv/vif1-tx.pcap \ options:rxq_pcap=hv/vif1-rx.pcap \ ofport-request=1 check ovs-vsctl add-port br-int vif2 -- \ set Interface vif2 external-ids:iface-id=vm1 \ options:tx_pcap=hv/vif2-tx.pcap \ options:rxq_pcap=hv/vif2-rx.pcap \ ofport-request=2 > + > +wait_for_ports_up vm0 vm1 > + Nit: I'd check for all ports and also wait for --wait=hv instead: check ovn-nbctl --wait=sb sync wait_for_ports_up > +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:04', src='f0:00:0f:01:02:03')/ \ > + IPv6(dst='1000::4', src='1000::3')/ \ > + UDP(sport=53, dport=4369)") > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif1 $packet > + > +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl Nit: extra space after 'Megaflow'. > +0 > +]) > + > +dnl Make sure that the packet was received by vm1. This ensures that the > +dnl packet was delivered as expected and the megaflow didn't have any matches > +dnl on IPv6 src or dst. > + > +echo $packet > expected > +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected]) > + > +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ > + IPv6(dst='1000::3', src='1000::4')/ \ > + UDP(sport=53, dport=4369)") > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=2 $packet > vm1_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif2 $packet > + > +AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl Nit: extra space after 'Megaflow'. > +0 > +]) > + > +dnl Make sure that the packet was received by vm0. This ensures that the > +dnl packet was delivered as expected and the megaflow didn't have any matches > +dnl on IPv6 src or dst. > +echo $packet > expected > +OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected]) > + > +dnl Add port security to vm0. The megaflow should now match on ipv6 src/dst. > +check ovn-nbctl lsp-set-port-security vm0 "f0:00:0f:01:02:03 10.0.0.3 1000::3" > +check ovn-nbctl --wait=hv sync > + > +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:04', src='f0:00:0f:01:02:03')/ \ > + IPv6(dst='1000::4', src='1000::3')/ \ > + UDP(sport=53, dport=4369)") > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif1 $packet > + > +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl Nit: extra space after 'Megaflow'. > +1 > +]) > + > +dnl Clear port security. > +check ovn-nbctl lsp-set-port-security vm0 "" > +check ovn-nbctl --wait=hv sync > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif1 $packet From this point on we don't really check that the packet is actually received on the other side. So the netdev-dummy/receive calls don't really do anything. We could check that the packets are received for the remaining cases too. > + > +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl Nit: extra space after 'Megaflow'. > +0 > +]) > + > +dnl Configure proxy arp/nd on the router port. The megaflow should now match > +dnl on ipv6 src/dst. > +check ovn-nbctl --wait=hv lsp-set-options sw0-lr0 router-port=lr0-sw0 arp_proxy="2000::1/64" > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif1 $packet > + > +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl Nit: extra space after 'Megaflow'. > +1 > +]) > + > +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ > + IPv6(dst='1000::3', src='1000::4')/ \ > + UDP(sport=53, dport=4369)") > + > +as hv > +ovs-appctl ofproto/trace br-int in_port=2 $packet > vm1_ip6_ofproto_trace.txt > +ovs-appctl netdev-dummy/receive vif2 $packet > + > +AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) > + > +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl Nit: extra space after 'Megaflow'. > +1 > +]) > + > +AT_CLEANUP > +]) Here's an incremental diff that covers the nits I had above (I also added some AS_BOX calls to make it more clear where in the test we are). If you feel ok with folding that in please also add my ack: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru diff --git a/tests/ovn.at b/tests/ovn.at index b8cb831b80..f9b5d70671 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -38784,15 +38784,21 @@ sim_add hv as hv check ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.1 -check ovs-vsctl add-port br-int vif1 -- set Interface vif1 \ -external-ids:iface-id=vm0 options:tx_pcap=hv/vif1-tx.pcap \ -options:rxq_pcap=hv/vif1-rx.pcap ofport-request=1 -check ovs-vsctl add-port br-int vif2 -- set Interface vif2 \ -external-ids:iface-id=vm1 options:tx_pcap=hv/vif2-tx.pcap \ -options:rxq_pcap=hv/vif2-rx.pcap ofport-request=2 +check ovs-vsctl add-port br-int vif1 -- \ + set Interface vif1 external-ids:iface-id=vm0 \ + options:tx_pcap=hv/vif1-tx.pcap \ + options:rxq_pcap=hv/vif1-rx.pcap \ + ofport-request=1 +check ovs-vsctl add-port br-int vif2 -- \ + set Interface vif2 external-ids:iface-id=vm1 \ + options:tx_pcap=hv/vif2-tx.pcap \ + options:rxq_pcap=hv/vif2-rx.pcap \ + ofport-request=2 -wait_for_ports_up vm0 vm1 +check ovn-nbctl --wait=sb sync +wait_for_ports_up +AS_BOX([No port security, to vm1]) packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:04', src='f0:00:0f:01:02:03')/ \ IPv6(dst='1000::4', src='1000::3')/ \ UDP(sport=53, dport=4369)") @@ -38803,7 +38809,7 @@ ovs-appctl netdev-dummy/receive vif1 $packet AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl 0 ]) @@ -38811,9 +38817,10 @@ dnl Make sure that the packet was received by vm1. This ensures that the dnl packet was delivered as expected and the megaflow didn't have any matches dnl on IPv6 src or dst. -echo $packet > expected -OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected]) +echo $packet >> expected-vif2 +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected-vif2]) +AS_BOX([No port security, to vm0]) packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ IPv6(dst='1000::3', src='1000::4')/ \ UDP(sport=53, dport=4369)") @@ -38824,16 +38831,17 @@ ovs-appctl netdev-dummy/receive vif2 $packet AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl 0 ]) dnl Make sure that the packet was received by vm0. This ensures that the dnl packet was delivered as expected and the megaflow didn't have any matches dnl on IPv6 src or dst. -echo $packet > expected -OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected]) +echo $packet >> expected-vif1 +OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1]) +AS_BOX([With port security, to vm1]) dnl Add port security to vm0. The megaflow should now match on ipv6 src/dst. check ovn-nbctl lsp-set-port-security vm0 "f0:00:0f:01:02:03 10.0.0.3 1000::3" check ovn-nbctl --wait=hv sync @@ -38848,10 +38856,15 @@ ovs-appctl netdev-dummy/receive vif1 $packet AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl 1 ]) +dnl Make sure that the packet was received by vm1. +echo $packet >> expected-vif2 +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected-vif2]) + +AS_BOX([Clear port security, to vm1]) dnl Clear port security. check ovn-nbctl lsp-set-port-security vm0 "" check ovn-nbctl --wait=hv sync @@ -38862,10 +38875,15 @@ ovs-appctl netdev-dummy/receive vif1 $packet AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl 0 ]) +dnl Make sure that the packet was received by vm1. +echo $packet >> expected-vif2 +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected-vif2]) + +AS_BOX([With proxy arp/nd, to vm1]) dnl Configure proxy arp/nd on the router port. The megaflow should now match dnl on ipv6 src/dst. check ovn-nbctl --wait=hv lsp-set-options sw0-lr0 router-port=lr0-sw0 arp_proxy="2000::1/64" @@ -38876,10 +38894,15 @@ ovs-appctl netdev-dummy/receive vif1 $packet AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl 1 ]) +dnl Make sure that the packet was received by vm1. +echo $packet >> expected-vif2 +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected-vif2]) + +AS_BOX([With proxy arp/nd, to vm0]) packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ IPv6(dst='1000::3', src='1000::4')/ \ UDP(sport=53, dport=4369)") @@ -38890,9 +38913,13 @@ ovs-appctl netdev-dummy/receive vif2 $packet AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) -AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl 1 ]) +dnl Make sure that the packet was received by vm0. +echo $packet >> expected-vif1 +OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected-vif1]) + AT_CLEANUP ])
On 8/16/24 12:04, Numan Siddique wrote: > > > On Fri, Aug 16, 2024, 5:38 AM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> wrote: > > On 8/15/24 18:52, numans@ovn.org <mailto:numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > > > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > > field. These flows when translated to datapath flows also match on > > ip6.dst, which means a separate datapath flow per destination IP > > address. This may cause significant performance issues in some > > setups (particularly ovs-dpdk telco deployments). > > > > This patch addresses this issue by matching on eth.mcast6 so that > > datapath flows for normal IPv6 traffic doesn't have to match on > > ip6.dst. IPv6 NS packets are generally multicast. A new logical > > match "nd_ns_mcast" is added for this purpose. > > > > After this patch, We no longer respond to IPv6 NS unicast packets. > > Let the target reply to it, so that the sender has the ability to > > monitor the targe liveness via the unicast ND solicitations. > > This behavior now matches the IPv4 ARP responder flows. Note that > > after the commit [1] which was recently added we now only respond > > to IPv4 ARP broadcast packets. > > > > A recent patch [2] from Ilya partially addressed the same datapath > > flow explosion issue by matching on eth.mcast6 for MLD packets. > > With this patch, we now address the datapath flow explosion issue > > for IPv6 traffic provided 2 conditions are met: > > a. All the logical ports of a logical switch are not configured > > with port security. > > b. The logical switch port of type router if configured > > with "arp_proxy" option doesn't include any IPv6 address(es). > > > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches > for MLD and IGMP.") > > > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > > missing from ovn-sb.xml and this patch adds it. > > > > Reported-at: https://issues.redhat.com/browse/FDP-728 > <https://issues.redhat.com/browse/FDP-728> > > Reported-by: Mike Pattrick <mkp@redhat.com <mailto:mkp@redhat.com>> > > Signed-off-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > > --- > > > > v2 -> v3 > > ------- > > The CI failures look unrelated to the change to me: > > https://github.com/ovsrobot/ovn/actions/runs/10407451042/job/28823295402 <https://github.com/ovsrobot/ovn/actions/runs/10407451042/job/28823295402> > https://github.com/ovsrobot/ovn/actions/runs/10407451041/job/28822927349 <https://github.com/ovsrobot/ovn/actions/runs/10407451041/job/28822927349> > > Recheck-request: github-robot > > > FYI - The CI has passed on my GitHub repo > - https://github.com/numansiddique/ovn/commit/27e40a0ed759c0e535c1d13e319082137a24e52f <https://github.com/numansiddique/ovn/commit/27e40a0ed759c0e535c1d13e319082137a24e52f> > Yes, it passes in my fork too. We do probably need to investigate those CI flakes at some point in the future and figure out if the test is broken or if there's an actual issue hiding somewhere. Worst case we could mark the tests as "unstable" so they get re-run in CI. Regards, Dumitru
On 8/15/24 18:52, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > field. These flows when translated to datapath flows also match on > ip6.dst, which means a separate datapath flow per destination IP > address. This may cause significant performance issues in some > setups (particularly ovs-dpdk telco deployments). > > This patch addresses this issue by matching on eth.mcast6 so that > datapath flows for normal IPv6 traffic doesn't have to match on > ip6.dst. IPv6 NS packets are generally multicast. A new logical > match "nd_ns_mcast" is added for this purpose. > > After this patch, We no longer respond to IPv6 NS unicast packets. > Let the target reply to it, so that the sender has the ability to > monitor the targe liveness via the unicast ND solicitations. > This behavior now matches the IPv4 ARP responder flows. Note that > after the commit [1] which was recently added we now only respond > to IPv4 ARP broadcast packets. > > A recent patch [2] from Ilya partially addressed the same datapath > flow explosion issue by matching on eth.mcast6 for MLD packets. > With this patch, we now address the datapath flow explosion issue > for IPv6 traffic provided 2 conditions are met: > a. All the logical ports of a logical switch are not configured > with port security. > b. The logical switch port of type router if configured > with "arp_proxy" option doesn't include any IPv6 address(es). > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches for MLD and IGMP.") > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > missing from ovn-sb.xml and this patch adds it. > > Reported-at: https://issues.redhat.com/browse/FDP-728 > Reported-by: Mike Pattrick <mkp@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > > v2 -> v3 > ------- > - Removed the system test as it was flaky and intead added the test in > ovn.at and used ofproto/trace as suggested by Ilya to check the > megaflows. > > v1 -> v2 > ------- > - Addressed review comments from Ilya. > > > lib/logical-fields.c | 3 + > northd/northd.c | 21 ++++--- > ovn-sb.xml | 3 + > tests/ovn-northd.at | 4 +- > tests/ovn.at | 144 ++++++++++++++++++++++++++++++++++++++++++- > 5 files changed, 163 insertions(+), 12 deletions(-) Thanks, Numan! I didn't review the test, but the code looks good to me. There is a couple of typos in the comment, see below. For the code with typos fixed: Acked-by: Ilya Maximets <i.maximets@ovn.org> > > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > index 2c9d3c61bf..5a8b53f2b6 100644 > --- a/lib/logical-fields.c > +++ b/lib/logical-fields.c > @@ -293,6 +293,9 @@ ovn_init_symtab(struct shash *symtab) > "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_ns", > "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255"); > + expr_symtab_add_predicate(symtab, "nd_ns_mcast", > + "ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && " > + "ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_na", > "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255"); > expr_symtab_add_predicate(symtab, "nd_rs", > diff --git a/northd/northd.c b/northd/northd.c > index c4824aae3b..c9ae1e958f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -9633,16 +9633,21 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > op->lflow_ref); > } > > - /* For ND solicitations, we need to listen for both the > - * unicast IPv6 address and its all-nodes multicast address, > - * but always respond with the unicast IPv6 address. */ > + /* For ND solicitations: > + * - Reply only for the all-nodes multicast address(es) of the > + * logical port IPv6 address(es). > + * > + * - Do not reply for unicast ND solicitations. Let the target > + * reply it, so that the sender has the ability to monitor "reply to it" ? > + * the targe liveness via the unicast ND solicitations. *target > + */ > for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) { > ds_clear(match); > - ds_put_format(match, > - "nd_ns && ip6.dst == {%s, %s} && nd.target == %s", > - op->lsp_addrs[i].ipv6_addrs[j].addr_s, > - op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > - op->lsp_addrs[i].ipv6_addrs[j].addr_s); > + ds_put_format( > + match, > + "nd_ns_mcast && ip6.dst == %s && nd.target == %s", > + op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > + op->lsp_addrs[i].ipv6_addrs[j].addr_s); > > ds_clear(actions); > ds_put_format(actions, Best regards, Ilya Maximets.
On Fri, Aug 16, 2024 at 8:13 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 8/15/24 18:52, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > IPv6 ND Solicitation (NS) responder logical flows match on ip6.dst > > field. These flows when translated to datapath flows also match on > > ip6.dst, which means a separate datapath flow per destination IP > > address. This may cause significant performance issues in some > > setups (particularly ovs-dpdk telco deployments). > > > > This patch addresses this issue by matching on eth.mcast6 so that > > datapath flows for normal IPv6 traffic doesn't have to match on > > ip6.dst. IPv6 NS packets are generally multicast. A new logical > > match "nd_ns_mcast" is added for this purpose. > > > > After this patch, We no longer respond to IPv6 NS unicast packets. > > Let the target reply to it, so that the sender has the ability to > > monitor the targe liveness via the unicast ND solicitations. > > This behavior now matches the IPv4 ARP responder flows. Note that > > after the commit [1] which was recently added we now only respond > > to IPv4 ARP broadcast packets. > > > > A recent patch [2] from Ilya partially addressed the same datapath > > flow explosion issue by matching on eth.mcast6 for MLD packets. > > With this patch, we now address the datapath flow explosion issue > > for IPv6 traffic provided 2 conditions are met: > > a. All the logical ports of a logical switch are not configured > > with port security. > > b. The logical switch port of type router if configured > > with "arp_proxy" option doesn't include any IPv6 address(es). > > > > [1] - c48ed1736a58 ("Do not reply on unicast arps for IPv4 targets.") > > [2] - 43c34f2e6676 ("logical-fields: Add missing multicast matches for MLD and IGMP.") > > > > Note: Documentation for 'eth.mcastv6' and 'ip6.mcast' predicates were > > missing from ovn-sb.xml and this patch adds it. > > > > Reported-at: https://issues.redhat.com/browse/FDP-728 > > Reported-by: Mike Pattrick <mkp@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > > > v2 -> v3 > > ------- > > - Removed the system test as it was flaky and intead added the test in > > ovn.at and used ofproto/trace as suggested by Ilya to check the > > megaflows. > > > > v1 -> v2 > > ------- > > - Addressed review comments from Ilya. > > > > > > lib/logical-fields.c | 3 + > > northd/northd.c | 21 ++++--- > > ovn-sb.xml | 3 + > > tests/ovn-northd.at | 4 +- > > tests/ovn.at | 144 ++++++++++++++++++++++++++++++++++++++++++- > > 5 files changed, 163 insertions(+), 12 deletions(-) > > Thanks, Numan! I didn't review the test, but the code looks good > to me. There is a couple of typos in the comment, see below. > > For the code with typos fixed: > > Acked-by: Ilya Maximets <i.maximets@ovn.org> > > > > > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > > index 2c9d3c61bf..5a8b53f2b6 100644 > > --- a/lib/logical-fields.c > > +++ b/lib/logical-fields.c > > @@ -293,6 +293,9 @@ ovn_init_symtab(struct shash *symtab) > > "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255"); > > expr_symtab_add_predicate(symtab, "nd_ns", > > "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255"); > > + expr_symtab_add_predicate(symtab, "nd_ns_mcast", > > + "ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && " > > + "ip.ttl == 255"); > > expr_symtab_add_predicate(symtab, "nd_na", > > "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255"); > > expr_symtab_add_predicate(symtab, "nd_rs", > > diff --git a/northd/northd.c b/northd/northd.c > > index c4824aae3b..c9ae1e958f 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -9633,16 +9633,21 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > > op->lflow_ref); > > } > > > > - /* For ND solicitations, we need to listen for both the > > - * unicast IPv6 address and its all-nodes multicast address, > > - * but always respond with the unicast IPv6 address. */ > > + /* For ND solicitations: > > + * - Reply only for the all-nodes multicast address(es) of the > > + * logical port IPv6 address(es). > > + * > > + * - Do not reply for unicast ND solicitations. Let the target > > + * reply it, so that the sender has the ability to monitor > > "reply to it" ? > > > + * the targe liveness via the unicast ND solicitations. > > *target > > > + */ > > for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) { > > ds_clear(match); > > - ds_put_format(match, > > - "nd_ns && ip6.dst == {%s, %s} && nd.target == %s", > > - op->lsp_addrs[i].ipv6_addrs[j].addr_s, > > - op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > > - op->lsp_addrs[i].ipv6_addrs[j].addr_s); > > + ds_put_format( > > + match, > > + "nd_ns_mcast && ip6.dst == %s && nd.target == %s", > > + op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, > > + op->lsp_addrs[i].ipv6_addrs[j].addr_s); > > > > ds_clear(actions); > > ds_put_format(actions, > > Best regards, Ilya Maximets. Thanks Dumitru and Ilya for the reviews. I applied this patch to the main and branch-24.09 after addressing the review comments. After the CI passes for other branches in my private github CI, I'll backport to other branches. Numan > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 2c9d3c61bf..5a8b53f2b6 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -293,6 +293,9 @@ ovn_init_symtab(struct shash *symtab) "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255"); expr_symtab_add_predicate(symtab, "nd_ns", "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255"); + expr_symtab_add_predicate(symtab, "nd_ns_mcast", + "ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && " + "ip.ttl == 255"); expr_symtab_add_predicate(symtab, "nd_na", "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255"); expr_symtab_add_predicate(symtab, "nd_rs", diff --git a/northd/northd.c b/northd/northd.c index c4824aae3b..c9ae1e958f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -9633,16 +9633,21 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, op->lflow_ref); } - /* For ND solicitations, we need to listen for both the - * unicast IPv6 address and its all-nodes multicast address, - * but always respond with the unicast IPv6 address. */ + /* For ND solicitations: + * - Reply only for the all-nodes multicast address(es) of the + * logical port IPv6 address(es). + * + * - Do not reply for unicast ND solicitations. Let the target + * reply it, so that the sender has the ability to monitor + * the targe liveness via the unicast ND solicitations. + */ for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) { ds_clear(match); - ds_put_format(match, - "nd_ns && ip6.dst == {%s, %s} && nd.target == %s", - op->lsp_addrs[i].ipv6_addrs[j].addr_s, - op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, - op->lsp_addrs[i].ipv6_addrs[j].addr_s); + ds_put_format( + match, + "nd_ns_mcast && ip6.dst == %s && nd.target == %s", + op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s, + op->lsp_addrs[i].ipv6_addrs[j].addr_s); ds_clear(actions); ds_put_format(actions, diff --git a/ovn-sb.xml b/ovn-sb.xml index de0bd636fd..c11296d7c4 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -1146,6 +1146,7 @@ <ul> <li><code>eth.bcast</code> expands to <code>eth.dst == ff:ff:ff:ff:ff:ff</code></li> <li><code>eth.mcast</code> expands to <code>eth.dst[40]</code></li> + <li><code>eth.mcastv6</code> expands to <code>eth.dst[32..47] == 0x3333</code></li> <li><code>vlan.present</code> expands to <code>vlan.tci[12]</code></li> <li><code>ip4</code> expands to <code>eth.type == 0x800</code></li> <li><code>ip4.src_mcast</code> expands to @@ -1161,8 +1162,10 @@ <li><code>ip.first_frag</code> expands to <code>ip.is_frag && !ip.later_frag</code></li> <li><code>arp</code> expands to <code>eth.type == 0x806</code></li> <li><code>rarp</code> expands to <code>eth.type == 0x8035</code></li> + <li><code>ip6.mcast</code> expands to <code>eth.mcastv6 && ip6.dst[120..127] == 0xff</code></li> <li><code>nd</code> expands to <code>icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl == 255</code></li> <li><code>nd_ns</code> expands to <code>icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255</code></li> + <li><code>nd_ns_mcast</code> expands to <code>ip6.mcast && icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255</code></li> <li><code>nd_na</code> expands to <code>icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255</code></li> <li><code>nd_rs</code> expands to <code>icmp6.type == 133 && icmp6.code == 0 && ip.ttl == 255</code></li> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 3c49c0d43b..93ccbce6b0 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9503,9 +9503,9 @@ AT_CAPTURE_FILE([S1flows]) AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | ovn_strip_lflows], [0], [dnl table=??(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff && inport == "S1-vm1"), action=(next;) - table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) + table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns_mcast && ip6.dst == ff02::1:ff00:10 && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) - table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) + table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns_mcast && ip6.dst == ff02::1:ff00:10 && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) ]) #Set the disable_arp_nd_rsp option and verify the flow diff --git a/tests/ovn.at b/tests/ovn.at index a1d689e849..b8cb831b80 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8617,7 +8617,7 @@ done # Complete Neighbor Solicitation packet and Neighbor Advertisement packet # vif1 -> NS -> vif2. vif1 <- NA <- ovn-controller. # vif2 will not receive NS packet, since ovn-controller will reply for it. -ns_packet=3333ffa1f9aefa163e94059886dd6000000000203afffd81ce49a9480000f8163efffe940598fd81ce49a9480000f8163efffea1f9ae8700e01160000000fd81ce49a9480000f8163efffea1f9ae0101fa163e940598 +ns_packet=3333ffa1f9aefa163e94059886dd6000000000203afffd81ce49a9480000f8163efffe940598ff0200000000000000000001ffa1f9ae8700e01160000000fd81ce49a9480000f8163efffea1f9ae0101fa163e940598 na_packet=fa163e940598fa163ea1f9ae86dd6000000000203afffd81ce49a9480000f8163efffea1f9aefd81ce49a9480000f8163efffe9405988800e9ed60000000fd81ce49a9480000f8163efffea1f9ae0201fa163ea1f9ae as hv1 ovs-appctl netdev-dummy/receive vif1 $ns_packet @@ -14417,7 +14417,7 @@ test_ns_na() { local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 packet=$(fmt_pkt " - Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') / + Ether(dst='33:33:ff:ff:ff:ff', src='${src_mac}') / IPv6(src='${src_ip}', dst='ff02::1:ff00:2') / ICMPv6ND_NS(tgt='${dst_ip}') ") @@ -38756,3 +38756,143 @@ OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) + +dnl This test checks that the megaflows translated by ovs-vswitchd +dnl don't match on IPv6 source and destination addresses for +dnl simple switching. +OVN_FOR_EACH_NORTHD([ +AT_SETUP([IPv6 switching - megaflow check for IPv6 src/dst matches]) +ovn_start + +check ovn-nbctl ls-add sw0 + +check ovn-nbctl lsp-add sw0 vm0 +check ovn-nbctl lsp-set-addresses vm0 "f0:00:0f:01:02:03 10.0.0.3 1000::3" + +check ovn-nbctl lsp-add sw0 vm1 +check ovn-nbctl lsp-set-addresses vm1 "f0:00:0f:01:02:04 10.0.0.4 1000::4" + +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-sw0 fa:16:3e:00:00:01 10.0.0.1 1000::1/64 +check ovn-nbctl lsp-add sw0 sw0-lr0 +check ovn-nbctl lsp-set-type sw0-lr0 router +check ovn-nbctl lsp-set-addresses sw0-lr0 router +check ovn-nbctl --wait=hv lsp-set-options sw0-lr0 router-port=lr0-sw0 + +net_add n1 +sim_add hv +as hv +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl add-port br-int vif1 -- set Interface vif1 \ +external-ids:iface-id=vm0 options:tx_pcap=hv/vif1-tx.pcap \ +options:rxq_pcap=hv/vif1-rx.pcap ofport-request=1 +check ovs-vsctl add-port br-int vif2 -- set Interface vif2 \ +external-ids:iface-id=vm1 options:tx_pcap=hv/vif2-tx.pcap \ +options:rxq_pcap=hv/vif2-rx.pcap ofport-request=2 + +wait_for_ports_up vm0 vm1 + +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:04', src='f0:00:0f:01:02:03')/ \ + IPv6(dst='1000::4', src='1000::3')/ \ + UDP(sport=53, dport=4369)") + +as hv +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif1 $packet + +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +0 +]) + +dnl Make sure that the packet was received by vm1. This ensures that the +dnl packet was delivered as expected and the megaflow didn't have any matches +dnl on IPv6 src or dst. + +echo $packet > expected +OVN_CHECK_PACKETS([hv/vif2-tx.pcap], [expected]) + +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ + IPv6(dst='1000::3', src='1000::4')/ \ + UDP(sport=53, dport=4369)") + +as hv +ovs-appctl ofproto/trace br-int in_port=2 $packet > vm1_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif2 $packet + +AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +0 +]) + +dnl Make sure that the packet was received by vm0. This ensures that the +dnl packet was delivered as expected and the megaflow didn't have any matches +dnl on IPv6 src or dst. +echo $packet > expected +OVN_CHECK_PACKETS([hv/vif1-tx.pcap], [expected]) + +dnl Add port security to vm0. The megaflow should now match on ipv6 src/dst. +check ovn-nbctl lsp-set-port-security vm0 "f0:00:0f:01:02:03 10.0.0.3 1000::3" +check ovn-nbctl --wait=hv sync + +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:04', src='f0:00:0f:01:02:03')/ \ + IPv6(dst='1000::4', src='1000::3')/ \ + UDP(sport=53, dport=4369)") + +as hv +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif1 $packet + +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +1 +]) + +dnl Clear port security. +check ovn-nbctl lsp-set-port-security vm0 "" +check ovn-nbctl --wait=hv sync + +as hv +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif1 $packet + +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [1], [dnl +0 +]) + +dnl Configure proxy arp/nd on the router port. The megaflow should now match +dnl on ipv6 src/dst. +check ovn-nbctl --wait=hv lsp-set-options sw0-lr0 router-port=lr0-sw0 arp_proxy="2000::1/64" + +as hv +ovs-appctl ofproto/trace br-int in_port=1 $packet > vm0_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif1 $packet + +AT_CAPTURE_FILE([vm0_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +1 +]) + +packet=$(fmt_pkt "Ether(dst='f0:00:0f:01:02:03', src='f0:00:0f:01:02:04')/ \ + IPv6(dst='1000::3', src='1000::4')/ \ + UDP(sport=53, dport=4369)") + +as hv +ovs-appctl ofproto/trace br-int in_port=2 $packet > vm1_ip6_ofproto_trace.txt +ovs-appctl netdev-dummy/receive vif2 $packet + +AT_CAPTURE_FILE([vm1_ip6_ofproto_trace.txt]) + +AT_CHECK([grep Megaflow vm0_ip6_ofproto_trace.txt | grep -e ipv6_src -e ipv6_dst -c], [0], [dnl +1 +]) + +AT_CLEANUP +])