From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command Date: Mon, 30 Jul 2018 17:45:32 -0700 Message-ID: <20180730174532.20010e0d@cakuba.netronome.com> References: <20180729205835.34850-1-dancol@google.com> <20180730172641.7d516231@cakuba.netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Joel Fernandes , linux-kernel , Tim Murray , netdev , Alexei Starovoitov , Lorenzo Colitti , Chenbo Feng , Mathieu Desnoyers , Alexei Starovoitov To: Daniel Colascione , Daniel Borkmann Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Mon, 30 Jul 2018 17:33:39 -0700, Daniel Colascione wrote: > On Mon, Jul 30, 2018 at 5:26 PM, Jakub Kicinski wrote: > > On Mon, 30 Jul 2018 03:25:43 -0700, Daniel Colascione wrote: > > > On Mon, Jul 30, 2018 at 3:04 AM, Daniel Borkmann wrote: > > > > Hmm, I don't think such UAPI as above is future-proof. In case we would want > > > > a similar mechanism in future for other maps, we would need a whole new bpf > > > > command or reuse BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES as a workaround though > > > > the underlying map may not even be a map-to-map. Additionally, we don't have > > > > any map object at hand in the above, so we couldn't make any finer grained > > > > decisions either. Something like below would be more suitable and leaves room > > > > for extending this further in future. > > > > > > YAGNI. Your proposed mechanism doesn't add anything under the current > > > implementation. > > > > FWIW in case of HW offload targeting a particular map may allow users > > to avoid a potentially slow sync with all the devices on the system. > > Sure. But such a thing doesn't exist right now (right?), and we can > add that more-efficient-in-that-one-case BPF interface when it lands. > I'd rather keep things simple for now. You mean map-in-map offload doesn't exist today? True, but it's on the roadmap for Netronome. Tangentially it would be really useful for us to have a "the map has actually been freed" notification/barrier. We have complaints of users creating huge maps and then trying to free and recreate them quickly, and the creation part failing with -ENOMEM, because the free from a workqueue didn't run, yet :( It'd probably require a different API to solve than what's discussed here, but since we're talking about syncing things I thought I'd put it out there...