On Wed, May 23, 2018 at 7:13 PM, Darrell Ball <dlu...@gmail.com> wrote:
> The struct erspan_metadata is updated to replace the 'version'
> placeholder with the erspan base hdr. Also, the erspan
> index is defined explicitly as ovs_16aligned_be32 to mirror
> its encoding.
> Changes to odp_util result from updating the erspan index
> type.
>
> CC: William Tu <u9012...@gmail.com>
> Fixes: 068794b43f0e ("erspan: Add flow-based erspan options")
> Signed-off-by: Darrell Ball <dlu...@gmail.com>
> ---
> lib/odp-util.c | 36 ++++++++++++++++++++----------------
> lib/packets.h | 6 +++---
> 2 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 105ac80..767281f 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -2786,9 +2786,9 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool
> is_mask,
>
> memcpy(&opts, nl_attr_get(a), attr_len);
>
> - tun->erspan_ver = opts.version;
> + tun->erspan_ver = opts.bh.ver;
> if (tun->erspan_ver == 1) {
> - tun->erspan_idx = ntohl(opts.u.index);
> + tun->erspan_idx = ntohl(get_16aligned_be32(&opts.u.index));
> } else if (tun->erspan_ver == 2) {
> tun->erspan_dir = opts.u.md2.dir;
> tun->erspan_hwid = get_hwid(&opts.u.md2);
> @@ -2890,10 +2890,11 @@ tun_key_to_attr(struct ofpbuf *a, const struct
> flow_tnl *tun_key,
> !strcmp(tnl_type, "ip6erspan")) &&
> (tun_key->erspan_ver == 1 || tun_key->erspan_ver == 2)) {
> struct erspan_metadata opts;
> + memset(&opts, 0, sizeof opts);
>
> - opts.version = tun_key->erspan_ver;
> - if (opts.version == 1) {
> - opts.u.index = htonl(tun_key->erspan_idx);
> + opts.bh.ver = tun_key->erspan_ver;
> + if (opts.bh.ver == 1) {
> + put_16aligned_be32(&opts.u.index, htonl(tun_key->erspan_idx));
> } else {
> opts.u.md2.dir = tun_key->erspan_dir;
> set_hwid(&opts.u.md2, tun_key->erspan_hwid);
> @@ -3368,22 +3369,23 @@ format_odp_tun_erspan_opt(const struct nlattr *attr,
> opts = nl_attr_get(attr);
> mask = mask_attr ? nl_attr_get(mask_attr) : NULL;
>
> - ver = (uint8_t)opts->version;
> + ver = opts->bh.ver;
> if (mask) {
> - ver_ma = (uint8_t)mask->version;
> + ver_ma = mask->bh.ver;
> }
>
> format_u8u(ds, "ver", ver, mask ? &ver_ma : NULL, verbose);
>
> - if (opts->version == 1) {
> + if (opts->bh.ver == 1) {
> if (mask) {
> ds_put_format(ds, "idx=%#"PRIx32"/%#"PRIx32",",
> - ntohl(opts->u.index),
> - ntohl(mask->u.index));
> + ntohl(get_16aligned_be32(&opts->u.index)),
> + ntohl(get_16aligned_be32(&mask->u.index)));
> } else {
> - ds_put_format(ds, "idx=%#"PRIx32",", ntohl(opts->u.index));
> + ds_put_format(ds, "idx=%#"PRIx32",",
> + ntohl(get_16aligned_be32(&opts->u.index)));
> }
> - } else if (opts->version == 2) {
> + } else if (opts->bh.ver == 2) {
> dir = opts->u.md2.dir;
> hwid = opts->u.md2.hwid;
> if (mask) {
> @@ -4859,10 +4861,11 @@ scan_erspan_metadata(const char *s,
>
> if (!strncmp(s, ")", 1)) {
> s += 1;
> - key->version = ver;
> - key->u.index = htonl(idx);
> + memset(&key->bh, 0, sizeof key->bh);
> + key->bh.ver = ver;
> + put_16aligned_be32(&key->u.index, htonl(idx));
> if (mask) {
> - mask->u.index = htonl(idx_mask);
> + put_16aligned_be32(&mask->u.index, htonl(idx_mask));
> }
> }
> return s - s_base;
> @@ -4882,7 +4885,8 @@ scan_erspan_metadata(const char *s,
>
> if (!strncmp(s, ")", 1)) {
> s += 1;
> - key->version = ver;
> + memset(&key->bh, 0, sizeof key->bh);
> + key->bh.ver = ver;
> key->u.md2.hwid = hwid;
> key->u.md2.dir = dir;
> if (mask) {
> diff --git a/lib/packets.h b/lib/packets.h
> index 7645a9d..5c013a3 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -1312,10 +1312,10 @@ struct erspan_md2 {
> };
>
> struct erspan_metadata {
> - int version;
> + struct erspan_base_hdr bh;
> union {
> - ovs_be32 index; /* Version 1 (type II)*/
> - struct erspan_md2 md2; /* Version 2 (type III) */
> + ovs_16aligned_be32 index; /* Version 1 (type II). */
> + struct erspan_md2 md2; /* Version 2 (type III). */
> } u;
> };
>
Thanks for the patch.
We shouldn't change this header since struct erspan_metadata is exposed
from kernel's UAPI header. (see include/uapi/linux/erspan.h).
OVS kernel module expects this binary format.
Regards,
William
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
