Hi,
I found an excessive amount of unaligned accesses on my AXP workstation
and tracked it down to ip_packet_match() in the ip_tables module.
indev and outdev are not properly aligned if set to nulldevname in
ipt_do_table().
This destroys the benefits of comparing names in units of (long) and
on architectures with expensive unaligned accesses (such as ia64 or
alpha), it even hurts a lot.
Find attached a patch against 2.6.0. A similar patch is needed for 2.4,
also attached.
Please consider merging them.
Looking at ip_packet_match(), I have two more thoughts:
* It should not be inlined. It's too large to benefit from inlining,
IMHO. (OTOH, it's only called from one place, so it does not
really matter.)
* There's a comment about the compiler being able to unroll the 2/4
(64/32bit) iter loop which is not completely appropriate: We don't
pass -funroll-loops, so gcc does not do it :-(
It would be beneficial though.
Regards,
--
Kurt Garloff <[email protected]> Cologne, DE
SUSE LINUX AG, Nuernberg, DE SUSE Labs (Head)
On Thu, Jan 08, 2004 at 12:00:11AM +0100, Kurt Garloff wrote:
> Hi,
>
> I found an excessive amount of unaligned accesses on my AXP workstation
> and tracked it down to ip_packet_match() in the ip_tables module.
> indev and outdev are not properly aligned if set to nulldevname in
> ipt_do_table().
> This destroys the benefits of comparing names in units of (long) and
> on architectures with expensive unaligned accesses (such as ia64 or
> alpha), it even hurts a lot.
Thanks for submitting your patch, this is the same bug report as
https://bugzilla.netfilter.org/cgi-bin/bugzilla/show_bug.cgi?id=84
> Find attached a patch against 2.6.0. A similar patch is needed for 2.4,
> also attached.
the fix is already in our patch-o-matic CVS tree, and I'm going to
submit this to davem together with some other fixes I have pending.
> Looking at ip_packet_match(), I have two more thoughts:
> * It should not be inlined. It's too large to benefit from inlining,
> IMHO. (OTOH, it's only called from one place, so it does not
> really matter.)
That is the idea. It's just split in two functions to make it more
readable. It's never intended to be called from somewhere else.
> * There's a comment about the compiler being able to unroll the 2/4
> (64/32bit) iter loop which is not completely appropriate: We don't
> pass -funroll-loops, so gcc does not do it :-(
> It would be beneficial though.
I'm not a compiler geek. Thanks for pointing this out, I will do some
testing and look at the compiler output to see what is happening.
> Regards,
> --
> Kurt Garloff <[email protected]> Cologne, DE
--
- Harald Welte <[email protected]> http://www.netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie