On 01/20/2013 04:38 PM, Shmulik Ladkani wrote:
Hi Vlad,
On Wed, 16 Jan 2013 13:17:57 -0500 Vlad Yasevich <vyasevic@xxxxxxxxxx> wrote:
@@ -156,6 +183,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ struct net_port_vlans vlan_info;
(here and at 'struct net_bridge' as well)
Not sure what the policy is; Isn't it preferred to enclose the new
fields under CONFIG_BRIDGE_VLAN_FILTERING?
Good catch. Missed this one.
+static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)
+{
+ struct net_bridge *br;
+
+ if (!vlans->port_idx)
+ br = container_of((vlans), struct net_bridge, vlan_info);
+ else
+ br = vlans_to_port(vlans)->br;
+
+ return br;
+}
Guess it would simplify things if the bridge "master port" had an 'nbp'
representation of its own ;-)
Yes that would simplify things for this case, but it will also add a lot
of state that's not necessary. I considered doing this, but the master
device doesn't really act as port in on things, just some.
+extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
Seems 'br_vlan_find' can be declared static within br_vlan.c.
Will check.
+extern void br_vlan_flush(struct net_bridge *br);
According to your preference, consider s/br_vlan_flush/br_vlans_flush/
since it better suggest acting on all bridge's vlans.
+extern void nbp_vlan_flush(struct net_port_vlans *vlans);
According to your preference, consider s/nbp_vlan_flush/nbp_vlans_flush/
since it better suggest acting on all port's vlans.
+void br_vlan_flush(struct net_bridge *br)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ struct hlist_node *tmp;
+ int i;
+
+ nbp_vlan_flush(&br->vlan_info);
+
+ /* Make sure that there are no vlans left in the bridge after
+ * all the ports have been removed.
+ */
Improper indent.
will fix
+ for (i = 0; i < BR_VID_HASH_SIZE; i++) {
+ hlist_for_each_entry_safe(vlan, node, tmp,
+ &br->vlan_hlist[i], hlist) {
+ br_vlan_del(vlan);
Can there be any vlans left at that point? Shouldn't del_nbp() on all
ports take care of that?
I think this function might have been a leftover from prior series when
bridge didn't have its vlan list. With the new series, I don't this it
is needed. I'll double-check.
Thanks
-vlad
Also, if there _were_ any vlans left (whose bitmap isn't cleared),
'br_vlan_del' won't do a thing.
Am I missing something?
Regards,
Shmulik
