Re: [ovs-dev] [PATCH ovs V3 00/25] Introducing HW offload support for openvswitch

2017-02-12 Thread Roi Dayan



On 08/02/2017 17:29, Roi Dayan wrote:

This patch series introduces rule offload functionality to dpif-netlink
via netdev ports new flow offloading API. The user can specify whether to
enable rule offloading or not via OVS configuration. Netdev providers
are able to implement netdev flow offload API in order to offload rules.

This patch series also implements one offload scheme for netdev-linux,
using TC flower classifier, which was chosen because its sort of natural
to state OVS DP rules for this classifier. However, the code can be
extended to support other classifiers such as U32, eBPF, etc which support
offload as well.

The use-case we are currently addressing is the newly sriov switchdev mode
in the Linux kernel which was introduced in version 4.8 [1][2].
This series was tested against sriov vfs vports representors of the
Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.


V2->V3:
- Code styling fixes
- Bug fixes
- Using already available macros/functions to match current OVS code
- Refactored code according to V2 review
- Replaced bool option skip-hw for string option tc-policy
- Added hw offload tests using policy skip_hw
- Fixed most compatability compiling issues
- Travis
https://travis-ci.org/roidayan/ovs/builds/199610124
- AppVeyor
https://ci.appveyor.com/project/roidayan/ovs/build/1.0.14
- Fixed compiling with DPDK enabled

TODO:
- need to fix datapath compiling issues found in travis after adding tc
  compatability headers
- need to fix failing test cases because of get_ifindex


V1->V2:
- Added generic netdev flow offloads API.
- Implemented relevant flow API in netdev-linux (and netdev-vport).
- Added a other_config hw-offload option to enable offloading (defaults to 
false).
- Fixed coding style to conform with OVS.
- Policy removed for now. (Will be discussed how best implemented later).


Thanks,
Paul & Roi





Hi,

We found memory leaks where we didn't release the reply allocated from 
tc_transact calls. We have this fixed for V4.


Thanks,
Roi

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 5/5] datapath: Fix cvlan test failure on the old kernel versions

2017-02-12 Thread Yi Yang
The root cause is the upcall re-inserts the VLAN back into the raw
packet data, but the TPID is hard coded to 0x8100. This affects
kernels for which HAVE_VLAN_INSERT_TAG_SET_PROTO is not set.

The below patch allows the cvlan and 802.ad tests to pass on debian
with 3.16 kernel.

Signed-off-by: Eric Garver 
Signed-off-by: Yi Yang 
---
 datapath/linux/compat/include/linux/if_vlan.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/datapath/linux/compat/include/linux/if_vlan.h 
b/datapath/linux/compat/include/linux/if_vlan.h
index d51e5c9..fc95b04 100644
--- a/datapath/linux/compat/include/linux/if_vlan.h
+++ b/datapath/linux/compat/include/linux/if_vlan.h
@@ -24,8 +24,9 @@
  * acceptable.
  */
 #define vlan_insert_tag_set_proto(skb, proto, vlan_tci) \
-   rpl_vlan_insert_tag_set_proto(skb, vlan_tci)
+   rpl_vlan_insert_tag_set_proto(skb, proto, vlan_tci)
 static inline struct sk_buff *rpl_vlan_insert_tag_set_proto(struct sk_buff 
*skb,
+   __be16 vlan_proto,
u16 vlan_tci)
 {
struct vlan_ethhdr *veth;
@@ -41,12 +42,12 @@ static inline struct sk_buff 
*rpl_vlan_insert_tag_set_proto(struct sk_buff *skb,
skb->mac_header -= VLAN_HLEN;
 
/* first, the ethernet type */
-   veth->h_vlan_proto = htons(ETH_P_8021Q);
+   veth->h_vlan_proto = vlan_proto;
 
/* now, the TCI */
veth->h_vlan_TCI = htons(vlan_tci);
 
-   skb->protocol = htons(ETH_P_8021Q);
+   skb->protocol = vlan_proto;
 
return skb;
 }
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 4/5] datapath: backport: openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink attributes

2017-02-12 Thread Yi Yang
commit 018c1dda5ff1e7bd1fe2d9fd1d0f5b82dc6fc0cd
Author: Eric Garver 
Date:   Wed Sep 7 12:56:59 2016 -0400

Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.

