On 31/03/20(Tue) 18:58, Vitaliy Makkoveev wrote:
> On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> > On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > > I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> > > No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> > > safe. Unfortunately, this way gives some performance impact, because we
> > > need to call pipex_lookup_by_session_id() in some places. This impact
> > > will gone after pipex_session becames safely shared between multiple
> > > holders and this is my next goal.
> >
> > I'd be more confident if we could go with the one-line change that you
> > submitted in the first mail of this thread without the debugging
> > messages.
> >
> > Mixing bug-fixes (or features) and refactoring is not a great
> > development practise as it might hide the point of the change and
> > introduce or expose new bugs.
> >
> I changed my original patch. Since npppd(8) ignores ioctl() errors and
> client will be connected without pppx(4) interface creation I decide to
> lie npppd(8).
Well better fix npppd(8), no? Not crashing the kernel is priority 1.
Then if the daemon has a bug, should the kernel work around it?
> Index: net/if_pppx.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 if_pppx.c
> --- net/if_pppx.c 26 Mar 2020 16:50:46 -0000 1.77
> +++ net/if_pppx.c 31 Mar 2020 15:48:26 -0000
> @@ -665,6 +665,13 @@ pppx_add_session(struct pppx_dev *pxd, s
> struct ifnet *over_ifp = NULL;
> #endif
>
> + /*
> + * XXX: prevent pxi destruction by pipex_timer()
> + * npppd(8) ignores pppx_add_session() errors so fake it
> + */
> + if (req->pr_timeout_sec != 0)
> + req->pr_timeout_sec = 0;
> +
> switch (req->pr_protocol) {
> #ifdef PIPEX_PPPOE
> case PIPEX_PROTO_PPPOE:
>
