From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
by dpdk.org (Postfix) with ESMTP id 7A8261C160
for <dev@dpdk.org>; Wed, 4 Apr 2018 13:40:03 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga007.jf.intel.com ([10.7.209.58])
by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
04 Apr 2018 04:40:02 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.48,405,1517904000"; d="scan'208";a="30616768"
Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66])
by orsmga007.jf.intel.com with ESMTP; 04 Apr 2018 04:40:01 -0700
Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by
IRSMSX152.ger.corp.intel.com (163.33.192.66) with Microsoft SMTP Server (TLS)
id 14.3.319.2; Wed, 4 Apr 2018 12:40:00 +0100
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.216]) by
irsmsx112.ger.corp.intel.com ([169.254.1.226]) with mapi id 14.03.0319.002;
Wed, 4 Apr 2018 12:40:00 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
CC: "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH v2 5/7] bpf: introduce basic RX/TX BPF filters
Thread-Index: AQHTyE05hT6KffBHaUu9pmUd9VBIM6PuBwGAgAEXGkCAAB/KAIABQBXA
Date: Wed, 4 Apr 2018 11:39:59 +0000
Message-ID: <2601191342CEEE43887BDE71AB977258A0AB8A4E@irsmsx105.ger.corp.intel.com>
References: <1520613725-9176-1-git-send-email-konstantin.ananyev@intel.com>
<1522431163-25621-6-git-send-email-konstantin.ananyev@intel.com>
<20180402224440.GB1501@jerin>
<2601191342CEEE43887BDE71AB977258A0AB7FA5@irsmsx105.ger.corp.intel.com>
<20180403170015.GA24166@jerin>
In-Reply-To: <20180403170015.GA24166@jerin>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTJhMTYyMjgtYTk1ZC00NDQ1LWI0ODctNzcxZmZmZTY0OTRkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Ino1aFZtWUhCaHVxYjlYaEdFWU1KUTZtRk05NlYwektNTkxXMzNIMGRHYjg9In0=
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.200.100
dlp-reaction: no-action
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v2 5/7] bpf: introduce basic RX/TX BPF filters
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 04 Apr 2018 11:40:04 -0000
Hi Jerin,
> > >
> > > > +/*
> > > > + * 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();
> > > > +}
> > > > +
> > > > +/*
> > > > + * 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();
> > >
> > > We earlier discussed this barrier. Will following scheme works out to
> > > fix the bpf_eth_cbi_wait() without cbi->use scheme?
> > >
> > > #ie. We need to exit from jitted or interpreted code irrespective of =
its
> > > state. IMO, We can do that by an _arch_ specific function to fill jit=
ted memory with
> > > "exit" opcode(value:0x95, exit, return r0),so that above code needs t=
o be come out i n anycase,
> > > on next instruction execution. I know, jitted memory is read-only in =
your
> > > design, I think, we can change the permission to "write" to the fill
> > > "exit" opcode(both jitted or interpreted case) for termination.
> > >
> > > What you think?
> >
> > Not sure I understand your proposal...
>=20
> If I understand it correctly, bpf_eth_cbi_wait() is used to _wait_ until
> eBPF program exits? Right?
Kind off, but not only.=20
After bpf_eth_cbi_wait() finishes it is guaranteed that data-path wouldn't=
try
to access the resources associated with given bpf_eth_cbi (bpf, jit), so we
can proceed with freeing them.=20
> . Instead of using bpf_eth_cbi_[un]use()
> scheme which involves the barrier. How about,
>=20
> in bpf_eth_cbi_wait()
> {
>=20
> memset the EBPF "program memory" with 0x95 value. Which is an "exit" and
> "return r0" EPBF opcode, Which makes program to terminate by it own
> as on 0x95 instruction, CPU decodes and it gets out from EPBF program.
>=20
> }
>=20
> In jitted case, it is not 0x95 instruction, which will be an arch
> specific instructions, We can have arch abstraction to generated
> such instruction for "exit" opcode. And use common code to fill the instr=
uctions
> to exit from EPBF program provided by arch code.
>=20
> Does that makes sense?
There is no much point in doing it.
What we need is a guarantee that after some point data-path wouldn't try to=
access
given bpf context, so we can destroy it.
Konstantin
>=20
>=20
> > Are you suggesting to change bpf_exec() and bpf_jit() to make them exec=
ute sync primitives in an arch specific manner?
> > But some users probably will use bpf_exec/jitted program in the environ=
ment that wouldn't require such synchronization.
> > For these people it would be just unnecessary slowdown.
> >
> > If you are looking for a ways to replace 'smp_rmb' in bpf_eth_cbi_unus=
e() with something arch specific, then
> > I can make cbi_inuse/cbi_unuse - arch specific with keeping current imp=
lementation as generic one.
> > Would that help?
> >
> > Konstantin
> >
> > >
> > > > + 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();
> > > > +
> > > > + puse =3D cbi->use;
> > > > +
> > > > + /* in use, busy wait till current RX/TX iteration is finished */
> > > > + if ((puse & BPF_ETH_CBI_INUSE) !=3D 0) {
> > > > + do {
> > > > + rte_pause();
> > > > + rte_compiler_barrier();
> > > > + nuse =3D cbi->use;
> > > > + } while (nuse =3D=3D puse);
> > > > + }
> > > > +}