Re: [ovs-dev] [PATCH] nsh: enable struct ovs_action_encap_nsh to support variable length

[Yang, Yi Y]

Please ignore this one and use this series 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337128.html which 
includes this one.

-Original Message-
From: Yang, Yi Y 
Sent: Wednesday, August 9, 2017 5:56 PM
To: d...@openvswitch.org
Cc: b...@ovn.org; Yang, Yi Y 
Subject: [PATCH] nsh: enable struct ovs_action_encap_nsh to support variable 
length

In order to adapt to MD type 1 and MD type 2 at the same time and avoid 
breaking Linux kernel uAPI later, we change struct ovs_action_encap_nsh to the 
below format.

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

struct ovs_action_encap_nsh will be allocated dynamically when it is used.

The following patch will change encap_nsh and decap_nsh into push_nsh and 
pop_nsh, respectively, Linux kernel guys prefer
push_* and pop_*.

Signed-off-by: Yi Yang 
---
 datapath/linux/compat/include/linux/openvswitch.h |   4 +-
 lib/odp-util.c| 101 +-
 tests/nsh.at  |  10 +--
 3 files changed, 65 insertions(+), 50 deletions(-)

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[];
 };
 
 /**
diff --git a/lib/odp-util.c b/lib/odp-util.c index ef8b39d..91452f5 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1785,7 +1785,8 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)  {
 int n = 0;
 int ret = 0;
-struct ovs_action_encap_nsh encap_nsh;
+struct ovs_action_encap_nsh *encap_nsh =
+xmalloc(sizeof *encap_nsh + OVS_ENCAP_NSH_MAX_MD_LEN);
 uint32_t spi;
 uint8_t si;
 uint32_t cd;
@@ -1796,11 +1797,11 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 }
 
 /* The default is NSH_M_TYPE1 */
-encap_nsh.flags = 0;
-encap_nsh.mdtype = NSH_M_TYPE1;
-encap_nsh.mdlen = NSH_M_TYPE1_MDLEN;
-encap_nsh.path_hdr = htonl(255);
-memset(encap_nsh.metadata, 0, NSH_M_TYPE1_MDLEN);
+encap_nsh->flags = 0;
+encap_nsh->mdtype = NSH_M_TYPE1;
+encap_nsh->mdlen = NSH_M_TYPE1_MDLEN;
+encap_nsh->path_hdr = htonl(255);
+memset(encap_nsh->metadata, 0, encap_nsh->mdlen);
 
 for (;;) {
 n += strspn(s + n, delimiters); @@ -1808,17 +1809,17 @@ 
parse_odp_encap_nsh_action(const char *s, struct ofpbuf *actions)
 break;
 }
 
