[patch net-next-2.6] bonding: allow resetting slave failure counters

[patch net-next-2.6] bonding: allow resetting slave failure counters

[Jiri Pirko]

This patch allows to reset failure counters for all enslaved devices.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 Documentation/networking/bonding.txt |    7 +++++++
 drivers/net/bonding/bond_sysfs.c     |   27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 675612f..2f51d73 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -782,6 +782,13 @@ resend_igmp
 
 	This option was added for bonding version 3.7.0.
 
+reset_failure_counters
+
+	This write-only control file will zero failure counters for
+	all slaves.  Note there is no appropriate module parameter for this
+	since it would not make much sense.
+	Write any value to perform reset.
+
 3. Configuring Bonding Devices
 ==============================
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 88fcb25..9b45164 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1572,6 +1572,32 @@ out:
 static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
 		   bonding_show_resend_igmp, bonding_store_resend_igmp);
 
+static ssize_t
+bonding_store_reset_failure_counters(struct device *d,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct slave *slave;
+	int i;
+	struct bonding *bond = to_bond(d);
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	read_lock(&bond->lock);
+	pr_info("%s: Resetting counters.\n", bond->dev->name);
+	bond_for_each_slave(bond, slave, i)
+		slave->link_failure_count = 0;
+	read_unlock(&bond->lock);
+
+	rtnl_unlock();
+
+	return count;
+}
+
+static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
+		   bonding_store_reset_failure_counters);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -1600,6 +1626,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_queue_id.attr,
 	&dev_attr_all_slaves_active.attr,
 	&dev_attr_resend_igmp.attr,
+	&dev_attr_reset_failure_counters.attr,
 	NULL,
 };
 
-- 
1.7.4.4

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Flavio Leitner]

On 06/01/2011 06:40 AM, Jiri Pirko wrote:
> This patch allows to reset failure counters for all enslaved devices.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
>  Documentation/networking/bonding.txt |    7 +++++++
>  drivers/net/bonding/bond_sysfs.c     |   27 +++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
> index 675612f..2f51d73 100644
> --- a/Documentation/networking/bonding.txt
> +++ b/Documentation/networking/bonding.txt
> @@ -782,6 +782,13 @@ resend_igmp
>  
>  	This option was added for bonding version 3.7.0.
>  
> +reset_failure_counters
> +
> +	This write-only control file will zero failure counters for
> +	all slaves.  Note there is no appropriate module parameter for this
> +	since it would not make much sense.
> +	Write any value to perform reset.

nit: many options mention when they were added.
i.e. This option was added for bonding version 3.7.1

fbl

>  3. Configuring Bonding Devices
>  ==============================
>  
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 88fcb25..9b45164 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -1572,6 +1572,32 @@ out:
>  static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
>  		   bonding_show_resend_igmp, bonding_store_resend_igmp);
>  
> +static ssize_t
> +bonding_store_reset_failure_counters(struct device *d,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	struct slave *slave;
> +	int i;
> +	struct bonding *bond = to_bond(d);
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	read_lock(&bond->lock);
> +	pr_info("%s: Resetting counters.\n", bond->dev->name);
> +	bond_for_each_slave(bond, slave, i)
> +		slave->link_failure_count = 0;
> +	read_unlock(&bond->lock);
> +
> +	rtnl_unlock();
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
> +		   bonding_store_reset_failure_counters);
> +
>  static struct attribute *per_bond_attrs[] = {
>  	&dev_attr_slaves.attr,
>  	&dev_attr_mode.attr,
> @@ -1600,6 +1626,7 @@ static struct attribute *per_bond_attrs[] = {
>  	&dev_attr_queue_id.attr,
>  	&dev_attr_all_slaves_active.attr,
>  	&dev_attr_resend_igmp.attr,
> +	&dev_attr_reset_failure_counters.attr,
>  	NULL,
>  };
>  

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[WANG Cong]

On Wed, 01 Jun 2011 11:40:49 +0200, Jiri Pirko wrote:

> This patch allows to reset failure counters for all enslaved devices.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
>  Documentation/networking/bonding.txt |    7 +++++++
>  drivers/net/bonding/bond_sysfs.c     |   27 +++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/networking/bonding.txt
> b/Documentation/networking/bonding.txt index 675612f..2f51d73 100644
> --- a/Documentation/networking/bonding.txt +++
> b/Documentation/networking/bonding.txt @@ -782,6 +782,13 @@ resend_igmp
>  
>  	This option was added for bonding version 3.7.0.
>  
> +reset_failure_counters
> +
> +	This write-only control file will zero failure counters for +	all
> slaves.  Note there is no appropriate module parameter for this 
> +	since
> it would not make much sense. +	Write any value to perform reset.
> +

Writing any value includes zero will work? I think only non-zero
number makes sense, if we write zero, nothing should happen.

Thanks.

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Jiri Pirko]

