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

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

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

[..]

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).

quoted

quoted

Do you think it is proper to include this implement in our patch series or we

can delivery a new patch for this?

Unless I am missing something basic, I dont see this as hard to do as explained
above in this patch series.

I did not mean it is difficult.
Since as my understanding, we will have the same problem in function of tc_setup_flow_action to
Setup the actions for a to be offloaded flower. So my proposal is to add a callback in action ops to implement
Both the function of flow_act_init and tc_setup_flow_action with a flag(maybe bind?) as a distinguish.
What is your opinion?

See above. I agree with your suggestion.

cheers,
jamal