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 Thu, Jun 01, 2017 at 07:10:22AM +0300, Leon Romanovsky wrote:
> 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@xxxxxxxxxxxx>
> > > > >
> >
> > > 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()

Assuming your stack trace is correct, it is showing the normal use case of ibnl_unicate()
by portmapper from the iWARP side.  I'm still waiting on evidence of this patch breaking
rest of RDMA as you so claimed.

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

We are all still here. :-)  But if we let you simply revert a patch that fixes portmapper
for all iWARP vendors then we may be not.

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

Here is a better solution.  Post a patch to the kernel that will not introduce a regression
and fix whatever the probelm you think there is, then I will personally review the patch.

Chien
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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