from:"Yang, Yi Y"

RE: [PATCH net-next v2] openvswitch: enable NSH support

2017-08-11 Thread Yang, Yi Y

So far, we're not clear how we can support MD type 2 better, as I explained 
before, we need to reuse tun_metadata in struct flow_tnl which is the thing 
Geneve is using. Geneve predefined 64 keys for this from tun_metadata0 to 
tun_metadata63, we will reuse it for MD type 2. But you know NSH is not tunnel, 
so it has to be changed to support both Geneve and NSH. Anyway, they won't be 
part of ovs_key_nsh.

-Original Message-
From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
Behalf Of Jiri Benc
Sent: Friday, August 11, 2017 5:11 PM
To: Yang, Yi Y <yi.y.y...@intel.com>
Cc: netdev@vger.kernel.org; d...@openvswitch.org
Subject: Re: [PATCH net-next v2] openvswitch: enable NSH support

On Fri, 11 Aug 2017 16:47:23 +0800, Yang, Yi wrote:
> is "__be32 context[4]" ok?

Yes, that looks better.

> So define three new netlink attributes
> 
> OVS_ACTION_ATTR_NSH_BASE_HEADER
> OVS_ACTION_ATTR_NSH_MD1_DATA
> OVS_ACTION_ATTR_NSH_MD2_DATA
> 
> OVS_ACTION_ATTR_PUSH_NSH is nested netlink attribute, it will nest 
> OVS_ACTION_ATTR_NSH_BASE_HEADER and OVS_ACTION_ATTR_NSH_MD1_DATA for 
> MD type 1, it will nest OVS_ACTION_ATTR_NSH_BASE_HEADER and 
> OVS_ACTION_ATTR_NSH_MD2_DATA for MD type 2. I'll compeletely remove 
> struct ovs_action_push_nsh, is it ok?

Yes, that's the way to do it.

What should be done with struct ovs_key_nsh? Even with "c" renamed to 
"context", it's still MD type 1 only structure. What is the plan for MD type 2 
support wrt. this structure?

Thanks,

 Jiri

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

2017-08-09 Thread Yang, Yi Y

struct ovs_action_encap_nsh is the only one way we transfer all the data for 
encap_nsh, netlink allows variable attribute, so I don't think we break netlink 
convention or abuse this variable feature.

Even if we bring nested attributes to handle this, OVS_ACTION_ATTR_ENCAP_NSH is 
still length-variable, OVS_NSH_ATTR_MD2 is also length-variable (it can be from 
0 to 248), so I don't think such way can avoid the issue you're addressing.

The result will be worse, it will make many difficulties when we transfer all 
the data for encap_nsh between OVS' components.

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Thursday, August 10, 2017 4:54 AM
To: Yang, Yi Y <yi.y.y...@intel.com>
Cc: Jan Scheurich <jan.scheur...@ericsson.com>; d...@openvswitch.org; 
netdev@vger.kernel.org; Jiri Benc <jb...@redhat.com>; da...@davemloft.net; 
Zoltán Balogh <zoltan.bal...@ericsson.com>
Subject: Re: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

On Wed, Aug 09, 2017 at 08:12:36PM +, Yang, Yi Y wrote:
> Ben, do you mean we bring two new attributes (OVS_NSH_ATTR_MD1 and
> OVS_NSH_ATTR_MD2) and embed one of them in OVS_ACTION_ATTR_ENCAP_NSH?

Yes.

> Anyway we need to use a struct or something else to transfer those 
> metadata between functions, how do you think we can handle this 
> without metadata in struct ovs_action_encap_nsh? I mean how we handle 
> the arguments for function encap_nsh.

I guess that a pointer to the embedded nlattr with type OVS_NSH_ATTR_MD1 or 
OVS_NSH_ATTR2 should work OK.

Keep in mind that I'm not a kernel-side maintainer of any kind.  I am only 
passing along what I've perceived to be common Netlink protocol design patterns.

