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.
Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
---
Changes in v5:
- none
Changes in v4:
- patch added in v4
drivers/net/macsec.c | 2 ++
include/net/macsec.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index b7e151439c48..c5cd4551c67c 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2383,6 +2383,7 @@ static int macsec_upd_txsa(struct sk_buff *skb, struct genl_info *info)
ctx.sa.assoc_num = assoc_num;
ctx.sa.tx_sa = tx_sa;
+ ctx.sa.update_pn = !!prev_pn.full64;
ctx.secy = secy;
ret = macsec_offload(ops->mdo_upd_txsa, &ctx);
@@ -2476,6 +2477,7 @@ static int macsec_upd_rxsa(struct sk_buff *skb, struct genl_info *info)
ctx.sa.assoc_num = assoc_num;
ctx.sa.rx_sa = rx_sa;
+ ctx.sa.update_pn = !!prev_pn.full64;
ctx.secy = secy;
ret = macsec_offload(ops->mdo_upd_rxsa, &ctx);
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
* @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.12023-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
On 21.09.2023 18:11, Sabrina Dubroca wrote:
> 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).
I updated octeontx2, mssc and mlx5. Atlantic is unclear.
Mark, Igor, in the atlantic MACsec driver, can the SAs be updated
without a PN update?
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c#L1641
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c#L678
>
>> 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
>>
>
--
Radu P.
Hi guys, > On 21.09.2023 18:11, Sabrina Dubroca wrote: >> 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 Thats correct, atlantic hardware requires full table to be in data buffer registers. I really doubt its possible to skip PN writing. >> 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). > > I updated octeontx2, mssc and mlx5. Atlantic is unclear. > > Mark, Igor, in the atlantic MACsec driver, can the SAs be updated > without a PN update? Reviewed the code and the docs I have - my view is it can not. All the packed record in macsec_api.c:set_egress_sa_record is expected by hardware in full. Regards, Igor
On 26.09.2023 15:16, Igor Russkikh wrote: > Hi guys, > >> On 21.09.2023 18:11, Sabrina Dubroca wrote: >>> 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 > > Thats correct, atlantic hardware requires full table to be in data buffer registers. > I really doubt its possible to skip PN writing. > >>> 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). >> >> I updated octeontx2, mssc and mlx5. Atlantic is unclear. >> >> Mark, Igor, in the atlantic MACsec driver, can the SAs be updated >> without a PN update? > > Reviewed the code and the docs I have - my view is it can not. > All the packed record in macsec_api.c:set_egress_sa_record is expected by hardware in full. Ok. I will not touch atlantic. -- Radu P.
© 2016 - 2026 Red Hat, Inc.