netdev - Re: [PATCH RFC] RDMA/core: Handle ARPHRD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D92D284-5031-4676-BFBE-A47B60255219@oracle.com>
Date: Sat, 3 Jun 2023 13:53:26 +0000
From: Chuck Lever III <chuck.lever@...cle.com>
To: Bernard Metzler <BMT@...ich.ibm.com>
CC: Tom Talpey <tom@...pey.com>, Chuck Lever <cel@...nel.org>,
        Jason Gunthorpe
	<jgg@...dia.com>,
        linux-rdma <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices



> On Jun 3, 2023, at 9:51 AM, Bernard Metzler <BMT@...ich.ibm.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Chuck Lever III <chuck.lever@...cle.com>
>> Sent: Saturday, 3 June 2023 02:33
>> To: Tom Talpey <tom@...pey.com>
>> Cc: Chuck Lever <cel@...nel.org>; Jason Gunthorpe <jgg@...dia.com>; linux-
>> rdma <linux-rdma@...r.kernel.org>; Bernard Metzler <BMT@...ich.ibm.com>;
>> netdev@...r.kernel.org
>> Subject: [EXTERNAL] Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices
>> 
>> 
>> 
>>> On Jun 2, 2023, at 6:18 PM, Tom Talpey <tom@...pey.com> wrote:
>>> 
>>> On 6/2/2023 3:24 PM, Chuck Lever wrote:
>>>> From: Chuck Lever <chuck.lever@...cle.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@...dia.com>
>>>> Signed-off-by: Chuck Lever <chuck.lever@...cle.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.


>> 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