Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Thu, 21 Sep 2023 17:11:41 +0200
From:   Sabrina Dubroca <sd@queasysnail.net>
To:     "Radu Pirea (NXP OSS)" <radu-nicolae.pirea@oss.nxp.com>
Cc:     andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, richardcochran@gmail.com,
        sebastian.tobuschat@oss.nxp.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5 3/7] net: macsec: indicate next pn update
 when offloading
Message-ID: <ZQxdLZJa0EpnxpCl@hog>
References: <20230920092237.121033-1-radu-nicolae.pirea@oss.nxp.com>
 <20230920092237.121033-4-radu-nicolae.pirea@oss.nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230920092237.121033-4-radu-nicolae.pirea@oss.nxp.com>
Precedence: bulk

2023-09-20, 12:22:33 +0300, Radu Pirea (NXP OSS) wrote:
> Indicate next PN update using update_pn flag in macsec_context.
> Offloaded MACsec implementations does not know whether or not the
> MACSEC_SA_ATTR_PN attribute was passed for an SA update and assume
> that next PN should always updated, but this is not always true.

This should probably go through net so that we can fix some drivers
that are currently doing the wrong thing. octeontx2 should be
fixable. atlantic looks like it would reset the PN to whatever was
read during the last dump, and it's unclear if that can be fixed
(AFAIU set_egress_sa_record writes the whole config at once).  mscc
doesn't seem to modify the PN (even if requested -- should it should
reject the update), and mlx5 doesn't allow PN update (by storing the
initial value of next_pn on SA creation).

> diff --git a/include/net/macsec.h b/include/net/macsec.h
> index ecae5eeb021a..42072fdcc183 100644
> --- a/include/net/macsec.h
> +++ b/include/net/macsec.h
> @@ -254,6 +254,7 @@ struct macsec_secy {
>   * @offload: MACsec offload status
>   * @secy: pointer to a MACsec SecY
>   * @rx_sc: pointer to a RX SC
> + * @update_pn: this flag indicates updating the next PN when updating the SA

nit: "this flag indicates" is not very useful, thus:

@update_pn: when updating the SA, update the next PN

>   * @assoc_num: association number of the target SA
>   * @key: key of the target SA
>   * @rx_sa: pointer to an RX SA if a RX SA is added/updated/removed
> @@ -274,6 +275,7 @@ struct macsec_context {
>  	struct macsec_secy *secy;
>  	struct macsec_rx_sc *rx_sc;
>  	struct {
> +		bool update_pn;
>  		unsigned char assoc_num;
>  		u8 key[MACSEC_MAX_KEY_LEN];
>  		union {
> -- 
> 2.34.1
> 

-- 
Sabrina