im going to commit this tomorrow unless anyone objects. i dont want to hold up vlan too much.
On Wed, Mar 02, 2016 at 11:48:49AM +1000, David Gwynne wrote: > ive got a large reworking of vlan(4) to make vlan tx mpsafe, which > affected mpw(4). > > the biggest effect was that it was no longer safe to traverse the > interface parent hierarchy looking for vlan interfaces. however, > from what i could tell from the rfc, such traversal is unecessary. > > this refactors the code so it only reinject vlan headers in tagged > mode for vlan interfaces in the same bridge as it. in raw mode it > just sends the packet as is. this is based on my interpretation of > 4.4.1 in rfc4448. > > this also has the benefit that you can assign local addresses on > mpw interfaces in raw mode and talk over them without making the > machine panic. it should also be possible to assign addresses on > tagged interfaces too. > > to be clear, not traversing the vlan interface parents is necessary > for making vlan itself mpsafe. i am confident we're still within > spec without that functionality, and actually get simplifications > and semantic improvements out of it this way. > > this diff relies on the vlan_input() code i sent to tech@ > > ok? > > Index: if_mpw.c > =================================================================== > RCS file: /cvs/src/sys/net/if_mpw.c,v > retrieving revision 1.12 > diff -u -p -r1.12 if_mpw.c > --- if_mpw.c 5 Dec 2015 10:07:55 -0000 1.12 > +++ if_mpw.c 2 Mar 2016 01:36:49 -0000 > @@ -351,164 +351,51 @@ extern void vlan_start(struct ifnet *ifp > struct mbuf * > mpw_vlan_handle(struct mbuf *m, struct mpw_softc *sc) > { > - int needsdummy = 0; > - int fakeifv = 0; > - struct ifvlan *ifv = NULL; > - struct ether_vlan_header *evh; > - struct ifnet *ifp, *ifp0; > - int nvlan, moff; > - struct ether_header eh; > - struct ifvlan fifv; > - struct vlan_shim { > - uint16_t vs_tpid; > - uint16_t vs_tci; > - } vs; > - > - ifp = ifp0 = if_get(m->m_pkthdr.ph_ifidx); > - KASSERT(ifp != NULL); > - if (ifp->if_start == vlan_start) > - ifv = ifp->if_softc; > - > - /* If we were relying on VLAN HW support, fake an ifv */ > - if (ifv == NULL && (m->m_flags & M_VLANTAG) == M_VLANTAG) { > - memset(&fifv, 0, sizeof(fifv)); > - fifv.ifv_tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag); > - fifv.ifv_prio = EVL_PRIOFTAG(m->m_pkthdr.ether_vtag); > - ifv = &fifv; > - fakeifv = 1; > - } > - > - /* > - * Always remove VLAN flag as we are inserting them here. Also we > - * might get a tagged packet with no VLAN interface, in this case > - * we can't do anything. > - */ > - m->m_flags &= ~M_VLANTAG; > - > - /* > - * Do VLAN managing. > - * > - * Case ethernet (raw): > - * No VLAN: just pass it. > - * One or more VLANs: insert VLAN tag back. > - * > - * NOTE: In case of raw access mode, the if_vlan will do the job > - * of dropping non tagged packets for us. > - */ > - if (sc->sc_type == IMR_TYPE_ETHERNET && ifv == NULL) { > - if_put(ifp0); > - return (m); > - } > - > - /* > - * Case ethernet-tagged: > - * 0 VLAN: Drop packet > - * 1 VLAN: Tag packet with dummy VLAN > - * >1 VLAN: Nothing > - */ > - if (sc->sc_type == IMR_TYPE_ETHERNET_TAGGED && ifv == NULL) { > - m_freem(m); > - if_put(ifp0); > - return (NULL); > - } > - > - /* Copy and remove ethernet header */ > - m_copydata(m, 0, sizeof(eh), (caddr_t) &eh); > - if (ntohs(eh.ether_type) == ETHERTYPE_VLAN || > - ntohs(eh.ether_type) == ETHERTYPE_QINQ) > - m_adj(m, sizeof(*evh)); > - else > - m_adj(m, sizeof(eh)); > - > - /* Count VLAN stack size */ > - nvlan = 0; > - while ((ifp = ifv->ifv_p) != NULL && ifp->if_start == vlan_start) { > - ifv = ifp->if_softc; > - nvlan++; > - } > - moff = sizeof(*evh) + (nvlan * EVL_ENCAPLEN); > - > - /* The mode ethernet tagged always need at least 2 VLANs */ > - if (sc->sc_type == IMR_TYPE_ETHERNET_TAGGED && nvlan == 0) { > - needsdummy = 1; > - moff += EVL_ENCAPLEN; > - } > - > - /* Add VLAN to the beginning of the packet */ > - M_PREPEND(m, moff, M_NOWAIT); > - if (m == NULL) { > - if_put(ifp0); > - return (NULL); > - } > - > - /* Copy original ethernet type */ > - moff -= sizeof(eh.ether_type); > - m_copyback(m, moff, sizeof(eh.ether_type), &eh.ether_type, M_NOWAIT); > - > - /* Fill inner VLAN values */ > - ifv = ifp0->if_softc; > - while (nvlan-- > 0) { > - vs.vs_tci = htons((ifv->ifv_prio << EVL_PRIO_BITS) + > - ifv->ifv_tag); > - vs.vs_tpid = htons(ifv->ifv_type); > - > - moff -= sizeof(vs); > - m_copyback(m, moff, sizeof(vs), &vs, M_NOWAIT); > - > - ifp = ifv->ifv_p; > - ifv = ifp->if_softc; > - } > - > - /* Copy ethernet header back */ > - evh = mtod(m, struct ether_vlan_header *); > - memcpy(evh->evl_dhost, eh.ether_dhost, sizeof(evh->evl_dhost)); > - memcpy(evh->evl_shost, eh.ether_shost, sizeof(evh->evl_shost)); > - > - if (fakeifv) > - ifv = &fifv; > - > - /* Insert the last VLAN and optionally a dummy VLAN */ > - if (needsdummy) { > - evh->evl_encap_proto = ntohs(ETHERTYPE_QINQ); > - evh->evl_tag = 0; > - > - vs.vs_tci = ntohs((m->m_pkthdr.pf.prio << EVL_PRIO_BITS) + > - ifv->ifv_tag); > - vs.vs_tpid = ntohs(ETHERTYPE_VLAN); > - m_copyback(m, moff, sizeof(vs), &vs, M_NOWAIT); > - } else { > - evh->evl_encap_proto = (nvlan > 0) ? > - ntohs(ETHERTYPE_QINQ) : ntohs(ETHERTYPE_VLAN); > - evh->evl_tag = ntohs((m->m_pkthdr.pf.prio << EVL_PRIO_BITS) + > - ifv->ifv_tag); > - } > - > - if_put(ifp0); > + struct ifnet *ifp; > + struct ifvlan *ifv; > + > + uint16_t type = ETHERTYPE_QINQ; > + uint16_t tag = 0; > + > + ifp = if_get(m->m_pkthdr.ph_ifidx); > + if (ifp != NULL && ifp->if_start == vlan_start) { > + ifv = ifp->if_softc; > + type = ifv->ifv_type; > + tag = ifv->ifv_tag; > + } > + if_put(ifp); > > - return (m); > + return (vlan_inject(m, type, tag)); > } > #endif /* NVLAN */ > > void > -mpw_start(struct ifnet *ifp0) > +mpw_start(struct ifnet *ifp) > { > - struct mpw_softc *sc = ifp0->if_softc; > - struct mbuf *m; > + struct mpw_softc *sc = ifp->if_softc; > struct rtentry *rt; > - struct ifnet *ifp; > + struct ifnet *p; > + struct mbuf *m; > struct shim_hdr *shim; > struct sockaddr_storage ss; > > - rt = rtalloc((struct sockaddr *) &sc->sc_nexthop, RT_RESOLVE, 0); > - if (!rtisvalid(rt)) { > - rtfree(rt); > + if (!ISSET(ifp->if_flags, IFF_RUNNING) || > + sc->sc_rshim.shim_label == 0 || > + sc->sc_type == IMR_TYPE_NONE) { > + IFQ_PURGE(&ifp->if_snd); > return; > } > > - ifp = if_get(rt->rt_ifidx); > - if (ifp == NULL) { > - rtfree(rt); > - return; > + rt = rtalloc((struct sockaddr *)&sc->sc_nexthop, RT_RESOLVE, 0); > + if (!rtisvalid(rt)) { > + IFQ_PURGE(&ifp->if_snd); > + goto rtfree; > + } > + > + p = if_get(rt->rt_ifidx); > + if (p == NULL) { > + IFQ_PURGE(&ifp->if_snd); > + goto rtfree; > } > > /* > @@ -516,38 +403,29 @@ mpw_start(struct ifnet *ifp0) > * the right place. > */ > memcpy(&ss, &sc->sc_nexthop, sizeof(sc->sc_nexthop)); > - ((struct sockaddr *) &ss)->sa_family = AF_MPLS; > - > - for (;;) { > - IFQ_DEQUEUE(&ifp0->if_snd, m); > - if (m == NULL) > - break; > - > - if ((ifp0->if_flags & IFF_RUNNING) == 0 || > - sc->sc_rshim.shim_label == 0 || > - sc->sc_type == IMR_TYPE_NONE) { > - m_freem(m); > - continue; > - } > - > -#if NVLAN > 0 > - m = mpw_vlan_handle(m, sc); > - if (m == NULL) > - continue; > -#else > - /* Ethernet tagged doesn't work without VLANs'*/ > - if (sc->sc_type == IMR_TYPE_ETHERNET_TAGGED) { > - m_freem(m); > - continue; > - } > -#endif /* NVLAN */ > + ((struct sockaddr *)&ss)->sa_family = AF_MPLS; > > + while ((m = ifq_dequeue(&ifp->if_snd)) != NULL) { > #if NBPFILTER > 0 > if (sc->sc_if.if_bpf) > bpf_mtap(sc->sc_if.if_bpf, m, BPF_DIRECTION_OUT); > #endif /* NBPFILTER */ > > - if (sc->sc_flags & IMR_FLAG_CONTROLWORD) { > + if (sc->sc_type == IMR_TYPE_ETHERNET_TAGGED) { > + #if NVLAN > 0 > + m = mpw_vlan_handle(m, sc); > + if (m == NULL) { > + ifp->if_oerrors++; > + continue; > + } > + #else > + /* Ethernet tagged doesn't work without VLANs'*/ > + m_freem(m); > + continue; > + #endif /* NVLAN */ > + } > + > + if (sc->sc_flags & IMR_FLAG_CONTROLWORD) { > M_PREPEND(m, sizeof(*shim), M_NOWAIT); > if (m == NULL) > continue; > @@ -567,9 +445,10 @@ mpw_start(struct ifnet *ifp0) > /* XXX: MPLS only uses domain 0 */ > m->m_pkthdr.ph_rtableid = 0; > > - mpls_output(ifp, m, (struct sockaddr *) &ss, rt); > + mpls_output(p, m, (struct sockaddr *)&ss, rt); > } > > - if_put(ifp); > + if_put(p); > +rtfree: > rtfree(rt); > }