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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 30, 2017 at 04:24:31PM -0500, Shiraz Saleem wrote:
> On Mon, May 29, 2017 at 11:24:23AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > The commit cea05eadded0 ("IB/core: Add flow control to the portmapper netlink calls")
> > changed netlink to be blocked for all RDMA clients. This workaround
> > worked perfectly for portmapper, but is not correct for the whole
> > NETLINK_RDMA family.
> >
> > The request/response should always be blocking and asynchronous
> > notification should always be non-blocking.
> >
> > It is library and user-space application to chose how to handle recvmgs,
> > as an example see nl_recvmsgs() and nl_socket_set_nonblocking() calls of
> > libnl library.
> >
> > Send timeout is not needed too and can be configured with SO_SNDTIMEO socket
> > option.
> >
> > This reverts commit cea05eadded0d4eb59f7be6e1f1560eb6bfde2bf.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> This commit was accepted with much feedback from you.
>
> https://patchwork.kernel.org/patch/9235137/
>
> Not clear from your description to determine what your trying to fix.
>
> Simply reverting a commit that solves a real problem is not acceptable.
>
> Please come up with a solution without breaking portmapper.

1. It was cosmetic comments, I didn't give my reviewed-by tag, so please
don't put blame on me,
2. The commit cea05eadde broke NETLINK semantics for whole RDMA.
3. The commit cea05eadde made libnl library (basic block of user-space part of netlink)
to work incorrectly and not according to _blocking/_nonblocking semantics.
4. Reverting is a common practice in Linux kernel. Patches are not
carved in stones.
5. I proposed a solution -> go and fix your user space program.

Thanks
>
> Shiraz

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux