netdev - Re: [PATCH net-next 2/2] net: sched: add helper support in act

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 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]

Date:   Tue, 4 Oct 2022 14:33:50 +0200
From:   Ilya Maximets <i.maximets@....org>
To:     Xin Long <lucien.xin@...il.com>, Paolo Abeni <pabeni@...hat.com>
Cc:     i.maximets@....org, network dev <netdev@...r.kernel.org>,
        davem@...emloft.net, kuba@...nel.org,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Davide Caratti <dcaratti@...hat.com>,
        Oz Shlomo <ozsh@...dia.com>, Paul Blakey <paulb@...dia.com>,
        Eelco Chaudron <echaudro@...hat.com>
Subject: Re: [PATCH net-next 2/2] net: sched: add helper support in act_ct

On 9/27/22 17:04, Xin Long wrote:
> On Tue, Sep 27, 2022 at 6:29 AM Paolo Abeni <pabeni@...hat.com> wrote:
>>
>> On Fri, 2022-09-23 at 11:28 -0400, Xin Long wrote:
>>> This patch is to add helper support in act_ct for OVS actions=ct(alg=xxx)
>>> offloading, which is corresponding to Commit cae3a2627520 ("openvswitch:
>>> Allow attaching helpers to ct action") in OVS kernel part.
>>>
>>> The difference is when adding TC actions family and proto cannot be got
>>> from the filter/match, other than helper name in tb[TCA_CT_HELPER_NAME],
>>> we also need to send the family in tb[TCA_CT_HELPER_FAMILY] and the
>>> proto in tb[TCA_CT_HELPER_PROTO] to kernel.
>>>
>>> Note when calling helper->help() in tcf_ct_act(), the packet will be
>>> dropped if skb's family and proto do not match the helper's.
>>>
>>> Reported-by: Ilya Maximets <i.maximets@....org>
>>
>> This tag is a bit out of place here, as it should belong to fixes. Do
>> you mean 'Suggested-by' ?
> This one was reported as an OVS bug, but from TC side, it's a feature.

My 2c:
- The fact that act_ct doesn't execute helpers attached to skb outside
  of TC (in OVS) can be considered as a bug.
- The ability to set helpers in act_ct itself is indeed a new feature.

Though it was decided to implement both things at the same time to
avoid confusion around what is supported and what is not supported,
especially since there will be no meaningful way to detect if the bug
actually fixed in the kernel or not.

CC: Eelco.

P.S. might also make sense to CC: ovs-dev on a next revision for visibility.

Best regards, Ilya Maximets.