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()? 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
> ---
> include/openvswitch/token-bucket.h | 3 ++-
> lib/token-bucket.c | 4 ++--
> lib/vlog.c | 3 ++-
> ofproto/pinsched.c | 2 +-
> 4 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/include/openvswitch/token-bucket.h
> b/include/openvswitch/token-bucket.h
> index 580747f61..d1191e956 100644
> --- a/include/openvswitch/token-bucket.h
> +++ b/include/openvswitch/token-bucket.h
> @@ -40,7 +40,8 @@ void token_bucket_init(struct token_bucket *,
> unsigned int rate, unsigned int burst);
> void token_bucket_set(struct token_bucket *,
> unsigned int rate, unsigned int burst);
> -bool token_bucket_withdraw(struct token_bucket *, unsigned int n);
> +bool token_bucket_withdraw(struct token_bucket *tb, unsigned int n,
> + long long int 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 0badeb46b..60eb26e53 100644
> --- a/lib/token-bucket.c
> +++ b/lib/token-bucket.c
> @@ -59,10 +59,10 @@ token_bucket_set(struct token_bucket *tb,
> * if 'tb' contained fewer than 'n' tokens (and thus 'n' tokens could not be
> * removed) . */
> bool
> -token_bucket_withdraw(struct token_bucket *tb, unsigned int n)
> +token_bucket_withdraw(struct token_bucket *tb, unsigned int n,
> + long long int now)
> {
> if (tb->tokens < n) {
> - long long int now = time_msec();
> if (now > tb->last_fill) {
> unsigned long long int elapsed_ull
> = (unsigned long long int) now - tb->last_fill;
> diff --git a/lib/vlog.c b/lib/vlog.c
> index b2653142f..380dcd479 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -1321,7 +1321,8 @@ vlog_should_drop(const struct vlog_module *module, enum
> vlog_level level,
> }
>
> ovs_mutex_lock(&rl->mutex);
> - if (!token_bucket_withdraw(&rl->token_bucket, VLOG_MSG_TOKENS)) {
> + if (!token_bucket_withdraw(&rl->token_bucket, VLOG_MSG_TOKENS,
> + time_msec())) {
> time_t now = time_now();
> if (!rl->n_dropped) {
> rl->first_dropped = now;
> diff --git a/ofproto/pinsched.c b/ofproto/pinsched.c
> index 59561f076..a39e4d2ee 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);
> + return token_bucket_withdraw(&ps->token_bucket, 1000, time_msec());
> }
>
> void
> --
> 2.39.1
>
> _______________________________________________
> 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
