Re: destroy ppp(4) on closing tty

[Yasuoka Masahiko <yasuoka%iij.ad.jp@localhost>]

On Fri, 4 Apr 2008 16:35:49 +0200
Quentin Garnier <cube%cubidou.net@localhost> wrote:
> On Fri, Apr 04, 2008 at 09:50:57AM -0400, Greg Troxel wrote:
> > Yasuoka Masahiko <yasuoka%iij.ad.jp@localhost> writes:
> > > I wonder why ppp(4) destroys itself when the attached tty is closed.  
> > > tun(4) doesn't destroy itself when the attached device file is closed.
> > >
> > > Is there somebody knows the reason?
> > 
> > Does the ppp(4) get created when something on the tty sets line
> > discipline to PPP?  If so, it makes sense to destroy on close.
> 
> What could be confusing I guess is that you can create a ppp interface
> with ifconfig, then get it used by pppd through the line discipline,
> which will eventually destroy it on close.

Thank you.  Yes, i confused that point.  My program can't get ppp
interface's i/o stastics after pppd(8) exits.

> That could be fixed by a simple flag set from the cloner interface.  The
> only reason ppp(4) is a cloner is so that pppd(8) can check there is
> support for it by listing the cloners.  Otherwise there's no point in
> letting the user do ifconfig ppp0 create.

A patch attached at the end of this message.

> > If a tun is created by a call on a descriptor, I'd say it too should be
> > destroyed on close.  If it's created by 'ifconfig tun0 create', then it
> > shouldn't be destroyed on close.
> 
> That's indeed the case.

How about 'device/tty close' case?

As far as my test, current behavior is

  -----------  -----------  ---------------
               created by
               ifconfig     device/tty open
  -----------  -----------  ---------------
  tun(4)       remain       remain
  ppp(4)       destroyed    destroyed
  -----------  -----------  ---------------

and after the patch,

  -----------  -----------  ---------------
               created by
               ifconfig     device/tty open
  -----------  -----------  ---------------
  tun(4)       remain       remain
  ppp(4)       remain       destroyed
  -----------  -----------  ---------------

Their behaviors sould be consistent?

--yasuoka

Index: if_ppp.c
===================================================================
RCS file: /vol/cvs/NetBSD/src/sys/net/if_ppp.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 if_ppp.c
--- if_ppp.c    2 Nov 2006 05:14:42 -0000       1.1.1.1
+++ if_ppp.c    5 Apr 2008 03:31:11 -0000
@@ -353,7 +353,7 @@ pppalloc(pid)
     pid_t pid;
 {
     struct ppp_softc *sc = NULL, *scf;
-    int i;
+    int i, sc_flags;
 
     simple_lock(&ppp_list_mutex);
     for (scf = LIST_FIRST(&ppp_softc_list); scf != NULL;
@@ -368,8 +368,11 @@ pppalloc(pid)
     }
     simple_unlock(&ppp_list_mutex);
 
-    if (sc == NULL)
+    sc_flags = 0;
+    if (sc == NULL) {
        sc = ppp_create(ppp_cloner.ifc_name, -1);
+       sc_flags |= SC_TTYCLONE;
+    }
 
 #ifdef __HAVE_GENERIC_SOFT_INTERRUPTS
     sc->sc_si = softintr_establish(IPL_SOFTNET, pppintr, sc);
@@ -378,7 +381,7 @@ pppalloc(pid)
        return (NULL);
     }
 #endif
-    sc->sc_flags = 0;
+    sc->sc_flags = sc_flags;
     sc->sc_mru = PPP_MRU;
     sc->sc_relinq = NULL;
     (void)memset(&sc->sc_stats, 0, sizeof(sc->sc_stats));
@@ -476,7 +479,8 @@ pppdealloc(sc)
        sc->sc_comp = 0;
     }
 #endif
-    (void)ppp_clone_destroy(&sc->sc_if);
+    if ((sc->sc_flags & SC_TTYCLONE) != 0)
+       (void)ppp_clone_destroy(&sc->sc_if);
 }
 
 /*
Index: if_ppp.h
===================================================================
RCS file: /vol/cvs/NetBSD/src/sys/net/if_ppp.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 if_ppp.h
--- if_ppp.h    2 Nov 2006 05:14:42 -0000       1.1.1.1
+++ if_ppp.h    5 Apr 2008 03:41:35 -0000
@@ -73,6 +73,7 @@
 /*
  * State bits in sc_flags, not changeable by user.
  */
+#define SC_TTYCLONE    0x00000200      /* created by tty open */
 #define SC_TIMEOUT     0x00000400      /* timeout is currently pending */
 #define SC_VJ_RESET    0x00000800      /* need to reset VJ decomp */
 #define SC_COMP_RUN    0x00001000      /* compressor has been inited */