netdev - Re: [PATCH net-next v1 1/7] net: flow_offload: provision conntrack info in ct

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: <875yda4dfc.fsf@nvidia.com>
Date:   Fri, 13 Jan 2023 18:15:55 +0200
From:   Vlad Buslov <vladbu@...dia.com>
To:     Simon Horman <simon.horman@...igine.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <pablo@...filter.org>, <netdev@...r.kernel.org>,
        <netfilter-devel@...r.kernel.org>, <jhs@...atatu.com>,
        <xiyou.wangcong@...il.com>, <jiri@...nulli.us>, <ozsh@...dia.com>,
        <marcelo.leitner@...il.com>,
        Baowen Zheng <baowen.zheng@...igine.com>,
        <oss-drivers@...igine.com>
Subject: Re: [PATCH net-next v1 1/7] net: flow_offload: provision conntrack
 info in ct_metadata

On Fri 13 Jan 2023 at 10:12, Simon Horman <simon.horman@...igine.com> wrote:
> + Baowen Zheng, oss-drivers@...igine.com
>
> On Tue, Jan 10, 2023 at 02:30:17PM +0100, Vlad Buslov wrote:
>> In order to offload connections in other states besides "established" the
>> driver offload callbacks need to have access to connection conntrack info.
>> Extend flow offload intermediate representation data structure
>> flow_action_entry->ct_metadata with new enum ip_conntrack_info field and
>> fill it in tcf_ct_flow_table_add_action_meta() callback.
>> 
>> Reject offloading IP_CT_NEW connections for now by returning an error in
>> relevant driver callbacks based on value of ctinfo. Support for offloading
>> such connections will need to be added to the drivers afterwards.
>> 
>> Signed-off-by: Vlad Buslov <vladbu@...dia.com>
>> ---
>
> ...
>
>> diff --git a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
>> index f693119541d5..2c550a1792b7 100644
>> --- a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
>> +++ b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
>> @@ -1964,6 +1964,23 @@ int nfp_fl_ct_stats(struct flow_cls_offload *flow,
>>  	return 0;
>>  }
>>  
>> +static bool
>> +nfp_fl_ct_offload_supported(struct flow_cls_offload *flow)
>> +{
>> +	struct flow_rule *flow_rule = flow->rule;
>> +	struct flow_action *flow_action =
>> +		&flow_rule->action;
>> +	struct flow_action_entry *act;
>> +	int i;
>> +
>> +	flow_action_for_each(i, act, flow_action) {
>> +		if (act->id == FLOW_ACTION_CT_METADATA)
>> +			return act->ct_metadata.ctinfo != IP_CT_NEW;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>
> Hi Vlad,
>
> Some feedback from Baowen Zheng, who asked me to pass it on here:
>
>   It is confusing that after FLOW_ACTION_CT_METADATA check, this functoin
>   will return false, that is -EOPNOTSUPP.
>
>   Since this function is only used to check nft table, It seems better to
>   change its name to nfp_fl_ct_offload_nft_supported(). This would make things
>   clearer and may avoid it being used in the wrong way.

Thanks for the suggestions! I will change the naming and send V2.