On Mon, May 18, 2020 at 5:43 AM Jamal Hadi Salim <j...@mojatatu.com> wrote: > > On 2020-05-17 9:10 p.m., Roman Mashak wrote: > > Cong Wang <xiyou.wangc...@gmail.com> writes: > > > >> On Sun, May 17, 2020 at 5:47 AM Roman Mashak <m...@mojatatu.com> wrote: > >>> > >>> When a new action is installed, firstuse field of 'tcf_t' is explicitly > >>> set > >>> to 0. Value of zero means "new action, not yet used"; as a packet hits the > >>> action, 'firstuse' is stamped with the current jiffies value. > >>> > >>> tcf_tm_dump() should return 0 for firstuse if action has not yet been hit. > >> > >> Your patch makes sense to me. > >> > >> Just one more thing, how about 'lastuse'? It is initialized with jiffies, > >> not 0, it seems we should initialize it to 0 too, as it is not yet used? > > > > Yes, exactly. I was planning to send a separate patch for this. > > > > Thanks for review, Cong. > > > > For these corner cases, firstuse using zero to indicate > "has not been used" is not ambigious. > lastuse has ambiguity because zero now has two meanings > if you check for the corner case in the kernel. > 1)Zero is a legit value when dumping or > getting (example an action was just hit when you dumped). > 2) zero also now means "has not been used".
Well, technically firstuse could be a legit 0 too, when the action was just hit for the first time right when dumping. So the ambiguity is same for both. > > My suggestion is to leave this alone in the kernel. > In user space/iproute2 check if lastused and created > are equal and declare "has not been used". Sounds a good idea to me. Thanks.