[PATCH bpf-next 0/2] bpf: offer maximum packet offset info

[PATCH bpf-next 0/2] bpf: offer maximum packet offset info

[Jiong Wang]

The maximum packet offset accessed by one BPF program is useful
information.

Because sometimes there could be packet split and it is possible for some
reasons (for example performance) we want to reject the BPF program if the
maximum packet size would trigger such split. Normally, MTU value is
treated as the maximum packet size, but one BPF program does not always
access the whole packet, it could only access the head portion of the data.

We could let verifier calculate the maximum packet offset ever used and
record it inside prog auxiliar information structure as a new field
"max_pkt_offset".

Jiong Wang (2):
  bpf: let verifier to calculate and record max_pkt_offset
  nfp: bpf: relax prog rejection through max_pkt_offset

 drivers/net/ethernet/netronome/nfp/bpf/offload.c |  9 +++++----
 include/linux/bpf.h                              |  1 +
 kernel/bpf/verifier.c                            | 12 ++++++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH bpf-next 1/2] bpf: let verifier to calculate and record max_pkt_offset

[Jiong Wang]

In check_packet_access, update max_pkt_offset after the offset has passed
__check_packet_access.

It should be safe to use u32 for max_pkt_offset as explained in code
comment.

Also, when there is tail call, the max_pkt_offset of the called program is
unknown, so conservatively set max_pkt_offset to MAX_PACKET_OFF for such
case.

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
 include/linux/bpf.h   |  1 +
 kernel/bpf/verifier.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 33014ae..b6a296e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -293,6 +293,7 @@ struct bpf_prog_aux {
 	atomic_t refcnt;
 	u32 used_map_cnt;
 	u32 max_ctx_offset;
+	u32 max_pkt_offset;
 	u32 stack_depth;
 	u32 id;
 	u32 func_cnt;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 98fa0be..6a248d8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1452,6 +1452,17 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
 		verbose(env, "R%d offset is outside of the packet\n", regno);
 		return err;
 	}
+
+	/* __check_packet_access has made sure "off + size - 1" is within u16.
+	 * reg->umax_value can't be bigger than MAX_PACKET_OFF which is 0xffff,
+	 * otherwise find_good_pkt_pointers would have refused to set range info
+	 * that __check_packet_access would have rejected this pkt access.
+	 * Therefore, "off + reg->umax_value + size - 1" won't overflow u32.
+	 */
+	env->prog->aux->max_pkt_offset =
+		max_t(u32, env->prog->aux->max_pkt_offset,
+		      off + reg->umax_value + size - 1);
+
 	return err;
 }
 
@@ -6128,6 +6139,7 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
 			 */
 			prog->cb_access = 1;
 			env->prog->aux->stack_depth = MAX_BPF_STACK;
+			env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
 
 			/* mark bpf_tail_call as different opcode to avoid
 			 * conditional branch in the interpeter for every normal
-- 
2.7.4

[PATCH bpf-next 2/2] nfp: bpf: relax prog rejection through max_pkt_offset

[Jiong Wang]

NFP is refusing to offload programs whenever the MTU is set to a value
larger than the max packet bytes that fits in NFP Cluster Target Memory
(CTM). However, a eBPF program doesn't always need to access the whole
packet data.

Verifier has always calculated maximum direct packet access (DPA) offset,
and kept it in max_pkt_offset inside prog auxiliar information. This patch
relax prog rejection based on max_pkt_offset.

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index ba8ceed..07bdc1f 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -489,14 +489,15 @@ nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog,
 		 struct netlink_ext_ack *extack)
 {
 	struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
-	unsigned int max_mtu, max_stack, max_prog_len;
+	unsigned int fw_mtu, pkt_off, max_stack, max_prog_len;
 	dma_addr_t dma_addr;
 	void *img;
 	int err;
 
-	max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
-	if (max_mtu < nn->dp.netdev->mtu) {
-		NL_SET_ERR_MSG_MOD(extack, "BPF offload not supported with MTU larger than HW packet split boundary");
+	fw_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
+	pkt_off = min(prog->aux->max_pkt_offset, nn->dp.netdev->mtu);
+	if (fw_mtu < pkt_off) {
+		NL_SET_ERR_MSG_MOD(extack, "BPF offload not supported with potential packet access beyond HW packet split boundary");
 		return -EOPNOTSUPP;
 	}
 
-- 
2.7.4

Re: [PATCH bpf-next 0/2] bpf: offer maximum packet offset info

[Daniel Borkmann]

On 11/08/2018 10:08 AM, Jiong Wang wrote:
> The maximum packet offset accessed by one BPF program is useful
> information.
> 
> Because sometimes there could be packet split and it is possible for some
> reasons (for example performance) we want to reject the BPF program if the
> maximum packet size would trigger such split. Normally, MTU value is
> treated as the maximum packet size, but one BPF program does not always
> access the whole packet, it could only access the head portion of the data.
> 
> We could let verifier calculate the maximum packet offset ever used and
> record it inside prog auxiliar information structure as a new field
> "max_pkt_offset".
> 
> Jiong Wang (2):
>   bpf: let verifier to calculate and record max_pkt_offset
>   nfp: bpf: relax prog rejection through max_pkt_offset
> 
>  drivers/net/ethernet/netronome/nfp/bpf/offload.c |  9 +++++----
>  include/linux/bpf.h                              |  1 +
>  kernel/bpf/verifier.c                            | 12 ++++++++++++
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 

Applied to bpf-next, thanks!