On Wed, Sep 18, 2013 at 5:07 PM, Simon Horman [off-list ref] wrote:

On Tue, Sep 17, 2013 at 11:38:18AM -0700, Pravin Shelar wrote:

quoted

On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman [off-list ref] wrote:

quoted

diff --git a/datapath/datapath.h b/datapath/datapath.h
index 5d50dd4..babae3b 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h

@@ -36,6 +36,10 @@

 #define SAMPLE_ACTION_DEPTH 3

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
+#define HAVE_INNER_PROTOCOL
+#endif
+
 /**
  * struct dp_stats_percpu - per-cpu packet processing statistics for a given
  * datapath.

@@ -93,11 +97,16 @@ struct datapath {
  * @pkt_key: The flow information extracted from the packet.  Must be nonnull.
  * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
  * packet is not being tunneled.
+ * @inner_protocol: Provides a substitute for the skb->inner_protocol field on
+ * kernels before 3.11.
  */
 struct ovs_skb_cb {
        struct sw_flow          *flow;
        struct sw_flow_key      *pkt_key;
        struct ovs_key_ipv4_tunnel  *tun_key;
+#ifndef HAVE_INNER_PROTOCOL
+       __be16                  inner_protocol;
+#endif
 };
 #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)

Can you move this to compat struct ovs_gso_cb {}

I think that you are correct and inner_protocol needs
to be in struct ovs_gso_cb so that it can
be accessed via skb_network_protocol() from
rpl___skb_gso_segment().

However I think it may also need to be present in struct ovs_cb
so that it can be set correctly.

Currently it is set unconditionally
in ovs_execute_actions() and Jesse has suggested setting
it conditionally in pop_mpls() (which is called by do_execute_actions()).
But regardless it seems to me that the field would need to be available
in struct ovs_cb.

Since the helper functions are also in the compat code, I think they
should have access to ovs_gso_cb.

quoted

I think we can simplify code by pushing vlan and then segmenting skb,
the way we do it for MPLS.

Are you thinking of something like the following which applies
prior to the MPLS code.

This is basically what I was thinking about. We might actually be able
to move all of this to compat code by having a replacement for
dev_queue_xmit() similar to what we have for ip_local_out() in the
tunnel code.