On Thu, Aug 06, 2020 at 01:25:14PM +0200, Martin Pieuchot wrote:
> On 05/08/20(Wed) 12:50, Vitaliy Makkoveev wrote:
> > pipex(4) and pppx(4) are ready to became a little bit more MP capable.
> > Diff below moves pppx(4) related `ifnet' out of KERNEL_LOCK().
>
> Nice, one comment below.
>
> > Index: sys/net/if_pppx.c
> >
> > [skip]
> >
> > + NET_LOCK();
> > pipex_ppp_output(m, pxi->pxi_session, proto);
> > + NET_UNLOCK();
>
> This means the lock is taken and released for every packet. It would be
> better to grab it outside the loop.
Ok, fixed.
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.98
diff -u -p -r1.98 if_pppx.c
--- sys/net/if_pppx.c 28 Jul 2020 09:53:36 -0000 1.98
+++ sys/net/if_pppx.c 6 Aug 2020 11:54:44 -0000
@@ -191,7 +191,7 @@ int pppx_set_session_descr(struct pppx_
struct pipex_session_descr_req *);
void pppx_if_destroy(struct pppx_dev *, struct pppx_if *);
-void pppx_if_start(struct ifnet *);
+void pppx_if_qstart(struct ifqueue *);
int pppx_if_output(struct ifnet *, struct mbuf *,
struct sockaddr *, struct rtentry *);
int pppx_if_ioctl(struct ifnet *, u_long, caddr_t);
@@ -683,13 +683,12 @@ pppx_add_session(struct pppx_dev *pxd, s
snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
ifp->if_mtu = req->pr_peer_mru; /* XXX */
ifp->if_flags = IFF_POINTOPOINT | IFF_MULTICAST | IFF_UP;
- ifp->if_xflags = IFXF_CLONED;
- ifp->if_start = pppx_if_start;
+ ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
+ ifp->if_qstart = pppx_if_qstart;
ifp->if_output = pppx_if_output;
ifp->if_ioctl = pppx_if_ioctl;
ifp->if_rtrequest = p2p_rtrequest;
ifp->if_type = IFT_PPP;
- ifq_set_maxlen(&ifp->if_snd, 1);
ifp->if_softc = pxi;
/* ifp->if_rdomain = req->pr_rdomain; */
@@ -864,21 +863,15 @@ pppx_if_destroy(struct pppx_dev *pxd, st
}
void
-pppx_if_start(struct ifnet *ifp)
+pppx_if_qstart(struct ifqueue *ifq)
{
+ struct ifnet *ifp = ifq->ifq_if;
struct pppx_if *pxi = (struct pppx_if *)ifp->if_softc;
struct mbuf *m;
int proto;
- if (!ISSET(ifp->if_flags, IFF_RUNNING))
- return;
-
- for (;;) {
- m = ifq_dequeue(&ifp->if_snd);
-
- if (m == NULL)
- break;
-
+ NET_LOCK();
+ while ((m = ifq_dequeue(ifq)) != NULL) {
proto = *mtod(m, int *);
m_adj(m, sizeof(proto));
@@ -887,6 +880,7 @@ pppx_if_start(struct ifnet *ifp)
pipex_ppp_output(m, pxi->pxi_session, proto);
}
+ NET_UNLOCK();
}
int
