Re: destroy ppp(4) on closing tty

[christos%zoulas.com@localhost (Christos Zoulas)]

On Apr 5,  1:16pm, yasuoka%iij.ad.jp@localhost (Yasuoka Masahiko) wrote:
-- Subject: Re: destroy ppp(4) on closing tty

| 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 */
| 
| 

I guess this is better now, why not make it consistent with tun and keep it
after close?

christos