Flow classifier proto-dst and TOS (and proto-src)

[Dan Siemon <dan@coverfire.com>]

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

cls_flow.c: flow_get_proto_dst()

The proto-dst key returns the destination port for UDP, TCP and a few
other protocols [see proto_ports_offset()]. For ICMP and IPIP it falls
back to:

return addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;

Since Linux maintains a dst_entry for each TOS value this causes the
returned value to be affected by the TOS which is unexpected and
probably broken.

Is there a reason why this doesn't return 0 for protocols that don't
have a notion of source and destination ports? It seems very odd to me
that a value which is not at all related to the traffic on the wire is
returned for this key.

There is a somewhat similar situation with flow_get_proto_src(). Here
the fallback value is:

return addr_fold(skb->sk);

It looks like this is 0 when the traffic doesn't originate locally and
even for local traffic I don't understand why the use of a effectively
random number here is useful.

For a long winded explanation of how I discovered this see:
http://www.coverfire.com/archives/2011/10/15/linux-flow-classifier-proto-dst-and-tos/

Below is a simple patch which makes these functions fallback to
returning 0 when the protocol doesn't have the notion of ports.

Signed-off-by: Dan Siemon <dan@coverfire.com>
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 6994214..7527e61 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -150,7 +150,7 @@ static u32 flow_get_proto_src(struct sk_buff *skb)
 	}
 	}
 
-	return addr_fold(skb->sk);
+	return 0;
 }
 
 static u32 flow_get_proto_dst(struct sk_buff *skb)
@@ -192,7 +192,7 @@ static u32 flow_get_proto_dst(struct sk_buff *skb)
 	}
 	}
 
-	return addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;
+	return 0;
 }
 
static u32 flow_get_iif(const struct sk_buff *skb)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]