[PATCH] bpf: don't verify classic bpfs

[PATCH] bpf: don't verify classic bpfs

[Yoav Winstein]

When classic BPFs with lots of branching instructions are compiled,
__rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times

results in ~1 minute of bpf validation.
This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
from classic BPF, allowing to safely skip over the validation.

Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
---
 app/test/test_bpf.c   | 2 ++
 lib/bpf/bpf_convert.c | 1 +
 lib/bpf/bpf_load.c    | 6 ++++--
 lib/bpf/rte_bpf.h     | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 53e3a31123..7aae290c1a 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -3407,6 +3407,8 @@ static const char * const sample_filters[] = {
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 1] = 0x03)"
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 2] < 0x04)"
 	" and ((ip[2:2] - 4 * (ip[0] & 0x0F) - 4 * ((tcp[12] & 0xF0) >> 4) > 69))",
+	/* Performance */
+	"ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or 4a:e8:e7:5b:76:ce or 0d:87:fa:7a:a6:6d or bb:fd:c6:4b:bc:ae or 4e:28:dc:f1:1c:f6 or 3d:f2:b7:99:54:55 or 1c:5a:90:4b:82:ce or a7:28:08:ea:c9:84 or 1f:c0:2f:2f:0a:01 or c6:64:91:e9:78:f2 or 5b:cc:3f:90:39:ae or 4c:38:8f:ed:16:5c or cb:89:cb:54:0f:4f or 1e:0d:d7:b2:21:84 or 91:07:2b:88:e0:96 or 6c:ca:cf:bf:cf:3e or b9:df:f1:d6:dd:11 or ea:34:0b:b3:96:9e or 70:e5:18:9f:22:93",
 	/* Other */
 	"len = 128",
 };
diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..bc3c9a5d8e 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -567,6 +567,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	/* Classic BPF programs use mbufs */
 	prm->prog_arg.type = RTE_BPF_ARG_PTR_MBUF;
 	prm->prog_arg.size = sizeof(struct rte_mbuf);
+	prm->skip_verification = true;
 
 	return prm;
 }
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index de43347405..e30797cb49 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -108,13 +108,15 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 		return NULL;
 	}
 
-	rc = __rte_bpf_validate(bpf);
+	if (!prm->skip_verification)
+		rc = __rte_bpf_validate(bpf);
+
 	if (rc == 0) {
 		__rte_bpf_jit(bpf);
 		if (mprotect(bpf, bpf->sz, PROT_READ) != 0)
 			rc = -ENOMEM;
 	}
