On Thu, Sep 1, 2016 at 1:45 PM, Eric Garver <e...@erig.me> wrote: > Add support for 802.1ad including the ability to push and pop double > tagged vlans. Add support for 802.1ad to netlink parsing and flow > conversion. Uses double nested encap attributes to represent double > tagged vlan. Inner TPID encoded along with ctci in nested attributes. > > This is based on Thomas F Herbert's original v20 patch. I made some > small clean ups and bug fixes. > > Signed-off-by: Thomas F Herbert <thomasfherb...@gmail.com> > Signed-off-by: Eric Garver <e...@erig.me>
Thanks for working on this. This version looks pretty clone to complete. > --- > net/openvswitch/actions.c | 16 +-- > net/openvswitch/flow.c | 64 ++++++++---- > net/openvswitch/flow.h | 8 +- > net/openvswitch/flow_netlink.c | 227 > ++++++++++++++++++++++++++++++----------- > net/openvswitch/vport.c | 7 +- > 5 files changed, 235 insertions(+), 87 deletions(-) > ... > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 0ea128eeeab2..13f6ebdf379b 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -302,24 +302,56 @@ static bool icmp6hdr_ok(struct sk_buff *skb) > sizeof(struct icmp6hdr)); > } > > -static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key) > +/** > + * Parse vlan tag from vlan header. > + * Returns ERROR on memory error. > + * 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 *vlan) > { > - struct qtag_prefix { > - __be16 eth_type; /* ETH_P_8021Q */ > - __be16 tci; > - }; > - struct qtag_prefix *qp; > + struct vlan_head *qp = (struct vlan_head *)skb->data; > > - if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16))) > + if (likely(!eth_type_vlan(qp->tpid))) > return 0; > > - if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) + > - sizeof(__be16)))) > + if (unlikely(skb->len < sizeof(struct vlan_head) + sizeof(__be16))) > + return 0; > + > + if (unlikely(!pskb_may_pull(skb, sizeof(struct vlan_head) + > + sizeof(__be16)))) > return -ENOMEM; > pskb_may_pull() can change skb->data, so you need to refresh qp pointer. > - qp = (struct qtag_prefix *) skb->data; > - key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT); > - __skb_pull(skb, sizeof(struct qtag_prefix)); > + vlan->tci = qp->tci | htons(VLAN_TAG_PRESENT); > + vlan->tpid = qp->tpid; > + > + __skb_pull(skb, sizeof(struct vlan_head)); > + return 1; > +} > + ... ... > diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c > index c78a6a1476fb..fbe9e0e4792d 100644 > --- a/net/openvswitch/flow_netlink.c > +++ b/net/openvswitch/flow_netlink.c ... > +static int __parse_vlan_from_nlattrs(struct sw_flow_match *match, > + u64 *key_attrs, bool inner, > + const struct nlattr **a, bool is_mask, > + bool log) > +{ > + int err; > + const struct nlattr *encap; > + > + err = encode_vlan_from_nlattrs(match, a, is_mask, inner, log); > + if (err) > + return err; > + > + *key_attrs &= ~(1 << OVS_KEY_ATTR_ENCAP); > + > + /* Ensure that tci key attribute isn't > + * overwritten by encapsulated customer tci. > + * Ethertype is cleared because it is c_tpid. > + */ > + *key_attrs &= ~(1 << OVS_KEY_ATTR_VLAN); > + *key_attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE); > + > + encap = a[OVS_KEY_ATTR_ENCAP]; > + > + if (is_mask) > + err = parse_flow_mask_nlattrs(encap, a, key_attrs, log); > + else > + err = parse_flow_nlattrs(encap, a, key_attrs, log); > + > + return err; > +} > + > +static int parse_vlan_from_nlattrs(struct sw_flow_match *match, > + u64 *key_attrs, bool *ie_valid, > + const struct nlattr **a, bool is_mask, > + bool log) > +{ > + int err; > + > + err = __parse_vlan_from_nlattrs(match, key_attrs, > + false, a, is_mask, log); > + if (err) > + return err; > + > + if (!is_mask) { > + if ((*key_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) && > + 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)))) { > + OVS_NLERR(log, "Invalid Inner VLAN frame"); > + return -EINVAL; > + } > + *ie_valid = true; > + } > + } else { > + if (*key_attrs & (1 << OVS_KEY_ATTR_ENCAP)) { > + if (!*ie_valid) { > + OVS_NLERR(log, "Encap mask attribute is set > for non-CVLAN frame."); > + return -EINVAL; > + } > + } > + } > + This check can be moved to __parse_vlan_from_nlattrs(). This way there is no need to check ATTR_VLAN and ATTR_ENCAP here and in ovs_nla_get_match() function. ... > @@ -1182,11 +1301,11 @@ int ovs_nla_get_match(struct net *net, struct > sw_flow_match *match, > bool log) > { > const struct nlattr *a[OVS_KEY_ATTR_MAX + 1]; > - const struct nlattr *encap; > struct nlattr *newmask = NULL; > u64 key_attrs = 0; > u64 mask_attrs = 0; > bool encap_valid = false; > + bool i_encap_valid = false; > int err; > > err = parse_flow_nlattrs(nla_key, a, &key_attrs, log); > @@ -1195,35 +1314,20 @@ int ovs_nla_get_match(struct net *net, 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)))) { > OVS_NLERR(log, "Invalid Vlan frame."); > return -EINVAL; > } ... ... > @@ -1464,17 +1547,37 @@ static int __ovs_nla_put_key(const struct sw_flow_key > *swkey, > ether_addr_copy(eth_key->eth_src, output->eth.src); > ether_addr_copy(eth_key->eth_dst, output->eth.dst); > > - if (swkey->eth.tci || swkey->eth.type == htons(ETH_P_8021Q)) { > + if (swkey->eth.vlan.tci || eth_type_vlan(swkey->eth.type)) { > __be16 eth_type; > - eth_type = !is_mask ? htons(ETH_P_8021Q) : htons(0xffff); > + eth_type = !is_mask ? output->eth.vlan.tpid : htons(0xffff); > if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, eth_type) || > - nla_put_be16(skb, OVS_KEY_ATTR_VLAN, output->eth.tci)) > + nla_put_be16(skb, OVS_KEY_ATTR_VLAN, > output->eth.vlan.tci)) > goto nla_put_failure; > encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP); > - if (!swkey->eth.tci) > + if (!swkey->eth.vlan.tci) > goto unencap; > - } else > - encap = NULL; > + > + if (swkey->eth.cvlan.tci || eth_type_vlan(swkey->eth.type)) { > + /* Customer tci is nested but uses same key attribute. > + */ > + eth_type = !is_mask ? output->eth.cvlan.tpid : > htons(0xffff); > + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, > eth_type) || > + nla_put_be16(skb, OVS_KEY_ATTR_VLAN, > output->eth.cvlan.tci)) > + goto nla_put_failure; > + in_encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP); > + if (!swkey->eth.cvlan.tci) > + goto unencap; > + } > + } there could be helper function to serialize "struct vlan_head" to avoid duplicate code above. > + > + if (eth_type_vlan(swkey->eth.type) && encap && in_encap) { I think check for eth.type is sufficient here. encap and in_encap should be true in this case anyways. > + /* There are 3 VLAN tags, we don't know anything about the > rest of the > + * packet, so truncate here. > + */ > + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, > output->eth.type)) After this output->eth.type would be serialized three times, can you move the check for third vlan tag after it is serialized so as to avoid it. > + goto nla_put_failure; > + goto unencap; > + } > > if (swkey->eth.type == htons(ETH_P_802_2)) { > /* _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev