* [PATCH iproute2-next 1/1] tc: report time an action was first used
@ 2020-05-17 13:28 Roman Mashak
2020-05-18 13:10 ` Jamal Hadi Salim
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Roman Mashak @ 2020-05-17 13:28 UTC (permalink / raw)
To: dsahern; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
Have print_tm() dump firstuse value along with install, lastuse
and expires.
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
tc/tc_util.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 12f865cc71bf..f6aa2ed552a9 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
print_uint(PRINT_FP, NULL, " used %u sec",
(unsigned int)(tm->lastuse/hz));
}
+ if (tm->firstuse != 0) {
+ print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
+ print_uint(PRINT_FP, NULL, " firstused %u sec",
+ (unsigned int)(tm->firstuse/hz));
+ }
if (tm->expires != 0) {
print_uint(PRINT_JSON, "expires", NULL, tm->expires);
print_uint(PRINT_FP, NULL, " expires %u sec",
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-17 13:28 [PATCH iproute2-next 1/1] tc: report time an action was first used Roman Mashak
@ 2020-05-18 13:10 ` Jamal Hadi Salim
2020-05-18 15:38 ` David Ahern
2020-05-18 15:36 ` David Ahern
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Jamal Hadi Salim @ 2020-05-18 13:10 UTC (permalink / raw)
To: Roman Mashak, dsahern; +Cc: stephen, netdev, kernel, xiyou.wangcong, jiri
On 2020-05-17 9:28 a.m., Roman Mashak wrote:
> Have print_tm() dump firstuse value along with install, lastuse
> and expires.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> tc/tc_util.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 12f865cc71bf..f6aa2ed552a9 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
> print_uint(PRINT_FP, NULL, " used %u sec",
> (unsigned int)(tm->lastuse/hz));
> }
> + if (tm->firstuse != 0) {
> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
> + print_uint(PRINT_FP, NULL, " firstused %u sec",
> + (unsigned int)(tm->firstuse/hz));
> + }
Maybe an else as well to print something like "firstused NEVER"
or alternatively just print 0 (to be backward compatible on old
kernels it will never be zero).
cheers,
jamal
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-18 13:10 ` Jamal Hadi Salim
@ 2020-05-18 15:38 ` David Ahern
2020-05-19 9:09 ` Jamal Hadi Salim
0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2020-05-18 15:38 UTC (permalink / raw)
To: Jamal Hadi Salim, Roman Mashak
Cc: stephen, netdev, kernel, xiyou.wangcong, jiri
On 5/18/20 7:10 AM, Jamal Hadi Salim wrote:
> On 2020-05-17 9:28 a.m., Roman Mashak wrote:
>> Have print_tm() dump firstuse value along with install, lastuse
>> and expires.
>>
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>> tc/tc_util.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>> index 12f865cc71bf..f6aa2ed552a9 100644
>> --- a/tc/tc_util.c
>> +++ b/tc/tc_util.c
>> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
>> print_uint(PRINT_FP, NULL, " used %u sec",
>> (unsigned int)(tm->lastuse/hz));
>> }
>> + if (tm->firstuse != 0) {
>> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
>> + print_uint(PRINT_FP, NULL, " firstused %u sec",
>> + (unsigned int)(tm->firstuse/hz));
>> + }
>
> Maybe an else as well to print something like "firstused NEVER"
> or alternatively just print 0 (to be backward compatible on old
> kernels it will never be zero).
>
existing times do not, so shouldn't this be consistent?
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-18 15:38 ` David Ahern
@ 2020-05-19 9:09 ` Jamal Hadi Salim
0 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2020-05-19 9:09 UTC (permalink / raw)
To: David Ahern, Roman Mashak; +Cc: stephen, netdev, kernel, xiyou.wangcong, jiri
On 2020-05-18 11:38 a.m., David Ahern wrote:
> On 5/18/20 7:10 AM, Jamal Hadi Salim wrote:
>> On 2020-05-17 9:28 a.m., Roman Mashak wrote:
>>> Have print_tm() dump firstuse value along with install, lastuse
>>> and expires.
>>>
>>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>>> ---
>>> tc/tc_util.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>>> index 12f865cc71bf..f6aa2ed552a9 100644
>>> --- a/tc/tc_util.c
>>> +++ b/tc/tc_util.c
>>> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
>>> print_uint(PRINT_FP, NULL, " used %u sec",
>>> (unsigned int)(tm->lastuse/hz));
>>> }
>>> + if (tm->firstuse != 0) {
>>> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
>>> + print_uint(PRINT_FP, NULL, " firstused %u sec",
>>> + (unsigned int)(tm->firstuse/hz));
>>> + }
>>
>> Maybe an else as well to print something like "firstused NEVER"
>> or alternatively just print 0 (to be backward compatible on old
>> kernels it will never be zero).
>>
>
> existing times do not, so shouldn't this be consistent?
>
Good point..
cheers,
jamal
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-17 13:28 [PATCH iproute2-next 1/1] tc: report time an action was first used Roman Mashak
2020-05-18 13:10 ` Jamal Hadi Salim
@ 2020-05-18 15:36 ` David Ahern
2020-05-18 16:41 ` Roman Mashak
2020-05-19 18:06 ` Stephen Hemminger
2020-05-19 18:11 ` David Ahern
3 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2020-05-18 15:36 UTC (permalink / raw)
To: Roman Mashak; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri
On 5/17/20 7:28 AM, Roman Mashak wrote:
> Have print_tm() dump firstuse value along with install, lastuse
> and expires.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> tc/tc_util.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 12f865cc71bf..f6aa2ed552a9 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
> print_uint(PRINT_FP, NULL, " used %u sec",
> (unsigned int)(tm->lastuse/hz));
> }
> + if (tm->firstuse != 0) {
> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
> + print_uint(PRINT_FP, NULL, " firstused %u sec",
> + (unsigned int)(tm->firstuse/hz));
> + }
> if (tm->expires != 0) {
> print_uint(PRINT_JSON, "expires", NULL, tm->expires);
> print_uint(PRINT_FP, NULL, " expires %u sec",
>
why does this function print different values for json and stdout?
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-18 15:36 ` David Ahern
@ 2020-05-18 16:41 ` Roman Mashak
2020-05-19 18:08 ` Stephen Hemminger
0 siblings, 1 reply; 11+ messages in thread
From: Roman Mashak @ 2020-05-18 16:41 UTC (permalink / raw)
To: David Ahern; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri
David Ahern <dsahern@gmail.com> writes:
> On 5/17/20 7:28 AM, Roman Mashak wrote:
>> Have print_tm() dump firstuse value along with install, lastuse
>> and expires.
>>
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>> tc/tc_util.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>> index 12f865cc71bf..f6aa2ed552a9 100644
>> --- a/tc/tc_util.c
>> +++ b/tc/tc_util.c
>> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
>> print_uint(PRINT_FP, NULL, " used %u sec",
>> (unsigned int)(tm->lastuse/hz));
>> }
>> + if (tm->firstuse != 0) {
>> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
>> + print_uint(PRINT_FP, NULL, " firstused %u sec",
>> + (unsigned int)(tm->firstuse/hz));
>> + }
>> if (tm->expires != 0) {
>> print_uint(PRINT_JSON, "expires", NULL, tm->expires);
>> print_uint(PRINT_FP, NULL, " expires %u sec",
>>
>
> why does this function print different values for json and stdout?
It prints times in jiffies for json mode, and in seconds otherwise. This
inconsistency is likely a bug, and a subject for another fix.
Last time this function was touched in commit
2704bd62558391c00bc1c3e7f8706de8332d8ba0 where json was added.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-18 16:41 ` Roman Mashak
@ 2020-05-19 18:08 ` Stephen Hemminger
2020-05-19 18:09 ` David Ahern
0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2020-05-19 18:08 UTC (permalink / raw)
To: Roman Mashak; +Cc: David Ahern, netdev, kernel, jhs, xiyou.wangcong, jiri
On Mon, 18 May 2020 12:41:10 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> David Ahern <dsahern@gmail.com> writes:
>
> > On 5/17/20 7:28 AM, Roman Mashak wrote:
> >> Have print_tm() dump firstuse value along with install, lastuse
> >> and expires.
> >>
> >> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> >> ---
> >> tc/tc_util.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/tc/tc_util.c b/tc/tc_util.c
> >> index 12f865cc71bf..f6aa2ed552a9 100644
> >> --- a/tc/tc_util.c
> >> +++ b/tc/tc_util.c
> >> @@ -760,6 +760,11 @@ void print_tm(FILE *f, const struct tcf_t *tm)
> >> print_uint(PRINT_FP, NULL, " used %u sec",
> >> (unsigned int)(tm->lastuse/hz));
> >> }
> >> + if (tm->firstuse != 0) {
> >> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
> >> + print_uint(PRINT_FP, NULL, " firstused %u sec",
> >> + (unsigned int)(tm->firstuse/hz));
> >> + }
> >> if (tm->expires != 0) {
> >> print_uint(PRINT_JSON, "expires", NULL, tm->expires);
> >> print_uint(PRINT_FP, NULL, " expires %u sec",
> >>
> >
> > why does this function print different values for json and stdout?
>
> It prints times in jiffies for json mode, and in seconds otherwise. This
> inconsistency is likely a bug, and a subject for another fix.
>
> Last time this function was touched in commit
> 2704bd62558391c00bc1c3e7f8706de8332d8ba0 where json was added.
iproute commands are not supposed to expose the jiffies version of
times in input or output.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-19 18:08 ` Stephen Hemminger
@ 2020-05-19 18:09 ` David Ahern
0 siblings, 0 replies; 11+ messages in thread
From: David Ahern @ 2020-05-19 18:09 UTC (permalink / raw)
To: Stephen Hemminger, Roman Mashak; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
On 5/19/20 12:08 PM, Stephen Hemminger wrote:
>> Last time this function was touched in commit
>> 2704bd62558391c00bc1c3e7f8706de8332d8ba0 where json was added.
>
> iproute commands are not supposed to expose the jiffies version of
> times in input or output.
>
Check your queue; I brought this up and Roman sent a bug fix.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-17 13:28 [PATCH iproute2-next 1/1] tc: report time an action was first used Roman Mashak
2020-05-18 13:10 ` Jamal Hadi Salim
2020-05-18 15:36 ` David Ahern
@ 2020-05-19 18:06 ` Stephen Hemminger
2020-05-19 18:11 ` David Ahern
3 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-05-19 18:06 UTC (permalink / raw)
To: Roman Mashak; +Cc: dsahern, netdev, kernel, jhs, xiyou.wangcong, jiri
On Sun, 17 May 2020 09:28:45 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> + if (tm->firstuse != 0) {
> + print_uint(PRINT_JSON, "first_used", NULL, tm->firstuse);
> + print_uint(PRINT_FP, NULL, " firstused %u sec",
> + (unsigned int)(tm->firstuse/hz));
> + }
The JSON should report in same units as non-json output.
If you are copying what expires does, it is wrong as well.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-17 13:28 [PATCH iproute2-next 1/1] tc: report time an action was first used Roman Mashak
` (2 preceding siblings ...)
2020-05-19 18:06 ` Stephen Hemminger
@ 2020-05-19 18:11 ` David Ahern
2020-05-24 12:36 ` Roman Mashak
3 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2020-05-19 18:11 UTC (permalink / raw)
To: Roman Mashak; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri
On 5/17/20 7:28 AM, Roman Mashak wrote:
> Have print_tm() dump firstuse value along with install, lastuse
> and expires.
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> tc/tc_util.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
I can merge master once Stephen commits the bug fix. Then resubmit this
patch.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH iproute2-next 1/1] tc: report time an action was first used
2020-05-19 18:11 ` David Ahern
@ 2020-05-24 12:36 ` Roman Mashak
0 siblings, 0 replies; 11+ messages in thread
From: Roman Mashak @ 2020-05-24 12:36 UTC (permalink / raw)
To: David Ahern; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri
David Ahern <dsahern@gmail.com> writes:
> On 5/17/20 7:28 AM, Roman Mashak wrote:
>> Have print_tm() dump firstuse value along with install, lastuse
>> and expires.
>>
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>> tc/tc_util.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>
> I can merge master once Stephen commits the bug fix. Then resubmit this
> patch.
Hi David,
Stephen has commited the fix, please merge the master branch and I will
resubmit the patch.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-24 12:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-17 13:28 [PATCH iproute2-next 1/1] tc: report time an action was first used Roman Mashak
2020-05-18 13:10 ` Jamal Hadi Salim
2020-05-18 15:38 ` David Ahern
2020-05-19 9:09 ` Jamal Hadi Salim
2020-05-18 15:36 ` David Ahern
2020-05-18 16:41 ` Roman Mashak
2020-05-19 18:08 ` Stephen Hemminger
2020-05-19 18:09 ` David Ahern
2020-05-19 18:06 ` Stephen Hemminger
2020-05-19 18:11 ` David Ahern
2020-05-24 12:36 ` Roman Mashak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).