| Message ID | 1466070694-8869-1-git-send-email-johnson.li@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 06/16/2016 04:51:34 AM: > From: Johnson Li <johnson.li@intel.com> > To: dev@openvswitch.org > Date: 06/15/2016 09:05 PM > Subject: [ovs-dev] [RFC PATCH 01/14] Add VxLAN-GPE extension for the > Openvswitch > Sent by: "dev" <dev-bounces@openvswitch.org> > > VxLAN Generic Protocol Extension (aka. VxLAN-GPE) extension is > an new IETF draft. Its definition is at: > https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01 > > This patch adds VxLAN-GPE implementation for the VxLAN tunneling > port. > > To create a VxLAN port with GPE extension: > $ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 \ > type=vxlan options:remote_ip=172.168.1.101 options:key=flow \ > options:dst_port=4790 options:exts=gpe > > Signed-off-by: Johnson Li <johnson.li@intel.com> > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/ > datapath/linux/compat/include/linux/openvswitch.h > index 3b39ebb..bd37594 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -287,6 +287,7 @@ enum ovs_vport_attr { > enum { > OVS_VXLAN_EXT_UNSPEC, > OVS_VXLAN_EXT_GBP, /* Flag or __u32 */ > + OVS_VXLAN_EXT_GPE, /* Flag, Generic Protocol Extension */ > __OVS_VXLAN_EXT_MAX, > }; > > diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c > index ddd3f5c..b746f41 100644 > --- a/datapath/vport-netdev.c > +++ b/datapath/vport-netdev.c > @@ -102,7 +102,8 @@ struct vport *ovs_netdev_link(struct vport > *vport, const char *name) > } > > if (vport->dev->flags & IFF_LOOPBACK || > - vport->dev->type != ARPHRD_ETHER || > + (vport->dev->type != ARPHRD_ETHER && > + vport->dev->type != ARPHRD_NONE) || > ovs_is_internal_dev(vport->dev)) { > err = -EINVAL; > goto error_put; > diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c > index c05f5d4..fa48aca 100644 > --- a/datapath/vport-vxlan.c > +++ b/datapath/vport-vxlan.c > @@ -52,6 +52,18 @@ static int vxlan_get_options(const struct vport > *vport, struct sk_buff *skb) > return -EMSGSIZE; > > nla_nest_end(skb, exts); > + } else if (vxlan->flags & VXLAN_F_GPE) { > + struct nlattr *exts; > + > + exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION); > + if (!exts) > + return -EMSGSIZE; > + > + if (vxlan->flags & VXLAN_F_GPE && > + nla_put_flag(skb, OVS_VXLAN_EXT_GPE)) > + return -EMSGSIZE; > + > + nla_nest_end(skb, exts); Nit: since this is almost identical to the previous block, I think a refactoring where this block is abstracted into a helper or a macro might make for easier reading. > } > > return 0; > @@ -59,6 +71,7 @@ static int vxlan_get_options(const struct vport > *vport, struct sk_buff *skb) > > static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = { > [OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, }, > + [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, }, > }; > > static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr, > @@ -76,6 +89,8 @@ static int vxlan_configure_exts(struct vport > *vport, struct nlattr *attr, > > if (exts[OVS_VXLAN_EXT_GBP]) > conf->flags |= VXLAN_F_GBP; > + else if (exts[OVS_VXLAN_EXT_GPE]) > + conf->flags |= VXLAN_F_GPE; > > return 0; > } > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 83a795c..3b05813 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -535,6 +535,8 @@ set_tunnel_config(struct netdev *dev_, const > struct smap *args) > while (ext) { > if (!strcmp(type, "vxlan") && !strcmp(ext, "gbp")) { > tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GBP); > + } else if (!strcmp(type, "vxlan") && !strcmp(ext, "gpe")) { > + tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GPE); > } else { > VLOG_WARN("%s: unknown extension '%s'", name, ext); > } > -- > 1.8.4.2 I know this is an RFC patch, but shouldn't the patch include some additional test code for this extension? Ryan
>"dev" <dev-bounces@openvswitch.org> wrote on 06/16/2016 04:51:34 AM: > > + return -EMSGSIZE; > > + > > + nla_nest_end(skb, exts); > > Nit: since this is almost identical to the previous block, I think > a refactoring where this block is abstracted into a helper or a > macro might make for easier reading. > Thanks, I will try to do that. > > VLOG_WARN("%s: unknown extension '%s'", name, ext); > > } > > -- > > 1.8.4.2 > > I know this is an RFC patch, but shouldn't the patch include > some additional test code for this extension? > > Ryan We had some test cases while developing. Test cases will be added in the next version of patch set.
Regards _Sugesh > -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Johnson Li > Sent: Thursday, June 16, 2016 10:52 AM > To: dev@openvswitch.org > Subject: [ovs-dev] [RFC PATCH 01/14] Add VxLAN-GPE extension for the > Openvswitch > > VxLAN Generic Protocol Extension (aka. VxLAN-GPE) extension is an new > IETF draft. Its definition is at: > https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01 > > This patch adds VxLAN-GPE implementation for the VxLAN tunneling port. [Sugesh] Again its only for the Kernel datapath. And also I feel the NSH and VxLAN-GPE patches can be devided into two set if possible. What do you think? > > To create a VxLAN port with GPE extension: > $ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 \ type=vxlan > options:remote_ip=172.168.1.101 options:key=flow \ > options:dst_port=4790 options:exts=gpe > > Signed-off-by: Johnson Li <johnson.li@intel.com> > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h > b/datapath/linux/compat/include/linux/openvswitch.h > index 3b39ebb..bd37594 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -287,6 +287,7 @@ enum ovs_vport_attr { enum { > OVS_VXLAN_EXT_UNSPEC, > OVS_VXLAN_EXT_GBP, /* Flag or __u32 */ > + OVS_VXLAN_EXT_GPE, /* Flag, Generic Protocol Extension */ > __OVS_VXLAN_EXT_MAX, > }; > > diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index > ddd3f5c..b746f41 100644 > --- a/datapath/vport-netdev.c > +++ b/datapath/vport-netdev.c > @@ -102,7 +102,8 @@ struct vport *ovs_netdev_link(struct vport *vport, > const char *name) > } > > if (vport->dev->flags & IFF_LOOPBACK || > - vport->dev->type != ARPHRD_ETHER || > + (vport->dev->type != ARPHRD_ETHER && > + vport->dev->type != ARPHRD_NONE) || > ovs_is_internal_dev(vport->dev)) { > err = -EINVAL; > goto error_put; > diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c index > c05f5d4..fa48aca 100644 > --- a/datapath/vport-vxlan.c > +++ b/datapath/vport-vxlan.c > @@ -52,6 +52,18 @@ static int vxlan_get_options(const struct vport *vport, > struct sk_buff *skb) > return -EMSGSIZE; > > nla_nest_end(skb, exts); > + } else if (vxlan->flags & VXLAN_F_GPE) { > + struct nlattr *exts; > + > + exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION); > + if (!exts) > + return -EMSGSIZE; > + > + if (vxlan->flags & VXLAN_F_GPE && > + nla_put_flag(skb, OVS_VXLAN_EXT_GPE)) > + return -EMSGSIZE; > + > + nla_nest_end(skb, exts); > } > > return 0; > @@ -59,6 +71,7 @@ static int vxlan_get_options(const struct vport *vport, > struct sk_buff *skb) > > static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = { > [OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, }, > + [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, }, > }; > > static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr, @@ - > 76,6 +89,8 @@ static int vxlan_configure_exts(struct vport *vport, struct > nlattr *attr, > > if (exts[OVS_VXLAN_EXT_GBP]) > conf->flags |= VXLAN_F_GBP; > + else if (exts[OVS_VXLAN_EXT_GPE]) > + conf->flags |= VXLAN_F_GPE; > > return 0; > } > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 83a795c..3b05813 > 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -535,6 +535,8 @@ set_tunnel_config(struct netdev *dev_, const struct > smap *args) > while (ext) { > if (!strcmp(type, "vxlan") && !strcmp(ext, "gbp")) { > tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GBP); > + } else if (!strcmp(type, "vxlan") && !strcmp(ext, "gpe")) { > + tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GPE); > } else { > VLOG_WARN("%s: unknown extension '%s'", name, ext); > } > -- > 1.8.4.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
"Li, Johnson" <johnson.li@intel.com> wrote on 06/15/2016 10:06:10 PM: > From: "Li, Johnson" <johnson.li@intel.com> > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: "dev@openvswitch.org" <dev@openvswitch.org> > Date: 06/15/2016 10:06 PM > Subject: RE: [RFC PATCH 01/14] Add VxLAN-GPE extension for the Openvswitch > > >"dev" <dev-bounces@openvswitch.org> wrote on 06/16/2016 04:51:34 AM: > > > > + return -EMSGSIZE; > > > + > > > + nla_nest_end(skb, exts); > > > > Nit: since this is almost identical to the previous block, I think > > a refactoring where this block is abstracted into a helper or a > > macro might make for easier reading. > > > Thanks, I will try to do that. > > > > VLOG_WARN("%s: unknown extension '%s'", name, ext); > > > } > > > -- > > > 1.8.4.2 > > > > I know this is an RFC patch, but shouldn't the patch include > > some additional test code for this extension? > > > > Ryan > We had some test cases while developing. Test cases will be added in the > next version of patch set. Great - I look forward to seeing them! Ryan
> Regards > _Sugesh > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Johnson Li > > Sent: Thursday, June 16, 2016 10:52 AM > > To: dev@openvswitch.org > > Subject: [ovs-dev] [RFC PATCH 01/14] Add VxLAN-GPE extension for the > > Openvswitch > > > > VxLAN Generic Protocol Extension (aka. VxLAN-GPE) extension is an new > > IETF draft. Its definition is at: > > https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01 > > > > This patch adds VxLAN-GPE implementation for the VxLAN tunneling port. > [Sugesh] Again its only for the Kernel datapath. > And also I feel the NSH and VxLAN-GPE patches can be devided into two set > if possible. What do you think? > [JL]: thanks, I will follow your advice to divide the patch set into two -- one for Kernel space and one for OvS control plane. > > > > To create a VxLAN port with GPE extension: > > $ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 \ type=vxlan > > options:remote_ip=172.168.1.101 options:key=flow \ > > options:dst_port=4790 options:exts=gpe > > > > Signed-off-by: Johnson Li <johnson.li@intel.com> > > > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h > > b/datapath/linux/compat/include/linux/openvswitch.h > > index 3b39ebb..bd37594 100644 > > --- a/datapath/linux/compat/include/linux/openvswitch.h > > +++ b/datapath/linux/compat/include/linux/openvswitch.h > > @@ -287,6 +287,7 @@ enum ovs_vport_attr { enum { > > OVS_VXLAN_EXT_UNSPEC, > > OVS_VXLAN_EXT_GBP, /* Flag or __u32 */ > > + OVS_VXLAN_EXT_GPE, /* Flag, Generic Protocol Extension */ > > __OVS_VXLAN_EXT_MAX, > > }; > > > > diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index > > ddd3f5c..b746f41 100644 > > --- a/datapath/vport-netdev.c > > +++ b/datapath/vport-netdev.c > > @@ -102,7 +102,8 @@ struct vport *ovs_netdev_link(struct vport *vport, > > const char *name) > > } > > > > if (vport->dev->flags & IFF_LOOPBACK || > > - vport->dev->type != ARPHRD_ETHER || > > + (vport->dev->type != ARPHRD_ETHER && > > + vport->dev->type != ARPHRD_NONE) || > > ovs_is_internal_dev(vport->dev)) { > > err = -EINVAL; > > goto error_put; > > diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c index > > c05f5d4..fa48aca 100644 > > --- a/datapath/vport-vxlan.c > > +++ b/datapath/vport-vxlan.c > > @@ -52,6 +52,18 @@ static int vxlan_get_options(const struct vport > > *vport, struct sk_buff *skb) > > return -EMSGSIZE; > > > > nla_nest_end(skb, exts); > > + } else if (vxlan->flags & VXLAN_F_GPE) { > > + struct nlattr *exts; > > + > > + exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION); > > + if (!exts) > > + return -EMSGSIZE; > > + > > + if (vxlan->flags & VXLAN_F_GPE && > > + nla_put_flag(skb, OVS_VXLAN_EXT_GPE)) > > + return -EMSGSIZE; > > + > > + nla_nest_end(skb, exts); > > } > > > > return 0; > > @@ -59,6 +71,7 @@ static int vxlan_get_options(const struct vport > > *vport, struct sk_buff *skb) > > > > static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = { > > [OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, }, > > + [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, }, > > }; > > > > static int vxlan_configure_exts(struct vport *vport, struct nlattr > > *attr, @@ - > > 76,6 +89,8 @@ static int vxlan_configure_exts(struct vport *vport, > > struct nlattr *attr, > > > > if (exts[OVS_VXLAN_EXT_GBP]) > > conf->flags |= VXLAN_F_GBP; > > + else if (exts[OVS_VXLAN_EXT_GPE]) > > + conf->flags |= VXLAN_F_GPE; > > > > return 0; > > } > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index > > 83a795c..3b05813 > > 100644 > > --- a/lib/netdev-vport.c > > +++ b/lib/netdev-vport.c > > @@ -535,6 +535,8 @@ set_tunnel_config(struct netdev *dev_, const > > struct smap *args) > > while (ext) { > > if (!strcmp(type, "vxlan") && !strcmp(ext, "gbp")) { > > tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GBP); > > + } else if (!strcmp(type, "vxlan") && !strcmp(ext, "gpe")) { > > + tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GPE); > > } else { > > VLOG_WARN("%s: unknown extension '%s'", name, ext); > > } > > -- > > 1.8.4.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index 3b39ebb..bd37594 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -287,6 +287,7 @@ enum ovs_vport_attr { enum { OVS_VXLAN_EXT_UNSPEC, OVS_VXLAN_EXT_GBP, /* Flag or __u32 */ + OVS_VXLAN_EXT_GPE, /* Flag, Generic Protocol Extension */ __OVS_VXLAN_EXT_MAX, }; diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index ddd3f5c..b746f41 100644 --- a/datapath/vport-netdev.c +++ b/datapath/vport-netdev.c @@ -102,7 +102,8 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name) } if (vport->dev->flags & IFF_LOOPBACK || - vport->dev->type != ARPHRD_ETHER || + (vport->dev->type != ARPHRD_ETHER && + vport->dev->type != ARPHRD_NONE) || ovs_is_internal_dev(vport->dev)) { err = -EINVAL; goto error_put; diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c index c05f5d4..fa48aca 100644 --- a/datapath/vport-vxlan.c +++ b/datapath/vport-vxlan.c @@ -52,6 +52,18 @@ static int vxlan_get_options(const struct vport *vport, struct sk_buff *skb) return -EMSGSIZE; nla_nest_end(skb, exts); + } else if (vxlan->flags & VXLAN_F_GPE) { + struct nlattr *exts; + + exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION); + if (!exts) + return -EMSGSIZE; + + if (vxlan->flags & VXLAN_F_GPE && + nla_put_flag(skb, OVS_VXLAN_EXT_GPE)) + return -EMSGSIZE; + + nla_nest_end(skb, exts); } return 0; @@ -59,6 +71,7 @@ static int vxlan_get_options(const struct vport *vport, struct sk_buff *skb) static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = { [OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, }, + [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, }, }; static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr, @@ -76,6 +89,8 @@ static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr, if (exts[OVS_VXLAN_EXT_GBP]) conf->flags |= VXLAN_F_GBP; + else if (exts[OVS_VXLAN_EXT_GPE]) + conf->flags |= VXLAN_F_GPE; return 0; } diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 83a795c..3b05813 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -535,6 +535,8 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) while (ext) { if (!strcmp(type, "vxlan") && !strcmp(ext, "gbp")) { tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GBP); + } else if (!strcmp(type, "vxlan") && !strcmp(ext, "gpe")) { + tnl_cfg.exts |= (1 << OVS_VXLAN_EXT_GPE); } else { VLOG_WARN("%s: unknown extension '%s'", name, ext); }
VxLAN Generic Protocol Extension (aka. VxLAN-GPE) extension is an new IETF draft. Its definition is at: https://tools.ietf.org/html/draft-ietf-nvo3-vxlan-gpe-01 This patch adds VxLAN-GPE implementation for the VxLAN tunneling port. To create a VxLAN port with GPE extension: $ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 \ type=vxlan options:remote_ip=172.168.1.101 options:key=flow \ options:dst_port=4790 options:exts=gpe Signed-off-by: Johnson Li <johnson.li@intel.com>