From 29084ea5d0e806abb02a69e18bae3d562a9202a5 Mon Sep 17 00:00:00 2001 From: Sabrina Dubroca Date: Thu, 7 Nov 2024 00:13:33 +0100 Subject: [PATCH 01/16] selftests: netdevsim: add test toggling macsec offload The test verifies that toggling offload works (both via rtnetlink and macsec's genetlink APIs). This is only possible when no SA is configured. Signed-off-by: Sabrina Dubroca Reviewed-by: Simon Horman Link: https://patch.msgid.link/bf8e27ee0d921caa4eb35f1e830eca6d4080ddb2.1730929545.git.sd@queasysnail.net Signed-off-by: Jakub Kicinski --- .../drivers/net/netdevsim/macsec-offload.sh | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh index 7babcfd76b22..1f2775846ea0 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh @@ -48,6 +48,27 @@ check $? ip macsec add "${MACSEC_NETDEV}" rx port 1235 address "1c:ed:de:ad:be:ef" 2> /dev/null check $? '' '' 1 +# can't disable macsec offload when SAs are configured +ip link set "${MACSEC_NETDEV}" type macsec offload off 2> /dev/null +check $? '' '' 1 + +ip macsec offload "${MACSEC_NETDEV}" off 2> /dev/null +check $? '' '' 1 + +# toggle macsec offload via rtnetlink +ip link set "${MACSEC_NETDEV}2" type macsec offload off +check $? + +ip link set "${MACSEC_NETDEV}2" type macsec offload mac +check $? + +# toggle macsec offload via genetlink +ip macsec offload "${MACSEC_NETDEV}2" off +check $? + +ip macsec offload "${MACSEC_NETDEV}2" mac +check $? + for dev in ${MACSEC_NETDEV}{,2,3} ; do ip link del $dev check $? -- 2.51.0 From 0f8800eb67ae9160d144d803f4f8d26ba6385213 Mon Sep 17 00:00:00 2001 From: Sabrina Dubroca Date: Thu, 7 Nov 2024 00:13:34 +0100 Subject: [PATCH 02/16] selftests: netdevsim: add ethtool features to macsec offload tests The test verifies that available features aren't changed by toggling offload on the device. Creating a device with offload off and then enabling it later should result in the same features as creating the device with offload enabled directly. Signed-off-by: Sabrina Dubroca Reviewed-by: Simon Horman Link: https://patch.msgid.link/ba801bd0a75b02de2dddbfc77f9efceb8b3d8a2e.1730929545.git.sd@queasysnail.net Signed-off-by: Jakub Kicinski --- .../drivers/net/netdevsim/macsec-offload.sh | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh index 1f2775846ea0..98033e6667d2 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh @@ -75,6 +75,39 @@ for dev in ${MACSEC_NETDEV}{,2,3} ; do done +# +# test ethtool features when toggling offload +# + +ip link add link $NSIM_NETDEV $MACSEC_NETDEV type macsec offload mac +TMP_FEATS_ON_1="$(ethtool -k $MACSEC_NETDEV)" + +ip link set $MACSEC_NETDEV type macsec offload off +TMP_FEATS_OFF_1="$(ethtool -k $MACSEC_NETDEV)" + +ip link set $MACSEC_NETDEV type macsec offload mac +TMP_FEATS_ON_2="$(ethtool -k $MACSEC_NETDEV)" + +[ "$TMP_FEATS_ON_1" = "$TMP_FEATS_ON_2" ] +check $? + +ip link del $MACSEC_NETDEV + +ip link add link $NSIM_NETDEV $MACSEC_NETDEV type macsec +check $? + +TMP_FEATS_OFF_2="$(ethtool -k $MACSEC_NETDEV)" +[ "$TMP_FEATS_OFF_1" = "$TMP_FEATS_OFF_2" ] +check $? + +ip link set $MACSEC_NETDEV type macsec offload mac +check $? + +TMP_FEATS_ON_3="$(ethtool -k $MACSEC_NETDEV)" +[ "$TMP_FEATS_ON_1" = "$TMP_FEATS_ON_3" ] +check $? + + if [ $num_errors -eq 0 ]; then echo "PASSED all $((num_passes)) checks" exit 0 -- 2.51.0 From b83db10996f5276f998bb5bb59da2fada560efbd Mon Sep 17 00:00:00 2001 From: Caleb Sander Mateos Date: Thu, 7 Nov 2024 11:30:51 -0700 Subject: [PATCH 03/16] mlx5/core: relax memory barrier in eq_update_ci() The memory barrier in eq_update_ci() after the doorbell write is a significant hot spot in mlx5_eq_comp_int(). Under heavy TCP load, we see 3% of CPU time spent on the mfence instruction. 98df6d5b877c ("net/mlx5: A write memory barrier is sufficient in EQ ci update") already relaxed the full memory barrier to just a write barrier in mlx5_eq_update_ci(), which duplicates eq_update_ci(). So replace mb() with wmb() in eq_update_ci() too. On strongly ordered architectures, no barrier is actually needed because the MMIO writes to the doorbell register are guaranteed to appear to the device in the order they were made. However, the kernel's ordered MMIO primitive writel() lacks a convenient big-endian interface. Therefore, we opt to stick with __raw_writel() + a barrier. Signed-off-by: Caleb Sander Mateos Reviewed-by: Parav Pandit Acked-by: Tariq Toukan Link: https://patch.msgid.link/20241107183054.2443218-1-csander@purestorage.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h index 4b7f7131c560..b1edc71ffc6d 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/eq.h @@ -72,7 +72,7 @@ static inline void eq_update_ci(struct mlx5_eq *eq, int arm) __raw_writel((__force u32)cpu_to_be32(val), addr); /* We still want ordering, just not swabbing, so add a barrier */ - mb(); + wmb(); } int mlx5_eq_table_init(struct mlx5_core_dev *dev); -- 2.51.0 From 619e4109e2588436327f23c04c54a9994b765636 Mon Sep 17 00:00:00 2001 From: Caleb Sander Mateos Date: Thu, 7 Nov 2024 11:30:52 -0700 Subject: [PATCH 04/16] mlx5/core: deduplicate {mlx5_,}eq_update_ci() The logic of eq_update_ci() is duplicated in mlx5_eq_update_ci(). The only additional work done by mlx5_eq_update_ci() is to increment eq->cons_index. Call eq_update_ci() from mlx5_eq_update_ci() to avoid the duplication. Signed-off-by: Caleb Sander Mateos Reviewed-by: Parav Pandit Acked-by: Tariq Toukan Link: https://patch.msgid.link/20241107183054.2443218-2-csander@purestorage.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mellanox/mlx5/core/eq.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c index 3fd2091c11c8..2b229b6226c6 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c @@ -801,15 +801,8 @@ EXPORT_SYMBOL(mlx5_eq_get_eqe); void mlx5_eq_update_ci(struct mlx5_eq *eq, u32 cc, bool arm) { - __be32 __iomem *addr = eq->doorbell + (arm ? 0 : 2); - u32 val; - eq->cons_index += cc; - val = (eq->cons_index & 0xffffff) | (eq->eqn << 24); - - __raw_writel((__force u32)cpu_to_be32(val), addr); - /* We still want ordering, just not swabbing, so add a barrier */ - wmb(); + eq_update_ci(eq, arm); } EXPORT_SYMBOL(mlx5_eq_update_ci); -- 2.51.0 From ca122473ebca0132b9563a98055f2f8d83e7bf59 Mon Sep 17 00:00:00 2001 From: Linu Cherian Date: Tue, 5 Nov 2024 18:26:18 +0530 Subject: [PATCH 05/16] octeontx2-af: Refactor few NPC mcam APIs Introduce lowlevel variant of rvu_mcam_remove/add_counter_from/to_rule for better code reuse, which assumes necessary locks are taken at higher level. These low level functions would be used for implementing default rule counter APIs in the subsequent patch. Signed-off-by: Linu Cherian Reviewed-by: Simon Horman Link: https://patch.msgid.link/20241105125620.2114301-2-lcherian@marvell.com Signed-off-by: Jakub Kicinski --- .../net/ethernet/marvell/octeontx2/af/rvu.h | 6 +- .../ethernet/marvell/octeontx2/af/rvu_npc.c | 89 ++++++++++++++++--- .../marvell/octeontx2/af/rvu_npc_fs.c | 36 ++------ 3 files changed, 92 insertions(+), 39 deletions(-) diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h index 5016ba82e142..d92a5f47a476 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h @@ -960,7 +960,11 @@ void rvu_npc_disable_default_entries(struct rvu *rvu, u16 pcifunc, int nixlf); void rvu_npc_enable_default_entries(struct rvu *rvu, u16 pcifunc, int nixlf); void rvu_npc_update_flowkey_alg_idx(struct rvu *rvu, u16 pcifunc, int nixlf, int group, int alg_idx, int mcam_index); - +void __rvu_mcam_remove_counter_from_rule(struct rvu *rvu, u16 pcifunc, + struct rvu_npc_mcam_rule *rule); +void __rvu_mcam_add_counter_to_rule(struct rvu *rvu, u16 pcifunc, + struct rvu_npc_mcam_rule *rule, + struct npc_install_flow_rsp *rsp); void rvu_npc_get_mcam_entry_alloc_info(struct rvu *rvu, u16 pcifunc, int blkaddr, int *alloc_cnt, int *enable_cnt); diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c index 97722ce8c4cb..c4ef1e83cc46 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c @@ -2975,9 +2975,9 @@ int rvu_mbox_handler_npc_mcam_shift_entry(struct rvu *rvu, return rc; } -int rvu_mbox_handler_npc_mcam_alloc_counter(struct rvu *rvu, - struct npc_mcam_alloc_counter_req *req, - struct npc_mcam_alloc_counter_rsp *rsp) +static int __npc_mcam_alloc_counter(struct rvu *rvu, + struct npc_mcam_alloc_counter_req *req, + struct npc_mcam_alloc_counter_rsp *rsp) { struct npc_mcam *mcam = &rvu->hw->mcam; u16 pcifunc = req->hdr.pcifunc; @@ -2998,11 +2998,9 @@ int rvu_mbox_handler_npc_mcam_alloc_counter(struct rvu *rvu, if (!req->contig && req->count > NPC_MAX_NONCONTIG_COUNTERS) return NPC_MCAM_INVALID_REQ; - mutex_lock(&mcam->lock); /* Check if unused counters are available or not */ if (!rvu_rsrc_free_count(&mcam->counters)) { - mutex_unlock(&mcam->lock); return NPC_MCAM_ALLOC_FAILED; } @@ -3035,12 +3033,27 @@ int rvu_mbox_handler_npc_mcam_alloc_counter(struct rvu *rvu, } } - mutex_unlock(&mcam->lock); return 0; } -int rvu_mbox_handler_npc_mcam_free_counter(struct rvu *rvu, - struct npc_mcam_oper_counter_req *req, struct msg_rsp *rsp) +int rvu_mbox_handler_npc_mcam_alloc_counter(struct rvu *rvu, + struct npc_mcam_alloc_counter_req *req, + struct npc_mcam_alloc_counter_rsp *rsp) +{ + struct npc_mcam *mcam = &rvu->hw->mcam; + int err; + + mutex_lock(&mcam->lock); + + err = __npc_mcam_alloc_counter(rvu, req, rsp); + + mutex_unlock(&mcam->lock); + return err; +} + +static int __npc_mcam_free_counter(struct rvu *rvu, + struct npc_mcam_oper_counter_req *req, + struct msg_rsp *rsp) { struct npc_mcam *mcam = &rvu->hw->mcam; u16 index, entry = 0; @@ -3050,10 +3063,8 @@ int rvu_mbox_handler_npc_mcam_free_counter(struct rvu *rvu, if (blkaddr < 0) return NPC_MCAM_INVALID_REQ; - mutex_lock(&mcam->lock); err = npc_mcam_verify_counter(mcam, req->hdr.pcifunc, req->cntr); if (err) { - mutex_unlock(&mcam->lock); return err; } @@ -3077,10 +3088,66 @@ int rvu_mbox_handler_npc_mcam_free_counter(struct rvu *rvu, index, req->cntr); } - mutex_unlock(&mcam->lock); return 0; } +int rvu_mbox_handler_npc_mcam_free_counter(struct rvu *rvu, + struct npc_mcam_oper_counter_req *req, struct msg_rsp *rsp) +{ + struct npc_mcam *mcam = &rvu->hw->mcam; + int err; + + mutex_lock(&mcam->lock); + + err = __npc_mcam_free_counter(rvu, req, rsp); + + mutex_unlock(&mcam->lock); + + return err; +} + +void __rvu_mcam_remove_counter_from_rule(struct rvu *rvu, u16 pcifunc, + struct rvu_npc_mcam_rule *rule) +{ + struct npc_mcam_oper_counter_req free_req = { 0 }; + struct msg_rsp free_rsp; + + if (!rule->has_cntr) + return; + + free_req.hdr.pcifunc = pcifunc; + free_req.cntr = rule->cntr; + + __npc_mcam_free_counter(rvu, &free_req, &free_rsp); + rule->has_cntr = false; +} + +void __rvu_mcam_add_counter_to_rule(struct rvu *rvu, u16 pcifunc, + struct rvu_npc_mcam_rule *rule, + struct npc_install_flow_rsp *rsp) +{ + struct npc_mcam_alloc_counter_req cntr_req = { 0 }; + struct npc_mcam_alloc_counter_rsp cntr_rsp = { 0 }; + int err; + + cntr_req.hdr.pcifunc = pcifunc; + cntr_req.contig = true; + cntr_req.count = 1; + + /* we try to allocate a counter to track the stats of this + * rule. If counter could not be allocated then proceed + * without counter because counters are limited than entries. + */ + err = __npc_mcam_alloc_counter(rvu, &cntr_req, &cntr_rsp); + if (!err && cntr_rsp.count) { + rule->cntr = cntr_rsp.cntr; + rule->has_cntr = true; + rsp->counter = rule->cntr; + } else { + rsp->counter = err; + } +} + int rvu_mbox_handler_npc_mcam_unmap_counter(struct rvu *rvu, struct npc_mcam_unmap_counter_req *req, struct msg_rsp *rsp) { diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c index 150635de2bd5..7a1c18b1486d 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c @@ -1081,44 +1081,26 @@ static void rvu_mcam_add_rule(struct npc_mcam *mcam, static void rvu_mcam_remove_counter_from_rule(struct rvu *rvu, u16 pcifunc, struct rvu_npc_mcam_rule *rule) { - struct npc_mcam_oper_counter_req free_req = { 0 }; - struct msg_rsp free_rsp; + struct npc_mcam *mcam = &rvu->hw->mcam; - if (!rule->has_cntr) - return; + mutex_lock(&mcam->lock); - free_req.hdr.pcifunc = pcifunc; - free_req.cntr = rule->cntr; + __rvu_mcam_remove_counter_from_rule(rvu, pcifunc, rule); - rvu_mbox_handler_npc_mcam_free_counter(rvu, &free_req, &free_rsp); - rule->has_cntr = false; + mutex_unlock(&mcam->lock); } static void rvu_mcam_add_counter_to_rule(struct rvu *rvu, u16 pcifunc, struct rvu_npc_mcam_rule *rule, struct npc_install_flow_rsp *rsp) { - struct npc_mcam_alloc_counter_req cntr_req = { 0 }; - struct npc_mcam_alloc_counter_rsp cntr_rsp = { 0 }; - int err; + struct npc_mcam *mcam = &rvu->hw->mcam; - cntr_req.hdr.pcifunc = pcifunc; - cntr_req.contig = true; - cntr_req.count = 1; + mutex_lock(&mcam->lock); - /* we try to allocate a counter to track the stats of this - * rule. If counter could not be allocated then proceed - * without counter because counters are limited than entries. - */ - err = rvu_mbox_handler_npc_mcam_alloc_counter(rvu, &cntr_req, - &cntr_rsp); - if (!err && cntr_rsp.count) { - rule->cntr = cntr_rsp.cntr; - rule->has_cntr = true; - rsp->counter = rule->cntr; - } else { - rsp->counter = err; - } + __rvu_mcam_add_counter_to_rule(rvu, pcifunc, rule, rsp); + + mutex_unlock(&mcam->lock); } static int npc_mcast_update_action_index(struct rvu *rvu, struct npc_install_flow_req *req, -- 2.51.0 From 70a7434bdb13b2da6888e8f75f2d2950573c4095 Mon Sep 17 00:00:00 2001 From: Linu Cherian Date: Tue, 5 Nov 2024 18:26:19 +0530 Subject: [PATCH 06/16] octeontx2-af: Knobs for NPC default rule counters Add devlink knobs to enable/disable counters on NPC default rule entries. Sample command to enable default rule counters: devlink dev param set name npc_def_rule_cntr value true cmode runtime Sample command to read the counter: cat /sys/kernel/debug/cn10k/npc/mcam_rules Signed-off-by: Linu Cherian Link: https://patch.msgid.link/20241105125620.2114301-3-lcherian@marvell.com Signed-off-by: Jakub Kicinski --- .../net/ethernet/marvell/octeontx2/af/rvu.h | 2 + .../marvell/octeontx2/af/rvu_devlink.c | 32 ++++++++++++++ .../ethernet/marvell/octeontx2/af/rvu_npc.c | 43 +++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h index d92a5f47a476..e8c6a6fe9bd5 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h @@ -525,6 +525,7 @@ struct rvu { struct mutex alias_lock; /* Serialize bar2 alias access */ int vfs; /* Number of VFs attached to RVU */ u16 vf_devid; /* VF devices id */ + bool def_rule_cntr_en; int nix_blkaddr[MAX_NIX_BLKS]; /* Mbox */ @@ -989,6 +990,7 @@ void npc_set_mcam_action(struct rvu *rvu, struct npc_mcam *mcam, void npc_read_mcam_entry(struct rvu *rvu, struct npc_mcam *mcam, int blkaddr, u16 src, struct mcam_entry *entry, u8 *intf, u8 *ena); +int npc_config_cntr_default_entries(struct rvu *rvu, bool enable); bool is_cgx_config_permitted(struct rvu *rvu, u16 pcifunc); bool is_mac_feature_supported(struct rvu *rvu, int pf, int feature); u32 rvu_cgx_get_fifolen(struct rvu *rvu); diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c index 7498ab429963..9c26e19a860b 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c @@ -1238,6 +1238,7 @@ enum rvu_af_dl_param_id { RVU_AF_DEVLINK_PARAM_ID_DWRR_MTU, RVU_AF_DEVLINK_PARAM_ID_NPC_MCAM_ZONE_PERCENT, RVU_AF_DEVLINK_PARAM_ID_NPC_EXACT_FEATURE_DISABLE, + RVU_AF_DEVLINK_PARAM_ID_NPC_DEF_RULE_CNTR_ENABLE, RVU_AF_DEVLINK_PARAM_ID_NIX_MAXLF, }; @@ -1358,6 +1359,32 @@ static int rvu_af_dl_npc_mcam_high_zone_percent_validate(struct devlink *devlink return 0; } +static int rvu_af_dl_npc_def_rule_cntr_get(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx) +{ + struct rvu_devlink *rvu_dl = devlink_priv(devlink); + struct rvu *rvu = rvu_dl->rvu; + + ctx->val.vbool = rvu->def_rule_cntr_en; + + return 0; +} + +static int rvu_af_dl_npc_def_rule_cntr_set(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx, + struct netlink_ext_ack *extack) +{ + struct rvu_devlink *rvu_dl = devlink_priv(devlink); + struct rvu *rvu = rvu_dl->rvu; + int err; + + err = npc_config_cntr_default_entries(rvu, ctx->val.vbool); + if (!err) + rvu->def_rule_cntr_en = ctx->val.vbool; + + return err; +} + static int rvu_af_dl_nix_maxlf_get(struct devlink *devlink, u32 id, struct devlink_param_gset_ctx *ctx) { @@ -1444,6 +1471,11 @@ static const struct devlink_param rvu_af_dl_params[] = { rvu_af_dl_npc_mcam_high_zone_percent_get, rvu_af_dl_npc_mcam_high_zone_percent_set, rvu_af_dl_npc_mcam_high_zone_percent_validate), + DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NPC_DEF_RULE_CNTR_ENABLE, + "npc_def_rule_cntr", DEVLINK_PARAM_TYPE_BOOL, + BIT(DEVLINK_PARAM_CMODE_RUNTIME), + rvu_af_dl_npc_def_rule_cntr_get, + rvu_af_dl_npc_def_rule_cntr_set, NULL), DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NIX_MAXLF, "nix_maxlf", DEVLINK_PARAM_TYPE_U16, BIT(DEVLINK_PARAM_CMODE_RUNTIME), diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c index c4ef1e83cc46..821fe242f821 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c @@ -2691,6 +2691,49 @@ void npc_mcam_rsrcs_reserve(struct rvu *rvu, int blkaddr, int entry_idx) npc_mcam_set_bit(mcam, entry_idx); } +int npc_config_cntr_default_entries(struct rvu *rvu, bool enable) +{ + struct npc_mcam *mcam = &rvu->hw->mcam; + struct npc_install_flow_rsp rsp; + struct rvu_npc_mcam_rule *rule; + int blkaddr; + + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0); + if (blkaddr < 0) + return -EINVAL; + + mutex_lock(&mcam->lock); + list_for_each_entry(rule, &mcam->mcam_rules, list) { + if (!is_mcam_entry_enabled(rvu, mcam, blkaddr, rule->entry)) + continue; + if (!rule->default_rule) + continue; + if (enable && !rule->has_cntr) { /* Alloc and map new counter */ + __rvu_mcam_add_counter_to_rule(rvu, rule->owner, + rule, &rsp); + if (rsp.counter < 0) { + dev_err(rvu->dev, + "%s: Failed to allocate cntr for default rule (err=%d)\n", + __func__, rsp.counter); + break; + } + npc_map_mcam_entry_and_cntr(rvu, mcam, blkaddr, + rule->entry, rsp.counter); + /* Reset counter before use */ + rvu_write64(rvu, blkaddr, + NPC_AF_MATCH_STATX(rule->cntr), 0x0); + } + + /* Free and unmap counter */ + if (!enable && rule->has_cntr) + __rvu_mcam_remove_counter_from_rule(rvu, rule->owner, + rule); + } + mutex_unlock(&mcam->lock); + + return 0; +} + int rvu_mbox_handler_npc_mcam_alloc_entry(struct rvu *rvu, struct npc_mcam_alloc_entry_req *req, struct npc_mcam_alloc_entry_rsp *rsp) -- 2.51.0 From 46799a41d292bf9970a847ce2392cf6afb845014 Mon Sep 17 00:00:00 2001 From: Linu Cherian Date: Tue, 5 Nov 2024 18:26:20 +0530 Subject: [PATCH 07/16] devlink: Add documentation for OcteonTx2 AF Add documentation for the following devlink params - npc_mcam_high_zone_percent - npc_def_rule_cntr - nix_maxlf Signed-off-by: Linu Cherian Link: https://patch.msgid.link/20241105125620.2114301-4-lcherian@marvell.com Signed-off-by: Jakub Kicinski --- .../networking/devlink/octeontx2.rst | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Documentation/networking/devlink/octeontx2.rst b/Documentation/networking/devlink/octeontx2.rst index d33a90dd44bf..84206537aedb 100644 --- a/Documentation/networking/devlink/octeontx2.rst +++ b/Documentation/networking/devlink/octeontx2.rst @@ -40,6 +40,27 @@ The ``octeontx2 AF`` driver implements the following driver-specific parameters. - runtime - Use to set the quantum which hardware uses for scheduling among transmit queues. Hardware uses weighted DWRR algorithm to schedule among all transmit queues. + * - ``npc_mcam_high_zone_percent`` + - u8 + - runtime + - Use to set the number of high priority zone entries in NPC MCAM that can be allocated + by a user, out of the three priority zone categories high, mid and low. + * - ``npc_def_rule_cntr`` + - bool + - runtime + - Use to enable or disable hit counters for the default rules in NPC MCAM. + Its not guaranteed that counters gets enabled and mapped to all the default rules, + since the counters are scarce and driver follows a best effort approach. + The default rule serves as the primary packet steering rule for a specific PF or VF, + based on its DMAC address which is installed by AF driver as part of its initialization. + Sample command to read hit counters for default rule from debugfs is as follows, + cat /sys/kernel/debug/cn10k/npc/mcam_rules + * - ``nix_maxlf`` + - u16 + - runtime + - Use to set the maximum number of LFs in NIX hardware block. This would be useful + to increase the availability of default resources allocated to enabled LFs like + MCAM entries for example. The ``octeontx2 PF`` driver implements the following driver-specific parameters. -- 2.51.0 From d9ccb18f83ea2bb654289b6ecf014fd267cc988b Mon Sep 17 00:00:00 2001 From: Omid Ehtemam-Haghighi Date: Tue, 5 Nov 2024 17:02:36 -0800 Subject: [PATCH 08/16] ipv6: Fix soft lockups in fib6_select_path under high next hop churn MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Soft lockups have been observed on a cluster of Linux-based edge routers located in a highly dynamic environment. Using the `bird` service, these routers continuously update BGP-advertised routes due to frequently changing nexthop destinations, while also managing significant IPv6 traffic. The lockups occur during the traversal of the multipath circular linked-list in the `fib6_select_path` function, particularly while iterating through the siblings in the list. The issue typically arises when the nodes of the linked list are unexpectedly deleted concurrently on a different core—indicated by their 'next' and 'previous' elements pointing back to the node itself and their reference count dropping to zero. This results in an infinite loop, leading to a soft lockup that triggers a system panic via the watchdog timer. Apply RCU primitives in the problematic code sections to resolve the issue. Where necessary, update the references to fib6_siblings to annotate or use the RCU APIs. Include a test script that reproduces the issue. The script periodically updates the routing table while generating a heavy load of outgoing IPv6 traffic through multiple iperf3 clients. It consistently induces infinite soft lockups within a couple of minutes. Kernel log: 0 [ffffbd13003e8d30] machine_kexec at ffffffff8ceaf3eb 1 [ffffbd13003e8d90] __crash_kexec at ffffffff8d0120e3 2 [ffffbd13003e8e58] panic at ffffffff8cef65d4 3 [ffffbd13003e8ed8] watchdog_timer_fn at ffffffff8d05cb03 4 [ffffbd13003e8f08] __hrtimer_run_queues at ffffffff8cfec62f 5 [ffffbd13003e8f70] hrtimer_interrupt at ffffffff8cfed756 6 [ffffbd13003e8fd0] __sysvec_apic_timer_interrupt at ffffffff8cea01af 7 [ffffbd13003e8ff0] sysvec_apic_timer_interrupt at ffffffff8df1b83d -- -- 8 [ffffbd13003d3708] asm_sysvec_apic_timer_interrupt at ffffffff8e000ecb [exception RIP: fib6_select_path+299] RIP: ffffffff8ddafe7b RSP: ffffbd13003d37b8 RFLAGS: 00000287 RAX: ffff975850b43600 RBX: ffff975850b40200 RCX: 0000000000000000 RDX: 000000003fffffff RSI: 0000000051d383e4 RDI: ffff975850b43618 RBP: ffffbd13003d3800 R8: 0000000000000000 R9: ffff975850b40200 R10: 0000000000000000 R11: 0000000000000000 R12: ffffbd13003d3830 R13: ffff975850b436a8 R14: ffff975850b43600 R15: 0000000000000007 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 9 [ffffbd13003d3808] ip6_pol_route at ffffffff8ddb030c 10 [ffffbd13003d3888] ip6_pol_route_input at ffffffff8ddb068c 11 [ffffbd13003d3898] fib6_rule_lookup at ffffffff8ddf02b5 12 [ffffbd13003d3928] ip6_route_input at ffffffff8ddb0f47 13 [ffffbd13003d3a18] ip6_rcv_finish_core.constprop.0 at ffffffff8dd950d0 14 [ffffbd13003d3a30] ip6_list_rcv_finish.constprop.0 at ffffffff8dd96274 15 [ffffbd13003d3a98] ip6_sublist_rcv at ffffffff8dd96474 16 [ffffbd13003d3af8] ipv6_list_rcv at ffffffff8dd96615 17 [ffffbd13003d3b60] __netif_receive_skb_list_core at ffffffff8dc16fec 18 [ffffbd13003d3be0] netif_receive_skb_list_internal at ffffffff8dc176b3 19 [ffffbd13003d3c50] napi_gro_receive at ffffffff8dc565b9 20 [ffffbd13003d3c80] ice_receive_skb at ffffffffc087e4f5 [ice] 21 [ffffbd13003d3c90] ice_clean_rx_irq at ffffffffc0881b80 [ice] 22 [ffffbd13003d3d20] ice_napi_poll at ffffffffc088232f [ice] 23 [ffffbd13003d3d80] __napi_poll at ffffffff8dc18000 24 [ffffbd13003d3db8] net_rx_action at ffffffff8dc18581 25 [ffffbd13003d3e40] __do_softirq at ffffffff8df352e9 26 [ffffbd13003d3eb0] run_ksoftirqd at ffffffff8ceffe47 27 [ffffbd13003d3ec0] smpboot_thread_fn at ffffffff8cf36a30 28 [ffffbd13003d3ee8] kthread at ffffffff8cf2b39f 29 [ffffbd13003d3f28] ret_from_fork at ffffffff8ce5fa64 30 [ffffbd13003d3f50] ret_from_fork_asm at ffffffff8ce03cbb Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table") Reported-by: Adrian Oliver Signed-off-by: Omid Ehtemam-Haghighi Cc: Shuah Khan Cc: Ido Schimmel Cc: Kuniyuki Iwashima Cc: Simon Horman Reviewed-by: David Ahern Link: https://patch.msgid.link/20241106010236.1239299-1-omid.ehtemamhaghighi@menlosecurity.com Signed-off-by: Jakub Kicinski --- net/ipv6/ip6_fib.c | 8 +- net/ipv6/route.c | 45 ++- tools/testing/selftests/net/Makefile | 1 + .../net/ipv6_route_update_soft_lockup.sh | 262 ++++++++++++++++++ 4 files changed, 297 insertions(+), 19 deletions(-) create mode 100755 tools/testing/selftests/net/ipv6_route_update_soft_lockup.sh diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 6383263bfd04..c134ba202c4c 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -1183,8 +1183,8 @@ next_iter: while (sibling) { if (sibling->fib6_metric == rt->fib6_metric && rt6_qualify_for_ecmp(sibling)) { - list_add_tail(&rt->fib6_siblings, - &sibling->fib6_siblings); + list_add_tail_rcu(&rt->fib6_siblings, + &sibling->fib6_siblings); break; } sibling = rcu_dereference_protected(sibling->fib6_next, @@ -1245,7 +1245,7 @@ add: fib6_siblings) sibling->fib6_nsiblings--; rt->fib6_nsiblings = 0; - list_del_init(&rt->fib6_siblings); + list_del_rcu(&rt->fib6_siblings); rt6_multipath_rebalance(next_sibling); return err; } @@ -1963,7 +1963,7 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn, &rt->fib6_siblings, fib6_siblings) sibling->fib6_nsiblings--; rt->fib6_nsiblings = 0; - list_del_init(&rt->fib6_siblings); + list_del_rcu(&rt->fib6_siblings); rt6_multipath_rebalance(next_sibling); } diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 038c1eeef0be..63d7681c929f 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -416,8 +416,8 @@ void fib6_select_path(const struct net *net, struct fib6_result *res, struct flowi6 *fl6, int oif, bool have_oif_match, const struct sk_buff *skb, int strict) { - struct fib6_info *sibling, *next_sibling; struct fib6_info *match = res->f6i; + struct fib6_info *sibling; if (!match->nh && (!match->fib6_nsiblings || have_oif_match)) goto out; @@ -443,8 +443,8 @@ void fib6_select_path(const struct net *net, struct fib6_result *res, if (fl6->mp_hash <= atomic_read(&match->fib6_nh->fib_nh_upper_bound)) goto out; - list_for_each_entry_safe(sibling, next_sibling, &match->fib6_siblings, - fib6_siblings) { + list_for_each_entry_rcu(sibling, &match->fib6_siblings, + fib6_siblings) { const struct fib6_nh *nh = sibling->fib6_nh; int nh_upper_bound; @@ -5195,14 +5195,18 @@ static void ip6_route_mpath_notify(struct fib6_info *rt, * nexthop. Since sibling routes are always added at the end of * the list, find the first sibling of the last route appended */ + rcu_read_lock(); + if ((nlflags & NLM_F_APPEND) && rt_last && rt_last->fib6_nsiblings) { - rt = list_first_entry(&rt_last->fib6_siblings, - struct fib6_info, - fib6_siblings); + rt = list_first_or_null_rcu(&rt_last->fib6_siblings, + struct fib6_info, + fib6_siblings); } if (rt) inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags); + + rcu_read_unlock(); } static bool ip6_route_mpath_should_notify(const struct fib6_info *rt) @@ -5547,17 +5551,21 @@ static size_t rt6_nlmsg_size(struct fib6_info *f6i) nexthop_for_each_fib6_nh(f6i->nh, rt6_nh_nlmsg_size, &nexthop_len); } else { - struct fib6_info *sibling, *next_sibling; struct fib6_nh *nh = f6i->fib6_nh; + struct fib6_info *sibling; nexthop_len = 0; if (f6i->fib6_nsiblings) { rt6_nh_nlmsg_size(nh, &nexthop_len); - list_for_each_entry_safe(sibling, next_sibling, - &f6i->fib6_siblings, fib6_siblings) { + rcu_read_lock(); + + list_for_each_entry_rcu(sibling, &f6i->fib6_siblings, + fib6_siblings) { rt6_nh_nlmsg_size(sibling->fib6_nh, &nexthop_len); } + + rcu_read_unlock(); } nexthop_len += lwtunnel_get_encap_size(nh->fib_nh_lws); } @@ -5721,7 +5729,7 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb, lwtunnel_fill_encap(skb, dst->lwtstate, RTA_ENCAP, RTA_ENCAP_TYPE) < 0) goto nla_put_failure; } else if (rt->fib6_nsiblings) { - struct fib6_info *sibling, *next_sibling; + struct fib6_info *sibling; struct nlattr *mp; mp = nla_nest_start_noflag(skb, RTA_MULTIPATH); @@ -5733,14 +5741,21 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb, 0) < 0) goto nla_put_failure; - list_for_each_entry_safe(sibling, next_sibling, - &rt->fib6_siblings, fib6_siblings) { + rcu_read_lock(); + + list_for_each_entry_rcu(sibling, &rt->fib6_siblings, + fib6_siblings) { if (fib_add_nexthop(skb, &sibling->fib6_nh->nh_common, sibling->fib6_nh->fib_nh_weight, - AF_INET6, 0) < 0) + AF_INET6, 0) < 0) { + rcu_read_unlock(); + goto nla_put_failure; + } } + rcu_read_unlock(); + nla_nest_end(skb, mp); } else if (rt->nh) { if (nla_put_u32(skb, RTA_NH_ID, rt->nh->id)) @@ -6177,7 +6192,7 @@ void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info, err = -ENOBUFS; seq = info->nlh ? info->nlh->nlmsg_seq : 0; - skb = nlmsg_new(rt6_nlmsg_size(rt), gfp_any()); + skb = nlmsg_new(rt6_nlmsg_size(rt), GFP_ATOMIC); if (!skb) goto errout; @@ -6190,7 +6205,7 @@ void inet6_rt_notify(int event, struct fib6_info *rt, struct nl_info *info, goto errout; } rtnl_notify(skb, net, info->portid, RTNLGRP_IPV6_ROUTE, - info->nlh, gfp_any()); + info->nlh, GFP_ATOMIC); return; errout: rtnl_set_sk_err(net, RTNLGRP_IPV6_ROUTE, err); diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 26a4883a65c9..8c4db5199a42 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -96,6 +96,7 @@ TEST_PROGS += fdb_flush.sh TEST_PROGS += fq_band_pktlimit.sh TEST_PROGS += vlan_hw_filter.sh TEST_PROGS += bpf_offload.py +TEST_PROGS += ipv6_route_update_soft_lockup.sh # YNL files, must be before "include ..lib.mk" YNL_GEN_FILES := ncdevmem diff --git a/tools/testing/selftests/net/ipv6_route_update_soft_lockup.sh b/tools/testing/selftests/net/ipv6_route_update_soft_lockup.sh new file mode 100755 index 000000000000..a6b2b1f9c641 --- /dev/null +++ b/tools/testing/selftests/net/ipv6_route_update_soft_lockup.sh @@ -0,0 +1,262 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Testing for potential kernel soft lockup during IPv6 routing table +# refresh under heavy outgoing IPv6 traffic. If a kernel soft lockup +# occurs, a kernel panic will be triggered to prevent associated issues. +# +# +# Test Environment Layout +# +# ┌----------------┐ ┌----------------┐ +# | SOURCE_NS | | SINK_NS | +# | NAMESPACE | | NAMESPACE | +# |(iperf3 clients)| |(iperf3 servers)| +# | | | | +# | | | | +# | ┌-----------| nexthops |---------┐ | +# | |veth_source|<--------------------------------------->|veth_sink|<┐ | +# | └-----------|2001:0DB8:1::0:1/96 2001:0DB8:1::1:1/96 |---------┘ | | +# | | ^ 2001:0DB8:1::1:2/96 | | | +# | | . . | fwd | | +# | ┌---------┐ | . . | | | +# | | IPv6 | | . . | V | +# | | routing | | . 2001:0DB8:1::1:80/96| ┌-----┐ | +# | | table | | . | | lo | | +# | | nexthop | | . └--------┴-----┴-┘ +# | | update | | ............................> 2001:0DB8:2::1:1/128 +# | └-------- ┘ | +# └----------------┘ +# +# The test script sets up two network namespaces, source_ns and sink_ns, +# connected via a veth link. Within source_ns, it continuously updates the +# IPv6 routing table by flushing and inserting IPV6_NEXTHOP_ADDR_COUNT nexthop +# IPs destined for SINK_LOOPBACK_IP_ADDR in sink_ns. This refresh occurs at a +# rate of 1/ROUTING_TABLE_REFRESH_PERIOD per second for TEST_DURATION seconds. +# +# Simultaneously, multiple iperf3 clients within source_ns generate heavy +# outgoing IPv6 traffic. Each client is assigned a unique port number starting +# at 5000 and incrementing sequentially. Each client targets a unique iperf3 +# server running in sink_ns, connected to the SINK_LOOPBACK_IFACE interface +# using the same port number. +# +# The number of iperf3 servers and clients is set to half of the total +# available cores on each machine. +# +# NOTE: We have tested this script on machines with various CPU specifications, +# ranging from lower to higher performance as listed below. The test script +# effectively triggered a kernel soft lockup on machines running an unpatched +# kernel in under a minute: +# +# - 1x Intel Xeon E-2278G 8-Core Processor @ 3.40GHz +# - 1x Intel Xeon E-2378G Processor 8-Core @ 2.80GHz +# - 1x AMD EPYC 7401P 24-Core Processor @ 2.00GHz +# - 1x AMD EPYC 7402P 24-Core Processor @ 2.80GHz +# - 2x Intel Xeon Gold 5120 14-Core Processor @ 2.20GHz +# - 1x Ampere Altra Q80-30 80-Core Processor @ 3.00GHz +# - 2x Intel Xeon Gold 5120 14-Core Processor @ 2.20GHz +# - 2x Intel Xeon Silver 4214 24-Core Processor @ 2.20GHz +# - 1x AMD EPYC 7502P 32-Core @ 2.50GHz +# - 1x Intel Xeon Gold 6314U 32-Core Processor @ 2.30GHz +# - 2x Intel Xeon Gold 6338 32-Core Processor @ 2.00GHz +# +# On less performant machines, you may need to increase the TEST_DURATION +# parameter to enhance the likelihood of encountering a race condition leading +# to a kernel soft lockup and avoid a false negative result. +# +# NOTE: The test may not produce the expected result in virtualized +# environments (e.g., qemu) due to differences in timing and CPU handling, +# which can affect the conditions needed to trigger a soft lockup. + +source lib.sh +source net_helper.sh + +TEST_DURATION=300 +ROUTING_TABLE_REFRESH_PERIOD=0.01 + +IPERF3_BITRATE="300m" + + +IPV6_NEXTHOP_ADDR_COUNT="128" +IPV6_NEXTHOP_ADDR_MASK="96" +IPV6_NEXTHOP_PREFIX="2001:0DB8:1" + + +SOURCE_TEST_IFACE="veth_source" +SOURCE_TEST_IP_ADDR="2001:0DB8:1::0:1/96" + +SINK_TEST_IFACE="veth_sink" +# ${SINK_TEST_IFACE} is populated with the following range of IPv6 addresses: +# 2001:0DB8:1::1:1 to 2001:0DB8:1::1:${IPV6_NEXTHOP_ADDR_COUNT} +SINK_LOOPBACK_IFACE="lo" +SINK_LOOPBACK_IP_MASK="128" +SINK_LOOPBACK_IP_ADDR="2001:0DB8:2::1:1" + +nexthop_ip_list="" +termination_signal="" +kernel_softlokup_panic_prev_val="" + +terminate_ns_processes_by_pattern() { + local ns=$1 + local pattern=$2 + + for pid in $(ip netns pids ${ns}); do + [ -e /proc/$pid/cmdline ] && grep -qe "${pattern}" /proc/$pid/cmdline && kill -9 $pid + done +} + +cleanup() { + echo "info: cleaning up namespaces and terminating all processes within them..." + + + # Terminate iperf3 instances running in the source_ns. To avoid race + # conditions, first iterate over the PIDs and terminate those + # associated with the bash shells running the + # `while true; do iperf3 -c ...; done` loops. In a second iteration, + # terminate the individual `iperf3 -c ...` instances. + terminate_ns_processes_by_pattern ${source_ns} while + terminate_ns_processes_by_pattern ${source_ns} iperf3 + + # Repeat the same process for sink_ns + terminate_ns_processes_by_pattern ${sink_ns} while + terminate_ns_processes_by_pattern ${sink_ns} iperf3 + + # Check if any iperf3 instances are still running. This could happen + # if a core has entered an infinite loop and the timeout for detecting + # the soft lockup has not expired, but either the test interval has + # already elapsed or the test was terminated manually (e.g., with ^C) + for pid in $(ip netns pids ${source_ns}); do + if [ -e /proc/$pid/cmdline ] && grep -qe 'iperf3' /proc/$pid/cmdline; then + echo "FAIL: unable to terminate some iperf3 instances. Soft lockup is underway. A kernel panic is on the way!" + exit ${ksft_fail} + fi + done + + if [ "$termination_signal" == "SIGINT" ]; then + echo "SKIP: Termination due to ^C (SIGINT)" + elif [ "$termination_signal" == "SIGALRM" ]; then + echo "PASS: No kernel soft lockup occurred during this ${TEST_DURATION} second test" + fi + + cleanup_ns ${source_ns} ${sink_ns} + + sysctl -qw kernel.softlockup_panic=${kernel_softlokup_panic_prev_val} +} + +setup_prepare() { + setup_ns source_ns sink_ns + + ip -n ${source_ns} link add name ${SOURCE_TEST_IFACE} type veth peer name ${SINK_TEST_IFACE} netns ${sink_ns} + + # Setting up the Source namespace + ip -n ${source_ns} addr add ${SOURCE_TEST_IP_ADDR} dev ${SOURCE_TEST_IFACE} + ip -n ${source_ns} link set dev ${SOURCE_TEST_IFACE} qlen 10000 + ip -n ${source_ns} link set dev ${SOURCE_TEST_IFACE} up + ip netns exec ${source_ns} sysctl -qw net.ipv6.fib_multipath_hash_policy=1 + + # Setting up the Sink namespace + ip -n ${sink_ns} addr add ${SINK_LOOPBACK_IP_ADDR}/${SINK_LOOPBACK_IP_MASK} dev ${SINK_LOOPBACK_IFACE} + ip -n ${sink_ns} link set dev ${SINK_LOOPBACK_IFACE} up + ip netns exec ${sink_ns} sysctl -qw net.ipv6.conf.${SINK_LOOPBACK_IFACE}.forwarding=1 + + ip -n ${sink_ns} link set ${SINK_TEST_IFACE} up + ip netns exec ${sink_ns} sysctl -qw net.ipv6.conf.${SINK_TEST_IFACE}.forwarding=1 + + + # Populate nexthop IPv6 addresses on the test interface in the sink_ns + echo "info: populating ${IPV6_NEXTHOP_ADDR_COUNT} IPv6 addresses on the ${SINK_TEST_IFACE} interface ..." + for IP in $(seq 1 ${IPV6_NEXTHOP_ADDR_COUNT}); do + ip -n ${sink_ns} addr add ${IPV6_NEXTHOP_PREFIX}::$(printf "1:%x" "${IP}")/${IPV6_NEXTHOP_ADDR_MASK} dev ${SINK_TEST_IFACE}; + done + + # Preparing list of nexthops + for IP in $(seq 1 ${IPV6_NEXTHOP_ADDR_COUNT}); do + nexthop_ip_list=$nexthop_ip_list" nexthop via ${IPV6_NEXTHOP_PREFIX}::$(printf "1:%x" $IP) dev ${SOURCE_TEST_IFACE} weight 1" + done +} + + +test_soft_lockup_during_routing_table_refresh() { + # Start num_of_iperf_servers iperf3 servers in the sink_ns namespace, + # each listening on ports starting at 5001 and incrementing + # sequentially. Since iperf3 instances may terminate unexpectedly, a + # while loop is used to automatically restart them in such cases. + echo "info: starting ${num_of_iperf_servers} iperf3 servers in the sink_ns namespace ..." + for i in $(seq 1 ${num_of_iperf_servers}); do + cmd="iperf3 --bind ${SINK_LOOPBACK_IP_ADDR} -s -p $(printf '5%03d' ${i}) --rcv-timeout 200 &>/dev/null" + ip netns exec ${sink_ns} bash -c "while true; do ${cmd}; done &" &>/dev/null + done + + # Wait for the iperf3 servers to be ready + for i in $(seq ${num_of_iperf_servers}); do + port=$(printf '5%03d' ${i}); + wait_local_port_listen ${sink_ns} ${port} tcp + done + + # Continuously refresh the routing table in the background within + # the source_ns namespace + ip netns exec ${source_ns} bash -c " + while \$(ip netns list | grep -q ${source_ns}); do + ip -6 route add ${SINK_LOOPBACK_IP_ADDR}/${SINK_LOOPBACK_IP_MASK} ${nexthop_ip_list}; + sleep ${ROUTING_TABLE_REFRESH_PERIOD}; + ip -6 route delete ${SINK_LOOPBACK_IP_ADDR}/${SINK_LOOPBACK_IP_MASK}; + done &" + + # Start num_of_iperf_servers iperf3 clients in the source_ns namespace, + # each sending TCP traffic on sequential ports starting at 5001. + # Since iperf3 instances may terminate unexpectedly (e.g., if the route + # to the server is deleted in the background during a route refresh), a + # while loop is used to automatically restart them in such cases. + echo "info: starting ${num_of_iperf_servers} iperf3 clients in the source_ns namespace ..." + for i in $(seq 1 ${num_of_iperf_servers}); do + cmd="iperf3 -c ${SINK_LOOPBACK_IP_ADDR} -p $(printf '5%03d' ${i}) --length 64 --bitrate ${IPERF3_BITRATE} -t 0 --connect-timeout 150 &>/dev/null" + ip netns exec ${source_ns} bash -c "while true; do ${cmd}; done &" &>/dev/null + done + + echo "info: IPv6 routing table is being updated at the rate of $(echo "1/${ROUTING_TABLE_REFRESH_PERIOD}" | bc)/s for ${TEST_DURATION} seconds ..." + echo "info: A kernel soft lockup, if detected, results in a kernel panic!" + + wait +} + +# Make sure 'iperf3' is installed, skip the test otherwise +if [ ! -x "$(command -v "iperf3")" ]; then + echo "SKIP: 'iperf3' is not installed. Skipping the test." + exit ${ksft_skip} +fi + +# Determine the number of cores on the machine +num_of_iperf_servers=$(( $(nproc)/2 )) + +# Check if we are running on a multi-core machine, skip the test otherwise +if [ "${num_of_iperf_servers}" -eq 0 ]; then + echo "SKIP: This test is not valid on a single core machine!" + exit ${ksft_skip} +fi + +# Since the kernel soft lockup we're testing causes at least one core to enter +# an infinite loop, destabilizing the host and likely affecting subsequent +# tests, we trigger a kernel panic instead of reporting a failure and +# continuing +kernel_softlokup_panic_prev_val=$(sysctl -n kernel.softlockup_panic) +sysctl -qw kernel.softlockup_panic=1 + +handle_sigint() { + termination_signal="SIGINT" + cleanup + exit ${ksft_skip} +} + +handle_sigalrm() { + termination_signal="SIGALRM" + cleanup + exit ${ksft_pass} +} + +trap handle_sigint SIGINT +trap handle_sigalrm SIGALRM + +(sleep ${TEST_DURATION} && kill -s SIGALRM $$)& + +setup_prepare +test_soft_lockup_during_routing_table_refresh -- 2.51.0 From 8b9a7bd4d6c83300e50bb1d7071c6032a07e2fed Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 6 Nov 2024 13:00:45 +0000 Subject: [PATCH 09/16] rxrpc: Add a tracepoint for aborts being proposed Add a tracepoint to rxrpc to trace the proposal of an abort. The abort is performed asynchronously by the I/O thread. Signed-off-by: David Howells cc: Marc Dionne cc: Simon Horman cc: linux-afs@lists.infradead.org Link: https://patch.msgid.link/726356.1730898045@warthog.procyon.org.uk Signed-off-by: Jakub Kicinski --- include/trace/events/rxrpc.h | 25 +++++++++++++++++++++++++ net/rxrpc/sendmsg.c | 1 + 2 files changed, 26 insertions(+) diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h index cc22596c7250..d03e0bd8c028 100644 --- a/include/trace/events/rxrpc.h +++ b/include/trace/events/rxrpc.h @@ -773,6 +773,31 @@ TRACE_EVENT(rxrpc_rx_done, TP_printk("r=%d a=%d", __entry->result, __entry->abort_code) ); +TRACE_EVENT(rxrpc_abort_call, + TP_PROTO(const struct rxrpc_call *call, int abort_code), + + TP_ARGS(call, abort_code), + + TP_STRUCT__entry( + __field(unsigned int, call_nr) + __field(enum rxrpc_abort_reason, why) + __field(int, abort_code) + __field(int, error) + ), + + TP_fast_assign( + __entry->call_nr = call->debug_id; + __entry->why = call->send_abort_why; + __entry->abort_code = abort_code; + __entry->error = call->send_abort_err; + ), + + TP_printk("c=%08x a=%d e=%d %s", + __entry->call_nr, + __entry->abort_code, __entry->error, + __print_symbolic(__entry->why, rxrpc_abort_reasons)) + ); + TRACE_EVENT(rxrpc_abort, TP_PROTO(unsigned int call_nr, enum rxrpc_abort_reason why, u32 cid, u32 call_id, rxrpc_seq_t seq, int abort_code, int error), diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c index 23d18fe5de9f..6abb8eec1b2b 100644 --- a/net/rxrpc/sendmsg.c +++ b/net/rxrpc/sendmsg.c @@ -29,6 +29,7 @@ bool rxrpc_propose_abort(struct rxrpc_call *call, s32 abort_code, int error, call->send_abort_why = why; call->send_abort_err = error; call->send_abort_seq = 0; + trace_rxrpc_abort_call(call, abort_code); /* Request abort locklessly vs rxrpc_input_call_event(). */ smp_store_release(&call->send_abort, abort_code); rxrpc_poke_call(call, rxrpc_call_poke_abort); -- 2.51.0 From fcf42409c6e15e47186de8bb051176aaf92c597a Mon Sep 17 00:00:00 2001 From: Mohammad Heib Date: Wed, 6 Nov 2024 20:08:11 +0200 Subject: [PATCH 10/16] bnxt_en: use irq_update_affinity_hint() irq_set_affinity_hint() is deprecated, Use irq_update_affinity_hint() instead. This removes the side-effect of actually applying the affinity. The driver does not really need to worry about spreading its IRQs across CPUs. The core code already takes care of that. when the driver applies the affinities by itself, it breaks the users' expectations: 1. The user configures irqbalance with IRQBALANCE_BANNED_CPULIST in order to prevent IRQs from being moved to certain CPUs that run a real-time workload. 2. bnxt_en device reopening will resets the affinity in bnxt_open(). 3. bnxt_en has no idea about irqbalance's config, so it may move an IRQ to a banned CPU. The real-time workload suffers unacceptable latency. Signed-off-by: Mohammad Heib Reviewed-by: Andy Gospodarek Reviewed-by: Somnath Kotur Link: https://patch.msgid.link/20241106180811.385175-1-mheib@redhat.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 98f589e1cbe4..e9aab2e2840e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -10882,7 +10882,7 @@ static void bnxt_free_irq(struct bnxt *bp) irq = &bp->irq_tbl[map_idx]; if (irq->requested) { if (irq->have_cpumask) { - irq_set_affinity_hint(irq->vector, NULL); + irq_update_affinity_hint(irq->vector, NULL); free_cpumask_var(irq->cpu_mask); irq->have_cpumask = 0; } @@ -10937,10 +10937,10 @@ static int bnxt_request_irq(struct bnxt *bp) irq->have_cpumask = 1; cpumask_set_cpu(cpumask_local_spread(i, numa_node), irq->cpu_mask); - rc = irq_set_affinity_hint(irq->vector, irq->cpu_mask); + rc = irq_update_affinity_hint(irq->vector, irq->cpu_mask); if (rc) { netdev_warn(bp->dev, - "Set affinity failed, IRQ = %d\n", + "Update affinity hint failed, IRQ = %d\n", irq->vector); break; } -- 2.51.0 From d9e2e290f7142d4c67c05ebbe37388d54a66c6c5 Mon Sep 17 00:00:00 2001 From: Mohammad Heib Date: Thu, 7 Nov 2024 13:50:02 +0200 Subject: [PATCH 11/16] nfp: use irq_update_affinity_hint() irq_set_affinity_hint() is deprecated, Use irq_update_affinity_hint() instead. This removes the side-effect of actually applying the affinity. The driver does not really need to worry about spreading its IRQs across CPUs. The core code already takes care of that. when the driver applies the affinities by itself, it breaks the users' expectations: 1. The user configures irqbalance with IRQBALANCE_BANNED_CPULIST in order to prevent IRQs from being moved to certain CPUs that run a real-time workload. 2. nfp device reopening will resets the affinity in nfp_net_netdev_open(). 3. nfp has no idea about irqbalance's config, so it may move an IRQ to a banned CPU. The real-time workload suffers unacceptable latency. Signed-off-by: Mohammad Heib Reviewed-by: Simon Horman Reviewed-by: Louis Peens Link: https://patch.msgid.link/20241107115002.413358-1-mheib@redhat.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c index 6e0929af0f72..98e098c09c03 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c @@ -829,7 +829,7 @@ nfp_net_prepare_vector(struct nfp_net *nn, struct nfp_net_r_vector *r_vec, return err; } - irq_set_affinity_hint(r_vec->irq_vector, &r_vec->affinity_mask); + irq_update_affinity_hint(r_vec->irq_vector, &r_vec->affinity_mask); nn_dbg(nn, "RV%02d: irq=%03d/%03d\n", idx, r_vec->irq_vector, r_vec->irq_entry); @@ -840,7 +840,7 @@ nfp_net_prepare_vector(struct nfp_net *nn, struct nfp_net_r_vector *r_vec, static void nfp_net_cleanup_vector(struct nfp_net *nn, struct nfp_net_r_vector *r_vec) { - irq_set_affinity_hint(r_vec->irq_vector, NULL); + irq_update_affinity_hint(r_vec->irq_vector, NULL); nfp_net_napi_del(&nn->dp, r_vec); free_irq(r_vec->irq_vector, r_vec); } -- 2.51.0 From 2cd78740effc587610ca2be6d803e3f61fc87ef6 Mon Sep 17 00:00:00 2001 From: Mohammad Heib Date: Thu, 7 Nov 2024 14:07:39 +0200 Subject: [PATCH 12/16] net: atlantic: use irq_update_affinity_hint() irq_set_affinity_hint() is deprecated, Use irq_update_affinity_hint() instead. This removes the side-effect of actually applying the affinity. The driver does not really need to worry about spreading its IRQs across CPUs. The core code already takes care of that. when the driver applies the affinities by itself, it breaks the users' expectations: 1. The user configures irqbalance with IRQBALANCE_BANNED_CPULIST in order to prevent IRQs from being moved to certain CPUs that run a real-time workload. 2. atlantic device reopening will resets the affinity in aq_ndev_open(). 3. atlantic has no idea about irqbalance's config, so it may move an IRQ to a banned CPU. The real-time workload suffers unacceptable latency. Signed-off-by: Mohammad Heib Reviewed-by: Simon Horman Link: https://patch.msgid.link/20241107120739.415743-1-mheib@redhat.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c index 43c71f6b314f..08630ee94251 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c @@ -162,8 +162,8 @@ int aq_pci_func_alloc_irq(struct aq_nic_s *self, unsigned int i, self->msix_entry_mask |= (1 << i); if (pdev->msix_enabled && affinity_mask) - irq_set_affinity_hint(pci_irq_vector(pdev, i), - affinity_mask); + irq_update_affinity_hint(pci_irq_vector(pdev, i), + affinity_mask); } return err; @@ -187,7 +187,7 @@ void aq_pci_func_free_irqs(struct aq_nic_s *self) continue; if (pdev->msix_enabled) - irq_set_affinity_hint(pci_irq_vector(pdev, i), NULL); + irq_update_affinity_hint(pci_irq_vector(pdev, i), NULL); free_irq(pci_irq_vector(pdev, i), irq_data); self->msix_entry_mask &= ~(1U << i); } -- 2.51.0 From 7eb4c2571443f4efe07dedf9f5a99d7cfe716415 Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Wed, 6 Nov 2024 08:59:36 +0100 Subject: [PATCH 13/16] dt-bindings: net: dsa: microchip: add internal MDIO bus description Add description for the internal MDIO bus, including integrated PHY nodes, to ksz DSA bindings. Signed-off-by: Oleksij Rempel Reviewed-by: Rob Herring (Arm) Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20241106075942.1636998-2-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski --- .../devicetree/bindings/net/dsa/microchip,ksz.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml index 30c0c3e6f37a..a4e463819d4d 100644 --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml @@ -81,6 +81,17 @@ properties: interrupts: maxItems: 1 + mdio: + $ref: /schemas/net/mdio.yaml# + unevaluatedProperties: false + patternProperties: + "^ethernet-phy@[0-9a-f]$": + type: object + $ref: /schemas/net/ethernet-phy.yaml# + unevaluatedProperties: false + description: + Integrated PHY node + required: - compatible - reg -- 2.51.0 From 698b20a679bee9c4021f35e195760798f3530c88 Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Wed, 6 Nov 2024 08:59:37 +0100 Subject: [PATCH 14/16] dt-bindings: net: dsa: microchip: add mdio-parent-bus property for internal MDIO Introduce `mdio-parent-bus` property in the ksz DSA bindings to reference the parent MDIO bus when the internal MDIO bus is attached to it, bypassing the main management interface. Signed-off-by: Oleksij Rempel Reviewed-by: Rob Herring (Arm) Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20241106075942.1636998-3-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski --- .../devicetree/bindings/net/dsa/microchip,ksz.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml index a4e463819d4d..121a4bbd147b 100644 --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml @@ -84,6 +84,15 @@ properties: mdio: $ref: /schemas/net/mdio.yaml# unevaluatedProperties: false + properties: + mdio-parent-bus: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle pointing to the MDIO bus controller connected to the + secondary MDIO interface. This property should be used when + the internal MDIO bus is accessed via a secondary MDIO + interface rather than the primary management interface. + patternProperties: "^ethernet-phy@[0-9a-f]$": type: object -- 2.51.0 From 9afaf0eec2ab6bcfa227ab528fbdf2881fa7a293 Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Wed, 6 Nov 2024 08:59:38 +0100 Subject: [PATCH 15/16] net: dsa: microchip: Refactor MDIO handling for side MDIO access Add support for accessing PHYs via a side MDIO interface in LAN937x switches. The existing code already supports accessing PHYs via main management interfaces, which can be SPI, I2C, or MDIO, depending on the chip variant. This patch enables using a side MDIO bus, where SPI is used for the main switch configuration and MDIO for managing the integrated PHYs. On LAN937x, this is optional, allowing them to operate in both configurations: SPI only, or SPI + MDIO. Typically, the SPI interface is used for switch configuration, while MDIO handles PHY management. Additionally, update interrupt controller code to support non-linear port to PHY address mapping, enabling correct interrupt handling for configurations where PHY addresses do not directly correspond to port indexes. This change ensures that the interrupt mechanism properly aligns with the new, flexible PHY address mappings introduced by side MDIO support. Signed-off-by: Oleksij Rempel Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20241106075942.1636998-4-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski --- drivers/net/dsa/microchip/ksz_common.c | 175 +++++++++++++++++++++++-- drivers/net/dsa/microchip/ksz_common.h | 59 +++++++++ 2 files changed, 224 insertions(+), 10 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index f73833e24622..e9460650d46e 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -2236,16 +2236,100 @@ static int ksz_sw_mdio_write(struct mii_bus *bus, int addr, int regnum, return dev->dev_ops->w_phy(dev, addr, regnum, val); } +/** + * ksz_parent_mdio_read - Read data from a PHY register on the parent MDIO bus. + * @bus: MDIO bus structure. + * @addr: PHY address on the parent MDIO bus. + * @regnum: Register number to read. + * + * This function provides a direct read operation on the parent MDIO bus for + * accessing PHY registers. By bypassing SPI or I2C, it uses the parent MDIO bus + * to retrieve data from the PHY registers at the specified address and register + * number. + * + * Return: Value of the PHY register, or a negative error code on failure. + */ +static int ksz_parent_mdio_read(struct mii_bus *bus, int addr, int regnum) +{ + struct ksz_device *dev = bus->priv; + + return mdiobus_read_nested(dev->parent_mdio_bus, addr, regnum); +} + +/** + * ksz_parent_mdio_write - Write data to a PHY register on the parent MDIO bus. + * @bus: MDIO bus structure. + * @addr: PHY address on the parent MDIO bus. + * @regnum: Register number to write to. + * @val: Value to write to the PHY register. + * + * This function provides a direct write operation on the parent MDIO bus for + * accessing PHY registers. Bypassing SPI or I2C, it uses the parent MDIO bus + * to modify the PHY register values at the specified address. + * + * Return: 0 on success, or a negative error code on failure. + */ +static int ksz_parent_mdio_write(struct mii_bus *bus, int addr, int regnum, + u16 val) +{ + struct ksz_device *dev = bus->priv; + + return mdiobus_write_nested(dev->parent_mdio_bus, addr, regnum, val); +} + +/** + * ksz_phy_addr_to_port - Map a PHY address to the corresponding switch port. + * @dev: Pointer to device structure. + * @addr: PHY address to map to a port. + * + * This function finds the corresponding switch port for a given PHY address by + * iterating over all user ports on the device. It checks if a port's PHY + * address in `phy_addr_map` matches the specified address and if the port + * contains an internal PHY. If a match is found, the index of the port is + * returned. + * + * Return: Port index on success, or -EINVAL if no matching port is found. + */ +static int ksz_phy_addr_to_port(struct ksz_device *dev, int addr) +{ + struct dsa_switch *ds = dev->ds; + struct dsa_port *dp; + + dsa_switch_for_each_user_port(dp, ds) { + if (dev->info->internal_phy[dp->index] && + dev->phy_addr_map[dp->index] == addr) + return dp->index; + } + + return -EINVAL; +} + +/** + * ksz_irq_phy_setup - Configure IRQs for PHYs in the KSZ device. + * @dev: Pointer to the KSZ device structure. + * + * Sets up IRQs for each active PHY connected to the KSZ switch by mapping the + * appropriate IRQs for each PHY and assigning them to the `user_mii_bus` in + * the DSA switch structure. Each IRQ is mapped based on the port's IRQ domain. + * + * Return: 0 on success, or a negative error code on failure. + */ static int ksz_irq_phy_setup(struct ksz_device *dev) { struct dsa_switch *ds = dev->ds; - int phy; + int phy, port; int irq; int ret; - for (phy = 0; phy < KSZ_MAX_NUM_PORTS; phy++) { + for (phy = 0; phy < PHY_MAX_ADDR; phy++) { if (BIT(phy) & ds->phys_mii_mask) { - irq = irq_find_mapping(dev->ports[phy].pirq.domain, + port = ksz_phy_addr_to_port(dev, phy); + if (port < 0) { + ret = port; + goto out; + } + + irq = irq_find_mapping(dev->ports[port].pirq.domain, PORT_SRC_PHY_INT); if (irq < 0) { ret = irq; @@ -2263,40 +2347,109 @@ out: return ret; } +/** + * ksz_irq_phy_free - Release IRQ mappings for PHYs in the KSZ device. + * @dev: Pointer to the KSZ device structure. + * + * Releases any IRQ mappings previously assigned to active PHYs in the KSZ + * switch by disposing of each mapped IRQ in the `user_mii_bus` structure. + */ static void ksz_irq_phy_free(struct ksz_device *dev) { struct dsa_switch *ds = dev->ds; int phy; - for (phy = 0; phy < KSZ_MAX_NUM_PORTS; phy++) + for (phy = 0; phy < PHY_MAX_ADDR; phy++) if (BIT(phy) & ds->phys_mii_mask) irq_dispose_mapping(ds->user_mii_bus->irq[phy]); } +/** + * ksz_mdio_register - Register and configure the MDIO bus for the KSZ device. + * @dev: Pointer to the KSZ device structure. + * + * This function sets up and registers an MDIO bus for the KSZ switch device, + * allowing access to its internal PHYs. If the device supports side MDIO, + * the function will configure the external MDIO controller specified by the + * "mdio-parent-bus" device tree property to directly manage internal PHYs. + * Otherwise, SPI or I2C access is set up for PHY access. + * + * Return: 0 on success, or a negative error code on failure. + */ static int ksz_mdio_register(struct ksz_device *dev) { + struct device_node *parent_bus_node; + struct mii_bus *parent_bus = NULL; struct dsa_switch *ds = dev->ds; struct device_node *mdio_np; struct mii_bus *bus; - int ret; + struct dsa_port *dp; + int ret, i; mdio_np = of_get_child_by_name(dev->dev->of_node, "mdio"); if (!mdio_np) return 0; + parent_bus_node = of_parse_phandle(mdio_np, "mdio-parent-bus", 0); + if (parent_bus_node && !dev->info->phy_side_mdio_supported) { + dev_err(dev->dev, "Side MDIO bus is not supported for this HW, ignoring 'mdio-parent-bus' property.\n"); + ret = -EINVAL; + + goto put_mdio_node; + } else if (parent_bus_node) { + parent_bus = of_mdio_find_bus(parent_bus_node); + if (!parent_bus) { + ret = -EPROBE_DEFER; + + goto put_mdio_node; + } + + dev->parent_mdio_bus = parent_bus; + } + bus = devm_mdiobus_alloc(ds->dev); if (!bus) { of_node_put(mdio_np); return -ENOMEM; } + if (dev->dev_ops->mdio_bus_preinit) { + ret = dev->dev_ops->mdio_bus_preinit(dev, !!parent_bus); + if (ret) + goto put_mdio_node; + } + + if (dev->dev_ops->create_phy_addr_map) { + ret = dev->dev_ops->create_phy_addr_map(dev, !!parent_bus); + if (ret) + goto put_mdio_node; + } else { + for (i = 0; i < dev->info->port_cnt; i++) + dev->phy_addr_map[i] = i; + } + bus->priv = dev; - bus->read = ksz_sw_mdio_read; - bus->write = ksz_sw_mdio_write; - bus->name = "ksz user smi"; - snprintf(bus->id, MII_BUS_ID_SIZE, "SMI-%d", ds->index); + if (parent_bus) { + bus->read = ksz_parent_mdio_read; + bus->write = ksz_parent_mdio_write; + bus->name = "KSZ side MDIO"; + snprintf(bus->id, MII_BUS_ID_SIZE, "ksz-side-mdio-%d", + ds->index); + } else { + bus->read = ksz_sw_mdio_read; + bus->write = ksz_sw_mdio_write; + bus->name = "ksz user smi"; + snprintf(bus->id, MII_BUS_ID_SIZE, "SMI-%d", ds->index); + } + + dsa_switch_for_each_user_port(dp, dev->ds) { + if (dev->info->internal_phy[dp->index] && + dev->phy_addr_map[dp->index] < PHY_MAX_ADDR) + bus->phy_mask |= BIT(dev->phy_addr_map[dp->index]); + } + + ds->phys_mii_mask = bus->phy_mask; bus->parent = ds->dev; - bus->phy_mask = ~ds->phys_mii_mask; ds->user_mii_bus = bus; @@ -2316,7 +2469,9 @@ static int ksz_mdio_register(struct ksz_device *dev) ksz_irq_phy_free(dev); } +put_mdio_node: of_node_put(mdio_np); + of_node_put(parent_bus_node); return ret; } diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index bec846e20682..bbb548af201e 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -65,6 +65,12 @@ struct ksz_chip_data { u8 num_tx_queues; u8 num_ipms; /* number of Internal Priority Maps */ bool tc_cbs_supported; + + /** + * @phy_side_mdio_supported: Indicates if the chip supports an additional + * side MDIO channel for accessing integrated PHYs. + */ + bool phy_side_mdio_supported; const struct ksz_dev_ops *ops; const struct phylink_mac_ops *phylink_mac_ops; bool phy_errata_9477; @@ -191,6 +197,22 @@ struct ksz_device { struct ksz_switch_macaddr *switch_macaddr; struct net_device *hsr_dev; /* HSR */ u8 hsr_ports; + + /** + * @phy_addr_map: Array mapping switch ports to their corresponding PHY + * addresses. + */ + u8 phy_addr_map[KSZ_MAX_NUM_PORTS]; + + /** + * @parent_mdio_bus: Pointer to the external MDIO bus controller. + * + * This points to an external MDIO bus controller that is used to access + * the PHYs integrated within the switch. Unlike an integrated MDIO + * bus, this external controller provides a direct path for managing + * the switch’s internal PHYs, bypassing the main SPI interface. + */ + struct mii_bus *parent_mdio_bus; }; /* List of supported models */ @@ -326,6 +348,43 @@ struct ksz_dev_ops { void (*port_cleanup)(struct ksz_device *dev, int port); void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port); int (*set_ageing_time)(struct ksz_device *dev, unsigned int msecs); + + /** + * @mdio_bus_preinit: Function pointer to pre-initialize the MDIO bus + * for accessing PHYs. + * @dev: Pointer to device structure. + * @side_mdio: Boolean indicating if the PHYs are accessed over a side + * MDIO bus. + * + * This function pointer is used to configure the MDIO bus for PHY + * access before initiating regular PHY operations. It enables either + * SPI/I2C or side MDIO access modes by unlocking necessary registers + * and setting up access permissions for the selected mode. + * + * Return: + * - 0 on success. + * - Negative error code on failure. + */ + int (*mdio_bus_preinit)(struct ksz_device *dev, bool side_mdio); + + /** + * @create_phy_addr_map: Function pointer to create a port-to-PHY + * address map. + * @dev: Pointer to device structure. + * @side_mdio: Boolean indicating if the PHYs are accessed over a side + * MDIO bus. + * + * This function pointer is responsible for mapping switch ports to PHY + * addresses according to the configured access mode (SPI or side MDIO) + * and the device’s strap configuration. The mapping setup may vary + * depending on the chip variant and configuration. Ensures the correct + * address mapping for PHY communication. + * + * Return: + * - 0 on success. + * - Negative error code on failure (e.g., invalid configuration). + */ + int (*create_phy_addr_map)(struct ksz_device *dev, bool side_mdio); int (*r_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 *val); int (*w_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 val); void (*r_mib_cnt)(struct ksz_device *dev, int port, u16 addr, -- 2.51.0 From 8bbba4161b6557ba8effc443072e70c16da24e6f Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Wed, 6 Nov 2024 08:59:39 +0100 Subject: [PATCH 16/16] net: dsa: microchip: cleanup error handling in ksz_mdio_register Replace repeated cleanup code with a single error path using a label. Signed-off-by: Oleksij Rempel Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20241106075942.1636998-5-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski --- drivers/net/dsa/microchip/ksz_common.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index e9460650d46e..26a4a8b49dc8 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -2409,8 +2409,8 @@ static int ksz_mdio_register(struct ksz_device *dev) bus = devm_mdiobus_alloc(ds->dev); if (!bus) { - of_node_put(mdio_np); - return -ENOMEM; + ret = -ENOMEM; + goto put_mdio_node; } if (dev->dev_ops->mdio_bus_preinit) { @@ -2455,10 +2455,8 @@ static int ksz_mdio_register(struct ksz_device *dev) if (dev->irq > 0) { ret = ksz_irq_phy_setup(dev); - if (ret) { - of_node_put(mdio_np); - return ret; - } + if (ret) + goto put_mdio_node; } ret = devm_of_mdiobus_register(ds->dev, bus, mdio_np); -- 2.51.0