Page MenuHomeFreeBSD
Differential D31484 Diff 93642 sys/netpfil/ipfw/ip_fw2.c

Changeset View

Standalone View

View Options

sys/netpfil/ipfw/ip_fw2.c

Context not available.
/* Use 128 tables by default */ /* Use 128 tables by default */
static unsigned int default_fw_tables = IPFW_TABLES_DEFAULT; static unsigned int default_fw_tables = IPFW_TABLES_DEFAULT;
static int jump_lookup_pos(struct ip_fw_chain *chain, struct ip_fw *f, int num,
int tablearg, int jump_backwards);
#ifndef LINEAR_SKIPTO #ifndef LINEAR_SKIPTO
static int jump_fast(struct ip_fw_chain *chain, struct ip_fw *f, int num, static int jump_cached(struct ip_fw_chain *chain, struct ip_fw *f, int num,
int tablearg, int jump_backwards); int tablearg, int jump_backwards);
#define JUMP(ch, f, num, targ, back) jump_fast(ch, f, num, targ, back) #define JUMP(ch, f, num, targ, back) jump_cached(ch, f, num, targ, back)
#else #else
static int jump_linear(struct ip_fw_chain *chain, struct ip_fw *f, int num, #define JUMP(ch, f, num, targ, back) jump_lookup_pos(ch, f, num, targ, back)
int tablearg, int jump_backwards);
#define JUMP(ch, f, num, targ, back) jump_linear(ch, f, num, targ, back)
#endif #endif
/* /*
Context not available.
args->flags |= IPFW_ARGS_REF; args->flags |= IPFW_ARGS_REF;
} }
#ifndef LINEAR_SKIPTO
/*
* Helper function to enable cached rule lookups using
* cached_id and cached_pos fields in ipfw rule.
*/
static int static int
jump_fast(struct ip_fw_chain *chain, struct ip_fw *f, int num, jump_lookup_pos(struct ip_fw_chain *chain, struct ip_fw *f, int num,
int tablearg, int jump_backwards) int tablearg, int jump_backwards)
{ {
int f_pos; int f_pos, i;
/* If possible use cached f_pos (in f->cached_pos), i = IP_FW_ARG_TABLEARG(chain, num, skipto);
* whose version is written in f->cached_id /* make sure we do not jump backward */
* (horrible hacks to avoid changing the ABI). if (jump_backwards == 0 && i <= f->rulenum)
*/ i = f->rulenum + 1;
if (num != IP_FW_TARG && f->cached_id == chain->id)
f_pos = f->cached_pos; #ifndef LINEAR_SKIPTO
else { if (chain->idxmap != NULL)
int i = IP_FW_ARG_TABLEARG(chain, num, skipto); f_pos = chain->idxmap[i];
/* make sure we do not jump backward */ else
if (jump_backwards == 0 && i <= f->rulenum) f_pos = ipfw_find_rule(chain, i, 0);
i = f->rulenum + 1; #else
if (chain->idxmap != NULL) f_pos = chain->idxmap[i];
f_pos = chain->idxmap[i]; #endif
melifaroUnsubmitted
Done
Inline Actions

I'd restructure the current logic. We only need to update cache if it's not IP_FW_TARG.
With that in mind, current code looks like:

if (cache_correct) f_pos = cache_value
 else {
  f_pos = caculate_value
  update_cache
}
return f_pos

I'd suggest moving all cache fetch/retrieve logic to a separate (inlined) function, so the code looks like the following:

jump_fast() {
  if (num != IP_FW_TARG)
   return (jump_fast_cache(chain, f, num))
 # IP_FW_TARG handler logic
}

jump_fast_cache() {
#if __LP_64__
  if (__predict_true(cache_valid))
   return (value)
 else {
  update-and-return
 }
#else

#endif
}
melifaro: I'd restructure the current logic. We only need to update cache if it's not IP_FW_TARG. With…
else
f_pos = ipfw_find_rule(chain, i, 0);
/* update the cache */
if (num != IP_FW_TARG) {
f->cached_id = chain->id;
f->cached_pos = f_pos;
}
}
return (f_pos); return (f_pos);
} }
#else
#ifndef LINEAR_SKIPTO
/* /*
* Helper function to enable real fast rule lookups. * Helper function to enable cached rule lookups using
* cache.id and cache.pos fields in ipfw rule.
*/ */
static int static int
jump_linear(struct ip_fw_chain *chain, struct ip_fw *f, int num, jump_cached(struct ip_fw_chain *chain, struct ip_fw *f, int num,
int tablearg, int jump_backwards) int tablearg, int jump_backwards)
{ {
struct ip_fw_jump_cache cache;
int f_pos; int f_pos;
melifaroUnsubmitted
Done
Inline Actions

I'd avoid using atomics in the fast path.

For 64-bit architectures, we can simply write a 64-bit value composed of both cached_id and cached_pos.

For 32-bit ones, we can enforce load/store ordering by adding release fence between pos and id, and acquire fence between reading id and pos.

melifaro: I'd avoid using atomics in the fast path. For 64-bit architectures, we can simply write a 64…
num = IP_FW_ARG_TABLEARG(chain, num, skipto); /* Can't use cache with IP_FW_TARG */
/* make sure we do not jump backward */ if (num == IP_FW_TARG)
if (jump_backwards == 0 && num <= f->rulenum) return jump_lookup_pos(chain, f, num, tablearg, jump_backwards);
num = f->rulenum + 1;
f_pos = chain->idxmap[num]; /* If possible use cached f_pos (in f->cache.pos),
* whose version is written in f->cache.id
* (horrible hacks to avoid changing the ABI).
*
* Multiple threads can execute the same rule simultaneously,
* we need to ensure that cache.pos is updated before cache.id.
*/
#ifdef __LP64__
cache.raw_value = f->cache.raw_value;
if (cache.id == chain->id)
return (cache.pos);
f_pos = jump_lookup_pos(chain, f, num, tablearg, jump_backwards);
cache.pos = f_pos;
cache.id = chain->id;
f->cache.raw_value = cache.raw_value;
#else
if (f->cache.id == chain->id) {
/* Load pos after id */
atomic_thread_fence_acq();
return (f->cache.pos);
}
f_pos = jump_lookup_pos(chain, f, num, tablearg, jump_backwards);
f->cache.pos = f_pos;
/* Store id after pos */
atomic_thread_fence_rel();
f->cache.id = chain->id;
#endif
return (f_pos); return (f_pos);
} }
Context not available.