From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 7A8261C160 for ; 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" To: Jerin Jacob CC: "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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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); > > > > + } > > > > +}