The odp_flow_format() function applies a wildcard mask when a mask for a given key was not present. However, in the context of installing flows in the datapath, the absence of a mask actually indicates that the key should be ignored, meaning it should not be masked at all.
To address this inconsistency, odp_flow_format() now includes an option to skip formatting keys that are missing a mask. Signed-off-by: Eelco Chaudron <[email protected]> --- lib/dpctl.c | 4 ++-- lib/dpif-netdev.c | 6 +++--- lib/dpif.c | 4 ++-- lib/odp-util.c | 31 ++++++++++++++++++------------- lib/odp-util.h | 2 +- lib/tnl-ports.c | 2 +- ofproto/ofproto-dpif.c | 2 +- tests/test-odp.c | 6 ++++-- 8 files changed, 32 insertions(+), 25 deletions(-) diff --git a/lib/dpctl.c b/lib/dpctl.c index f764cf164..77bf4bf53 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -877,7 +877,7 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports, ds_put_cstr(ds, ", "); } odp_flow_format(f->key, f->key_len, f->mask, f->mask_len, ports, ds, - dpctl_p->verbosity); + dpctl_p->verbosity, false); ds_put_cstr(ds, ", "); dpif_flow_stats_format(&f->stats, ds); @@ -2883,7 +2883,7 @@ dpctl_normalize_actions(int argc, const char *argv[], ds_clear(&s); odp_flow_format(keybuf.data, keybuf.size, NULL, 0, NULL, - &s, dpctl_p->verbosity); + &s, dpctl_p->verbosity, false); dpctl_print(dpctl_p, "input flow: %s\n", ds_cstr(&s)); error = odp_flow_key_to_flow(keybuf.data, keybuf.size, &flow, &error_s); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index f0594e5f5..f6d6d01cf 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3039,7 +3039,7 @@ log_netdev_flow_change(const struct dp_netdev_flow *flow, ds_put_cstr(&ds, " "); odp_flow_format(key_buf.data, key_buf.size, mask_buf.data, mask_buf.size, - NULL, &ds, false); + NULL, &ds, false, true); if (old_actions) { ds_put_cstr(&ds, ", old_actions:"); format_odp_actions(&ds, old_actions->actions, old_actions->size, @@ -3859,7 +3859,7 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len, ds_init(&s); odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s, - true); + true, true); VLOG_ERR("internal error parsing flow mask %s (%s)", ds_cstr(&s), odp_key_fitness_to_string(fitness)); ds_destroy(&s); @@ -3888,7 +3888,7 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len, struct ds s; ds_init(&s); - odp_flow_format(key, key_len, NULL, 0, NULL, &s, true); + odp_flow_format(key, key_len, NULL, 0, NULL, &s, true, false); VLOG_ERR("internal error parsing flow key %s", ds_cstr(&s)); ds_destroy(&s); } diff --git a/lib/dpif.c b/lib/dpif.c index 245c8b5ee..a064f717f 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1805,7 +1805,7 @@ log_flow_message(const struct dpif *dpif, int error, odp_format_ufid(ufid, &ds); ds_put_cstr(&ds, " "); } - odp_flow_format(key, key_len, mask, mask_len, NULL, &ds, true); + odp_flow_format(key, key_len, mask, mask_len, NULL, &ds, true, true); if (stats) { ds_put_cstr(&ds, ", "); dpif_flow_stats_format(stats, &ds); @@ -1906,7 +1906,7 @@ log_execute_message(const struct dpif *dpif, } ds_put_format(&ds, " on packet %s", packet); ds_put_format(&ds, " with metadata "); - odp_flow_format(md.data, md.size, NULL, 0, NULL, &ds, true); + odp_flow_format(md.data, md.size, NULL, 0, NULL, &ds, true, false); ds_put_format(&ds, " mtu %d", execute->mtu); vlog(module, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr(&ds)); ds_destroy(&ds); diff --git a/lib/odp-util.c b/lib/odp-util.c index 7c80a7e88..ee1868202 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -4269,7 +4269,7 @@ mask_empty(const struct nlattr *ma) static void format_odp_key_attr__(const struct nlattr *a, const struct nlattr *ma, const struct hmap *portno_names, struct ds *ds, - bool verbose) + bool verbose, bool skip_no_mask) { enum ovs_key_attr attr = nl_attr_type(a); char namebuf[OVS_KEY_ATTR_BUFSIZE]; @@ -4285,10 +4285,10 @@ format_odp_key_attr__(const struct nlattr *a, const struct nlattr *ma, if (ma && nl_attr_get_size(ma) && nl_attr_get_size(a)) { odp_flow_format(nl_attr_get(a), nl_attr_get_size(a), nl_attr_get(ma), nl_attr_get_size(ma), NULL, ds, - verbose); + verbose, skip_no_mask); } else if (nl_attr_get_size(a)) { odp_flow_format(nl_attr_get(a), nl_attr_get_size(a), NULL, 0, NULL, - ds, verbose); + ds, verbose, skip_no_mask); } break; @@ -4596,7 +4596,7 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma, { if (check_attr_len(ds, a, ma, ovs_flow_key_attr_lens, OVS_KEY_ATTR_MAX, false)) { - format_odp_key_attr__(a, ma, portno_names, ds, verbose); + format_odp_key_attr__(a, ma, portno_names, ds, verbose, false); } } @@ -4710,13 +4710,16 @@ odp_format_ufid(const ovs_u128 *ufid, struct ds *ds) } /* Appends to 'ds' a string representation of the 'key_len' bytes of - * OVS_KEY_ATTR_* attributes in 'key'. If non-null, additionally formats the - * 'mask_len' bytes of 'mask' which apply to 'key'. If 'portno_names' is - * non-null, translates odp port number to its name. */ + * OVS_KEY_ATTR_* attributes in 'key'. If non-null, additionally formats the + * 'mask_len' bytes of 'mask' which apply to 'key'. If 'portno_names' is + * non-null, translates odp port number to its name. If 'skip_no_mask' is set + * to true, OVS_KEY_ATTR_* entries without a mask will not be printed, even + * when verbose mode is 'true'. */ void odp_flow_format(const struct nlattr *key, size_t key_len, const struct nlattr *mask, size_t mask_len, - const struct hmap *portno_names, struct ds *ds, bool verbose) + const struct hmap *portno_names, struct ds *ds, bool verbose, + bool skip_no_mask) { if (key_len) { const struct nlattr *a; @@ -4734,7 +4737,8 @@ odp_flow_format(const struct nlattr *key, size_t key_len, attr_type) : NULL); if (!check_attr_len(ds, a, ma, ovs_flow_key_attr_lens, - OVS_KEY_ATTR_MAX, false)) { + OVS_KEY_ATTR_MAX, false) + || (skip_no_mask && !ma)) { continue; } @@ -4765,7 +4769,8 @@ odp_flow_format(const struct nlattr *key, size_t key_len, if (!first_field) { ds_put_char(ds, ','); } - format_odp_key_attr__(a, ma, portno_names, ds, verbose); + format_odp_key_attr__(a, ma, portno_names, ds, verbose, + skip_no_mask); first_field = false; } else if (attr_type == OVS_KEY_ATTR_ETHERNET && !has_packet_type_key) { @@ -4836,7 +4841,7 @@ void odp_flow_key_format(const struct nlattr *key, size_t key_len, struct ds *ds) { - odp_flow_format(key, key_len, NULL, 0, NULL, ds, true); + odp_flow_format(key, key_len, NULL, 0, NULL, ds, true, false); } static bool @@ -7747,7 +7752,7 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, struct ds s; ds_init(&s); - odp_flow_format(key, key_len, NULL, 0, NULL, &s, true); + odp_flow_format(key, key_len, NULL, 0, NULL, &s, true, false); VLOG_ERR("internal error parsing flow key %s (%s)", ds_cstr(&s), odp_key_fitness_to_string(fitness)); ds_destroy(&s); @@ -7767,7 +7772,7 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, ds_init(&s); odp_flow_format(key, key_len, mask, mask_len, NULL, &s, - true); + true, true); VLOG_ERR("internal error parsing flow mask %s (%s)", ds_cstr(&s), odp_key_fitness_to_string(fitness)); ds_destroy(&s); diff --git a/lib/odp-util.h b/lib/odp-util.h index e454dbfcd..339ffc14f 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -171,7 +171,7 @@ void odp_format_ufid(const ovs_u128 *ufid, struct ds *); void odp_flow_format(const struct nlattr *key, size_t key_len, const struct nlattr *mask, size_t mask_len, const struct hmap *portno_names, struct ds *, - bool verbose); + bool verbose, bool skip_no_mask); void odp_flow_key_format(const struct nlattr *, size_t, struct ds *); int odp_flow_from_string(const char *s, const struct simap *port_names, struct ofpbuf *, struct ofpbuf *, char **errorp); diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c index bb0b0b0c5..a1dec89d4 100644 --- a/lib/tnl-ports.c +++ b/lib/tnl-ports.c @@ -347,7 +347,7 @@ tnl_port_show_v(struct ds *ds) mask_len = buf.size; /* build string. */ - odp_flow_format(key, key_len, mask, mask_len, NULL, ds, false); + odp_flow_format(key, key_len, mask, mask_len, NULL, ds, false, false); ds_put_format(ds, "\n"); } } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index d3c353b9d..4659d8a3b 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -6874,7 +6874,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn, ds_put_cstr(&ds, " "); } odp_flow_format(f.key, f.key_len, f.mask, f.mask_len, - portno_names, &ds, verbosity); + portno_names, &ds, verbosity, false); ds_put_cstr(&ds, ", "); dpif_flow_stats_format(&f.stats, &ds); ds_put_cstr(&ds, ", actions:"); diff --git a/tests/test-odp.c b/tests/test-odp.c index 0ddfd4070..1e64241ad 100644 --- a/tests/test-odp.c +++ b/tests/test-odp.c @@ -105,7 +105,8 @@ parse_keys(bool wc_keys) ds_init(&out); if (wc_keys) { odp_flow_format(odp_key.data, odp_key.size, - odp_mask.data, odp_mask.size, NULL, &out, false); + odp_mask.data, odp_mask.size, NULL, &out, false, + false); } else { odp_flow_key_format(odp_key.data, odp_key.size, &out); } @@ -219,7 +220,8 @@ parse_filter(char *filter_parse) /* Convert odp_key to string. */ ds_init(&out); odp_flow_format(odp_key.data, odp_key.size, - odp_mask.data, odp_mask.size, NULL, &out, false); + odp_mask.data, odp_mask.size, NULL, &out, false, + false); puts(ds_cstr(&out)); ds_destroy(&out); -- 2.46.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
