Re: [patch] Fix ppp/slip wrong behaviour in 2.2.14

From: Stanislav V. Voronyi (stas@esc.kharkov.com)
Date: Thu Feb 03 2000 - 09:46:15 EST


In message <00020310255700.04042@argo.linuxcare.com.au> Paul Mackerras
    writes:

>On Thu, 03 Feb 2000, Stanislav V. Voronyi wrote:

>> I work at quite big ISP all running linux and after
>> upgrade from 2.0 to 2.2 kernel we found a problem: if pppd or
>> slcall was killed by SIGKILL for instance, network interface

>Why do you use SIGKILL rather than SIGTERM?

        Have you ever run modem pool with dozens of discret modems
on analog telephone lines with awful quality ? I do. And I every
day oblige to use kill -9 xxxx as last method to kill hanged pppd
process. And now with 2.2 kernel I oblige to seek ppp interface
and than do ifconfig pppXX down since in 2.2 it does not close
automaticaly, what my patch fixed.

>> still up and it is very bad for Radius server because Radius
>> decide that IP# which was used on this interface freed and can
>> be used further. But since interface still UP gated continued
>> to announce this address through OSPF.

>There is more to this issue than meets the eye, perhaps. The problem with
>having the interface go down automatically when the modem hangs up is that
>pppd then immediately loses all control of the interface. The interface
>can then be reassigned to another call before the original pppd realizes
>that it's gone. This means for instance that pppd may report 0 bytes sent
>and received for the connection.

>With the code the way it is, at least pppd gets to read the stats and
>clean up, after the modem hangs up, before it releases the ppp interface.

        But if pppd fails undepend on reason ppp interface must be closed.
And this kernel job isn't it ?

>In the new PPP driver in the linux 2.3.xx kernel, we have a /dev/ppp
>device which gives pppd a way to control the ppp interface independently
>of the tty. This lets us fix the problem properly. If the open instance
>of /dev/ppp gets closed (e.g. by pppd dying) it marks the interface down
>and releases it.

        Do you suppouse that if this bug fixed in 2.3 kernel its good reason
to not fix in 2.2 kernel ? And apart from ppp there is also slip driver with
the same problems. And for slip both problems still have place in 2.3 kernel
too.

>> From other side if I would do ifconfig pppXX down or

>Why would you do that?

        All what I've made in this patch that all from practical needings
of big ISP with dozens of servers all running Linux. And even don't pay
attention to my needings - what do you think more correct: have in system
useless command that do only half of job and ask people "Why would you do that ?"
or make this command complete and useful ?

>> + /* down interface if it is still UP */
>> + if (ppp2dev(ppp)->start) {
>> + dev_close(ppp2dev(ppp));
>> + unregister_netdevice(ppp2dev(ppp));
>> + }

>Some comments about this code: first, wouldn't it be simpler to check
>(ppp2dev(ppp)->flags & IFF_UP)? Secondly, I am pretty sure you need to

        Look once more to my code and you'll find that dev->start and
dev->flags & IFF_UP are not similar.

>put rtnl_lock() / rtnl_unlock() around the dev_close and probably also the
>unregister_netdevice.

        I think you are right here. I make this calls in patch for 2.3 kernel,
but I forget about it in 2.2. Here is corrected patch:

diff -u --recursive work/v2.2/linux.dist/CREDITS linux-2.2.14/CREDITS
--- work/v2.2/linux.dist/CREDITS Tue Jan 4 20:12:10 2000
+++ linux-2.2.14/CREDITS Thu Jan 27 00:31:52 2000
@@ -2116,6 +2116,16 @@
 S: Moorhead, Minnesota 56560
 S: USA
 
+N: Stanislav Voronyi
+E: stas@esc.kharkov.com
+D: SLIP linefill/keepalive option
+D: SLIP/PPP interface behaviour fix
+D: Console driver hacks
+D: AWE 3DSE support
+W: http://esc.kharkov.com/stas/
+S: ap4, 5, Kashuby st.
+S: Kharkov, Ukraine, 61034
+
 N: Jos Vos
 E: jos@xos.nl
 W: http://www.xos.nl/
