diff mbox

[net-next,7/7] switchdev: update documentation on FDB ageing_time

Message ID 1442606151-50429-8-git-send-email-sfeldma@gmail.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Scott Feldman Sept. 18, 2015, 7:55 p.m. UTC
From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 Documentation/networking/switchdev.txt |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Vivien Didelot Sept. 18, 2015, 9:35 p.m. UTC | #1
On Sep. Friday 18 (38) 12:55 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko Sept. 19, 2015, 6:58 a.m. UTC | #2
Fri, Sep 18, 2015 at 09:55:51PM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>

Acked-by: Jiri Pirko <jiri@resnulli.us>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roopa Prabhu Sept. 20, 2015, 1:21 a.m. UTC | #3
On 9/18/15, 12:55 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>   Documentation/networking/switchdev.txt |   24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
> index 476df04..67e43ee 100644
> --- a/Documentation/networking/switchdev.txt
> +++ b/Documentation/networking/switchdev.txt
> @@ -239,20 +239,20 @@ The driver should initialize the attributes to the hardware defaults.
>   FDB Ageing
>   ^^^^^^^^^^
>   
> -There are two FDB ageing models supported: 1) ageing by the device, and 2)
> -ageing by the kernel.  Ageing by the device is preferred if many FDB entries
> -are supported.  The driver calls call_switchdev_notifiers(SWITCHDEV_FDB_DEL,
> -...) to age out the FDB entry.  In this model, ageing by the kernel should be
> -turned off.  XXX: how to turn off ageing in kernel on a per-port basis or
> -otherwise prevent the kernel from ageing out the FDB entry?
> -
> -In the kernel ageing model, the standard bridge ageing mechanism is used to age
> -out stale FDB entries.  To keep an FDB entry "alive", the driver should refresh
> -the FDB entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  The
> +The bridge will skip ageing FDB entries marked with NTF_EXT_LEARNED and it is
> +the responsibility of the port driver/device to age out these entries.  If the
> +port device supports ageing, when the FDB entry expires, it will notify the
> +driver which in turn will notify the bridge with SWITCHDEV_FDB_DEL.  If the
> +device does not support ageing, the driver can simulate ageing using a
> +garbage collection timer to monitor FBD entries.  Expired entries will be
> +notified to the bridge using SWITCHDEV_FDB_DEL.  See rocker driver for
> +example of driver running ageing timer.
We do rely on the bridge driver ageing out entries. We have gone from 
hardware ageing to ageing in the switch driver to ultimately ageing in 
the bridge driver.  :). And we keep the fdb entries in the bridge driver 
"alive" by using 'NTF_USE' from the user-space driver.

> +
> +To keep an NTF_EXT_LEARNED entry "alive", the driver should refresh the FDB
> +entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  The
>
Even with your current patches, looks like the switch driver will need 
to refresh the entries anyways to keep the "last-used" time to now.
In which case is there much value in switch driver doing the ageing ?.

I am thinking keeping the default behavior of the bridge driver to age 
and anything else configurable might be a better option.

Thanks,
Roopa



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Sept. 20, 2015, 2:21 a.m. UTC | #4
On Sat, Sep 19, 2015 at 6:21 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 9/18/15, 12:55 PM, sfeldma@gmail.com wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> ---
>>   Documentation/networking/switchdev.txt |   24 ++++++++++++------------
>>   1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/networking/switchdev.txt
>> b/Documentation/networking/switchdev.txt
>> index 476df04..67e43ee 100644
>> --- a/Documentation/networking/switchdev.txt
>> +++ b/Documentation/networking/switchdev.txt
>> @@ -239,20 +239,20 @@ The driver should initialize the attributes to the
>> hardware defaults.
>>   FDB Ageing
>>   ^^^^^^^^^^
>>   -There are two FDB ageing models supported: 1) ageing by the device, and
>> 2)
>> -ageing by the kernel.  Ageing by the device is preferred if many FDB
>> entries
>> -are supported.  The driver calls
>> call_switchdev_notifiers(SWITCHDEV_FDB_DEL,
>> -...) to age out the FDB entry.  In this model, ageing by the kernel
>> should be
>> -turned off.  XXX: how to turn off ageing in kernel on a per-port basis or
>> -otherwise prevent the kernel from ageing out the FDB entry?
>> -
>> -In the kernel ageing model, the standard bridge ageing mechanism is used
>> to age
>> -out stale FDB entries.  To keep an FDB entry "alive", the driver should
>> refresh
>> -the FDB entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD,
>> ...).  The
>> +The bridge will skip ageing FDB entries marked with NTF_EXT_LEARNED and
>> it is
>> +the responsibility of the port driver/device to age out these entries.
>> If the
>> +port device supports ageing, when the FDB entry expires, it will notify
>> the
>> +driver which in turn will notify the bridge with SWITCHDEV_FDB_DEL.  If
>> the
>> +device does not support ageing, the driver can simulate ageing using a
>> +garbage collection timer to monitor FBD entries.  Expired entries will be
>> +notified to the bridge using SWITCHDEV_FDB_DEL.  See rocker driver for
>> +example of driver running ageing timer.
>
> We do rely on the bridge driver ageing out entries. We have gone from
> hardware ageing to ageing in the switch driver to ultimately ageing in the
> bridge driver.  :). And we keep the fdb entries in the bridge driver "alive"
> by using 'NTF_USE' from the user-space driver.

Yes, your switch driver is in user-space so you have to use NTF_USE to
refresh the entry since you cannot use the kernel driver model to
call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  Consequently, your
entries are not marked with NTF_EXT_LEARNED, so this patch is a no-op
for you.  You can continue to use the bridge driver to age out your
entries.

>> +To keep an NTF_EXT_LEARNED entry "alive", the driver should refresh the
>> FDB
>> +entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  The
>>
> Even with your current patches, looks like the switch driver will need to
> refresh the entries anyways to keep the "last-used" time to now.
> In which case is there much value in switch driver doing the ageing ?.

"should" not "must".

Value is for the many learned FDB entries case, to move the ageing
function to hardware.

> I am thinking keeping the default behavior of the bridge driver to age and
> anything else configurable might be a better option.

I'd rather someone add that knob when it's actually needed.  When the
first in-kernel switchdev driver that wants to use the bridge driver's
ageing function, then we can make that adjustment.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roopa Prabhu Sept. 20, 2015, 2:24 p.m. UTC | #5
On 9/19/15, 7:21 PM, Scott Feldman wrote:
> Yes, your switch driver is in user-space so you have to use NTF_USE to
> refresh the entry since you cannot use the kernel driver model to
> call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  Consequently, your
> entries are not marked with NTF_EXT_LEARNED, so this patch is a no-op
> for you.  You can continue to use the bridge driver to age out your
> entries.
yes, correct.  I was not really saying this because it will cause us any 
problems.
I was trying to say this for switchdev in general.

> I'd rather someone add that knob when it's actually needed. When the 
> first in-kernel switchdev driver that wants to use the bridge driver's 
> ageing function, then we can make that adjustment. 
I was suggesting the other way around. Keep the default to what is in 
the kernel today and the first in-kernel switchdev driver that wants to age,
should introduce the ability to not age in the bridge driver (Rocker 
will continue to work as it does today). Because, I am only concerned 
that rocker may end up being the only device that uses the default 
behavior introduced by this patch. And every real hardware uses the 
bridge driver to age (because there are no in kernel examples today).  I 
am curious to know who else is using hardware ageing today.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman Sept. 20, 2015, 3:56 p.m. UTC | #6
On Sun, Sep 20, 2015 at 7:24 AM, roopa <roopa@cumulusnetworks.com> wrote:
> On 9/19/15, 7:21 PM, Scott Feldman wrote:
>>
>> Yes, your switch driver is in user-space so you have to use NTF_USE to
>> refresh the entry since you cannot use the kernel driver model to
>> call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  Consequently, your
>> entries are not marked with NTF_EXT_LEARNED, so this patch is a no-op
>> for you.  You can continue to use the bridge driver to age out your
>> entries.
>
> yes, correct.  I was not really saying this because it will cause us any
> problems.
> I was trying to say this for switchdev in general.
>
>> I'd rather someone add that knob when it's actually needed. When the first
>> in-kernel switchdev driver that wants to use the bridge driver's ageing
>> function, then we can make that adjustment.
>
> I was suggesting the other way around. Keep the default to what is in the
> kernel today and the first in-kernel switchdev driver that wants to age,
> should introduce the ability to not age in the bridge driver (Rocker will
> continue to work as it does today). Because, I am only concerned that rocker
> may end up being the only device that uses the default behavior introduced
> by this patch. And every real hardware uses the bridge driver to age
> (because there are no in kernel examples today).  I am curious to know who
> else is using hardware ageing today.

A driver patch for a (real) hardware device which does the ageing in
hw is around the corner.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index 476df04..67e43ee 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -239,20 +239,20 @@  The driver should initialize the attributes to the hardware defaults.
 FDB Ageing
 ^^^^^^^^^^
 
-There are two FDB ageing models supported: 1) ageing by the device, and 2)
-ageing by the kernel.  Ageing by the device is preferred if many FDB entries
-are supported.  The driver calls call_switchdev_notifiers(SWITCHDEV_FDB_DEL,
-...) to age out the FDB entry.  In this model, ageing by the kernel should be
-turned off.  XXX: how to turn off ageing in kernel on a per-port basis or
-otherwise prevent the kernel from ageing out the FDB entry?
-
-In the kernel ageing model, the standard bridge ageing mechanism is used to age
-out stale FDB entries.  To keep an FDB entry "alive", the driver should refresh
-the FDB entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  The
+The bridge will skip ageing FDB entries marked with NTF_EXT_LEARNED and it is
+the responsibility of the port driver/device to age out these entries.  If the
+port device supports ageing, when the FDB entry expires, it will notify the
+driver which in turn will notify the bridge with SWITCHDEV_FDB_DEL.  If the
+device does not support ageing, the driver can simulate ageing using a
+garbage collection timer to monitor FBD entries.  Expired entries will be
+notified to the bridge using SWITCHDEV_FDB_DEL.  See rocker driver for
+example of driver running ageing timer.
+
+To keep an NTF_EXT_LEARNED entry "alive", the driver should refresh the FDB
+entry by calling call_switchdev_notifiers(SWITCHDEV_FDB_ADD, ...).  The
 notification will reset the FDB entry's last-used time to now.  The driver
 should rate limit refresh notifications, for example, no more than once a
-second.  If the FDB entry expires, fdb_delete is called to remove entry from
-the device.
+second.  (The last-used time is visible using the bridge -s fdb option).
 
 STP State Change on Port
 ^^^^^^^^^^^^^^^^^^^^^^^^