-
+	
 	if (rc != 0) {
 		rte_bpf_destroy(bpf);
 		rte_errno = -rc;
diff --git a/lib/bpf/rte_bpf.h b/lib/bpf/rte_bpf.h
index 80ebb0210f..6f3b4e3c31 100644
--- a/lib/bpf/rte_bpf.h
+++ b/lib/bpf/rte_bpf.h
@@ -94,6 +94,7 @@ struct rte_bpf_prm {
 	/**< array of external symbols that eBPF code is allowed to reference */
 	uint32_t nb_xsym; /**< number of elements in xsym */
 	struct rte_bpf_arg prog_arg; /**< eBPF program input arg description */
+	bool skip_verification; /**< should skip eBPF verification before load */
 };
 
 /**
-- 
2.34.1

Re: [PATCH] bpf: don't verify classic bpfs

[Stephen Hemminger]

On Sun, 12 May 2024 08:55:45 +0300
Yoav Winstein <yoav.w@claroty.com> wrote:

> When classic BPFs with lots of branching instructions are compiled,
> __rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
> 'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times
> 
> results in ~1 minute of bpf validation.
> This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
> from classic BPF, allowing to safely skip over the validation.
> 
> Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
> ---

No.
Wallpapering over a performance bug in the BPF library is not
the best way to handle this. Please analyze the problem in the BPF
library; it should be fixed there.

RE: [PATCH] bpf: don't verify classic bpfs

[Konstantin Ananyev]

> On Sun, 12 May 2024 08:55:45 +0300
> Yoav Winstein <yoav.w@claroty.com> wrote:
> 
> > When classic BPFs with lots of branching instructions are compiled,
> > __rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
> > 'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times
> >
> > results in ~1 minute of bpf validation.
> > This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
> > from classic BPF, allowing to safely skip over the validation.
> >
> > Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
> > ---
> 
> No.
> Wallpapering over a performance bug in the BPF library is not
> the best way to handle this. Please analyze the problem in the BPF
> library; it should be fixed there.

+1
Blindly disabling verification for all cBPFs is the worst possible option here.
We need at least try to understand what exactly causing such slowdown.

Re: [PATCH] bpf: don't verify classic bpfs

[Thomas Monjalon]

16/05/2024 11:36, Konstantin Ananyev:
> 
> > On Sun, 12 May 2024 08:55:45 +0300
> > Yoav Winstein <yoav.w@claroty.com> wrote:
> > 
> > > When classic BPFs with lots of branching instructions are compiled,
> > > __rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
> > > 'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times
> > >
> > > results in ~1 minute of bpf validation.
> > > This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
> > > from classic BPF, allowing to safely skip over the validation.
> > >
> > > Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
> > > ---
> > 
> > No.
> > Wallpapering over a performance bug in the BPF library is not
> > the best way to handle this. Please analyze the problem in the BPF
> > library; it should be fixed there.
> 
> +1
> Blindly disabling verification for all cBPFs is the worst possible option here.
> We need at least try to understand what exactly causing such slowdown.

+1

You didn't mention it is also breaking ABI compatibility.

RE: [PATCH] bpf: don't verify classic bpfs

[Konstantin Ananyev]

> > > > When classic BPFs with lots of branching instructions are compiled,
> > > > __rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
> > > > 'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times
> > > >
> > > > results in ~1 minute of bpf validation.
> > > > This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
> > > > from classic BPF, allowing to safely skip over the validation.
> > > >
> > > > Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
> > > > ---
> > >
> > > No.
> > > Wallpapering over a performance bug in the BPF library is not
> > > the best way to handle this. Please analyze the problem in the BPF
> > > library; it should be fixed there.
> >
> > +1
> > Blindly disabling verification for all cBPFs is the worst possible option here.
> > We need at least try to understand what exactly causing such slowdown.
> 
> +1
> 
> You didn't mention it is also breaking ABI compatibility.

Yep, it does, thanks Thomas for highlighting it.

Yoav,  can I ask submitter to check would:
https://patchwork.dpdk.org/project/dpdk/list/?series=32321
fix the problem you are facing?
I think that the root cause is the same.
 

[PATCH] bpf: don't verify classic bpfs

[Yoav Winstein]

When classic BPFs with lots of branching instructions are compiled,
__rte_bpf_bpf_validate runs way too slow. A simple bpf such as:
'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times

results in ~1 minute of bpf validation.
This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating
from classic BPF, allowing to safely skip over the validation.

Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
---
 app/test/test_bpf.c   | 2 ++
 lib/bpf/bpf_convert.c | 1 +
 lib/bpf/bpf_load.c    | 4 +++-
 lib/bpf/rte_bpf.h     | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 53e3a31123..7aae290c1a 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -3407,6 +3407,8 @@ static const char * const sample_filters[] = {
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 1] = 0x03)"
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 2] < 0x04)"
 	" and ((ip[2:2] - 4 * (ip[0] & 0x0F) - 4 * ((tcp[12] & 0xF0) >> 4) > 69))",
+	/* Performance */
+	"ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or 4a:e8:e7:5b:76:ce or 0d:87:fa:7a:a6:6d or bb:fd:c6:4b:bc:ae or 4e:28:dc:f1:1c:f6 or 3d:f2:b7:99:54:55 or 1c:5a:90:4b:82:ce or a7:28:08:ea:c9:84 or 1f:c0:2f:2f:0a:01 or c6:64:91:e9:78:f2 or 5b:cc:3f:90:39:ae or 4c:38:8f:ed:16:5c or cb:89:cb:54:0f:4f or 1e:0d:d7:b2:21:84 or 91:07:2b:88:e0:96 or 6c:ca:cf:bf:cf:3e or b9:df:f1:d6:dd:11 or ea:34:0b:b3:96:9e or 70:e5:18:9f:22:93",
 	/* Other */
 	"len = 128",
 };
diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..bc3c9a5d8e 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -567,6 +567,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	/* Classic BPF programs use mbufs */
 	prm->prog_arg.type = RTE_BPF_ARG_PTR_MBUF;
 	prm->prog_arg.size = sizeof(struct rte_mbuf);
+	prm->skip_verification = true;
 
 	return prm;
 }
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index de43347405..024e8dbc29 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -108,7 +108,9 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 		return NULL;
 	}
 
-	rc = __rte_bpf_validate(bpf);
+	if (!prm->skip_verification)
+		rc = __rte_bpf_validate(bpf);
+
 	if (rc == 0) {
 		__rte_bpf_jit(bpf);
 		if (mprotect(bpf, bpf->sz, PROT_READ) != 0)
diff --git a/lib/bpf/rte_bpf.h b/lib/bpf/rte_bpf.h
index 80ebb0210f..6f3b4e3c31 100644
--- a/lib/bpf/rte_bpf.h
+++ b/lib/bpf/rte_bpf.h
@@ -94,6 +94,7 @@ struct rte_bpf_prm {
 	/**< array of external symbols that eBPF code is allowed to reference */
 	uint32_t nb_xsym; /**< number of elements in xsym */
 	struct rte_bpf_arg prog_arg; /**< eBPF program input arg description */
+	bool skip_verification; /**< should skip eBPF verification before load */
 };
 
 /**
-- 
2.34.1

[PATCH] bpf: don't verify classic bpfs

[Yoav Winstein]

I noticed that when classic BPFs with lots of branching instructions are compiled, __rte_bpf_bpf_validate runs way too slow. A simple bpf such as: 'ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or ...' 12 times results in ~1 minute of bpf validation. This patch makes __rte_bpf_bpf_validate be aware of bpf_prm originating from classic BPF, allowing to safely skip over the validation.

Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
---
 app/test/test_bpf.c   | 2 ++
 lib/bpf/bpf_convert.c | 1 +
 lib/bpf/bpf_load.c    | 6 ++++--
 lib/bpf/rte_bpf.h     | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 53e3a31123..7aae290c1a 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -3407,6 +3407,8 @@ static const char * const sample_filters[] = {
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 1] = 0x03)"
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 2] < 0x04)"
 	" and ((ip[2:2] - 4 * (ip[0] & 0x0F) - 4 * ((tcp[12] & 0xF0) >> 4) > 69))",
+	/* Performance */
+	"ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or 4a:e8:e7:5b:76:ce or 0d:87:fa:7a:a6:6d or bb:fd:c6:4b:bc:ae or 4e:28:dc:f1:1c:f6 or 3d:f2:b7:99:54:55 or 1c:5a:90:4b:82:ce or a7:28:08:ea:c9:84 or 1f:c0:2f:2f:0a:01 or c6:64:91:e9:78:f2 or 5b:cc:3f:90:39:ae or 4c:38:8f:ed:16:5c or cb:89:cb:54:0f:4f or 1e:0d:d7:b2:21:84 or 91:07:2b:88:e0:96 or 6c:ca:cf:bf:cf:3e or b9:df:f1:d6:dd:11 or ea:34:0b:b3:96:9e or 70:e5:18:9f:22:93",
 	/* Other */
 	"len = 128",
 };
diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..bc3c9a5d8e 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -567,6 +567,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	/* Classic BPF programs use mbufs */
 	prm->prog_arg.type = RTE_BPF_ARG_PTR_MBUF;
 	prm->prog_arg.size = sizeof(struct rte_mbuf);
+	prm->skip_verification = true;
 
 	return prm;
 }
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index de43347405..e30797cb49 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -108,13 +108,15 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 		return NULL;
 	}
 
-	rc = __rte_bpf_validate(bpf);
+	if (!prm->skip_verification)
+		rc = __rte_bpf_validate(bpf);
+
 	if (rc == 0) {
 		__rte_bpf_jit(bpf);
 		if (mprotect(bpf, bpf->sz, PROT_READ) != 0)
 			rc = -ENOMEM;
 	}
