diff mbox series

[net-next] net: dsa: hellcreek: Offload bridge port flags

Message ID 20210314125208.17378-1-kurt@kmk-computers.de (mailing list archive)
State Accepted
Commit db7284a6ccc4a6d7714645141f7dcee0fcb4e57d
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: dsa: hellcreek: Offload bridge port flags | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: kurt@linutronix.de linux@armlinux.org.uk
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 141 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Kurt Kanzenbach March 14, 2021, 12:52 p.m. UTC
The switch implements unicast and multicast filtering per port.
Add support for it. By default filtering is disabled.

Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 drivers/net/dsa/hirschmann/hellcreek.c | 129 ++++++++++++++++++++-----
 1 file changed, 104 insertions(+), 25 deletions(-)

Comments

Leon Romanovsky March 14, 2021, 1:43 p.m. UTC | #1
On Sun, Mar 14, 2021 at 01:52:08PM +0100, Kurt Kanzenbach wrote:
> The switch implements unicast and multicast filtering per port.
> Add support for it. By default filtering is disabled.
>
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
> ---
>  drivers/net/dsa/hirschmann/hellcreek.c | 129 ++++++++++++++++++++-----
>  1 file changed, 104 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index c1f873a4fbc4..6cba02307bda 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -600,6 +600,83 @@ static void hellcreek_setup_vlan_membership(struct dsa_switch *ds, int port,
>  		hellcreek_unapply_vlan(hellcreek, upstream, vid);
>  }
>
> +static void hellcreek_port_set_ucast_flood(struct hellcreek *hellcreek,
> +					   int port, bool enable)
> +{
> +	struct hellcreek_port *hellcreek_port;
> +	u16 val;
> +
> +	hellcreek_port = &hellcreek->ports[port];
> +
> +	dev_dbg(hellcreek->dev, "%s unicast flooding on port %d\n",
> +		enable ? "Enable" : "Disable", port);
> +
> +	mutex_lock(&hellcreek->reg_lock);
> +
> +	hellcreek_select_port(hellcreek, port);
> +	val = hellcreek_port->ptcfg;
> +	if (enable)
> +		val &= ~HR_PTCFG_UUC_FLT;
> +	else
> +		val |= HR_PTCFG_UUC_FLT;
> +	hellcreek_write(hellcreek, val, HR_PTCFG);
> +	hellcreek_port->ptcfg = val;
> +
> +	mutex_unlock(&hellcreek->reg_lock);
> +}
> +
> +static void hellcreek_port_set_mcast_flood(struct hellcreek *hellcreek,
> +					   int port, bool enable)
> +{
> +	struct hellcreek_port *hellcreek_port;
> +	u16 val;
> +
> +	hellcreek_port = &hellcreek->ports[port];
> +
> +	dev_dbg(hellcreek->dev, "%s multicast flooding on port %d\n",
> +		enable ? "Enable" : "Disable", port);
> +
> +	mutex_lock(&hellcreek->reg_lock);
> +
> +	hellcreek_select_port(hellcreek, port);
> +	val = hellcreek_port->ptcfg;
> +	if (enable)
> +		val &= ~HR_PTCFG_UMC_FLT;
> +	else
> +		val |= HR_PTCFG_UMC_FLT;
> +	hellcreek_write(hellcreek, val, HR_PTCFG);
> +	hellcreek_port->ptcfg = val;
> +
> +	mutex_unlock(&hellcreek->reg_lock);
> +}
> +
> +static int hellcreek_pre_bridge_flags(struct dsa_switch *ds, int port,
> +				      struct switchdev_brport_flags flags,
> +				      struct netlink_ext_ack *extack)
> +{
> +	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
> +				  struct switchdev_brport_flags flags,
> +				  struct netlink_ext_ack *extack)
> +{
> +	struct hellcreek *hellcreek = ds->priv;
> +
> +	if (flags.mask & BR_FLOOD)
> +		hellcreek_port_set_ucast_flood(hellcreek, port,
> +					       !!(flags.val & BR_FLOOD));
> +
> +	if (flags.mask & BR_MCAST_FLOOD)
> +		hellcreek_port_set_mcast_flood(hellcreek, port,
> +					       !!(flags.val & BR_MCAST_FLOOD));
> +
> +	return 0;
> +}
> +
>  static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
>  				      struct net_device *br)
>  {
> @@ -1719,31 +1796,33 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
>  }
>
>  static const struct dsa_switch_ops hellcreek_ds_ops = {
> -	.get_ethtool_stats   = hellcreek_get_ethtool_stats,
> -	.get_sset_count	     = hellcreek_get_sset_count,
> -	.get_strings	     = hellcreek_get_strings,
> -	.get_tag_protocol    = hellcreek_get_tag_protocol,
> -	.get_ts_info	     = hellcreek_get_ts_info,
> -	.phylink_validate    = hellcreek_phylink_validate,
> -	.port_bridge_join    = hellcreek_port_bridge_join,
> -	.port_bridge_leave   = hellcreek_port_bridge_leave,
> -	.port_disable	     = hellcreek_port_disable,
> -	.port_enable	     = hellcreek_port_enable,
> -	.port_fdb_add	     = hellcreek_fdb_add,
> -	.port_fdb_del	     = hellcreek_fdb_del,
> -	.port_fdb_dump	     = hellcreek_fdb_dump,
> -	.port_hwtstamp_set   = hellcreek_port_hwtstamp_set,
> -	.port_hwtstamp_get   = hellcreek_port_hwtstamp_get,
> -	.port_prechangeupper = hellcreek_port_prechangeupper,
> -	.port_rxtstamp	     = hellcreek_port_rxtstamp,
> -	.port_setup_tc	     = hellcreek_port_setup_tc,
> -	.port_stp_state_set  = hellcreek_port_stp_state_set,
> -	.port_txtstamp	     = hellcreek_port_txtstamp,
> -	.port_vlan_add	     = hellcreek_vlan_add,
> -	.port_vlan_del	     = hellcreek_vlan_del,
> -	.port_vlan_filtering = hellcreek_vlan_filtering,
> -	.setup		     = hellcreek_setup,
> -	.teardown	     = hellcreek_teardown,
> +	.get_ethtool_stats     = hellcreek_get_ethtool_stats,
> +	.get_sset_count	       = hellcreek_get_sset_count,
> +	.get_strings	       = hellcreek_get_strings,
> +	.get_tag_protocol      = hellcreek_get_tag_protocol,
> +	.get_ts_info	       = hellcreek_get_ts_info,
> +	.phylink_validate      = hellcreek_phylink_validate,
> +	.port_bridge_flags     = hellcreek_bridge_flags,
> +	.port_bridge_join      = hellcreek_port_bridge_join,
> +	.port_bridge_leave     = hellcreek_port_bridge_leave,
> +	.port_disable	       = hellcreek_port_disable,
> +	.port_enable	       = hellcreek_port_enable,
> +	.port_fdb_add	       = hellcreek_fdb_add,
> +	.port_fdb_del	       = hellcreek_fdb_del,
> +	.port_fdb_dump	       = hellcreek_fdb_dump,
> +	.port_hwtstamp_set     = hellcreek_port_hwtstamp_set,
> +	.port_hwtstamp_get     = hellcreek_port_hwtstamp_get,
> +	.port_pre_bridge_flags = hellcreek_pre_bridge_flags,
> +	.port_prechangeupper   = hellcreek_port_prechangeupper,
> +	.port_rxtstamp	       = hellcreek_port_rxtstamp,
> +	.port_setup_tc	       = hellcreek_port_setup_tc,
> +	.port_stp_state_set    = hellcreek_port_stp_state_set,
> +	.port_txtstamp	       = hellcreek_port_txtstamp,
> +	.port_vlan_add	       = hellcreek_vlan_add,
> +	.port_vlan_del	       = hellcreek_vlan_del,
> +	.port_vlan_filtering   = hellcreek_vlan_filtering,
> +	.setup		       = hellcreek_setup,
> +	.teardown	       = hellcreek_teardown,
>  };

