Re: [PATCH rdma-next] Revert "IB/core: Add flow control to the portmapper netlink calls"

Chien Tin Tung
2017-06-06T18:17:41Z

On Mon, Jun 05, 2017 at 11:08:16AM -0400, Doug Ledford wrote:
> On Mon, 2017-06-05 at 09:03 +0300, Leon Romanovsky wrote:
> > On Sun, Jun 04, 2017 at 11:50:43PM -0500, Chien Tin Tung wrote:
> Leon is not a native english speaker, as I assume you aren't.  I would
> suggest that it might be more about failure to understand each other
> and failure to get at the root of the issue than anything that would
> legitimately do damage to anyone's credibility.  You might wish to calm
> down and read my posts in this thread.  I hope I was able to clarify
> things a bit.

Alright, this is my attempt at world peace.

I started with a long email but decided to keep it short. Please
ask for details if you have questions and remember to keep an open mind.

Here are some links for those interested in understanding the background
leading to this thread:

* Tatyana Nikolova's original commit 30dc5e63d6a5 which introduced
ibnl_unicast()
* Leon's comment to Mustafa's original proposal -
https://patchwork.kernel.org/patch/9235137/
* Mustafa Ismail's accepted patch, cea05eadded0d, to ibnl_unicast()

Netlink is fundamentally sockets. You can substitute the word socket
for Netlink when you try to get a sense of Netlink functionality. In
the RDMA subsystem, we use Netlink to communicate between user-space
and kernel.

Current controversy surrounds ibnl_unicast(). It is used to send NL
message from the kernel to the user and the NL flow can be triggered
from both user-space and kernel code.

ibnl_unicast() uses netlink_unicast with the following signature:

int netlink_unicast(struct sock *ssk, struct sk_buff *skb,
u32 portid, int nonblock)

The last parameter, nonblock, is important. It allows the caller to
control the call to wait and retry on failure, provided a timeout value
is set, or not block even with a timeout value set. The actual mechanism
is very elegant and can be used to solve the issue at hand.

Let me go back to the problem Mustafa tried to solve. Specifically it
is an issue with the call to iwpm_add_mapping() failing. That function
sends a NL message from the kernel to user-space portmapper daemon
using the kernel NL socket. Mustafa originally proposed to
have two ibnl_unicast functions, one blocking and the other
one non-blocking. It was shot down by Leon. In hindsight, we should of
pushed ahead and implemented those two functions. The blocking function
for iwpm_add_mapping() and the non-blocking function for any NL
interaction from user-space. This solution can still be implemented
solving everyone's problem and bring world peace. :-)

Chien