netdev - Re: [PATCH net-next 1/6] net: dsa: use a single switch statement for port setup

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, 19 Aug 2019 13:20:29 -0400
From:   Vivien Didelot <vivien.didelot@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, marek.behun@....cz, davem@...emloft.net,
        andrew@...n.ch
Subject: Re: [PATCH net-next 1/6] net: dsa: use a single switch statement for
 port setup

Hi Florian,

On Mon, 19 Aug 2019 10:14:24 -0700, Florian Fainelli <f.fainelli@...il.com> wrote:
> On 8/18/19 10:35 AM, Vivien Didelot wrote:
> > It is currently difficult to read the different steps involved in the
> > setup and teardown of ports in the DSA code. Keep it simple with a
> > single switch statement for each port type: UNUSED, CPU, DSA, or USER.
> > 
> > Also no need to call devlink_port_unregister from within dsa_port_setup
> > as this step is inconditionally handled by dsa_port_teardown on error.
> > 
> > Signed-off-by: Vivien Didelot <vivien.didelot@...il.com>
> > ---
> 
> [snip]
> 
> >  	case DSA_PORT_TYPE_CPU:
> > +		memset(dlp, 0, sizeof(*dlp));
> > +		devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_CPU,
> > +				       dp->index, false, 0, id, len);
> > +		err = devlink_port_register(dl, dlp, dp->index);
> > +		if (err)
> > +			return err;
> 
> This is shared between all port flavors with the exception that the
> flavor type is different, maybe we should create a helper function and
> factor out even more code. I don't feel great about repeating 3 times t
> the same code without making use of a fall through.

Nah I did not want to use an helper because the code is still too
noodly, if you look at user ports setup, we continue to setup devlink
after the slave creation, so here I prefered to keep things readable
and expose all steps first, before writing yet another helper.


Thanks,

	Vivien