* 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: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: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: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).