> 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. Thxs, Håkon >> >>> >>> The fix could be to change the above statement, but it is better to >>> keep the two *_rcv_skb() functions equal in this respect and instead >>> change the callback functions in the rdma subsystem to the correct >>> convention. >> >> AFAIR, RTNETLINK has the same implementation as RDMA netlink. > > With the exception of the callback functions, as noted above. > > > Thxs, Håkon > >> >> Thanks >
