On 12/12/19 9:14 AM, Mark Haywood wrote:
On 12/12/19 7:37 AM, Håkon Bugge wrote:
On 12 Dec 2019, at 13:27, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
On Thu, Dec 12, 2019 at 01:16:54PM +0100, Håkon Bugge wrote:
On 12 Dec 2019, at 13:10, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
On Thu, Dec 12, 2019 at 12:59:51PM +0100, Håkon Bugge wrote:
On 12 Dec 2019, at 12:40, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
On Wed, Dec 11, 2019 at 08:31:18PM +0100, Håkon Bugge wrote:
On 11 Dec 2019, at 14:13, Håkon Bugge
<haakon.bugge@xxxxxxxxxx> wrote:
On 11 Dec 2019, at 13:39, Leon Romanovsky <leon@xxxxxxxxxx>
wrote:
On Wed, Dec 11, 2019 at 11:34:00AM +0100, Håkon Bugge wrote:
In rdma_nl_rcv_skb(), the local variable err is assigned the
return
value of the supplied callback function, which could be one of
ib_nl_handle_resolve_resp(), ib_nl_handle_set_timeout(), or
ib_nl_handle_ip_res_resp(). These three functions all return
skb->len
on success.
rdma_nl_rcv_skb() is merely a copy of netlink_rcv_skb(). The
callback
functions used by the latter have the convention: "Returns 0 on
success or a negative error code".
In particular, the statement (equal for both functions):
if (nlh->nlmsg_flags & NLM_F_ACK || err)
implies that rdma_nl_rcv_skb() always will ack a message,
independent
of the NLM_F_ACK being set in nlmsg_flags or not.
The more accurate description is that rdma_nl_rcv_skb()
always generates
NLMSG_ERROR without relation to NLM_F_ACK flag. The NLM_F_ACK
flag is
requested to get acknowledges for the success.
Yes. And when, lets say a legitimate path record response,
containing N positive bytes, is sent back from ibacm to the
kernel, rdma_nl_rcv_skb() think this is an error, due to "if
(nlh->nlmsg_flags & NLM_F_ACK || err)" _and_
ib_nl_handle_resolve_resp() returning N.
How did you test this patch?
Do we have open-source applications which don't set NLM_F_ACK for
ib_nl_*() calls?
As I alluded to above, yes, ibacm doesn't set it.
In this regards, I'm amazed that this patch didn't break ibacm.
On the contrary. The patch avoids the kernel sending back an
error/ACK for every path record / resolve response.
As long as ibacm continues to work with this patch, i'm ok.
What type of testing did you perform?
I'll let Mark respond to the testing. The background is that ibacm
was very *liberal* when it comes checking the requests it received
from the kernel. In an attempt to tighten that, Mark discovered that
ibacm received an unexpected ACK from the kernel just after having
sent a response.
Without this patch, for every response to a RDMA_NL_LS_OP_RESOLVE
request, ibacm receives an ACK with a nlmsgerr error value equal to
the length of the response message.
To be clear, ibacm does nothing with the ACK other than write a log
message that it received an unexpected request. That is why this patch
has no ill-effect on ibacm and is why ...
With this patch, no ACKs are received.
this is preferable.
If I add the NLM_F_ACK to the nlmsg_header flags when responding to
the RDMA_NL_LS_OP_RESOLVE request, ibacm once again receives the ACKs.
Mark
That aside, I think the RDMA NL callbacks shall adhere to the
RTNETLINK conventions, thus, that's why this commit changes the
callbacks and not the rdma_nl_rcv_skb().
Thxs, Håkon
Thanks
Håkon
Thanks
