From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com ([209.85.128.194]:43768 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752095AbeCXQCl (ORCPT ); Sat, 24 Mar 2018 12:02:41 -0400 Received: by mail-wr0-f194.google.com with SMTP id p53so7362419wrc.10 for ; Sat, 24 Mar 2018 09:02:40 -0700 (PDT) Date: Sat, 24 Mar 2018 17:02:38 +0100 From: Jiri Pirko To: David Ahern Cc: netdev@vger.kernel.org, davem@davemloft.net, roopa@cumulusnetworks.com, shm@cumulusnetworks.com, jiri@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, David Ahern Subject: Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink Message-ID: <20180324160238.GE1891@nanopsycho> References: <20180322225757.10377-1-dsa@cumulusnetworks.com> <20180322225757.10377-8-dsa@cumulusnetworks.com> <20180323065010.GM2074@nanopsycho.orion> <03eade79-1727-3a31-8e31-a0a7f51b72cf@cumulusnetworks.com> <20180323150149.GD2125@nanopsycho> <20180323150516.GE2125@nanopsycho> <20180324072621.GA1891@nanopsycho> <2eadcfb9-6dd2-d533-7ae3-c9299a70a172@cumulusnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2eadcfb9-6dd2-d533-7ae3-c9299a70a172@cumulusnetworks.com> Sender: netdev-owner@vger.kernel.org List-ID: Sat, Mar 24, 2018 at 04:05:38PM CET, dsa@cumulusnetworks.com wrote: >On 3/24/18 1:26 AM, Jiri Pirko wrote: >> Fri, Mar 23, 2018 at 04:13:14PM CET, dsa@cumulusnetworks.com wrote: >>> On 3/23/18 9:05 AM, Jiri Pirko wrote: >>>> Fri, Mar 23, 2018 at 04:03:40PM CET, dsa@cumulusnetworks.com wrote: >>>>> On 3/23/18 9:01 AM, Jiri Pirko wrote: >>>>>> Fri, Mar 23, 2018 at 03:31:02PM CET, dsa@cumulusnetworks.com wrote: >>>>>>> On 3/23/18 12:50 AM, Jiri Pirko wrote: >>>>>>>>> +void nsim_devlink_setup(struct netdevsim *ns) >>>>>>>>> +{ >>>>>>>>> + struct net *net = dev_net(ns->netdev); >>>>>>>>> + bool *reg_devlink = net_generic(net, nsim_devlink_id); >>>>>>>>> + struct devlink *devlink; >>>>>>>>> + int err = -ENOMEM; >>>>>>>>> + >>>>>>>>> + /* only one device per namespace controls devlink */ >>>>>>>>> + if (!*reg_devlink) { >>>>>>>>> + ns->devlink = NULL; >>>>>>>>> + return; >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + devlink = devlink_alloc(&nsim_devlink_ops, 0); >>>>>>>>> + if (!devlink) >>>>>>>>> + return; >>>>>>>>> + >>>>>>>>> + devlink_net_set(devlink, net); >>>>>>>>> + err = devlink_register(devlink, &ns->dev); >>>>>>>> >>>>>>>> This reg_devlink construct looks odd. Why don't you leave the devlink >>>>>>>> instance in init_ns? >>>>>>> >>>>>>> It is a per-network namespace resource controller. Since struct devlink >>>>>> >>>>>> Wait a second. What do you mean by "per-network namespace"? Devlink >>>>>> instance is always associated with one physical device. Like an ASIC. >>>>>> >>>>>> >>>>>>> has a net entry, the simplest design is to put it into the namespace of >>>>>>> the controller. Without it, controlling resource sizes in namespace >>>>>>> 'foobar' has to be done from init_net, which is just wrong. >>>>> >>>>> you need to look at how netdevsim creates a device per netdevice. >>>> >>>> That means one devlink instance for each netdevsim device, doesn't it? >>>> >>> >>> yes. >> >> Still not sure how to handle namespaces in devlink. Originally, I >> thought it would be okay to leave all devlink instances in init_ns. >> Because what happens if you move netdev to another namespace? Should the >> devlink move as well? What if you have multiple ports, each in different >> namespace. Can user move devlink instance to another namespace? Etc. >> > >The devlink instance is associated with a 'struct device' and those do >not change namespaces AFAIK. Yeah. But you put devlink instance into namespace according to struct net_device. That is mismatch.