Re: [PATCH v7 net-next 01/12] bridge: Add vlan filtering infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/31/2013 03:13 PM, Vlad Yasevich wrote:
On 01/31/2013 02:57 PM, Michał Mirosław wrote:
2013/1/31 Vlad Yasevich <vyasevic@xxxxxxxxxx>:
Adds an optional infrustructure component to bridge that would allow
native vlan filtering in the bridge.  Each bridge port (as well
as the bridge device) now get a VLAN bitmap.  Each bit in the bitmap
is associated with a vlan id.  This way if the bit corresponding to
the vid is set in the bitmap that the packet with vid is allowed to
enter and exit the port.

Write access the bitmap is protected by RTNL and read access
protected by RCU.
[...]
+static int __vlan_del(struct net_port_vlans *v, u16 vid)
+{
+       unsigned long first_bit;
+       unsigned long last_bit;
+
+       if (!test_bit(vid, v->vlan_bitmap))
+               return -EINVAL;
+
+       /* Check to see if any other vlans are in this table.  If this
+        * is the last vlan, delete the whole structure.  If this is
not the
+        * last vlan, just clear the bit.
+        */
+       first_bit = find_first_bit(v->vlan_bitmap, BR_VLAN_BITMAP_LEN);
+       last_bit = find_last_bit(v->vlan_bitmap, BR_VLAN_BITMAP_LEN);
+
+       if (v->port_idx && vid) {
+               struct net_device *dev = vlans_to_port(v)->dev;
+
+               if (dev->features & NETIF_F_HW_VLAN_FILTER)
+                       dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, vid);
+       }
+
+       clear_bit(vid, v->vlan_bitmap);
+       if (first_bit == last_bit) {

if (bitmap_empty(v->vlan_bitmap, BR_VLAN_BITMAP_LEN))

Yeah.  I didn't have the clear_bit about before, but with it
bitmap_empty() is much better.


+               if (v->port_idx) {
+                       struct net_bridge_port *p = vlans_to_port(v);
+                       rcu_assign_pointer(p->vlan_info, NULL);
+               } else {
+                       struct net_bridge *br = vlans_to_bridge(v);
+                       rcu_assign_pointer(br->vlan_info, NULL);
+               }

You seem to use vlans_to_port/vlans_to_bridge only to get at
vlan_info. Maybe that could be abstracted to a single interface, or
even change v->parent to be a 'net_port_vlans **'?

Hmm..  net_port_vlan** has appeal.  I'll see if I can make it work.

So, I went about rewriting this only to realize that there is a bug in
patch 10 and I need the conversion functions to fix it.
I can't really abstract it to a single interface without adding the whole nbp/port layer on top of the bridge device and that's overkill. Changing to net_port_vlans** doesn't buy me anything other then obfuscation.

So I think I am going to keep the much simpler parent pointer and conversion functions since I will need them later.

Thanks
-vlad

Thanks
-vlad

Best Regards,
Michał Mirosław





[Index of Archives]     [Netdev]     [AoE Tools]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux