From e4e0fc419308c73b50614b20b5fda286b894f618 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Mon, 18 Jun 2018 01:11:01 +0200 Subject: [PATCH] PPP: set PPP_NUM_TIMEOUTS_PER_PCB value to computed number of simultaneous timers A deep analysis of simultaneously running timers showed we can use a slightly smaller value for PPP_NUM_TIMEOUTS_PER_PCB, value which was arbitrarily chosen to a safe value based on the number of enabled PPP features. Add the boring and long analysis to the end of our internal header file and use the result in ppp_opts.h. Signed-off-by: Sylvain Rochet --- src/include/netif/ppp/ppp_impl.h | 88 ++++++++++++++++++++++++++++++++ src/include/netif/ppp/ppp_opts.h | 5 +- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/include/netif/ppp/ppp_impl.h b/src/include/netif/ppp/ppp_impl.h index d63db0b6..d10ea7b0 100644 --- a/src/include/netif/ppp/ppp_impl.h +++ b/src/include/netif/ppp/ppp_impl.h @@ -621,6 +621,94 @@ void ppp_dump_packet(ppp_pcb *pcb, const char *tag, unsigned char *p, int len); /* dump packet to debug log if interesting */ #endif /* PRINTPKT_SUPPORT */ +/* + * Number of necessary timers analysis. + * + * PPP use at least one timer per each of its protocol, but not all protocols are + * active at the same time, thus the number of necessary timeouts is actually + * lower than enabled protocols. Here is the actual necessary timeouts based + * on code analysis. + * + * Note that many features analysed here are not working at all and are only + * there for a comprehensive analysis of necessary timers in order to prevent + * having to redo that each time we add a feature. + * + * Timer list + * + * | holdoff timeout + * | low level protocol timeout (PPPoE or PPPoL2P) + * | LCP delayed UP + * | LCP retransmit (FSM) + * | LCP Echo timer + * .| PAP or CHAP or EAP authentication + * . | ECP retransmit (FSM) + * . | CCP retransmit (FSM) when MPPE is enabled + * . | CCP retransmit (FSM) when MPPE is NOT enabled + * . | IPCP retransmit (FSM) + * . .| IP6CP retransmit (FSM) + * . . | Idle time limit + * . . | Max connect time + * . . | Max octets + * . . | CCP RACK timeout + * . . . + * PPP_PHASE_DEAD + * PPP_PHASE_HOLDOFF + * | . . . + * PPP_PHASE_INITIALIZE + * | . . . + * PPP_PHASE_ESTABLISH + * | . . . + * |. . . + * | . . + * PPP_PHASE_AUTHENTICATE + * | . . + * || . . + * PPP_PHASE_NETWORK + * | || . . + * | ||| . + * PPP_PHASE_RUNNING + * | .||||| + * | . |||| + * PPP_PHASE_TERMINATE + * | . |||| + * PPP_PHASE_NETWORK + * |. . + * PPP_PHASE_ESTABLISH + * PPP_PHASE_DISCONNECT + * PPP_PHASE_DEAD + * + * Alright, PPP basic retransmission and LCP Echo consume one timer. + * 1 + * + * If authentication is enabled one timer is necessary during authentication. + * 1 + PPP_AUTH_SUPPORT + * + * If ECP is enabled one timer is necessary before IPCP and/or IP6CP, one more + * is necessary if CCP is enabled (only with MPPE support but we don't care much + * up to this detail level). + * 1 + ECP_SUPPORT + CCP_SUPPORT + * + * If CCP is enabled it might consume a timer during IPCP or IP6CP, thus + * we might use IPCP, IP6CP and CCP timers simultaneously. + * 1 + PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + CCP_SUPPORT + * + * When entering running phase, IPCP or IP6CP is still running. If idle time limit + * is enabled one more timer is necessary. Same for max connect time and max + * octets features. Furthermore CCP RACK might be used past this point. + * 1 + PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT -1 + PPP_IDLETIMELIMIT + PPP_MAXCONNECT + MAXOCTETS + CCP_SUPPORT + * + * IPv4 or IPv6 must be enabled, therefore we don't need to take care the authentication + * and the CCP + ECP case, thus reducing overall complexity. + * 1 + LWIP_MAX(PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + CCP_SUPPORT, PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT -1 + PPP_IDLETIMELIMIT + PPP_MAXCONNECT + MAXOCTETS + CCP_SUPPORT) + * + * We don't support PPP_IDLETIMELIMIT + PPP_MAXCONNECT + MAXOCTETS features + * and adding those defines to ppp_opts.h just for having the value always + * defined to 0 isn't worth it. + * 1 + LWIP_MAX(PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + CCP_SUPPORT, PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT -1 + CCP_SUPPORT) + * + * Thus, the following is enough for now. + * 1 + PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + CCP_SUPPORT + */ #endif /* PPP_SUPPORT */ #endif /* LWIP_HDR_PPP_IMPL_H */ diff --git a/src/include/netif/ppp/ppp_opts.h b/src/include/netif/ppp/ppp_opts.h index c987c110..6f1ff0ea 100644 --- a/src/include/netif/ppp/ppp_opts.h +++ b/src/include/netif/ppp/ppp_opts.h @@ -82,10 +82,11 @@ /** * PPP_NUM_TIMEOUTS_PER_PCB: the number of sys_timeouts running in parallel per - * ppp_pcb. This is a conservative default which needs to be checked... + * ppp_pcb. See the detailed explanation at the end of ppp_impl.h about simultaneous + * timers analysis. */ #ifndef PPP_NUM_TIMEOUTS_PER_PCB -#define PPP_NUM_TIMEOUTS_PER_PCB 6 +#define PPP_NUM_TIMEOUTS_PER_PCB (1 + PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + CCP_SUPPORT) #endif /* The number of sys_timeouts required for the PPP module */