Re: [PATCH bpf-next v2 1/2] bpf: Add ifindex to bpf_sk_lookup

On Thu, Nov 04, 2021 at 07:06 PM CET, 'Yonghong Song' via kernel-team+notifications wrote:

On 11/4/21 5:23 AM, Mark Pashmfouroush wrote:

quoted

It may be helpful to have access to the ifindex during bpf socket
lookup. An example may be to scope certain socket lookup logic to
specific interfaces, i.e. an interface may be made exempt from custom
lookup code.
Add the ifindex of the arriving connection to the bpf_sk_lookup API.
Signed-off-by: Mark Pashmfouroush <redacted>

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 24b7ed2677af..0012a5176a32 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h

@@ -1374,6 +1374,7 @@ struct bpf_sk_lookup_kern {
  		const struct in6_addr *daddr;
  	} v6;
  	struct sock	*selected_sk;
+	u32		ifindex;

In struct __sk_buff, we have two ifindex related fields:

        __u32 ingress_ifindex;
        __u32 ifindex;

Does newly-added ifindex corresponds to skb->ingress_ifindex or
skb->ifindex? From comments:
  > +	__u32 ifindex;		/* The arriving interface. Determined by inet_iif. */

looks like it corresponds to ingress? Should be use the name
ingress_ifindex to be consistent with __sk_buff?

On ingress these two (skb->skb_iif and skb->dev-ifindex) are the same,
if I read the code correctly [1].

That said, I agree that ingress_ifindex would be less ambiguous (iif ->
ingress interface, can't get that wrong).

Also, as Yonghong points out __sk_buff and xdp_md context objects
already use this identifier for the same bit of information, so it will
be less of surprise.

[1] https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L5258

[...]