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' :) ). 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)? 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... Steve.