cvs commit: src/sys/netgraph/netflow ng_netflow.c

[Louis Mamakos]

On Feb 5, 2008, at 9:17 AM, Gleb Smirnoff wrote:

> On Sun, Feb 03, 2008 at 11:36:49AM -0500, Louis Mamakos wrote:
> L> > Gleb Smirnoff пишет:
> L> >>  you should have asked me for review before committing! This is
> L> >> not a bug, this is a feature. This was quite clear from the  
> comments,
> L> >> that you removed:
> L> >> -       /* if export hook disconnected stop running expire(). */
> L> >>  This is intended behavior. We must not lose information unless
> L> >> user explicitly wants to lose information. In the latter case
> L> >> he will connect ng_hole(4) node to the "export" hook. But we  
> must
> L> >> not lose information if user runs some script that swaps  
> receiving
> L> >> node on the "export" hook.
> L> >>  Please backout this change!
> L> >
> L> > Expire process was not depending completely on connected hook  
> even before
> L> > this commit. For example, every TCP session closing forces some  
> data
> L> > export. So even with export hook disconnected some data still  
> will be lost
> L> > and not just lost, but it was leading to memory leak which I  
> have fixed
> L> > with other commit.
>
> That's true. The active TCP close should be reworked. And the new  
> active expiry
> feature violates the original design, when no export hook ment, no  
> data lose. :(
>
> L>  If there's a concern about no losing the netflow data, then it's  
> likely that
> L>  it's usually the case that an export hook is connected.  If a  
> user wanted to
> L>  change the export arrangement for the netflow data, then just  
> disconnected
> L>  and reconnecting to the export hook won't caused data to be lost  
> if the
> L>  expiry parameters are set to something reasonable.
>
> Since expiry runs periodically, then it can race with hook change

I'm not sure why I'd have an expectation that I would never, under any
circumstances lose data when switching the export hook.  If I really,
really wanted to arrange for that, perhaps I'd connect the export hook
to a "tee" node so I could swap in different export destinations in a
"make before break" sort of arrangement.

>
>
> L>  Finally, in the absence of infinite amounts of memory, data will  
> eventually
> L>  be lost.  The only decision is over what duration data should be  
> kept around
> L>  so that it might be harvested.  It's a huge surprise that the  
> netflow module
> L>  consumes large amounts of kernel memory.  As a user, I expected  
> the
> L>  expiration timers to be the policy that I specify to control how  
> long the
> L>  netflow stats are stored, and my expectation wasn't met.
>
> Huge surprise? How can you expect a kernel module that stores a lot  
> of data
> consume a little kernel memory?

I suppose the problem is that I had no expectation that a kernel  
module, would
consume unbounded amounts of kernel resources.  I certainly didn't  
expect that
it would have a need to store "a lot of data" given that there are  
documented
parameters on how the in-kernel state should be expired.  That this  
expiration
doesn't occur is a significant difference that would I would have  
expected as
reasonable behavior.

You start with the presumption that the data being collected is so  
precious that
it cannot be dropped under any circumstances.  That's probably a faulty
premise to begin with, given that most of the netflow export happens  
on an
unreliable UDP transport.
>
> I agree that the behavior should be documented in manual page and  
> using
> ng_hole(4) for your case should be advised. If you send me a manual  
> page patch,
> I can commit it.

Driving the kernel into resource exhaustion for no really good reason  
doesn't
seem like the right default behavior.  I really think that the netflow
module should default into a safe mode of operation rather than  
unexpected
consumption of a limited resource.

Louis Mamakos