fixed vlangroup opration at RT3050.

[yamori813_yahoo.co.jp]

Summary

original code can't modify vlangroup at RT3050. check only member bit untag setting.

Test Plan

FON2405(RT3050)

[sgalabov]

Can you explain the problem in a bit more detail?
The code you're suggesting to change (from memory) is intended to do the following:

1. check which ports are configured to untag VLANs
2. get the subset of VLAN member ports that are also untagged.
3. compare the 2 values and if they match - go ahead; if they don't match - error out.

The idea is that if you want to add a port as an untagged member of a certain VLAN it should already be configured as a port that passes only untagged traffic (for the case of RT305x, which can't do per-VLAN untagging, or for the case of, say RT5350, which can do per-VLAN untagging or per-port untagging, based on bit POC2_UNTAG_VLAN in the MTKSWITCH_POC2 register.

Can you describe the setup that is failing for you? Especially, if you're trying to add ports as untagged members of a given VLAN, have all these been also configured as untagged ports prior to this?

[sgalabov]

OK, one issue exists with the existing code:

• if more ports are configured as "untagged" than are configured as "untagged members" for the given VLAN the check would fail.

But there's an issue with the proposed change too - it basically changes the semantics of the entire check.

Probably we should change the code like so:

if ((val & POC2_UNTAG_VLAN) == 0 ||
    sc->sc_switchtype == MTK_SWITCH_RT3050) {
        val &= VUB_MASK;
        tmp = v->es_untagged_ports & v->es_member_ports;
        if ((val & tmp) != tmp) {
                /* Cannot accomodate request */
                MTKSWITCH_UNLOCK(sc);
                return (ENOTSUP);
        }
}

[yamori813_yahoo.co.jp]

if delete to member in vlangroup the current code make error.

% etherswitchcfg vlangroup0 members 0,1,2,3,5,6
etherswitchcfg: ioctl(IOETHERSWITCHSETVLANGROUP): Operation not supported

also can't add vlangroup.

% etherswitchcfg vlangroup1 members 0,1
etherswitchcfg: ioctl(IOETHERSWITCHSETVLANGROUP): Operation not supported

[yamori813_yahoo.co.jp]

if statement is same as current code. But POC2_UNTAG_VLAN don't have RT3050. I think check RT3050 first is good.

[sgalabov]

Can you please test with the patch found at:
https://people.freebsd.org/~sgalabov/mtkswitch.patch

[yamori813_yahoo.co.jp]

This is always false in patch.

tmp = v->es_untagged_ports & v->es_member_ports;
/* fail if untagged members are not a subset of all members */
if (tmp != v->es_untagged_ports) {

Because of all v->es_untagged_ports bits contain in v->es_member_ports.

[sgalabov]

In D9607#198883, @yamori813_yahoo.co.jp wrote:

This is always false in patch.

tmp = v->es_untagged_ports & v->es_member_ports;
/* fail if untagged members are not a subset of all members */
if (tmp != v->es_untagged_ports) {

Because of all v->es_untagged_ports bits contain in v->es_member_ports.

Yes, it's expected to be false, but IMHO it makes the code flow more readable and checks for conditions that can't be satisfied in a more consistent way. The question is does it work right now?

[yamori813_yahoo.co.jp]

# etherswitchcfg vlangroup0 members 0,6   <- work
vlangroup0:
        vlan: 1
        members 0,6
# etherswitchcfg vlangroup1 members 0,1  <- work
vlangroup1:
        vlan: 0
        members 0,1
# etherswitchcfg port2 addtag
port2:
        pvid: 1
        flags=4<ADDTAG>
        media: Ethernet autoselect (none)
        status: no carrier
# etherswitchcfg vlangroup2 members 0,1,2   <-  Is this OK ?
vlangroup2:
        vlan: 0
        members 0,1,2t
#

[sgalabov]

OK, v2 patch is here:
https://people.freebsd.org/~sgalabov/mtkswitch_v2.patch

Please let me know if that's any better (it should be).

[yamori813_yahoo.co.jp]

v2 patch is good.

# etherswitchcfg vlangroup0 members 0,6
vlangroup0:
        vlan: 1
        members 0,6
# etherswitchcfg vlangroup1 members 0,1
vlangroup1:
        vlan: 0
        members 0,1
# etherswitchcfg port2 addtag
port2:
        pvid: 1
        flags=4<ADDTAG>
        media: Ethernet autoselect (none)
        status: no carrier
# etherswitchcfg vlangroup2 members 0,1,2
etherswitchcfg: ioctl(IOETHERSWITCHSETVLANGROUP): Operation not supported
# etherswitchcfg vlangroup2 members 0,1,2t
vlangroup2:
        vlan: 0
        members 0,1,2t
#

Thanks

[adrian]

ok, commit away please!