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?
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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
