From patchwork Wed Oct 5 19:21:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Garver X-Patchwork-Id: 678568 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 3sq5Hl3Wzbz9sD5 for ; Thu, 6 Oct 2016 06:21:39 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754323AbcJETVf (ORCPT ); Wed, 5 Oct 2016 15:21:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39438 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753340AbcJETVe (ORCPT ); Wed, 5 Oct 2016 15:21:34 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D488C9E632; Wed, 5 Oct 2016 19:21:33 +0000 (UTC) Received: from localhost (vpn-56-114.rdu2.redhat.com [10.10.56.114]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u95JLW8u029716; Wed, 5 Oct 2016 15:21:33 -0400 Date: Wed, 5 Oct 2016 15:21:32 -0400 From: Eric Garver To: Jiri Benc Cc: Eyal Birger , "netdev@vger.kernel.org" , pravin shelar Subject: Re: [PATCH net-next v2 1/3] openvswitch: normalize vlan rx path Message-ID: <20161005192132.GR25403@egarver> Mail-Followup-To: Jiri Benc , Eyal Birger , "netdev@vger.kernel.org" , pravin shelar References: <20161005192319.713d92e1@griffin> <20161005184426.GQ25403@egarver> <20161005210709.79732b27@griffin> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161005210709.79732b27@griffin> User-Agent: Mutt/1.7.0 (2016-08-17) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 05 Oct 2016 19:21:33 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Oct 05, 2016 at 09:07:09PM +0200, Jiri Benc wrote: > On Wed, 5 Oct 2016 14:44:26 -0400, Eric Garver wrote: > > On Wed, Oct 05, 2016 at 08:31:52PM +0300, Eyal Birger wrote: > > > Just seemed less future safe to keep a pointer to an old packet lying around. > > > > I agree. Alternatively refresh the eth pointer. > > Sorry guys, that just doesn't make sense. Everyone should know that > reloading of skb pointer means the former pointers to its data may > become invalid. Please point me to any place in the kernel where we > reload the data pointer "just because" even when not used. > How about this incremental change? diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 7ef02752d4ba..0dd36f353c53 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -562,7 +562,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) struct sw_flow *flow; struct sw_flow_actions *sf_acts; struct datapath *dp; - struct ethhdr *eth; struct vport *input_vport; u16 mru = 0; int len; @@ -584,14 +583,12 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) nla_memcpy(__skb_put(packet, len), a[OVS_PACKET_ATTR_PACKET], len); skb_reset_mac_header(packet); - eth = eth_hdr(packet); /* Normally, setting the skb 'protocol' field would be handled by a * call to eth_type_trans(), but it assumes there's a sending * device, which we may not have. */ - if (eth_proto_is_802_3(eth->h_proto)) - packet->protocol = eth->h_proto; - else + packet->protocol = eth_hdr(packet)->h_proto; + if (!eth_proto_is_802_3(packet->protocol)) packet->protocol = htons(ETH_P_802_2); if (eth_type_vlan(packet->protocol)) {