netdev - Re: [PATCH net-next 2/6] net: dsa: do not skip -EOPNOTSUPP in dsa_port_vid

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: <f179fa10-3123-d055-1c67-0d24adf3cb08@gmail.com>
Date:   Fri, 23 Aug 2019 01:06:58 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Vivien Didelot <vivien.didelot@...il.com>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, f.fainelli@...il.com, andrew@...n.ch
Subject: Re: [PATCH net-next 2/6] net: dsa: do not skip -EOPNOTSUPP in
 dsa_port_vid_add

Hi Vivien,

On 8/22/19 11:13 PM, Vivien Didelot wrote:
> Currently dsa_port_vid_add returns 0 if the switch returns -EOPNOTSUPP.
> 
> This function is used in the tag_8021q.c code to offload the PVID of
> ports, which would simply not work if .port_vlan_add is not supported
> by the underlying switch.
> 
> Do not skip -EOPNOTSUPP in dsa_port_vid_add but only when necessary,
> that is to say in dsa_slave_vlan_rx_add_vid.
> 

Do you know why Florian suppressed -EOPNOTSUPP in 061f6a505ac3 ("net: 
dsa: Add ndo_vlan_rx_{add, kill}_vid implementation")?
I forced a return value of -EOPNOTSUPP here and when I create a VLAN 
sub-interface nothing breaks, it just says:
RTNETLINK answers: Operation not supported
which IMO is expected.

> Signed-off-by: Vivien Didelot <vivien.didelot@...il.com>
> ---
>   net/dsa/port.c  | 4 ++--
>   net/dsa/slave.c | 7 +++++--
>   2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index f75301456430..ef28df7ecbde 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -382,8 +382,8 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
>   
>   	trans.ph_prepare = true;
>   	err = dsa_port_vlan_add(dp, &vlan, &trans);
> -	if (err == -EOPNOTSUPP)
> -		return 0;
> +	if (err)
> +		return err;
>   
>   	trans.ph_prepare = false;
>   	return dsa_port_vlan_add(dp, &vlan, &trans);
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 33f41178afcc..9d61d9dbf001 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1082,8 +1082,11 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
>   			return -EBUSY;
>   	}
>   
> -	/* This API only allows programming tagged, non-PVID VIDs */
> -	return dsa_port_vid_add(dp, vid, 0);
> +	ret = dsa_port_vid_add(dp, vid, 0);
> +	if (ret && ret != -EOPNOTSUPP)
> +		return ret;
> +
> +	return 0;
>   }
>   
>   static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
> 

Regards,
-Vladimir