-
+	
 	if (rc != 0) {
 		rte_bpf_destroy(bpf);
 		rte_errno = -rc;
diff --git a/lib/bpf/rte_bpf.h b/lib/bpf/rte_bpf.h
index 80ebb0210f..6f3b4e3c31 100644
--- a/lib/bpf/rte_bpf.h
+++ b/lib/bpf/rte_bpf.h
@@ -94,6 +94,7 @@ struct rte_bpf_prm {
 	/**< array of external symbols that eBPF code is allowed to reference */
 	uint32_t nb_xsym; /**< number of elements in xsym */
 	struct rte_bpf_arg prog_arg; /**< eBPF program input arg description */
+	bool skip_verification; /**< should skip eBPF verification before load */
 };
 
 /**
-- 
2.34.1

[PATCH] bpf: don't verify classic bpfs

[Yoav Winstein]

Signed-off-by: Yoav Winstein <yoav.w@claroty.com>
---
 app/test/test_bpf.c   | 2 ++
 lib/bpf/bpf_convert.c | 1 +
 lib/bpf/bpf_load.c    | 7 +++++--
 lib/bpf/rte_bpf.h     | 1 +
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 53e3a31123..7aae290c1a 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -3407,6 +3407,8 @@ static const char * const sample_filters[] = {
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 1] = 0x03)"
 	" and (tcp[((tcp[12] & 0xF0) >> 4 ) * 4 + 2] < 0x04)"
 	" and ((ip[2:2] - 4 * (ip[0] & 0x0F) - 4 * ((tcp[12] & 0xF0) >> 4) > 69))",
+	/* Performance */
+	"ether host a0:38:6d:af:17:eb or b3:a3:ff:b6:c1:ef or 4a:e8:e7:5b:76:ce or 0d:87:fa:7a:a6:6d or bb:fd:c6:4b:bc:ae or 4e:28:dc:f1:1c:f6 or 3d:f2:b7:99:54:55 or 1c:5a:90:4b:82:ce or a7:28:08:ea:c9:84 or 1f:c0:2f:2f:0a:01 or c6:64:91:e9:78:f2 or 5b:cc:3f:90:39:ae or 4c:38:8f:ed:16:5c or cb:89:cb:54:0f:4f or 1e:0d:d7:b2:21:84 or 91:07:2b:88:e0:96 or 6c:ca:cf:bf:cf:3e or b9:df:f1:d6:dd:11 or ea:34:0b:b3:96:9e or 70:e5:18:9f:22:93",
 	/* Other */
 	"len = 128",
 };
diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
index d7ff2b4325..bc3c9a5d8e 100644
--- a/lib/bpf/bpf_convert.c
+++ b/lib/bpf/bpf_convert.c
@@ -567,6 +567,7 @@ rte_bpf_convert(const struct bpf_program *prog)
 	/* Classic BPF programs use mbufs */
 	prm->prog_arg.type = RTE_BPF_ARG_PTR_MBUF;
 	prm->prog_arg.size = sizeof(struct rte_mbuf);
+	prm->skip_verification = true;
 
 	return prm;
 }
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index de43347405..9d42c862aa 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -108,13 +108,16 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 		return NULL;
 	}
 
-	rc = __rte_bpf_validate(bpf);
+	if (!prm->skip_verification) {
+		rc = __rte_bpf_validate(bpf);
+	}
+
 	if (rc == 0) {
 		__rte_bpf_jit(bpf);
 		if (mprotect(bpf, bpf->sz, PROT_READ) != 0)
 			rc = -ENOMEM;
 	}
-
+	
 	if (rc != 0) {
 		rte_bpf_destroy(bpf);
 		rte_errno = -rc;
diff --git a/lib/bpf/rte_bpf.h b/lib/bpf/rte_bpf.h
index 80ebb0210f..6f3b4e3c31 100644
--- a/lib/bpf/rte_bpf.h
+++ b/lib/bpf/rte_bpf.h
@@ -94,6 +94,7 @@ struct rte_bpf_prm {
 	/**< array of external symbols that eBPF code is allowed to reference */
 	uint32_t nb_xsym; /**< number of elements in xsym */
 	struct rte_bpf_arg prog_arg; /**< eBPF program input arg description */
+	bool skip_verification; /**< should skip eBPF verification before load */
 };
 
 /**
-- 
2.34.1