On Tue, Aug 26, 2014 at 4:50 PM, Joe Stringer <[email protected]> wrote:
> Signed-off-by: Joe Stringer <[email protected]>
> ---
> datapath/flow_netlink.c | 219
> +++++++++++++++++++----------------------------
> 1 file changed, 86 insertions(+), 133 deletions(-)
>
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 69d1919..aa2cece 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -287,161 +287,121 @@ 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)
> +static int parse_nlattrs(const struct nlattr *attr,
> + const struct nlattr *a[], int maxtype,
> + const struct nla_policy *policy,
> + u64 *attrsp, bool dup, bool nz)
> {
> - 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)
> -{
> - 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;
> - }
> -
> - if (attrs & (1ULL << type)) {
> - OVS_NLERR("Duplicate key attribute (type %d).\n",
> type);
> - return -EINVAL;
> - }
> -
> - 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;
> - }
> + u64 attrs = 0;
> + int i, err;
>
> - 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, dup, nz);
> + if (err)
> + return err;
>
> + for (i = 0; i < maxtype; i++)
> + if (a[i])
> + attrs |= (1ULL << i);
> *attrsp = attrs;
> +
I have not finished review but I was wonder if you can get rid of the
attributes bit mask also?
> 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);
> + return parse_nlattrs(attr, a, OVS_KEY_ATTR_MAX, ovs_key_policy,
> + attrsp, false, true);
> }
>
> static int parse_flow_nlattrs(const struct nlattr *attr,
> const struct nlattr *a[], u64 *attrsp)
> {
> - return __parse_flow_nlattrs(attr, a, attrsp, false);
> + return parse_nlattrs(attr, a, OVS_KEY_ATTR_MAX, ovs_key_policy,
> + attrsp, false, 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;
> + u64 attrs = 0;
> + 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,
> - };
> + err = parse_nlattrs(attr, a, OVS_TUNNEL_KEY_ATTR_MAX,
> + ovs_tunnel_key_policy, &attrs, 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 (!(attrs & (1ULL << 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 +412,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 +443,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 +456,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 +469,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 +480,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");
> @@ -903,10 +858,8 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool
> is_attr_mask_key)
>
> /* The nlattr stream should already have been validated */
> nla_for_each_nested(nla, attr, rem) {
> - /* We assume that ovs_key_lens[type] == -1 means that type is
> a
> - * nested attribute
> - */
> - if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
> + if (is_attr_mask_key &&
> + ovs_key_policy[nla_type(nla)].type == NLA_NESTED)
> nlattr_set(nla, val, false);
> else
> memset(nla_data(nla), val, nla_len(nla));
> @@ -1605,8 +1558,8 @@ static int validate_set(const struct nlattr *a,
> return -EINVAL;
>
> if (key_type > OVS_KEY_ATTR_MAX ||
> - (ovs_key_lens[key_type] != nla_len(ovs_key) &&
> - ovs_key_lens[key_type] != -1))
> + (ovs_key_policy[key_type].len != nla_len(ovs_key) &&
> + ovs_key_policy[key_type].type != NLA_NESTED))
> return -EINVAL;
>
> switch (key_type) {
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
