patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9367, init: Correct minimal...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9367: init: Correct minimal MEMP_NUM_SYS_TIMEOUT setting check

Submitter:  Axel Lin <axellin>
Submitted:  Wed 31 May 2017 08:10:25 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Thu 27 Jul 2017 06:53:30 PM UTC, comment #5: 

Pushed & added PPP_NUM_TIMEOUTS_PER_PCB to ppp_opts.h. This is set to 6 for now but overridable. I could be a constant once we know the exact number. I don't know where the 6 in opt.h came from though...

Simon Goldschmidt <goldsimon>
Group administrator
Tue 13 Jun 2017 05:38:13 PM UTC, comment #4: 

I first have to figure out how much timers are actually used simultaneously for PPP, for example IPCP and IP6CP are only running once Authentication phase is passed so auth timers and IP timers can't run concurrently. Furthermore once low level protocol is established (Ethernet or L2TP) no timers are running for them too, and this is before authentication phase. Thus, at first first, link level, authentication, and establish timers can be shared.

Sylvain

Sylvain Rochet <gradator>
Group Member
Wed 31 May 2017 03:15:08 PM UTC, comment #3: 

Hi Sylvain,

Thanks for your comment.

In my case, I set MEMP_NUM_SYS_TIMEOUT to a too small value but it was not cached by #error.
(I have AUTH + PAP|CHAP|EAP + LCP enabled)
I didn't figure out the problem until today I hit the assertion:
LWIP_ASSERT("sys_timeout: timeout != NULL, pool MEMP_SYS_TIMEOUT is empty", timeout != NULL);

That is why I think the minimal MEMP_NUM_SYS_TIMEOUT setting check needs fix.

I'm wondering if it make sense to define LWIP_NUM_SYS_TIMEOUT_INTERNAL as below:

#define LWIP_NUM_SYS_TIMEOUT_INTERNAL   (LWIP_TCP + IP_REASSEMBLY + LWIP_ARP + (2 * LWIP_DHCP) + LWIP_AUTOIP + LWIP_IGMP + LWIP_DNS + \
                                         (PPP_SUPPORT (1 + (PPP_AUTH_SUPPORT 2) + PPP_IPV4_SUPPORT + PPP_IPV6_SUPPORT + PPPOE_SUPPORT) * MEMP_NUM_PPP_PCB) + \
                                         (LWIP_IPV6 ? (1 + LWIP_IPV6_REASS + LWIP_IPV6_MLD) : 0))

Axel Lin <axellin>
Group Member
Wed 31 May 2017 09:35:29 AM UTC, comment #2: 

I have mixed feelings about #error-ing about that, what about PPP users which don't use authentication, nor IP6CP, nor PPPo(E|L2TP) ? They basically only need one timeout for LCP.

This could be acceptable if we condition the timeout number value on which PPP features are actually enabled.

Sylvain

Sylvain Rochet <gradator>
Group Member
Wed 31 May 2017 08:13:15 AM UTC, comment #1: 

v2: Fix a trivial typo in commit message. s/MEMP_NUM_SYS_TIMEOUT/LWIP_NUM_SYS_TIMEOUT_INTERNAL/

(file #40821)

Axel Lin <axellin>
Group Member
Wed 31 May 2017 08:10:25 AM UTC, original submission:  

According to commit 1f780e86d504 ("PPP timeouts required depend on the number of allowed PPP sessions"):
Per PPP needs 6 timeouts (AUTH + PAP|CHAP|EAP + LCP + IPCP + IP6CP + PPPoE).

So update the minimal MEMP_NUM_SYS_TIMEOUT setting check accordingly.
Since we have MEMP_NUM_SYS_TIMEOUT so just switch to use it.

Axel Lin <axellin>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by gradator (Posted a comment)
  • -email is unavailable- added by axellin (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-27 goldsimon StatusNone Done
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2017-05-31 axellin Attached File- Added 0001-init-Correct-minimal-MEMP_NUM_SYS_TIMEOUT-setting-ch-v2.patch, #40821
    2017-05-31 axellin Attached File- Added 0001-init-Correct-minimal-MEMP_NUM_SYS_TIMEOUT-setting-ch.patch, #40820

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code