From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [PATCH net-next 6/6] netdevsim: Add simple FIB resource controller via devlink Date: Fri, 6 Apr 2018 07:35:39 +0200 Message-ID: <20180406053539.GA19345@nanopsycho> References: <20180328012200.15175-1-dsa@cumulusnetworks.com> <20180328012200.15175-7-dsa@cumulusnetworks.com> <20180405172718.GA9125@nanopsycho> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, davem@davemloft.net, roopa@cumulusnetworks.com, shm@cumulusnetworks.com, jiri@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, andy.roulin@gmail.com To: David Ahern Return-path: Received: from mail-wm0-f49.google.com ([74.125.82.49]:38690 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750749AbeDFFfl (ORCPT ); Fri, 6 Apr 2018 01:35:41 -0400 Received: by mail-wm0-f49.google.com with SMTP id i3so457691wmf.3 for ; Thu, 05 Apr 2018 22:35:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Thu, Apr 05, 2018 at 10:10:29PM CEST, dsa@cumulusnetworks.com wrote: >On 4/5/18 11:27 AM, Jiri Pirko wrote: >> Wed, Mar 28, 2018 at 03:22:00AM CEST, dsa@cumulusnetworks.com wrote: >>> Add devlink support to netdevsim and use it to implement a simple, >>> profile based resource controller. Only one controller is needed >>> per namespace, so the first netdevsim netdevice in a namespace >>> registers with devlink. If that device is deleted, the resource >>> settings are deleted. >> >> I don't understand why you add 1:1 fixed relationship between >> netnamespace and devlink instance. That is highly misleading and reader >> might think that those 2 are somehow related. They are not. You can have >> multiple devlink instances for many ports in a single namespace. > >The netdevsim devlink instance is an example of limiting the number of >FIB entries and FIB rules for a namespace. It is currently limited to >the init_net based on past discussion. > >It does not make sense to have multiple resource controllers for the >same network namespace, hence the limit of only registering with devlink >on the first device create. Devlink instance represents an ASIC. 1:1. There is no relation with network namespaces and should not be. I have no clue why you think so. The model looks as I described it down below in the picture. > >> >> Could you please clarify? >> >> Also, to see the relationship between individual netdevsim netdevices >> and the parent devlink instance, we should use devlink_port >> instances, like this: >> >> devlink1 devlink2 >> | | | | >> dl_port1_1 dlport1_2 dlport2_1 dlport2_2 >> | | | | >> eth0 eth1 eth2 eth3 >> >> Note that "devlink instance" reprensents one ASIC. >> The address of the devlink instance is the bus address of the ASIC. >> Here, you use address of some/first netdevsim netdev instance. > >The ASIC here is the kernel tables in a namespace. It does not make >sense to have 2 devlink instances for a single namespace. Again. No clue why you build relationship with namespace. > >> >> The way it is implemented in netdevsim by this patch is wrong on >> so many levels :( >> >> Could you please fix this? I'm more than happy to help you with this, >> please say so. Thanks! > >What is there to fix? > >Not creating a netdevsim device per netdevsim netdevice? That is >completely unrelated to the devlink change. To fit the model. Multiple devlink instances, each representing one "virtual" ASIC, devlink_port instances, 1 for each netdevsim port. Netdevsim port should simulate real devices. No real device should have 1:1 relation with network namespace. That is just simply wrong.