On Thu, 28 Jun 2018 09:42:06 +0200, Jiri Benc wrote:
> On Wed, 27 Jun 2018 11:49:49 +0200, Daniel Borkmann wrote:
> > Looks good to me, and yes in BPF case a mask like TUNNEL_OPTIONS_PRESENT is
> > right approach since this is opaque info and solely defined by the BPF prog
> > that is using the generic helper.
>
> Wouldn't it make sense to introduce some safeguards here (in a backward
> compatible way, of course)? It's easy to mistakenly set data for a
> different tunnel type in a BPF program and then be surprised by the
> result. It might help users if such usage was detected by the kernel,
> one way or another.
Well, that's how it works today ;)
> I'm thinking about something like the BPF program voluntarily
> specifying the type of the data; if not specified, the wildcard would be
> used as it is now.
Hmm... in practice we could steal top bits of the size parameter for
some flags, since it seems to be limited to values < 256 today? Is it
worth it?
It would look something along the lines of:
---
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 59b19b6a40d7..194b40efa8e8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2213,6 +2213,13 @@ enum bpf_func_id {
/* BPF_FUNC_perf_event_output for sk_buff input context. */
#define BPF_F_CTXLEN_MASK (0xfffffULL << 32)
+#define BPF_F_TUN_VXLAN (1U << 31)
+#define BPF_F_TUN_GENEVE (1U << 30)
+#define BPF_F_TUN_ERSPAN (1U << 29)
+#define BPF_F_TUN_FLAGS_ALL (BPF_F_TUN_VXLAN | \
+ BPF_F_TUN_GENEVE | \
+ BPF_F_TUN_ERSPAN)
+
/* Mode for BPF_FUNC_skb_adjust_room helper. */
enum bpf_adj_room_mode {
BPF_ADJ_ROOM_NET,
diff --git a/net/core/filter.c b/net/core/filter.c
index dade922678f6..cc592a1e8945 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3576,6 +3576,22 @@ BPF_CALL_3(bpf_skb_set_tunnel_opt, struct sk_buff *, skb,
{
struct ip_tunnel_info *info = skb_tunnel_info(skb);
const struct metadata_dst *md = this_cpu_ptr(md_dst);
+ __be16 tun_flags;
+ u32 flags;
+
+ BUILD_BUG_ON(BPF_F_TUN_FLAGS_ALL & IP_TUNNEL_OPTS_MAX);
+
+ flags = size & BPF_F_TUN_FLAGS_ALL;
+ size &= ~flags;
+ if (flags & BPF_F_TUN_VXLAN)
+ tun_flags |= TUNNEL_VXLAN_OPT;
+ if (flags & BPF_F_TUN_GENEVE)
+ tun_flags |= TUNNEL_GENEVE_OPT;
+ if (flags & BPF_F_TUN_ERSPAN)
+ tun_flags |= TUNNEL_ERSPAN_OPT;
+ /* User didn't specify the tunnel type, for backward compat set all */
+ if (!(tun_flags & TUNNEL_OPTIONS_PRESENT))
+ tun_flags |= TUNNEL_OPTIONS_PRESENT;
if (unlikely(info != &md->u.tun_info || (size & (sizeof(u32) - 1))))
return -EINVAL;
