Thank you for working on this, please see some comments inline.
> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Sunday 21 June 2026 17:24
> To: [email protected]
> Cc: Stephen Hemminger <[email protected]>; Konstantin Ananyev
> <[email protected]>;
> Marat Khalili <[email protected]>
> Subject: [PATCH v3 6/6] test/bpf: check that bpf_convert can be JIT'd
>
> Add followup in bpf conversion tests to make sure resulting
> code was also run through JIT and that JIT produces
> same results as non-JIT.
>
> Reduce log output to make it easier to match which
> expression might be causing issues.
>
> Signed-off-by: Stephen Hemminger <[email protected]>
> ---
> app/test/test_bpf.c | 94 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
> index 3a88434c3c..973dd7d659 100644
> --- a/app/test/test_bpf.c
> +++ b/app/test/test_bpf.c
> @@ -8,6 +8,7 @@
> #include <inttypes.h>
> #include <unistd.h>
>
> +#include <rte_byteorder.h>
> #include <rte_memory.h>
> #include <rte_debug.h>
> #include <rte_hexdump.h>
> @@ -32,6 +33,7 @@ test_bpf(void)
> #include <rte_bpf.h>
> #include <rte_ether.h>
> #include <rte_ip.h>
> +#include <rte_udp.h>
>
>
> /* Tests of most simple BPF programs (no instructions, one instruction etc.)
> */
> @@ -4529,6 +4531,7 @@ test_bpf_match(pcap_t *pcap, const char *str,
> int ret = -1;
> uint64_t rc;
>
> + printf("%s '%s'\n", __func__, str);
> if (pcap_compile(pcap, &fcode, str, 1, PCAP_NETMASK_UNKNOWN)) {
> printf("%s@%d: pcap_compile(\"%s\") failed: %s;\n",
> __func__, __LINE__, str, pcap_geterr(pcap));
> @@ -4550,6 +4553,24 @@ test_bpf_match(pcap_t *pcap, const char *str,
> }
>
> rc = rte_bpf_exec(bpf, mb);
> +#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)
We are going to have a few of these lines, I think something like
RTE_BPF_JIT_SUPPORTED is warranted.
> + {
> + struct rte_bpf_jit jit;
> +
> + rte_bpf_get_jit(bpf, &jit);
Out of abundance of caution I would also prefill jit with zeroes and check the
return code here.
> + if (jit.func == NULL) {
> + printf("%s@%d: no JIT generated\n", __func__, __LINE__);
> + goto error;
> + }
> +
> + fflush(stdout);
> + uint64_t rc_jit = jit.func(mb);
> + if (rc_jit != rc) {
> + printf("%s@%d: JIT return code does not match\n",
> __func__, __LINE__);
> + goto error;
> + }
> + }
> +#endif
> /* The return code from bpf capture filter is non-zero if matched */
> ret = (rc == 0);
> error:
> @@ -4560,23 +4581,16 @@ test_bpf_match(pcap_t *pcap, const char *str,
> return ret;
> }
>
> -/* Basic sanity test can we match a IP packet */
> -static int
> -test_bpf_filter_sanity(pcap_t *pcap)
> +/* Setup mbuf for filter test */
> +static void
> +dummy_ip_prep(void *data, uint16_t plen)
> {
> - const uint32_t plen = 100;
> - struct rte_mbuf mb, *m;
> - uint8_t tbuf[RTE_MBUF_DEFAULT_BUF_SIZE];
> struct {
> struct rte_ether_hdr eth_hdr;
> struct rte_ipv4_hdr ip_hdr;
> - } *hdr;
> + struct rte_udp_hdr udp_hdr;
> + } *hdr = data;
>
> - memset(&mb, 0, sizeof(mb));
> - dummy_mbuf_prep(&mb, tbuf, sizeof(tbuf), plen);
> - m = &mb;
> -
> - hdr = rte_pktmbuf_mtod(m, typeof(hdr));
> hdr->eth_hdr = (struct rte_ether_hdr) {
> .dst_addr.addr_bytes = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
> .ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4),
> @@ -4589,13 +4603,32 @@ test_bpf_filter_sanity(pcap_t *pcap)
> .src_addr = rte_cpu_to_be_32(RTE_IPV4_LOOPBACK),
> .dst_addr = rte_cpu_to_be_32(RTE_IPV4_BROADCAST),
> };
> + hdr->udp_hdr = (struct rte_udp_hdr) {
> + .src_port = rte_rand_max(UINT16_MAX),
> + .dst_port = rte_cpu_to_be_16(9), /* discard port */
> + .dgram_len = rte_cpu_to_be_16(plen - sizeof(struct
> rte_ipv4_hdr)),
> + .dgram_cksum = 0,
> + };
> +}
> +
> +
> +/* Basic sanity test can we match a IP packet */
> +static int
> +test_bpf_filter_sanity(pcap_t *pcap)
> +{
> + struct rte_mbuf mb = { 0 };
> + uint8_t tbuf[RTE_MBUF_DEFAULT_BUF_SIZE];
> + const uint32_t plen = 100;
> +
> + dummy_mbuf_prep(&mb, tbuf, sizeof(tbuf), plen);
> + dummy_ip_prep(rte_pktmbuf_mtod(&mb, void *), plen);
>
> - if (test_bpf_match(pcap, "ip", m) != 0) {
> + if (test_bpf_match(pcap, "ip", &mb) != 0) {
> printf("%s@%d: filter \"ip\" doesn't match test data\n",
> __func__, __LINE__);
> return -1;
> }
> - if (test_bpf_match(pcap, "not ip", m) == 0) {
> + if (test_bpf_match(pcap, "not ip", &mb) == 0) {
Not a new bug, but this condition should be for non-positive, not just zero.
> printf("%s@%d: filter \"not ip\" does match test data\n",
> __func__, __LINE__);
> return -1;
> @@ -4648,10 +4681,15 @@ static const char * const sample_filters[] = {
> static int
> test_bpf_filter(pcap_t *pcap, const char *s)
> {
> + struct rte_mbuf mb = { 0 };
> + uint8_t tbuf[RTE_MBUF_DEFAULT_BUF_SIZE];
> + const uint32_t plen = 100;
> struct bpf_program fcode;
> struct rte_bpf_prm *prm = NULL;
> struct rte_bpf *bpf = NULL;
> + int ret = -1;
>
> + printf("%s '%s'\n", __func__, s);
> if (pcap_compile(pcap, &fcode, s, 1, PCAP_NETMASK_UNKNOWN)) {
> printf("%s@%d: pcap_compile('%s') failed: %s;\n",
> __func__, __LINE__, s, pcap_geterr(pcap));
> @@ -4665,8 +4703,10 @@ test_bpf_filter(pcap_t *pcap, const char *s)
> goto error;
> }
>
> +#ifdef DEBUG
> printf("bpf convert for \"%s\" produced:\n", s);
> rte_bpf_dump(stdout, prm->ins, prm->nb_ins);
> +#endif
>
> bpf = rte_bpf_load(prm);
> if (bpf == NULL) {
> @@ -4675,6 +4715,30 @@ test_bpf_filter(pcap_t *pcap, const char *s)
> goto error;
> }
>
> + dummy_mbuf_prep(&mb, tbuf, sizeof(tbuf), plen);
> + dummy_ip_prep(rte_pktmbuf_mtod(&mb, void *), plen);
> +
> + uint64_t rc = rte_bpf_exec(bpf, &mb);
Would it be hard to check the result against a known correct answer?
> +#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)
> + {
> + struct rte_bpf_jit jit;
> +
> + rte_bpf_get_jit(bpf, &jit);
Same suggestion regarding zeroing jit and check return code here.
> + if (jit.func == NULL) {
> + printf("%s@%d: no JIT generated\n", __func__, __LINE__);
> + goto error;
> + }
> +
> + fflush(stdout);
> + uint64_t rc_jit = jit.func(&mb);
> + if (rc_jit != rc) {
> + printf("%s@%d: JIT return code does not match\n",
> __func__, __LINE__);
> + goto error;
> + }
> + }
> +#endif
> + ret = 0;
> +
Are `test_bpf_filter` and `test_bpf_filter_sanity` substantially different any
more, or could they just be merged?
> error:
> if (bpf)
> rte_bpf_destroy(bpf);
> @@ -4685,7 +4749,7 @@ test_bpf_filter(pcap_t *pcap, const char *s)
>
> rte_free(prm);
> pcap_freecode(&fcode);
> - return (bpf == NULL) ? -1 : 0;
> + return ret;
> }
>
> static int
> --
> 2.53.0
