[PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

v2:

Resending now that the dust has cleared in 3.18 on "self" vs. hwmode debate for
brport settings.  learning_sync is now set/cleared using "self" qualifier on
brport.

v1:

Add 'learned_sync' flag to turn on/off syncing of learned MAC addresses from
offload device to bridge's FDB.   Flag is be set/cleared on offload device port
using "self" qualifier:

  $ sudo bridge link set dev swp1 learning_sync on self

  $ bridge -d link show dev swp1
  2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 2
      hairpin off guard off root_block off fastleave off learning off flood off
  2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0
      learning on learning_sync on

Adds new IFLA_BRPORT_LEARNED_SYNCED attribute for IFLA_PROTINFO on the SELF
brport.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 bridge/link.c     |   12 ++++++++++++
 man/man8/bridge.8 |    6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/bridge/link.c b/bridge/link.c
index 3f77aab..c8555f8 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -188,6 +188,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 				if (prtb[IFLA_BRPORT_LEARNING])
 					print_onoff(fp, "learning",
 						    rta_getattr_u8(prtb[IFLA_BRPORT_LEARNING]));
+				if (prtb[IFLA_BRPORT_LEARNING_SYNC])
+					print_onoff(fp, "learning_sync",
+						    rta_getattr_u8(prtb[IFLA_BRPORT_LEARNING_SYNC]));
 				if (prtb[IFLA_BRPORT_UNICAST_FLOOD])
 					print_onoff(fp, "flood",
 						    rta_getattr_u8(prtb[IFLA_BRPORT_UNICAST_FLOOD]));
@@ -221,6 +224,7 @@ static void usage(void)
 	fprintf(stderr, "                               [ fastleave {on | off} ]\n");
 	fprintf(stderr,	"                               [ root_block {on | off} ]\n");
 	fprintf(stderr,	"                               [ learning {on | off} ]\n");
+	fprintf(stderr,	"                               [ learning_sync {on | off} ]\n");
 	fprintf(stderr,	"                               [ flood {on | off} ]\n");
 	fprintf(stderr, "                               [ hwmode {vepa | veb} ]\n");
 	fprintf(stderr, "       bridge link show [dev DEV]\n");
@@ -252,6 +256,7 @@ static int brlink_modify(int argc, char **argv)
 	} req;
 	char *d = NULL;
 	__s8 learning = -1;
+	__s8 learning_sync = -1;
 	__s8 flood = -1;
 	__s8 hairpin = -1;
 	__s8 bpdu_guard = -1;
@@ -295,6 +300,10 @@ static int brlink_modify(int argc, char **argv)
 			NEXT_ARG();
 			if (!on_off("learning", &learning, *argv))
 				exit(-1);
