diff mbox

[net-next,repost] net: sched: add helpers to handle extended actions

Message ID 1493712720-1501-1-git-send-email-jiri@resnulli.us
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jiri Pirko May 2, 2017, 8:12 a.m. UTC 
From: Jiri Pirko <jiri@mellanox.com>

Jump is now the only one using value action opcode. This is going to
change soon. So introduce helpers to work with this. Convert TC_ACT_JUMP.

This also fixes the TC_ACT_JUMP check, which is incorrectly done as a
bit check, not a value check.

Fixes: e0ee84ded796 ("net sched actions: Complete the JUMPX opcode")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
Dave, I'm sending this for -net-next although I know it is closed. But
the mentioned commit is not yet in -net. Feel free to take this either
to -net-next or -net, whatever suits you better. Thanks.
---
 include/uapi/linux/pkt_cls.h | 15 ++++++++++++++-
 net/sched/act_api.c          |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Jamal Hadi Salim May 2, 2017, 11:59 a.m. UTC | #1 
On 17-05-02 04:12 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Jump is now the only one using value action opcode. This is going to
> change soon. So introduce helpers to work with this. Convert TC_ACT_JUMP.
>
> This also fixes the TC_ACT_JUMP check, which is incorrectly done as a
> bit check, not a value check.
>
> Fixes: e0ee84ded796 ("net sched actions: Complete the JUMPX opcode")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> Dave, I'm sending this for -net-next although I know it is closed. But
> the mentioned commit is not yet in -net. Feel free to take this either
> to -net-next or -net, whatever suits you better. Thanks.

I think you are pushing the boundary a little calling it a bug fix
and this could go with your patch series instead.
The name TC_ACT_EXT_CMP should be TC_ACT_EXT_CMP_OPCODE
Other than that:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
Jiri Pirko May 2, 2017, 12:07 p.m. UTC | #2 
Tue, May 02, 2017 at 01:59:20PM CEST, jhs@mojatatu.com wrote:
>On 17-05-02 04:12 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Jump is now the only one using value action opcode. This is going to
>> change soon. So introduce helpers to work with this. Convert TC_ACT_JUMP.
>> 
>> This also fixes the TC_ACT_JUMP check, which is incorrectly done as a
>> bit check, not a value check.
>> 
>> Fixes: e0ee84ded796 ("net sched actions: Complete the JUMPX opcode")
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> Dave, I'm sending this for -net-next although I know it is closed. But
>> the mentioned commit is not yet in -net. Feel free to take this either
>> to -net-next or -net, whatever suits you better. Thanks.
>
>I think you are pushing the boundary a little calling it a bug fix

Well, it is a bugfix. Otherwise we could not use bit 1 for anything else
then jump in the future. This is also UAPI. That's why I'm pushing it as
a fix.

>and this could go with your patch series instead.
>The name TC_ACT_EXT_CMP should be TC_ACT_EXT_CMP_OPCODE

I was thinking about it as well, I would like to keep it shorter. The
current name is quite appropriate and it is clear what the macro does.


>Other than that:
>
>Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
 
Thanks.
David Miller May 2, 2017, 7:34 p.m. UTC | #3 
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue,  2 May 2017 10:12:00 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Jump is now the only one using value action opcode. This is going to
> change soon. So introduce helpers to work with this. Convert TC_ACT_JUMP.
> 
> This also fixes the TC_ACT_JUMP check, which is incorrectly done as a
> bit check, not a value check.
> 
> Fixes: e0ee84ded796 ("net sched actions: Complete the JUMPX opcode")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> Dave, I'm sending this for -net-next although I know it is closed. But
> the mentioned commit is not yet in -net. Feel free to take this either
> to -net-next or -net, whatever suits you better. Thanks.

Applied to net-next, thanks Jiri.
diff mbox

Patch

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index f1129e3..d613be3 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -37,7 +37,20 @@  enum {
 #define TC_ACT_QUEUED		5
 #define TC_ACT_REPEAT		6
 #define TC_ACT_REDIRECT		7
-#define TC_ACT_JUMP		0x10000000
+
+/* There is a special kind of actions called "extended actions",
+ * which need a value parameter. These have a local opcode located in
+ * the highest nibble, starting from 1. The rest of the bits
+ * are used to carry the value. These two parts together make
+ * a combined opcode.
+ */
+#define __TC_ACT_EXT_SHIFT 28
+#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
+#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
+#define TC_ACT_EXT_CMP(combined, opcode) \
+	(((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
+
+#define TC_ACT_JUMP __TC_ACT_EXT(1)
 
 /* Action type identifiers*/
 enum {
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 7f2cd70..a90e8f3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -453,7 +453,7 @@  int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 		if (ret == TC_ACT_REPEAT)
 			goto repeat;	/* we need a ttl - JHS */
 
-		if (ret & TC_ACT_JUMP) {
+		if (TC_ACT_EXT_CMP(ret, TC_ACT_JUMP)) {
 			jmp_prgcnt = ret & TCA_ACT_MAX_PRIO_MASK;
 			if (!jmp_prgcnt || (jmp_prgcnt > nr_actions)) {
 				/* faulty opcode, stop pipeline */