Re: [ovs-dev] [PATCH] nsh: enable struct ovs_action_encap_nsh to support variable length
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 YSubject: [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
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)