Subject: kern/5898: Two ppp bugs fixed
To: None <gnats-bugs@gnats.netbsd.org>
From: Felix A. Croes <felix@simplex.nl>
List: netbsd-bugs
Date: 08/02/1998 16:01:58
>Number:         5898
>Category:       kern
>Synopsis:       PPP last packet often unsent + pppstart() has wrong prototype
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug  2 07:05:00 1998
>Last-Modified:
>Originator:     Felix A. Croes
>Organization:
	Dworkin B.V.
>Release:        NetBSD 1.3.2, NetBSD-current
>Environment:
	NetBSD 1.3.2 i386 -O2 -m486 -fomit-frame-pointer
System: NetBSD tyrathect 1.3.2 NetBSD 1.3.2 (TYRATHECT) #0: Sun Aug 2 15:22:40 CEST 1998 root@tyrathect:/usr/src/sys/arch/i386/compile/TYRATHECT i386


>Description:
1. pppintr() does not call pppasyncstart() if the output queues are empty.
   However, it should also check for a partially built output packet.
2. The prototype of pppstart() is incorrect: this is the standard line start
   function which cannot have more than one argument, the tty.
>How-To-Repeat:
1. Create a ppp connection over a serial line.  Ftp to the NetBSD host,
   turn on hash mark printing if necessary, turn off prompt and mget a lot
   of files (each 100K or so).  Observed behaviour: the transmission will
   often pause, sometimes for only a second and sometimes much longer, just
   at or just before reaching 100% for each file.
>Fix:
1. add a check for sc->sc_outm in pppintr().
2. No force flag can be used.  However, force is only 1 if pppstart() is
   called from ppp_timeout().  This means that the force flag check can
   be replaced with a (sc->sc_flags & SC_TIMEOUT) check.

*** /sys/net/if_ppp.c.old	Tue May  5 08:53:34 1998
--- /sys/net/if_ppp.c	Sun Aug  2 15:14:32 1998
***************
*** 1013,1019 ****
      s = splsoftnet();
      for (i = 0; i < NPPP; ++i, ++sc) {
  	if (!(sc->sc_flags & SC_TBUSY)
! 	    && (sc->sc_if.if_snd.ifq_head || sc->sc_fastq.ifq_head)) {
  	    s2 = splimp();
  	    sc->sc_flags |= SC_TBUSY;
  	    splx(s2);
--- 1013,1020 ----
      s = splsoftnet();
      for (i = 0; i < NPPP; ++i, ++sc) {
  	if (!(sc->sc_flags & SC_TBUSY)
! 	    && (sc->sc_if.if_snd.ifq_head || sc->sc_fastq.ifq_head
! 		|| sc->sc_outm)) {
  	    s2 = splimp();
  	    sc->sc_flags |= SC_TBUSY;
  	    splx(s2);
*** /sys/net/ppp_tty.c.old	Tue May  5 08:53:36 1998
--- /sys/net/ppp_tty.c	Sun Aug  2 15:20:46 1998
***************
*** 119,125 ****
  int	ppptioctl __P((struct tty *tp, u_long cmd, caddr_t data, int flag,
  		       struct proc *));
  int	pppinput __P((int c, struct tty *tp));
! int	pppstart __P((struct tty *tp, int));
  
  static u_int16_t pppfcs __P((u_int16_t fcs, u_char *cp, int len));
  static void	pppasyncstart __P((struct ppp_softc *));
--- 119,125 ----
  int	ppptioctl __P((struct tty *tp, u_long cmd, caddr_t data, int flag,
  		       struct proc *));
  int	pppinput __P((int c, struct tty *tp));
! int	pppstart __P((struct tty *tp));
  
  static u_int16_t pppfcs __P((u_int16_t fcs, u_char *cp, int len));
  static void	pppasyncstart __P((struct ppp_softc *));
***************
*** 664,670 ****
  
      /* Call pppstart to start output again if necessary. */
      s = spltty();
!     pppstart(tp, 0);
  
      /*
       * This timeout is needed for operation on a pseudo-tty,
--- 664,670 ----
  
      /* Call pppstart to start output again if necessary. */
      s = spltty();
!     pppstart(tp);
  
      /*
       * This timeout is needed for operation on a pseudo-tty,
***************
*** 705,713 ****
   * Called at spltty or higher.
   */
  int
! pppstart(tp, force)
      register struct tty *tp;
-     int force;
  {
      register struct ppp_softc *sc = (struct ppp_softc *) tp->t_sc;
  
--- 705,712 ----
   * Called at spltty or higher.
   */
  int
! pppstart(tp)
      register struct tty *tp;
  {
      register struct ppp_softc *sc = (struct ppp_softc *) tp->t_sc;
  
***************
*** 723,729 ****
       * or been disconnected from the ppp unit, then tell if_ppp.c that
       * we need more output.
       */
!     if (CCOUNT(&tp->t_outq) >= PPP_LOWAT && !force)
  	return 0;
      if (!((tp->t_state & TS_CARR_ON) == 0 && (tp->t_cflag & CLOCAL) == 0)
  	&& sc != NULL && tp == (struct tty *) sc->sc_devp) {
--- 722,729 ----
       * or been disconnected from the ppp unit, then tell if_ppp.c that
       * we need more output.
       */
!     if (CCOUNT(&tp->t_outq) >= PPP_LOWAT
! 	&& (sc == NULL || (sc->sc_flags & SC_TIMEOUT)))
  	return 0;
      if (!((tp->t_state & TS_CARR_ON) == 0 && (tp->t_cflag & CLOCAL) == 0)
  	&& sc != NULL && tp == (struct tty *) sc->sc_devp) {
***************
*** 746,752 ****
  
      s = spltty();
      sc->sc_flags &= ~SC_TIMEOUT;
!     pppstart(tp, 1);
      splx(s);
  }
  
--- 746,752 ----
  
      s = spltty();
      sc->sc_flags &= ~SC_TIMEOUT;
!     pppstart(tp);
      splx(s);
  }
  
>Audit-Trail:
>Unformatted: