Re: [PATCH bpf v2 1/2] bpf: Derive source IP addr via bpf_*_fib_lookup()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/5/23 1:16 PM, Martynas wrote:
@@ -5992,6 +5995,19 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
   	params->rt_metric = res.f6i->fib6_metric;
   	params->ifindex = dev->ifindex;
+ if (flags & BPF_FIB_LOOKUP_SET_SRC) {
+		if (res.f6i->fib6_prefsrc.plen) {
+			*(struct in6_addr *)params->ipv6_src = res.f6i->fib6_prefsrc.addr;

A nit. just noticed. Similar to the "*dst" assignment a few lines above:

			*src = res.f6i->fib6_prefsrc.addr;

+		} else {
+			err = ipv6_bpf_stub->ipv6_dev_get_saddr(net, dev,
+								&fl6.daddr, 0,
+								(struct in6_addr *)
+								params->ipv6_src);

Same here. Use the "src".

+			if (err)
+				return BPF_FIB_LKUP_RET_NO_SRC_ADDR;

This error also implies BPF_FIB_LKUP_RET_NO_NEIGH. I don't have a clean way of
improving the API. May be others have some ideas.

Considering dev has no saddr is probably (?) an unlikely case, it should be ok
to leave it as is but at least a comment in the uapi will be needed. Otherwise,
the bpf prog may use the 0 dmac as-is.

I expect that a user of the helper checks that err == 0 before using any of the output params.

For example, the bpf prog gets BPF_FIB_LKUP_RET_NO_NEIGH and learns neigh is not available but ipv6_dst (and the optional ipv6_src) is still valid.

If the bpf prog gets BPF_FIB_LKUP_RET_NO_SRC_ADDR, intuitively, only ipv6_src is not available. The bpf prog will continue to use the ipv6_dst and dmac (which is actually 0).



I feel the current bpf_ipv[46]_fib_lookup helper is doing many things
in one
function and then requires different BPF_FIB_LOOKUP_* bits to select
what/how to
do. In the future, it may be worth to consider breaking it into smaller
kfunc(s). e.g. the __ipv[46]_neigh_lookup could be in its own kfunc.


Yep, good idea. At least it seems that the neigh lookup could live in its own function.

To be clear, it could be independent of this set.

Thanks.





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux