[PATCH bpf 0/5] flow_dissector: lay groundwork for calling BPF hook from eth_get_headlen

[PATCH bpf 0/5] flow_dissector: lay groundwork for calling BPF hook from eth_get_headlen

[Stanislav Fomichev]

This patch series fixes the existing BPF flow dissector API to
support calling BPF progs from the eth_get_headlen context (the support
itself will be added in bpf-next tree).

The summary of the changes:
  * fix VLAN handling in bpf_flow.c, we don't need to peek back and look
    at skb->vlan_present; add selftests
  * pass and use flow_keys->n_proto instead of skb->protocol
  * fix clamping of flow_keys->nhoff for packets with nhoff > 0
  * prohibit access to most of the __sk_buff fields from BPF flow
    dissector progs; only data/data_end/flow_keys are allowed (all input
    is now passed via flow_keys)
  * finally, document BPF flow dissector program environment

Stanislav Fomichev (5):
  selftests/bpf: fix vlan handling in flow dissector program
  net/flow_dissector: pass flow_keys->n_proto to BPF programs
  flow_dissector: fix clamping of BPF flow_keys for non-zero nhoff
  flow_dissector: allow access only to a subset of __sk_buff fields
  flow_dissector: document BPF flow dissector environment

 .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
 net/core/filter.c                             |  16 +--
 net/core/flow_dissector.c                     |   4 +-
 .../selftests/bpf/prog_tests/flow_dissector.c |  68 +++++++++++
 tools/testing/selftests/bpf/progs/bpf_flow.c  |  19 ++-
 5 files changed, 196 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/networking/bpf_flow_dissector.txt

-- 
2.21.0.392.gf8f6787159e-goog

[PATCH bpf 1/5] selftests/bpf: fix vlan handling in flow dissector program

[Stanislav Fomichev]

When we tail call PROG(VLAN) from parse_eth_proto we don't need to peek
back to handle vlan proto because we didn't adjust nhoff/thoff yet. Use
flow_keys->n_proto, that we set in parse_eth_proto instead and
properly increment nhoff as well.

Also, always use skb->protocol and don't look at skb->vlan_present.
skb->vlan_present indicates that vlan information is stored out-of-band
in skb->vlan_{tci,proto} and vlan header is already pulled from skb.
That means, skb->vlan_present == true is not relevant for BPF flow
dissector.

Add simple test cases with VLAN tagged frames:
  * single vlan for ipv4
  * double vlan for ipv6

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 .../selftests/bpf/prog_tests/flow_dissector.c | 68 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/bpf_flow.c  | 15 ++--
 2 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
index bcbd928c96ab..fc818bc1d729 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
@@ -39,6 +39,58 @@ static struct bpf_flow_keys pkt_v6_flow_keys = {
 	.n_proto = __bpf_constant_htons(ETH_P_IPV6),
 };
 
+#define VLAN_HLEN	4
+
+static struct {
+	struct ethhdr eth;
+	__u16 vlan_tci;
+	__u16 vlan_proto;
+	struct iphdr iph;
+	struct tcphdr tcp;
+} __packed pkt_vlan_v4 = {
+	.eth.h_proto = __bpf_constant_htons(ETH_P_8021Q),
+	.vlan_proto = __bpf_constant_htons(ETH_P_IP),
+	.iph.ihl = 5,
+	.iph.protocol = IPPROTO_TCP,
+	.iph.tot_len = __bpf_constant_htons(MAGIC_BYTES),
+	.tcp.urg_ptr = 123,
+	.tcp.doff = 5,
+};
+
+static struct bpf_flow_keys pkt_vlan_v4_flow_keys = {
+	.nhoff = VLAN_HLEN,
+	.thoff = VLAN_HLEN + sizeof(struct iphdr),
+	.addr_proto = ETH_P_IP,
+	.ip_proto = IPPROTO_TCP,
+	.n_proto = __bpf_constant_htons(ETH_P_IP),
+};
+
+static struct {
+	struct ethhdr eth;
+	__u16 vlan_tci;
+	__u16 vlan_proto;
+	__u16 vlan_tci2;
+	__u16 vlan_proto2;
+	struct ipv6hdr iph;
+	struct tcphdr tcp;
+} __packed pkt_vlan_v6 = {
+	.eth.h_proto = __bpf_constant_htons(ETH_P_8021AD),
+	.vlan_proto = __bpf_constant_htons(ETH_P_8021Q),
+	.vlan_proto2 = __bpf_constant_htons(ETH_P_IPV6),
+	.iph.nexthdr = IPPROTO_TCP,
+	.iph.payload_len = __bpf_constant_htons(MAGIC_BYTES),
+	.tcp.urg_ptr = 123,
+	.tcp.doff = 5,
+};
+
+static struct bpf_flow_keys pkt_vlan_v6_flow_keys = {
+	.nhoff = VLAN_HLEN * 2,
+	.thoff = VLAN_HLEN * 2 + sizeof(struct ipv6hdr),
+	.addr_proto = ETH_P_IPV6,
+	.ip_proto = IPPROTO_TCP,
+	.n_proto = __bpf_constant_htons(ETH_P_IPV6),
+};
+
 void test_flow_dissector(void)
 {
 	struct bpf_flow_keys flow_keys;
@@ -68,5 +120,21 @@ void test_flow_dissector(void)
 	      err, errno, retval, duration, size, sizeof(flow_keys));
 	CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys);
 
