On 7/22/26 10:18 AM, Ales Musil via dev wrote:
> Similarly to 80187a8031b6 [0], use eth.src for nd_lookup if nd.tll is zero.
> As per RFC 4861 nd.tll is allowed to be 0 in reply to unicast ND NS. This
> enables the MAC Binding refresh to work when we receive reply to unicast
> ND NS, it should also prevent unnecessary pinctrl calls when we already
> know given MAC Binding.
Hi, Ales. Thanks for the patch! The change looks good to me in general,
just a few comments below.
>
> [0] 80187a8031b6 ("ovn-northd: Add flow to use eth.src if nd.tll is 0 in
> put_nd() action.")
>
> Signed-off-by: Ales Musil <[email protected]>
The change seems to be fixing an issue. Do we need a fixes tag here?
> ---
> northd/northd.c | 20 ++++++++++++++++++++
> tests/ovn-northd.at | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 22108dd82..14e1d699b 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -14899,6 +14899,7 @@ build_neigh_learning_flows_for_lrouter(
> * REGBIT_LOOKUP_NEIGHBOR_RESULT bit. If
> * "always_learn_from_arp_request" is set to false,
> * REGBIT_LOOKUP_NEIGHBOR_IP_RESULT bit is set.
> + * The lookup backs up to eth.src in case when nd.tll == 0.
> *
> * For IPv6 ND NS packets, table LOOKUP_NEIGHBOR does a lookup
> * for the (ip6.src, nd.sll) in the mac binding table using the
> @@ -14936,6 +14937,14 @@ build_neigh_learning_flows_for_lrouter(
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100, "nd_na",
> ds_cstr(actions), lflow_ref);
>
> + ds_clear(actions);
> + ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT
> + " = lookup_nd(inport, nd.target, eth.src); %s%snext;",
> + learn_from_arp_request ? "" :
> + REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" = 1; ", flood);
> + ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 105,
> + "nd_na && nd.tll == 0", ds_cstr(actions), lflow_ref);
> +
> if (!learn_from_arp_request) {
> /* Add flow to skip GARP LLA if we don't know it already.
> * From RFC 2461, section 4.4, Neighbor Advertisement Message
> @@ -14953,6 +14962,17 @@ build_neigh_learning_flows_for_lrouter(
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110,
> "nd_na && ip6.src == fe80::/10 && ip6.dst == ff00::/8",
> ds_cstr(actions), lflow_ref);
> +
> + ds_clear(actions);
> + ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT
> + " = lookup_nd(inport, nd.target, eth.src); "
> + REGBIT_LOOKUP_NEIGHBOR_IP_RESULT
> + " = lookup_nd_ip(inport, nd.target); %snext;",
> + flood);
> + ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 115,
> + "nd_na && nd.tll == 0 && "
> + "ip6.src == fe80::/10 && ip6.dst == ff00::/8",
> + ds_cstr(actions), lflow_ref);
> }
>
> ds_clear(actions);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 562e6a0e8..a61a70035 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -9317,6 +9317,7 @@ AT_CHECK([cat lrflows | grep -e lr_in_lookup_neighbor
> -e lr_in_learn_neighbor |
> table=??(lr_in_lookup_neighbor), priority=100 , match=(arp.op == 2),
> action=(reg9[[2]] = lookup_arp(inport, arp.spa, arp.sha); next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_na),
> action=(reg9[[2]] = lookup_nd(inport, nd.target, nd.tll); next;)
> table=??(lr_in_lookup_neighbor), priority=100 , match=(nd_ns),
> action=(reg9[[2]] = lookup_nd(inport, ip6.src, nd.sll); next;)
> + table=??(lr_in_lookup_neighbor), priority=105 , match=(nd_na && nd.tll ==
> 0), action=(reg9[[2]] = lookup_nd(inport, nd.target, eth.src); next;)
This covers the learn_from_arp_request flow, but we should probably have
another test for the priority 115 flow as well.
And we need a documentation update for the 'Neighbor lookup' lookup section
in ovn-logical-flows.7.rst.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
