Skip to content

Commit

Permalink
bridge: Fix log spam about prefixes.
Browse files Browse the repository at this point in the history
If prefixes are set for the flow table, ovs-vswitchd will print them
out to the log whenever something changes in the database.  Since
normally prefixes will be set for every OpenFlow table, it will print
255 log messages per iteration.  This is very annoying in dynamic
environments like Kubernetes, where database changes can happen
frequently, obscuring and erasing useful logs on log rotation.

These log messages are not very important.  The information can be
looked up in the database and normally the values will not actually
change after initial setup.  Move the log to debug level.

While at it, rate limit the warnings about misconfigured prefixes,
as they may be too much as well.  And make the print out a little
nicer by only printing once if multiple adjacent tables have the
same prefixes configured.  In most cases that will reduce the amount
of logs from 255 lines to 1 per iteration with debug logging enabled.
We're also now logging the default values as well, since it's under
debug and will not actually add that many log lines with the new
collapsed format.  This makes debug logs more accurate/useful.

An additional improvement might be to not print if nothing actually
changed, but that will require either per-table per-bridge tracking
of previous values or changing parts of the ofproto API to tell the
bridge layer if something changed or not.  Doesn't seem necessary
at the moment.

Fixes: 13751fd ("Classifier: Track address prefixes.")
Acked-by: Eelco Chaudron <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
igsilya committed Dec 11, 2024
1 parent 61d4bb8 commit 314aa33
Showing 1 changed file with 29 additions and 9 deletions.
38 changes: 29 additions & 9 deletions vswitchd/bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -4017,6 +4017,8 @@ static void
bridge_configure_tables(struct bridge *br)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
char *prev_prefixes = NULL;
int prev_start = 0;
int n_tables;
int i, j;

Expand Down Expand Up @@ -4078,18 +4080,21 @@ bridge_configure_tables(struct bridge *br)
}
mf = mf_from_name(name);
if (!mf) {
VLOG_WARN("bridge %s: 'prefixes' with unknown field: %s",
br->name, name);
VLOG_WARN_RL(&rl, "bridge %s: "
"'prefixes' with unknown field: %s",
br->name, name);
continue;
}
if (mf->flow_be32ofs < 0 || mf->n_bits % 32) {
VLOG_WARN("bridge %s: 'prefixes' with incompatible field: "
"%s", br->name, name);
VLOG_WARN_RL(&rl, "bridge %s: "
"'prefixes' with incompatible field: %s",
br->name, name);
continue;
}
if (s.n_prefix_fields >= ARRAY_SIZE(s.prefix_fields)) {
VLOG_WARN("bridge %s: 'prefixes' with too many fields, "
"field not used: %s", br->name, name);
VLOG_WARN_RL(&rl, "bridge %s: "
"'prefixes' with too many fields, "
"field not used: %s", br->name, name);
continue;
}
use_default_prefixes = false;
Expand All @@ -4101,8 +4106,10 @@ bridge_configure_tables(struct bridge *br)
s.n_prefix_fields = ARRAY_SIZE(default_prefix_fields);
memcpy(s.prefix_fields, default_prefix_fields,
sizeof default_prefix_fields);
} else {
}
if (VLOG_IS_DBG_ENABLED()) {
struct ds ds = DS_EMPTY_INITIALIZER;

for (int k = 0; k < s.n_prefix_fields; k++) {
if (k) {
ds_put_char(&ds, ',');
Expand All @@ -4112,15 +4119,28 @@ bridge_configure_tables(struct bridge *br)
if (s.n_prefix_fields == 0) {
ds_put_cstr(&ds, "none");
}
VLOG_INFO("bridge %s table %d: Prefix lookup with: %s.",
br->name, i, ds_cstr(&ds));
if (!prev_prefixes) {
prev_prefixes = ds_steal_cstr(&ds);
prev_start = i;
} else if (prev_prefixes && strcmp(prev_prefixes, ds_cstr(&ds))) {
VLOG_DBG("bridge %s tables %d-%d: Prefix lookup with: %s.",
br->name, prev_start, i - 1, prev_prefixes);
free(prev_prefixes);
prev_prefixes = ds_steal_cstr(&ds);
prev_start = i;
}
ds_destroy(&ds);
}

ofproto_configure_table(br->ofproto, i, &s);

free(s.groups);
}
if (prev_prefixes) {
VLOG_DBG("bridge %s tables %d-%d: Prefix lookup with: %s.",
br->name, prev_start, n_tables - 1, prev_prefixes);
free(prev_prefixes);
}
for (; j < br->cfg->n_flow_tables; j++) {
VLOG_WARN_RL(&rl, "bridge %s: ignoring configuration for flow table "
"%"PRId64" not supported by this datapath", br->name,
Expand Down

0 comments on commit 314aa33

Please sign in to comment.