On Wed, 31 Aug 2022 22:46:15 +0200 Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > The bpf_skb_set_tunnel_key() helper has a number of flags we pass in, e.g. > BPF_F_ZERO_CSUM_TX, BPF_F_DONT_FRAGMENT, BPF_F_SEQ_NUMBER, and then based on > those flags we set: > > [...] > info->key.tun_flags = TUNNEL_KEY | TUNNEL_CSUM | TUNNEL_NOCACHE; > if (flags & BPF_F_DONT_FRAGMENT) > info->key.tun_flags |= TUNNEL_DONT_FRAGMENT; > if (flags & BPF_F_ZERO_CSUM_TX) > info->key.tun_flags &= ~TUNNEL_CSUM; > if (flags & BPF_F_SEQ_NUMBER) > info->key.tun_flags |= TUNNEL_SEQ; > [...] > > Should we similarly only expose those which are interesting/relevant to BPF > program authors as a __u16 tunnel_flags and not the whole set? Which ones > do you have a need for? TUNNEL_SEQ, TUNNEL_CSUM, TUNNEL_KEY, and then the > TUNNEL_OPTIONS_PRESENT? Indeed, I noticed this and considered various approaches: 1. Convert the "interesting" internal TUNNEL_xxx flags back to BPF_F_yyy and place into the new 'tunnel_flags' field. This has 2 drawbacks: - The BPF_F_yyy flags are from *set_tunnel_key* enumeration space, e.g. BPF_F_ZERO_CSUM_TX. I find it awkward that it is "returned" into tunnel_flags from a *get_tunnel_key* call. - Not all "interesting" TUNNEL_xxx flags can be mapped to existing BPF_F_yyy flags, and it doesn't make sense to create new BPF_F_yyy flags just for purposes of the returned tunnel_flags. 2. Place key.tun_flags into 'tunnel_flags' but mask them, keeping only "interesting" flags. That's ok, but the drawback is that what's "intersting" for my usecase might be limiting for other usecases. Therefore I decided to expose what's in key.tun_flags *as is*, which seems most flexible. The bpf user can just choose to ingore bits he's not interested in. The TUNNEL_xxx are uapi, so no harm exposing them back in the get_tunnel_key call. WDYT? Best, Shmulik
