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

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

 



On Fri, Sep 29, 2017 at 08:27:08AM -0700, Vishwanathapura, Niranjana wrote:
> You are putting a wrong interpretation to this patch.
> EM provides the encapsulation configuration as in the opa_vnic documentation.
> EM will be opensourced when it is ready (VNIC is development in progress).
>
> As mentioned, this patch is a simple *debug* hook to that encapsulation
> configuration and no more.
> That is why it is put under DEBUGFS.
>
> I do see valid reason in Jason's and your earlier comments that it should a
> formal support to the admins.
> But this patch was not mainly for that. I don't expect DEBUGFS to be turned
> on in all deployments.
> I understand the question whether or not this patch should be in the kernel
> tree.
> But it is a useful debug hook for debugging/triaging issues (similar to NIC
> having debug hook read/write some register), and convenient to have it
> available in the kernel than maintaining it separately.

I want to summarize the responses:
1. If you want to know (read) which vport is configured by EM, the
tracepoints/prints will be enough to achieve it and no need to provide
any debugfs.
2. If you want to debug and configure (write) the vport, you should answer on two
questions.
2.a. Will it be local change and won't be propagated to EM?
2.b. Will this change be propagated to EM?

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.

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.

There is no need to add "one-time" interface to clean the code.

Thanks

>
> Niranjana
>
> > Thanks
>
>

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