| Message ID | 20191030142040.19404-1-vladbu@mellanox.com |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2,net-next,v2] tc: implement support for action flags | expand |
On 10/30/19 8:20 AM, Vlad Buslov wrote: > Implement setting and printing of action flags with single available flag > value "no_percpu" that translates to kernel UAPI TCA_ACT_FLAGS value > TCA_ACT_FLAGS_NO_PERCPU_STATS. Update man page with information regarding > usage of action flags. > > Example usage: > > # tc actions add action gact drop no_percpu > # sudo tc actions list action gact > total acts 1 > > action order 0: gact action drop > random type none pass val 0 > index 1 ref 1 bind 0 > no_percpu > > Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Applied to iproute2-next. Thanks,
On Wed, Oct 30, 2019 at 7:20 AM Vlad Buslov <vladbu@mellanox.com> wrote: > > Implement setting and printing of action flags with single available flag > value "no_percpu" that translates to kernel UAPI TCA_ACT_FLAGS value > TCA_ACT_FLAGS_NO_PERCPU_STATS. Update man page with information regarding > usage of action flags. > > Example usage: > > # tc actions add action gact drop no_percpu > # sudo tc actions list action gact > total acts 1 > > action order 0: gact action drop > random type none pass val 0 > index 1 ref 1 bind 0 > no_percpu would be nice to just call it no_percpu_stats to match the flag name ?. Current choice of word leaves room for possible conflict with other percpu flags in the future.. > > Signed-off-by: Vlad Buslov <vladbu@mellanox.com> > --- > > Notes: > Changes V1 -> V2: > > - Rework the change to use action API TCA_ACT_FLAGS instead of > per-action flags implementation. > > include/uapi/linux/pkt_cls.h | 5 +++++ > man/man8/tc-actions.8 | 14 ++++++++++++++ > tc/m_action.c | 19 +++++++++++++++++++ > 3 files changed, 38 insertions(+) > > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h > index a6aa466fac9e..c6ad22f76ede 100644 > --- a/include/uapi/linux/pkt_cls.h > +++ b/include/uapi/linux/pkt_cls.h > @@ -16,9 +16,14 @@ enum { > TCA_ACT_STATS, > TCA_ACT_PAD, > TCA_ACT_COOKIE, > + TCA_ACT_FLAGS, > __TCA_ACT_MAX > }; > > +#define TCA_ACT_FLAGS_NO_PERCPU_STATS 1 /* Don't use percpu allocator for > + * actions stats. > + */ > + > #define TCA_ACT_MAX __TCA_ACT_MAX > #define TCA_OLD_COMPAT (TCA_ACT_MAX+1) > #define TCA_ACT_MAX_PRIO 32 > diff --git a/man/man8/tc-actions.8 b/man/man8/tc-actions.8 > index f46166e3f685..bee59f7247fa 100644 > --- a/man/man8/tc-actions.8 > +++ b/man/man8/tc-actions.8 > @@ -47,6 +47,8 @@ actions \- independently defined actions in tc > ] [ > .I COOKIESPEC > ] [ > +.I FLAGS > +] [ > .I CONTROL > ] > > @@ -71,6 +73,10 @@ ACTNAME > := > .BI cookie " COOKIE" > > +.I FLAGS > +:= > +.I no_percpu > + > .I ACTDETAIL > := > .I ACTNAME ACTPARAMS > @@ -186,6 +192,14 @@ As such, it can be used as a correlating value for maintaining user state. > The value to be stored is completely arbitrary and does not require a specific > format. It is stored inside the action structure itself. > > +.TP > +.I FLAGS > +Action-specific flags. Currently, the only supported flag is > +.I no_percpu > +which indicates that action is expected to have minimal software data-path > +traffic and doesn't need to allocate stat counters with percpu allocator. > +This option is intended to be used by hardware-offloaded actions. > + > .TP > .BI since " MSTIME" > When dumping large number of actions, a millisecond time-filter can be > diff --git a/tc/m_action.c b/tc/m_action.c > index 36c744bbe374..4da810c8c0aa 100644 > --- a/tc/m_action.c > +++ b/tc/m_action.c > @@ -250,6 +250,16 @@ done0: > addattr_l(n, MAX_MSG, TCA_ACT_COOKIE, > &act_ck, act_ck_len); > > + if (*argv && strcmp(*argv, "no_percpu") == 0) { > + struct nla_bitfield32 flags = > + { TCA_ACT_FLAGS_NO_PERCPU_STATS, > + TCA_ACT_FLAGS_NO_PERCPU_STATS }; > + > + addattr_l(n, MAX_MSG, TCA_ACT_FLAGS, &flags, > + sizeof(struct nla_bitfield32)); > + NEXT_ARG_FWD(); > + } > + > addattr_nest_end(n, tail); > ok++; > } > @@ -318,6 +328,15 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg) > strsz, b1, sizeof(b1))); > print_string(PRINT_FP, NULL, "%s", _SL_); > } > + if (tb[TCA_ACT_FLAGS]) { > + struct nla_bitfield32 *flags = RTA_DATA(tb[TCA_ACT_FLAGS]); > + > + if (flags->selector & TCA_ACT_FLAGS_NO_PERCPU_STATS) > + print_bool(PRINT_ANY, "no_percpu", "\tno_percpu", > + flags->value & > + TCA_ACT_FLAGS_NO_PERCPU_STATS); > + print_string(PRINT_FP, NULL, "%s", _SL_); > + } > > return 0; > } > -- > 2.21.0 >
On Mon 04 Nov 2019 at 18:49, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On Wed, Oct 30, 2019 at 7:20 AM Vlad Buslov <vladbu@mellanox.com> wrote: >> >> Implement setting and printing of action flags with single available flag >> value "no_percpu" that translates to kernel UAPI TCA_ACT_FLAGS value >> TCA_ACT_FLAGS_NO_PERCPU_STATS. Update man page with information regarding >> usage of action flags. >> >> Example usage: >> >> # tc actions add action gact drop no_percpu >> # sudo tc actions list action gact >> total acts 1 >> >> action order 0: gact action drop >> random type none pass val 0 >> index 1 ref 1 bind 0 >> no_percpu > > would be nice to just call it no_percpu_stats to match the flag name ?. > Current choice of word leaves room for possible conflict with other > percpu flags in the future.. I didn't find any other places in action code that uses percpu allocator directly and decided to name it "no_percpu" since it seems to me that iproute2 developers prefer brevity when naming such things (notice "val" and "ref" in example output). > > >> >> Signed-off-by: Vlad Buslov <vladbu@mellanox.com> >> --- >> >> Notes: >> Changes V1 -> V2: >> >> - Rework the change to use action API TCA_ACT_FLAGS instead of >> per-action flags implementation. >> >> include/uapi/linux/pkt_cls.h | 5 +++++ >> man/man8/tc-actions.8 | 14 ++++++++++++++ >> tc/m_action.c | 19 +++++++++++++++++++ >> 3 files changed, 38 insertions(+) >> >> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h >> index a6aa466fac9e..c6ad22f76ede 100644 >> --- a/include/uapi/linux/pkt_cls.h >> +++ b/include/uapi/linux/pkt_cls.h >> @@ -16,9 +16,14 @@ enum { >> TCA_ACT_STATS, >> TCA_ACT_PAD, >> TCA_ACT_COOKIE, >> + TCA_ACT_FLAGS, >> __TCA_ACT_MAX >> }; >> >> +#define TCA_ACT_FLAGS_NO_PERCPU_STATS 1 /* Don't use percpu allocator for >> + * actions stats. >> + */ >> + >> #define TCA_ACT_MAX __TCA_ACT_MAX >> #define TCA_OLD_COMPAT (TCA_ACT_MAX+1) >> #define TCA_ACT_MAX_PRIO 32 >> diff --git a/man/man8/tc-actions.8 b/man/man8/tc-actions.8 >> index f46166e3f685..bee59f7247fa 100644 >> --- a/man/man8/tc-actions.8 >> +++ b/man/man8/tc-actions.8 >> @@ -47,6 +47,8 @@ actions \- independently defined actions in tc >> ] [ >> .I COOKIESPEC >> ] [ >> +.I FLAGS >> +] [ >> .I CONTROL >> ] >> >> @@ -71,6 +73,10 @@ ACTNAME >> := >> .BI cookie " COOKIE" >> >> +.I FLAGS >> +:= >> +.I no_percpu >> + >> .I ACTDETAIL >> := >> .I ACTNAME ACTPARAMS >> @@ -186,6 +192,14 @@ As such, it can be used as a correlating value for maintaining user state. >> The value to be stored is completely arbitrary and does not require a specific >> format. It is stored inside the action structure itself. >> >> +.TP >> +.I FLAGS >> +Action-specific flags. Currently, the only supported flag is >> +.I no_percpu >> +which indicates that action is expected to have minimal software data-path >> +traffic and doesn't need to allocate stat counters with percpu allocator. >> +This option is intended to be used by hardware-offloaded actions. >> + >> .TP >> .BI since " MSTIME" >> When dumping large number of actions, a millisecond time-filter can be >> diff --git a/tc/m_action.c b/tc/m_action.c >> index 36c744bbe374..4da810c8c0aa 100644 >> --- a/tc/m_action.c >> +++ b/tc/m_action.c >> @@ -250,6 +250,16 @@ done0: >> addattr_l(n, MAX_MSG, TCA_ACT_COOKIE, >> &act_ck, act_ck_len); >> >> + if (*argv && strcmp(*argv, "no_percpu") == 0) { >> + struct nla_bitfield32 flags = >> + { TCA_ACT_FLAGS_NO_PERCPU_STATS, >> + TCA_ACT_FLAGS_NO_PERCPU_STATS }; >> + >> + addattr_l(n, MAX_MSG, TCA_ACT_FLAGS, &flags, >> + sizeof(struct nla_bitfield32)); >> + NEXT_ARG_FWD(); >> + } >> + >> addattr_nest_end(n, tail); >> ok++; >> } >> @@ -318,6 +328,15 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg) >> strsz, b1, sizeof(b1))); >> print_string(PRINT_FP, NULL, "%s", _SL_); >> } >> + if (tb[TCA_ACT_FLAGS]) { >> + struct nla_bitfield32 *flags = RTA_DATA(tb[TCA_ACT_FLAGS]); >> + >> + if (flags->selector & TCA_ACT_FLAGS_NO_PERCPU_STATS) >> + print_bool(PRINT_ANY, "no_percpu", "\tno_percpu", >> + flags->value & >> + TCA_ACT_FLAGS_NO_PERCPU_STATS); >> + print_string(PRINT_FP, NULL, "%s", _SL_); >> + } >> >> return 0; >> } >> -- >> 2.21.0 >>
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h index a6aa466fac9e..c6ad22f76ede 100644 --- a/include/uapi/linux/pkt_cls.h +++ b/include/uapi/linux/pkt_cls.h @@ -16,9 +16,14 @@ enum { TCA_ACT_STATS, TCA_ACT_PAD, TCA_ACT_COOKIE, + TCA_ACT_FLAGS, __TCA_ACT_MAX }; +#define TCA_ACT_FLAGS_NO_PERCPU_STATS 1 /* Don't use percpu allocator for + * actions stats. + */ + #define TCA_ACT_MAX __TCA_ACT_MAX #define TCA_OLD_COMPAT (TCA_ACT_MAX+1) #define TCA_ACT_MAX_PRIO 32 diff --git a/man/man8/tc-actions.8 b/man/man8/tc-actions.8 index f46166e3f685..bee59f7247fa 100644 --- a/man/man8/tc-actions.8 +++ b/man/man8/tc-actions.8 @@ -47,6 +47,8 @@ actions \- independently defined actions in tc ] [ .I COOKIESPEC ] [ +.I FLAGS +] [ .I CONTROL ] @@ -71,6 +73,10 @@ ACTNAME := .BI cookie " COOKIE" +.I FLAGS +:= +.I no_percpu + .I ACTDETAIL := .I ACTNAME ACTPARAMS @@ -186,6 +192,14 @@ As such, it can be used as a correlating value for maintaining user state. The value to be stored is completely arbitrary and does not require a specific format. It is stored inside the action structure itself. +.TP +.I FLAGS +Action-specific flags. Currently, the only supported flag is +.I no_percpu +which indicates that action is expected to have minimal software data-path +traffic and doesn't need to allocate stat counters with percpu allocator. +This option is intended to be used by hardware-offloaded actions. + .TP .BI since " MSTIME" When dumping large number of actions, a millisecond time-filter can be diff --git a/tc/m_action.c b/tc/m_action.c index 36c744bbe374..4da810c8c0aa 100644 --- a/tc/m_action.c +++ b/tc/m_action.c @@ -250,6 +250,16 @@ done0: addattr_l(n, MAX_MSG, TCA_ACT_COOKIE, &act_ck, act_ck_len); + if (*argv && strcmp(*argv, "no_percpu") == 0) { + struct nla_bitfield32 flags = + { TCA_ACT_FLAGS_NO_PERCPU_STATS, + TCA_ACT_FLAGS_NO_PERCPU_STATS }; + + addattr_l(n, MAX_MSG, TCA_ACT_FLAGS, &flags, + sizeof(struct nla_bitfield32)); + NEXT_ARG_FWD(); + } + addattr_nest_end(n, tail); ok++; } @@ -318,6 +328,15 @@ static int tc_print_one_action(FILE *f, struct rtattr *arg) strsz, b1, sizeof(b1))); print_string(PRINT_FP, NULL, "%s", _SL_); } + if (tb[TCA_ACT_FLAGS]) { + struct nla_bitfield32 *flags = RTA_DATA(tb[TCA_ACT_FLAGS]); + + if (flags->selector & TCA_ACT_FLAGS_NO_PERCPU_STATS) + print_bool(PRINT_ANY, "no_percpu", "\tno_percpu", + flags->value & + TCA_ACT_FLAGS_NO_PERCPU_STATS); + print_string(PRINT_FP, NULL, "%s", _SL_); + } return 0; }
Implement setting and printing of action flags with single available flag value "no_percpu" that translates to kernel UAPI TCA_ACT_FLAGS value TCA_ACT_FLAGS_NO_PERCPU_STATS. Update man page with information regarding usage of action flags. Example usage: # tc actions add action gact drop no_percpu # sudo tc actions list action gact total acts 1 action order 0: gact action drop random type none pass val 0 index 1 ref 1 bind 0 no_percpu Signed-off-by: Vlad Buslov <vladbu@mellanox.com> --- Notes: Changes V1 -> V2: - Rework the change to use action API TCA_ACT_FLAGS instead of per-action flags implementation. include/uapi/linux/pkt_cls.h | 5 +++++ man/man8/tc-actions.8 | 14 ++++++++++++++ tc/m_action.c | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+)