Jakub Kicinski wrote: > On Thu, 5 Jun 2025 15:50:31 +0200 Maciej Żenczykowski wrote: > > On Thu, Jun 5, 2025 at 3:22 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > > > I wonder if this shouldn't drop dst even when doing ipv4->ipv4 or > > > > ipv6->ipv6 -- it's encapping, presumably old dst is irrelevant... > > > > > > I keep going back and forth on this. You definitely have a point, > > > but I feel like there are levels to how BPF prog can make the dst > > > irrelevant: > > > - change proto > > > - encap > > > - adjust room but not set any encap flag > > > - overwrite the addrs without calling any helpers > > > First case we have to cover for safety, last we can't possibly cover. > > > So the question is whether we should draw the line somewhere in > > > the middle, or leave this patch as is and if the actual use case arrives > > > - let BPF call skb_dst_drop() as a kfunc. Right now I'm leaning towards > > > the latter. > > > > > > Does that make sense? Does anyone else have an opinion? > > > > It does make a fair bit of sense. > > Question: does calling it as a kfunc require kernel BTF? > > Specifically some ram limited devices want to disable CONFIG_DEBUG_INFO_BTF... > > I know normal bpf helpers don't need that... > > I guess you could always convert ipv4 -> ipv6 -> ipv4 ;-) > > Not sure how BPF folks feel about that, but technically we could > also add a flag to bpf_skb_adjust_room() or bpf_skb_change_proto(). To invert the question: what is the value in keeping the dst? The test refers to a nat6to4.bpf.o, but this is not included.
