| Message ID | 1515191361-107730-3-git-send-email-u9012063@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: erspan: add support for openvswitch | expand |
On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012063@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 <u9012063@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 >
On Mon, Jan 8, 2018 at 4:03 PM, Pravin Shelar <pshelar@ovn.org> wrote: > On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012063@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? > Thanks. Yes, it will break the previous API, which uses OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS only to store 32-bit erspan index. Should I create another tunnel key attr? Something like: - OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* be32 ERSPAN index. */ + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV1, /* compatibility for erspan v1, be32 ERSPAN index. */ + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV2, /* supporting both v1 and v2 using Nested OVS_ERSPAN_OPT_* */ Regards, William
On Tue, Jan 9, 2018 at 7:17 AM, William Tu <u9012063@gmail.com> wrote: > On Mon, Jan 8, 2018 at 4:03 PM, Pravin Shelar <pshelar@ovn.org> wrote: >> On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012063@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? >> > Thanks. > Yes, it will break the previous API, which uses > OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS only to store 32-bit erspan index. > Should I create another tunnel key attr? > Something like: > - OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* be32 ERSPAN index. */ > + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV1, /* compatibility for > erspan v1, be32 ERSPAN index. */ > + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV2, /* supporting both > v1 and v2 using Nested OVS_ERSPAN_OPT_* */ > Sounds good, you could deprecate the V1 if v2 can handle both cases.
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; }
The patch adds support for configuring the erspan version II fields for openvswitch. Signed-off-by: William Tu <u9012063@gmail.com> --- include/uapi/linux/openvswitch.h | 12 +++- net/openvswitch/flow_netlink.c | 125 +++++++++++++++++++++++++++++++++++---- 2 files changed, 126 insertions(+), 11 deletions(-)