diff mbox

[net-next,v2,1/3] openvswitch: normalize vlan rx path

Message ID 20161005192132.GR25403@egarver
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Garver Oct. 5, 2016, 7:21 p.m. UTC
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?

Comments

Jiri Benc Oct. 5, 2016, 9:07 p.m. UTC | #1
On Wed, 5 Oct 2016 15:21:32 -0400, Eric Garver wrote:
> How about this incremental change?

Feel free to submit it as a standalone patch. It has nothing to do with
this patchset. In this particular regard, the code is identical before
and after the patchset and is in no way altered by this patch.

 Jiri
diff mbox

Patch

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)) {