Send from my phone
> Op 15 jul. 2023 om 06:01 heeft miter <[email protected]> het volgende > geschreven: > > Hi Eelco, > >> On 7/14/2023 9:18 PM, Eelco Chaudron wrote: >> >>> On 14 Jul 2023, at 13:34, Ilya Maximets wrote: >>> >>> On 7/14/23 12:42, Eelco Chaudron wrote: >>>> >>>> On 1 Jul 2023, at 16:40, [email protected] wrote: >>>> >>>>> From: Lin Huang <[email protected]> >>>>> >>>>> Now, token-bucket 'last_fill' is updated by token_bucket_withdraw() >>>>> itself. >>>>> Add a new function parameter 'now' to update timestamp by caller. >>>>> >>>>> Signed-off-by: Lin Huang <[email protected]> >>>> Thank for the patch Lin, >>>> >>>> However what about updating this entire patch to use time_now() instead of >>>> time_msec()? >>> Hmm. But time_now() is in seconds. It's a much lower resolution >>> for a bucket. We'll also need to change all the users to adjust >>> the configuration. >>> >>> Or am I missing something? >> >> ACK my bad, my browser tab compare did not work out great :( >> >> So my only comment would be maybe to avoid the time_now() in >> vlog_should_drop() under the mutex. >> >> //Eelco > I think we don't need to change the time_now() in vlog_should_drop(). > token_bucket_withdraw needs msec, but first_dropped needs sec. Well you can divide by 1000 and save a syscall under the mutex. >>> Bets regards, Ilya Maximets. >>> >>>> This makes it more clear for me. I did a quick change to see how it looks. >>>> >>>> This is what it looks like, let me know what you think: >>>> >>>> >>>> diff --git a/include/openvswitch/token-bucket.h >>>> b/include/openvswitch/token-bucket.h >>>> index d1191e956..3ae6b03fb 100644 >>>> --- a/include/openvswitch/token-bucket.h >>>> +++ b/include/openvswitch/token-bucket.h >>>> @@ -19,6 +19,7 @@ >>>> >>>> #include <limits.h> >>>> #include <stdbool.h> >>>> +#include <time.h> >>>> >>>> #ifdef __cplusplus >>>> extern "C" { >>>> @@ -41,7 +42,7 @@ void token_bucket_init(struct token_bucket *, >>>> void token_bucket_set(struct token_bucket *, >>>> unsigned int rate, unsigned int burst); >>>> bool token_bucket_withdraw(struct token_bucket *tb, unsigned int n, >>>> - long long int now); >>>> + time_t now); >>>> void token_bucket_wait_at(struct token_bucket *, unsigned int n, >>>> const char *where); >>>> #define token_bucket_wait(bucket, n) \ >>>> diff --git a/lib/token-bucket.c b/lib/token-bucket.c >>>> index 60eb26e53..65fe924eb 100644 >>>> --- a/lib/token-bucket.c >>>> +++ b/lib/token-bucket.c >>>> @@ -60,7 +60,7 @@ token_bucket_set(struct token_bucket *tb, >>>> * removed) . */ >>>> bool >>>> token_bucket_withdraw(struct token_bucket *tb, unsigned int n, >>>> - long long int now) >>>> + time_t now) >>>> { >>>> if (tb->tokens < n) { >>>> if (now > tb->last_fill) { >>>> diff --git a/lib/vlog.c b/lib/vlog.c >>>> index 380dcd479..91f2ae905 100644 >>>> --- a/lib/vlog.c >>>> +++ b/lib/vlog.c >>>> @@ -1320,10 +1320,10 @@ vlog_should_drop(const struct vlog_module *module, >>>> enum vlog_level level, >>>> return true; >>>> } >>>> >>>> + time_t now = time_now(); >>>> + >>>> ovs_mutex_lock(&rl->mutex); >>>> - if (!token_bucket_withdraw(&rl->token_bucket, VLOG_MSG_TOKENS, >>>> - time_msec())) { >>>> - time_t now = time_now(); >>>> + if (!token_bucket_withdraw(&rl->token_bucket, VLOG_MSG_TOKENS, now)) { >>>> if (!rl->n_dropped) { >>>> rl->first_dropped = now; >>>> } >>>> diff --git a/ofproto/pinsched.c b/ofproto/pinsched.c >>>> index a39e4d2ee..e857bf996 100644 >>>> --- a/ofproto/pinsched.c >>>> +++ b/ofproto/pinsched.c >>>> @@ -184,7 +184,7 @@ get_tx_packet(struct pinsched *ps) >>>> static bool >>>> get_token(struct pinsched *ps) >>>> { >>>> - return token_bucket_withdraw(&ps->token_bucket, 1000, time_msec()); >>>> + return token_bucket_withdraw(&ps->token_bucket, 1000, time_now()); >>>> } >>>> >>>> void >>>> >>>> >>>> Cheers, >>>> >>>> Eelco > > -- > Best regards, Huang Lin. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
