Re: [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support

On Tue, Aug 09, 2016 at 08:47:40AM -0700, pravin shelar wrote:

On Mon, Aug 8, 2016 at 8:17 AM, Simon Horman [off-list ref] wrote:

...

quoted

Hi Pravin,

I have made an attempt to implement your suggestion to the extent that
I understand it. The following is an incremental change on top
of this patch-set. Does it move things closer to what you have in mind?

Following approach looks good to me. I have posted couple of comments.

Thanks, I am rather glad to hear that.

quoted

Light testing seems to indicate that it works for GSO skbs
received over both L3 and L2 GRE tunnels by OvS with both
IP-in-MPLS and IP (without MPLS) payloads.

Thanks for testing it. Can you also add those tests to OVS kmod test suite?
..

Sure, I will look into doing that.
Am I correct in thinking Joe Stringer is the best person to contact if
I run into trouble there?

quoted

diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
index 2055e57ed1c3..113cba89653d 100644
--- a/net/mpls/mpls_gso.c
+++ b/net/mpls/mpls_gso.c

@@ -39,16 +39,18 @@ static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
        mpls_features = skb->dev->mpls_features & features;
        segs = skb_mac_gso_segment(skb, mpls_features);

-
-       /* Restore outer protocol. */
-       skb->protocol = mpls_protocol;
-
        /* Re-pull the mac header that the call to skb_mac_gso_segment()
         * above pulled.  It will be re-pushed after returning
         * skb_mac_gso_segment(), an indirect caller of this function.
         */
        __skb_pull(skb, skb->data - skb_mac_header(skb));

+       /* Restore outer protocol. */
+       skb->protocol = mpls_protocol;
+       if (!IS_ERR(segs))
+               for (skb = segs; skb; skb = skb->next)
+                       skb->protocol = mpls_protocol;
+

skb_mac_gso_segment() can also return NULL. Therefore segs should be
checked for NULL case.

Sure, I will fix that.
I think that can be trivially resolved using IS_ERR_OR_NULL()

quoted

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 0001f651c934..424164222f1e 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c

...

quoted

@@ -308,8 +319,8 @@ static int pop_eth(struct sk_buff *skb, struct sw_flow_key *key)
 {
        skb_pull_rcsum(skb, ETH_HLEN);
        skb_reset_mac_header(skb);
-       skb->mac_len -= ETH_HLEN;

I am not sure why this line is removed.

I will restore it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev