ok yasuoka
On Thu, 30 Jun 2022 12:26:55 +0300
Vitaliy Makkoveev <m...@openbsd.org> wrote:
> yasuoka@ remonded me, long time ago pipex(4) sessions can't be deleted
> until both input and output queues become empty:
>
> pipex_timer(void *ignored_arg)
> {
> /* ... */
> switch (session->state) {
> /* ... */
> 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;
> /* ... */
> }
>
> Such dead sessions were linked to the stack and the `ip_forward' flag
> was used to prevent packets forwarding.
>
> But since we started to unlink close session from the stack, this logic
> became unnecessary. Also pipex(4) session could be closed just after
> close request.
>
> I want to remove it. This makes the pipex(4) session flags immutable and
> reduces locking games.
>
> This diff removes PIPEXCSESSION call only from npppd(8). It deletes
> session just after PIPEXCSESSION ioctl(2) call so nothing changed in
> session life within kernel space. I will modify kernel and pipex(4) man
> page with separate diff, after I finish to fix pipex(4) locking.
>
> Index: usr.sbin/npppd/npppd/npppd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 npppd.c
> --- usr.sbin/npppd/npppd/npppd.c 15 Nov 2021 15:14:24 -0000 1.52
> +++ usr.sbin/npppd/npppd/npppd.c 30 Jun 2022 08:49:29 -0000
> @@ -114,7 +114,6 @@ static struct in_addr loop; /* initializ
> static uint32_t str_hash(const void *, int);
>
> #ifdef USE_NPPPD_PIPEX
> -static int npppd_ppp_pipex_ip_disable(npppd *, npppd_ppp *);
> static void pipex_periodic(npppd *);
> #endif /* USE_NPPPD_PIPEX */
>
> @@ -1246,62 +1245,6 @@ npppd_ppp_pipex_disable(npppd *_this, np
> return error;
> }
>
> -/* XXX: s/npppd_ppp_pipex_ip_disable/npppd_ppp_pipex_stop/ ?? */
> -
> -/** Stop PIPEX of the {@link npppd_ppp ppp} */
> -static int
> -npppd_ppp_pipex_ip_disable(npppd *_this, npppd_ppp *ppp)
> -{
> - struct pipex_session_config_req req;
> -#ifdef USE_NPPPD_PPPOE
> - pppoe_session *pppoe;
> -#endif
> -#ifdef USE_NPPPD_PPTP
> - pptp_call *call;
> -#endif
> -#ifdef USE_NPPPD_L2TP
> - l2tp_call *l2tp;
> -#endif
> - if (ppp->pipex_started == 0)
> - return 0; /* not started */
> -
> - bzero(&req, sizeof(req));
> - switch(ppp->tunnel_type) {
> -#ifdef USE_NPPPD_PPPOE
> - case NPPPD_TUNNEL_PPPOE:
> - pppoe = (pppoe_session *)ppp->phy_context;
> -
> - /* PPPoE specific information */
> - req.pcr_protocol = PIPEX_PROTO_PPPOE;
> - req.pcr_session_id = pppoe->session_id;
> - break;
> -#endif
> -#ifdef USE_NPPPD_PPTP
> - case NPPPD_TUNNEL_PPTP:
> - call = (pptp_call *)ppp->phy_context;
> -
> - /* PPTP specific information */
> - req.pcr_session_id = call->id;
> - req.pcr_protocol = PIPEX_PROTO_PPTP;
> - break;
> -#endif
> -#ifdef USE_NPPPD_L2TP
> - case NPPPD_TUNNEL_L2TP:
> - l2tp = (l2tp_call *)ppp->phy_context;
> -
> - /* L2TP specific context */
> - req.pcr_session_id = l2tp->session_id;
> - req.pcr_protocol = PIPEX_PROTO_L2TP;
> - break;
> -#endif
> - default:
> - return 1;
> - }
> - req.pcr_ip_forward = 0;
> -
> - return ioctl(_this->iface[ppp->ifidx].devf, PIPEXCSESSION, &req);
> -}
> -
> static void
> pipex_periodic(npppd *_this)
> {
> @@ -1565,11 +1508,6 @@ npppd_set_ip_enabled(npppd *_this, npppd
> hl->key = ppp1->username;
> }
> }
> -#ifdef USE_NPPPD_PIPEX
> - if (npppd_ppp_pipex_ip_disable(_this, ppp) != 0)
> - ppp_log(ppp, LOG_ERR,
> - "npppd_ppp_pipex_ip_disable() failed: %m");
> -#endif /* USE_NPPPD_PIPEX */
> }
> }
>
>