-if (ovs_scan_len(s, , "flags=%"SCNi8, _nsh.flags)) {
+if (ovs_scan_len(s, , "flags=%"SCNi8, _nsh->flags)) {
 continue;
 }
-if (ovs_scan_len(s, , "mdtype=%"SCNi8, _nsh.mdtype)) {
-switch (encap_nsh.mdtype) {
+if (ovs_scan_len(s, , "mdtype=%"SCNi8, _nsh->mdtype)) {
+switch (encap_nsh->mdtype) {
 case NSH_M_TYPE1:
 /* This is the default format. */;
 break;
 case NSH_M_TYPE2:
 /* Length will be updated later. */
-encap_nsh.mdlen = 0;
+encap_nsh->mdlen = 0;
 break;
 default:
 ret = -EINVAL;
@@ -1826,24 +1827,24 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 }
 continue;
 }
-if (ovs_scan_len(s, , "np=%"SCNi8, _nsh.np)) {
+if (ovs_scan_len(s, , "np=%"SCNi8, _nsh->np)) {
 continue;
 }
 if (ovs_scan_len(s, , "spi=0x%"SCNx32, )) {
-encap_nsh.path_hdr =
+encap_nsh->path_hdr =
 htonl(((spi << NSH_SPI_SHIFT) & NSH_SPI_MASK) |
-(ntohl(encap_nsh.path_hdr) & ~NSH_SPI_MASK));
+(ntohl(encap_nsh->path_hdr) & 
+ ~NSH_SPI_MASK));
 continue;
 }
 if (ovs_scan_len(s, , "si=%"SCNi8, )) {
-encap_nsh.path_hdr =
+encap_nsh->path_hdr =
 htonl((si << NSH_SI_SHIFT) |
-(ntohl(encap_nsh.path_hdr) & ~NSH_SI_MASK));
+(ntohl(encap_nsh->path_hdr) & 
+ ~NSH_SI_MASK));
 continue;
 }
-if (encap_nsh.mdtype == NSH_M_TYPE1) {
+if (encap_nsh->mdtype == NSH_M_TYPE1) {
 struct nsh_md1_ctx *md1 =
-  

[ovs-dev] [PATCH] nsh: enable struct ovs_action_encap_nsh to support variable length

[Yi Yang]

In order to adapt to MD type 1 and MD type 2 at the same
time and avoid breaking Linux kernel uAPI later, we change
struct ovs_action_encap_nsh to the below format.

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

struct ovs_action_encap_nsh will be allocated dynamically when
it is used.

The following patch will change encap_nsh and decap_nsh into
push_nsh and pop_nsh, respectively, Linux kernel guys prefer
push_* and pop_*.

Signed-off-by: Yi Yang 
---
 datapath/linux/compat/include/linux/openvswitch.h |   4 +-
 lib/odp-util.c| 101 +-
 tests/nsh.at  |  10 +--
 3 files changed, 65 insertions(+), 50 deletions(-)

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[];
 };
 
 /**
diff --git a/lib/odp-util.c b/lib/odp-util.c
index ef8b39d..91452f5 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1785,7 +1785,8 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 {
 int n = 0;
 int ret = 0;
-struct ovs_action_encap_nsh encap_nsh;
+struct ovs_action_encap_nsh *encap_nsh =
+xmalloc(sizeof *encap_nsh + OVS_ENCAP_NSH_MAX_MD_LEN);
 uint32_t spi;
 uint8_t si;
 uint32_t cd;
@@ -1796,11 +1797,11 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 }
 
 /* The default is NSH_M_TYPE1 */
-encap_nsh.flags = 0;
-encap_nsh.mdtype = NSH_M_TYPE1;
-encap_nsh.mdlen = NSH_M_TYPE1_MDLEN;
-encap_nsh.path_hdr = htonl(255);
-memset(encap_nsh.metadata, 0, NSH_M_TYPE1_MDLEN);
+encap_nsh->flags = 0;
+encap_nsh->mdtype = NSH_M_TYPE1;
+encap_nsh->mdlen = NSH_M_TYPE1_MDLEN;
+encap_nsh->path_hdr = htonl(255);
+memset(encap_nsh->metadata, 0, encap_nsh->mdlen);
 
 for (;;) {
 n += strspn(s + n, delimiters);
@@ -1808,17 +1809,17 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 break;
 }
 
-if (ovs_scan_len(s, , "flags=%"SCNi8, _nsh.flags)) {
+if (ovs_scan_len(s, , "flags=%"SCNi8, _nsh->flags)) {
 continue;
 }
-if (ovs_scan_len(s, , "mdtype=%"SCNi8, _nsh.mdtype)) {
-switch (encap_nsh.mdtype) {
+if (ovs_scan_len(s, , "mdtype=%"SCNi8, _nsh->mdtype)) {
+switch (encap_nsh->mdtype) {
 case NSH_M_TYPE1:
 /* This is the default format. */;
 break;
 case NSH_M_TYPE2:
 /* Length will be updated later. */
-encap_nsh.mdlen = 0;
+encap_nsh->mdlen = 0;
 break;
 default:
 ret = -EINVAL;
@@ -1826,24 +1827,24 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
 }
 continue;
 }
-if (ovs_scan_len(s, , "np=%"SCNi8, _nsh.np)) {
+if (ovs_scan_len(s, , "np=%"SCNi8, _nsh->np)) {
 continue;
 }
 if (ovs_scan_len(s, , "spi=0x%"SCNx32, )) {
-encap_nsh.path_hdr =
+encap_nsh->path_hdr =
 htonl(((spi << NSH_SPI_SHIFT) & NSH_SPI_MASK) |
-(ntohl(encap_nsh.path_hdr) & ~NSH_SPI_MASK));
+(ntohl(encap_nsh->path_hdr) & ~NSH_SPI_MASK));
 continue;
 }
 if (ovs_scan_len(s, , "si=%"SCNi8, )) {
-encap_nsh.path_hdr =
+encap_nsh->path_hdr =
 htonl((si << NSH_SI_SHIFT) |
-(ntohl(encap_nsh.path_hdr) & ~NSH_SI_MASK));
+(ntohl(encap_nsh->path_hdr) & ~NSH_SI_MASK));
 continue;
 }
-if (encap_nsh.mdtype == NSH_M_TYPE1) {
+if (encap_nsh->mdtype == NSH_M_TYPE1) {
 struct nsh_md1_ctx *md1 =
-ALIGNED_CAST(struct nsh_md1_ctx *, encap_nsh.metadata);
+ALIGNED_CAST(struct nsh_md1_ctx *, encap_nsh->metadata);
 if (ovs_scan_len(s, , "c1=0x%"SCNx32, )) {
 put_16aligned_be32(>c[0], htonl(cd));
 continue;
@@ -1861,30 +1862,34 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)