> On 20 Nov 2015, at 9:36 PM, Stuart Henderson <st...@openbsd.org> wrote:
>
> Currently pppoe(4) is the only user of sppp(4) and it uses PP_NOFRAMING.
> Do we expect sppp(4) to be used for anything other than pppoe(4) in the
> future? If not, we can simplify things by removing the code to support
> framing.
>
> Diff also removes some useless ifdefs while there.
sppp will go away before it gets used for something else. im fine with you
trimming it down.
dlg
>
>
> Index: if_pppoe.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 if_pppoe.c
> --- if_pppoe.c 25 Oct 2015 11:58:11 -0000 1.49
> +++ if_pppoe.c 20 Nov 2015 11:32:07 -0000
> @@ -223,8 +223,7 @@ pppoe_clone_create(struct if_clone *ifc,
> sc->sc_sppp.pp_if.if_flags = IFF_SIMPLEX | IFF_POINTOPOINT |
> IFF_MULTICAST;
> sc->sc_sppp.pp_if.if_type = IFT_PPP;
> sc->sc_sppp.pp_if.if_hdrlen = sizeof(struct ether_header) +
> PPPOE_HEADERLEN;
> - sc->sc_sppp.pp_flags |= PP_KEEPALIVE | /* use LCP keepalive */
> - PP_NOFRAMING; /* no serial
> encapsulation */
> + sc->sc_sppp.pp_flags |= PP_KEEPALIVE; /* use LCP keepalive */
> sc->sc_sppp.pp_framebytes = PPPOE_HEADERLEN; /* framing added to ppp
> packets */
> sc->sc_sppp.pp_if.if_ioctl = pppoe_ioctl;
> sc->sc_sppp.pp_if.if_start = pppoe_start;
> Index: if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.146
> diff -u -p -r1.146 if_spppsubr.c
> --- if_spppsubr.c 11 Nov 2015 01:49:17 -0000 1.146
> +++ if_spppsubr.c 20 Nov 2015 11:32:08 -0000
> @@ -162,6 +162,8 @@
> #define STATE_ACK_SENT 8
> #define STATE_OPENED 9
>
> +#define PKTHDRLEN 2
> +
> struct ppp_header {
> u_char address;
> u_char control;
> @@ -445,16 +447,10 @@ sppp_input(struct ifnet *ifp, struct mbu
> /* mark incoming routing domain */
> m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
>
> - if (sp->pp_flags & PP_NOFRAMING) {
> - m_copydata(m, 0, sizeof(ht.protocol), (caddr_t)&ht.protocol);
> - m_adj(m, 2);
> - ht.control = PPP_UI;
> - ht.address = PPP_ALLSTATIONS;
> - } else {
> - /* Get PPP header. */
> - m_copydata(m, 0, sizeof(ht), (caddr_t)&ht);
> - m_adj (m, PPP_HEADER_LEN);
> - }
> + m_copydata(m, 0, sizeof(ht.protocol), (caddr_t)&ht.protocol);
> + m_adj(m, 2);
> + ht.control = PPP_UI;
> + ht.address = PPP_ALLSTATIONS;
>
> /* preserve the alignment */
> if (m->m_len < m->m_pkthdr.len) {
> @@ -557,7 +553,6 @@ sppp_output(struct ifnet *ifp, struct mb
> struct sockaddr *dst, struct rtentry *rt)
> {
> struct sppp *sp = (struct sppp*) ifp;
> - struct ppp_header *h;
> struct timeval tv;
> int s, rv = 0;
> u_int16_t protocol;
> @@ -620,29 +615,6 @@ sppp_output(struct ifnet *ifp, struct mb
> }
> }
>
> - if (sp->pp_flags & PP_NOFRAMING)
> - goto skip_header;
> - /*
> - * Prepend general data packet PPP header. For now, IP only.
> - */
> - M_PREPEND (m, PPP_HEADER_LEN, M_DONTWAIT);
> - if (!m) {
> - if (ifp->if_flags & IFF_DEBUG)
> - log(LOG_DEBUG, SPP_FMT "no memory for transmit
> header\n",
> - SPP_ARGS(ifp));
> - ++ifp->if_oerrors;
> - splx (s);
> - return (ENOBUFS);
> - }
> - /*
> - * May want to check size of packet
> - * (albeit due to the implementation it's always enough)
> - */
> - h = mtod (m, struct ppp_header*);
> - h->address = PPP_ALLSTATIONS; /* broadcast address */
> - h->control = PPP_UI; /* Unnumbered Info */
> -
> - skip_header:
> switch (dst->sa_family) {
> case AF_INET: /* Internet Protocol */
> /*
> @@ -681,20 +653,17 @@ sppp_output(struct ifnet *ifp, struct mb
> return (EAFNOSUPPORT);
> }
>
> - if (sp->pp_flags & PP_NOFRAMING) {
> - M_PREPEND(m, 2, M_DONTWAIT);
> - if (m == NULL) {
> - if (ifp->if_flags & IFF_DEBUG)
> - log(LOG_DEBUG, SPP_FMT
> - "no memory for transmit header\n",
> - SPP_ARGS(ifp));
> - ++ifp->if_oerrors;
> - splx(s);
> - return (ENOBUFS);
> - }
> - *mtod(m, u_int16_t *) = protocol;
> - } else
> - h->protocol = protocol;
> + M_PREPEND(m, 2, M_DONTWAIT);
> + if (m == NULL) {
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_DEBUG, SPP_FMT
> + "no memory for transmit header\n",
> + SPP_ARGS(ifp));
> + ++ifp->if_oerrors;
> + splx(s);
> + return (ENOBUFS);
> + }
> + *mtod(m, u_int16_t *) = protocol;
>
> /*
> * Queue message on interface, and start output if interface
> @@ -888,7 +857,6 @@ sppp_ioctl(struct ifnet *ifp, u_long cmd
> }
> break;
>
> -#ifdef SIOCSIFMTU
> case SIOCSIFMTU:
> if (ifr->ifr_mtu < 128 ||
> (sp->lcp.their_mru > 0 &&
> @@ -898,33 +866,12 @@ sppp_ioctl(struct ifnet *ifp, u_long cmd
> }
> ifp->if_mtu = ifr->ifr_mtu;
> break;
> -#endif
> -#ifdef SLIOCSETMTU
> - case SLIOCSETMTU:
> - if (*(short*)data < 128 ||
> - (sp->lcp.their_mru > 0 &&
> - *(short*)data > sp->lcp.their_mru)) {
> - splx(s);
> - return (EINVAL);
> - }
> - ifp->if_mtu = *(short*)data;
> - break;
> -#endif
> -#ifdef SIOCGIFMTU
> case SIOCGIFMTU:
> ifr->ifr_mtu = ifp->if_mtu;
> break;
> -#endif
> -#ifdef SIOCGIFHARDMTU
> case SIOCGIFHARDMTU:
> ifr->ifr_hardmtu = ifp->if_hardmtu;
> break;
> -#endif
> -#ifdef SLIOCGETMTU
> - case SLIOCGETMTU:
> - *(short*)data = ifp->if_mtu;
> - break;
> -#endif
> case SIOCADDMULTI:
> case SIOCDELMULTI:
> break;
> @@ -956,31 +903,19 @@ sppp_cp_send(struct sppp *sp, u_short pr
> u_char ident, u_short len, void *data)
> {
> STDDCL;
> - struct ppp_header *h;
> struct lcp_header *lh;
> struct mbuf *m;
> - size_t pkthdrlen;
> -
> - pkthdrlen = (sp->pp_flags & PP_NOFRAMING) ? 2 : PPP_HEADER_LEN;
>
> - if (len > MHLEN - pkthdrlen - LCP_HEADER_LEN)
> - len = MHLEN - pkthdrlen - LCP_HEADER_LEN;
> + if (len > MHLEN - PKTHDRLEN - LCP_HEADER_LEN)
> + len = MHLEN - PKTHDRLEN - LCP_HEADER_LEN;
> MGETHDR (m, M_DONTWAIT, MT_DATA);
> if (! m)
> return;
> - m->m_pkthdr.len = m->m_len = pkthdrlen + LCP_HEADER_LEN + len;
> + m->m_pkthdr.len = m->m_len = PKTHDRLEN + LCP_HEADER_LEN + len;
> m->m_pkthdr.ph_ifidx = 0;
>
> - if (sp->pp_flags & PP_NOFRAMING) {
> - *mtod(m, u_int16_t *) = htons(proto);
> - lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> - } else {
> - h = mtod (m, struct ppp_header*);
> - h->address = PPP_ALLSTATIONS; /* broadcast address */
> - h->control = PPP_UI; /* Unnumbered Info */
> - h->protocol = htons (proto); /* Link Control Protocol */
> - lh = (struct lcp_header*) (h + 1);
> - }
> + *mtod(m, u_int16_t *) = htons(proto);
> + lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> lh->type = type;
> lh->ident = ident;
> lh->len = htons (LCP_HEADER_LEN + len);
> @@ -997,14 +932,15 @@ sppp_cp_send(struct sppp *sp, u_short pr
> sppp_print_bytes ((u_char*) (lh+1), len);
> addlog(">\n");
> }
> +
> + len = m->m_pkthdr.len + sp->pp_framebytes;
> if (mq_enqueue(&sp->pp_cpq, m) != 0) {
> - ++ifp->if_oerrors;
> - m = NULL;
> + ifp->if_oerrors++;
> + return;
> }
> - if (!(ifp->if_flags & IFF_OACTIVE))
> - (*ifp->if_start) (ifp);
> - if (m != NULL)
> - ifp->if_obytes += m->m_pkthdr.len + sp->pp_framebytes;
> +
> + ifp->if_obytes += len;
> + if_start(ifp);
> }
>
> /*
> @@ -4040,12 +3976,10 @@ sppp_auth_send(const struct cp *cp, stru
> unsigned int type, u_int id, ...)
> {
> STDDCL;
> - struct ppp_header *h;
> struct lcp_header *lh;
> struct mbuf *m;
> u_char *p;
> int len;
> - size_t pkthdrlen;
> unsigned int mlen;
> const char *msg;
> va_list ap;
> @@ -4055,18 +3989,8 @@ sppp_auth_send(const struct cp *cp, stru
> return;
> m->m_pkthdr.ph_ifidx = 0;
>
> - if (sp->pp_flags & PP_NOFRAMING) {
> - *mtod(m, u_int16_t *) = htons(cp->proto);
> - pkthdrlen = 2;
> - lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> - } else {
> - h = mtod (m, struct ppp_header*);
> - h->address = PPP_ALLSTATIONS; /* broadcast address */
> - h->control = PPP_UI; /* Unnumbered Info */
> - h->protocol = htons(cp->proto);
> - pkthdrlen = PPP_HEADER_LEN;
> - lh = (struct lcp_header*)(h + 1);
> - }
> + *mtod(m, u_int16_t *) = htons(cp->proto);
> + lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
>
> lh->type = type;
> lh->ident = id;
> @@ -4078,7 +4002,7 @@ sppp_auth_send(const struct cp *cp, stru
> while ((mlen = (unsigned int)va_arg(ap, size_t)) != 0) {
> msg = va_arg(ap, const char *);
> len += mlen;
> - if (len > MHLEN - pkthdrlen - LCP_HEADER_LEN) {
> + if (len > MHLEN - PKTHDRLEN - LCP_HEADER_LEN) {
> va_end(ap);
> m_freem(m);
> return;
> @@ -4089,7 +4013,7 @@ sppp_auth_send(const struct cp *cp, stru
> }
> va_end(ap);
>
> - m->m_pkthdr.len = m->m_len = pkthdrlen + LCP_HEADER_LEN + len;
> + m->m_pkthdr.len = m->m_len = PKTHDRLEN + LCP_HEADER_LEN + len;
> lh->len = htons (LCP_HEADER_LEN + len);
>
> if (debug) {
> @@ -4101,14 +4025,15 @@ sppp_auth_send(const struct cp *cp, stru
> sppp_print_bytes((u_char*) (lh+1), len);
> addlog(">\n");
> }
> +
> + len = m->m_pkthdr.len + sp->pp_framebytes;
> if (mq_enqueue(&sp->pp_cpq, m) != 0) {
> - ++ifp->if_oerrors;
> - m = NULL;
> + ifp->if_oerrors++;
> + return;
> }
> - if (! (ifp->if_flags & IFF_OACTIVE))
> - (*ifp->if_start) (ifp);
> - if (m != NULL)
> - ifp->if_obytes += m->m_pkthdr.len + sp->pp_framebytes;
> +
> + ifp->if_obytes += len;
> + if_start(ifp);
> }
>
> /*
>
