Date: Wed, 20 Jan 2010 13:34:05 +0000 (UTC) From: Luigi Rizzo <luigi@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r202695 - in user/luigi/ipfw3-head: sbin/ipfw sys/netinet sys/netinet/ipfw Message-ID: <201001201334.o0KDY5ts014761@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: luigi Date: Wed Jan 20 13:34:05 2010 New Revision: 202695 URL: http://svn.freebsd.org/changeset/base/202695 Log: remove a nasty bug with the hashtable -- the hash function must return an unsigned, otherwise a % operator might return negative values. On passing, also fix various glitches with configuration commands. Modified: user/luigi/ipfw3-head/sbin/ipfw/dummynet.c user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.c user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.h user/luigi/ipfw3-head/sys/netinet/ipfw/dn_sched_wf2q.c user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c Modified: user/luigi/ipfw3-head/sbin/ipfw/dummynet.c ============================================================================== --- user/luigi/ipfw3-head/sbin/ipfw/dummynet.c Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sbin/ipfw/dummynet.c Wed Jan 20 13:34:05 2010 (r202695) @@ -331,6 +331,15 @@ list_pipes(struct dn_id *oid, struct dn_ flush_buf(buf); printf("unrecognized object %d size %d\n", oid->type, oid->len); break; + case DN_CMD_GET: + printf("respond to cmd %d buflen %d\n", oid->type, oid->id); + break; + case DN_SCH: { + struct new_sch *s = (struct new_sch *)s; + sprintf(buf + strlen(buf), " type %s", s->type); + } + break; + case DN_PIPE: { struct new_pipe *p = (struct new_pipe *)oid; double b = p->bandwidth; @@ -921,22 +930,27 @@ ipfw_config_pipe(int ac, char **av) goto end_mask; case TOK_DSTIP: + mask->addr_type = 4; p32 = &mask->dst_ip; break; case TOK_SRCIP: + mask->addr_type = 4; p32 = &mask->src_ip; break; case TOK_DSTIP6: + mask->addr_type = 6; pa6 = &mask->dst_ip6; break; case TOK_SRCIP6: + mask->addr_type = 6; pa6 = &mask->src_ip6; break; case TOK_FLOWID: + mask->addr_type = 6; p20 = &mask->flow_id6; break; Modified: user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h ============================================================================== --- user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h Wed Jan 20 13:34:05 2010 (r202695) @@ -155,7 +155,7 @@ struct new_fs { */ struct new_inst { struct dn_id oid; - struct ipfw_flow_id id; + struct ipfw_flow_id fid; uint32_t length; /* Queue lenght, in packets */ uint32_t len_bytes; /* Queue lenght, in bytes */ uint32_t drops; Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.c ============================================================================== --- user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.c Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.c Wed Jan 20 13:34:05 2010 (r202695) @@ -123,7 +123,7 @@ heap_init(struct dn_heap *h, int size, i */ #define RESET_OFFSET(h, i) do { \ if (h->ofs > 0) \ - *((int32_t *)((char *)(h->p[i].object) + h->ofs)) = -1; \ + *((int32_t *)((char *)(h->p[i].object) + h->ofs)) = -16; \ } while (0) int @@ -308,7 +308,7 @@ struct dn_ht { int buckets; /* how many buckets */ int entries; /* how many entries */ int ofs; /* offset of link field */ - int (*hash)(uintptr_t, int, void *arg); + uint32_t (*hash)(uintptr_t, int, void *arg); int (*match)(void *_el, uintptr_t key, int, void *); void *(*new)(uintptr_t, int, void *); void **ht; /* bucket heads */ @@ -321,7 +321,7 @@ struct dn_ht { */ struct dn_ht * dn_ht_init(struct dn_ht *ht, int buckets, int ofs, - int (*h)(uintptr_t, int, void *), + uint32_t (*h)(uintptr_t, int, void *), int (*match)(void *, uintptr_t, int, void *), void *(*new)(uintptr_t, int, void *)) { @@ -398,7 +398,8 @@ dn_ht_find(struct dn_ht *ht, uintptr_t k return NULL; i = (ht->buckets == 1) ? 0 : (ht->hash(key, flags, arg) % ht->buckets); - // printf("%s key %p in bucket %d\n", __FUNCTION__, (void *)key, i); + // printf("%s key %p in bucket %d entries %d\n", + // __FUNCTION__, (void *)key, i, ht->entries); for (pp = &ht->ht[i]; (p = *pp); pp = (void **)((char *)p + ht->ofs)) { if (flags & DNHT_MATCH_PTR) { if (key == (uintptr_t)p) Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.h ============================================================================== --- user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.h Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sys/netinet/ipfw/dn_heap.h Wed Jan 20 13:34:05 2010 (r202695) @@ -156,7 +156,7 @@ int heap_scan(struct dn_heap *, int (*)( struct dn_ht; /* should be opaque */ struct dn_ht *dn_ht_init(struct dn_ht *, int buckets, int ofs, - int (*hash)(uintptr_t, int, void *), + uint32_t (*hash)(uintptr_t, int, void *), int (*match)(void *, uintptr_t, int, void *), void *(*new)(uintptr_t, int, void *)); void dn_ht_free(struct dn_ht *, int flags); Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/dn_sched_wf2q.c ============================================================================== --- user/luigi/ipfw3-head/sys/netinet/ipfw/dn_sched_wf2q.c Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sys/netinet/ipfw/dn_sched_wf2q.c Wed Jan 20 13:34:05 2010 (r202695) @@ -232,7 +232,7 @@ static int wf2qp_new_sched(struct new_sch_inst *_si) { struct wf2qp_si *si = (struct wf2qp_si *)(_si + 1); - int ofs = offsetof(struct wf2qp_queue, heap_pos); + int ofs = sizeof(*_si) + offsetof(struct wf2qp_queue, heap_pos); /* only idle-heap supports extract from middle */ if (heap_init(&si->idle_heap, 16, ofs) || Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c ============================================================================== --- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c Wed Jan 20 13:31:12 2010 (r202694) +++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c Wed Jan 20 13:34:05 2010 (r202695) @@ -139,7 +139,7 @@ flow_id_mask(struct ipfw_flow_id *mask, } /* XXX we may want a better hash function */ -static int +static uint32_t flow_id_hash(struct ipfw_flow_id *id) { uint32_t i; @@ -200,12 +200,12 @@ flow_id_cmp(struct ipfw_flow_id *id1, st /*--- support functions for the qht hashtable ---- * Entries are hashed by flow-id */ -static int +static uint32_t q_hash(uintptr_t key, int flags, void *arg) { /* compute the hash slot from the flow id */ struct ipfw_flow_id *id = (flags & DNHT_KEY_IS_OBJ) ? - &((struct new_queue *)key)->ni.id : + &((struct new_queue *)key)->ni.fid : (struct ipfw_flow_id *)key; return flow_id_hash(id); @@ -214,17 +214,16 @@ q_hash(uintptr_t key, int flags, void *a static int q_match(void *obj, uintptr_t key, int flags, void *arg) { - struct new_queue *o; + struct new_queue *o = (struct new_queue *)obj; struct ipfw_flow_id *id2; if (flags & DNHT_KEY_IS_OBJ) { /* compare pointers */ - id2 = &((struct new_queue *)key)->ni.id; + id2 = &((struct new_queue *)key)->ni.fid; } else { id2 = (struct ipfw_flow_id *)key; } - o = (struct new_queue *)obj; - return flow_id_cmp(&o->ni.id, id2) == 0; + return (0 == flow_id_cmp(&o->ni.fid, id2)); } /* @@ -245,15 +244,10 @@ q_new(uintptr_t key, int flags, void *ar set_oid(&q->ni.oid, DN_QUEUE, size); if (fs->fs.flags & DN_HAVE_MASK) - q->ni.id = *(struct ipfw_flow_id *)key; + q->ni.fid = *(struct ipfw_flow_id *)key; q->fs = fs; q->_si = template->_si; - printf("-- %s fs %p si %p mask p %d f %d 0x%08x:%d -> 0x%08x:%d\n", - __FUNCTION__, q->fs, q->_si, - q->ni.id.proto, q->ni.id.flags, - q->ni.id.src_ip, q->ni.id.src_port, - q->ni.id.dst_ip, q->ni.id.dst_port); - + if (fs->sched->fp->new_queue) fs->sched->fp->new_queue(q); dn_cfg.queue_count++; @@ -299,8 +293,10 @@ q_delete_cb(void *q, void *arg) static void qht_delete(struct new_fsk *fs, int flags) { +#if 0 printf("+++ %s fs %d start flags %d qht %p\n", __FUNCTION__, fs->fs.fs_nr, flags, fs->_qht); +#endif if (!fs->_qht) return; if (fs->fs.flags & DN_HAVE_MASK) { @@ -355,12 +351,12 @@ ipdn_q_find(struct new_fsk *fs, struct n * * These are hashed by flow-id */ -static int +static uint32_t si_hash(uintptr_t key, int flags, void *arg) { /* compute the hash slot from the flow id */ struct ipfw_flow_id *id = (flags & DNHT_KEY_IS_OBJ) ? - &((struct new_sch_inst *)key)->ni.id : + &((struct new_sch_inst *)key)->ni.fid : (struct ipfw_flow_id *)key; return flow_id_hash(id); @@ -373,9 +369,9 @@ si_match(void *obj, uintptr_t key, int f struct ipfw_flow_id *id2; id2 = (flags & DNHT_KEY_IS_OBJ) ? - &((struct new_sch_inst *)key)->ni.id : + &((struct new_sch_inst *)key)->ni.fid : (struct ipfw_flow_id *)key; - return flow_id_cmp(&o->ni.id, id2) == 0; + return flow_id_cmp(&o->ni.fid, id2) == 0; } /* @@ -406,7 +402,7 @@ si_new(uintptr_t key, int flags, void *a goto error; } if (s->sch.flags & DN_HAVE_MASK) - si->ni.id = *(struct ipfw_flow_id *)key; + si->ni.fid = *(struct ipfw_flow_id *)key; dn_cfg.si_count++; return si; @@ -489,10 +485,10 @@ schk_reset_credit(struct new_schk *s) * New allocations are put in the fsunlinked list, from which * they are removed when they point to a specific scheduler. */ -static int +static uint32_t fsk_hash(uintptr_t key, int flags, void *arg) { - int i = !(flags & DNHT_KEY_IS_OBJ) ? key : + uint32_t i = !(flags & DNHT_KEY_IS_OBJ) ? key : ((struct new_fsk *)key)->fs.fs_nr; return ( (i>>8)^(i>>4)^i ); @@ -533,11 +529,13 @@ fsk_detach(struct new_fsk *fs, int flags { if (flags & DN_DELETE_FS) flags |= DN_DELETE; +#if 0 printf("%s fs %d from sched %d flags %s %s %s\n", __FUNCTION__, fs->fs.fs_nr, fs->fs.sched_nr, (flags & DN_DELETE_FS) ? "DEL_FS":"", (flags & DN_DELETE) ? "DEL":"", (flags & DN_DETACH) ? "DET":""); +#endif if (flags & DN_DETACH) { /* detach from the list */ struct new_fsk_head *h; h = fs->sched ? &fs->sched->fsk_list : &dn_cfg.fsu; @@ -569,12 +567,13 @@ fsk_detach_list(struct new_fsk_head *h, struct new_fsk *fs; int n = 0; /* only for stats */ - printf("+++ %s head %p flags %x\n", __FUNCTION__, h, flags); + // printf("+++ %s head %p flags %x\n", __FUNCTION__, h, flags); while ((fs = SLIST_FIRST(h))) { SLIST_REMOVE_HEAD(h, sch_chain); + n++; fsk_detach(fs, flags); } - printf("+++ %s done %d flowsets\n", __FUNCTION__, n); + // printf("+++ %s done flowsets\n", __FUNCTION__, n); } /* @@ -610,10 +609,10 @@ delete_fs(int i, int locked) * struct new_schk so we can cast between the two. We use this trick * because in the create phase (but it should be fixed). */ -static int +static uint32_t schk_hash(uintptr_t key, int flags, void *_arg) { - int i = !(flags & DNHT_KEY_IS_OBJ) ? key : + uint32_t i = !(flags & DNHT_KEY_IS_OBJ) ? key : ((struct new_schk *)key)->sch.sched_nr; return ( (i>>8)^(i>>4)^i ); } @@ -670,13 +669,14 @@ static int schk_delete_cb(void *obj, void *arg) { struct new_schk *s = obj; +#if 0 int i = s->sch.sched_nr; int a = (int)arg; - printf(">>> %s sched %d arg %s%s\n", - __FUNCTION__, s->sch.sched_nr, + __FUNCTION__, i, a&DN_DELETE ? "DEL ":"", a&DN_DELETE_FS ? "DEL_FS":""); +#endif fsk_detach_list(&s->fsk_list, arg ? DN_DELETE : 0); /* no more flowset pointing to us now */ if (s->sch.flags & DN_HAVE_MASK) { @@ -688,7 +688,7 @@ schk_delete_cb(void *obj, void *arg) s->fp->destroy(s); bzero(s, sizeof(*s)); free(obj, M_DUMMYNET); - printf("<<< %s done sched %d destroyed\n", __FUNCTION__, i); + // printf("<<< %s done sched %d destroyed\n", __FUNCTION__, i); dn_cfg.schk_count--; return DNHT_SCAN_DEL; } @@ -722,8 +722,8 @@ copy_obj(char **start, char *end, void * struct dn_id *o = _o; int have = end - *start; - if (have < o->len) { - printf("%s overflow type %d have %d need %d\n", + if (have < o->len || o->len == 0 || o->type == 0) { + printf("XXX %s ERROR type %d have %d need %d\n", __FUNCTION__, o->type, have, o->len); return 1; } @@ -764,14 +764,20 @@ copy_data_helper(void *_o, void *_arg) if (a->type == DN_SCH) { /* scanning schedulers */ struct new_schk *s = _o; if (a->flags & DN_C_PIPE) { + printf("copy pipe %d\n", s->sch.sched_nr); if (copy_obj(a->start, a->end, &s->pipe)) return DNHT_SCAN_END; + printf("copy int-fs for pipe %d %p\n", + s->sch.sched_nr, s->fs); if (copy_flowset(a, s->fs, 0)) return DNHT_SCAN_END; + printf("copy pipe %d done\n", s->sch.sched_nr); } if (a->flags & DN_C_SCH) { + printf("copy sched %d\n", s->sch.sched_nr); if (copy_obj(a->start, a->end, &s->sch)) return DNHT_SCAN_END; + printf("copy sched %d done\n", s->sch.sched_nr); } if (a->flags & DN_C_SCH_INST) { printf("XXX todo: scan sched instances\n"); @@ -781,8 +787,11 @@ copy_data_helper(void *_o, void *_arg) struct new_fsk *fs = _o; /* if extra is set, only copy unlinked ones */ if (a->extra == 0 || fs->sched == NULL) { + printf("copy fs %d to sched %d\n", fs->fs.fs_nr, + fs->sched ? fs->sched->sch.sched_nr : -1); if (copy_flowset(a, fs, 0)) return DNHT_SCAN_END; + printf("copy fs %d done\n",fs->fs.fs_nr); } } return 0; @@ -798,8 +807,10 @@ locate_scheduler(int i) static void fsk_attach(struct new_fsk *fs, struct new_schk *s) { +#if 0 printf("remove fs %d from fsunlinked, link to sched %d\n", fs->fs.fs_nr, s->sch.sched_nr); +#endif SLIST_REMOVE(&dn_cfg.fsu, fs, new_fsk, sch_chain); fs->sched = s; SLIST_INSERT_HEAD(&s->fsk_list, fs, sch_chain); @@ -807,7 +818,7 @@ fsk_attach(struct new_fsk *fs, struct ne s->fp->new_fsk(fs); if (!fs->_qht) return; - printf("+++ %s TODO requeue from fs %d to sch %d\n", + printf("XXX %s TODO requeue from fs %d to sch %d\n", __FUNCTION__, fs->fs.fs_nr, s->sch.sched_nr); } @@ -914,7 +925,7 @@ config_fs(struct new_fs *nfs, struct dn_ i = nfs->fs_nr; if (i <= 0 || i >= 3*DN_MAX_ID) return NULL; - printf("%s flowset %d\n", __FUNCTION__, i); + // printf("%s flowset %d\n", __FUNCTION__, i); /* XXX other sanity checks */ if (nfs->flags & DN_QSIZE_IS_BYTES) { if (nfs->qsize > dn_cfg.pipe_byte_limit) @@ -927,10 +938,15 @@ config_fs(struct new_fs *nfs, struct dn_ } if (nfs->flags & DN_HAVE_MASK) { /* make sure we have some buckets */ - if (nfs->buckets < 1) + if (nfs->buckets < 1) { nfs->buckets = dn_cfg.hash_size; - else if (nfs->buckets > dn_cfg.max_hash_size) + printf("%s force buckets to %d\n", + __FUNCTION__, nfs->buckets); + } else if (nfs->buckets > dn_cfg.max_hash_size) { nfs->buckets = dn_cfg.max_hash_size; + printf("%s clamp buckets to %d\n", + __FUNCTION__, nfs->buckets); + } } else { nfs->buckets = 1; /* we only need 1 */ } @@ -953,7 +969,7 @@ config_fs(struct new_fs *nfs, struct dn_ } dn_cfg.id++; if (bcmp(&fs->fs, nfs, sizeof(*nfs)) == 0) { - printf("%s no change\n", __FUNCTION__); + printf("%s flowset %d unchanged\n", __FUNCTION__, i); break; /* no change, nothing to do */ } s = locate_scheduler(nfs->sched_nr); @@ -961,6 +977,9 @@ config_fs(struct new_fs *nfs, struct dn_ * queues if we need to reattach. Then update the * configuration, and possibly attach to the new sched. */ + printf("%s fs %d changed args/sched %d %p to %d %p\n", + __FUNCTION__, fs->fs.fs_nr, + fs->fs.sched_nr, fs->sched, nfs->sched_nr, s); if (fs->sched) { int flags = s ? DN_DETACH : (DN_DETACH | DN_DELETE); flags |= DN_DELETE; /* XXX temporary */ @@ -1000,7 +1019,7 @@ config_sched(struct new_sch *_nsch, stru if (i <= 0 || i >= DN_MAX_ID) return EINVAL; /* XXX other sanity checks */ - bzero(&p, sizeof(0)); + bzero(&p, sizeof(p)); DN_BH_WLOCK(); again: /* run twice, for wfq and fifo */ a.fp = find_sched_type(a.sch->oid.subtype, a.sch->type); @@ -1022,10 +1041,10 @@ again: /* run twice, for wfq and fifo */ } a.fp = s->fp; } + /* normalize type and subtype */ a.sch->oid.subtype = a.fp->type; - printf("%s start i %d ty %s -> %s\n", - __FUNCTION__, i, a.sch->type, - s ? (s->fp?"old":"new"):"none"); + bzero(a.sch->type, sizeof(a.sch->type)); + strlcpy(a.sch->type, a.fp->name, sizeof(a.sch->type)); if (s == NULL) { DN_BH_WUNLOCK(); printf("cannot allocate scheduler\n"); @@ -1044,12 +1063,19 @@ again: /* run twice, for wfq and fifo */ /* already existing. */ printf("sched %d type changed from %s to %s\n", i, s->fp->name, a.fp->name); + printf(" type/sub %d/%d -> %d/%d\n", + s->sch.oid.type, s->sch.oid.subtype, + a.sch->oid.type, a.sch->oid.subtype); + if (s->pipe.pipe_nr == 0) + printf("XXX WARNING pipe 0 for sched %d\n", + s->sch.sched_nr); p = s->pipe; /* preserve pipe */ /* remove from the hash */ dn_ht_find(dn_cfg.schedhash, i, DNHT_REMOVE, NULL); /* Detach flowsets, preserve queues. */ - schk_delete_cb(s, NULL); - printf("schk_delete_cb done\n"); + // schk_delete_cb(s, NULL); + // XXX temporarily, kill queues + schk_delete_cb(s, (void *)DN_DELETE); goto again; } else { printf("%s sched %d unchanged type %s\n", __FUNCTION__, i, @@ -1080,11 +1106,12 @@ again: /* run twice, for wfq and fifo */ if (i < DN_MAX_ID) { /* update the FIFO instance */ i += DN_MAX_ID; a.sch->sched_nr = i; + /* now configure a FIFO instance */ a.sch->oid.subtype = DN_SCHED_FIFO; + bzero(a.sch->type, sizeof(a.sch->type)); goto again; } DN_BH_WUNLOCK(); - printf("%s end\n", __FUNCTION__); return 0; } @@ -1327,7 +1354,7 @@ dummynet_get(struct sockopt *sopt) if (start == NULL) return sooptcopyout(sopt, &cmd, sizeof(cmd)); printf("have %d:%d sched %d, %d:%d pipes %d, %d:%d flows %d, " - "%d.%d si %d, %d.%d queues %d\n", + "%d:%d si %d, %d:%d queues %d\n", dn_cfg.schk_count, sizeof(struct new_sch), DN_SCH, dn_cfg.schk_count, sizeof(struct new_pipe), DN_PIPE, dn_cfg.fsk_count, sizeof(struct new_fs), DN_FS,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201001201334.o0KDY5ts014761>