RE: [PATCH v3 5/5] bpf: remove use of VLA

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> Replace variable length array with two part loop which
> applies filter in bursts up to 32 packets.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/bpf/bpf_pkt.c   | 81 ++++++++++++++++++++++++++-------------------
>  lib/bpf/meson.build |  2 --
>  2 files changed, 47 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/bpf/bpf_pkt.c b/lib/bpf/bpf_pkt.c
> index 087ac0f244..15d36739a0 100644
> --- a/lib/bpf/bpf_pkt.c
> +++ b/lib/bpf/bpf_pkt.c
> @@ -157,13 +157,16 @@ bpf_eth_cbh_add(struct bpf_eth_cbh *cbh, uint16_t
> port, uint16_t queue)
>  /*
>   * BPF packet processing routines.
>   */
> +#define BPF_FILTER_BURST	32u
> 
>  static inline uint32_t
>  apply_filter(struct rte_mbuf *mb[], const uint64_t rc[], uint32_t num,
>  	uint32_t drop)
>  {
>  	uint32_t i, j, k;
> -	struct rte_mbuf *dr[num];
> +	struct rte_mbuf *dr[BPF_FILTER_BURST];
> +
> +	RTE_ASSERT(num <= BPF_FILTER_BURST);
> 
>  	for (i = 0, j = 0, k = 0; i != num; i++) {

After looking at the code more closely - it might not be that straightforward
as I thought:  apply_filter() groups 'good' packets consequently, and for
'bad' packets does one of two things:
a) if drop != 0, then it frees them out  (RX filter)
b) if drop == 0, then it places them after 'good' packets (TX filter)
While a) is simple to do with fixed length array (we just need read and write index),
to support b) - we need to do something more awkward, i.e. - walk mb[] in backwards
order, and/or keep copying bad packets to the end at each iteration.
All that looks quite complicated, so if for now you'll decide to switch back to alloca()
based approach (first version of the patch) - I'll be ok with that.
Sorry for not spotting that earlier.
Konstantin

> 
> @@ -191,65 +194,75 @@ static inline uint32_t
>  pkt_filter_vm(const struct rte_bpf *bpf, struct rte_mbuf *mb[], uint32_t num,
>  	uint32_t drop)
>  {
> -	uint32_t i;
> -	void *dp[num];
> -	uint64_t rc[num];
> +	uint32_t matched = 0;
> +
> +	for (uint32_t i = 0; i < num; i += BPF_FILTER_BURST) {
> +		uint32_t burst_sz = RTE_MIN(num, BPF_FILTER_BURST);
> +		void *dp[BPF_FILTER_BURST];
> +		uint64_t rc[BPF_FILTER_BURST];
> 
> -	for (i = 0; i != num; i++)
> -		dp[i] = rte_pktmbuf_mtod(mb[i], void *);
> +		for (uint32_t j = 0; j < burst_sz; j++)
> +			dp[j] = rte_pktmbuf_mtod(mb[i + j], void *);
> 
> -	rte_bpf_exec_burst(bpf, dp, rc, num);
> -	return apply_filter(mb, rc, num, drop);
> +		rte_bpf_exec_burst(bpf, dp, rc, burst_sz);
> +		matched += apply_filter(mb + i, rc, burst_sz, drop);
> +	}
> +	return matched;
>  }
> 
>  static inline uint32_t
>  pkt_filter_jit(const struct rte_bpf_jit *jit, struct rte_mbuf *mb[],
>  	uint32_t num, uint32_t drop)
>  {
> -	uint32_t i, n;
> -	void *dp;
> -	uint64_t rc[num];
> -
> -	n = 0;
> -	for (i = 0; i != num; i++) {
> -		dp = rte_pktmbuf_mtod(mb[i], void *);
> -		rc[i] = jit->func(dp);
> -		n += (rc[i] == 0);
> -	}
> +	uint32_t matched = 0;
> +
> +	for (uint32_t i = 0; i < num; i += BPF_FILTER_BURST) {
> +		uint32_t burst_sz = RTE_MIN(num, BPF_FILTER_BURST);
> +		uint64_t rc[BPF_FILTER_BURST];
> +
> +		for (uint32_t j = 0; j < burst_sz; j++) {
> +			void *dp = rte_pktmbuf_mtod(mb[i + j], void *);
> 
> -	if (n != 0)
> -		num = apply_filter(mb, rc, num, drop);
> +			rc[j] = jit->func(dp);
> +		}
> 
> -	return num;
> +		matched += apply_filter(mb + i, rc, burst_sz, drop);
> +	}
> +	return matched;
>  }
> 
>  static inline uint32_t
>  pkt_filter_mb_vm(const struct rte_bpf *bpf, struct rte_mbuf *mb[], uint32_t
> num,
>  	uint32_t drop)
>  {
> -	uint64_t rc[num];
> +	uint32_t matched = 0;
> +
> +	for (uint32_t i = 0; i < num; i += BPF_FILTER_BURST) {
> +		uint32_t burst_sz = RTE_MIN(num, BPF_FILTER_BURST);
> +		uint64_t rc[BPF_FILTER_BURST];
> 
> -	rte_bpf_exec_burst(bpf, (void **)mb, rc, num);
> -	return apply_filter(mb, rc, num, drop);
> +		rte_bpf_exec_burst(bpf, (void **)mb, rc, burst_sz);
> +		matched += apply_filter(mb + i, rc, burst_sz, drop);
> +	}
> +	return matched;
>  }
> 
>  static inline uint32_t
>  pkt_filter_mb_jit(const struct rte_bpf_jit *jit, struct rte_mbuf *mb[],
>  	uint32_t num, uint32_t drop)
>  {
> -	uint32_t i, n;
> -	uint64_t rc[num];
> +	uint32_t matched = 0;
> 
> -	n = 0;
> -	for (i = 0; i != num; i++) {
> -		rc[i] = jit->func(mb[i]);
> -		n += (rc[i] == 0);
> -	}
> +	for (uint32_t i = 0; i < num; i += BPF_FILTER_BURST) {
> +		uint32_t burst_sz = RTE_MIN(num, BPF_FILTER_BURST);
> +		uint64_t rc[BPF_FILTER_BURST];
> 
> -	if (n != 0)
> -		num = apply_filter(mb, rc, num, drop);
> +		for (uint32_t j = 0; j < burst_sz; j++)
> +			rc[j] = jit->func(mb[i + j]);
> 
> -	return num;
> +		matched += apply_filter(mb + i, rc, burst_sz, drop);
> +	}
> +	return matched;
>  }
> 
>  /*
> diff --git a/lib/bpf/meson.build b/lib/bpf/meson.build
> index 28df7f469a..aa258a9061 100644
> --- a/lib/bpf/meson.build
> +++ b/lib/bpf/meson.build
> @@ -7,8 +7,6 @@ if is_windows
>      subdir_done()
>  endif
> 
> -cflags += no_wvla_cflag
> -
>  if arch_subdir == 'x86' and dpdk_conf.get('RTE_ARCH_32')
>      build = false
>      reason = 'not supported on 32-bit x86'
> --
> 2.51.0