On Fri, Nov 30, 2018 at 12:21:11PM +1000, David Gwynne wrote: > i have a plan to allow virtual interfaces (eg, vlan, etherip, etc) to > provide their own output functions so they can bypass the ifq machinery > and push the packet onto the underlying layer directly. > > they'll still need to get an ethernet header though. vlan needs to get > the ethernet header and put the vlan shim into it, therefore > ether_resolve is exposed. etherip doesnt need a shim, it just wants > ethernet encapsulating the payload before adding its own headers to the > packet, therefore there is ether_encap. > > does this make sense? > > ok?
Two comments below. Apart from that OK claudio@ > Index: net/if_ethersubr.c > =================================================================== > RCS file: /cvs/src/sys/net/if_ethersubr.c,v > retrieving revision 1.253 > diff -u -p -r1.253 if_ethersubr.c > --- net/if_ethersubr.c 13 Mar 2018 01:31:48 -0000 1.253 > +++ net/if_ethersubr.c 30 Nov 2018 02:02:58 -0000 > @@ -178,24 +178,18 @@ ether_rtrequest(struct ifnet *ifp, int r > break; > } > } > -/* > - * Ethernet output routine. > - * Encapsulate a packet of type family for the local net. > - * Assumes that ifp is actually pointer to arpcom structure. > - */ > + > int > -ether_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, > - struct rtentry *rt) > +ether_resolve(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, > + struct rtentry *rt, struct ether_header *eh) > { > - u_int16_t etype; > - u_char edst[ETHER_ADDR_LEN]; > - u_char *esrc; > - struct mbuf *mcopy = NULL; > - struct ether_header *eh; > struct arpcom *ac = (struct arpcom *)ifp; > sa_family_t af = dst->sa_family; > int error = 0; > > + if (!ISSET(ifp->if_flags, IFF_RUNNING)) > + senderr(ENETDOWN); > + > KASSERT(rt != NULL || ISSET(m->m_flags, M_MCAST|M_BCAST) || > af == AF_UNSPEC || af == pseudo_AF_HDRCMPLT); > > @@ -207,28 +201,31 @@ ether_output(struct ifnet *ifp, struct m > } > #endif > > - esrc = ac->ac_enaddr; > - > - if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING)) > - senderr(ENETDOWN); > - > switch (af) { > case AF_INET: > - error = arpresolve(ifp, rt, m, dst, edst); > + error = arpresolve(ifp, rt, m, dst, eh->ether_dhost); > if (error) > - return (error == EAGAIN ? 0 : error); > + return (error); > + eh->ether_type = htons(ETHERTYPE_IP); > + > /* If broadcasting on a simplex interface, loopback a copy */ > - if ((m->m_flags & M_BCAST) && (ifp->if_flags & IFF_SIMPLEX) && > - !m->m_pkthdr.pf.routed) > + if (ISSET(m->m_flags, M_BCAST) && > + ISSET(ifp->if_flags, IFF_SIMPLEX) && > + !m->m_pkthdr.pf.routed) { > + struct mbuf *mcopy; > + > + /* XXX Should we input an unencrypted IPsec packet? */ > mcopy = m_copym(m, 0, M_COPYALL, M_NOWAIT); > - etype = htons(ETHERTYPE_IP); > + if (mcopy != NULL) > + if_input_local(ifp, mcopy, af); > + } > break; > #ifdef INET6 > case AF_INET6: > - error = nd6_resolve(ifp, rt, m, dst, edst); > + error = nd6_resolve(ifp, rt, m, dst, eh->ether_dhost); > if (error) > - return (error == EAGAIN ? 0 : error); > - etype = htons(ETHERTYPE_IPV6); > + return (error); > + eh->ether_type = htons(ETHERTYPE_IPV6); > break; > #endif > #ifdef MPLS > @@ -242,72 +239,102 @@ ether_output(struct ifnet *ifp, struct m > senderr(ENETUNREACH); > > switch (dst->sa_family) { > - case AF_LINK: > - if (satosdl(dst)->sdl_alen < sizeof(edst)) > - senderr(EHOSTUNREACH); > - memcpy(edst, LLADDR(satosdl(dst)), > - sizeof(edst)); > - break; > + case AF_LINK: > + if (satosdl(dst)->sdl_alen < sizeof(eh->ether_dhost)) > + senderr(EHOSTUNREACH); > + memcpy(eh->ether_dhost, LLADDR(satosdl(dst)), > + sizeof(eh->ether_dhost)); > + break; > #ifdef INET6 > - case AF_INET6: > - error = nd6_resolve(ifp, rt, m, dst, edst); > - if (error) > - return (error == EAGAIN ? 0 : error); > - break; > + case AF_INET6: > + error = nd6_resolve(ifp, rt, m, dst, eh->ether_dhost); > + if (error) > + return (error == EAGAIN ? 0 : error); In other places you removed the EAGAIN handling, why not here... > + break; > #endif > - case AF_INET: > - case AF_MPLS: > - error = arpresolve(ifp, rt, m, dst, edst); > - if (error) > - return (error == EAGAIN ? 0 : error); > - break; > - default: > - senderr(EHOSTUNREACH); > + case AF_INET: > + case AF_MPLS: > + error = arpresolve(ifp, rt, m, dst, eh->ether_dhost); > + if (error) > + return (error == EAGAIN ? 0 : error); ... and here. > + break; > + default: > + senderr(EHOSTUNREACH); > } > /* XXX handling for simplex devices in case of M/BCAST ?? */ > if (m->m_flags & (M_BCAST | M_MCAST)) > - etype = htons(ETHERTYPE_MPLS_MCAST); > + eh->ether_type = htons(ETHERTYPE_MPLS_MCAST); > else > - etype = htons(ETHERTYPE_MPLS); > + eh->ether_type = htons(ETHERTYPE_MPLS); > break; > #endif /* MPLS */ > case pseudo_AF_HDRCMPLT: > - eh = (struct ether_header *)dst->sa_data; > - esrc = eh->ether_shost; > - /* FALLTHROUGH */ > + /* take the whole header from the sa */ > + memcpy(eh, dst->sa_data, sizeof(*eh)); > + return (0); > > case AF_UNSPEC: > - eh = (struct ether_header *)dst->sa_data; > - memcpy(edst, eh->ether_dhost, sizeof(edst)); > - /* AF_UNSPEC doesn't swap the byte order of the ether_type. */ > - etype = eh->ether_type; > + /* take the dst and type from the sa, but get src below */ > + memcpy(eh, dst->sa_data, sizeof(*eh)); > break; > > default: > - printf("%s: can't handle af%d\n", ifp->if_xname, > - dst->sa_family); > + printf("%s: can't handle af%d\n", ifp->if_xname, af); > senderr(EAFNOSUPPORT); > } > > - /* XXX Should we feed-back an unencrypted IPsec packet ? */ > - if (mcopy) > - if_input_local(ifp, mcopy, dst->sa_family); > + memcpy(eh->ether_shost, ac->ac_enaddr, sizeof(eh->ether_shost)); > > - M_PREPEND(m, sizeof(struct ether_header) + ETHER_ALIGN, M_DONTWAIT); > - if (m == NULL) > - return (ENOBUFS); > - m_adj(m, ETHER_ALIGN); > - eh = mtod(m, struct ether_header *); > - eh->ether_type = etype; > - memcpy(eh->ether_dhost, edst, sizeof(eh->ether_dhost)); > - memcpy(eh->ether_shost, esrc, sizeof(eh->ether_shost)); > + return (0); > > - return (if_enqueue(ifp, m)); > bad: > m_freem(m); > return (error); > } > > +struct mbuf* > +ether_encap(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, > + struct rtentry *rt, int *errorp) > +{ > + struct ether_header eh; > + int error; > + > + error = ether_resolve(ifp, m, dst, rt, &eh); > + switch (error) { > + case 0: > + break; > + case EAGAIN: > + error = 0; > + default: > + *errorp = error; > + return (NULL); > + } > + > + m = m_prepend(m, ETHER_ALIGN + sizeof(eh), M_DONTWAIT); > + if (m == NULL) { > + *errorp = ENOBUFS; > + return (NULL); > + } > + > + m_adj(m, ETHER_ALIGN); > + memcpy(mtod(m, struct ether_header *), &eh, sizeof(eh)); > + > + return (m); > +} > + > +int > +ether_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst, > + struct rtentry *rt) > +{ > + int error; > + > + m = ether_encap(ifp, m, dst, rt, &error); > + if (m == NULL) > + return (error); > + > + return (if_enqueue(ifp, m)); > +} > + > /* > * Process a received Ethernet packet; > * the packet is in the mbuf chain m without > Index: netinet/if_ether.h > =================================================================== > RCS file: /cvs/src/sys/netinet/if_ether.h,v > retrieving revision 1.73 > diff -u -p -r1.73 if_ether.h > --- netinet/if_ether.h 29 Nov 2016 10:09:57 -0000 1.73 > +++ netinet/if_ether.h 30 Nov 2018 02:02:58 -0000 > @@ -240,8 +245,13 @@ void ether_ifattach(struct ifnet *); > void ether_ifdetach(struct ifnet *); > int ether_ioctl(struct ifnet *, struct arpcom *, u_long, caddr_t); > int ether_input(struct ifnet *, struct mbuf *, void *); > -int ether_output(struct ifnet *, > - struct mbuf *, struct sockaddr *, struct rtentry *); > +int ether_resolve(struct ifnet *, struct mbuf *, struct sockaddr *, > + struct rtentry *, struct ether_header *); > +struct mbuf * > + ether_encap(struct ifnet *, struct mbuf *, struct sockaddr *, > + struct rtentry *, int *); > +int ether_output(struct ifnet *, struct mbuf *, struct sockaddr *, > + struct rtentry *); > void ether_rtrequest(struct ifnet *, int, struct rtentry *); > char *ether_sprintf(u_char *); > > -- :wq Claudio