[PATCH][XFRM] export SAD info

Dave,

Something ive been meaning to do since you made the hash changes. I will
be doing one also for policy. Against latest Linus tree because i am
having strange challenges syncing net-2.6.22.

cheers,
jamal

That patch has xfrm_state_num being mucked with; just ignore that bit.
I need to send a patch against net-2.6.22 and i will clean that up -
just need some feedback.

Would it make sense to have those vars as u32 instead of unsigned int?

cheers,
jamal

From: jamal <redacted>
Date: Wed, 25 Apr 2007 11:54:50 -0400

That patch has xfrm_state_num being mucked with; just ignore that bit.
I need to send a patch against net-2.6.22 and i will clean that up -
just need some feedback.

Would it make sense to have those vars as u32 instead of unsigned int?

I'm ambivalent, "unsigned int" happens to be 32-bit on every platform.
So changing it would cause no harm :-)

On Thu, 2007-26-04 at 00:18 -0700, David Miller wrote:

From: jamal <redacted>

quoted

Would it make sense to have those vars as u32 instead of unsigned int?

I'm ambivalent, "unsigned int" happens to be 32-bit on every platform.
So changing it would cause no harm :-)

If unsigned int is always u32 i will leave it as is.

I would have liked to just do a read_lock_bh when retrieving the table
metadata; however, the state table lock is defined as DEFINE_SPINLOCK
unlike the policy table which is defined as DEFINE_RWLOCK.
Any objection to change the state lock to be RW?

BTW, if i can get the SADinfo, then i should be able to set it from user
space too;->
So that would be my next change unless there is objection.

One other angle is start rejecting additions to the table after some
point. To test, I wrote a little DOS tool that just kept adding entries
until an OOM hit. It is a lot of fun to watch when you hit a point that
swap is guzzling 2G or more. The add latency starts going up
exponentially.
I would like to enable the admin to set the proper param settings for
upper bound. Exceeding the upper bounds of the max entries a table
should have returns ENOMEM for any new entries. By default current
behavior is maintained.

Thoughts?

cheers,
jamal

From: jamal <redacted>
Date: Thu, 26 Apr 2007 09:10:10 -0400

I would have liked to just do a read_lock_bh when retrieving the table
metadata; however, the state table lock is defined as DEFINE_SPINLOCK
unlike the policy table which is defined as DEFINE_RWLOCK.
Any objection to change the state lock to be RW?

I wouldn't mind if it actually helped anything.

The SMP cache line transactions are more expensive than the
execution of the code blocks they are protecting.  rwlock's
rarely help, and when they do (the execution path is more
expensive than the SMP atomic operations) then you're holding
the lock too long :-)

One other angle is start rejecting additions to the table after some
point. To test, I wrote a little DOS tool that just kept adding entries
until an OOM hit. It is a lot of fun to watch when you hit a point that
swap is guzzling 2G or more. The add latency starts going up
exponentially.

I would prefer a dynamic algorithm that reacts to system memory
pressure and yet-another-knob that people will get wrong and
there is no sane default for.

I plan to do away with all the GC threshold madness in the
routing cache, for example, and just let the MM layer call
back into us when there is memory pressure to trigger GC.

See set_shrinker() and friends.  The MM calls into these
handlers in response to memory pressure.  There is no
reason the networking can't hook into this and do things
properly instead of the ad-hoc manner we currently use.

On Thu, 2007-26-04 at 14:18 -0700, David Miller wrote:

I wouldn't mind if it actually helped anything.

The SMP cache line transactions are more expensive than the
execution of the code blocks they are protecting.  rwlock's
rarely help, and when they do (the execution path is more
expensive than the SMP atomic operations) then you're holding
the lock too long :-)

Ok ;->
So if i was to make any change, it would be for consistency
with SPD. If this is sufficiently compelling i will send a patch.

I would prefer a dynamic algorithm that reacts to system memory
pressure and yet-another-knob that people will get wrong and
there is no sane default for.

This would certainly be a better approach if doable.

I plan to do away with all the GC threshold madness in the
routing cache, for example, and just let the MM layer call
back into us when there is memory pressure to trigger GC.

See set_shrinker() and friends.  The MM calls into these
handlers in response to memory pressure.  There is no
reason the networking can't hook into this and do things
properly instead of the ad-hoc manner we currently use.

Scanning the kernel ... 
I wasnt aware of this, neat; not many areas in the kernel seem to use
it. I find this stuff interesting, so i may get too verbose ;->

One approach i tried was to write an oom_handler - but it seemed to
get invoked a little too late, i.e when shit has already hit the fan.
If only i could get notified just before swap kicks in or just when some
preconfigured (by me) memmory threshold is hit.... This may do it? I
will experiment. Actually for it to work well, I will need to know when
the memory threshold is crossed as it goes down and when it is going up
as more memory gets freed.

I can see the shrinker working well with dynamically createable 
entries (route cache, arp cache, contrack etc); shrinking a SAD, SPD,
FIB etc that was created by some user space app without user space
consent or at least notification may be unacceptable (imagine Quagga/BGP
adding FIB entries and the kernel deciding its gonna run out of mem and
starting to delete entries; worse deleting SAD entries may be a security
risk etc etc). My problem is more related to these sorts of user
controlled tables.  
One approach that may work to address the above is to send a signal to
user space when the low mem threshold is approaching.. User space then
uses that info to slow down its abuse of memory. I think that signaling
maybe achievable by a genlmsg being sent to a multicast group which a
user space app will have to subscribe to.
Another approach is to use the shrinker callback to set a lowmem
condition to start rejecting any new table additions. A timer to 
retry would take it back; a callback from the VM to say "you can go
ahead and alloc more now" would be better of course - i couldnt see this
anywhere in the VM code, but it is one of those subsystem i dont pay
attention to, it may be there.

Thoughts? ;->

cheers,
jamal

From: jamal <redacted>
Date: Wed, 25 Apr 2007 11:42:41 -0400

[XFRM] export SAD info

On a system with a lot of SAs, counting SAD entries chews useful
CPU time since you need to dump the whole SAD to user space;
i.e something like ip xfrm state ls | grep -i src | wc -l
I have seen taking literally minutes on a 40K SAs when the system
is swapping.
With this patch, some of the SAD info (that was already being tracked)
is exposed to user space. i.e you do:
ip xfrm state count
And you get the count; you can also pass -s to the command line and
get the hash info.

Signed-off-by: Jamal Hadi Salim <redacted>

Applied, thanks Jamal.

Thanks Dave.
Here's a missing bit to get in sync with latest net-2.6

I will send you the policy side sometime tonight or tommorow.

cheers,
jamal

From: jamal <redacted>
Date: Thu, 26 Apr 2007 08:55:54 -0400

Here's a missing bit to get in sync with latest net-2.6

Applied, thanks.