Wed, Jun 01, 2011 at 03:28:37PM CEST, fbl@redhat.com wrote:
>On 06/01/2011 06:40 AM, Jiri Pirko wrote:
>> This patch allows to reset failure counters for all enslaved devices.
>> 
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>>  Documentation/networking/bonding.txt |    7 +++++++
>>  drivers/net/bonding/bond_sysfs.c     |   27 +++++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+), 0 deletions(-)
>> 
>> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>> index 675612f..2f51d73 100644
>> --- a/Documentation/networking/bonding.txt
>> +++ b/Documentation/networking/bonding.txt
>> @@ -782,6 +782,13 @@ resend_igmp
>>  
>>  	This option was added for bonding version 3.7.0.
>>  
>> +reset_failure_counters
>> +
>> +	This write-only control file will zero failure counters for
>> +	all slaves.  Note there is no appropriate module parameter for this
>> +	since it would not make much sense.
>> +	Write any value to perform reset.
>
>nit: many options mention when they were added.
>i.e. This option was added for bonding version 3.7.1

hmm, is that necessary Jay, Andy?

/me thinks this versioning should be removed at all in the first place...

>
>fbl
>
>>  3. Configuring Bonding Devices
>>  ==============================
>>  
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 88fcb25..9b45164 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -1572,6 +1572,32 @@ out:
>>  static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
>>  		   bonding_show_resend_igmp, bonding_store_resend_igmp);
>>  
>> +static ssize_t
>> +bonding_store_reset_failure_counters(struct device *d,
>> +				     struct device_attribute *attr,
>> +				     const char *buf, size_t count)
>> +{
>> +	struct slave *slave;
>> +	int i;
>> +	struct bonding *bond = to_bond(d);
>> +
>> +	if (!rtnl_trylock())
>> +		return restart_syscall();
>> +
>> +	read_lock(&bond->lock);
>> +	pr_info("%s: Resetting counters.\n", bond->dev->name);
>> +	bond_for_each_slave(bond, slave, i)
>> +		slave->link_failure_count = 0;
>> +	read_unlock(&bond->lock);
>> +
>> +	rtnl_unlock();
>> +
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
>> +		   bonding_store_reset_failure_counters);
>> +
>>  static struct attribute *per_bond_attrs[] = {
>>  	&dev_attr_slaves.attr,
>>  	&dev_attr_mode.attr,
>> @@ -1600,6 +1626,7 @@ static struct attribute *per_bond_attrs[] = {
>>  	&dev_attr_queue_id.attr,
>>  	&dev_attr_all_slaves_active.attr,
>>  	&dev_attr_resend_igmp.attr,
>> +	&dev_attr_reset_failure_counters.attr,
>>  	NULL,
>>  };
>>  
>

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Jay Vosburgh]

Jiri Pirko <jpirko@redhat.com> wrote:

