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