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

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

[Kurt Kanzenbach]

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,
 };
 
 static int hellcreek_probe(struct platform_device *pdev)
-- 
2.30.2

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

[Leon Romanovsky]

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
>

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

[patchwork-bot+netdevbpf]

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

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

[Vladimir Oltean]

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);
> +}

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

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

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

[Vladimir Oltean]

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?

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

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]