This is based on Thomas F Herbert's original v20 patch. I made some
small clean ups and bug fixes.

Signed-off-by: Thomas F Herbert 
Signed-off-by: Eric Garver 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Yi Yang 
Acked-by: Eric Garver 
---
 datapath/actions.c  |  16 ++-
 datapath/flow.c |  65 +++---
 datapath/flow.h |   8 +-
 datapath/flow_netlink.c | 311 +---
 datapath/vport.c|   7 +-
 5 files changed, 282 insertions(+), 125 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 82833d0..52a6858 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -247,20 +247,24 @@ static int pop_vlan(struct sk_buff *skb, struct 
sw_flow_key *key)
int err;
 
err = skb_vlan_pop(skb);
-   if (skb_vlan_tag_present(skb))
+   if (skb_vlan_tag_present(skb)) {
invalidate_flow_key(key);
-   else
-   key->eth.tci = 0;
+   } else {
+   key->eth.vlan.tci = 0;
+   key->eth.vlan.tpid = 0;
+   }
return err;
 }
 
 static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 const struct ovs_action_push_vlan *vlan)
 {
-   if (skb_vlan_tag_present(skb))
+   if (skb_vlan_tag_present(skb)) {
invalidate_flow_key(key);
-   else
-   key->eth.tci = vlan->vlan_tci;
+   } else {
+   key->eth.vlan.tci = vlan->vlan_tci;
+   key->eth.vlan.tpid = vlan->vlan_tpid;
+   }
return skb_vlan_push(skb, vlan->vlan_tpid,
 ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
 }
diff --git a/datapath/flow.c b/datapath/flow.c
index 390286c..f55b487 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -301,24 +301,57 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
  sizeof(struct icmp6hdr));
 }
 
