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

Submitted by:  G Smith <gsmith>
Submitted on:  Wed 30 Aug 2017 04:55:41 PM UTC  
 
Category: PPPSeverity: 3 - Normal
Item Group: Feature RequestStatus: Wont Fix
Privacy: PublicAssigned to: Sylvain Rochet <gradator>
Open/Closed: ClosedPlanned Release: None
lwIP version: 2.0.2

(Jump to the original submission 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>
Project 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>
Project MemberIn charge of this item.
Mon 08 Jan 2018 08:40:18 PM UTC, comment #5:

Sylvain, can you say anything about this?

Simon Goldschmidt <goldsimon>
Project 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>
Project 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>
Project 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>
Project 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>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by gradator (Posted a comment)
  • -unavailable- added by dziegel (Updated the item)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by axellin (Posted a comment)
  • -unavailable- added by gsmith (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 10 Jan 2018 08:58:28 PM UTCgradatorItem GroupCompiler Warning=>Feature Request
      StatusNone=>Wont Fix
      Open/ClosedOpen=>Closed
    Sun 05 Nov 2017 10:54:55 AM UTCdziegelAssigned toNone=>gradator

    Back to the top


    Powered by Savane 3.1-cleanup1