From owner-cvs-src@FreeBSD.ORG Mon Apr 23 10:52:46 2007 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B5B9116A402; Mon, 23 Apr 2007 10:52:46 +0000 (UTC) (envelope-from bms@FreeBSD.org) Received: from out4.smtp.messagingengine.com (out4.smtp.messagingengine.com [66.111.4.28]) by mx1.freebsd.org (Postfix) with ESMTP id 79AAE13C4B7; Mon, 23 Apr 2007 10:52:46 +0000 (UTC) (envelope-from bms@FreeBSD.org) Received: from compute2.internal (compute2.internal [10.202.2.42]) by out1.messagingengine.com (Postfix) with ESMTP id B75D6217545; Mon, 23 Apr 2007 06:52:52 -0400 (EDT) Received: from heartbeat1.messagingengine.com ([10.202.2.160]) by compute2.internal (MEProxy); Mon, 23 Apr 2007 06:52:46 -0400 X-Sasl-enc: Ae5HViFXazqZ13xtIlpzhNpOchCWKhdLmV8kSmcyzI4F 1177325566 Received: from [192.168.123.18] (82-35-112-254.cable.ubr07.dals.blueyonder.co.uk [82.35.112.254]) by mail.messagingengine.com (Postfix) with ESMTP id C327A293BE; Mon, 23 Apr 2007 06:52:45 -0400 (EDT) Message-ID: <462C8FFB.7030309@FreeBSD.org> Date: Mon, 23 Apr 2007 11:52:43 +0100 From: "Bruce M. Simpson" User-Agent: Thunderbird 1.5.0.10 (X11/20070407) MIME-Version: 1.0 To: Gleb Smirnoff References: <200704140101.l3E11kum000736@repoman.freebsd.org> <20070423095356.GB2742@FreeBSD.org> In-Reply-To: <20070423095356.GB2742@FreeBSD.org> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/contrib/pf/net if_pfsync.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Apr 2007 10:52:46 -0000 Gleb, With all due respect, I disagree with some of the points you raise in this message. It could well also be argued that the addition of new subsystems such as carp, pfsync etc., without thinking through the implications that this has had for the use of structures which have not until now been allocated outside of netinet or net, was also a 'bad design idea'. One need only review struct ifnet to see this. However, this doesn't help us. FreeBSD continues to be used for real applications. As I am sure you arew well aware, the network stack has a number of issues relating to interfaces going away during runtime. This is because the architecture was never initially designed to cope with this situation. Gleb Smirnoff wrote: > > Sorry for late reply on this commit, I've had email problems. > > Bruce, I still think that freeing multicast memberships in the > in_ifdetach() was a bad design idea. Memory should be allocated and > freed by the same module, otherwise we've got a messy architecture. Whilst I agree that how in_purgemaddrs() works is far from ideal, it exists because there has been no API contract in the code, express or implied, other than 'FreeBSD's network stack will continue to leak memory in these situations', which is not acceptable in an operating system used for real applications, to my mind. Reference counting in in_multi may be relied upon up until the point at which the netinet stack is implicitly detached from the interface. With the current code, removal of in_purgemaddrs() reintroduces the previous memory leak. After that, all bets are off. pfsync specifically relies on a detach handler which is independent of netinet. The fix is consistent with the behaviour of the rest of the code in that the ifp is still valid however netinet has been implicitly detached from the ifp. Fixing the root problem requires that we re-think how protocol domains are attached to struct ifnet, and I think I already mentioned this in private to Robert. I agree with you that the current fix is not ideal, however it's the most appropriate fix for the code as it currently stands. I stand by my work, you are free to improve upon it. Kind regards BMS