openvswitch: Add support for 802.1AD
Subsystems (3)
v9 cover openvswitch: Add support for 802.1AD
Add support for 802.1AD to the openvswitch kernel module.
V9 includes changes suggested by reviewers
Although the Open Flow specification specified support for 802.1AD (qinq) as well as push and pop vlan headers, So far Open vSwitch has only supported a single tag header.
This patch accompanies version 7 of the patch submitted to openvswitch dev list.
For discussion, history and previous versions, of the kernel patch, see the OVS dev mailing list, openvswitch.org/pipermail/dev/..
Thomas F Herbert (2):
openvswitch: 802.1ad uapi changes
Flow handling, actions, and parsing
include/uapi/linux/openvswitch.h | 17 ++-- net/openvswitch/flow.c | 82 ++++++++++++--- net/openvswitch/flow.h | 3 + net/openvswitch/flow_netlink.c | 212 +++++++++++++++++++++++++++++++++------ 4 files changed, 265 insertions(+), 49 deletions(-)
Signature · 1 line
2.1.0
PATCH v9 1/2 openvswitch: 802.1ad uapi changes.
openvswitch: Add support for 8021.AD
Change the description of the VLAN tpid field.
--- include/uapi/linux/openvswitch.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
include/uapi/linux/openvswitch.h +9 -8
@@ -559,13 +559,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). */ };
@@ -605,9 +605,10 @@ struct ovs_action_hash {* is copied from the value to the packet header field, rest of the bits are * left unchanged. The non-masked value bits must be passed in as zeroes. * Masking is not supported for the %OVS_KEY_ATTR_TUNNEL attribute. -* @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_PUSH_MPLS: Push a new MPLS label stack entry onto the
Signature · 1 line
2.1.0
PATCH v7 2/2 openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
In reply to Thomas F HerbertAdd support for 802.1ad including the ability to push and pop double tagged vlans.
--- net/openvswitch/flow.c | 82 +++++++++++++--- net/openvswitch/flow.h | 3 + net/openvswitch/flow_netlink.c | 212 +++++++++++++++++++++++++++++++++++------ 3 files changed, 256 insertions(+), 41 deletions(-)
net/openvswitch/flow.c +70 -12
@@ -298,21 +298,78 @@ static bool icmp6hdr_ok(struct sk_buff *skb)static int parse_vlan (struct sk_buff *skb,struct sw_flow_key *key) {struct qtag_prefix { -__be16 eth_type;/* ETH_P_8021Q */ +__be16 eth_type;/* ETH_P_8021Q or ETH_P_8021AD */ __be16 tci; }; -struct qtag_prefix *qp; +struct qtag_prefix *qp = (struct qtag_prefix *)skb->data; -if (unlikely (skb->len <sizeof (struct qtag_prefix ) +sizeof (__be16))) +struct qinqtag_prefix { +__be16 eth_type;/* ETH_P_8021Q or ETH_P_8021AD */ +__be16 tci; +__be16 inner_tpid;/* ETH_P_8021Q */ +__be16 ctci; + }; + +if (likely (skb_vlan_tag_present (skb))) { + key->eth.tci =htons (skb->vlan_tci); + +/* Case where upstream +* processing has already stripped the outer vlan tag. +*/ +if (unlikely (skb->vlan_proto ==htons (ETH_P_8021AD ))) { +if (unlikely (skb->len <sizeof (struct qtag_prefix ) + +sizeof (__be16))) { + key->eth.tci =0 ; +return 0 ; + } + +if (unlikely (!pskb_may_pull (skb, +sizeof (struct qtag_prefix ) + +sizeof (__be16)))) { +return -ENOMEM ; + } + +if (likely (qp->eth_type ==htons (ETH_P_8021Q ))) { + key->eth.ctci = qp->tci | +htons (VLAN_TAG_PRESENT ); +__skb_pull (skb,sizeof (struct qtag_prefix )); + } + }return 0 ; + } -if (unlikely (!pskb_may_pull (skb,sizeof (struct qtag_prefix ) + -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 )); +if (qp->eth_type ==htons (ETH_P_8021AD )) { +struct qinqtag_prefix *qinqp = + (struct qinqtag_prefix *)skb->data; + +if (unlikely (skb->len <sizeof (struct qinqtag_prefix ) + +sizeof (__be16))) +return 0 ; + +if (unlikely (!pskb_may_pull (skb,sizeof (struct qinqtag_prefix ) + +sizeof (__be16)))) { +return -ENOMEM ; + } + key->eth.tci = qinqp->tci |htons (VLAN_TAG_PRESENT ); + key->eth.ctci = qinqp->ctci |htons (VLAN_TAG_PRESENT ); + +__skb_pull (skb,sizeof (struct qinqtag_prefix )); + +return 0 ; + } +if (qp->eth_type ==htons (ETH_P_8021Q )) { +if (unlikely (skb->len <sizeof (struct qtag_prefix ) + +sizeof (__be16))) +return -ENOMEM ; + +if (unlikely (!pskb_may_pull (skb,sizeof (struct qtag_prefix ) + +sizeof (__be16)))) +return 0 ; + key->eth.tci = qp->tci |htons (VLAN_TAG_PRESENT ); + +__skb_pull (skb,sizeof (struct qtag_prefix )); + }return 0 ; }
@@ -474,9 +531,10 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)*/ key->eth.tci =0 ; -if (skb_vlan_tag_present (skb)) - key->eth.tci =htons (skb->vlan_tci); -else if (eth->h_proto ==htons (ETH_P_8021Q )) + key->eth.ctci =0 ; +if ((skb_vlan_tag_present (skb)) || + (eth->h_proto ==htons (ETH_P_8021Q )) || + (eth->h_proto ==htons (ETH_P_8021AD )))if (unlikely (parse_vlan (skb, key)))return -ENOMEM ;
net/openvswitch/flow.h +3 -0
@@ -134,6 +134,9 @@ struct sw_flow_key {u8 src[ETH_ALEN ];/* Ethernet source address. */ u8 dst[ETH_ALEN ];/* Ethernet destination address. */ __be16 tci;/* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */ +__be16 ctci;/* 0 if no CVLAN, VLAN_TAG_PRESENT set +* otherwise. +*/ __be16 type;/* Ethernet frame type. */ } eth;union {
net/openvswitch/flow_netlink.c +183 -29
265 lines not on this page · raw message
Signature · 1 line
2.1.0
Re: [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
Quoted 5 lines · Thomas F Herbert
Add support for 802.1ad including the ability to push and pop double tagged vlans.
Signed-off-by Thomas F Herbert <[email protected]>---
...
...
Quoted 5 lines
net/openvswitch/flow_netlink.c +0
-0
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 780
Quoted 10 lines
net/openvswitch/flow_netlink.c +7
-0@@ -771,6 +771,51 @@ static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs,return 0 ; } +static int eth_type_vlan (__be16 ethertype) +{ +if (ethertype ==htons (ETH_P_8021Q ) || ethertype ==htons (ETH_P_8021AD )) +return true; +return false; +} +
You have open-coded same comparison in flow.c. May be you can define it in header file if_vlan.h. There are some use cases in that file too for this function.
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 784
Quoted 4 lines
net/openvswitch/flow_netlink.c +4
-0+static int _ovs_vlan_from_nlattrs (struct sw_flow_match *match,u64 attrs, +const struct nlattr **a,bool is_mask, +bool log) +{
_ovs_vlan_from_nlattrs() is setting 8021AD, can you change name so that it is clear.
Quoted 7 lines
net/openvswitch/flow_netlink.c +7
-0+/* This should be nested inner or "customer" tci" */ +if (attrs & (1 <<OVS_KEY_ATTR_VLAN )) { +__be16 ctci; + + ctci =nla_get_be16 (a[OVS_KEY_ATTR_VLAN ]); +if (!(ctci &htons (VLAN_TAG_PRESENT ))) { +if (is_mask)
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 801
Quoted 10 lines
net/openvswitch/flow_netlink.c +10
-0+OVS_NLERR (log,"VLAN CTCI mask does not have exact match for VLAN_TAG_PRESENT bit." ); + else + OVS_NLERR(log, "VLAN CTCI does not have VLAN_TAG_PRESENT bit set."); + +return -EINVAL ; + } +if (is_mask) +SW_FLOW_KEY_PUT (match, eth.ctci,htons (0xffff ), + is_mask); +else
8021AD mask from user parameters is ignored and 0xffff is set. You need to set default 0xffff mask for ctci and then override it with user mask if given in the key.
Quoted 7 lines
net/openvswitch/flow_netlink.c +7
-0+SW_FLOW_KEY_PUT (match, eth.ctci, ctci, is_mask); + } +return 0 ; +} + +static int ovs_vlan_from_nlattrs (struct sw_flow_match *match,u64 attrs, +const struct nlattr **a,bool log)
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 818
Quoted 10 lines
net/openvswitch/flow_netlink.c +10
-0+{ +return _ovs_vlan_from_nlattrs (match, attrs, a, false, log); +} + +static int ovs_vlan_mask_from_nlattrs (struct sw_flow_match *match,u64 attrs, +const struct nlattr **a,bool log) +{ +return _ovs_vlan_from_nlattrs (match, attrs, a, true, log); +} +
I do not see value in these functions. Can you directly call _ovs_vlan_from_nlattrs().
Quoted 28 lines
net/openvswitch/flow_netlink.c +21
-0static int ovs_key_from_nlattrs (struct sw_flow_match *match,u64 attrs,const struct nlattr **a,bool is_mask,bool log)@@ -1024,6 +1069,113 @@ static void mask_set_nlattr(struct nlattr *attr, u8 val)nlattr_set (attr, val, ovs_key_lens); } +static int _parse_vlan_from_nlattrs (const struct nlattr *nla, +struct sw_flow_match *match, +u64 *key_attrs, +const struct nlattr **a,bool is_mask, +bool log) +{ +int err; +__be16 tci; + +if (!is_mask) { +u64 v_attrs =0 ; + + tci =nla_get_be16 (a[OVS_KEY_ATTR_VLAN ]); + +if (tci &htons (VLAN_TAG_PRESENT )) { +if (unlikely ((nla_get_be16 (a[OVS_KEY_ATTR_ETHERTYPE ]) == +htons (ETH_P_8021AD )))) { + err =parse_flow_nlattrs (nla, a, &v_attrs, log); +if (err) +return err; +if (v_attrs) {
net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1102
Quoted 10 lines
net/openvswitch/flow_netlink.c +10
-0+ err =ovs_vlan_from_nlattrs (match, + v_attrs, a, + log); +if (err) +return err; + } +/* Insure that tci key attribute isn't +* overwritten by encapsulated customer tci. +*/ + v_attrs &= ~(1 <<OVS_KEY_ATTR_VLAN );
We also need to clear v_attrs when key has single vlan tag which is else part of this block.
Quoted 8 lines
net/openvswitch/flow_netlink.c +8
-0+ *key_attrs |= v_attrs; + }else { + err =parse_flow_nlattrs (nla, a, key_attrs, + log); +if (err) +return err; + } + }else if (!tci) {
net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1120
Quoted 10 lines
net/openvswitch/flow_netlink.c +10
-0+/* Corner case for truncated 802.1Q header. */ +if (nla_len (nla)) { +OVS_NLERR (log,"Truncated 802.1Q header has non-zero encap attribute." ); +return -EINVAL ; + } + }else { +OVS_NLERR (log,"Encap attr is set for non-VLAN frame" ); +return -EINVAL ; + } +
For double vlan tag case we need to have double encap attributes in flow key; one for each tag. So flow key should look like:
eth_type(0x88A8),vlan(vid=10),encap(eth_type(0x08100), vlan(vid=20), encap(eth_type(0x0800), ...))
Can you adjust vlan parsing code according ?
Quoted 8 lines
net/openvswitch/flow_netlink.c +8
-0+ }else { +u64 mask_v_attrs =0 ; + + tci =0 ; +if (a[OVS_KEY_ATTR_VLAN ]) + tci =nla_get_be16 (a[OVS_KEY_ATTR_VLAN ]); + +if (!(tci &htons (VLAN_TAG_PRESENT ))) {
...
...
net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1168
Quoted 9 lines
net/openvswitch/flow_netlink.c +9
-0+ +static int parse_vlan_from_nlattrs (const struct nlattr *nla, +struct sw_flow_match *match, +u64 *key_attrs, +const struct nlattr **a, +bool log) +{ +return _parse_vlan_from_nlattrs (nla, match, key_attrs, a, false, log); +}
You can move the key parsing block from _parse_vlan_from_nlattrs() here and move mask block in function bellow and get ride of _parse_vlan_from_nlattrs() function.
Quoted 11 lines
net/openvswitch/flow_netlink.c +10
-0+ +static int parse_vlan_mask_from_nlattrs (const struct nlattr *nla, +struct sw_flow_match *match, +u64 *key_attrs, +const struct nlattr **a, +bool log) +{ +return _parse_vlan_from_nlattrs (nla, match, key_attrs, a, true, log); +} +/**
A message that is not in the archive (sent off-list or not yet received): CALnjE+pY4b4WxJbu8qzuexq+hcagNA8iwQ0DsPuh_o-PRaP8KA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
Quoted 6 lines · Pravin Shelar
Quoted 4 lines · Thomas F Herbert
Add support for 802.1ad including the ability to push and pop double tagged vlans.
Signed-off-by Thomas F Herbert <[email protected]>
...
Quoted 8 lines
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 801
Quoted 4 lines
net/openvswitch/flow_netlink.c +4
-0+if (is_mask) +SW_FLOW_KEY_PUT (match, eth.ctci,htons (0xffff ), + is_mask); +else 8021AD mask from user parameters is ignored and
0xffffis set. You need to set default0xffffmask for ctci and then override it with user mask if given in the key.
Pravin, once again, thanks for your review.
I am thinking you are correct. I did it this way because it is the way single tagged vlans are handled in the original vlan code. Which raises two issues. 1. When I change this, I should also change the tci code for consistency and should that be a separate patch? 2. The implication is that so far all vlan vid mask matching has been done in user space only.
Quoted 3 lines
Quoted 3 lines
(quoted diff) +2
-0+ SW_FLOW_KEY_PUT(match, eth.ctci, ctci, is_mask); + }
...
Quoted 6 lines
net/openvswitch/flow_netlink.c static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, · line 818
Quoted 4 lines
net/openvswitch/flow_netlink.c +4
-0+{ +return _ovs_vlan_from_nlattrs (match, attrs, a, true, log); +} +I do not see value in these functions. Can you directly call
_ovs_vlan_from_nlattrs().
OK
Quoted 34 lines
Quoted 34 lines
static int ovs_key_from_nlattrs(struct
sw_flow_match*match, u64 attrs,
const struct nlattr **a, boolis_mask, bool log)net/openvswitch/flow_netlink.c +26
-0@@ -1024,6 +1069,113 @@ static void mask_set_nlattr(struct nlattr *attr, u8 val)nlattr_set (attr, val, ovs_key_lens); } +static int _parse_vlan_from_nlattrs (const struct nlattr *nla, +struct sw_flow_match *match, +u64 *key_attrs, +const struct nlattr **a,bool is_mask, +bool log) +{ +int err; +__be16 tci; + +if (!is_mask) { +u64 v_attrs =0 ; + + tci =nla_get_be16 (a[OVS_KEY_ATTR_VLAN ]); + +if (tci &htons (VLAN_TAG_PRESENT )) { +if (unlikely ((nla_get_be16 (a[OVS_KEY_ATTR_ETHERTYPE ]) == +htons (ETH_P_8021AD )))) { + err =parse_flow_nlattrs (nla, a, &v_attrs, log); +if (err) +return err; +if (v_attrs) { + err =ovs_vlan_from_nlattrs (match, + v_attrs, a, + log); +if (err) +return err;
Quoted 8 lines
net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1102
Quoted 5 lines
net/openvswitch/flow_netlink.c +5
-0+ } +/* Insure that tci key attribute isn't +* overwritten by encapsulated customer tci. +*/ + v_attrs &= ~(1 <<OVS_KEY_ATTR_VLAN );We also need to clear
v_attrswhen key has single vlan tag which is else part of this block.
This code is implemented this way because I have been using only a single encapsulation level for double tagged vlans. I sneak the inner tag into the encap and and then clear it here because the flow key has only one attribute type for vlan and v_attrs is only used inside the encapsulation. Along with this change, I can also update Documentation/networking/openvswitch.txt to show the double nested encapsulation flow key.
Quoted 19 lines
Quoted 9 lines
net/openvswitch/flow_netlink.c +8
-0+ *key_attrs |= v_attrs; + }else { + err =parse_flow_nlattrs (nla, a, key_attrs, + log); +if (err) +return err; + } + }else if (!tci) {net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1120
Quoted 10 lines
net/openvswitch/flow_netlink.c +10
-0+/* Corner case for truncated 802.1Q header. */ +if (nla_len (nla)) { +OVS_NLERR (log,"Truncated 802.1Q header has non-zero encap attribute." ); +return -EINVAL ; + } + }else { +OVS_NLERR (log,"Encap attr is set for non-VLAN frame" ); +return -EINVAL ; + } +
Quoted 7 lines
For double vlan tag case we need to have double encap attributes in flow key; one for each tag. So flow key should look like:
eth_type(0x88A8),vlan(vid=10),encap(eth_type(0x08100), vlan(vid=20),
encap(eth_type(0x0800), ...))Can you adjust vlan parsing code according ?
Yes, I think you are right. I should change the code to use two levels of encapsulation for double tagged vlans. This also would be the best for consistency with future implementation of 802.1ah.
Quoted 2 lines
...
Quoted 6 lines
net/openvswitch/flow_netlink.c static void mask_set_nlattr(struct nlattr *attr, u8 val) · line 1168
Quoted 3 lines
net/openvswitch/flow_netlink.c +3
-0+{ +return _parse_vlan_from_nlattrs (nla, match, key_attrs, a, false, log); +}You can move the key parsing block from
_parse_vlan_from_nlattrs()here and move mask block in function bellow and get ride of_parse_vlan_from_nlattrs()function.
OK. I think this will make the code more straight forward.
Quoted 12 lines
Quoted 12 lines
net/openvswitch/flow_netlink.c +10
-0+ +static int parse_vlan_mask_from_nlattrs (const struct nlattr *nla, +struct sw_flow_match *match, +u64 *key_attrs, +const struct nlattr **a, +bool log) +{ +return _parse_vlan_from_nlattrs (nla, match, key_attrs, a, true, log); +} +/**
Signature · 5 lines
Thomas F. Herbert _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev