| Message ID | 20220217082803.3881-2-jianbol@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | flow_offload: add tc police parameters | expand |
On February 17, 2022 4:28 PM, Jianbo wrote: >The current police offload action entry is missing exceed/notexceed actions >and parameters that can be configured by tc police action. >Add the missing parameters as a pre-step for offloading police actions to >hardware. > >Signed-off-by: Jianbo Liu <jianbol@nvidia.com> >Signed-off-by: Roi Dayan <roid@nvidia.com> >Reviewed-by: Ido Schimmel <idosch@nvidia.com> >--- > include/net/flow_offload.h | 13 ++++++++++ > include/net/tc_act/tc_police.h | 30 ++++++++++++++++++++++ > net/sched/act_police.c | 46 ++++++++++++++++++++++++++++++++++ > 3 files changed, 89 insertions(+) > >diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h index >5b8c54eb7a6b..94cde6bbc8a5 100644 >--- a/include/net/flow_offload.h >+++ b/include/net/flow_offload.h >@@ -148,6 +148,8 @@ enum flow_action_id { > FLOW_ACTION_MPLS_MANGLE, > FLOW_ACTION_GATE, > FLOW_ACTION_PPPOE_PUSH, >+ FLOW_ACTION_JUMP, >+ FLOW_ACTION_PIPE, > NUM_FLOW_ACTIONS, > }; > >@@ -235,9 +237,20 @@ struct flow_action_entry { > struct { /* FLOW_ACTION_POLICE */ > u32 burst; > u64 rate_bytes_ps; >+ u64 peakrate_bytes_ps; >+ u32 avrate; >+ u16 overhead; > u64 burst_pkt; > u64 rate_pkt_ps; > u32 mtu; >+ struct { >+ enum flow_action_id act_id; >+ u32 index; >+ } exceed; >+ struct { >+ enum flow_action_id act_id; >+ u32 index; >+ } notexceed; It seems exceed and notexceed use the same format struct, will it be more simpler to define as: struct { enum flow_action_id act_id; u32 index; } exceed, notexceed; > } police; > struct { /* FLOW_ACTION_CT */ > int action; >diff --git a/include/net/tc_act/tc_police.h b/include/net/tc_act/tc_police.h >index 72649512dcdd..283bde711a42 100644 >--- a/include/net/tc_act/tc_police.h >+++ b/include/net/tc_act/tc_police.h >@@ -159,4 +159,34 @@ static inline u32 tcf_police_tcfp_mtu(const struct >tc_action *act) > return params->tcfp_mtu; > } > >+static inline u64 tcf_police_peakrate_bytes_ps(const struct tc_action >+*act) { >+ struct tcf_police *police = to_police(act); >+ struct tcf_police_params *params; >+ >+ params = rcu_dereference_protected(police->params, >+ lockdep_is_held(&police->tcf_lock)); >+ return params->peak.rate_bytes_ps; >+} >+ >+static inline u32 tcf_police_tcfp_ewma_rate(const struct tc_action >+*act) { >+ struct tcf_police *police = to_police(act); >+ struct tcf_police_params *params; >+ >+ params = rcu_dereference_protected(police->params, >+ lockdep_is_held(&police->tcf_lock)); >+ return params->tcfp_ewma_rate; >+} >+ >+static inline u16 tcf_police_rate_overhead(const struct tc_action *act) >+{ >+ struct tcf_police *police = to_police(act); >+ struct tcf_police_params *params; >+ >+ params = rcu_dereference_protected(police->params, >+ lockdep_is_held(&police->tcf_lock)); >+ return params->rate.overhead; >+} >+ > #endif /* __NET_TC_POLICE_H */ >diff --git a/net/sched/act_police.c b/net/sched/act_police.c index >0923aa2b8f8a..0457b6c9c4e7 100644 >--- a/net/sched/act_police.c >+++ b/net/sched/act_police.c >@@ -405,20 +405,66 @@ static int tcf_police_search(struct net *net, struct >tc_action **a, u32 index) > return tcf_idr_search(tn, a, index); > } > >+static int tcf_police_act_to_flow_act(int tc_act, int *index) { >+ int act_id = -EOPNOTSUPP; >+ >+ if (!TC_ACT_EXT_OPCODE(tc_act)) { >+ if (tc_act == TC_ACT_OK) >+ act_id = FLOW_ACTION_ACCEPT; >+ else if (tc_act == TC_ACT_SHOT) >+ act_id = FLOW_ACTION_DROP; >+ else if (tc_act == TC_ACT_PIPE) >+ act_id = FLOW_ACTION_PIPE; >+ } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { >+ act_id = FLOW_ACTION_GOTO; >+ *index = tc_act & TC_ACT_EXT_VAL_MASK; For the TC_ACT_GOTO_CHAIN action, the goto_chain information is missing from software to hardware, is it useful for hardware to check? >+ } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { >+ act_id = FLOW_ACTION_JUMP; >+ *index = tc_act & TC_ACT_EXT_VAL_MASK; >+ } >+ >+ return act_id; >+} >+ > static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data, > u32 *index_inc, bool bind) > { > if (bind) { > struct flow_action_entry *entry = entry_data; >+ struct tcf_police *police = to_police(act); >+ struct tcf_police_params *p; >+ int act_id; >+ >+ p = rcu_dereference_protected(police->params, >+ lockdep_is_held(&police->tcf_lock)); > > entry->id = FLOW_ACTION_POLICE; > entry->police.burst = tcf_police_burst(act); > entry->police.rate_bytes_ps = > tcf_police_rate_bytes_ps(act); >+ entry->police.peakrate_bytes_ps = >tcf_police_peakrate_bytes_ps(act); >+ entry->police.avrate = tcf_police_tcfp_ewma_rate(act); >+ entry->police.overhead = tcf_police_rate_overhead(act); > entry->police.burst_pkt = tcf_police_burst_pkt(act); > entry->police.rate_pkt_ps = > tcf_police_rate_pkt_ps(act); > entry->police.mtu = tcf_police_tcfp_mtu(act); >+ >+ act_id = tcf_police_act_to_flow_act(police->tcf_action, >+ &entry- >>police.exceed.index); >+ if (act_id < 0) >+ return act_id; >+ >+ entry->police.exceed.act_id = act_id; >+ >+ act_id = tcf_police_act_to_flow_act(p->tcfp_result, >+ &entry- >>police.notexceed.index); >+ if (act_id < 0) >+ return act_id; >+ >+ entry->police.notexceed.act_id = act_id; >+ > *index_inc = 1; > } else { > struct flow_offload_action *fl_action = entry_data; >-- >2.26.2
On 2022-02-17 12:25 PM, Baowen Zheng wrote: > On February 17, 2022 4:28 PM, Jianbo wrote: >> The current police offload action entry is missing exceed/notexceed actions >> and parameters that can be configured by tc police action. >> Add the missing parameters as a pre-step for offloading police actions to >> hardware. >> >> Signed-off-by: Jianbo Liu <jianbol@nvidia.com> >> Signed-off-by: Roi Dayan <roid@nvidia.com> >> Reviewed-by: Ido Schimmel <idosch@nvidia.com> >> --- >> include/net/flow_offload.h | 13 ++++++++++ >> include/net/tc_act/tc_police.h | 30 ++++++++++++++++++++++ >> net/sched/act_police.c | 46 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 89 insertions(+) >> >> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h index >> 5b8c54eb7a6b..94cde6bbc8a5 100644 >> --- a/include/net/flow_offload.h >> +++ b/include/net/flow_offload.h >> @@ -148,6 +148,8 @@ enum flow_action_id { >> FLOW_ACTION_MPLS_MANGLE, >> FLOW_ACTION_GATE, >> FLOW_ACTION_PPPOE_PUSH, >> + FLOW_ACTION_JUMP, >> + FLOW_ACTION_PIPE, >> NUM_FLOW_ACTIONS, >> }; >> >> @@ -235,9 +237,20 @@ struct flow_action_entry { >> struct { /* FLOW_ACTION_POLICE */ >> u32 burst; >> u64 rate_bytes_ps; >> + u64 peakrate_bytes_ps; >> + u32 avrate; >> + u16 overhead; >> u64 burst_pkt; >> u64 rate_pkt_ps; >> u32 mtu; >> + struct { >> + enum flow_action_id act_id; >> + u32 index; >> + } exceed; >> + struct { >> + enum flow_action_id act_id; >> + u32 index; >> + } notexceed; > It seems exceed and notexceed use the same format struct, will it be more simpler to define as: > struct { > enum flow_action_id act_id; > u32 index; > } exceed, notexceed; right. it can be. > >> } police; >> struct { /* FLOW_ACTION_CT */ >> int action; >> diff --git a/include/net/tc_act/tc_police.h b/include/net/tc_act/tc_police.h >> index 72649512dcdd..283bde711a42 100644 >> --- a/include/net/tc_act/tc_police.h >> +++ b/include/net/tc_act/tc_police.h >> @@ -159,4 +159,34 @@ static inline u32 tcf_police_tcfp_mtu(const struct >> tc_action *act) >> return params->tcfp_mtu; >> } >> >> +static inline u64 tcf_police_peakrate_bytes_ps(const struct tc_action >> +*act) { >> + struct tcf_police *police = to_police(act); >> + struct tcf_police_params *params; >> + >> + params = rcu_dereference_protected(police->params, >> + lockdep_is_held(&police->tcf_lock)); >> + return params->peak.rate_bytes_ps; >> +} >> + >> +static inline u32 tcf_police_tcfp_ewma_rate(const struct tc_action >> +*act) { >> + struct tcf_police *police = to_police(act); >> + struct tcf_police_params *params; >> + >> + params = rcu_dereference_protected(police->params, >> + lockdep_is_held(&police->tcf_lock)); >> + return params->tcfp_ewma_rate; >> +} >> + >> +static inline u16 tcf_police_rate_overhead(const struct tc_action *act) >> +{ >> + struct tcf_police *police = to_police(act); >> + struct tcf_police_params *params; >> + >> + params = rcu_dereference_protected(police->params, >> + lockdep_is_held(&police->tcf_lock)); >> + return params->rate.overhead; >> +} >> + >> #endif /* __NET_TC_POLICE_H */ >> diff --git a/net/sched/act_police.c b/net/sched/act_police.c index >> 0923aa2b8f8a..0457b6c9c4e7 100644 >> --- a/net/sched/act_police.c >> +++ b/net/sched/act_police.c >> @@ -405,20 +405,66 @@ static int tcf_police_search(struct net *net, struct >> tc_action **a, u32 index) >> return tcf_idr_search(tn, a, index); >> } >> >> +static int tcf_police_act_to_flow_act(int tc_act, int *index) { >> + int act_id = -EOPNOTSUPP; >> + >> + if (!TC_ACT_EXT_OPCODE(tc_act)) { >> + if (tc_act == TC_ACT_OK) >> + act_id = FLOW_ACTION_ACCEPT; >> + else if (tc_act == TC_ACT_SHOT) >> + act_id = FLOW_ACTION_DROP; >> + else if (tc_act == TC_ACT_PIPE) >> + act_id = FLOW_ACTION_PIPE; >> + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { >> + act_id = FLOW_ACTION_GOTO; >> + *index = tc_act & TC_ACT_EXT_VAL_MASK; > For the TC_ACT_GOTO_CHAIN action, the goto_chain information is missing from software to hardware, is it useful for hardware to check? > what information do you mean? >> + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { >> + act_id = FLOW_ACTION_JUMP; >> + *index = tc_act & TC_ACT_EXT_VAL_MASK; >> + } >> + >> + return act_id; >> +} >> + >> static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data, >> u32 *index_inc, bool bind) >> { >> if (bind) { >> struct flow_action_entry *entry = entry_data; >> + struct tcf_police *police = to_police(act); >> + struct tcf_police_params *p; >> + int act_id; >> + >> + p = rcu_dereference_protected(police->params, >> + lockdep_is_held(&police->tcf_lock)); >> >> entry->id = FLOW_ACTION_POLICE; >> entry->police.burst = tcf_police_burst(act); >> entry->police.rate_bytes_ps = >> tcf_police_rate_bytes_ps(act); >> + entry->police.peakrate_bytes_ps = >> tcf_police_peakrate_bytes_ps(act); >> + entry->police.avrate = tcf_police_tcfp_ewma_rate(act); >> + entry->police.overhead = tcf_police_rate_overhead(act); >> entry->police.burst_pkt = tcf_police_burst_pkt(act); >> entry->police.rate_pkt_ps = >> tcf_police_rate_pkt_ps(act); >> entry->police.mtu = tcf_police_tcfp_mtu(act); >> + >> + act_id = tcf_police_act_to_flow_act(police->tcf_action, >> + &entry- >>> police.exceed.index); >> + if (act_id < 0) >> + return act_id; >> + >> + entry->police.exceed.act_id = act_id; >> + >> + act_id = tcf_police_act_to_flow_act(p->tcfp_result, >> + &entry- >>> police.notexceed.index); >> + if (act_id < 0) >> + return act_id; >> + >> + entry->police.notexceed.act_id = act_id; >> + >> *index_inc = 1; >> } else { >> struct flow_offload_action *fl_action = entry_data; >> -- >> 2.26.2 >
On, February 17, 2022 8:10 PM, Roi wrote: >On 2022-02-17 12:25 PM, Baowen Zheng wrote: >> On February 17, 2022 4:28 PM, Jianbo wrote: >>> The current police offload action entry is missing exceed/notexceed >>> actions and parameters that can be configured by tc police action. >>> Add the missing parameters as a pre-step for offloading police >>> actions to hardware. >>> >>> Signed-off-by: Jianbo Liu <jianbol@nvidia.com> >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Reviewed-by: Ido Schimmel <idosch@nvidia.com> >>> --- >>> include/net/flow_offload.h | 13 ++++++++++ >>> include/net/tc_act/tc_police.h | 30 ++++++++++++++++++++++ >>> net/sched/act_police.c | 46 >++++++++++++++++++++++++++++++++++ >>> 3 files changed, 89 insertions(+) >>> >>> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h >>> index >>> 5b8c54eb7a6b..94cde6bbc8a5 100644 >>> --- a/include/net/flow_offload.h >>> +++ b/include/net/flow_offload.h >>> @@ -148,6 +148,8 @@ enum flow_action_id { >>> FLOW_ACTION_MPLS_MANGLE, >>> FLOW_ACTION_GATE, >>> FLOW_ACTION_PPPOE_PUSH, >>> + FLOW_ACTION_JUMP, >>> + FLOW_ACTION_PIPE, >>> NUM_FLOW_ACTIONS, >>> }; >>> >>> @@ -235,9 +237,20 @@ struct flow_action_entry { >>> struct { /* FLOW_ACTION_POLICE */ >>> u32 burst; >>> u64 rate_bytes_ps; >>> + u64 peakrate_bytes_ps; >>> + u32 avrate; >>> + u16 overhead; >>> u64 burst_pkt; >>> u64 rate_pkt_ps; >>> u32 mtu; >>> + struct { >>> + enum flow_action_id act_id; >>> + u32 index; >>> + } exceed; >>> + struct { >>> + enum flow_action_id act_id; >>> + u32 index; >>> + } notexceed; >> It seems exceed and notexceed use the same format struct, will it be more >simpler to define as: >> struct { >> enum flow_action_id act_id; >> u32 index; >> } exceed, notexceed; > >right. it can be. > >> >>> } police; >>> struct { /* FLOW_ACTION_CT */ >>> int action; >>> diff --git a/include/net/tc_act/tc_police.h >>> b/include/net/tc_act/tc_police.h index 72649512dcdd..283bde711a42 >>> 100644 >>> --- a/include/net/tc_act/tc_police.h >>> +++ b/include/net/tc_act/tc_police.h >>> @@ -159,4 +159,34 @@ static inline u32 tcf_police_tcfp_mtu(const >>> struct tc_action *act) >>> return params->tcfp_mtu; >>> } >>> >>> +static inline u64 tcf_police_peakrate_bytes_ps(const struct >>> +tc_action >>> +*act) { >>> + struct tcf_police *police = to_police(act); >>> + struct tcf_police_params *params; >>> + >>> + params = rcu_dereference_protected(police->params, >>> + lockdep_is_held(&police->tcf_lock)); >>> + return params->peak.rate_bytes_ps; >>> +} >>> + >>> +static inline u32 tcf_police_tcfp_ewma_rate(const struct tc_action >>> +*act) { >>> + struct tcf_police *police = to_police(act); >>> + struct tcf_police_params *params; >>> + >>> + params = rcu_dereference_protected(police->params, >>> + lockdep_is_held(&police->tcf_lock)); >>> + return params->tcfp_ewma_rate; >>> +} >>> + >>> +static inline u16 tcf_police_rate_overhead(const struct tc_action >>> +*act) { >>> + struct tcf_police *police = to_police(act); >>> + struct tcf_police_params *params; >>> + >>> + params = rcu_dereference_protected(police->params, >>> + lockdep_is_held(&police->tcf_lock)); >>> + return params->rate.overhead; >>> +} >>> + >>> #endif /* __NET_TC_POLICE_H */ >>> diff --git a/net/sched/act_police.c b/net/sched/act_police.c index >>> 0923aa2b8f8a..0457b6c9c4e7 100644 >>> --- a/net/sched/act_police.c >>> +++ b/net/sched/act_police.c >>> @@ -405,20 +405,66 @@ static int tcf_police_search(struct net *net, >>> struct tc_action **a, u32 index) >>> return tcf_idr_search(tn, a, index); } >>> >>> +static int tcf_police_act_to_flow_act(int tc_act, int *index) { >>> + int act_id = -EOPNOTSUPP; >>> + >>> + if (!TC_ACT_EXT_OPCODE(tc_act)) { >>> + if (tc_act == TC_ACT_OK) >>> + act_id = FLOW_ACTION_ACCEPT; >>> + else if (tc_act == TC_ACT_SHOT) >>> + act_id = FLOW_ACTION_DROP; >>> + else if (tc_act == TC_ACT_PIPE) >>> + act_id = FLOW_ACTION_PIPE; >>> + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { >>> + act_id = FLOW_ACTION_GOTO; >>> + *index = tc_act & TC_ACT_EXT_VAL_MASK; >> For the TC_ACT_GOTO_CHAIN action, the goto_chain information is missing >from software to hardware, is it useful for hardware to check? >> > >what information do you mean? Sorry, I do not realize the chain index is in the return value of index, so please just ignore. It seems the definition of index is a little confused since in TC_ACT_GOTO_CHAIN case, it means chain index and in TC_ACT_JUMP case, it means jump count. Just a suggestion, can we change the index definition as a union as: union { u32 chain_index; u32 jmp_cnt; { WDYT? > >>> + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { >>> + act_id = FLOW_ACTION_JUMP; >>> + *index = tc_act & TC_ACT_EXT_VAL_MASK; >>> + } >>> + >>> + return act_id; >>> +} >>> + >>> static int tcf_police_offload_act_setup(struct tc_action *act, void >*entry_data, >>> u32 *index_inc, bool bind) >>> { >>> if (bind) { >>> struct flow_action_entry *entry = entry_data; >>> + struct tcf_police *police = to_police(act); >>> + struct tcf_police_params *p; >>> + int act_id; >>> + >>> + p = rcu_dereference_protected(police->params, >>> + lockdep_is_held(&police- >>tcf_lock)); >>> >>> entry->id = FLOW_ACTION_POLICE; >>> entry->police.burst = tcf_police_burst(act); >>> entry->police.rate_bytes_ps = >>> tcf_police_rate_bytes_ps(act); >>> + entry->police.peakrate_bytes_ps = >>> tcf_police_peakrate_bytes_ps(act); >>> + entry->police.avrate = tcf_police_tcfp_ewma_rate(act); >>> + entry->police.overhead = tcf_police_rate_overhead(act); >>> entry->police.burst_pkt = tcf_police_burst_pkt(act); >>> entry->police.rate_pkt_ps = >>> tcf_police_rate_pkt_ps(act); >>> entry->police.mtu = tcf_police_tcfp_mtu(act); >>> + >>> + act_id = tcf_police_act_to_flow_act(police->tcf_action, >>> + &entry- >>>> police.exceed.index); >>> + if (act_id < 0) >>> + return act_id; >>> + >>> + entry->police.exceed.act_id = act_id; >>> + >>> + act_id = tcf_police_act_to_flow_act(p->tcfp_result, >>> + &entry- >>>> police.notexceed.index); >>> + if (act_id < 0) >>> + return act_id; >>> + >>> + entry->police.notexceed.act_id = act_id; >>> + >>> *index_inc = 1; >>> } else { >>> struct flow_offload_action *fl_action = entry_data; >>> -- >>> 2.26.2 >>
On Fri, 2022-02-18 at 01:46 +0000, Baowen Zheng wrote: > On, February 17, 2022 8:10 PM, Roi wrote: > > On 2022-02-17 12:25 PM, Baowen Zheng wrote: > > > On February 17, 2022 4:28 PM, Jianbo wrote: > > > > The current police offload action entry is missing > > > > exceed/notexceed > > > > actions and parameters that can be configured by tc police > > > > action. > > > > Add the missing parameters as a pre-step for offloading police > > > > actions to hardware. > > > > > > > > Signed-off-by: Jianbo Liu <jianbol@nvidia.com> > > > > Signed-off-by: Roi Dayan <roid@nvidia.com> > > > > Reviewed-by: Ido Schimmel <idosch@nvidia.com> > > > > --- > > > > include/net/flow_offload.h | 13 ++++++++++ > > > > include/net/tc_act/tc_police.h | 30 ++++++++++++++++++++++ > > > > net/sched/act_police.c | 46 > > ++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 89 insertions(+) > > > > > > > > diff --git a/include/net/flow_offload.h > > > > b/include/net/flow_offload.h > > > > index > > > > 5b8c54eb7a6b..94cde6bbc8a5 100644 > > > > --- a/include/net/flow_offload.h > > > > +++ b/include/net/flow_offload.h > > > > @@ -148,6 +148,8 @@ enum flow_action_id { > > > > FLOW_ACTION_MPLS_MANGLE, > > > > FLOW_ACTION_GATE, > > > > FLOW_ACTION_PPPOE_PUSH, > > > > + FLOW_ACTION_JUMP, > > > > + FLOW_ACTION_PIPE, > > > > NUM_FLOW_ACTIONS, > > > > }; > > > > > > > > @@ -235,9 +237,20 @@ struct flow_action_entry { > > > > struct { /* > > > > FLOW_ACTION_POLICE */ > > > > u32 burst; > > > > u64 rate_bytes_ps; > > > > + u64 peakrate_bytes_ > > > > ps; > > > > + u32 avrate; > > > > + u16 overhead; > > > > u64 burst_pkt; > > > > u64 rate_pkt_ps; > > > > u32 mtu; > > > > + struct { > > > > + enum flow_action_id act_id; > > > > + u32 index; > > > > + } exceed; > > > > + struct { > > > > + enum flow_action_id act_id; > > > > + u32 index; > > > > + } notexceed; > > > It seems exceed and notexceed use the same format struct, will it > > > be more > > simpler to define as: > > > struct { > > > enum flow_action_id act_id; > > > u32 index; > > > } exceed, notexceed; > > > > right. it can be. > > > > > > > > > } police; > > > > struct { /* > > > > FLOW_ACTION_CT */ > > > > int action; > > > > diff --git a/include/net/tc_act/tc_police.h > > > > b/include/net/tc_act/tc_police.h index > > > > 72649512dcdd..283bde711a42 > > > > 100644 > > > > --- a/include/net/tc_act/tc_police.h > > > > +++ b/include/net/tc_act/tc_police.h > > > > @@ -159,4 +159,34 @@ static inline u32 > > > > tcf_police_tcfp_mtu(const > > > > struct tc_action *act) > > > > return params->tcfp_mtu; > > > > } > > > > > > > > +static inline u64 tcf_police_peakrate_bytes_ps(const struct > > > > +tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->peak.rate_bytes_ps; > > > > +} > > > > + > > > > +static inline u32 tcf_police_tcfp_ewma_rate(const struct > > > > tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->tcfp_ewma_rate; > > > > +} > > > > + > > > > +static inline u16 tcf_police_rate_overhead(const struct > > > > tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->rate.overhead; > > > > +} > > > > + > > > > #endif /* __NET_TC_POLICE_H */ > > > > diff --git a/net/sched/act_police.c b/net/sched/act_police.c > > > > index > > > > 0923aa2b8f8a..0457b6c9c4e7 100644 > > > > --- a/net/sched/act_police.c > > > > +++ b/net/sched/act_police.c > > > > @@ -405,20 +405,66 @@ static int tcf_police_search(struct net > > > > *net, > > > > struct tc_action **a, u32 index) > > > > return tcf_idr_search(tn, a, index); } > > > > > > > > +static int tcf_police_act_to_flow_act(int tc_act, int *index) > > > > { > > > > + int act_id = -EOPNOTSUPP; > > > > + > > > > + if (!TC_ACT_EXT_OPCODE(tc_act)) { > > > > + if (tc_act == TC_ACT_OK) > > > > + act_id = FLOW_ACTION_ACCEPT; > > > > + else if (tc_act == TC_ACT_SHOT) > > > > + act_id = FLOW_ACTION_DROP; > > > > + else if (tc_act == TC_ACT_PIPE) > > > > + act_id = FLOW_ACTION_PIPE; > > > > + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { > > > > + act_id = FLOW_ACTION_GOTO; > > > > + *index = tc_act & TC_ACT_EXT_VAL_MASK; > > > For the TC_ACT_GOTO_CHAIN action, the goto_chain information is > > > missing > > from software to hardware, is it useful for hardware to check? > > > > > > > what information do you mean? > Sorry, I do not realize the chain index is in the return value of > index, so please just ignore. OK > It seems the definition of index is a little confused since in > TC_ACT_GOTO_CHAIN case, it means chain index and in TC_ACT_JUMP case, > it means jump count. > Just a suggestion, can we change the index definition as a union as: > union { > u32 chain_index; > u32 jmp_cnt; > { > WDYT? > Yes, we will consider that. Thanks!
On Fri, 2022-02-18 at 01:46 +0000, Baowen Zheng wrote: > On, February 17, 2022 8:10 PM, Roi wrote: > > On 2022-02-17 12:25 PM, Baowen Zheng wrote: > > > On February 17, 2022 4:28 PM, Jianbo wrote: > > > > The current police offload action entry is missing > > > > exceed/notexceed > > > > actions and parameters that can be configured by tc police > > > > action. > > > > Add the missing parameters as a pre-step for offloading police > > > > actions to hardware. > > > > > > > > Signed-off-by: Jianbo Liu <jianbol@nvidia.com> > > > > Signed-off-by: Roi Dayan <roid@nvidia.com> > > > > Reviewed-by: Ido Schimmel <idosch@nvidia.com> > > > > --- > > > > include/net/flow_offload.h | 13 ++++++++++ > > > > include/net/tc_act/tc_police.h | 30 ++++++++++++++++++++++ > > > > net/sched/act_police.c | 46 > > ++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 89 insertions(+) > > > > > > > > diff --git a/include/net/flow_offload.h > > > > b/include/net/flow_offload.h > > > > index > > > > 5b8c54eb7a6b..94cde6bbc8a5 100644 > > > > --- a/include/net/flow_offload.h > > > > +++ b/include/net/flow_offload.h > > > > @@ -148,6 +148,8 @@ enum flow_action_id { > > > > FLOW_ACTION_MPLS_MANGLE, > > > > FLOW_ACTION_GATE, > > > > FLOW_ACTION_PPPOE_PUSH, > > > > + FLOW_ACTION_JUMP, > > > > + FLOW_ACTION_PIPE, > > > > NUM_FLOW_ACTIONS, > > > > }; > > > > > > > > @@ -235,9 +237,20 @@ struct flow_action_entry { > > > > struct { /* > > > > FLOW_ACTION_POLICE */ > > > > u32 burst; > > > > u64 rate_bytes_ps; > > > > + u64 peakrate_bytes_ > > > > ps; > > > > + u32 avrate; > > > > + u16 overhead; > > > > u64 burst_pkt; > > > > u64 rate_pkt_ps; > > > > u32 mtu; > > > > + struct { > > > > + enum flow_action_id act_id; > > > > + u32 index; > > > > + } exceed; > > > > + struct { > > > > + enum flow_action_id act_id; > > > > + u32 index; > > > > + } notexceed; > > > It seems exceed and notexceed use the same format struct, will it > > > be more > > simpler to define as: > > > struct { > > > enum flow_action_id act_id; > > > u32 index; > > > } exceed, notexceed; > > > > right. it can be. > > > > > > > > > } police; > > > > struct { /* > > > > FLOW_ACTION_CT */ > > > > int action; > > > > diff --git a/include/net/tc_act/tc_police.h > > > > b/include/net/tc_act/tc_police.h index > > > > 72649512dcdd..283bde711a42 > > > > 100644 > > > > --- a/include/net/tc_act/tc_police.h > > > > +++ b/include/net/tc_act/tc_police.h > > > > @@ -159,4 +159,34 @@ static inline u32 > > > > tcf_police_tcfp_mtu(const > > > > struct tc_action *act) > > > > return params->tcfp_mtu; > > > > } > > > > > > > > +static inline u64 tcf_police_peakrate_bytes_ps(const struct > > > > +tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->peak.rate_bytes_ps; > > > > +} > > > > + > > > > +static inline u32 tcf_police_tcfp_ewma_rate(const struct > > > > tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->tcfp_ewma_rate; > > > > +} > > > > + > > > > +static inline u16 tcf_police_rate_overhead(const struct > > > > tc_action > > > > +*act) { > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *params; > > > > + > > > > + params = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police->tcf_lock)); > > > > + return params->rate.overhead; > > > > +} > > > > + > > > > #endif /* __NET_TC_POLICE_H */ > > > > diff --git a/net/sched/act_police.c b/net/sched/act_police.c > > > > index > > > > 0923aa2b8f8a..0457b6c9c4e7 100644 > > > > --- a/net/sched/act_police.c > > > > +++ b/net/sched/act_police.c > > > > @@ -405,20 +405,66 @@ static int tcf_police_search(struct net > > > > *net, > > > > struct tc_action **a, u32 index) > > > > return tcf_idr_search(tn, a, index); } > > > > > > > > +static int tcf_police_act_to_flow_act(int tc_act, int *index) > > > > { > > > > + int act_id = -EOPNOTSUPP; > > > > + > > > > + if (!TC_ACT_EXT_OPCODE(tc_act)) { > > > > + if (tc_act == TC_ACT_OK) > > > > + act_id = FLOW_ACTION_ACCEPT; > > > > + else if (tc_act == TC_ACT_SHOT) > > > > + act_id = FLOW_ACTION_DROP; > > > > + else if (tc_act == TC_ACT_PIPE) > > > > + act_id = FLOW_ACTION_PIPE; > > > > + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { > > > > + act_id = FLOW_ACTION_GOTO; > > > > + *index = tc_act & TC_ACT_EXT_VAL_MASK; > > > For the TC_ACT_GOTO_CHAIN action, the goto_chain information is > > > missing > > from software to hardware, is it useful for hardware to check? > > > > > > > what information do you mean? > Sorry, I do not realize the chain index is in the return value of > index, so please just ignore. > It seems the definition of index is a little confused since in > TC_ACT_GOTO_CHAIN case, it means chain index and in TC_ACT_JUMP case, > it means jump count. > Just a suggestion, can we change the index definition as a union as: > union { > u32 chain_index; > u32 jmp_cnt; > { > WDYT? > > Hi Baowen, If changing to inline union, either the pointer of chain_index or jmp_cnt should be passed to tcf_police_act_to_flow_act(). But the caller doesn't know which one to use, because it doesn't know if the action is goto or jump. Besides, it's not a must as we alreay know what type the action is from act_id. So what about just renaming index to extval? Thanks! Jianbo > > > > + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { > > > > + act_id = FLOW_ACTION_JUMP; > > > > + *index = tc_act & TC_ACT_EXT_VAL_MASK; > > > > + } > > > > + > > > > + return act_id; > > > > +} > > > > + > > > > static int tcf_police_offload_act_setup(struct tc_action *act, > > > > void > > *entry_data, > > > > u32 *index_inc, bool > > > > bind) > > > > { > > > > if (bind) { > > > > struct flow_action_entry *entry = entry_data; > > > > + struct tcf_police *police = to_police(act); > > > > + struct tcf_police_params *p; > > > > + int act_id; > > > > + > > > > + p = rcu_dereference_protected(police->params, > > > > + > > > > lockdep_is_held(&police- > > > tcf_lock)); > > > > > > > > entry->id = FLOW_ACTION_POLICE; > > > > entry->police.burst = tcf_police_burst(act); > > > > entry->police.rate_bytes_ps = > > > > tcf_police_rate_bytes_ps(act); > > > > + entry->police.peakrate_bytes_ps = > > > > tcf_police_peakrate_bytes_ps(act); > > > > + entry->police.avrate = > > > > tcf_police_tcfp_ewma_rate(act); > > > > + entry->police.overhead = > > > > tcf_police_rate_overhead(act); > > > > entry->police.burst_pkt = > > > > tcf_police_burst_pkt(act); > > > > entry->police.rate_pkt_ps = > > > > tcf_police_rate_pkt_ps(act); > > > > entry->police.mtu = tcf_police_tcfp_mtu(act); > > > > + > > > > + act_id = tcf_police_act_to_flow_act(police- > > > > >tcf_action, > > > > + &entry- > > > > > police.exceed.index); > > > > + if (act_id < 0) > > > > + return act_id; > > > > + > > > > + entry->police.exceed.act_id = act_id; > > > > + > > > > + act_id = tcf_police_act_to_flow_act(p- > > > > >tcfp_result, > > > > + &entry- > > > > > police.notexceed.index); > > > > + if (act_id < 0) > > > > + return act_id; > > > > + > > > > + entry->police.notexceed.act_id = act_id; > > > > + > > > > *index_inc = 1; > > > > } else { > > > > struct flow_offload_action *fl_action = > > > > entry_data; > > > > -- > > > > 2.26.2 > > >
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h index 5b8c54eb7a6b..94cde6bbc8a5 100644 --- a/include/net/flow_offload.h +++ b/include/net/flow_offload.h @@ -148,6 +148,8 @@ enum flow_action_id { FLOW_ACTION_MPLS_MANGLE, FLOW_ACTION_GATE, FLOW_ACTION_PPPOE_PUSH, + FLOW_ACTION_JUMP, + FLOW_ACTION_PIPE, NUM_FLOW_ACTIONS, }; @@ -235,9 +237,20 @@ struct flow_action_entry { struct { /* FLOW_ACTION_POLICE */ u32 burst; u64 rate_bytes_ps; + u64 peakrate_bytes_ps; + u32 avrate; + u16 overhead; u64 burst_pkt; u64 rate_pkt_ps; u32 mtu; + struct { + enum flow_action_id act_id; + u32 index; + } exceed; + struct { + enum flow_action_id act_id; + u32 index; + } notexceed; } police; struct { /* FLOW_ACTION_CT */ int action; diff --git a/include/net/tc_act/tc_police.h b/include/net/tc_act/tc_police.h index 72649512dcdd..283bde711a42 100644 --- a/include/net/tc_act/tc_police.h +++ b/include/net/tc_act/tc_police.h @@ -159,4 +159,34 @@ static inline u32 tcf_police_tcfp_mtu(const struct tc_action *act) return params->tcfp_mtu; } +static inline u64 tcf_police_peakrate_bytes_ps(const struct tc_action *act) +{ + struct tcf_police *police = to_police(act); + struct tcf_police_params *params; + + params = rcu_dereference_protected(police->params, + lockdep_is_held(&police->tcf_lock)); + return params->peak.rate_bytes_ps; +} + +static inline u32 tcf_police_tcfp_ewma_rate(const struct tc_action *act) +{ + struct tcf_police *police = to_police(act); + struct tcf_police_params *params; + + params = rcu_dereference_protected(police->params, + lockdep_is_held(&police->tcf_lock)); + return params->tcfp_ewma_rate; +} + +static inline u16 tcf_police_rate_overhead(const struct tc_action *act) +{ + struct tcf_police *police = to_police(act); + struct tcf_police_params *params; + + params = rcu_dereference_protected(police->params, + lockdep_is_held(&police->tcf_lock)); + return params->rate.overhead; +} + #endif /* __NET_TC_POLICE_H */ diff --git a/net/sched/act_police.c b/net/sched/act_police.c index 0923aa2b8f8a..0457b6c9c4e7 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -405,20 +405,66 @@ static int tcf_police_search(struct net *net, struct tc_action **a, u32 index) return tcf_idr_search(tn, a, index); } +static int tcf_police_act_to_flow_act(int tc_act, int *index) +{ + int act_id = -EOPNOTSUPP; + + if (!TC_ACT_EXT_OPCODE(tc_act)) { + if (tc_act == TC_ACT_OK) + act_id = FLOW_ACTION_ACCEPT; + else if (tc_act == TC_ACT_SHOT) + act_id = FLOW_ACTION_DROP; + else if (tc_act == TC_ACT_PIPE) + act_id = FLOW_ACTION_PIPE; + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { + act_id = FLOW_ACTION_GOTO; + *index = tc_act & TC_ACT_EXT_VAL_MASK; + } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { + act_id = FLOW_ACTION_JUMP; + *index = tc_act & TC_ACT_EXT_VAL_MASK; + } + + return act_id; +} + static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data, u32 *index_inc, bool bind) { if (bind) { struct flow_action_entry *entry = entry_data; + struct tcf_police *police = to_police(act); + struct tcf_police_params *p; + int act_id; + + p = rcu_dereference_protected(police->params, + lockdep_is_held(&police->tcf_lock)); entry->id = FLOW_ACTION_POLICE; entry->police.burst = tcf_police_burst(act); entry->police.rate_bytes_ps = tcf_police_rate_bytes_ps(act); + entry->police.peakrate_bytes_ps = tcf_police_peakrate_bytes_ps(act); + entry->police.avrate = tcf_police_tcfp_ewma_rate(act); + entry->police.overhead = tcf_police_rate_overhead(act); entry->police.burst_pkt = tcf_police_burst_pkt(act); entry->police.rate_pkt_ps = tcf_police_rate_pkt_ps(act); entry->police.mtu = tcf_police_tcfp_mtu(act); + + act_id = tcf_police_act_to_flow_act(police->tcf_action, + &entry->police.exceed.index); + if (act_id < 0) + return act_id; + + entry->police.exceed.act_id = act_id; + + act_id = tcf_police_act_to_flow_act(p->tcfp_result, + &entry->police.notexceed.index); + if (act_id < 0) + return act_id; + + entry->police.notexceed.act_id = act_id; + *index_inc = 1; } else { struct flow_offload_action *fl_action = entry_data;