-static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+/**
+ * Parse vlan tag from vlan header.
+ * Returns ERROR on memory error.
+ * Returns 0 if it encounters a non-vlan or incomplete packet.
+ * Returns 1 after successfully parsing vlan tag.
+ */
+static int parse_vlan_tag(struct sk_buff *skb, struct vlan_head *key_vh)
 {
-   struct qtag_prefix {
-   __be16 eth_type; /* ETH_P_8021Q */
-   __be16 tci;
-   };
-   struct qtag_prefix *qp;
+   struct vlan_head *vh = (struct vlan_head *)skb->data;
 
-   if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
+   if (likely(!eth_type_vlan(vh->tpid)))
return 0;
 
-   if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
-sizeof(__be16
+   if (unlikely(skb->len < sizeof(struct vlan_head) + sizeof(__be16)))
+   return 0;
+
+   if (unlikely(!pskb_may_pull(skb, sizeof(struct vlan_head) +
+sizeof(__be16
return -ENOMEM;
 
-   qp = (struct qtag_prefix *) skb->data;
-   key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
-   __skb_pull(skb, sizeof(struct qtag_prefix));
+   vh = (struct vlan_head *)skb->data;
+   key_vh->tci = vh->tci | htons(VLAN_TAG_PRESENT);
+   key_vh->tpid = vh->tpid;
+
+   __skb_pull(skb, sizeof(struct vlan_head));
+   return 1;
+}
+
+static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+{
+   int res;
+
+   key->eth.vlan.tci = 0;
+   key->eth.vlan.tpid = 0;
+   key->eth.cvlan.tci = 0;
+   key->eth.cvlan.tpid = 0;
+
+   if (likely(skb_vlan_tag_present(skb))) {
+   key->eth.vlan.tci = htons(skb->vlan_tci);
+   key->eth.vlan.tpid = skb->vlan_proto;
+   } else {
+   /* Parse outer vlan tag in the non-accelerated case. */
+   res = parse_vlan_tag(skb, >eth.vlan);
+   if (res <= 0)
+   return res;
+   }
+
+   /* Parse inner vlan tag. */
+   res = parse_vlan_tag(skb, >eth.cvlan);
+   if (res <= 0)
+   return res;
 
return 0;
 }
@@ -479,12 +512,8 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
 * update skb->csum here.
 */
 
-   key->eth.tci = 0;
-   if (skb_vlan_tag_present(skb))
-   key->eth.tci = htons(skb->vlan_tci);
-   

[ovs-dev] [PATCH v3 2/5] datapath: backport: openvswitch: 802.1ad uapi changes.

2017-02-12 Thread Yi Yang
commit 8c146bb9d59aa2ac45222171916ece186c4b3943
Author: Thomas F Herbert 
Date:   Wed Sep 7 12:56:57 2016 -0400

openvswitch: Add support for 8021.AD

Change the description of the VLAN tpid field.

Signed-off-by: Thomas F Herbert 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Yi Yang 
Acked-by: Eric Garver 
---
 datapath/linux/compat/include/linux/openvswitch.h | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 425d3a4..b6f43b3 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -629,13 +629,13 @@ struct ovs_action_push_mpls {
  * @vlan_tci: Tag control identifier (TCI) to push.  The CFI bit must be set
  * (but it will not be set in the 802.1Q header that is pushed).
  *
- * The @vlan_tpid value is typically %ETH_P_8021Q.  The only acceptable TPID
- * values are those that the kernel module also parses as 802.1Q headers, to
- * prevent %OVS_ACTION_ATTR_PUSH_VLAN followed by %OVS_ACTION_ATTR_POP_VLAN
- * from having surprising results.
+ * The @vlan_tpid value is typically %ETH_P_8021Q or %ETH_P_8021AD.
+ * The only acceptable TPID values are those that the kernel module also parses
+ * as 802.1Q or 802.1AD headers, to prevent %OVS_ACTION_ATTR_PUSH_VLAN followed
+ * by %OVS_ACTION_ATTR_POP_VLAN from having surprising results.
  */
 struct ovs_action_push_vlan {
-   __be16 vlan_tpid;   /* 802.1Q TPID. */
+   __be16 vlan_tpid;   /* 802.1Q or 802.1ad TPID. */
__be16 vlan_tci;/* 802.1Q TCI (VLAN ID and priority). */
 };
 
@@ -755,9 +755,10 @@ enum ovs_nat_attr {
  * @OVS_ACTION_ATTR_TRUNC: Output packet to port with truncated packet size.
  * @OVS_ACTION_ATTR_USERSPACE: Send packet to userspace according to nested
  * %OVS_USERSPACE_ATTR_* attributes.
- * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q header onto the
- * packet.
- * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q header off the packet.
+ * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q or 802.1ad header
+ * onto the packet.
+ * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q or 802.1ad header
+ * from the packet.
  * @OVS_ACTION_ATTR_SAMPLE: Probabilitically executes actions, as specified in
  * the nested %OVS_SAMPLE_ATTR_* attributes.
  * @OVS_ACTION_ATTR_SET: Replaces the contents of an existing header.  The
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 3/5] datapath: backport: vlan: Check for vlan ethernet types for 8021.q or 802.1ad

2017-02-12 Thread Yi Yang
commit fe19c4f971a55cea3be442d8032a5f6021702791
Author: Eric Garver 
Date:   Wed Sep 7 12:56:58 2016 -0400

This is to simplify using double tagged vlans. This function allows all
valid vlan ethertypes to be checked in a single function call.
Also replace some instances that check for both ETH_P_8021Q and
ETH_P_8021AD.

Patch based on one originally by Thomas F Herbert.

Signed-off-by: Thomas F Herbert 
Signed-off-by: Eric Garver 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Yi Yang 
---
 acinclude.m4  |  1 +
 datapath/linux/compat/include/linux/if_vlan.h | 32 ---
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 9166da8..f219bec 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -664,6 +664,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [__vlan_insert_tag])
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_get_protocol])
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [skb_vlan_tagged])
+  OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [eth_type_vlan])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/u64_stats_sync.h], 
[u64_stats_fetch_begin_irq])
 
