[ovs-dev] [PATCH v3 13/16] datapath: upcall: Fix vlan handling.
Yi Yang
yi.y.yang at intel.com
Mon Feb 6 13:04:47 UTC 2017
>From net-next commit df30f7408b187929dbde72661c7f7c615268f1d0.
Networking stack accelerate vlan tag handling by
keeping topmost vlan header in skb. This works as
long as packet remains in OVS datapath. But during
OVS upcall vlan header is pushed on to the packet.
When such packet is sent back to OVS datapath, core
networking stack might not handle it correctly. Following
patch avoids this issue by accelerating the vlan tag
during flow key extract. This simplifies datapath by
bringing uniform packet processing for packets from
all code paths.
Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets").
CC: Jarno Rajahalme <jarno at ovn.org>
CC: Jiri Benc <jbenc at redhat.com>
Signed-off-by: Pravin B Shelar <pshelar at ovn.org>
Signed-off-by: David S. Miller <davem at davemloft.net>
Signed-off-by: Yi Yang <yi.y.yang at intel.com>
---
acinclude.m4 | 1 +
datapath/datapath.c | 1 -
datapath/flow.c | 54 ++++++++++++++--------------
datapath/linux/compat/include/linux/skbuff.h | 5 +++
datapath/linux/compat/skbuff-openvswitch.c | 8 +++--
5 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index f219bec..bda4d99 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -600,6 +600,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
[OVS_DEFINE([HAVE_L4_RXHASH])])
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_ensure_writable])
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_pop])
+ OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [__skb_vlan_pop])
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_push])
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_clear_hash_if_not_l4])
OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_postpush_rcsum])
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 1deb62d..dd19a0e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -617,7 +617,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
packet->priority = flow->key.phy.priority;
packet->mark = flow->key.phy.skb_mark;
- packet->protocol = flow->key.eth.type;
rcu_read_lock();
dp = get_dp_rcu(net, ovs_header->dp_ifindex);
diff --git a/datapath/flow.c b/datapath/flow.c
index a627b1a..1478afc 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -307,7 +307,8 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
* 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)
+static int parse_vlan_tag(struct sk_buff *skb, struct vlan_head *key_vh,
+ bool untag_vlan)
{
struct vlan_head *vh = (struct vlan_head *)skb->data;
@@ -325,7 +326,20 @@ static int parse_vlan_tag(struct sk_buff *skb, struct vlan_head *key_vh)
key_vh->tci = vh->tci | htons(VLAN_TAG_PRESENT);
key_vh->tpid = vh->tpid;
- __skb_pull(skb, sizeof(struct vlan_head));
+ if (unlikely(untag_vlan)) {
+ int offset = skb->data - skb_mac_header(skb);
+ u16 tci;
+ int err;
+
+ __skb_push(skb, offset);
+ err = __skb_vlan_pop(skb, &tci);
+ __skb_pull(skb, offset);
+ if (err)
+ return err;
+ __vlan_hwaccel_put_tag(skb, key_vh->tpid, tci);
+ } else {
+ __skb_pull(skb, sizeof(struct vlan_head));
+ }
return 1;
}
@@ -351,13 +365,13 @@ static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
key->eth.vlan.tpid = skb->vlan_proto;
} else {
/* Parse outer vlan tag in the non-accelerated case. */
- res = parse_vlan_tag(skb, &key->eth.vlan);
+ res = parse_vlan_tag(skb, &key->eth.vlan, true);
if (res <= 0)
return res;
}
/* Parse inner vlan tag. */
- res = parse_vlan_tag(skb, &key->eth.cvlan);
+ res = parse_vlan_tag(skb, &key->eth.cvlan, false);
if (res <= 0)
return res;
@@ -807,29 +821,15 @@ int ovs_flow_key_extract_userspace(struct net *net, const struct nlattr *attr,
if (err)
return err;
- if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
- /* key_extract assumes that skb->protocol is set-up for
- * layer 3 packets which is the case for other callers,
- * in particular packets recieved from the network stack.
- * Here the correct value can be set from the metadata
- * extracted above.
- */
- skb->protocol = key->eth.type;
- } else {
- struct ethhdr *eth;
-
- skb_reset_mac_header(skb);
- eth = eth_hdr(skb);
-
- /* 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))
- skb->protocol = eth->h_proto;
- else
- skb->protocol = htons(ETH_P_802_2);
- }
+ /* key_extract assumes that skb->protocol is set-up for
+ * layer 3 packets which is the case for other callers,
+ * in particular packets received from the network stack.
+ * Here the correct value can be set from the metadata
+ * extracted above.
+ * For L2 packet key eth type would be zero. skb protocol
+ * would be set to correct value later during key-extact.
+ */
+ skb->protocol = key->eth.type;
return key_extract(skb, key);
}
diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h
index a2cbd78..6c45939 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -263,6 +263,11 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
int rpl_skb_ensure_writable(struct sk_buff *skb, int write_len);
#endif
+#ifndef HAVE___SKB_VLAN_POP
+#define __skb_vlan_pop rpl___skb_vlan_pop
+int rpl___skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
+#endif
+
#ifndef HAVE_SKB_VLAN_POP
#define skb_vlan_pop rpl_skb_vlan_pop
int rpl_skb_vlan_pop(struct sk_buff *skb);
diff --git a/datapath/linux/compat/skbuff-openvswitch.c b/datapath/linux/compat/skbuff-openvswitch.c
index aa6ff42..4cdeedc 100644
--- a/datapath/linux/compat/skbuff-openvswitch.c
+++ b/datapath/linux/compat/skbuff-openvswitch.c
@@ -141,9 +141,9 @@ int rpl_skb_ensure_writable(struct sk_buff *skb, int write_len)
EXPORT_SYMBOL_GPL(rpl_skb_ensure_writable);
#endif
-#ifndef HAVE_SKB_VLAN_POP
+#if !defined(HAVE___SKB_VLAN_POP) || !defined(HAVE_SKB_VLAN_POP)
/* remove VLAN header from packet and update csum accordingly. */
-static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
+int rpl___skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
{
struct vlan_hdr *vhdr;
unsigned int offset = skb->data - skb_mac_header(skb);
@@ -174,7 +174,9 @@ pull:
return err;
}
+#endif
+#ifndef HAVE_SKB_VLAN_POP
int rpl_skb_vlan_pop(struct sk_buff *skb)
{
u16 vlan_tci;
@@ -189,7 +191,7 @@ int rpl_skb_vlan_pop(struct sk_buff *skb)
skb->len < VLAN_ETH_HLEN))
return 0;
- err = __skb_vlan_pop(skb, &vlan_tci);
+ err = rpl___skb_vlan_pop(skb, &vlan_tci);
if (err)
return err;
}
--
2.1.0
More information about the dev
mailing list