[PATCH net-next] net: bridge: add support for sticky fdb entries

[PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

Add support for entries which are "sticky", i.e. will not change their port
if they show up from a different one. A new ndm flag is introduced for that
purpose - NTF_STICKY. We allow to set it only to non-local entries.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
I'll send the selftest for sticky with the iproute2 patch if this one is
accepted. We've had multiple requests to support such flag and now it's
also needed for some eVPN and clag setups.

 include/uapi/linux/neighbour.h |  1 +
 net/bridge/br_fdb.c            | 19 ++++++++++++++++---
 net/bridge/br_private.h        |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 904db6148476..998155444e0d 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -43,6 +43,7 @@ enum {
 #define NTF_PROXY	0x08	/* == ATF_PUBL */
 #define NTF_EXT_LEARNED	0x10
 #define NTF_OFFLOADED   0x20
+#define NTF_STICKY	0x40
 #define NTF_ROUTER	0x80
 
 /*
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 502f66349530..26569ed06a4d 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 			unsigned long now = jiffies;
 
 			/* fastpath: update of existing entry */
-			if (unlikely(source != fdb->dst)) {
+			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
 				fdb->dst = source;
 				fdb_modified = true;
 				/* Take over HW learned entry */
@@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
 		ndm->ndm_flags |= NTF_OFFLOADED;
 	if (fdb->added_by_external_learn)
 		ndm->ndm_flags |= NTF_EXT_LEARNED;
+	if (fdb->is_sticky)
+		ndm->ndm_flags |= NTF_STICKY;
 
 	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
 		goto nla_put_failure;
@@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
 
 /* Update (create or replace) forwarding database entry */
 static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
-			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
+			 const u8 *addr, u16 state, u16 flags, u16 vid,
+			 u8 is_sticky)
 {
 	struct net_bridge_fdb_entry *fdb;
 	bool modified = false;
@@ -789,6 +792,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 		return -EINVAL;
 	}
 
+	if (is_sticky && (state & NUD_PERMANENT))
+		return -EINVAL;
+
 	fdb = br_fdb_find(br, addr, vid);
 	if (fdb == NULL) {
 		if (!(flags & NLM_F_CREATE))
@@ -832,6 +838,12 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 
 		modified = true;
 	}
+
+	if (is_sticky != fdb->is_sticky) {
+		fdb->is_sticky = is_sticky;
+		modified = true;
+	}
+
 	fdb->added_by_user = 1;
 
 	fdb->used = jiffies;
@@ -865,7 +877,8 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
 	} else {
 		spin_lock_bh(&br->hash_lock);
 		err = fdb_add_entry(br, p, addr, ndm->ndm_state,
-				    nlh_flags, vid);
+				    nlh_flags, vid,
+				    !!(ndm->ndm_flags & NTF_STICKY));
 		spin_unlock_bh(&br->hash_lock);
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 11ed2029985f..d21035a17f4c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -181,6 +181,7 @@ struct net_bridge_fdb_entry {
 	struct hlist_node		fdb_node;
 	unsigned char			is_local:1,
 					is_static:1,
+					is_sticky:1,
 					added_by_user:1,
 					added_by_external_learn:1,
 					offloaded:1;
-- 
2.11.0

[Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

Add support for entries which are "sticky", i.e. will not change their port
if they show up from a different one. A new ndm flag is introduced for that
purpose - NTF_STICKY. We allow to set it only to non-local entries.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
I'll send the selftest for sticky with the iproute2 patch if this one is
accepted. We've had multiple requests to support such flag and now it's
also needed for some eVPN and clag setups.

 include/uapi/linux/neighbour.h |  1 +
 net/bridge/br_fdb.c            | 19 ++++++++++++++++---
 net/bridge/br_private.h        |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 904db6148476..998155444e0d 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -43,6 +43,7 @@ enum {
 #define NTF_PROXY	0x08	/* == ATF_PUBL */
 #define NTF_EXT_LEARNED	0x10
 #define NTF_OFFLOADED   0x20
+#define NTF_STICKY	0x40
 #define NTF_ROUTER	0x80
 
 /*
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 502f66349530..26569ed06a4d 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 			unsigned long now = jiffies;
 
 			/* fastpath: update of existing entry */
-			if (unlikely(source != fdb->dst)) {
+			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
 				fdb->dst = source;
 				fdb_modified = true;
 				/* Take over HW learned entry */
@@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
 		ndm->ndm_flags |= NTF_OFFLOADED;
 	if (fdb->added_by_external_learn)
 		ndm->ndm_flags |= NTF_EXT_LEARNED;
+	if (fdb->is_sticky)
+		ndm->ndm_flags |= NTF_STICKY;
 
 	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
 		goto nla_put_failure;
@@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
 
 /* Update (create or replace) forwarding database entry */
 static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
-			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
+			 const u8 *addr, u16 state, u16 flags, u16 vid,
+			 u8 is_sticky)
 {
 	struct net_bridge_fdb_entry *fdb;
 	bool modified = false;
@@ -789,6 +792,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 		return -EINVAL;
 	}
 
+	if (is_sticky && (state & NUD_PERMANENT))
+		return -EINVAL;
+
 	fdb = br_fdb_find(br, addr, vid);
 	if (fdb == NULL) {
 		if (!(flags & NLM_F_CREATE))
@@ -832,6 +838,12 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 
 		modified = true;
 	}
+
+	if (is_sticky != fdb->is_sticky) {
+		fdb->is_sticky = is_sticky;
+		modified = true;
+	}
+
 	fdb->added_by_user = 1;
 
 	fdb->used = jiffies;
@@ -865,7 +877,8 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
 	} else {
 		spin_lock_bh(&br->hash_lock);
 		err = fdb_add_entry(br, p, addr, ndm->ndm_state,
-				    nlh_flags, vid);
+				    nlh_flags, vid,
+				    !!(ndm->ndm_flags & NTF_STICKY));
 		spin_unlock_bh(&br->hash_lock);
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 11ed2029985f..d21035a17f4c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -181,6 +181,7 @@ struct net_bridge_fdb_entry {
 	struct hlist_node		fdb_node;
 	unsigned char			is_local:1,
 					is_static:1,
+					is_sticky:1,
 					added_by_user:1,
 					added_by_external_learn:1,
 					offloaded:1;
-- 
2.11.0

Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Stephen Hemminger]

On Mon, 10 Sep 2018 13:16:01 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> Add support for entries which are "sticky", i.e. will not change their port
> if they show up from a different one. A new ndm flag is introduced for that
> purpose - NTF_STICKY. We allow to set it only to non-local entries.

Is there a name for this in other network switch API's?

> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> I'll send the selftest for sticky with the iproute2 patch if this one is
> accepted. We've had multiple requests to support such flag and now it's
> also needed for some eVPN and clag setups.
> 
>  include/uapi/linux/neighbour.h |  1 +
>  net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>  net/bridge/br_private.h        |  1 +
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index 904db6148476..998155444e0d 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -43,6 +43,7 @@ enum {
>  #define NTF_PROXY	0x08	/* == ATF_PUBL */
>  #define NTF_EXT_LEARNED	0x10
>  #define NTF_OFFLOADED   0x20
> +#define NTF_STICKY	0x40
>  #define NTF_ROUTER	0x80
>  
>  /*
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 502f66349530..26569ed06a4d 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  			unsigned long now = jiffies;
>  
>  			/* fastpath: update of existing entry */
> -			if (unlikely(source != fdb->dst)) {
> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>  				fdb->dst = source;
>  				fdb_modified = true;
>  				/* Take over HW learned entry */
> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>  		ndm->ndm_flags |= NTF_OFFLOADED;
>  	if (fdb->added_by_external_learn)
>  		ndm->ndm_flags |= NTF_EXT_LEARNED;
> +	if (fdb->is_sticky)
> +		ndm->ndm_flags |= NTF_STICKY;
>  
>  	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>  		goto nla_put_failure;
> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>  
>  /* Update (create or replace) forwarding database entry */
>  static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
> +			 u8 is_sticky)

Why not change the API to take a full ndm flags, someone is sure to add more later.

Re: [Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Stephen Hemminger]

On Mon, 10 Sep 2018 13:16:01 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> Add support for entries which are "sticky", i.e. will not change their port
> if they show up from a different one. A new ndm flag is introduced for that
> purpose - NTF_STICKY. We allow to set it only to non-local entries.

Is there a name for this in other network switch API's?

> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> I'll send the selftest for sticky with the iproute2 patch if this one is
> accepted. We've had multiple requests to support such flag and now it's
> also needed for some eVPN and clag setups.
> 
>  include/uapi/linux/neighbour.h |  1 +
>  net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>  net/bridge/br_private.h        |  1 +
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index 904db6148476..998155444e0d 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -43,6 +43,7 @@ enum {
>  #define NTF_PROXY	0x08	/* == ATF_PUBL */
>  #define NTF_EXT_LEARNED	0x10
>  #define NTF_OFFLOADED   0x20
> +#define NTF_STICKY	0x40
>  #define NTF_ROUTER	0x80
>  
>  /*
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 502f66349530..26569ed06a4d 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  			unsigned long now = jiffies;
>  
>  			/* fastpath: update of existing entry */
> -			if (unlikely(source != fdb->dst)) {
> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>  				fdb->dst = source;
>  				fdb_modified = true;
>  				/* Take over HW learned entry */
> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>  		ndm->ndm_flags |= NTF_OFFLOADED;
>  	if (fdb->added_by_external_learn)
>  		ndm->ndm_flags |= NTF_EXT_LEARNED;
> +	if (fdb->is_sticky)
> +		ndm->ndm_flags |= NTF_STICKY;
>  
>  	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>  		goto nla_put_failure;
> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>  
>  /* Update (create or replace) forwarding database entry */
>  static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
> +			 u8 is_sticky)

Why not change the API to take a full ndm flags, someone is sure to add more later.

Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Grygorii Strashko]

On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
> On Mon, 10 Sep 2018 13:16:01 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> 
>> Add support for entries which are "sticky", i.e. will not change their port
>> if they show up from a different one. A new ndm flag is introduced for that
>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
> 
> Is there a name for this in other network switch API's?

In TI CPSW hardware we have following definition for similar FDB/ALE entries
(AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):

Block (BLOCK)  – The block bit indicates that a packet with a matching source or
destination address should be dropped (block the address).
0 – Address is not blocked.
1 – Drop a packet with a matching source or destination address (secure must be zero)
    If block and secure are both set, then they no longer mean block and secure.
When both are set, the block and secure bits indicate that the packet is 
a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
If both bits are set then the packet is forwarded if the receive port is in
   the Forwarding/Blocking/Learning state.
If both bits are not set then the packet is forwarded if the receive port is in
   the Forwarding state.

Secure (SECURE) - This bit indicates that a packet with a matching source address
should be dropped if the received port number is not equal to the table entry port_number.
0 – Received port number is a don’t care.
1 – Drop the packet if the received port is not the secure port for the source
address and do not update the address (block must be zero)

Updating Process:
if ((source address found) and (receive port number != port_number) and (secure or block))
then do not update address

"sticky" would mean SECURE+BLOCK = supervisory (super) packet

> 
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> I'll send the selftest for sticky with the iproute2 patch if this one is
>> accepted. We've had multiple requests to support such flag and now it's
>> also needed for some eVPN and clag setups.
>>
>>   include/uapi/linux/neighbour.h |  1 +
>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>   net/bridge/br_private.h        |  1 +
>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index 904db6148476..998155444e0d 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -43,6 +43,7 @@ enum {
>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>   #define NTF_EXT_LEARNED	0x10
>>   #define NTF_OFFLOADED   0x20
>> +#define NTF_STICKY	0x40
>>   #define NTF_ROUTER	0x80
>>   
>>   /*
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 502f66349530..26569ed06a4d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>   			unsigned long now = jiffies;
>>   
>>   			/* fastpath: update of existing entry */
>> -			if (unlikely(source != fdb->dst)) {
>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>   				fdb->dst = source;
>>   				fdb_modified = true;
>>   				/* Take over HW learned entry */
>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>   	if (fdb->added_by_external_learn)
>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>> +	if (fdb->is_sticky)
>> +		ndm->ndm_flags |= NTF_STICKY;
>>   
>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>   		goto nla_put_failure;
>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>   
>>   /* Update (create or replace) forwarding database entry */
>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>> +			 u8 is_sticky)
> 
> Why not change the API to take a full ndm flags, someone is sure to add more later.
> 

-- 
regards,
-grygorii

Re: [Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Grygorii Strashko]

On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
> On Mon, 10 Sep 2018 13:16:01 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> 
>> Add support for entries which are "sticky", i.e. will not change their port
>> if they show up from a different one. A new ndm flag is introduced for that
>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
> 
> Is there a name for this in other network switch API's?

In TI CPSW hardware we have following definition for similar FDB/ALE entries
(AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):

Block (BLOCK)  – The block bit indicates that a packet with a matching source or
destination address should be dropped (block the address).
0 – Address is not blocked.
1 – Drop a packet with a matching source or destination address (secure must be zero)
    If block and secure are both set, then they no longer mean block and secure.
When both are set, the block and secure bits indicate that the packet is 
a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
If both bits are set then the packet is forwarded if the receive port is in
   the Forwarding/Blocking/Learning state.
If both bits are not set then the packet is forwarded if the receive port is in
   the Forwarding state.

Secure (SECURE) - This bit indicates that a packet with a matching source address
should be dropped if the received port number is not equal to the table entry port_number.
0 – Received port number is a don’t care.
1 – Drop the packet if the received port is not the secure port for the source
address and do not update the address (block must be zero)

Updating Process:
if ((source address found) and (receive port number != port_number) and (secure or block))
then do not update address

"sticky" would mean SECURE+BLOCK = supervisory (super) packet

> 
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> I'll send the selftest for sticky with the iproute2 patch if this one is
>> accepted. We've had multiple requests to support such flag and now it's
>> also needed for some eVPN and clag setups.
>>
>>   include/uapi/linux/neighbour.h |  1 +
>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>   net/bridge/br_private.h        |  1 +
>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index 904db6148476..998155444e0d 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -43,6 +43,7 @@ enum {
>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>   #define NTF_EXT_LEARNED	0x10
>>   #define NTF_OFFLOADED   0x20
>> +#define NTF_STICKY	0x40
>>   #define NTF_ROUTER	0x80
>>   
>>   /*
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 502f66349530..26569ed06a4d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>   			unsigned long now = jiffies;
>>   
>>   			/* fastpath: update of existing entry */
>> -			if (unlikely(source != fdb->dst)) {
>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>   				fdb->dst = source;
>>   				fdb_modified = true;
>>   				/* Take over HW learned entry */
>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>   	if (fdb->added_by_external_learn)
>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>> +	if (fdb->is_sticky)
>> +		ndm->ndm_flags |= NTF_STICKY;
>>   
>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>   		goto nla_put_failure;
>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>   
>>   /* Update (create or replace) forwarding database entry */
>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>> +			 u8 is_sticky)
> 
> Why not change the API to take a full ndm flags, someone is sure to add more later.
> 

-- 
regards,
-grygorii

Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

On 11/09/18 02:22, Grygorii Strashko wrote:
> 
> 
> On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
>> On Mon, 10 Sep 2018 13:16:01 +0300
>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>
>>> Add support for entries which are "sticky", i.e. will not change their port
>>> if they show up from a different one. A new ndm flag is introduced for that
>>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>>
>> Is there a name for this in other network switch API's?
> 
> In TI CPSW hardware we have following definition for similar FDB/ALE entries
> (AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):
> 
> Block (BLOCK)  – The block bit indicates that a packet with a matching source or
> destination address should be dropped (block the address).
> 0 – Address is not blocked.
> 1 – Drop a packet with a matching source or destination address (secure must be zero)
>     If block and secure are both set, then they no longer mean block and secure.
> When both are set, the block and secure bits indicate that the packet is 
> a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
> If both bits are set then the packet is forwarded if the receive port is in
>    the Forwarding/Blocking/Learning state.
> If both bits are not set then the packet is forwarded if the receive port is in
>    the Forwarding state.
> 
> Secure (SECURE) - This bit indicates that a packet with a matching source address
> should be dropped if the received port number is not equal to the table entry port_number.
> 0 – Received port number is a don’t care.
> 1 – Drop the packet if the received port is not the secure port for the source
> address and do not update the address (block must be zero)
> 
> Updating Process:
> if ((source address found) and (receive port number != port_number) and (secure or block))
> then do not update address
> 
> "sticky" would mean SECURE+BLOCK = supervisory (super) packet
> 

That could work. but we haven't really made any arrangements w.r.t. to current implementations.
So if IUC secure+block should be: 
   - block:
      1 – Drop a packet with a matching source or destination address (secure must be zero)
      If block and secure are both set, then they no longer mean block and secure.

    - secure:
      1 – Drop the packet if the received port is not the secure port for the source
          address and do not update the address (block must be zero)

These could mean a very different config based on their description. Feel free to add.

>>
>>>
>>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>> ---
>>> I'll send the selftest for sticky with the iproute2 patch if this one is
>>> accepted. We've had multiple requests to support such flag and now it's
>>> also needed for some eVPN and clag setups.
>>>
>>>   include/uapi/linux/neighbour.h |  1 +
>>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>>   net/bridge/br_private.h        |  1 +
>>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>> index 904db6148476..998155444e0d 100644
>>> --- a/include/uapi/linux/neighbour.h
>>> +++ b/include/uapi/linux/neighbour.h
>>> @@ -43,6 +43,7 @@ enum {
>>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>>   #define NTF_EXT_LEARNED	0x10
>>>   #define NTF_OFFLOADED   0x20
>>> +#define NTF_STICKY	0x40
>>>   #define NTF_ROUTER	0x80
>>>   
>>>   /*
>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>> index 502f66349530..26569ed06a4d 100644
>>> --- a/net/bridge/br_fdb.c
>>> +++ b/net/bridge/br_fdb.c
>>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>   			unsigned long now = jiffies;
>>>   
>>>   			/* fastpath: update of existing entry */
>>> -			if (unlikely(source != fdb->dst)) {
>>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>>   				fdb->dst = source;
>>>   				fdb_modified = true;
>>>   				/* Take over HW learned entry */
>>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>>   	if (fdb->added_by_external_learn)
>>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>>> +	if (fdb->is_sticky)
>>> +		ndm->ndm_flags |= NTF_STICKY;
>>>   
>>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>   		goto nla_put_failure;
>>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>>   
>>>   /* Update (create or replace) forwarding database entry */
>>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>>> +			 u8 is_sticky)
>>
>> Why not change the API to take a full ndm flags, someone is sure to add more later.

Sure, I've added a similar "fix" on my bridge todo list which is getting very long by the way,

but it is not the point of this patch.

Re: [Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

On 11/09/18 02:22, Grygorii Strashko wrote:
> 
> 
> On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
>> On Mon, 10 Sep 2018 13:16:01 +0300
>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>
>>> Add support for entries which are "sticky", i.e. will not change their port
>>> if they show up from a different one. A new ndm flag is introduced for that
>>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>>
>> Is there a name for this in other network switch API's?
> 
> In TI CPSW hardware we have following definition for similar FDB/ALE entries
> (AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):
> 
> Block (BLOCK)  – The block bit indicates that a packet with a matching source or
> destination address should be dropped (block the address).
> 0 – Address is not blocked.
> 1 – Drop a packet with a matching source or destination address (secure must be zero)
>     If block and secure are both set, then they no longer mean block and secure.
> When both are set, the block and secure bits indicate that the packet is 
> a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
> If both bits are set then the packet is forwarded if the receive port is in
>    the Forwarding/Blocking/Learning state.
> If both bits are not set then the packet is forwarded if the receive port is in
>    the Forwarding state.
> 
> Secure (SECURE) - This bit indicates that a packet with a matching source address
> should be dropped if the received port number is not equal to the table entry port_number.
> 0 – Received port number is a don’t care.
> 1 – Drop the packet if the received port is not the secure port for the source
> address and do not update the address (block must be zero)
> 
> Updating Process:
> if ((source address found) and (receive port number != port_number) and (secure or block))
> then do not update address
> 
> "sticky" would mean SECURE+BLOCK = supervisory (super) packet
> 

That could work. but we haven't really made any arrangements w.r.t. to current implementations.
So if IUC secure+block should be: 
   - block:
      1 – Drop a packet with a matching source or destination address (secure must be zero)
      If block and secure are both set, then they no longer mean block and secure.

    - secure:
      1 – Drop the packet if the received port is not the secure port for the source
          address and do not update the address (block must be zero)

These could mean a very different config based on their description. Feel free to add.

>>
>>>
>>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>> ---
>>> I'll send the selftest for sticky with the iproute2 patch if this one is
>>> accepted. We've had multiple requests to support such flag and now it's
>>> also needed for some eVPN and clag setups.
>>>
>>>   include/uapi/linux/neighbour.h |  1 +
>>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>>   net/bridge/br_private.h        |  1 +
>>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>> index 904db6148476..998155444e0d 100644
>>> --- a/include/uapi/linux/neighbour.h
>>> +++ b/include/uapi/linux/neighbour.h
>>> @@ -43,6 +43,7 @@ enum {
>>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>>   #define NTF_EXT_LEARNED	0x10
>>>   #define NTF_OFFLOADED   0x20
>>> +#define NTF_STICKY	0x40
>>>   #define NTF_ROUTER	0x80
>>>   
>>>   /*
>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>> index 502f66349530..26569ed06a4d 100644
>>> --- a/net/bridge/br_fdb.c
>>> +++ b/net/bridge/br_fdb.c
>>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>   			unsigned long now = jiffies;
>>>   
>>>   			/* fastpath: update of existing entry */
>>> -			if (unlikely(source != fdb->dst)) {
>>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>>   				fdb->dst = source;
>>>   				fdb_modified = true;
>>>   				/* Take over HW learned entry */
>>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>>   	if (fdb->added_by_external_learn)
>>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>>> +	if (fdb->is_sticky)
>>> +		ndm->ndm_flags |= NTF_STICKY;
>>>   
>>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>   		goto nla_put_failure;
>>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>>   
>>>   /* Update (create or replace) forwarding database entry */
>>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>>> +			 u8 is_sticky)
>>
>> Why not change the API to take a full ndm flags, someone is sure to add more later.

Sure, I've added a similar "fix" on my bridge todo list which is getting very long by the way,

but it is not the point of this patch.

Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

On 11/09/18 02:55, Nikolay Aleksandrov wrote:
> On 11/09/18 02:22, Grygorii Strashko wrote:
>>
>>
>> On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
>>> On Mon, 10 Sep 2018 13:16:01 +0300
>>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>
>>>> Add support for entries which are "sticky", i.e. will not change their port
>>>> if they show up from a different one. A new ndm flag is introduced for that
>>>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>>>
>>> Is there a name for this in other network switch API's?
>>
>> In TI CPSW hardware we have following definition for similar FDB/ALE entries
>> (AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):
>>
>> Block (BLOCK)  – The block bit indicates that a packet with a matching source or
>> destination address should be dropped (block the address).
>> 0 – Address is not blocked.
>> 1 – Drop a packet with a matching source or destination address (secure must be zero)
>>     If block and secure are both set, then they no longer mean block and secure.
>> When both are set, the block and secure bits indicate that the packet is 
>> a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
>> If both bits are set then the packet is forwarded if the receive port is in
>>    the Forwarding/Blocking/Learning state.
>> If both bits are not set then the packet is forwarded if the receive port is in
>>    the Forwarding state.
>>
>> Secure (SECURE) - This bit indicates that a packet with a matching source address
>> should be dropped if the received port number is not equal to the table entry port_number.
>> 0 – Received port number is a don’t care.
>> 1 – Drop the packet if the received port is not the secure port for the source
>> address and do not update the address (block must be zero)
>>
>> Updating Process:
>> if ((source address found) and (receive port number != port_number) and (secure or block))
>> then do not update address
>>
>> "sticky" would mean SECURE+BLOCK = supervisory (super) packet
>>
> 
> That could work. but we haven't really made any arrangements w.r.t. to current implementations.
> So if IUC secure+block should be: 
>    - block:
>       1 – Drop a packet with a matching source or destination address (secure must be zero)
>       If block and secure are both set, then they no longer mean block and secure.> 
>     - secure:
>       1 – Drop the packet if the received port is not the secure port for the source
>           address and do not update the address (block must be zero)
> 
> These could mean a very different config based on their description. Feel free to add.
> 

Nevermind my comment, got it. To support them fully though we will need to have more options.
They should be achievable through firewall as well.

>>>
>>>>
>>>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>> ---
>>>> I'll send the selftest for sticky with the iproute2 patch if this one is
>>>> accepted. We've had multiple requests to support such flag and now it's
>>>> also needed for some eVPN and clag setups.
>>>>
>>>>   include/uapi/linux/neighbour.h |  1 +
>>>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>>>   net/bridge/br_private.h        |  1 +
>>>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>>> index 904db6148476..998155444e0d 100644
>>>> --- a/include/uapi/linux/neighbour.h
>>>> +++ b/include/uapi/linux/neighbour.h
>>>> @@ -43,6 +43,7 @@ enum {
>>>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>>>   #define NTF_EXT_LEARNED	0x10
>>>>   #define NTF_OFFLOADED   0x20
>>>> +#define NTF_STICKY	0x40
>>>>   #define NTF_ROUTER	0x80
>>>>   
>>>>   /*
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index 502f66349530..26569ed06a4d 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>>   			unsigned long now = jiffies;
>>>>   
>>>>   			/* fastpath: update of existing entry */
>>>> -			if (unlikely(source != fdb->dst)) {
>>>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>>>   				fdb->dst = source;
>>>>   				fdb_modified = true;
>>>>   				/* Take over HW learned entry */
>>>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>>>   	if (fdb->added_by_external_learn)
>>>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>>>> +	if (fdb->is_sticky)
>>>> +		ndm->ndm_flags |= NTF_STICKY;
>>>>   
>>>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>>   		goto nla_put_failure;
>>>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>>>   
>>>>   /* Update (create or replace) forwarding database entry */
>>>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>>>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>>>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>>>> +			 u8 is_sticky)
>>>
>>> Why not change the API to take a full ndm flags, someone is sure to add more later.
> 
> Sure, I've added a similar "fix" on my bridge todo list which is getting very long by the way,
> 
> but it is not the point of this patch.
> 

I'll send v2 with your proposed change, indeed more people will probably make use of it in the
future. Thanks for the feedback.

Cheers,
 Nik

Re: [Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

On 11/09/18 02:55, Nikolay Aleksandrov wrote:
> On 11/09/18 02:22, Grygorii Strashko wrote:
>>
>>
>> On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
>>> On Mon, 10 Sep 2018 13:16:01 +0300
>>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>
>>>> Add support for entries which are "sticky", i.e. will not change their port
>>>> if they show up from a different one. A new ndm flag is introduced for that
>>>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>>>
>>> Is there a name for this in other network switch API's?
>>
>> In TI CPSW hardware we have following definition for similar FDB/ALE entries
>> (AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):
>>
>> Block (BLOCK)  – The block bit indicates that a packet with a matching source or
>> destination address should be dropped (block the address).
>> 0 – Address is not blocked.
>> 1 – Drop a packet with a matching source or destination address (secure must be zero)
>>     If block and secure are both set, then they no longer mean block and secure.
>> When both are set, the block and secure bits indicate that the packet is 
>> a unicast supervisory (super) packet and they determine the unicast forward state test criteria. 
>> If both bits are set then the packet is forwarded if the receive port is in
>>    the Forwarding/Blocking/Learning state.
>> If both bits are not set then the packet is forwarded if the receive port is in
>>    the Forwarding state.
>>
>> Secure (SECURE) - This bit indicates that a packet with a matching source address
>> should be dropped if the received port number is not equal to the table entry port_number.
>> 0 – Received port number is a don’t care.
>> 1 – Drop the packet if the received port is not the secure port for the source
>> address and do not update the address (block must be zero)
>>
>> Updating Process:
>> if ((source address found) and (receive port number != port_number) and (secure or block))
>> then do not update address
>>
>> "sticky" would mean SECURE+BLOCK = supervisory (super) packet
>>
> 
> That could work. but we haven't really made any arrangements w.r.t. to current implementations.
> So if IUC secure+block should be: 
>    - block:
>       1 – Drop a packet with a matching source or destination address (secure must be zero)
>       If block and secure are both set, then they no longer mean block and secure.> 
>     - secure:
>       1 – Drop the packet if the received port is not the secure port for the source
>           address and do not update the address (block must be zero)
> 
> These could mean a very different config based on their description. Feel free to add.
> 

Nevermind my comment, got it. To support them fully though we will need to have more options.
They should be achievable through firewall as well.

>>>
>>>>
>>>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>> ---
>>>> I'll send the selftest for sticky with the iproute2 patch if this one is
>>>> accepted. We've had multiple requests to support such flag and now it's
>>>> also needed for some eVPN and clag setups.
>>>>
>>>>   include/uapi/linux/neighbour.h |  1 +
>>>>   net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>>>   net/bridge/br_private.h        |  1 +
>>>>   3 files changed, 18 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>>> index 904db6148476..998155444e0d 100644
>>>> --- a/include/uapi/linux/neighbour.h
>>>> +++ b/include/uapi/linux/neighbour.h
>>>> @@ -43,6 +43,7 @@ enum {
>>>>   #define NTF_PROXY	0x08	/* == ATF_PUBL */
>>>>   #define NTF_EXT_LEARNED	0x10
>>>>   #define NTF_OFFLOADED   0x20
>>>> +#define NTF_STICKY	0x40
>>>>   #define NTF_ROUTER	0x80
>>>>   
>>>>   /*
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index 502f66349530..26569ed06a4d 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>>   			unsigned long now = jiffies;
>>>>   
>>>>   			/* fastpath: update of existing entry */
>>>> -			if (unlikely(source != fdb->dst)) {
>>>> +			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>>>   				fdb->dst = source;
>>>>   				fdb_modified = true;
>>>>   				/* Take over HW learned entry */
>>>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>   		ndm->ndm_flags |= NTF_OFFLOADED;
>>>>   	if (fdb->added_by_external_learn)
>>>>   		ndm->ndm_flags |= NTF_EXT_LEARNED;
>>>> +	if (fdb->is_sticky)
>>>> +		ndm->ndm_flags |= NTF_STICKY;
>>>>   
>>>>   	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>>   		goto nla_put_failure;
>>>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>>>   
>>>>   /* Update (create or replace) forwarding database entry */
>>>>   static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>>>> -			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>>>> +			 const u8 *addr, u16 state, u16 flags, u16 vid,
>>>> +			 u8 is_sticky)
>>>
>>> Why not change the API to take a full ndm flags, someone is sure to add more later.
> 
> Sure, I've added a similar "fix" on my bridge todo list which is getting very long by the way,
> 
> but it is not the point of this patch.
> 

I'll send v2 with your proposed change, indeed more people will probably make use of it in the
future. Thanks for the feedback.

Cheers,
 Nik

[PATCH net-next v2] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

Add support for entries which are "sticky", i.e. will not change their port
if they show up from a different one. A new ndm flag is introduced for that
purpose - NTF_STICKY. We allow to set it only to non-local entries.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: As Stephen suggested pass the whole ndm_flags because it will probably
    be used by someone else soon.

I'll send the selftest for sticky with the iproute2 patch if this one is
accepted. We've had multiple requests to support such flag and now it's
also needed for some eVPN and clag setups.

 include/uapi/linux/neighbour.h |  1 +
 net/bridge/br_fdb.c            | 19 ++++++++++++++++---
 net/bridge/br_private.h        |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 904db6148476..998155444e0d 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -43,6 +43,7 @@ enum {
 #define NTF_PROXY	0x08	/* == ATF_PUBL */
 #define NTF_EXT_LEARNED	0x10
 #define NTF_OFFLOADED   0x20
+#define NTF_STICKY	0x40
 #define NTF_ROUTER	0x80
 
 /*
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 502f66349530..a56ed7f2a3a3 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 			unsigned long now = jiffies;
 
 			/* fastpath: update of existing entry */
-			if (unlikely(source != fdb->dst)) {
+			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
 				fdb->dst = source;
 				fdb_modified = true;
 				/* Take over HW learned entry */
@@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
 		ndm->ndm_flags |= NTF_OFFLOADED;
 	if (fdb->added_by_external_learn)
 		ndm->ndm_flags |= NTF_EXT_LEARNED;
+	if (fdb->is_sticky)
+		ndm->ndm_flags |= NTF_STICKY;
 
 	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
 		goto nla_put_failure;
@@ -772,8 +774,10 @@ int br_fdb_dump(struct sk_buff *skb,
 
 /* Update (create or replace) forwarding database entry */
 static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
-			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
+			 const u8 *addr, u16 state, u16 flags, u16 vid,
+			 u8 ndm_flags)
 {
+	u8 is_sticky = !!(ndm_flags & NTF_STICKY);
 	struct net_bridge_fdb_entry *fdb;
 	bool modified = false;
 
@@ -789,6 +793,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 		return -EINVAL;
 	}
 
+	if (is_sticky && (state & NUD_PERMANENT))
+		return -EINVAL;
+
 	fdb = br_fdb_find(br, addr, vid);
 	if (fdb == NULL) {
 		if (!(flags & NLM_F_CREATE))
@@ -832,6 +839,12 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 
 		modified = true;
 	}
+
+	if (is_sticky != fdb->is_sticky) {
+		fdb->is_sticky = is_sticky;
+		modified = true;
+	}
+
 	fdb->added_by_user = 1;
 
 	fdb->used = jiffies;
@@ -865,7 +878,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
 	} else {
 		spin_lock_bh(&br->hash_lock);
 		err = fdb_add_entry(br, p, addr, ndm->ndm_state,
-				    nlh_flags, vid);
+				    nlh_flags, vid, ndm->ndm_flags);
 		spin_unlock_bh(&br->hash_lock);
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 11ed2029985f..d21035a17f4c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -181,6 +181,7 @@ struct net_bridge_fdb_entry {
 	struct hlist_node		fdb_node;
 	unsigned char			is_local:1,
 					is_static:1,
+					is_sticky:1,
 					added_by_user:1,
 					added_by_external_learn:1,
 					offloaded:1;
-- 
2.11.0

[Bridge] [PATCH net-next v2] net: bridge: add support for sticky fdb entries

[Nikolay Aleksandrov]

Add support for entries which are "sticky", i.e. will not change their port
if they show up from a different one. A new ndm flag is introduced for that
purpose - NTF_STICKY. We allow to set it only to non-local entries.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: As Stephen suggested pass the whole ndm_flags because it will probably
    be used by someone else soon.

I'll send the selftest for sticky with the iproute2 patch if this one is
accepted. We've had multiple requests to support such flag and now it's
also needed for some eVPN and clag setups.

 include/uapi/linux/neighbour.h |  1 +
 net/bridge/br_fdb.c            | 19 ++++++++++++++++---
 net/bridge/br_private.h        |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 904db6148476..998155444e0d 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -43,6 +43,7 @@ enum {
 #define NTF_PROXY	0x08	/* == ATF_PUBL */
 #define NTF_EXT_LEARNED	0x10
 #define NTF_OFFLOADED   0x20
+#define NTF_STICKY	0x40
 #define NTF_ROUTER	0x80
 
 /*
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 502f66349530..a56ed7f2a3a3 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 			unsigned long now = jiffies;
 
 			/* fastpath: update of existing entry */
-			if (unlikely(source != fdb->dst)) {
+			if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
 				fdb->dst = source;
 				fdb_modified = true;
 				/* Take over HW learned entry */
@@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
 		ndm->ndm_flags |= NTF_OFFLOADED;
 	if (fdb->added_by_external_learn)
 		ndm->ndm_flags |= NTF_EXT_LEARNED;
+	if (fdb->is_sticky)
+		ndm->ndm_flags |= NTF_STICKY;
 
 	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
 		goto nla_put_failure;
@@ -772,8 +774,10 @@ int br_fdb_dump(struct sk_buff *skb,
 
 /* Update (create or replace) forwarding database entry */
 static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
-			 const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
+			 const u8 *addr, u16 state, u16 flags, u16 vid,
+			 u8 ndm_flags)
 {
+	u8 is_sticky = !!(ndm_flags & NTF_STICKY);
 	struct net_bridge_fdb_entry *fdb;
 	bool modified = false;
 
@@ -789,6 +793,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 		return -EINVAL;
 	}
 
+	if (is_sticky && (state & NUD_PERMANENT))
+		return -EINVAL;
+
 	fdb = br_fdb_find(br, addr, vid);
 	if (fdb == NULL) {
 		if (!(flags & NLM_F_CREATE))
@@ -832,6 +839,12 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
 
 		modified = true;
 	}
+
+	if (is_sticky != fdb->is_sticky) {
+		fdb->is_sticky = is_sticky;
+		modified = true;
+	}
+
 	fdb->added_by_user = 1;
 
 	fdb->used = jiffies;
@@ -865,7 +878,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
 	} else {
 		spin_lock_bh(&br->hash_lock);
 		err = fdb_add_entry(br, p, addr, ndm->ndm_state,
-				    nlh_flags, vid);
+				    nlh_flags, vid, ndm->ndm_flags);
 		spin_unlock_bh(&br->hash_lock);
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 11ed2029985f..d21035a17f4c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -181,6 +181,7 @@ struct net_bridge_fdb_entry {
 	struct hlist_node		fdb_node;
 	unsigned char			is_local:1,
 					is_static:1,
+					is_sticky:1,
 					added_by_user:1,
 					added_by_external_learn:1,
 					offloaded:1;
-- 
2.11.0

Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Roopa Prabhu]

On Mon, Sep 10, 2018 at 2:18 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 10 Sep 2018 13:16:01 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> Add support for entries which are "sticky", i.e. will not change their port
>> if they show up from a different one. A new ndm flag is introduced for that
>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>
> Is there a name for this in other network switch API's?


In all switch ASIC's static fdb entries implicitly don't move.
Since the kernel does not follow the same for static fdb entries, we
want a new flag to
explicitly make the mac moves not possible.

As nikolay says, primary request for this came from an E-VPN rfc. It
uses the name 'sticky'...
hence the name 'sticky'  https://tools.ietf.org/html/rfc7432#section-15.2



>
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> I'll send the selftest for sticky with the iproute2 patch if this one is
>> accepted. We've had multiple requests to support such flag and now it's
>> also needed for some eVPN and clag setups.
>>
>>  include/uapi/linux/neighbour.h |  1 +
>>  net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>  net/bridge/br_private.h        |  1 +
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index 904db6148476..998155444e0d 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -43,6 +43,7 @@ enum {
>>  #define NTF_PROXY    0x08    /* == ATF_PUBL */
>>  #define NTF_EXT_LEARNED      0x10
>>  #define NTF_OFFLOADED   0x20
>> +#define NTF_STICKY   0x40
>>  #define NTF_ROUTER   0x80
>>
>>  /*
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 502f66349530..26569ed06a4d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>                       unsigned long now = jiffies;
>>
>>                       /* fastpath: update of existing entry */
>> -                     if (unlikely(source != fdb->dst)) {
>> +                     if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>                               fdb->dst = source;
>>                               fdb_modified = true;
>>                               /* Take over HW learned entry */
>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>               ndm->ndm_flags |= NTF_OFFLOADED;
>>       if (fdb->added_by_external_learn)
>>               ndm->ndm_flags |= NTF_EXT_LEARNED;
>> +     if (fdb->is_sticky)
>> +             ndm->ndm_flags |= NTF_STICKY;
>>
>>       if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>               goto nla_put_failure;
>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>
>>  /* Update (create or replace) forwarding database entry */
>>  static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> -                      const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>> +                      const u8 *addr, u16 state, u16 flags, u16 vid,
>> +                      u8 is_sticky)
>
> Why not change the API to take a full ndm flags, someone is sure to add more later.

Re: [Bridge] [PATCH net-next] net: bridge: add support for sticky fdb entries

[Roopa Prabhu]

On Mon, Sep 10, 2018 at 2:18 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 10 Sep 2018 13:16:01 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> Add support for entries which are "sticky", i.e. will not change their port
>> if they show up from a different one. A new ndm flag is introduced for that
>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>
> Is there a name for this in other network switch API's?


In all switch ASIC's static fdb entries implicitly don't move.
Since the kernel does not follow the same for static fdb entries, we
want a new flag to
explicitly make the mac moves not possible.

As nikolay says, primary request for this came from an E-VPN rfc. It
uses the name 'sticky'...
hence the name 'sticky'  https://tools.ietf.org/html/rfc7432#section-15.2



>
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> I'll send the selftest for sticky with the iproute2 patch if this one is
>> accepted. We've had multiple requests to support such flag and now it's
>> also needed for some eVPN and clag setups.
>>
>>  include/uapi/linux/neighbour.h |  1 +
>>  net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>  net/bridge/br_private.h        |  1 +
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index 904db6148476..998155444e0d 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -43,6 +43,7 @@ enum {
>>  #define NTF_PROXY    0x08    /* == ATF_PUBL */
>>  #define NTF_EXT_LEARNED      0x10
>>  #define NTF_OFFLOADED   0x20
>> +#define NTF_STICKY   0x40
>>  #define NTF_ROUTER   0x80
>>
>>  /*
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 502f66349530..26569ed06a4d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>                       unsigned long now = jiffies;
>>
>>                       /* fastpath: update of existing entry */
>> -                     if (unlikely(source != fdb->dst)) {
>> +                     if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>                               fdb->dst = source;
>>                               fdb_modified = true;
>>                               /* Take over HW learned entry */
>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>               ndm->ndm_flags |= NTF_OFFLOADED;
>>       if (fdb->added_by_external_learn)
>>               ndm->ndm_flags |= NTF_EXT_LEARNED;
>> +     if (fdb->is_sticky)
>> +             ndm->ndm_flags |= NTF_STICKY;
>>
>>       if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>               goto nla_put_failure;
>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>
>>  /* Update (create or replace) forwarding database entry */
>>  static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> -                      const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>> +                      const u8 *addr, u16 state, u16 flags, u16 vid,
>> +                      u8 is_sticky)
>
> Why not change the API to take a full ndm flags, someone is sure to add more later.

Re: [PATCH net-next v2] net: bridge: add support for sticky fdb entries

[David Miller]

From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 11 Sep 2018 09:39:53 +0300

> Add support for entries which are "sticky", i.e. will not change their port
> if they show up from a different one. A new ndm flag is introduced for that
> purpose - NTF_STICKY. We allow to set it only to non-local entries.
> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> v2: As Stephen suggested pass the whole ndm_flags because it will probably
>     be used by someone else soon.

Applied, thanks Nikolay.

Re: [Bridge] [PATCH net-next v2] net: bridge: add support for sticky fdb entries

[David Miller]

From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 11 Sep 2018 09:39:53 +0300

> Add support for entries which are "sticky", i.e. will not change their port
> if they show up from a different one. A new ndm flag is introduced for that
> purpose - NTF_STICKY. We allow to set it only to non-local entries.
> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> v2: As Stephen suggested pass the whole ndm_flags because it will probably
>     be used by someone else soon.

Applied, thanks Nikolay.