diff --git a/datapath/linux/compat/include/linux/if_vlan.h 
b/datapath/linux/compat/include/linux/if_vlan.h
index c8ddef1..d51e5c9 100644
--- a/datapath/linux/compat/include/linux/if_vlan.h
+++ b/datapath/linux/compat/include/linux/if_vlan.h
@@ -100,6 +100,25 @@ static inline struct sk_buff 
*rpl___vlan_hwaccel_put_tag(struct sk_buff *skb,
 #define __vlan_hwaccel_put_tag rpl___vlan_hwaccel_put_tag
 #endif
 
+#ifndef HAVE_ETH_TYPE_VLAN
+/**
+ * eth_type_vlan - check for valid vlan ether type.
+ * @ethertype: ether type to check
+ *
+ * Returns true if the ether type is a vlan ether type.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+   switch (ethertype) {
+   case htons(ETH_P_8021Q):
+   case htons(ETH_P_8021AD):
+   return true;
+   default:
+   return false;
+   }
+}
+#endif
+
 /* All of these were introduced in a single commit preceding 2.6.33, so
  * presumably all of them or none of them are present. */
 #ifndef VLAN_PRIO_MASK
@@ -188,7 +207,7 @@ static inline __be16 __vlan_get_protocol(struct sk_buff 
*skb, __be16 type,
 * present at mac_len - VLAN_HLEN (if mac_len > 0), or at
 * ETH_HLEN otherwise
 */
-   if (type == htons(ETH_P_8021Q) || type == htons(ETH_P_8021AD)) {
+   if (eth_type_vlan(type)) {
if (vlan_depth) {
if (WARN_ON(vlan_depth < VLAN_HLEN))
return 0;
@@ -206,8 +225,7 @@ static inline __be16 __vlan_get_protocol(struct sk_buff 
*skb, __be16 type,
vh = (struct vlan_hdr *)(skb->data + vlan_depth);
type = vh->h_vlan_encapsulated_proto;
vlan_depth += VLAN_HLEN;
-   } while (type == htons(ETH_P_8021Q) ||
-type == htons(ETH_P_8021AD));
+   } while (eth_type_vlan(type));
}
 
if (depth)
@@ -241,8 +259,7 @@ static inline __be16 vlan_get_protocol(struct sk_buff *skb)
 static inline bool skb_vlan_tagged(const struct sk_buff *skb)
 {
if (!skb_vlan_tag_present(skb) &&
-   likely(skb->protocol != htons(ETH_P_8021Q) &&
-  skb->protocol != htons(ETH_P_8021AD)))
+   likely(!eth_type_vlan(skb->protocol)))
return false;
 
return true;
@@ -262,15 +279,14 @@ static inline bool skb_vlan_tagged_multi(const struct 
sk_buff *skb)
if (!skb_vlan_tag_present(skb)) {
struct vlan_ethhdr *veh;
 
-   if (likely(protocol != htons(ETH_P_8021Q) &&
-  protocol != htons(ETH_P_8021AD)))
+   if (likely(!eth_type_vlan(protocol)))
return false;
 
veh = (struct vlan_ethhdr *)skb->data;
protocol = veh->h_vlan_encapsulated_proto;
}
 
-   if (protocol != htons(ETH_P_8021Q) && protocol != htons(ETH_P_8021AD))
+   if (!eth_type_vlan(protocol))
return false;
 
return true;
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 1/5] datapath: backport: vlan: Introduce helper functions to check if skb is tagged

2017-02-12 Thread Yi Yang
commit f5a7fb88e1f82542ca14ba93a1d4fa35471c60ca
Author: Toshiaki Makita 
Date:   Fri Mar 27 14:31:11 2015 +0900

vlan: Introduce helper functions to check if skb is tagged

Separate the two checks for single vlan and multiple vlans in
netif_skb_features().  This allows us to move the check for multiple
vlans to another function later.

Signed-off-by: Toshiaki Makita 
Signed-off-by: David S. Miller 

Signed-off-by: Yi Yang 
Acked-by: Eric Garver 
---
 acinclude.m4  |  1 +
 datapath/linux/compat/include/linux/if_vlan.h | 49 +++
 2 files changed, 50 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index e8b64b5..9166da8 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -663,6 +663,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_insert_tag_set_proto])
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [__vlan_insert_tag])
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_get_protocol])
+  OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [skb_vlan_tagged])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/u64_stats_sync.h], 
[u64_stats_fetch_begin_irq])
 
diff --git a/datapath/linux/compat/include/linux/if_vlan.h 
b/datapath/linux/compat/include/linux/if_vlan.h
index a8d7bfa..c8ddef1 100644
--- a/datapath/linux/compat/include/linux/if_vlan.h
+++ b/datapath/linux/compat/include/linux/if_vlan.h
@@ -229,4 +229,53 @@ static inline __be16 vlan_get_protocol(struct sk_buff *skb)
 }
 
 #endif
