From owner-freebsd-net@FreeBSD.ORG Sun Jan 9 23:27:11 2011 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 72820106566C for ; Sun, 9 Jan 2011 23:27:11 +0000 (UTC) (envelope-from jroberson@jroberson.net) Received: from mail-qy0-f182.google.com (mail-qy0-f182.google.com [209.85.216.182]) by mx1.freebsd.org (Postfix) with ESMTP id 31EE88FC0C for ; Sun, 9 Jan 2011 23:27:11 +0000 (UTC) Received: by qyk36 with SMTP id 36so18345821qyk.13 for ; Sun, 09 Jan 2011 15:27:10 -0800 (PST) Received: by 10.224.37.139 with SMTP id x11mr26643185qad.152.1294615629678; Sun, 09 Jan 2011 15:27:09 -0800 (PST) Received: from [10.0.1.198] ([72.253.42.56]) by mx.google.com with ESMTPS id l12sm16992741qcu.43.2011.01.09.15.27.07 (version=SSLv3 cipher=RC4-MD5); Sun, 09 Jan 2011 15:27:08 -0800 (PST) Date: Sun, 9 Jan 2011 13:29:59 -1000 (HST) From: Jeff Roberson X-X-Sender: jroberson@desktop To: "Bjoern A. Zeeb" In-Reply-To: <20110109102339.K14966@maildrop.int.zabbadoz.net> Message-ID: References: <20110109102339.K14966@maildrop.int.zabbadoz.net> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-net@freebsd.org Subject: Re: vlan changes to support ipoib X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Jan 2011 23:27:11 -0000 On Sun, 9 Jan 2011, Bjoern A. Zeeb wrote: > On Sat, 8 Jan 2011, Jeff Roberson wrote: > >> Hi Folks, >> >> I have made some changes to vlans and I was wondering if anyone would care >> to review or test. Especially current vlan users. The diff is here: >> >> http://people.freebsd.org/~jeff/vlan.diff >> >> I can't say that I like adding all of the function pointers but with vlan >> support available as a loadable module I have little choice. The new >> functions allow a driver to fetch the virtual interface associated with a >> tag, cache and fetch a cookie pointer in a virtual interface, and fetch the >> tag of a virtual interface. >> >> Additionally, the driver was changed to make no assumptions about address >> size unless the type is IFT_ETHER. This means for multicast entries we >> store a full sockaddr_dl rather than an ethernet address. It actually >> simplifies the code here. I also simplified the VLAN_ARRAY stuff by moving >> it into functions that match the hash names so there aren't ifdef's >> everywhere. I've tested both hash and array. >> >> I also changed the global mtx to an sx lock so the vlan_config event >> handlers can allocate memory. The way ipoib works requires a full new >> ipoib softc when a vlan is created. I didn't want to duplicate the vlan >> config logic so I store that softc using the cookie field and fetch it when >> necessary. As a result ipoib also doesn't need the weird vlan_input() >> recursion since the packets appear on the correct device as they are >> received. >> >> I am committing this to my infiniband tree immediately but I would like >> some review before I merge to current. > > May I ask you to split the diff up into logical junks for both review > and commit? Otherwise possible errors not caught with by review will > just be a lot harder to track down later. Most if not all of the chunks are interdependent or useless without each other. When I commit to current it will probably be a merge from my branch anyway. What would you like to see separated? > > Some of the stuff like "Initialize from parent" seems to be simple > enough to be possibly merged just upfront leaving the real beef. I notice that I don't have many comments. Perhaps if I documented better what I have done it would be sufficient? Thanks, Jeff > > /bz > > -- > Bjoern A. Zeeb You have to have visions! > Going to jail sucks -- All my daemons like it! > http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/jails.html >