kern/48935: ppp: ipcp gets stuck in stopped state

[tilman%code-monkey.de@localhost]

>Number:         48935
>Category:       kern
>Synopsis:       ppp: ipcp gets stuck in stopped state
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jun 21 11:00:01 +0000 2014
>Originator:     Tilman Sauerbeck
>Release:        6.1.4_PATCH
>Organization:
>Environment:
NetBSD ganesha 6.1.4_PATCH NetBSD 6.1.4_PATCH (SHEEVAPLUG) #0: Sat Jun 21 
12:26:35 CEST 2014  
tilman@brimstone:/tmp/netbsd/src/sys/arch/evbarm/compile/obj/SHEEVAPLUG evbarm
>Description:
I'm using a DSL modem to connect to the internet via pppoe. My ISP closes the 
connection if it has been up for 24 hours.

The problem is that seldomly, re-establishing the connection will fail.

I have tracked down the problem:
What happens is that during the connection attempt, a timeout occurs when 
configuring IPCP (= sppp_to_event() is called for IPCP with rst_counter being 
0).
This leads to IPCP's tlf hook to be run, puts IPCP in the stopped state and 
runs sppp_lcp_check_and_close(). The latter function then realizes that there 
are no more NCPs started, and calls lcp.Close(). This leads to sppp_lcp_tld() 
being called, which currently only brings down those CPs that are marked 
"started" in lcp.protos.

The issue is that at this point, IPCP is *not* included in the lcp.protos 
bitmask and thus is not brought down and closed but will remain in the stopped 
state.

>How-To-Repeat:
Wait for a timeout to happen when IPCP is configured.
>Fix:
Something like this might help. I haven't been able to reproduce a timeout with 
this patch though so I don't know for sure if it will fix the problem:

diff --git a/sys/net/if_spppsubr.c b/sys/net/if_spppsubr.c
index 590bc50..c04f5b2 100644
--- a/sys/net/if_spppsubr.c
+++ b/sys/net/if_spppsubr.c
@@ -2591,7 +2591,6 @@ sppp_lcp_tld(struct sppp *sp)
 {
        STDDCL;
        int i;
-       uint32_t mask;
 
        sp->pp_phase = SPPP_PHASE_TERMINATE;
 
@@ -2606,9 +2605,13 @@ sppp_lcp_tld(struct sppp *sp)
         * the Close second to prevent the upper layers from sending
         * ``a flurry of terminate-request packets'', as the RFC
         * describes it.
+        *
+        * Note that we ignore lcp.protos here: this prevents CPs
+        * from remaining in the stopped state forever.
         */
-       for (i = 0, mask = 1; i < IDX_COUNT; i++, mask <<= 1)
-               if ((sp->lcp.protos & mask) && ((cps[i])->flags & CP_LCP) == 0) 
{
+       for (i = 0; i < IDX_COUNT; i++)
+               if ((sp->state[cps[i]->protoidx] != STATE_INITIAL) &&
+                   ((cps[i])->flags & CP_LCP) == 0) {
                        (cps[i])->Down(sp);
                        (cps[i])->Close(sp);
                }