cvs commit: src/sys/contrib/pf/net if_pfsync.c

[Bruce M. Simpson]

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