Skip to content

Commit

Permalink
Merge branch 'net-macsec-remove-the-preparation-phase-when-offloading…
Browse files Browse the repository at this point in the history 
…-operations'

Antoine Tenart says:

====================
net: macsec: remove the preparation phase when offloading operations

It was reported[1] the 2-step phase offloading of MACsec operations did
not fit well and device drivers were mostly ignoring the first phase
(preparation). In addition the s/w fallback in case h/w rejected an
operation, which could have taken advantage of this design, never was
implemented and it's probably not a good idea anyway (at least
unconditionnally). So let's remove this logic which only makes the code
more complex for no advantage, before there are too many drivers
providing MACsec offloading.

This series removes the first phase (preparation) of the MACsec h/w
offloading. The modifications are split per-driver and in a way that
makes bissection working with logical steps; but I can squash some
patches if needed.

This was tested on the MSCC PHY but not on the Altantic nor mlx5e NICs.

[1] https://lore.kernel.org/all/166322893264.61080.12133865599607623050@kwain/T/
====================

Link: https://lore.kernel.org/r/20220921135118.968595-1-atenart@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Sep 23, 2022
2 parents aacdecd + 99383f1 commit f416bdf
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 183 deletions.
57 changes: 0 additions & 57 deletions drivers/net/ethernet/aquantia/atlantic/aq_macsec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,6 @@ static int aq_mdo_dev_open(struct macsec_context *ctx)
struct aq_nic_s *nic = netdev_priv(ctx->netdev);
int ret = 0;

if (ctx->prepare)
return 0;

if (netif_carrier_ok(nic->ndev))
ret = aq_apply_secy_cfg(nic, ctx->secy);

Expand All @@ -306,9 +303,6 @@ static int aq_mdo_dev_stop(struct macsec_context *ctx)
struct aq_nic_s *nic = netdev_priv(ctx->netdev);
int i;

if (ctx->prepare)
return 0;