+		} else if (strcmp(*argv, "learning_sync") == 0) {
+			NEXT_ARG();
+			if (!on_off("learning_sync", &learning_sync, *argv))
+				exit(-1);
 		} else if (strcmp(*argv, "flood") == 0) {
 			NEXT_ARG();
 			if (!on_off("flood", &flood, *argv))
@@ -359,6 +368,9 @@ static int brlink_modify(int argc, char **argv)
 		addattr8(&req.n, sizeof(req), IFLA_BRPORT_UNICAST_FLOOD, flood);
 	if (learning >= 0)
 		addattr8(&req.n, sizeof(req), IFLA_BRPORT_LEARNING, learning);
+	if (learning_sync >= 0)
+		addattr8(&req.n, sizeof(req), IFLA_BRPORT_LEARNING_SYNC,
+			 learning_sync);
 
 	if (cost > 0)
 		addattr32(&req.n, sizeof(req), IFLA_BRPORT_COST, cost);
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index cb3fb46..e344db2 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -38,6 +38,7 @@ bridge \- show / manipulate bridge addresses and devices
 .BR fastleave " { " on " | " off " } ] [ "
 .BR root_block " { " on " | " off " } ] [ "
 .BR learning " { " on " | " off " } ] [ "
+.BR learning_sync " { " on " | " off " } ] [ "
 .BR flood " { " on " | " off " } ] [ "
 .BR hwmode " { " vepa " | " veb " } ] "
 
@@ -263,6 +264,11 @@ not.  If learning if off, the bridge will end up flooding any traffic for which
 it has no FDB entry.  By default this flag is on.
 
 .TP
+.BR "learning_sync on " or " learning_sync off "
+Controls whether a given port will sync MAC addresses learned on device port to
+bridge FDB.
+
+.TP
 .BR "flooding on " or " flooding off "
 Controls whether a given port will flood unicast traffic for which there is no FDB entry.  By default this flag is on.
 
-- 
1.7.10.4

Re: [PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[Stephen Hemminger]

On Mon, 29 Dec 2014 12:20:07 -0800
sfeldma@gmail.com wrote:

> From: Scott Feldman <sfeldma@gmail.com>
> 
> v2:
> 
> Resending now that the dust has cleared in 3.18 on "self" vs. hwmode debate for
> brport settings.  learning_sync is now set/cleared using "self" qualifier on
> brport.
> 
> v1:
> 
> Add 'learned_sync' flag to turn on/off syncing of learned MAC addresses from
> offload device to bridge's FDB.   Flag is be set/cleared on offload device port
> using "self" qualifier:
> 
>   $ sudo bridge link set dev swp1 learning_sync on self
> 
>   $ bridge -d link show dev swp1
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 2
>       hairpin off guard off root_block off fastleave off learning off flood off
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0
>       learning on learning_sync on
> 
> Adds new IFLA_BRPORT_LEARNED_SYNCED attribute for IFLA_PROTINFO on the SELF
> brport.
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>

Applied

Re: [PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[Siva Mannem]

Hi,


> +.BR "learning_sync on " or " learning_sync off "
> +Controls whether a given port will sync MAC addresses learned on device port to
> +bridge FDB.
> +

For the FDB entries synced from device port to bridge FDB, can the
device port also mention that it will take care of aging the synced
entries? I am thinking of a use case where the port supports hardware
learning and hardware aging?

Regards,
Siva.



On Tue, Dec 30, 2014 at 1:50 AM,  <sfeldma@gmail.com> wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> v2:
>
> Resending now that the dust has cleared in 3.18 on "self" vs. hwmode debate for
> brport settings.  learning_sync is now set/cleared using "self" qualifier on
> brport.
>
> v1:
>
> Add 'learned_sync' flag to turn on/off syncing of learned MAC addresses from
> offload device to bridge's FDB.   Flag is be set/cleared on offload device port
> using "self" qualifier:
>
>   $ sudo bridge link set dev swp1 learning_sync on self
>
>   $ bridge -d link show dev swp1
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 2
>       hairpin off guard off root_block off fastleave off learning off flood off
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0
>       learning on learning_sync on
>
> Adds new IFLA_BRPORT_LEARNED_SYNCED attribute for IFLA_PROTINFO on the SELF
> brport.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>  bridge/link.c     |   12 ++++++++++++
>  man/man8/bridge.8 |    6 ++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/bridge/link.c b/bridge/link.c
> index 3f77aab..c8555f8 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -188,6 +188,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
>                                 if (prtb[IFLA_BRPORT_LEARNING])
>                                         print_onoff(fp, "learning",
>                                                     rta_getattr_u8(prtb[IFLA_BRPORT_LEARNING]));
> +                               if (prtb[IFLA_BRPORT_LEARNING_SYNC])
> +                                       print_onoff(fp, "learning_sync",
> +                                                   rta_getattr_u8(prtb[IFLA_BRPORT_LEARNING_SYNC]));
>                                 if (prtb[IFLA_BRPORT_UNICAST_FLOOD])
>                                         print_onoff(fp, "flood",
>                                                     rta_getattr_u8(prtb[IFLA_BRPORT_UNICAST_FLOOD]));
> @@ -221,6 +224,7 @@ static void usage(void)
>         fprintf(stderr, "                               [ fastleave {on | off} ]\n");
>         fprintf(stderr, "                               [ root_block {on | off} ]\n");
>         fprintf(stderr, "                               [ learning {on | off} ]\n");
> +       fprintf(stderr, "                               [ learning_sync {on | off} ]\n");
>         fprintf(stderr, "                               [ flood {on | off} ]\n");
>         fprintf(stderr, "                               [ hwmode {vepa | veb} ]\n");
>         fprintf(stderr, "       bridge link show [dev DEV]\n");
> @@ -252,6 +256,7 @@ static int brlink_modify(int argc, char **argv)
>         } req;
>         char *d = NULL;
>         __s8 learning = -1;
> +       __s8 learning_sync = -1;
>         __s8 flood = -1;
>         __s8 hairpin = -1;
>         __s8 bpdu_guard = -1;
> @@ -295,6 +300,10 @@ static int brlink_modify(int argc, char **argv)
>                         NEXT_ARG();
>                         if (!on_off("learning", &learning, *argv))
>                                 exit(-1);
> +               } else if (strcmp(*argv, "learning_sync") == 0) {
> +                       NEXT_ARG();
> +                       if (!on_off("learning_sync", &learning_sync, *argv))
> +                               exit(-1);
>                 } else if (strcmp(*argv, "flood") == 0) {
>                         NEXT_ARG();
>                         if (!on_off("flood", &flood, *argv))
> @@ -359,6 +368,9 @@ static int brlink_modify(int argc, char **argv)
>                 addattr8(&req.n, sizeof(req), IFLA_BRPORT_UNICAST_FLOOD, flood);
>         if (learning >= 0)
>                 addattr8(&req.n, sizeof(req), IFLA_BRPORT_LEARNING, learning);
> +       if (learning_sync >= 0)
> +               addattr8(&req.n, sizeof(req), IFLA_BRPORT_LEARNING_SYNC,
> +                        learning_sync);
>
>         if (cost > 0)
>                 addattr32(&req.n, sizeof(req), IFLA_BRPORT_COST, cost);
> diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
> index cb3fb46..e344db2 100644
> --- a/man/man8/bridge.8
> +++ b/man/man8/bridge.8
> @@ -38,6 +38,7 @@ bridge \- show / manipulate bridge addresses and devices
>  .BR fastleave " { " on " | " off " } ] [ "
>  .BR root_block " { " on " | " off " } ] [ "
>  .BR learning " { " on " | " off " } ] [ "
> +.BR learning_sync " { " on " | " off " } ] [ "
>  .BR flood " { " on " | " off " } ] [ "
>  .BR hwmode " { " vepa " | " veb " } ] "
>
> @@ -263,6 +264,11 @@ not.  If learning if off, the bridge will end up flooding any traffic for which
>  it has no FDB entry.  By default this flag is on.
>
>  .TP
> +.BR "learning_sync on " or " learning_sync off "
> +Controls whether a given port will sync MAC addresses learned on device port to
> +bridge FDB.
> +
> +.TP
>  .BR "flooding on " or " flooding off "
>  Controls whether a given port will flood unicast traffic for which there is no FDB entry.  By default this flag is on.
>
> --
> 1.7.10.4
>
> --
> 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

Re: [PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[Scott Feldman]

On Fri, Jan 2, 2015 at 9:14 AM, Siva Mannem <siva.mannem.lnx@gmail.com> wrote:
> Hi,
>
>
>> +.BR "learning_sync on " or " learning_sync off "
>> +Controls whether a given port will sync MAC addresses learned on device port to
>> +bridge FDB.
>> +
>
> For the FDB entries synced from device port to bridge FDB, can the
> device port also mention that it will take care of aging the synced
> entries? I am thinking of a use case where the port supports hardware
> learning and hardware aging?

I think the aging settings are per-bridge, not per-bridge-port, so the
policy control you're talking about wouldn't end up here on
/sbin/bridge link.

However, I would argue even with hardware aging capability, we still
should use Linux for aging since all the controls are already there
and it just works.  It keeps the swdev model simple and the swdev
driver simple.  Do you have a counter-argument for why enabling
hardware aging would be better?

-scott

Re: [PATCH iproute2 v2] bridge/link: add learning_sync policy flag

[Siva Mannem]

> I think the aging settings are per-bridge, not per-bridge-port, so the
> policy control you're talking about wouldn't end up here on
> /sbin/bridge link.
>
Agree.

> However, I would argue even with hardware aging capability, we still
> should use Linux for aging since all the controls are already there
> and it just works.  It keeps the swdev model simple and the swdev
> driver simple.  Do you have a counter-argument for why enabling
> hardware aging would be better?

When traffic is being forwarded in hardware and if the entries are to
be software aged, the aging logic needs to periodically(once every
configured age interval) retrieve the FDB entry's hit bit in the
hardware. If this bit is not set it deletes the entry. If the hit bit
is set, it clears it. This could be CPU intensive if the FDB size runs
into few thousand entries.

If hardware aging is enabled, the above job is offloaded to hardware.

The current aging logic does not  differentiate between software
forwarded and hardware forwarded entries. When the traffic is being
forwarded in hardware, the aging logic is deleting the entries after
age interval.

So the solution could be

1)When hardware aging is enabled, only the notifications(probably via
br_fdb_external_learn_delete())  delete the fdb entries.
2)When software aging is enabled, aging logic needs to differentiate
between software forwarded and hardware forwarded entries. For entries
forwarded in hardware, the above aging logic needs to be implemented.



On Fri, Jan 2, 2015 at 11:27 PM, Scott Feldman <sfeldma@gmail.com> wrote:
> On Fri, Jan 2, 2015 at 9:14 AM, Siva Mannem <siva.mannem.lnx@gmail.com> wrote:
>> Hi,
>>
>>
>>> +.BR "learning_sync on " or " learning_sync off "
>>> +Controls whether a given port will sync MAC addresses learned on device port to
>>> +bridge FDB.
>>> +
>>
>> For the FDB entries synced from device port to bridge FDB, can the
>> device port also mention that it will take care of aging the synced
>> entries? I am thinking of a use case where the port supports hardware
>> learning and hardware aging?
>
> I think the aging settings are per-bridge, not per-bridge-port, so the
> policy control you're talking about wouldn't end up here on
> /sbin/bridge link.
>
> However, I would argue even with hardware aging capability, we still
> should use Linux for aging since all the controls are already there
> and it just works.  It keeps the swdev model simple and the swdev
> driver simple.  Do you have a counter-argument for why enabling
> hardware aging would be better?
>
> -scott