diff -u --recursive work/v2.2/linux.dist/drivers/net/ppp.c linux-2.2.14/drivers/net/ppp.c
--- work/v2.2/linux.dist/drivers/net/ppp.c Wed Oct 27 03:53:40 1999
+++ linux-2.2.14/drivers/net/ppp.c Sun Jan 30 13:45:25 2000
@@ -421,6 +421,10 @@
  * Called when the line discipline is changed to something
  * else, the tty is closed, or the tty detects a hangup.
  */
+/*
+ * Sane behaviour on interface down / line discipline change
+ * -- Stanislav Voronyi (VSV).
+ */
 
 static void
 ppp_tty_close (struct tty_struct *tty)
@@ -439,6 +443,7 @@
                 ppp->tty = ppp->backup_tty = 0;
                 return;
         }
+
         if (tty == ppp->backup_tty)
                 ppp->backup_tty = 0;
         if (tty != ppp->tty)
@@ -457,6 +462,16 @@
 
                 ppp_async_release(ppp);
                 ppp_release(ppp);
+
+ /* down interface if it is still UP */
+ if (ppp2dev(ppp)->start) {
+ rtnl_lock();
+ dev_close(ppp2dev(ppp));
+ unregister_netdevice(ppp2dev(ppp));
+ rtnl_unlock();
+ }
+ /* VSV end */
+
                 MOD_DEC_USE_COUNT;
         }
 }
@@ -1532,6 +1547,8 @@
                 printk(KERN_ERR "ppp: %s not active\n", dev->name);
                 return -ENXIO;
         }
+ dev->tbusy = 0;
+ dev->start = 1;
 
         MOD_INC_USE_COUNT;
 
@@ -1548,6 +1565,13 @@
         struct ppp *ppp = dev2ppp (dev);
 
         CHECK_PPP_MAGIC(ppp);
+ dev->tbusy = 1;
+ dev->start = 0;
+
+ /* if interface going down first -> force hangup
+ -- Stanislav Voronyi. */
+ if (ppp->tty)
+ tty_hangup(ppp->tty);
 
         MOD_DEC_USE_COUNT;
 
diff -u --recursive work/v2.2/linux.dist/drivers/net/slip.c linux-2.2.14/drivers/net/slip.c
--- work/v2.2/linux.dist/drivers/net/slip.c Wed Oct 27 03:53:40 1999
+++ linux-2.2.14/drivers/net/slip.c Sun Jan 30 13:05:16 2000
@@ -52,6 +52,7 @@
  * have been ported to Linux SLIP driver.
  * Vitaly E. Lavrov : Sane behaviour on tty hangup.
  * Alexey Kuznetsov : Cleanup interfaces to tty&netdevice modules.
+ * Stanislav Voronyi : Sane behaviour on interface down / line discipline change
  */
 
 #define SL_CHECK_TRANSMIT
@@ -550,6 +551,10 @@
         sl->rcount = 0;
         sl->xleft = 0;
         end_bh_atomic();
+ /* If netdevice going down first -> force hangup
+ -- Stanislav Voronyi */
+ if (sl->tty)
+ tty_hangup(sl->tty);
 
         MOD_DEC_USE_COUNT;
         return 0;
@@ -924,6 +929,14 @@
    I see no way to make more with current tty code. --ANK
  */
 
+/*
+ I found that this function called from tty_set_ldisc()
+ and tty_hangup() before setting new discipline.
+ So we can close netdevice if line discipline was changed.
+ -- Stanislav Voronyi.
+*/
+
+
 /*
  * Close down a SLIP channel.
  * This means flushing out any pending queues, and then restoring the
@@ -943,6 +956,16 @@
         sl->tty = NULL;
         if (!sl->leased)
                 sl->line = 0;
+
+ /* VSV down interface if it is active
+ we do it after sl->tty = NULL; to prevent loop */
+ if ( sl->dev->start ){
+ rtnl_lock();
+ dev_close(sl->dev);
+ unregister_netdevice(sl->dev);
+ rtnl_unlock();
+ }
+ /* VSV end */
 
         /* VSV = very important to remove timers */
 #ifdef CONFIG_SLIP_SMART

-- 
Stanislav V. Voroniy                            |Tel. +380 (572) 128302
Head of System Administration Department        |                210901
Centre of Information Technologies "Vostok" Ltd.|Fax. +380 (572) 213117

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Feb 07 2000 - 21:00:09 EST