netdev - Re: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of filter and actions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <37b378de-efd7-8512-e322-78dfe07f977d@mojatatu.com>
Date:   Wed, 3 Nov 2021 11:35:13 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Baowen Zheng <baowen.zheng@...igine.com>,
        Simon Horman <simon.horman@...igine.com>,
        Vlad Buslov <vladbu@...dia.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Roi Dayan <roid@...dia.com>, Ido Schimmel <idosch@...dia.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Baowen Zheng <notifications@...hub.com>,
        Louis Peens <louis.peens@...igine.com>,
        oss-drivers <oss-drivers@...igine.com>,
        Oz Shlomo <ozsh@...dia.com>
Subject: Re: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of
 filter and actions

On 2021-11-03 10:48, Baowen Zheng wrote:
> On November 3, 2021 10:16 PM, Jamal Hadi Salim wrote:
>> On 2021-11-03 10:03, Baowen Zheng wrote:
>>> Thanks for your reply.
>>> On November 3, 2021 9:34 PM, Jamal Hadi Salim wrote:
>>>> On 2021-11-03 08:33, Jamal Hadi Salim wrote:
>>>>> On 2021-11-03 07:30, Baowen Zheng wrote:
>>>>>> On November 3, 2021 6:14 PM, Jamal Hadi Salim wrote:
>>
>>
>> [..]
>>
>>> Sorry for more clarification about another case that Vlad mentioned:
>>> #add a policer action with skip_hw
>>> tc actions add action police skip_hw rate ... index 20 #Now add a
>>> filter5 which has no flag tc filter add dev $DEV1 proto ip parent
>>> ffff: flower \
>>>          ip_proto icmp action police index 20 I think the filter5 could
>>> be legal, since it will not run in hardware.
>>> Driver will check failed when try to offload this filter. So the filter5 will only
>> run in software.
>>> WDYT?
>>>
>>
>> I think this one also has ambiguity. If the filter doesnt specify skip_sw or
>> skip_hw it will run both in s/w and h/w. I am worried if that looks suprising to
>> someone debugging after because in h/w there is filter 5 but no policer but in
>> s/w twin we have filter 5 and policer index 20.
> In this case, the filter will not in h/w because when the driver tries to offload the filter,
> It will found the action is not in h/w and return failed, then the filter will not in h/w, so the filter will only
> In software.

So you have partial failure? That doesnt sound good. What do you return
to the user - "success" or "somehow success"?
I worry it is still ambigous. Did the user really intend to do that?
If they did maybe they should have just added it to s/w instead of h/w
and s/w and then get saved by the driver?

>> It could be design intent, but in my opinion we have priorities to resolve such
>> ambiguities in policies.
>>
>> If we use the rule which says the flags have to match exactly then we can
>> simplify resolving any ambiguity - which will make it illegal, no?
> When you mentioned " match exactly ", do you mean the flags of the filter and the actions should be
> exactly same?
> Please consider the case that filter has flag and the action does not have any flag. 

See above.

> I think we should allow this case.
> Because it is legal before our patch, we do not expect to break this use case, yes?
> So maybe the "match exactly" just limits action flags, when action has flags(skip_sw or skip_hw), the filter must have
> exactly the same flags.

Maybe i am missing something but nothing should break.
I think what you mean is when the action is specified with
the filter. The flags should be the same in that case.

Example, filter 1:
tc filter add dev $DEV1 proto ip paren ffff: flower \
ip_proto icmp action police blah

where flag is 0 implies this filter goes both in h/w and s/w.
If i dump the policer or the filter i will see some index provided by
the kernel and i should be able to see both s/w and h/w
counters.

Same thing if i did:

Example filter 2:
tc filter add dev $DEV1 proto ip paren ffff: flower \
skip_sw ip_proto udp action police blah

both filter + action will have where flag of skip_sw when i dump
implies this filter goes only in h/w and any displayed index
is allocated by the kernel.


Our challenge is when someone specifies a specific action by index
and tries to use it ambigously.

cheers,
jamal