port: read a trunk's members through one function

[DrDoof]

Small on its own, and the first piece of something larger, so let me say where it is going as well as what it does.

The member mask of an aggregation group lives in four registers at RTL837X_TRK_MBR_CTRL_BASE. Two places decode it by hand today, the lag command and the JSON behind the aggregation page, with the same two lines each, and a third writes it. My Spanning Tree work in #325 has a fourth copy of the decode, which is what made me look. port_lag_members_get() now sits next to port_lag_members_set(), the two readers call it, and the writer is untouched.

It answers from the hardware, so it covers a group configured with lag and a group a protocol brought up without either having to declare itself, which matters for the direction below.

No behaviour change. The expression is the one that was already written in both places, and neither caller depended on what the read left in sfr_data, since both go on to read the hash register next. 26 bytes of BANK1, 34 fewer of BANK2 and one byte of xdata for the banked parameter, so eight bytes less code in total. I read the generated assembly rather than assuming: the address still comes out as 0x4f38 plus the group shifted twice.

What it does not give is worth being blunt about, because the title could be read as more than it is. There is still no way to configure an aggregate as one interface. VLAN membership, PVID, egress tagging, ingress filtering, isolation, the learning constraint and MTU are all per physical port in this ASIC: the VLAN table holds a bitmask of physical ports, PVID sits at PVID_BASE + ((port >> 1) << 2), the egress tag bits at port << 1, isolation and the learning constraint at + (port << 2). Nothing about a trunk exists at that level, so a trunk interface has to be a software layer, not a register.

Nor does this add a port to group map. port_lag_members_get() answers "who is in group N", not "which group is port N in".

The practical consequence of both, today: nothing stops a member of a working aggregate being given a different VLAN from its peers. The group then forwards asymmetrically depending on which member the hash picks, and no part of the firmware says a word about it.

As for how I would like to continue, there are three steps of which this is the first, and I would rather agree the shape with you than turn up with all of it at once.

Second, a port to group lookup, added when there is a consumer for it rather than before. #325 is that consumer: it keeps a private membership map and a private port to entity array purely because there was nothing shared to use. When that lands, it drops both and gets smaller.

Third, the expansion layer, which is the part that actually delivers a trunk interface. A command naming a group expands to the member mask before touching any register, membership changes re-apply the group's configuration to the port that joined or left, and the saved configuration stores the group form rather than one line per member. #325 already establishes that last pattern for the STP commands in config.js, so generalising it is extending something rather than inventing it. The open question there is what happens when a member is configured individually: refuse it, or accept it and re-sync the group.

If you would rather that whole direction stayed out of the tree, say so and I will keep it on my side and leave this PR as the plain deduplication it is, which stands on its own either way.

[vDorst]

Need lag to be on __xdata?

Maybe this is micro-optimization but we can skip the write&read to/from sfr_data.

uint16_t port_lag_members_get(uint8_t lag) __banked
{
	reg_read(RTL837X_TRK_MBR_CTRL_BASE + (lag << 2));
	return ((uint16_t)SFR_DATA_8 << 8) | SFR_DATA_0;
}

[DrDoof]

Both in, thanks. The parameter loses __xdata and the read goes through reg_read(), so the xdata byte is gone and the function is smaller.

[vDorst]

Thanks!