+	err = bpf_prog_test_run(prog_fd, 10, &pkt_vlan_v4, sizeof(pkt_vlan_v4),
+				&flow_keys, &size, &retval, &duration);
+	CHECK(size != sizeof(flow_keys) || err || retval != 1, "vlan_ipv4",
+	      "err %d errno %d retval %d duration %d size %u/%lu\n",
+	      err, errno, retval, duration, size, sizeof(flow_keys));
+	CHECK_FLOW_KEYS("vlan_ipv4_flow_keys", flow_keys,
+			pkt_vlan_v4_flow_keys);
+
+	err = bpf_prog_test_run(prog_fd, 10, &pkt_vlan_v6, sizeof(pkt_vlan_v6),
+				&flow_keys, &size, &retval, &duration);
+	CHECK(size != sizeof(flow_keys) || err || retval != 1, "vlan_ipv6",
+	      "err %d errno %d retval %d duration %d size %u/%lu\n",
+	      err, errno, retval, duration, size, sizeof(flow_keys));
+	CHECK_FLOW_KEYS("vlan_ipv6_flow_keys", flow_keys,
+			pkt_vlan_v6_flow_keys);
+
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/progs/bpf_flow.c b/tools/testing/selftests/bpf/progs/bpf_flow.c
index 284660f5aa95..f177c7a6a6c7 100644
--- a/tools/testing/selftests/bpf/progs/bpf_flow.c
+++ b/tools/testing/selftests/bpf/progs/bpf_flow.c
@@ -119,10 +119,7 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
 SEC("flow_dissector")
 int _dissect(struct __sk_buff *skb)
 {
-	if (!skb->vlan_present)
-		return parse_eth_proto(skb, skb->protocol);
-	else
-		return parse_eth_proto(skb, skb->vlan_proto);
+	return parse_eth_proto(skb, skb->protocol);
 }
 
 /* Parses on IPPROTO_* */
@@ -336,15 +333,9 @@ PROG(VLAN)(struct __sk_buff *skb)
 {
 	struct bpf_flow_keys *keys = skb->flow_keys;
 	struct vlan_hdr *vlan, _vlan;
-	__be16 proto;
-
-	/* Peek back to see if single or double-tagging */
-	if (bpf_skb_load_bytes(skb, keys->thoff - sizeof(proto), &proto,
-			       sizeof(proto)))
-		return BPF_DROP;
 
 	/* Account for double-tagging */
-	if (proto == bpf_htons(ETH_P_8021AD)) {
+	if (keys->n_proto == bpf_htons(ETH_P_8021AD)) {
 		vlan = bpf_flow_dissect_get_header(skb, sizeof(*vlan), &_vlan);
 		if (!vlan)
 			return BPF_DROP;
@@ -352,6 +343,7 @@ PROG(VLAN)(struct __sk_buff *skb)
 		if (vlan->h_vlan_encapsulated_proto != bpf_htons(ETH_P_8021Q))
 			return BPF_DROP;
 
+		keys->nhoff += sizeof(*vlan);
 		keys->thoff += sizeof(*vlan);
 	}
 
@@ -359,6 +351,7 @@ PROG(VLAN)(struct __sk_buff *skb)
 	if (!vlan)
 		return BPF_DROP;
 
+	keys->nhoff += sizeof(*vlan);
 	keys->thoff += sizeof(*vlan);
 	/* Only allow 8021AD + 8021Q double tagging and no triple tagging.*/
 	if (vlan->h_vlan_encapsulated_proto == bpf_htons(ETH_P_8021AD) ||
-- 
2.21.0.392.gf8f6787159e-goog

[PATCH bpf 2/5] net/flow_dissector: pass flow_keys->n_proto to BPF programs

[Stanislav Fomichev]

This is a preparation for the next commit that would prohibit access to
the most fields of __sk_buff from the BPF programs.

Instead of requiring BPF flow dissector programs to look into skb,
pass all input data in the flow_keys.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 net/core/flow_dissector.c                    | 1 +
 tools/testing/selftests/bpf/progs/bpf_flow.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index bb1a54747d64..9b84250039df 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -707,6 +707,7 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
 	/* Pass parameters to the BPF program */
 	memset(flow_keys, 0, sizeof(*flow_keys));
 	cb->qdisc_cb.flow_keys = flow_keys;
+	flow_keys->n_proto = skb->protocol;
 	flow_keys->nhoff = skb_network_offset(skb);
 	flow_keys->thoff = flow_keys->nhoff;
 
diff --git a/tools/testing/selftests/bpf/progs/bpf_flow.c b/tools/testing/selftests/bpf/progs/bpf_flow.c
index f177c7a6a6c7..75b17cada539 100644
--- a/tools/testing/selftests/bpf/progs/bpf_flow.c
+++ b/tools/testing/selftests/bpf/progs/bpf_flow.c
@@ -92,7 +92,6 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
 {
 	struct bpf_flow_keys *keys = skb->flow_keys;
 
-	keys->n_proto = proto;
 	switch (proto) {
 	case bpf_htons(ETH_P_IP):
 		bpf_tail_call(skb, &jmp_table, IP);
@@ -119,7 +118,9 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
 SEC("flow_dissector")
 int _dissect(struct __sk_buff *skb)
 {
-	return parse_eth_proto(skb, skb->protocol);
+	struct bpf_flow_keys *keys = skb->flow_keys;
+
+	return parse_eth_proto(skb, keys->n_proto);
 }
 
 /* Parses on IPPROTO_* */
@@ -358,6 +359,7 @@ PROG(VLAN)(struct __sk_buff *skb)
 	    vlan->h_vlan_encapsulated_proto == bpf_htons(ETH_P_8021Q))
 		return BPF_DROP;
 
+	keys->n_proto = vlan->h_vlan_encapsulated_proto;
 	return parse_eth_proto(skb, vlan->h_vlan_encapsulated_proto);
 }
 
-- 
2.21.0.392.gf8f6787159e-goog

[PATCH bpf 3/5] flow_dissector: fix clamping of BPF flow_keys for non-zero nhoff

[Stanislav Fomichev]

Don't allow BPF program to set flow_keys->nhoff to less than initial
value. We currently don't read the value afterwards in anything but
the tests, but it's still a good practice to return consistent
values to the test programs.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 net/core/flow_dissector.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 9b84250039df..94a450b2191a 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -717,7 +717,8 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
 	/* Restore state */
 	memcpy(cb, &cb_saved, sizeof(cb_saved));
 
-	flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, 0, skb->len);
+	flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff,
+				   skb_network_offset(skb), skb->len);
 	flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
 				   flow_keys->nhoff, skb->len);
 
