Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/nat64/mod/stateful/bib/db.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ int bib_find(struct bib *db, struct tuple *tuple,
struct bib_session *result);
int bib_add_session(struct bib *db, struct session_entry *new,
struct collision_cb *cb);
void bib_clean(struct bib *db, struct net *ns);
void bib_clean(struct bib *db, struct net *ns, u64 *max_session_rm,
bool *pending_rm);

/* These are used by userspace request handling. */

Expand Down
3 changes: 2 additions & 1 deletion include/nat64/mod/stateful/bib/pkt_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ void pktqueue_put_node(struct pktqueue_session *node);
* outside.
*/
unsigned int pktqueue_prepare_clean(struct pktqueue *queue,
struct list_head *probes);
struct list_head *probes, u64 *max_session_rm, u64 *sessions_rm,
bool *pending_rm);
/**
* Sends the ICMP errors contained in the @probe list.
*/
Expand Down
13 changes: 13 additions & 0 deletions include/nat64/mod/stateful/global_timer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef _JOOL_MOD_GLOBAL_TIMER_H
#define _JOOL_MOD_GLOBAL_TIMER_H

/**
* @file
* Timer used to trigger some of Jool's events. Always runs, as long as Jool
* is modprobed. At time of writing, this induces fragment expiration.
*/

int global_timer_init(void);
void global_timer_destroy(void);

#endif /* _JOOL_MOD_GLOBAL_TIMER_H */
13 changes: 13 additions & 0 deletions include/nat64/mod/stateful/session_timer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef _JOOL_MOD_SESSION_TIMER_H
#define _JOOL_MOD_SESSION_TIMER_H

/**
* @file
* Timer used to trigger some of Jool's events. Always runs, as long as Jool
* is modprobed. At time of writing, this induces session expiration.
*/

int session_timer_init(void);
void session_timer_destroy(void);

#endif /* _JOOL_MOD_SESSION_TIMER_H */
17 changes: 0 additions & 17 deletions include/nat64/mod/stateful/timer.h

This file was deleted.

3 changes: 2 additions & 1 deletion mod/stateful/Kbuild
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ jool += bib/db.o
jool += bib/entry.o
jool += bib/pkt_queue.o

jool += timer.o
jool += global_timer.o
jool += session_timer.o
jool += fragment_db.o
jool += determine_incoming_tuple.o
jool += filtering_and_updating.o
Expand Down
56 changes: 43 additions & 13 deletions mod/stateful/bib/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -1972,38 +1972,63 @@ int bib_add_session(struct bib *db,

static void __clean(struct expire_timer *expirer,
struct bib_table *table,
struct list_head *probes)
struct list_head *probes,
u64 *max_session_rm,
u64 *sessions_rm,
bool *pending_rm)
{
struct tabled_session *session;
struct tabled_session *tmp;
struct collision_cb cb;
u64 session_cnt;

cb.cb = expirer->decide_fate_cb;
cb.arg = NULL;

log_debug("++ TOTAL Session count = %llu", table->session_count);

list_for_each_entry_safe(session, tmp, &expirer->sessions, list_hook) {
/*
* "list" is sorted by expiration date,
* so stop on the first unexpired session.
*/
if (time_before(jiffies, session->update_time + expirer->timeout))
break;

if (*pending_rm || *sessions_rm >= *max_session_rm) {
*pending_rm = 1;
log_debug("++ Session pending to rem");
break;
}
session_cnt = table->session_count;
decide_fate(&cb, table, session, probes);
}
*sessions_rm += session_cnt - table->session_count;
};
}

