Re: [PATCH 12/13] net: dsa: lan9303: Added "stp_enable" sysfs attribute

From: Florian Fainelli
Date: Mon Jul 24 2017 - 12:56:12 EST


On 07/20/2017 06:42 AM, Egil Hjelmeland wrote:
> Must be set to 1 by user space when STP is used on the lan9303.
> If bridging without local STP, leave at 0, so external STP BPDUs
> are forwarded.
>
> Hopefully the kernel can be improved so the driver can handle this
> without user intervention, and this control can be removed.

Same here, we can't have a driver-specific sysfs attribute just for
this, either we find a way to have the bridge's STP settings propagate
correctly to the switch driver, or you have to make better decisions
based on hints/calls you are getting from switchdev -> dsa -> driver.

>
> Signed-off-by: Egil Hjelmeland <egil.hjelmeland@xxxxxxxxxxx>
> ---
> Documentation/networking/dsa/lan9303.txt | 23 ++++++++++++++++++++++
> drivers/net/dsa/lan9303-core.c | 33 ++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/Documentation/networking/dsa/lan9303.txt b/Documentation/networking/dsa/lan9303.txt
> index ace91c821ce7..0694b6646d2a 100644
> --- a/Documentation/networking/dsa/lan9303.txt
> +++ b/Documentation/networking/dsa/lan9303.txt
> @@ -40,6 +40,10 @@ When a user port is enabled, the driver creates sysfs directory
> - alr_dump (RO): List the 168 first entries of the ALR table.
> Including port 0 entires. This file is identical for both ports.
> Format: MAC; list of ports; (l)earned / (s)tatic
> + - stp_enable (RW): Must be set to 1 when STP is used. Installs an ALR
> + entry so that received STP BPDUs are only sent to port 0.
> + When 0 (default) received STP BPDUs are forwarded to all ports.
> + This file is identical for both ports.
> - swe_bcst_throt (RW): Set/get 6.4.7 Broadcast Storm Control
> Throttle Level for the port. Accesses the corresponding bits of
> the SWE_BCST_THROT register (13.4.3.23).
> @@ -49,3 +53,22 @@ Driver limitations
> ==================
>
> - No support for VLAN
> +
> +
> +Bridging notes
> +==============
> +When the user ports are bridged, broadcasts, multicasts and unknown
> +frames with unknown destination are flooded by the chip. Therefore SW
> +flooding must be disabled by:
> +
> + echo 0 > /sys/class/net/p1/brport/broadcast_flood
> + echo 0 > /sys/class/net/p1/brport/multicast_flood
> + echo 0 > /sys/class/net/p1/brport/unicast_flood
> + echo 0 > /sys/class/net/p2/brport/broadcast_flood
> + echo 0 > /sys/class/net/p2/brport/multicast_flood
> + echo 0 > /sys/class/net/p2/brport/unicast_flood
> +
> +If enabling local STP, the LAN9303 must be configured to forward
> +BPDUs only to port 0, by writing 1 to "stp_enable" of one of the ports:
> +
> + echo 1 > /sys/class/net/p1/lan9303/stp_enable
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index b682aa4f1fca..54646eb38185 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -187,6 +187,8 @@
> #define MII_LAN911X_SPECIAL_MODES 0x12
> #define MII_LAN911X_SPECIAL_CONTROL_STATUS 0x1f
>
> +#define eth_stp_addr eth_reserved_addr_base
> +
> static const struct regmap_range lan9303_valid_regs[] = {
> regmap_reg_range(0x14, 0x17), /* misc, interrupt */
> regmap_reg_range(0x19, 0x19), /* endian test */
> @@ -988,9 +990,40 @@ alr_dump_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(alr_dump);
>
> +static ssize_t
> +stp_enable_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct dsa_port *dp = dsa_net_device_to_dsa_port(to_net_dev(dev));
> + struct lan9303 *chip = dp->ds->priv;
> + char result = lan9303_alr_cache_find_mac(chip, eth_stp_addr) ?
> + '1' : '0';
> + return scnprintf(buf, PAGE_SIZE, "%c\n", result);
> +}
> +
> +static ssize_t
> +stp_enable_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct dsa_port *dp = dsa_net_device_to_dsa_port(to_net_dev(dev));
> + struct lan9303 *chip = dp->ds->priv;
> + unsigned long enable;
> + int ret = kstrtoul(buf, 0, &enable);
> +
> + if (ret)
> + return ret;
> + if (enable)
> + lan9303_alr_add_port(chip, eth_stp_addr, 0, true);
> + else
> + lan9303_alr_del_port(chip, eth_stp_addr, 0);
> + return len;
> +}
> +static DEVICE_ATTR_RW(stp_enable);
> +
> static struct attribute *lan9303_attrs[] = {
> &dev_attr_swe_bcst_throt.attr,
> &dev_attr_alr_dump.attr,
> + &dev_attr_stp_enable.attr,
> NULL
> };
>
>


--
Florian