From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f67.google.com ([209.85.215.67]:44939 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751022AbeBZUJL (ORCPT ); Mon, 26 Feb 2018 15:09:11 -0500 Received: by mail-lf0-f67.google.com with SMTP id v9so24101203lfa.11 for ; Mon, 26 Feb 2018 12:09:10 -0800 (PST) Subject: Re: [PATCH iproute2-next v3 8/8] iplink: Reduce number of arguments to iplink_parse() From: Serhey Popovych To: David Ahern , Stephen Hemminger Cc: netdev@vger.kernel.org References: <1519304526-18848-1-git-send-email-serhe.popovych@gmail.com> <1519304526-18848-9-git-send-email-serhe.popovych@gmail.com> <20180226100638.789a770d@xeon-e3> <4df74e2b-7248-1b48-3b2b-0ca2b10ea7de@gmail.com> <0834a77a-4b9c-d132-990b-f0afb92768fb@gmail.com> Message-ID: Date: Mon, 26 Feb 2018 22:09:03 +0200 MIME-Version: 1.0 In-Reply-To: <0834a77a-4b9c-d132-990b-f0afb92768fb@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="KFP5baz6bfo3erpZgrw7ZEB4kMn1Wh1Pb" Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --KFP5baz6bfo3erpZgrw7ZEB4kMn1Wh1Pb Content-Type: multipart/mixed; boundary="DKPu53C52ZGyTT0njyQIjoJtc5vUmNi85"; protected-headers="v1" From: Serhey Popovych To: David Ahern , Stephen Hemminger Cc: netdev@vger.kernel.org Message-ID: Subject: Re: [PATCH iproute2-next v3 8/8] iplink: Reduce number of arguments to iplink_parse() References: <1519304526-18848-1-git-send-email-serhe.popovych@gmail.com> <1519304526-18848-9-git-send-email-serhe.popovych@gmail.com> <20180226100638.789a770d@xeon-e3> <4df74e2b-7248-1b48-3b2b-0ca2b10ea7de@gmail.com> <0834a77a-4b9c-d132-990b-f0afb92768fb@gmail.com> In-Reply-To: <0834a77a-4b9c-d132-990b-f0afb92768fb@gmail.com> --DKPu53C52ZGyTT0njyQIjoJtc5vUmNi85 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 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 vet= h >> 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 whi= ch >> 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. >=20 > 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. >=20 >=20 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) >=20 --DKPu53C52ZGyTT0njyQIjoJtc5vUmNi85-- --KFP5baz6bfo3erpZgrw7ZEB4kMn1Wh1Pb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQEcBAEBAgAGBQJalGljAAoJEBTawMmQ61bB1H8H/iJRaqRmrwX1PDPrT6dvB0Ew rx6f56G7NqrYzUZSOijYo50iq8t5say/AO0AUOvK07kWc8deYhj4fdyQFtNfNHnw sSwhoVSZakS5Pmi8kOS+QKV9Ej/UOp4pONKvDRcq0UmC7sgxKpFpHYaMfsDCEN4V SafqVYrx0BZUcXSC05IXUQpaKwX0oplNnEJexPoXbw9eX9XE1TWMEDD7hujLUWR4 zObks+kvdBJrXdnZypnwtTPT0cNgeEJUOyCCsMpOHT6TsqcE80Bd5dfwcjTWBUd/ Ov0//8GNapvONwaUjz8FgZjSMrjsJ24p5JS19d3jJr7UV9PfFvq5/w31A79VLJk= =bCiy -----END PGP SIGNATURE----- --KFP5baz6bfo3erpZgrw7ZEB4kMn1Wh1Pb--