Thank you very much.
I'll look at applying the diff but it's been a while since I've done it.
Brodey
On Sun, 1 Dec 2024 at 17:48, David Gwynne <da...@gwynne.id.au> wrote:
> On Wed, Nov 27, 2024 at 09:14:19AM -0500, Brodey Dover wrote:
> > Thanks. The MTU is auto negotiated to 1492. max-mss is 1440 in pf.
> >
> > I don't think OpenBSD has netisr or an equivalent since I don't see
> > anything in the sysctl list, but it was implemented in FreeBSD and has
> > allowed a number of people to fully saturate the mutli-gig symmetrical
> > connections offered by newer ISPs using PPPoE. I should add *ON
> > mutli-core/slower speed CPUs".
>
> the diff below might improve pppoe rx performance.
>
> > On Wed, 27 Nov 2024 at 03:03, Christer Solskogen <
> > christer.solsko...@gmail.com> wrote:
> >
> > > On Tue, Nov 26, 2024 at 10:59???PM Brodey Dover <dover...@gmail.com>
> wrote:
> > > >
> > > > So my modem is too buggy to do any DMZ work, thank you ISP.
> > > >
> > > > But the modem does pull 2375/2375. That???s down/up, which is why I
> was
> > > thinking there was a serious bottleneck on the OBSD side.
> > > >
> > >
> > > It's at least 20 years since I used PPPoE, but I seem to remember that
> > > I had to lower the MTU to get the full speed. 1492 if I remember
> > > correctly.
> > >
> > > --
> > > chs
>
> Index: if_ethersubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> diff -u -p -r1.293 if_ethersubr.c
> --- if_ethersubr.c 14 Feb 2024 22:41:48 -0000 1.293
> +++ if_ethersubr.c 30 Nov 2024 06:29:47 -0000
> @@ -561,7 +561,8 @@ ether_input(struct ifnet *ifp, struct mb
> if (mq_enqueue(&pppoediscinq, m) == 0)
> schednetisr(NETISR_PPPOE);
> } else {
> - if (mq_enqueue(&pppoeinq, m) == 0)
> + m = pppoe_vinput(ifp, m);
> + if (m != NULL && mq_enqueue(&pppoeinq, m) == 0)
> schednetisr(NETISR_PPPOE);
> }
> return;
> Index: if_pppoe.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> diff -u -p -r1.84 if_pppoe.c
> --- if_pppoe.c 26 Jun 2024 01:40:49 -0000 1.84
> +++ if_pppoe.c 30 Nov 2024 06:29:47 -0000
> @@ -42,6 +42,8 @@
> #include <sys/socket.h>
> #include <sys/syslog.h>
> #include <sys/ioctl.h>
> +#include <sys/smr.h>
> +#include <sys/percpu.h>
> #include <net/if.h>
> #include <net/if_var.h>
> #include <net/if_types.h>
> @@ -124,7 +126,9 @@ struct pppoe_softc {
> struct sppp sc_sppp; /* contains a struct ifnet as
> first element */
> LIST_ENTRY(pppoe_softc) sc_list;/* [K] */
> unsigned int sc_eth_ifidx; /* [K] */
> + caddr_t sc_bpf;
>
> + SMR_LIST_ENTRY(pppoe_softc) sc_session_entry; /* [K] */
> int sc_state; /* [K] discovery phase or session
> connected */
> struct ether_addr sc_dest; /* [K] hardware address of
> concentrator */
> u_int16_t sc_session; /* [K] PPPoE session id */
> @@ -175,6 +179,7 @@ static struct pppoe_softc *pppoe_find_so
> static struct mbuf *pppoe_get_mbuf(size_t len);
>
> LIST_HEAD(pppoe_softc_head, pppoe_softc) pppoe_softc_list;
> +SMR_LIST_HEAD(pppoe_softc_sessions, pppoe_softc) pppoe_sessions; /* [K] */
>
> /* interface cloning */
> int pppoe_clone_create(struct if_clone *, int);
> @@ -209,9 +214,19 @@ void
> pppoeattach(int count)
> {
> LIST_INIT(&pppoe_softc_list);
> + SMR_LIST_INIT(&pppoe_sessions);
> if_clone_attach(&pppoe_cloner);
> }
>
> +static void
> +pppoe_set_state(struct pppoe_softc *sc, int state)
> +{
> + KERNEL_ASSERT_LOCKED();
> + if (sc->sc_state == PPPOE_STATE_SESSION)
> + SMR_LIST_REMOVE_LOCKED(sc, sc_session_entry);
> + sc->sc_state = state;
> +}
> +
> /* Create a new interface. */
> int
> pppoe_clone_create(struct if_clone *ifc, int unit)
> @@ -230,6 +245,8 @@ pppoe_clone_create(struct if_clone *ifc,
> sc->sc_sppp.pp_if.if_hdrlen = sizeof(struct ether_header) +
> PPPOE_HEADERLEN;
> 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_input = p2p_input;
> + sc->sc_sppp.pp_if.if_bpf_mtap = p2p_bpf_mtap;
> sc->sc_sppp.pp_if.if_ioctl = pppoe_ioctl;
> sc->sc_sppp.pp_if.if_start = pppoe_start;
> sc->sc_sppp.pp_if.if_rtrequest = p2p_rtrequest;
> @@ -243,11 +260,14 @@ pppoe_clone_create(struct if_clone *ifc,
> /* init timer for interface watchdog */
> timeout_set_proc(&sc->sc_timeout, pppoe_timeout, sc);
>
> + if_counters_alloc(&sc->sc_sppp.pp_if);
> if_attach(&sc->sc_sppp.pp_if);
> if_alloc_sadl(&sc->sc_sppp.pp_if);
> sppp_attach(&sc->sc_sppp.pp_if);
> #if NBPFILTER > 0
> - bpfattach(&sc->sc_sppp.pp_if.if_bpf, &sc->sc_sppp.pp_if,
> DLT_PPP_ETHER, 0);
> + bpfattach(&sc->sc_bpf, &sc->sc_sppp.pp_if, DLT_PPP_ETHER, 0);
> + bpfattach(&sc->sc_sppp.pp_if.if_bpf, &sc->sc_sppp.pp_if,
> + DLT_LOOP, sizeof(uint32_t));
> #endif
>
> NET_LOCK();
> @@ -274,6 +294,7 @@ pppoe_clone_destroy(struct ifnet *ifp)
> NET_UNLOCK();
>
> timeout_del(&sc->sc_timeout);
> + pppoe_set_state(sc, PPPOE_STATE_INITIAL);
>
> sppp_detach(&sc->sc_sppp.pp_if);
> if_detach(ifp);
> @@ -289,6 +310,8 @@ pppoe_clone_destroy(struct ifnet *ifp)
> if (sc->sc_relay_sid)
> free(sc->sc_relay_sid, M_DEVBUF, sc->sc_relay_sid_len);
>
> + smr_barrier();
> +
> free(sc, M_DEVBUF, sizeof(*sc));
>
> return (0);
> @@ -318,6 +341,28 @@ pppoe_find_softc_by_session(u_int sessio
> return (NULL);
> }
>
> +static struct pppoe_softc *
> +pppoe_smr_find_by_session(u_int session, u_int ifidx)
> +{
> + struct pppoe_softc *sc;
> +
> + if (session == 0)
> + return (NULL);
> +
> + smr_read_enter();
> + SMR_LIST_FOREACH(sc, &pppoe_sessions, sc_session_entry) {
> + if (sc->sc_session == session &&
> + sc->sc_eth_ifidx == ifidx) {
> + /* XXX if_ref() */
> + refcnt_take(&sc->sc_sppp.pp_if.if_refcnt);
> + break;
> + }
> + }
> + smr_read_leave();
> +
> + return (sc);
> +}
> +
> /*
> * Check host unique token passed and return appropriate softc pointer,
> * or NULL if token is bogus.
> @@ -603,7 +648,7 @@ breakbreak:
>
> memcpy(&sc->sc_dest, eh->ether_shost, sizeof(sc->sc_dest));
> sc->sc_padr_retried = 0;
> - sc->sc_state = PPPOE_STATE_PADR_SENT;
> + pppoe_set_state(sc, PPPOE_STATE_PADR_SENT);
> if ((err = pppoe_send_padr(sc)) != 0) {
> PPPOEDEBUG(("%s: failed to send PADR, error=%d\n",
> sc->sc_sppp.pp_if.if_xname, err));
> @@ -616,12 +661,16 @@ breakbreak:
> if (sc == NULL)
> goto done;
>
> + KERNEL_ASSERT_LOCKED();
> +
> sc->sc_session = session;
> timeout_del(&sc->sc_timeout);
> PPPOEDEBUG(("%s: session 0x%x connected\n",
> sc->sc_sppp.pp_if.if_xname, session));
> sc->sc_state = PPPOE_STATE_SESSION;
> getmicrouptime(&sc->sc_session_time);
> + SMR_LIST_INSERT_HEAD_LOCKED(&pppoe_sessions, sc,
> + sc_session_entry);
> sc->sc_sppp.pp_up(&sc->sc_sppp); /* notify upper
> layers */
>
> break;
> @@ -635,7 +684,7 @@ breakbreak:
> sc->sc_sppp.pp_if.if_xname, session));
>
> /* clean up softc */
> - sc->sc_state = PPPOE_STATE_INITIAL;
> + pppoe_set_state(sc, PPPOE_STATE_PADR_SENT);
> memcpy(&sc->sc_dest, etherbroadcastaddr,
> sizeof(sc->sc_dest));
> if (sc->sc_ac_cookie) {
> free(sc->sc_ac_cookie, M_DEVBUF,
> @@ -677,6 +726,108 @@ pppoe_disc_input(struct mbuf *m)
> m_freem(m);
> }
>
> +struct mbuf *
> +pppoe_vinput(struct ifnet *ifp0, struct mbuf *m)
> +{
> + struct pppoe_softc *sc;
> + struct ifnet *ifp;
> + struct ether_header *eh;
> + struct pppoehdr *ph;
> + uint16_t proto;
> + int hlen = sizeof(*eh) + sizeof(*ph);
> + int phlen;
> + int plen;
> + int af = AF_UNSPEC;
> +#if NBPFILTER > 0
> + caddr_t if_bpf;
> +#endif
> + time_t now;
> +
> + smr_read_enter();
> + sc = SMR_LIST_FIRST(&pppoe_sessions);
> + smr_read_leave();
> + if (sc == NULL)
> + return (m);
> +
> + if (m->m_pkthdr.len < hlen)
> + return (m);
> + if (m->m_len < hlen) {
> + m = m_pullup(m, hlen);
> + if (m == NULL)
> + return (NULL);
> + }
> +
> + eh = mtod(m, struct ether_header *);
> + ph = (struct pppoehdr *)(eh + 1);
> + if (ph->vertype != PPPOE_VERTYPE)
> + return (m);
> + if (ph->code != 0)
> + return (m);
> +
> + sc = pppoe_smr_find_by_session(ntohs(ph->session), ifp0->if_index);
> + if (sc == NULL) {
> + /* no session, don't waste any more time */
> + m_freem(m);
> + return (NULL);
> + }
> +
> + ifp = &sc->sc_sppp.pp_if;
> +
> + plen = ntohs(ph->plen);
> + if (plen < sizeof(proto))
> + goto drop;
> +
> + phlen = hlen + sizeof(proto);
> + if (m->m_pkthdr.len < phlen)
> + goto drop;
> + if (m->m_len < phlen) {
> + m = m_pullup(m, phlen);
> + if (m == NULL)
> + goto put;
> + }
> +
> + proto = *(uint16_t *)(mtod(m, caddr_t) + hlen);
> + af = sppp_proto_up(ifp, proto);
> + if (af == AF_UNSPEC)
> + goto put;
> +
> +#if NBPFILTER > 0
> + if_bpf = sc->sc_bpf;
> + if (if_bpf) {
> + m_adj(m, sizeof(*eh));
> + bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_IN);
> + m_adj(m, phlen - sizeof(*eh));
> + } else
> +#endif
> + m_adj(m, phlen);
> +
> + plen -= sizeof(proto);
> + if (m->m_pkthdr.len < plen) {
> + counters_inc(ifp->if_counters, ifc_ierrors);
> + goto drop;
> + }
> +
> + if (m->m_pkthdr.len > plen)
> + m_adj(m, plen - m->m_pkthdr.len);
> +
> + /* XXX not 64bit or MP safe */
> + now = getuptime();
> + if (sc->sc_sppp.pp_last_activity < now)
> + sc->sc_sppp.pp_last_activity = now;
> +
> + m->m_pkthdr.ph_family = af;
> + if_vinput(ifp, m);
> +done:
> + m = NULL;
> +put:
> + if_put(ifp);
> +
> + return (m);
> +drop:
> + m_freem(m);
> + goto done;
> +}
> +
> /* Input function for data packets */
> void
> pppoe_data_input(struct mbuf *m)
> @@ -731,8 +882,8 @@ pppoe_data_input(struct mbuf *m)
> plen = ntohs(ph->plen);
>
> #if NBPFILTER > 0
> - if(sc->sc_sppp.pp_if.if_bpf)
> - bpf_mtap(sc->sc_sppp.pp_if.if_bpf, m, BPF_DIRECTION_IN);
> + if (sc->sc_bpf)
> + bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_IN);
> #endif
>
> m_adj(m, PPPOE_HEADERLEN);
> @@ -927,7 +1078,7 @@ pppoe_ioctl(struct ifnet *ifp, unsigned
> && sc->sc_state >= PPPOE_STATE_PADI_SENT
> && sc->sc_state < PPPOE_STATE_SESSION) {
> timeout_del(&sc->sc_timeout);
> - sc->sc_state = PPPOE_STATE_INITIAL;
> + pppoe_set_state(sc, PPPOE_STATE_INITIAL);
> sc->sc_padi_retried = 0;
> sc->sc_padr_retried = 0;
> memcpy(&sc->sc_dest, etherbroadcastaddr,
> @@ -965,7 +1116,8 @@ pppoe_ioctl(struct ifnet *ifp, unsigned
> if (sc->sc_state >= PPPOE_STATE_PADI_SENT
> &&
> sc->sc_state < PPPOE_STATE_SESSION) {
> timeout_del(&sc->sc_timeout);
> - sc->sc_state = PPPOE_STATE_INITIAL;
> + pppoe_set_state(sc,
> + PPPOE_STATE_INITIAL);
> sc->sc_padi_retried = 0;
> sc->sc_padr_retried = 0;
> memcpy(&sc->sc_dest,
> @@ -1136,7 +1288,7 @@ pppoe_timeout(void *arg)
> if (sc->sc_padr_retried >= PPPOE_DISC_MAXPADR) {
> memcpy(&sc->sc_dest, etherbroadcastaddr,
> sizeof(sc->sc_dest));
> - sc->sc_state = PPPOE_STATE_PADI_SENT;
> + pppoe_set_state(sc, PPPOE_STATE_PADI_SENT);
> sc->sc_padr_retried = 0;
> if ((err = pppoe_send_padi(sc)) != 0) {
> PPPOEDEBUG(("%s: failed to send PADI,
> error=%d\n",
> @@ -1179,7 +1331,7 @@ pppoe_connect(struct pppoe_softc *sc)
> x = splnet();
>
> /* save state, in case we fail to send PADI */
> - sc->sc_state = PPPOE_STATE_PADI_SENT;
> + pppoe_set_state(sc, PPPOE_STATE_PADI_SENT);
> sc->sc_padr_retried = 0;
> err = pppoe_send_padi(sc);
> if (err != 0)
> @@ -1211,7 +1363,7 @@ pppoe_disconnect(struct pppoe_softc *sc)
> }
>
> /* cleanup softc */
> - sc->sc_state = PPPOE_STATE_INITIAL;
> + pppoe_set_state(sc, PPPOE_STATE_INITIAL);
> memcpy(&sc->sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
> if (sc->sc_ac_cookie) {
> free(sc->sc_ac_cookie, M_DEVBUF, sc->sc_ac_cookie_len);
> @@ -1239,14 +1391,14 @@ pppoe_abort_connect(struct pppoe_softc *
> {
> printf("%s: could not establish connection\n",
> sc->sc_sppp.pp_if.if_xname);
> - sc->sc_state = PPPOE_STATE_CLOSING;
> + pppoe_set_state(sc, PPPOE_STATE_CLOSING);
>
> /* notify upper layer */
> sc->sc_sppp.pp_down(&sc->sc_sppp);
>
> /* clear connection state */
> memcpy(&sc->sc_dest, etherbroadcastaddr, sizeof(sc->sc_dest));
> - sc->sc_state = PPPOE_STATE_INITIAL;
> + pppoe_set_state(sc, PPPOE_STATE_INITIAL);
> }
>
> /* Send a PADR packet */
> @@ -1385,7 +1537,7 @@ pppoe_tlf(struct sppp *sp)
> * machine gets confused by this. We must return from this
> * function and defer disconnecting to the timeout handler.
> */
> - sc->sc_state = PPPOE_STATE_CLOSING;
> + pppoe_set_state(sc, PPPOE_STATE_CLOSING);
> timeout_add_msec(&sc->sc_timeout, 20);
> }
>
> @@ -1417,9 +1569,8 @@ pppoe_start(struct ifnet *ifp)
> PPPOE_ADD_HEADER(p, 0, sc->sc_session, len);
>
> #if NBPFILTER > 0
> - if(sc->sc_sppp.pp_if.if_bpf)
> - bpf_mtap(sc->sc_sppp.pp_if.if_bpf, m,
> - BPF_DIRECTION_OUT);
> + if (sc->sc_bpf)
> + bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_OUT);
> #endif
>
> pppoe_output(sc, m);
> Index: if_pppoe.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppoe.h,v
> diff -u -p -r1.8 if_pppoe.h
> --- if_pppoe.h 29 Jun 2022 09:08:07 -0000 1.8
> +++ if_pppoe.h 30 Nov 2024 06:29:47 -0000
> @@ -69,5 +69,8 @@ struct pppoeconnectionstate {
> extern struct mbuf_queue pppoediscinq;
> extern struct mbuf_queue pppoeinq;
>
> +int pppoe_if_exists(void);
> +struct mbuf *pppoe_vinput(struct ifnet *, struct mbuf *);
> +
> #endif /* _KERNEL */
> #endif /* _NET_IF_PPPOE_H_ */
> Index: if_sppp.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if_sppp.h,v
> diff -u -p -r1.30 if_sppp.h
> --- if_sppp.h 17 Nov 2021 18:00:24 -0000 1.30
> +++ if_sppp.h 30 Nov 2024 06:29:47 -0000
> @@ -232,6 +232,7 @@ struct sppp {
> void sppp_attach (struct ifnet *ifp);
> void sppp_detach (struct ifnet *ifp);
> void sppp_input (struct ifnet *ifp, struct mbuf *m);
> +int sppp_proto_up(struct ifnet *ifp, uint16_t);
>
> /* Workaround */
> void spppattach (struct ifnet *ifp);
> Index: if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> diff -u -p -r1.194 if_spppsubr.c
> --- if_spppsubr.c 22 Jun 2024 10:22:29 -0000 1.194
> +++ if_spppsubr.c 30 Nov 2024 06:29:47 -0000
> @@ -415,6 +415,30 @@ spppattach(struct ifnet *ifp)
> {
> }
>
> +int
> +sppp_proto_up(struct ifnet *ifp, uint16_t proto)
> +{
> + struct sppp *sp = (struct sppp *)ifp;
> + int af = AF_UNSPEC;
> +
> + switch (ntohs(proto)) {
> + case PPP_IP:
> + if (sp->state[IDX_IPCP] == STATE_OPENED)
> + af = AF_INET;
> + break;
> +#ifdef INET6
> + case PPP_IPV6:
> + if (sp->state[IDX_IPV6CP] == STATE_OPENED)
> + af = AF_INET6;
> + break;
> +#endif
> + default:
> + break;
> + }
> +
> + return (af);
> +}
> +
> /*
> * Process the received packet.
> */
>
