testing checksum in helpers is redundant with tcp_window_tracking

testing checksum in helpers is redundant with tcp_window_tracking

[netfilter]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Would it not be a good idea to have some kind of macro defined when
tcp_window_tracking in built into netfilter so that all of the helpers
and other code that are doing things like TCP checksum verification
and whatnot, can avoid wasting cycles doing it a second time?

Then helpers could wrap their tcp checksum calls with something like:

#ifndef TCP_WINDOW_TRACKING
...
#endif

Or is a runtime check a better idea?  If so what test could be done at
runtime to know that tcp_window_tracking is checking TCP checksums?

b.

-- 
Brian J. Murrell

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: testing checksum in helpers is redundant with tcp_window_tracking

[Patrick Schaaf]

> Then helpers could wrap their tcp checksum calls with something like:
> 
> #ifndef TCP_WINDOW_TRACKING
> ...
> #endif

Please don't do it that way. If you have to write the same ifdef several
times in several .c files, you are doing something wrong.

The good way is this:

- make all places call a single "check sum in conntrack helper" function.
- define that function as a 'static inline' in one of the existing .h files,
  where it logically fits. Make that definition dependant on the
  WINDOW_TRACKING define, providing a completely empty 'static inline'
  function when the window tracking compile option is set.

Done this way, the call sites are not uglified by ifdefs, and it's still
a compile-time go-away-code situation. This scheme is used in lots of
places in the kernel.

What happens with incremental modular compiles, and the user
flipping the WINDOW_TRACKING switch? i.e. some modules compiled one
way, and some compiled the other way?

For a possible other approach: isn't there some flag in the skbuff,
normally used by the local TCP stack, which can remember that a checksum
was done, and it was ok? I remember such things being introduced when
they started supporting hardware checksums, but I'm ignorant of any
detail. Could it be that the conntrack/helpers might just also use
that infrastructure?

best regards
  Patrick

Re: testing checksum in helpers is redundant with tcp_window_tracking

[Jozsef Kadlecsik]

On Thu, 27 Feb 2003 netfilter@interlinx.bc.ca wrote:

> Would it not be a good idea to have some kind of macro defined when
> tcp_window_tracking in built into netfilter so that all of the helpers
> and other code that are doing things like TCP checksum verification
> and whatnot, can avoid wasting cycles doing it a second time?
>
> Then helpers could wrap their tcp checksum calls with something like:
>
> #ifndef TCP_WINDOW_TRACKING
> ...
> #endif

No, I think that is a bad idea. The tcp-window-tracking patch patches the
default helpers (ftp, irc) and remove the redundant checkings. However the
current status of the patch is 'extra'. Not even 'base'. No other
subsystem/patch should be made dependent of it.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: testing checksum in helpers is redundant with tcp_window_tracking

[netfilter]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Fri, Feb 28, 2003 at 11:33:15AM +0100, Jozsef Kadlecsik wrote:
> 
> No, I think that is a bad idea. The tcp-window-tracking patch patches the
> default helpers (ftp, irc) and remove the redundant checkings.

Right.  I have noticed this.  But based on that idea, it should patch
all of the helpers, but that becomes a maintenance nightmare.

> However the
> current status of the patch is 'extra'.

Right.  I understand that.

> No other
> subsystem/patch should be made dependent of it.

If it were a requirement type of dependancy, I would completely agree
with you, but I am talking about an optional "take advantage of it if
it's there and business as usual if it's not" type dependency.

Adding a macro to tcp-window-tracking (simply signifying it's in the
kernel) and wrapping the tcp checks in helpers with a test for the
macro does not introduce any loss of functionality at all.  A module
writer doesn't even have to use it if he doesn't want to, but if he
wants his module to be as efficient as possible in the presence of
tcp-window-tracking, he can use the macro.

b.

-- 
Brian J. Murrell

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]