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

Reply via email to