* [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices @ 2023-06-02 19:24 Chuck Lever 2023-06-02 22:18 ` Tom Talpey 2023-06-06 15:48 ` Jason Gunthorpe 0 siblings, 2 replies; 11+ messages in thread From: Chuck Lever @ 2023-06-02 19:24 UTC (permalink / raw) To: jgg; +Cc: Chuck Lever, linux-rdma, BMT, tom, netdev 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; } ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-02 19:24 [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices Chuck Lever @ 2023-06-02 22:18 ` Tom Talpey 2023-06-03 0:33 ` Chuck Lever III 2023-06-06 15:48 ` Jason Gunthorpe 1 sibling, 1 reply; 11+ messages in thread From: Tom Talpey @ 2023-06-02 22:18 UTC (permalink / raw) To: Chuck Lever, jgg; +Cc: Chuck Lever, linux-rdma, BMT, netdev 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); } Tom. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-02 22:18 ` Tom Talpey @ 2023-06-03 0:33 ` Chuck Lever III 2023-06-03 13:51 ` Bernard Metzler 0 siblings, 1 reply; 11+ messages in thread From: Chuck Lever III @ 2023-06-03 0:33 UTC (permalink / raw) To: Tom Talpey Cc: Chuck Lever, Jason Gunthorpe, linux-rdma, Bernard Metzler, netdev > 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. 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. -- Chuck Lever ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-03 0:33 ` Chuck Lever III @ 2023-06-03 13:51 ` Bernard Metzler 2023-06-03 13:53 ` Chuck Lever III 2023-06-05 19:05 ` Jason Gunthorpe 0 siblings, 2 replies; 11+ messages in thread From: Bernard Metzler @ 2023-06-03 13:51 UTC (permalink / raw) To: Chuck Lever III, Tom Talpey Cc: Chuck Lever, Jason Gunthorpe, linux-rdma, netdev > -----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. > 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-03 13:51 ` Bernard Metzler @ 2023-06-03 13:53 ` Chuck Lever III 2023-06-03 13:55 ` Bernard Metzler 2023-06-05 19:05 ` Jason Gunthorpe 1 sibling, 1 reply; 11+ messages in thread From: Chuck Lever III @ 2023-06-03 13:53 UTC (permalink / raw) To: Bernard Metzler Cc: Tom Talpey, Chuck Lever, Jason Gunthorpe, linux-rdma, netdev > 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. >> 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-03 13:53 ` Chuck Lever III @ 2023-06-03 13:55 ` Bernard Metzler 0 siblings, 0 replies; 11+ messages in thread From: Bernard Metzler @ 2023-06-03 13:55 UTC (permalink / raw) To: Chuck Lever III Cc: Tom Talpey, Chuck Lever, Jason Gunthorpe, linux-rdma, netdev > -----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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-03 13:51 ` Bernard Metzler 2023-06-03 13:53 ` Chuck Lever III @ 2023-06-05 19:05 ` Jason Gunthorpe 1 sibling, 0 replies; 11+ messages in thread From: Jason Gunthorpe @ 2023-06-05 19:05 UTC (permalink / raw) To: Bernard Metzler Cc: Chuck Lever III, Tom Talpey, Chuck Lever, linux-rdma, netdev On Sat, Jun 03, 2023 at 01:51:10PM +0000, Bernard Metzler wrote: > 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. GIDs are part of verbs, but I'm not really sure how iwarp fit itself into verbs with the mandatory CM and all. Possibly the only purpose of the GID is to convey some information from the CM to uverbs layer. Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-02 19:24 [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices Chuck Lever 2023-06-02 22:18 ` Tom Talpey @ 2023-06-06 15:48 ` Jason Gunthorpe 2023-06-06 20:15 ` Chuck Lever III 1 sibling, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2023-06-06 15:48 UTC (permalink / raw) To: Chuck Lever; +Cc: Chuck Lever, linux-rdma, BMT, tom, netdev On Fri, Jun 02, 2023 at 03:24:30PM -0400, 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); It seems believable, should it be locked to iwarp devices? More broadly, should iwarp devices just always do this and skip all the rest of it? I think it also has to check that the returned netdev in the sgid_attr matches the egress netdev selected? Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-06 15:48 ` Jason Gunthorpe @ 2023-06-06 20:15 ` Chuck Lever III 2023-06-06 23:17 ` Jason Gunthorpe 0 siblings, 1 reply; 11+ messages in thread From: Chuck Lever III @ 2023-06-06 20:15 UTC (permalink / raw) To: Jason Gunthorpe Cc: Chuck Lever, linux-rdma, Bernard Metzler, Tom Talpey, netdev > On Jun 6, 2023, at 11:48 AM, Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Fri, Jun 02, 2023 at 03:24:30PM -0400, 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); > > It seems believable, should it be locked to iwarp devices? > > More broadly, should iwarp devices just always do this and skip all > the rest of it? > > I think it also has to check that the returned netdev in the sgid_attr > matches the egress netdev selected? Both @ndev and sgid_attr.ndev are NULL here. So I assume you want the code to check, on return, that sgid_attr.device == device? -- Chuck Lever ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-06 20:15 ` Chuck Lever III @ 2023-06-06 23:17 ` Jason Gunthorpe 2023-06-07 14:48 ` Chuck Lever III 0 siblings, 1 reply; 11+ messages in thread From: Jason Gunthorpe @ 2023-06-06 23:17 UTC (permalink / raw) To: Chuck Lever III Cc: Chuck Lever, linux-rdma, Bernard Metzler, Tom Talpey, netdev On Tue, Jun 06, 2023 at 08:15:36PM +0000, Chuck Lever III wrote: > > > > On Jun 6, 2023, at 11:48 AM, Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > On Fri, Jun 02, 2023 at 03:24:30PM -0400, 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); > > > > It seems believable, should it be locked to iwarp devices? > > > > More broadly, should iwarp devices just always do this and skip all > > the rest of it? > > > > I think it also has to check that the returned netdev in the sgid_attr > > matches the egress netdev selected? > > Both @ndev and sgid_attr.ndev are NULL here. The nedev to check is the dev_addr->bound_dev_if, that represents the netdev It is some iwarp mistake that the sgid attr's don't have proper netdevs, that is newer than iwarp so it never got updated. So, maybe it is too hard to fix for this, and maybe we can just assume it all has to be right Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices 2023-06-06 23:17 ` Jason Gunthorpe @ 2023-06-07 14:48 ` Chuck Lever III 0 siblings, 0 replies; 11+ messages in thread From: Chuck Lever III @ 2023-06-07 14:48 UTC (permalink / raw) To: Jason Gunthorpe Cc: Chuck Lever, linux-rdma, Bernard Metzler, Tom Talpey, netdev > On Jun 6, 2023, at 7:17 PM, Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Tue, Jun 06, 2023 at 08:15:36PM +0000, Chuck Lever III wrote: >> >> >>> On Jun 6, 2023, at 11:48 AM, Jason Gunthorpe <jgg@nvidia.com> wrote: >>> >>> On Fri, Jun 02, 2023 at 03:24:30PM -0400, 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); >>> >>> It seems believable, should it be locked to iwarp devices? >>> >>> More broadly, should iwarp devices just always do this and skip all >>> the rest of it? >>> >>> I think it also has to check that the returned netdev in the sgid_attr >>> matches the egress netdev selected? >> >> Both @ndev and sgid_attr.ndev are NULL here. > > The nedev to check is the dev_addr->bound_dev_if, that represents the netdev > > It is some iwarp mistake that the sgid attr's don't have proper > netdevs, that is newer than iwarp so it never got updated. > > So, maybe it is too hard to fix for this, and maybe we can just assume > it all has to be right sgid_attr.index is not set it looks like. I will send you an "official" version of this patch and we can go from there. -- Chuck Lever ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-07 14:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-02 19:24 [PATCH RFC] RDMA/core: Handle ARPHRD_NONE devices Chuck Lever 2023-06-02 22:18 ` Tom Talpey 2023-06-03 0:33 ` Chuck Lever III 2023-06-03 13:51 ` Bernard Metzler 2023-06-03 13:53 ` Chuck Lever III 2023-06-03 13:55 ` Bernard Metzler 2023-06-05 19:05 ` Jason Gunthorpe 2023-06-06 15:48 ` Jason Gunthorpe 2023-06-06 20:15 ` Chuck Lever III 2023-06-06 23:17 ` Jason Gunthorpe 2023-06-07 14:48 ` Chuck Lever III
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).