diff mbox series

[ovs-dev] pinctrl: Use correct IPv6 dst for nd_ns action for IPv4 over v6.

Message ID 20260509012348.1485266-1-numans@ovn.org
State Accepted
Headers show
Series [ovs-dev] pinctrl: Use correct IPv6 dst for nd_ns action for IPv4 over v6. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Numan Siddique May 9, 2026, 1:23 a.m. UTC
From: Numan Siddique <numans@ovn.org>

If a logical router with dynamic-routing enabled learns a route
whose nexthop is an IPv6 address, then ovn-northd adds a logical
flow in the lr_in_ip_routing stage, but it doesn't add the
priority-200 logical flow in lr_in_arp_request to resolve the
mac of the nexthop. The below prioroty-100 logical flow gets invoked
instead.

table=27(lr_in_arp_request  ), priority=100  ,
  match=(eth.dst == 00:00:00:00:00:00 && reg9[9] == 0),
  action=(nd_ns { nd.target = xxreg0; output; }; next;)

And this doesn't set the correct Solicit IPv6 destination address.

This patch fixes the issue in pinctrl handler function for nd_ns
action by using the IPv6 destination address stored in xxreg0.
This action assumes that xxreg0 is populated with the proper address.

Other alternative fix is for ovn-northd to add a logical flow
in lr_in_arp_request just like it adds for static routes.

I think its better to fix it in pinctrl instead.

Found this issue while testing dynamic routing feature.

Assisted-by: Claude Opus 4.7, Claude Code
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 controller/pinctrl.c | 26 +++++++++++------
 tests/ovn.at         | 66 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 8 deletions(-)

Comments

Ales Musil May 13, 2026, 8:34 a.m. UTC | #1
On Sat, May 9, 2026 at 3:47 AM <numans@ovn.org> wrote:

> From: Numan Siddique <numans@ovn.org>
>
> If a logical router with dynamic-routing enabled learns a route
> whose nexthop is an IPv6 address, then ovn-northd adds a logical
> flow in the lr_in_ip_routing stage, but it doesn't add the
> priority-200 logical flow in lr_in_arp_request to resolve the
> mac of the nexthop. The below prioroty-100 logical flow gets invoked
> instead.
>
> table=27(lr_in_arp_request  ), priority=100  ,
>   match=(eth.dst == 00:00:00:00:00:00 && reg9[9] == 0),
>   action=(nd_ns { nd.target = xxreg0; output; }; next;)
>
> And this doesn't set the correct Solicit IPv6 destination address.
>
> This patch fixes the issue in pinctrl handler function for nd_ns
> action by using the IPv6 destination address stored in xxreg0.
> This action assumes that xxreg0 is populated with the proper address.
>
> Other alternative fix is for ovn-northd to add a logical flow
> in lr_in_arp_request just like it adds for static routes.
>
> I think its better to fix it in pinctrl instead.
>
> Found this issue while testing dynamic routing feature.
>
> Assisted-by: Claude Opus 4.7, Claude Code
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>

Hi Numan,

thank you for the patch. The commit is missing Fixes: bcba1b74c82c
("pinctrl: Handle arp/nd for other address families.").

 controller/pinctrl.c | 26 +++++++++++------
>  tests/ovn.at         | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 1a5407183c..452470ff74 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6534,17 +6534,27 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const
> struct flow *ip_flow,
>
>      /* We might be here without actually currently handling an IPv6
> packet.
>       * This can happen in the case where we route IPv4 packets over an
> IPv6
> -     * link.
> -     * In these cases we have no destination IPv6 address from the packet
> that
> -     * we can reuse. But we receive the actual destination IPv6 address
> via
> -     * userdata anyway, so what we pass to compose_nd_ns is irrelevant.
> -     * This is just a hope since we do not parse the userdata. If we land
> here
> -     * for whatever reason without being an IPv6 packet and without
> userdata we
> -     * will send out a wrong packet.
> -     */
> +     * link (e.g. RFC 5549 / BGP unnumbered, where an IPv4 destination is
> +     * resolved via an IPv6 link-local nexthop).
> +     *
> +     * In that case we have no destination IPv6 address in the trigger
> packet
> +     * to reuse. compose_nd_ns() needs a valid destination so it can
> derive
> +     * the correct solicited-node multicast (ff02::1:ff{addr[13:16]}) for
> +     * eth.dst and ip6.dst -- userdata only sets nd.target on the new
> packet
> +     * and does not rewrite ip6.dst, so a wrong ipv6_dst here egresses on
> the
> +     * wire as-is.
> +     *
> +     * The fallback nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores
> the
> +     * actual IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking
> the
> +     * nd_ns action, so for the IPv4-over-IPv6 case read xxreg0 from the
> +     * trigger packet's flow metadata. */
>      struct in6_addr ipv6_dst = IN6ADDR_EXACT_INIT;
>      if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
>          ipv6_dst = ip_flow->ipv6_dst;
> +    } else {
> +        ovs_be128 nexthop_be =
> +            hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0));
> +         memcpy(&ipv6_dst, &nexthop_be, sizeof ipv6_dst);
>

