buglwIP - A Lightweight TCP/IP stack - Bugs: bug #51897, PPP Stats Support in LCP

 
 

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

bug #51897: PPP Stats Support in LCP

Submitter:  G Smith <gsmith>
Submitted:  Wed 30 Aug 2017 04:55:41 PM UTC
   
 
Category:  PPP Severity:  3 - Normal
Item Group:  Feature Request Status:  Wont Fix
Privacy:  Public Assigned to:  gradator
Open/Closed:  Closed Planned Release:  None
lwIP version:  2.0.2

Jump to the original submission

Wed 10 Jan 2018 09:31:34 PM UTC, comment #7: 

Thanks for resolving this.

All we could do is to clarify which of the options in ppp.h (not ppp_opts.h) are really unsupported (in contrast to "rarely used PPP feature") to prevent false positives like this in the future...

Simon Goldschmidt <goldsimon>
Group administrator
Wed 10 Jan 2018 08:58:28 PM UTC, comment #6: 

PPP_LCP_ADAPTIVE isn't supported. This isn't even supported from pppd upstream, I merged the patch from Debian package because it looked interesting at the time.

This isn't even advertised in PPP opt.h configuration header, all features that aren't in opt.h aren't supported. PPP_LCP_ADAPTIVE is no exception.

Anyway, one willing PPP_LCP_ADAPTIVE support is very welcomed to provide a patch using sys_timeout/sys_now lwIP functions instead of the ugly hack using stats support :-)  The hack is there because this is the only way for pppd to know that something happened (data packets don't reach the userland pppd process) in the Linux kernel, we don't need it since we don't have this barrier.

Sylvain

Sylvain Rochet <gradator>
Group Member
Mon 08 Jan 2018 08:40:18 PM UTC, comment #5: 

Sylvain, can you say anything about this?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 17 Nov 2017 08:52:06 PM UTC, comment #4: 

Sylvain, do you have any comments on this? To me it seems like:
- PPP_STATS_SUPPORT is broken or not supported (struct vjstat is redefined in 2 headers)
- PPP_LCP_ADAPTIVE seems to depend on PPP_STATS_SUPPORT

So summed up, PPP_LCP_ADAPTIVE does not seem to work? That should be documented, I guess (unless it's fixed, of course. I don't know how important this is).

Simon Goldschmidt <goldsimon>
Group administrator
Thu 31 Aug 2017 07:00:24 PM UTC, comment #3: 

Ehrm, it doesn't seem to me that 'link_stats.pkts_in' is ever increased, so this is probably not working anyway?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 31 Aug 2017 02:26:56 PM UTC, comment #2: 

That's true, don't know why I missed that.  However, the problem remains that link_stats isn't declared without PPP_STATS_SUPPORT.  So is there another variable that can be used to see if there was any traffic previously besides link_stats.pkts_in?

G Smith <gsmith>
Thu 31 Aug 2017 01:28:14 AM UTC, comment #1: 

That won't work because with your change last_pkts_in is always 0.

Axel Lin <axellin>
Group Member
Wed 30 Aug 2017 04:55:41 PM UTC, original submission:  

I have recently enabled the PPP_LCP_ADAPTIVE feature to test it out, as I'm having problems with one of the peers responding to echo requests.  Anyhow, in lcp.c, around line 2744, the compiler complains that 'link_stats' is undeclared.  I think this is because I don' t have PPP_STATS_SUPPORT enabled and the #endif is not encompassing the next few lines as it should.

Please see the original code and my fix below.  (I'm running Lwip 2.0.2.)


The offending code is this:
#if PPP_LCP_ADAPTIVE
    /*
     * If adaptive echos have been enabled, only send the echo request if
     * no traffic was received since the last one.
     */
    if (pcb->settings.lcp_echo_adaptive) {
static unsigned int last_pkts_in = 0;

#if PPP_STATS_SUPPORT
update_link_stats(f->unit);
link_stats_valid = 0;
#endif /* PPP_STATS_SUPPORT */

if (link_stats.pkts_in != last_pkts_in) {
    last_pkts_in = link_stats.pkts_in;
    return;
}
    }
#endif


My fix is to move the PPP_STATS_SUPPORT #endif down 5 lines:
#if PPP_LCP_ADAPTIVE
    /*
     * If adaptive echos have been enabled, only send the echo request if
     * no traffic was received since the last one.
     */
    if (pcb->settings.lcp_echo_adaptive) {
static unsigned int last_pkts_in = 0;

#if PPP_STATS_SUPPORT
update_link_stats(f->unit);
link_stats_valid = 0;

if (link_stats.pkts_in != last_pkts_in) {
    last_pkts_in = link_stats.pkts_in;
    return;
}
#endif /* PPP_STATS_SUPPORT */
    }
#endif

G Smith <gsmith>

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gradator (Posted a comment)
  • -email is unavailable- added by dziegel (Updated the item)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by axellin (Posted a comment)
  • -email is unavailable- added by gsmith (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-01-10 gradator Item GroupCompiler Warning Feature Request
        StatusNone Wont Fix
        Open/ClosedOpen Closed
    2017-11-05 dziegel Assigned toNone gradator

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code