Re: [PATCH net-next v11 4/4] net sched actions: add time filter for action dumping

From: Jiri Pirko <jiri@resnulli.us>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
	xiyou.wangcong@gmail.com, dsahern@gmail.com,
	eric.dumazet@gmail.com, mrv@mojatatu.com,
	simon.horman@netronome.com, alex.aring@gmail.com
Subject: Re: [PATCH net-next v11 4/4] net sched actions: add time filter for action dumping
Date: Tue, 25 Jul 2017 13:34:52 +0200	[thread overview]
Message-ID: <20170725113452.GB3186@nanopsycho> (raw)
In-Reply-To: <bdd4e9f8-7d64-813b-fd6c-147d0bc3c0af@mojatatu.com>

Tue, Jul 25, 2017 at 01:27:03PM CEST, jhs@mojatatu.com wrote:
>On 17-07-24 07:34 AM, Jiri Pirko wrote:
>> Mon, Jul 24, 2017 at 03:35:46AM CEST, jhs@mojatatu.com wrote:
>> > From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>> 
>> > @@ -128,6 +129,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
>> > 			if (index < s_i)
>> > 				continue;
>> > 
>> > +			if (jiffy_since &&
>> > +			    time_after(jiffy_since,
>> > +				       (unsigned long)p->tcfa_tm.lastuse))
>> 
>> You don't need to check jiffy_since==0. Also, nicer  ^^ this with a space :)
>> 
>
>Assuming that time_after() would work fine for jiffy_since being zero,
>but:
>wouldnt it be more efficient to just not call time_after() altogether?

time_after is pretty trivial. But your call.

>
>> Other than this, looks fine.
>
>Ok, please no more changes - I am exhausted ;-> So speak for this
>update or send patches afterwards if you dont like something.
>
>cheers,
>jamal
>