netdev - Re: [PATCH iproute2-next v2] tc: add skip_hw and skip

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]

Date:   Wed, 2 Feb 2022 13:15:54 +0200
From:   Roi Dayan <roid@...dia.com>
To:     Baowen Zheng <baowen.zheng@...igine.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Victor Nogueira <victor@...atatu.com>
CC:     David Ahern <dsahern@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        oss-drivers <oss-drivers@...igine.com>,
        "Simon Horman" <simon.horman@...igine.com>
Subject: Re: [PATCH iproute2-next v2] tc: add skip_hw and skip_sw to control
 action offload



On 2022-02-02 11:37 AM, Baowen Zheng wrote:
> Hi Roi:
> Thanks for bring this to us, please see the inline comments.
> 
>> On 2022-02-02 10:39 AM, Roi Dayan wrote:
>>>
>>>
>>> On 2022-01-31 9:40 PM, Jamal Hadi Salim wrote:
>>>> On 2022-01-26 08:41, Victor Nogueira wrote:
>>>>> On Wed, Jan 26, 2022 at 3:55 AM Baowen Zheng
>>>>> <baowen.zheng@...igine.com> wrote:
>>>>>>
>>>>>> Add skip_hw and skip_sw flags for user to control whether offload
>>>>>> action to hardware.
>>>>>>
>>>>>> Also we add hw_count to show how many hardwares accept to offload
>>>>>> the action.
>>>>>>
>>>>>> Change man page to describe the usage of skip_sw and skip_hw flag.
>>>>>>
>>>>>> An example to add and query action as below.
>>>>>>
>>>>>> $ tc actions add action police rate 1mbit burst 100k index 100
>>>>>> skip_sw
>>>>>>
>>>>>> $ tc -s -d actions list action police total acts 1
>>>>>>       action order 0:  police 0x64 rate 1Mbit burst 100Kb mtu 2Kb
>>>>>> action reclassify overhead 0b linklayer ethernet
>>>>>>       ref 1 bind 0  installed 2 sec used 2 sec
>>>>>>       Action statistics:
>>>>>>       Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>>>>>>       backlog 0b 0p requeues 0
>>>>>>       skip_sw in_hw in_hw_count 1
>>>>>>       used_hw_stats delayed
>>>>>>
>>>>>> Signed-off-by: baowen zheng <baowen.zheng@...igine.com>
>>>>>> Signed-off-by: Simon Horman <simon.horman@...igine.com>
>>>>>
>>>>> I applied this version, tested it and can confirm the breakage in
>>>>> tdc is gone.
>>>>> Tested-by: Victor Nogueira <victor@...atatu.com>
>>>>
>>>> Acked-by: Jamal Hadi Salim <jhs@...atatu.com>
>>>>
>>>> cheers,
>>>> jamal
>>>
>>>
>>> Hi Sorry for not catching this early enough but I see an issue now
>>> with this patch. adding an offload tc rule and dumping it shows
>>> actions not_in_hw.
>>>
>>> example rule in_hw and action marked as not_in_hw
>>>
>>> filter parent ffff: protocol arp pref 8 flower chain 0 handle 0x1
>>> dst_mac e4:11:22:11:4a:51 src_mac e4:11:22:11:4a:50
>>>     eth_type arp
>>>     in_hw in_hw_count 1
>>>           action order 1: gact action drop
>>>            random type none pass val 0
>>>            index 2 ref 1 bind 1
>>>           not_in_hw
>>>           used_hw_stats delayed
>>>
>>>
>>> so the action was not created/offloaded outside the filter but it is
>>> acting as offloaded.
> Hi Roi, the flag in_hw and not_in_hw in action section describes if the action is offloaded as an action independent of any filter. So the actions created along with the filter will be marked with not_in_hw.
> This is to be compatible with what we do in Linux upstream 8cbfe93 ("flow_offload: allow user to offload tc action to net device").
> 

I understand it's for the actions offload but there is not confusing
output between if actions were created explicitly or by the filter.

In the example above the action is "offloaded". the matching and action
are both done in hw.
maybe if action is created by the filter should not dump in_hw/not_in_hw
flags at all.

>>>
>>> also shouldn't the indent be more 1 space in like random/index to note
>>> it's part of the action order 1.
>  From my environment, I did not find this indent issue, I will make more check to verify.

its ok. i saw the indents from different commit and got fixed, I needed
to refetch. i dont see the indent issues now.

>>>
>>> Thanks,
>>> Roi
>>>
>>
>> also, not tested. what is printed if match is not supported but uses offloaded
>> action?
> If match is not supported but uses offloaded action, the match will be marked as not_in_hw and the action will be marked as in_hw since the action is offloaded independent from filter rule.
>>
>> it could print filter not_in_hw but action in_hw?