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