In theory this would have been seen by now, but it seems that all drivers used as DSA conduit interfaces thus far have had ethtool_ops set, and it's hard to even find modern Ethernet drivers (and not VF ones) which don't use ethtool. Here is the unfiltered list of drivers which register any sort of net_device but don't set its ethtool_ops pointer. I don't think any of them 'risks' being used as a DSA conduit, maybe except for moxart, rnpbge and icssm, I'm not sure. - drivers/net/can/dev/dev.c - drivers/net/wwan/qcom_bam_dmux.c - drivers/net/wwan/t7xx/t7xx_netdev.c - drivers/net/arcnet/arcnet.c - drivers/net/hamradio/ - drivers/net/slip/slip.c - drivers/net/ethernet/ezchip/nps_enet.c - drivers/net/ethernet/moxa/moxart_ether.c - drivers/net/ethernet/wangxun/txgbevf/txgbevf_main.c - drivers/net/ethernet/wangxun/ngbevf/ngbevf_main.c - drivers/net/ethernet/huawei/hinic3/hinic3_main.c - drivers/net/ethernet/i825xx/ - drivers/net/ethernet/ti/icssm/icssm_prueth.c - drivers/net/ethernet/seeq/ - drivers/net/ethernet/litex/litex_liteeth.c - drivers/net/ethernet/sunplus/spl2sw_driver.c - drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c - drivers/net/ipa/ - drivers/net/wireless/microchip/wilc1000/ - drivers/net/wireless/mediatek/mt76/dma.c - drivers/net/wireless/ath/ath12k/ - drivers/net/wireless/ath/ath11k/ - drivers/net/wireless/ath/ath6kl/ - drivers/net/wireless/ath/ath10k/ - drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c - drivers/net/wireless/virtual/mac80211_hwsim.c - drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c - drivers/net/wireless/realtek/rtw89/core.c - drivers/net/wireless/realtek/rtw88/pci.c - drivers/net/caif/ - drivers/net/plip/ - drivers/net/wan/ - drivers/net/mctp/ - drivers/net/ppp/ - drivers/net/thunderbolt/ Nonetheless, it's good for the framework not to make such assumptions, and not panic when coming across such kind of host device in the future. Signed-off-by: Vladimir Oltean --- net/dsa/conduit.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/dsa/conduit.c b/net/dsa/conduit.c index 4ae255cfb23f..f80795b3d046 100644 --- a/net/dsa/conduit.c +++ b/net/dsa/conduit.c @@ -26,7 +26,7 @@ static int dsa_conduit_get_regs_len(struct net_device *dev) int ret = 0; int len; - if (ops->get_regs_len) { + if (ops && ops->get_regs_len) { netdev_lock_ops(dev); len = ops->get_regs_len(dev); netdev_unlock_ops(dev); @@ -59,7 +59,7 @@ static void dsa_conduit_get_regs(struct net_device *dev, int port = cpu_dp->index; int len; - if (ops->get_regs_len && ops->get_regs) { + if (ops && ops->get_regs_len && ops->get_regs) { netdev_lock_ops(dev); len = ops->get_regs_len(dev); if (len < 0) { @@ -97,7 +97,7 @@ static void dsa_conduit_get_ethtool_stats(struct net_device *dev, int port = cpu_dp->index; int count = 0; - if (ops->get_sset_count && ops->get_ethtool_stats) { + if (ops && ops->get_sset_count && ops->get_ethtool_stats) { netdev_lock_ops(dev); count = ops->get_sset_count(dev, ETH_SS_STATS); ops->get_ethtool_stats(dev, stats, data); @@ -118,11 +118,11 @@ static void dsa_conduit_get_ethtool_phy_stats(struct net_device *dev, int port = cpu_dp->index; int count = 0; - if (dev->phydev && !ops->get_ethtool_phy_stats) { + if (dev->phydev && (!ops || !ops->get_ethtool_phy_stats)) { count = phy_ethtool_get_sset_count(dev->phydev); if (count >= 0) phy_ethtool_get_stats(dev->phydev, stats, data); - } else if (ops->get_sset_count && ops->get_ethtool_phy_stats) { + } else if (ops && ops->get_sset_count && ops->get_ethtool_phy_stats) { netdev_lock_ops(dev); count = ops->get_sset_count(dev, ETH_SS_PHY_STATS); ops->get_ethtool_phy_stats(dev, stats, data); @@ -145,9 +145,9 @@ static int dsa_conduit_get_sset_count(struct net_device *dev, int sset) netdev_lock_ops(dev); if (sset == ETH_SS_PHY_STATS && dev->phydev && - !ops->get_ethtool_phy_stats) + (!ops || !ops->get_ethtool_phy_stats)) count = phy_ethtool_get_sset_count(dev->phydev); - else if (ops->get_sset_count) + else if (ops && ops->get_sset_count) count = ops->get_sset_count(dev, sset); netdev_unlock_ops(dev); -- 2.34.1 Suppress some checkpatch 'CHECK' messages about u8 being preferable over uint8_t, etc. No functional change. Signed-off-by: Vladimir Oltean --- net/dsa/conduit.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/net/dsa/conduit.c b/net/dsa/conduit.c index f80795b3d046..c210e3129655 100644 --- a/net/dsa/conduit.c +++ b/net/dsa/conduit.c @@ -89,7 +89,7 @@ static void dsa_conduit_get_regs(struct net_device *dev, static void dsa_conduit_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *stats, - uint64_t *data) + u64 *data) { struct dsa_port *cpu_dp = dev->dsa_ptr; const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; @@ -110,7 +110,7 @@ static void dsa_conduit_get_ethtool_stats(struct net_device *dev, static void dsa_conduit_get_ethtool_phy_stats(struct net_device *dev, struct ethtool_stats *stats, - uint64_t *data) + u64 *data) { struct dsa_port *cpu_dp = dev->dsa_ptr; const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; @@ -160,8 +160,8 @@ static int dsa_conduit_get_sset_count(struct net_device *dev, int sset) return count; } -static void dsa_conduit_get_strings(struct net_device *dev, uint32_t stringset, - uint8_t *data) +static void dsa_conduit_get_strings(struct net_device *dev, u32 stringset, + u8 *data) { struct dsa_port *cpu_dp = dev->dsa_ptr; const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; @@ -169,8 +169,7 @@ static void dsa_conduit_get_strings(struct net_device *dev, uint32_t stringset, int port = cpu_dp->index; int len = ETH_GSTRING_LEN; int mcount = 0, count, i; - uint8_t pfx[4]; - uint8_t *ndata; + u8 pfx[4], *ndata; snprintf(pfx, sizeof(pfx), "p%.2d", port); /* We do not want to be NULL-terminated, since this is a prefix */ -- 2.34.1 Currently there is no way to see packet counters on cascade ports, and no clarity on how the API for that would look like. Because it's something that is currently needed, just extend the hack where ethtool -S on the conduit interface dumps CPU port counters, and also use it to dump counters of cascade ports. Note that the "pXX_" naming convention changes to "sXX_pYY", to distinguish between ports having the same index but belonging to different switches. This has a slight chance of causing regressions to existing tooling: - grepping for "p04_counter_name" still works, but might return more than one string now - grepping for " p04_counter_name" no longer works Signed-off-by: Vladimir Oltean --- net/dsa/conduit.c | 126 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 93 insertions(+), 33 deletions(-) diff --git a/net/dsa/conduit.c b/net/dsa/conduit.c index c210e3129655..a1b044467bd6 100644 --- a/net/dsa/conduit.c +++ b/net/dsa/conduit.c @@ -87,25 +87,51 @@ static void dsa_conduit_get_regs(struct net_device *dev, } } +static ssize_t dsa_conduit_append_port_stats(struct dsa_switch *ds, int port, + u64 *data, size_t start) +{ + int count; + + if (!ds->ops->get_sset_count) + return 0; + + count = ds->ops->get_sset_count(ds, port, ETH_SS_STATS); + if (count < 0) + return count; + + if (ds->ops->get_ethtool_stats) + ds->ops->get_ethtool_stats(ds, port, data + start); + + return count; +} + static void dsa_conduit_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *stats, u64 *data) { - struct dsa_port *cpu_dp = dev->dsa_ptr; + struct dsa_port *dp, *cpu_dp = dev->dsa_ptr; const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; - struct dsa_switch *ds = cpu_dp->ds; - int port = cpu_dp->index; - int count = 0; + struct dsa_switch_tree *dst = cpu_dp->dst; + int count, mcount = 0; if (ops && ops->get_sset_count && ops->get_ethtool_stats) { netdev_lock_ops(dev); - count = ops->get_sset_count(dev, ETH_SS_STATS); + mcount = ops->get_sset_count(dev, ETH_SS_STATS); ops->get_ethtool_stats(dev, stats, data); netdev_unlock_ops(dev); } - if (ds->ops->get_ethtool_stats) - ds->ops->get_ethtool_stats(ds, port, data + count); + list_for_each_entry(dp, &dst->ports, list) { + if (!dsa_port_is_dsa(dp) && !dsa_port_is_cpu(dp)) + continue; + + count = dsa_conduit_append_port_stats(dp->ds, dp->index, + data, mcount); + if (count < 0) + return; + + mcount += count; + } } static void dsa_conduit_get_ethtool_phy_stats(struct net_device *dev, @@ -136,11 +162,18 @@ static void dsa_conduit_get_ethtool_phy_stats(struct net_device *dev, ds->ops->get_ethtool_phy_stats(ds, port, data + count); } +static void dsa_conduit_append_port_sset_count(struct dsa_switch *ds, int port, + int sset, int *count) +{ + if (ds->ops->get_sset_count) + *count += ds->ops->get_sset_count(ds, port, sset); +} + static int dsa_conduit_get_sset_count(struct net_device *dev, int sset) { - struct dsa_port *cpu_dp = dev->dsa_ptr; + struct dsa_port *dp, *cpu_dp = dev->dsa_ptr; const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; - struct dsa_switch *ds = cpu_dp->ds; + struct dsa_switch_tree *dst = cpu_dp->dst; int count = 0; netdev_lock_ops(dev); @@ -154,26 +187,57 @@ static int dsa_conduit_get_sset_count(struct net_device *dev, int sset) if (count < 0) count = 0; - if (ds->ops->get_sset_count) - count += ds->ops->get_sset_count(ds, cpu_dp->index, sset); + list_for_each_entry(dp, &dst->ports, list) { + if (!dsa_port_is_dsa(dp) && !dsa_port_is_cpu(dp)) + continue; + + dsa_conduit_append_port_sset_count(dp->ds, dp->index, sset, + &count); + } return count; } -static void dsa_conduit_get_strings(struct net_device *dev, u32 stringset, - u8 *data) +static ssize_t dsa_conduit_append_port_strings(struct dsa_switch *ds, int port, + u32 stringset, u8 *data, + size_t start) { - struct dsa_port *cpu_dp = dev->dsa_ptr; - const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; - struct dsa_switch *ds = cpu_dp->ds; - int port = cpu_dp->index; int len = ETH_GSTRING_LEN; - int mcount = 0, count, i; - u8 pfx[4], *ndata; + u8 pfx[8], *ndata; + int count, i; + + if (!ds->ops->get_strings) + return 0; - snprintf(pfx, sizeof(pfx), "p%.2d", port); + snprintf(pfx, sizeof(pfx), "s%.2d_p%.2d", ds->index, port); /* We do not want to be NULL-terminated, since this is a prefix */ pfx[sizeof(pfx) - 1] = '_'; + ndata = data + start * len; + /* This function copies ETH_GSTRINGS_LEN bytes, we will mangle + * the output after to prepend our CPU port prefix we + * constructed earlier + */ + ds->ops->get_strings(ds, port, stringset, ndata); + count = ds->ops->get_sset_count(ds, port, stringset); + if (count < 0) + return count; + + for (i = 0; i < count; i++) { + memmove(ndata + (i * len + sizeof(pfx)), + ndata + i * len, len - sizeof(pfx)); + memcpy(ndata + i * len, pfx, sizeof(pfx)); + } + + return count; +} + +static void dsa_conduit_get_strings(struct net_device *dev, u32 stringset, + u8 *data) +{ + struct dsa_port *dp, *cpu_dp = dev->dsa_ptr; + const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops; + struct dsa_switch_tree *dst = cpu_dp->dst; + int count, mcount = 0; netdev_lock_ops(dev); if (stringset == ETH_SS_PHY_STATS && dev->phydev && @@ -191,21 +255,17 @@ static void dsa_conduit_get_strings(struct net_device *dev, u32 stringset, } netdev_unlock_ops(dev); - if (ds->ops->get_strings) { - ndata = data + mcount * len; - /* This function copies ETH_GSTRINGS_LEN bytes, we will mangle - * the output after to prepend our CPU port prefix we - * constructed earlier - */ - ds->ops->get_strings(ds, port, stringset, ndata); - count = ds->ops->get_sset_count(ds, port, stringset); + list_for_each_entry(dp, &dst->ports, list) { + if (!dsa_port_is_dsa(dp) && !dsa_port_is_cpu(dp)) + continue; + + count = dsa_conduit_append_port_strings(dp->ds, dp->index, + stringset, data, + mcount); if (count < 0) return; - for (i = 0; i < count; i++) { - memmove(ndata + (i * len + sizeof(pfx)), - ndata + i * len, len - sizeof(pfx)); - memcpy(ndata + i * len, pfx, sizeof(pfx)); - } + + mcount += count; } } -- 2.34.1