From: Bernard Metzler <[email protected]>
Tunnel devices have zero GIDs, so skip the zero GID check when
setting up soft iWARP over a tunnel device.
Suggested-by: Bernard Metzler <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
drivers/infiniband/core/cache.c | 4 +++-
drivers/infiniband/sw/siw/siw_main.c | 1 +
include/rdma/iw_cm.h | 9 ++++++++-
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 2e91d8879326..2493ca4f2739 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -41,6 +41,7 @@
#include <net/addrconf.h>
#include <rdma/ib_cache.h>
+#include <rdma/iw_cm.h>
#include "core_priv.h"
@@ -441,7 +442,8 @@ static int add_modify_gid(struct ib_gid_table *table,
* leave other unused entries as the zero GID. Convert zero GIDs to
* empty table entries instead of storing them.
*/
- if (rdma_is_zero_gid(&attr->gid))
+ if (rdma_is_zero_gid(&attr->gid) &&
+ !(attr->device->iw_driver_flags & IW_F_STORE_0GID))
return 0;
entry = alloc_gid_entry(attr);
diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
index dacc174604bf..842a039fa457 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -359,6 +359,7 @@ static struct siw_device *siw_device_create(struct net_device *netdev)
/* Disable TCP port mapping */
base_dev->iw_driver_flags = IW_F_NO_PORT_MAP;
+ base_dev->iw_driver_flags = IW_F_STORE_0GID;
sdev->attrs.max_qp = SIW_MAX_QP;
sdev->attrs.max_qp_wr = SIW_MAX_QP_WR;
diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h
index 03abd30e6c8c..c48f2cbe37b5 100644
--- a/include/rdma/iw_cm.h
+++ b/include/rdma/iw_cm.h
@@ -90,7 +90,14 @@ enum iw_flags {
* reserve the port. This is required for soft iwarp
* to play in the port mapped iwarp space.
*/
- IW_F_NO_PORT_MAP = (1 << 0),
+ IW_F_NO_PORT_MAP = BIT(0),
+
+ /*
+ * This flag allows the insertion of zero GIDs into the
+ * stored GID table. That is needed to enable soft iWARP
+ * on tunnel devices.
+ */
+ IW_F_STORE_0GID = BIT(1),
};
/**
> -----Original Message-----
> From: Chuck Lever <[email protected]>
> Sent: Thursday, 27 April 2023 19:15
> To: Bernard Metzler <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: [EXTERNAL] [PATCH RFC] RDMA/core: Store zero GIDs in some cases
>
> From: Bernard Metzler <[email protected]>
>
> Tunnel devices have zero GIDs, so skip the zero GID check when
> setting up soft iWARP over a tunnel device.
>
> Suggested-by: Bernard Metzler <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> drivers/infiniband/core/cache.c | 4 +++-
> drivers/infiniband/sw/siw/siw_main.c | 1 +
> include/rdma/iw_cm.h | 9 ++++++++-
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/cache.c
> b/drivers/infiniband/core/cache.c
> index 2e91d8879326..2493ca4f2739 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -41,6 +41,7 @@
> #include <net/addrconf.h>
>
> #include <rdma/ib_cache.h>
> +#include <rdma/iw_cm.h>
>
> #include "core_priv.h"
>
> @@ -441,7 +442,8 @@ static int add_modify_gid(struct ib_gid_table *table,
> * leave other unused entries as the zero GID. Convert zero GIDs to
> * empty table entries instead of storing them.
> */
> - if (rdma_is_zero_gid(&attr->gid))
> + if (rdma_is_zero_gid(&attr->gid) &&
> + !(attr->device->iw_driver_flags & IW_F_STORE_0GID))
> return 0;
>
> entry = alloc_gid_entry(attr);
> diff --git a/drivers/infiniband/sw/siw/siw_main.c
> b/drivers/infiniband/sw/siw/siw_main.c
> index dacc174604bf..842a039fa457 100644
> --- a/drivers/infiniband/sw/siw/siw_main.c
> +++ b/drivers/infiniband/sw/siw/siw_main.c
> @@ -359,6 +359,7 @@ static struct siw_device *siw_device_create(struct
> net_device *netdev)
>
> /* Disable TCP port mapping */
> base_dev->iw_driver_flags = IW_F_NO_PORT_MAP;
> + base_dev->iw_driver_flags = IW_F_STORE_0GID;
>
That overwrites the first assignment. Probably better
'|= IW_F_STORE_0GID;' ? Or put them on one line...
> sdev->attrs.max_qp = SIW_MAX_QP;
> sdev->attrs.max_qp_wr = SIW_MAX_QP_WR;
> diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h
> index 03abd30e6c8c..c48f2cbe37b5 100644
> --- a/include/rdma/iw_cm.h
> +++ b/include/rdma/iw_cm.h
> @@ -90,7 +90,14 @@ enum iw_flags {
> * reserve the port. This is required for soft iwarp
> * to play in the port mapped iwarp space.
> */
> - IW_F_NO_PORT_MAP = (1 << 0),
> + IW_F_NO_PORT_MAP = BIT(0),
> +
> + /*
> + * This flag allows the insertion of zero GIDs into the
> + * stored GID table. That is needed to enable soft iWARP
> + * on tunnel devices.
> + */
> + IW_F_STORE_0GID = BIT(1),
> };
>
> /**
>
> On Apr 27, 2023, at 1:46 PM, Bernard Metzler <[email protected]> wrote:
>
>
>
>> -----Original Message-----
>> From: Chuck Lever <[email protected]>
>> Sent: Thursday, 27 April 2023 19:15
>> To: Bernard Metzler <[email protected]>
>> Cc: [email protected]; [email protected]
>> Subject: [EXTERNAL] [PATCH RFC] RDMA/core: Store zero GIDs in some cases
>>
>> From: Bernard Metzler <[email protected]>
>>
>> Tunnel devices have zero GIDs, so skip the zero GID check when
>> setting up soft iWARP over a tunnel device.
>>
>> Suggested-by: Bernard Metzler <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> drivers/infiniband/core/cache.c | 4 +++-
>> drivers/infiniband/sw/siw/siw_main.c | 1 +
>> include/rdma/iw_cm.h | 9 ++++++++-
>> 3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/cache.c
>> b/drivers/infiniband/core/cache.c
>> index 2e91d8879326..2493ca4f2739 100644
>> --- a/drivers/infiniband/core/cache.c
>> +++ b/drivers/infiniband/core/cache.c
>> @@ -41,6 +41,7 @@
>> #include <net/addrconf.h>
>>
>> #include <rdma/ib_cache.h>
>> +#include <rdma/iw_cm.h>
>>
>> #include "core_priv.h"
>>
>> @@ -441,7 +442,8 @@ static int add_modify_gid(struct ib_gid_table *table,
>> * leave other unused entries as the zero GID. Convert zero GIDs to
>> * empty table entries instead of storing them.
>> */
>> - if (rdma_is_zero_gid(&attr->gid))
>> + if (rdma_is_zero_gid(&attr->gid) &&
>> + !(attr->device->iw_driver_flags & IW_F_STORE_0GID))
>> return 0;
>>
>> entry = alloc_gid_entry(attr);
>> diff --git a/drivers/infiniband/sw/siw/siw_main.c
>> b/drivers/infiniband/sw/siw/siw_main.c
>> index dacc174604bf..842a039fa457 100644
>> --- a/drivers/infiniband/sw/siw/siw_main.c
>> +++ b/drivers/infiniband/sw/siw/siw_main.c
>> @@ -359,6 +359,7 @@ static struct siw_device *siw_device_create(struct
>> net_device *netdev)
>>
>> /* Disable TCP port mapping */
>> base_dev->iw_driver_flags = IW_F_NO_PORT_MAP;
>> + base_dev->iw_driver_flags = IW_F_STORE_0GID;
>>
> That overwrites the first assignment. Probably better
> '|= IW_F_STORE_0GID;' ? Or put them on one line...
D'oh! Will fix.
>> sdev->attrs.max_qp = SIW_MAX_QP;
>> sdev->attrs.max_qp_wr = SIW_MAX_QP_WR;
>> diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h
>> index 03abd30e6c8c..c48f2cbe37b5 100644
>> --- a/include/rdma/iw_cm.h
>> +++ b/include/rdma/iw_cm.h
>> @@ -90,7 +90,14 @@ enum iw_flags {
>> * reserve the port. This is required for soft iwarp
>> * to play in the port mapped iwarp space.
>> */
>> - IW_F_NO_PORT_MAP = (1 << 0),
>> + IW_F_NO_PORT_MAP = BIT(0),
>> +
>> + /*
>> + * This flag allows the insertion of zero GIDs into the
>> + * stored GID table. That is needed to enable soft iWARP
>> + * on tunnel devices.
>> + */
>> + IW_F_STORE_0GID = BIT(1),
>> };
>>
>> /**
--
Chuck Lever
> -----Original Message-----
> From: Chuck Lever III <[email protected]>
> Sent: Thursday, 27 April 2023 19:48
> To: Bernard Metzler <[email protected]>
> Cc: Chuck Lever <[email protected]>; [email protected]; Linux NFS
> Mailing List <[email protected]>
> Subject: [EXTERNAL] Re: [PATCH RFC] RDMA/core: Store zero GIDs in some
> cases
>
>
>
> > On Apr 27, 2023, at 1:46 PM, Bernard Metzler <[email protected]> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: Chuck Lever <[email protected]>
> >> Sent: Thursday, 27 April 2023 19:15
> >> To: Bernard Metzler <[email protected]>
> >> Cc: [email protected]; [email protected]
> >> Subject: [EXTERNAL] [PATCH RFC] RDMA/core: Store zero GIDs in some cases
> >>
> >> From: Bernard Metzler <[email protected]>
> >>
> >> Tunnel devices have zero GIDs, so skip the zero GID check when
> >> setting up soft iWARP over a tunnel device.
> >>
> >> Suggested-by: Bernard Metzler <[email protected]>
> >> Signed-off-by: Chuck Lever <[email protected]>
> >> ---
> >> drivers/infiniband/core/cache.c | 4 +++-
> >> drivers/infiniband/sw/siw/siw_main.c | 1 +
> >> include/rdma/iw_cm.h | 9 ++++++++-
> >> 3 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/core/cache.c
> >> b/drivers/infiniband/core/cache.c
> >> index 2e91d8879326..2493ca4f2739 100644
> >> --- a/drivers/infiniband/core/cache.c
> >> +++ b/drivers/infiniband/core/cache.c
> >> @@ -41,6 +41,7 @@
> >> #include <net/addrconf.h>
> >>
> >> #include <rdma/ib_cache.h>
> >> +#include <rdma/iw_cm.h>
> >>
> >> #include "core_priv.h"
> >>
> >> @@ -441,7 +442,8 @@ static int add_modify_gid(struct ib_gid_table
> *table,
> >> * leave other unused entries as the zero GID. Convert zero GIDs to
> >> * empty table entries instead of storing them.
> >> */
> >> - if (rdma_is_zero_gid(&attr->gid))
> >> + if (rdma_is_zero_gid(&attr->gid) &&
> >> + !(attr->device->iw_driver_flags & IW_F_STORE_0GID))
> >> return 0;
> >>
> >> entry = alloc_gid_entry(attr);
> >> diff --git a/drivers/infiniband/sw/siw/siw_main.c
> >> b/drivers/infiniband/sw/siw/siw_main.c
> >> index dacc174604bf..842a039fa457 100644
> >> --- a/drivers/infiniband/sw/siw/siw_main.c
> >> +++ b/drivers/infiniband/sw/siw/siw_main.c
> >> @@ -359,6 +359,7 @@ static struct siw_device *siw_device_create(struct
> >> net_device *netdev)
> >>
> >> /* Disable TCP port mapping */
> >> base_dev->iw_driver_flags = IW_F_NO_PORT_MAP;
> >> + base_dev->iw_driver_flags = IW_F_STORE_0GID;
> >>
> > That overwrites the first assignment. Probably better
> > '|= IW_F_STORE_0GID;' ? Or put them on one line...
>
> D'oh! Will fix.
Otherwise looks good of course. Could you
please check if that does not break 'normal' Ethernet
behavior? I am off from any kernel test infrastructure
for the next four (!!) weeks unfortunately. Sorry about
that.
Thanks for the patch!
Bernard.
>
> >> sdev->attrs.max_qp = SIW_MAX_QP;
> >> sdev->attrs.max_qp_wr = SIW_MAX_QP_WR;
> >> diff --git a/include/rdma/iw_cm.h b/include/rdma/iw_cm.h
> >> index 03abd30e6c8c..c48f2cbe37b5 100644
> >> --- a/include/rdma/iw_cm.h
> >> +++ b/include/rdma/iw_cm.h
> >> @@ -90,7 +90,14 @@ enum iw_flags {
> >> * reserve the port. This is required for soft iwarp
> >> * to play in the port mapped iwarp space.
> >> */
> >> - IW_F_NO_PORT_MAP = (1 << 0),
> >> + IW_F_NO_PORT_MAP = BIT(0),
> >> +
> >> + /*
> >> + * This flag allows the insertion of zero GIDs into the
> >> + * stored GID table. That is needed to enable soft iWARP
> >> + * on tunnel devices.
> >> + */
> >> + IW_F_STORE_0GID = BIT(1),
> >> };
> >>
> >> /**
>
>
> --
> Chuck Lever
>
On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
> From: Bernard Metzler <[email protected]>
>
> Tunnel devices have zero GIDs, so skip the zero GID check when
> setting up soft iWARP over a tunnel device.
Huh? Why? How does that make any sense?
Jason
> On Apr 28, 2023, at 9:39 AM, Jason Gunthorpe <[email protected]> wrote:
>
> On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
>> From: Bernard Metzler <[email protected]>
>>
>> Tunnel devices have zero GIDs, so skip the zero GID check when
>> setting up soft iWARP over a tunnel device.
>
> Huh? Why? How does that make any sense?
Read it as a cry for help.
The scenario is attempting to set up a soft iWARP device
with a slave that is a tunnel device. The set up seems to
work, but when connecting, the ULP gets an ADDR_ERROR
because the setup did not add an entry to the GID table.
--
Chuck Lever
On Fri, Apr 28, 2023 at 01:42:24PM +0000, Chuck Lever III wrote:
>
>
> > On Apr 28, 2023, at 9:39 AM, Jason Gunthorpe <[email protected]> wrote:
> >
> > On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
> >> From: Bernard Metzler <[email protected]>
> >>
> >> Tunnel devices have zero GIDs, so skip the zero GID check when
> >> setting up soft iWARP over a tunnel device.
> >
> > Huh? Why? How does that make any sense?
>
> Read it as a cry for help.
>
> The scenario is attempting to set up a soft iWARP device
> with a slave that is a tunnel device. The set up seems to
> work, but when connecting, the ULP gets an ADDR_ERROR
> because the setup did not add an entry to the GID table.
Don't assign a 0 IP to the tunnel?
Jason
On Fri, Apr 28, 2023 at 01:58:53PM +0000, Chuck Lever III wrote:
>
>
> > On Apr 28, 2023, at 9:47 AM, Jason Gunthorpe <[email protected]> wrote:
> >
> > On Fri, Apr 28, 2023 at 01:42:24PM +0000, Chuck Lever III wrote:
> >>
> >>
> >>> On Apr 28, 2023, at 9:39 AM, Jason Gunthorpe <[email protected]> wrote:
> >>>
> >>> On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
> >>>> From: Bernard Metzler <[email protected]>
> >>>>
> >>>> Tunnel devices have zero GIDs, so skip the zero GID check when
> >>>> setting up soft iWARP over a tunnel device.
> >>>
> >>> Huh? Why? How does that make any sense?
> >>
> >> Read it as a cry for help.
> >>
> >> The scenario is attempting to set up a soft iWARP device
> >> with a slave that is a tunnel device. The set up seems to
> >> work, but when connecting, the ULP gets an ADDR_ERROR
> >> because the setup did not add an entry to the GID table.
> >
> > Don't assign a 0 IP to the tunnel?
>
> That's a little cryptic... can you expand?
>
> Right now I have a Tailscale VPN device with assigned IP
> addresses:
>
> 3: tailscale0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1280 qdisc fq_codel state UNKNOWN group default qlen 500
> link/none inet 100.64.0.16/32 scope global tailscale0
> valid_lft forever preferred_lft forever
> inet6 fd7a:115c:a1e0::10/128 scope global valid_lft forever preferred_lft forever
> inet6 fe80::725c:1b6d:60ed:fce4/64 scope link stable-privacy valid_lft forever preferred_lft forever
>
> And after that i/f is UP, I've done this:
That seems OK..
> $ sudo rdma link add siw0 type siw netdev tailscale0
>
> With the patch I sent, I can do NFS/RDMA via soft iWARP through
> the tunnel. I'm not at all claiming that's a good fix, but only
> that this scenario is supposed to work, but currently doesn't.
Then there is something wrong in SIW, it should not be reporting 0
GIDs to the core code for that kind of device.
I don't remember what iwarp uses for it's guid format .. Maybe it was
mac adress or something and tunnels don't have a MAC, it should make
up a dummy GID for the tunnel instead of using 0..
Jason
> On Apr 28, 2023, at 9:47 AM, Jason Gunthorpe <[email protected]> wrote:
>
> On Fri, Apr 28, 2023 at 01:42:24PM +0000, Chuck Lever III wrote:
>>
>>
>>> On Apr 28, 2023, at 9:39 AM, Jason Gunthorpe <[email protected]> wrote:
>>>
>>> On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
>>>> From: Bernard Metzler <[email protected]>
>>>>
>>>> Tunnel devices have zero GIDs, so skip the zero GID check when
>>>> setting up soft iWARP over a tunnel device.
>>>
>>> Huh? Why? How does that make any sense?
>>
>> Read it as a cry for help.
>>
>> The scenario is attempting to set up a soft iWARP device
>> with a slave that is a tunnel device. The set up seems to
>> work, but when connecting, the ULP gets an ADDR_ERROR
>> because the setup did not add an entry to the GID table.
>
> Don't assign a 0 IP to the tunnel?
That's a little cryptic... can you expand?
Right now I have a Tailscale VPN device with assigned IP
addresses:
3: tailscale0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1280 qdisc fq_codel state UNKNOWN group default qlen 500
link/none inet 100.64.0.16/32 scope global tailscale0
valid_lft forever preferred_lft forever
inet6 fd7a:115c:a1e0::10/128 scope global valid_lft forever preferred_lft forever
inet6 fe80::725c:1b6d:60ed:fce4/64 scope link stable-privacy valid_lft forever preferred_lft forever
And after that i/f is UP, I've done this:
$ sudo rdma link add siw0 type siw netdev tailscale0
With the patch I sent, I can do NFS/RDMA via soft iWARP through
the tunnel. I'm not at all claiming that's a good fix, but only
that this scenario is supposed to work, but currently doesn't.
--
Chuck Lever
> -----Original Message-----
> From: Jason Gunthorpe <[email protected]>
> Sent: Friday, 28 April 2023 16:04
> To: Chuck Lever III <[email protected]>
> Cc: Chuck Lever <[email protected]>; Bernard Metzler <[email protected]>;
> linux-rdma <[email protected]>; Linux NFS Mailing List <linux-
> [email protected]>
> Subject: [EXTERNAL] Re: [PATCH RFC] RDMA/core: Store zero GIDs in some
> cases
>
> On Fri, Apr 28, 2023 at 01:58:53PM +0000, Chuck Lever III wrote:
> >
> >
> > > On Apr 28, 2023, at 9:47 AM, Jason Gunthorpe <[email protected]> wrote:
> > >
> > > On Fri, Apr 28, 2023 at 01:42:24PM +0000, Chuck Lever III wrote:
> > >>
> > >>
> > >>> On Apr 28, 2023, at 9:39 AM, Jason Gunthorpe <[email protected]> wrote:
> > >>>
> > >>> On Thu, Apr 27, 2023 at 01:14:43PM -0400, Chuck Lever wrote:
> > >>>> From: Bernard Metzler <[email protected]>
> > >>>>
> > >>>> Tunnel devices have zero GIDs, so skip the zero GID check when
> > >>>> setting up soft iWARP over a tunnel device.
> > >>>
> > >>> Huh? Why? How does that make any sense?
> > >>
> > >> Read it as a cry for help.
> > >>
> > >> The scenario is attempting to set up a soft iWARP device
> > >> with a slave that is a tunnel device. The set up seems to
> > >> work, but when connecting, the ULP gets an ADDR_ERROR
> > >> because the setup did not add an entry to the GID table.
> > >
> > > Don't assign a 0 IP to the tunnel?
> >
> > That's a little cryptic... can you expand?
> >
> > Right now I have a Tailscale VPN device with assigned IP
> > addresses:
> >
> > 3: tailscale0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1280
> qdisc fq_codel state UNKNOWN group default qlen 500
> > link/none inet 100.64.0.16/32 scope global tailscale0
> > valid_lft forever preferred_lft forever
> > inet6 fd7a:115c:a1e0::10/128 scope global valid_lft
> forever preferred_lft forever
> > inet6 fe80::725c:1b6d:60ed:fce4/64 scope link stable-privacy
> valid_lft forever preferred_lft forever
> >
> > And after that i/f is UP, I've done this:
>
> That seems OK..
>
> > $ sudo rdma link add siw0 type siw netdev tailscale0
> >
> > With the patch I sent, I can do NFS/RDMA via soft iWARP through
> > the tunnel. I'm not at all claiming that's a good fix, but only
> > that this scenario is supposed to work, but currently doesn't.
>
> Then there is something wrong in SIW, it should not be reporting 0
> GIDs to the core code for that kind of device.
>
> I don't remember what iwarp uses for it's guid format .. Maybe it was
> mac adress or something and tunnels don't have a MAC, it should make
> up a dummy GID for the tunnel instead of using 0..
>
Yes. it is taken from the MAC address and we don't have one
here. I don't remember the iwcm code by heart - maybe any unique
GID makes it and there is no need to have the MAC there.
Unfortunately I cannot look into it deeper
right now since away from my desk a long time, until end of
May. Sorry about that confusion.
Bernard