On Tue, Jun 07, 2016 at 22:02 +0100, Stuart Henderson wrote:
> On 2016/06/07 21:49, Vincent Gross wrote:
> >
> > It's how henning@ set things up when integrating the new queuing mechanism.
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160
> >
> > > Is there any use for this apart for vlan(4) interfaces?
> >
> > AFAICT, no.
>
> My understanding is that it is also meant to be used for interface
> output queues. So you could use this to prioritize ARP over IP
> traffic if you wanted. (Well..you could anyway, but you don't
> have as many options - many switches map the priorities 0-7
> onto 4 queues so there is only lower-priority than the default
> 'prio=3').
>
> > > Should it
> > > really be part of "struct ifnet" ?
> > >
> >
> > sthen@ pointed out that struct if_data was heavily used by our ports, and
> > that such a change would require a version bump. Now, I may have overlooked
> > a better place for it.
>
> I think it could go at the end of struct if_data without causing
> trouble, though since it doesn't need to be exported to userland
> isn't it better directly in ifnet rather than if_data? (if_data
> is included in RTM_IFINFO's if_msghdr, whereas you need an ioctl
> to see the non-if_data parts of struct ifnet from userland).
>
> > I don't think we should make a special case for vlan(4), this kind of detail
> > do not belong to the arp(4) or bpf(4) layer.
>
> btw, since this is a perfect fit for the vlan priority for pppoe
> control packets that I was looking at recently, here's a diff
> to use it there.
>
I think so too. OK mikeb after vgross' diff goes in.
> Index: if_sppp.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_sppp.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 if_sppp.h
> --- if_sppp.h 30 May 2016 23:30:11 -0000 1.24
> +++ if_sppp.h 7 Jun 2016 20:53:28 -0000
> @@ -56,7 +56,6 @@ enum ppp_phase {
>
> #define AUTHMAXLEN 256 /* including terminating '\0' */
> #define AUTHCHALEN 16 /* length of the challenge we send */
> -#define SPPP_CTL_PRIO 7 /* priority to use for control packets
> */
>
> /*
> * Definitions to pass struct sppp data down into the kernel using the
> Index: if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 if_spppsubr.c
> --- if_spppsubr.c 30 May 2016 23:30:10 -0000 1.153
> +++ if_spppsubr.c 7 Jun 2016 20:53:28 -0000
> @@ -914,7 +914,7 @@ sppp_cp_send(struct sppp *sp, u_short pr
> return;
> m->m_pkthdr.len = m->m_len = PKTHDRLEN + LCP_HEADER_LEN + len;
> m->m_pkthdr.ph_ifidx = 0;
> - m->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m->m_pkthdr.pf.prio = sp->pp_if.if_llprio;
>
> *mtod(m, u_int16_t *) = htons(proto);
> lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> @@ -3992,7 +3992,7 @@ sppp_auth_send(const struct cp *cp, stru
> if (! m)
> return;
> m->m_pkthdr.ph_ifidx = 0;
> - m->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m->m_pkthdr.pf.prio = sp->pp_if.if_llprio;
>
> *mtod(m, u_int16_t *) = htons(cp->proto);
> lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
>
> Index: if_pppoe.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 if_pppoe.c
> --- if_pppoe.c 30 May 2016 23:30:11 -0000 1.56
> +++ if_pppoe.c 7 Jun 2016 20:53:28 -0000
> @@ -163,7 +163,7 @@ static void pppoe_timeout(void *);
> /* sending actual protocol control packets */
> static int pppoe_send_padi(struct pppoe_softc *);
> static int pppoe_send_padr(struct pppoe_softc *);
> -static int pppoe_send_padt(unsigned int, u_int, const u_int8_t *);
> +static int pppoe_send_padt(unsigned int, u_int, const u_int8_t *, u_int8_t);
>
> /* raw output */
> static int pppoe_output(struct pppoe_softc *, struct mbuf *);
> @@ -696,7 +696,7 @@ pppoe_data_input(struct mbuf *m)
> #ifdef PPPOE_TERM_UNKNOWN_SESSIONS
> printf("pppoe (data): input for unknown session 0x%x, sending
> PADT\n",
> session);
> - pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost);
> + pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost, 0);
> #endif
> goto drop;
> }
> @@ -1011,7 +1011,7 @@ pppoe_send_padi(struct pppoe_softc *sc)
> m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN); /* header len + payload
> len */
> if (m0 == NULL)
> return (ENOBUFS);
> - m0->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m0->m_pkthdr.pf.prio = sc->sc_sppp.pp_if.if_llprio;
>
> /* fill in pkt */
> p = mtod(m0, u_int8_t *);
> @@ -1170,7 +1170,8 @@ pppoe_disconnect(struct pppoe_softc *sc)
> PPPOEDEBUG(("%s: disconnecting\n",
> sc->sc_sppp.pp_if.if_xname));
> err = pppoe_send_padt(sc->sc_eth_ifidx,
> - sc->sc_session, (const u_int8_t *)&sc->sc_dest);
> + sc->sc_session, (const u_int8_t *)&sc->sc_dest,
> + sc->sc_sppp.pp_if.if_llprio);
> }
>
> /* cleanup softc */
> @@ -1238,7 +1239,7 @@ pppoe_send_padr(struct pppoe_softc *sc)
> m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN);
> if (m0 == NULL)
> return (ENOBUFS);
> - m0->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m0->m_pkthdr.pf.prio = sc->sc_sppp.pp_if.if_llprio;
>
> p = mtod(m0, u_int8_t *);
> PPPOE_ADD_HEADER(p, PPPOE_CODE_PADR, 0, len);
> @@ -1285,7 +1286,7 @@ pppoe_send_padr(struct pppoe_softc *sc)
>
> /* Send a PADT packet. */
> static int
> -pppoe_send_padt(unsigned int ifidx, u_int session, const u_int8_t *dest)
> +pppoe_send_padt(unsigned int ifidx, u_int session, const u_int8_t *dest,
> u_int8_t prio)
> {
> struct ether_header *eh;
> struct sockaddr dst;
> @@ -1302,7 +1303,7 @@ pppoe_send_padt(unsigned int ifidx, u_in
> if_put(eth_if);
> return (ENOBUFS);
> }
> - m0->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m0->m_pkthdr.pf.prio = prio;
>
> p = mtod(m0, u_int8_t *);
> PPPOE_ADD_HEADER(p, PPPOE_CODE_PADT, session, 0);
>