This patch is a perfect example why vertical space alignment is a bad thing.
Addition of one function caused to so much churn at the end.

Thanks

>
>  static int hellcreek_probe(struct platform_device *pdev)
> --
> 2.30.2
>
patchwork-bot+netdevbpf@kernel.org March 15, 2021, 7:40 p.m. UTC | #2
Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Sun, 14 Mar 2021 13:52:08 +0100 you wrote:
> The switch implements unicast and multicast filtering per port.
> Add support for it. By default filtering is disabled.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
> ---
>  drivers/net/dsa/hirschmann/hellcreek.c | 129 ++++++++++++++++++++-----
>  1 file changed, 104 insertions(+), 25 deletions(-)

Here is the summary with links:
  - [net-next] net: dsa: hellcreek: Offload bridge port flags
    https://git.kernel.org/netdev/net-next/c/db7284a6ccc4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Vladimir Oltean March 15, 2021, 8:08 p.m. UTC | #3
On Sun, Mar 14, 2021 at 01:52:08PM +0100, Kurt Kanzenbach wrote:
> The switch implements unicast and multicast filtering per port.
> Add support for it. By default filtering is disabled.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
> ---
>  drivers/net/dsa/hirschmann/hellcreek.c | 129 ++++++++++++++++++++-----
>  1 file changed, 104 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index c1f873a4fbc4..6cba02307bda 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -600,6 +600,83 @@ static void hellcreek_setup_vlan_membership(struct dsa_switch *ds, int port,
>  		hellcreek_unapply_vlan(hellcreek, upstream, vid);
>  }
>  
> +static void hellcreek_port_set_ucast_flood(struct hellcreek *hellcreek,
> +					   int port, bool enable)
> +{
> +	struct hellcreek_port *hellcreek_port;
> +	u16 val;
> +
> +	hellcreek_port = &hellcreek->ports[port];
> +
> +	dev_dbg(hellcreek->dev, "%s unicast flooding on port %d\n",
> +		enable ? "Enable" : "Disable", port);
> +
> +	mutex_lock(&hellcreek->reg_lock);
> +
> +	hellcreek_select_port(hellcreek, port);
> +	val = hellcreek_port->ptcfg;
> +	if (enable)
> +		val &= ~HR_PTCFG_UUC_FLT;
> +	else
> +		val |= HR_PTCFG_UUC_FLT;

What does 'unknown unicast filtering' mean/do, exactly?
The semantics of BR_FLOOD are on egress: all unicast packets with an
unknown destination that are received on ports from this bridging domain
can be flooded towards port X if that port has flooding enabled.
When I hear "filtering", I imagine an ingress setting, am I wrong?

> +	hellcreek_write(hellcreek, val, HR_PTCFG);
> +	hellcreek_port->ptcfg = val;
> +
> +	mutex_unlock(&hellcreek->reg_lock);
> +}
Kurt Kanzenbach March 15, 2021, 8:33 p.m. UTC | #4
On Mon Mar 15 2021, Vladimir Oltean wrote:
> On Sun, Mar 14, 2021 at 01:52:08PM +0100, Kurt Kanzenbach wrote:
>> +	if (enable)
>> +		val &= ~HR_PTCFG_UUC_FLT;
>> +	else
>> +		val |= HR_PTCFG_UUC_FLT;
>
> What does 'unknown unicast filtering' mean/do, exactly?
> The semantics of BR_FLOOD are on egress: all unicast packets with an
> unknown destination that are received on ports from this bridging domain
> can be flooded towards port X if that port has flooding enabled.
> When I hear "filtering", I imagine an ingress setting, am I wrong?

