From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id A92881C0E8 for ; Thu, 5 Apr 2018 14:51:19 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Apr 2018 05:51:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,411,1517904000"; d="scan'208";a="31787030" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga006.jf.intel.com with ESMTP; 05 Apr 2018 05:51:17 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.216]) by IRSMSX153.ger.corp.intel.com ([169.254.9.75]) with mapi id 14.03.0319.002; Thu, 5 Apr 2018 13:51:16 +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/KAIABQBXAgABb24CAAR+0YA== Date: Thu, 5 Apr 2018 12:51:16 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258A0AB9849@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> <2601191342CEEE43887BDE71AB977258A0AB8A4E@irsmsx105.ger.corp.intel.com> <20180404175146.GA8576@jerin> In-Reply-To: <20180404175146.GA8576@jerin> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjRmMDg4YmUtYmRkYy00YTIyLTljNGUtOGEzYWVhNTgxN2YyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkwyNURKaXplUzVvM3lqXC81SE1scU9WelFPQVMxcWlXOURZOThOT29GZVhFPSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [163.33.239.180] 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: Thu, 05 Apr 2018 12:51:20 -0000 Hi Jerin, >=20 > > > > > > > > > > > > > +/* > > > > > > + * 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 ou= t 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= jitted memory with > > > > > "exit" opcode(value:0x95, exit, return r0),so that above code nee= ds to 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 f= ill > > > > > "exit" opcode(both jitted or interpreted case) for termination. > > > > > > > > > > What you think? > > > > > > > > Not sure I understand your proposal... > > > > > > If I understand it correctly, bpf_eth_cbi_wait() is used to _wait_ un= til > > > eBPF program exits? Right? > > > > Kind off, but not only. > > After bpf_eth_cbi_wait() finishes it is guaranteed that data-path woul= dn't try > > to access the resources associated with given bpf_eth_cbi (bpf, jit), s= o we > > can proceed with freeing them. > > > > > . Instead of using bpf_eth_cbi_[un]use() > > > scheme which involves the barrier. How about, > > > > > > in bpf_eth_cbi_wait() > > > { > > > > > > 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= . > > > > > > } > > > > > > 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 i= nstructions > > > to exit from EPBF program provided by arch code. > > > > > > Does that makes sense? > > > > There is no much point in doing it. >=20 > It helps in avoiding the barrier on non x86 case. Right?=20 Nope, I believe it doesn't, see below. > So it is useful > thing. Right? and avoid the extra logic in fastpath increment/decrement > "inuse" counters for all the archs. >=20 > > What we need is a guarantee that after some point data-path wouldn't tr= y to access > > given bpf context, so we can destroy it. >=20 > Is there any reason why you think, above proposed solution wont > guarantee the termination eBPF program? >=20 > -ie, > 1)memset to "exit" instruction in eBPF memory Even when code is just interpreted (bpf_exec()) - there still be cases=20 when you need to synchronize execution thread with thread updating the code (32bit systems, 16B LDDW instruction, etc.). =20 With JIT-ed code things will become much more complicated (icache, variable= size instructions) and I can't see how it could be done without extra synchronization between= execute and update threads. > 2)Wait for N instruction cycles to terminate the program. There is no way to guarantee that execution would take exactly N cycles. Execution thread could be preempted/interrupted, it could be executing sysc= all, there could be CPU stall (access slow memory, cpu freq change, etc.).=20 So even we'll solve all problems with 1) - it wouldn't buy us a safe soluti= on. Actually quite a lot of research was done how to speedup slow/fast path syn= chronization in user-space: https://lwn.net/Articles/573424/ some theory beyond: https://lttng.org/files/thesis/desnoyers-dissertation-2009-12-v27.pdf (chap= ter 6) They even introduced a new syscall in Linux for these purposes: http://man7.org/linux/man-pages/man2/membarrier.2.html I thought about something similar based on membarrier(), but it has few implications: 1. only latest linux kernels (4.14+)=20 2. Not sure is it available on non x86 platforms. 3. Need to measure real impact. Because of 1) and 2) we probably would need both mb() and membarrier() code= paths. Anyway - it is probably worth investigating for more generic solution, but I suppose it is out of scope for that patch. Konstantin > Where N can maximum cycles required to complete an eBPF instruction. >=20 > OR >=20 > Are you recommending the eBPF program termination is not just enough, the= re are others stuffs to > relinquish in order to free the bpf context? if so, what other stuffs to > relinquish apart from eBPF program termination.