nit: memcpy is misaligned with the ovs_be128 above.


>      }
>      compose_nd_ns(&packet, ip_flow->dl_src, &ipv6_src,
>                    &ipv6_dst);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index fbaa63d99c..37e01dc1a9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -46214,3 +46214,69 @@ AT_CHECK([grep -q "WARN.*dynamic-routing"
> hv/ovn-controller.log], [1])
>  OVN_CLEANUP([hv])
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([IPv4 over v6 Neigh solicitation test])
> +ovn_start
> +
> +net_add n1
> +sim_add hv
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl lsp-add sw0 sw0-port1
> +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3"
> +
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> 1000::1/64
> +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
> +
> +check ovn-nbctl ls-add public
> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13
> +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
> +
> +# localnet port
> +check ovn-nbctl lsp-add-localnet-port public ln-public phys
> +
> +check ovn-nbctl lrp-set-gateway-chassis lr0-public hv 20
> +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.3
> +
> +check ovs-vsctl add-port br-int lsp -- \
> +    set Interface lsp external_ids:iface-id=lsp
>

nit: This lsp port doesn't seem to be used.


> +
> +ovs-vsctl -- add-port br-int vif1 -- \
> +    set interface vif1 external-ids:iface-id=sw0-port1 \
> +    options:tx_pcap=hv/vif1-tx.pcap \
> +    options:rxq_pcap=hv/vif1-rx.pcap \
> +    ofport-request=1
> +
> +wait_for_ports_up
> +
> +# Add a learnt route manually
> +dp_uuid=$(fetch_column datapath _uuid external_ids:name=lr0)
> +lrp_uuid=$(fetch_column port_binding _uuid logical_port=lr0-public)
> +
> +ovn-sbctl create learned_route datapath=$dp_uuid logical_port=$lrp_uuid \


nit: check_uuid.


> +ip_prefix=0.0.0.0/0 nexthop='"fe80::42:ff:fe00:1ff"'
> +
> +check ovn-nbctl --wait=hv sync
> +
> +# Send an IPv4 packet from sw0-port1 destined to outside
> +packet=$(fmt_pkt "Ether(dst='00:00:00:00:ff:01',
> src='50:54:00:00:00:01')/ \
> +                      IP(dst='8.8.8.8', src='10.0.0.3')/ICMP()")
> +check as hv ovs-appctl netdev-dummy/receive vif1 $packet
> +
> +nd_ns=$(fmt_pkt "Ether(dst='33:33:ff:00:01:ff', src='00:00:20:20:12:13')/
> \
> +                       IPv6(src='fe80::200:20ff:fe20:1213', \
> +
>  dst='ff02::1:ff00:1ff')/ICMPv6ND_NS(tgt='fe80::42:ff:fe00:1ff')/\
> +                       ICMPv6NDOptSrcLLAddr(lladdr='00:00:20:20:12:13')")
> +
> +echo $nd_ns > expected_nd_ns
> +OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns])
> +
> +OVN_CLEANUP([hv])
> +AT_CLEANUP
> +])
> +
> --
> 2.53.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With those addressed:
Acked-by: Ales Musil <amusil@redhat.com>

