Re: [PATCH v4 04/10] flow_offload: allow user to offload tc action to net device

On 2021-11-24 06:10, Jamal Hadi Salim wrote:

On 2021-11-23 21:11, Baowen Zheng wrote:

quoted

On November 24, 2021 3:04 AM, Jamal Hadi Salim wrote:

[..]

quoted

quoted

The simplest approach seems to be adding a field in ops struct and call it
hw_id (we already have id which represents the s/w side).
All your code in flow_action_init() then becomes something like:

         if (!fl_action)
                 return -EINVAL;

         fl_action->extack = extack;
         fl_action->command = cmd;
         fl_action->index = act->tcfa_index;


         fl_action->id = act->hwid;

And modules continue to work. Did i miss something?

Hi Jamal, for your suggestion, I think it will work for most of the case. But there maybe some kind of actions
that will be assigned different hw_id in different case, such as the gact, we need to think about this case.
So I will prefer to add a callback in action ops struct to implement the flow_action_init function for the new added
Standalone action.
WDYT?

Yes, the callback makes sense. I imagine this would be needed also
if you offload mirred (selecting whether to mirror or redirect).

BTW, I think i am able to parse your earlier message better. There is
an equivalent piece of code in cls_api.c. I didnt realize you had
cutnpasted from that code.
So this callback change has to be a separate patch. i.e
patchset 1 to
1) add the callback 2) simplify cls_api.c code
patchset 2: Your patchset that then uses the cb.

I am also wondering why that code is in the cls_api.c to begin with...

cheers,
jamal

I think if you add the action
callback then you can also simplify that.

Unfortunately that is now a separate patch given tha