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: <DM5PR1301MB2172F4949E810BDE380AF800E78C9@DM5PR1301MB2172.namprd13.prod.outlook.com>
Date:   Wed, 3 Nov 2021 07:57:46 +0000
From:   Baowen Zheng <baowen.zheng@...igine.com>
To:     Simon Horman <simon.horman@...igine.com>,
        Vlad Buslov <vladbu@...dia.com>,
        Jamal Hadi Salim <jhs@...atatu.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>
Subject: RE: [RFC/PATCH net-next v3 8/8] flow_offload: validate flags of
 filter and actions

On November 2, 2021 8:40 PM, Simon Horman wrote:
>On Mon, Nov 01, 2021 at 09:38:34AM +0200, Vlad Buslov wrote:
>> On Mon 01 Nov 2021 at 05:29, Baowen Zheng
><baowen.zheng@...igine.com> wrote:
>> > 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
>
>..
>
>> >>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.
>>
>> My suggestion was to forgo the skip_sw flag for shared action offload
>> and, consecutively, remove the validation code, not to add even more
>> checks. I still don't see a practical case where skip_sw shared action
>> is useful. But I don't have any strong feelings about this flag, so if
>> Jamal thinks it is necessary, then fine by me.
>
>FWIIW, my feelings are the same as Vlad's.
>
>I think these flags add complexity that would be nice to avoid.
>But if Jamal thinks its necessary, then including the flags implementation is
>fine by me.
Thanks Simon. Jamal, do you think it is necessary to keep the skip_sw flag for user to specify
the action should not run in software?