vlan patch

[Yar Tikhiy]

On Fri, Oct 21, 2005 at 10:40:28AM +0400, Gleb Smirnoff wrote:
> On Fri, Oct 21, 2005 at 10:06:55AM +0400, Yar Tikhiy wrote:
> Y> On Fri, Oct 21, 2005 at 09:30:33AM +0400, Gleb Smirnoff wrote:
> Y> > On Thu, Oct 20, 2005 at 12:57:21PM +0400, Yar Tikhiy wrote:
> Y> > Y> The hash code consists of literally a couple of #define's.  And the
> Y> > Y> difference between ng_vlan(4) and vlan(4) is that each ng_vlan node
> Y> > Y> gets its own instance of the hash table.  OTOH, in vlan(4) we need
> Y> > Y> to decide if the hash table will be per parent interface or a single
> Y> > Y> global instance.  In the latter case we could hash by a combination
> Y> > Y> of the VLAN tag and parent's ifindex.  Perhaps this approach will
> Y> > Y> yield more CPU cache hits during hash table lookups.  In addition,
> Y> > Y> it will be thriftier in using memory.  Locking the global hash table
> Y> > Y> should not be an issue as we can use an sx lock in this case for
> Y> > Y> optimal read access.
> Y> > 
> Y> > The sx lock is slow. We'd better use per interface hash, and thus
> Y> > get locking instantly, with per-vlan lock. In other case, we will
> Y> > acquire per-vlan lock + the sx lock on every packet. The sx lock
> Y> > actually means mtx_lock+mtx_unlock, thus we will make 3 mutex
> Y> > operations instead of one.
> Y> 
> Y> OK, let's forget about sx locks.  However, a per-interface hash is
> Y> associated with a _physical_ interface, hence we must find the vlan
> Y> to lock using the hash first.  If there were a physical interface
> Y> lock held by its driver in each case, it could protect the hash as
> Y> well.  Can we rely on this?
> 
> Oops. When speaking about per-vlan lock, I really meant per-trunk lock. 
> 
> Or are you going to implement per-vlan lock? Is this going to be a benefit?
> Since all packets on trunk are serialized by NIC driver, can there be any
> benefit in creating a mutex per vlan interface, not per vlan trunk?

I think you make a good point.  Indeed, today 2 threads won't process
at once 2 vlan packets coming from the same parent interface.  The
curious question is whether this state of matters will stay forever.
The other approach would be to have separate threads for ip_input,
ipfw, ip_output, and the rest of network stack functions, but it
appears too inefficient in our case due to the overhead of context
switches and locking.  So I agree that locking vlans per trunk is
better today.

Do you think we should stick the vlan mutex into struct ifnet, too,
along with the pointer to the vlan hash?  Perhaps, it's time to
separate vlan-related fields into a separate struct?

-- 
Yar