>
> -----Original Message-----
> > Date: Fri, 9 Mar 2018 16:42:03 +0000
> > From: Konstantin Ananyev <konstantin.anan...@intel.com>
> > To: dev@dpdk.org
> > CC: Konstantin Ananyev <konstantin.anan...@intel.com>
> > Subject: [dpdk-dev] [PATCH v1 3/5] bpf: introduce basic RX/TX BPF filters
> > X-Mailer: git-send-email 1.7.0.7
> >
> > Introduce API to install BPF based filters on ethdev RX/TX path.
> > Current implementation is pure SW one, based on ethdev RX/TX
> > callback mechanism.
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.anan...@intel.com>
> > ---
> > lib/librte_bpf/Makefile | 2 +
> > lib/librte_bpf/bpf_pkt.c | 524
> > +++++++++++++++++++++++++++++++++++++
> > lib/librte_bpf/rte_bpf_ethdev.h | 50 ++++
> > lib/librte_bpf/rte_bpf_version.map | 4 +
> > 4 files changed, 580 insertions(+)
> > create mode 100644 lib/librte_bpf/bpf_pkt.c
> > create mode 100644 lib/librte_bpf/rte_bpf_ethdev.h
> >
> > diff --git a/lib/librte_bpf/Makefile b/lib/librte_bpf/Makefile
> > +
> > +/*
> > + * information about all installed BPF rx/tx callbacks
> > + */
> > +
> > +struct bpf_eth_cbi {
> > + uint32_t use; /*usage counter */
> > + void *cb; /* callback handle */
> > + struct rte_bpf *bpf;
> > + struct rte_bpf_jit jit;
> > +} __rte_cache_aligned;
> > +
> > +/*
> > + * Odd number means that callback is used by datapath.
> > + * Even number means that callback is not used by datapath.
> > + */
> > +#define BPF_ETH_CBI_INUSE 1
> > +
> > +static struct bpf_eth_cbi
> > rx_cbi[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> > +static struct bpf_eth_cbi
> > tx_cbi[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
>
> How about allocating this memory from huge page?
Yep, in v2 will switch to using rte_malloc() for cbi allocation.
>
> > +
> > +/*
> > + * Marks given callback as used by datapath.
> > + */
> > +static __rte_always_inline void
> > +bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
> > +{
> > + cbi->use++;
> > + /* make sure no store/load reordering could happen */
> > + rte_smp_mb();
>
> This is an full barrier on non x86.
This is a full barrier on x86 too.
Unfortunately I think it is unavoidable, though open for suggestions.
> How about a light version of this
> logic? See below.
>
> > +}
> > +
> > +/*
> > + * Marks given callback list as not used by datapath.
> > + */
> > +static __rte_always_inline void
> > +bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi)
> > +{
> > + /* make sure all previous loads are completed */
> > + rte_smp_rmb();
> > + cbi->use++;
> > +}
> > +
> > +/*
> > + * Waits till datapath finished using given callback.
> > + */
> > +static void
> > +bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
> > +{
> > + uint32_t nuse, puse;
> > +
> > + /* make sure all previous loads and stores are completed */
> > + rte_smp_mb();
> > +
>
> Read conjunction with below change
>
> #if 0
> > + puse = cbi->use;
> > +
> > + /* in use, busy wait till current RX/TX iteration is finished */
> > + if ((puse & BPF_ETH_CBI_INUSE) != 0) {
> > + do {
> > + rte_pause();
> > + rte_compiler_barrier();
> > + nuse = cbi->use;
> > + } while (nuse == puse);
> > + }
> #else
> cbi->cb = NULL;
> while (likely(cb->done != 1)) {
> rte_pause();
> rte_smb_rmb();
> }
>
> or any other logic using flag to wait until callback completes.
> #endif
I thought about it, but such approach makes control-path function progress
dependent
on simultaneous invocation of data-path functions.
In some cases it would cause control-path to hang.
Let say there is no traffic for that port/queue (i.e. tx_burst() wouldn't be
called),
or user invokes control-path and data-path function from the same thread, i.e:
rte_bpf_eth_rx_elf_load(port, queue, ...);
....
rte_eth_rx_burst(port, queue, ...);
...
rte_bpf_eth_rx_unload(port,queue);
Konstantin
>
> > +}
> > +
> > +
> > +/*
> > + * RX/TX callbacks for raw data bpf.
> > + */
> > +
> > +static uint16_t
> > +bpf_rx_callback_vm(__rte_unused uint16_t port, __rte_unused uint16_t queue,
> > + struct rte_mbuf *pkt[], uint16_t nb_pkts,
> > + __rte_unused uint16_t max_pkts, void *user_param)
> > +{
> > + struct bpf_eth_cbi *cbi;
> > + uint16_t rc;
> > +
> > + cbi = user_param;
> > +
>
> Read conjunction with above change
>
> #if 0
> > + bpf_eth_cbi_inuse(cbi);
> > + rc = (cbi->cb != NULL) ?
> > + pkt_filter_vm(cbi->bpf, pkt, nb_pkts, 1) :
> > + nb_pkts;
> > + bpf_eth_cbi_unuse(cbi);
> #else
> if (likely(cbi->cb != NULL))
> return pkt_filter_vm(cbi->bpf, pkt, nb_pkts, 1) :
> else {
> cbi->done = 1;
> rte_smb_wmb();
> return nb_pkts;
> }
> #endif
>
> > + return rc;
> > +}
> > +
