Wed, Sep 17, 2014 at 08:25:22PM CEST, valentin.spreck...@informatik.uni-oldenburg.de wrote: >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/
I like this patches. Look very clean. Will test them on wr1043v2. Thanks. > >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 >> > >_______________________________________________ >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