> -Original Message-
> From: netdev-ow...@vger.kernel.org 
> [mailto:netdev-ow...@vger.kernel.org] On Behalf Of Ben Pfaff
> Sent: Thursday, August 10, 2017 2:09 AM
> To: Yang, Yi Y <yi.y.y...@intel.com>
> Cc: Jan Scheurich <jan.scheur...@ericsson.com>; d...@openvswitch.org; 
> netdev@vger.kernel.org; Jiri Benc <jb...@redhat.com>; 
> da...@davemloft.net; Zoltán Balogh <zoltan.bal...@ericsson.com>
> Subject: Re: [ovs-dev] [PATCH net-next] openvswitch: add NSH support
> 
> On Wed, Aug 09, 2017 at 09:41:51AM +, Yang, Yi Y wrote:
> > Hi,  Jan
> > 
> > I have worked out a patch, will send it quickly for Ben. In 
> > addition, I also will send out a patch to change encap_nsh 
> > _nsh to push_nsh and pop_nsh. Per comments from all the 
> > people, we all agreed to do so :-)
> > 
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index bc6c94b..4936c12 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -793,7 +793,7 @@ struct ovs_action_push_eth {
> > struct ovs_key_ethernet addresses;  };
> > 
> > -#define OVS_ENCAP_NSH_MAX_MD_LEN 16
> > +#define OVS_ENCAP_NSH_MAX_MD_LEN 248
> >  /*
> >   * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
> >   * @flags: NSH header flags.
> > @@ -809,7 +809,7 @@ struct ovs_action_encap_nsh {
> >  uint8_t mdlen;
> >  uint8_t np;
> >  __be32 path_hdr;
> > -uint8_t metadata[OVS_ENCAP_NSH_MAX_MD_LEN];
> > +uint8_t metadata[];
> >  };
> 
> This brings the overall format of ovs_action_encap_nsh to:
> 
> struct ovs_action_encap_nsh {
> uint8_t flags;
> uint8_t mdtype;
> uint8_t mdlen;
> uint8_t np;
> __be32 path_hdr;
> uint8_t metadata[];
> };
> 
> This is an unusual format for a Netlink attribute.  More commonly, one would 
> put variable-length data into an attribute of its own, which allows that data 
> to be handled using the regular Netlink means.  Then the mdlen and metadata 
> members could be removed, since they would be part of the additional 
> attribute, and one might expect the mdtype member to be removed as well since 
> each type of metadata would be in a different attribute type.
> 
> So, a format closer to what I expect to see in Netlink is something 
> like
> this:
> 
> /**
>  * enum ovs_nsh_attr - Metadata attributes for %OVS_ACTION_ENCAP_NSH action.
>  *
>  * @OVS_NSH_ATTR_MD1: Contains 16-byte NSH type-1 metadata.
>  * @OVS_NSH_ATTR_MD2: Contains 0- to 255-byte variable-length NSH 
> type-2
>  * metadata. */
> enum ovs_nsh_attr {
> OVS_NSH_ATTR_MD1,
> OVS_NSH_ATTR_MD2
> };
> 
> /*
>  * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
>  *
>  * @path_hdr: NSH service path id and service index.
>  * @flags: NSH header flags.
>  * @np: NSH next_protocol: Inner packet type.
>  *
>  * Followed by either %OVS_NSH_ATTR_MD1 or %OVS_NSH_ATTR_MD2 attribute.
>  */
> struct ovs_action_encap_nsh {
> __be32 path_hdr;
> uint8_t flags;
> uint8_t np;
> };

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

2017-08-09 Thread Yang, Yi Y

Ben, do you mean we bring two new attributes (OVS_NSH_ATTR_MD1 and   
OVS_NSH_ATTR_MD2) and embed one of them in OVS_ACTION_ATTR_ENCAP_NSH? Anyway we 
need to use a struct or something else to transfer those metadata between 
functions, how do you think we can handle this without metadata in struct 
ovs_action_encap_nsh? I mean how we handle the arguments for function encap_nsh.

-Original Message-
From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
Behalf Of Ben Pfaff
Sent: Thursday, August 10, 2017 2:09 AM
To: Yang, Yi Y <yi.y.y...@intel.com>
Cc: Jan Scheurich <jan.scheur...@ericsson.com>; d...@openvswitch.org; 
netdev@vger.kernel.org; Jiri Benc <jb...@redhat.com>; da...@davemloft.net; 
Zoltán Balogh <zoltan.bal...@ericsson.com>
Subject: Re: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

