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? 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
