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

Leon Romanovsky
2017-06-01T04:10:22Z

[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]

On Wed, May 31, 2017 at 12:42:45PM -0500, Shiraz Saleem wrote:
> On Wed, May 31, 2017 at 07:04:37AM +0300, Leon Romanovsky wrote:
> > 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-Nfu5REtnQAJWk0Htik3J/w@public.gmane.org>
> > > >
>
> > 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.
>
> How? Is libnl calling ibnl_unicast? As far we can understand ibnl_unicast is only called
> by portmapper kernel code.

Yes, libnl is calling to ibnl_unicast() and this is why it brought my
attention to regression caused by commit which I'm reverting.

Bottom-up flow:
ibnl_unicast
send_nlmsg_done
iwpm_send_mapinfo
iwpm_mapping_info_cb
[RDMA_NL_IWPM_MAPINFO] = {.dump = iwpm_mapping_info_cb}
ibnl_rcv_msg
netlink_rcv_skb(skb, &ibnl_rcv_msg);
-----
libnl:
nl_recvmsgs()

>
> > 4. Reverting is a common practice in Linux kernel. Patches are not
> > carved in stones.
>
> Reverting a patch that's introduced during RC cycle is fine, introducing
> regression is NOT and that is what you are doing by simply proposing to revert
> this patch. Reverting this patch will introduce a REGRESSION error with respect to
> port mapping functionality for all iWARP vendors.

Interesting and how did all these iWARP vendors survive before your
patch?

>
> > 5. I proposed a solution -> go and fix your user space program.
> This is a kernel patch you are trying to revert, you are breaking existing
> kernel functionality. Nothing to do with user space.

Sorry, but it is not the case. The user space is broken.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]