RE: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices

[Bernard Metzler <BMT@zurich.ibm.com>]

> -----Original Message-----
> From: Chuck Lever III <chuck.lever@oracle.com>
> Sent: Saturday, 3 June 2023 15:53
> To: Bernard Metzler <BMT@zurich.ibm.com>
> Cc: Tom Talpey <tom@talpey.com>; Chuck Lever <cel@kernel.org>; Jason
> Gunthorpe <jgg@nvidia.com>; linux-rdma <linux-rdma@vger.kernel.org>;
> netdev@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices
> 
> 
> 
> > On Jun 3, 2023, at 9:51 AM, Bernard Metzler <BMT@zurich.ibm.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: Chuck Lever III <chuck.lever@oracle.com>
> >> Sent: Saturday, 3 June 2023 02:33
> >> To: Tom Talpey <tom@talpey.com>
> >> Cc: Chuck Lever <cel@kernel.org>; Jason Gunthorpe <jgg@nvidia.com>;
> linux-
> >> rdma <linux-rdma@vger.kernel.org>; Bernard Metzler <BMT@zurich.ibm.com>;
> >> netdev@vger.kernel.org
> >> Subject: [EXTERNAL] Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE
> devices
> >>
> >>
> >>
> >>> On Jun 2, 2023, at 6:18 PM, Tom Talpey <tom@talpey.com> wrote:
> >>>
> >>> On 6/2/2023 3:24 PM, Chuck Lever wrote:
> >>>> From: Chuck Lever <chuck.lever@oracle.com>
> >>>> We would like to enable the use of siw on top of a VPN that is
> >>>> constructed and managed via a tun device. That hasn't worked up
> >>>> until now because ARPHRD_NONE devices (such as tun devices) have
> >>>> no GID for the RDMA/core to look up.
> >>>> But it turns out that the egress device has already been picked for
> >>>> us. addr_handler() just has to do the right thing with it.
> >>>> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> >>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >>>> ---
> >>>> drivers/infiniband/core/cma.c |    4 ++++
> >>>> 1 file changed, 4 insertions(+)
> >>>> diff --git a/drivers/infiniband/core/cma.c
> >> b/drivers/infiniband/core/cma.c
> >>>> index 56e568fcd32b..3351dc5afa17 100644
> >>>> --- a/drivers/infiniband/core/cma.c
> >>>> +++ b/drivers/infiniband/core/cma.c
> >>>> @@ -704,11 +704,15 @@ cma_validate_port(struct ib_device *device, u32
> >> port,
> >>>>  ndev = dev_get_by_index(dev_addr->net, bound_if_index);
> >>>>  if (!ndev)
> >>>>  return ERR_PTR(-ENODEV);
> >>>> + } else if (dev_type == ARPHRD_NONE) {
> >>>> + sgid_attr = rdma_get_gid_attr(device, port, 0);
> >>>> + goto out;
> >>>>  } else {
> >>>>  gid_type = IB_GID_TYPE_IB;
> >>>>  }
> >>>>    sgid_attr = rdma_find_gid_by_port(device, gid, gid_type, port,
> >> ndev);
> >>>> +out:
> >>>>  dev_put(ndev);
> >>>>  return sgid_attr;
> >>>> }
> >>>
> >>> I like it, but doesn't this test in siw_main.c also need to change?
> >>>
> >>> static struct siw_device *siw_device_create(struct net_device *netdev)
> >>> {
> >>> ...
> >>> --> if (netdev->type != ARPHRD_LOOPBACK && netdev->type != ARPHRD_NONE)
> {
> >>> addrconf_addr_eui48((unsigned char *)&base_dev->node_guid,
> >>>   netdev->dev_addr);
> >>> } else {
> >>> /*
> >>> * This device does not have a HW address,
> >>> * but connection mangagement lib expects gid != 0
> >>> */
> >>> size_t len = min_t(size_t, strlen(base_dev->name), 6);
> >>> char addr[6] = { };
> >>>
> >>> memcpy(addr, base_dev->name, len);
> >>> addrconf_addr_eui48((unsigned char *)&base_dev->node_guid,
> >>>   addr);
> >>> }
> >>
> >> I'm not sure that code does anything. The base_dev's name field
> >> is actually not initialized at that point, so nothing is copied
> >> here.
> >>
> > Oh in that case it’s an issue here.
> 
> I have a patch that fabricates a proper GID here that I can
> post separately.
> 
Sounds good!
> 
> >> If you're asking whether siw needs to build a non-zero GID to
> >> make the posted patch work, more testing is needed; but I don't
> >> believe the GID has any relevance -- the egress ib_device is
> >> selected based entirely on the source IP address in this case.
> >>
> >
> > The whole GID based address resolution I think is an
> > artefact of IB/RoCE address handling. iWarp is supposed to
> > run on TCP streams, which endpoints are well defined by L3
> > addresses. IP routing shall define the outgoing interface...
> > siw tries to play well and invents GIDs to satisfy
> > the RDMA core concepts. But a GID is not part of the iWarp
> > concept. I am not sure for 'real' HW iWarp devices, but to
> > me it looks like the iwcm code could be done more
> > independently, if no application expects valid GIDs.
> 
> 
> --
> Chuck Lever
>