It means that frames without matching fdb entries towards this port are
discarded. There's also ingress filtering which works based on VLANs and
is used already.

Thanks,
Kurt
Vladimir Oltean March 15, 2021, 9:34 p.m. UTC | #5
On Mon, Mar 15, 2021 at 09:33:44PM +0100, Kurt Kanzenbach wrote:
> On Mon Mar 15 2021, Vladimir Oltean wrote:
> > On Sun, Mar 14, 2021 at 01:52:08PM +0100, Kurt Kanzenbach wrote:
> >> +	if (enable)
> >> +		val &= ~HR_PTCFG_UUC_FLT;
> >> +	else
> >> +		val |= HR_PTCFG_UUC_FLT;
> >
> > What does 'unknown unicast filtering' mean/do, exactly?
> > The semantics of BR_FLOOD are on egress: all unicast packets with an
> > unknown destination that are received on ports from this bridging domain
> > can be flooded towards port X if that port has flooding enabled.
> > When I hear "filtering", I imagine an ingress setting, am I wrong?
> 
> It means that frames without matching fdb entries towards this port are
> discarded.

The phrasing is still not crystal clear, sorry.
You have a switch with 2 user ports, lan0 and lan1, and one CPU port.
lan0 and lan1 are under br0. lan0 has 'unknown unicast filtering'
disabled, lan1 has it enabled, and the CPU port has it disabled.
You receive a packet from lan0 towards an unknown unicast destination.
Is the packet discarded or is it sent to the CPU port?
Kurt Kanzenbach March 16, 2021, 8:38 a.m. UTC | #6
On Mon Mar 15 2021, Vladimir Oltean wrote:
> On Mon, Mar 15, 2021 at 09:33:44PM +0100, Kurt Kanzenbach wrote:
>> On Mon Mar 15 2021, Vladimir Oltean wrote:
>> > On Sun, Mar 14, 2021 at 01:52:08PM +0100, Kurt Kanzenbach wrote:
>> >> +	if (enable)
>> >> +		val &= ~HR_PTCFG_UUC_FLT;
>> >> +	else
>> >> +		val |= HR_PTCFG_UUC_FLT;
>> >
>> > What does 'unknown unicast filtering' mean/do, exactly?
>> > The semantics of BR_FLOOD are on egress: all unicast packets with an
>> > unknown destination that are received on ports from this bridging domain
>> > can be flooded towards port X if that port has flooding enabled.
>> > When I hear "filtering", I imagine an ingress setting, am I wrong?
>> 
>> It means that frames without matching fdb entries towards this port are
>> discarded.
>
> The phrasing is still not crystal clear, sorry.
> You have a switch with 2 user ports, lan0 and lan1, and one CPU port.
> lan0 and lan1 are under br0. lan0 has 'unknown unicast filtering'
> disabled, lan1 has it enabled, and the CPU port has it disabled.
> You receive a packet from lan0 towards an unknown unicast destination.
> Is the packet discarded or is it sent to the CPU port?

