payload: refine payload expr merging

nf_tables can handle payload exprs for sizes <= sizeof(u32) via a direct
operation from the eval loop, rather than a a call to the payload
expression.  Two loads for four byte quantities are thus faster than a
single load for an 8 byte load.

ip saddr 1.2.3.4 ip daddr 2.3.4.5

is faster with this applied, even though it involves two payload and two
two compare expressions, just because all can be handled from the main
loop without any calls to expression ops.

Keep merging for linklayer and when at least one of the expressions
already exceeded the 4 byte "limit" anyway.

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/payload.c b/src/payload.c
index fab97b118dca8b5c2180dc59a334148afc8dc3dd..f638b0c0848c41a7972f5d6d4f90459f7e8054df 100644 (file)
--- a/src/payload.c
+++ b/src/payload.c
@@ -721,7 +721,33 @@ bool payload_can_merge(const struct expr *e1, const struct expr *e2)
        if (total < e1->len || total > (NFT_REG_SIZE * BITS_PER_BYTE))
                return false;
 
-       return true;
+       /* could return true after this, the expressions are mergeable.
+        *
+        * However, there are some caveats.
+        *
+        * Loading anything <= sizeof(u32) with base >= network header
+        * is fast, because its handled directly from eval loop in the
+        * kernel.
+        *
+        * We thus restrict merging a bit more.
+        */
+
+       /* can still be handled by fastpath after merge */
+       if (total <= NFT_REG32_SIZE * BITS_PER_BYTE)
+               return true;
+
+       /* Linklayer base is not handled in fastpath, merge */
+       if (e1->payload.base == PROTO_BASE_LL_HDR)
+               return true;
+
+       /* Also merge if at least one expression is already
+        * above REG32 size, in this case merging is faster.
+        */
+       if (e1->len > (NFT_REG32_SIZE * BITS_PER_BYTE) ||
+           e2->len > (NFT_REG32_SIZE * BITS_PER_BYTE))
+               return true;
+
+       return false;
 }
 
 /**

diff --git a/tests/shell/testcases/nft-f/dumps/0012different_defines_0.nft b/tests/shell/testcases/nft-f/dumps/0012different_defines_0.nft
index c67d25b64172bf90165e0e49724f6b71d518032a..7abced86860140afb71a4e10129eda16856b534f 100644 (file)
--- a/tests/shell/testcases/nft-f/dumps/0012different_defines_0.nft
+++ b/tests/shell/testcases/nft-f/dumps/0012different_defines_0.nft
@@ -4,7 +4,7 @@ table inet t {
                iifname { "whatever" } iif { "lo" } meta mark 0x0000007b
                ct state established,related,new
                ct state != established | related | new
-               ip saddr 10.0.0.0 ip saddr 10.0.0.0 ip daddr 10.0.0.2
+               ip saddr 10.0.0.0 ip daddr 10.0.0.2 ip saddr 10.0.0.0
                ip6 daddr fe0::1 ip6 saddr fe0::2
                ip saddr vmap { 10.0.0.0 : drop, 10.0.0.2 : accept }
                ip6 daddr vmap { fe0::1 : drop, fe0::2 : accept }