netdev - Re: [PATCH v2 net-next 05/22] net: dsa: Add more convenient functions for installing port VLANs

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: <d9f4ec44-7cc6-0ffe-9d02-eb1b1b818340@gmail.com>
Date:   Tue, 9 Apr 2019 19:01:48 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, vivien.didelot@...il.com,
        andrew@...n.ch, davem@...emloft.net
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        georg.waibel@...sor-technik.de
Subject: Re: [PATCH v2 net-next 05/22] net: dsa: Add more convenient functions
 for installing port VLANs



On 4/9/2019 5:56 PM, Vladimir Oltean wrote:
> This hides the need to perform a two-phase transaction and construct a
> switchdev_obj_port_vlan struct.
> 
> Call graph (including a function that will be introduced in a follow-up
> patch) looks like this now (same for the *_vlan_del function):
> 
> dsa_slave_vlan_rx_add_vid   dsa_port_setup_8021q_tagging
>             |                        |
>             |                        |
>             |          +-------------+
>             |          |
>             v          v
>            dsa_port_vid_add      dsa_slave_port_obj_add
>                   |                         |
>                   +-------+         +-------+
>                           |         |
>                           v         v
>                        dsa_port_vlan_add
> 
> Signed-off-by: Vladimir Oltean <olteanv@...il.com>
> ---
> Changes in v2:
> Renamed __dsa_port_vlan_add to dsa_port_vid_add and not to
> dsa_port_vlan_add_trans, as suggested, because the corresponding _del function
> does not have a transactional phase and the naming is more uniform this way.

Thanks the name does look a lot better now.

[snip]

> +int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
> +{
> +	struct switchdev_obj_port_vlan vlan = {
> +		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
> +		.vid_begin = vid,
> +		.vid_end = vid,
> +	};

Any reasons why this function does not take a flags argument? Also, did
you intend to migrate dsa_slave_vlan_rx_kill_vid() to use this helper
for deleting a VLAN ID?
-- 
Florian