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 <yi.y.y...@intel.com>
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 <yi.y.y...@intel.com>
---
 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, &n, "flags=%"SCNi8, &encap_nsh.flags)) {
+        if (ovs_scan_len(s, &n, "flags=%"SCNi8, &encap_nsh->flags)) {
             continue;
         }
-        if (ovs_scan_len(s, &n, "mdtype=%"SCNi8, &encap_nsh.mdtype)) {
-            switch (encap_nsh.mdtype) {
+        if (ovs_scan_len(s, &n, "mdtype=%"SCNi8, &encap_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, &n, "np=%"SCNi8, &encap_nsh.np)) {
+        if (ovs_scan_len(s, &n, "np=%"SCNi8, &encap_nsh->np)) {
             continue;
         }
         if (ovs_scan_len(s, &n, "spi=0x%"SCNx32, &spi)) {
-            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, &n, "si=%"SCNi8, &si)) {
-            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, &n, "c1=0x%"SCNx32, &cd)) {
                 put_16aligned_be32(&md1->c[0], htonl(cd));
                 continue;
@@ -1861,30 +1862,34 @@ parse_odp_encap_nsh_action(const char *s, struct ofpbuf 
*actions)
                 continue;
             }
         }
-        else if (encap_nsh.mdtype == NSH_M_TYPE2) {
+        else if (encap_nsh->mdtype == NSH_M_TYPE2) {
             struct ofpbuf b;
             char buf[512];
             size_t mdlen;
             if (ovs_scan_len(s, &n, "md2=0x%511[0-9a-fA-F]", buf)) {
-                ofpbuf_use_stub(&b, encap_nsh.metadata,
+                ofpbuf_use_stub(&b, encap_nsh->metadata,
                                 OVS_ENCAP_NSH_MAX_MD_LEN);
                 ofpbuf_put_hex(&b, buf, &mdlen);
-                encap_nsh.mdlen = mdlen;
+                encap_nsh->mdlen = mdlen;
                 ofpbuf_uninit(&b);
             }
             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,
-                          &encap_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(&md1->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,
-                      &encap_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
diff --git a/tests/nsh.at b/tests/nsh.at index aa80a2a..cf567e7 100644
--- a/tests/nsh.at
+++ b/tests/nsh.at
@@ -195,7 +195,7 @@ ovs-vsctl set bridge br0 datapath_type=dummy \
         add-port br0 v4 -- set Interface v4 type=patch options:peer=v3 
ofport_request=4])
 
 AT_DATA([flows.txt], [dnl
-    
table=0,in_port=1,ip,actions=encap(nsh(md_type=2,tlv(0x1000,10,0x12345678))),set_field:0x1234->nsh_spi,encap(ethernet),set_field:11:22:33:44:55:66->dl_dst,3
+    
+ table=0,in_port=1,ip,actions=encap(nsh(md_type=2,tlv(0x1000,10,0x12345
+ 678),tlv(0x2000,20,0xfedcba9876543210))),set_field:0x1234->nsh_spi,enc
+ ap(ethernet),set_field:11:22:33:44:55:66->dl_dst,3
     
table=0,in_port=4,dl_type=0x894f,nsh_mdtype=2,nsh_spi=0x1234,actions=decap(),decap(),2
 ])
 
@@ -205,7 +205,7 @@ AT_CHECK([
     ovs-ofctl -Oopenflow13 dump-flows br0 | ofctl_strip | sort | grep actions  
], [0], [dnl
  in_port=4,dl_type=0x894f,nsh_mdtype=2,nsh_spi=0x1234 
actions=decap(),decap(),output:2
- ip,in_port=1 
actions=encap(nsh(md_type=2,tlv(0x1000,10,0x12345678))),set_field:0x1234->nsh_spi,encap(ethernet),set_field:11:22:33:44:55:66->eth_dst,output:3
+ ip,in_port=1 
+ actions=encap(nsh(md_type=2,tlv(0x1000,10,0x12345678),tlv(0x2000,20,0x
+ fedcba9876543210))),set_field:0x1234->nsh_spi,encap(ethernet),set_fiel
+ d:11:22:33:44:55:66->eth_dst,output:3
 ])
 
 AT_CHECK([
@@ -216,7 +216,7 @@ Flow: 
icmp,in_port=1,vlan_tci=0x0000,dl_src=00:11:22:33:44:55,dl_dst=66:77:88:99
 bridge("br0")
 -------------
  0. ip,in_port=1, priority 32768
-    encap(nsh(md_type=2,tlv(0x1000,10,0x12345678)))
+    
+ encap(nsh(md_type=2,tlv(0x1000,10,0x12345678),tlv(0x2000,20,0xfedcba98
+ 76543210)))
     set_field:0x1234->nsh_spi
     encap(ethernet)
     set_field:11:22:33:44:55:66->eth_dst
@@ -230,7 +230,7 @@ bridge("br0")
 
 Final flow: 
in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=11:22:33:44:55:66,dl_type=0x894f,nsh_flags=0,nsh_mdtype=2,nsh_np=3,nsh_spi=0x1234,nsh_si=255,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
 Megaflow: recirc_id=0,eth,ip,in_port=1,dl_dst=66:77:88:99:aa:bb,nw_frag=no
-Datapath actions: 
encap_nsh(flags=0,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a0412345678),push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:66),pop_eth,decap_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x1)
+Datapath actions: 
+encap_nsh(flags=0,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a041234567
+820001408fedcba9876543210),push_eth(src=00:00:00:00:00:00,dst=11:22:33:
+44:55:66),pop_eth,decap_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x
+1)
 ])
 
 AT_CHECK([
@@ -264,7 +264,7 @@ ovs-appctl time/warp 1000  AT_CHECK([
     ovs-appctl dpctl/dump-flows dummy@ovs-dummy | strip_used | grep -v ipv6 | 
sort  ], [0], [flow-dump from non-dpdk interfaces:
-recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),ipv4(frag=no),
 packets:1, bytes:98, used:0.0s, 
actions:encap_nsh(flags=0,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a0412345678),push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:66),pop_eth,decap_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x3)
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9
+e),eth_type(0x0800),ipv4(frag=no), packets:1, bytes:98, used:0.0s, 
+actions:encap_nsh(flags=0,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a0
+41234567820001408fedcba9876543210),push_eth(src=00:00:00:00:00:00,dst=1
+1:22:33:44:55:66),pop_eth,decap_nsh(),set(eth(dst=11:22:33:44:55:66)),r
+ecirc(0x3)
 
recirc_id(0x3),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:1, bytes:98, used:0.0s, actions:2
 ])
 
--
2.1.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to