Changeset View
Changeset View
Standalone View
Standalone View
sys/netinet/ip_reass.c
Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines | |||||
#define IPQ_TRYLOCK(i) mtx_trylock(&V_ipq[i].lock) | #define IPQ_TRYLOCK(i) mtx_trylock(&V_ipq[i].lock) | ||||
#define IPQ_UNLOCK(i) mtx_unlock(&V_ipq[i].lock) | #define IPQ_UNLOCK(i) mtx_unlock(&V_ipq[i].lock) | ||||
#define IPQ_LOCK_ASSERT(i) mtx_assert(&V_ipq[i].lock, MA_OWNED) | #define IPQ_LOCK_ASSERT(i) mtx_assert(&V_ipq[i].lock, MA_OWNED) | ||||
VNET_DEFINE_STATIC(int, ipreass_maxbucketsize); | VNET_DEFINE_STATIC(int, ipreass_maxbucketsize); | ||||
#define V_ipreass_maxbucketsize VNET(ipreass_maxbucketsize) | #define V_ipreass_maxbucketsize VNET(ipreass_maxbucketsize) | ||||
void ipreass_init(void); | void ipreass_init(void); | ||||
void ipreass_drain(void); | void ipreass_drain(struct ifnet *); | ||||
void ipreass_slowtimo(void); | void ipreass_slowtimo(void); | ||||
#ifdef VIMAGE | #ifdef VIMAGE | ||||
void ipreass_destroy(void); | void ipreass_destroy(void); | ||||
#endif | #endif | ||||
static int sysctl_maxfragpackets(SYSCTL_HANDLER_ARGS); | static int sysctl_maxfragpackets(SYSCTL_HANDLER_ARGS); | ||||
static int sysctl_maxfragbucketsize(SYSCTL_HANDLER_ARGS); | static int sysctl_maxfragbucketsize(SYSCTL_HANDLER_ARGS); | ||||
static void ipreass_zone_change(void *); | static void ipreass_zone_change(void *); | ||||
static void ipreass_drain_tomax(void); | static void ipreass_drain_tomax(void); | ||||
▲ Show 20 Lines • Show All 487 Lines • ▼ Show 20 Lines | for (int i = 0; i < IPREASS_NHASH; i++) { | ||||
IPQ_UNLOCK(i); | IPQ_UNLOCK(i); | ||||
} | } | ||||
} | } | ||||
/* | /* | ||||
* Drain off all datagram fragments. | * Drain off all datagram fragments. | ||||
*/ | */ | ||||
void | void | ||||
ipreass_drain(void) | ipreass_drain(struct ifnet *ifp) | ||||
{ | { | ||||
for (int i = 0; i < IPREASS_NHASH; i++) { | for (int i = 0; i < IPREASS_NHASH; i++) { | ||||
IPQ_LOCK(i); | IPQ_LOCK(i); | ||||
while(!TAILQ_EMPTY(&V_ipq[i].head)) | while(!TAILQ_EMPTY(&V_ipq[i].head)) | ||||
ipq_drop(&V_ipq[i], TAILQ_FIRST(&V_ipq[i].head)); | ipq_drop(&V_ipq[i], TAILQ_FIRST(&V_ipq[i].head)); | ||||
KASSERT(V_ipq[i].count == 0, | KASSERT(V_ipq[i].count == 0, | ||||
("%s: V_ipq[%d] count %d (V_ipq=%p)", __func__, i, | ("%s: V_ipq[%d] count %d (V_ipq=%p)", __func__, i, | ||||
V_ipq[i].count, V_ipq)); | V_ipq[i].count, V_ipq)); | ||||
IPQ_UNLOCK(i); | IPQ_UNLOCK(i); | ||||
} | } | ||||
} | } | ||||
#ifdef VIMAGE | #ifdef VIMAGE | ||||
/* | /* | ||||
* Destroy IP reassembly structures. | * Destroy IP reassembly structures. | ||||
*/ | */ | ||||
void | void | ||||
bz: You may want to go and have a look how "shutdown" is defined in if_detach_internal(). I think… | |||||
Done Inline ActionsI'll update the code. hselasky: I'll update the code. | |||||
Done Inline ActionsThis block can be changed to: /* * Skip processing if IPv4 reassembly is not initialised * or torn down by ipreass_destroy(). */ if (V_ipq_zone == NULL) return; }}} bz: This block can be changed to:
{{{
/*
* Skip processing if IPv4 reassembly is not… | |||||
ipreass_destroy(void) | ipreass_destroy(void) | ||||
{ | { | ||||
ipreass_drain(); | ipreass_drain(NULL); | ||||
Done Inline ActionsYou can fold these two into one line. bz: You can fold these two into one line. | |||||
uma_zdestroy(V_ipq_zone); | uma_zdestroy(V_ipq_zone); | ||||
Done Inline ActionsPlease add a V_ipq_zone = NULL; here. (If you really want to be fuzzy, you can use a temporary variable but VNET shutdown is single threaded so ...) bz: Please add a V_ipq_zone = NULL; here. (If you really want to be fuzzy, you can use a temporary… | |||||
for (int i = 0; i < IPREASS_NHASH; i++) | for (int i = 0; i < IPREASS_NHASH; i++) | ||||
Done Inline ActionsCan you add a KASSERT that ifp != NULL? bz: Can you add a KASSERT that ifp != NULL? | |||||
mtx_destroy(&V_ipq[i].lock); | mtx_destroy(&V_ipq[i].lock); | ||||
} | } | ||||
Done Inline ActionsI'd remove the blank line. bz: I'd remove the blank line. | |||||
#endif | #endif | ||||
/* | /* | ||||
* After maxnipq has been updated, propagate the change to UMA. The UMA zone | * After maxnipq has been updated, propagate the change to UMA. The UMA zone | ||||
* max has slightly different semantics than the sysctl, for historical | * max has slightly different semantics than the sysctl, for historical | ||||
* reasons. | * reasons. | ||||
*/ | */ | ||||
Done Inline ActionsCan you do the assignment not on the declaration line; also the declaration could happen at the beginning of the function. Also Why is it mb and not just m? bz: Can you do the assignment not on the declaration line; also the declaration could happen at… | |||||
static void | static void | ||||
ipreass_drain_tomax(void) | ipreass_drain_tomax(void) | ||||
{ | { | ||||
struct ipq *fp; | struct ipq *fp; | ||||
int target; | int target; | ||||
/* | /* | ||||
* Make sure each bucket is under the new limit. If | * Make sure each bucket is under the new limit. If | ||||
▲ Show 20 Lines • Show All 70 Lines • ▼ Show 20 Lines | if (max > 0) { | ||||
* and place an extreme upper bound. | * and place an extreme upper bound. | ||||
*/ | */ | ||||
max = uma_zone_set_max(V_ipq_zone, max); | max = uma_zone_set_max(V_ipq_zone, max); | ||||
V_ipreass_maxbucketsize = imax(max / (IPREASS_NHASH / 2), 1); | V_ipreass_maxbucketsize = imax(max / (IPREASS_NHASH / 2), 1); | ||||
ipreass_drain_tomax(); | ipreass_drain_tomax(); | ||||
V_noreass = 0; | V_noreass = 0; | ||||
} else if (max == 0) { | } else if (max == 0) { | ||||
V_noreass = 1; | V_noreass = 1; | ||||
ipreass_drain(); | ipreass_drain(NULL); | ||||
} else if (max == -1) { | } else if (max == -1) { | ||||
V_noreass = 0; | V_noreass = 0; | ||||
uma_zone_set_max(V_ipq_zone, 0); | uma_zone_set_max(V_ipq_zone, 0); | ||||
V_ipreass_maxbucketsize = INT_MAX; | V_ipreass_maxbucketsize = INT_MAX; | ||||
} else | } else | ||||
return (EINVAL); | return (EINVAL); | ||||
return (0); | return (0); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 78 Lines • Show Last 20 Lines |
You may want to go and have a look how "shutdown" is defined in if_detach_internal(). I think using the same logic here and in IPv6 is probably wise.