Regards,
Ales
Numan Siddique May 15, 2026, 4:25 a.m. UTC | #2
On Wed, May 13, 2026 at 4:35 AM Ales Musil <amusil@redhat.com> wrote:
>
>
>
> On Sat, May 9, 2026 at 3:47 AM <numans@ovn.org> wrote:
>>
>> From: Numan Siddique <numans@ovn.org>
>>
>> If a logical router with dynamic-routing enabled learns a route
>> whose nexthop is an IPv6 address, then ovn-northd adds a logical
>> flow in the lr_in_ip_routing stage, but it doesn't add the
>> priority-200 logical flow in lr_in_arp_request to resolve the
>> mac of the nexthop. The below prioroty-100 logical flow gets invoked
>> instead.
>>
>> table=27(lr_in_arp_request  ), priority=100  ,
>>   match=(eth.dst == 00:00:00:00:00:00 && reg9[9] == 0),
>>   action=(nd_ns { nd.target = xxreg0; output; }; next;)
>>
>> And this doesn't set the correct Solicit IPv6 destination address.
>>
>> This patch fixes the issue in pinctrl handler function for nd_ns
>> action by using the IPv6 destination address stored in xxreg0.
>> This action assumes that xxreg0 is populated with the proper address.
>>
>> Other alternative fix is for ovn-northd to add a logical flow
>> in lr_in_arp_request just like it adds for static routes.
>>
>> I think its better to fix it in pinctrl instead.
>>
>> Found this issue while testing dynamic routing feature.
>>
>> Assisted-by: Claude Opus 4.7, Claude Code
>> Signed-off-by: Numan Siddique <numans@ovn.org>
>> ---
>
>
> Hi Numan,
>
> thank you for the patch. The commit is missing Fixes: bcba1b74c82c ("pinctrl: Handle arp/nd for other address families.").
>
>>  controller/pinctrl.c | 26 +++++++++++------
>>  tests/ovn.at         | 66 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 84 insertions(+), 8 deletions(-)
>>
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index 1a5407183c..452470ff74 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -6534,17 +6534,27 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow,
>>
>>      /* We might be here without actually currently handling an IPv6 packet.
>>       * This can happen in the case where we route IPv4 packets over an IPv6
>> -     * link.
>> -     * In these cases we have no destination IPv6 address from the packet that
>> -     * we can reuse. But we receive the actual destination IPv6 address via
>> -     * userdata anyway, so what we pass to compose_nd_ns is irrelevant.
>> -     * This is just a hope since we do not parse the userdata. If we land here
>> -     * for whatever reason without being an IPv6 packet and without userdata we
>> -     * will send out a wrong packet.
>> -     */
>> +     * link (e.g. RFC 5549 / BGP unnumbered, where an IPv4 destination is
>> +     * resolved via an IPv6 link-local nexthop).
>> +     *
>> +     * In that case we have no destination IPv6 address in the trigger packet
>> +     * to reuse. compose_nd_ns() needs a valid destination so it can derive
>> +     * the correct solicited-node multicast (ff02::1:ff{addr[13:16]}) for
>> +     * eth.dst and ip6.dst -- userdata only sets nd.target on the new packet
>> +     * and does not rewrite ip6.dst, so a wrong ipv6_dst here egresses on the
>> +     * wire as-is.
>> +     *
>> +     * The fallback nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores the
>> +     * actual IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking the
>> +     * nd_ns action, so for the IPv4-over-IPv6 case read xxreg0 from the
>> +     * trigger packet's flow metadata. */
>>      struct in6_addr ipv6_dst = IN6ADDR_EXACT_INIT;
>>      if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
>>          ipv6_dst = ip_flow->ipv6_dst;
>> +    } else {
>> +        ovs_be128 nexthop_be =
>> +            hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0));
>> +         memcpy(&ipv6_dst, &nexthop_be, sizeof ipv6_dst);
>
>
> nit: memcpy is misaligned with the ovs_be128 above.
>
>>
>>      }
>>      compose_nd_ns(&packet, ip_flow->dl_src, &ipv6_src,
>>                    &ipv6_dst);
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index fbaa63d99c..37e01dc1a9 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -46214,3 +46214,69 @@ AT_CHECK([grep -q "WARN.*dynamic-routing" hv/ovn-controller.log], [1])
>>  OVN_CLEANUP([hv])
>>  AT_CLEANUP
>>  ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([IPv4 over v6 Neigh solicitation test])
>> +ovn_start
>> +
>> +net_add n1
>> +sim_add hv
>> +ovs-vsctl add-br br-phys
>> +ovn_attach n1 br-phys 192.168.0.1
>> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
>> +
>> +check ovn-nbctl ls-add sw0
>> +check ovn-nbctl lsp-add sw0 sw0-port1
>> +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3"
>> +
>> +check ovn-nbctl lr-add lr0
>> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
>> +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
>> +
>> +check ovn-nbctl ls-add public
>> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13
>> +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
>> +
>> +# localnet port
>> +check ovn-nbctl lsp-add-localnet-port public ln-public phys
>> +
>> +check ovn-nbctl lrp-set-gateway-chassis lr0-public hv 20
>> +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.3
>> +
>> +check ovs-vsctl add-port br-int lsp -- \
>> +    set Interface lsp external_ids:iface-id=lsp
>
>
> nit: This lsp port doesn't seem to be used.
>
>>
>> +
>> +ovs-vsctl -- add-port br-int vif1 -- \
>> +    set interface vif1 external-ids:iface-id=sw0-port1 \
>> +    options:tx_pcap=hv/vif1-tx.pcap \
>> +    options:rxq_pcap=hv/vif1-rx.pcap \
>> +    ofport-request=1
>> +
>> +wait_for_ports_up
>> +
>> +# Add a learnt route manually
>> +dp_uuid=$(fetch_column datapath _uuid external_ids:name=lr0)
>> +lrp_uuid=$(fetch_column port_binding _uuid logical_port=lr0-public)
>> +
>> +ovn-sbctl create learned_route datapath=$dp_uuid logical_port=$lrp_uuid \
>
>
> nit: check_uuid.
>
>>
>> +ip_prefix=0.0.0.0/0 nexthop='"fe80::42:ff:fe00:1ff"'
>> +
>> +check ovn-nbctl --wait=hv sync
>> +
>> +# Send an IPv4 packet from sw0-port1 destined to outside
>> +packet=$(fmt_pkt "Ether(dst='00:00:00:00:ff:01', src='50:54:00:00:00:01')/ \
>> +                      IP(dst='8.8.8.8', src='10.0.0.3')/ICMP()")
>> +check as hv ovs-appctl netdev-dummy/receive vif1 $packet
>> +
>> +nd_ns=$(fmt_pkt "Ether(dst='33:33:ff:00:01:ff', src='00:00:20:20:12:13')/ \
>> +                       IPv6(src='fe80::200:20ff:fe20:1213', \
>> +                       dst='ff02::1:ff00:1ff')/ICMPv6ND_NS(tgt='fe80::42:ff:fe00:1ff')/\
>> +                       ICMPv6NDOptSrcLLAddr(lladdr='00:00:20:20:12:13')")
>> +
>> +echo $nd_ns > expected_nd_ns
>> +OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns])
>> +
>> +OVN_CLEANUP([hv])
>> +AT_CLEANUP
>> +])
>> +
>> --
>> 2.53.0
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
> With those addressed:
> Acked-by: Ales Musil <amusil@redhat.com>

