On 12.03.2025 14:11, Ilya Maximets wrote: > On 3/12/25 11:22, Vladislav Odintsov wrote: >> Hi Han, Ilya and Adrian, >> >> I'm also suffering from these messages in logs. Han, do you have plans >> on working with requested changes? >> >> If you've got no time at the moment, I can continue this work addressing >> reviewers' comments. Ilya, you said 1kB is a small chunk of data and we >> shouldn't care too much about it. Which value is okay to ignore from >> your perspective? > Are you suffering from excessive logging or from not enough granularity > in the logs? This patch increases the accuracy and doesn't reduce the > amount of logs. > > If the amount of logs is a problem, then maybe we should move the logs > for individual load shifts to DBG level (maybe keep a particularly heavy > ones like >1MB move at INFO) and have a separate overview of the change > instead showing the load distribution after the rebalancing, e.g.: > > bond X: rebalanced (now carrying: portA:XXXkB, portB:YYYkB, portC:ZZZkB) > > For the debug logs we could improve the accuracy as this patch does. > > What do you think?
We suffer from excessive logging of "0 kB" records. There are tons of them. So I planned to take your proposal and do not to log these events at all. But I like your last idea about hiding messages of re-balancing <1MB under DBG log level and keep INFO for others. If you guys are okay with that, I can prepare and send a patch. > > Best regards, Ilya Maximets. > >> On 05.08.2024 10:08, Adrián Moreno wrote: >>> On Fri, Aug 02, 2024 at 06:38:19PM GMT, Ilya Maximets wrote: >>>> On 7/2/24 16:36, Han Ding wrote: >>>>> When the delta is less than 1024 in bond_shift_load, it print "shift 0kB >>>>> of load". >>>>> Like this: >>>>> bond dpdkbond0: shift 0kB of load (with hash 71) from nic1 to nic2 (now >>>>> carrying 20650165kB and 8311662kB load, respectively) >>>> Hi, Han. Thanks for the patch! >>>> >>>> I'm curious, is this information about movements that small >>>> practically useful? >>>> >>> I had the same thought. >>> I guess it should not happen very often given how the algorithm works but >>> OTOH, printing "0kB" is definitely not useful. >>> >>>>> Signed-off-by: Han Ding <[email protected]> >>>>> --- >>>>> ofproto/bond.c | 24 +++++++++++++++++------- >>>>> 1 file changed, 17 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/ofproto/bond.c b/ofproto/bond.c >>>>> index c31869a..5b1975d 100644 >>>>> --- a/ofproto/bond.c >>>>> +++ b/ofproto/bond.c >>>>> @@ -1192,13 +1192,23 @@ 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); >>>>> + if (delta >= 1024) { >>>>> + 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); >>>>> + } else { >>>>> + VLOG_INFO("bond %s: shift %"PRIu64"B of load (with hash >>>>> %"PRIdPTR") " >>>>> + "from %s to %s (now carrying %"PRIu64"kB and " >>> Apart from Ilya's comment, missing one more indentation space in this >>> line (and all others). >>> >>> Thanks. >>> >>>>> + "%"PRIu64"kB load, respectively)", >>>>> + bond->name, delta, hash - bond->hash, >>>>> + from->name, to->name, >>>>> + (from->tx_bytes - delta) / 1024, >>>>> + (to->tx_bytes + delta) / 1024); >>>>> + } >>>> I'd suggest instead of copying the whole thing, just replace "kB" >>>> with another %s and use a couple of ternary operators to produce >>>> correct value and "kB" or "B" depending on the delta. >>>> >>>> Best regards, Ilya Maximets. >>>> _______________________________________________ >>>> dev mailing list >>>> [email protected] >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev -- Regards, Vladislav Odintsov _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
