Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 191938 - net-analyzer/net-snmp-5.4.1-r1 - ppp reconnect breaks MRTG
Summary: net-analyzer/net-snmp-5.4.1-r1 - ppp reconnect breaks MRTG
Status: RESOLVED UPSTREAM
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: x86 Linux
: High enhancement (vote)
Assignee: Gentoo Netmon project
URL: http://www.mail-archive.com/net-snmp-...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-10 04:02 UTC by Yuxans Yao
Modified: 2007-12-27 16:13 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yuxans Yao 2007-09-10 04:02:12 UTC
Internet is connected by DSL ppp(use "persist"). This connection disconnect sometimes and then reconnect automatically. 

After reconnection, thers is no data in MRTG.

To solve the problem I have to restart snmpd.

Find the patch: http://sourceforge.net/tracker/index.php?func=detail&aid=1513191&group_id=12694&atid=312694

Reproducible: Always

Steps to Reproduce:
1.start ppp
2.start snmpd
3.link diconnect,ppp auto reconnect.("ppp" use "persist")
4.MRTG data error.

Actual Results:  
http://sourceforge.net/tracker/index.php?func=detail&aid=1513191&group_id=12694&atid=312694

The patch available.

Expected Results:  
"USE flag" to use the patch.

My conf and ebuild diff

File: /etc/conf.d/net
config_ppp0=( "ppp" )
link_ppp0="eth0"
plugins_ppp0=( "pppoe" )
username_ppp0='******'
password_ppp0='******'
pppd_ppp0=(
    "noauth"
    "defaultroute"
    "default-asyncmap"
    "ipcp-accept-remote"
    "ipcp-accept-local"
    "lcp-echo-interval 15"
    "lcp-echo-failure 3"
    "persist"
    "maxfail 0"
    "holdoff 2"
    "mru 1492"
    "mtu 1492"
    "debug"
    "lock"
)

depend_ppp0()
{
    need net.eth0
    before ez-ipupdate
    before upnpd
    before snmpd
}


diff /usr/local/portage/net-analyzer/net-snmp/net-snmp-5.4.1-r1.ebuild /usr/portage/net-analyzer/net-snmp/net-snmp-5.4.1-r1.ebuild 
14c14
< IUSE="diskio doc dyndev elf ipv6 lm_sensors mfd-rewrites minimal perl python rpm selinux smux ssl tcpd X sendmail"
---
> IUSE="diskio doc elf ipv6 lm_sensors mfd-rewrites minimal perl python rpm selinux smux ssl tcpd X sendmail"
63,66d62
< 
<       if use dyndev; then
<               epatch "${FILESDIR}"/net-snmp-dynamic-devs.diff
<       fi
Comment 1 Raphael Marichez (Falco) (RETIRED) gentoo-dev 2007-09-14 23:24:56 UTC
Good idea, but this patch seems too wrong to go into the tree.


+void se_remove_value_from_slist(const char *listname, const char *label) 
+{ 
+   struct snmp_enum_list_str *sptr, *lastp = NULL; 
+   struct snmp_enum_list *list; 
+   if (!listname) 
+     return; 
+ 
+   for (sptr = sliststorage; 
+       sptr != NULL; lastp = sptr, sptr = sptr->next) 
+     if (sptr->name && strcmp(sptr->name, listname) == 0) 
+       se_remove_value_from_list(&sptr->list, label); 
+} 
+


Obviously "struct snmp_enum_list *list" is useless



+int se_remove_value_from_list(struct snmp_enum_list **list, const char *label) 
+{ 
+   struct snmp_enum_list *lastlist; 
+   if(!list) 
+     return SE_DNE; 
+ 
+   lastlist = NULL; 
+   while(*list) { 
+      if(strcmp((*list)->label, label) == 0) { 
+        free((*list)->label); 
+        if(lastlist) 
+          lastlist->next = (*list)->next; 
+        free(*list); 
+        *list = NULL; 
+        return 0; 
+      } 
+      lastlist = *list; 
+      (*list) = (*list)->next; 
+   } 
+
+} 

unless i'm wrong, it is really buggy.

1) it should not be "*list = NULL;" but "*list = (*list)->next;". Otherwise, you lose all the following elements

2) He is changing the life *list pointer. That's not good, since there is no backup of it. Instead of that, He has to copy *list into a local variable, and increments this local variable. He should never modify *list unless the first element of the chain is to be deleted.

I would suggest something more like the following.
{
  struct snmp_enum_list *lastlist; 
  struct snmp_enum_list *curlist; 
  struct snmp_enum_list *nextlist; 
  if (!list) 
        return SE_DNE; 
 
  lastlist=NULL; // previous pointer
  curlist=*list; // current pointer
  while(curlist) { 
      nextlist = curlist->next;  // next pointer
      if( strcmp(curlist->label, label) == 0) {
        free(curlist);· // be freed now since we've saved the next pointer
        if(lastlist)· 
          lastlist->next = nextlist; 
        else  
          *list = nextlist; // only case to change the pointer
      } else { 
        lastlist = curlist; // save
      } 
      curlist = nextlist;·  // go further
  } 
} 
Comment 2 Peter Volkov (RETIRED) gentoo-dev 2007-12-27 16:13:57 UTC
Thank you for report. The correct fix, as per upstream, is to make this solution "configurable". Enable/disable such feature at build time is not an option.

Generally the resolution of this bug should be taken upstream.