>Wed, Jun 01, 2011 at 03:28:37PM CEST, fbl@redhat.com wrote:
>>On 06/01/2011 06:40 AM, Jiri Pirko wrote:
>>> This patch allows to reset failure counters for all enslaved devices.
>>> 
>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>> ---
>>>  Documentation/networking/bonding.txt |    7 +++++++
>>>  drivers/net/bonding/bond_sysfs.c     |   27 +++++++++++++++++++++++++++
>>>  2 files changed, 34 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>>> index 675612f..2f51d73 100644
>>> --- a/Documentation/networking/bonding.txt
>>> +++ b/Documentation/networking/bonding.txt
>>> @@ -782,6 +782,13 @@ resend_igmp
>>>  
>>>  	This option was added for bonding version 3.7.0.
>>>  
>>> +reset_failure_counters
>>> +
>>> +	This write-only control file will zero failure counters for
>>> +	all slaves.  Note there is no appropriate module parameter for this
>>> +	since it would not make much sense.
>>> +	Write any value to perform reset.
>>
>>nit: many options mention when they were added.
>>i.e. This option was added for bonding version 3.7.1
>
>hmm, is that necessary Jay, Andy?
>
>/me thinks this versioning should be removed at all in the first place...

	The "this dingus was added in version X.Y.Z" is there because
users sometimes read the most recent version of the documentation (that
they get from the internet) and then would become confused when their
older distro driver lacked some option described in the documentation.

	I don't know if this is "good" or "bad" in an absolute sense,
but I stopped getting questions of that sort after I put these notes
into the documentation.  I don't really see a down side, so for this
patch I'd like to see the version go up and one of these notes in the
documentation.

>>
>>fbl
>>
>>>  3. Configuring Bonding Devices
>>>  ==============================
>>>  
>>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>> index 88fcb25..9b45164 100644
>>> --- a/drivers/net/bonding/bond_sysfs.c
>>> +++ b/drivers/net/bonding/bond_sysfs.c
>>> @@ -1572,6 +1572,32 @@ out:
>>>  static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
>>>  		   bonding_show_resend_igmp, bonding_store_resend_igmp);
>>>  
>>> +static ssize_t
>>> +bonding_store_reset_failure_counters(struct device *d,
>>> +				     struct device_attribute *attr,
>>> +				     const char *buf, size_t count)
>>> +{
>>> +	struct slave *slave;
>>> +	int i;
>>> +	struct bonding *bond = to_bond(d);
>>> +
>>> +	if (!rtnl_trylock())
>>> +		return restart_syscall();
>>> +
>>> +	read_lock(&bond->lock);
>>> +	pr_info("%s: Resetting counters.\n", bond->dev->name);

	I'd stick "failure" in here somewhere so it's clear what's been
reset.  The printk can be done outside the lock as well.

	-J

>>> +	bond_for_each_slave(bond, slave, i)
>>> +		slave->link_failure_count = 0;
>>> +	read_unlock(&bond->lock);
>>> +
>>> +	rtnl_unlock();
>>> +
>>> +	return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
>>> +		   bonding_store_reset_failure_counters);
>>> +
>>>  static struct attribute *per_bond_attrs[] = {
>>>  	&dev_attr_slaves.attr,
>>>  	&dev_attr_mode.attr,
>>> @@ -1600,6 +1626,7 @@ static struct attribute *per_bond_attrs[] = {
>>>  	&dev_attr_queue_id.attr,
>>>  	&dev_attr_all_slaves_active.attr,
>>>  	&dev_attr_resend_igmp.attr,
>>> +	&dev_attr_reset_failure_counters.attr,
>>>  	NULL,
>>>  };
>>>  
>>

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

[patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[Jiri Pirko]

This patch allows to reset failure counters for all enslaved devices.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

v1->v2: added version bump
	added version note to doc
	moved and adjusted printk
---
 Documentation/networking/bonding.txt |    9 +++++++++
 drivers/net/bonding/bond_sysfs.c     |   28 ++++++++++++++++++++++++++++
 drivers/net/bonding/bonding.h        |    4 ++--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 675612f..7a8da01 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -782,6 +782,15 @@ resend_igmp
 
 	This option was added for bonding version 3.7.0.
 
+reset_failure_counters
+
+	This write-only control file will zero failure counters for
+	all slaves.  Note there is no appropriate module parameter for this
+	since it would not make much sense.
+	Write any value to perform reset.
+
+	This option was added for bonding version 3.7.2.
+
 3. Configuring Bonding Devices
 ==============================
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 88fcb25..342b4ed 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1572,6 +1572,33 @@ out:
 static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
 		   bonding_show_resend_igmp, bonding_store_resend_igmp);
 
+static ssize_t
+bonding_store_reset_failure_counters(struct device *d,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct slave *slave;
+	int i;
+	struct bonding *bond = to_bond(d);
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	pr_info("%s: Resetting failure counters.\n", bond->dev->name);
+
+	read_lock(&bond->lock);
+	bond_for_each_slave(bond, slave, i)
+		slave->link_failure_count = 0;
+	read_unlock(&bond->lock);
+
+	rtnl_unlock();
+
+	return count;
+}
+
+static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
+		   bonding_store_reset_failure_counters);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -1600,6 +1627,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_queue_id.attr,
 	&dev_attr_all_slaves_active.attr,
 	&dev_attr_resend_igmp.attr,
