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

Changeset View

Standalone View

View Options

sys/netpfil/ipfw/ip_fw2.c

Context not available.
* whose version is written in f->cached_id * whose version is written in f->cached_id
* (horrible hacks to avoid changing the ABI). * (horrible hacks to avoid changing the ABI).
*/ */
if (num != IP_FW_TARG && f->cached_id == chain->id) if (num != IP_FW_TARG && atomic_load_32(&f->cached_id) == chain->id)
f_pos = f->cached_pos; f_pos = f->cached_pos;
else { else {
int i = IP_FW_ARG_TABLEARG(chain, num, skipto); int i = IP_FW_ARG_TABLEARG(chain, num, skipto);
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…
Context not available.
f_pos = ipfw_find_rule(chain, i, 0); f_pos = ipfw_find_rule(chain, i, 0);
/* update the cache */ /* update the cache */
if (num != IP_FW_TARG) { if (num != IP_FW_TARG) {
f->cached_id = chain->id; /*
f->cached_pos = f_pos; * We don't care about thread synchronization there.
* Even if multiple threads update the cache simultaneously,
* we still get the correct cache value.
*/
f->cached_pos = f_pos;
atomic_store_32(&f->cached_id, chain->id);
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…
} }
} }
Context not available.