On Sun, Oct 25, 2015 at 5:11 PM, Thomas F Herbert <thomasfherb...@gmail.com> 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. Outer > TPID is also encoded in the flow key. > > Signed-off-by: Thomas F Herbert <thomasfherb...@gmail.com> This patch does not apply on current master due to conflicts related net-branch merge.
> --- > net/openvswitch/actions.c | 6 +- > net/openvswitch/flow.c | 76 ++++++++++++---- > net/openvswitch/flow.h | 8 +- > net/openvswitch/flow_netlink.c | 199 > +++++++++++++++++++++++++++++++++++++---- > net/openvswitch/vport-netdev.c | 4 +- > 5 files changed, 252 insertions(+), 41 deletions(-) > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index c8db44a..ed19e2b 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -302,24 +302,68 @@ 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 (likely(!eth_type_vlan(qp->tpid))) > + return 0; > > - if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16))) > + if (unlikely(skb->len < sizeof(struct vlan_head) + sizeof(__be16))) > return 0; Why do we need extra sizeof(__be16) bytes here? > > - if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) + > - sizeof(__be16)))) > + if (unlikely(!pskb_may_pull(skb, sizeof(struct vlan_head) + > + 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)); > + 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 c92d6a2..7e90f8c 100644 > --- a/net/openvswitch/flow_netlink.c > +++ b/net/openvswitch/flow_netlink.c ... > + > +static int parse_vlan_from_nlattrs(const struct nlattr **nla, > + struct sw_flow_match *match, > + u64 *key_attrs, bool *ie_valid, > + const struct nlattr **a, bool is_mask, > + bool log) > +{ > + int err; > + const struct nlattr *encap; > + u64 v_attrs = 0; > + > + if (!is_mask) { > + err = __parse_vlan_from_nlattrs(nla, match, key_attrs, > + false, a, is_mask, log); > + if (err) > + return err; > + > + /* Another encap attribute here indicates > + * the presence of a double tagged vlan. > + */ > + encap = a[OVS_KEY_ATTR_ENCAP]; > + > + err = parse_flow_nlattrs(encap, a, &v_attrs, log); > + if (err) > + return err; > + > + if ((v_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) && > + eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]))) { > + if (!((v_attrs & (1 << OVS_KEY_ATTR_VLAN)) && > + (v_attrs & (1 << OVS_KEY_ATTR_ENCAP)))) { > + OVS_NLERR(log, "Invalid Inner VLAN frame"); > + return -EINVAL; > + } > + *ie_valid = true; > + err = __parse_vlan_from_nlattrs(&encap, match, > &v_attrs, > + true, a, is_mask, > log); > + if (err) > + return err; > + *key_attrs |= v_attrs; > + } > + } else { > + err = __parse_vlan_from_nlattrs(nla, match, key_attrs, > + false, a, is_mask, log); > + if (err) > + return err; > + > + encap = a[OVS_KEY_ATTR_ENCAP]; > + > + err = parse_flow_nlattrs(encap, a, &v_attrs, log); > + if (err) > + return err; > + > + if (v_attrs & 1 << OVS_KEY_ATTR_ENCAP) { Missing parentheses ... > @@ -1099,25 +1240,27 @@ 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))) { > + eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]))) { > __be16 tci; > > - 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)))) { This is not required change. > 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)) { After checks in encode_vlan_from_nlattrs() function there is no need to have checks here. > - err = parse_flow_nlattrs(encap, a, &key_attrs, log); > + err = parse_vlan_from_nlattrs(&encap, match, > + &key_attrs, > + &i_encap_valid, a, > false, > + log); > if (err) > return err; > + Added white space. > } else if (!tci) { > /* Corner case for truncated 802.1Q header. */ > if (nla_len(encap)) { > @@ -1169,7 +1312,7 @@ int ovs_nla_get_match(struct net *net, struct > sw_flow_match *match, > goto free_newmask; > > /* Always match on tci. */ > - SW_FLOW_KEY_PUT(match, eth.tci, htons(0xffff), true); > + SW_FLOW_KEY_PUT(match, eth.vlan.tci, htons(0xffff), true); Also need to exact match on inner tci. > > if (mask_attrs & 1 << OVS_KEY_ATTR_ENCAP) { > __be16 eth_type = 0; > @@ -1188,10 +1331,13 @@ int ovs_nla_get_match(struct net *net, struct > sw_flow_match *match, > if (eth_type == htons(0xffff)) { Same as above, after checks in encode_vlan_from_nlattrs() these checks looks redundant. > mask_attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE); > encap = a[OVS_KEY_ATTR_ENCAP]; > - err = parse_flow_mask_nlattrs(encap, a, > - &mask_attrs, > log); > + err = parse_vlan_from_nlattrs(&nla_mask, > match, > + &mask_attrs, > + &i_encap_valid, > + a, true, log); > if (err) > goto free_newmask; > + > } else { > OVS_NLERR(log, "VLAN frames must have an > exact match on the TPID (mask=%x).", > ntohs(eth_type)); ... > @@ -1368,17 +1515,29 @@ 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)) { > - __be16 eth_type; > - eth_type = !is_mask ? htons(ETH_P_8021Q) : htons(0xffff); > - if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, eth_type) || > - nla_put_be16(skb, OVS_KEY_ATTR_VLAN, output->eth.tci)) > + if (swkey->eth.vlan.tci || eth_type_vlan(swkey->eth.type)) { > + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, > + output->eth.vlan.tpid) || > + 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 > + if (swkey->eth.cvlan.tci) { > + /* Customer tci is nested but uses same key attribute. > + */ > + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, > + output->eth.cvlan.tpid) || > + 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; (!swkey->eth.cvlan.tci) is never going to be true. since inside if (swkey->eth.cvlan.tci) block. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev