Date: Mon, 23 Apr 2007 11:52:43 +0100 From: "Bruce M. Simpson" <bms@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> 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 Message-ID: <462C8FFB.7030309@FreeBSD.org> In-Reply-To: <20070423095356.GB2742@FreeBSD.org> References: <200704140101.l3E11kum000736@repoman.freebsd.org> <20070423095356.GB2742@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?462C8FFB.7030309>