| Message ID | 20180612073420.45736-3-hare@suse.de (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
On Tue, Jun 12, 2018 at 10:36 AM Hannes Reinecke <hare@suse.de> wrote: > > Allowing several interface (separated by whitespaces) to be specified > when writing to the /sys/module/rdma_rxe/parameters/add sysfs attribute. > > Signed-off-by: Hannes Reinecke <hare@suse.com> > --- > drivers/infiniband/sw/rxe/rxe_sysfs.c | 67 +++++++++++++++++++---------------- > 1 file changed, 37 insertions(+), 30 deletions(-) > I don't if I like complicating the interface without a real need. If this is a real issue for you I suggest that you solve it by adding to the script in librxe what do you say? Moni -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 12, 2018 at 09:34:20AM +0200, Hannes Reinecke wrote: > Allowing several interface (separated by whitespaces) to be specified > when writing to the /sys/module/rdma_rxe/parameters/add sysfs attribute. Nope, we want this sysfs interface to go away, not become more complicated. Add needs to be done over netlink, not via module parameters or sysfs. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 12 Jun 2018 08:44:59 -0600 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Tue, Jun 12, 2018 at 09:34:20AM +0200, Hannes Reinecke wrote: > > Allowing several interface (separated by whitespaces) to be > > specified when writing to the /sys/module/rdma_rxe/parameters/add > > sysfs attribute. > > Nope, we want this sysfs interface to go away, not become more > complicated. > > Add needs to be done over netlink, not via module parameters or sysfs. > Good point. (I hate the sysfs interface anyway.) Will be looking into it. Cheers, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Good point. > (I hate the sysfs interface anyway.) > Will be looking into it. > Sure, go ahead but remember that bonding driver still uses the sysfs interface and probably for a good reason. So, consider this when you make plans. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 13 Jun 2018 09:16:56 +0300 Moni Shoua <monis@mellanox.com> wrote: > > > > Good point. > > (I hate the sysfs interface anyway.) > > Will be looking into it. > > > Sure, go ahead but remember that bonding driver still uses the sysfs > interface and probably for a good reason. So, consider this when you > make plans. > ??? Not sure what this got to do with it... Anyway, current plan is to hook into iproute2 'rdma' tool; we already have: ./rdma/rdma link show 1/1: rxe0/1: state ACTIVE physical_state LINK_UP netdev eth1 2/1: rxe1/1: state ACTIVE physical_state LINK_UP netdev eth2 Plan is to add an interface rdma link add <dev/port> netdev <ifname> rdma link del <dev/port> netdev <ifname> to cover the functionality of the current sysfs interface. D'accord? Cheers, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 10:03 AM Hannes Reinecke <hare@suse.de> wrote: > > On Wed, 13 Jun 2018 09:16:56 +0300 > Moni Shoua <monis@mellanox.com> wrote: > > > > > > > Good point. > > > (I hate the sysfs interface anyway.) > > > Will be looking into it. > > > > > Sure, go ahead but remember that bonding driver still uses the sysfs > > interface and probably for a good reason. So, consider this when you > > make plans. > > > > ??? > Not sure what this got to do with it... > bonding module uses sysfs interface to add slaves the discussion here is about the sysfs interace in rxe to add device the relevance looks pretty clear to me. Anyway, your plan is to add an interface and not replace it so as I said before, go ahead > Anyway, current plan is to hook into iproute2 'rdma' tool; we already > have: > > ./rdma/rdma link show > 1/1: rxe0/1: state ACTIVE physical_state LINK_UP netdev eth1 > 2/1: rxe1/1: state ACTIVE physical_state LINK_UP netdev eth2 > > Plan is to add an interface > > rdma link add <dev/port> netdev <ifname> > rdma link del <dev/port> netdev <ifname> > > to cover the functionality of the current sysfs interface. > D'accord? > > Cheers, > > Hannes > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 09:03:16AM +0200, Hannes Reinecke wrote: > On Wed, 13 Jun 2018 09:16:56 +0300 > Moni Shoua <monis@mellanox.com> wrote: > > > > > > > Good point. > > > (I hate the sysfs interface anyway.) > > > Will be looking into it. > > > > > Sure, go ahead but remember that bonding driver still uses the sysfs > > interface and probably for a good reason. So, consider this when you > > make plans. > > > > ??? > Not sure what this got to do with it... > > Anyway, current plan is to hook into iproute2 'rdma' tool; we already > have: > > ./rdma/rdma link show > 1/1: rxe0/1: state ACTIVE physical_state LINK_UP netdev eth1 > 2/1: rxe1/1: state ACTIVE physical_state LINK_UP netdev eth2 > > Plan is to add an interface > > rdma link add <dev/port> netdev <ifname> > rdma link del <dev/port> netdev <ifname> > > to cover the functionality of the current sysfs interface. > D'accord? > +1, please don't forget ability to provide netdev_index in addition to netdev name. It will be helpful for automatic tools. Thanks > Cheers, > > Hannes > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 09:03:16AM +0200, Hannes Reinecke wrote: > On Wed, 13 Jun 2018 09:16:56 +0300 > Moni Shoua <monis@mellanox.com> wrote: > > > > > > > Good point. > > > (I hate the sysfs interface anyway.) > > > Will be looking into it. > > > > > Sure, go ahead but remember that bonding driver still uses the sysfs > > interface and probably for a good reason. So, consider this when you > > make plans. > > > > ??? > Not sure what this got to do with it... > > Anyway, current plan is to hook into iproute2 'rdma' tool; we already > have: > > ./rdma/rdma link show > 1/1: rxe0/1: state ACTIVE physical_state LINK_UP netdev eth1 > 2/1: rxe1/1: state ACTIVE physical_state LINK_UP netdev eth2 > > Plan is to add an interface > > rdma link add <dev/port> netdev <ifname> > rdma link del <dev/port> netdev <ifname> It should be modeled after 'ip link add' rdma link add link NET_DEV # The physical ethernet device to create rxe on name NAME # Name of the RDMA device, port is always 1 index INDEX # RDMA device INDEX # (optional type rxe # Specify this is a RXE device (vs SIW someday) <any future rxe specific options> Maybe call 'link' 'netdev' for rdma-tool, but the other options are needed > to cover the functionality of the current sysfs interface. > D'accord? Yes please, Bernard has wanted this for soft iwarp as well. I would then want to deprecate the module parmater very quickly. The sysfs can stick around, but maybe could print a deprecation warning. Also, 'type rxe' should trigger module auto-loading of the rxe module. Thanks, Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c index bb54992e3ff2..eaf76f7c7472 100644 --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c @@ -71,9 +71,9 @@ static void rxe_set_port_state(struct net_device *ndev) static int rxe_param_set_add(const char *val, const struct kernel_param *kp) { - int len; int err = 0; - char intf[32]; + char **intf_argv = NULL; + int intf_argc, i; struct net_device *ndev = NULL; struct rxe_dev *rxe; @@ -82,38 +82,45 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp) err = -EINVAL; goto err; } - len = sanitize_arg(val, intf, sizeof(intf)); - if (!len) { - pr_err("add: invalid interface name\n"); - err = -EINVAL; - goto err; - } - - ndev = dev_get_by_name(&init_net, intf); - if (!ndev) { - pr_err("interface %s not found\n", intf); - err = -EINVAL; - goto err; + intf_argv = argv_split(GFP_KERNEL, val, &intf_argc); + for (i = 0; i < intf_argc; i++) { + ndev = dev_get_by_name(&init_net, intf_argv[i]); + if (!ndev) { + pr_err("interface %s not found\n", intf_argv[i]); + err = -EINVAL; + goto err; + } + + if (net_to_rxe(ndev)) { + pr_err("already configured on %s\n", intf_argv[i]); + err = -EINVAL; + goto err; + } + + rxe = rxe_net_add(ndev); + if (!rxe) { + pr_err("failed to add %s\n", intf_argv[i]); + err = -EINVAL; + goto err; + } + + rxe_set_port_state(ndev); + pr_info("added %s to %s\n", rxe->ib_dev.name, intf_argv[i]); } - - if (net_to_rxe(ndev)) { - pr_err("already configured on %s\n", intf); - err = -EINVAL; - goto err; - } - - rxe = rxe_net_add(ndev); - if (!rxe) { - pr_err("failed to add %s\n", intf); - err = -EINVAL; - goto err; - } - - rxe_set_port_state(ndev); - pr_info("added %s to %s\n", rxe->ib_dev.name, intf); err: if (ndev) dev_put(ndev); + + if (intf_argv) { + for (i = 0; i < intf_argc; i++) { + rxe = get_rxe_by_name(intf_argv[i]); + if (rxe) { + list_del(&rxe->list); + rxe_remove(rxe); + } + } + argv_free(intf_argv); + } return err; }
Allowing several interface (separated by whitespaces) to be specified when writing to the /sys/module/rdma_rxe/parameters/add sysfs attribute. Signed-off-by: Hannes Reinecke <hare@suse.com> --- drivers/infiniband/sw/rxe/rxe_sysfs.c | 67 +++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 30 deletions(-)