Re: [PATCH net-next 00/11] mlxsw: extend line card model by devices and info

From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Ido Schimmel <idosch@idosch.org>,
	Ido Schimmel <idosch@nvidia.com>,
	netdev@vger.kernel.org, davem@davemloft.net, pabeni@redhat.com,
	jiri@nvidia.com, petrm@nvidia.com, dsahern@gmail.com,
	andrew@lunn.ch, mlxsw@nvidia.com
Subject: Re: [PATCH net-next 00/11] mlxsw: extend line card model by devices and info
Date: Thu, 26 May 2022 11:05:53 +0200	[thread overview]
Message-ID: <Yo9C8aKhTuqsQW1q@nanopsycho> (raw)
In-Reply-To: <20220525085054.70f297ac@kernel.org>

Wed, May 25, 2022 at 05:50:54PM CEST, kuba@kernel.org wrote:
>On Wed, 25 May 2022 08:20:45 +0200 Jiri Pirko wrote:
>> >We talked about this earlier in the thread, I think. If you need both
>> >info and flash per LC just make them a separate devlink instance and
>> >let them have all the objects they need. Then just put the instance
>> >name under lc info.  
>> 
>> I don't follow :/ What do you mean be "separate devlink instance" here?
>> Could you draw me an example?
>
>Separate instance:
>
>	for (i = 0; i < sw->num_lcs; i++) {
>		devlink_register(&sw->lc_dl[i]);
>		devlink_line_card_link(&sw->lc[i], &sw->lc_dl[i]);
>	}
>
>then report that under the linecard
>
>	nla_nest_start(msg, DEVLINK_SUBORDINATE_INSTANCE);
>	devlink_nl_put_handle(msg, lc->devlink);
>	nla_nest_end(msg...)
>
>then user can update the linecard like any devlink instance, switch,
>NIC etc. It's better code reuse and I don't see any downside, TBH.

I think you already suggested something like that. What you mean is that
linecard would be modeled as any other devlink instance. Sorry, but that
does not make any sense to me at all :/ Should devlink port not be
devlink port and rather modeled as a separate devlink instance too? Same
to the rest of the objects we already have. Should we have a tree of
same devlink objects each overloaded to some specific type. If yes, that
is completely changing the devlink modelling.

The handle of the devlink object is bus_name/dev_name. In other words,
there is a device on a bus and for each you can create devlink instance.
Linecards is not on a any bus, is is not modeled in /sys. What would be
the handle?

What you suggest seems to me like completely broken :/

I don't understand the motivation :/ The only I can think of that you
will have the same "devlink dev flash" mechanism, but that's it.
I'm not sure I understand why we cannot resolve the flashing otherwise,
for example as I suggested with "devlink lc info" and "devlink lc
flash". What is wrong with that?