Thanks Jan, I have folded it to my local branch and will include it in next version.
-----Original Message----- From: Jan Scheurich [mailto:jan.scheur...@ericsson.com] Sent: Tuesday, July 18, 2017 12:30 AM To: Yang, Yi Y <yi.y.y...@intel.com>; d...@openvswitch.org Cc: Mengke Liu <mengke....@intel.com>; Li, Ricky <ricky...@intel.com>; Li, Johnson <johnson...@intel.com> Subject: RE: [PATCH 1/6] userspace: Add support for NSH MD1 match fields Hi Yi, The following incremental makes the formatting of NSH fields in OpenFlow and datapath flows more consistent and easier to use. - Consistently format nsh_spi as hex without leading zeros. - Consistently format MD1 context headers nsh_c<X> without leading zeros. - Format nsh_c<X> headers only if md_type is MD1. - Bugfix: format_odp_encap_nsh_action() was casting the nlattr to incorrect type (struct ovs_key_nsh). This is nor corrected. The new version also dumps the complete TLV metadata as hex sequence in case of MD2 format. diff --git a/lib/match.c b/lib/match.c index 67fa958..a1e4c5c 100644 --- a/lib/match.c +++ b/lib/match.c @@ -1158,9 +1158,9 @@ format_be32_masked_hex(struct ds *s, const char *name, if (mask != htonl(0)) { ds_put_format(s, "%s%s=%s", colors.param, name, colors.end); if (mask == OVS_BE32_MAX) { - ds_put_format(s, "0x%08"PRIx32, ntohl(value)); + ds_put_format(s, "0x%"PRIx32, ntohl(value)); } else { - ds_put_format(s, "0x%08"PRIx32"/0x%08"PRIx32, + ds_put_format(s, "0x%"PRIx32"/0x%"PRIx32, ntohl(value), ntohl(mask)); } ds_put_char(s, ','); @@ -1260,18 +1260,20 @@ format_nsh_masked(struct ds *s, const struct flow *f, const struct flow *m) if (m->nsh.np) format_uint8_masked(s, "nsh_np", f->nsh.np, m->nsh.np); if (m->nsh.spi) - format_be32_masked(s, "nsh_spi", f->nsh.spi, m->nsh.spi); + format_be32_masked_hex(s, "nsh_spi", f->nsh.spi, m->nsh.spi); if (m->nsh.si) format_uint8_masked(s, "nsh_si", f->nsh.si, m->nsh.si); - if (m->nsh.c1) - format_be32_masked_hex(s, "nsh_c1", f->nsh.c1, m->nsh.c1); - if (m->nsh.c2) - format_be32_masked_hex(s, "nsh_c2", f->nsh.c2, m->nsh.c2); - if (m->nsh.c3) - format_be32_masked_hex(s, "nsh_c3", f->nsh.c3, m->nsh.c3); - if (m->nsh.c4) - format_be32_masked_hex(s, "nsh_c4", f->nsh.c4, m->nsh.c4); + if (m->nsh.mdtype == UINT8_MAX && f->nsh.mdtype == NSH_M_TYPE1) { + if (m->nsh.c1) + format_be32_masked_hex(s, "nsh_c1", f->nsh.c1, m->nsh.c1); + if (m->nsh.c2) + format_be32_masked_hex(s, "nsh_c2", f->nsh.c2, m->nsh.c2); + if (m->nsh.c3) + format_be32_masked_hex(s, "nsh_c3", f->nsh.c3, m->nsh.c3); + if (m->nsh.c4) + format_be32_masked_hex(s, "nsh_c4", f->nsh.c4, m->nsh.c4); + } } /* Appends a string representation of 'match' to 's'. If 'priority' is diff --git a/lib/odp-util.c b/lib/odp-util.c index 1d8e24b..122f5b7 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -302,9 +302,9 @@ format_be32_masked(struct ds *s, bool *first, const char *name, } ds_put_format(s, "%s=", name); if (mask == OVS_BE32_MAX) { - ds_put_format(s, "0x%08"PRIx32, ntohl(value)); + ds_put_format(s, "0x%"PRIx32, ntohl(value)); } else { - ds_put_format(s, "0x%08"PRIx32"/0x%08"PRIx32, + ds_put_format(s, "0x%"PRIx32"/0x%"PRIx32, ntohl(value), ntohl(mask)); } *first = false; @@ -340,17 +340,36 @@ format_nsh_key_mask(struct ds *ds, const struct ovs_key_nsh *key, } static void -format_odp_encap_nsh_action(struct ds *ds, const struct nlattr *attr) +format_odp_encap_nsh_action(struct ds *ds, + const struct ovs_action_encap_nsh +*encap_nsh) { - if (nl_attr_type(attr) != OVS_ACTION_ATTR_ENCAP_NSH) { - return; - } - ds_put_cstr(ds, "encap_nsh("); + uint32_t path_hdr = ntohl(encap_nsh->path_hdr); + uint32_t spi = (path_hdr & NSH_SPI_MASK) >> NSH_SPI_SHIFT; + uint8_t si = (path_hdr & NSH_SI_MASK) >> NSH_SI_SHIFT; + bool first = true; - size_t len = nl_attr_get_size(attr); - if (len) { - const struct ovs_key_nsh *nsh_key = nl_attr_get(attr); - format_nsh_key(ds, nsh_key); + ds_put_cstr(ds, "encap_nsh("); + format_uint8_masked(ds, &first, "flags", encap_nsh->flags, UINT8_MAX); + format_uint8_masked(ds, &first, "mdtype", encap_nsh->mdtype, UINT8_MAX); + format_uint8_masked(ds, &first, "np", encap_nsh->np, UINT8_MAX); + format_be32_masked(ds, &first, "spi", htonl(spi), OVS_BE32_MAX); + format_uint8_masked(ds, &first, "si", si, UINT8_MAX); + switch (encap_nsh->mdtype) { + case NSH_M_TYPE1: { + struct nsh_md1_ctx *md1_ctx = + ALIGNED_CAST(struct nsh_md1_ctx *, encap_nsh->metadata); + format_be32_masked(ds, &first, "c1", md1_ctx->c1, OVS_BE32_MAX); + format_be32_masked(ds, &first, "c2", md1_ctx->c2, OVS_BE32_MAX); + format_be32_masked(ds, &first, "c3", md1_ctx->c3, OVS_BE32_MAX); + format_be32_masked(ds, &first, "c4", md1_ctx->c4, OVS_BE32_MAX); + break; + } + case NSH_M_TYPE2: + ds_put_cstr(ds, ",md2="); + ds_put_hex(ds, encap_nsh->metadata, encap_nsh->mdlen); + break; + default: + OVS_NOT_REACHED(); } ds_put_format(ds, ")"); } @@ -1038,7 +1057,7 @@ format_odp_action(struct ds *ds, const struct nlattr *a, format_odp_clone_action(ds, a, portno_names); break; case OVS_ACTION_ATTR_ENCAP_NSH: - format_odp_encap_nsh_action(ds, a); + format_odp_encap_nsh_action(ds, nl_attr_get(a)); break; case OVS_ACTION_ATTR_DECAP_NSH: ds_put_cstr(ds, "decap_nsh()"); _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev