Hi,

Am 17.09.2014 19:50, schrieb John Crispin:
> 
> 
> On 17/09/2014 19:47, Florian Fainelli wrote:
>> On 08/31/2014 10:42 AM, Jiri Pirko wrote:
>>> Sat, Jul 19, 2014 at 09:49:38PM CEST, nolt...@gmail.com wrote:
>>>> Commit 40842 reverted the fix for tagged+untagged VLANs on
>>>> AR8327: https://dev.openwrt.org/changeset/40777 
>>>> https://dev.openwrt.org/changeset/40842
>>>>
>>>> According to jow, some people experienced some "issues" on
>>>> older devices. Can anyone tell me what were those issues?
>>>>
>>>> Anyway, that patch modified some parts of the ar8216/ar8236, so
>>>> I suppose any device with those switches were affected. 
>>>> However, I've modified the patch keeping the ar8216/ar8236 as
>>>> much untouched as possible. Could anyone test it on those
>>>> devices?
>>>>
>>>> BTW, this works for me on a TP-Link WDR4300 (ar8327).
>>>
>>>
>>> I tested the patch on TL-WR1043ND v2 with Atheros AR8327 rev. 4.
>>> Vlans are working as expected. Please include this into BB (might
>>> need repost)
>>>
>>> Thanks!
>>>
>>> Tested-by: Jiri Pirko <j...@resnulli.us>
>>
>> Unless there are further objections, we should probably just go
>> ahead and apply this patch since it affects a bunch of users.
> 
> and then the other bunch complains, as we had before. we keep applying
> and reversing this i think. maybe we should just see what the real bug
> is ?
> 
>       John

I'm interested in this feature. I tried to understand what the revoked
patch changes and rewrote it. I submitted my changes two months ago:
http://patchwork.openwrt.org/patch/5957/
http://patchwork.openwrt.org/patch/5958/

My patches attempt to minimize changes in non-ar8327-specific code.

Neither the revert commit nor ticket #12181 describe the issues. So I'm
not sure, if my patches have them too. Does anyone know details about
the issues?

- Valentin Spreckels

> 
>>
>>>
>>>
>>>>
>>>> Signed-off-by: Saverio Proto <ziopr...@gmail.com> 
>>>> Signed-off-by: Álvaro Fernández <nolt...@gmail.com> --- diff
>>>> --git a/target/linux/generic/files/drivers/net/phy/ar8216.c
>>>> b/target/linux/generic/files/drivers/net/phy/ar8216.c index
>>>> 3846159..9eae624 100644 ---
>>>> a/target/linux/generic/files/drivers/net/phy/ar8216.c +++
>>>> b/target/linux/generic/files/drivers/net/phy/ar8216.c @@ -78,7
>>>> +78,7 @@ struct ar8xxx_chip { u32 (*read_port_status)(struct
>>>> ar8xxx_priv *priv, int port); int (*atu_flush)(struct
>>>> ar8xxx_priv *priv); void (*vtu_flush)(struct ar8xxx_priv
>>>> *priv); -  void (*vtu_load_vlan)(struct ar8xxx_priv *priv, u32
>>>> vid, u32 port_mask); +     void (*vtu_load_vlan)(struct ar8xxx_priv
>>>> *priv, u32 vlan);
>>>>
>>>> const struct ar8xxx_mib_desc *mib_decs; unsigned num_mibs; @@
>>>> -112,7 +112,12 @@ struct ar8327_led { enum ar8327_led_pattern
>>>> pattern; };
>>>>
>>>> +struct ar8216_data { +    u8 vlan_tagged; +}; + struct
>>>> ar8327_data { +    u8 vlan_tagged[AR8X16_MAX_VLANS]; u32
>>>> port0_status; u32 port6_status;
>>>>
>>>> @@ -138,6 +143,7 @@ struct ar8xxx_priv { u8 chip_rev; const
>>>> struct ar8xxx_chip *chip; union { +                struct ar8216_data
>>>> ar8216; struct ar8327_data ar8327; } chip_data; bool
>>>> initialized; @@ -159,7 +165,6 @@ struct ar8xxx_priv { bool
>>>> vlan; u16 vlan_id[AR8X16_MAX_VLANS]; u8
>>>> vlan_table[AR8X16_MAX_VLANS]; -    u8 vlan_tagged; u16
>>>> pvid[AR8X16_MAX_PORTS];
>>>>
>>>> /* mirroring */ @@ -641,7 +646,7 @@ ar8216_mangle_rx(struct
>>>> net_device *dev, struct sk_buff *skb) port = buf[0] & 0xf;
>>>>
>>>> /* no need to fix up packets coming from a tagged source */ -
>>>> if (priv->vlan_tagged & (1 << port)) +     if
>>>> (priv->chip_data.ar8216.vlan_tagged & BIT(port)) return;
>>>>
>>>> /* lookup port vid from local table, the switch passes an
>>>> invalid vlan id */ @@ -695,10 +700,13 @@
>>>> ar8216_vtu_flush(struct ar8xxx_priv *priv) }
>>>>
>>>> static void -ar8216_vtu_load_vlan(struct ar8xxx_priv *priv, u32
>>>> vid, u32 port_mask) +ar8216_vtu_load_vlan(struct ar8xxx_priv
>>>> *priv, u32 vlan) { u32 op;
>>>>
>>>> +  u32 vid = priv->vlan_id[vlan]; +        u32 port_mask =
>>>> priv->vlan_table[vlan]; + op = AR8216_VTU_OP_LOAD | (vid <<
>>>> AR8216_VTU_VID_S); ar8216_vtu_op(priv, op, port_mask); } @@
>>>> -1705,12 +1713,16 @@ ar8327_vtu_flush(struct ar8xxx_priv
>>>> *priv) }
>>>>
>>>> static void -ar8327_vtu_load_vlan(struct ar8xxx_priv *priv, u32
>>>> vid, u32 port_mask) +ar8327_vtu_load_vlan(struct ar8xxx_priv
>>>> *priv, u32 vlan) { u32 op; u32 val; int i;
>>>>
>>>> +  u32 vid = priv->vlan_id[vlan]; +        u32 port_mask =
>>>> priv->vlan_table[vlan]; +  u32 tagged =
>>>> priv->chip_data.ar8327.vlan_tagged[vlan]; + op =
>>>> AR8327_VTU_FUNC1_OP_LOAD | (vid << AR8327_VTU_FUNC1_VID_S); val
>>>> = AR8327_VTU_FUNC0_VALID | AR8327_VTU_FUNC0_IVL; for (i = 0; i
>>>> < AR8327_NUM_PORTS; i++) { @@ -1720,7 +1732,7 @@
>>>> ar8327_vtu_load_vlan(struct ar8xxx_priv *priv, u32 vid, u32
>>>> port_mask) mode = AR8327_VTU_FUNC0_EG_MODE_NOT; else if
>>>> (priv->vlan == 0) mode = AR8327_VTU_FUNC0_EG_MODE_KEEP; -          else
>>>> if (priv->vlan_tagged & BIT(i)) +          else if (tagged & BIT(i)) 
>>>> mode = AR8327_VTU_FUNC0_EG_MODE_TAG; else mode =
>>>> AR8327_VTU_FUNC0_EG_MODE_UNTAG; @@ -1734,26 +1746,22 @@ static
>>>> void ar8327_setup_port(struct ar8xxx_priv *priv, int port, u32
>>>> egress, u32 ingress, u32 members, u32 pvid) { -    u32 t; -        u32
>>>> mode; +    u32 mode, t; + +        if (priv->vlan) { +             pvid =
>>>> priv->vlan_id[priv->pvid[port]]; +         mode =
>>>> AR8327_PORT_VLAN1_OUT_MODE_UNMOD; +                ingress =
>>>> AR8216_IN_SECURE; +        } else { +              pvid = port; +          
>>>> mode =
>>>> AR8327_PORT_VLAN1_OUT_MODE_UNTOUCH; +              ingress =
>>>> AR8216_IN_PORT_ONLY; +     }
>>>>
>>>> t = pvid << AR8327_PORT_VLAN0_DEF_SVID_S; t |= pvid <<
>>>> AR8327_PORT_VLAN0_DEF_CVID_S; priv->write(priv,
>>>> AR8327_REG_PORT_VLAN0(port), t);
>>>>
>>>> -  mode = AR8327_PORT_VLAN1_OUT_MODE_UNMOD; -      switch (egress) { 
>>>> -  case AR8216_OUT_KEEP: -         mode =
>>>> AR8327_PORT_VLAN1_OUT_MODE_UNTOUCH; -              break; -        case
>>>> AR8216_OUT_STRIP_VLAN: -           mode =
>>>> AR8327_PORT_VLAN1_OUT_MODE_UNTAG; -                break; -        case
>>>> AR8216_OUT_ADD_VLAN: -             mode = AR8327_PORT_VLAN1_OUT_MODE_TAG; 
>>>> -          break; -        } - t = AR8327_PORT_VLAN1_PORT_VLAN_PROP; t |= 
>>>> mode
>>>> << AR8327_PORT_VLAN1_OUT_MODE_S; priv->write(priv,
>>>> AR8327_REG_PORT_VLAN1(port), t); @@ -1851,23 +1859,21 @@
>>>> ar8xxx_sw_get_port_link(struct switch_dev *dev, int port, }
>>>>
>>>> static int -ar8xxx_sw_get_ports(struct switch_dev *dev, struct
>>>> switch_val *val) +ar8xxx_sw_get_ports(struct switch_val *val,
>>>> int ports, u8 port_mask, u8 tagged) { -    struct ar8xxx_priv
>>>> *priv = swdev_to_ar8xxx(dev); -    u8 ports =
>>>> priv->vlan_table[val->port_vlan]; int i;
>>>>
>>>> val->len = 0; -    for (i = 0; i < dev->ports; i++) { +    for (i =
>>>> 0; i < ports; i++) { struct switch_port *p;
>>>>
>>>> -          if (!(ports & (1 << i))) +              if (!(port_mask & 
>>>> BIT(i))) 
>>>> continue;
>>>>
>>>> p = &val->value.ports[val->len++]; p->id = i; -            if
>>>> (priv->vlan_tagged & (1 << i)) -                   p->flags = (1 <<
>>>> SWITCH_PORT_FLAG_TAGGED); +                if (tagged & BIT(i)) +          
>>>>         p->flags
>>>> = BIT(SWITCH_PORT_FLAG_TAGGED); else p->flags = 0; } @@
>>>> -1875,20 +1881,55 @@ ar8xxx_sw_get_ports(struct switch_dev
>>>> *dev, struct switch_val *val) }
>>>>
>>>> static int -ar8xxx_sw_set_ports(struct switch_dev *dev, struct
>>>> switch_val *val) +ar8216_sw_get_ports(struct switch_dev *dev,
>>>> struct switch_val *val) +{ +       int ports = dev->ports; +       struct
>>>> ar8xxx_priv *priv = swdev_to_ar8xxx(dev); +        u8 port_mask =
>>>> priv->vlan_table[val->port_vlan]; +        u8 tagged =
>>>> priv->chip_data.ar8216.vlan_tagged; + +    return
>>>> ar8xxx_sw_get_ports(val, ports, port_mask, tagged); +} + 
>>>> +static int +ar8327_sw_get_ports(struct switch_dev *dev, struct
>>>> switch_val *val) +{ +      int ports = dev->ports; +       struct
>>>> ar8xxx_priv *priv = swdev_to_ar8xxx(dev); +        u8 port_mask =
>>>> priv->vlan_table[val->port_vlan]; +        u8 tagged =
>>>> priv->chip_data.ar8327.vlan_tagged[val->port_vlan]; + +    return
>>>> ar8xxx_sw_get_ports(val, ports, port_mask, tagged); +} + 
>>>> +static int +ar8216_sw_set_ports(struct switch_dev *dev, struct
>>>> switch_val *val) { struct ar8xxx_priv *priv =
>>>> swdev_to_ar8xxx(dev); u8 *vt =
>>>> &priv->vlan_table[val->port_vlan]; +       u8 *tagged =
>>>> &priv->chip_data.ar8216.vlan_tagged; int i, j;
>>>>
>>>> *vt = 0; for (i = 0; i < val->len; i++) { struct switch_port *p
>>>> = &val->value.ports[i];
>>>>
>>>> -          if (p->flags & (1 << SWITCH_PORT_FLAG_TAGGED)) { -
>>>> priv->vlan_tagged |= (1 << p->id); +               if (p->flags &
>>>> BIT(SWITCH_PORT_FLAG_TAGGED)) { + +                        /* if port was 
>>>> untagged
>>>> before then +                       * remove him from other vlans */ +
>>>> if(*tagged & BIT(p->id)){ +                                for (j = 0; j <
>>>> AR8X16_MAX_VLANS; j++) { +                                 if (j == 
>>>> val->port_vlan) +
>>>> continue; + +                                      priv->vlan_table[j] &= 
>>>> ~(BIT(p->id)); +                         } 
>>>> +                  } + +                   *tagged |= BIT(p->id); } else { 
>>>> -
>>>> priv->vlan_tagged &= ~(1 << p->id); +                      *tagged &=
>>>> ~(BIT(p->id)); priv->pvid[p->id] = val->port_vlan;
>>>>
>>>> /* make sure that an untagged port does not @@ -1896,11
>>>> +1937,38 @@ ar8xxx_sw_set_ports(struct switch_dev *dev, struct
>>>> switch_val *val) for (j = 0; j < AR8X16_MAX_VLANS; j++) { if (j
>>>> == val->port_vlan) continue; -                             
>>>> priv->vlan_table[j] &= ~(1 <<
>>>> p->id); + +                                priv->vlan_table[j] &= 
>>>> ~(BIT(p->id)); } }
>>>>
>>>> -          *vt |= 1 << p->id; +            *vt |= BIT(p->id); +    } +     
>>>> return 0; +} 
>>>> + +static int +ar8327_sw_set_ports(struct switch_dev *dev,
>>>> struct switch_val *val) +{ +       struct ar8xxx_priv *priv =
>>>> swdev_to_ar8xxx(dev); +    u8 *vt =
>>>> &priv->vlan_table[val->port_vlan]; +       u8 *vlan_tagged =
>>>> priv->chip_data.ar8327.vlan_tagged; +      u8 *tagged =
>>>> &vlan_tagged[val->port_vlan]; + +  int i; + +      *vt = 0; +      *tagged
>>>> = 0; +     for (i = 0; i < val->len; i++) { +              struct 
>>>> switch_port
>>>> *p = &val->value.ports[i]; + +             if (p->flags &
>>>> BIT(SWITCH_PORT_FLAG_TAGGED)) { +                  *tagged |= BIT(p->id); 
>>>> +                }
>>>> else { +                   priv->pvid[p->id] = val->port_vlan; +           
>>>> } + +           *vt |=
>>>> BIT(p->id); } return 0; } @@ -2019,13 +2087,12 @@
>>>> ar8xxx_sw_hw_apply(struct switch_dev *dev) continue;
>>>>
>>>> for (i = 0; i < dev->ports; i++) { -                               u8 mask 
>>>> = (1 << i); +
>>>> u8 mask = BIT(i); if (vp & mask) portmask[i] |= vp & ~mask; }
>>>>
>>>> -                  priv->chip->vtu_load_vlan(priv, priv->vlan_id[j], -
>>>> priv->vlan_table[j]); +                    priv->chip->vtu_load_vlan(priv, 
>>>> j); 
>>>> } } else { /* vlan disabled: @@ -2034,8 +2101,8 @@
>>>> ar8xxx_sw_hw_apply(struct switch_dev *dev) if (i ==
>>>> AR8216_PORT_CPU) continue;
>>>>
>>>> -                  portmask[i] = 1 << AR8216_PORT_CPU; -
>>>> portmask[AR8216_PORT_CPU] |= (1 << i); +                   portmask[i] =
>>>> BIT(AR8216_PORT_CPU); +                    portmask[AR8216_PORT_CPU] |= 
>>>> BIT(i); 
>>>> } }
>>>>
>>>> @@ -2046,7 +2113,7 @@ ar8xxx_sw_hw_apply(struct switch_dev
>>>> *dev)
>>>>
>>>> if (priv->vlan) { pvid = priv->vlan_id[priv->pvid[i]]; -                   
>>>> if
>>>> (priv->vlan_tagged & (1 << i)) +                   if
>>>> (priv->chip_data.ar8216.vlan_tagged & BIT(i)) egress =
>>>> AR8216_OUT_ADD_VLAN; else egress = AR8216_OUT_STRIP_VLAN; @@
>>>> -2442,8 +2509,8 @@ static const struct switch_dev_ops
>>>> ar8xxx_sw_ops = { }, .get_port_pvid = ar8xxx_sw_get_pvid, 
>>>> .set_port_pvid = ar8xxx_sw_set_pvid, -     .get_vlan_ports =
>>>> ar8xxx_sw_get_ports, -     .set_vlan_ports = ar8xxx_sw_set_ports, +
>>>> .get_vlan_ports = ar8216_sw_get_ports, +   .set_vlan_ports =
>>>> ar8216_sw_set_ports, .apply_config = ar8xxx_sw_hw_apply, 
>>>> .reset_switch = ar8xxx_sw_reset_switch, .get_port_link =
>>>> ar8xxx_sw_get_port_link, @@ -2464,8 +2531,8 @@ static const
>>>> struct switch_dev_ops ar8327_sw_ops = { }, .get_port_pvid =
>>>> ar8xxx_sw_get_pvid, .set_port_pvid = ar8xxx_sw_set_pvid, -
>>>> .get_vlan_ports = ar8xxx_sw_get_ports, -   .set_vlan_ports =
>>>> ar8xxx_sw_set_ports, +     .get_vlan_ports = ar8327_sw_get_ports, +
>>>> .set_vlan_ports = ar8327_sw_set_ports, .apply_config =
>>>> ar8xxx_sw_hw_apply, .reset_switch = ar8xxx_sw_reset_switch, 
>>>> .get_port_link = ar8xxx_sw_get_port_link, 
>>>> _______________________________________________ openwrt-devel
>>>> mailing list openwrt-devel@lists.openwrt.org 
>>>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>>
>>>>
> _______________________________________________
>>> openwrt-devel mailing list openwrt-devel@lists.openwrt.org 
>>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>>
>> _______________________________________________ openwrt-devel
>> mailing list openwrt-devel@lists.openwrt.org 
>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> 

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to