On 10/11/2015 02:21 PM, Ido Schimmel wrote: > Wed, Sep 30, 2015 at 09:16:54PM IDT, razor@xxxxxxxxxxxxx wrote: >> From: Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> >> >> When a new port is being added we need to make vlgrp available after >> rhashtable has been initialized and when removing a port we need to >> flush the vlans and free the resources after we're sure noone can use >> the port, i.e. after it's removed from the port list and synchronize_rcu >> is executed. > > Hi Nikolay, > > Changing the order of port deinit breaks symmetry with the init > sequence. It also introduces a problem for switchdev drivers. Flushing > the VLANs clears HW VLAN filters and then, when port is unlinked from > bridge and CHANGEUPPER is received, port is configured to direct traffic > to CPU (as it's not offloaded anymore). Doing the reverse (like in this > patch) renders the port unusable. > > Regarding the reason for this change, are you afraid that vlgrp will be > accessed in bridge's rx handler or xmit function after it's freed? If > so, maybe we can access it using RCU primitives? That way, both the rx > handler and xmit function (executed under RCU lock) will either have a > valid copy or not. Looking at previous iterations of this code, I see > that was the case with the 'net_port_vlans' struct. > > I can start working on a fix if you agree with the proposed solution. > > Thanks. > Hi, Ah, I didn't know about this, I feared that something might rely on the particular order of the operations but didn't have a way to test this one in particular. Anyway, your proposed solution sounds good to me. Thank you, Nik