Re: [PATCH net-next v2 4/4] bpf: drop the stale dst when redirecting to an explicit nexthop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] bpf: missing Fixes trailer for commit ba452c9e996d in commit
  message
--

commit 69d202e339fb4761ba7ad2174f002c73503bb498
Author: Jamal Hadi Salim <jhs@mojatatu.com>

> bpf: drop the stale dst when redirecting to an explicit nexthop
>
> This is a followup of the review of the teql arp-queue fix as reported
> by Sashiko.
>
> The issue:
> __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6() install a dst for
> the redirect target only when the program lets the helper do the FIB
> lookup. With BPF_F_NEXTHOP the program supplies the nexthop, the lookup is
> skipped, and whatever dst the packet arrived with stays attached while
> bpf_out_neigh_v{4,6}() switch skb->dev to the target device. If that
> device's neighbour is incomplete the packet is parked on its arp_queue with
> the original route's dst; draining the queue re-looks the neighbour up from
> that dst and builds the frame with the original device's link-layer header,
> then transmits it on the target device.
>
> The Fix:
> Release the dst when the nexthop is supplied. That branch never consults
> the route dst, so nothing that describes the route is lost, and the queue
> drain keeps the target device's parking neighbour.
>
> The release goes through skb_dst_drop_keep_metadata(), so encapsulation
> parameters a route carries are handed over to a metadata dst instead of
> being lost, and a metadata dst already on the packet is left in place. Both
> wrappers take their existing drop path when that copy cannot be allocated,
> so a packet is never sent with the stale route.
>
> Conditions to recreate the bug:
> Two network namespaces, A (10.10.0.1/24) and B (10.1.0.1/24) in one, their
> veth peers in the other with only A's peer addressed (10.10.0.2/24) so B's
> nexthop cannot be resolved by ARP. Resolve A's neighbour for 10.10.0.2 as
> permanent and attach a cls_bpf egress program to A, with vb the index of B:
>
>     struct bpf_redir_neigh nh = { .nh_family = AF_INET,
>                                   .ipv4_nh = htonl(0x0a010002) };
>
>     return bpf_redirect_neigh(vb, &nh, sizeof(nh), 0);
>
> Send one packet to 10.10.0.2. It is parked on B's incomplete neighbour
> for 10.1.0.2; once that neighbour becomes valid the frame leaves B
> carrying A's source address and A's neighbour as the destination.
>
> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
> Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
> Link: https://lore.kernel.org/netdev/avTfQjFQYR2NNdRg@chamomile/
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

[Severity: Low]
Should this commit include a Fixes: tag for the commit that added the
explicit nexthop branch in __bpf_redirect_neigh_v4() and
__bpf_redirect_neigh_v6()?

  Fixes: ba452c9e996d ("bpf: Fix bpf_redirect_neigh helper api to support supplying nexthop")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CORE-9NOG.v2.r0.20261112151659@mojatatu.com?part=4