mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-22 01:51:26 +00:00
Code Issues Releases Wiki Activity

dpif: Fix flow put debug message match content.

Browse Source 
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.

This was found during a debug session of the ‘datapath - ping between two
ports on cvlan’ test case. The log message was showing the following:

  put[create] ufid:XX recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(3),
    skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),
    eth(src=12:f6:8b:52:f9:75,dst=6e:48:c8:77:d3:8c),eth_type(0x88a8),
    vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
    vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x0800),
    ipv4(src=10.2.2.2,dst=10.2.2.1,proto=1,tos=0,ttl=64,frag=no),
    icmp(type=0,code=0))), actions:2

Where it should have shown the below:

  put[create] ufid:XX recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(3),
    skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),
    eth(src=12:f6:8b:52:f9:75,dst=6e:48:c8:77:d3:8c),eth_type(0x88a8),
    vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100)), actions:2

Acked-by: Simon Horman <horms@ovn.org>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
This commit is contained in:
Eelco Chaudron 2024-09-03 11:37:16 +02:00
parent 604e54fc3c
commit 252ee0f182
8 changed files with 32 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/dpctl.c
View File

@ -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);

6
lib/dpif-netdev.c
View File

@ -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);
}

4
lib/dpif.c
View File

@ -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);

31
lib/odp-util.c
View File

@ -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);

2
lib/odp-util.h
View File

@ -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);

2
lib/tnl-ports.c
View File

@ -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");
}
}

2
ofproto/ofproto-dpif.c
View File

@ -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:");

6
tests/test-odp.c
View File

@ -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);