Page MenuHomeFreeBSD

libalias: tidy up housekeeping
ClosedPublic

Authored by donner on May 15 2021, 3:40 PM.

Details

Summary
  • Replace current expensive, but sparse housekeeping by a lightweight, repetitive action.
  • Check for expiry before each use of a flow (housekeeping on demand)
Test Plan

Not obviously broken (housekeeping is not tested in D30408)

1_instance:1_singleinit  ->  passed  [0.004s]
1_instance:2_destroynull  ->  expected_failure: Code expects valid pointer.  [0.015s]
1_instance:3_multiinit  ->  passed  [0.005s]
1_instance:4_multiinstance  ->  passed  [0.041s]
2_natout:1_simplemasq  ->  passed  [0.003s]
2_natout:2_unregistered  ->  passed  [0.003s]
2_natout:3_cgn  ->  passed  [0.003s]
2_natout:4_udp  ->  passed  [0.003s]
2_natout:5_sameport  ->  passed  [0.003s]
2_natout:6_cleartable  ->  passed  [0.003s]
2_natout:7_stress  ->  passed  [0.045s]
3_natin:1_portforward  ->  passed  [0.003s]
3_natin:2_portoverlap  ->  passed  [0.003s]
3_natin:3_redirectany  ->  passed  [0.003s]
3_natin:4_redirectaddr  ->  passed  [0.003s]
3_natin:5_lsnat  ->  passed  [0.003s]
3_natin:6_oneshot  ->  passed  [0.003s]

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

donner added a subscriber: lev.

Added @lev for his offer to do performance tests.

Performance improvement is about 1% if running on full speed.

Old algorithms runs once per second for a substantiational amount of time. New algorithms runs once per packet inspecting a single flow. So full speed operation does 1630254 inspections (number of packets), while the old variant does about 22143 (number of flows). There is room for improvement.

The major part of processing time in libalias is gettimeofday(3).

By reducing the amount of calls:

diff --git a/sys/netinet/libalias/alias_db.c b/sys/netinet/libalias/alias_db.c
index f607fbc429db..24c0de357ef4 100644
--- a/sys/netinet/libalias/alias_db.c
+++ b/sys/netinet/libalias/alias_db.c
@@ -2109,6 +2109,7 @@ HouseKeeping(struct libalias *la)
 {
 #ifndef _KERNEL
        struct timeval tv;
+       static int cnt = 0;
 #endif

        LIBALIAS_LOCK_ASSERT(la);
@@ -2120,8 +2121,11 @@ HouseKeeping(struct libalias *la)
 #ifdef _KERNEL
        la->timeStamp = time_uptime;
 #else
-       gettimeofday(&tv, NULL);
-       la->timeStamp = tv.tv_sec;
+       if (cnt++ > 1000) {
+               gettimeofday(&tv, NULL);
+               la->timeStamp = tv.tv_sec;
+               cnt = 0;
+       }
 #endif
        IncrementalCleanup(la);
 }

A huge boost in performance occur:

Running perfomance test with parameters:
  Maximum Runtime (max_seconds) = 10
  Amount of valid connections (batch_size) = 2000
  Amount of random, incoming packets (batch_size) = 1000
  Repeat count of a random, incoming packet (attack_size) = 1000
  Amount of open port forwardings (redir_size) = 2000

RND SECOND newNAT RANDOM ATTACK useNAT
  1    0.0   0.54   0.16   0.06   0.10
  2    0.0   0.66   0.16   0.06   0.10
  3    0.0   0.76   0.20   0.06   0.10
...
138    9.6  53.94  16.88   1.26   0.10
139    9.7  62.47  18.58   1.19   0.11
140    9.9  58.73  15.83   1.25

Results
   Rounds  :       139
newNAT ok  :    280000
newNAT fail:         0
useNAT ok  :   2308440 (out)
useNAT fail:         0 (out)
useNAT ok  :  18673558 (in)
useNAT fail:         0 (in)
RANDOM ok  :      1845
RANDOM fail:    138155
ATTACK ok  :         0
ATTACK fail:    140000
             ---------
      Total:  21541998

That's a factor of 20!

  • Housekeeping is cheap, so check expire before each use.
  • Replace per instance global variable timeStamp by a real global one.
donner edited the test plan for this revision. (Show Details)

Any objections to commit this?

  • Separate the gettimeofday syscall handling into an extra review.

This should ease reviewing.

donner edited the test plan for this revision. (Show Details)
donner edited the test plan for this revision. (Show Details)

Rebase

  • Remove another, now unused entry in struct libalias.

Change looks good, but someone else must approve it for -net.

This revision is now accepted and ready to land.Wed, Jun 9, 12:55 PM

The major part of processing time in libalias is gettimeofday(3).

I suggest you make a separate thread which sleeps 1 second and updates a local variable with the timestamp you want to use. That way, if the number of packets is low, you don't get any side effects.

The major part of processing time in libalias is gettimeofday(3).

I suggest you make a separate thread which sleeps 1 second and updates a local variable with the timestamp you want to use. That way, if the number of packets is low, you don't get any side effects.

Can for example use a static constructor function to create this thread, to make integration seamless.

The major part of processing time in libalias is gettimeofday(3).

I suggest you make a separate thread which sleeps 1 second and updates a local variable with the timestamp you want to use. That way, if the number of packets is low, you don't get any side effects.

It's a library to be used by arbitrary third party programs, and the kernel itself. I don't even think about dealing with processes or threads created by the lib itself.

The major part of processing time in libalias is gettimeofday(3).

I suggest you make a separate thread which sleeps 1 second and updates a local variable with the timestamp you want to use. That way, if the number of packets is low, you don't get any side effects.

Can for example use a static constructor function to create this thread, to make integration seamless.

My solution for this problem is D30566

This revision was automatically updated to reflect the committed changes.