Download raw body.
Kill pipexintr()
On Tue, 28 Oct 2025 15:53:29 +0300
Vitaliy Makkoveev <mvs@openbsd.org> wrote:
> It was used because the netlock state is unpredictable while
> pipex_pppoe_output() was called. So the PPPoE output was deferred to
> netisr to make sure the locks are held. However, the protocol of our
> peer is always AF_UNSPEC, so ether_output() and following ether_resolve()
> are just "memcpy(eh, dst->sa_data, sizeof(*eh));" where `dst' is the
> immutable data which belongs to pipex session.
yes
> So I propose to manually fill ethernet header and enqueue packet.
> Someone could say this the copy-paste from ether_output(), but it is
> very small and this time I intentionally want to use it instead of
> non-obvious ifp->if_output().
I'd ok if you want.
ok yasuoka
On Tue, 28 Oct 2025 15:53:29 +0300
Vitaliy Makkoveev <mvs@openbsd.org> wrote:
> It was used because the netlock state is unpredictable while
> pipex_pppoe_output() was called. So the PPPoE output was deferred to
> netisr to make sure the locks are held. However, the protocol of our
> peer is always AF_UNSPEC, so ether_output() and following ether_resolve()
> are just "memcpy(eh, dst->sa_data, sizeof(*eh));" where `dst' is the
> immutable data which belongs to pipex session.
>
> So I propose to manually fill ethernet header and enqueue packet.
> Someone could say this the copy-paste from ether_output(), but it is
> very small and this time I intentionally want to use it instead of
> non-obvious ifp->if_output().
>
> ok?
>
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> diff -u -p -r1.741 if.c
> --- sys/net/if.c 9 Sep 2025 09:16:18 -0000 1.741
> +++ sys/net/if.c 28 Oct 2025 12:20:05 -0000
> @@ -1103,10 +1103,6 @@ if_netisr(void *unused)
> if (n & (1 << NETISR_BRIDGE))
> bridgeintr();
> #endif
> -#ifdef PIPEX
> - if (n & (1 << NETISR_PIPEX))
> - pipexintr();
> -#endif
> #if NPPPOE > 0
> if (n & (1 << NETISR_PPPOE)) {
> KERNEL_LOCK();
> Index: sys/net/netisr.h
> ===================================================================
> RCS file: /cvs/src/sys/net/netisr.h,v
> diff -u -p -r1.61 netisr.h
> --- sys/net/netisr.h 6 Jul 2023 04:55:05 -0000 1.61
> +++ sys/net/netisr.h 28 Oct 2025 12:20:05 -0000
> @@ -44,7 +44,6 @@
> #define NETISR_IP 2 /* same as AF_INET */
> #define NETISR_ARP 18 /* same as AF_LINK */
> #define NETISR_IPV6 24 /* same as AF_INET6 */
> -#define NETISR_PIPEX 27 /* for pipex processing */
> #define NETISR_PPP 28 /* for PPP processing */
> #define NETISR_BRIDGE 29 /* for bridge processing */
> #define NETISR_PPPOE 30 /* for pppoe processing */
> @@ -63,7 +62,6 @@ void ipintr(void);
> void ip6intr(void);
> void pppintr(void);
> void bridgeintr(void);
> -void pipexintr(void);
> void pppoeintr(void);
>
> #define schednetisr(anisr) \
> Index: sys/net/pipex.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex.c,v
> diff -u -p -r1.161 pipex.c
> --- sys/net/pipex.c 27 Aug 2025 22:39:04 -0000 1.161
> +++ sys/net/pipex.c 28 Oct 2025 12:20:05 -0000
> @@ -183,46 +183,6 @@ pipex_ioctl(void *ownersc, u_long cmd, c
> }
>
> /************************************************************************
> - * Software Interrupt Handler
> - ************************************************************************/
> -
> -void
> -pipexintr(void)
> -{
> - struct mbuf_list ml;
> - struct mbuf *m;
> - struct pipex_session *session;
> -
> - NET_ASSERT_LOCKED();
> -
> - mq_delist(&pipexoutq, &ml);
> -
> - while ((m = ml_dequeue(&ml)) != NULL) {
> - struct ifnet *ifp;
> -
> - session = m->m_pkthdr.ph_cookie;
> -
> - ifp = if_get(session->proto.pppoe.over_ifidx);
> - if (ifp != NULL) {
> - struct pipex_pppoe_header *pppoe;
> - int len;
> -
> - pppoe = mtod(m, struct pipex_pppoe_header *);
> - len = ntohs(pppoe->length);
> - ifp->if_output(ifp, m, &session->peer.sa, NULL);
> - counters_pkt(session->stat_counters, pxc_opackets,
> - pxc_obytes, len);
> - } else {
> - m_freem(m);
> - counters_inc(session->stat_counters, pxc_oerrors);
> - }
> - if_put(ifp);
> -
> - pipex_rele_session(session);
> - }
> -}
> -
> -/************************************************************************
> * Session management functions
> ************************************************************************/
> int
> @@ -1335,6 +1295,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
> void
> pipex_pppoe_output(struct mbuf *m0, struct pipex_session *session)
> {
> + struct ifnet *ifp;
> struct pipex_pppoe_header *pppoe;
> int len, padlen;
>
> @@ -1345,7 +1306,7 @@ pipex_pppoe_output(struct mbuf *m0, stru
> M_PREPEND(m0, sizeof(struct pipex_pppoe_header), M_NOWAIT);
> if (m0 == NULL) {
> PIPEX_DBG((NULL, LOG_ERR,
> - "<%s> cannot prepend header.", __func__));
> + "<%s> cannot prepend pppoe header.", __func__));
> counters_inc(session->stat_counters, pxc_oerrors);
> return;
> }
> @@ -1362,15 +1323,37 @@ pipex_pppoe_output(struct mbuf *m0, stru
> pppoe->length = htons(len);
>
> m0->m_pkthdr.ph_ifidx = session->proto.pppoe.over_ifidx;
> - refcnt_take(&session->pxs_refcnt);
> - m0->m_pkthdr.ph_cookie = session;
> m0->m_flags &= ~(M_BCAST|M_MCAST);
>
> - if (mq_enqueue(&pipexoutq, m0) != 0) {
> + M_PREPEND(m0, ETHER_ALIGN + sizeof(struct ether_header), M_NOWAIT);
> + if (m0 == NULL) {
> + PIPEX_DBG((NULL, LOG_ERR,
> + "<%s> cannot prepend ethernet header.", __func__));
> counters_inc(session->stat_counters, pxc_oerrors);
> - pipex_rele_session(session);
> - } else
> - schednetisr(NETISR_PIPEX);
> + return;
> + }
> +
> + ifp = if_get(session->proto.pppoe.over_ifidx);
> + if (ifp != NULL) {
> + struct arpcom *ac = (struct arpcom *)ifp;
> + struct ether_header *eh;
> +
> + /* setup ethernet header information */
> + m_adj(m0, ETHER_ALIGN);
> + eh = mtod(m0, struct ether_header *);
> + memcpy(eh, session->peer.sa.sa_data, sizeof(*eh));
> + memcpy(eh->ether_shost, ac->ac_enaddr, sizeof(eh->ether_shost));
> +
> + if (if_enqueue(ifp, m0) == 0)
> + counters_pkt(session->stat_counters, pxc_opackets,
> + pxc_obytes, len);
> + else
> + counters_inc(session->stat_counters, pxc_oerrors);
> + } else {
> + m_freem(m0);
> + counters_inc(session->stat_counters, pxc_oerrors);
> + }
> + if_put(ifp);
> }
> #endif /* PIPEX_PPPOE */
>
>
Kill pipexintr()