On Fri, Sep 14, 2018 at 01:11:23PM -0500, Steve Wise wrote: > > > On 9/13/2018 2:01 PM, Steve Wise wrote: > > > > > > On 9/13/2018 1:54 PM, Jason Gunthorpe wrote: > >> On Thu, Sep 13, 2018 at 01:03:42PM -0500, Steve Wise wrote: > >>> > >>> > >>> On 9/13/2018 12:19 PM, Jason Gunthorpe wrote: > >>>> On Thu, Sep 13, 2018 at 12:05:27PM -0500, Steve Wise wrote: > >>>>> > >>>>> > >>>>> On 7/25/2018 2:05 PM, Jason Gunthorpe wrote: > >>>>>> On Mon, Jul 16, 2018 at 02:07:57PM -0500, Steve Wise wrote: > >>>>>>> > >>>>>>> > >>>>>>> On 3/1/2018 11:18 AM, Jason Gunthorpe wrote: > >>>>>>>> On Thu, Mar 01, 2018 at 10:28:00AM -0600, Steve Wise wrote: > >>>>>>>>>> On Thu, Mar 01, 2018 at 09:29:04AM -0600, Steve Wise wrote: > >>>>>>>>>> > >>>>>>>>>>> Maybe to add a network interface to a soft-rdma device like rxe, we > >>>>>>>>> could > >>>>>>>>>> create a syntax like this: > >>>>>>>>>>> rdma link set rxe_eth0 dev eth0 > >>>>>>>>>> More like: > >>>>>>>>>> > >>>>>>>>>> rdma link add rxe_eth0 type rxe dev eth0 > >>>>>>>>>> > >>>>>>>>>> 'type rxe' triggers the kernel to dispatch to the rxe or siw driver to > >>>>>>>>>> create the interface. > >>>>>>>>> Type doesn't sound right though. in the current rdma synax, it would be > >>>>>>>>> 'dev'. Maybe: > >>>>>>>>> > >>>>>>>>> rdma link add rxe_eth0 dev rxe netdev eth0 > >>>>>>>> Type is the consistent tag with 'ip link add'. > >>>>>>> > >>>>>>> Hey guys, > >>>>>>> > >>>>>>> I'm starting to think about how to implement 'rdma link add/delete' for > >>>>>>> rxe. Does it make sense to add new members to the rdma_nldev_command > >>>>>>> enum in include/uapi/rdma/rdma_netlink.h for adding and deleting > >>>>>>> soft-rdma links to netdev interfaces? Something like > >>>>>>> RDMA_NLDEV_CMD_LINK_ADD and RDMA_NLDEV_CMD_LINK_DEL? Then add handlers > >>>>>>> to the nldev_cb_table array in drivers/infiniband/core/nldev.c. Can > >>>>>>> we assume there is only one soft-rdma driver for each rdma transport? > >>>>>>> That would enable a simple core->driver dispatch via an array of > >>>>>>> driver-specific handler functions that are indexed by the soft-rdma > >>>>>>> transport type. Or do we need something more general? > >>>>>>> > >>>>>>> Just thinking out loud here. What do you all think? > >>>>>> > >>>>>> You may as well follow the pattern ip link add uses.. > >>>>>> > >>>>>> See struct rtnl_link_ops ipoib_link_ops > >>>>>> > >>>>>> This way we can keep rxe as a module without creating a link time > >>>>>> dependency on the core module. > >>>>>> > >>>>>> Core code should match the 'type' tag with the '.kind' string member > >>>>>> just as ip does, and should pas a netlink attrs bundle in to the callback > >>>>>> like ip, see ipoib_new_child_link() > >>>>>> > >>>>>> The result should return the rdma id and name of the newly created > >>>>>> interface. > >>>>>> > >>>>> > >>>>> Should the rxe driver just register with an actual rtnl_link_ops? And > >>>>> use that rtnl core facility to dispatch callbacks? > >>>>> > >>>>> Or should we create a similar rdma_link_ops struct that includes the > >>>>> '.kind' and other appropriate fields, and have our own dispatcher in > >>>>> drivers/infiniband/core? Seems like we could simplify it some some... > >>>> > >>>> I think we have to do this to get the proper command line > >>>> interface.. Otherwise in userspace you have to parse the kind and > >>>> send a unique per driver message.. > >>>> > >>> > >>> I'm not advocating using RTM_NEWLINK, but just so we're on the same > >>> page: that infrastructure does already have a IFLA_INFO_KIND attribute. > >>> So our'rdma link add' could build and send an RTM_NEWLINK message and > >>> the net/rtnetlink.c core code would dispatch to whoever registered with > >>> .kind == 'rxe'. (and in the future .kind = 'siw' :) ). > >> > >> RTM_NEWLINK is only for creating netdevices.. > >> > >>> So I don't understand your comment about user space having to send a > >>> unique per-driver message? It would be a common RTM_NEWLINK message, but > >>> INFO_KIND attribute would identify the rdma driver. Am I missing > >>> something (probably)? > >> > >> We need a RDMA_NEWLINK with a RDMALA_INFO_KIND string that dispatches > >> to the right driver > >> > >>> Regardless, I would rather create an rdma-specific message and > >>> supporting structs/attributes to keep it simple. I mean these links are > >>> not 'ip link' netdev links. They are rdma links... > >> > >> Yes, rdma specific, but RDMA driver generic. > >> > >> Jason > >> > > > > Got it. Thanks Jason! > > > > Since the rdma core currently assigned device name strings, I'm thinking > to just follow that scheme. This leads to slightly different command > syntax: > > rdma link add TYPE dev DEV > > EG: rdma link add rxe dev eth0 > > Then the kernel would return the resulting ib_device name and index. I would rather we specify the ib_device name from userspace from the start.. Jason