Hi Jacub, > On Jun 7, 2019, at 3:02 PM, Jakub Kicinski <jakub.kicin...@netronome.com> > wrote: > > On Fri, 7 Jun 2019 20:42:55 +0000, Patel, Vedang wrote: >>> Thanks for the changes, since you now validate no unknown flags are >>> passed, perhaps there is no need to check if flags are == ~0? >>> >>> IS_ENABLED() could just do: (flags) & TCA_TAPRIO_ATTR_FLAG_TXTIME_ASSIST >>> No? >>> >> This is specifically done so that user does not have to specify the >> offload flags when trying to install the another schedule which will >> be switched to at a later point of time (i.e. the admin schedule >> introduced in Vinicius’ last series). Setting taprio_flags to ~0 >> will help us distinguish between the flags parameter not specified >> and flags set to 0. > > I'm not super clear on this, because of backward compat you have to > treat attr not present as unset. Let's see: > > new qdisc: > - flags attr = 0 -> txtime not used > - flags attr = 1 -> txtime used > -> no flags attr -> txtime not used > change qdisc: > - flags attr = old flags attr -> leave unchanged > - flags attr != old flags attr -> error > - no flags attr -> leave txtime unchanged > > Doesn't that cover the cases? Were you planning to have no flag attr > on change mean disabled rather than no change?
You covered all the cases above. Thinking a bit more about it, yes you are right. Initiializing flags to 0 will work. I will incorporate this change in the next version. Thanks, Vedang
