From owner-freebsd-current@FreeBSD.ORG Tue Sep 9 12:17:19 2014 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B66A11E9 for ; Tue, 9 Sep 2014 12:17:19 +0000 (UTC) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebius.int.ru", Issuer "cell.glebius.int.ru" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 2C508104 for ; Tue, 9 Sep 2014 12:17:18 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.9/8.14.9) with ESMTP id s89CH9LR045055 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 9 Sep 2014 16:17:09 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.9/8.14.9/Submit) id s89CH8FJ045054; Tue, 9 Sep 2014 16:17:08 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Tue, 9 Sep 2014 16:17:08 +0400 From: Gleb Smirnoff To: Luigi Rizzo Subject: Re: RFC: please put back spare fields in struct ifnet (removed in svn 270870) Message-ID: <20140909121708.GE17059@glebius.int.ru> References: <20140909103719.GB17059@glebius.int.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: George Neville-Neil , FreeBSD Current , Stefano Garzarella X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Sep 2014 12:17:19 -0000 Luigi, On Tue, Sep 09, 2014 at 01:01:13PM +0200, Luigi Rizzo wrote: L> > The harm is obvious: someone commits code that _uses_ spare field L> > without assigning it a new name. Spare field is a placeholder. Of L> > course you can use it while you experiment. However, I don't see L> > problem with patching your source tree where you experiment. L> L> The problem with _not_ having spares is that you have to recompile L> everything after patching the headers. With the spares, i could L> make netmap a simple add-on kernel module with no dependencies. L> Same on linux for what matters (there wasn't a spare there, but L> nobody used ax25 and i could check and reuse it). What if another kernel hacker comes and also uses if_pspare[0] for his own super-duper feature? What would you do, if then a user reports that enabling netmap on his box instapanics if he had some other feature turned on? L> > The ABI plan for 'struct ifnet' is that the struct becomes L> > opaque for device drivers. So its size and alignment no longer L> > matters. Those who want to add new fields to struct ifnet, L> > would also need to add accessor methods. Bits of this plan L> > are already committed by Marcel, but its only first step. L> L> ​spare fields <-> spare accessors​ Let me repeat: in future device drivers will not be capable to dereference struct ifnet. So, any feature pointers would be accessed via functions, making struct ifnet no longer part of ABI. This hasn't yet happened, but one already can (and should!) use accessors in any new code. L> > I'm afraid that if fields are there back, the situation that L> > happened with netmap (use of spare field) would repeat. L> L> ​the spare pointer used by netmap was clearly indicated by a comment, L> and giving it a dedicated name instead of if_pspare[0] was L> expected to happen (hopefully together with a __FreeBSD_version bump, L> which has not happened). I don't agree that /* 1 netmap, 7 TDB */ is a clear comment. I understood as "in future one of those is to be used for netmap". Regarding __FreeBSD_version: the field should have been renamed in the commit that brought in netmap, not in my commit. My commit removed a field that must not be used, so it is not a reason to bump. If you need closest __FreeBSD_version for the change, then please use 1100030, it happened next day. L> I am not worried that the name change was missed when deleting if_pspare[]. L> Mistakes happen and the error was promptly corrected (apart from the L> version bump). L> L> What worries me is losing some flexibility in dealing with L> out-of-tree kernel modules. Just put a properly named placeholder for them as a quick solution. A nicer solution would be to add an API for storing vendor-specific pointers on ifnet, providing a cookie. I'm discussing that now with kmacy@, who also thinks in that direction. -- Totus tuus, Glebius.