Thanks Ales.  I addressed your comments and applied to main.
I'll backport to the branches once I run the CI.  I'll update here once done.

Numan

>
> Regards,
> Ales
Numan Siddique May 15, 2026, 9:38 p.m. UTC | #3
On Fri, May 15, 2026 at 12:25 AM Numan Siddique <numans@ovn.org> wrote:
>
> On Wed, May 13, 2026 at 4:35 AM Ales Musil <amusil@redhat.com> wrote:
> >
> >
> >
> > On Sat, May 9, 2026 at 3:47 AM <numans@ovn.org> wrote:
> >>
> >> From: Numan Siddique <numans@ovn.org>
> >>
> >> If a logical router with dynamic-routing enabled learns a route
> >> whose nexthop is an IPv6 address, then ovn-northd adds a logical
> >> flow in the lr_in_ip_routing stage, but it doesn't add the
> >> priority-200 logical flow in lr_in_arp_request to resolve the
> >> mac of the nexthop. The below prioroty-100 logical flow gets invoked
> >> instead.
> >>
> >> table=27(lr_in_arp_request  ), priority=100  ,
> >>   match=(eth.dst == 00:00:00:00:00:00 && reg9[9] == 0),
> >>   action=(nd_ns { nd.target = xxreg0; output; }; next;)
> >>
> >> And this doesn't set the correct Solicit IPv6 destination address.
> >>
> >> This patch fixes the issue in pinctrl handler function for nd_ns
> >> action by using the IPv6 destination address stored in xxreg0.
> >> This action assumes that xxreg0 is populated with the proper address.
> >>
> >> Other alternative fix is for ovn-northd to add a logical flow
> >> in lr_in_arp_request just like it adds for static routes.
> >>
> >> I think its better to fix it in pinctrl instead.
> >>
> >> Found this issue while testing dynamic routing feature.
> >>
> >> Assisted-by: Claude Opus 4.7, Claude Code
> >> Signed-off-by: Numan Siddique <numans@ovn.org>
> >> ---
> >
> >
> > Hi Numan,
> >
> > thank you for the patch. The commit is missing Fixes: bcba1b74c82c ("pinctrl: Handle arp/nd for other address families.").
> >
> >>  controller/pinctrl.c | 26 +++++++++++------
> >>  tests/ovn.at         | 66 ++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 84 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> >> index 1a5407183c..452470ff74 100644
> >> --- a/controller/pinctrl.c
> >> +++ b/controller/pinctrl.c
> >> @@ -6534,17 +6534,27 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow,
> >>
> >>      /* We might be here without actually currently handling an IPv6 packet.
> >>       * This can happen in the case where we route IPv4 packets over an IPv6
> >> -     * link.
> >> -     * In these cases we have no destination IPv6 address from the packet that
> >> -     * we can reuse. But we receive the actual destination IPv6 address via
> >> -     * userdata anyway, so what we pass to compose_nd_ns is irrelevant.
> >> -     * This is just a hope since we do not parse the userdata. If we land here
> >> -     * for whatever reason without being an IPv6 packet and without userdata we
> >> -     * will send out a wrong packet.
> >> -     */
> >> +     * link (e.g. RFC 5549 / BGP unnumbered, where an IPv4 destination is
> >> +     * resolved via an IPv6 link-local nexthop).
> >> +     *
> >> +     * In that case we have no destination IPv6 address in the trigger packet
> >> +     * to reuse. compose_nd_ns() needs a valid destination so it can derive
> >> +     * the correct solicited-node multicast (ff02::1:ff{addr[13:16]}) for
> >> +     * eth.dst and ip6.dst -- userdata only sets nd.target on the new packet
> >> +     * and does not rewrite ip6.dst, so a wrong ipv6_dst here egresses on the
> >> +     * wire as-is.
> >> +     *
> >> +     * The fallback nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores the
> >> +     * actual IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking the
> >> +     * nd_ns action, so for the IPv4-over-IPv6 case read xxreg0 from the
> >> +     * trigger packet's flow metadata. */
> >>      struct in6_addr ipv6_dst = IN6ADDR_EXACT_INIT;
> >>      if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
> >>          ipv6_dst = ip_flow->ipv6_dst;
> >> +    } else {
> >> +        ovs_be128 nexthop_be =
> >> +            hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0));
> >> +         memcpy(&ipv6_dst, &nexthop_be, sizeof ipv6_dst);
> >
> >
> > nit: memcpy is misaligned with the ovs_be128 above.
> >
> >>
> >>      }
> >>      compose_nd_ns(&packet, ip_flow->dl_src, &ipv6_src,
> >>                    &ipv6_dst);
> >> diff --git a/tests/ovn.at b/tests/ovn.at
> >> index fbaa63d99c..37e01dc1a9 100644
> >> --- a/tests/ovn.at
> >> +++ b/tests/ovn.at
> >> @@ -46214,3 +46214,69 @@ AT_CHECK([grep -q "WARN.*dynamic-routing" hv/ovn-controller.log], [1])
> >>  OVN_CLEANUP([hv])
> >>  AT_CLEANUP
> >>  ])
> >> +
> >> +OVN_FOR_EACH_NORTHD([
> >> +AT_SETUP([IPv4 over v6 Neigh solicitation test])
> >> +ovn_start
> >> +
> >> +net_add n1
> >> +sim_add hv
> >> +ovs-vsctl add-br br-phys
> >> +ovn_attach n1 br-phys 192.168.0.1
> >> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> >> +
> >> +check ovn-nbctl ls-add sw0
> >> +check ovn-nbctl lsp-add sw0 sw0-port1
> >> +check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3"
> >> +
> >> +check ovn-nbctl lr-add lr0
> >> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
> >> +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
> >> +
> >> +check ovn-nbctl ls-add public
> >> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13
> >> +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
> >> +
> >> +# localnet port
> >> +check ovn-nbctl lsp-add-localnet-port public ln-public phys
> >> +
> >> +check ovn-nbctl lrp-set-gateway-chassis lr0-public hv 20
> >> +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.3
> >> +
> >> +check ovs-vsctl add-port br-int lsp -- \
> >> +    set Interface lsp external_ids:iface-id=lsp
> >
> >
> > nit: This lsp port doesn't seem to be used.
> >
> >>
> >> +
> >> +ovs-vsctl -- add-port br-int vif1 -- \
> >> +    set interface vif1 external-ids:iface-id=sw0-port1 \
> >> +    options:tx_pcap=hv/vif1-tx.pcap \
> >> +    options:rxq_pcap=hv/vif1-rx.pcap \
> >> +    ofport-request=1
> >> +
> >> +wait_for_ports_up
> >> +
> >> +# Add a learnt route manually
> >> +dp_uuid=$(fetch_column datapath _uuid external_ids:name=lr0)
> >> +lrp_uuid=$(fetch_column port_binding _uuid logical_port=lr0-public)
> >> +
> >> +ovn-sbctl create learned_route datapath=$dp_uuid logical_port=$lrp_uuid \
> >
> >
> > nit: check_uuid.
> >
> >>
> >> +ip_prefix=0.0.0.0/0 nexthop='"fe80::42:ff:fe00:1ff"'
> >> +
> >> +check ovn-nbctl --wait=hv sync
> >> +
> >> +# Send an IPv4 packet from sw0-port1 destined to outside
> >> +packet=$(fmt_pkt "Ether(dst='00:00:00:00:ff:01', src='50:54:00:00:00:01')/ \
> >> +                      IP(dst='8.8.8.8', src='10.0.0.3')/ICMP()")
> >> +check as hv ovs-appctl netdev-dummy/receive vif1 $packet
> >> +
> >> +nd_ns=$(fmt_pkt "Ether(dst='33:33:ff:00:01:ff', src='00:00:20:20:12:13')/ \
> >> +                       IPv6(src='fe80::200:20ff:fe20:1213', \
> >> +                       dst='ff02::1:ff00:1ff')/ICMPv6ND_NS(tgt='fe80::42:ff:fe00:1ff')/\
> >> +                       ICMPv6NDOptSrcLLAddr(lladdr='00:00:20:20:12:13')")
> >> +
> >> +echo $nd_ns > expected_nd_ns
> >> +OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns])
> >> +
> >> +OVN_CLEANUP([hv])
> >> +AT_CLEANUP
> >> +])
> >> +
> >> --
> >> 2.53.0
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >
> > With those addressed:
> > Acked-by: Ales Musil <amusil@redhat.com>
>
> Thanks Ales.  I addressed your comments and applied to main.
> I'll backport to the branches once I run the CI.  I'll update here once done.

