From patchwork Sat Oct 15 16:51:02 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Siemon X-Patchwork-Id: 119987 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9BF6DB6F68 for ; Sun, 16 Oct 2011 04:01:55 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753533Ab1JORBu (ORCPT ); Sat, 15 Oct 2011 13:01:50 -0400 Received: from alpha.coverfire.com ([69.41.199.58]:44295 "EHLO alpha.coverfire.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752929Ab1JORBu (ORCPT ); Sat, 15 Oct 2011 13:01:50 -0400 X-Greylist: delayed 646 seconds by postgrey-1.27 at vger.kernel.org; Sat, 15 Oct 2011 13:01:50 EDT Received: from [192.168.88.98] (ganymede [192.168.88.98]) (authenticated bits=0) by alpha.coverfire.com (8.14.4/8.14.4) with ESMTP id p9FGp3Jp023889 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Sat, 15 Oct 2011 12:51:03 -0400 Subject: Flow classifier proto-dst and TOS (and proto-src) From: Dan Siemon To: netdev Date: Sat, 15 Oct 2011 12:51:02 -0400 X-Mailer: Evolution 3.0.3 (3.0.3-1.fc15) Message-ID: <1318697463.7169.21.camel@ganymede> Mime-Version: 1.0 X-Spam-Status: No, score=-0.4 required=5.0 tests=ALL_TRUSTED,BAYES_00, DATE_IN_FUTURE_96_Q autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on alpha.coverfire.com Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 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)