From: Jiri Pirko <redacted>
There a is need for some userspace API that would allow to expose things
that are not directly related to any device class like net_device of
ib_device, but rather chip-wide/switch-ASIC-wide stuff.
Use cases:
1) get/set of port type (Ethernet/InfiniBand)
2) setting up port splitters - split port into multiple ones and squash again,
enables usage of splitter cable
3) setting up shared buffers - shared among multiple ports within
one chip (work in progress)
4) configuration of switch wide properties - resources division etc - This will
allow to pass configuration that is unacceptable to be passed as
a module option.
First patch of this set introduces a new generic Netlink based interface,
called "devlink". It is similar to nl80211 model and it is heavily
influenced by it, including the API definition. The devlink introduction patch
implements use cases 1) and 2). Other 2 are in development atm and will
be addressed by follow-ups.
It is very convenient for drivers to use devlink, as you can see in other
patches in this set.
Counterpart for devlink is userspace tool for now called "dl". Command line
interface and outputs are derived from "ip" tool so it should be easy
for users to get used to it.
It is available here as a standalone tool for now:
https://github.com/jpirko/devlink
After this is merge in kernel, I will include the "dl" or "devlink" tool
into iproute2 toolset.
Port type setting example:
myhost:~$ dl help
Usage: dl [ OPTIONS ] OBJECT { COMMAND | help }
where OBJECT := { dev | port | monitor }
OPTIONS := { -v/--verbose }
myhost:~$ dl dev help
Usage: dl dev show [DEV]
Usage: dl dev set DEV [ name NEWNAME ]
myhost:~$ dl dev show
0: devlink0: bus pci dev 0000:01:00.0
myhost:~$ dl port help
Usage: dl port show [DEV/PORT_INDEX]
Usage: dl port set DEV/PORT_INDEX [ type { eth | ib | auto} ]
Usage: dl port split DEV/PORT_INDEX count
Usage: dl port unsplit DEV/PORT_INDEX
myhost:~$ dl port show
devlink0/1: type ib ibdev mlx4_0
devlink0/2: type ib ibdev mlx4_0
myhost:~$ sudo dl port set devlink0/1 type eth
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
devlink0/2: type ib ibdev mlx4_0
myhost:~$ sudo dl port set devlink0/2 type auto
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
devlink0/2: type ib(auto) ibdev mlx4_0
Port splitting example:
myswitch:~$ dl port
devlink0/1: type eth netdev eth0
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
myswitch:~$ sudo dl port split devlink0/1 2
myswitch:~$ dl port
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
devlink0/1: type eth netdev eth0 split_group 16
devlink0/2: type eth netdev eth32 split_group 16
myswitch:~$ sudo dl port unsplit devlink0/1
myswitch:~$ dl port
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
devlink0/1: type eth netdev eth0
Ido Schimmel (4):
mlxsw: spectrum: Unmap local port from module during teardown
mlxsw: spectrum: Store local port to module mapping during init
mlxsw: spectrum: Mark unused ports using NULL
mlxsw: spectrum: Introduce port splitting
Jiri Pirko (5):
Introduce devlink infrastructure
mlx4: Implement devlink interface
mlx4: Implement port type setting via devlink interface
mlxsw: Implement devlink interface
mlxsw: core: Add devlink port splitter callbacks
MAINTAINERS | 8 +
drivers/infiniband/hw/mlx4/main.c | 7 +
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +-
drivers/net/ethernet/mellanox/mlx4/intf.c | 9 +
drivers/net/ethernet/mellanox/mlx4/main.c | 129 +++-
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/core.c | 56 +-
drivers/net/ethernet/mellanox/mlxsw/core.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/port.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 238 ++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 8 +-
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 20 +
include/linux/mlx4/driver.h | 3 +
include/net/devlink.h | 156 +++++
include/uapi/linux/devlink.h | 73 ++
net/Kconfig | 7 +
net/core/Makefile | 1 +
net/core/devlink.c | 887 +++++++++++++++++++++++++
18 files changed, 1557 insertions(+), 59 deletions(-)
create mode 100644 include/net/devlink.h
create mode 100644 include/uapi/linux/devlink.h
create mode 100644 net/core/devlink.c
--
2.5.0
From: Jiri Pirko <redacted>
Introduce devlink infrastructure for drivers to register and expose to
userspace via generic Netlink interface.
There are two basic objects defined:
devlink - one instance for every "parent device", for example switch ASIC
devlink port - one instance for every physical port of the device.
This initial portion implements basic get/dump of objects to userspace.
Also, port splitter and port type setting is implemented.
Signed-off-by: Jiri Pirko <redacted>
---
MAINTAINERS | 8 +
include/net/devlink.h | 156 ++++++++
include/uapi/linux/devlink.h | 73 ++++
net/Kconfig | 7 +
net/core/Makefile | 1 +
net/core/devlink.c | 887 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 1132 insertions(+)
create mode 100644 include/net/devlink.h
create mode 100644 include/uapi/linux/devlink.h
create mode 100644 net/core/devlink.c
@@ -396,6 +396,13 @@ config DST_CACHEbool"dst cache"defaultn+configNET_DEVLINK+tristate"Network physical/parent device Netlink interface"+help+Networkphysical/parentdeviceNetlinkinterfaceprovides+infrastructuretosupportaccesstophysicalchip-wideconfigand+monitoring.+endif# if NET# Used by archs to tell that they support BPF_JIT
@@ -0,0 +1,887 @@+/*+*net/core/devlink.c-Networkphysical/parentdeviceNetlinkinterface+*+*Heavilyinspiredbynet/wireless/+*Copyright(c)2016MellanoxTechnologies.Allrightsreserved.+*Copyright(c)2016JiriPirko<jiri@mellanox.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/++#include<linux/kernel.h>+#include<linux/module.h>+#include<linux/types.h>+#include<linux/slab.h>+#include<linux/gfp.h>+#include<linux/device.h>+#include<linux/list.h>+#include<linux/netdevice.h>+#include<linux/sysfs.h>+#include<linux/bitops.h>+#include<rdma/ib_verbs.h>+#include<net/netlink.h>+#include<net/genetlink.h>+#include<net/rtnetlink.h>+#include<net/net_namespace.h>+#include<net/sock.h>+#include<net/devlink.h>++staticLIST_HEAD(devlink_list);++/* devlink_mutex+*+*Anoveralllockguardingeveryoperationcommingfromuserspace.+*Ifalsoguardsdevlinkdeviceslistanditistakenwhen+*driverregisters/unregistersit.+*/+staticDEFINE_MUTEX(devlink_mutex);++/* devlink_port_mutex+*+*Sharedlocktoguardlistsofportsinalldevlinkdevices.+*/+staticDEFINE_MUTEX(devlink_port_mutex);++staticstructnet*devlink_net(conststructdevlink*devlink)+{+returnread_pnet(&devlink->_net);+}++staticvoiddevlink_net_set(structdevlink*devlink,structnet*net)+{+write_pnet(&devlink->_net,net);+}++staticbooldevlink_name_exists(structnet*net,constchar*name)+{+structdevlink*devlink;++list_for_each_entry(devlink,&devlink_list,list){+if(strcmp(devlink_name(devlink),name)==0&&+net_eq(devlink_net(devlink),net))+returntrue;+}+returnfalse;+}++staticstructdevlink*devlink_get_by_index(structnet*net,intindex)+{+structdevlink*devlink;++list_for_each_entry(devlink,&devlink_list,list){+if(devlink->index==index&&+net_eq(devlink_net(devlink),net))+returndevlink;+}+returnNULL;+}++staticstructdevlink*devlink_get_from_attrs(structnet*net,+structnlattr**attrs)+{+if(attrs[DEVLINK_ATTR_INDEX]){+u32index=nla_get_u32(attrs[DEVLINK_ATTR_INDEX]);+structdevlink*devlink;++devlink=devlink_get_by_index(net,index);+if(!devlink)+returnERR_PTR(-ENODEV);+returndevlink;+}+returnERR_PTR(-EINVAL);+}++staticstructdevlink*devlink_get_from_info(structgenl_info*info)+{+returndevlink_get_from_attrs(genl_info_net(info),info->attrs);+}++staticstructdevlink_port*devlink_port_get_by_index(structdevlink*devlink,+intport_index)+{+structdevlink_port*devlink_port;++list_for_each_entry(devlink_port,&devlink->port_list,list){+if(devlink_port->index==port_index)+returndevlink_port;+}+returnNULL;+}++staticbooldevlink_port_index_exists(structdevlink*devlink,intport_index)+{+returndevlink_port_get_by_index(devlink,port_index);+}++staticstructdevlink_port*devlink_port_get_from_attrs(structdevlink*devlink,+structnlattr**attrs)+{+if(attrs[DEVLINK_ATTR_PORT_INDEX]){+u32port_index=nla_get_u32(attrs[DEVLINK_ATTR_PORT_INDEX]);+structdevlink_port*devlink_port;++devlink_port=devlink_port_get_by_index(devlink,port_index);+if(!devlink_port)+returnERR_PTR(-ENODEV);+returndevlink_port;+}+returnERR_PTR(-EINVAL);+}++staticstructdevlink_port*devlink_port_get_from_info(structdevlink*devlink,+structgenl_info*info)+{+returndevlink_port_get_from_attrs(devlink,info->attrs);+}++#define DEVLINK_NL_FLAG_NEED_PORT BIT(0)++staticintdevlink_nl_pre_doit(conststructgenl_ops*ops,+structsk_buff*skb,structgenl_info*info)+{+structdevlink*devlink;++mutex_lock(&devlink_mutex);+devlink=devlink_get_from_info(info);+if(IS_ERR(devlink)){+mutex_unlock(&devlink_mutex);+returnPTR_ERR(devlink);+}+info->user_ptr[0]=devlink;+if(ops->internal_flags&DEVLINK_NL_FLAG_NEED_PORT){+structdevlink_port*devlink_port;++mutex_lock(&devlink_port_mutex);+devlink_port=devlink_port_get_from_info(devlink,info);+if(IS_ERR(devlink_port)){+mutex_unlock(&devlink_port_mutex);+mutex_unlock(&devlink_mutex);+returnPTR_ERR(devlink_port);+}+info->user_ptr[1]=devlink_port;+}+return0;+}++staticvoiddevlink_nl_post_doit(conststructgenl_ops*ops,+structsk_buff*skb,structgenl_info*info)+{+if(ops->internal_flags&DEVLINK_NL_FLAG_NEED_PORT)+mutex_unlock(&devlink_port_mutex);+mutex_unlock(&devlink_mutex);+}++staticstructgenl_familydevlink_nl_family={+.id=GENL_ID_GENERATE,+.name=DEVLINK_GENL_NAME,+.version=DEVLINK_GENL_VERSION,+.maxattr=DEVLINK_ATTR_MAX,+.netnsok=true,+.pre_doit=devlink_nl_pre_doit,+.post_doit=devlink_nl_post_doit,+};++enumdevlink_multicast_groups{+DEVLINK_MCGRP_CONFIG,+};++staticconststructgenl_multicast_groupdevlink_nl_mcgrps[]={+[DEVLINK_MCGRP_CONFIG]={.name=DEVLINK_GENL_MCGRP_CONFIG_NAME},+};++staticintdevlink_nl_fill(structsk_buff*msg,structdevlink*devlink,+enumdevlink_commandcmd,u32portid,+u32seq,intflags)+{+void*hdr;++hdr=genlmsg_put(msg,portid,seq,&devlink_nl_family,flags,cmd);+if(!hdr)+return-EMSGSIZE;++if(nla_put_u32(msg,DEVLINK_ATTR_INDEX,devlink->index))+gotonla_put_failure;+if(nla_put_string(msg,DEVLINK_ATTR_NAME,devlink_name(devlink)))+gotonla_put_failure;++if(devlink->dev.parent){+structdevice*dev=devlink->dev.parent;++if(nla_put_string(msg,DEVLINK_ATTR_BUS_NAME,dev->bus->name))+gotonla_put_failure;+if(nla_put_string(msg,DEVLINK_ATTR_DEV_NAME,dev_name(dev)))+gotonla_put_failure;+}++genlmsg_end(msg,hdr);+return0;++nla_put_failure:+genlmsg_cancel(msg,hdr);+return-EMSGSIZE;+}++staticvoiddevlink_notify(structdevlink*devlink,enumdevlink_commandcmd)+{+structsk_buff*msg;+interr;++WARN_ON(cmd!=DEVLINK_CMD_NEW&&cmd!=DEVLINK_CMD_DEL);++msg=nlmsg_new(NLMSG_DEFAULT_SIZE,GFP_KERNEL);+if(!msg)+return;++err=devlink_nl_fill(msg,devlink,cmd,0,0,0);+if(err){+nlmsg_free(msg);+return;+}++genlmsg_multicast_netns(&devlink_nl_family,devlink_net(devlink),+msg,0,DEVLINK_MCGRP_CONFIG,GFP_KERNEL);+}++staticintdevlink_nl_port_fill(structsk_buff*msg,structdevlink*devlink,+structdevlink_port*devlink_port,+enumdevlink_commandcmd,u32portid,+u32seq,intflags)+{+void*hdr;++hdr=genlmsg_put(msg,portid,seq,&devlink_nl_family,flags,cmd);+if(!hdr)+return-EMSGSIZE;++if(nla_put_u32(msg,DEVLINK_ATTR_INDEX,devlink->index))+gotonla_put_failure;+if(nla_put_u32(msg,DEVLINK_ATTR_PORT_INDEX,devlink_port->index))+gotonla_put_failure;+if(nla_put_u16(msg,DEVLINK_ATTR_PORT_TYPE,devlink_port->type))+gotonla_put_failure;+if(devlink_port->desired_type!=DEVLINK_PORT_TYPE_NOTSET&&+nla_put_u16(msg,DEVLINK_ATTR_PORT_DESIRED_TYPE,+devlink_port->desired_type))+gotonla_put_failure;+if(devlink_port->type==DEVLINK_PORT_TYPE_ETH){+structnet_device*netdev=devlink_port->type_dev;++if(netdev&&+(nla_put_u32(msg,DEVLINK_ATTR_PORT_NETDEV_IFINDEX,+netdev->ifindex)||+nla_put_string(msg,DEVLINK_ATTR_PORT_NETDEV_NAME,+netdev->name)))+gotonla_put_failure;+}+if(devlink_port->type==DEVLINK_PORT_TYPE_IB){+structib_device*ibdev=devlink_port->type_dev;++if(ibdev&&+nla_put_string(msg,DEVLINK_ATTR_PORT_IBDEV_NAME,+ibdev->name))+gotonla_put_failure;+}+if(devlink_port->split&&+nla_put_u32(msg,DEVLINK_ATTR_PORT_SPLIT_GROUP,+devlink_port->split_group))+gotonla_put_failure;++genlmsg_end(msg,hdr);+return0;++nla_put_failure:+genlmsg_cancel(msg,hdr);+return-EMSGSIZE;+}++staticvoiddevlink_port_notify(structdevlink_port*devlink_port,+enumdevlink_commandcmd)+{+structdevlink*devlink=devlink_port->devlink;+structsk_buff*msg;+interr;++if(!devlink_port->registered)+return;++WARN_ON(cmd!=DEVLINK_CMD_PORT_NEW&&cmd!=DEVLINK_CMD_PORT_DEL);++msg=nlmsg_new(NLMSG_DEFAULT_SIZE,GFP_KERNEL);+if(!msg)+return;++err=devlink_nl_port_fill(msg,devlink,devlink_port,cmd,0,0,0);+if(err){+nlmsg_free(msg);+return;+}++genlmsg_multicast_netns(&devlink_nl_family,devlink_net(devlink),+msg,0,DEVLINK_MCGRP_CONFIG,GFP_KERNEL);+}++staticintdevlink_nl_cmd_get_doit(structsk_buff*skb,structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+structsk_buff*msg;+interr;++msg=nlmsg_new(NLMSG_DEFAULT_SIZE,GFP_KERNEL);+if(!msg)+return-ENOMEM;++err=devlink_nl_fill(msg,devlink,DEVLINK_CMD_NEW,+info->snd_portid,info->snd_seq,0);+if(err){+nlmsg_free(msg);+returnerr;+}++returngenlmsg_reply(msg,info);+}++staticintdevlink_nl_cmd_get_dumpit(structsk_buff*msg,+structnetlink_callback*cb)+{+structdevlink*devlink;+intstart=cb->args[0];+intidx=0;+interr;++mutex_lock(&devlink_mutex);+list_for_each_entry(devlink,&devlink_list,list){+if(!net_eq(devlink_net(devlink),sock_net(msg->sk)))+continue;+if(idx<start){+idx++;+continue;+}+err=devlink_nl_fill(msg,devlink,DEVLINK_CMD_NEW,+NETLINK_CB(cb->skb).portid,+cb->nlh->nlmsg_seq,NLM_F_MULTI);+if(err)+gotoout;+idx++;+}+out:+mutex_unlock(&devlink_mutex);++cb->args[0]=idx;+returnmsg->len;+}++staticintdevlink_rename(structdevlink*devlink,constchar*newname)+{+interr;++if(strcmp(newname,devlink_name(devlink))==0)+return0;+if(devlink_name_exists(devlink_net(devlink),newname))+return-EINVAL;+if(!dev_valid_name(newname))+return-EINVAL;+err=device_rename(&devlink->dev,newname);+if(err)+returnerr;+devlink_notify(devlink,DEVLINK_CMD_NEW);+return0;+}++staticintdevlink_nl_cmd_set_doit(structsk_buff*skb,structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+interr;++if(info->attrs[DEVLINK_ATTR_NAME]){+err=devlink_rename(devlink,+nla_data(info->attrs[DEVLINK_ATTR_NAME]));+if(err)+returnerr;+}+return0;+}++staticintdevlink_nl_cmd_port_get_doit(structsk_buff*skb,+structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+structdevlink_port*devlink_port=info->user_ptr[1];+structsk_buff*msg;+interr;++msg=nlmsg_new(NLMSG_DEFAULT_SIZE,GFP_KERNEL);+if(!msg)+return-ENOMEM;++err=devlink_nl_port_fill(msg,devlink,devlink_port,+DEVLINK_CMD_PORT_NEW,+info->snd_portid,info->snd_seq,0);+if(err){+nlmsg_free(msg);+returnerr;+}++returngenlmsg_reply(msg,info);+}++staticintdevlink_nl_cmd_port_get_dumpit(structsk_buff*msg,+structnetlink_callback*cb)+{+structdevlink*devlink;+structdevlink_port*devlink_port;+intstart=cb->args[0];+intidx=0;+interr;++mutex_lock(&devlink_mutex);+mutex_lock(&devlink_port_mutex);+list_for_each_entry(devlink,&devlink_list,list){+if(!net_eq(devlink_net(devlink),sock_net(msg->sk)))+continue;+list_for_each_entry(devlink_port,&devlink->port_list,list){+if(idx<start){+idx++;+continue;+}+err=devlink_nl_port_fill(msg,devlink,devlink_port,+DEVLINK_CMD_NEW,+NETLINK_CB(cb->skb).portid,+cb->nlh->nlmsg_seq,+NLM_F_MULTI);+if(err)+gotoout;+idx++;+}+}+out:+mutex_unlock(&devlink_port_mutex);+mutex_unlock(&devlink_mutex);++cb->args[0]=idx;+returnmsg->len;+}++staticintdevlink_port_type_set(structdevlink*devlink,+structdevlink_port*devlink_port,+enumdevlink_port_typeport_type)++{+interr;++if(devlink->ops&&devlink->ops->port_type_set){+if(port_type==DEVLINK_PORT_TYPE_NOTSET)+return-EINVAL;+err=devlink->ops->port_type_set(devlink_port,port_type);+if(err)+returnerr;+devlink_port->desired_type=port_type;+devlink_port_notify(devlink_port,DEVLINK_CMD_PORT_NEW);+return0;+}+return-EOPNOTSUPP;+}++staticintdevlink_nl_cmd_port_set_doit(structsk_buff*skb,+structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+structdevlink_port*devlink_port=info->user_ptr[1];+interr;++if(info->attrs[DEVLINK_ATTR_PORT_TYPE]){+enumdevlink_port_typeport_type;++port_type=nla_get_u16(info->attrs[DEVLINK_ATTR_PORT_TYPE]);+err=devlink_port_type_set(devlink,devlink_port,port_type);+if(err)+returnerr;+}+return0;+}++staticintdevlink_port_split(structdevlink*devlink,+u32port_index,u32count)++{+if(devlink->ops&&devlink->ops->port_split)+returndevlink->ops->port_split(devlink,port_index,count);+return-EOPNOTSUPP;+}++staticintdevlink_nl_cmd_port_split_doit(structsk_buff*skb,+structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+u32port_index;+u32count;++if(!info->attrs[DEVLINK_ATTR_PORT_INDEX]||+!info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT])+return-EINVAL;++port_index=nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);+count=nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT]);+returndevlink_port_split(devlink,port_index,count);+}++staticintdevlink_port_unsplit(structdevlink*devlink,u32port_index)++{+if(devlink->ops&&devlink->ops->port_unsplit)+returndevlink->ops->port_unsplit(devlink,port_index);+return-EOPNOTSUPP;+}++staticintdevlink_nl_cmd_port_unsplit_doit(structsk_buff*skb,+structgenl_info*info)+{+structdevlink*devlink=info->user_ptr[0];+u32port_index;++if(!info->attrs[DEVLINK_ATTR_PORT_INDEX])+return-EINVAL;++port_index=nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);+returndevlink_port_unsplit(devlink,port_index);+}++staticconststructnla_policydevlink_nl_policy[DEVLINK_ATTR_MAX+1]={+[DEVLINK_ATTR_INDEX]={.type=NLA_U32},+[DEVLINK_ATTR_NAME]={.type=NLA_NUL_STRING,+.len=DEVLINK_ATTR_NAME_MAX_LEN},+[DEVLINK_ATTR_PORT_INDEX]={.type=NLA_U32},+[DEVLINK_ATTR_PORT_TYPE]={.type=NLA_U16},+[DEVLINK_ATTR_PORT_SPLIT_COUNT]={.type=NLA_U32},+};++staticconststructgenl_opsdevlink_nl_ops[]={+{+.cmd=DEVLINK_CMD_GET,+.doit=devlink_nl_cmd_get_doit,+.dumpit=devlink_nl_cmd_get_dumpit,+.policy=devlink_nl_policy,+/* can be retrieved by unprivileged users */+},+{+.cmd=DEVLINK_CMD_SET,+.doit=devlink_nl_cmd_set_doit,+.policy=devlink_nl_policy,+.flags=GENL_ADMIN_PERM,+},+{+.cmd=DEVLINK_CMD_PORT_GET,+.doit=devlink_nl_cmd_port_get_doit,+.dumpit=devlink_nl_cmd_port_get_dumpit,+.policy=devlink_nl_policy,+.internal_flags=DEVLINK_NL_FLAG_NEED_PORT,+/* can be retrieved by unprivileged users */+},+{+.cmd=DEVLINK_CMD_PORT_SET,+.doit=devlink_nl_cmd_port_set_doit,+.policy=devlink_nl_policy,+.flags=GENL_ADMIN_PERM,+.internal_flags=DEVLINK_NL_FLAG_NEED_PORT,+},+{+.cmd=DEVLINK_CMD_PORT_SPLIT,+.doit=devlink_nl_cmd_port_split_doit,+.policy=devlink_nl_policy,+.flags=GENL_ADMIN_PERM,+},+{+.cmd=DEVLINK_CMD_PORT_UNSPLIT,+.doit=devlink_nl_cmd_port_unsplit_doit,+.policy=devlink_nl_policy,+.flags=GENL_ADMIN_PERM,+},+};++staticstructclassdevlink_class;++/**+*devlink_alloc-Allocatenewdevlinkinstanceresources+*+*@ops:ops+*@priv_size:sizeofuserprivatedata+*+*Allocatenewdevlinkinstanceresources,includingdevlinkindex+*andname.+*/+structdevlink*devlink_alloc(conststructdevlink_ops*ops,size_tpriv_size)+{+staticatomic_tdev_counter=ATOMIC_INIT(0);+structdevlink*devlink;++devlink=kzalloc(sizeof(*devlink)+priv_size,GFP_KERNEL);+if(!devlink)+returnNULL;+devlink->ops=ops;+devlink_net_set(devlink,&init_net);++different_name:+devlink->index=atomic_inc_return(&dev_counter);+if(devlink->index<0){+/* wrapped */+atomic_dec(&dev_counter);+kfree(devlink);+returnNULL;+}+/* atomic_inc_return makes it start at 1, make it start at 0 */+devlink->index--;++dev_set_name(&devlink->dev,DEVLINK_GENL_NAME"%d",devlink->index);+if(devlink_name_exists(devlink_net(devlink),devlink_name(devlink)))+gotodifferent_name;++INIT_LIST_HEAD(&devlink->port_list);+device_initialize(&devlink->dev);+devlink->dev.class=&devlink_class;+devlink->dev.platform_data=devlink;+returndevlink;+}+EXPORT_SYMBOL_GPL(devlink_alloc);++/**+*devlink_register-Registerdevlinkinstance+*+*@devlink:devlink+*/+intdevlink_register(structdevlink*devlink)+{+interr;++mutex_lock(&devlink_mutex);+err=device_add(&devlink->dev);+if(err)+gotounlock;+list_add_tail(&devlink->list,&devlink_list);+devlink_notify(devlink,DEVLINK_CMD_NEW);+unlock:+mutex_unlock(&devlink_mutex);+returnerr;+}+EXPORT_SYMBOL_GPL(devlink_register);++/**+*devlink_unregister-Unregisterdevlinkinstance+*+*@devlink:devlink+*/+voiddevlink_unregister(structdevlink*devlink)+{+mutex_lock(&devlink_mutex);+devlink_notify(devlink,DEVLINK_CMD_DEL);+list_del(&devlink->list);+device_del(&devlink->dev);+mutex_unlock(&devlink_mutex);+}+EXPORT_SYMBOL_GPL(devlink_unregister);++/**+*devlink_free-Freedevlinkinstanceresources+*+*@devlink:devlink+*/+voiddevlink_free(structdevlink*devlink)+{+put_device(&devlink->dev);+}+EXPORT_SYMBOL_GPL(devlink_free);++/**+*devlink_port_register-Registerdevlinkport+*+*@devlink:devlink+*@devlink_port:devlinkport+*@port_index+*+*Registerdevlinkportwithprovidedportindex.Usercanuse+*anyindexing,evenhw-relatedone.devlink_portstructure+*isconvenienttobeembeddedinsideuserdriverprivatestructure.+*Notethatthecallershouldtakecareofzeroingthedevlink_port+*structure.+*/+intdevlink_port_register(structdevlink*devlink,+structdevlink_port*devlink_port,+unsignedintport_index)+{+mutex_lock(&devlink_port_mutex);+if(devlink_port_index_exists(devlink,port_index)){+mutex_unlock(&devlink_port_mutex);+return-EEXIST;+}+devlink_port->devlink=devlink;+devlink_port->index=port_index;+devlink_port->type=DEVLINK_PORT_TYPE_NOTSET;+devlink_port->registered=true;+list_add_tail(&devlink_port->list,&devlink->port_list);+mutex_unlock(&devlink_port_mutex);+devlink_port_notify(devlink_port,DEVLINK_CMD_PORT_NEW);+return0;+}+EXPORT_SYMBOL_GPL(devlink_port_register);++/**+*devlink_port_unregister-Unregisterdevlinkport+*+*@devlink_port:devlinkport+*/+voiddevlink_port_unregister(structdevlink_port*devlink_port)+{+devlink_port_notify(devlink_port,DEVLINK_CMD_PORT_DEL);+mutex_lock(&devlink_port_mutex);+list_del(&devlink_port->list);+mutex_unlock(&devlink_port_mutex);+}+EXPORT_SYMBOL_GPL(devlink_port_unregister);++staticvoid__devlink_port_type_set(structdevlink_port*devlink_port,+enumdevlink_port_typetype,+void*type_dev)+{+devlink_port->type=type;+devlink_port->type_dev=type_dev;+devlink_port_notify(devlink_port,DEVLINK_CMD_PORT_NEW);+}++/**+*devlink_port_type_eth_set-SetporttypetoEthernet+*+*@devlink_port:devlinkport+*@netdev:relatednetdevice+*/+voiddevlink_port_type_eth_set(structdevlink_port*devlink_port,+structnet_device*netdev)+{+return__devlink_port_type_set(devlink_port,+DEVLINK_PORT_TYPE_ETH,netdev);+}+EXPORT_SYMBOL_GPL(devlink_port_type_eth_set);++/**+*devlink_port_type_ib_set-SetporttypetoInfiniBand+*+*@devlink_port:devlinkport+*@ibdev:relatedIBdevice+*/+voiddevlink_port_type_ib_set(structdevlink_port*devlink_port,+structib_device*ibdev)+{+return__devlink_port_type_set(devlink_port,+DEVLINK_PORT_TYPE_IB,ibdev);+}+EXPORT_SYMBOL_GPL(devlink_port_type_ib_set);++/**+*devlink_port_type_clear-Clearporttype+*+*@devlink_port:devlinkport+*/+voiddevlink_port_type_clear(structdevlink_port*devlink_port)+{+return__devlink_port_type_set(devlink_port,+DEVLINK_PORT_TYPE_NOTSET,NULL);+}+EXPORT_SYMBOL_GPL(devlink_port_type_clear);++/**+*devlink_port_split_set-Setportissplitted+*+*@devlink_port:devlinkport+*@split_group:splitgroup-identifiesgroupsplittedportispartof+*/+voiddevlink_port_split_set(structdevlink_port*devlink_port,+u32split_group)+{+devlink_port->split=true;;+devlink_port->split_group=split_group;+devlink_port_notify(devlink_port,DEVLINK_CMD_PORT_NEW);+}+EXPORT_SYMBOL_GPL(devlink_port_split_set);++staticvoid__devlink_free(structdevlink*devlink)+{+kfree(devlink);+}++staticstructdevlink*dev_to_devlink(structdevice*dev)+{+returncontainer_of(dev,structdevlink,dev);+}++staticssize_tindex_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+returnsprintf(buf,"%d\n",dev_to_devlink(dev)->index);+}+staticDEVICE_ATTR_RO(index);++staticssize_tname_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+returnsprintf(buf,"%s\n",dev_name(dev));+}+staticDEVICE_ATTR_RO(name);++staticstructattribute*devlink_attrs[]={+&dev_attr_index.attr,+&dev_attr_name.attr,+NULL,+};+ATTRIBUTE_GROUPS(devlink);++staticvoiddevlink_dev_release(structdevice*dev)+{+__devlink_free(dev_to_devlink(dev));+}++staticconstvoid*devlink_namespace(structdevice*dev)+{+returndevlink_net(dev_to_devlink(dev));+}++staticstructclassdevlink_class={+.name=DEVLINK_GENL_NAME,+.owner=THIS_MODULE,+.dev_release=devlink_dev_release,+.dev_groups=devlink_groups,+.ns_type=&net_ns_type_operations,+.namespace=devlink_namespace,+};++staticint__initdevlink_module_init(void)+{+interr;++err=class_register(&devlink_class);+if(err)+returnerr;+err=genl_register_family_with_ops_groups(&devlink_nl_family,+devlink_nl_ops,+devlink_nl_mcgrps);+if(err)+gotoerr_genl_register;+return0;++err_genl_register:+class_unregister(&devlink_class);+returnerr;+}++staticvoid__exitdevlink_module_exit(void)+{+genl_unregister_family(&devlink_nl_family);+class_unregister(&devlink_class);+}++module_init(devlink_module_init);+module_exit(devlink_module_exit);++MODULE_LICENSE("GPL v2");+MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");+MODULE_DESCRIPTION("Network physical device Netlink interface");+MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
@@ -2033,8 +2034,11 @@ void mlx4_en_destroy_netdev(struct net_device *dev)en_dbg(DRV,priv,"Destroying netdev on port:%d\n",priv->port);/* Unregister device - this will close the port if it was up */-if(priv->registered)+if(priv->registered){+devlink_port_type_clear(mlx4_get_devlink_port(mdev->dev,+priv->port));unregister_netdev(dev);+}if(priv->allocated)mlx4_free_hwq_res(mdev->dev,&priv->res,MLX4_EN_PAGE_SIZE);
@@ -3050,6 +3054,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,}priv->registered=1;+devlink_port_type_eth_set(mlx4_get_devlink_port(mdev->dev,priv->port),+dev);return0;
@@ -2873,6 +2879,7 @@ static int mlx4_init_port_info(struct mlx4_dev *dev, int port)err=device_create_file(&dev->persist->pdev->dev,&info->port_attr);if(err){mlx4_err(dev,"Failed to create file for port %d\n",port);+devlink_port_unregister(&info->devlink_port);info->port=-1;}
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Allow to expose the same ability to set port type in a generic way
using devlink interface.
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 86 +++++++++++++++++++++++--------
1 file changed, 65 insertions(+), 21 deletions(-)
@@ -1052,36 +1052,20 @@ static ssize_t show_port_type(struct device *dev,returnstrlen(buf);}-staticssize_tset_port_type(structdevice*dev,-structdevice_attribute*attr,-constchar*buf,size_tcount)+staticint__set_port_type(structmlx4_port_info*info,+enummlx4_port_typeport_type){-structmlx4_port_info*info=container_of(attr,structmlx4_port_info,-port_attr);structmlx4_dev*mdev=info->dev;structmlx4_priv*priv=mlx4_priv(mdev);enummlx4_port_typetypes[MLX4_MAX_PORTS];enummlx4_port_typenew_types[MLX4_MAX_PORTS];-staticDEFINE_MUTEX(set_port_type_mutex);inti;interr=0;-mutex_lock(&set_port_type_mutex);--if(!strcmp(buf,"ib\n"))-info->tmp_type=MLX4_PORT_TYPE_IB;-elseif(!strcmp(buf,"eth\n"))-info->tmp_type=MLX4_PORT_TYPE_ETH;-elseif(!strcmp(buf,"auto\n"))-info->tmp_type=MLX4_PORT_TYPE_AUTO;-else{-mlx4_err(mdev,"%s is not supported port type\n",buf);-err=-EINVAL;-gotoerr_out;-}-mlx4_stop_sense(mdev);mutex_lock(&priv->port_mutex);+info->tmp_type=port_type;+/* Possible type is always the one that was delivered */mdev->caps.possible_type[info->port]=info->tmp_type;
@@ -1123,6 +1107,37 @@ static ssize_t set_port_type(struct device *dev,out:mlx4_start_sense(mdev);mutex_unlock(&priv->port_mutex);++returnerr;+}++staticssize_tset_port_type(structdevice*dev,+structdevice_attribute*attr,+constchar*buf,size_tcount)+{+structmlx4_port_info*info=container_of(attr,structmlx4_port_info,+port_attr);+structmlx4_dev*mdev=info->dev;+enummlx4_port_typeport_type;+staticDEFINE_MUTEX(set_port_type_mutex);+interr;++mutex_lock(&set_port_type_mutex);++if(!strcmp(buf,"ib\n")){+port_type=MLX4_PORT_TYPE_IB;+}elseif(!strcmp(buf,"eth\n")){+port_type=MLX4_PORT_TYPE_ETH;+}elseif(!strcmp(buf,"auto\n")){+port_type=MLX4_PORT_TYPE_AUTO;+}else{+mlx4_err(mdev,"%s is not supported port type\n",buf);+err=-EINVAL;+gotoerr_out;+}++err=__set_port_type(info,port_type);+err_out:mutex_unlock(&set_port_type_mutex);
@@ -1417,6 +1421,14 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port)gotoport_not_usable;}+devlink_port=&mlxsw_sp_port->devlink_port;+err=devlink_port_register(devlink,devlink_port,local_port);+if(err){+dev_err(mlxsw_sp->bus_info->dev,"Port %d: Failed to register devlink port\n",+mlxsw_sp_port->local_port);+gotoerr_devlink_port_register;+}+err=mlxsw_sp_port_system_port_mapping_set(mlxsw_sp_port);if(err){dev_err(mlxsw_sp->bus_info->dev,"Port %d: Failed to set system port mapping\n",
@@ -1007,6 +1011,14 @@ static int mlxsw_sx_port_create(struct mlxsw_sx *mlxsw_sx, u8 local_port)gotoport_not_usable;}+devlink_port=&mlxsw_sx_port->devlink_port;+err=devlink_port_register(devlink,devlink_port,local_port);+if(err){+dev_err(mlxsw_sx->bus_info->dev,"Port %d: Failed to register devlink port\n",+mlxsw_sx_port->local_port);+gotoerr_devlink_port_register;+}+err=mlxsw_sx_port_system_port_mapping_set(mlxsw_sx_port);if(err){dev_err(mlxsw_sx->bus_info->dev,"Port %d: Failed to set system port mapping\n",
From: Ido Schimmel <redacted>
When splitting a port we replace it with 2 or 4 other ports. To be able
to do that we need to remove the original port netdev and unmap it from
its module. However, we first mark it as disabled, as active ports
cannot be unmapped.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 11 +++++++++++
1 file changed, 11 insertions(+)
From: Ido Schimmel <redacted>
The port netdevs are each associated with a different local port number
in the device. These local ports are grouped into groups of 4 (e.g.
(1-4), (5-8)) called clusters. The cluster constitutes the one of two
possible modules they can be mapped to. This mapping is board-specific
and done by the device's firmware during init.
When splitting a port by 4, the device requires us to first unmap all
the ports in the cluster and then map each to a single lane in the module
associated with the port netdev used as the handle for the operation.
This means that two port netdevs will disappear, as only 100Gb/s (4
lanes) ports can be split and we are guaranteed to have two of these
((1, 3), (5, 7) etc.) in a cluster.
When unsplit occurs we need to reinstantiate the two original 100Gb/s
ports and map each to its origianl module. Therefore, during driver init
store the initial local port to module mapping, so it can be used later
during unsplitting.
Note that a by 2 split doesn't require us to store the mapping, as we
only need to reinstantiate one port whose module is known.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 36 +++++++++++---------------
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
2 files changed, 16 insertions(+), 21 deletions(-)
From: Ido Schimmel <redacted>
When splitting and unsplitting we'll destroy usable ports on the fly, so
mark them using a NULL pointer to indicate that their local port number
is free and can be re-used.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 1 +
1 file changed, 1 insertion(+)
From: Ido Schimmel <redacted>
Allow a user to split or unsplit a port using the newly introduced
devlink ops.
Once split, the original netdev is destroyed and 2 or 4 others are
created, according to user configuration. The new ports are like any
other port, with the sole difference of supporting a lower maximum
speed. When unsplit, the reverse process takes place.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/port.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 178 ++++++++++++++++++++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 5 +-
3 files changed, 182 insertions(+), 3 deletions(-)
@@ -1563,7 +1604,7 @@ static int mlxsw_sp_ports_create(struct mlxsw_sp *mlxsw_sp)if(!width)continue;mlxsw_sp->port_to_module[i]=module;-err=mlxsw_sp_port_create(mlxsw_sp,i);+err=__mlxsw_sp_port_create(mlxsw_sp,i,false,module);if(err)gotoerr_port_create;}
@@ -1577,6 +1618,137 @@ err_port_module_info_get:returnerr;}+staticu8mlxsw_sp_cluster_base_port_get(u8local_port)+{+u8offset=(local_port-1)%MLXSW_SP_PORTS_PER_CLUSTER_MAX;++returnlocal_port-offset;+}++staticintmlxsw_sp_port_split(void*priv,u8local_port,unsignedintcount)+{+structmlxsw_sp*mlxsw_sp=priv;+structmlxsw_sp_port*mlxsw_sp_port;+u8width=MLXSW_PORT_MODULE_MAX_WIDTH/count;+u8module,cur_width,base_port;+inti;+interr;++mlxsw_sp_port=mlxsw_sp->ports[local_port];+if(!mlxsw_sp_port){+dev_err(mlxsw_sp->bus_info->dev,"Port number \"%d\" does not exist\n",+local_port);+return-EINVAL;+}++if(count!=2&&count!=4){+netdev_err(mlxsw_sp_port->dev,"Port can only be split into 2 or 4 ports\n");+return-EINVAL;+}++err=mlxsw_sp_port_module_info_get(mlxsw_sp,local_port,&module,+&cur_width);+if(err){+netdev_err(mlxsw_sp_port->dev,"Failed to get port's width\n");+returnerr;+}++if(cur_width!=MLXSW_PORT_MODULE_MAX_WIDTH){+netdev_err(mlxsw_sp_port->dev,"Port cannot be split further\n");+return-EINVAL;+}++/* Make sure we have enough slave (even) ports for the split. */+if(count==2){+base_port=local_port;+if(mlxsw_sp->ports[base_port+1]){+netdev_err(mlxsw_sp_port->dev,"Invalid split configuration\n");+return-EINVAL;+}+}else{+base_port=mlxsw_sp_cluster_base_port_get(local_port);+if(mlxsw_sp->ports[base_port+1]||+mlxsw_sp->ports[base_port+3]){+netdev_err(mlxsw_sp_port->dev,"Invalid split configuration\n");+return-EINVAL;+}+}++for(i=0;i<count;i++)+mlxsw_sp_port_remove(mlxsw_sp,base_port+i);++for(i=0;i<count;i++){+err=mlxsw_sp_port_create(mlxsw_sp,base_port+i,true,+module,width,i*width);+if(err){+dev_err(mlxsw_sp->bus_info->dev,"Failed to create split port\n");+gotoerr_port_create;+}+}++return0;++err_port_create:+for(i--;i>=0;i--)+mlxsw_sp_port_remove(mlxsw_sp,base_port+i);+for(i=0;i<count/2;i++){+module=mlxsw_sp->port_to_module[base_port+i*2];+mlxsw_sp_port_create(mlxsw_sp,base_port+i*2,false,+module,MLXSW_PORT_MODULE_MAX_WIDTH,0);+}+returnerr;+}++staticintmlxsw_sp_port_unsplit(void*priv,u8local_port)+{+structmlxsw_sp*mlxsw_sp=priv;+structmlxsw_sp_port*mlxsw_sp_port;+u8module,cur_width,base_port;+unsignedintcount;+inti;+interr;++mlxsw_sp_port=mlxsw_sp->ports[local_port];+if(!mlxsw_sp_port){+dev_err(mlxsw_sp->bus_info->dev,"Port number \"%d\" does not exist\n",+local_port);+return-EINVAL;+}++if(!mlxsw_sp_port->split){+netdev_err(mlxsw_sp_port->dev,"Port wasn't split\n");+return-EINVAL;+}++err=mlxsw_sp_port_module_info_get(mlxsw_sp,local_port,&module,+&cur_width);+if(err){+netdev_err(mlxsw_sp_port->dev,"Failed to get port's width\n");+returnerr;+}+count=cur_width==1?4:2;++base_port=mlxsw_sp_cluster_base_port_get(local_port);++/* Determine which ports to remove. */+if(count==2&&local_port>=base_port+2)+base_port=base_port+2;++for(i=0;i<count;i++)+mlxsw_sp_port_remove(mlxsw_sp,base_port+i);++for(i=0;i<count/2;i++){+module=mlxsw_sp->port_to_module[base_port+i*2];+err=mlxsw_sp_port_create(mlxsw_sp,base_port+i*2,false,+module,MLXSW_PORT_MODULE_MAX_WIDTH,+0);+if(err)+dev_err(mlxsw_sp->bus_info->dev,"Failed to reinstantiate port\n");+}++return0;+}+staticvoidmlxsw_sp_pude_event_func(conststructmlxsw_reg_info*reg,char*pude_pl,void*priv){
From: John Fastabend <john.fastabend@gmail.com> Date: 2016-02-22 20:33:09
On 16-02-22 10:32 AM, Jiri Pirko wrote:
From: Ido Schimmel <redacted>
When splitting a port we replace it with 2 or 4 other ports. To be able
to do that we need to remove the original port netdev and unmap it from
its module. However, we first mark it as disabled, as active ports
cannot be unmapped.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
Hi Jiri, Ido,
You've sort of lost me on this port splitting/unsplitting thread. What
does this actually do? Are you just creating two netdevs and LAGing them
in the hardware, I'm guessing not or you wouldn't have some device API
for it and would do it using normal methods.
If its something to do with physical layout of the board itself why
don't you trigger this based on some init time introspection or an
interrupt if someone plugs in a port splitting cable/module (does that
exist?).
Thanks,
John
From: John Fastabend <john.fastabend@gmail.com> Date: 2016-02-22 21:09:20
On 16-02-22 01:00 PM, Ido Schimmel wrote:
Hi John,
Mon, Feb 22, 2016 at 10:32:47PM IST, john.fastabend@gmail.com wrote:
quoted
On 16-02-22 10:32 AM, Jiri Pirko wrote:
quoted
From: Ido Schimmel <redacted>
When splitting a port we replace it with 2 or 4 other ports. To be able
to do that we need to remove the original port netdev and unmap it from
its module. However, we first mark it as disabled, as active ports
cannot be unmapped.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
Hi Jiri, Ido,
You've sort of lost me on this port splitting/unsplitting thread. What
does this actually do? Are you just creating two netdevs and LAGing them
in the hardware, I'm guessing not or you wouldn't have some device API
for it and would do it using normal methods.
Yep, it's not LAG. You basically have a mapping between a physical
module and a local port, which is represented by a port netdev.
Each module has 4 lanes, so if you connect a splitter (say a 2x) you can
map each 2 lanes to a different port and assign each a new local port.
These are completely independent from each other, but they can only give
you 50Gb/s max, as opposed to the original 100Gb/s (as it had 4 lanes
all to itself).
quoted
If its something to do with physical layout of the board itself why
don't you trigger this based on some init time introspection or an
interrupt if someone plugs in a port splitting cable/module (does that
exist?).
We currently don't have an event that tells us that a splitter is
connected. Also, had we created / destroyed these based on events, then
an accidental removal of the splitter would cause all the configuration
setup on these ports to disappear (say VLANs on a bridged port, unicast
flooding etc.).
I still think this would be the better implementation. The configuration
should be saved in some daemon anyways and setup based on netdev events
so its not like it would disappear in any real system.
But seeing you don't get an interrupt or anything I guess manual
configuration is going to be the best you can do.
Hi John,
Mon, Feb 22, 2016 at 10:32:47PM IST, john.fastabend@gmail.com wrote:
On 16-02-22 10:32 AM, Jiri Pirko wrote:
quoted
From: Ido Schimmel <redacted>
When splitting a port we replace it with 2 or 4 other ports. To be able
to do that we need to remove the original port netdev and unmap it from
its module. However, we first mark it as disabled, as active ports
cannot be unmapped.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
Hi Jiri, Ido,
You've sort of lost me on this port splitting/unsplitting thread. What
does this actually do? Are you just creating two netdevs and LAGing them
in the hardware, I'm guessing not or you wouldn't have some device API
for it and would do it using normal methods.
Yep, it's not LAG. You basically have a mapping between a physical
module and a local port, which is represented by a port netdev.
Each module has 4 lanes, so if you connect a splitter (say a 2x) you can
map each 2 lanes to a different port and assign each a new local port.
These are completely independent from each other, but they can only give
you 50Gb/s max, as opposed to the original 100Gb/s (as it had 4 lanes
all to itself).
If its something to do with physical layout of the board itself why
don't you trigger this based on some init time introspection or an
interrupt if someone plugs in a port splitting cable/module (does that
exist?).
We currently don't have an event that tells us that a splitter is
connected. Also, had we created / destroyed these based on events, then
an accidental removal of the splitter would cause all the configuration
setup on these ports to disappear (say VLANs on a bridged port, unicast
flooding etc.).
Thanks.
From: Jiri Pirko <redacted>
Introduce devlink infrastructure for drivers to register and expose to
userspace via generic Netlink interface.
There are two basic objects defined:
devlink - one instance for every "parent device", for example switch ASIC
devlink port - one instance for every physical port of the device.
Like i have expressed earlier, the only thing that bothers me here is that we are creating a new devlink object for switch port when there
is an existing netdev object.
Is there a chance that the drivers you are targeting can still create netdevs for physical ports ?
It would make things so much more consistent and simpler to manage without a cost of adding yet another interface.
The port splitter support is needed at the netdev api too (ie rtnetlink). Most switchdev drivers that expose netdevs
would benefit from a native 'ip link' way to configure port splitting. This will be useful for nic drivers too.
thanks,
Roopa
From: Andy Gospodarek <hidden> Date: 2016-02-23 04:50:38
On Mon, Feb 22, 2016 at 12:32:47PM -0800, John Fastabend wrote:
On 16-02-22 10:32 AM, Jiri Pirko wrote:
quoted
From: Ido Schimmel <redacted>
When splitting a port we replace it with 2 or 4 other ports. To be able
to do that we need to remove the original port netdev and unmap it from
its module. However, we first mark it as disabled, as active ports
cannot be unmapped.
Signed-off-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
Hi Jiri, Ido,
You've sort of lost me on this port splitting/unsplitting thread. What
does this actually do? Are you just creating two netdevs and LAGing them
in the hardware, I'm guessing not or you wouldn't have some device API
for it and would do it using normal methods.
If its something to do with physical layout of the board itself why
don't you trigger this based on some init time introspection or an
interrupt if someone plugs in a port splitting cable/module (does that
exist?).
In some implementations there are interrupts that fire when modules are
connected or removed. Even if an interrupt doesn't fire, it would be
possible to have a timer event/workqueue in a driver that could read
QFSP VPD information periodically to note that something has changed in
the hardware and reconfigure as needed. I actually would perfer keying
on something like that to perform a configuration change like rather
than requiring the user configure it.
Today you have to swap a 4x10GbE for a 1x40GbE module and then
performing significant software config (possibly flashing EEPROMs in the
case of some NICs) each time. That seems too error prone as a user
could swap out a QSFP on what they think is one port on a switch and
then perform a software change on another port without realizing it.
That accidental disruption seems less tolerable than the one where there
wrong physical port was added and removed and config needs to be
applied. Network management apps can properly solve the problem where
devices come and go accidently due to connecting the wrong QSFP, so that
seems like a less critical use-case to handle than what I described
above.
From: Andy Gospodarek <hidden> Date: 2016-02-23 05:12:20
On Mon, Feb 22, 2016 at 07:31:55PM +0100, Jiri Pirko wrote:
From: Jiri Pirko <redacted>
There a is need for some userspace API that would allow to expose things
that are not directly related to any device class like net_device of
ib_device, but rather chip-wide/switch-ASIC-wide stuff.
Use cases:
1) get/set of port type (Ethernet/InfiniBand)
2) setting up port splitters - split port into multiple ones and squash again,
enables usage of splitter cable
3) setting up shared buffers - shared among multiple ports within
one chip (work in progress)
4) configuration of switch wide properties - resources division etc - This will
allow to pass configuration that is unacceptable to be passed as
a module option.
I'm generally a fan of use cases #3 and #4 (as we have previously
discussed), but I'm not sure I agree that the implementation for #2
right now.
I'm not sure I would like userspace to have control over whether or not
a port should be split or not when the hardware can be queried to
determine this.
First patch of this set introduces a new generic Netlink based interface,
called "devlink". It is similar to nl80211 model and it is heavily
influenced by it, including the API definition. The devlink introduction patch
implements use cases 1) and 2). Other 2 are in development atm and will
be addressed by follow-ups.
It is very convenient for drivers to use devlink, as you can see in other
patches in this set.
Counterpart for devlink is userspace tool for now called "dl". Command line
interface and outputs are derived from "ip" tool so it should be easy
for users to get used to it.
It is available here as a standalone tool for now:
https://github.com/jpirko/devlink
After this is merge in kernel, I will include the "dl" or "devlink" tool
into iproute2 toolset.
Port type setting example:
myhost:~$ dl help
Usage: dl [ OPTIONS ] OBJECT { COMMAND | help }
where OBJECT := { dev | port | monitor }
OPTIONS := { -v/--verbose }
myhost:~$ dl dev help
Usage: dl dev show [DEV]
Usage: dl dev set DEV [ name NEWNAME ]
myhost:~$ dl dev show
0: devlink0: bus pci dev 0000:01:00.0
myhost:~$ dl port help
Usage: dl port show [DEV/PORT_INDEX]
Usage: dl port set DEV/PORT_INDEX [ type { eth | ib | auto} ]
Usage: dl port split DEV/PORT_INDEX count
Usage: dl port unsplit DEV/PORT_INDEX
myhost:~$ dl port show
devlink0/1: type ib ibdev mlx4_0
devlink0/2: type ib ibdev mlx4_0
myhost:~$ sudo dl port set devlink0/1 type eth
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
myhost:~$ sudo dl port set devlink0/2 type auto
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
devlink0/2: type ib(auto) ibdev mlx4_0
Port splitting example:
myswitch:~$ dl port
devlink0/1: type eth netdev eth0
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
myswitch:~$ sudo dl port split devlink0/1 2
myswitch:~$ dl port
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
devlink0/1: type eth netdev eth0 split_group 16
devlink0/2: type eth netdev eth32 split_group 16
myswitch:~$ sudo dl port unsplit devlink0/1
myswitch:~$ dl port
devlink0/3: type eth netdev eth1
devlink0/5: type eth netdev eth2
...
devlink0/63: type eth netdev eth31
devlink0/1: type eth netdev eth0
Ido Schimmel (4):
mlxsw: spectrum: Unmap local port from module during teardown
mlxsw: spectrum: Store local port to module mapping during init
mlxsw: spectrum: Mark unused ports using NULL
mlxsw: spectrum: Introduce port splitting
Jiri Pirko (5):
Introduce devlink infrastructure
mlx4: Implement devlink interface
mlx4: Implement port type setting via devlink interface
mlxsw: Implement devlink interface
mlxsw: core: Add devlink port splitter callbacks
MAINTAINERS | 8 +
drivers/infiniband/hw/mlx4/main.c | 7 +
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +-
drivers/net/ethernet/mellanox/mlx4/intf.c | 9 +
drivers/net/ethernet/mellanox/mlx4/main.c | 129 +++-
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/core.c | 56 +-
drivers/net/ethernet/mellanox/mlxsw/core.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/port.h | 2 +
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 238 ++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 8 +-
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 20 +
include/linux/mlx4/driver.h | 3 +
include/net/devlink.h | 156 +++++
include/uapi/linux/devlink.h | 73 ++
net/Kconfig | 7 +
net/core/Makefile | 1 +
net/core/devlink.c | 887 +++++++++++++++++++++++++
18 files changed, 1557 insertions(+), 59 deletions(-)
create mode 100644 include/net/devlink.h
create mode 100644 include/uapi/linux/devlink.h
create mode 100644 net/core/devlink.c
--
2.5.0
Tue, Feb 23, 2016 at 06:12:15AM CET, gospo@cumulusnetworks.com wrote:
On Mon, Feb 22, 2016 at 07:31:55PM +0100, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
There a is need for some userspace API that would allow to expose things
that are not directly related to any device class like net_device of
ib_device, but rather chip-wide/switch-ASIC-wide stuff.
Use cases:
1) get/set of port type (Ethernet/InfiniBand)
2) setting up port splitters - split port into multiple ones and squash again,
enables usage of splitter cable
3) setting up shared buffers - shared among multiple ports within
one chip (work in progress)
4) configuration of switch wide properties - resources division etc - This will
allow to pass configuration that is unacceptable to be passed as
a module option.
I'm generally a fan of use cases #3 and #4 (as we have previously
discussed), but I'm not sure I agree that the implementation for #2
right now.
I'm not sure I would like userspace to have control over whether or not
a port should be split or not when the hardware can be queried to
determine this.
I was thinking about this myself for a long time and initially it made
more sense to me the approach you are suggesting. But after lot of
thining, I saw a lot of issues, I changed my mind and now I believe that
splitter should be set by user.
when you set the splitter port, you have 2 or 4 ports created in
hardware. They looks exactly the same as unsplitted ports, they are only
wired up differently. There is a netdev created for every splitter port,
user would add it to bridge, bond, vlan, set routes, define static fdb
entry and hw would learn fdbs itself. Now when someone disconnects the
splitter cable, 2 things may happen:
1) yourway. Netdevs disappear, all configs and state info will disappear
with it.
2) myway. Netdevs stay, only link is down. This is the same behaviour is
if someone uplugs cable for unplitted port.
There are also similar issues before you plug the splitter cable. So it
makes sense to let user co configure this. He knows what he wants. 1)
does not look like correct behaviour to me, 2) does.
<snip>
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
Mon, Feb 22, 2016 at 11:29:06PM CET, roopa@cumulusnetworks.com wrote:
On 2/22/16, 10:31 AM, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
Introduce devlink infrastructure for drivers to register and expose to
userspace via generic Netlink interface.
There are two basic objects defined:
devlink - one instance for every "parent device", for example switch ASIC
devlink port - one instance for every physical port of the device.
Like i have expressed earlier, the only thing that bothers me here is that we are creating a new devlink object for switch port when there
is an existing netdev object.
Sometimes it is, sometimes it is not (IB). I bevelieve that there is a
need for a "handle" for a physical port, regardless what type it is.
The same instance exists all the time, even if you change the type or if
the netdev is not created yet (init phase)
Is there a chance that the drivers you are targeting can still create netdevs for physical ports ?
It would make things so much more consistent and simpler to manage without a cost of adding yet another interface.
So you see a problem in having devlink_port handle here? It is just an
index, very simple. But you would rather see something like:
myhost:~$ dl dev show
0: devlink0: bus pci dev 0000:01:00.0
myhost:~$ dl port show
ens4: type eth parent devlink0
ens5: type eth parent devlink0
?
There are 2 problems with that approach:
1) It's hard to use this for IB devices, they don't necessary have
netdev associated.
2) You have to have the netdevs created (know ifindex) in order to work
with that from userspace. But there are usecases, for example during
initialization that netdevs are not yet present and user needs to
specify port for configuration (that is usecase #4 I listed in the cover
letter)
But it is very easy to change dl userspace tool to be able to use netdev
names to specify ports when possible. Then you could do something like
for example:
myswitch:~$ sudo dl port split eth0 2
The port splitter support is needed at the netdev api too (ie rtnetlink). Most switchdev drivers that expose netdevs
would benefit from a native 'ip link' way to configure port splitting. This will be useful for nic drivers too.
Why would it be needed in rtnetlink too if it would be exported using
devlink? I don't get it. Sounds similar like if you would expose
wireless-specific stuff via rtnetlink in parallel to nl80211.
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 11:26:08
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this
will be integrated into udev/systemd for stable device names. While one
can build wrapper code around devlink to have stable devlink ports, I
don't see a reason to include kernel code which actually has more
problems than the sysfs approach. This harms admins to use those devices
and will additionally require user space to write boiler plate code.
Thanks,
Hannes
Tue, Feb 23, 2016 at 12:26:00PM CET, hannes@stressinduktion.org wrote:
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this will be
integrated into udev/systemd for stable device names. While one can build
wrapper code around devlink to have stable devlink ports, I don't see a
reason to include kernel code which actually has more problems than the sysfs
approach. This harms admins to use those devices and will additionally
require user space to write boiler plate code.
Sysfs is not the place to do this things. It was already discussed here
multiple times. There was and attempt to use configfs, which was also
refused. Netlink is the only place to go. For multiple reasons,
including well defined api and behaviour, notifications, etc.
I think it is quite trivial to teach udev to name devlinkX devices
according to pci address (or any other address). That's all what is
needed here. I don't understand your concerns.
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 13:28:12
On 23.02.2016 13:21, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 12:26:00PM CET, hannes@stressinduktion.org wrote:
quoted
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this will be
integrated into udev/systemd for stable device names. While one can build
wrapper code around devlink to have stable devlink ports, I don't see a
reason to include kernel code which actually has more problems than the sysfs
approach. This harms admins to use those devices and will additionally
require user space to write boiler plate code.
Sysfs is not the place to do this things. It was already discussed here
multiple times. There was and attempt to use configfs, which was also
refused. Netlink is the only place to go. For multiple reasons,
including well defined api and behaviour, notifications, etc.
I am not against netlink at all. My fear with this interface is simply:
1) we introduce another ifindex/name like identifiers. It took a long
time until this stuff finally worked fine with linux. It needs
persistent storage in userspace being applied at boot time. Why this
complications for this probably lesser often used interface?
2) The actual devlink attributes get managed from inside devlink and not
the driver. So driver need to modify devlink.c/devlink.h in core to add
new attributes.
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
For 2) I don't really know what drivers want, not sure if it is easier
to add some small helper functions to add sysfs attributes to kobjects
without necessarily holding a net_device. Thus mellanox drivers can use
it and I am not sure how many other networking cards allow switching
ports between ib and eth type. Port splitting only happens for
interfaces which already have a net_device, no?
I think it is quite trivial to teach udev to name devlinkX devices
according to pci address (or any other address). That's all what is
needed here. I don't understand your concerns.
I don't think that this interface needs the same complexity as network
interfaces.
I am not sure, but one of the initial problems was that this information
should already be there before the driver actually gets loaded, no?
These changes don't solve this problem either?
Thanks,
Hannes
Tue, Feb 23, 2016 at 02:28:05PM CET, hannes@stressinduktion.org wrote:
On 23.02.2016 13:21, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 12:26:00PM CET, hannes@stressinduktion.org wrote:
quoted
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this will be
integrated into udev/systemd for stable device names. While one can build
wrapper code around devlink to have stable devlink ports, I don't see a
reason to include kernel code which actually has more problems than the sysfs
approach. This harms admins to use those devices and will additionally
require user space to write boiler plate code.
Sysfs is not the place to do this things. It was already discussed here
multiple times. There was and attempt to use configfs, which was also
refused. Netlink is the only place to go. For multiple reasons,
including well defined api and behaviour, notifications, etc.
I am not against netlink at all. My fear with this interface is simply:
1) we introduce another ifindex/name like identifiers. It took a long time
until this stuff finally worked fine with linux. It needs persistent storage
in userspace being applied at boot time. Why this complications for this
probably lesser often used interface?
Lesser often where? On switches, this interface will be used all the
time. You have to have some handle to manipulate the chip-wide stuff. In
our case it is devlink0. Similar to wireless, they have phy0. I believe
it is completely legit.
2) The actual devlink attributes get managed from inside devlink and not the
driver. So driver need to modify devlink.c/devlink.h in core to add new
attributes.
That is exactly the point! Vendors cannot add their own specific crap,
they have to do things in generic way and extend devlink iface
accordingly. That's what we do now with ASIC shared buffer configuration
via devlink for example (in addition to port type and splitter).
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
For 2) I don't really know what drivers want, not sure if it is easier to add
some small helper functions to add sysfs attributes to kobjects without
necessarily holding a net_device. Thus mellanox drivers can use it and I am
not sure how many other networking cards allow switching ports between ib and
eth type. Port splitting only happens for interfaces which already have a
net_device, no?
Not necessarily. IB ports that has no net_device could be split as well.
Hannes, again, sysfs approach was refused couple of times in past for this
purpose. Please leave sysfs alone.
quoted
I think it is quite trivial to teach udev to name devlinkX devices
according to pci address (or any other address). That's all what is
needed here. I don't understand your concerns.
I don't think that this interface needs the same complexity as network
interfaces.
Again, it aligns nicely with what they to in wireless in nl80211
interface. I don't see any complexity.
I am not sure, but one of the initial problems was that this information
should already be there before the driver actually gets loaded, no? These
changes don't solve this problem either?
This is planned to be implemented in near future. Basically there would
be possible to use DEVLINK_CMD_NEW to add devlink iface for specific device
even before the driver gets loaded to serve as a place holder to set values
of some predefined set of options. Once the driver registers, it can read
those and act accordingly. For example, we need that to set "profile" of
our asic. This is a substitute to module options which are completely
inappropriate for this usecase.
From: Andy Gospodarek <hidden> Date: 2016-02-23 14:34:23
On Tue, Feb 23, 2016 at 08:32:03AM +0100, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 06:12:15AM CET, gospo@cumulusnetworks.com wrote:
quoted
On Mon, Feb 22, 2016 at 07:31:55PM +0100, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
There a is need for some userspace API that would allow to expose things
that are not directly related to any device class like net_device of
ib_device, but rather chip-wide/switch-ASIC-wide stuff.
Use cases:
1) get/set of port type (Ethernet/InfiniBand)
2) setting up port splitters - split port into multiple ones and squash again,
enables usage of splitter cable
3) setting up shared buffers - shared among multiple ports within
one chip (work in progress)
4) configuration of switch wide properties - resources division etc - This will
allow to pass configuration that is unacceptable to be passed as
a module option.
I'm generally a fan of use cases #3 and #4 (as we have previously
discussed), but I'm not sure I agree that the implementation for #2
right now.
I'm not sure I would like userspace to have control over whether or not
a port should be split or not when the hardware can be queried to
determine this.
I was thinking about this myself for a long time and initially it made
more sense to me the approach you are suggesting. But after lot of
thining, I saw a lot of issues, I changed my mind and now I believe that
splitter should be set by user.
It seems like now we have convinced each other to change our initial
positions.
As you recall I was previously in favor of handling mapping of
switch-ports to physical ports in netlink and performing breakout
configuration that way as well. Having Mellanox hardware perform in the
way you describe would certainly not prevent a solution to come later
that did automatic detection and reconfiguration, so if the concensus is
that handling breakouts in this way is OK, you are not going to see
significant argument from me. (I checked back with a few others on what
is done in the closed-source world and some manual intervention is often
needed there as well, so let's stick with what you have).
when you set the splitter port, you have 2 or 4 ports created in
hardware. They looks exactly the same as unsplitted ports, they are only
wired up differently. There is a netdev created for every splitter port,
user would add it to bridge, bond, vlan, set routes, define static fdb
entry and hw would learn fdbs itself. Now when someone disconnects the
splitter cable, 2 things may happen:
1) yourway. Netdevs disappear, all configs and state info will disappear
with it.
2) myway. Netdevs stay, only link is down. This is the same behaviour is
if someone uplugs cable for unplitted port.
There are also similar issues before you plug the splitter cable. So it
makes sense to let user co configure this. He knows what he wants. 1)
does not look like correct behaviour to me, 2) does.
The bottom line here is that for the switch offload case most chips need
to be reconfigured significantly to function in a way that makes moving
from 1x100GbE > 2x50GbE, so whether one use case is better or worse
doesn't really matter that much.
<snip>
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Tue, Feb 23, 2016 at 03:34:19PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Wait. The only thing which will be renamed by udev is "devlink0". The
suffixes "/1" and "/2" are direct indexes as used inside the driver.
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 15:16:19
On 23.02.2016 15:26, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 02:28:05PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 13:21, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 12:26:00PM CET, hannes@stressinduktion.org wrote:
quoted
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this will be
integrated into udev/systemd for stable device names. While one can build
wrapper code around devlink to have stable devlink ports, I don't see a
reason to include kernel code which actually has more problems than the sysfs
approach. This harms admins to use those devices and will additionally
require user space to write boiler plate code.
Sysfs is not the place to do this things. It was already discussed here
multiple times. There was and attempt to use configfs, which was also
refused. Netlink is the only place to go. For multiple reasons,
including well defined api and behaviour, notifications, etc.
I am not against netlink at all. My fear with this interface is simply:
1) we introduce another ifindex/name like identifiers. It took a long time
until this stuff finally worked fine with linux. It needs persistent storage
in userspace being applied at boot time. Why this complications for this
probably lesser often used interface?
Lesser often where? On switches, this interface will be used all the
time. You have to have some handle to manipulate the chip-wide stuff. In
our case it is devlink0. Similar to wireless, they have phy0. I believe
it is completely legit.
Lesser often as you e.g. refer to the interface name in nftables or
netfilter, or in setsockopt etc. They are not being referenced as often
as interface names, so the question is: do they need nice looking names?
quoted
2) The actual devlink attributes get managed from inside devlink and not the
driver. So driver need to modify devlink.c/devlink.h in core to add new
attributes.
That is exactly the point! Vendors cannot add their own specific crap,
they have to do things in generic way and extend devlink iface
accordingly. That's what we do now with ASIC shared buffer configuration
via devlink for example (in addition to port type and splitter).
If this is part of the design, okay.
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
> ?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too,
and adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
quoted
For 2) I don't really know what drivers want, not sure if it is easier to add
some small helper functions to add sysfs attributes to kobjects without
necessarily holding a net_device. Thus mellanox drivers can use it and I am
not sure how many other networking cards allow switching ports between ib and
eth type. Port splitting only happens for interfaces which already have a
net_device, no?
Not necessarily. IB ports that has no net_device could be split as well.
Hannes, again, sysfs approach was refused couple of times in past for this
purpose. Please leave sysfs alone.
Sorry, I couldn't find the references or the reasons.
Actually the sysfs knob is in the kernel right now.
quoted
quoted
I think it is quite trivial to teach udev to name devlinkX devices
according to pci address (or any other address). That's all what is
needed here. I don't understand your concerns.
I don't think that this interface needs the same complexity as network
interfaces.
Again, it aligns nicely with what they to in wireless in nl80211
interface. I don't see any complexity.
The interface names must be kept stable from user space.
Sorry to be such a pedantic ass*** here, but isn't nl80211 the other way
around? You have an interface as an anchor and can use that to discover
the other interfaces using the same phy?
I have no experience here how those get managed by wpa_supplicant, but
at least as a user, you specific interfaces and not phys.
I look more into this and how they deal with that, thanks.
quoted
I am not sure, but one of the initial problems was that this information
should already be there before the driver actually gets loaded, no? These
changes don't solve this problem either?
This is planned to be implemented in near future. Basically there would
be possible to use DEVLINK_CMD_NEW to add devlink iface for specific device
even before the driver gets loaded to serve as a place holder to set values
of some predefined set of options. Once the driver registers, it can read
those and act accordingly. For example, we need that to set "profile" of
our asic. This is a substitute to module options which are completely
inappropriate for this usecase.
From: Andy Gospodarek <hidden> Date: 2016-02-23 15:20:13
On Tue, Feb 23, 2016 at 03:26:27PM +0100, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 02:28:05PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 13:21, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 12:26:00PM CET, hannes@stressinduktion.org wrote:
quoted
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this will be
integrated into udev/systemd for stable device names. While one can build
wrapper code around devlink to have stable devlink ports, I don't see a
reason to include kernel code which actually has more problems than the sysfs
approach. This harms admins to use those devices and will additionally
require user space to write boiler plate code.
Sysfs is not the place to do this things. It was already discussed here
multiple times. There was and attempt to use configfs, which was also
refused. Netlink is the only place to go. For multiple reasons,
including well defined api and behaviour, notifications, etc.
I am not against netlink at all. My fear with this interface is simply:
1) we introduce another ifindex/name like identifiers. It took a long time
until this stuff finally worked fine with linux. It needs persistent storage
in userspace being applied at boot time. Why this complications for this
probably lesser often used interface?
Lesser often where? On switches, this interface will be used all the
time. You have to have some handle to manipulate the chip-wide stuff. In
our case it is devlink0. Similar to wireless, they have phy0. I believe
it is completely legit.
quoted
2) The actual devlink attributes get managed from inside devlink and not the
driver. So driver need to modify devlink.c/devlink.h in core to add new
attributes.
That is exactly the point! Vendors cannot add their own specific crap,
they have to do things in generic way and extend devlink iface
accordingly. That's what we do now with ASIC shared buffer configuration
via devlink for example (in addition to port type and splitter).
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
quoted
For 2) I don't really know what drivers want, not sure if it is easier to add
some small helper functions to add sysfs attributes to kobjects without
necessarily holding a net_device. Thus mellanox drivers can use it and I am
not sure how many other networking cards allow switching ports between ib and
eth type. Port splitting only happens for interfaces which already have a
net_device, no?
Not necessarily. IB ports that has no net_device could be split as well.
Hannes, again, sysfs approach was refused couple of times in past for this
purpose. Please leave sysfs alone.
quoted
quoted
I think it is quite trivial to teach udev to name devlinkX devices
according to pci address (or any other address). That's all what is
needed here. I don't understand your concerns.
I don't think that this interface needs the same complexity as network
interfaces.
Again, it aligns nicely with what they to in wireless in nl80211
interface. I don't see any complexity.
quoted
I am not sure, but one of the initial problems was that this information
should already be there before the driver actually gets loaded, no? These
changes don't solve this problem either?
This is planned to be implemented in near future. Basically there would
be possible to use DEVLINK_CMD_NEW to add devlink iface for specific device
even before the driver gets loaded to serve as a place holder to set values
s/driver/network driver/ right?
of some predefined set of options. Once the driver registers, it can read
those and act accordingly. For example, we need that to set "profile" of
our asic. This is a substitute to module options which are completely
inappropriate for this usecase.
FWIW, I DO like the idea that the PCI driver contains this information
and netdev creation in the network driver depends on this mapping. We
see these issues on a regular basis and while have solved it other
ways (rtnl_link_ops and genl which is why I like a cross-vendor way to
do it like this).
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
Tue, Feb 23, 2016 at 04:20:09PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
I am not sure, but one of the initial problems was that this information
should already be there before the driver actually gets loaded, no? These
changes don't solve this problem either?
This is planned to be implemented in near future. Basically there would
be possible to use DEVLINK_CMD_NEW to add devlink iface for specific device
even before the driver gets loaded to serve as a place holder to set values
s/driver/network driver/ right?
right.
quoted
of some predefined set of options. Once the driver registers, it can read
those and act accordingly. For example, we need that to set "profile" of
our asic. This is a substitute to module options which are completely
inappropriate for this usecase.
FWIW, I DO like the idea that the PCI driver contains this information
and netdev creation in the network driver depends on this mapping. We
see these issues on a regular basis and while have solved it other
ways (rtnl_link_ops and genl which is why I like a cross-vendor way to
do it like this).
From: Andy Gospodarek <hidden> Date: 2016-02-23 15:55:32
On Tue, Feb 23, 2016 at 03:45:51PM +0100, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 03:34:19PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Wait. The only thing which will be renamed by udev is "devlink0". The
suffixes "/1" and "/2" are direct indexes as used inside the driver.
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
So I think I had invisioned a slightly different workflow than what you
just described.
- Load PCI driver
- Setup devlink attributes for your hardware
- Create netdevs in network driver based on those attributes
You don't need a netdev to reference any of the devlink specific
parameters do you?
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 15:57:24
On 23.02.2016 16:30, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
I argue for this stable topology identifier to be the default.
Especially if you add device info before the actual module is loaded
(this is during initramfs, when udev cannot rename devlink names to
stable ones), a user has to deal with pre-devlink-ids before rename and
after. Do you have plans how to address that?
Current initramfs for stable interface names uses EUI48 based mac
addresses most of the time and udev runs then after the pivot_root.
The devlink names can easily be aliases in user space.
Bye,
Hannes
Tue, Feb 23, 2016 at 04:55:28PM CET, gospo@cumulusnetworks.com wrote:
On Tue, Feb 23, 2016 at 03:45:51PM +0100, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 03:34:19PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Wait. The only thing which will be renamed by udev is "devlink0". The
suffixes "/1" and "/2" are direct indexes as used inside the driver.
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
So I think I had invisioned a slightly different workflow than what you
just described.
- Load PCI driver
- Setup devlink attributes for your hardware
- Create netdevs in network driver based on those attributes
You don't need a netdev to reference any of the devlink specific
parameters do you?
No, I have a devlink handle and a devlink port index. That is enough.
Tue, Feb 23, 2016 at 04:57:17PM CET, hannes@stressinduktion.org wrote:
On 23.02.2016 16:30, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
I argue for this stable topology identifier to be the default. Especially if
you add device info before the actual module is loaded (this is during
initramfs, when udev cannot rename devlink names to stable ones), a user has
to deal with pre-devlink-ids before rename and after. Do you have plans how
to address that?
You can still access devlink using pci_addr using dl. I don't see a
problem.
Current initramfs for stable interface names uses EUI48 based mac addresses
most of the time and udev runs then after the pivot_root.
The devlink names can easily be aliases in user space.
I don't want to store them anywhere. I just use "dl" tool and pass the
name there.
From: Andy Gospodarek <hidden> Date: 2016-02-23 16:19:41
On Tue, Feb 23, 2016 at 05:01:31PM +0100, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 04:55:28PM CET, gospo@cumulusnetworks.com wrote:
quoted
On Tue, Feb 23, 2016 at 03:45:51PM +0100, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 03:34:19PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Wait. The only thing which will be renamed by udev is "devlink0". The
suffixes "/1" and "/2" are direct indexes as used inside the driver.
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
So I think I had invisioned a slightly different workflow than what you
just described.
- Load PCI driver
- Setup devlink attributes for your hardware
- Create netdevs in network driver based on those attributes
You don't need a netdev to reference any of the devlink specific
parameters do you?
No, I have a devlink handle and a devlink port index. That is enough.
That's what I read as well and why I wondered why you said this:
quoted
quoted
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
It just doesn't seem like referencing the netdev when performing set
operations was needed.
I see no issue having it appear a dump action after the fact, but
it seems like devlink[unit number]/[port number] would be enough to
reference the proper hardware mapping.
Tue, Feb 23, 2016 at 05:19:37PM CET, gospo@cumulusnetworks.com wrote:
On Tue, Feb 23, 2016 at 05:01:31PM +0100, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:55:28PM CET, gospo@cumulusnetworks.com wrote:
quoted
On Tue, Feb 23, 2016 at 03:45:51PM +0100, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 03:34:19PM CET, gospo@cumulusnetworks.com wrote:
<snip>
quoted
quoted
quoted
quoted
myhost:~$ dl port show
devlink0/1: type eth netdev ens4
^^^^^^^^^^^
quoted
devlink0/2: type ib ibdev mlx4_0
^^^^^^^^^^^^
I think my only other question about this implementation is whether or
not one would really want to have the true netdev/ibdev names mapped
here.
Would be as reasonable to simply specify the type (and there may be more
types within ethernet that could be useful in multi-chip configurations)
and then let normal infrastructure that exists today figure out how to
map the names for the netdevs to the devices?
What normal infrastructure you have in mind? There is no info about
devlink port mapping to netdev/ibdev anywhere. Only here. I might be
missing something but I fail to see what's wrong with it.
I was simply wondering out loud if we _really_ wanted to name netdevs
this way. I was suggesting that output could be like this:
myhost:~$ dl port show
devlink0/1: type eth
devlink0/2: type ib
mnd that udev/systemd/biosdevname/etc would take care of naming the
device whataever it wanted. This appears to be essentially the same
concern Hannes has.
Wait. The only thing which will be renamed by udev is "devlink0". The
suffixes "/1" and "/2" are direct indexes as used inside the driver.
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
So I think I had invisioned a slightly different workflow than what you
just described.
- Load PCI driver
- Setup devlink attributes for your hardware
- Create netdevs in network driver based on those attributes
You don't need a netdev to reference any of the devlink specific
parameters do you?
No, I have a devlink handle and a devlink port index. That is enough.
That's what I read as well and why I wondered why you said this:
quoted
quoted
quoted
And you need some link to netdev in case netdev exists - therefore
"netdev ens4" attribute is there. There's no other way to get the
mapping of "devlink0/1" to "ens4" anywhere else.
It just doesn't seem like referencing the netdev when performing set
operations was needed.
I see no issue having it appear a dump action after the fact, but
it seems like devlink[unit number]/[port number] would be enough to
reference the proper hardware mapping.
I said nothing about needing netdev for set operation Andy. It is there
in a get list as an attribute for your convenience so you know what
netdev that specific port relates to. That's it.
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 16:45:16
On 23.02.2016 17:04, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 04:57:17PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 16:30, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
I argue for this stable topology identifier to be the default. Especially if
you add device info before the actual module is loaded (this is during
initramfs, when udev cannot rename devlink names to stable ones), a user has
to deal with pre-devlink-ids before rename and after. Do you have plans how
to address that?
You can still access devlink using pci_addr using dl. I don't see a
problem.
I don't really see a reason why the devlink indexes/names exist inside
the kernel instead of a stable topology identifier. They confuse users
and add more unnecessary code to the kernel. Shells have environment
variables for that. ;) This is a low-level kernel setting tool IMHO.
I just see the problem that users use the devlink* names and we get
reports because stuff breaks because they don't use the stable
identifiers. That is all.
quoted
Current initramfs for stable interface names uses EUI48 based mac addresses
most of the time and udev runs then after the pivot_root.
The devlink names can easily be aliases in user space.
I don't want to store them anywhere. I just use "dl" tool and pass the
name there.
Tue, Feb 23, 2016 at 05:45:07PM CET, hannes@stressinduktion.org wrote:
On 23.02.2016 17:04, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:57:17PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 16:30, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
I argue for this stable topology identifier to be the default. Especially if
you add device info before the actual module is loaded (this is during
initramfs, when udev cannot rename devlink names to stable ones), a user has
to deal with pre-devlink-ids before rename and after. Do you have plans how
to address that?
You can still access devlink using pci_addr using dl. I don't see a
problem.
I don't really see a reason why the devlink indexes/names exist inside the
kernel instead of a stable topology identifier. They confuse users and add
more unnecessary code to the kernel. Shells have environment variables for
that. ;) This is a low-level kernel setting tool IMHO.
I just see the problem that users use the devlink* names and we get reports
because stuff breaks because they don't use the stable identifiers. That is
all.
you can have stable devlink name using udev. For pre-udev usage, you can
use pci address directly. I don't like to use pci address for every
dl command. I would like to have some more convenient name handle -
devlink name.
quoted
quoted
Current initramfs for stable interface names uses EUI48 based mac addresses
most of the time and udev runs then after the pivot_root.
The devlink names can easily be aliases in user space.
I don't want to store them anywhere. I just use "dl" tool and pass the
name there.
Sorry?
you said - "The devlink names can easily be aliases in user space." -
where do you want to store them?
From: Hannes Frederic Sowa <hidden> Date: 2016-02-23 17:07:16
On 23.02.2016 17:55, Jiri Pirko wrote:
Tue, Feb 23, 2016 at 05:45:07PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 17:04, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:57:17PM CET, hannes@stressinduktion.org wrote:
quoted
On 23.02.2016 16:30, Jiri Pirko wrote:
quoted
Tue, Feb 23, 2016 at 04:16:11PM CET, hannes@stressinduktion.org wrote:
<snip>
quoted
quoted
quoted
1) is easily solvable, just drop the ifindex style attributes and always
force the user to enter the bus and bus-topology id.
But why? Use can easily get that info and map it to devlink index. It
aligns with nl80211 iface.
Do you really want to do commands like:
myhost:~$ dl dev show pci_0000:01:00.0
?
Yes, exactly I would. I would put them into a boot-up script based on my
system configuration and can be sure it will work the next boot, too, and
adapt them when I replace the hardware or do some configuration changes.
I think sysadmins or scripts are the primary users of this interface not
kernel developers which switch their settings around all the time, no?
I can easily add this to the userspace tool to accept "pci_0000:01:00.0"
format and to map it internally to devlink index. No problem.
I argue for this stable topology identifier to be the default. Especially if
you add device info before the actual module is loaded (this is during
initramfs, when udev cannot rename devlink names to stable ones), a user has
to deal with pre-devlink-ids before rename and after. Do you have plans how
to address that?
You can still access devlink using pci_addr using dl. I don't see a
problem.
I don't really see a reason why the devlink indexes/names exist inside the
kernel instead of a stable topology identifier. They confuse users and add
more unnecessary code to the kernel. Shells have environment variables for
that. ;) This is a low-level kernel setting tool IMHO.
I just see the problem that users use the devlink* names and we get reports
because stuff breaks because they don't use the stable identifiers. That is
all.
you can have stable devlink name using udev. For pre-udev usage, you can
use pci address directly. I don't like to use pci address for every
dl command. I would like to have some more convenient name handle -
devlink name.
Okay then - I was not sure if this low-level kernel command line tool
deserves changes to the whole booting infrastructure to make it safe to use.
quoted
quoted
quoted
Current initramfs for stable interface names uses EUI48 based mac addresses
most of the time and udev runs then after the pivot_root.
The devlink names can easily be aliases in user space.
I don't want to store them anywhere. I just use "dl" tool and pass the
name there.
Sorry?
you said - "The devlink names can easily be aliases in user space." -
where do you want to store them?
You can easily store them in your shell environment or add a feature to
the dl command to read aliases from a file (will be much more simple
than walking all devlinks and checking for a match if you implement it
the reverse way).
Note, if you want to have kernel provided names, they need to be stored
in user space anyway because of udev renaming and the persisting database.
Bye,
Hannes
From: Stephen Hemminger <stephen@networkplumber.org> Date: 2016-02-23 17:32:07
On Tue, 23 Feb 2016 12:26:00 +0100
Hannes Frederic Sowa [off-list ref] wrote:
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this
will be integrated into udev/systemd for stable device names. While one
can build wrapper code around devlink to have stable devlink ports, I
don't see a reason to include kernel code which actually has more
problems than the sysfs approach. This harms admins to use those devices
and will additionally require user space to write boiler plate code.
Thanks,
Hannes
I appreciate that you need to have a lighterweight model for
network devices. But have to agree with Hannes.
This code breaks the model expected by applications like Quagga, SNMP
and lots of other legacy code. Is this really going to work with the
legacy Linux model.
It was already discussed here multiple times. There was and attempt
to use configfs, which was also refused. Netlink is the only place
to go. For multiple reasons, including well defined api and
behaviour, notifications, etc.
+ * An overall lock guarding every operation comming from userspace.
+ * If also guards devlink devices list and it is taken when
+ * driver registers/unregisters it.
Tue, Feb 23, 2016 at 06:31:39PM CET, stephen@networkplumber.org wrote:
On Tue, 23 Feb 2016 12:26:00 +0100
Hannes Frederic Sowa [off-list ref] wrote:
quoted
Hi Jiri,
On 22.02.2016 19:31, Jiri Pirko wrote:
quoted
From: Jiri Pirko <redacted>
So far, there has been an mlx4-specific sysfs file allowing user to
change port type to either Ethernet of InfiniBand. This is very
inconvenient.
Again, I want to express my concerns regarding all of this until this
will be integrated into udev/systemd for stable device names. While one
can build wrapper code around devlink to have stable devlink ports, I
don't see a reason to include kernel code which actually has more
problems than the sysfs approach. This harms admins to use those devices
and will additionally require user space to write boiler plate code.
Thanks,
Hannes
I appreciate that you need to have a lighterweight model for
network devices. But have to agree with Hannes.
No, I don't need to have lighterweight model for network device. This
patch does nothing like that.
This code breaks the model expected by applications like Quagga, SNMP
and lots of other legacy code. Is this really going to work with the
legacy Linux model.
No, this patch does not break anything. The original netdev still stay.
Wed, Feb 24, 2016 at 08:02:32AM CET, Yuval.Mintz@qlogic.com wrote:
quoted
+ * An overall lock guarding every operation comming from userspace.
+ * If also guards devlink devices list and it is taken when
+ * driver registers/unregisters it.
Why is this PORT_NEW? Shouldn't it be PORT_SET?
Also, curly bracers are repeatedly on last line of function [if this file].
Is this by design?
SET is only for userspace->kernel messages. NEW is for reporting events
back. This is consistend with rest of the netlink messages out there,
including rtnl and nl80211