I get that trunk ports should not be able to be added to bridges or have carp
interfaces hanging off them, but I think the value of making the if_type
immutable outweighs this usability feature. Especially when you consider that
you can have an interface that is already a member of a bridge (or have the
other things on it) and then add it to trunk, and the system thinks that it is
fine.
It gets more confusing when you think about whether things like vlan or bpe are
"service delimiting" or not, and how they're supposed to interact with trunk.
If trunk is enabled on a physical interface, should vlan be allowed to coexist
with it? If the trunk is doing LACP, you could argue no, but if it's doing
failover or broadcast then maybe you want that to operate independently for
different vlans and the "native" vlan on the one physical interface.
If we ever want to support "independent" LACP trunk port operation like a
variety of switches do now, then it makes sense to maintain IFT_ETHER too.
dlg
>
> On 11 Jun 2019, at 17:32, Reyk Floeter <r...@openbsd.org> wrote:
>
> Hi,
>
> the initial intention was to differentiate a trunk port from a regular
> Ethernet interface.
>
> As long as an interface is a member of a trunk, it is not a fully featured
> Ethernet interface. The changed type prevented from using it elsewhere.
>
> I‘m not so familiar with the current network stack anymore, so maybe there
> are other ways to do it these days, but you should test that a trunk port
> cannot be attached to a bridge or carp or anything like this.
>
> You also forgot a comment that mentions the type as well.
>
> Reyk
>
>> Am 11.06.2019 um 08:33 schrieb David Gwynne <da...@gwynne.id.au>:
>>
>> i think trunk(4) is the only thing left in the kernel that modifies an
>> interfaces if_type at runtime. this diff removes that fiddling, so
>> hopefully we can say that if_type is immutable after this.
>>
>> however, while this diff reads well to me, i don't actually know if it
>> works. could someone kick the tyres for me?
>>
>> cheers,
>> dlg
>>
>> Index: if_trunk.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_trunk.c,v
>> retrieving revision 1.140
>> diff -u -p -r1.140 if_trunk.c
>> --- if_trunk.c 11 May 2019 18:10:45 -0000 1.140
>> +++ if_trunk.c 11 Jun 2019 06:31:29 -0000
>> @@ -330,10 +330,7 @@ trunk_port_create(struct trunk_softc *tr
>> }
>> }
>>
>> - /* Change the interface type */
>> - tp->tp_iftype = ifp->if_type;
>> - ifp->if_type = IFT_IEEE8023ADLAG;
>> -
>> + /* Change the interface methods */
>> tp->tp_ioctl = ifp->if_ioctl;
>> ifp->if_ioctl = trunk_port_ioctl;
>>
>> @@ -422,9 +419,7 @@ trunk_port_destroy(struct trunk_port *tp
>> if (tr->tr_port_destroy != NULL)
>> (*tr->tr_port_destroy)(tp);
>>
>> - /* Restore interface type. */
>> - ifp->if_type = tp->tp_iftype;
>> -
>> + /* Restore interface methods. */
>> ifp->if_ioctl = tp->tp_ioctl;
>> ifp->if_output = tp->tp_output;
>>
>> @@ -474,8 +469,7 @@ trunk_port_ioctl(struct ifnet *ifp, u_lo
>> int error = 0;
>>
>> /* Should be checked by the caller */
>> - if (ifp->if_type != IFT_IEEE8023ADLAG ||
>> - (tp = trunk_port_get(NULL, ifp)) == NULL ||
>> + if ((tp = trunk_port_get(NULL, ifp)) == NULL ||
>> (tr = (struct trunk_softc *)tp->tp_trunk) == NULL) {
>> error = EINVAL;
>> goto fallback;
>> @@ -521,8 +515,7 @@ trunk_port_output(struct ifnet *ifp, str
>> struct rtentry *rt)
>> {
>> /* restrict transmission on trunk members to bpf only */
>> - if (ifp->if_type == IFT_IEEE8023ADLAG &&
>> - (m_tag_find(m, PACKET_TAG_DLT, NULL) == NULL)) {
>> + if ((m_tag_find(m, PACKET_TAG_DLT, NULL) == NULL)) {
>> m_freem(m);
>> return (EBUSY);
>> }
>> @@ -1123,10 +1116,6 @@ trunk_input(struct ifnet *ifp, struct mb
>> eh = mtod(m, struct ether_header *);
>> if (ETHER_IS_MULTICAST(eh->ether_dhost))
>> ifp->if_imcasts++;
>> -
>> - /* Should be checked by the caller */
>> - if (ifp->if_type != IFT_IEEE8023ADLAG)
>> - goto bad;
>>
>> tp = (struct trunk_port *)cookie;
>> if ((tr = (struct trunk_softc *)tp->tp_trunk) == NULL)
>>
>