static void clean_table(struct bib_table *table, struct net *ns)
static void clean_table(struct bib_table *table,
struct net *ns,
u64 *max_session_rm,
u64 *sessions_rm,
bool *pending_rm)
{
LIST_HEAD(probes);
LIST_HEAD(icmps);

if (*pending_rm)
return;

spin_lock_bh(&table->lock);
__clean(&table->est_timer, table, &probes);
__clean(&table->trans_timer, table, &probes);
__clean(&table->syn4_timer, table, &probes);
__clean(&table->est_timer, table, &probes, max_session_rm, sessions_rm,
pending_rm);
__clean(&table->trans_timer, table, &probes, max_session_rm, sessions_rm,
pending_rm);
__clean(&table->syn4_timer, table, &probes, max_session_rm, sessions_rm,
pending_rm);

if (table->pkt_queue) {
table->pkt_count -= pktqueue_prepare_clean(table->pkt_queue,
&icmps);
table->pkt_count -= pktqueue_prepare_clean(table->pkt_queue, &icmps,
max_session_rm, sessions_rm, pending_rm);
}
spin_unlock_bh(&table->lock);

Expand All @@ -2014,11 +2039,16 @@ static void clean_table(struct bib_table *table, struct net *ns)
/**
* Forgets or downgrades (from EST to TRANS) old sessions.
*/
void bib_clean(struct bib *db, struct net *ns)
{
clean_table(&db->udp, ns);
clean_table(&db->tcp, ns);
clean_table(&db->icmp, ns);
void bib_clean(struct bib *db,
struct net *ns,
u64 *max_session_rm,
bool *pending_rm)
{
u64 sessions_rm = 0;
clean_table(&db->udp, ns, max_session_rm, &sessions_rm, pending_rm);
clean_table(&db->tcp, ns, max_session_rm, &sessions_rm, pending_rm);
clean_table(&db->icmp, ns, max_session_rm, &sessions_rm, pending_rm);
log_debug("+ Session total removed: %llu", sessions_rm);
}

static struct rb_node *find_starting_point(struct bib_table *table,
Expand Down
11 changes: 10 additions & 1 deletion mod/stateful/bib/pkt_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ void pktqueue_put_node(struct pktqueue_session *node)
* @probes.
*/
unsigned int pktqueue_prepare_clean(struct pktqueue *queue,
struct list_head *probes)
struct list_head *probes,
u64 *max_session_rm,
u64 *sessions_rm,
bool *pending_rm)
{
struct pktqueue_session *node, *tmp;
const unsigned long TIMEOUT = get_timeout();
Expand All @@ -211,9 +214,15 @@ unsigned int pktqueue_prepare_clean(struct pktqueue *queue,
if (time_before(jiffies, node->update_time + TIMEOUT))
break;

if (*pending_rm || *sessions_rm >= *max_session_rm) {
*pending_rm = 1;
log_debug("++ Session pending to rem in pktqueue");
break;
}
rm(queue, node);
list_add(&node->list_hook, probes);
removed++;
(*sessions_rm)++;
}

return removed;
Expand Down
7 changes: 3 additions & 4 deletions mod/stateful/timer.c → mod/stateful/global_timer.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "nat64/mod/stateful/timer.h"
#include "nat64/mod/stateful/global_timer.h"

#include "nat64/mod/common/xlator.h"
#include "nat64/mod/stateful/fragment_db.h"
Expand All @@ -12,7 +12,6 @@ static struct timer_list timer;
static int clean_state(struct xlator *jool, void *args)
{
fragdb_clean(jool->nat64.frag);
bib_clean(jool->nat64.bib, jool->ns);
joold_clean(jool->nat64.joold, jool->nat64.bib);
return 0;
}
Expand All @@ -26,7 +25,7 @@ static void timer_function(unsigned long arg)
/**
* This function should be always called *after* other init()s.
*/
int timer_init(void)
int global_timer_init(void)
{
init_timer(&timer);
timer.function = timer_function;
Expand All @@ -39,7 +38,7 @@ int timer_init(void)
/**
* This function should be always called *before* other destroy()s.
*/
void timer_destroy(void)
void global_timer_destroy(void)
{
del_timer_sync(&timer);
}
19 changes: 13 additions & 6 deletions mod/stateful/nf_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
#include "nat64/mod/common/xlator.h"
#include "nat64/mod/common/nl/nl_handler.h"
#include "nat64/mod/stateful/fragment_db.h"
#include "nat64/mod/stateful/global_timer.h"
#include "nat64/mod/stateful/joold.h"
#include "nat64/mod/stateful/timer.h"
#include "nat64/mod/stateful/session_timer.h"
#include "nat64/mod/stateful/pool4/db.h"
#include "nat64/mod/stateful/pool4/rfc6056.h"
#include "nat64/mod/stateful/bib/db.h"
Expand Down Expand Up @@ -135,9 +136,12 @@ static int __init jool_init(void)
error = nlhandler_init();
if (error)
goto nlhandler_fail;
error = timer_init();
error = global_timer_init();
if (error)
goto timer_fail;
goto global_timer_fail;
error = session_timer_init();
if (error)
goto session_timer_fail;
error = logtime_init();
if (error)
goto logtime_fail;
Expand All @@ -161,8 +165,10 @@ static int __init jool_init(void)
instance_fail:
logtime_destroy();
logtime_fail:
timer_destroy();
timer_fail:
session_timer_destroy();
session_timer_fail:
global_timer_destroy();
global_timer_fail:
nlhandler_destroy();
nlhandler_fail:
xlator_destroy();
Expand All @@ -183,7 +189,8 @@ static void __exit jool_exit(void)
nf_unregister_hooks(nfho, ARRAY_SIZE(nfho));

logtime_destroy();
timer_destroy();
session_timer_destroy();
global_timer_destroy();
nlhandler_destroy();
xlator_destroy();
rfc6056_destroy();
Expand Down
71 changes: 71 additions & 0 deletions mod/stateful/session_timer.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#include "nat64/mod/stateful/session_timer.h"

#include "nat64/mod/common/xlator.h"
#include "nat64/mod/stateful/bib/db.h"

#define INITIAL_TIMER_PERIOD msecs_to_jiffies(2000)
#define MAX_SESSIONS_RM 1024

static struct timer_list timer;

struct clean_params {
bool pend_rm;
u64 max_session_rm;
};

static struct clean_params sess_params;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that you might have understandably missed while reading the documentation is that a kernel can have multiple Jool instances, each of them will have a distinct BIB/session database, and each of these databases will have a separate spinlock.

If sess_params is a global variable, then all the instances will share the same "session parameters", and that means that, if one translator faces stress, it will affect the session removal limit of the other translators as well. This is not what we want, because a laggy spinlock only affects its own translator.

I believe that the session parameters should belong to the bib_table structure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the parameters and the timers belong to the bib_table structure.


static int clean_state(struct xlator *jool, void *args)
{
struct clean_params *params = args;
bib_clean(jool->nat64.bib, jool->ns, &params->max_session_rm,
&params->pend_rm);
return 0;
}

static void update_params(unsigned long arg)
{
if (sess_params.pend_rm) {
timer.data = msecs_to_jiffies(jiffies_to_msecs(arg) / 2);
sess_params.max_session_rm <<= 1;
log_debug("+ Session timer NEW msecs and max_sess_rm: %u - %llu",
(jiffies_to_msecs(arg) / 2), sess_params.max_session_rm);
} else {
timer.data = INITIAL_TIMER_PERIOD;
sess_params.max_session_rm = MAX_SESSIONS_RM;
log_debug("+ Session timer RESET msecs and max_sess_rm: %u - %llu",
jiffies_to_msecs(INITIAL_TIMER_PERIOD),
sess_params.max_session_rm);
}
sess_params.pend_rm = 0;
}

static void timer_function(unsigned long arg)
{
xlator_foreach(clean_state, &sess_params);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice: This is the line that causes the timer (which is global) to be run on each translator instance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been removed.

update_params(arg);
mod_timer(&timer, jiffies + timer.data);
}

/**
* This function should be always called *after* other init()s.
*/
int session_timer_init(void)
{
init_timer(&timer);
timer.function = timer_function;
timer.expires = 0;
timer.data = 0;
sess_params.pend_rm = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really a bug by any means, but notice that the kernel API declares boolean identifiers which Jool can access from anywhere, so you can make pend_rm more "boolean looking" by replacing 0 and 1 by more meaningful labels.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, now the boolean labels are used.

sess_params.max_session_rm = MAX_SESSIONS_RM;
mod_timer(&timer, jiffies + INITIAL_TIMER_PERIOD);
return 0;
}

/**
* This function should be always called *before* other destroy()s.
*/
void session_timer_destroy(void)
{
del_timer_sync(&timer);
}