Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

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

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

On Mon, Oct 02, 2017 at 08:47:08PM -0400, Dennis Dalessandro wrote:
> On 10/2/2017 12:23 AM, Leon Romanovsky wrote:
> > Upstream is not a development playground and you should submit your code once
> > you think that it is ready. So we are assuming that VNIC is working and
> > you are interested to debug your EM and such code doesn't belong to the kernel.
>
> This patch aside, I do have to take some issue with this statement. While
> upstream is not a "playground" we should be submitting code early. I keep
> harping on iterative development, show your work. Granted that doesn't mean
> write broken code and toss it over the fence to kernel.org, but pieces that
> can be broken up and tested on their own are best.

We are talking about the same. Large feature (VNIC) was divided to
sub-tasks like patches and maybe sub-features. Those sub-features were
submitted early (iterative development), which is good and sounds right
for me too.

But the expectation is that these building blocks are tested and working
and you don't need to debug them.

For sure, there are always exceptions to the statements above and
various counters, states, dumps e.t.c are good examples for
such exceptions, because they add visibility to the system, but they
don't change the system.

>
> > If you still insist on 2.a, the solution should be in your company: add
> > debugfs locally, write tests, find and fix bugs and submit them to
> > upstream.
>
> Now back to this patch series. debugfs vs NetLink, I don't think it really
> matters if the rationale for having the patch in the first place is wrong.
> In other words you would have still opposed this even if it were NetLink I
> assume? We are discussing internally but for now I think this series can be
> dropped.

I am more than happy to see OPA-VNIC configurations in NetLink, but
don't forget that it will be part of ABI and users will use it, so you
need to ensure that the OPA-VNIC network won't die after one of the
nodes will change vport value.

Before you are rushing to implement it via netlink, you should ask
yourself WHY this functionality is needed for the users.

>
> > There is no need to add "one-time" interface to clean the code.
>
> That I would agree to.
>
> -Denny

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