From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933912AbdC3NWf (ORCPT ); Thu, 30 Mar 2017 09:22:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54978 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933856AbdC3NWU (ORCPT ); Thu, 30 Mar 2017 09:22:20 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 81B522EF189 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jbenc@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 81B522EF189 Date: Thu, 30 Mar 2017 15:22:15 +0200 From: Jiri Benc To: Yi-Hung Wei Cc: joe@ovn.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net] openvswitch: Fix ovs_flow_key_update() Message-ID: <20170330152215.7b4f9703@griffin> In-Reply-To: <1490832850-6012-1-git-send-email-yihung.wei@gmail.com> References: <1490832850-6012-1-git-send-email-yihung.wei@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 30 Mar 2017 13:22:19 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 29 Mar 2017 17:14:10 -0700, Yi-Hung Wei wrote: > ovs_flow_key_update() is called when the flow key is invalid, and it is > used to update and revalidate the flow key. Commit 329f45bc4f19 > ("openvswitch: add mac_proto field to the flow key") introduces mac_proto > field to flow key and use it to determine whether the flow key is valid. > However, the commit does not update the code path in ovs_flow_key_update() > to revalidate the flow key which may cause BUG_ON() on execute_recirc(). > This patch addresses the aforementioned issue. Good catch. > int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key) > { > + int res; > + > + res = key_extract_mac_proto(skb); > + if (res < 0) > + return res; > + key->mac_proto = res; > + > return key_extract(skb, key); > } But this should just reset the SW_FLOW_KEY_INVALID flag, there's no need to recompute mac_proto. Something like this: int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key) { - return key_extract(skb, key); + int res; + + res = key_extract(skb, key); + if (!res) + key->mac_proto &= ~SW_FLOW_KEY_INVALID; + return res; } Thanks, Jiri