for (i = 0; i < AQ_MACSEC_MAX_SC; i++) {
if (nic->macsec_cfg->txsc_idx_busy & BIT(i))
aq_clear_secy(nic, nic->macsec_cfg->aq_txsc[i].sw_secy,
Expand Down Expand Up @@ -466,9 +460,6 @@ static int aq_mdo_add_secy(struct macsec_context *ctx)
if (txsc_idx == AQ_MACSEC_MAX_SC)
return -ENOSPC;

if (ctx->prepare)
return 0;

cfg->sc_sa = sc_sa;
cfg->aq_txsc[txsc_idx].hw_sc_idx = aq_to_hw_sc_idx(txsc_idx, sc_sa);
cfg->aq_txsc[txsc_idx].sw_secy = secy;
Expand All @@ -492,9 +483,6 @@ static int aq_mdo_upd_secy(struct macsec_context *ctx)
if (txsc_idx < 0)
return -ENOENT;

if (ctx->prepare)
return 0;

if (netif_carrier_ok(nic->ndev) && netif_running(secy->netdev))
ret = aq_set_txsc(nic, txsc_idx);

Expand Down Expand Up @@ -543,9 +531,6 @@ static int aq_mdo_del_secy(struct macsec_context *ctx)
struct aq_nic_s *nic = netdev_priv(ctx->netdev);
int ret = 0;

if (ctx->prepare)
return 0;

if (!nic->macsec_cfg)
return 0;

Expand Down Expand Up @@ -601,9 +586,6 @@ static int aq_mdo_add_txsa(struct macsec_context *ctx)
if (txsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

aq_txsc = &cfg->aq_txsc[txsc_idx];
set_bit(ctx->sa.assoc_num, &aq_txsc->tx_sa_idx_busy);

Expand Down Expand Up @@ -631,9 +613,6 @@ static int aq_mdo_upd_txsa(struct macsec_context *ctx)
if (txsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

aq_txsc = &cfg->aq_txsc[txsc_idx];
if (netif_carrier_ok(nic->ndev) && netif_running(secy->netdev))
ret = aq_update_txsa(nic, aq_txsc->hw_sc_idx, secy,
Expand Down Expand Up @@ -681,9 +660,6 @@ static int aq_mdo_del_txsa(struct macsec_context *ctx)
if (txsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

ret = aq_clear_txsa(nic, &cfg->aq_txsc[txsc_idx], ctx->sa.assoc_num,
AQ_CLEAR_ALL);

Expand Down Expand Up @@ -780,9 +756,6 @@ static int aq_mdo_add_rxsc(struct macsec_context *ctx)
if (rxsc_idx >= rxsc_idx_max)
return -ENOSPC;

if (ctx->prepare)
return 0;

cfg->aq_rxsc[rxsc_idx].hw_sc_idx = aq_to_hw_sc_idx(rxsc_idx,
cfg->sc_sa);
cfg->aq_rxsc[rxsc_idx].sw_secy = ctx->secy;
Expand All @@ -809,9 +782,6 @@ static int aq_mdo_upd_rxsc(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -ENOENT;

if (ctx->prepare)
return 0;

if (netif_carrier_ok(nic->ndev) && netif_running(ctx->secy->netdev))
ret = aq_set_rxsc(nic, rxsc_idx);

Expand Down Expand Up @@ -876,9 +846,6 @@ static int aq_mdo_del_rxsc(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -ENOENT;

if (ctx->prepare)
return 0;

if (netif_carrier_ok(nic->ndev))
clear_type = AQ_CLEAR_ALL;

Expand Down Expand Up @@ -948,9 +915,6 @@ static int aq_mdo_add_rxsa(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

aq_rxsc = &nic->macsec_cfg->aq_rxsc[rxsc_idx];
set_bit(ctx->sa.assoc_num, &aq_rxsc->rx_sa_idx_busy);

Expand Down Expand Up @@ -978,9 +942,6 @@ static int aq_mdo_upd_rxsa(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

if (netif_carrier_ok(nic->ndev) && netif_running(secy->netdev))
ret = aq_update_rxsa(nic, cfg->aq_rxsc[rxsc_idx].hw_sc_idx,
secy, ctx->sa.rx_sa, NULL,
Expand Down Expand Up @@ -1029,9 +990,6 @@ static int aq_mdo_del_rxsa(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

ret = aq_clear_rxsa(nic, &cfg->aq_rxsc[rxsc_idx], ctx->sa.assoc_num,
AQ_CLEAR_ALL);

Expand All @@ -1044,9 +1002,6 @@ static int aq_mdo_get_dev_stats(struct macsec_context *ctx)
struct aq_macsec_common_stats *stats = &nic->macsec_cfg->stats;
struct aq_hw_s *hw = nic->aq_hw;

if (ctx->prepare)
return 0;

aq_get_macsec_common_stats(hw, stats);

ctx->stats.dev_stats->OutPktsUntagged = stats->out.untagged_pkts;
Expand All @@ -1073,9 +1028,6 @@ static int aq_mdo_get_tx_sc_stats(struct macsec_context *ctx)
if (txsc_idx < 0)
return -ENOENT;

if (ctx->prepare)
return 0;

aq_txsc = &nic->macsec_cfg->aq_txsc[txsc_idx];
stats = &aq_txsc->stats;
aq_get_txsc_stats(hw, aq_txsc->hw_sc_idx, stats);
Expand Down Expand Up @@ -1106,9 +1058,6 @@ static int aq_mdo_get_tx_sa_stats(struct macsec_context *ctx)
if (txsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

aq_txsc = &cfg->aq_txsc[txsc_idx];
sa_idx = aq_txsc->hw_sc_idx | ctx->sa.assoc_num;
stats = &aq_txsc->tx_sa_stats[ctx->sa.assoc_num];
Expand Down Expand Up @@ -1147,9 +1096,6 @@ static int aq_mdo_get_rx_sc_stats(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -ENOENT;

if (ctx->prepare)
return 0;

aq_rxsc = &cfg->aq_rxsc[rxsc_idx];
for (i = 0; i < MACSEC_NUM_AN; i++) {
if (!test_bit(i, &aq_rxsc->rx_sa_idx_busy))
Expand Down Expand Up @@ -1196,9 +1142,6 @@ static int aq_mdo_get_rx_sa_stats(struct macsec_context *ctx)
if (rxsc_idx < 0)
return -EINVAL;

if (ctx->prepare)
return 0;

aq_rxsc = &cfg->aq_rxsc[rxsc_idx];
stats = &aq_rxsc->rx_sa_stats[ctx->sa.assoc_num];
sa_idx = aq_rxsc->hw_sc_idx | ctx->sa.assoc_num;
Expand Down
36 changes: 0 additions & 36 deletions drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,6 @@ static int mlx5e_macsec_add_txsa(struct macsec_context *ctx)
struct mlx5e_macsec *macsec;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -595,9 +592,6 @@ static int mlx5e_macsec_upd_txsa(struct macsec_context *ctx)
struct net_device *netdev;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -658,9 +652,6 @@ static int mlx5e_macsec_del_txsa(struct macsec_context *ctx)
struct mlx5e_macsec *macsec;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);
macsec = priv->macsec;
macsec_device = mlx5e_macsec_get_macsec_device_context(macsec, ctx);
Expand Down Expand Up @@ -713,9 +704,6 @@ static int mlx5e_macsec_add_rxsc(struct macsec_context *ctx)
struct mlx5e_macsec *macsec;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);
macsec = priv->macsec;
macsec_device = mlx5e_macsec_get_macsec_device_context(macsec, ctx);
Expand Down Expand Up @@ -793,9 +781,6 @@ static int mlx5e_macsec_upd_rxsc(struct macsec_context *ctx)
int i;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -844,9 +829,6 @@ static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
int err = 0;
int i;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -912,9 +894,6 @@ static int mlx5e_macsec_add_rxsa(struct macsec_context *ctx)
struct list_head *list;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -999,9 +978,6 @@ static int mlx5e_macsec_upd_rxsa(struct macsec_context *ctx)
struct list_head *list;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -1058,9 +1034,6 @@ static int mlx5e_macsec_del_rxsa(struct macsec_context *ctx)
struct list_head *list;
int err = 0;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);

macsec = priv->macsec;
Expand Down Expand Up @@ -1110,9 +1083,6 @@ static int mlx5e_macsec_add_secy(struct macsec_context *ctx)
struct mlx5e_macsec *macsec;
int err = 0;

if (ctx->prepare)
return 0;

if (!mlx5e_macsec_secy_features_validate(ctx))
return -EINVAL;

Expand Down Expand Up @@ -1213,9 +1183,6 @@ static int mlx5e_macsec_upd_secy(struct macsec_context *ctx)
struct mlx5e_macsec *macsec;
int i, err = 0;

if (ctx->prepare)
return 0;

if (!mlx5e_macsec_secy_features_validate(ctx))
return -EINVAL;

Expand Down Expand Up @@ -1274,9 +1241,6 @@ static int mlx5e_macsec_del_secy(struct macsec_context *ctx)
int err = 0;
int i;

if (ctx->prepare)
return 0;

mutex_lock(&priv->macsec->lock);
macsec = priv->macsec;
macsec_device = mlx5e_macsec_get_macsec_device_context(macsec, ctx);
Expand Down
14 changes: 0 additions & 14 deletions drivers/net/macsec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1663,22 +1663,8 @@ static int macsec_offload(int (* const func)(struct macsec_context *),
if (ctx->offload == MACSEC_OFFLOAD_PHY)
mutex_lock(&ctx->phydev->lock);

/* Phase I: prepare. The drive should fail here if there are going to be
* issues in the commit phase.
*/
ctx->prepare = true;
ret = (*func)(ctx);
if (ret)
goto phy_unlock;

/* Phase II: commit. This step cannot fail. */
ctx->prepare = false;
ret = (*func)(ctx);
/* This should never happen: commit is not allowed to fail */
if (unlikely(ret))
WARN(1, "MACsec offloading commit failed (%d)\n", ret);

phy_unlock:
if (ctx->offload == MACSEC_OFFLOAD_PHY)
mutex_unlock(&ctx->phydev->lock);

Expand Down
Loading

0 comments on commit f416bdf

Please sign in to comment.