+	&dev_attr_reset_failure_counters.attr,
 	NULL,
 };
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ea1d005..62528f8 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -24,8 +24,8 @@
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
-#define DRV_VERSION	"3.7.1"
-#define DRV_RELDATE	"April 27, 2011"
+#define DRV_VERSION	"3.7.2"
+#define DRV_RELDATE	"Jun 1, 2011"
 #define DRV_NAME	"bonding"
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
 
-- 
1.7.4.4

Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[Flavio Leitner]

On 06/01/2011 01:31 PM, Jiri Pirko wrote:
> This patch allows to reset failure counters for all enslaved devices.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> 
> v1->v2: added version bump
> 	added version note to doc
> 	moved and adjusted printk
> ---
>  Documentation/networking/bonding.txt |    9 +++++++++
>  drivers/net/bonding/bond_sysfs.c     |   28 ++++++++++++++++++++++++++++
>  drivers/net/bonding/bonding.h        |    4 ++--
>  3 files changed, 39 insertions(+), 2 deletions(-)

Reviewed-by: Flavio Leitner <fbl@redhat.com>
Thanks!
fbl

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[David Miller]

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 01 Jun 2011 09:13:39 -0700

> 	The "this dingus was added in version X.Y.Z" is there because
> users sometimes read the most recent version of the documentation (that
> they get from the internet) and then would become confused when their
> older distro driver lacked some option described in the documentation.

I disagree with this whole concept, because distros backport features
like this into their kernel and therefore the feature is showing up in
version X.Y.$(Z-20).

And distro kernels are what %99.9999 of users are exposed to.

Therefore, I think this version specification is not only pointless
but even a possible cause of confusion.

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Flavio Leitner]

On 06/01/2011 04:03 PM, David Miller wrote:
> From: Jay Vosburgh <fubar@us.ibm.com>
> Date: Wed, 01 Jun 2011 09:13:39 -0700
> 
>> 	The "this dingus was added in version X.Y.Z" is there because
>> users sometimes read the most recent version of the documentation (that
>> they get from the internet) and then would become confused when their
>> older distro driver lacked some option described in the documentation.
> 
> I disagree with this whole concept, because distros backport features
> like this into their kernel and therefore the feature is showing up in
> version X.Y.$(Z-20).

It doesn't matter the version if the user can find the feature, so
distros backporting features works and that info is not useful at all. 
However, when the user doesn't find the feature and search the internet,
then that info is helpful.

fbl

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[David Miller]

From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 01 Jun 2011 16:11:07 -0300

> On 06/01/2011 04:03 PM, David Miller wrote:
>> From: Jay Vosburgh <fubar@us.ibm.com>
>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>> 
>>> 	The "this dingus was added in version X.Y.Z" is there because
>>> users sometimes read the most recent version of the documentation (that
>>> they get from the internet) and then would become confused when their
>>> older distro driver lacked some option described in the documentation.
>> 
>> I disagree with this whole concept, because distros backport features
>> like this into their kernel and therefore the feature is showing up in
>> version X.Y.$(Z-20).
> 
> It doesn't matter the version if the user can find the feature, so
> distros backporting features works and that info is not useful at all. 
> However, when the user doesn't find the feature and search the internet,
> then that info is helpful.

So how is the user going to find that FC14 has the feature even
though his FC13 kernel does not?

I'll say it again, this version stuff is completely pointless.

If the user is dabbling with upstream kernels he's a minority,
and clueful enough to figure out this stuff himself.

Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[Nicolas de Pesloüan]

Le 01/06/2011 18:31, Jiri Pirko a écrit :
> This patch allows to reset failure counters for all enslaved devices.

