* [PATCH net-next V9 0/2] openvswitch: Add support for 802.1AD
@ 2015-05-13 0:06 Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V9 1/2] openvswitch: 802.1ad uapi changes Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing Thomas F Herbert
0 siblings, 2 replies; 5+ messages in thread
From: Thomas F Herbert @ 2015-05-13 0:06 UTC (permalink / raw)
To: netdev, pshelar; +Cc: therbert, ccp, dev, Thomas F Herbert
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(-)
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next V9 1/2] openvswitch: 802.1ad uapi changes.
2015-05-13 0:06 [PATCH net-next V9 0/2] openvswitch: Add support for 802.1AD Thomas F Herbert
@ 2015-05-13 0:06 ` Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing Thomas F Herbert
1 sibling, 0 replies; 5+ messages in thread
From: Thomas F Herbert @ 2015-05-13 0:06 UTC (permalink / raw)
To: netdev, pshelar; +Cc: therbert, ccp, dev, Thomas F Herbert
openvswitch: Add support for 8021.AD
Change the description of the VLAN tpid field.
Signed-off-by: Thomas F Herbert <thomasfherbert@gmail.com>
---
include/uapi/linux/openvswitch.h | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index bbd49a0..f2ccdef 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -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
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
2015-05-13 0:06 [PATCH net-next V9 0/2] openvswitch: Add support for 802.1AD Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V9 1/2] openvswitch: 802.1ad uapi changes Thomas F Herbert
@ 2015-05-13 0:06 ` Thomas F Herbert
2015-05-14 7:33 ` Pravin Shelar
1 sibling, 1 reply; 5+ messages in thread
From: Thomas F Herbert @ 2015-05-13 0:06 UTC (permalink / raw)
To: netdev, pshelar; +Cc: therbert, ccp, dev, Thomas F Herbert
Add support for 802.1ad including the ability to push and pop double
tagged vlans.
Signed-off-by: Thomas F Herbert <thomasfherbert@gmail.com>
---
net/openvswitch/flow.c | 82 +++++++++++++---
net/openvswitch/flow.h | 3 +
net/openvswitch/flow_netlink.c | 212 +++++++++++++++++++++++++++++++++++------
3 files changed, 256 insertions(+), 41 deletions(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 2dacc7b..9c73a2e 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -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;
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index a076e44..fa83c61 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -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 {
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index c691b1a..062e180 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -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;
+}
+
+static int _ovs_vlan_from_nlattrs(struct sw_flow_match *match, u64 attrs,
+ const struct nlattr **a, bool is_mask,
+ bool log)
+{
+ /* 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)
+ 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
+ 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)
+{
+ 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);
+}
+
static 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) {
+ 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);
+ *key_attrs |= v_attrs;
+ } else {
+ err = parse_flow_nlattrs(nla, a, key_attrs,
+ log);
+ if (err)
+ return err;
+ }
+ } else if (!tci) {
+ /* 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;
+ }
+
+ } 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))) {
+ OVS_NLERR(log, "VLAN tag present bit must have an exact match (tci_mask=%x).",
+ ntohs(tci));
+ err = -EINVAL;
+ return err;
+ }
+ err = parse_flow_mask_nlattrs(nla, a, &mask_v_attrs,
+ log);
+ if (err)
+ return err;
+
+ if (mask_v_attrs & (1ULL << OVS_KEY_ATTR_VLAN)) {
+ __be16 ctci = 0;
+
+ ctci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
+ err = ovs_vlan_mask_from_nlattrs(match, mask_v_attrs,
+ a, log);
+ if (err)
+ return err;
+
+ if (!(ctci & htons(VLAN_TAG_PRESENT))) {
+ OVS_NLERR(log, "VLAN ctag present bit must have an exact match (ctci_mask=%x).",
+ ntohs(ctci));
+ err = -EINVAL;
+ return err;
+ }
+ mask_v_attrs &= ~(1ULL << OVS_KEY_ATTR_VLAN);
+ *key_attrs |= mask_v_attrs;
+ }
+ }
+ return 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);
+}
+
+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);
+}
+
/**
* ovs_nla_get_match - parses Netlink attributes into a flow key and
* mask. In case the 'mask' is NULL, the flow is treated as exact match
@@ -1058,35 +1210,24 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if ((key_attrs & (1 << OVS_KEY_ATTR_ETHERNET)) &&
(key_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) &&
- (nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) == htons(ETH_P_8021Q))) {
- __be16 tci;
+ eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]))) {
- if (!((key_attrs & (1 << OVS_KEY_ATTR_VLAN)) &&
- (key_attrs & (1 << OVS_KEY_ATTR_ENCAP)))) {
+ if (!((key_attrs & (1ULL << OVS_KEY_ATTR_VLAN)) &&
+ (key_attrs & (1ULL << OVS_KEY_ATTR_ENCAP)))) {
OVS_NLERR(log, "Invalid Vlan frame.");
return -EINVAL;
}
key_attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE);
- tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
encap = a[OVS_KEY_ATTR_ENCAP];
key_attrs &= ~(1 << OVS_KEY_ATTR_ENCAP);
encap_valid = true;
- if (tci & htons(VLAN_TAG_PRESENT)) {
- err = parse_flow_nlattrs(encap, a, &key_attrs, log);
- if (err)
- return err;
- } else if (!tci) {
- /* Corner case for truncated 802.1Q header. */
- if (nla_len(encap)) {
- 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;
- }
+ err = parse_vlan_from_nlattrs(encap, match, &key_attrs, a,
+ log);
+ if (err)
+ return err;
+
}
err = ovs_key_from_nlattrs(match, key_attrs, a, false, log);
@@ -1132,7 +1273,6 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if (mask_attrs & 1 << OVS_KEY_ATTR_ENCAP) {
__be16 eth_type = 0;
- __be16 tci = 0;
if (!encap_valid) {
OVS_NLERR(log, "Encap mask attribute is set for non-VLAN frame.");
@@ -1158,15 +1298,12 @@ int ovs_nla_get_match(struct sw_flow_match *match,
goto free_newmask;
}
- if (a[OVS_KEY_ATTR_VLAN])
- tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
-
- if (!(tci & htons(VLAN_TAG_PRESENT))) {
- OVS_NLERR(log, "VLAN tag present bit must have an exact match (tci_mask=%x).",
- ntohs(tci));
- err = -EINVAL;
+ err = parse_vlan_mask_from_nlattrs(encap, match,
+ &mask_attrs,
+ a, log);
+ if (err)
goto free_newmask;
- }
+
}
err = ovs_key_from_nlattrs(match, mask_attrs, a, true, log);
@@ -1331,6 +1468,22 @@ static int __ovs_nla_put_key(const struct sw_flow_key *swkey,
encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP);
if (!swkey->eth.tci)
goto unencap;
+ } else if (swkey->eth.ctci || swkey->eth.type == htons(ETH_P_8021AD)) {
+ __be16 eth_type;
+
+ eth_type = !is_mask ? htons(ETH_P_8021AD) : htons(0xffff);
+ if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, eth_type) ||
+ nla_put_be16(skb, OVS_KEY_ATTR_VLAN, output->eth.tci))
+ goto nla_put_failure;
+ encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP);
+ if (!swkey->eth.tci)
+ goto unencap;
+ /* Customer tci is nested but uses same key attribute.
+ */
+ if (nla_put_be16(skb, OVS_KEY_ATTR_VLAN, output->eth.ctci))
+ goto nla_put_failure;
+ if (!swkey->eth.ctci)
+ goto unencap;
} else
encap = NULL;
@@ -2078,7 +2231,8 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
case OVS_ACTION_ATTR_PUSH_VLAN:
vlan = nla_data(a);
- if (vlan->vlan_tpid != htons(ETH_P_8021Q))
+ if ((vlan->vlan_tpid != htons(ETH_P_8021Q)) &&
+ (vlan->vlan_tpid != htons(ETH_P_8021AD)))
return -EINVAL;
if (!(vlan->vlan_tci & htons(VLAN_TAG_PRESENT)))
return -EINVAL;
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
2015-05-13 0:06 ` [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing Thomas F Herbert
@ 2015-05-14 7:33 ` Pravin Shelar
[not found] ` <CALnjE+pY4b4WxJbu8qzuexq+hcagNA8iwQ0DsPuh_o-PRaP8KA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Pravin Shelar @ 2015-05-14 7:33 UTC (permalink / raw)
To: Thomas F Herbert; +Cc: netdev, therbert, ccp, dev
On Tue, May 12, 2015 at 5:06 PM, Thomas F Herbert
<thomasfherbert@gmail.com> wrote:
> Add support for 802.1ad including the ability to push and pop double
> tagged vlans.
>
> Signed-off-by: Thomas F Herbert <thomasfherbert@gmail.com>
> ---
...
...
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index c691b1a..062e180 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -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.
> +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.
> + /* 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)
> + 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.
> + 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)
> +{
> + 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().
> static 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) {
> + 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.
> + *key_attrs |= v_attrs;
> + } else {
> + err = parse_flow_nlattrs(nla, a, key_attrs,
> + log);
> + if (err)
> + return err;
> + }
> + } else if (!tci) {
> + /* 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 ?
> + } 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))) {
...
...
> +
> +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.
> +
> +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);
> +}
> +
> /**
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing
[not found] ` <CALnjE+pY4b4WxJbu8qzuexq+hcagNA8iwQ0DsPuh_o-PRaP8KA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-05-19 16:55 ` Thomas F Herbert
0 siblings, 0 replies; 5+ messages in thread
From: Thomas F Herbert @ 2015-05-19 16:55 UTC (permalink / raw)
To: Pravin Shelar
Cc: ccp-HUUA3fb44xqsTnJN9+BGXg, netdev, dev-yBygre7rU0TnMu66kgdUjQ,
therbert-H+wXaHxf7aLQT0dZR+AlfA
On 5/14/15 3:33 AM, Pravin Shelar wrote:
> On Tue, May 12, 2015 at 5:06 PM, Thomas F Herbert
> <thomasfherbert@gmail.com> wrote:
>> Add support for 802.1ad including the ability to push and pop double
>> tagged vlans.
>>
>> Signed-off-by: Thomas F Herbert <thomasfherbert@gmail.com>
...
>> + 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.
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.
>
>> + SW_FLOW_KEY_PUT(match, eth.ctci, ctci, is_mask);
>> + }
...
>> +{
>> + 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
>
>> static 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) {
>> + 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.
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.
>
>> + *key_attrs |= v_attrs;
>> + } else {
>> + err = parse_flow_nlattrs(nla, a, key_attrs,
>> + log);
>> + if (err)
>> + return err;
>> + }
>> + } else if (!tci) {
>> + /* 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 ?
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.
>
>
...
>> +{
>> + 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.
>
>> +
>> +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);
>> +}
>> +
>> /**
--
Thomas F. Herbert
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-19 16:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13 0:06 [PATCH net-next V9 0/2] openvswitch: Add support for 802.1AD Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V9 1/2] openvswitch: 802.1ad uapi changes Thomas F Herbert
2015-05-13 0:06 ` [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing Thomas F Herbert
2015-05-14 7:33 ` Pravin Shelar
[not found] ` <CALnjE+pY4b4WxJbu8qzuexq+hcagNA8iwQ0DsPuh_o-PRaP8KA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-05-19 16:55 ` Thomas F Herbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).