On Tue, Mar 31, 2020 at 06:15:46PM +0200, Martin Pieuchot wrote:
> 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.
I made patch for npppd(8) too. I include it to this email below, without
starting new thread, ok? If ioctl(PIPEXASESSION) failed and error !=
ENXIO it means that pipex is enabled and session creation failed so down
this connection.
> Then if the daemon has a bug, should the kernel work around it?
In most common cases should :(
---- cut begin
Index: usr.sbin/npppd/npppd/ppp.c
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/npppd/ppp.c,v
retrieving revision 1.28
diff -u -p -r1.28 ppp.c
--- usr.sbin/npppd/npppd/ppp.c 27 Feb 2019 04:52:19 -0000 1.28
+++ usr.sbin/npppd/npppd/ppp.c 31 Mar 2020 19:59:21 -0000
@@ -1134,8 +1134,12 @@ ppp_on_network_pipex(npppd_ppp *_this)
(!MPPE_MUST_NEGO(_this) || _this->ccp.fsm.state == OPENED ||
_this->ccp.fsm.state == STOPPED)) {
/* IPCP is opened and MPPE is not required or MPPE is opened */
- if (npppd_ppp_pipex_enable(_this->pppd, _this) != 0)
+ if (npppd_ppp_pipex_enable(_this->pppd, _this) != 0) {
ppp_log(_this, LOG_WARNING, "failed enable pipex: %m");
+ /* failed to create pipex session */
+ ppp_phy_downed(_this);
+ return;
+ }
ppp_log(_this, LOG_NOTICE, "Using pipex=%s",
(_this->pipex_enabled != 0)? "yes" : "no");
_this->pipex_started = 1;
---- cut end
And patch for pppx()
---- cut begin
Index: sys/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
--- sys/net/if_pppx.c 26 Mar 2020 16:50:46 -0000 1.77
+++ sys/net/if_pppx.c 31 Mar 2020 20:00:33 -0000
@@ -665,6 +665,12 @@ pppx_add_session(struct pppx_dev *pxd, s
struct ifnet *over_ifp = NULL;
#endif
+ /*
+ * XXX: prevent pxi destruction by pipex_timer()
+ */
+ if (req->pr_timeout_sec != 0)
+ return (EINVAL);
+
switch (req->pr_protocol) {
#ifdef PIPEX_PPPOE
case PIPEX_PROTO_PPPOE:
---- cut end