Hi Jiri,

Why do we need a way to reset those counters? What is the problem with having those counters 
monotonically increase until the system is rebooted? Do we have a way to reset other network 
statistics (/sys/class/net/eth0/statistics/* for example)?

Except from this "do we need this feature" question, the code sounds good to me.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

	Nicolas.

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Nicolas de Pesloüan]

Le 01/06/2011 21:11, Flavio Leitner a écrit :
> On 06/01/2011 04:03 PM, David Miller wrote:
>> From: Jay Vosburgh<fubar@us.ibm.com>
>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>
>>> 	The "this dingus was added in version X.Y.Z" is there because
>>> users sometimes read the most recent version of the documentation (that
>>> they get from the internet) and then would become confused when their
>>> older distro driver lacked some option described in the documentation.
>>
>> I disagree with this whole concept, because distros backport features
>> like this into their kernel and therefore the feature is showing up in
>> version X.Y.$(Z-20).
>
> It doesn't matter the version if the user can find the feature, so
> distros backporting features works and that info is not useful at all.
> However, when the user doesn't find the feature and search the internet,
> then that info is helpful.

There are *many* new features that get included into the kernel without documenting the exact first 
version that provide them. Why should we need this for bonding? Also, because we lack a table that 
gives the kernel version matching a bonding version, the user is not really helped by "you need 
version X.Y.Z of bonding to have this feature".

So, I'm not sure it helps...

	Nicolas.

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Jay Vosburgh]

David Miller <davem@davemloft.net> wrote:

>From: Flavio Leitner <fbl@redhat.com>
>Date: Wed, 01 Jun 2011 16:11:07 -0300
>
>> On 06/01/2011 04:03 PM, David Miller wrote:
>>> From: Jay Vosburgh <fubar@us.ibm.com>
>>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>> 
>>>> 	The "this dingus was added in version X.Y.Z" is there because
>>>> users sometimes read the most recent version of the documentation (that
>>>> they get from the internet) and then would become confused when their
>>>> older distro driver lacked some option described in the documentation.
>>> 
>>> I disagree with this whole concept, because distros backport features
>>> like this into their kernel and therefore the feature is showing up in
>>> version X.Y.$(Z-20).
>> 
>> It doesn't matter the version if the user can find the feature, so
>> distros backporting features works and that info is not useful at all. 
>> However, when the user doesn't find the feature and search the internet,
>> then that info is helpful.
>
>So how is the user going to find that FC14 has the feature even
>though his FC13 kernel does not?
>
>I'll say it again, this version stuff is completely pointless.
>
>If the user is dabbling with upstream kernels he's a minority,
>and clueful enough to figure out this stuff himself.

	The problem was that users would search the internet for bonding
documentation, and get the version out of the current mainline.  That
document described options not present in their distro kernel, and I got
questions asking why they couldn't enable some option present in
mainline but not their distro kernel.  To try and minimize that
confusion, I started documenting when features were added (by bonding
driver version, not by kernel version).

	Maybe this doesn't make as much difference today (for whatever
reason), but it certainly seemed to help back in the day.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Jiri Pirko]

Wed, Jun 01, 2011 at 09:34:45PM CEST, nicolas.2p.debian@gmail.com wrote:
>Le 01/06/2011 21:11, Flavio Leitner a écrit :
>>On 06/01/2011 04:03 PM, David Miller wrote:
>>>From: Jay Vosburgh<fubar@us.ibm.com>
>>>Date: Wed, 01 Jun 2011 09:13:39 -0700
>>>
>>>>	The "this dingus was added in version X.Y.Z" is there because
>>>>users sometimes read the most recent version of the documentation (that
>>>>they get from the internet) and then would become confused when their
>>>>older distro driver lacked some option described in the documentation.
>>>
>>>I disagree with this whole concept, because distros backport features
>>>like this into their kernel and therefore the feature is showing up in
>>>version X.Y.$(Z-20).
>>
>>It doesn't matter the version if the user can find the feature, so
>>distros backporting features works and that info is not useful at all.
>>However, when the user doesn't find the feature and search the internet,
>>then that info is helpful.
>
>There are *many* new features that get included into the kernel
>without documenting the exact first version that provide them. Why
>should we need this for bonding? Also, because we lack a table that
>gives the kernel version matching a bonding version, the user is not
>really helped by "you need version X.Y.Z of bonding to have this
>feature".


I think that doing versioning on multiple parts of the same code is only
confusing. Kernel version should be only version for whole kernel code.
Changes should be only documented in descriptions of changesets. Any
other way is redundant, might be not accurate, and rather confusing.

Jirka

>
>So, I'm not sure it helps...
>
>	Nicolas.

Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[Jiri Pirko]

Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>This patch allows to reset failure counters for all enslaved devices.
>
>Hi Jiri,
>
>Why do we need a way to reset those counters? What is the problem
>with having those counters monotonically increase until the system is
>rebooted? Do we have a way to reset other network statistics
>(/sys/class/net/eth0/statistics/* for example)?

Well, it's handy to be able to clear this counters when you resolve
a problem so future issues are crearly seen as non-zero.				   
				   
>
>Except from this "do we need this feature" question, the code sounds good to me.
>
>Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
>	Nicolas.

Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 1 Jun 2011 22:08:55 +0200

> Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>>This patch allows to reset failure counters for all enslaved devices.
>>
>>Hi Jiri,
>>
>>Why do we need a way to reset those counters? What is the problem
>>with having those counters monotonically increase until the system is
>>rebooted? Do we have a way to reset other network statistics
>>(/sys/class/net/eth0/statistics/* for example)?
> 
> Well, it's handy to be able to clear this counters when you resolve
> a problem so future issues are crearly seen as non-zero.				   

We don't allow this in any way for netdev counters, for good reasons,
so please don't add things like this.

Re: [patch net-next-2.6] bonding: allow resetting slave failure counters

[Flavio Leitner]

On 06/01/2011 04:22 PM, David Miller wrote:
> From: Flavio Leitner <fbl@redhat.com>
> Date: Wed, 01 Jun 2011 16:11:07 -0300
> 
>> On 06/01/2011 04:03 PM, David Miller wrote:
>>> From: Jay Vosburgh <fubar@us.ibm.com>
>>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>>
>>>> 	The "this dingus was added in version X.Y.Z" is there because
>>>> users sometimes read the most recent version of the documentation (that
>>>> they get from the internet) and then would become confused when their
>>>> older distro driver lacked some option described in the documentation.
>>>
>>> I disagree with this whole concept, because distros backport features
>>> like this into their kernel and therefore the feature is showing up in
>>> version X.Y.$(Z-20).
>>
>> It doesn't matter the version if the user can find the feature, so
>> distros backporting features works and that info is not useful at all. 
>> However, when the user doesn't find the feature and search the internet,
>> then that info is helpful.
> 
> So how is the user going to find that FC14 has the feature even
> though his FC13 kernel does not?
> 
> I'll say it again, this version stuff is completely pointless.
> 
> If the user is dabbling with upstream kernels he's a minority,
> and clueful enough to figure out this stuff himself.

If the distro's bonding version is updated accordingly then
you have something to compare.  Anyway, I agree that it seems
more confusing than helpful.

fbl

Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters

[Jiri Pirko]

Wed, Jun 01, 2011 at 10:12:43PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Wed, 1 Jun 2011 22:08:55 +0200
>
>> Wed, Jun 01, 2011 at 09:23:11PM CEST, nicolas.2p.debian@gmail.com wrote:
>>>Le 01/06/2011 18:31, Jiri Pirko a écrit :
>>>>This patch allows to reset failure counters for all enslaved devices.
>>>
>>>Hi Jiri,
>>>
>>>Why do we need a way to reset those counters? What is the problem
>>>with having those counters monotonically increase until the system is
>>>rebooted? Do we have a way to reset other network statistics
>>>(/sys/class/net/eth0/statistics/* for example)?
>> 
>> Well, it's handy to be able to clear this counters when you resolve
>> a problem so future issues are crearly seen as non-zero.				   
>
>We don't allow this in any way for netdev counters, for good reasons,
>so please don't add things like this.
>

Well technically this is not netdev counter directly. But fair enough.

Thanks.

Jirka