It's sent to the CPU port. Anyway, I'll double check it.

Thanks,
Kurt
diff mbox series

Patch

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index c1f873a4fbc4..6cba02307bda 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -600,6 +600,83 @@  static void hellcreek_setup_vlan_membership(struct dsa_switch *ds, int port,
 		hellcreek_unapply_vlan(hellcreek, upstream, vid);
 }
 
+static void hellcreek_port_set_ucast_flood(struct hellcreek *hellcreek,
+					   int port, bool enable)
+{
+	struct hellcreek_port *hellcreek_port;
+	u16 val;
+
+	hellcreek_port = &hellcreek->ports[port];
+
+	dev_dbg(hellcreek->dev, "%s unicast flooding on port %d\n",
+		enable ? "Enable" : "Disable", port);
+
+	mutex_lock(&hellcreek->reg_lock);
+
+	hellcreek_select_port(hellcreek, port);
+	val = hellcreek_port->ptcfg;
+	if (enable)
+		val &= ~HR_PTCFG_UUC_FLT;
+	else
+		val |= HR_PTCFG_UUC_FLT;
+	hellcreek_write(hellcreek, val, HR_PTCFG);
+	hellcreek_port->ptcfg = val;
+
+	mutex_unlock(&hellcreek->reg_lock);
+}
+
+static void hellcreek_port_set_mcast_flood(struct hellcreek *hellcreek,
+					   int port, bool enable)
+{
+	struct hellcreek_port *hellcreek_port;
+	u16 val;
+
+	hellcreek_port = &hellcreek->ports[port];
+
+	dev_dbg(hellcreek->dev, "%s multicast flooding on port %d\n",
+		enable ? "Enable" : "Disable", port);
+
+	mutex_lock(&hellcreek->reg_lock);
+
+	hellcreek_select_port(hellcreek, port);
+	val = hellcreek_port->ptcfg;
+	if (enable)
+		val &= ~HR_PTCFG_UMC_FLT;
+	else
+		val |= HR_PTCFG_UMC_FLT;
+	hellcreek_write(hellcreek, val, HR_PTCFG);
+	hellcreek_port->ptcfg = val;
+
+	mutex_unlock(&hellcreek->reg_lock);
+}
+
+static int hellcreek_pre_bridge_flags(struct dsa_switch *ds, int port,
+				      struct switchdev_brport_flags flags,
+				      struct netlink_ext_ack *extack)
+{
+	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int hellcreek_bridge_flags(struct dsa_switch *ds, int port,
+				  struct switchdev_brport_flags flags,
+				  struct netlink_ext_ack *extack)
+{
+	struct hellcreek *hellcreek = ds->priv;
+
+	if (flags.mask & BR_FLOOD)
+		hellcreek_port_set_ucast_flood(hellcreek, port,
+					       !!(flags.val & BR_FLOOD));
+
+	if (flags.mask & BR_MCAST_FLOOD)
+		hellcreek_port_set_mcast_flood(hellcreek, port,
+					       !!(flags.val & BR_MCAST_FLOOD));
+
+	return 0;
+}
+
 static int hellcreek_port_bridge_join(struct dsa_switch *ds, int port,
 				      struct net_device *br)
 {
@@ -1719,31 +1796,33 @@  static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
 }
 
 static const struct dsa_switch_ops hellcreek_ds_ops = {
-	.get_ethtool_stats   = hellcreek_get_ethtool_stats,
-	.get_sset_count	     = hellcreek_get_sset_count,
-	.get_strings	     = hellcreek_get_strings,
-	.get_tag_protocol    = hellcreek_get_tag_protocol,
-	.get_ts_info	     = hellcreek_get_ts_info,
-	.phylink_validate    = hellcreek_phylink_validate,
-	.port_bridge_join    = hellcreek_port_bridge_join,
-	.port_bridge_leave   = hellcreek_port_bridge_leave,
-	.port_disable	     = hellcreek_port_disable,
-	.port_enable	     = hellcreek_port_enable,
-	.port_fdb_add	     = hellcreek_fdb_add,
-	.port_fdb_del	     = hellcreek_fdb_del,
-	.port_fdb_dump	     = hellcreek_fdb_dump,
-	.port_hwtstamp_set   = hellcreek_port_hwtstamp_set,
-	.port_hwtstamp_get   = hellcreek_port_hwtstamp_get,
-	.port_prechangeupper = hellcreek_port_prechangeupper,
-	.port_rxtstamp	     = hellcreek_port_rxtstamp,
-	.port_setup_tc	     = hellcreek_port_setup_tc,
-	.port_stp_state_set  = hellcreek_port_stp_state_set,
-	.port_txtstamp	     = hellcreek_port_txtstamp,
-	.port_vlan_add	     = hellcreek_vlan_add,
-	.port_vlan_del	     = hellcreek_vlan_del,
-	.port_vlan_filtering = hellcreek_vlan_filtering,
-	.setup		     = hellcreek_setup,
-	.teardown	     = hellcreek_teardown,
+	.get_ethtool_stats     = hellcreek_get_ethtool_stats,
+	.get_sset_count	       = hellcreek_get_sset_count,
+	.get_strings	       = hellcreek_get_strings,
+	.get_tag_protocol      = hellcreek_get_tag_protocol,
+	.get_ts_info	       = hellcreek_get_ts_info,
+	.phylink_validate      = hellcreek_phylink_validate,
+	.port_bridge_flags     = hellcreek_bridge_flags,
+	.port_bridge_join      = hellcreek_port_bridge_join,
+	.port_bridge_leave     = hellcreek_port_bridge_leave,
+	.port_disable	       = hellcreek_port_disable,
+	.port_enable	       = hellcreek_port_enable,
+	.port_fdb_add	       = hellcreek_fdb_add,
+	.port_fdb_del	       = hellcreek_fdb_del,
+	.port_fdb_dump	       = hellcreek_fdb_dump,
+	.port_hwtstamp_set     = hellcreek_port_hwtstamp_set,
+	.port_hwtstamp_get     = hellcreek_port_hwtstamp_get,
+	.port_pre_bridge_flags = hellcreek_pre_bridge_flags,
+	.port_prechangeupper   = hellcreek_port_prechangeupper,
+	.port_rxtstamp	       = hellcreek_port_rxtstamp,
+	.port_setup_tc	       = hellcreek_port_setup_tc,
+	.port_stp_state_set    = hellcreek_port_stp_state_set,
+	.port_txtstamp	       = hellcreek_port_txtstamp,
+	.port_vlan_add	       = hellcreek_vlan_add,
+	.port_vlan_del	       = hellcreek_vlan_del,
+	.port_vlan_filtering   = hellcreek_vlan_filtering,
+	.setup		       = hellcreek_setup,
+	.teardown	       = hellcreek_teardown,
 };
 
 static int hellcreek_probe(struct platform_device *pdev)