Re: [PATCH RDMA/netlink] RDMA/netlink: Adhere to returning zero on success

[Leon Romanovsky <leon@kernel.org>]

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@oracle.com> wrote:
> >
> >
> >
> >> On 11 Dec 2019, at 13:39, Leon Romanovsky <leon@kernel.org> 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?

Thanks

>
> 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
> >
>