-- 
2.21.0.392.gf8f6787159e-goog

[PATCH bpf 4/5] flow_dissector: allow access only to a subset of __sk_buff fields

[Stanislav Fomichev]

Use whitelist instead of a blacklist and allow only a small set of
fields that might be relevant in the context of flow dissector:
  * data
  * data_end
  * flow_keys

This is required for the eth_get_headlen case where we have only a
chunk of data to dissect (i.e. trying to read the other skb fields
doesn't make sense).

Note, that it is a breaking API change! However, we've provided
flow_keys->n_proto as a substitute for skb->protocol; and there is
no need to manually handle skb->vlan_present. So even if we
break somebody, the migration is trivial. Unfortunately, we can't
support eth_get_headlen use-case without those breaking changes.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 net/core/filter.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 647c63a7b25b..fc92ebc4e200 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6613,14 +6613,8 @@ static bool flow_dissector_is_valid_access(int off, int size,
 					   const struct bpf_prog *prog,
 					   struct bpf_insn_access_aux *info)
 {
-	if (type == BPF_WRITE) {
-		switch (off) {
-		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
-			break;
-		default:
-			return false;
-		}
-	}
+	if (type == BPF_WRITE)
+		return false;
 
 	switch (off) {
 	case bpf_ctx_range(struct __sk_buff, data):
@@ -6632,11 +6626,7 @@ static bool flow_dissector_is_valid_access(int off, int size,
 	case bpf_ctx_range_ptr(struct __sk_buff, flow_keys):
 		info->reg_type = PTR_TO_FLOW_KEYS;
 		break;
-	case bpf_ctx_range(struct __sk_buff, tc_classid):
-	case bpf_ctx_range(struct __sk_buff, data_meta):
-	case bpf_ctx_range_till(struct __sk_buff, family, local_port):
-	case bpf_ctx_range(struct __sk_buff, tstamp):
-	case bpf_ctx_range(struct __sk_buff, wire_len):
+	default:
 		return false;
 	}
 
-- 
2.21.0.392.gf8f6787159e-goog

[PATCH bpf 5/5] flow_dissector: document BPF flow dissector environment

[Stanislav Fomichev]

Short doc on what BPF flow dissector should expect in the input
__sk_buff and flow_keys.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/networking/bpf_flow_dissector.txt

diff --git a/Documentation/networking/bpf_flow_dissector.txt b/Documentation/networking/bpf_flow_dissector.txt
new file mode 100644
index 000000000000..513be8e20afb
--- /dev/null
+++ b/Documentation/networking/bpf_flow_dissector.txt
@@ -0,0 +1,115 @@
+==================
+BPF Flow Dissector
+==================
+
+Overview
+========
+
+Flow dissector is a routine that parses metadata out of the packets. It's
+used in the various places in the networking subsystem (RFS, flow hash, etc).
+
+BPF flow dissector is an attempt to reimplement C-based flow dissector logic
+in BPF to gain all the benefits of BPF verifier (namely, limits on the
+number of instructions and tail calls).
+
+API
+===
+
+BPF flow dissector programs operate on an __sk_buff. However, only the
+limited set of fields is allowed: data, data_end and flow_keys. flow_keys
+is 'struct bpf_flow_keys' and contains flow dissector input and
+output arguments.
+
+The inputs are:
+  * nhoff - initial offset of the networking header
+  * thoff - initial offset of the transport header, initialized to nhoff
+  * n_proto - L3 protocol type, parsed out of L2 header
+
+Flow dissector BPF program should fill out the rest of the 'struct
+bpf_flow_keys' fields. Input arguments nhoff/thoff/n_proto should be also
+adjusted accordingly.
+
+The return code of the BPF program is either BPF_OK to indicate successful
+dissection, or BPF_DROP to indicate parsing error.
+
+__sk_buff->data
+===============
+
+In the VLAN-less case, this is what the initial state of the BPF flow
+dissector looks like:
++------+------+------------+-----------+
+| DMAC | SMAC | ETHER_TYPE | L3_HEADER |
++------+------+------------+-----------+
+                            ^
+                            |
+                            +-- flow dissector starts here
+
+skb->data + flow_keys->nhoff point to the first byte of L3_HEADER.
+flow_keys->thoff = nhoff
+flow_keys->n_proto = ETHER_TYPE
+
+
+In case of VLAN, flow dissector can be called with the two different states.
+
+Pre-VLAN parsing:
++------+------+------+-----+-----------+-----------+
+| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
++------+------+------+-----+-----------+-----------+
+                      ^
+                      |
+                      +-- flow dissector starts here
+
+skb->data + flow_keys->nhoff point the to first byte of TCI.
+flow_keys->thoff = nhoff
+flow_keys->n_proto = TPID
+
+Please note that TPID can be 802.1AD and, hence, BPF program would
+have to parse VLAN information twice for double tagged packets.
+
+
+Post-VLAN parsing:
++------+------+------+-----+-----------+-----------+
+| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
++------+------+------+-----+-----------+-----------+
+                                        ^
+                                        |
+                                        +-- flow dissector starts here
+
+skb->data + flow_keys->nhoff point the to first byte of L3_HEADER.
+flow_keys->thoff = nhoff
+flow_keys->n_proto = ETHER_TYPE
+
+In this case VLAN information has been processed before the flow dissector
+and BPF flow dissector is not required to handle it.
+
+
+The takeaway here is as follows: BPF flow dissector program can be called with
+the optional VLAN header and should gracefully handle both cases: when single
+or double VLAN is present and when it is not present. The same program
+can be called for both cases and would have to be written carefully to
+handle both cases.
+
+
+Reference Implementation
+========================
+
+See tools/testing/selftests/bpf/progs/bpf_flow.c for the reference
+implementation and tools/testing/selftests/bpf/flow_dissector_load.[hc] for
+the loader. bpftool can be used to load BPF flow dissector program as well.
+
+The reference implementation is organized as follows:
+* jmp_table map that contains sub-programs for each supported L3 protocol
+* _dissect routine - entry point; it does input n_proto parsing and does
+  bpf_tail_call to the appropriate L3 handler
+
+Since BPF at this point doesn't support looping (or any jumping back),
+jmp_table is used instead to handle multiple levels of encapsulation (and
+IPv6 options).
+
+
+Current Limitations
+===================
+BPF flow dissector doesn't support exporting all the metadata that in-kernel
+C-based implementation can export. Notable example is single VLAN (802.1Q)
+and double VLAN (802.1AD) tags. Please refer to the 'struct bpf_flow_keys'
+for a set of information that's currently can be exported from the BPF context.
-- 
2.21.0.392.gf8f6787159e-goog

Re: [PATCH bpf 5/5] flow_dissector: document BPF flow dissector environment

[Petar Penkov]

On Mon, Apr 1, 2019 at 1:57 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Short doc on what BPF flow dissector should expect in the input
> __sk_buff and flow_keys.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
>  1 file changed, 115 insertions(+)
>  create mode 100644 Documentation/networking/bpf_flow_dissector.txt
>
> diff --git a/Documentation/networking/bpf_flow_dissector.txt b/Documentation/networking/bpf_flow_dissector.txt
> new file mode 100644
> index 000000000000..513be8e20afb
> --- /dev/null
> +++ b/Documentation/networking/bpf_flow_dissector.txt
> @@ -0,0 +1,115 @@
> +==================
> +BPF Flow Dissector
> +==================
> +
> +Overview
> +========
> +
> +Flow dissector is a routine that parses metadata out of the packets. It's
> +used in the various places in the networking subsystem (RFS, flow hash, etc).
> +
> +BPF flow dissector is an attempt to reimplement C-based flow dissector logic
> +in BPF to gain all the benefits of BPF verifier (namely, limits on the
> +number of instructions and tail calls).
> +
> +API
> +===
> +
> +BPF flow dissector programs operate on an __sk_buff. However, only the
> +limited set of fields is allowed: data, data_end and flow_keys. flow_keys
> +is 'struct bpf_flow_keys' and contains flow dissector input and
> +output arguments.
> +
> +The inputs are:
> +  * nhoff - initial offset of the networking header
> +  * thoff - initial offset of the transport header, initialized to nhoff
> +  * n_proto - L3 protocol type, parsed out of L2 header
> +
> +Flow dissector BPF program should fill out the rest of the 'struct
> +bpf_flow_keys' fields. Input arguments nhoff/thoff/n_proto should be also
> +adjusted accordingly.
> +
> +The return code of the BPF program is either BPF_OK to indicate successful
> +dissection, or BPF_DROP to indicate parsing error.
I don't think this is actually enforced. I believe the current code
just checks if the status is BPF_OK or not, rather than BPF_OK,
BPF_DROP, or neither.

> +
> +__sk_buff->data
> +===============
> +
> +In the VLAN-less case, this is what the initial state of the BPF flow
> +dissector looks like:
> ++------+------+------------+-----------+
> +| DMAC | SMAC | ETHER_TYPE | L3_HEADER |
> ++------+------+------------+-----------+
> +                            ^
> +                            |
> +                            +-- flow dissector starts here
> +
> +skb->data + flow_keys->nhoff point to the first byte of L3_HEADER.
> +flow_keys->thoff = nhoff
> +flow_keys->n_proto = ETHER_TYPE
> +
> +
> +In case of VLAN, flow dissector can be called with the two different states.
> +
> +Pre-VLAN parsing:
> ++------+------+------+-----+-----------+-----------+
> +| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
> ++------+------+------+-----+-----------+-----------+
> +                      ^
> +                      |
> +                      +-- flow dissector starts here
> +
> +skb->data + flow_keys->nhoff point the to first byte of TCI.
> +flow_keys->thoff = nhoff
> +flow_keys->n_proto = TPID
> +
> +Please note that TPID can be 802.1AD and, hence, BPF program would
> +have to parse VLAN information twice for double tagged packets.
> +
> +
> +Post-VLAN parsing:
> ++------+------+------+-----+-----------+-----------+
> +| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
> ++------+------+------+-----+-----------+-----------+
> +                                        ^
> +                                        |
> +                                        +-- flow dissector starts here
> +
> +skb->data + flow_keys->nhoff point the to first byte of L3_HEADER.
> +flow_keys->thoff = nhoff
> +flow_keys->n_proto = ETHER_TYPE
> +
> +In this case VLAN information has been processed before the flow dissector
> +and BPF flow dissector is not required to handle it.
> +
> +
> +The takeaway here is as follows: BPF flow dissector program can be called with
> +the optional VLAN header and should gracefully handle both cases: when single
> +or double VLAN is present and when it is not present. The same program
> +can be called for both cases and would have to be written carefully to
> +handle both cases.
> +
> +
> +Reference Implementation
> +========================
> +
> +See tools/testing/selftests/bpf/progs/bpf_flow.c for the reference
> +implementation and tools/testing/selftests/bpf/flow_dissector_load.[hc] for
> +the loader. bpftool can be used to load BPF flow dissector program as well.
> +
> +The reference implementation is organized as follows:
> +* jmp_table map that contains sub-programs for each supported L3 protocol
> +* _dissect routine - entry point; it does input n_proto parsing and does
> +  bpf_tail_call to the appropriate L3 handler
> +
> +Since BPF at this point doesn't support looping (or any jumping back),
> +jmp_table is used instead to handle multiple levels of encapsulation (and
> +IPv6 options).
> +
> +
> +Current Limitations
> +===================
> +BPF flow dissector doesn't support exporting all the metadata that in-kernel
> +C-based implementation can export. Notable example is single VLAN (802.1Q)
> +and double VLAN (802.1AD) tags. Please refer to the 'struct bpf_flow_keys'
> +for a set of information that's currently can be exported from the BPF context.
> --
> 2.21.0.392.gf8f6787159e-goog
>

Re: [PATCH bpf 5/5] flow_dissector: document BPF flow dissector environment

[Stanislav Fomichev]

On 04/02, Petar Penkov wrote:
> On Mon, Apr 1, 2019 at 1:57 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Short doc on what BPF flow dissector should expect in the input
> > __sk_buff and flow_keys.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
> >  1 file changed, 115 insertions(+)
> >  create mode 100644 Documentation/networking/bpf_flow_dissector.txt
> >
> > diff --git a/Documentation/networking/bpf_flow_dissector.txt b/Documentation/networking/bpf_flow_dissector.txt
> > new file mode 100644
> > index 000000000000..513be8e20afb
> > --- /dev/null
> > +++ b/Documentation/networking/bpf_flow_dissector.txt
> > @@ -0,0 +1,115 @@
> > +==================
> > +BPF Flow Dissector
> > +==================
> > +
> > +Overview
> > +========
> > +
> > +Flow dissector is a routine that parses metadata out of the packets. It's
> > +used in the various places in the networking subsystem (RFS, flow hash, etc).
> > +
> > +BPF flow dissector is an attempt to reimplement C-based flow dissector logic
> > +in BPF to gain all the benefits of BPF verifier (namely, limits on the
> > +number of instructions and tail calls).
> > +
> > +API
> > +===
> > +
> > +BPF flow dissector programs operate on an __sk_buff. However, only the
> > +limited set of fields is allowed: data, data_end and flow_keys. flow_keys
> > +is 'struct bpf_flow_keys' and contains flow dissector input and
> > +output arguments.
> > +
> > +The inputs are:
> > +  * nhoff - initial offset of the networking header
> > +  * thoff - initial offset of the transport header, initialized to nhoff
> > +  * n_proto - L3 protocol type, parsed out of L2 header
> > +
> > +Flow dissector BPF program should fill out the rest of the 'struct
> > +bpf_flow_keys' fields. Input arguments nhoff/thoff/n_proto should be also
> > +adjusted accordingly.
> > +
> > +The return code of the BPF program is either BPF_OK to indicate successful
> > +dissection, or BPF_DROP to indicate parsing error.
> I don't think this is actually enforced. I believe the current code
> just checks if the status is BPF_OK or not, rather than BPF_OK,
> BPF_DROP, or neither.
It's not universally enforced, but some codepaths in the kernel look at
the returned value (e.g. skb_get_poff and eth_get_headlen), so it's
better to set the expectations :-)

> > +
> > +__sk_buff->data
> > +===============
> > +
> > +In the VLAN-less case, this is what the initial state of the BPF flow
> > +dissector looks like:
> > ++------+------+------------+-----------+
> > +| DMAC | SMAC | ETHER_TYPE | L3_HEADER |
> > ++------+------+------------+-----------+
> > +                            ^
> > +                            |
> > +                            +-- flow dissector starts here
> > +
> > +skb->data + flow_keys->nhoff point to the first byte of L3_HEADER.
> > +flow_keys->thoff = nhoff
> > +flow_keys->n_proto = ETHER_TYPE
> > +
> > +
> > +In case of VLAN, flow dissector can be called with the two different states.
> > +
> > +Pre-VLAN parsing:
> > ++------+------+------+-----+-----------+-----------+
> > +| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
> > ++------+------+------+-----+-----------+-----------+
> > +                      ^
> > +                      |
> > +                      +-- flow dissector starts here
> > +
> > +skb->data + flow_keys->nhoff point the to first byte of TCI.
> > +flow_keys->thoff = nhoff
> > +flow_keys->n_proto = TPID
> > +
> > +Please note that TPID can be 802.1AD and, hence, BPF program would
> > +have to parse VLAN information twice for double tagged packets.
> > +
> > +
> > +Post-VLAN parsing:
> > ++------+------+------+-----+-----------+-----------+
> > +| DMAC | SMAC | TPID | TCI |ETHER_TYPE | L3_HEADER |
> > ++------+------+------+-----+-----------+-----------+
> > +                                        ^
> > +                                        |
> > +                                        +-- flow dissector starts here
> > +
> > +skb->data + flow_keys->nhoff point the to first byte of L3_HEADER.
> > +flow_keys->thoff = nhoff
> > +flow_keys->n_proto = ETHER_TYPE
> > +
> > +In this case VLAN information has been processed before the flow dissector
> > +and BPF flow dissector is not required to handle it.
> > +
> > +
> > +The takeaway here is as follows: BPF flow dissector program can be called with
> > +the optional VLAN header and should gracefully handle both cases: when single
> > +or double VLAN is present and when it is not present. The same program
> > +can be called for both cases and would have to be written carefully to
> > +handle both cases.
> > +
> > +
> > +Reference Implementation
> > +========================
> > +
> > +See tools/testing/selftests/bpf/progs/bpf_flow.c for the reference
> > +implementation and tools/testing/selftests/bpf/flow_dissector_load.[hc] for
> > +the loader. bpftool can be used to load BPF flow dissector program as well.
> > +
> > +The reference implementation is organized as follows:
> > +* jmp_table map that contains sub-programs for each supported L3 protocol
> > +* _dissect routine - entry point; it does input n_proto parsing and does
> > +  bpf_tail_call to the appropriate L3 handler
> > +
> > +Since BPF at this point doesn't support looping (or any jumping back),
> > +jmp_table is used instead to handle multiple levels of encapsulation (and
> > +IPv6 options).
> > +
> > +
> > +Current Limitations
> > +===================
> > +BPF flow dissector doesn't support exporting all the metadata that in-kernel
> > +C-based implementation can export. Notable example is single VLAN (802.1Q)
> > +and double VLAN (802.1AD) tags. Please refer to the 'struct bpf_flow_keys'
> > +for a set of information that's currently can be exported from the BPF context.
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >

Re: [PATCH bpf 5/5] flow_dissector: document BPF flow dissector environment

[Jesper Dangaard Brouer]

On Mon,  1 Apr 2019 13:57:34 -0700
Stanislav Fomichev <sdf@google.com> wrote:

> diff --git a/Documentation/networking/bpf_flow_dissector.txt b/Documentation/networking/bpf_flow_dissector.txt
> new file mode 100644
> index 000000000000..513be8e20afb
> --- /dev/null
> +++ b/Documentation/networking/bpf_flow_dissector.txt

It looks like you use the RST format, but you use suffix .txt and not .rst.

If you don't know, these files get rendered on:
 https://www.kernel.org/doc/html/latest/bpf/index.html

And GitHub also render this stuff e.g.
 https://github.com/torvalds/linux/blob/master/Documentation/bpf/bpf_devel_QA.rst
 

> @@ -0,0 +1,115 @@
> +==================
> +BPF Flow Dissector
> +==================
> +
> +Overview
> +========
> +
> +Flow dissector is a routine that parses metadata out of the packets. It's
> +used in the various places in the networking subsystem (RFS, flow hash, etc).
> +
> +BPF flow dissector is an attempt to reimplement C-based flow dissector logic
> +in BPF to gain all the benefits of BPF verifier (namely, limits on the
> +number of instructions and tail calls).
> +
> +API
> +===
> +


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH bpf 5/5] flow_dissector: document BPF flow dissector environment

[Stanislav Fomichev]

On 04/03, Jesper Dangaard Brouer wrote:
> On Mon,  1 Apr 2019 13:57:34 -0700
> Stanislav Fomichev <sdf@google.com> wrote:
> 
> > diff --git a/Documentation/networking/bpf_flow_dissector.txt b/Documentation/networking/bpf_flow_dissector.txt
> > new file mode 100644
> > index 000000000000..513be8e20afb
> > --- /dev/null
> > +++ b/Documentation/networking/bpf_flow_dissector.txt
> 
> It looks like you use the RST format, but you use suffix .txt and not .rst.
Thanks for the suggestion, let me try to rename it to .rst and build local
htmldocs to make sure it renders correctly (I'm not sure about the ascii art).
If that looks good, I'll follow up with a rename patch.

> 
> If you don't know, these files get rendered on:
>  https://www.kernel.org/doc/html/latest/bpf/index.html
> 
> And GitHub also render this stuff e.g.
>  https://github.com/torvalds/linux/blob/master/Documentation/bpf/bpf_devel_QA.rst
>  
> 
> > @@ -0,0 +1,115 @@
> > +==================
> > +BPF Flow Dissector
> > +==================
> > +
> > +Overview
> > +========
> > +
> > +Flow dissector is a routine that parses metadata out of the packets. It's
> > +used in the various places in the networking subsystem (RFS, flow hash, etc).
> > +
> > +BPF flow dissector is an attempt to reimplement C-based flow dissector logic
> > +in BPF to gain all the benefits of BPF verifier (namely, limits on the
> > +number of instructions and tail calls).
> > +
> > +API
> > +===
> > +
> 
> 
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH bpf 0/5] flow_dissector: lay groundwork for calling BPF hook from eth_get_headlen

[Willem de Bruijn]

On Mon, Apr 1, 2019 at 4:57 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> This patch series fixes the existing BPF flow dissector API to
> support calling BPF progs from the eth_get_headlen context (the support
> itself will be added in bpf-next tree).
>
> The summary of the changes:
>   * fix VLAN handling in bpf_flow.c, we don't need to peek back and look
>     at skb->vlan_present; add selftests
>   * pass and use flow_keys->n_proto instead of skb->protocol
>   * fix clamping of flow_keys->nhoff for packets with nhoff > 0
>   * prohibit access to most of the __sk_buff fields from BPF flow
>     dissector progs; only data/data_end/flow_keys are allowed (all input
>     is now passed via flow_keys)
>   * finally, document BPF flow dissector program environment
>
> Stanislav Fomichev (5):
>   selftests/bpf: fix vlan handling in flow dissector program
>   net/flow_dissector: pass flow_keys->n_proto to BPF programs
>   flow_dissector: fix clamping of BPF flow_keys for non-zero nhoff
>   flow_dissector: allow access only to a subset of __sk_buff fields
>   flow_dissector: document BPF flow dissector environment

For the series:

Acked-by: Willem de Bruijn <willemb@google.com>

This looks great to me. Thanks, Stan!

>
>  .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
>  net/core/filter.c                             |  16 +--
>  net/core/flow_dissector.c                     |   4 +-
>  .../selftests/bpf/prog_tests/flow_dissector.c |  68 +++++++++++
>  tools/testing/selftests/bpf/progs/bpf_flow.c  |  19 ++-
>  5 files changed, 196 insertions(+), 26 deletions(-)
>  create mode 100644 Documentation/networking/bpf_flow_dissector.txt
>
> --
> 2.21.0.392.gf8f6787159e-goog

Re: [PATCH bpf 0/5] flow_dissector: lay groundwork for calling BPF hook from eth_get_headlen

[Petar Penkov]

On Tue, Apr 2, 2019 at 1:18 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Mon, Apr 1, 2019 at 4:57 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > This patch series fixes the existing BPF flow dissector API to
> > support calling BPF progs from the eth_get_headlen context (the support
> > itself will be added in bpf-next tree).
> >
> > The summary of the changes:
> >   * fix VLAN handling in bpf_flow.c, we don't need to peek back and look
> >     at skb->vlan_present; add selftests
> >   * pass and use flow_keys->n_proto instead of skb->protocol
> >   * fix clamping of flow_keys->nhoff for packets with nhoff > 0
> >   * prohibit access to most of the __sk_buff fields from BPF flow
> >     dissector progs; only data/data_end/flow_keys are allowed (all input
> >     is now passed via flow_keys)
> >   * finally, document BPF flow dissector program environment
> >
> > Stanislav Fomichev (5):
> >   selftests/bpf: fix vlan handling in flow dissector program
> >   net/flow_dissector: pass flow_keys->n_proto to BPF programs
> >   flow_dissector: fix clamping of BPF flow_keys for non-zero nhoff
> >   flow_dissector: allow access only to a subset of __sk_buff fields
> >   flow_dissector: document BPF flow dissector environment
>
> For the series:
>
> Acked-by: Willem de Bruijn <willemb@google.com>
>
> This looks great to me. Thanks, Stan!
>
Acked-by: Petar Penkov <peterpenkov96@gmail.com>

Thanks, Stan!
> >
> >  .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
> >  net/core/filter.c                             |  16 +--
> >  net/core/flow_dissector.c                     |   4 +-
> >  .../selftests/bpf/prog_tests/flow_dissector.c |  68 +++++++++++
> >  tools/testing/selftests/bpf/progs/bpf_flow.c  |  19 ++-
> >  5 files changed, 196 insertions(+), 26 deletions(-)
> >  create mode 100644 Documentation/networking/bpf_flow_dissector.txt
> >
> > --
> > 2.21.0.392.gf8f6787159e-goog

Re: [PATCH bpf 0/5] flow_dissector: lay groundwork for calling BPF hook from eth_get_headlen

[Daniel Borkmann]

On 04/01/2019 10:57 PM, Stanislav Fomichev wrote:
> This patch series fixes the existing BPF flow dissector API to
> support calling BPF progs from the eth_get_headlen context (the support
> itself will be added in bpf-next tree).
> 
> The summary of the changes:
>   * fix VLAN handling in bpf_flow.c, we don't need to peek back and look
>     at skb->vlan_present; add selftests
>   * pass and use flow_keys->n_proto instead of skb->protocol
>   * fix clamping of flow_keys->nhoff for packets with nhoff > 0
>   * prohibit access to most of the __sk_buff fields from BPF flow
>     dissector progs; only data/data_end/flow_keys are allowed (all input
>     is now passed via flow_keys)
>   * finally, document BPF flow dissector program environment
> 
> Stanislav Fomichev (5):
>   selftests/bpf: fix vlan handling in flow dissector program
>   net/flow_dissector: pass flow_keys->n_proto to BPF programs
>   flow_dissector: fix clamping of BPF flow_keys for non-zero nhoff
>   flow_dissector: allow access only to a subset of __sk_buff fields
>   flow_dissector: document BPF flow dissector environment
> 
>  .../networking/bpf_flow_dissector.txt         | 115 ++++++++++++++++++
>  net/core/filter.c                             |  16 +--
>  net/core/flow_dissector.c                     |   4 +-
>  .../selftests/bpf/prog_tests/flow_dissector.c |  68 +++++++++++
>  tools/testing/selftests/bpf/progs/bpf_flow.c  |  19 ++-
>  5 files changed, 196 insertions(+), 26 deletions(-)
>  create mode 100644 Documentation/networking/bpf_flow_dissector.txt

LGTM, applied, thanks!