All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next 0/3] mlxsw: small driver update
@ 2015-08-24 14:45 Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 1/3] mlxsw: Remove duplicate included header Jiri Pirko
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Jiri Pirko @ 2015-08-24 14:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, ogerlitz

From: Jiri Pirko <jiri@mellanox.com>

Ido Schimmel (1):
  mlxsw: Remove duplicate included header

Jiri Pirko (2):
  mlxsw: expose EMAD transactions statistics via debugfs
  mlxsw: adjust log messages level in __mlxsw_emad_transmit

 drivers/net/ethernet/mellanox/mlxsw/core.c | 60 ++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 8 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [patch net-next 1/3] mlxsw: Remove duplicate included header
  2015-08-24 14:45 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
@ 2015-08-24 14:45 ` Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 3/3] mlxsw: adjust log messages level in __mlxsw_emad_transmit Jiri Pirko
  2 siblings, 0 replies; 20+ messages in thread
From: Jiri Pirko @ 2015-08-24 14:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, ogerlitz, Jiri Pirko

From: Ido Schimmel <idosch@mellanox.com>

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Elad Raz <eladr@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 09325b7..0415ff6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -48,7 +48,6 @@
 #include <linux/skbuff.h>
 #include <linux/etherdevice.h>
 #include <linux/types.h>
-#include <linux/wait.h>
 #include <linux/string.h>
 #include <linux/gfp.h>
 #include <linux/random.h>
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-24 14:45 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 1/3] mlxsw: Remove duplicate included header Jiri Pirko
@ 2015-08-24 14:45 ` Jiri Pirko
  2015-08-25 21:25   ` David Miller
  2015-08-24 14:45 ` [patch net-next 3/3] mlxsw: adjust log messages level in __mlxsw_emad_transmit Jiri Pirko
  2 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-24 14:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, ogerlitz, Jiri Pirko

From: Jiri Pirko <jiri@mellanox.com>

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Elad Raz <eladr@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 51 ++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 0415ff6..6ee3f45 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -98,6 +98,12 @@ struct mlxsw_core {
 		bool trans_active;
 		struct mutex lock; /* One EMAD transaction at a time. */
 		bool use_emad;
+		struct {
+			u64 trans;
+			u32 fails;
+			u32 retries;
+			u32 timeouts;
+		} stats;
 	} emad;
 	struct mlxsw_core_pcpu_stats __percpu *pcpu_stats;
 	struct dentry *dbg_dir;
@@ -390,6 +396,7 @@ static int __mlxsw_emad_transmit(struct mlxsw_core *mlxsw_core,
 		dev_warn(mlxsw_core->bus_info->dev, "EMAD timed-out (tid=%llx)\n",
 			 mlxsw_core->emad.tid);
 		mlxsw_core->emad.trans_active = false;
+		mlxsw_core->emad.stats.timeouts++;
 		return -EIO;
 	}
 
@@ -463,8 +470,10 @@ retry:
 		if (!err || err != -EAGAIN)
 			goto out;
 	}
-	if (n_retry++ < MLXSW_EMAD_MAX_RETRY)
+	if (n_retry++ < MLXSW_EMAD_MAX_RETRY) {
+		mlxsw_core->emad.stats.retries++;
 		goto retry;
+	}
 
 out:
 	dev_kfree_skb(skb);
@@ -671,6 +680,35 @@ static const struct file_operations mlxsw_core_rx_stats_dbg_ops = {
 	.llseek = seq_lseek
 };
 
