* [PATCH rdma-next 0/3] Skip holes in GID tables
@ 2021-11-22 11:53 Leon Romanovsky
2021-11-22 11:53 ` [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes Leon Romanovsky
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Leon Romanovsky @ 2021-11-22 11:53 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang
From: Leon Romanovsky <leonro@nvidia.com>
Hi,
This short series extends rdma_query_gid() callers to skip holes
in GID tables.
Avihai Horon (3):
RDMA/core: Modify rdma_query_gid() to return accurate error codes
RDMA/core: Let ib_find_gid() continue search even after empty entry
RDMA/cma: Let cma_resolve_ib_dev() continue search even after empty
entry
drivers/infiniband/core/cache.c | 8 ++++++--
drivers/infiniband/core/cma.c | 14 +++++++++++---
drivers/infiniband/core/device.c | 3 +++
3 files changed, 20 insertions(+), 5 deletions(-)
--
2.33.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes 2021-11-22 11:53 [PATCH rdma-next 0/3] Skip holes in GID tables Leon Romanovsky @ 2021-11-22 11:53 ` Leon Romanovsky 2021-12-07 18:41 ` Jason Gunthorpe 2021-11-22 11:53 ` [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry Leon Romanovsky 2021-11-22 11:53 ` [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() " Leon Romanovsky 2 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2021-11-22 11:53 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Avihai Horon, linux-kernel, linux-rdma, Mark Zhang From: Avihai Horon <avihaih@nvidia.com> Modify rdma_query_gid() to return -ENOENT for empty entries. This will make error reporting more accurate and will be used in next patches. Signed-off-by: Avihai Horon <avihaih@nvidia.com> Reviewed-by: Mark Zhang <markzhang@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/core/cache.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c index 0c98dd3dee67..dd66f1a6e792 100644 --- a/drivers/infiniband/core/cache.c +++ b/drivers/infiniband/core/cache.c @@ -963,9 +963,13 @@ int rdma_query_gid(struct ib_device *device, u32 port_num, table = rdma_gid_table(device, port_num); read_lock_irqsave(&table->rwlock, flags); - if (index < 0 || index >= table->sz || - !is_gid_entry_valid(table->data_vec[index])) + if (index < 0 || index >= table->sz) + goto done; + + if (!is_gid_entry_valid(table->data_vec[index])) { + res = -ENOENT; goto done; + } memcpy(gid, &table->data_vec[index]->attr.gid, sizeof(*gid)); res = 0; -- 2.33.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes 2021-11-22 11:53 ` [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes Leon Romanovsky @ 2021-12-07 18:41 ` Jason Gunthorpe 0 siblings, 0 replies; 9+ messages in thread From: Jason Gunthorpe @ 2021-12-07 18:41 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang On Mon, Nov 22, 2021 at 01:53:56PM +0200, Leon Romanovsky wrote: > From: Avihai Horon <avihaih@nvidia.com> > > Modify rdma_query_gid() to return -ENOENT for empty entries. This will > make error reporting more accurate and will be used in next patches. > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > Reviewed-by: Mark Zhang <markzhang@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > drivers/infiniband/core/cache.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c > index 0c98dd3dee67..dd66f1a6e792 100644 > +++ b/drivers/infiniband/core/cache.c > @@ -963,9 +963,13 @@ int rdma_query_gid(struct ib_device *device, u32 port_num, > table = rdma_gid_table(device, port_num); > read_lock_irqsave(&table->rwlock, flags); > > - if (index < 0 || index >= table->sz || > - !is_gid_entry_valid(table->data_vec[index])) > + if (index < 0 || index >= table->sz) > + goto done; > + > + if (!is_gid_entry_valid(table->data_vec[index])) { > + res = -ENOENT; > goto done; > + } Please get rid of the default assignment to res so this code is all consistent @@ -955,7 +955,7 @@ int rdma_query_gid(struct ib_device *device, u32 port_num, { struct ib_gid_table *table; unsigned long flags; - int res = -EINVAL; + int res; if (!rdma_is_port_valid(device, port_num)) return -EINVAL; @@ -963,9 +963,15 @@ int rdma_query_gid(struct ib_device *device, u32 port_num, table = rdma_gid_table(device, port_num); read_lock_irqsave(&table->rwlock, flags); - if (index < 0 || index >= table->sz || - !is_gid_entry_valid(table->data_vec[index])) + if (index < 0 || index >= table->sz) { + res = -EINVAL; goto done; + } + + if (!is_gid_entry_valid(table->data_vec[index])) { + res = -ENOENT; + goto done; + } Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry 2021-11-22 11:53 [PATCH rdma-next 0/3] Skip holes in GID tables Leon Romanovsky 2021-11-22 11:53 ` [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes Leon Romanovsky @ 2021-11-22 11:53 ` Leon Romanovsky 2021-12-07 18:43 ` Jason Gunthorpe 2021-11-22 11:53 ` [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() " Leon Romanovsky 2 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2021-11-22 11:53 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Avihai Horon, linux-kernel, linux-rdma, Mark Zhang, Michael S. Tsirkin From: Avihai Horon <avihaih@nvidia.com> Currently, ib_find_gid() will stop searching after encountering the first empty GID table entry. This behavior is wrong since neither IB nor RoCE spec enforce tightly packed GID tables. For example, when a valid GID entry exists at index N, and if a GID entry is empty at index N-1, ib_find_gid() will fail to find the valid entry. Fix it by making ib_find_gid() continue searching even after encountering missing entries. Fixes: 5eb620c81ce3 ("IB/core: Add helpers for uncached GID and P_Key searches") Signed-off-by: Avihai Horon <avihaih@nvidia.com> Reviewed-by: Mark Zhang <markzhang@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/core/device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 22a4adda7981..b5d8443030d4 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -2460,8 +2460,11 @@ int ib_find_gid(struct ib_device *device, union ib_gid *gid, for (i = 0; i < device->port_data[port].immutable.gid_tbl_len; ++i) { ret = rdma_query_gid(device, port, i, &tmp_gid); + if (ret == -ENOENT) + continue; if (ret) return ret; + if (!memcmp(&tmp_gid, gid, sizeof *gid)) { *port_num = port; if (index) -- 2.33.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry 2021-11-22 11:53 ` [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry Leon Romanovsky @ 2021-12-07 18:43 ` Jason Gunthorpe 2021-12-08 6:51 ` Leon Romanovsky 0 siblings, 1 reply; 9+ messages in thread From: Jason Gunthorpe @ 2021-12-07 18:43 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang, Michael S. Tsirkin On Mon, Nov 22, 2021 at 01:53:57PM +0200, Leon Romanovsky wrote: > From: Avihai Horon <avihaih@nvidia.com> > > Currently, ib_find_gid() will stop searching after encountering the > first empty GID table entry. This behavior is wrong since neither IB > nor RoCE spec enforce tightly packed GID tables. > > For example, when a valid GID entry exists at index N, and if a GID > entry is empty at index N-1, ib_find_gid() will fail to find the valid > entry. > > Fix it by making ib_find_gid() continue searching even after > encountering missing entries. > > Fixes: 5eb620c81ce3 ("IB/core: Add helpers for uncached GID and P_Key searches") > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > Reviewed-by: Mark Zhang <markzhang@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > drivers/infiniband/core/device.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index 22a4adda7981..b5d8443030d4 100644 > +++ b/drivers/infiniband/core/device.c > @@ -2460,8 +2460,11 @@ int ib_find_gid(struct ib_device *device, union ib_gid *gid, > for (i = 0; i < device->port_data[port].immutable.gid_tbl_len; > ++i) { > ret = rdma_query_gid(device, port, i, &tmp_gid); > + if (ret == -ENOENT) > + continue; > if (ret) > return ret; There is no return code from rdma_query_gid that means stop searching, so just write if (ret) continue Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry 2021-12-07 18:43 ` Jason Gunthorpe @ 2021-12-08 6:51 ` Leon Romanovsky 2021-12-09 0:13 ` Jason Gunthorpe 0 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2021-12-08 6:51 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang, Michael S. Tsirkin On Tue, Dec 07, 2021 at 02:43:04PM -0400, Jason Gunthorpe wrote: > On Mon, Nov 22, 2021 at 01:53:57PM +0200, Leon Romanovsky wrote: > > From: Avihai Horon <avihaih@nvidia.com> > > > > Currently, ib_find_gid() will stop searching after encountering the > > first empty GID table entry. This behavior is wrong since neither IB > > nor RoCE spec enforce tightly packed GID tables. > > > > For example, when a valid GID entry exists at index N, and if a GID > > entry is empty at index N-1, ib_find_gid() will fail to find the valid > > entry. > > > > Fix it by making ib_find_gid() continue searching even after > > encountering missing entries. > > > > Fixes: 5eb620c81ce3 ("IB/core: Add helpers for uncached GID and P_Key searches") > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > > Reviewed-by: Mark Zhang <markzhang@nvidia.com> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > drivers/infiniband/core/device.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > index 22a4adda7981..b5d8443030d4 100644 > > +++ b/drivers/infiniband/core/device.c > > @@ -2460,8 +2460,11 @@ int ib_find_gid(struct ib_device *device, union ib_gid *gid, > > for (i = 0; i < device->port_data[port].immutable.gid_tbl_len; > > ++i) { > > ret = rdma_query_gid(device, port, i, &tmp_gid); > > + if (ret == -ENOENT) > > + continue; > > if (ret) > > return ret; > > There is no return code from rdma_query_gid that means stop searching, In rdma_query_gid() any error stopped searching, and here we continue same behaviour as before. You can argue that this function can't really get illegal parameters and it never returns -EINVAL, but someone needs to check all callers that this is true. > so just write > > if (ret) > continue As long as we don't delete input validity checks, it is not correct. Thanks > > Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry 2021-12-08 6:51 ` Leon Romanovsky @ 2021-12-09 0:13 ` Jason Gunthorpe 0 siblings, 0 replies; 9+ messages in thread From: Jason Gunthorpe @ 2021-12-09 0:13 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang, Michael S. Tsirkin On Wed, Dec 08, 2021 at 08:51:59AM +0200, Leon Romanovsky wrote: > On Tue, Dec 07, 2021 at 02:43:04PM -0400, Jason Gunthorpe wrote: > > On Mon, Nov 22, 2021 at 01:53:57PM +0200, Leon Romanovsky wrote: > > > From: Avihai Horon <avihaih@nvidia.com> > > > > > > Currently, ib_find_gid() will stop searching after encountering the > > > first empty GID table entry. This behavior is wrong since neither IB > > > nor RoCE spec enforce tightly packed GID tables. > > > > > > For example, when a valid GID entry exists at index N, and if a GID > > > entry is empty at index N-1, ib_find_gid() will fail to find the valid > > > entry. > > > > > > Fix it by making ib_find_gid() continue searching even after > > > encountering missing entries. > > > > > > Fixes: 5eb620c81ce3 ("IB/core: Add helpers for uncached GID and P_Key searches") > > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > > > Reviewed-by: Mark Zhang <markzhang@nvidia.com> > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > > drivers/infiniband/core/device.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > > index 22a4adda7981..b5d8443030d4 100644 > > > +++ b/drivers/infiniband/core/device.c > > > @@ -2460,8 +2460,11 @@ int ib_find_gid(struct ib_device *device, union ib_gid *gid, > > > for (i = 0; i < device->port_data[port].immutable.gid_tbl_len; > > > ++i) { > > > ret = rdma_query_gid(device, port, i, &tmp_gid); > > > + if (ret == -ENOENT) > > > + continue; > > > if (ret) > > > return ret; > > > > There is no return code from rdma_query_gid that means stop searching, > > In rdma_query_gid() any error stopped searching, and here we continue > same behaviour as before. You can argue that this function can't really > get illegal parameters and it never returns -EINVAL, but someone needs > to check all callers that this is true. > > > so just write > > > > if (ret) > > continue > > As long as we don't delete input validity checks, it is not correct. It is fine, there is no return condition that means stop searching, and even if we fail the validity checks that is a WARN_ON and keep going, not a stop searching event here. So just do continue, no need for complications. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() continue search even after empty entry 2021-11-22 11:53 [PATCH rdma-next 0/3] Skip holes in GID tables Leon Romanovsky 2021-11-22 11:53 ` [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes Leon Romanovsky 2021-11-22 11:53 ` [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry Leon Romanovsky @ 2021-11-22 11:53 ` Leon Romanovsky 2021-12-07 18:44 ` Jason Gunthorpe 2 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2021-11-22 11:53 UTC (permalink / raw) To: Doug Ledford, Jason Gunthorpe Cc: Avihai Horon, linux-kernel, linux-rdma, Mark Zhang From: Avihai Horon <avihaih@nvidia.com> Currently, when cma_resolve_ib_dev() searches for a matching GID it will stop searching after encountering the first empty GID table entry. This behavior is wrong since neither IB nor RoCE spec enforce tightly packed GID tables. For example, when the matching valid GID entry exists at index N, and if a GID entry is empty at index N-1, cma_resolve_ib_dev() will fail to find the matching valid entry. Fix it by making cma_resolve_ib_dev() continue searching even after encountering missing entries. Fixes: f17df3b0dede ("RDMA/cma: Add support for AF_IB to rdma_resolve_addr()") Signed-off-by: Avihai Horon <avihaih@nvidia.com> Reviewed-by: Mark Zhang <markzhang@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com> --- drivers/infiniband/core/cma.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 835ac54d4a24..b669002c9255 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -766,6 +766,7 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv) unsigned int p; u16 pkey, index; enum ib_port_state port_state; + int ret; int i; cma_dev = NULL; @@ -784,9 +785,16 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv) if (ib_get_cached_port_state(cur_dev->device, p, &port_state)) continue; - for (i = 0; !rdma_query_gid(cur_dev->device, - p, i, &gid); - i++) { + + for (i = 0; i < cur_dev->device->port_data[p].immutable.gid_tbl_len; + ++i) { + ret = rdma_query_gid(cur_dev->device, p, i, + &gid); + if (ret == -ENOENT) + continue; + if (ret) + break; + if (!memcmp(&gid, dgid, sizeof(gid))) { cma_dev = cur_dev; sgid = gid; -- 2.33.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() continue search even after empty entry 2021-11-22 11:53 ` [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() " Leon Romanovsky @ 2021-12-07 18:44 ` Jason Gunthorpe 0 siblings, 0 replies; 9+ messages in thread From: Jason Gunthorpe @ 2021-12-07 18:44 UTC (permalink / raw) To: Leon Romanovsky Cc: Doug Ledford, Avihai Horon, linux-kernel, linux-rdma, Mark Zhang On Mon, Nov 22, 2021 at 01:53:58PM +0200, Leon Romanovsky wrote: > From: Avihai Horon <avihaih@nvidia.com> > > Currently, when cma_resolve_ib_dev() searches for a matching GID it will > stop searching after encountering the first empty GID table entry. This > behavior is wrong since neither IB nor RoCE spec enforce tightly packed > GID tables. > > For example, when the matching valid GID entry exists at index N, and if > a GID entry is empty at index N-1, cma_resolve_ib_dev() will fail to > find the matching valid entry. > > Fix it by making cma_resolve_ib_dev() continue searching even after > encountering missing entries. > > Fixes: f17df3b0dede ("RDMA/cma: Add support for AF_IB to rdma_resolve_addr()") > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > Reviewed-by: Mark Zhang <markzhang@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > drivers/infiniband/core/cma.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 835ac54d4a24..b669002c9255 100644 > +++ b/drivers/infiniband/core/cma.c > @@ -766,6 +766,7 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv) > unsigned int p; > u16 pkey, index; > enum ib_port_state port_state; > + int ret; > int i; > > cma_dev = NULL; > @@ -784,9 +785,16 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv) > > if (ib_get_cached_port_state(cur_dev->device, p, &port_state)) > continue; > - for (i = 0; !rdma_query_gid(cur_dev->device, > - p, i, &gid); > - i++) { > + > + for (i = 0; i < cur_dev->device->port_data[p].immutable.gid_tbl_len; > + ++i) { > + ret = rdma_query_gid(cur_dev->device, p, i, > + &gid); > + if (ret == -ENOENT) > + continue; > + if (ret) > + break; Same here, just always continue Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-12-09 0:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-22 11:53 [PATCH rdma-next 0/3] Skip holes in GID tables Leon Romanovsky 2021-11-22 11:53 ` [PATCH rdma-next 1/3] RDMA/core: Modify rdma_query_gid() to return accurate error codes Leon Romanovsky 2021-12-07 18:41 ` Jason Gunthorpe 2021-11-22 11:53 ` [PATCH rdma-next 2/3] RDMA/core: Let ib_find_gid() continue search even after empty entry Leon Romanovsky 2021-12-07 18:43 ` Jason Gunthorpe 2021-12-08 6:51 ` Leon Romanovsky 2021-12-09 0:13 ` Jason Gunthorpe 2021-11-22 11:53 ` [PATCH rdma-next 3/3] RDMA/cma: Let cma_resolve_ib_dev() " Leon Romanovsky 2021-12-07 18:44 ` Jason Gunthorpe
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).