netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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-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 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: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 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-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-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
                   ` (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).