On Wed, Aug 09, 2017 at 09:41:51AM +, Yang, Yi Y wrote:
> Hi,  Jan
> 
> I have worked out a patch, will send it quickly for Ben. In addition, 
> I also will send out a patch to change encap_nsh _nsh to 
> push_nsh and pop_nsh. Per comments from all the people, we all agreed 
> to do so :-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index bc6c94b..4936c12 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -793,7 +793,7 @@ struct ovs_action_push_eth {
> struct ovs_key_ethernet addresses;  };
> 
> -#define OVS_ENCAP_NSH_MAX_MD_LEN 16
> +#define OVS_ENCAP_NSH_MAX_MD_LEN 248
>  /*
>   * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
>   * @flags: NSH header flags.
> @@ -809,7 +809,7 @@ struct ovs_action_encap_nsh {
>  uint8_t mdlen;
>  uint8_t np;
>  __be32 path_hdr;
> -uint8_t metadata[OVS_ENCAP_NSH_MAX_MD_LEN];
> +uint8_t metadata[];
>  };

This brings the overall format of ovs_action_encap_nsh to:

struct ovs_action_encap_nsh {
uint8_t flags;
uint8_t mdtype;
uint8_t mdlen;
uint8_t np;
__be32 path_hdr;
uint8_t metadata[];
};

This is an unusual format for a Netlink attribute.  More commonly, one would 
put variable-length data into an attribute of its own, which allows that data 
to be handled using the regular Netlink means.  Then the mdlen and metadata 
members could be removed, since they would be part of the additional attribute, 
and one might expect the mdtype member to be removed as well since each type of 
metadata would be in a different attribute type.

So, a format closer to what I expect to see in Netlink is something like
this:

/**
 * enum ovs_nsh_attr - Metadata attributes for %OVS_ACTION_ENCAP_NSH action.
 *
 * @OVS_NSH_ATTR_MD1: Contains 16-byte NSH type-1 metadata.
 * @OVS_NSH_ATTR_MD2: Contains 0- to 255-byte variable-length NSH type-2
 * metadata. */
enum ovs_nsh_attr {
OVS_NSH_ATTR_MD1,
OVS_NSH_ATTR_MD2
};

/*
 * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
 *
 * @path_hdr: NSH service path id and service index.
 * @flags: NSH header flags.
 * @np: NSH next_protocol: Inner packet type.
 *
 * Followed by either %OVS_NSH_ATTR_MD1 or %OVS_NSH_ATTR_MD2 attribute.
 */
struct ovs_action_encap_nsh {
__be32 path_hdr;
uint8_t flags;
uint8_t np;
};

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

2017-08-09 Thread Yang, Yi Y

OVS_ENCAP_NSH_MAX_MD_LEN);
 ofpbuf_put_hex(, buf, );
-encap_nsh.mdlen = mdlen;
+encap_nsh->mdlen = mdlen;
 ofpbuf_uninit();
 }
 continue;
 }
 }
 out:
-if (ret < 0) {
-return ret;
-} else {
-size_t size = offsetof(struct ovs_action_encap_nsh, metadata)
-+ ROUND_UP(encap_nsh.mdlen, 4);
-nl_msg_put_unspec(actions, OVS_ACTION_ATTR_ENCAP_NSH,
-  _nsh, size);
-return n;
+if (ret >= 0) {
+size_t size = sizeof(struct ovs_action_encap_nsh)
+  + ROUND_UP(encap_nsh->mdlen, 4);
+size_t pad_len = size - sizeof(struct ovs_action_encap_nsh)
+ - encap_nsh->mdlen;
+if (encap_nsh->mdlen > NSH_M_TYPE1_MDLEN && pad_len > 0) {
+memset(encap_nsh->metadata + encap_nsh->mdlen, 0, pad_len);
+}
+nl_msg_put_unspec(actions, OVS_ACTION_ATTR_ENCAP_NSH, encap_nsh, size);
+ret = n;
 }
+free(encap_nsh);
+return ret;
 }

 static int
