netdev - Re: [RFC 02/11] Add RoCE driver framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160914130032.GB26069@leon.nu>
Date:   Wed, 14 Sep 2016 16:00:32 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     "Mintz, Yuval" <Yuval.Mintz@...ium.com>
Cc:     Yuval Mintz <Yuval.Mintz@...gic.com>,
        Mark Bloch <markb@...lanox.com>,
        Ram Amrani <Ram.Amrani@...gic.com>,
        "dledford@...hat.com" <dledford@...hat.com>,
        David Miller <davem@...emloft.net>,
        Ariel Elior <Ariel.Elior@...gic.com>,
        Michal Kalderon <Michal.Kalderon@...gic.com>,
        Rajesh Borundia <rajesh.borundia@...gic.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [RFC 02/11] Add RoCE driver framework

On Wed, Sep 14, 2016 at 08:15:23AM +0000, Mintz, Yuval wrote:
> > > >> >> +uint debug;
> > > >> >> +module_param(debug, uint, 0);
> > > > >>> +MODULE_PARM_DESC(debug, "Default debug msglevel");
> > > >>
> > > >> >Why are you adding this as a module parameter?
> > > >>
> > > >>  I believe this is mostly to follow same line as qede which also defines
> > > > > 'debug' module parameter for allowing easy user control of debug
> > > > > prints [& specifically for probe prints, which can't be controlled
> > > > > otherwise].
> > >
> > > > Can you give us an example where dynamic debug and tracing infrastructures
> > > > are not enough?
> > >
> > > > AFAIK, most of these debug module parameters are legacy copy/paste
> > > > code which is useless in real life scenarios.
> > >
> > > Define 'enough'; Using dynamic debug you can provide all the necessary
> > > information and at an even better granularity that's achieved by suggested
> > > infrastructure,  but is harder for an end-user to use. Same goes for tracing.
> > >
> > > The 'debug' option provides an easy grouping for prints related to a specific
> > > area in the driver.
> >
> > It is hard to agree with you that user which knows how-to load modules
> > with parameters won't success to enable debug prints.
>
> I think you're giving too much credit to the end-user. :-D
>
> > In addition, global increase in debug level for whole driver will create
> > printk storm in dmesg and give nothing to debuggability.
>
> So basically, what you're claiming is that ethtool 'msglvl' setting for devices
> is completely obselete. While this *might* be true, we use it extensively
> in our qede and qed drivers; The debug module parameter merely provides
> a manner of setting the debug value prior to initial probe for all interfaces.
> qedr follows the same practice.

Thanks for this excellent example. Ethtool 'msglvl' adds this
dynamically, while your DEBUG argument works for loading module
only.

If you want dynamic prints, you have two options:
1. Add support of ethtool to whole RDMA stack.
2. Use dynamic tracing infrastructure.

Which option do you prefer?

>

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)