Fri, Jan 15, 2021 at 07:01:45PM CET, ido...@idosch.org wrote: >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
Hmm, okay, that might work. And I can add state "FAILED_PROVISION" what would indicate that after the type was set by the user, driver was not able to successfully provision. The the user has to set "notype" & "type x" again. Sounds good? > >> >> >> >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. You are right. > >> >> > >> >> $ 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 >> >>