Re: [Xen-devel] xen + openvswitch

[Ben Pfaff <blp@xxxxxxxxxxxxxxx>]

Vijay Chander <vijay.chander@xxxxxxxxx> writes:

> On Wed, Jan 18, 2012 at 9:14 PM, Vijay Chander <vijay.chander@xxxxxxxxx>
> wrote:
>
>     Has anyone seen performance issues with xen 4.1.0 (the one used in
>     xencenter 6.0)Â
>     using open vswitch as opposed to native linux bridge ?
>    
>     We are tussling with needing the support guest vlan tagging and
>     performance.

XenServer 6.0.0 shipped with an Open vSwitch build that contained
a serious VLAN performance bug.  It is fixed on the Open vSwitch
"vlan-maint" branch.  The fix should also appear in the next
XenServer release.

Here is the Open vSwitch commit that fixed the problem.

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp@xxxxxxxxxx>
Date: Wed, 23 Mar 2011 10:34:37 -0700
Subject: [PATCH] datapath: Add compatibility with sk_buff's vlan_tci before 
2.6.33.

Between 2.6.27 and 2.6.32, the vlan_tci member of struct sk_buff was the
raw value of the 802.1Q header's TCI field, without the CFI bit being set.
In 2.6.33 and later, the CFI bit is always set if an 802.1Q header is
present, correcting a corner case.

Until now, OVS has not consistently dealt with this.  If a packet arrived
at a datapath from a network device directly, or if it was set with an
ODP_ACTION_ATTR_SET_DL_TCI action, then the CFI bit would not be set in
vlan_tci.  In flow_extract(), OVS copies vlan_tci directly to dl_tci in the
flow structure (via vlan_get_tci()), so the CFI bit would also not be set
in dl_tci.  But if OVS had to send a packet up to userspace (converting the
vlan_tci back to an 802.1Q header along the way) and got it back, then it
would set the VLAN CFI bit in dl_tci when it parsed the 802.1Q header in
parse_vlan().  This had the effect that a flow set up by userspace (with
the CFI bit set) would never be matched by a packet arriving from a network
device, because they would have different dl_tci values.

This fixes the problem, by making the vlan_get_tci() and vlan_set_tci()
interface consistent across kernel versions.  Now, they always accept or
return a value where the VLAN CFI bit is set if an 802.1Q header is
present.

Build-tested only.

Problem isolated by Ethan Jackson <ethan@xxxxxxxxxx>

Signed-off-by: Ben Pfaff <blp@xxxxxxxxxx>
Acked-by: Jesse Gross <jesse@xxxxxxxxxx>
Reported-by: Ram Jothikumar <rjothikumar@xxxxxxxxxx>
Bug #4915.
---
 datapath/vlan.h |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/datapath/vlan.h b/datapath/vlan.h
index 02a6290..7e1084e 100644
--- a/datapath/vlan.h
+++ b/datapath/vlan.h
@@ -13,6 +13,29 @@
 #include <linux/skbuff.h>
 #include <linux/version.h>
 
+/**
+ * DOC: VLAN tag manipulation.
+ *
+ * &struct sk_buff handling of VLAN tags has evolved over time:
+ *
+ * In 2.6.26 and earlier, VLAN tags did not have any generic representation in
+ * an skb, other than as a raw 802.1Q header inside the packet data.
+ *
+ * In 2.6.27 &struct sk_buff added a @vlan_tci member.  Between 2.6.27 and
+ * 2.6.32, its value was the raw contents of the 802.1Q TCI field, or zero if
+ * no 802.1Q header was present.  This worked OK except for the corner case of
+ * an 802.1Q header with an all-0-bits TCI, which could not be represented.
+ *
+ * In 2.6.33, @vlan_tci semantics changed.  Now, if an 802.1Q header is
+ * present, then the VLAN_TAG_PRESENT bit is always set.  This fixes the
+ * all-0-bits TCI corner case.
+ *
+ * For compatibility we emulate the 2.6.33+ behavior on earlier kernel
+ * versions.  The client must not access @vlan_tci directly.  Instead, use
+ * vlan_get_tci() to read it or vlan_set_tci() to write it, with semantics
+ * equivalent to those on 2.6.33+.
+ */
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27)
 #define NEED_VLAN_FIELD
 #endif
@@ -22,11 +45,18 @@ static inline void vlan_copy_skb_tci(struct sk_buff *skb) { 
}
 
 static inline u16 vlan_get_tci(struct sk_buff *skb)
 {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
+       if (skb->vlan_tci)
+               return skb->vlan_tci | VLAN_TAG_PRESENT;
+#endif
        return skb->vlan_tci;
 }
 
 static inline void vlan_set_tci(struct sk_buff *skb, u16 vlan_tci)
 {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
+       vlan_tci &= ~VLAN_TAG_PRESENT;
+#endif
        skb->vlan_tci = vlan_tci;
 }
 #else
-- 
1.7.2.5


-- 
Ben Pfaff 
http://benpfaff.org

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel