On 8/30/22 18:05, Mike Pattrick wrote:
> Add additional logging for debug and info level with a focus on code
> related to bond members coming up, go down, and changing.
>
> Several existing log messages were modified to handle sub 1kB log
> messages with more grace. Instead of reporting 0kB of traffic load
> shifting from one member to another, we now suppress these messages at
> the INFO level and display exact byte count at the debug level.
>
> Signed-off-by: Mike Pattrick <[email protected]>
>
> --
>
> Since v5:
> - Implemented reverse xmas tree
> - Adjusted spacing and wrapping
> - Corrected info log units
>
> Since v4:
> - Removed prefixes
> - Removed unrequired conditional
>
> Since v3:
> - Removed log if bond members were significantly deviant
> - Normalized log prefixes, including LACP
> - Added details to the member enable/disable log
>
> Since v2:
> - Normalized log capitalization and periods
> - Summarized info level hash transfers
> - Added log if bond members were significantly deviant
>
> ---
> lib/lacp.c | 26 +++++++++---
> ofproto/bond.c | 113 +++++++++++++++++++++++++++++++++----------------
> 2 files changed, 98 insertions(+), 41 deletions(-)
>
...
> @@ -1166,14 +1183,6 @@ bond_shift_load(struct bond_entry *hash, struct
> bond_member *to)
> struct bond *bond = from->bond;
> uint64_t delta = hash->tx_bytes;
>
> - VLOG_INFO("bond %s: shift %"PRIu64"kB of load (with hash %"PRIdPTR") "
> - "from %s to %s (now carrying %"PRIu64"kB and "
> - "%"PRIu64"kB load, respectively)",
> - bond->name, delta / 1024, hash - bond->hash,
> - from->name, to->name,
> - (from->tx_bytes - delta) / 1024,
> - (to->tx_bytes + delta) / 1024);
> -
> /* Shift load away from 'from' to 'to'. */
> from->tx_bytes -= delta;
> to->tx_bytes += delta;
> @@ -1393,6 +1402,12 @@ bond_rebalance(struct bond *bond)
> reinsert_bal(&bals, from);
> reinsert_bal(&bals, to);
> }
> + VLOG_INFO("%s: shifted %"PRIuSIZE" hashes totaling %"PRIu64
> + "B from %s (%"PRIu64"B) to %s (%"PRIu64"B)",
> + bond->name, cnt,
> + (overload + to->tx_bytes - from->tx_bytes) / 2,
> + from->name, from->tx_bytes, to->name, to->tx_bytes);
> +
Hi, Mike. Sorry for the late reply.
The patch seems fine in general, but I'm not sure we need to change
the structure of the above log message. One thing we're loosing
by re-wording it is clarity of what is a new load and what was the
old load on bond members. If I would read the message "from A (X B)
to B (Y B)" not knowing the implementation I would say that X was
the load on A before the move and Y is a load on B after the move.
But in reality both values are values after the move.
We should probably keep the old structure or add more clarity in
some other way. Maybe something like this:
"bond0: shifted K hashes totaling zB from A to B (now carrying
XkB and YkB load, respectively)"
What do you think?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
