diff mbox series

[net-next,v7,2/8] net: dsa: Give drivers the chance to veto certain upper devices

Message ID 20201028074221.29326-3-kurt@linutronix.de
State Changes Requested
Delegated to: David Miller
Headers show
Series Hirschmann Hellcreek DSA driver | expand

Checks

Context Check Description
jkicinski/cover_letter success Link
jkicinski/fixes_present success Link
jkicinski/patch_count success Link
jkicinski/tree_selection success Clearly marked for net-next
jkicinski/subject_prefix success Link
jkicinski/source_inline success Was 0 now: 0
jkicinski/verify_signedoff success Link
jkicinski/module_param success Was 0 now: 0
jkicinski/build_32bit fail Errors and warnings before: 23 this patch: 23
jkicinski/kdoc success Errors and warnings before: 0 this patch: 0
jkicinski/verify_fixes success Link
jkicinski/checkpatch fail Link
jkicinski/build_allmodconfig_warn success Errors and warnings before: 19 this patch: 19
jkicinski/header_inline success Link
jkicinski/stable success Stable not CCed

Commit Message

Kurt Kanzenbach Oct. 28, 2020, 7:42 a.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

Some switches rely on unique pvids to ensure port separation in
standalone mode, because they don't have a port forwarding matrix
configurable in hardware. So, setups like a group of 2 uppers with the
same VLAN, swp0.100 and swp1.100, will cause traffic tagged with VLAN
100 to be autonomously forwarded between these switch ports, in spite
of there being no bridge between swp0 and swp1.

These drivers need to prevent this from happening. They need to have
VLAN filtering enabled in standalone mode (so they'll drop frames tagged
with unknown VLANs) and they can only accept an 8021q upper on a port as
long as it isn't installed on any other port too. So give them the
chance to veto bad user requests.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 include/net/dsa.h |  6 ++++++
 net/dsa/slave.c   | 12 ++++++++++++
 2 files changed, 18 insertions(+)

Comments

Florian Fainelli Oct. 29, 2020, 2:22 a.m. UTC | #1
On 10/28/2020 12:42 AM, Kurt Kanzenbach wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Some switches rely on unique pvids to ensure port separation in
> standalone mode, because they don't have a port forwarding matrix
> configurable in hardware. So, setups like a group of 2 uppers with the
> same VLAN, swp0.100 and swp1.100, will cause traffic tagged with VLAN
> 100 to be autonomously forwarded between these switch ports, in spite
> of there being no bridge between swp0 and swp1.
> 
> These drivers need to prevent this from happening. They need to have
> VLAN filtering enabled in standalone mode (so they'll drop frames tagged
> with unknown VLANs) and they can only accept an 8021q upper on a port as
> long as it isn't installed on any other port too. So give them the
> chance to veto bad user requests.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
>  include/net/dsa.h |  6 ++++++
>  net/dsa/slave.c   | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 04e93bafb7bd..4e60d2610f20 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -536,6 +536,12 @@ struct dsa_switch_ops {
>  	void	(*get_regs)(struct dsa_switch *ds, int port,
>  			    struct ethtool_regs *regs, void *p);
>  
> +	/*
> +	 * Upper device tracking.
> +	 */
> +	int	(*port_prechangeupper)(struct dsa_switch *ds, int port,
> +				       struct netdev_notifier_changeupper_info *info);
> +
>  	/*
>  	 * Bridge integration
>  	 */
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 3bc5ca40c9fb..1919a025c06f 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1987,10 +1987,22 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
>  	switch (event) {
>  	case NETDEV_PRECHANGEUPPER: {
>  		struct netdev_notifier_changeupper_info *info = ptr;
> +		struct dsa_switch *ds;
> +		struct dsa_port *dp;
> +		int err;
>  
>  		if (!dsa_slave_dev_check(dev))
>  			return dsa_prevent_bridging_8021q_upper(dev, ptr);
>  
> +		dp = dsa_slave_to_port(dev);
> +		ds = dp->ds;
> +
> +		if (ds->ops->port_prechangeupper) {
> +			err = ds->ops->port_prechangeupper(ds, dp->index, ptr);

I would pass 'info' instead of 'ptr' here even if there is no functional
difference, this would be clearer. Not a reason to resubmit if
everything else is fine in this series:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 04e93bafb7bd..4e60d2610f20 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -536,6 +536,12 @@  struct dsa_switch_ops {
 	void	(*get_regs)(struct dsa_switch *ds, int port,
 			    struct ethtool_regs *regs, void *p);
 
+	/*
+	 * Upper device tracking.
+	 */
+	int	(*port_prechangeupper)(struct dsa_switch *ds, int port,
+				       struct netdev_notifier_changeupper_info *info);
+
 	/*
 	 * Bridge integration
 	 */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3bc5ca40c9fb..1919a025c06f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1987,10 +1987,22 @@  static int dsa_slave_netdevice_event(struct notifier_block *nb,
 	switch (event) {
 	case NETDEV_PRECHANGEUPPER: {
 		struct netdev_notifier_changeupper_info *info = ptr;
+		struct dsa_switch *ds;
+		struct dsa_port *dp;
+		int err;
 
 		if (!dsa_slave_dev_check(dev))
 			return dsa_prevent_bridging_8021q_upper(dev, ptr);
 
+		dp = dsa_slave_to_port(dev);
+		ds = dp->ds;
+
+		if (ds->ops->port_prechangeupper) {
+			err = ds->ops->port_prechangeupper(ds, dp->index, ptr);
+			if (err)
+				return notifier_from_errno(err);
+		}
+
 		if (is_vlan_dev(info->upper_dev))
 			return dsa_slave_check_8021q_upper(dev, ptr);
 		break;