* [PATCH net-next 0/2] Add ATU/VTU statistics @ 2018-03-27 21:59 Andrew Lunn 2018-03-27 21:59 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics Andrew Lunn 2018-03-27 21:59 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy Andrew Lunn 0 siblings, 2 replies; 8+ messages in thread From: Andrew Lunn @ 2018-03-27 21:59 UTC (permalink / raw) To: David Miller; +Cc: netdev, Andrew Lunn Previous patches have added basic support for Address Translation Unit and VLAN translation Unit violation interrupts. Add statistics counters for when these occur, which can be accessed using ethtool. Remove one of the particularly spammy warnings from VTU violations, not that we have a counter for it. Andrew Lunn (2): net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics net: dsa: mv88e6xxx: Make VTU miss violations less spammy drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++----- drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 +++--- drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- 6 files changed, 77 insertions(+), 29 deletions(-) -- 2.16.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics 2018-03-27 21:59 [PATCH net-next 0/2] Add ATU/VTU statistics Andrew Lunn @ 2018-03-27 21:59 ` Andrew Lunn 2018-03-28 18:17 ` Florian Fainelli 2018-03-27 21:59 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy Andrew Lunn 1 sibling, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2018-03-27 21:59 UTC (permalink / raw) To: David Miller; +Cc: netdev, Andrew Lunn Count the numbers of various ATU and VTU violation statistics and return them as part of the ethtool -S statistics. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++----- drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++-- drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- 6 files changed, 78 insertions(+), 28 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 9a5d786b4885..186021f98c5d 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip, STATS_TYPE_BANK0 | STATS_TYPE_BANK1); } +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = { + "atu_member_violation", + "atu_miss_violation", + "atu_full_violation", + "vtu_member_violation", + "vtu_miss_violation", +}; + +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) + strlcpy(data + i * ETH_GSTRING_LEN, + mv88e6xxx_atu_vtu_stats_strings[i], + ETH_GSTRING_LEN); +} + static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, uint8_t *data) { @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, if (chip->info->ops->serdes_get_strings) { data += count * ETH_GSTRING_LEN; - chip->info->ops->serdes_get_strings(chip, port, data); + count = chip->info->ops->serdes_get_strings(chip, port, data); } + data += count * ETH_GSTRING_LEN; + mv88e6xxx_atu_vtu_get_strings(data); + mutex_unlock(&chip->reg_lock); } @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port) if (chip->info->ops->serdes_get_sset_count) serdes_count = chip->info->ops->serdes_get_sset_count(chip, port); - if (serdes_count < 0) + if (serdes_count < 0) { count = serdes_count; - else - count += serdes_count; + goto out; + } + count += serdes_count; + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); + out: mutex_unlock(&chip->reg_lock); @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port, 0); } +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port, + uint64_t *data) +{ + *data++ = chip->ports[port].atu_member_violation; + *data++ = chip->ports[port].atu_miss_violation; + *data++ = chip->ports[port].atu_full_violation; + *data++ = chip->ports[port].vtu_member_violation; + *data++ = chip->ports[port].vtu_miss_violation; +} + static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port, uint64_t *data) { @@ -849,12 +883,14 @@ static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port, if (chip->info->ops->stats_get_stats) count = chip->info->ops->stats_get_stats(chip, port, data); + mutex_lock(&chip->reg_lock); if (chip->info->ops->serdes_get_stats) { data += count; - mutex_lock(&chip->reg_lock); - chip->info->ops->serdes_get_stats(chip, port, data); - mutex_unlock(&chip->reg_lock); + count = chip->info->ops->serdes_get_stats(chip, port, data); } + data += count; + mv88e6xxx_atu_vtu_get_stats(chip, port, data); + mutex_unlock(&chip->reg_lock); } static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index bad211014e91..80490f66bc06 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -194,6 +194,11 @@ struct mv88e6xxx_port_hwtstamp { struct mv88e6xxx_port { u64 serdes_stats[2]; + u64 atu_member_violation; + u64 atu_miss_violation; + u64 atu_full_violation; + u64 vtu_member_violation; + u64 vtu_miss_violation; }; struct mv88e6xxx_chip { @@ -409,10 +414,10 @@ struct mv88e6xxx_ops { /* Statistics from the SERDES interface */ int (*serdes_get_sset_count)(struct mv88e6xxx_chip *chip, int port); - void (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port, - uint8_t *data); - void (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port, - uint64_t *data); + int (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port, + uint8_t *data); + int (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port, + uint64_t *data); /* VLAN Translation Unit operations */ int (*vtu_getnext)(struct mv88e6xxx_chip *chip, diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c index 20d941f4273b..307410898fc9 100644 --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c @@ -336,8 +336,6 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id) if (err) goto out; - mutex_unlock(&chip->reg_lock); - if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) { dev_err_ratelimited(chip->dev, "ATU age out violation for %pM\n", @@ -348,17 +346,23 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id) dev_err_ratelimited(chip->dev, "ATU member violation for %pM portvec %x\n", entry.mac, entry.portvec); + chip->ports[entry.portvec].atu_member_violation++; } - if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) + if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) { dev_err_ratelimited(chip->dev, "ATU miss violation for %pM portvec %x\n", entry.mac, entry.portvec); + chip->ports[entry.portvec].atu_miss_violation++; + } - if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) + if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) { dev_err_ratelimited(chip->dev, "ATU full violation for %pM portvec %x\n", entry.mac, entry.portvec); + chip->ports[entry.portvec].atu_full_violation++; + } + mutex_unlock(&chip->reg_lock); return IRQ_HANDLED; diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c index 7997961647de..2cbaf946e7ed 100644 --- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c +++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c @@ -539,18 +539,20 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id) if (err) goto out; - mutex_unlock(&chip->reg_lock); - spid = val & MV88E6XXX_G1_VTU_OP_SPID_MASK; if (val & MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION) { dev_err_ratelimited(chip->dev, "VTU member violation for vid %d, source port %d\n", entry.vid, spid); + chip->ports[spid].vtu_member_violation++; } - if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) + if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) { dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n", entry.vid, spid); + chip->ports[spid].vtu_miss_violation++; + } + mutex_unlock(&chip->reg_lock); return IRQ_HANDLED; diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c index b6166424216a..fb058fd35c0d 100644 --- a/drivers/net/dsa/mv88e6xxx/serdes.c +++ b/drivers/net/dsa/mv88e6xxx/serdes.c @@ -106,20 +106,21 @@ int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port) return 0; } -void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip, - int port, uint8_t *data) +int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip, + int port, uint8_t *data) { struct mv88e6352_serdes_hw_stat *stat; int i; if (!mv88e6352_port_has_serdes(chip, port)) - return; + return 0; for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) { stat = &mv88e6352_serdes_hw_stats[i]; memcpy(data + i * ETH_GSTRING_LEN, stat->string, ETH_GSTRING_LEN); } + return ARRAY_SIZE(mv88e6352_serdes_hw_stats); } static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip, @@ -149,8 +150,8 @@ static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip, return val; } -void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, - uint64_t *data) +int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, + uint64_t *data) { struct mv88e6xxx_port *mv88e6xxx_port = &chip->ports[port]; struct mv88e6352_serdes_hw_stat *stat; @@ -158,7 +159,7 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, int i; if (!mv88e6352_port_has_serdes(chip, port)) - return; + return 0; BUILD_BUG_ON(ARRAY_SIZE(mv88e6352_serdes_hw_stats) > ARRAY_SIZE(mv88e6xxx_port->serdes_stats)); @@ -169,6 +170,8 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, mv88e6xxx_port->serdes_stats[i] += value; data[i] = mv88e6xxx_port->serdes_stats[i]; } + + return ARRAY_SIZE(mv88e6352_serdes_hw_stats); } /* Set the power on/off for 10GBASE-R and 10GBASE-X4/X2 */ diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h index 641baa75f910..1897c01c6e19 100644 --- a/drivers/net/dsa/mv88e6xxx/serdes.h +++ b/drivers/net/dsa/mv88e6xxx/serdes.h @@ -45,8 +45,8 @@ int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on); int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on); int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port); -void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip, - int port, uint8_t *data); -void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, - uint64_t *data); +int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip, + int port, uint8_t *data); +int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port, + uint64_t *data); #endif -- 2.16.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics 2018-03-27 21:59 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics Andrew Lunn @ 2018-03-28 18:17 ` Florian Fainelli 2018-03-28 19:33 ` Andrew Lunn 0 siblings, 1 reply; 8+ messages in thread From: Florian Fainelli @ 2018-03-28 18:17 UTC (permalink / raw) To: Andrew Lunn, David Miller; +Cc: netdev On 03/27/2018 02:59 PM, Andrew Lunn wrote: > Count the numbers of various ATU and VTU violation statistics and > return them as part of the ethtool -S statistics. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++----- > drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- > drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++-- > drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- > drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- > 6 files changed, 78 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 9a5d786b4885..186021f98c5d 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip, > STATS_TYPE_BANK0 | STATS_TYPE_BANK1); > } > > +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = { Why not const char *? > + "atu_member_violation", > + "atu_miss_violation", > + "atu_full_violation", > + "vtu_member_violation", > + "vtu_miss_violation", > +}; > + > +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) > +{ > + int i; unsigned int i? > + > + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) > + strlcpy(data + i * ETH_GSTRING_LEN, > + mv88e6xxx_atu_vtu_stats_strings[i], > + ETH_GSTRING_LEN); > +} > + > static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, > uint8_t *data) > { > @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, > > if (chip->info->ops->serdes_get_strings) { > data += count * ETH_GSTRING_LEN; > - chip->info->ops->serdes_get_strings(chip, port, data); > + count = chip->info->ops->serdes_get_strings(chip, port, data); > } > > + data += count * ETH_GSTRING_LEN; > + mv88e6xxx_atu_vtu_get_strings(data); > + > mutex_unlock(&chip->reg_lock); > } > > @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port) > if (chip->info->ops->serdes_get_sset_count) > serdes_count = chip->info->ops->serdes_get_sset_count(chip, > port); > - if (serdes_count < 0) > + if (serdes_count < 0) { > count = serdes_count; > - else > - count += serdes_count; > + goto out; > + } > + count += serdes_count; > + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); > + > out: > mutex_unlock(&chip->reg_lock); > > @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port, > 0); > } > > +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port, > + uint64_t *data) > +{ > + *data++ = chip->ports[port].atu_member_violation; > + *data++ = chip->ports[port].atu_miss_violation; > + *data++ = chip->ports[port].atu_full_violation; > + *data++ = chip->ports[port].vtu_member_violation; > + *data++ = chip->ports[port].vtu_miss_violation; This looks fine, but I suppose you could just have an u64 pointer which is initialized to point to atu_member_violation, and then just do pointer arithmetics to iterate, this would avoid possibly missing that function in case new ATU/VTU violations are handled in the future? -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics 2018-03-28 18:17 ` Florian Fainelli @ 2018-03-28 19:33 ` Andrew Lunn 2018-03-28 19:57 ` Florian Fainelli 0 siblings, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2018-03-28 19:33 UTC (permalink / raw) To: Florian Fainelli; +Cc: David Miller, netdev On Wed, Mar 28, 2018 at 11:17:19AM -0700, Florian Fainelli wrote: > On 03/27/2018 02:59 PM, Andrew Lunn wrote: > > Count the numbers of various ATU and VTU violation statistics and > > return them as part of the ethtool -S statistics. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++----- > > drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- > > drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- > > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++-- > > drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- > > drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- > > 6 files changed, 78 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > > index 9a5d786b4885..186021f98c5d 100644 > > --- a/drivers/net/dsa/mv88e6xxx/chip.c > > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > > @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip, > > STATS_TYPE_BANK0 | STATS_TYPE_BANK1); > > } > > > > +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = { > > Why not const char *? The ethtool call passes i uint8_t *data to receive the copy into. I'm keeping it consistent. > > +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) > > +{ > > + int i; > > unsigned int i? I could do, but it seems unlikely it will overflow 31 bits. > > + > > + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) > > + strlcpy(data + i * ETH_GSTRING_LEN, > > + mv88e6xxx_atu_vtu_stats_strings[i], > > + ETH_GSTRING_LEN); > > +} > > + > > static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, > > uint8_t *data) > > { > > @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, > > > > if (chip->info->ops->serdes_get_strings) { > > data += count * ETH_GSTRING_LEN; > > - chip->info->ops->serdes_get_strings(chip, port, data); > > + count = chip->info->ops->serdes_get_strings(chip, port, data); > > } > > > > + data += count * ETH_GSTRING_LEN; > > + mv88e6xxx_atu_vtu_get_strings(data); > > + > > mutex_unlock(&chip->reg_lock); > > } > > > > @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port) > > if (chip->info->ops->serdes_get_sset_count) > > serdes_count = chip->info->ops->serdes_get_sset_count(chip, > > port); > > - if (serdes_count < 0) > > + if (serdes_count < 0) { > > count = serdes_count; > > - else > > - count += serdes_count; > > + goto out; > > + } > > + count += serdes_count; > > + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); > > + > > out: > > mutex_unlock(&chip->reg_lock); > > > > @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port, > > 0); > > } > > > > +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port, > > + uint64_t *data) > > +{ > > + *data++ = chip->ports[port].atu_member_violation; > > + *data++ = chip->ports[port].atu_miss_violation; > > + *data++ = chip->ports[port].atu_full_violation; > > + *data++ = chip->ports[port].vtu_member_violation; > > + *data++ = chip->ports[port].vtu_miss_violation; > > This looks fine, but I suppose you could just have an u64 pointer which > is initialized to point to atu_member_violation, and then just do > pointer arithmetics to iterate, this would avoid possibly missing that > function in case new ATU/VTU violations are handled in the future? KISS. This works and is obvious. Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics 2018-03-28 19:33 ` Andrew Lunn @ 2018-03-28 19:57 ` Florian Fainelli 0 siblings, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2018-03-28 19:57 UTC (permalink / raw) To: Andrew Lunn; +Cc: David Miller, netdev On 03/28/2018 12:33 PM, Andrew Lunn wrote: > On Wed, Mar 28, 2018 at 11:17:19AM -0700, Florian Fainelli wrote: >> On 03/27/2018 02:59 PM, Andrew Lunn wrote: >>> Count the numbers of various ATU and VTU violation statistics and >>> return them as part of the ethtool -S statistics. >>> >>> Signed-off-by: Andrew Lunn <andrew@lunn.ch> >>> --- >>> drivers/net/dsa/mv88e6xxx/chip.c | 50 ++++++++++++++++++++++++++++----- >>> drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--- >>> drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++--- >>> drivers/net/dsa/mv88e6xxx/global1_vtu.c | 8 ++++-- >>> drivers/net/dsa/mv88e6xxx/serdes.c | 15 ++++++---- >>> drivers/net/dsa/mv88e6xxx/serdes.h | 8 +++--- >>> 6 files changed, 78 insertions(+), 28 deletions(-) >>> >>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c >>> index 9a5d786b4885..186021f98c5d 100644 >>> --- a/drivers/net/dsa/mv88e6xxx/chip.c >>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c >>> @@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip, >>> STATS_TYPE_BANK0 | STATS_TYPE_BANK1); >>> } >>> >>> +static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = { >> >> Why not const char *? > > The ethtool call passes i uint8_t *data to receive the copy into. I'm > keeping it consistent. Fair enough. > >>> +static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data) >>> +{ >>> + int i; >> >> unsigned int i? > > I could do, but it seems unlikely it will overflow 31 bits. The size cannot be negative, so unsigned int would seem like a natural choice. > >>> + >>> + for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++) >>> + strlcpy(data + i * ETH_GSTRING_LEN, >>> + mv88e6xxx_atu_vtu_stats_strings[i], >>> + ETH_GSTRING_LEN); >>> +} >>> + >>> static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, >>> uint8_t *data) >>> { >>> @@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, >>> >>> if (chip->info->ops->serdes_get_strings) { >>> data += count * ETH_GSTRING_LEN; >>> - chip->info->ops->serdes_get_strings(chip, port, data); >>> + count = chip->info->ops->serdes_get_strings(chip, port, data); >>> } >>> >>> + data += count * ETH_GSTRING_LEN; >>> + mv88e6xxx_atu_vtu_get_strings(data); >>> + >>> mutex_unlock(&chip->reg_lock); >>> } >>> >>> @@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port) >>> if (chip->info->ops->serdes_get_sset_count) >>> serdes_count = chip->info->ops->serdes_get_sset_count(chip, >>> port); >>> - if (serdes_count < 0) >>> + if (serdes_count < 0) { >>> count = serdes_count; >>> - else >>> - count += serdes_count; >>> + goto out; >>> + } >>> + count += serdes_count; >>> + count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); >>> + >>> out: >>> mutex_unlock(&chip->reg_lock); >>> >>> @@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port, >>> 0); >>> } >>> >>> +static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port, >>> + uint64_t *data) >>> +{ >>> + *data++ = chip->ports[port].atu_member_violation; >>> + *data++ = chip->ports[port].atu_miss_violation; >>> + *data++ = chip->ports[port].atu_full_violation; >>> + *data++ = chip->ports[port].vtu_member_violation; >>> + *data++ = chip->ports[port].vtu_miss_violation; >> >> This looks fine, but I suppose you could just have an u64 pointer which >> is initialized to point to atu_member_violation, and then just do >> pointer arithmetics to iterate, this would avoid possibly missing that >> function in case new ATU/VTU violations are handled in the future? > > KISS. This works and is obvious. Fair enough. -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy 2018-03-27 21:59 [PATCH net-next 0/2] Add ATU/VTU statistics Andrew Lunn 2018-03-27 21:59 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics Andrew Lunn @ 2018-03-27 21:59 ` Andrew Lunn 2018-03-28 18:11 ` Florian Fainelli 1 sibling, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2018-03-27 21:59 UTC (permalink / raw) To: David Miller; +Cc: netdev, Andrew Lunn VTU miss violations can happen under normal conditions. Don't spam the kernel log. The statistics counter will indicate it is happening, if anybody is interested. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/dsa/mv88e6xxx/global1_vtu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c index 2cbaf946e7ed..e0f1b4f6e29f 100644 --- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c +++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c @@ -547,11 +547,9 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id) chip->ports[spid].vtu_member_violation++; } - if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) { - dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n", - entry.vid, spid); + if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) chip->ports[spid].vtu_miss_violation++; - } + mutex_unlock(&chip->reg_lock); return IRQ_HANDLED; -- 2.16.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy 2018-03-27 21:59 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy Andrew Lunn @ 2018-03-28 18:11 ` Florian Fainelli 2018-03-28 19:34 ` Andrew Lunn 0 siblings, 1 reply; 8+ messages in thread From: Florian Fainelli @ 2018-03-28 18:11 UTC (permalink / raw) To: Andrew Lunn, David Miller; +Cc: netdev On 03/27/2018 02:59 PM, Andrew Lunn wrote: > VTU miss violations can happen under normal conditions. Don't spam the > kernel log. The statistics counter will indicate it is happening, if > anybody is interested. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> Reported-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c > index 2cbaf946e7ed..e0f1b4f6e29f 100644 > --- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c > +++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c > @@ -547,11 +547,9 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id) > chip->ports[spid].vtu_member_violation++; > } > > - if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) { > - dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n", > - entry.vid, spid); Why not keep it as a dev_dbg() message? Ideally we would want to keep those message around when the port is enslaved to a bridge, and vlan filtering is enabled. In other cases, I agree this is just spam with the current error level. > + if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) > chip->ports[spid].vtu_miss_violation++; > - } > + > mutex_unlock(&chip->reg_lock); > > return IRQ_HANDLED; > -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy 2018-03-28 18:11 ` Florian Fainelli @ 2018-03-28 19:34 ` Andrew Lunn 0 siblings, 0 replies; 8+ messages in thread From: Andrew Lunn @ 2018-03-28 19:34 UTC (permalink / raw) To: Florian Fainelli; +Cc: David Miller, netdev On Wed, Mar 28, 2018 at 11:11:07AM -0700, Florian Fainelli wrote: > On 03/27/2018 02:59 PM, Andrew Lunn wrote: > > VTU miss violations can happen under normal conditions. Don't spam the > > kernel log. The statistics counter will indicate it is happening, if > > anybody is interested. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > Reported-by: Florian Fainelli <f.fainelli@gmail.com> > > > --- > > drivers/net/dsa/mv88e6xxx/global1_vtu.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c > > index 2cbaf946e7ed..e0f1b4f6e29f 100644 > > --- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c > > +++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c > > @@ -547,11 +547,9 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id) > > chip->ports[spid].vtu_member_violation++; > > } > > > > - if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) { > > - dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n", > > - entry.vid, spid); > > Why not keep it as a dev_dbg() message? O.K, will do. Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-28 19:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-27 21:59 [PATCH net-next 0/2] Add ATU/VTU statistics Andrew Lunn 2018-03-27 21:59 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics Andrew Lunn 2018-03-28 18:17 ` Florian Fainelli 2018-03-28 19:33 ` Andrew Lunn 2018-03-28 19:57 ` Florian Fainelli 2018-03-27 21:59 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy Andrew Lunn 2018-03-28 18:11 ` Florian Fainelli 2018-03-28 19:34 ` Andrew Lunn
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).