* [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
@ 2016-04-27 18:45 Florian Fainelli
2016-04-27 19:03 ` Andrew Lunn
2016-04-28 21:16 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2016-04-27 18:45 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
include switch-side statistics, which is useful for debugging purposes,
when the switch is not properly connected to the Ethernet MAC (duplex
mismatch, (RG)MII electrical issues etc.).
We accomplish this by retaining the original copy of the master netdev's
ethtool_ops, and just overload the 3 operations we care about:
get_sset_count, get_strings and get_ethtool_stats so as to intercept
these calls and call into the original master_netdev ethtool_ops, plus
our own.
We take this approach as opposed to providing a set of DSA helper
functions that would retrive the CPU port's statistics, because the
entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
used as CPU conduit interfaces, therefore, statistics overlay in such
drivers would simply not scale.
The new ethtool -S <iface> output would therefore look like this now:
<iface> statistics
p<2 digits cpu port number>_<switch MIB counter names>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes from RFC:
- prepend the CPU port as a prefix to make it clear what the stats are
about, master netdev interface stats are unchanged
include/net/dsa.h | 5 ++++
net/dsa/slave.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d280aba97e2..8e86af87c84f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -111,6 +111,11 @@ struct dsa_switch_tree {
enum dsa_tag_protocol tag_protocol;
/*
+ * Original copy of the master netdev ethtool_ops
+ */
+ struct ethtool_ops master_ethtool_ops;
+
+ /*
* The switch and port to which the CPU is attached.
*/
s8 cpu_switch;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3b6750f5e68b..5ea8a40c8d33 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -666,6 +666,78 @@ static void dsa_slave_get_strings(struct net_device *dev,
}
}
+static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats,
+ uint64_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ s8 cpu_port = dst->cpu_port;
+ int count = 0;
+
+ if (dst->master_ethtool_ops.get_sset_count) {
+ count = dst->master_ethtool_ops.get_sset_count(dev,
+ ETH_SS_STATS);
+ dst->master_ethtool_ops.get_ethtool_stats(dev, stats, data);
+ }
+
+ if (ds->drv->get_ethtool_stats)
+ ds->drv->get_ethtool_stats(ds, cpu_port, data + count);
+}
+
+static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ int count = 0;
+
+ if (dst->master_ethtool_ops.get_sset_count)
+ count += dst->master_ethtool_ops.get_sset_count(dev, sset);
+
+ if (sset == ETH_SS_STATS && ds->drv->get_sset_count)
+ count += ds->drv->get_sset_count(ds);
+
+ return count;
+}
+
+static void dsa_cpu_port_get_strings(struct net_device *dev,
+ uint32_t stringset, uint8_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ s8 cpu_port = dst->cpu_port;
+ int len = ETH_GSTRING_LEN;
+ int mcount = 0, count;
+ unsigned int i;
+ uint8_t pfx[4];
+ uint8_t *ndata;
+
+ snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
+ /* We do not want to be NULL-terminated, since this is a prefix */
+ pfx[sizeof(pfx) - 1] = '_';
+
+ if (dst->master_ethtool_ops.get_sset_count) {
+ mcount = dst->master_ethtool_ops.get_sset_count(dev,
+ ETH_SS_STATS);
+ dst->master_ethtool_ops.get_strings(dev, stringset, data);
+ }
+
+ if (stringset == ETH_SS_STATS && ds->drv->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->drv->get_strings(ds, cpu_port, ndata);
+ count = ds->drv->get_sset_count(ds);
+ 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));
+ }
+ }
+}
+
static void dsa_slave_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats,
uint64_t *data)
@@ -821,6 +893,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_eee = dsa_slave_get_eee,
};
+static struct ethtool_ops dsa_cpu_port_ethtool_ops;
+
static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_open = dsa_slave_open,
.ndo_stop = dsa_slave_close,
@@ -1038,6 +1112,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
int port, char *name)
{
struct net_device *master = ds->dst->master_netdev;
+ struct dsa_switch_tree *dst = ds->dst;
struct net_device *slave_dev;
struct dsa_slave_priv *p;
int ret;
@@ -1049,6 +1124,19 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
slave_dev->features = master->vlan_features;
slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
+ if (master->ethtool_ops != &dsa_cpu_port_ethtool_ops) {
+ memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
+ sizeof(struct ethtool_ops));
+ memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
+ sizeof(struct ethtool_ops));
+ dsa_cpu_port_ethtool_ops.get_sset_count =
+ dsa_cpu_port_get_sset_count;
+ dsa_cpu_port_ethtool_ops.get_ethtool_stats =
+ dsa_cpu_port_get_ethtool_stats;
+ dsa_cpu_port_ethtool_ops.get_strings =
+ dsa_cpu_port_get_strings;
+ master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
+ }
eth_hw_addr_inherit(slave_dev, master);
slave_dev->priv_flags |= IFF_NO_QUEUE;
slave_dev->netdev_ops = &dsa_slave_netdev_ops;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
2016-04-27 18:45 [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev Florian Fainelli
@ 2016-04-27 19:03 ` Andrew Lunn
2016-04-27 21:43 ` Florian Fainelli
2016-04-28 21:16 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2016-04-27 19:03 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
> + if (stringset == ETH_SS_STATS && ds->drv->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->drv->get_strings(ds, cpu_port, ndata);
> + count = ds->drv->get_sset_count(ds);
> + 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));
Hi Florian
Did you check what happens if this causes the NULL terminator to be
discarded? Does ethtool handle that? As i said before, it is unclear
if one is required.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
2016-04-27 19:03 ` Andrew Lunn
@ 2016-04-27 21:43 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2016-04-27 21:43 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, davem, vivien.didelot
On 27/04/16 12:03, Andrew Lunn wrote:
>> + if (stringset == ETH_SS_STATS && ds->drv->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->drv->get_strings(ds, cpu_port, ndata);
>> + count = ds->drv->get_sset_count(ds);
>> + 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));
>
> Hi Florian
>
> Did you check what happens if this causes the NULL terminator to be
> discarded? Does ethtool handle that? As i said before, it is unclear
> if one is required.
I just did yes. So ethtool has a do_gstringset() function which
NULL-terminates every strings set except the statistics kind
(ETH_SS_STATS or ETH_SS_PHY_STATS) but this is not much of a problem
because it limits the output to ETH_GSTRING_LEN anyway.
After injecting a bit of error in net/dsa/slave.c to have a much bigger
prefix making us push the stats names, the stats are correcty truncated
by ethtool. So we seem to be good to go with the current code in kernel
and user space.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
2016-04-27 18:45 [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev Florian Fainelli
2016-04-27 19:03 ` Andrew Lunn
@ 2016-04-28 21:16 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-04-28 21:16 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 27 Apr 2016 11:45:14 -0700
> This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
> include switch-side statistics, which is useful for debugging purposes,
> when the switch is not properly connected to the Ethernet MAC (duplex
> mismatch, (RG)MII electrical issues etc.).
>
> We accomplish this by retaining the original copy of the master netdev's
> ethtool_ops, and just overload the 3 operations we care about:
> get_sset_count, get_strings and get_ethtool_stats so as to intercept
> these calls and call into the original master_netdev ethtool_ops, plus
> our own.
>
> We take this approach as opposed to providing a set of DSA helper
> functions that would retrive the CPU port's statistics, because the
> entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
> used as CPU conduit interfaces, therefore, statistics overlay in such
> drivers would simply not scale.
>
> The new ethtool -S <iface> output would therefore look like this now:
> <iface> statistics
> p<2 digits cpu port number>_<switch MIB counter names>
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks Florian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
2016-06-04 0:05 [PATCH net-next 0/9] net: dsa: misc improvements Florian Fainelli
@ 2016-06-04 0:05 ` Florian Fainelli
2016-06-04 0:06 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2016-06-04 0:05 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, john, Florian Fainelli
This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
include switch-side statistics, which is useful for debugging purposes,
when the switch is not properly connected to the Ethernet MAC (duplex
mismatch, (RG)MII electrical issues etc.).
We accomplish this by retaining the original copy of the master netdev's
ethtool_ops, and just overload the 3 operations we care about:
get_sset_count, get_strings and get_ethtool_stats so as to intercept
these calls and call into the original master_netdev ethtool_ops, plus
our own.
We take this approach as opposed to providing a set of DSA helper
functions that would retrive the CPU port's statistics, because the
entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
used as CPU conduit interfaces, therefore, statistics overlay in such
drivers would simply not scale.
The new ethtool -S <iface> output would therefore look like this now:
<iface> statistics
p<2 digits cpu port number>_<switch MIB counter names>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes from RFC:
- prepend the CPU port as a prefix to make it clear what the stats are
about, master netdev interface stats are unchanged
include/net/dsa.h | 5 ++++
net/dsa/slave.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d280aba97e2..8e86af87c84f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -111,6 +111,11 @@ struct dsa_switch_tree {
enum dsa_tag_protocol tag_protocol;
/*
+ * Original copy of the master netdev ethtool_ops
+ */
+ struct ethtool_ops master_ethtool_ops;
+
+ /*
* The switch and port to which the CPU is attached.
*/
s8 cpu_switch;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3b6750f5e68b..5ea8a40c8d33 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -666,6 +666,78 @@ static void dsa_slave_get_strings(struct net_device *dev,
}
}
+static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats,
+ uint64_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ s8 cpu_port = dst->cpu_port;
+ int count = 0;
+
+ if (dst->master_ethtool_ops.get_sset_count) {
+ count = dst->master_ethtool_ops.get_sset_count(dev,
+ ETH_SS_STATS);
+ dst->master_ethtool_ops.get_ethtool_stats(dev, stats, data);
+ }
+
+ if (ds->drv->get_ethtool_stats)
+ ds->drv->get_ethtool_stats(ds, cpu_port, data + count);
+}
+
+static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ int count = 0;
+
+ if (dst->master_ethtool_ops.get_sset_count)
+ count += dst->master_ethtool_ops.get_sset_count(dev, sset);
+
+ if (sset == ETH_SS_STATS && ds->drv->get_sset_count)
+ count += ds->drv->get_sset_count(ds);
+
+ return count;
+}
+
+static void dsa_cpu_port_get_strings(struct net_device *dev,
+ uint32_t stringset, uint8_t *data)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds = dst->ds[0];
+ s8 cpu_port = dst->cpu_port;
+ int len = ETH_GSTRING_LEN;
+ int mcount = 0, count;
+ unsigned int i;
+ uint8_t pfx[4];
+ uint8_t *ndata;
+
+ snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
+ /* We do not want to be NULL-terminated, since this is a prefix */
+ pfx[sizeof(pfx) - 1] = '_';
+
+ if (dst->master_ethtool_ops.get_sset_count) {
+ mcount = dst->master_ethtool_ops.get_sset_count(dev,
+ ETH_SS_STATS);
+ dst->master_ethtool_ops.get_strings(dev, stringset, data);
+ }
+
+ if (stringset == ETH_SS_STATS && ds->drv->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->drv->get_strings(ds, cpu_port, ndata);
+ count = ds->drv->get_sset_count(ds);
+ 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));
+ }
+ }
+}
+
static void dsa_slave_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats,
uint64_t *data)
@@ -821,6 +893,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_eee = dsa_slave_get_eee,
};
+static struct ethtool_ops dsa_cpu_port_ethtool_ops;
+
static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_open = dsa_slave_open,
.ndo_stop = dsa_slave_close,
@@ -1038,6 +1112,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
int port, char *name)
{
struct net_device *master = ds->dst->master_netdev;
+ struct dsa_switch_tree *dst = ds->dst;
struct net_device *slave_dev;
struct dsa_slave_priv *p;
int ret;
@@ -1049,6 +1124,19 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
slave_dev->features = master->vlan_features;
slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
+ if (master->ethtool_ops != &dsa_cpu_port_ethtool_ops) {
+ memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
+ sizeof(struct ethtool_ops));
+ memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
+ sizeof(struct ethtool_ops));
+ dsa_cpu_port_ethtool_ops.get_sset_count =
+ dsa_cpu_port_get_sset_count;
+ dsa_cpu_port_ethtool_ops.get_ethtool_stats =
+ dsa_cpu_port_get_ethtool_stats;
+ dsa_cpu_port_ethtool_ops.get_strings =
+ dsa_cpu_port_get_strings;
+ master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
+ }
eth_hw_addr_inherit(slave_dev, master);
slave_dev->priv_flags |= IFF_NO_QUEUE;
slave_dev->netdev_ops = &dsa_slave_netdev_ops;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev
2016-06-04 0:05 ` [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev Florian Fainelli
@ 2016-06-04 0:06 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2016-06-04 0:06 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, john
On 06/03/2016 05:05 PM, Florian Fainelli wrote:
> This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
> include switch-side statistics, which is useful for debugging purposes,
> when the switch is not properly connected to the Ethernet MAC (duplex
> mismatch, (RG)MII electrical issues etc.).
>
> We accomplish this by retaining the original copy of the master netdev's
> ethtool_ops, and just overload the 3 operations we care about:
> get_sset_count, get_strings and get_ethtool_stats so as to intercept
> these calls and call into the original master_netdev ethtool_ops, plus
> our own.
>
> We take this approach as opposed to providing a set of DSA helper
> functions that would retrive the CPU port's statistics, because the
> entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
> used as CPU conduit interfaces, therefore, statistics overlay in such
> drivers would simply not scale.
Disregard this patch, it was left in the patches folder...
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-04 0:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-27 18:45 [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev Florian Fainelli
2016-04-27 19:03 ` Andrew Lunn
2016-04-27 21:43 ` Florian Fainelli
2016-04-28 21:16 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2016-06-04 0:05 [PATCH net-next 0/9] net: dsa: misc improvements Florian Fainelli
2016-06-04 0:05 ` [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev Florian Fainelli
2016-06-04 0:06 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).