+
+#ifndef HAVE_SKB_VLAN_TAGGED
+/**
+ * skb_vlan_tagged - check if skb is vlan tagged.
+ * @skb: skbuff to query
+ *
+ * Returns true if the skb is tagged, regardless of whether it is hardware
+ * accelerated or not.
+ */
+static inline bool skb_vlan_tagged(const struct sk_buff *skb)
+{
+   if (!skb_vlan_tag_present(skb) &&
+   likely(skb->protocol != htons(ETH_P_8021Q) &&
+  skb->protocol != htons(ETH_P_8021AD)))
+   return false;
+
+   return true;
+}
+
+/**
+ * skb_vlan_tagged_multi - check if skb is vlan tagged with multiple headers.
+ * @skb: skbuff to query
+ *
+ * Returns true if the skb is tagged with multiple vlan headers, regardless
+ * of whether it is hardware accelerated or not.
+ */
+static inline bool skb_vlan_tagged_multi(const struct sk_buff *skb)
+{
+   __be16 protocol = skb->protocol;
+
+   if (!skb_vlan_tag_present(skb)) {
+   struct vlan_ethhdr *veh;
+
+   if (likely(protocol != htons(ETH_P_8021Q) &&
+  protocol != htons(ETH_P_8021AD)))
+   return false;
+
+   veh = (struct vlan_ethhdr *)skb->data;
+   protocol = veh->h_vlan_encapsulated_proto;
+   }
+
+   if (protocol != htons(ETH_P_8021Q) && protocol != htons(ETH_P_8021AD))
+   return false;
+
+   return true;
+}
+
+#endif /* HAVE_SKB_VLAN_TAGGED */
+
 #endif /* linux/if_vlan.h wrapper */
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 0/5] Backport 802.1ad patches

2017-02-12 Thread Yi Yang
This patch set is to backport 802.1ad patches Eric Garver did to ovs, per 
discussion in ovs-dev mailing list, this fix patch
"openvswitch: upcall: Fix vlan handling" depends on them, l3 patch set depends 
on this patch set and "openvswitch: upcall: Fix vlan handling".

Yi Yang (5):
  datapath: backport: vlan: Introduce helper functions to check if skb
is tagged
  datapath: backport: openvswitch: 802.1ad uapi changes.
  datapath: backport: vlan: Check for vlan ethernet types for 8021.q or
802.1ad
  datapath: backport: openvswitch: 802.1AD Flow handling, actions, vlan
parsing, netlink attributes
  datapath: Fix cvlan test failure on the old kernel versions

 acinclude.m4  |   2 +
 datapath/actions.c|  16 +-
 datapath/flow.c   |  65 +++--
 datapath/flow.h   |   8 +-
 datapath/flow_netlink.c   | 311 +++---
 datapath/linux/compat/include/linux/if_vlan.h |  78 +-
 datapath/linux/compat/include/linux/openvswitch.h |  17 +-
 datapath/vport.c  |   7 +-
 8 files changed, 365 insertions(+), 139 deletions(-)

-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3 00/16] port Jiri Benc's L3 patchset to ovs

2017-02-12 Thread Yang, Yi Y
Joe, got it, thanks a lot.

-Original Message-
From: Joe Stringer [mailto:j...@ovn.org] 
Sent: Saturday, February 11, 2017 5:38 AM
To: Yang, Yi Y 
Cc: Multanen, Eric W ; ovs dev 
; Yi-Hung Wei 
Subject: Re: [ovs-dev] [PATCH v3 00/16] port Jiri Benc's L3 patchset to ovs

On 9 February 2017 at 17:45, Yang, Yi Y  wrote:
> Joe, thank you for explanation, let us push 802.1ad backport patches first, 
> Eric Garver has acked two of them, I'll rework another one patch he 
> commented. What Valentine mentioned has no business with 802.1 ad, it is what 
> we want to consider after Eric Garver's rtnetlink userspace patches are 
> merged, we have no way to use in-kernel data path to do it without rtnetlink 
> userspace patches.
>
> Can you point out which patches  for 802.1ad are missing? I almost checked 
> all the 802.1 ad patches, ovs doesn't use most of them, most of them make 
> nonsense to ovs. Eric, please help point out if I missed something.

