ping?
> On 1 Jul 2020, at 22:42, Vitaliy Makkoveev <[email protected]> wrote:
>
> pipex(4) has 2 mbuf queues: `pipexinq' and `pipexoutq'. When mbuf passed
> to pipex it goes to one of these queues and pipexintr() will be
> scheduled to process them. pipexintr() called from `netisr' context.
>
> It's true for pppac(4) but for pppx(4) only incoming mbufs go to
> `pipexinq. Outgoing mbufs go directly to stack. pppx(4) enabled in
> npppd.conf(5) by default so I guess it's the common case of pipex(4)
> usage.
>
> The code looks like there is no requirements to this delayed mbufs
> processing, we can pass it directly to stack as we do for pppx(4)
> outgoing traffic.
>
> Also we have some troubles with pipexintr() as it was described in [1].
> It's protection of `ph_cookie'. We don't this protection this time and
> we can't because we should brake if_get(9) logic.
>
> Diff below removes pipexintr(). Now all mbufs passed directly without
> enqueueing within pipex(4). We also can destroy sessions safe in all
> cases. We also can use if_get(9) instead using unreferenced pointers to
> `ifnet' within pipex(4). We also avoided context switch while we
> processing mbufs within pipex(4). We decreased latency.
>
> I'm seeding debian torrents with this diff an all goes well.
>
> 1. https://marc.info/?t=159300809000002&r=1&w=2
>
> Index: lib/libc/sys/sysctl.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/sysctl.2,v
> retrieving revision 1.40
> diff -u -p -r1.40 sysctl.2
> --- lib/libc/sys/sysctl.2 17 May 2020 05:48:39 -0000 1.40
> +++ lib/libc/sys/sysctl.2 1 Jul 2020 19:20:22 -0000
> @@ -2033,35 +2033,11 @@ The currently defined variable names are
> .Bl -column "Third level name" "integer" "Changeable" -offset indent
> .It Sy "Third level name" Ta Sy "Type" Ta Sy "Changeable"
> .It Dv PIPEXCTL_ENABLE Ta integer Ta yes
> -.It Dv PIPEXCTL_INQ Ta node Ta not applicable
> -.It Dv PIPEXCTL_OUTQ Ta node Ta not applicable
> .El
> .Bl -tag -width "123456"
> .It Dv PIPEXCTL_ENABLE
> If set to 1, enable PIPEX processing.
> The default is 0.
> -.It Dv PIPEXCTL_INQ Pq Va net.pipex.inq
> -Fourth level comprises an array of
> -.Vt struct ifqueue
> -structures containing information about the PIPEX packet input queue.
> -The forth level names for the elements of
> -.Vt struct ifqueue
> -are the same as described in
> -.Li ip.arpq
> -in the
> -.Dv PF_INET
> -section.
> -.It Dv PIPEXCTL_OUTQ Pq Va net.pipex.outq
> -Fourth level comprises an array of
> -.Vt struct ifqueue
> -structures containing information about PIPEX packet output queue.
> -The forth level names for the elements of
> -.Vt struct ifqueue
> -are the same as described in
> -.Li ip.arpq
> -in the
> -.Dv PF_INET
> -section.
> .El
> .El
> .Ss CTL_VFS
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.611
> diff -u -p -r1.611 if.c
> --- sys/net/if.c 30 Jun 2020 09:31:38 -0000 1.611
> +++ sys/net/if.c 1 Jul 2020 19:20:27 -0000
> @@ -1012,13 +1012,6 @@ if_netisr(void *unused)
> KERNEL_UNLOCK();
> }
> #endif
> -#ifdef PIPEX
> - if (n & (1 << NETISR_PIPEX)) {
> - KERNEL_LOCK();
> - pipexintr();
> - KERNEL_UNLOCK();
> - }
> -#endif
> t |= n;
> }
>
> Index: sys/net/netisr.h
> ===================================================================
> RCS file: /cvs/src/sys/net/netisr.h,v
> retrieving revision 1.51
> diff -u -p -r1.51 netisr.h
> --- sys/net/netisr.h 6 Aug 2019 22:57:54 -0000 1.51
> +++ sys/net/netisr.h 1 Jul 2020 19:20:27 -0000
> @@ -48,7 +48,6 @@
> #define NETISR_IPV6 24 /* same as AF_INET6 */
> #define NETISR_ISDN 26 /* same as AF_E164 */
> #define NETISR_PPP 28 /* for PPP processing */
> -#define NETISR_PIPEX 27 /* for pipex processing */
> #define NETISR_BRIDGE 29 /* for bridge processing */
> #define NETISR_PPPOE 30 /* for pppoe processing */
> #define NETISR_SWITCH 31 /* for switch dataplane */
> @@ -68,7 +67,6 @@ void bridgeintr(void);
> void pppoeintr(void);
> void switchintr(void);
> void pfsyncintr(void);
> -void pipexintr(void);
>
> #define schednetisr(anisr)
> \
> do { \
> Index: sys/net/pipex.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.117
> diff -u -p -r1.117 pipex.c
> --- sys/net/pipex.c 30 Jun 2020 14:05:13 -0000 1.117
> +++ sys/net/pipex.c 1 Jul 2020 19:20:28 -0000
> @@ -97,10 +97,6 @@ struct radix_node_head *pipex_rd_head6 =
> struct timeout pipex_timer_ch; /* callout timer context */
> int pipex_prune = 1; /* walk list every seconds */
>
> -/* pipex traffic queue */
> -struct mbuf_queue pipexinq = MBUF_QUEUE_INITIALIZER(IFQ_MAXLEN, IPL_NET);
> -struct mbuf_queue pipexoutq = MBUF_QUEUE_INITIALIZER(IFQ_MAXLEN, IPL_NET);
> -
> /* borrow an mbuf pkthdr field */
> #define ph_ppp_proto ether_vtag
>
> @@ -713,82 +709,6 @@ pipex_lookup_by_session_id(int protocol,
> }
>
> /***********************************************************************
> - * Queue and Software Interrupt Handler
> - ***********************************************************************/
> -void
> -pipexintr(void)
> -{
> - struct pipex_session *pkt_session;
> - u_int16_t proto;
> - struct mbuf *m;
> - struct mbuf_list ml;
> -
> - /* ppp output */
> - mq_delist(&pipexoutq, &ml);
> - while ((m = ml_dequeue(&ml)) != NULL) {
> - pkt_session = m->m_pkthdr.ph_cookie;
> - if (pkt_session == NULL) {
> - m_freem(m);
> - continue;
> - }
> - proto = m->m_pkthdr.ph_ppp_proto;
> -
> - m->m_pkthdr.ph_cookie = NULL;
> - m->m_pkthdr.ph_ppp_proto = 0;
> -
> - if (pkt_session->is_multicast != 0) {
> - struct pipex_session *session;
> - struct mbuf *m0;
> -
> - LIST_FOREACH(session, &pipex_session_list,
> - session_list) {
> - if (session->pipex_iface !=
> - pkt_session->pipex_iface)
> - continue;
> - if (session->ip_forward == 0 &&
> - session->ip6_forward == 0)
> - continue;
> - m0 = m_copym(m, 0, M_COPYALL, M_NOWAIT);
> - if (m0 == NULL) {
> - session->stat.oerrors++;
> - continue;
> - }
> - pipex_ppp_output(m0, session, proto);
> - }
> - m_freem(m);
> - } else
> - pipex_ppp_output(m, pkt_session, proto);
> - }
> -
> - /* ppp input */
> - mq_delist(&pipexinq, &ml);
> - while ((m = ml_dequeue(&ml)) != NULL) {
> - pkt_session = m->m_pkthdr.ph_cookie;
> - if (pkt_session == NULL) {
> - m_freem(m);
> - continue;
> - }
> - pipex_ppp_input(m, pkt_session, 0);
> - }
> -}
> -
> -Static int
> -pipex_ppp_enqueue(struct mbuf *m0, struct pipex_session *session,
> - struct mbuf_queue *mq)
> -{
> - m0->m_pkthdr.ph_cookie = session;
> - /* XXX need to support other protocols */
> - m0->m_pkthdr.ph_ppp_proto = PPP_IP;
> -
> - if (mq_enqueue(mq, m0) != 0)
> - return (1);
> -
> - schednetisr(NETISR_PIPEX);
> -
> - return (0);
> -}
> -
> -/***********************************************************************
> * Timer functions
> ***********************************************************************/
> Static void
> @@ -840,13 +760,6 @@ pipex_timer(void *ignored_arg)
> /* FALLTHROUGH */
>
> case PIPEX_STATE_CLOSED:
> - /*
> - * mbuf queued in pipexinq or pipexoutq may have a
> - * refererce to this session.
> - */
> - if (!mq_empty(&pipexinq) || !mq_empty(&pipexoutq))
> - continue;
> -
> pipex_destroy_session(session);
> break;
>
> @@ -947,8 +860,26 @@ pipex_ip_output(struct mbuf *m0, struct
> m0->m_flags &= ~(M_BCAST|M_MCAST);
>
> /* output ip packets to the session tunnel */
> - if (pipex_ppp_enqueue(m0, session, &pipexoutq))
> - goto dropped;
> + if (session->is_multicast != 0) {
> + struct pipex_session *session_tmp;
> + struct mbuf *m;
> +
> + LIST_FOREACH(session_tmp, &pipex_session_list, session_list) {
> + if (session->pipex_iface != session_tmp->pipex_iface)
> + continue;
> + if (session->ip_forward == 0 &&
> + session->ip6_forward == 0)
> + continue;
> + m = m_copym(m0, 0, M_COPYALL, M_NOWAIT);
> + if (m == NULL) {
> + session->stat.oerrors++;
> + continue;
> + }
> + pipex_ppp_output(m, session_tmp, PPP_IP);
> + }
> + m_freem(m);
> + } else
> + pipex_ppp_output(m0, session, PPP_IP);
>
> return;
> drop:
> @@ -1207,7 +1138,7 @@ pipex_ip6_input(struct mbuf *m0, struct
>
> Static struct mbuf *
> pipex_common_input(struct pipex_session *session, struct mbuf *m0, int hlen,
> - int plen, int useq)
> + int plen)
> {
> int proto, ppphlen;
> u_char code;
> @@ -1255,19 +1186,10 @@ pipex_common_input(struct pipex_session
> m_adj(m0, plen - m0->m_pkthdr.len);
> }
>
> - if (!useq) {
> - pipex_ppp_input(m0, session, 0);
> - return (NULL);
> - }
> -
> - /* input ppp packets to kernel session */
> - if (pipex_ppp_enqueue(m0, session, &pipexinq) != 0)
> - goto dropped;
> - else
> - return (NULL);
> + pipex_ppp_input(m0, session, 0);
> + return (NULL);
> drop:
> m_freem(m0);
> -dropped:
> session->stat.ierrors++;
> return (NULL);
>
> @@ -1363,7 +1285,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
> sizeof(struct pipex_pppoe_header), (caddr_t)&pppoe);
>
> hlen = sizeof(struct ether_header) + sizeof(struct pipex_pppoe_header);
> - if ((m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length), 0))
> + if ((m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length)))
> == NULL)
> return (NULL);
> m_freem(m0);
> @@ -1658,7 +1580,7 @@ pipex_pptp_input(struct mbuf *m0, struct
> pipex_pptp_output(NULL, session, 0, 1);
> }
>
> - if ((m0 = pipex_common_input(session, m0, hlen, ntohs(gre->len), 1))
> + if ((m0 = pipex_common_input(session, m0, hlen, ntohs(gre->len)))
> == NULL) {
> /* ok, The packet is for PIPEX */
> if (!rewind)
> @@ -2092,7 +2014,7 @@ pipex_l2tp_input(struct mbuf *m0, int of
>
> length -= hlen + offset;
> hlen += off0 + offset;
> - if ((m0 = pipex_common_input(session, m0, hlen, length, 1)) == NULL) {
> + if ((m0 = pipex_common_input(session, m0, hlen, length)) == NULL) {
> /* ok, The packet is for PIPEX */
> if (!rewind)
> session->proto.l2tp.nr_gap += nseq;
> @@ -3010,12 +2932,6 @@ pipex_sysctl(int *name, u_int namelen, v
> return (ENOTDIR);
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &pipex_enable));
> - case PIPEXCTL_INQ:
> - return (sysctl_mq(name + 1, namelen - 1,
> - oldp, oldlenp, newp, newlen, &pipexinq));
> - case PIPEXCTL_OUTQ:
> - return (sysctl_mq(name + 1, namelen - 1,
> - oldp, oldlenp, newp, newlen, &pipexoutq));
> default:
> return (ENOPROTOOPT);
> }
> Index: sys/net/pipex.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 pipex.h
> --- sys/net/pipex.h 5 Jun 2020 19:50:59 -0000 1.23
> +++ sys/net/pipex.h 1 Jul 2020 19:20:28 -0000
> @@ -33,15 +33,11 @@
> * Names for pipex sysctl objects
> */
> #define PIPEXCTL_ENABLE 1
> -#define PIPEXCTL_INQ 2
> -#define PIPEXCTL_OUTQ 3
> -#define PIPEXCTL_MAXID 4
> +#define PIPEXCTL_MAXID 2
>
> #define PIPEXCTL_NAMES { \
> { 0, 0 }, \
> { "enable", CTLTYPE_INT }, \
> - { "inq", CTLTYPE_NODE }, \
> - { "outq", CTLTYPE_NODE }, \
> }
>
> #define PIPEX_PROTO_L2TP 1 /* protocol L2TP */
> Index: sys/net/pipex_local.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex_local.h,v
> retrieving revision 1.35
> diff -u -p -r1.35 pipex_local.h
> --- sys/net/pipex_local.h 18 Jun 2020 14:20:12 -0000 1.35
> +++ sys/net/pipex_local.h 1 Jul 2020 19:20:28 -0000
> @@ -402,7 +402,8 @@ void pipex_ip_input (st
> #ifdef INET6
> void pipex_ip6_input (struct mbuf *, struct pipex_session *);
> #endif
> -struct mbuf *pipex_common_input(struct pipex_session *, struct
> mbuf *, int, int, int);
> +struct mbuf *pipex_common_input(struct pipex_session *,
> + struct mbuf *, int, int);
>
> #ifdef PIPEX_PPPOE
> void pipex_pppoe_output (struct mbuf *, struct pipex_session
> *);
