On 28/8/2014 8:48 PM, Devesh Sharma wrote:
Hi Matan,
Thanks for quick response, my further comments are inline below:
-----Original Message-----
From: Matan Barak [mailto:mat...@mellanox.com]
Sent: Thursday, August 28, 2014 9:51 PM
To: Devesh Sharma; Jason Gunthorpe; Or Gerlitz
Cc: Doug Ledford; Roland Dreier; Yishai Hadas; linux-rdma@vger.kernel.org
Subject: Re: [PATCH libibverbs V5 2/2] Use neighbour lookup for RoCE UD
QPs Eth L2 resolution
On 28/8/2014 12:48 PM, Devesh Sharma wrote:
Hi Matan,
I have been watching this thread for quite some time. I have a Basic
question, do you think ib_uverbs_create_ah() in uverbs_cmd.c Should
resolve to l2 address? Presently it is not calling
rdma_addr_find_dmac_by_grh(), am I missing something here?
-Regards
Devesh
Hi Devesh,
Some vendors don't call ib_uverbs_create_ah and do all this creation in
userspace only. It's true that it might be a lot easier to do that resolution in
kernel, but it could create dependency of new versions of libibverbs and the
Which dependency you are specifying here, I see the scheme posted in this
series adds dependency on libibverbs.
Do you anticipate modification even when uverbs interface is used?
If we had resolved the address handle in kernel space, we would have
created a dependency between the user libraries to and new version odf
the uverbs module. The scheme posted here only adds a dependency between
the provider's library to libibverbs.
Resolving in kernel space would still require creating a dependency on a
new provider library - as currently some providers don't even call the
kernel when a new address handle is created.
provider library tn new kernels only.
I would like to avoid creating such a dependency.
Okay, got it, any suggestions for those who use uverbs interface.
I think another patch is needed for such vendors?
There are 2 options here - if all the required information (including
the MAC address) is contained in the address handle user-space
structure, you could just use the proposed method. However, if you
register some kind of an address handle with the hardware and then only
use it via user-space, we'll need another patch for uverbs.
Matan
-----Original Message-----
From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
ow...@vger.kernel.org] On Behalf Of Jason Gunthorpe
Sent: Tuesday, August 26, 2014 9:39 PM
To: Or Gerlitz
Cc: Doug Ledford; Matan Barak; Roland Dreier; Yishai Hadas; linux-
r...@vger.kernel.org
Subject: Re: [PATCH libibverbs V5 2/2] Use neighbour lookup for RoCE
UD QPs Eth L2 resolution
On Tue, Aug 26, 2014 at 03:18:37PM +0300, Or Gerlitz wrote:
On 25/08/2014 22:33, Doug Ledford wrote:
On Aug 25, 2014, at 1:33 PM, Jason Gunthorpe
<jguntho...@obsidianresearch.com> wrote:
+ timer_fd = timerfd_create(CLOCK_MONOTONIC,
TFD_NONBLOCK | TFD_CLOEXEC);
+ if (-1 == timer_fd) {
+ print_err("Couldn't create timer\n");
+ return timer_fd;
+ }
The use of timerfd will impact the minimum OS version, have you
checked this is OK? Does RHEL5 still work?
It was added in linux v2.6.25. I think that an API that's more
than
6.5 years old is valid.
RHEL5 is using 2.6.18 as their base kernel. You should at least
consult with the OFED people to determine if this is a problem for
them.
Please don't. This code should not be changed for something as
ancient
as rhel5.
Indeed. Telling people to avoid using constructs/mechanisms ~6-7
years after they were introduced isn't something we want nor need to
do.
I looked myself and it looks like OFED has dropped support for these
old distros so there isn't any problem.
However, I still think this use of timerfd is fairly gratuitous, and
looking closer, causes little bugs:
+ if (timerfd_settime(timer_fd, 0, &timer_time, NULL)) {
+ print_err("Couldn't set timer\n");
+ return -1;
^^^^^^^^^^^^^^
leaks timer_fd
Alos, I noticed:
+ /* wait for an incoming message on the netlink socket */
+ ret = select(nfds, &fdset, NULL, NULL, NULL);
+
+ if (ret) {
Fails to detect error return from select.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma"
in the body of a message to majord...@vger.kernel.org More
majordomo
info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
