lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOrHB_DNKnYcftg46jx+SjkciP7c81OQod0xWZvyvum-R6DZYQ@mail.gmail.com>
Date:   Mon, 8 Jan 2018 16:03:22 -0800
From:   Pravin Shelar <pshelar@....org>
To:     William Tu <u9012063@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] openvswitch: add erspan version II support

On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012063@...il.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 <u9012063@...il.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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