On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012...@gmail.com> wrote:
> The patch adds support for configuring the erspan version II
> fields for openvswitch.
>
The patch looks good, But it could change userspace API for
OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, how are we going to handle
compatibility?
> Signed-off-by: William Tu <u9012...@gmail.com>
> ---
> include/uapi/linux/openvswitch.h | 12 +++-
> net/openvswitch/flow_netlink.c | 125
> +++++++++++++++++++++++++++++++++++----
> 2 files changed, 126 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h
> b/include/uapi/linux/openvswitch.h
> index 4265d7f9e1f2..3b1950c59a0c 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -273,6 +273,16 @@ enum {
>
> #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
>
> +enum {
> + OVS_ERSPAN_OPT_UNSPEC,
> + OVS_ERSPAN_OPT_IDX, /* be32 index */
> + OVS_ERSPAN_OPT_VER, /* u8 version number */
> + OVS_ERSPAN_OPT_DIR, /* u8 direction */
> + OVS_ERSPAN_OPT_HWID, /* u8 hardware ID */
> + __OVS_ERSPAN_OPT_MAX,
> +};
> +
> +#define OVS_ERSPAN_OPT_MAX (__OVS_ERSPAN_OPT_MAX - 1)
>
> /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
> */
> @@ -363,7 +373,7 @@ enum ovs_tunnel_key_attr {
> OVS_TUNNEL_KEY_ATTR_IPV6_SRC, /* struct in6_addr src IPv6
> address. */
> OVS_TUNNEL_KEY_ATTR_IPV6_DST, /* struct in6_addr dst IPv6
> address. */
> OVS_TUNNEL_KEY_ATTR_PAD,
> - OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* be32 ERSPAN index. */
> + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* Nested OVS_ERSPAN_OPT_* */
> __OVS_TUNNEL_KEY_ATTR_MAX
> };
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index bce1f78b0de5..696198cf3765 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -334,8 +334,10 @@ size_t ovs_tun_key_attr_size(void)
> * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
> */
> + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_SRC */
> - + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_DST */
> - + nla_total_size(4); /* OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS */
> + + nla_total_size(2); /* OVS_TUNNEL_KEY_ATTR_TP_DST */
> + /* OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with
> + * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
> + */
> }
>
> static size_t ovs_nsh_key_attr_size(void)
> @@ -386,6 +388,13 @@ static const struct ovs_len_tbl
> ovs_vxlan_ext_key_lens[OVS_VXLAN_EXT_MAX + 1] =
> [OVS_VXLAN_EXT_GBP] = { .len = sizeof(u32) },
> };
>
> +static const struct ovs_len_tbl ovs_erspan_opt_lens[OVS_ERSPAN_OPT_MAX + 1]
> = {
> + [OVS_ERSPAN_OPT_IDX] = { .len = sizeof(u32) },
> + [OVS_ERSPAN_OPT_VER] = { .len = sizeof(u8) },
> + [OVS_ERSPAN_OPT_DIR] = { .len = sizeof(u8) },
> + [OVS_ERSPAN_OPT_HWID] = { .len = sizeof(u8) },
> +};
> +
> static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX
> + 1] = {
> [OVS_TUNNEL_KEY_ATTR_ID] = { .len = sizeof(u64) },
> [OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = { .len = sizeof(u32) },
> @@ -402,7 +411,8 @@ static const struct ovs_len_tbl
> ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1]
> .next =
> ovs_vxlan_ext_key_lens },
> [OVS_TUNNEL_KEY_ATTR_IPV6_SRC] = { .len = sizeof(struct
> in6_addr) },
> [OVS_TUNNEL_KEY_ATTR_IPV6_DST] = { .len = sizeof(struct
> in6_addr) },
> - [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = sizeof(u32) },
> + [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = OVS_ATTR_NESTED,
> + .next = ovs_erspan_opt_lens },
> };
>
> static const struct ovs_len_tbl
> @@ -640,16 +650,78 @@ static int erspan_tun_opt_from_nlattr(const struct
> nlattr *attr,
> {
> unsigned long opt_key_offset;
> struct erspan_metadata opts;
> + struct nlattr *a;
> + u16 hwid, dir;
> + int rem;
>
> BUILD_BUG_ON(sizeof(opts) > sizeof(match->key->tun_opts));
>
> memset(&opts, 0, sizeof(opts));
> - opts.u.index = nla_get_be32(attr);
> + nla_for_each_nested(a, attr, rem) {
> + int type = nla_type(a);
>
> - /* Index has only 20-bit */
> - if (ntohl(opts.u.index) & ~INDEX_MASK) {
> - OVS_NLERR(log, "ERSPAN index number %x too large.",
> - ntohl(opts.u.index));
> + if (type > OVS_ERSPAN_OPT_MAX) {
> + OVS_NLERR(log, "ERSPAN option %d out of range max %d",
> + type, OVS_ERSPAN_OPT_MAX);
> + return -EINVAL;
> + }
> +
> + if (!check_attr_len(nla_len(a),
> + ovs_erspan_opt_lens[type].len)) {
> + OVS_NLERR(log, "ERSPAN option %d has unexpected len
> %d expected %d",
> + type, nla_len(a),
> + ovs_erspan_opt_lens[type].len);
> + return -EINVAL;
> + }
> +
> + switch (type) {
> + case OVS_ERSPAN_OPT_IDX:
> + opts.u.index = nla_get_be32(a);
> + if (ntohl(opts.u.index) & ~INDEX_MASK) {
> + OVS_NLERR(log,
> + "ERSPAN index number %x too large.",
> + ntohl(opts.u.index));
> + return -EINVAL;
> + }
> + break;
> + case OVS_ERSPAN_OPT_VER:
> + opts.version = nla_get_u8(a);
> + if (opts.version != 1 && opts.version != 2) {
> + OVS_NLERR(log,
> + "ERSPAN version %d not supported.",
> + opts.version);
> + return -EINVAL;
> + }
> + break;
> + case OVS_ERSPAN_OPT_DIR:
> + dir = nla_get_u8(a);
> + if (dir != 0 && dir != 1) {
> + OVS_NLERR(log,
> + "ERSPAN direction %d invalid.",
> + dir);
> + return -EINVAL;
> + }
> + opts.u.md2.dir = dir;
> + break;
> + case OVS_ERSPAN_OPT_HWID:
> + hwid = nla_get_u8(a);
> + if (hwid & ~(HWID_MASK >> HWID_OFFSET)) {
> + OVS_NLERR(log,
> + "ERSPAN hardware ID %x invalid.",
> + hwid);
> + return -EINVAL;
> + }
> + set_hwid(&opts.u.md2, hwid);
> + break;
> + default:
> + OVS_NLERR(log, "Unknown ERSPAN opt attribute %d",
> + type);
> + return -EINVAL;
> + }
> + }
> + if (rem) {
> + OVS_NLERR(log, "ERSPAN opt message has %d unknown bytes.",
> + rem);
> return -EINVAL;
> }
>
> @@ -846,6 +918,39 @@ static int vxlan_opt_to_nlattr(struct sk_buff *skb,
> return 0;
> }
>
> +static int erspan_opt_to_nlattr(struct sk_buff *skb,
> + const void *tun_opts, int swkey_tun_opts_len)
> +{
> + const struct erspan_metadata *opts = tun_opts;
> + struct nlattr *nla;
> +
> + nla = nla_nest_start(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS);
> + if (!nla)
> + return -EMSGSIZE;
> +
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_VER, opts->version) < 0)
> + return -EMSGSIZE;
> +
> + if (opts->version == 1) {
> + if (nla_put_be32(skb, OVS_ERSPAN_OPT_IDX, opts->u.index) < 0)
> + return -EMSGSIZE;
> +
> + } else if (opts->version == 2) {
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_DIR,
> + opts->u.md2.dir) < 0)
> + return -EMSGSIZE;
> +
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_HWID,
> + get_hwid(&opts->u.md2)) < 0)
> + return -EMSGSIZE;
> + } else {
> + return -EINVAL;
> + }
> +
> + nla_nest_end(skb, nla);
> + return 0;
> +}
> +
> static int __ip_tun_to_nlattr(struct sk_buff *skb,
> const struct ip_tunnel_key *output,
> const void *tun_opts, int swkey_tun_opts_len,
> @@ -906,8 +1011,8 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb,
> vxlan_opt_to_nlattr(skb, tun_opts,
> swkey_tun_opts_len))
> return -EMSGSIZE;
> else if (output->tun_flags & TUNNEL_ERSPAN_OPT &&
> - nla_put_be32(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,
> - ((struct erspan_metadata
> *)tun_opts)->u.index))
> + erspan_opt_to_nlattr(skb, tun_opts,
> + swkey_tun_opts_len))
> return -EMSGSIZE;
> }
>
> --
> 2.7.4
>