I backported all the way until branch-25.03.

Numan

>
> Numan
>
> >
> > Regards,
> > Ales
diff mbox series

Patch

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 1a5407183c..452470ff74 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6534,17 +6534,27 @@  pinctrl_handle_nd_ns(struct rconn *swconn, const struct flow *ip_flow,
 
     /* We might be here without actually currently handling an IPv6 packet.
      * This can happen in the case where we route IPv4 packets over an IPv6
-     * link.
-     * In these cases we have no destination IPv6 address from the packet that
-     * we can reuse. But we receive the actual destination IPv6 address via
-     * userdata anyway, so what we pass to compose_nd_ns is irrelevant.
-     * This is just a hope since we do not parse the userdata. If we land here
-     * for whatever reason without being an IPv6 packet and without userdata we
-     * will send out a wrong packet.
-     */
+     * link (e.g. RFC 5549 / BGP unnumbered, where an IPv4 destination is
+     * resolved via an IPv6 link-local nexthop).
+     *
+     * In that case we have no destination IPv6 address in the trigger packet
+     * to reuse. compose_nd_ns() needs a valid destination so it can derive
+     * the correct solicited-node multicast (ff02::1:ff{addr[13:16]}) for
+     * eth.dst and ip6.dst -- userdata only sets nd.target on the new packet
+     * and does not rewrite ip6.dst, so a wrong ipv6_dst here egresses on the
+     * wire as-is.
+     *
+     * The fallback nd_ns logical flow in S_ROUTER_IN_ARP_REQUEST stores the
+     * actual IPv6 nexthop in xxreg0 (REG_NEXT_HOP_IPV6) before invoking the
+     * nd_ns action, so for the IPv4-over-IPv6 case read xxreg0 from the
+     * trigger packet's flow metadata. */
     struct in6_addr ipv6_dst = IN6ADDR_EXACT_INIT;
     if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
         ipv6_dst = ip_flow->ipv6_dst;
+    } else {
+        ovs_be128 nexthop_be =
+            hton128(flow_get_xxreg(&pin->flow_metadata.flow, 0));
+         memcpy(&ipv6_dst, &nexthop_be, sizeof ipv6_dst);
     }
     compose_nd_ns(&packet, ip_flow->dl_src, &ipv6_src,
                   &ipv6_dst);
diff --git a/tests/ovn.at b/tests/ovn.at
index fbaa63d99c..37e01dc1a9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -46214,3 +46214,69 @@  AT_CHECK([grep -q "WARN.*dynamic-routing" hv/ovn-controller.log], [1])
 OVN_CLEANUP([hv])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([IPv4 over v6 Neigh solicitation test])
+ovn_start
+
+net_add n1
+sim_add hv
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 sw0-port1
+check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3"
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
+check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
+
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13
+check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
+
+# localnet port
+check ovn-nbctl lsp-add-localnet-port public ln-public phys
+
+check ovn-nbctl lrp-set-gateway-chassis lr0-public hv 20
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.110 10.0.0.3
+
+check ovs-vsctl add-port br-int lsp -- \
+    set Interface lsp external_ids:iface-id=lsp
+
+ovs-vsctl -- add-port br-int vif1 -- \
+    set interface vif1 external-ids:iface-id=sw0-port1 \
+    options:tx_pcap=hv/vif1-tx.pcap \
+    options:rxq_pcap=hv/vif1-rx.pcap \
+    ofport-request=1
+
+wait_for_ports_up
+
+# Add a learnt route manually
+dp_uuid=$(fetch_column datapath _uuid external_ids:name=lr0)
+lrp_uuid=$(fetch_column port_binding _uuid logical_port=lr0-public)
+
+ovn-sbctl create learned_route datapath=$dp_uuid logical_port=$lrp_uuid \
+ip_prefix=0.0.0.0/0 nexthop='"fe80::42:ff:fe00:1ff"'
+
+check ovn-nbctl --wait=hv sync
+
+# Send an IPv4 packet from sw0-port1 destined to outside
+packet=$(fmt_pkt "Ether(dst='00:00:00:00:ff:01', src='50:54:00:00:00:01')/ \
+                      IP(dst='8.8.8.8', src='10.0.0.3')/ICMP()")
+check as hv ovs-appctl netdev-dummy/receive vif1 $packet
+
+nd_ns=$(fmt_pkt "Ether(dst='33:33:ff:00:01:ff', src='00:00:20:20:12:13')/ \
+                       IPv6(src='fe80::200:20ff:fe20:1213', \
+                       dst='ff02::1:ff00:1ff')/ICMPv6ND_NS(tgt='fe80::42:ff:fe00:1ff')/\
+                       ICMPv6NDOptSrcLLAddr(lladdr='00:00:20:20:12:13')")
+
+echo $nd_ns > expected_nd_ns
+OVN_CHECK_PACKETS_CONTAIN([hv/br-phys_n1-tx.pcap], [expected_nd_ns])
+
+OVN_CLEANUP([hv])
+AT_CLEANUP
+])
+