On 5/14/15 3:33 AM, Pravin Shelar wrote:
On Tue, May 12, 2015 at 5:06 PM, Thomas F Herbert
<thomasfherb...@gmail.com> wrote:
Add support for 802.1ad including the ability to push and pop double
tagged vlans.
Signed-off-by: Thomas F Herbert <thomasfherb...@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
