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
