On 13/02/2023 20:43, Marcelo Ricardo Leitner wrote:

On Mon, Feb 13, 2023 at 06:13:34PM +0200, Paul Blakey wrote:

quoted

On 10/02/2023 04:21, Marcelo Ricardo Leitner wrote:

quoted

On Mon, Feb 06, 2023 at 07:43:57PM +0200, Paul Blakey wrote:

quoted

For drivers to support partial offload of a filter's action list,
add support for action miss to specify an action instance to
continue from in sw.

CT action in particular can't be fully offloaded, as new connections
need to be handled in software. This imposes other limitations on
the actions that can be offloaded together with the CT action, such
as packet modifications.

Assign each action on a filter's action list a unique miss_cookie
which drivers can then use to fill action_miss part of the tc skb
extension. On getting back this miss_cookie, find the action
instance with relevant cookie and continue classifying from there.

Signed-off-by: Paul Blakey <redacted>
Reviewed-by: Jiri Pirko <redacted>
Reviewed-by: Simon Horman <redacted>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
  include/linux/skbuff.h     |   6 +-
  include/net/flow_offload.h |   1 +
  include/net/pkt_cls.h      |  34 +++---
  include/net/sch_generic.h  |   2 +
  net/openvswitch/flow.c     |   3 +-
  net/sched/act_api.c        |   2 +-
  net/sched/cls_api.c        | 213 +++++++++++++++++++++++++++++++++++--
  7 files changed, 234 insertions(+), 27 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1fa95b916342..9b9aa854068f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -311,12 +311,16 @@ struct nf_bridge_info {
   * and read by ovs to recirc_id.
   */
  struct tc_skb_ext {
-	__u32 chain;
+	union {
+		u64 act_miss_cookie;
+		__u32 chain;
+	};
  	__u16 mru;
  	__u16 zone;
  	u8 post_ct:1;
  	u8 post_ct_snat:1;
  	u8 post_ct_dnat:1;
+	u8 act_miss:1; /* Set if act_miss_cookie is used */
  };
  #endif

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 0400a0ac8a29..88db7346eb7a 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h

@@ -228,6 +228,7 @@ void flow_action_cookie_destroy(struct flow_action_cookie *cookie);
  struct flow_action_entry {
  	enum flow_action_id		id;
  	u32				hw_index;
+	u64				miss_cookie;

The per-action stats patchset is adding a cookie for the actions as
well, and exactly on this struct:

@@ -228,6 +228,7 @@ struct flow_action_cookie *flow_action_cookie_create(void *data,
  struct flow_action_entry {
         enum flow_action_id             id;
         u32                             hw_index;
+       unsigned long                   act_cookie;
         enum flow_action_hw_stats       hw_stats;
         action_destr                    destructor;
         void                            *destructor_priv;

There, it is a simple value: the act pointer itself. Here, it is already more
complex. Can them be merged into only one maybe?
If not, perhaps act_cookie should be renamed to stats_cookie then.

I don't think it can be shared, actions can be shared between multiple
filters, while the miss cookie would be different for each used instance
(takes the filter in to account).

Good point. So it would at best be a masked value that part A works
for the miss here and part B for the stats, which is pretty much what
the two cookies are giving, just without having to do bit gymnasics,
yes.

act cookie is using 64 bits (to store the pointer and void a mapping), and I'm using at least

32bits, so there is not simple type that will contain both.

So I'll rename the act_cookie to stats_cookie once I rebase.