Re: [PATCH net-next 4/4] net: dsa: bcm_sf2: Add VLAN support

[Vivien Didelot]

Hi Florian,

Florian Fainelli  writes:

> Another possible approach would have been to allocate the vlan structure
> upong port_vlan_prepare() though that would typically result in more
> fragmentation over time once se start using more VLANs.

This is indeed what the switchdev prepare phase is for. If anything goes
wrong before calling the commit phase, switchdev will free the allocated
data.

Thanks,

Vivien

Re: [PATCH net-next 4/4] net: dsa: bcm_sf2: Add VLAN support

[Andrew Lunn]

On Fri, Jun 10, 2016 at 11:47:48AM -0700, Florian Fainelli wrote:
> On 06/10/2016 05:00 AM, Andrew Lunn wrote:
> >> @@ -148,6 +155,9 @@ struct bcm_sf2_priv {
> >>struct device_node  *master_mii_dn;
> >>struct mii_bus  *slave_mii_bus;
> >>struct mii_bus  *master_mii_bus;
> >> +
> >> +  /* Cache of programmed VLANs */
> >> +  struct bcm_sf2_vlan vlans[VLAN_N_VID];
> > 
> > Hi Florian
> > 
> > This is a 16Kbyte array. So i assume the whole priv structure needs 5
> > pages. Have you had any trouble allocating this much memory,
> > particularly once it has been used for a while and fragmented?
> 
> Well, since this is using the old binding, we can't unload the driver,
> it's built into the kernel, so initializes early enough we have got
> plenty of memory.

Don't you want to use the new binding at some point?

> > I just wondered if it might be better to use vmalloc() for the
> > vlans.
> 
> That's a very good point, I can't really see a drawback to doing this,
> will submit a patch moving this to a dynamic allocation.
> 
> Another possible approach would have been to allocate the vlan structure
> upong port_vlan_prepare() though that would typically result in more
> fragmentation over time once se start using more VLANs.

It is a trade off, code complexity vs saved memory. I don't think such
a table is too bad, assuming your not trying to run the whole system
in 32Mbyes of RAM.

   Andrew

Re: [PATCH net-next 4/4] net: dsa: bcm_sf2: Add VLAN support

[Florian Fainelli]

On 06/10/2016 05:00 AM, Andrew Lunn wrote:
>> @@ -148,6 +155,9 @@ struct bcm_sf2_priv {
>>  struct device_node  *master_mii_dn;
>>  struct mii_bus  *slave_mii_bus;
>>  struct mii_bus  *master_mii_bus;
>> +
>> +/* Cache of programmed VLANs */
>> +struct bcm_sf2_vlan vlans[VLAN_N_VID];
> 
> Hi Florian
> 
> This is a 16Kbyte array. So i assume the whole priv structure needs 5
> pages. Have you had any trouble allocating this much memory,
> particularly once it has been used for a while and fragmented?

Well, since this is using the old binding, we can't unload the driver,
it's built into the kernel, so initializes early enough we have got
plenty of memory.

> 
> I just wondered if it might be better to use vmalloc() for the
> vlans.

That's a very good point, I can't really see a drawback to doing this,
will submit a patch moving this to a dynamic allocation.

Another possible approach would have been to allocate the vlan structure
upong port_vlan_prepare() though that would typically result in more
fragmentation over time once se start using more VLANs.
-- 
Florian

Re: [PATCH net-next 4/4] net: dsa: bcm_sf2: Add VLAN support

[Andrew Lunn]

> @@ -148,6 +155,9 @@ struct bcm_sf2_priv {
>   struct device_node  *master_mii_dn;
>   struct mii_bus  *slave_mii_bus;
>   struct mii_bus  *master_mii_bus;
> +
> + /* Cache of programmed VLANs */
> + struct bcm_sf2_vlan vlans[VLAN_N_VID];

Hi Florian

This is a 16Kbyte array. So i assume the whole priv structure needs 5
pages. Have you had any trouble allocating this much memory,
particularly once it has been used for a while and fragmented?

I just wondered if it might be better to use vmalloc() for the
vlans.

Andrew

[PATCH net-next 4/4] net: dsa: bcm_sf2: Add VLAN support

[Florian Fainelli]

Add support for configuring VLANs on the Broadcom Starfigther2 switch.
This is all done through the bridge vlan facility just like other DSA
drivers.

Signed-off-by: Florian Fainelli 
---
 drivers/net/dsa/bcm_sf2.c | 266 +-
 drivers/net/dsa/bcm_sf2.h |  10 ++
 2 files changed, 275 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index d726f5906ef9..cd1d630ae3a9 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -467,7 +467,7 @@ static int bcm_sf2_fast_age_op(struct bcm_sf2_priv *priv)
u32 reg;
 
reg = core_readl(priv, CORE_FAST_AGE_CTRL);
-   reg |= EN_AGE_PORT | EN_AGE_DYNAMIC | FAST_AGE_STR_DONE;
+   reg |= EN_AGE_PORT | EN_AGE_VLAN | EN_AGE_DYNAMIC | FAST_AGE_STR_DONE;
core_writel(priv, reg, CORE_FAST_AGE_CTRL);
 
do {
@@ -498,13 +498,86 @@ static int bcm_sf2_sw_fast_age_port(struct dsa_switch 
*ds, int port)
return bcm_sf2_fast_age_op(priv);
 }
 
+static int bcm_sf2_sw_fast_age_vlan(struct bcm_sf2_priv *priv, u16 vid)
+{
+   core_writel(priv, vid, CORE_FAST_AGE_VID);
+
+   return bcm_sf2_fast_age_op(priv);
+}
+
+static int bcm_sf2_vlan_op_wait(struct bcm_sf2_priv *priv)
+{
+   unsigned int timeout = 10;
+   u32 reg;
+
+   do {
+   reg = core_readl(priv, CORE_ARLA_VTBL_RWCTRL);
+   if (!(reg & ARLA_VTBL_STDN))
+   return 0;
+
+   usleep_range(1000, 2000);
+   } while (timeout--);
+
+   return -ETIMEDOUT;
+}
+
+static int bcm_sf2_vlan_op(struct bcm_sf2_priv *priv, u8 op)
+{
+   core_writel(priv, ARLA_VTBL_STDN | op, CORE_ARLA_VTBL_RWCTRL);
+
+   return bcm_sf2_vlan_op_wait(priv);
+}
+
+static void bcm_sf2_set_vlan_entry(struct bcm_sf2_priv *priv, u16 vid,
+  struct bcm_sf2_vlan *vlan)
+{
+   int ret;
+
+   core_writel(priv, vid & VTBL_ADDR_INDEX_MASK, CORE_ARLA_VTBL_ADDR);
+   core_writel(priv, vlan->untag << UNTAG_MAP_SHIFT | vlan->members,
+   CORE_ARLA_VTBL_ENTRY);
+
+   ret = bcm_sf2_vlan_op(priv, ARLA_VTBL_CMD_WRITE);
+   if (ret)
+   pr_err("failed to write VLAN entry\n");
+}
+
+static int bcm_sf2_get_vlan_entry(struct bcm_sf2_priv *priv, u16 vid,
+ struct bcm_sf2_vlan *vlan)
+{
+   u32 entry;
+   int ret;
+
+   core_writel(priv, vid & VTBL_ADDR_INDEX_MASK, CORE_ARLA_VTBL_ADDR);
+
+   ret = bcm_sf2_vlan_op(priv, ARLA_VTBL_CMD_READ);
+   if (ret)
+   return ret;
+
+   entry = core_readl(priv, CORE_ARLA_VTBL_ENTRY);
+   vlan->members = entry & FWD_MAP_MASK;
+   vlan->untag = (entry >> UNTAG_MAP_SHIFT) & UNTAG_MAP_MASK;
+
+   return 0;
+}
+
 static int bcm_sf2_sw_br_join(struct dsa_switch *ds, int port,
  struct net_device *bridge)
 {
struct bcm_sf2_priv *priv = ds_to_priv(ds);
+   s8 cpu_port = ds->dst->cpu_port;
unsigned int i;
u32 reg, p_ctl;
 
+   /* Make this port leave the all VLANs join since we will have proper
+* VLAN entries from now on
+*/
+   reg = core_readl(priv, CORE_JOIN_ALL_VLAN_EN);
+   reg &= ~BIT(port);
+   if ((reg & BIT(cpu_port)) == BIT(cpu_port))
+   reg &= ~BIT(cpu_port);
+   core_writel(priv, reg, CORE_JOIN_ALL_VLAN_EN);
+
priv->port_sts[port].bridge_dev = bridge;
p_ctl = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(port));
 
@@ -536,6 +609,7 @@ static void bcm_sf2_sw_br_leave(struct dsa_switch *ds, int 
port)
 {
struct bcm_sf2_priv *priv = ds_to_priv(ds);
struct net_device *bridge = priv->port_sts[port].bridge_dev;
+   s8 cpu_port = ds->dst->cpu_port;
unsigned int i;
u32 reg, p_ctl;
 
@@ -559,6 +633,13 @@ static void bcm_sf2_sw_br_leave(struct dsa_switch *ds, int 
port)
core_writel(priv, p_ctl, CORE_PORT_VLAN_CTL_PORT(port));
priv->port_sts[port].vlan_ctl_mask = p_ctl;
priv->port_sts[port].bridge_dev = NULL;
+
+   /* Make this port join all VLANs without VLAN entries */
+   reg = core_readl(priv, CORE_JOIN_ALL_VLAN_EN);
+   reg |= BIT(port);
+   if (!(reg & BIT(cpu_port)))
+   reg |= BIT(cpu_port);
+   core_writel(priv, reg, CORE_JOIN_ALL_VLAN_EN);
 }
 
 static void bcm_sf2_sw_br_set_stp_state(struct dsa_switch *ds, int port,
@@ -1312,6 +1393,182 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, 
int port,
return p->ethtool_ops->set_wol(p, wol);
 }
 
+static void bcm_sf2_enable_vlan(struct bcm_sf2_priv *priv, bool enable)
+{
+   u32 mgmt, vc0, vc1, vc4, vc5;
+
+   mgmt = core_readl(priv, CORE_SWMODE);
+   vc0 = core_readl(priv, CORE_VLAN_CTRL0);
+   vc1 = core_readl(priv, CORE_VLAN_CTRL1);
+   vc4 = core_readl(priv, CORE_VLAN_CTRL4);
+   vc5 = core_readl(priv, CORE_VLAN_CTRL5);
+
+   mgmt &=