Re: [PATCH V2 08/10] i40iw: Control debug error prints using env variable

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2335 bytes --]

On 12/14/2016 4:21 PM, Jason Gunthorpe wrote:
> On Wed, Dec 14, 2016 at 07:11:11PM +0200, Leon Romanovsky wrote:
> 
>>>> This patch duplicates already existing code in most of providers and libraries
>>>> in rdma-core, while two of our main goals for creating this consolidated
>>>> library were simplification for users and reduce code duplication.
>>>>
>>>> It will be very beneficial if you:
>>>> 1. Use and promote general pr_debug(..), srp_tools has nice piece of code,
>>>> to be general code.
>>>
>>> [Tatyana Nikolova] The debug/error printing macros available in
>>> rdma-core use different mechanisms to report information, for
>>> instance, they set/check one or more variables, or they use a bit
>>> mask to enable debug level. They also print to different outputs:
>>> stderr/stdout, debug files or syslog.
>>
>> At the end, all these prints are for debug. It is hard to see any
>> objections to see output from them in one place.
> 
> Yes, let us just use stderr for now for provider debugging. If someone
> wants syslog then that can be a later patch. It makes no sense that
> there are difference here.
> 
>> in rdma-core/util/env.h|c
>>
>> #define SET_VAR(type, var, field) \
>> 		(struct ##name*)env->field = get_env_var(...)
>>
>> void *get_env_vars(enum typ)
>> {
>> 	void *env;
>> 	switch(type) {
>> 	case I40W_ENV:
>> 		env = malloc(sizeof(struct i40w_env));
>> 		....
>> 		SET_VAR(i40w_env, "I40W_DEBUG", debug);
>> 		...
>> }
> 
> Why?
> 
> I was thinking more like a standard:
> 
>   VERBS_PROVIDER_DEBUG=qp,ah,blah
> 
> parser since other than mlx5 that is what providers use env vars for.
> 
> I'm not sure I agree at all with what mlx5 is doing with tuning
> parameters via env vars :\

Options (not necessarily tuning though) have long been enabled via env
vars for shared libraries.  That really isn't uncommon.  Env vars are
easy to set on a per-app or per-user or per-container basis.  The other
option, config files, are not so easy to separate out.

And one thing we might need this for in the future is to reserve QPs for
IPoIB use so we can actually have containers with consistent IPoIB
device hw addresses by using GID + Fixed QP number.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]