[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <9F71A296-D9DF-4C4F-844E-CB3DE01B3F34@chopps.org>
Date: Sat, 6 Apr 2024 08:36:09 -0400
From: Christian Hopps <chopps@...pps.org>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: Christian Hopps <chopps@...pps.org>,
Antony Antony <antony.antony@...unet.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
netdev@...r.kernel.org,
devel@...ux-ipsec.org,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [devel-ipsec] [PATCH ipsec-next v6] xfrm: Add Direction to the SA
in or out
> On Apr 5, 2024, at 17:56, Sabrina Dubroca via Devel <devel@...ux-ipsec.org> wrote:
>
> Hi Antony,
>
> 2024-04-05, 14:40:07 +0200, Antony Antony wrote:
>> This patch introduces the 'dir' attribute, 'in' or 'out', to the
>> xfrm_state, SA, enhancing usability by delineating the scope of values
>> based on direction. An input SA will now exclusively encompass values
>> pertinent to input, effectively segregating them from output-related
>> values.
>
> But this patch isn't doing that for existing properties (I'm thinking
> of replay window, not sure if any others are relevant [1]). Why not?
>
> [1] that should include values passed via xfrm_usersa_info too,
> not just XFRMA_* attributes
>
> Adding these checks should be safe (wrt breakage of API): Old software
> would not be passing XFRMA_SA_DIR, so adding checks when it is provided
> would not break anything there. Only new software using the attribute
> would benefit from having directed SAs and restriction on which attributes
> can be used (and that's fine).
>
> Right now the new attribute is 100% duplicate of the existing offload
> direction, so I don't see much point.
A subset of the ipsec has requested that IPTFS utilize this new directional attribute as most of IPTFS config is send-only. So IPTFS is the planned first user and is also now blocked on this patch.
Chris.
>
>> This change aims to streamline the configuration process and
>> improve the overall clarity of SA attributes.
>>
>> This feature sets the groundwork for future patches, including
>> the upcoming IP-TFS patch.
>>
>> Currently, dir is only allowed when HW OFFLOAD is set.
>>
>> ---
>
> BTW, everything after this '---' will get cut, including your sign-off.
>
>> v5->v6:
>> - XFRMA_SA_DIR only allowed with HW OFFLOAD
>>
>> v4->v5:
>> - add details to commit message
>>
>> v3->v4:
>> - improve HW OFFLOAD DIR check check other direction
>>
>> v2->v3:
>> - delete redundant XFRM_SA_DIR_USE
>> - use u8 for "dir"
>> - fix HW OFFLOAD DIR check
>>
>> v1->v2:
>> - use .strict_start_type in struct nla_policy xfrma_policy
>> - delete redundant XFRM_SA_DIR_MAX enum
>> ---
>>
>> Signed-off-by: Antony Antony <antony.antony@...unet.com>
>> Reviewed-by: Leon Romanovsky <leonro@...dia.com>
>
> nit: If I'm making non-trivial changes to the contents of the patch, I
> typically drop the review (and test) tags I got on previous versions,
> since they may no longer apply.
>
> --
> Sabrina
>
> --
> Devel mailing list
> Devel@...ux-ipsec.org
> https://linux-ipsec.org/mailman/listinfo/devel
>
Powered by blists - more mailing lists