Re: [PATCH rdma-next V2 5/5] IB/core: Integrate IB address resolution module into core

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

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

On Mon, May 16, 2016 at 04:27:05PM -0400, Doug Ledford wrote:
> On 05/16/2016 02:54 PM, Leon Romanovsky wrote:
> > On Mon, May 16, 2016 at 01:42:34PM -0400, Doug Ledford wrote:
> >> On 05/16/2016 12:30 PM, Leon Romanovsky wrote:
> >>> On Mon, May 16, 2016 at 11:09:44AM -0400, Doug Ledford wrote:
> >>>> On 05/15/2016 06:51 AM, Mark Bloch wrote:
> >>>>
> >>>>>> But, just as importantly, after reading addr.c to see how it uses the ibnl
> >>>>>> infrastructure, I don't even see what the original problem can be.
> >>>>> As it stands today:
> >>>>> - ibnl is part of ib_core.
> >>>>> - ib_core needs ib_addr.
> >>>>>
> >>>>> So if we add ibnl usage to ib_addr it means ib_addr will need ib_core,
> >>>>> which causes a dependency cycle.
> >>>>
> >>>> Right, but in that case, this patch needs to be part of the series that
> >>>> adds the ibnl support into the ib_addr functionality.  Because you split
> >>>> them into separate series, this was a patch looking for a problem to
> >>>> solve and it wasn't clear what it was.  If I had taken the other series
> >>>> and not this series, it would have broken things.  So please keep
> >>>> patches like this together with the other patches that depend on it.
> >>>>
> >>>> That said, I also don't want to redo modules if we don't have to.  As my
> >>>> previous email points out, changing modules breaks init scripts and
> >>>> systemd unit files.  It is to be avoided when possible.
> >>>
> >>> Sorry,
> >>> I was in the mood of fixing things when I wrote and sent this patch.
> >>> The question is which version will you more likely to accept: this one
> >>> (remove ib_addr module) or previous one (add ib_netlink module)?
> >>
> >> Can you build netlink in and then init the ib_addr module after the
> >> netlink init is complete?  Wouldn't that resolve the dependency ordering
> >> issue without changing the module names?
> > 
> > It seems reasonable and we will test it, before reposting.
> 
> Thanks.
> 
> > However generally speaking, I agree with Jason and Ira that this module
> > (ib_addr.ko) is useless as module and can be part of ib_core.ko.
> > 
> > It doesn't seem as a big deal to fix all that init scripts (remove two
> > lines).
> 
> It's not removing two lines.  If someone wants to be able to boot both
> kernels prior to this change and kernel post this change, they have to
> now write their script to handle both conditions sanely.  And they have
> to know to do so, where as very often they simply get caught off guard
> and things are broken for some period of time.  If the change to ib_addr
> actually solved a problem that couldn't be solved otherwise, that would
> be one thing.  But it doesn't.  It's a complete no-op.

I think that your position is too tight for not allowing modules
restructure. It looks like that this complexity doesn't exist outside
testing scripts.

For my testings, I'm using Debian Sid with inbox scripts/packages and it handles
correctly IB stack with different kernels and different set of modules.

> 
> 
> -- 
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>               GPG KeyID: 0E572FDD
> 
> 



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]