netdev - Re: [patch net-next RFC 00/10] introduce line card support for modular switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210115165559.GS3565223@nanopsycho.orion>
Date:   Fri, 15 Jan 2021 17:55:59 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        jacob.e.keller@...el.com, roopa@...dia.com, mlxsw@...dia.com
Subject: Re: [patch net-next RFC 00/10] introduce line card support for
 modular switch

Fri, Jan 15, 2021 at 04:43:57PM CET, idosch@...sch.org wrote:
>On Wed, Jan 13, 2021 at 01:12:12PM +0100, Jiri Pirko wrote:
>> # Create a new netdevsim device, with no ports and 2 line cards:
>> $ echo "10 0 2" >/sys/bus/netdevsim/new_device
>> $ devlink port # No ports are listed
>> $ devlink lc
>> netdevsim/netdevsim10:
>>   lc 0 state unprovisioned
>>     supported_types:
>>        card1port card2ports card4ports
>>   lc 1 state unprovisioned
>>     supported_types:
>>        card1port card2ports card4ports
>> 
>> # Note that driver advertizes supported line card types. In case of
>> # netdevsim, these are 3.
>> 
>> $ devlink lc provision netdevsim/netdevsim10 lc 0 type card4ports
>
>Why do we need a separate command for that? You actually introduced
>'DEVLINK_CMD_LINECARD_SET' in patch #1, but it's never used.
>
>I prefer:
>
>devlink lc set netdevsim/netdevsim10 index 0 state provision type card4ports

This is misleading. This is actually not setting state. The state gets
changed upon successful provisioning process. Also, one may think that
he can set other states, but he can't. I don't like this at all :/


>devlink lc set netdevsim/netdevsim10 index 0 state unprovision
>
>It is consistent with the GET/SET/NEW/DEL pattern used by other
>commands.

Not really, see split port for example. This is similar to that.

>
>> $ devlink lc
>> netdevsim/netdevsim10:
>>   lc 0 state provisioned type card4ports
>>     supported_types:
>>        card1port card2ports card4ports
>>   lc 1 state unprovisioned
>>     supported_types:
>>        card1port card2ports card4ports
>> $ devlink port
>> netdevsim/netdevsim10/1000: type eth netdev eni10nl0p1 flavour physical lc 0 port 1 splittable false
>> netdevsim/netdevsim10/1001: type eth netdev eni10nl0p2 flavour physical lc 0 port 2 splittable false
>> netdevsim/netdevsim10/1002: type eth netdev eni10nl0p3 flavour physical lc 0 port 3 splittable false
>> netdevsim/netdevsim10/1003: type eth netdev eni10nl0p4 flavour physical lc 0 port 4 splittable false
>> #                                                 ^^                    ^^^^
>> #                                     netdev name adjusted          index of a line card this port belongs to
>> 
>> $ ip link set eni10nl0p1 up 
>> $ ip link show eni10nl0p1   
>> 165: eni10nl0p1: <NO-CARRIER,BROADCAST,NOARP,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
>>     link/ether 7e:2d:05:93:d3:d1 brd ff:ff:ff:ff:ff:ff
>> 
>> # Now activate the line card using debugfs. That emulates plug-in event
>> # on real hardware:
>> $ echo "Y"> /sys/kernel/debug/netdevsim/netdevsim10/linecards/0/active
>> $ ip link show eni10nl0p1
>> 165: eni10nl0p1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
>>     link/ether 7e:2d:05:93:d3:d1 brd ff:ff:ff:ff:ff:ff
>> # The carrier is UP now.
>> 
>> Jiri Pirko (10):
>>   devlink: add support to create line card and expose to user
>>   devlink: implement line card provisioning
>>   devlink: implement line card active state
>>   devlink: append split port number to the port name
>>   devlink: add port to line card relationship set
>>   netdevsim: introduce line card support
>>   netdevsim: allow port objects to be linked with line cards
>>   netdevsim: create devlink line card object and implement provisioning
>>   netdevsim: implement line card activation
>>   selftests: add netdevsim devlink lc test
>> 
>>  drivers/net/netdevsim/bus.c                   |  21 +-
>>  drivers/net/netdevsim/dev.c                   | 370 ++++++++++++++-
>>  drivers/net/netdevsim/netdev.c                |   2 +
>>  drivers/net/netdevsim/netdevsim.h             |  23 +
>>  include/net/devlink.h                         |  44 ++
>>  include/uapi/linux/devlink.h                  |  25 +
>>  net/core/devlink.c                            | 443 +++++++++++++++++-
>>  .../drivers/net/netdevsim/devlink.sh          |  62 ++-
>>  8 files changed, 964 insertions(+), 26 deletions(-)
>> 
>> -- 
>> 2.26.2
>>