Re: [Openvpn-devel] [Openvpn-users] [TESTING NEEDED] Using --inactive and --ping seems to defeat each other

[Gert D. <ge...@gr...>]

Hi,

On Fri, Apr 30, 2010 at 06:24:20PM +0100, Davide Brini wrote:
> Well, the obvious (and probably wrong, probably inefficient) way could be to 
> just check if the packet at hand is a ping message, and if it is do not record 
> it as "activity", eg
> 
> if (! is_ping_msg (&c->c2.buf))
>    register_activity (c, size);

Looks good to me :-) (without having looked at the code in question at all).

> This is in two places: in process_outgoing_tun() and process_outgoing_link(). 

As you wrote in the next e-mail, this won't ever be "outgoing on tun",
so only one place needs changing...

> However, I'm not sure about the overhead of performing that for each and every 
> processed packet. is_ping_msg() calls buf_string_match(), which in turn calls 
> memcmp().
> But, it should also be noted that process_incoming_link() DOES call 
> is_ping_msg() for every incoming packet, so... 

I'm a big fan of symmetry here - if the incoming packet path explictly
not-counts ping packets, it's a bit hard to explain to users why the
outgoing packet path *does* count them, and they have to fiddle with
byte counts to get "inactivity".

I'd go that route... 

Efficiency is a valid concern, of course.  Since is_ping_msg() and
buf_string_match() are both (very short) inline functions, and memcmp() 
on "normal" architectures is also inlined by gcc, this "should not" add 
unduly overhead.  (Of course, down that road is hell and windows vista).

gert
-- 
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             ge...@gr...
fax: +49-89-35655025                        ge...@ne...