From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABCC0C282DA for ; Fri, 19 Apr 2019 19:33:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 63D3021479 for ; Fri, 19 Apr 2019 19:33:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=resnulli-us.20150623.gappssmtp.com header.i=@resnulli-us.20150623.gappssmtp.com header.b="Frjs3sE1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727518AbfDSTdZ (ORCPT ); Fri, 19 Apr 2019 15:33:25 -0400 Received: from mail-wm1-f68.google.com ([209.85.128.68]:51424 "EHLO mail-wm1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726248AbfDSTdY (ORCPT ); Fri, 19 Apr 2019 15:33:24 -0400 Received: by mail-wm1-f68.google.com with SMTP id 4so7193501wmf.1 for ; Fri, 19 Apr 2019 12:33:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=vQRnylyMjIzwkBNOeMHwdikVjKl7Dx5ITkJW9v9jsVg=; b=Frjs3sE1IOhBEM63UHc32yN8md3eew9ekvaZZtxLqkot6AKNYxG2OJwu1MsUzUV91h vNQzUsp3cA5ygK2wKyCxuM8+faVoSSZkLZojXFn46R11kMPKDc/xlBSg2ouIjti4GFqI nju7xtw3XMKg8X/xSEY9dHRxll3huzWYCLfApR1nmjT5kYCkqvDZqVp8FkFc8PrPSKeR Rjze+7/Iiwt4A5wRBqx65gmQrGBXWf+WF4OBFvq6jnrAfQNNuSEv40ZdvkqmRTNZCYEK zKoqorvJ0kaKA5BLJpyEpElSVQd8nqGNpyYRHeUfJjDMRQMuXZamdivob5R7GYil67Le 6OmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=vQRnylyMjIzwkBNOeMHwdikVjKl7Dx5ITkJW9v9jsVg=; b=ScDYJLyjN8n69MK2wsVeqTDLUFb3DAtOpOt+2iE/9tmW2XiCaoPiPcQ8XKn+e/NWAP QZtxZa9hhPTafUxaAXyfk/10XjwwESY4wx5sZzOJ+j00IJDxRXS8fo7wosSb9BVIQT7x XyV2eLxf0A+dOgUW4Mj7hY9n/hr3RiRLQDPSPcldvVr+g8cHT4+tpzBxSEebYlfBB2/7 9iii4j6SYbSSqTBJpn+IYjJiP23AfPNgSQs5EqkKjOCOET6PZ4QyrGIfR7ZVHSdhecXu veCgCkL13B6O4Hqs15jagmKAbtsWenQ5V8i6/u0f7kI38OdKFCox769FgZCknvchjq4J rJAg== X-Gm-Message-State: APjAAAViCnGoeMgegT3iN+uxGaxEr4b3GmDJZdMGYnaIfLqRSW57Y+p3 D1Ii/FofDznhO58cGQbhvghgVWalKUk= X-Google-Smtp-Source: APXvYqxA+nBzjC+ZUTUA33pYw5JadIytYA3Oq0XuTDe23VVicrsM8D+q6XLQvnvkZ3bVjEBjYIGxSA== X-Received: by 2002:a1c:e1c5:: with SMTP id y188mr1256120wmg.39.1555651502861; Thu, 18 Apr 2019 22:25:02 -0700 (PDT) Received: from localhost (ip-213-220-234-223.net.upcbroadband.cz. [213.220.234.223]) by smtp.gmail.com with ESMTPSA id g6sm4763665wro.73.2019.04.18.22.25.01 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 18 Apr 2019 22:25:02 -0700 (PDT) Date: Fri, 19 Apr 2019 07:25:01 +0200 From: Jiri Pirko To: Jakub Kicinski Cc: netdev@vger.kernel.org, davem@davemloft.net, mlxsw@mellanox.com Subject: Re: [patch net-next rfc 00/15] netdevsim: impement proper device model Message-ID: <20190419052501.GA2247@nanopsycho> References: <20190413162112.8203-1-jiri@resnulli.us> <20190415122709.45dd4b09@cakuba.netronome.com> <20190416085937.GC2122@nanopsycho> <20190416110459.35b4b674@cakuba.netronome.com> <20190418072256.GA2196@nanopsycho.orion> <20190418100748.04efa546@cakuba.netronome.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190418100748.04efa546@cakuba.netronome.com> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Thu, Apr 18, 2019 at 07:07:48PM CEST, jakub.kicinski@netronome.com wrote: >On Thu, 18 Apr 2019 09:22:56 +0200, Jiri Pirko wrote: >> Tue, Apr 16, 2019 at 08:04:59PM CEST, jakub.kicinski@netronome.com wrote: >> >On Tue, 16 Apr 2019 10:59:37 +0200, Jiri Pirko wrote: >> >> >> 4) netdevsim instances are created by "ip link add" which is great for >> >> >> soft devices with no hw backend. The rtnl core allocates netdev and >> >> >> calls into driver holding rtnl mutex. For hw-backed devices, this >> >> >> flow is wrong as it breaks order in which things are done. >> >> >> >> >> >> This patchset adjust netdevsim to fix all above. >> >> >> >> >> >> In order to support proper devlink and devlink port instances and to be >> >> >> able to emulate real devices, there is need to implement bus probe and >> >> >> instantiate everything from there. User can specify device id and port >> >> >> count to be instantianted. For example: >> >> >> >> >> >> echo "10 4" > /sys/bus/netdevsim/new_device >> >> > >> >> >I really don't like the design where ID has to be allocated by user >> >> >space. It's a step back. >> >> > >> >> >I also dislike declaring ports from the start. In real drivers ports >> >> >are never "atomically" registered, they are crated and destroyed one >> >> >> >> Care to define "atomically" here? It is done in a very similar way >> >> to how it is done in mlxsw for example. Same flows. >> >> >> >> >> >> >by one, and a lot of races/UAFs/bugs lie in those small periods of >> >> >time where one netdev got unregistered, but other are still around... >> >> >> >> Same here. Not sure where do you see the differences. >> > >> >The difference is that today I can do this: >> > >> >create a netdevsim1 with shared dev 1 >> >create some state associated with shared dev 1 >> >create a netdevsim2 with shared dev 1 >> >check if all the shared dev 1 state created for netdevsim1 is visible >> > via netdevsim2 >> >> Hmm, you are testing netdevsim implementation then, not the kernel >> interfaces. What is the point of testing netdevsim? > >BPF offload tries to leave as much code as possible in the core, and >make the drivers simple. I'm testing whether the core reacts correctly, >netdevsim just calls register/unregister. > >> >destroy netdevsim1 >> >check the shared dev 1 state again >> > >> >If I say "give me 2 ports" from the start, that makes the testing >> >(which is the whole point of this code) harder. >> > >> >> Also, I plan to implement port splitting in follow-up patchset. All >> >> flows are there as well. >> > >> >Sure, let's just be clear that we won't be merging an ABI that has just >> >a netdevsim implementation, right? I have some reservations about the >> >> So what do you suggest? Allow to somehow add and remove ports during >> test? You can already do that with VFs. Do you want to do that with >> netdevsim "physical" ports? If yes, how? I can imagine to extend devlink >> port api with something like: >> >> $ sudo devlink dev >> netdevsim/netdevsim0 >> $ sudo devlink port >> netdevsim/netdevsim0/0: type eth netdev eth0 flavour physical >> >> $ sudo devlink dev port add netdevsim/netdevsim0 index 22 >> $ sudo devlink port >> netdevsim/netdevsim0/0: type eth netdev eni0p1 flavour physical >> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical >> >> $ sudo devlink port del netdevsim/netdevsim0/0 >> $ sudo devlink port >> netdevsim/netdevsim0/22: type eth netdev eni0p23 flavour physical >> >> But I see only usecase for this extension for netdevsim, not for real >> devices.. > >Hm.. I'm getting lost, sorry, I'm probably confusing myself here.. > >Netdevsim is supposed to test real, existing kernel interfaces and core >code. What we do today with linking based on netdevs is quite simple >and works very well for the BPF offload tests. > >If you want to test some devlink code, that's also real, perfect. > >For BPF tests we want the ability to add and remove netdevs to a sdev >during tests, yes. That ability cannot be lost. Yeah, but since the api is changing, the original approach of using IFLA_LINK cannot be used anymore. Not to mention is is abuse of the API from the very beginning. You want to add/del ports during lifetime, I'm trying to find interface. Basically whe have two options: 1) devlink extension I suggested above 2) sysfs files to add/del ports. Similar to "new_device" and "del_device". I like the idea of having 2) better, the reason is this is very speficic to netdevsim and not really applicable on real devices. > > >> >"port splitting" or device slicing, which should be discussed over real >> >code, not netdevsim.