On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote:
> On Thu, 30 Jul 2020 15:34:09 +0300
> Vitaliy Makkoveev <[email protected]> wrote:
> > On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote:
> >> Hi,
> >>
> >> sys/net/if_ethersubr.c:
> >> 372 void
> >> 373 ether_input(struct ifnet *ifp, struct mbuf *m)
> >> (snip)
> >> 519 #if NPPPOE > 0 || defined(PIPEX)
> >> 520 case ETHERTYPE_PPPOEDISC:
> >> 521 case ETHERTYPE_PPPOE:
> >> 522 if (m->m_flags & (M_MCAST | M_BCAST))
> >> 523 goto dropanyway;
> >> 524 #ifdef PIPEX
> >> 525 if (pipex_enable) {
> >> 526 struct pipex_session *session;
> >> 527
> >> 528 if ((session = pipex_pppoe_lookup_session(m))
> >> != NULL) {
> >> 529 pipex_pppoe_input(m, session);
> >> 530 return;
> >> 531 }
> >> 532 }
> >> 533 #endif
> >>
> >> previously a packet which branchces to #529 is enqueued.
> >>
> >> If the diff removes the queue, then the pipex input routine is
> >> executed by the NIC's interrupt handler.
> >>
> >> The queues had been made to avoid that kind of situations.
> >
> > It's not enqueued in pppoe case. According pipex_pppoe_input() code we
> > call pipex_common_input() with `useq' argument set to '0', so we don't
> > enqueue mbuf(9) but pass it to pipex_ppp_input() which will pass it to
> > ipv{4,6}_input().
>
> You are right. Sorry, I forgot about this which I did that by myself.
>
I'm interesting the reason why you did that.
> >> Also I don't see a relation of the use-after-free problem and killing
> >> queues. Can't we fix the problem unless we kill the queues?
> >
> > Yes we can. Reference counters allow us to keep orphan sessions in these
> > queues without use after free issue.
> >
> > I will wait your commentaries current enqueuing before to do something.
>
> I have another concern.
>
> You might know, when L2TP/IPsec is used heavily, the crypto thread
> uses 100% of 1 CPU core. In that case, that thread becomes like
> below:
>
> crypto thread -> udp_userreq -> pipex_l2tp_input
>
> some clients are using MPPE(RC4 encryption) on CCP. It's not so
> light.
>
> How do we offload this for CPUs? I am thinking that "pipex" can have
> a dedicated thread. Do we have another scenario?
>
I suppose you mean udp_input(). What is you call "crypto thread"? I did
a little backtrace but I didn't find this thread.
ether_resolve
if_input_local
ipv4_input
ip_input_if
ip_ours
ip_deliver
udp_input (through pr_input)
pipex_l2tp_input
ipi{,6}_mloopback
if_input_local
ipv4_input
...
udp_input (through pr_input)
pipex_l2tp_input
loinput
if_input_local
ipv4_input
...
udp_input (through pr_input)
pipex_l2tp_input
Also various pseudo drivers call ipv{4,6}_input() and underlay
udp_unput() too.
Except nfs, we call udp_usrreq() through socket layer only. Do you mean
userland as "crypto thread"?
But upd_input(), udp_usrreq() and pipexintr() are serialized by
NET_LOCK(). We should move pipex(4) under it's own rwlock to allow them
simultaneous execution. Also, should we move outgoing pppx(4) mbufs to
queue too?
