[ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[Tad]

If an SNMP session is closed or a trapsink is removed (which closes a 
session), the main select loop of snmpd will fail and re-init SNMP.

The cause appears to be outstanding select which returne EBADF = bad 
file descriptor(pointer?) for the socket that was closed out from under it.

Didn't know where this should be posted for the net-snmp guys, esp, 
cause I don't know if it affects their tree.  They don't have this fix 
as below in their latest.  Any suggestions on this question?

The simple fix appears to be to simply continue as if nothing happened, 
but it's possible that snmp_select_info() never should have added the 
closing socket to fdset in the first place.  It was getting very hard 
for me to determine whether the select was already outstanding when the 
socket closed or whether someone didn't set a proper "closing" flag that 
snmp_select_info() would recognize.  I tried updating  the 
snmp_select_info() to rev 1.245 of the 4.2 net-snmp tree which looks at 
closing sessions, but that didn't seem to help.

Here's the quick fix.  Most people probably don't close trapsinks too 
often and perhaps didn't even notice snmp re-init when they did.

snmpd.c : receive()
...
    count = select(numfds, &fdset, 0, 0, tvp);

    if (count > 0){
        snmp_read(&fdset);
    } else switch(count){
        case 0:
                snmp_timeout();
                break;
        case -1:
-        if (errno == EINTR){
+        if (errno == EINTR || errno == EBADF){
            continue;
        } else {
                    snmp_log_perror("select");
        }


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[Andrew Lunn]

> Here's the quick fix.  Most people probably don't close trapsinks too 
> often and perhaps didn't even notice snmp re-init when they did.
> 
> snmpd.c : receive()
> ...
>    count = select(numfds, &fdset, 0, 0, tvp);
> 
>    if (count > 0){
>        snmp_read(&fdset);
>    } else switch(count){
>        case 0:
>                snmp_timeout();
>                break;
>        case -1:
> -        if (errno == EINTR){
> +        if (errno == EINTR || errno == EBADF){
>            continue;
>        } else {
>                    snmp_log_perror("select");
>        }
> 

Hi Tod

Please send a real patch. Take a look at the other patches sent to the
ecos-discuss list. Edit the ChangeLog file to show what you changed
and why. Then do cvs diff -u packacges/net/snmp and send the patch it
generates. We don't accept fixes in any other format.

           Thanks
                Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[Tad]

I guess I'm kinda confused.  I thought that perfect patches went to 
ecos-patches and proposed fixes went to ecos-discuss.  Was trying to 
follow the rules proposed last time I posted.
- Tod
+ Tad

Andrew Lunn wrote:
>> Here's the quick fix.  Most people probably don't close trapsinks too 
>> often and perhaps didn't even notice snmp re-init when they did.
>>
>> snmpd.c : receive()
>> ...
>>    count = select(numfds, &fdset, 0, 0, tvp);
>>
>>    if (count > 0){
>>        snmp_read(&fdset);
>>    } else switch(count){
>>        case 0:
>>                snmp_timeout();
>>                break;
>>        case -1:
>> -        if (errno == EINTR){
>> +        if (errno == EINTR || errno == EBADF){
>>            continue;
>>        } else {
>>                    snmp_log_perror("select");
>>        }
>>
>>     
>
> Hi Tod
>
> Please send a real patch. Take a look at the other patches sent to the
> ecos-discuss list. Edit the ChangeLog file to show what you changed
> and why. Then do cvs diff -u packacges/net/snmp and send the patch it
> generates. We don't accept fixes in any other format.
>
>            Thanks
>                 Andrew
>
>
>   

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[Andrew Lunn]

On Fri, Jun 15, 2007 at 12:11:58PM -0800, Tad wrote:
> I guess I'm kinda confused.  I thought that perfect patches went to 
> ecos-patches and proposed fixes went to ecos-discuss.  Was trying to 
> follow the rules proposed last time I posted.

So this is not a perfect fix? What is wrong with this fix? What would
you like to discuss about this fix? All you did was provide a fix, but
gave us no clues as to why you have not made a proper patch.....

     Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] SNMP re-inits when session closed or trapsink removed -- FIX

[Tad]

Andrew Lunn wrote:
> So this is not a perfect fix? What is wrong with this fix? What would
> you like to discuss about this fix? All you did was provide a fix, but
> gave us no clues as to why you have not made a proper patch.....

I guess I thought that my original comment suggested there might be a 
more appropriate fix.

> The simple fix appears to be to simply continue as if nothing 
> happened, but it's possible that snmp_select_info() never should have 
> added the closing socket to fdset in the first place. It was getting 
> very hard for me to determine whether the select was already 
> outstanding when the socket closed or whether someone didn't set a 
> proper "closing" flag that snmp_select_info() would recognize. I tried 
> updating the snmp_select_info() to rev 1.245 of the 4.2 net-snmp tree 
> which looks at closing sessions, but that didn't seem to help.


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss