Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i494840e7:Fastmail
Date:   Tue, 25 Jul 2023 13:12:46 +0300
From:   Ido Schimmel <idosch@idosch.org>
To:     Ratheesh Kannoth <rkannoth@marvell.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com
Subject: Re: [PATCH net-next] flow_dissector: Add IPSEC dissectors
Message-ID: <ZL+gHkLnLE91YBkS@shredder>
References: <20230725032451.505189-1-rkannoth@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230725032451.505189-1-rkannoth@marvell.com>
Precedence: bulk

On Tue, Jul 25, 2023 at 08:54:51AM +0530, Ratheesh Kannoth wrote:
>  Support for dissecting IPSEC field SPI (which is
>  32bits in size) for ESP and AH packets.
> 
>  This implementation does not support NAT-T
>  (encapsulation of ESP packets over UDP).
> 
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>  include/net/flow_dissector.h |  9 ++++++
>  include/net/flow_offload.h   |  6 ++++
>  include/uapi/linux/pkt_cls.h |  3 ++
>  net/core/flow_dissector.c    | 53 +++++++++++++++++++++++++++++++++++-
>  net/core/flow_offload.c      |  7 +++++
>  net/sched/cls_flower.c       | 18 ++++++++++++

Please split flow dissector and flower changes into separate patches.
Also, you can't add the flow offload bits without a corresponding driver
change. Nobody calls the exported flow_rule_match_ipsec() function.

[...]

> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 7865f5a9885b..a90b0e3d351f 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -594,6 +594,9 @@ enum {
>  
>  	TCA_FLOWER_KEY_L2TPV3_SID,	/* be32 */
>  
> +	TCA_FLOWER_KEY_SPI,		/* be32 */
> +	TCA_FLOWER_KEY_SPI_MASK,	/* be32 */
> +

This will break existing user space on new kernels. New attributes must
be added at the end.

>  	TCA_FLOWER_L2_MISS,		/* u8 */
>  
>  	TCA_FLOWER_KEY_CFM,		/* nested */