From owner-freebsd-net@FreeBSD.ORG Mon Jan 10 11:10:07 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 728ED1065696 for ; Mon, 10 Jan 2011 11:10:07 +0000 (UTC) (envelope-from bzeeb-lists@lists.zabbadoz.net) Received: from mail.cksoft.de (mail.cksoft.de [IPv6:2001:4068:10::3]) by mx1.freebsd.org (Postfix) with ESMTP id F22608FC2A for ; Mon, 10 Jan 2011 11:10:06 +0000 (UTC) Received: from localhost (amavis.fra.cksoft.de [192.168.74.71]) by mail.cksoft.de (Postfix) with ESMTP id 06E6F41C7C3; Mon, 10 Jan 2011 12:10:06 +0100 (CET) X-Virus-Scanned: amavisd-new at cksoft.de Received: from mail.cksoft.de ([192.168.74.103]) by localhost (amavis.fra.cksoft.de [192.168.74.71]) (amavisd-new, port 10024) with ESMTP id 28dIqrQnwH9p; Mon, 10 Jan 2011 12:10:05 +0100 (CET) Received: by mail.cksoft.de (Postfix, from userid 66) id 5033541C7C9; Mon, 10 Jan 2011 12:10:05 +0100 (CET) Received: from maildrop.int.zabbadoz.net (maildrop.int.zabbadoz.net [10.111.66.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.int.zabbadoz.net (Postfix) with ESMTP id 04A844448F3; Mon, 10 Jan 2011 11:08:52 +0000 (UTC) Date: Mon, 10 Jan 2011 11:08:52 +0000 (UTC) From: "Bjoern A. Zeeb" X-X-Sender: bz@maildrop.int.zabbadoz.net To: Jeff Roberson In-Reply-To: Message-ID: <20110110103329.W14966@maildrop.int.zabbadoz.net> References: <20110109102339.K14966@maildrop.int.zabbadoz.net> X-OpenPGP-Key: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 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: Mon, 10 Jan 2011 11:10:07 -0000 On Sun, 9 Jan 2011, Jeff Roberson wrote: > 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? - the noise (ifp initialization, ..) - the address size changes in preparation for .. - locking can maybe come with the rest of the (*f) for ipoib I think as that's what needs it. You could go by the paragraphs you have above as well. >> >> 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? I fear the one "large" commit that in 16 month from now someone will have no idea about. Been there. I know it's additional work but it sometimes really helps. Having debugged too many network stack bugs from corner cases the last 2 years, I'd really appreciate it. /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