Re: [PATCH net-next 07/14] devlink: Implement port params registration

Jiri Pirko
2025-02-28T15:21:18Z

Fri, Feb 28, 2025 at 02:23:00PM +0100, przemyslaw.kitszel@intel.com wrote:
>On 2/28/25 13:28, Jiri Pirko wrote:
>> Fri, Feb 28, 2025 at 12:58:38PM +0100, przemyslaw.kitszel@intel.com wrote:
>> > On 2/28/25 03:12, Saeed Mahameed wrote:
>> > > From: Saeed Mahameed <saeedm@nvidia.com>
>> > >
>> > > Port params infrastructure is incomplete and needs a bit of plumbing to
>> > > support port params commands from netlink.
>> > >
>> > > Introduce port params registration API, very similar to current devlink
>> > > params API, add the params xarray to devlink_port structure and
>> > > decouple devlink params registration routines from the devlink
>> > > structure.
>> > >
>> > > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
>> > > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>> > > ---
>> > > include/net/devlink.h | 14 ++++
>> > > net/devlink/param.c | 150 ++++++++++++++++++++++++++++++++++--------
>> > > net/devlink/port.c | 3 +
>> > > 3 files changed, 140 insertions(+), 27 deletions(-)
>> > For me devlink and devlink-port should be really the same, to the point
>> > that the only difference is `bool is_port` flag inside of the
>> > struct devlink. Then you could put special logic if really desired (to
>> > exclude something for port).
>>
>> Why? Why other devlink objects shouldn't be the same as well. Then we
>> can have one union. Does not make sense to me. The only think dev and
>> port is sharing would be params. What else? Totally different beast.
>
>Instead of focusing on differences try to find similarities.
>
>health reporters per port and "toplevel",
>just by grepping:
>devlink_nl_sb_pool_fill()
>devlink_nl_sb_port_pool_fill(),

Sharedbuffer is separate story.

>
>devlink_region_create()
>devlink_port_region_create()

Okay, regions I missed.

>
>and there is no reason to assume that someone will not want to
>extend ports to have devlink resources or other thing

But looks at differences. They are huge.

But perhaps I'm missing the point. What you want to achieve? Just to
reduce API? That is always a tradeoff. I don't think the pros top the
cons here.