On 08/06/17(Thu) 23:46, Alexander Bluhm wrote:
> On Tue, Jun 06, 2017 at 03:36:12PM +0200, Martin Pieuchot wrote:
> > +#define SALEN(sa) (*(u_char *)sa)
>
> Put () around macro arguments.
> #define SALEN(sa) (*(u_char *)(sa))
Done.
> > -int
> > +static int
> > rn_refines(void *m_arg, void *n_arg)
>
> > -int
> > +static int
> > rn_inithead0(struct radix_node_head *rnh, int offset)
>
> Could you keep these kernel funtions global for debugging?
Sure.
> > static int last_zeroed = 0;
> > + char addmask_key[KEYLEN_LIMIT];
>
> The static variable last_zeroed does not look MP safe. If I get
> this code correctly it is only an optimization to avoid multiple
> zeroing in addmask_key. This does not work anyway with addmask_key
> on the stack.
You're right, I removed the 'static'.
> > *addmask_key = last_zeroed = mlen;
>
> Should this be SALEN(addmask_key)?
Yes and we can drop 'last_zeroed' since it isn't used afterward.
Index: net/radix.c
===================================================================
RCS file: /cvs/src/sys/net/radix.c,v
retrieving revision 1.56
diff -u -p -r1.56 radix.c
--- net/radix.c 24 Jan 2017 10:08:30 -0000 1.56
+++ net/radix.c 9 Jun 2017 13:08:50 -0000
@@ -48,24 +48,32 @@
#include <net/radix.h>
-static unsigned int max_keylen;
-struct radix_node_head *mask_rnhead;
-static char *addmask_key;
-static char normal_chars[] = {0, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, -1};
-static char *rn_zeros, *rn_ones;
+#define SALEN(sa) (*(u_char *)(sa))
-struct pool rtmask_pool; /* pool for radix_mask structures */
+/*
+ * Read-only variables, allocated & filled during rn_init().
+ */
+static char *rn_zeros; /* array of 0s */
+static char *rn_ones; /* array of 1s */
+static unsigned int max_keylen; /* size of the above arrays */
+#define KEYLEN_LIMIT 64 /* maximum allowed keylen */
-#define rn_masktop (mask_rnhead->rnh_treetop)
+
+struct radix_node_head *mask_rnhead; /* head of shared mask tree */
+struct pool rtmask_pool; /* pool for radix_mask structures */
static inline int rn_satisfies_leaf(char *, struct radix_node *, int);
static inline int rn_lexobetter(void *, void *);
static inline struct radix_mask *rn_new_radix_mask(struct radix_node *,
struct radix_mask *);
+int rn_refines(void *, void *);
+int rn_inithead0(struct radix_node_head *, int);
+struct radix_node *rn_addmask(void *, int, int);
struct radix_node *rn_insert(void *, struct radix_node_head *, int *,
struct radix_node [2]);
struct radix_node *rn_newpair(void *, int, struct radix_node[2]);
+void rn_link_dupedkey(struct radix_node *, struct radix_node *, int);
static inline struct radix_node *rn_search(void *, struct radix_node *);
struct radix_node *rn_search_m(void *, struct radix_node *, void *);
@@ -413,9 +421,10 @@ rn_addmask(void *n_arg, int search, int
caddr_t cp, cplim;
int b = 0, mlen, j;
int maskduplicated, m0, isnormal;
- static int last_zeroed = 0;
+ int last_zeroed = 0;
+ char addmask_key[KEYLEN_LIMIT];
- if ((mlen = *(u_char *)netmask) > max_keylen)
+ if ((mlen = SALEN(netmask)) > max_keylen)
mlen = max_keylen;
if (skip == 0)
skip = 1;
@@ -438,8 +447,8 @@ rn_addmask(void *n_arg, int search, int
}
if (m0 < last_zeroed)
memset(addmask_key + m0, 0, last_zeroed - m0);
- *addmask_key = last_zeroed = mlen;
- tm = rn_search(addmask_key, rn_masktop);
+ SALEN(addmask_key) = mlen;
+ tm = rn_search(addmask_key, mask_rnhead->rnh_treetop);
if (memcmp(addmask_key, tm->rn_key, mlen) != 0)
tm = NULL;
if (tm || search)
@@ -452,7 +461,7 @@ rn_addmask(void *n_arg, int search, int
memcpy(cp, addmask_key, mlen);
tm = rn_insert(cp, mask_rnhead, &maskduplicated, tm);
if (maskduplicated) {
- log(LOG_ERR, "rn_addmask: mask impossibly already in tree\n");
+ log(LOG_ERR, "%s: mask impossibly already in tree\n", __func__);
free(saved_tm, M_RTABLE, 0);
return (tm);
}
@@ -464,6 +473,9 @@ rn_addmask(void *n_arg, int search, int
for (cp = netmask + skip; (cp < cplim) && *(u_char *)cp == 0xff;)
cp++;
if (cp != cplim) {
+ static const char normal_chars[] = {
+ 0, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, -1
+ };
for (j = 0x80; (j & *cp) != 0; j >>= 1)
b++;
if (*cp != normal_chars[b] || cp != (cplim - 1))
@@ -488,9 +500,9 @@ rn_lexobetter(void *m_arg, void *n_arg)
* first. The netmasks were normalized before calling this function and
* don't have unneeded trailing zeros.
*/
- if (*mp > *np)
+ if (SALEN(mp) > SALEN(np))
return 1;
- if (*mp < *np)
+ if (SALEN(mp) < SALEN(np))
return 0;
/*
* Must return the first difference between the masks
@@ -756,6 +768,8 @@ rn_addroute(void *v_arg, void *n_arg, st
struct radix_node *tt, *saved_tt, *tm = NULL;
int keyduplicated;
+ KERNEL_ASSERT_LOCKED();
+
/*
* In dealing with non-contiguous masks, there may be
* many different routes which have the same mask.
@@ -905,7 +919,9 @@ rn_delete(void *v_arg, void *n_arg, stru
int off = top->rn_off;
int vlen;
- vlen = *(u_char *)v;
+ KERNEL_ASSERT_LOCKED();
+
+ vlen = SALEN(v);
/*
* Implement a lookup similar to rn_lookup but we need to save
@@ -1050,6 +1066,8 @@ rn_walktree(struct radix_node_head *h, i
int error;
struct radix_node *base, *next;
struct radix_node *rn = h->rnh_treetop;
+
+ KERNEL_ASSERT_LOCKED();
/*
* This gets complicated because we may delete the node
* while applying the function f to it, so we need to calculate
@@ -1138,13 +1156,15 @@ rn_inithead0(struct radix_node_head *rnh
/*
* rn_init() can be called multiple time with a different key length
- * as long as not radix tree head has been allocated.
+ * as long as no radix tree head has been allocated.
*/
void
rn_init(unsigned int keylen)
{
char *cp, *cplim;
+ KASSERT(keylen <= KEYLEN_LIMIT);
+
if (max_keylen == 0) {
pool_init(&rtmask_pool, sizeof(struct radix_mask), 0,
IPL_SOFTNET, 0, "rtmask", NULL);
@@ -1155,14 +1175,14 @@ rn_init(unsigned int keylen)
KASSERT(mask_rnhead == NULL);
- free(rn_zeros, M_RTABLE, 3 * max_keylen);
- rn_zeros = mallocarray(3, keylen, M_RTABLE, M_NOWAIT | M_ZERO);
+ free(rn_zeros, M_RTABLE, 2 * max_keylen);
+ rn_zeros = mallocarray(2, keylen, M_RTABLE, M_NOWAIT | M_ZERO);
if (rn_zeros == NULL)
panic("cannot initialize a radix tree without memory");
max_keylen = keylen;
- rn_ones = cp = rn_zeros + max_keylen;
- addmask_key = cplim = rn_ones + max_keylen;
+ cp = rn_ones = rn_zeros + max_keylen;
+ cplim = rn_ones + max_keylen;
while (cp < cplim)
*cp++ = -1;
}
Index: net/radix.h
===================================================================
RCS file: /cvs/src/sys/net/radix.h,v
retrieving revision 1.29
diff -u -p -r1.29 radix.h
--- net/radix.h 14 Nov 2016 08:56:12 -0000 1.29
+++ net/radix.h 6 Jun 2017 13:21:56 -0000
@@ -98,14 +98,10 @@ struct radix_node_head {
void rn_init(unsigned int);
int rn_inithead(void **, int);
-int rn_inithead0(struct radix_node_head *, int);
-int rn_refines(void *, void *);
-void rn_link_dupedkey(struct radix_node *, struct radix_node *, int);
int rn_walktree(struct radix_node_head *,
int (*)(struct radix_node *, void *, u_int), void *);
-struct radix_node *rn_addmask(void *, int, int);
struct radix_node *rn_addroute(void *, void *, struct radix_node_head *,
struct radix_node [2], u_int8_t);
struct radix_node *rn_delete(void *, void *, struct radix_node_head *,