+static int mlxsw_core_emad_stats_dbg_read(struct seq_file *file, void *data)
+{
+	struct mlxsw_core *mlxsw_core = file->private;
+
+	if (mutex_lock_interruptible(&mlxsw_core->emad.lock))
+		return -EINTR;
+	seq_printf(file, "transactions: %llu\n", mlxsw_core->emad.stats.trans);
+	seq_printf(file, "fails: %u\n", mlxsw_core->emad.stats.fails);
+	seq_printf(file, "retries: %u\n", mlxsw_core->emad.stats.retries);
+	seq_printf(file, "timeouts: %u\n", mlxsw_core->emad.stats.timeouts);
+	mutex_unlock(&mlxsw_core->emad.lock);
+	return 0;
+}
+
+static int mlxsw_core_emad_stats_dbg_open(struct inode *inode, struct file *f)
+{
+	struct mlxsw_core *mlxsw_core = inode->i_private;
+
+	return single_open(f, mlxsw_core_emad_stats_dbg_read, mlxsw_core);
+}
+
+static const struct file_operations mlxsw_core_emad_stats_dbg_ops = {
+	.owner = THIS_MODULE,
+	.open = mlxsw_core_emad_stats_dbg_open,
+	.release = single_release,
+	.read = seq_read,
+	.llseek = seq_lseek
+};
+
 static void mlxsw_core_buf_dump_dbg(struct mlxsw_core *mlxsw_core,
 				    const char *buf, size_t size)
 {
@@ -768,6 +806,8 @@ static int mlxsw_core_debugfs_init(struct mlxsw_core *mlxsw_core)
 	mlxsw_core->dbg.psid_blob.size = sizeof(bus_info->psid);
 	debugfs_create_blob("psid", S_IRUGO, mlxsw_core->dbg_dir,
 			    &mlxsw_core->dbg.psid_blob);
+	debugfs_create_file("emad_stats", S_IRUGO, mlxsw_core->dbg_dir,
+			    mlxsw_core, &mlxsw_core_emad_stats_dbg_ops);
 	return 0;
 }
 
@@ -1107,8 +1147,10 @@ retry:
 	err = mlxsw_cmd_access_reg(mlxsw_core, in_mbox, out_mbox);
 	if (!err) {
 		err = mlxsw_emad_process_status(mlxsw_core, out_mbox);
-		if (err == -EAGAIN && n_retry++ < MLXSW_EMAD_MAX_RETRY)
+		if (err == -EAGAIN && n_retry++ < MLXSW_EMAD_MAX_RETRY) {
+			mlxsw_core->emad.stats.retries++;
 			goto retry;
+		}
 	}
 
 	if (!err)
@@ -1137,6 +1179,7 @@ static int mlxsw_core_reg_access(struct mlxsw_core *mlxsw_core,
 		return -EINTR;
 	}
 
