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 <[email protected]>
Signed-off-by: Roi Dayan <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
---
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;
} 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;
--
2.26.2
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 <[email protected]>
>Signed-off-by: Roi Dayan <[email protected]>
>Reviewed-by: Ido Schimmel <[email protected]>
>---
> 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 <[email protected]>
>> Signed-off-by: Roi Dayan <[email protected]>
>> Reviewed-by: Ido Schimmel <[email protected]>
>> ---
>> 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 <[email protected]>
>>> Signed-off-by: Roi Dayan <[email protected]>
>>> Reviewed-by: Ido Schimmel <[email protected]>
>>> ---
>>> 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 <[email protected]>
> > > > Signed-off-by: Roi Dayan <[email protected]>
> > > > Reviewed-by: Ido Schimmel <[email protected]>
> > > > ---
> > > > 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 <[email protected]>
> > > > Signed-off-by: Roi Dayan <[email protected]>
> > > > Reviewed-by: Ido Schimmel <[email protected]>
> > > > ---
> > > > 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
> > >