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

[Florian Fainelli]

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@xxxxxxxxx>
> ---
> 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