From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755235AbcCRRPB (ORCPT ); Fri, 18 Mar 2016 13:15:01 -0400 Received: from slow1-d.mail.gandi.net ([217.70.178.86]:37062 "EHLO slow1-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752301AbcCRRO6 convert rfc822-to-8bit (ORCPT ); Fri, 18 Mar 2016 13:14:58 -0400 X-Originating-IP: 208.91.1.34 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: [patch] openvswitch: using a bit shift as a mask From: Jarno Rajahalme In-Reply-To: <20160317104136.GC3100@mwanda> Date: Fri, 18 Mar 2016 10:05:54 -0700 Cc: Pravin Shelar , Pablo Neira Ayuso , dev@openvswitch.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Content-Transfer-Encoding: 8BIT Message-Id: References: <20160317104136.GC3100@mwanda> To: Dan Carpenter X-Mailer: Apple Mail (2.2104) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Mar 17, 2016, at 3:41 AM, Dan Carpenter wrote: > > The original condition is never true. We want to test if BIT(0) is set > but the code is ANDing with zero. > > Fixes: 05752523e565 ('openvswitch: Interface with NAT.') > Signed-off-by: Dan Carpenter > > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index dc5eb29..29c82d6 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -668,7 +668,7 @@ static int ovs_ct_nat(struct net *net, struct sw_flow_key *key, > */ > if (info->nat & OVS_CT_NAT && ctinfo != IP_CT_NEW && > ct->status & IPS_NAT_MASK && > - (!(ct->status & IPS_EXPECTED_BIT) || info->commit)) { > + (!(ct->status & IPS_EXPECTED) || info->commit)) { > /* NAT an established or related connection like before. */ > if (CTINFO2DIR(ctinfo) == IP_CT_DIR_REPLY) > /* This is the REPLY direction for a connection Thanks for spotting this! Maybe it would be even better to use the test_bit() function, like this: > + (!test_bit(IPS_EXPECTED_BIT, &ct->status) || info->commit)) { Jarno From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarno Rajahalme Date: Fri, 18 Mar 2016 17:05:54 +0000 Subject: Re: [patch] openvswitch: using a bit shift as a mask Message-Id: List-Id: References: <20160317104136.GC3100@mwanda> In-Reply-To: <20160317104136.GC3100@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: Pravin Shelar , Pablo Neira Ayuso , dev@openvswitch.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org > On Mar 17, 2016, at 3:41 AM, Dan Carpenter wrote: > > The original condition is never true. We want to test if BIT(0) is set > but the code is ANDing with zero. > > Fixes: 05752523e565 ('openvswitch: Interface with NAT.') > Signed-off-by: Dan Carpenter > > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index dc5eb29..29c82d6 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -668,7 +668,7 @@ static int ovs_ct_nat(struct net *net, struct sw_flow_key *key, > */ > if (info->nat & OVS_CT_NAT && ctinfo != IP_CT_NEW && > ct->status & IPS_NAT_MASK && > - (!(ct->status & IPS_EXPECTED_BIT) || info->commit)) { > + (!(ct->status & IPS_EXPECTED) || info->commit)) { > /* NAT an established or related connection like before. */ > if (CTINFO2DIR(ctinfo) = IP_CT_DIR_REPLY) > /* This is the REPLY direction for a connection Thanks for spotting this! Maybe it would be even better to use the test_bit() function, like this: > + (!test_bit(IPS_EXPECTED_BIT, &ct->status) || info->commit)) { Jarno