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

Reply via email to