On Wed, Sep 10, 2014 at 3:16 AM, Joe Stringer <joestrin...@nicira.com> wrote: > Reduce duplicate code by using nla_policy and nla_parse_strict(). > Thanks for simplifying this code. I have couple of comments.
> Signed-off-by: Joe Stringer <joestrin...@nicira.com> > --- > v2: Remove attrs bitmasks from most functions. > Remove key/mask wrappers for parse_nlattrs(). > Rebase against flag-based nla_parse_strict(). > --- > datapath/flow_netlink.c | 420 > ++++++++++++++++++++++------------------------- > 1 file changed, 192 insertions(+), 228 deletions(-) > > diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c > index 6c74841..2e3e181 100644 > --- a/datapath/flow_netlink.c > +++ b/datapath/flow_netlink.c > @@ -111,11 +111,26 @@ static void update_range(struct sw_flow_match *match, > sizeof((match)->key->field)); \ > } while (0) > > +static u64 attrs_to_bitmask(const struct nlattr **a, size_t len) > +{ > + size_t i; > + u64 mask = 0; > + > + for (i = 0; i < len; i++) > + if (a[i]) > + mask |= (1ULL << i); > + return mask; > +} > + > static bool match_validate(const struct sw_flow_match *match, > - u64 key_attrs, u64 mask_attrs) > + const struct nlattr **key_attrs, > + const struct nlattr **mask_attrs) > { > u64 key_expected = 1ULL << OVS_KEY_ATTR_ETHERNET; > - u64 mask_allowed = key_attrs; /* At most allow all key attributes */ > + u64 attrs, mask_allowed; > + > + /* At most allow all key attributes */ > + mask_allowed = attrs_to_bitmask(key_attrs, OVS_KEY_ATTR_MAX); > > /* The following mask attributes allowed only if they > * pass the validation tests. */ > @@ -229,17 +244,19 @@ static bool match_validate(const struct sw_flow_match > *match, > } > } > > - if ((key_attrs & key_expected) != key_expected) { > + attrs = attrs_to_bitmask(key_attrs, OVS_KEY_ATTR_MAX); > + if ((attrs & key_expected) != attrs) { > /* Key attributes check failed. */ > OVS_NLERR("Missing expected key attributes (key_attrs=%llx, > expected=%llx).\n", > - (unsigned long long)key_attrs, (unsigned long > long)key_expected); > + (unsigned long long)attrs, (unsigned long > long)key_expected); > return false; > } > > - if ((mask_attrs & mask_allowed) != mask_attrs) { > + attrs = attrs_to_bitmask(mask_attrs, OVS_KEY_ATTR_MAX); > + if ((attrs & mask_allowed) != attrs) { > /* Mask attributes check failed. */ > OVS_NLERR("Contain more than allowed mask fields > (mask_attrs=%llx, mask_allowed=%llx).\n", > - (unsigned long long)mask_attrs, (unsigned > long long)mask_allowed); > + (unsigned long long)attrs, (unsigned long > long)mask_allowed); > return false; > } > > @@ -287,161 +304,107 @@ size_t ovs_key_attr_size(void) > } > > /* The size of the argument for each %OVS_KEY_ATTR_* Netlink attribute. */ > -static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = { > - [OVS_KEY_ATTR_ENCAP] = -1, > - [OVS_KEY_ATTR_PRIORITY] = sizeof(u32), > - [OVS_KEY_ATTR_IN_PORT] = sizeof(u32), > - [OVS_KEY_ATTR_SKB_MARK] = sizeof(u32), > - [OVS_KEY_ATTR_ETHERNET] = sizeof(struct ovs_key_ethernet), > - [OVS_KEY_ATTR_VLAN] = sizeof(__be16), > - [OVS_KEY_ATTR_ETHERTYPE] = sizeof(__be16), > - [OVS_KEY_ATTR_IPV4] = sizeof(struct ovs_key_ipv4), > - [OVS_KEY_ATTR_IPV6] = sizeof(struct ovs_key_ipv6), > - [OVS_KEY_ATTR_TCP] = sizeof(struct ovs_key_tcp), > - [OVS_KEY_ATTR_TCP_FLAGS] = sizeof(__be16), > - [OVS_KEY_ATTR_UDP] = sizeof(struct ovs_key_udp), > - [OVS_KEY_ATTR_SCTP] = sizeof(struct ovs_key_sctp), > - [OVS_KEY_ATTR_ICMP] = sizeof(struct ovs_key_icmp), > - [OVS_KEY_ATTR_ICMPV6] = sizeof(struct ovs_key_icmpv6), > - [OVS_KEY_ATTR_ARP] = sizeof(struct ovs_key_arp), > - [OVS_KEY_ATTR_ND] = sizeof(struct ovs_key_nd), > - [OVS_KEY_ATTR_DP_HASH] = sizeof(u32), > - [OVS_KEY_ATTR_RECIRC_ID] = sizeof(u32), > - [OVS_KEY_ATTR_TUNNEL] = -1, > - [OVS_KEY_ATTR_MPLS] = sizeof(struct ovs_key_mpls), > +static const struct nla_policy ovs_key_policy[OVS_KEY_ATTR_MAX + 1] = { > + [OVS_KEY_ATTR_ENCAP] = { .type = NLA_NESTED }, > + [OVS_KEY_ATTR_PRIORITY] = { .len = sizeof(u32) }, > + [OVS_KEY_ATTR_IN_PORT] = { .len = sizeof(u32) }, > + [OVS_KEY_ATTR_SKB_MARK] = { .len = sizeof(u32) }, > + [OVS_KEY_ATTR_ETHERNET] = { .len = sizeof(struct ovs_key_ethernet) }, > + [OVS_KEY_ATTR_VLAN] = { .len = sizeof(__be16) }, > + [OVS_KEY_ATTR_ETHERTYPE] = { .len = sizeof(__be16) }, > + [OVS_KEY_ATTR_IPV4] = { .len = sizeof(struct ovs_key_ipv4) }, > + [OVS_KEY_ATTR_IPV6] = { .len = sizeof(struct ovs_key_ipv6) }, > + [OVS_KEY_ATTR_TCP] = { .len = sizeof(struct ovs_key_tcp) }, > + [OVS_KEY_ATTR_TCP_FLAGS] = { .len = sizeof(__be16) }, > + [OVS_KEY_ATTR_UDP] = { .len = sizeof(struct ovs_key_udp) }, > + [OVS_KEY_ATTR_SCTP] = { .len = sizeof(struct ovs_key_sctp) }, > + [OVS_KEY_ATTR_ICMP] = { .len = sizeof(struct ovs_key_icmp) }, > + [OVS_KEY_ATTR_ICMPV6] = { .len = sizeof(struct ovs_key_icmpv6) }, > + [OVS_KEY_ATTR_ARP] = { .len = sizeof(struct ovs_key_arp) }, > + [OVS_KEY_ATTR_ND] = { .len = sizeof(struct ovs_key_nd) }, > + [OVS_KEY_ATTR_DP_HASH] = { .len = sizeof(u32) }, > + [OVS_KEY_ATTR_RECIRC_ID] = { .len = sizeof(u32) }, > + [OVS_KEY_ATTR_TUNNEL] = { .type = NLA_NESTED }, > + [OVS_KEY_ATTR_MPLS] = { .len = sizeof(struct ovs_key_mpls) }, > }; > > -static bool is_all_zero(const u8 *fp, size_t size) > -{ > - int i; > - > - if (!fp) > - return false; > - > - for (i = 0; i < size; i++) > - if (fp[i]) > - return false; > - > - return true; > -} > - > -static int __parse_flow_nlattrs(const struct nlattr *attr, > - const struct nlattr *a[], > - u64 *attrsp, bool nz) > +static int parse_nlattrs(const struct nlattr *attr, > + const struct nlattr *a[], int maxtype, > + const struct nla_policy *policy, bool dup, bool nz) > { > - const struct nlattr *nla; > - u64 attrs; > - int rem; > - > - attrs = *attrsp; > - nla_for_each_nested(nla, attr, rem) { > - u16 type = nla_type(nla); > - int expected_len; > - > - if (type > OVS_KEY_ATTR_MAX) { > - OVS_NLERR("Unknown key attribute (type=%d, > max=%d).\n", > - type, OVS_KEY_ATTR_MAX); > - return -EINVAL; > - } > + int err; > + u8 flags = NLA_PARSE_F_NOINIT | NLA_PARSE_F_UNKNOWN > + | NLA_PARSE_F_TRAILING | NLA_PARSE_F_EXACT_LEN; > > - if (attrs & (1ULL << type)) { > - OVS_NLERR("Duplicate key attribute (type %d).\n", > type); > - return -EINVAL; > - } > + if (!dup) > + flags |= NLA_PARSE_F_DUPLICATE; > + if (nz) > + flags |= NLA_PARSE_F_NONZERO; > > - expected_len = ovs_key_lens[type]; > - if (nla_len(nla) != expected_len && expected_len != -1) { > - OVS_NLERR("Key attribute has unexpected length > (type=%d" > - ", length=%d, expected=%d).\n", type, > - nla_len(nla), expected_len); > - return -EINVAL; > - } > - > - if (!nz || !is_all_zero(nla_data(nla), expected_len)) { > - attrs |= 1ULL << type; > - a[type] = nla; > - } > - } > - if (rem) { > - OVS_NLERR("Message has %d unknown bytes.\n", rem); > - return -EINVAL; > - } > + err = nla_parse_strict(a, maxtype, nla_data(attr), nla_len(attr), > + policy, flags); > + if (err) > + return err; > > - *attrsp = attrs; > return 0; > } > > -static int parse_flow_mask_nlattrs(const struct nlattr *attr, > - const struct nlattr *a[], u64 *attrsp) > -{ > - return __parse_flow_nlattrs(attr, a, attrsp, true); > -} > - > -static int parse_flow_nlattrs(const struct nlattr *attr, > - const struct nlattr *a[], u64 *attrsp) > -{ > - return __parse_flow_nlattrs(attr, a, attrsp, false); > -} > - > static int ipv4_tun_from_nlattr(const struct nlattr *attr, > struct sw_flow_match *match, bool is_mask) > { > - struct nlattr *a; > - int rem; > + static const struct nla_policy > ovs_tunnel_key_policy[OVS_TUNNEL_KEY_ATTR_MAX + 1] = { > + [OVS_TUNNEL_KEY_ATTR_ID] = { .len = sizeof(u64) }, > + [OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = { .len = sizeof(u32) }, > + [OVS_TUNNEL_KEY_ATTR_IPV4_DST] = { .len = sizeof(u32) }, > + [OVS_TUNNEL_KEY_ATTR_TOS] = { .len = 1 }, > + [OVS_TUNNEL_KEY_ATTR_TTL] = { .len = 1 }, > + [OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT] = { .len = 0 }, > + [OVS_TUNNEL_KEY_ATTR_CSUM] = { .len = 0 }, > + [OVS_TUNNEL_KEY_ATTR_TP_SRC] = { .len = sizeof(u16) }, > + [OVS_TUNNEL_KEY_ATTR_TP_DST] = { .len = sizeof(u16) }, > + [OVS_TUNNEL_KEY_ATTR_OAM] = { .len = 0 }, > + [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = { .type = NLA_NESTED }, > + }; > + const struct nlattr *a[OVS_TUNNEL_KEY_ATTR_MAX + 1]; > bool ttl = false; > __be16 tun_flags = 0; > + enum ovs_tunnel_key_attr type; > + int err; > > - nla_for_each_nested(a, attr, rem) { > - int type = nla_type(a); > - static const u32 ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX > + 1] = { > - [OVS_TUNNEL_KEY_ATTR_ID] = sizeof(u64), > - [OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = sizeof(u32), > - [OVS_TUNNEL_KEY_ATTR_IPV4_DST] = sizeof(u32), > - [OVS_TUNNEL_KEY_ATTR_TOS] = 1, > - [OVS_TUNNEL_KEY_ATTR_TTL] = 1, > - [OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT] = 0, > - [OVS_TUNNEL_KEY_ATTR_CSUM] = 0, > - [OVS_TUNNEL_KEY_ATTR_TP_SRC] = sizeof(u16), > - [OVS_TUNNEL_KEY_ATTR_TP_DST] = sizeof(u16), > - [OVS_TUNNEL_KEY_ATTR_OAM] = 0, > - [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = -1, > - }; > + memset(a, 0, sizeof(*a) * (OVS_TUNNEL_KEY_ATTR_MAX + 1)); > + err = parse_nlattrs(attr, a, OVS_TUNNEL_KEY_ATTR_MAX, > + ovs_tunnel_key_policy, true, is_mask); > + if (err) > + return err; > > - if (type > OVS_TUNNEL_KEY_ATTR_MAX) { > - OVS_NLERR("Unknown IPv4 tunnel attribute (type=%d, > max=%d).\n", > - type, OVS_TUNNEL_KEY_ATTR_MAX); > - return -EINVAL; > - } > + for (type = 0; type <= OVS_TUNNEL_KEY_ATTR_MAX; type++) { > + const struct nlattr *nla; > > - if (ovs_tunnel_key_lens[type] != nla_len(a) && > - ovs_tunnel_key_lens[type] != -1) { > - OVS_NLERR("IPv4 tunnel attribute type has unexpected " > - " length (type=%d, length=%d, > expected=%d).\n", > - type, nla_len(a), > ovs_tunnel_key_lens[type]); > - return -EINVAL; > - } > + if (!a[type]) > + continue; > > + nla = a[type]; > switch (type) { > case OVS_TUNNEL_KEY_ATTR_ID: > SW_FLOW_KEY_PUT(match, tun_key.tun_id, > - nla_get_be64(a), is_mask); > + nla_get_be64(nla), is_mask); > tun_flags |= TUNNEL_KEY; > break; > case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: > SW_FLOW_KEY_PUT(match, tun_key.ipv4_src, > - nla_get_be32(a), is_mask); > + nla_get_be32(nla), is_mask); > break; > case OVS_TUNNEL_KEY_ATTR_IPV4_DST: > SW_FLOW_KEY_PUT(match, tun_key.ipv4_dst, > - nla_get_be32(a), is_mask); > + nla_get_be32(nla), is_mask); > break; > case OVS_TUNNEL_KEY_ATTR_TOS: > SW_FLOW_KEY_PUT(match, tun_key.ipv4_tos, > - nla_get_u8(a), is_mask); > + nla_get_u8(nla), is_mask); > break; > case OVS_TUNNEL_KEY_ATTR_TTL: > SW_FLOW_KEY_PUT(match, tun_key.ipv4_ttl, > - nla_get_u8(a), is_mask); > + nla_get_u8(nla), is_mask); > ttl = true; > break; > case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT: > @@ -452,29 +415,29 @@ static int ipv4_tun_from_nlattr(const struct nlattr > *attr, > break; > case OVS_TUNNEL_KEY_ATTR_TP_SRC: > SW_FLOW_KEY_PUT(match, tun_key.tp_src, > - nla_get_be16(a), is_mask); > + nla_get_be16(nla), is_mask); > break; > case OVS_TUNNEL_KEY_ATTR_TP_DST: > SW_FLOW_KEY_PUT(match, tun_key.tp_dst, > - nla_get_be16(a), is_mask); > + nla_get_be16(nla), is_mask); > break; > case OVS_TUNNEL_KEY_ATTR_OAM: > tun_flags |= TUNNEL_OAM; > break; > case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS: > tun_flags |= TUNNEL_OPTIONS_PRESENT; > - if (nla_len(a) > sizeof(match->key->tun_opts)) { > + if (nla_len(nla) > sizeof(match->key->tun_opts)) { > OVS_NLERR("Geneve option length exceeds " > "maximum size (len %d, max %zu).\n", > - nla_len(a), > + nla_len(nla), > sizeof(match->key->tun_opts)); > return -EINVAL; > } > > - if (nla_len(a) % 4 != 0) { > + if (nla_len(nla) % 4 != 0) { > OVS_NLERR("Geneve option length is not " > "a multiple of 4 (len %d).\n", > - nla_len(a)); > + nla_len(nla)); > return -EINVAL; > } > > @@ -483,7 +446,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr, > * additional options will be silently matched. > */ > if (!is_mask) { > - SW_FLOW_KEY_PUT(match, tun_opts_len, > nla_len(a), > + SW_FLOW_KEY_PUT(match, tun_opts_len, > nla_len(nla), > false); > } else { > /* This is somewhat unusual because it looks > at > @@ -496,10 +459,10 @@ static int ipv4_tun_from_nlattr(const struct nlattr > *attr, > * variable length and we won't have the > * information later. > */ > - if (match->key->tun_opts_len != nla_len(a)) { > + if (match->key->tun_opts_len != nla_len(nla)) > { > OVS_NLERR("Geneve option key length > (%d)" > " is different from mask length > (%d).", > - match->key->tun_opts_len, > nla_len(a)); > + match->key->tun_opts_len, > nla_len(nla)); > return -EINVAL; > } > > @@ -509,8 +472,8 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr, > > SW_FLOW_KEY_MEMCPY_OFFSET(match, > (unsigned long)GENEVE_OPTS((struct > sw_flow_key *)0, > - nla_len(a)), > - nla_data(a), nla_len(a), is_mask); > + nla_len(nla)), > + nla_data(nla), nla_len(nla), is_mask); > break; > default: > OVS_NLERR("Unknown IPv4 tunnel attribute (%d).\n", > type); > @@ -520,11 +483,6 @@ static int ipv4_tun_from_nlattr(const struct nlattr > *attr, > > SW_FLOW_KEY_PUT(match, tun_key.tun_flags, tun_flags, is_mask); > > - if (rem > 0) { > - OVS_NLERR("IPv4 tunnel attribute has %d unknown bytes.\n", > rem); > - return -EINVAL; > - } > - > if (!is_mask) { > if (!match->key->tun_key.ipv4_dst) { > OVS_NLERR("IPv4 tunnel destination address is > zero.\n"); > @@ -611,30 +569,31 @@ int ovs_nla_put_egress_tunnel_key(struct sk_buff *skb, > egress_tun_info->options_len); > } > > -static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs, > +static int metadata_from_nlattrs(struct sw_flow_match *match, > const struct nlattr **a, bool is_mask) > { > - if (*attrs & (1ULL << OVS_KEY_ATTR_DP_HASH)) { > + if (a[OVS_KEY_ATTR_DP_HASH]) { > u32 hash_val = nla_get_u32(a[OVS_KEY_ATTR_DP_HASH]); > > SW_FLOW_KEY_PUT(match, ovs_flow_hash, hash_val, is_mask); > - *attrs &= ~(1ULL << OVS_KEY_ATTR_DP_HASH); > + a[OVS_KEY_ATTR_DP_HASH] = NULL; > } If you change a[], then match_validate() does not work. > > - if (*attrs & (1ULL << OVS_KEY_ATTR_RECIRC_ID)) { > + if (a[OVS_KEY_ATTR_RECIRC_ID]) { > u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]); > > SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask); > - *attrs &= ~(1ULL << OVS_KEY_ATTR_RECIRC_ID); > + a[OVS_KEY_ATTR_RECIRC_ID] = NULL; > } > > - if (*attrs & (1ULL << OVS_KEY_ATTR_PRIORITY)) { > - SW_FLOW_KEY_PUT(match, phy.priority, > - nla_get_u32(a[OVS_KEY_ATTR_PRIORITY]), is_mask); > - *attrs &= ~(1ULL << OVS_KEY_ATTR_PRIORITY); > + if (a[OVS_KEY_ATTR_PRIORITY]) { > + u32 priority = nla_get_u32(a[OVS_KEY_ATTR_PRIORITY]); > + > + SW_FLOW_KEY_PUT(match, phy.priority, priority, is_mask); > + a[OVS_KEY_ATTR_PRIORITY] = NULL; > } > > - if (*attrs & (1ULL << OVS_KEY_ATTR_IN_PORT)) { > + if (a[OVS_KEY_ATTR_IN_PORT]) { > u32 in_port = nla_get_u32(a[OVS_KEY_ATTR_IN_PORT]); > > if (is_mask) { > @@ -646,36 +605,36 @@ static int metadata_from_nlattrs(struct sw_flow_match > *match, u64 *attrs, > } > > SW_FLOW_KEY_PUT(match, phy.in_port, in_port, is_mask); > - *attrs &= ~(1ULL << OVS_KEY_ATTR_IN_PORT); > + a[OVS_KEY_ATTR_IN_PORT] = NULL; > } else if (!is_mask) { > SW_FLOW_KEY_PUT(match, phy.in_port, DP_MAX_PORTS, is_mask); > } > > - if (*attrs & (1ULL << OVS_KEY_ATTR_SKB_MARK)) { > + if (a[OVS_KEY_ATTR_SKB_MARK]) { > uint32_t mark = nla_get_u32(a[OVS_KEY_ATTR_SKB_MARK]); > > SW_FLOW_KEY_PUT(match, phy.skb_mark, mark, is_mask); > - *attrs &= ~(1ULL << OVS_KEY_ATTR_SKB_MARK); > + a[OVS_KEY_ATTR_SKB_MARK] = NULL; > } > - if (*attrs & (1ULL << OVS_KEY_ATTR_TUNNEL)) { > + if (a[OVS_KEY_ATTR_TUNNEL]) { > if (ipv4_tun_from_nlattr(a[OVS_KEY_ATTR_TUNNEL], match, > is_mask)) > return -EINVAL; > - *attrs &= ~(1ULL << OVS_KEY_ATTR_TUNNEL); > + a[OVS_KEY_ATTR_TUNNEL] = NULL; > } > return 0; > } > > -static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs, > +static int ovs_key_from_nlattrs(struct sw_flow_match *match, > const struct nlattr **a, bool is_mask) > { > - int err; > + int i, err; > > - err = metadata_from_nlattrs(match, &attrs, a, is_mask); > + err = metadata_from_nlattrs(match, a, is_mask); > if (err) > return err; > > - if (attrs & (1ULL << OVS_KEY_ATTR_ETHERNET)) { > + if (a[OVS_KEY_ATTR_ETHERNET]) { > const struct ovs_key_ethernet *eth_key; > > eth_key = nla_data(a[OVS_KEY_ATTR_ETHERNET]); > @@ -683,10 +642,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > eth_key->eth_src, ETH_ALEN, is_mask); > SW_FLOW_KEY_MEMCPY(match, eth.dst, > eth_key->eth_dst, ETH_ALEN, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERNET); > + a[OVS_KEY_ATTR_ETHERNET] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_VLAN)) { > + if (a[OVS_KEY_ATTR_VLAN]) { > __be16 tci; > > tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]); > @@ -700,10 +659,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > } > > SW_FLOW_KEY_PUT(match, eth.tci, tci, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_VLAN); > + a[OVS_KEY_ATTR_VLAN] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) { > + if (a[OVS_KEY_ATTR_ETHERTYPE]) { > __be16 eth_type; > > eth_type = nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]); > @@ -717,12 +676,12 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > } > > SW_FLOW_KEY_PUT(match, eth.type, eth_type, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE); > + a[OVS_KEY_ATTR_ETHERTYPE] = NULL; > } else if (!is_mask) { > SW_FLOW_KEY_PUT(match, eth.type, htons(ETH_P_802_2), is_mask); > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_IPV4)) { > + if (a[OVS_KEY_ATTR_IPV4]) { > const struct ovs_key_ipv4 *ipv4_key; > > ipv4_key = nla_data(a[OVS_KEY_ATTR_IPV4]); > @@ -743,10 +702,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > ipv4_key->ipv4_src, is_mask); > SW_FLOW_KEY_PUT(match, ipv4.addr.dst, > ipv4_key->ipv4_dst, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4); > + a[OVS_KEY_ATTR_IPV4] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_IPV6)) { > + if (a[OVS_KEY_ATTR_IPV6]) { > const struct ovs_key_ipv6 *ipv6_key; > > ipv6_key = nla_data(a[OVS_KEY_ATTR_IPV6]); > @@ -779,10 +738,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > sizeof(match->key->ipv6.addr.dst), > is_mask); > > - attrs &= ~(1ULL << OVS_KEY_ATTR_IPV6); > + a[OVS_KEY_ATTR_IPV6] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_ARP)) { > + if (a[OVS_KEY_ATTR_ARP]) { > const struct ovs_key_arp *arp_key; > > arp_key = nla_data(a[OVS_KEY_ATTR_ARP]); > @@ -803,54 +762,54 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > SW_FLOW_KEY_MEMCPY(match, ipv4.arp.tha, > arp_key->arp_tha, ETH_ALEN, is_mask); > > - attrs &= ~(1ULL << OVS_KEY_ATTR_ARP); > + a[OVS_KEY_ATTR_ARP] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_MPLS)) { > + if (a[OVS_KEY_ATTR_MPLS]) { > const struct ovs_key_mpls *mpls_key; > > mpls_key = nla_data(a[OVS_KEY_ATTR_MPLS]); > SW_FLOW_KEY_PUT(match, mpls.top_lse, > mpls_key->mpls_lse, is_mask); > > - attrs &= ~(1ULL << OVS_KEY_ATTR_MPLS); > + a[OVS_KEY_ATTR_MPLS] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_TCP)) { > + if (a[OVS_KEY_ATTR_TCP]) { > const struct ovs_key_tcp *tcp_key; > > tcp_key = nla_data(a[OVS_KEY_ATTR_TCP]); > SW_FLOW_KEY_PUT(match, tp.src, tcp_key->tcp_src, is_mask); > SW_FLOW_KEY_PUT(match, tp.dst, tcp_key->tcp_dst, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_TCP); > + a[OVS_KEY_ATTR_TCP] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_TCP_FLAGS)) { > + if (a[OVS_KEY_ATTR_TCP_FLAGS]) { > SW_FLOW_KEY_PUT(match, tp.flags, > nla_get_be16(a[OVS_KEY_ATTR_TCP_FLAGS]), > is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_TCP_FLAGS); > + a[OVS_KEY_ATTR_TCP_FLAGS] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_UDP)) { > + if (a[OVS_KEY_ATTR_UDP]) { > const struct ovs_key_udp *udp_key; > > udp_key = nla_data(a[OVS_KEY_ATTR_UDP]); > SW_FLOW_KEY_PUT(match, tp.src, udp_key->udp_src, is_mask); > SW_FLOW_KEY_PUT(match, tp.dst, udp_key->udp_dst, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_UDP); > + a[OVS_KEY_ATTR_UDP] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_SCTP)) { > + if (a[OVS_KEY_ATTR_SCTP]) { > const struct ovs_key_sctp *sctp_key; > > sctp_key = nla_data(a[OVS_KEY_ATTR_SCTP]); > SW_FLOW_KEY_PUT(match, tp.src, sctp_key->sctp_src, is_mask); > SW_FLOW_KEY_PUT(match, tp.dst, sctp_key->sctp_dst, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_SCTP); > + a[OVS_KEY_ATTR_SCTP] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_ICMP)) { > + if (a[OVS_KEY_ATTR_ICMP]) { > const struct ovs_key_icmp *icmp_key; > > icmp_key = nla_data(a[OVS_KEY_ATTR_ICMP]); > @@ -858,10 +817,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > htons(icmp_key->icmp_type), is_mask); > SW_FLOW_KEY_PUT(match, tp.dst, > htons(icmp_key->icmp_code), is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_ICMP); > + a[OVS_KEY_ATTR_ICMP] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_ICMPV6)) { > + if (a[OVS_KEY_ATTR_ICMPV6]) { > const struct ovs_key_icmpv6 *icmpv6_key; > > icmpv6_key = nla_data(a[OVS_KEY_ATTR_ICMPV6]); > @@ -869,10 +828,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > htons(icmpv6_key->icmpv6_type), is_mask); > SW_FLOW_KEY_PUT(match, tp.dst, > htons(icmpv6_key->icmpv6_code), is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_ICMPV6); > + a[OVS_KEY_ATTR_ICMPV6] = NULL; > } > > - if (attrs & (1ULL << OVS_KEY_ATTR_ND)) { > + if (a[OVS_KEY_ATTR_ND]) { > const struct ovs_key_nd *nd_key; > > nd_key = nla_data(a[OVS_KEY_ATTR_ND]); > @@ -884,13 +843,14 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > nd_key->nd_sll, ETH_ALEN, is_mask); > SW_FLOW_KEY_MEMCPY(match, ipv6.nd.tll, > nd_key->nd_tll, ETH_ALEN, is_mask); > - attrs &= ~(1ULL << OVS_KEY_ATTR_ND); > + a[OVS_KEY_ATTR_ND] = NULL; > } > > - if (attrs != 0) { > - OVS_NLERR("Unknown key attributes (%llx).\n", > - (unsigned long long)attrs); > - return -EINVAL; > + for (i = 0; i < OVS_KEY_ATTR_MAX; i++) { > + if (a[i]) { > + OVS_NLERR("Unknown key attribute (%d).\n", i); > + return -EINVAL; > + } > } > Do we still need it with NLA_PARSE_F_UNKNOWN flag? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev