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

Message-ID: <DM5PR1301MB21728931E03CFE4FA45C5DD3E78A9@DM5PR1301MB2172.namprd13.prod.outlook.com>
Date:   Mon, 1 Nov 2021 03:29:49 +0000
From:   Baowen Zheng <baowen.zheng@...igine.com>
To:     Jamal Hadi Salim <jhs@...atatu.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-31 9:31 PM, Jamal Hadi Salim wrote:
>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).
>
I think the cases you mentioned make sense to us. But what Vlad concerns is the use
case as: 
#add a policer offload it
tc actions add action police skip_sw rate ... index 20
#now add filter4 which can't be  offloaded
tc filter add dev $DEV1 proto ip parent ffff: flower \
ip_proto tcp action police index 20
it is possible the filter4 can't be offloaded, then filter4 will run in software,
should this be legal? 
Originally I think this is legal, but as comments of Vlad, this should not be legal, since the action
will not be executed in software. I think what Vlad concerns is do we really need skip_sw flag for
an action? If a packet matches the filter in software, the action should not be skip_sw. 
If we choose to omit the skip_sw flag and just keep skip_hw, it will simplify our work. 
Of course, we can also keep skip_sw by adding more check to avoid the above case. 

Vlad, I am not sure if I understand your idea correctly.