netdev - Re: [PATCH iproute2-next v3 8/8] iplink: Reduce number of arguments to iplink

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]

Date:   Mon, 26 Feb 2018 22:09:03 +0200
From:   Serhey Popovych <serhe.popovych@...il.com>
To:     David Ahern <dsahern@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next v3 8/8] iplink: Reduce number of arguments
 to iplink_parse()

Serhey Popovych wrote:
> David Ahern wrote:
>> On 2/26/18 11:20 AM, Serhey Popovych wrote:
>>> Stephen Hemminger wrote:
>>>> On Thu, 22 Feb 2018 15:02:06 +0200
>>>> Serhey Popovych <serhe.popovych@...il.com> wrote:
>>>>
>>>>> +struct iplink_parse_args {
>>>>> +	const char *dev;
>>>>> +	const char *name;
>>>>> +	const char *type;
>>>>> +
>>>>> +	/* This definitely must be the last one and initialized
>>>>> +	 * by the caller of iplink_parse() that will initialize rest.
>>>>> +	 */
>>>>> +	struct iplink_req *req;
>>>>> +};
>>>>> +
>>>>
>>>> No control block please.
>>> Accepted.
>>>
>>>> If you have too many arguments, then that means you need to do
>>>> some refactoring.
>>>
>>> So using structure as single argument to a function isn't an option?
>>>
>>>>
>>>
>>>
>>
>> As I mentioned before, iplink_parse should not be used by vxcan or veth
>> as they only want a subset of the parsing. Once you take those users
>> out, iplink_parse becomes local to iplink.c with a single user. In which
>> case I suspect the compiler will always inline the function so no
>> refactoring on the number of arguments is needed.
>>
> I will implement cut down function to parse vxcan and veth peer device
> parameters and reuse it in iplink_parse() to avoid code duplications.
> 
> But my final goal not to refactor on number of arguments to parse,
> that's side product of this series, I want to take @name, @dev and
> other parameters for later use. In ->parse_opt() modules @name, @dev
> and others are not available easily. It seems only way to get them is
> to parse supplied netlink buffer.
> 
> 
While looking on how to make iplink_parse_light() that will be used with
vxcan and veth I found following problems:

  1) need to copy nearly all parameters parsing code (except vf, alias,
     carrier, master, protodown, link-netnsid, addrgenmode). this will
     be mitigated by re using iplink_parse_light() in iplink_parse()

  2) how to add attributes like IFLA_GROUP? in caller? this will give
     even more code duplications.

  3) there is high risk of adding regression either via conflicting
     matches() parameter in userspace or missing kernel attribute
     previously supported.

Sorry, I do not like this approach. I do not want to broke anything,
I just want @name and @dev parameters in ->parse_opt().


  2)
> 




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