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

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

Sorry for reply this message again.
On November 24, 2021 10:11 AM, Baowen Zheng wrote:

quoted

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

[..]

quoted

quoted

BTW: shouldnt extack be used here instead of returning just -EINVAL?
I didnt stare long enough but it seems extack is not passed when
deleting from hardware? I saw a NULL being passed in one of the patches.

Maybe I misunderstand what you mean previously, when I look through the implement in
flow_action_init, I did not found we use the extack to make a log before return -EINVAL.
So could you please figure it out? Maybe I miss something or misunderstand again.

I mean there are maybe 1-2 places where you called that function
flow_action_init() with extack being NULL but the others with
legitimate extack.
I pointed to offload delete as an example. This may have existed
before your changes (but it is hard to tell from just eyeballing
patches); regardless it is a problem for debugging incase some
delete offload fails, no?

BTW:
now that i am looking at the patches again - small details:
struct flow_offload_action is sometimes initialized and sometimes
not (and sometimes allocated and sometimes off the stack). Maybe
to be consistent pick one style and stick with it.

cheers,
jamal