@@ -6798,19 +6803,22 @@ odp_put_encap_nsh_action(struct ofpbuf *odp_actions,
  const struct flow *flow,
  struct ofpbuf *encap_data)
 {
-struct ovs_action_encap_nsh encap_nsh;
-
-encap_nsh.flags = flow->nsh.flags;
-encap_nsh.mdtype = flow->nsh.mdtype;
-encap_nsh.np = flow->nsh.np;
-encap_nsh.path_hdr = htonl((ntohl(flow->nsh.spi) << NSH_SPI_SHIFT) |
+size_t size;
+size_t pad_len;
+struct ovs_action_encap_nsh *encap_nsh =
+xmalloc(sizeof *encap_nsh + OVS_ENCAP_NSH_MAX_MD_LEN);
+
+encap_nsh->flags = flow->nsh.flags;
+encap_nsh->mdtype = flow->nsh.mdtype;
+encap_nsh->np = flow->nsh.np;
+encap_nsh->path_hdr = htonl((ntohl(flow->nsh.spi) << NSH_SPI_SHIFT) |
flow->nsh.si);

-switch (encap_nsh.mdtype) {
+switch (encap_nsh->mdtype) {
 case NSH_M_TYPE1: {
 struct nsh_md1_ctx *md1 =
-ALIGNED_CAST(struct nsh_md1_ctx *, encap_nsh.metadata);
-encap_nsh.mdlen = NSH_M_TYPE1_MDLEN;
+ALIGNED_CAST(struct nsh_md1_ctx *, encap_nsh->metadata);
+encap_nsh->mdlen = NSH_M_TYPE1_MDLEN;
 for (int i = 0; i < 4; i++) {
 put_16aligned_be32(>c[i], flow->nsh.c[i]);
 }
@@ -6819,18 +6827,25 @@ odp_put_encap_nsh_action(struct ofpbuf *odp_actions,
 case NSH_M_TYPE2:
 if (encap_data) {
 ovs_assert(encap_data->size < OVS_ENCAP_NSH_MAX_MD_LEN);
-encap_nsh.mdlen = encap_data->size;
-memcpy(encap_nsh.metadata, encap_data->data, encap_data->size);
+encap_nsh->mdlen = encap_data->size;
+memcpy(encap_nsh->metadata, encap_data->data, encap_data->size);
 } else {
-encap_nsh.mdlen = 0;
+encap_nsh->mdlen = 0;
 }
 break;
 default:
-encap_nsh.mdlen = 0;
+encap_nsh->mdlen = 0;
 break;
 }
-nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_ENCAP_NSH,
-  _nsh, sizeof(encap_nsh));
+size = sizeof(struct ovs_action_encap_nsh)
+   + ROUND_UP(encap_nsh->mdlen, 4);
+pad_len = size - sizeof(struct ovs_action_encap_nsh)
+  - encap_nsh->mdlen;
+if (pad_len > 0) {
+    memset(encap_nsh->metadata + encap_nsh->mdlen, 0, pad_len);
+}
+nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_ENCAP_NSH, encap_nsh, size);
+free(encap_nsh);
 }

 static void

-Original Message-
From: Jan Scheurich [mailto:jan.scheur...@ericsson.com] 
Sent: Wednesday, August 9, 2017 4:32 PM
To: Ben Pfaff <b...@ovn.org>; Yang, Yi Y <yi.y.y...@intel.com>
Cc: d...@openvswitch.org; netdev@vger.kernel.org; Jiri Benc <jb...@redhat.com>; 
da...@davemloft.net; Zoltán Balogh <zoltan.bal...@ericsson.com>
Subject: RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

Hi all,

In OVS 2.8 we support only fixed size NSH MD1 context data for matching and in 
set/copy_field actions. OVS parses an MD2 NSH header but does not make any TLV 
headers available to OF. The plan is to add support for matching/manipulating 
NSH MD2 TLVs through a new infrastructure of generic TLV match fields that can 
be dynamically mapped to specific protocol TLVs, similar to the way this is 
done for GENEVE tunnel metadata TLVs today. But this is work for an upcoming 
OVS release.

However, in encap() and decap() NSH actions we do support MD2 format already. 
The encap_nsh datapath action is agnostic of the MD format. Any MD2 TLV 
metadata are provided as encap properties in the OF encap() operation. They are 
translated by the ofproto layer and forwarded as opaque byte sequence in the 
encap

RE: [PATCH net-next] openvswitch: add NSH support

2017-08-08 Thread Yang, Yi Y

Hi, Jiri

Thank you for your comments.

__be32 c[4] is the name Ben Pfaff suggested, the original name is c1, c2, c3, 
c4, they are context data, so c seems ok, too :-)

OVS has merged it and has the same name, maybe the better way is adding comment 
/* Context data */ after it.

For MD type 2, struct ovs_key_nsh is very difficult to cover it, so far we 
don't know how to support MD type 2 better, Geneve defined 64 tun_metadata0-63 
to handle this, those keys are parts of struct flow_tnl, the highest 
possibility is to reuse those keys.

So for future MD type 2, we will have two parts of keys, one is from struct 
ovs_key_nsh, another is from struct flow_tnl, this won't break the old uAPI.

"#define OVS_ENCAP_NSH_MAX_MD_LEN 16" is changed per Ben's comment from 256, 
Ben thinks 256 is too big but we only support MD type 1 now. We still have ways 
to extend it, for example:

struct ovs_action_encap_nsh * oaen = (struct ovs_action_encap_nsh *) malloc 
(sizeof(struct ovs_action_encap_nsh) + ANY_SIZE);

nl_msg_put_unspec(actions, OVS_ACTION_ATTR_ENCAP_NSH,
  oaen, sizeof(struct ovs_action_encap_nsh) + ANY_SIZE);

In addition, we also need to consider, OVS userspace code must be consistent 
with here, so keeping it intact will be better, we have way to support 
dynamically extension when we add MD type 2 support.

About action name, unfortunately, userspace data plane has named them as 
encap_nsh & decap_nsh, Jan, what do you think about Jiri's suggestion?

But from my understanding, encap_* & decap_* are better because they 
corresponding to generic encap & decap actions, in addition, encap semantics 
are different from push, encap just pushed an empty header with default values, 
users must use set_field to set the content of the header.

Again, OVS userspace code must be consistent with here, so keeping it intact 
will be better because OVS userspace code was there.

-Original Message-
From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
Behalf Of Jiri Benc
Sent: Tuesday, August 8, 2017 10:28 PM
To: Yang, Yi Y <yi.y.y...@intel.com>
Cc: netdev@vger.kernel.org; d...@openvswitch.org; da...@davemloft.net
Subject: Re: [PATCH net-next] openvswitch: add NSH support

On Tue,  8 Aug 2017 12:59:40 +0800, Yi Yang wrote:
> +struct ovs_key_nsh {
> + __u8 flags;
> + __u8 mdtype;
> + __u8 np;
> + __u8 pad;
> + __be32 path_hdr;
> + __be32 c[4];

"c" is a very poor name. Please rename it to something that expresses what this 
field contains.

Also, this looks like MD type 1 only. How are those fields going to work with 
MD type 2? I don't think MD type 2 implementation is necessary in this patch 
but I'd like to know how this is going to work - it's uAPI and thus set in 
stone once this is merged. The uAPI needs to be designed with future use in 
mind.

> +#define OVS_ENCAP_NSH_MAX_MD_LEN 16
> +/*
> + * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
> + * @flags: NSH header flags.
> + * @mdtype: NSH metadata type.
> + * @mdlen: Length of NSH metadata in bytes.
> + * @np: NSH next_protocol: Inner packet type.
> + * @path_hdr: NSH service path id and service index.
> + * @metadata: NSH metadata for MD type 1 or 2  */ struct 
> +ovs_action_encap_nsh {
> + __u8 flags;
> + __u8 mdtype;
> + __u8 mdlen;
> + __u8 np;
> + __be32 path_hdr;
> + __u8 metadata[OVS_ENCAP_NSH_MAX_MD_LEN];

This is wrong. The metadata size is set to a fixed size by this and cannot be 
ever extended, or at least not easily. Netlink has attributes for exactly these 
cases and that's what needs to be used here.

> @@ -835,6 +866,8 @@ enum ovs_action_attr {
>   OVS_ACTION_ATTR_TRUNC,/* u32 struct ovs_action_trunc. */
>   OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
>   OVS_ACTION_ATTR_POP_ETH,  /* No argument. */
> + OVS_ACTION_ATTR_ENCAP_NSH,/* struct ovs_action_encap_nsh. */
> + OVS_ACTION_ATTR_DECAP_NSH,/* No argument. */

Use "push" and "pop", not "encap" and "decap" to be consistent with the naming 
in the rest of the file. We use encap and decap for tunnel operations. This 
code does not use lwtunnels, thus push and pop is more appropriate.

 Jiri

RE: [ovs-dev] [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets

2016-11-15 Thread Yang, Yi Y

Got it, thanks, I'll follow your discussion thread.

-Original Message-
From: Thadeu Lima de Souza Cascardo [mailto:casca...@cascardo.eti.br] 
Sent: Wednesday, November 16, 2016 3:01 AM
To: Yang, Yi Y <yi.y.y...@intel.com>; Jiri Benc <jb...@redhat.com>; 
netdev@vger.kernel.org
Cc: d...@openvswitch.org; Simon Horman <simon.hor...@netronome.com>; 
egar...@redhat.com
Subject: Re: [ovs-dev] [PATCH net-next v13 0/8] openvswitch: support for layer 
3 encapsulated packets

On November 15, 2016 11:57:21 AM GMT-02:00, "Yang, Yi Y" <yi.y.y...@intel.com> 
wrote:
>Hi, Jiri
>
>I'm very glad to see you're continuing this work :-), I asked Simon 
>about this twice, but nobody replies. I also remember Cascardo has a 
>patch set to collaborate with this patch set, I asked Cascardo, but 
>nobody responds, will you continue to do Cascardo's " create tunnel 
>devices using rtnetlink interface" patch set? I test the old one v3, 
>that can work with vxlan module in kernel, but if I build ovs with 
>option " --with-linux=/lib/modules/`uname -r`/build", ovs vxlan module 
>is built in vport_vxlan module, when I create vxlan-gpe port, kernel 
>will automatically load vxlan module in the kernel instead of using the 
>APIs in vport_vxlan module.
>
>Cascardo, are you still working on this?
>
>-Original Message-
>From: netdev-ow...@vger.kernel.org
>[mailto:netdev-ow...@vger.kernel.org] On Behalf Of Jiri Benc
>Sent: Thursday, November 10, 2016 11:28 PM
>To: netdev@vger.kernel.org
>Cc: d...@openvswitch.org; Pravin Shelar <pshe...@ovn.org>; Lorand Jakab 
><loja...@cisco.com>; Simon Horman <simon.hor...@netronome.com>
>Subject: [PATCH net-next v13 0/8] openvswitch: support for layer 3 
>encapsulated packets
>
>At the core of this patch set is removing the assumption in Open 
>vSwitch datapath that all packets have Ethernet header.
>
>The implementation relies on the presence of pop_eth and push_eth 
>actions in datapath flows to facilitate adding and removing Ethernet 
>headers as appropriate. The construction of such flows is left up to 
>user-space.
>
>This series is based on work by Simon Horman, Lorand Jakab, Thomas 
>Morin and others. I kept Lorand's and Simon's s-o-b in the patches that 
>are derived from v11 to record their authorship of parts of the code.
>
>Changes from v12 to v13:
>
>* Addressed Pravin's feedback.
>* Removed the GRE vport conversion patch; L3 GRE ports should be 
>created by
>  rtnetlink instead.
>
>Main changes from v11 to v12:
>
>* The patches were restructured and split differently for easier 
>review.
>* They were rebased and adjusted to the current net-next. Especially 
>MPLS handling is different (and easier) thanks to the recent MPLS GSO 
>rework.
>* Several bugs were discovered and fixed. The most notable is fragment
>handling: header adjustment for ARPHRD_NONE devices on tx needs to be 
>done after refragmentation, not before it. This required significant 
>changes in the patchset. Another one is stricter checking of attributes 
>(match on
>L2
>  vs. L3 packet) at the kernel level.
>* Instead of is_layer3 bool, a mac_proto field is used.
>
>Jiri Benc (8):
>  openvswitch: use hard_header_len instead of hardcoded ETH_HLEN
>  openvswitch: add mac_proto field to the flow key
>  openvswitch: pass mac_proto to ovs_vport_send
>  openvswitch: support MPLS push and pop for L3 packets
>  openvswitch: add processing of L3 packets
>  openvswitch: netlink: support L3 packets
>  openvswitch: add Ethernet push and pop actions
>  openvswitch: allow L3 netdev ports
>
> include/uapi/linux/openvswitch.h |  15 
> net/openvswitch/actions.c| 111 +---
> net/openvswitch/datapath.c   |  13 +--
> net/openvswitch/flow.c   | 105 +--
> net/openvswitch/flow.h   |  22 +
>net/openvswitch/flow_netlink.c   | 179
>++-
> net/openvswitch/vport-netdev.c   |   9 +-
> net/openvswitch/vport.c  |  31 +--
> net/openvswitch/vport.h  |   2 +-
> 9 files changed, 353 insertions(+), 134 deletions(-)
>
>--
>1.8.3.1
>
>___
>dev mailing list
>d...@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Hi.

I am still working on this. Just see my recent discussion with Pravin about the 
way to support out of tree drivers. If you have any opinion on that, please 
share on that thread, preferably with a patch. It's likely that Eric Garver 
will take this task as he was already working with me.

Cascardo.

RE: [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets

2016-11-15 Thread Yang, Yi Y

Hi, Jiri

I'm very glad to see you're continuing this work :-), I asked Simon about this 
twice, but nobody replies. I also remember Cascardo has a patch set to 
collaborate with this patch set, I asked Cascardo, but nobody responds, will 
you continue to do Cascardo's " create tunnel devices using rtnetlink 
interface" patch set? I test the old one v3, that can work with vxlan module in 
kernel, but if I build ovs with option " --with-linux=/lib/modules/`uname 
-r`/build", ovs vxlan module is built in vport_vxlan module, when I create 
vxlan-gpe port, kernel will automatically load vxlan module in the kernel 
instead of using the APIs in vport_vxlan module. 

Cascardo, are you still working on this?

-Original Message-
From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
Behalf Of Jiri Benc
Sent: Thursday, November 10, 2016 11:28 PM
To: netdev@vger.kernel.org
Cc: d...@openvswitch.org; Pravin Shelar ; Lorand Jakab 
; Simon Horman 
Subject: [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated 
packets

At the core of this patch set is removing the assumption in Open vSwitch 
datapath that all packets have Ethernet header.

The implementation relies on the presence of pop_eth and push_eth actions in 
datapath flows to facilitate adding and removing Ethernet headers as 
appropriate. The construction of such flows is left up to user-space.

This series is based on work by Simon Horman, Lorand Jakab, Thomas Morin and 
others. I kept Lorand's and Simon's s-o-b in the patches that are derived from 
v11 to record their authorship of parts of the code.

Changes from v12 to v13:

* Addressed Pravin's feedback.
* Removed the GRE vport conversion patch; L3 GRE ports should be created by
  rtnetlink instead.

Main changes from v11 to v12:

* The patches were restructured and split differently for easier review.
* They were rebased and adjusted to the current net-next. Especially MPLS
  handling is different (and easier) thanks to the recent MPLS GSO rework.
* Several bugs were discovered and fixed. The most notable is fragment
  handling: header adjustment for ARPHRD_NONE devices on tx needs to be done
  after refragmentation, not before it. This required significant changes in
  the patchset. Another one is stricter checking of attributes (match on L2
  vs. L3 packet) at the kernel level.
* Instead of is_layer3 bool, a mac_proto field is used.

Jiri Benc (8):
  openvswitch: use hard_header_len instead of hardcoded ETH_HLEN
  openvswitch: add mac_proto field to the flow key
  openvswitch: pass mac_proto to ovs_vport_send
  openvswitch: support MPLS push and pop for L3 packets
  openvswitch: add processing of L3 packets
  openvswitch: netlink: support L3 packets
  openvswitch: add Ethernet push and pop actions
  openvswitch: allow L3 netdev ports

 include/uapi/linux/openvswitch.h |  15 
 net/openvswitch/actions.c| 111 +---
 net/openvswitch/datapath.c   |  13 +--
 net/openvswitch/flow.c   | 105 +--
 net/openvswitch/flow.h   |  22 +
 net/openvswitch/flow_netlink.c   | 179 ++-
 net/openvswitch/vport-netdev.c   |   9 +-
 net/openvswitch/vport.c  |  31 +--
 net/openvswitch/vport.h  |   2 +-
 9 files changed, 353 insertions(+), 134 deletions(-)

--
1.8.3.1

RE: [ovs-dev] [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support

2016-05-09 Thread Yang, Yi Y

I think it is still necessary to keep eth_type in push_eth action, for the 
classifier case, it will push_nsh then push_eth for the original frame, this 
will need to set eth_type to 0x894f by push_eth, otherwise push_eth won't know 
this eth_type.

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Simon Horman
Sent: Monday, May 09, 2016 4:18 PM
To: Jiri Benc 
Cc: d...@openvswitch.org; netdev@vger.kernel.org
Subject: Re: [ovs-dev] [PATCH v9 net-next 4/7] openvswitch: add layer 3 
flow/port support

On Fri, May 06, 2016 at 11:35:04AM +0200, Jiri Benc wrote:
> On Wed,  4 May 2016 16:36:30 +0900, Simon Horman wrote:
> > +static int push_eth(struct sk_buff *skb, struct sw_flow_key *key,
> > +   const struct ovs_action_push_eth *ethh) {
> > +   int err;
> > +
> > +   /* De-accelerate any hardware accelerated VLAN tag added to a previous
> > +* Ethernet header */
> > +   err = skb_vlan_deaccel(skb);
> > +   if (unlikely(err))
> > +   return err;
> > +
> > +   /* Add the new Ethernet header */
> > +   if (skb_cow_head(skb, ETH_HLEN) < 0)
> > +   return -ENOMEM;
> > +
> > +   skb_push(skb, ETH_HLEN);
> > +   skb_reset_mac_header(skb);
> > +   skb_reset_mac_len(skb);
> > +
> > +   ether_addr_copy(eth_hdr(skb)->h_source, ethh->addresses.eth_src);
> > +   ether_addr_copy(eth_hdr(skb)->h_dest, ethh->addresses.eth_dst);
> > +   eth_hdr(skb)->h_proto = ethh->eth_type;
> 
> This doesn't seem right. We know the packet type, it's skb->protocol.
> We should fill in that.

I think that makes sense. Possibly the eth_type field can be removed from 
ovs_action_push_eth.

> In addition, we should check whether mac_len > 0 and in such case, 
> change skb->protocol to ETH_P_TEB first (and store that value in the 
> pushed eth header).
> 
> Similarly on pop_eth, we need to check skb->protocol and if it is 
> ETH_P_TEB, call eth_type_trans on the modified frame to set the new
> skb->protocol correctly. It's probably not that simple, as we'd need a
> version of eth_type_trans that doesn't need a net device.

I'm not sure I understand the interaction with ETH_P_TEB here.

In my mind skb->protocol == ETH_P_TEB may be used early on in OvS's receive 
processing to find the inner protocol from the packet and at that point
skb->protocol is set to that value. And that for further packet 
skb->processing
the fact the packet was received as TEB is transparent.

Conversely, skb->protocol may be set as necessary on transmit as a packet exits 
OvS.
___
dev mailing list
d...@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

RE: [PATCH net-next v2] openvswitch: enable NSH support

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

RE: [PATCH net-next] openvswitch: add NSH support

RE: [ovs-dev] [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets

RE: [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets

RE: [ovs-dev] [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support

8 matches

Site Navigation

Mail list logo

Footer information