Some interfaces are marked as busy ( promiscuitous ?) so they can’t be
added in two bridges for example, this could be extended and make things
more consistent?
Just my two cents
On Tue, Jun 11, 2019 at 8:27 PM David Gwynne <da...@gwynne.id.au> wrote:
> 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)
> >>
> >
>
> --
--
---------------------------------------------------------------------------------------------------------------------
Knowing is not enough; we must apply. Willing is not enough; we must do