I believe there's "net: mpls: Fixups for GSO" and one other more trivial one. 
I've been working with Yi-Hung to try to figure out those backports, so we 
should be able to proceed with those shortly, then I'll look at 802.1ad.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3 00/16] port Jiri Benc's L3 patchset to ovs

2017-02-12 Thread Yang, Yi Y
Eric and Joe, thank you so much for your verification and fix, I'll resubmit a 
new version including this fix and missing part.

-Original Message-
From: Eric Garver [mailto:e...@erig.me] 
Sent: Saturday, February 11, 2017 9:22 PM
To: Joe Stringer ; Yang, Yi Y 
Cc: ovs dev 
Subject: Re: [ovs-dev] [PATCH v3 00/16] port Jiri Benc's L3 patchset to ovs

On Fri, Feb 10, 2017 at 01:33:04PM -0800, Joe Stringer wrote:
> On 10 February 2017 at 12:25, Eric Garver  wrote:
> > On Fri, Feb 10, 2017 at 11:24:58AM -0500, Eric Garver wrote:
> >> On Tue, Feb 07, 2017 at 02:30:46PM -0800, Joe Stringer wrote:
> >> > On 7 February 2017 at 09:44, Joe Stringer  wrote:
> >> > > On 6 February 2017 at 16:46, Yang, Yi Y  wrote:
> >> > >> Joe, I checked current ovs and net-next kernel, obviously some 
> >> > >> patches from net-next are selectively backported to ovs, but others 
> >> > >> are not, I'm not sure what the policy is for a new patch. It will be 
> >> > >> better that the person who did the patch backports it to ovs at the 
> >> > >> same time, but nobody did so.
> >> > >
> >> > > That's supposed to be the policy; However, depending on the 
> >> > > patch sometimes openvswitch isn't even the main target of the 
> >> > > change, so the contributor may not be aware they should do so. 
> >> > > There may be added difficulty if the previous contributor 
> >> > > didn't do their backport. I think that lately there hasn't been 
> >> > > particularly close co-ordination between the trees, but ideally 
> >> > > I think that as we approach an OVS release, we would try to sync them 
> >> > > up.
> >> > >
> >> > >> My 802.1ad backport has included all the things l3 patch set 
> >> > >> depends on, so I think you can give it a go :-)
> >> > >
> >> > > Kicking off a build on my local tester, I can at least report 
> >> > > back on that. I see you've tested on a few platforms as well, that's 
> >> > > great.
> >> >
> >> > Reporting back, I suspect that some of the older kernels don't 
> >> > treat the double-tagged vlans right with this series?
> >> >
> >> > I was using an Ubuntu trusty VM with kernel 3.13.0-92-generic 
> >> > plus this backport and it seems to be consistently failing this test:
> >> >
> >> > 4: datapath - ping between two ports on cvlan FAILED 
> >> > (system-traffic.at:88)
> >> >
> >> > The test output just shows that a ping tries about 10 times and 
> >> > fails all of the times. I didn't investigate further.
> >>
> >> Hi Joe,
> >>
> >> Thanks for testing those backports.
> >>
> >> I ran them on debian with 3.16 kernel. I see the same failures.
> >> Interestingly, if when the test hangs (when waiting on the ping) I 
> >> ping in a different window namespace to namespace it will work as expected.
> >> This ping should be equivalent to what the test is doing. I can't 
> >> explain this yet, but I'll keep investigating.
> >
> > I found the cause. The upcall re-inserts the VLAN back into the raw 
> > packet data, but the TPID is hard coded to 0x8100.
> > This affects kernels for which HAVE_VLAN_INSERT_TAG_SET_PROTO is not 
> > set.
> >
> > The below patch allows the cvlan and 802.ad tests (not upstream yet) 
> > to pass on debian with 3.16 kernel. However, it may be better 
> > backport a modern version of vlan_insert_tag_set_proto().
> 
> Thanks for figuring this out. Will you or Yi submit this?

I think it makes sense for it to be submitted with the 802.1ad series.

Yi,

Can you add the fix I suggest above to your 802.1ad series?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev