* [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
@ 2015-06-23 12:28 ` Nikolay Aleksandrov
0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Aleksandrov @ 2015-06-23 12:28 UTC (permalink / raw)
To: netdev; +Cc: Nikolay Aleksandrov, vyasevich, bridge, davem
Add a new argument to br_fdb_delete_by_port which allows to specify a
vid to match when flushing entries and use it in nbp_vlan_delete() to
flush the dynamically learned entries of the vlan/port pair when removing
a vlan from a port. Before this patch only the local mac was being
removed and the dynamically learned ones were left to expire.
Note that the do_all argument is still respected and if specified, the
vid will be ignored.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
note: There are 2 warnings, the C99 comment was already there and the 81
char line is the prototype and I think it looks better this way.
One unclear thing is about vid 0, I see that it can be passed to
nbp_vlan_(add|delete) although the comment says it shouldn't.
Adding Vlad to CC list since he has introduced the vlan filtering
infrastructure.
net/bridge/br_fdb.c | 7 +++++--
net/bridge/br_if.c | 4 ++--
net/bridge/br_private.h | 2 +-
net/bridge/br_stp_if.c | 2 +-
net/bridge/br_sysfs_if.c | 2 +-
net/bridge/br_vlan.c | 1 +
6 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index be84b7e5a3da..9e9875da0a4f 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -330,9 +330,11 @@ void br_fdb_flush(struct net_bridge *br)
/* Flush all entries referring to a specific port.
* if do_all is set also flush static entries
+ * if vid is set delete all entries that match the vlan_id
*/
void br_fdb_delete_by_port(struct net_bridge *br,
const struct net_bridge_port *p,
+ u16 vid,
int do_all)
{
int i;
@@ -347,8 +349,9 @@ void br_fdb_delete_by_port(struct net_bridge *br,
if (f->dst != p)
continue;
- if (f->is_static && !do_all)
- continue;
+ if (!do_all)
+ if (f->is_static || (vid && f->vlan_id != vid))
+ continue;
if (f->is_local)
fdb_delete_local(br, p, f);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 1849d96b3c91..a538cb1199a3 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -249,7 +249,7 @@ static void del_nbp(struct net_bridge_port *p)
list_del_rcu(&p->list);
nbp_vlan_flush(p);
- br_fdb_delete_by_port(br, p, 1);
+ br_fdb_delete_by_port(br, p, 0, 1);
nbp_update_port_count(br);
netdev_upper_dev_unlink(dev, br->dev);
@@ -278,7 +278,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
del_nbp(p);
}
- br_fdb_delete_by_port(br, NULL, 1);
+ br_fdb_delete_by_port(br, NULL, 0, 1);
br_vlan_flush(br);
del_timer_sync(&br->gc_timer);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 5dccced71269..8b21146b24a0 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -387,7 +387,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr);
void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr);
void br_fdb_cleanup(unsigned long arg);
void br_fdb_delete_by_port(struct net_bridge *br,
- const struct net_bridge_port *p, int do_all);
+ const struct net_bridge_port *p, u16 vid, int do_all);
struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
const unsigned char *addr, __u16 vid);
int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 41146872c1b4..762d490a16f2 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -111,7 +111,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
del_timer(&p->forward_delay_timer);
del_timer(&p->hold_timer);
- br_fdb_delete_by_port(br, p, 0);
+ br_fdb_delete_by_port(br, p, 0, 0);
br_multicast_disable_port(p);
br_configuration_update(br);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 4905845a94e9..efe415ad842a 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -160,7 +160,7 @@ static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
static int store_flush(struct net_bridge_port *p, unsigned long v)
{
- br_fdb_delete_by_port(p->br, p, 0); // Don't delete local entry
+ br_fdb_delete_by_port(p->br, p, 0, 0); // Don't delete local entry
return 0;
}
static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 17fc358a5432..9e1b9c1b4880 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -741,6 +741,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
return -EINVAL;
br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
+ br_fdb_delete_by_port(port->br, port, vid, 0);
return __vlan_del(pv, vid);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Bridge] [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
@ 2015-06-23 12:28 ` Nikolay Aleksandrov
0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Aleksandrov @ 2015-06-23 12:28 UTC (permalink / raw)
To: netdev; +Cc: Nikolay Aleksandrov, vyasevich, bridge, davem
Add a new argument to br_fdb_delete_by_port which allows to specify a
vid to match when flushing entries and use it in nbp_vlan_delete() to
flush the dynamically learned entries of the vlan/port pair when removing
a vlan from a port. Before this patch only the local mac was being
removed and the dynamically learned ones were left to expire.
Note that the do_all argument is still respected and if specified, the
vid will be ignored.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
note: There are 2 warnings, the C99 comment was already there and the 81
char line is the prototype and I think it looks better this way.
One unclear thing is about vid 0, I see that it can be passed to
nbp_vlan_(add|delete) although the comment says it shouldn't.
Adding Vlad to CC list since he has introduced the vlan filtering
infrastructure.
net/bridge/br_fdb.c | 7 +++++--
net/bridge/br_if.c | 4 ++--
net/bridge/br_private.h | 2 +-
net/bridge/br_stp_if.c | 2 +-
net/bridge/br_sysfs_if.c | 2 +-
net/bridge/br_vlan.c | 1 +
6 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index be84b7e5a3da..9e9875da0a4f 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -330,9 +330,11 @@ void br_fdb_flush(struct net_bridge *br)
/* Flush all entries referring to a specific port.
* if do_all is set also flush static entries
+ * if vid is set delete all entries that match the vlan_id
*/
void br_fdb_delete_by_port(struct net_bridge *br,
const struct net_bridge_port *p,
+ u16 vid,
int do_all)
{
int i;
@@ -347,8 +349,9 @@ void br_fdb_delete_by_port(struct net_bridge *br,
if (f->dst != p)
continue;
- if (f->is_static && !do_all)
- continue;
+ if (!do_all)
+ if (f->is_static || (vid && f->vlan_id != vid))
+ continue;
if (f->is_local)
fdb_delete_local(br, p, f);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 1849d96b3c91..a538cb1199a3 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -249,7 +249,7 @@ static void del_nbp(struct net_bridge_port *p)
list_del_rcu(&p->list);
nbp_vlan_flush(p);
- br_fdb_delete_by_port(br, p, 1);
+ br_fdb_delete_by_port(br, p, 0, 1);
nbp_update_port_count(br);
netdev_upper_dev_unlink(dev, br->dev);
@@ -278,7 +278,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
del_nbp(p);
}
- br_fdb_delete_by_port(br, NULL, 1);
+ br_fdb_delete_by_port(br, NULL, 0, 1);
br_vlan_flush(br);
del_timer_sync(&br->gc_timer);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 5dccced71269..8b21146b24a0 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -387,7 +387,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr);
void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr);
void br_fdb_cleanup(unsigned long arg);
void br_fdb_delete_by_port(struct net_bridge *br,
- const struct net_bridge_port *p, int do_all);
+ const struct net_bridge_port *p, u16 vid, int do_all);
struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
const unsigned char *addr, __u16 vid);
int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 41146872c1b4..762d490a16f2 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -111,7 +111,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
del_timer(&p->forward_delay_timer);
del_timer(&p->hold_timer);
- br_fdb_delete_by_port(br, p, 0);
+ br_fdb_delete_by_port(br, p, 0, 0);
br_multicast_disable_port(p);
br_configuration_update(br);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 4905845a94e9..efe415ad842a 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -160,7 +160,7 @@ static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
static int store_flush(struct net_bridge_port *p, unsigned long v)
{
- br_fdb_delete_by_port(p->br, p, 0); // Don't delete local entry
+ br_fdb_delete_by_port(p->br, p, 0, 0); // Don't delete local entry
return 0;
}
static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 17fc358a5432..9e1b9c1b4880 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -741,6 +741,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
return -EINVAL;
br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
+ br_fdb_delete_by_port(port->br, port, vid, 0);
return __vlan_del(pv, vid);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
2015-06-23 12:28 ` [Bridge] " Nikolay Aleksandrov
@ 2015-06-23 15:02 ` Toshiaki Makita
-1 siblings, 0 replies; 8+ messages in thread
From: Toshiaki Makita @ 2015-06-23 15:02 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: vyasevich, bridge, davem
On 15/06/23 (火) 21:28, Nikolay Aleksandrov wrote:
> Add a new argument to br_fdb_delete_by_port which allows to specify a
> vid to match when flushing entries and use it in nbp_vlan_delete() to
> flush the dynamically learned entries of the vlan/port pair when removing
> a vlan from a port. Before this patch only the local mac was being
> removed and the dynamically learned ones were left to expire.
> Note that the do_all argument is still respected and if specified, the
> vid will be ignored.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> note: There are 2 warnings, the C99 comment was already there and the 81
> char line is the prototype and I think it looks better this way.
> One unclear thing is about vid 0, I see that it can be passed to
> nbp_vlan_(add|delete) although the comment says it shouldn't.
nbp_vlan_add() should not be called with vid 0 because vid 0 of fdb
entries is internally used when vlan_filtering is disabled, and such
entries are exposed to userspace as those without any vlan id.
bridge's vlan_filtering is respecting 802.1Q-2011 spec, which says vid 0
cannot be used as a normal vlan id.
If vlan 0 can be added, it should be a bug.
Toshiaki Makita
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Bridge] [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
@ 2015-06-23 15:02 ` Toshiaki Makita
0 siblings, 0 replies; 8+ messages in thread
From: Toshiaki Makita @ 2015-06-23 15:02 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: vyasevich, bridge, davem
On 15/06/23 (火) 21:28, Nikolay Aleksandrov wrote:
> Add a new argument to br_fdb_delete_by_port which allows to specify a
> vid to match when flushing entries and use it in nbp_vlan_delete() to
> flush the dynamically learned entries of the vlan/port pair when removing
> a vlan from a port. Before this patch only the local mac was being
> removed and the dynamically learned ones were left to expire.
> Note that the do_all argument is still respected and if specified, the
> vid will be ignored.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> note: There are 2 warnings, the C99 comment was already there and the 81
> char line is the prototype and I think it looks better this way.
> One unclear thing is about vid 0, I see that it can be passed to
> nbp_vlan_(add|delete) although the comment says it shouldn't.
nbp_vlan_add() should not be called with vid 0 because vid 0 of fdb
entries is internally used when vlan_filtering is disabled, and such
entries are exposed to userspace as those without any vlan id.
bridge's vlan_filtering is respecting 802.1Q-2011 spec, which says vid 0
cannot be used as a normal vlan id.
If vlan 0 can be added, it should be a bug.
Toshiaki Makita
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
2015-06-23 15:02 ` [Bridge] " Toshiaki Makita
@ 2015-06-23 15:16 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 8+ messages in thread
From: Nikolay Aleksandrov @ 2015-06-23 15:16 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: netdev, vyasevich, bridge, davem
> On Jun 23, 2015, at 6:02 PM, Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
>
> On 15/06/23 (火) 21:28, Nikolay Aleksandrov wrote:
>> Add a new argument to br_fdb_delete_by_port which allows to specify a
>> vid to match when flushing entries and use it in nbp_vlan_delete() to
>> flush the dynamically learned entries of the vlan/port pair when removing
>> a vlan from a port. Before this patch only the local mac was being
>> removed and the dynamically learned ones were left to expire.
>> Note that the do_all argument is still respected and if specified, the
>> vid will be ignored.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> note: There are 2 warnings, the C99 comment was already there and the 81
>> char line is the prototype and I think it looks better this way.
>> One unclear thing is about vid 0, I see that it can be passed to
>> nbp_vlan_(add|delete) although the comment says it shouldn't.
>
> nbp_vlan_add() should not be called with vid 0 because vid 0 of fdb
> entries is internally used when vlan_filtering is disabled, and such
> entries are exposed to userspace as those without any vlan id.
> bridge's vlan_filtering is respecting 802.1Q-2011 spec, which says vid 0
> cannot be used as a normal vlan id.
> If vlan 0 can be added, it should be a bug.
>
> Toshiaki Makita
Right, that is my understanding as well. I’ll look into this and post a fix
if necessary.
Thanks,
Nik
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Bridge] [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
@ 2015-06-23 15:16 ` Nikolay Aleksandrov
0 siblings, 0 replies; 8+ messages in thread
From: Nikolay Aleksandrov @ 2015-06-23 15:16 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: netdev, vyasevich, bridge, davem
> On Jun 23, 2015, at 6:02 PM, Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
>
> On 15/06/23 (火) 21:28, Nikolay Aleksandrov wrote:
>> Add a new argument to br_fdb_delete_by_port which allows to specify a
>> vid to match when flushing entries and use it in nbp_vlan_delete() to
>> flush the dynamically learned entries of the vlan/port pair when removing
>> a vlan from a port. Before this patch only the local mac was being
>> removed and the dynamically learned ones were left to expire.
>> Note that the do_all argument is still respected and if specified, the
>> vid will be ignored.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> note: There are 2 warnings, the C99 comment was already there and the 81
>> char line is the prototype and I think it looks better this way.
>> One unclear thing is about vid 0, I see that it can be passed to
>> nbp_vlan_(add|delete) although the comment says it shouldn't.
>
> nbp_vlan_add() should not be called with vid 0 because vid 0 of fdb
> entries is internally used when vlan_filtering is disabled, and such
> entries are exposed to userspace as those without any vlan id.
> bridge's vlan_filtering is respecting 802.1Q-2011 spec, which says vid 0
> cannot be used as a normal vlan id.
> If vlan 0 can be added, it should be a bug.
>
> Toshiaki Makita
Right, that is my understanding as well. I’ll look into this and post a fix
if necessary.
Thanks,
Nik
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
2015-06-23 12:28 ` [Bridge] " Nikolay Aleksandrov
@ 2015-06-30 13:50 ` Vlad Yasevich
-1 siblings, 0 replies; 8+ messages in thread
From: Vlad Yasevich @ 2015-06-30 13:50 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: bridge, davem
On 06/23/2015 08:28 AM, Nikolay Aleksandrov wrote:
> Add a new argument to br_fdb_delete_by_port which allows to specify a
> vid to match when flushing entries and use it in nbp_vlan_delete() to
> flush the dynamically learned entries of the vlan/port pair when removing
> a vlan from a port. Before this patch only the local mac was being
> removed and the dynamically learned ones were left to expire.
> Note that the do_all argument is still respected and if specified, the
> vid will be ignored.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> note: There are 2 warnings, the C99 comment was already there and the 81
> char line is the prototype and I think it looks better this way.
> One unclear thing is about vid 0, I see that it can be passed to
> nbp_vlan_(add|delete) although the comment says it shouldn't.
> Adding Vlad to CC list since he has introduced the vlan filtering
> infrastructure.
sorry, missed this one somewhow...
There is no explicit check inside the nbp_vlan_[add|del] because most checks
are at higher level functions. It should be impossible to call it with vid of
0.
This code looks fine to me.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
>
> net/bridge/br_fdb.c | 7 +++++--
> net/bridge/br_if.c | 4 ++--
> net/bridge/br_private.h | 2 +-
> net/bridge/br_stp_if.c | 2 +-
> net/bridge/br_sysfs_if.c | 2 +-
> net/bridge/br_vlan.c | 1 +
> 6 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index be84b7e5a3da..9e9875da0a4f 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -330,9 +330,11 @@ void br_fdb_flush(struct net_bridge *br)
>
> /* Flush all entries referring to a specific port.
> * if do_all is set also flush static entries
> + * if vid is set delete all entries that match the vlan_id
> */
> void br_fdb_delete_by_port(struct net_bridge *br,
> const struct net_bridge_port *p,
> + u16 vid,
> int do_all)
> {
> int i;
> @@ -347,8 +349,9 @@ void br_fdb_delete_by_port(struct net_bridge *br,
> if (f->dst != p)
> continue;
>
> - if (f->is_static && !do_all)
> - continue;
> + if (!do_all)
> + if (f->is_static || (vid && f->vlan_id != vid))
> + continue;
>
> if (f->is_local)
> fdb_delete_local(br, p, f);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 1849d96b3c91..a538cb1199a3 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -249,7 +249,7 @@ static void del_nbp(struct net_bridge_port *p)
> list_del_rcu(&p->list);
>
> nbp_vlan_flush(p);
> - br_fdb_delete_by_port(br, p, 1);
> + br_fdb_delete_by_port(br, p, 0, 1);
> nbp_update_port_count(br);
>
> netdev_upper_dev_unlink(dev, br->dev);
> @@ -278,7 +278,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
> del_nbp(p);
> }
>
> - br_fdb_delete_by_port(br, NULL, 1);
> + br_fdb_delete_by_port(br, NULL, 0, 1);
>
> br_vlan_flush(br);
> del_timer_sync(&br->gc_timer);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 5dccced71269..8b21146b24a0 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -387,7 +387,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr);
> void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr);
> void br_fdb_cleanup(unsigned long arg);
> void br_fdb_delete_by_port(struct net_bridge *br,
> - const struct net_bridge_port *p, int do_all);
> + const struct net_bridge_port *p, u16 vid, int do_all);
> struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
> const unsigned char *addr, __u16 vid);
> int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 41146872c1b4..762d490a16f2 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -111,7 +111,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
> del_timer(&p->forward_delay_timer);
> del_timer(&p->hold_timer);
>
> - br_fdb_delete_by_port(br, p, 0);
> + br_fdb_delete_by_port(br, p, 0, 0);
> br_multicast_disable_port(p);
>
> br_configuration_update(br);
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index 4905845a94e9..efe415ad842a 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -160,7 +160,7 @@ static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
>
> static int store_flush(struct net_bridge_port *p, unsigned long v)
> {
> - br_fdb_delete_by_port(p->br, p, 0); // Don't delete local entry
> + br_fdb_delete_by_port(p->br, p, 0, 0); // Don't delete local entry
> return 0;
> }
> static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 17fc358a5432..9e1b9c1b4880 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -741,6 +741,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
> return -EINVAL;
>
> br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
> + br_fdb_delete_by_port(port->br, port, vid, 0);
>
> return __vlan_del(pv, vid);
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Bridge] [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete
@ 2015-06-30 13:50 ` Vlad Yasevich
0 siblings, 0 replies; 8+ messages in thread
From: Vlad Yasevich @ 2015-06-30 13:50 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: bridge, davem
On 06/23/2015 08:28 AM, Nikolay Aleksandrov wrote:
> Add a new argument to br_fdb_delete_by_port which allows to specify a
> vid to match when flushing entries and use it in nbp_vlan_delete() to
> flush the dynamically learned entries of the vlan/port pair when removing
> a vlan from a port. Before this patch only the local mac was being
> removed and the dynamically learned ones were left to expire.
> Note that the do_all argument is still respected and if specified, the
> vid will be ignored.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> note: There are 2 warnings, the C99 comment was already there and the 81
> char line is the prototype and I think it looks better this way.
> One unclear thing is about vid 0, I see that it can be passed to
> nbp_vlan_(add|delete) although the comment says it shouldn't.
> Adding Vlad to CC list since he has introduced the vlan filtering
> infrastructure.
sorry, missed this one somewhow...
There is no explicit check inside the nbp_vlan_[add|del] because most checks
are at higher level functions. It should be impossible to call it with vid of
0.
This code looks fine to me.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
>
> net/bridge/br_fdb.c | 7 +++++--
> net/bridge/br_if.c | 4 ++--
> net/bridge/br_private.h | 2 +-
> net/bridge/br_stp_if.c | 2 +-
> net/bridge/br_sysfs_if.c | 2 +-
> net/bridge/br_vlan.c | 1 +
> 6 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index be84b7e5a3da..9e9875da0a4f 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -330,9 +330,11 @@ void br_fdb_flush(struct net_bridge *br)
>
> /* Flush all entries referring to a specific port.
> * if do_all is set also flush static entries
> + * if vid is set delete all entries that match the vlan_id
> */
> void br_fdb_delete_by_port(struct net_bridge *br,
> const struct net_bridge_port *p,
> + u16 vid,
> int do_all)
> {
> int i;
> @@ -347,8 +349,9 @@ void br_fdb_delete_by_port(struct net_bridge *br,
> if (f->dst != p)
> continue;
>
> - if (f->is_static && !do_all)
> - continue;
> + if (!do_all)
> + if (f->is_static || (vid && f->vlan_id != vid))
> + continue;
>
> if (f->is_local)
> fdb_delete_local(br, p, f);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 1849d96b3c91..a538cb1199a3 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -249,7 +249,7 @@ static void del_nbp(struct net_bridge_port *p)
> list_del_rcu(&p->list);
>
> nbp_vlan_flush(p);
> - br_fdb_delete_by_port(br, p, 1);
> + br_fdb_delete_by_port(br, p, 0, 1);
> nbp_update_port_count(br);
>
> netdev_upper_dev_unlink(dev, br->dev);
> @@ -278,7 +278,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
> del_nbp(p);
> }
>
> - br_fdb_delete_by_port(br, NULL, 1);
> + br_fdb_delete_by_port(br, NULL, 0, 1);
>
> br_vlan_flush(br);
> del_timer_sync(&br->gc_timer);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 5dccced71269..8b21146b24a0 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -387,7 +387,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr);
> void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr);
> void br_fdb_cleanup(unsigned long arg);
> void br_fdb_delete_by_port(struct net_bridge *br,
> - const struct net_bridge_port *p, int do_all);
> + const struct net_bridge_port *p, u16 vid, int do_all);
> struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
> const unsigned char *addr, __u16 vid);
> int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 41146872c1b4..762d490a16f2 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -111,7 +111,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
> del_timer(&p->forward_delay_timer);
> del_timer(&p->hold_timer);
>
> - br_fdb_delete_by_port(br, p, 0);
> + br_fdb_delete_by_port(br, p, 0, 0);
> br_multicast_disable_port(p);
>
> br_configuration_update(br);
> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> index 4905845a94e9..efe415ad842a 100644
> --- a/net/bridge/br_sysfs_if.c
> +++ b/net/bridge/br_sysfs_if.c
> @@ -160,7 +160,7 @@ static BRPORT_ATTR(hold_timer, S_IRUGO, show_hold_timer, NULL);
>
> static int store_flush(struct net_bridge_port *p, unsigned long v)
> {
> - br_fdb_delete_by_port(p->br, p, 0); // Don't delete local entry
> + br_fdb_delete_by_port(p->br, p, 0, 0); // Don't delete local entry
> return 0;
> }
> static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 17fc358a5432..9e1b9c1b4880 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -741,6 +741,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
> return -EINVAL;
>
> br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
> + br_fdb_delete_by_port(port->br, port, vid, 0);
>
> return __vlan_del(pv, vid);
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-06-30 13:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-23 12:28 [PATCH net-next] bridge: vlan: flush the dynamically learned entries on port vlan delete Nikolay Aleksandrov
2015-06-23 12:28 ` [Bridge] " Nikolay Aleksandrov
2015-06-23 15:02 ` Toshiaki Makita
2015-06-23 15:02 ` [Bridge] " Toshiaki Makita
2015-06-23 15:16 ` Nikolay Aleksandrov
2015-06-23 15:16 ` [Bridge] " Nikolay Aleksandrov
2015-06-30 13:50 ` Vlad Yasevich
2015-06-30 13:50 ` [Bridge] " Vlad Yasevich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.