+	mlxsw_core->emad.stats.trans++;
 	cur_tid = mlxsw_core->emad.tid;
 	dev_dbg(mlxsw_core->bus_info->dev, "Reg access (tid=%llx,reg_id=%x(%s),type=%s)\n",
 		cur_tid, reg->id, mlxsw_reg_id_str(reg->id),
@@ -1153,10 +1196,12 @@ static int mlxsw_core_reg_access(struct mlxsw_core *mlxsw_core,
 		err = mlxsw_core_reg_access_emad(mlxsw_core, reg,
 						 payload, type);
 
-	if (err)
+	if (err) {
 		dev_err(mlxsw_core->bus_info->dev, "Reg access failed (tid=%llx,reg_id=%x(%s),type=%s)\n",
 			cur_tid, reg->id, mlxsw_reg_id_str(reg->id),
 			mlxsw_core_reg_access_type_str(type));
+		mlxsw_core->emad.stats.fails++;
+	}
 
 	mutex_unlock(&mlxsw_core->emad.lock);
 	return err;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [patch net-next 3/3] mlxsw: adjust log messages level in __mlxsw_emad_transmit
  2015-08-24 14:45 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 1/3] mlxsw: Remove duplicate included header Jiri Pirko
  2015-08-24 14:45 ` [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs Jiri Pirko
@ 2015-08-24 14:45 ` Jiri Pirko
  2 siblings, 0 replies; 20+ messages in thread
From: Jiri Pirko @ 2015-08-24 14:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, ogerlitz, Jiri Pirko

From: Jiri Pirko <jiri@mellanox.com>

When transmit fails, it is an error, not a warning.
Do not warn when timeout happens as that is handled by a counter.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Elad Raz <eladr@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 6ee3f45..dfafb83 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -382,8 +382,8 @@ static int __mlxsw_emad_transmit(struct mlxsw_core *mlxsw_core,
 
 	err = mlxsw_core_skb_transmit(mlxsw_core->driver_priv, skb, tx_info);
 	if (err) {
-		dev_warn(mlxsw_core->bus_info->dev, "Failed to transmit EMAD (tid=%llx)\n",
-			 mlxsw_core->emad.tid);
+		dev_err(mlxsw_core->bus_info->dev, "Failed to transmit EMAD (tid=%llx)\n",
+			mlxsw_core->emad.tid);
 		dev_kfree_skb(skb);
 		return err;
 	}
@@ -393,8 +393,8 @@ static int __mlxsw_emad_transmit(struct mlxsw_core *mlxsw_core,
 				 !(mlxsw_core->emad.trans_active),
 				 msecs_to_jiffies(MLXSW_EMAD_TIMEOUT_MS));
 	if (!ret) {
-		dev_warn(mlxsw_core->bus_info->dev, "EMAD timed-out (tid=%llx)\n",
-			 mlxsw_core->emad.tid);
+		dev_dbg(mlxsw_core->bus_info->dev, "EMAD timed-out (tid=%llx)\n",
+			mlxsw_core->emad.tid);
 		mlxsw_core->emad.trans_active = false;
 		mlxsw_core->emad.stats.timeouts++;
 		return -EIO;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-24 14:45 ` [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs Jiri Pirko
@ 2015-08-25 21:25   ` David Miller
  2015-08-26  5:52     ` Jiri Pirko
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2015-08-25 21:25 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, eladr, ogerlitz, jiri

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 24 Aug 2015 16:45:46 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Elad Raz <eladr@mellanox.com>

Enough with this debugfs madness.

Expose this stuff through standard interfaces.

They are simple statistics for crying out loud!

I'm not applying this, and I'm really getting irritated about how much
garbage people put into debugfs when it has _NO_ business being there.

Sorry.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-25 21:25   ` David Miller
@ 2015-08-26  5:52     ` Jiri Pirko
  2015-08-26  6:08       ` David Miller
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-26  5:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, idosch, eladr, ogerlitz, jiri

Tue, Aug 25, 2015 at 11:25:21PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 24 Aug 2015 16:45:46 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> Signed-off-by: Elad Raz <eladr@mellanox.com>
>
>Enough with this debugfs madness.
>
>Expose this stuff through standard interfaces.
>
>They are simple statistics for crying out loud!

They are simple statistics. But they does not fit into any existing
interface. This is about EMAD packets. They are not per-netdevice, but
per-pcidevice. So I cannot put them into ethtool.

I see no other iface to expose this other than debugfs. Please suggest
some other way, I don't see it :/

Thanks.


>
>I'm not applying this, and I'm really getting irritated about how much
>garbage people put into debugfs when it has _NO_ business being there.

I think that is the primary purpose of this iface, To put arbitrary
debugging garbage there. Am I missing something?


>
>Sorry.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26  5:52     ` Jiri Pirko
@ 2015-08-26  6:08       ` David Miller
  2015-08-26  7:37         ` Jiri Pirko
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2015-08-26  6:08 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, eladr, ogerlitz, jiri

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 26 Aug 2015 07:52:15 +0200

> They are simple statistics. But they does not fit into any existing
> interface. This is about EMAD packets. They are not per-netdevice, but
> per-pcidevice. So I cannot put them into ethtool.
> 
> I see no other iface to expose this other than debugfs. Please suggest
> some other way, I don't see it :/

Then create one, instead of crapping up the driver with debugfs
craziness.

>>I'm not applying this, and I'm really getting irritated about how much
>>garbage people put into debugfs when it has _NO_ business being there.
> 
> I think that is the primary purpose of this iface, To put arbitrary
> debugging garbage there. Am I missing something?

It's not garbage if it's useful for someone.

If it's not useful, why even bother?

This is why I hate debugfs, it's a fundamentally flawed facility.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26  6:08       ` David Miller
@ 2015-08-26  7:37         ` Jiri Pirko
  2015-08-26 17:49           ` David Miller
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-26  7:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, idosch, eladr, ogerlitz, jiri

Wed, Aug 26, 2015 at 08:08:21AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Wed, 26 Aug 2015 07:52:15 +0200
>
>> They are simple statistics. But they does not fit into any existing
>> interface. This is about EMAD packets. They are not per-netdevice, but
>> per-pcidevice. So I cannot put them into ethtool.
>> 
>> I see no other iface to expose this other than debugfs. Please suggest
>> some other way, I don't see it :/
>
>Then create one, instead of crapping up the driver with debugfs
>craziness.

I'm not sure it is possible to come up with a generic interface for
arbitraty statistics for some generic PCI device.

I can imagine the pushback saying "hey, put that statistics into subtree
specific area, like netdev etc". And that is correct. In vast majority
of cases, that can be done.

In mlxsw case however, 36 netdevices are sharing 1 pci device. And the
stuff related to that pci device cannot be exposed via netdev.

I don't think that are much more cases like this. Therefore I think that
for this cases, debugfs might be a good way to expose debugging stats.


>
>>>I'm not applying this, and I'm really getting irritated about how much
>>>garbage people put into debugfs when it has _NO_ business being there.
>> 
>> I think that is the primary purpose of this iface, To put arbitrary
>> debugging garbage there. Am I missing something?
>
>It's not garbage if it's useful for someone.
>
>If it's not useful, why even bother?
>
>This is why I hate debugfs, it's a fundamentally flawed facility.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26  7:37         ` Jiri Pirko
@ 2015-08-26 17:49           ` David Miller
  2015-08-26 18:21             ` Scott Feldman
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2015-08-26 17:49 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, eladr, ogerlitz, jiri

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 26 Aug 2015 09:37:57 +0200

> I don't think that are much more cases like this. Therefore I think that
> for this cases, debugfs might be a good way to expose debugging stats.

Scott wanted to do similar things in rocker.  DSA guys too.

Every switch device is going to have some kind of hierarchy like
this, it's not a unique situation.

So I don't buy the "this is a special situation" thing at all.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26 17:49           ` David Miller
@ 2015-08-26 18:21             ` Scott Feldman
  2015-08-26 18:36               ` Florian Fainelli
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Scott Feldman @ 2015-08-26 18:21 UTC (permalink / raw)
  To: David Miller
  Cc: Jiří Pírko, Netdev, Ido Schimmel, eladr, ogerlitz,
	Jiri Pirko

On Wed, Aug 26, 2015 at 10:49 AM, David Miller <davem@davemloft.net> wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Wed, 26 Aug 2015 09:37:57 +0200
>
>> I don't think that are much more cases like this. Therefore I think that
>> for this cases, debugfs might be a good way to expose debugging stats.
>
> Scott wanted to do similar things in rocker.  DSA guys too.
>
> Every switch device is going to have some kind of hierarchy like
> this, it's not a unique situation.

We've been able to get buy so far without a user-visible device for
the switch.  The switch ports are represented by netdevs, so that's
easy.  How can we create an object for the switch itself, so we can
attach common interfaces for the user to dump switch-level stats or
tables?   Using another netdev doesn't seem right.  Do we need a new
device class for switches, and then create some common tool/interfaces
for switch device class?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26 18:21             ` Scott Feldman
@ 2015-08-26 18:36               ` Florian Fainelli
  2015-08-27  0:26               ` David Miller
  2015-08-27  5:40               ` Jiri Pirko
  2 siblings, 0 replies; 20+ messages in thread
From: Florian Fainelli @ 2015-08-26 18:36 UTC (permalink / raw)
  To: Scott Feldman, David Miller
  Cc: Jiří Pírko, Netdev, Ido Schimmel, eladr, ogerlitz,
	Jiri Pirko

On 26/08/15 11:21, Scott Feldman wrote:
> On Wed, Aug 26, 2015 at 10:49 AM, David Miller <davem@davemloft.net> wrote:
>> From: Jiri Pirko <jiri@resnulli.us>
>> Date: Wed, 26 Aug 2015 09:37:57 +0200
>>
>>> I don't think that are much more cases like this. Therefore I think that
>>> for this cases, debugfs might be a good way to expose debugging stats.
>>
>> Scott wanted to do similar things in rocker.  DSA guys too.
>>
>> Every switch device is going to have some kind of hierarchy like
>> this, it's not a unique situation.
> 
> We've been able to get buy so far without a user-visible device for
> the switch.  The switch ports are represented by netdevs, so that's
> easy.  How can we create an object for the switch itself, so we can
> attach common interfaces for the user to dump switch-level stats or
> tables?   Using another netdev doesn't seem right.  Do we need a new
> device class for switches, and then create some common tool/interfaces
> for switch device class?

I agree this is something crucially missing. If we try to list what
could be missing currently, there is mostly:

* switch-wide statistics, tables, databases
* controlling a firmware agent running on the switch
* restarting/re-configuring the switch hardware

All of these already have proper ethtool control interfaces, so using
something that understands a specialized net_device might be the easiest
way to go, but we would need a way to put it in the non network device
name space so tools and users to do not get confused?

We could also have a specific SET_NETDEV_DEVTYPE() which helps make that
specific device be part of a "switch-mgmt" class for instance?
-- 
Florian

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26 18:21             ` Scott Feldman
  2015-08-26 18:36               ` Florian Fainelli
@ 2015-08-27  0:26               ` David Miller
  2015-08-27  5:40               ` Jiri Pirko
  2 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2015-08-27  0:26 UTC (permalink / raw)
  To: sfeldma; +Cc: jiri, netdev, idosch, eladr, ogerlitz, jiri

From: Scott Feldman <sfeldma@gmail.com>
Date: Wed, 26 Aug 2015 11:21:59 -0700

> Using another netdev doesn't seem right.  Do we need a new device
> class for switches, and then create some common tool/interfaces for
> switch device class?

This is probably what we will need to do.

There has also been a discussion lately about making light weight
netdev objects, of which these new switch things can be a super-class
of.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-26 18:21             ` Scott Feldman
  2015-08-26 18:36               ` Florian Fainelli
  2015-08-27  0:26               ` David Miller
@ 2015-08-27  5:40               ` Jiri Pirko
  2015-08-27  6:01                 ` David Miller
  2 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-27  5:40 UTC (permalink / raw)
  To: Scott Feldman
  Cc: David Miller, Netdev, Ido Schimmel, eladr, ogerlitz, Jiri Pirko

Wed, Aug 26, 2015 at 08:21:59PM CEST, sfeldma@gmail.com wrote:
>On Wed, Aug 26, 2015 at 10:49 AM, David Miller <davem@davemloft.net> wrote:
>> From: Jiri Pirko <jiri@resnulli.us>
>> Date: Wed, 26 Aug 2015 09:37:57 +0200
>>
>>> I don't think that are much more cases like this. Therefore I think that
>>> for this cases, debugfs might be a good way to expose debugging stats.
>>
>> Scott wanted to do similar things in rocker.  DSA guys too.
>>
>> Every switch device is going to have some kind of hierarchy like
>> this, it's not a unique situation.
>
>We've been able to get buy so far without a user-visible device for
>the switch.  The switch ports are represented by netdevs, so that's
>easy.  How can we create an object for the switch itself, so we can
>attach common interfaces for the user to dump switch-level stats or
>tables?   Using another netdev doesn't seem right.  Do we need a new
>device class for switches, and then create some common tool/interfaces
>for switch device class?

Switch object itselt would not help you to expose rocker internals. I
don't think that you can find generic way, same for all drivers, to
expose internal tables and stuff. That is hw specific.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-27  5:40               ` Jiri Pirko
@ 2015-08-27  6:01                 ` David Miller
  2015-08-27  6:27                   ` Jiri Pirko
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2015-08-27  6:01 UTC (permalink / raw)
  To: jiri; +Cc: sfeldma, netdev, idosch, eladr, ogerlitz, jiri

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 27 Aug 2015 07:40:04 +0200

> Switch object itselt would not help you to expose rocker internals. I
> don't think that you can find generic way, same for all drivers, to
> expose internal tables and stuff. That is hw specific.

Tables are datastructures with names and types.

Is it not possible to describe datastructures and their types with
user visible interfaces?

Anyone against what I am saying right now is simply lazy.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-27  6:01                 ` David Miller
@ 2015-08-27  6:27                   ` Jiri Pirko
  2015-08-27  6:36                     ` David Miller
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-27  6:27 UTC (permalink / raw)
  To: David Miller; +Cc: sfeldma, netdev, idosch, eladr, ogerlitz, jiri

Thu, Aug 27, 2015 at 08:01:15AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Thu, 27 Aug 2015 07:40:04 +0200
>
>> Switch object itselt would not help you to expose rocker internals. I
>> don't think that you can find generic way, same for all drivers, to
>> expose internal tables and stuff. That is hw specific.
>
>Tables are datastructures with names and types.
>
>Is it not possible to describe datastructures and their types with
>user visible interfaces?

I'm not saying it is not possible, it certainly is. But I think that for
example rocker internals have no value to default user, he should not
care and he cannot find out what is going on there without knowledge or
rocker.c code. The question is, do we need some standard interface to
expose random debugging data? I don't think so, I think that debugfs is
exactly the tool to be used in that case.

>
>Anyone against what I am saying right now is simply lazy.

Not lazy, just thinking :)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-27  6:27                   ` Jiri Pirko
@ 2015-08-27  6:36                     ` David Miller
  2015-08-27  6:40                       ` Jiri Pirko
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2015-08-27  6:36 UTC (permalink / raw)
  To: jiri; +Cc: sfeldma, netdev, idosch, eladr, ogerlitz, jiri

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 27 Aug 2015 08:27:04 +0200

> I'm not saying it is not possible, it certainly is. But I think that
> for example rocker internals have no value to default user, he
> should not care and he cannot find out what is going on there
> without knowledge or rocker.c code. The question is, do we need some
> standard interface to expose random debugging data? I don't think
> so, I think that debugfs is exactly the tool to be used in that
> case.

If it is only interesting to rocker.c maintainer, he can keep a local
patch he applies when he needs such a facility.

This discussion is becomming circular.

If it's useful, it needs a well defined interface.

If it's not useful, it doesn't belong in the tree.

Therefore, debugfs is useless.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-27  6:36                     ` David Miller
@ 2015-08-27  6:40                       ` Jiri Pirko
  2015-09-16 13:14                         ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-08-27  6:40 UTC (permalink / raw)
  To: David Miller; +Cc: sfeldma, netdev, idosch, eladr, ogerlitz, jiri

Thu, Aug 27, 2015 at 08:36:03AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Thu, 27 Aug 2015 08:27:04 +0200
>
>> I'm not saying it is not possible, it certainly is. But I think that
>> for example rocker internals have no value to default user, he
>> should not care and he cannot find out what is going on there
>> without knowledge or rocker.c code. The question is, do we need some
>> standard interface to expose random debugging data? I don't think
>> so, I think that debugfs is exactly the tool to be used in that
>> case.
>
>If it is only interesting to rocker.c maintainer, he can keep a local
>patch he applies when he needs such a facility.
>
>This discussion is becomming circular.
>
>If it's useful, it needs a well defined interface.
>
>If it's not useful, it doesn't belong in the tree.
>
>Therefore, debugfs is useless.

Fair enough.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs
  2015-08-27  6:40                       ` Jiri Pirko
@ 2015-09-16 13:14                         ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 20+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-09-16 13:14 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: David Miller, sfeldma, netdev, idosch, eladr, ogerlitz, jiri

On Thu, Aug 27, 2015 at 08:40:29AM +0200, Jiri Pirko wrote:
> Thu, Aug 27, 2015 at 08:36:03AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Thu, 27 Aug 2015 08:27:04 +0200
> >
> >> I'm not saying it is not possible, it certainly is. But I think that
> >> for example rocker internals have no value to default user, he
> >> should not care and he cannot find out what is going on there
> >> without knowledge or rocker.c code. The question is, do we need some
> >> standard interface to expose random debugging data? I don't think
> >> so, I think that debugfs is exactly the tool to be used in that
> >> case.
> >
> >If it is only interesting to rocker.c maintainer, he can keep a local
> >patch he applies when he needs such a facility.
> >
> >This discussion is becomming circular.
> >
> >If it's useful, it needs a well defined interface.
> >
> >If it's not useful, it doesn't belong in the tree.
> >
> >Therefore, debugfs is useless.
> 
> Fair enough.

Late reply, sorry, but another idea is to leave the stats in place (as
they were going to be calculated even with debugfs unmounted) and (for
now at least) fetch them with systemtap, perf or something like that.
Then the stats are there for when you need them and with an interface as
flexible as it can get. Even if you happen to do a post-mortem analysis,
the info would at least be there.

  Marcelo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch net-next 0/3] mlxsw: small driver update
  2015-11-19 11:27 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
@ 2015-11-20 16:06 ` David Miller
  0 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2015-11-20 16:06 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, eladr, yotamg

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 19 Nov 2015 12:27:37 +0100

> Couple of VLAN-related patches.

Series applied.

I'm really pleased with this driver and work you guys are doing
on it.

Thanks!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [patch net-next 0/3] mlxsw: small driver update
@ 2015-11-19 11:27 Jiri Pirko
  2015-11-20 16:06 ` David Miller
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Pirko @ 2015-11-19 11:27 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg

From: Jiri Pirko <jiri@mellanox.com>

Couple of VLAN-related patches.

Ido Schimmel (3):
  mlxsw: spectrum: Use correct PVID value when removing VLANs
  mlxsw: spectrum: Unify setting of HW VLAN filters
  mlxsw: spectrum: Add error paths to __mlxsw_sp_port_vlans_add

 .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   | 129 ++++++++++++++-------
 1 file changed, 84 insertions(+), 45 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-11-20 16:07 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-24 14:45 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
2015-08-24 14:45 ` [patch net-next 1/3] mlxsw: Remove duplicate included header Jiri Pirko
2015-08-24 14:45 ` [patch net-next 2/3] mlxsw: expose EMAD transactions statistics via debugfs Jiri Pirko
2015-08-25 21:25   ` David Miller
2015-08-26  5:52     ` Jiri Pirko
2015-08-26  6:08       ` David Miller
2015-08-26  7:37         ` Jiri Pirko
2015-08-26 17:49           ` David Miller
2015-08-26 18:21             ` Scott Feldman
2015-08-26 18:36               ` Florian Fainelli
2015-08-27  0:26               ` David Miller
2015-08-27  5:40               ` Jiri Pirko
2015-08-27  6:01                 ` David Miller
2015-08-27  6:27                   ` Jiri Pirko
2015-08-27  6:36                     ` David Miller
2015-08-27  6:40                       ` Jiri Pirko
2015-09-16 13:14                         ` Marcelo Ricardo Leitner
2015-08-24 14:45 ` [patch net-next 3/3] mlxsw: adjust log messages level in __mlxsw_emad_transmit Jiri Pirko
2015-11-19 11:27 [patch net-next 0/3] mlxsw: small driver update Jiri Pirko
2015-11-20 16:06 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.