[v2,2/2] bpf: fix convert API can be undefined
Checks
Commit Message
rte_bpf_convert() implementation depends on libpcap.
Right now it is defined only when this library is installed and
RTE_PORT_PCAP is defined.
Fix that by providing for such case stub rte_bpf_convert()
implementation that will always return an error.
Also move stub for another function (rte_bpf_elf_load) into
the same place (bpf_stub.c).
Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/bpf/bpf_load.c | 18 ------------------
lib/bpf/bpf_stub.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
lib/bpf/meson.build | 1 +
lib/bpf/rte_bpf.h | 5 +----
4 files changed, 47 insertions(+), 22 deletions(-)
create mode 100644 lib/bpf/bpf_stub.c
Comments
On Mon, 1 Nov 2021 16:10:13 +0000
Konstantin Ananyev <konstantin.ananyev@intel.com> wrote:
> rte_bpf_convert() implementation depends on libpcap.
> Right now it is defined only when this library is installed and
> RTE_PORT_PCAP is defined.
> Fix that by providing for such case stub rte_bpf_convert()
> implementation that will always return an error.
> Also move stub for another function (rte_bpf_elf_load) into
> the same place (bpf_stub.c).
>
> Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Wouldn't it be better to fail compiling a program using unimplemented calls
rather than forcing the user to see a failure later?
> > rte_bpf_convert() implementation depends on libpcap.
> > Right now it is defined only when this library is installed and
> > RTE_PORT_PCAP is defined.
> > Fix that by providing for such case stub rte_bpf_convert()
> > implementation that will always return an error.
> > Also move stub for another function (rte_bpf_elf_load) into
> > the same place (bpf_stub.c).
> >
> > Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
> Wouldn't it be better to fail compiling a program using unimplemented calls
> rather than forcing the user to see a failure later?
You mean to keep things as they are right now?
That way we'll have to put #ifdef RTE_PORT_PCAP around every call to bpf_convert().
Doesn't look like a good thing to me.
Also in other places we use similar approach: if HW/SW is not available provide
a dummy implementation that will report an error.
On Tue, 2 Nov 2021 10:54:59 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> > > rte_bpf_convert() implementation depends on libpcap.
> > > Right now it is defined only when this library is installed and
> > > RTE_PORT_PCAP is defined.
> > > Fix that by providing for such case stub rte_bpf_convert()
> > > implementation that will always return an error.
> > > Also move stub for another function (rte_bpf_elf_load) into
> > > the same place (bpf_stub.c).
> > >
> > > Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
> > >
> > > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >
> > Wouldn't it be better to fail compiling a program using unimplemented calls
> > rather than forcing the user to see a failure later?
>
> You mean to keep things as they are right now?
> That way we'll have to put #ifdef RTE_PORT_PCAP around every call to bpf_convert().
> Doesn't look like a good thing to me.
> Also in other places we use similar approach: if HW/SW is not available provide
> a dummy implementation that will report an error.
Is there a way to do both, return an error and report a warning in the build?
On Mon, 1 Nov 2021 16:10:13 +0000
Konstantin Ananyev <konstantin.ananyev@intel.com> wrote:
> rte_bpf_convert() implementation depends on libpcap.
> Right now it is defined only when this library is installed and
> RTE_PORT_PCAP is defined.
> Fix that by providing for such case stub rte_bpf_convert()
> implementation that will always return an error.
> Also move stub for another function (rte_bpf_elf_load) into
> the same place (bpf_stub.c).
>
> Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Should also change dumpcap to print more complete error?
diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index baf9eee46666..fff460eee518 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -285,7 +285,7 @@ static void compile_filter(void)
bpf_prm = rte_bpf_convert(&bf);
if (bpf_prm == NULL)
rte_exit(EXIT_FAILURE,
- "bpf convert failed\n");
+ "bpf convert failed: %s\n", rte_strerror(rte_errno));
if (dump_bpf) {
printf("cBPF program (%u insns)\n", bf.bf_len);
> On Tue, 2 Nov 2021 10:54:59 +0000
> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
>
> > > > rte_bpf_convert() implementation depends on libpcap.
> > > > Right now it is defined only when this library is installed and
> > > > RTE_PORT_PCAP is defined.
> > > > Fix that by providing for such case stub rte_bpf_convert()
> > > > implementation that will always return an error.
> > > > Also move stub for another function (rte_bpf_elf_load) into
> > > > the same place (bpf_stub.c).
> > > >
> > > > Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF")
> > > >
> > > > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > >
> > > Wouldn't it be better to fail compiling a program using unimplemented calls
> > > rather than forcing the user to see a failure later?
> >
> > You mean to keep things as they are right now?
> > That way we'll have to put #ifdef RTE_PORT_PCAP around every call to bpf_convert().
> > Doesn't look like a good thing to me.
> > Also in other places we use similar approach: if HW/SW is not available provide
> > a dummy implementation that will report an error.
>
> Is there a way to do both, return an error and report a warning in the build?
Sounds like a good idea, will add a warning to meson.build.
@@ -130,21 +130,3 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
return bpf;
}
-
-#ifndef RTE_LIBRTE_BPF_ELF
-struct rte_bpf *
-rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
- const char *sname)
-{
- if (prm == NULL || fname == NULL || sname == NULL) {
- rte_errno = EINVAL;
- return NULL;
- }
-
- RTE_BPF_LOG(ERR, "%s() is not supported with current config\n"
- "rebuild with libelf installed\n",
- __func__);
- rte_errno = ENOTSUP;
- return NULL;
-}
-#endif
new file mode 100644
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018-2021 Intel Corporation
+ */
+
+#include "bpf_impl.h"
+#include <rte_errno.h>
+
+/**
+ * Contains stubs for unimplemented public API functions
+ */
+
+#ifndef RTE_LIBRTE_BPF_ELF
+struct rte_bpf *
+rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
+ const char *sname)
+{
+ if (prm == NULL || fname == NULL || sname == NULL) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
+
+ RTE_BPF_LOG(ERR, "%s() is not supported with current config\n"
+ "rebuild with libelf installed\n",
+ __func__);
+ rte_errno = ENOTSUP;
+ return NULL;
+}
+#endif
+
+#ifndef RTE_PORT_PCAP
+struct rte_bpf_prm *
+rte_bpf_convert(const struct bpf_program *prog)
+{
+ if (prog == NULL) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
+
+ RTE_BPF_LOG(ERR, "%s() is not supported with current config\n"
+ "rebuild with libpcap installed\n",
+ __func__);
+ rte_errno = ENOTSUP;
+ return NULL;
+}
+#endif
@@ -12,6 +12,7 @@ sources = files('bpf.c',
'bpf_exec.c',
'bpf_load.c',
'bpf_pkt.c',
+ 'bpf_stub.c',
'bpf_validate.c')
if arch_subdir == 'x86' and dpdk_conf.get('RTE_ARCH_64')
@@ -212,8 +212,6 @@ __rte_experimental
void
rte_bpf_dump(FILE *f, const struct ebpf_insn *buf, uint32_t len);
-#ifdef RTE_PORT_PCAP
-
struct bpf_program;
/**
@@ -228,13 +226,12 @@ struct bpf_program;
* Possible rte_errno errors include:
* - EINVAL - invalid parameter passed to function
* - ENOMEM - can't reserve enough memory
+ * - ENOTSUP - operation not supported
*/
__rte_experimental
struct rte_bpf_prm *
rte_bpf_convert(const struct bpf_program *prog);
-#endif
-
#ifdef __cplusplus
}
#endif