Skip to content

L
linux
Commit f416bdfb authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files
Options

Merge branch 'net-macsec-remove-the-preparation-phase-when-offloading-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.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents aacdecda 99383f12
Show whitespace changes
Inline Side-by-side
Showing with 39 additions and 183 deletions
drivers/net/ethernet/aquantia/atlantic/aq_macsec.c
View file @ f416bdfb
......@@ -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);
......@@ -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,
......@@ -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;
......@@ -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);
......@@ -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;
......@@ -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);
......@@ -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,
......@@ -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);
......@@ -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;
......@@ -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);
......@@ -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;
......@@ -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);
......@@ -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,
......@@ -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);
......@@ -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;
......@@ -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);
......@@ -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];
......@@ -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))
......@@ -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;
......
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
View file @ f416bdfb
......@@ -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;
......@@ -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;
......@@ -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);
......@@ -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);
......@@ -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;
......@@ -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;
......@@ -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;
......@@ -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;
......@@ -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;
......@@ -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;
......@@ -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;
......@@ -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);
......
drivers/net/macsec.c
View file @ f416bdfb
......@@ -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);
......
drivers/net/phy/mscc/mscc_macsec.c
View file @ f416bdfb
......@@ -706,14 +706,6 @@ static int __vsc8584_macsec_add_rxsa(struct macsec_context *ctx,
struct phy_device *phydev = ctx->phydev;
struct vsc8531_private *priv = phydev->priv;
if (!flow) {
flow = vsc8584_macsec_alloc_flow(priv, MACSEC_INGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
memcpy(flow->key, ctx->sa.key, priv->secy->key_len);
}
flow->assoc_num = ctx->sa.assoc_num;
flow->rx_sa = ctx->sa.rx_sa;
......@@ -730,24 +722,13 @@ static int __vsc8584_macsec_add_rxsa(struct macsec_context *ctx,
static int __vsc8584_macsec_add_txsa(struct macsec_context *ctx,
struct macsec_flow *flow, bool update)
{
struct phy_device *phydev = ctx->phydev;
struct vsc8531_private *priv = phydev->priv;
if (!flow) {
flow = vsc8584_macsec_alloc_flow(priv, MACSEC_EGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
memcpy(flow->key, ctx->sa.key, priv->secy->key_len);
}
flow->assoc_num = ctx->sa.assoc_num;
flow->tx_sa = ctx->sa.tx_sa;
/* Always match untagged packets on egress */
flow->match.untagged = 1;
return vsc8584_macsec_add_flow(phydev, flow, update);
return vsc8584_macsec_add_flow(ctx->phydev, flow, update);
}
static int vsc8584_macsec_dev_open(struct macsec_context *ctx)
......@@ -755,10 +736,6 @@ static int vsc8584_macsec_dev_open(struct macsec_context *ctx)
struct vsc8531_private *priv = ctx->phydev->priv;
struct macsec_flow *flow, *tmp;
/* No operation to perform before the commit step */
if (ctx->prepare)
return 0;
list_for_each_entry_safe(flow, tmp, &priv->macsec_flows, list)
vsc8584_macsec_flow_enable(ctx->phydev, flow);
......@@ -770,10 +747,6 @@ static int vsc8584_macsec_dev_stop(struct macsec_context *ctx)
struct vsc8531_private *priv = ctx->phydev->priv;
struct macsec_flow *flow, *tmp;
/* No operation to perform before the commit step */
if (ctx->prepare)
return 0;
list_for_each_entry_safe(flow, tmp, &priv->macsec_flows, list)
vsc8584_macsec_flow_disable(ctx->phydev, flow);
......@@ -785,13 +758,9 @@ static int vsc8584_macsec_add_secy(struct macsec_context *ctx)
struct vsc8531_private *priv = ctx->phydev->priv;
struct macsec_secy *secy = ctx->secy;
if (ctx->prepare) {
if (priv->secy)
return -EEXIST;
return 0;
}
priv->secy = secy;
vsc8584_macsec_flow_default_action(ctx->phydev, MACSEC_EGR,
......@@ -807,10 +776,6 @@ static int vsc8584_macsec_del_secy(struct macsec_context *ctx)
struct vsc8531_private *priv = ctx->phydev->priv;
struct macsec_flow *flow, *tmp;
/* No operation to perform before the commit step */
if (ctx->prepare)
return 0;
list_for_each_entry_safe(flow, tmp, &priv->macsec_flows, list)
vsc8584_macsec_del_flow(ctx->phydev, flow);
......@@ -823,10 +788,6 @@ static int vsc8584_macsec_del_secy(struct macsec_context *ctx)
static int vsc8584_macsec_upd_secy(struct macsec_context *ctx)
{
/* No operation to perform before the commit step */
if (ctx->prepare)
return 0;
vsc8584_macsec_del_secy(ctx);
return vsc8584_macsec_add_secy(ctx);
}
......@@ -847,10 +808,6 @@ static int vsc8584_macsec_del_rxsc(struct macsec_context *ctx)
struct vsc8531_private *priv = ctx->phydev->priv;
struct macsec_flow *flow, *tmp;
/* No operation to perform before the commit step */
if (ctx->prepare)
return 0;
list_for_each_entry_safe(flow, tmp, &priv->macsec_flows, list) {
if (flow->bank == MACSEC_INGR && flow->rx_sa &&
flow->rx_sa->sc->sci == ctx->rx_sc->sci)
......@@ -862,33 +819,40 @@ static int vsc8584_macsec_del_rxsc(struct macsec_context *ctx)
static int vsc8584_macsec_add_rxsa(struct macsec_context *ctx)
{
struct macsec_flow *flow = NULL;
if (ctx->prepare)
return __vsc8584_macsec_add_rxsa(ctx, flow, false);
struct phy_device *phydev = ctx->phydev;
struct vsc8531_private *priv = phydev->priv;
struct macsec_flow *flow;
int ret;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_INGR);
flow = vsc8584_macsec_alloc_flow(priv, MACSEC_INGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
vsc8584_macsec_flow_enable(ctx->phydev, flow);
memcpy(flow->key, ctx->sa.key, priv->secy->key_len);
ret = __vsc8584_macsec_add_rxsa(ctx, flow, false);
if (ret)
return ret;
vsc8584_macsec_flow_enable(phydev, flow);
return 0;
}
static int vsc8584_macsec_upd_rxsa(struct macsec_context *ctx)
{
struct macsec_flow *flow;
int ret;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_INGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
if (ctx->prepare) {
/* Make sure the flow is disabled before updating it */
vsc8584_macsec_flow_disable(ctx->phydev, flow);
return __vsc8584_macsec_add_rxsa(ctx, flow, true);
}
ret = __vsc8584_macsec_add_rxsa(ctx, flow, true);
if (ret)
return ret;
vsc8584_macsec_flow_enable(ctx->phydev, flow);
return 0;
......@@ -899,11 +863,8 @@ static int vsc8584_macsec_del_rxsa(struct macsec_context *ctx)
struct macsec_flow *flow;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_INGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
if (ctx->prepare)
return 0;
vsc8584_macsec_del_flow(ctx->phydev, flow);
return 0;
......@@ -911,33 +872,40 @@ static int vsc8584_macsec_del_rxsa(struct macsec_context *ctx)
static int vsc8584_macsec_add_txsa(struct macsec_context *ctx)
{
struct macsec_flow *flow = NULL;
if (ctx->prepare)
return __vsc8584_macsec_add_txsa(ctx, flow, false);
struct phy_device *phydev = ctx->phydev;
struct vsc8531_private *priv = phydev->priv;
struct macsec_flow *flow;
int ret;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_EGR);
flow = vsc8584_macsec_alloc_flow(priv, MACSEC_EGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
vsc8584_macsec_flow_enable(ctx->phydev, flow);
memcpy(flow->key, ctx->sa.key, priv->secy->key_len);
ret = __vsc8584_macsec_add_txsa(ctx, flow, false);
if (ret)
return ret;
vsc8584_macsec_flow_enable(phydev, flow);
return 0;
}
static int vsc8584_macsec_upd_txsa(struct macsec_context *ctx)
{
struct macsec_flow *flow;
int ret;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_EGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
if (ctx->prepare) {
/* Make sure the flow is disabled before updating it */
vsc8584_macsec_flow_disable(ctx->phydev, flow);
return __vsc8584_macsec_add_txsa(ctx, flow, true);
}
ret = __vsc8584_macsec_add_txsa(ctx, flow, true);
if (ret)
return ret;
vsc8584_macsec_flow_enable(ctx->phydev, flow);
return 0;
......@@ -948,11 +916,8 @@ static int vsc8584_macsec_del_txsa(struct macsec_context *ctx)
struct macsec_flow *flow;
flow = vsc8584_macsec_find_flow(ctx, MACSEC_EGR);
if (IS_ERR(flow))
return PTR_ERR(flow);
if (ctx->prepare)
return 0;
vsc8584_macsec_del_flow(ctx->phydev, flow);
return 0;
......
include/net/macsec.h
View file @ f416bdfb
......@@ -271,8 +271,6 @@ struct macsec_context {
struct macsec_rx_sa_stats *rx_sa_stats;
struct macsec_dev_stats *dev_stats;
} stats;
u8 prepare:1;
};
/**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7