| Message ID | 1516820776-7995-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 Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote: > The patch adds support for openvswitch to configure erspan > v1 and v2. The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added > to uapi as a binary blob to support all ERSPAN v1 and v2's > fields. Note that Previous commit "openvswitch: Add erspan tunnel > support." was reverted since it does not design properly. > > Signed-off-by: William Tu <u9012063@gmail.com> > --- > include/uapi/linux/openvswitch.h | 2 +- > net/openvswitch/flow_netlink.c | 90 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 90 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h > index dcfab5e3b55c..158c2e45c0a5 100644 > --- a/include/uapi/linux/openvswitch.h > +++ b/include/uapi/linux/openvswitch.h > @@ -273,7 +273,6 @@ enum { > > #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1) > > - > /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels. > */ > enum { > @@ -363,6 +362,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, /* struct erspan_metadata */ > __OVS_TUNNEL_KEY_ATTR_MAX > }; > Since this is uapi, we need to define the struct erspan_metadata in a UAPI header file. Also lets move field 'version' to the begining of the struct for easy expansion later. struct erspan_metadata { int version; union { __be32 index; /* Version 1 (type II)*/ struct erspan_md2 md2; /* Version 2 (type III) */ } u; }; > diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c > index f143908b651d..9d00c24b2836 100644 > --- a/net/openvswitch/flow_netlink.c > +++ b/net/openvswitch/flow_netlink.c > @@ -49,6 +49,7 @@ > #include <net/mpls.h> > #include <net/vxlan.h> > #include <net/tun_proto.h> > +#include <net/erspan.h> > > #include "flow_netlink.h" > > @@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void) > + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_CSUM */ > + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_OAM */ > + nla_total_size(256) /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */ > - /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with > + /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and > + * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with > * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it. > */ > + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_SRC */ > @@ -400,6 +402,7 @@ 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 = OVS_ATTR_VARIABLE }, > }; > > static const struct ovs_len_tbl > @@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr, > return 0; > } > > +static int erspan_tun_opt_from_nlattr(const struct nlattr *a, > + struct sw_flow_match *match, bool is_mask, > + bool log) > +{ > + unsigned long opt_key_offset; > + > + BUILD_BUG_ON(sizeof(struct erspan_metadata) > > + sizeof(match->key->tun_opts)); > + > + if (nla_len(a) > sizeof(match->key->tun_opts)) { > + OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).", > + nla_len(a), sizeof(match->key->tun_opts)); > + return -EINVAL; > + } > + > + if (!is_mask) > + SW_FLOW_KEY_PUT(match, tun_opts_len, > + sizeof(struct erspan_metadata), false); > + else > + SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true); > + > + opt_key_offset = TUN_METADATA_OFFSET(nla_len(a)); > + SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a), > + nla_len(a), is_mask); > + return 0; > +} > + ... > @@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key) > return 0; > } > > +static int validate_erspan_opts(struct sw_flow_key *key, bool log) > +{ I do not see any need to validate ERSPAN key values, except the total len, which should be less than 255. > + > static int validate_and_copy_set_tun(const struct nlattr *attr, > struct sw_flow_actions **sfa, bool log) > { > @@ -2486,6 +2569,11 @@ static int validate_and_copy_set_tun(const struct nlattr *attr, > break; > case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: > break; > + case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS: > + err = validate_erspan_opts(&key, log); > + if (err < 0) > + return err; > + break; > } > }; > > -- > 2.7.4 >
Thanks for the review. On Thu, Jan 25, 2018 at 9:32 AM, Pravin Shelar <pshelar@ovn.org> wrote: > On Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote: >> The patch adds support for openvswitch to configure erspan >> v1 and v2. The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added >> to uapi as a binary blob to support all ERSPAN v1 and v2's >> fields. Note that Previous commit "openvswitch: Add erspan tunnel >> support." was reverted since it does not design properly. >> >> Signed-off-by: William Tu <u9012063@gmail.com> >> --- >> include/uapi/linux/openvswitch.h | 2 +- >> net/openvswitch/flow_netlink.c | 90 +++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 90 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h >> index dcfab5e3b55c..158c2e45c0a5 100644 >> --- a/include/uapi/linux/openvswitch.h >> +++ b/include/uapi/linux/openvswitch.h >> @@ -273,7 +273,6 @@ enum { >> >> #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1) >> >> - >> /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels. >> */ >> enum { >> @@ -363,6 +362,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, /* struct erspan_metadata */ >> __OVS_TUNNEL_KEY_ATTR_MAX >> }; >> > Since this is uapi, we need to define the struct erspan_metadata in a > UAPI header file. Should I define "struct erspan_metadata" in include/uapi/linux/openvswitch.h? Or I'm planning to create a new file in uapi "include/uapi/linux/erspan.h", define "struct erspan_metadata" there, and remove its duplicate at include/net/erspan.h. > > Also lets move field 'version' to the begining of the struct for easy > expansion later. > struct erspan_metadata { > int version; > union { > __be32 index; /* Version 1 (type II)*/ > struct erspan_md2 md2; /* Version 2 (type III) */ > } u; > }; > Sure, will do it. >> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c >> index f143908b651d..9d00c24b2836 100644 >> --- a/net/openvswitch/flow_netlink.c >> +++ b/net/openvswitch/flow_netlink.c >> @@ -49,6 +49,7 @@ >> #include <net/mpls.h> >> #include <net/vxlan.h> >> #include <net/tun_proto.h> >> +#include <net/erspan.h> >> >> #include "flow_netlink.h" >> >> @@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void) >> + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_CSUM */ >> + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_OAM */ >> + nla_total_size(256) /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */ >> - /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with >> + /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and >> + * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with >> * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it. >> */ >> + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_SRC */ >> @@ -400,6 +402,7 @@ 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 = OVS_ATTR_VARIABLE }, >> }; >> >> static const struct ovs_len_tbl >> @@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr, >> return 0; >> } >> >> +static int erspan_tun_opt_from_nlattr(const struct nlattr *a, >> + struct sw_flow_match *match, bool is_mask, >> + bool log) >> +{ >> + unsigned long opt_key_offset; >> + >> + BUILD_BUG_ON(sizeof(struct erspan_metadata) > >> + sizeof(match->key->tun_opts)); >> + >> + if (nla_len(a) > sizeof(match->key->tun_opts)) { >> + OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).", >> + nla_len(a), sizeof(match->key->tun_opts)); >> + return -EINVAL; >> + } >> + >> + if (!is_mask) >> + SW_FLOW_KEY_PUT(match, tun_opts_len, >> + sizeof(struct erspan_metadata), false); >> + else >> + SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true); >> + >> + opt_key_offset = TUN_METADATA_OFFSET(nla_len(a)); >> + SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a), >> + nla_len(a), is_mask); >> + return 0; >> +} >> + > ... >> @@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key) >> return 0; >> } >> >> +static int validate_erspan_opts(struct sw_flow_key *key, bool log) >> +{ > I do not see any need to validate ERSPAN key values, except the total > len, which should be less than 255. > OK, the total len has been checked at erspan_tun_opt_from_nlattr(). I will remove this extra validation. Thanks William
On Thu, Jan 25, 2018 at 10:34 AM, William Tu <u9012063@gmail.com> wrote: > Thanks for the review. > > On Thu, Jan 25, 2018 at 9:32 AM, Pravin Shelar <pshelar@ovn.org> wrote: >> On Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote: >>> The patch adds support for openvswitch to configure erspan >>> v1 and v2. The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added >>> to uapi as a binary blob to support all ERSPAN v1 and v2's >>> fields. Note that Previous commit "openvswitch: Add erspan tunnel >>> support." was reverted since it does not design properly. >>> >>> Signed-off-by: William Tu <u9012063@gmail.com> >>> --- >>> include/uapi/linux/openvswitch.h | 2 +- >>> net/openvswitch/flow_netlink.c | 90 +++++++++++++++++++++++++++++++++++++++- >>> 2 files changed, 90 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h >>> index dcfab5e3b55c..158c2e45c0a5 100644 >>> --- a/include/uapi/linux/openvswitch.h >>> +++ b/include/uapi/linux/openvswitch.h >>> @@ -273,7 +273,6 @@ enum { >>> >>> #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1) >>> >>> - >>> /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels. >>> */ >>> enum { >>> @@ -363,6 +362,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, /* struct erspan_metadata */ >>> __OVS_TUNNEL_KEY_ATTR_MAX >>> }; >>> >> Since this is uapi, we need to define the struct erspan_metadata in a >> UAPI header file. > > Should I define "struct erspan_metadata" in include/uapi/linux/openvswitch.h? > > Or I'm planning to create a new file in uapi "include/uapi/linux/erspan.h", > define "struct erspan_metadata" there, and remove its duplicate at > include/net/erspan.h. > Better to define separate header for ERSPAN. Thanks, Pravin.
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h index dcfab5e3b55c..158c2e45c0a5 100644 --- a/include/uapi/linux/openvswitch.h +++ b/include/uapi/linux/openvswitch.h @@ -273,7 +273,6 @@ enum { #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1) - /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels. */ enum { @@ -363,6 +362,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, /* struct erspan_metadata */ __OVS_TUNNEL_KEY_ATTR_MAX }; diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c index f143908b651d..9d00c24b2836 100644 --- a/net/openvswitch/flow_netlink.c +++ b/net/openvswitch/flow_netlink.c @@ -49,6 +49,7 @@ #include <net/mpls.h> #include <net/vxlan.h> #include <net/tun_proto.h> +#include <net/erspan.h> #include "flow_netlink.h" @@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void) + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_CSUM */ + nla_total_size(0) /* OVS_TUNNEL_KEY_ATTR_OAM */ + nla_total_size(256) /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */ - /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with + /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and + * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it. */ + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_SRC */ @@ -400,6 +402,7 @@ 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 = OVS_ATTR_VARIABLE }, }; static const struct ovs_len_tbl @@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr, return 0; } +static int erspan_tun_opt_from_nlattr(const struct nlattr *a, + struct sw_flow_match *match, bool is_mask, + bool log) +{ + unsigned long opt_key_offset; + + BUILD_BUG_ON(sizeof(struct erspan_metadata) > + sizeof(match->key->tun_opts)); + + if (nla_len(a) > sizeof(match->key->tun_opts)) { + OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).", + nla_len(a), sizeof(match->key->tun_opts)); + return -EINVAL; + } + + if (!is_mask) + SW_FLOW_KEY_PUT(match, tun_opts_len, + sizeof(struct erspan_metadata), false); + else + SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true); + + opt_key_offset = TUN_METADATA_OFFSET(nla_len(a)); + SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a), + nla_len(a), is_mask); + return 0; +} + static int ip_tun_from_nlattr(const struct nlattr *attr, struct sw_flow_match *match, bool is_mask, bool log) @@ -738,6 +768,20 @@ static int ip_tun_from_nlattr(const struct nlattr *attr, break; case OVS_TUNNEL_KEY_ATTR_PAD: break; + case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS: + if (opts_type) { + OVS_NLERR(log, "Multiple metadata blocks provided"); + return -EINVAL; + } + + err = erspan_tun_opt_from_nlattr(a, match, is_mask, + log); + if (err) + return err; + + tun_flags |= TUNNEL_ERSPAN_OPT; + opts_type = type; + break; default: OVS_NLERR(log, "Unknown IP tunnel attribute %d", type); @@ -862,6 +906,10 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb, else if (output->tun_flags & TUNNEL_VXLAN_OPT && vxlan_opt_to_nlattr(skb, tun_opts, swkey_tun_opts_len)) return -EMSGSIZE; + else if (output->tun_flags & TUNNEL_ERSPAN_OPT && + nla_put(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, + swkey_tun_opts_len, tun_opts)) + return -EMSGSIZE; } return 0; @@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key) return 0; } +static int validate_erspan_opts(struct sw_flow_key *key, bool log) +{ + int opts_len = key->tun_opts_len; + struct erspan_metadata *md; + + md = (struct erspan_metadata *)TUN_METADATA_OPTS(key, opts_len); + if (md->version == 1) { + if (ntohl(md->u.index) & ~INDEX_MASK) { + OVS_NLERR(log, + "ERSPAN index number %x too large.", + ntohl(md->u.index)); + return -EINVAL; + } + } else if (md->version == 2) { + struct erspan_md2 *md2 = &md->u.md2; + u8 hwid = get_hwid(md2); + u8 dir = md2->dir; + + if (hwid & ~(HWID_MASK >> HWID_OFFSET)) { + OVS_NLERR(log, "ERSPAN hardware id %x invalid.", hwid); + return -EINVAL; + } + + if (dir != 0 && dir != 1) { + OVS_NLERR(log, "ERSPAN direction %d invalid.", dir); + return -EINVAL; + } + } else { + OVS_NLERR(log, "ERSPAN version %d invalid.", md->version); + return -EINVAL; + } + + return 0; +} + static int validate_and_copy_set_tun(const struct nlattr *attr, struct sw_flow_actions **sfa, bool log) { @@ -2486,6 +2569,11 @@ static int validate_and_copy_set_tun(const struct nlattr *attr, break; case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: break; + case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS: + err = validate_erspan_opts(&key, log); + if (err < 0) + return err; + break; } };
The patch adds support for openvswitch to configure erspan v1 and v2. The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added to uapi as a binary blob to support all ERSPAN v1 and v2's fields. Note that Previous commit "openvswitch: Add erspan tunnel support." was reverted since it does not design properly. Signed-off-by: William Tu <u9012063@gmail.com> --- include/uapi/linux/openvswitch.h | 2 +- net/openvswitch/flow_netlink.c | 90 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-)