On 8/15/24 18:52, num...@ovn.org wrote: > From: Numan Siddique <num...@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 <m...@redhat.com> > Signed-off-by: Numan Siddique <num...@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.maxim...@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. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev