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-next>] [day] [month] [year] [list]

Date:   Sun, 31 Oct 2021 09:30:46 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Baowen Zheng <baowen.zheng@...igine.com>,
        Vlad Buslov <vladbu@...dia.com>
Cc:     Simon Horman <simon.horman@...igine.com>,
        "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>
Subject: Re: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of
 filter and actions

On 2021-10-30 22:27, Baowen Zheng wrote:
> Thanks for your review, after some considerarion, I think I understand what you are meaning.
> 

[..]

>>>> I know Jamal suggested to have skip_sw for actions, but it complicates
>>>> the code and I'm still not entirely understand why it is necessary.
>>>
>>> If the hardware can independently accept an action offload then
>>> skip_sw per action makes total sense. BTW, my understanding is
>>
>> Example configuration that seems bizarre to me is when offloaded shared
>> action has skip_sw flag set but filter doesn't. Then behavior of
>> classifier that points to such action diverges between hardware and
>> software (different lists of actions are applied). We always try to make
>> offloaded TC data path behave exactly the same as software and, even
>> though here it would be explicit and deliberate, I don't see any
>> practical use-case for this.
> We add the skip_sw to keep compatible with the filter flags and give the user an
> option to specify if the action should run in software. I understand what you mean,
> maybe our example is not proper, we need to prevent the filter to run in software if the
> actions it applies is skip_sw, so we need to add more validation to check about this.
> Also I think your suggestion makes full sense if there is no use case to specify the action
> should not run in sw and indeed it will make our implement more simple if we omit the
> skip_sw option.
> Jamal, WDYT?


Let me use an example to illustrate my concern:

#add a policer offload it
tc actions add action police skip_sw rate ... index 20
#now add filter1 which is offloaded
tc filter add dev $DEV1 proto ip parent ffff: flower \
     skip_sw ip_proto tcp action police index 20
#add filter2 likewise offloaded
tc filter add dev $DEV1 proto ip parent ffff: flower \
     skip_sw ip_proto udp action police index 20

All good so far...
#Now add a filter3 which is s/w only
tc filter add dev $DEV1 proto ip parent ffff: flower \
     skip_hw ip_proto icmp action police index 20

filter3 should not be allowed.

If we had added the policer without skip_sw and without
skip_hw then i think filter3 should have been legal
(we just need to account for stats in_hw vs in_sw).

Not sure if that makes sense (and addresses Vlad's earlier
comment).


cheers,
jamal