On Fri, Jan 15, 2021 at 05:55:59PM +0100, Jiri Pirko wrote: > Fri, Jan 15, 2021 at 04:43:57PM CET, ido...@idosch.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 :/
So make state a read-only attribute. You really only care about setting the type. To provision: # devlink lc set netdevsim/netdevsim10 index 0 type card4ports To unprovsion: # devlink lc set netdevsim/netdevsim10 index 0 type none Or: # devlink lc set netdevsim/netdevsim10 index 0 notype > > > >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. It's not. The split command creates new objects whereas this command modifies an existing object. > > > > >> $ 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 > >>