* New GID query API broke EFA
@ 2020-10-22 10:58 Gal Pressman
2020-10-22 11:21 ` Leon Romanovsky
0 siblings, 1 reply; 9+ messages in thread
From: Gal Pressman @ 2020-10-22 10:58 UTC (permalink / raw)
To: Leon Romanovsky, Avihai Horon, Maor Gottlieb, Jason Gunthorpe
Cc: linux-rdma, Leybovich, Yossi
Hi all,
The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query
API to user space") currently breaks EFA, as ibv_query_gid() no longer works.
The problem is that the IOCTL call checks for:
if (!rdma_ib_or_roce(ib_dev, port_num))
return -EOPNOTSUPP;
EFA is neither of these, but it uses GIDs.
Any objections to remove the check? Any other solutions come to mind?
Gal
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: New GID query API broke EFA 2020-10-22 10:58 New GID query API broke EFA Gal Pressman @ 2020-10-22 11:21 ` Leon Romanovsky 2020-10-22 11:49 ` Gal Pressman 2020-10-22 12:10 ` Jason Gunthorpe 0 siblings, 2 replies; 9+ messages in thread From: Leon Romanovsky @ 2020-10-22 11:21 UTC (permalink / raw) To: Gal Pressman Cc: Avihai Horon, Maor Gottlieb, Jason Gunthorpe, linux-rdma, Leybovich, Yossi On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: > Hi all, > > The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query > API to user space") currently breaks EFA, as ibv_query_gid() no longer works. > > The problem is that the IOCTL call checks for: > if (!rdma_ib_or_roce(ib_dev, port_num)) > return -EOPNOTSUPP; > > EFA is neither of these, but it uses GIDs. > > Any objections to remove the check? Any other solutions come to mind? We added this check to protect access to rdma_get_gid_attr() for devices without GID table. 1234 table = rdma_gid_table(device, port_num); 1235 if (index < 0 || index >= table->sz) 1236 return ERR_PTR(-EINVAL); So you can extend that function to return for table == NULL an error and remove rdma_ib_or_roce() Thanks > > Gal ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 11:21 ` Leon Romanovsky @ 2020-10-22 11:49 ` Gal Pressman 2020-10-22 12:10 ` Jason Gunthorpe 1 sibling, 0 replies; 9+ messages in thread From: Gal Pressman @ 2020-10-22 11:49 UTC (permalink / raw) To: Leon Romanovsky Cc: Avihai Horon, Maor Gottlieb, Jason Gunthorpe, linux-rdma, Leybovich, Yossi On 22/10/2020 14:21, Leon Romanovsky wrote: > On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: >> Hi all, >> >> The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query >> API to user space") currently breaks EFA, as ibv_query_gid() no longer works. >> >> The problem is that the IOCTL call checks for: >> if (!rdma_ib_or_roce(ib_dev, port_num)) >> return -EOPNOTSUPP; >> >> EFA is neither of these, but it uses GIDs. >> >> Any objections to remove the check? Any other solutions come to mind? > > We added this check to protect access to rdma_get_gid_attr() for devices > without GID table. > 1234 table = rdma_gid_table(device, port_num); > 1235 if (index < 0 || index >= table->sz) > 1236 return ERR_PTR(-EINVAL); > > So you can extend that function to return for table == NULL an error and > remove rdma_ib_or_roce() Thanks Leon, I will send a patch. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 11:21 ` Leon Romanovsky 2020-10-22 11:49 ` Gal Pressman @ 2020-10-22 12:10 ` Jason Gunthorpe 2020-10-22 12:12 ` Gal Pressman 2020-10-22 12:23 ` Leon Romanovsky 1 sibling, 2 replies; 9+ messages in thread From: Jason Gunthorpe @ 2020-10-22 12:10 UTC (permalink / raw) To: Leon Romanovsky Cc: Gal Pressman, Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: > On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: > > Hi all, > > > > The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query > > API to user space") currently breaks EFA, as ibv_query_gid() no longer works. > > > > The problem is that the IOCTL call checks for: > > if (!rdma_ib_or_roce(ib_dev, port_num)) > > return -EOPNOTSUPP; > > > > EFA is neither of these, but it uses GIDs. > > > > Any objections to remove the check? Any other solutions come to mind? > > We added this check to protect access to rdma_get_gid_attr() for devices > without GID table. > 1234 table = rdma_gid_table(device, port_num); > 1235 if (index < 0 || index >= table->sz) > 1236 return ERR_PTR(-EINVAL); > > So you can extend that function to return for table == NULL an error and > remove rdma_ib_or_roce() How does table == NULL ever? Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 12:10 ` Jason Gunthorpe @ 2020-10-22 12:12 ` Gal Pressman 2020-10-22 12:15 ` Gal Pressman 2020-10-22 12:23 ` Leon Romanovsky 1 sibling, 1 reply; 9+ messages in thread From: Gal Pressman @ 2020-10-22 12:12 UTC (permalink / raw) To: Jason Gunthorpe, Leon Romanovsky Cc: Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On 22/10/2020 15:10, Jason Gunthorpe wrote: > On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: >> On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: >>> Hi all, >>> >>> The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query >>> API to user space") currently breaks EFA, as ibv_query_gid() no longer works. >>> >>> The problem is that the IOCTL call checks for: >>> if (!rdma_ib_or_roce(ib_dev, port_num)) >>> return -EOPNOTSUPP; >>> >>> EFA is neither of these, but it uses GIDs. >>> >>> Any objections to remove the check? Any other solutions come to mind? >> >> We added this check to protect access to rdma_get_gid_attr() for devices >> without GID table. >> 1234 table = rdma_gid_table(device, port_num); >> 1235 if (index < 0 || index >= table->sz) >> 1236 return ERR_PTR(-EINVAL); >> >> So you can extend that function to return for table == NULL an error and >> remove rdma_ib_or_roce() > > How does table == NULL ever? A driver with gid_tbl_len == 0 would make the allocation return NULL, no? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 12:12 ` Gal Pressman @ 2020-10-22 12:15 ` Gal Pressman 2020-10-22 12:23 ` Leon Romanovsky 0 siblings, 1 reply; 9+ messages in thread From: Gal Pressman @ 2020-10-22 12:15 UTC (permalink / raw) To: Jason Gunthorpe, Leon Romanovsky Cc: Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On 22/10/2020 15:12, Gal Pressman wrote: > On 22/10/2020 15:10, Jason Gunthorpe wrote: >> On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: >>> On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: >>>> Hi all, >>>> >>>> The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query >>>> API to user space") currently breaks EFA, as ibv_query_gid() no longer works. >>>> >>>> The problem is that the IOCTL call checks for: >>>> if (!rdma_ib_or_roce(ib_dev, port_num)) >>>> return -EOPNOTSUPP; >>>> >>>> EFA is neither of these, but it uses GIDs. >>>> >>>> Any objections to remove the check? Any other solutions come to mind? >>> >>> We added this check to protect access to rdma_get_gid_attr() for devices >>> without GID table. >>> 1234 table = rdma_gid_table(device, port_num); >>> 1235 if (index < 0 || index >= table->sz) >>> 1236 return ERR_PTR(-EINVAL); >>> >>> So you can extend that function to return for table == NULL an error and >>> remove rdma_ib_or_roce() >> >> How does table == NULL ever? > > A driver with gid_tbl_len == 0 would make the allocation return NULL, no? Nevermind, we don't really allocate with gid_tbl_len as the size, and any allocation failure will fail ib_register_device(). So I guess there is no way for it to be NULL. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 12:15 ` Gal Pressman @ 2020-10-22 12:23 ` Leon Romanovsky 0 siblings, 0 replies; 9+ messages in thread From: Leon Romanovsky @ 2020-10-22 12:23 UTC (permalink / raw) To: Gal Pressman Cc: Jason Gunthorpe, Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On Thu, Oct 22, 2020 at 03:15:55PM +0300, Gal Pressman wrote: > On 22/10/2020 15:12, Gal Pressman wrote: > > On 22/10/2020 15:10, Jason Gunthorpe wrote: > >> On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: > >>> On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: > >>>> Hi all, > >>>> > >>>> The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query > >>>> API to user space") currently breaks EFA, as ibv_query_gid() no longer works. > >>>> > >>>> The problem is that the IOCTL call checks for: > >>>> if (!rdma_ib_or_roce(ib_dev, port_num)) > >>>> return -EOPNOTSUPP; > >>>> > >>>> EFA is neither of these, but it uses GIDs. > >>>> > >>>> Any objections to remove the check? Any other solutions come to mind? > >>> > >>> We added this check to protect access to rdma_get_gid_attr() for devices > >>> without GID table. > >>> 1234 table = rdma_gid_table(device, port_num); > >>> 1235 if (index < 0 || index >= table->sz) > >>> 1236 return ERR_PTR(-EINVAL); > >>> > >>> So you can extend that function to return for table == NULL an error and > >>> remove rdma_ib_or_roce() > >> > >> How does table == NULL ever? > > > > A driver with gid_tbl_len == 0 would make the allocation return NULL, no? > > Nevermind, we don't really allocate with gid_tbl_len as the size, and any > allocation failure will fail ib_register_device(). > So I guess there is no way for it to be NULL. I think so too. Thanks ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 12:10 ` Jason Gunthorpe 2020-10-22 12:12 ` Gal Pressman @ 2020-10-22 12:23 ` Leon Romanovsky 2020-10-22 16:37 ` Jason Gunthorpe 1 sibling, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2020-10-22 12:23 UTC (permalink / raw) To: Jason Gunthorpe Cc: Gal Pressman, Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On Thu, Oct 22, 2020 at 09:10:35AM -0300, Jason Gunthorpe wrote: > On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: > > On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: > > > Hi all, > > > > > > The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query > > > API to user space") currently breaks EFA, as ibv_query_gid() no longer works. > > > > > > The problem is that the IOCTL call checks for: > > > if (!rdma_ib_or_roce(ib_dev, port_num)) > > > return -EOPNOTSUPP; > > > > > > EFA is neither of these, but it uses GIDs. > > > > > > Any objections to remove the check? Any other solutions come to mind? > > > > We added this check to protect access to rdma_get_gid_attr() for devices > > without GID table. > > 1234 table = rdma_gid_table(device, port_num); > > 1235 if (index < 0 || index >= table->sz) > > 1236 return ERR_PTR(-EINVAL); > > > > So you can extend that function to return for table == NULL an error and > > remove rdma_ib_or_roce() > > How does table == NULL ever? ok, you are right in that regards. However, mlx5 IB representors don't support GIDs and sets gid_tbl_len to be zero. Do we want to rely on the assumption that table will always exist? Thanks > > Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: New GID query API broke EFA 2020-10-22 12:23 ` Leon Romanovsky @ 2020-10-22 16:37 ` Jason Gunthorpe 0 siblings, 0 replies; 9+ messages in thread From: Jason Gunthorpe @ 2020-10-22 16:37 UTC (permalink / raw) To: Leon Romanovsky Cc: Gal Pressman, Avihai Horon, Maor Gottlieb, linux-rdma, Leybovich, Yossi On Thu, Oct 22, 2020 at 03:23:01PM +0300, Leon Romanovsky wrote: > On Thu, Oct 22, 2020 at 09:10:35AM -0300, Jason Gunthorpe wrote: > > On Thu, Oct 22, 2020 at 02:21:00PM +0300, Leon Romanovsky wrote: > > > On Thu, Oct 22, 2020 at 01:58:29PM +0300, Gal Pressman wrote: > > > > Hi all, > > > > > > > > The new IOCTL query GID API 9f85cbe50aa0 ("RDMA/uverbs: Expose the new GID query > > > > API to user space") currently breaks EFA, as ibv_query_gid() no longer works. > > > > > > > > The problem is that the IOCTL call checks for: > > > > if (!rdma_ib_or_roce(ib_dev, port_num)) > > > > return -EOPNOTSUPP; > > > > > > > > EFA is neither of these, but it uses GIDs. > > > > > > > > Any objections to remove the check? Any other solutions come to mind? > > > > > > We added this check to protect access to rdma_get_gid_attr() for devices > > > without GID table. > > > 1234 table = rdma_gid_table(device, port_num); > > > 1235 if (index < 0 || index >= table->sz) > > > 1236 return ERR_PTR(-EINVAL); > > > > > > So you can extend that function to return for table == NULL an error and > > > remove rdma_ib_or_roce() > > > > How does table == NULL ever? > > ok, you are right in that regards. > However, mlx5 IB representors don't support GIDs and sets gid_tbl_len to be zero. > Do we want to rely on the assumption that table will always exist? We do already, seems like the rdma_ib_or_roce should just be deleted Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-22 16:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-22 10:58 New GID query API broke EFA Gal Pressman 2020-10-22 11:21 ` Leon Romanovsky 2020-10-22 11:49 ` Gal Pressman 2020-10-22 12:10 ` Jason Gunthorpe 2020-10-22 12:12 ` Gal Pressman 2020-10-22 12:15 ` Gal Pressman 2020-10-22 12:23 ` Leon Romanovsky 2020-10-22 12:23 ` Leon Romanovsky 2020-10-22 16:37 ` 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).