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