Date: Mon, 29 Apr 2019 04:51:30 +0000 (UTC) From: Navdeep Parhar <np@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r346877 - stable/11/sys/dev/cxgbe Message-ID: <201904290451.x3T4pUrc013151@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: np Date: Mon Apr 29 04:51:30 2019 New Revision: 346877 URL: https://svnweb.freebsd.org/changeset/base/346877 Log: MFC r337830: cxgbe(4): Use two hashes instead of a table to keep track of hashfilters. Two because the driver needs to look up a hashfilter by its 4-tuple or tid. A couple of fixes while here: - Reject attempts to add duplicate hashfilters. - Do not assume that any part of the 4-tuple that isn't specified is 0. This makes it consistent with all other mandatory parameters that already require explicit user input. Modified: stable/11/sys/dev/cxgbe/adapter.h stable/11/sys/dev/cxgbe/offload.h stable/11/sys/dev/cxgbe/t4_filter.c stable/11/sys/dev/cxgbe/t4_main.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/cxgbe/adapter.h ============================================================================== --- stable/11/sys/dev/cxgbe/adapter.h Mon Apr 29 04:42:18 2019 (r346876) +++ stable/11/sys/dev/cxgbe/adapter.h Mon Apr 29 04:51:30 2019 (r346877) @@ -1208,7 +1208,7 @@ int t4_filter_rpl(struct sge_iq *, const struct rss_he int t4_hashfilter_ao_rpl(struct sge_iq *, const struct rss_header *, struct mbuf *); int t4_hashfilter_tcb_rpl(struct sge_iq *, const struct rss_header *, struct mbuf *); int t4_del_hashfilter_rpl(struct sge_iq *, const struct rss_header *, struct mbuf *); -void free_hftid_tab(struct tid_info *); +void free_hftid_hash(struct tid_info *); static inline struct wrqe * alloc_wrqe(int wr_len, struct sge_wrq *wrq) Modified: stable/11/sys/dev/cxgbe/offload.h ============================================================================== --- stable/11/sys/dev/cxgbe/offload.h Mon Apr 29 04:42:18 2019 (r346876) +++ stable/11/sys/dev/cxgbe/offload.h Mon Apr 29 04:51:30 2019 (r346877) @@ -127,11 +127,13 @@ struct tid_info { */ struct mtx hftid_lock __aligned(CACHE_LINE_SIZE); struct cv hftid_cv; - union { - void **hftid_tab; - void **tid_tab; - }; + void **tid_tab; u_int tids_in_use; + + void *hftid_hash_4t; /* LIST_HEAD(, filter_entry) *hftid_hash_4t; */ + u_long hftid_4t_mask; + void *hftid_hash_tid; /* LIST_HEAD(, filter_entry) *hftid_hash_tid; */ + u_long hftid_tid_mask; }; struct t4_range { Modified: stable/11/sys/dev/cxgbe/t4_filter.c ============================================================================== --- stable/11/sys/dev/cxgbe/t4_filter.c Mon Apr 29 04:42:18 2019 (r346876) +++ stable/11/sys/dev/cxgbe/t4_filter.c Mon Apr 29 04:51:30 2019 (r346877) @@ -33,6 +33,7 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> #include <sys/eventhandler.h> +#include <sys/fnv_hash.h> #include <sys/systm.h> #include <sys/kernel.h> #include <sys/module.h> @@ -53,6 +54,9 @@ __FBSDID("$FreeBSD$"); #include "t4_smt.h" struct filter_entry { + LIST_ENTRY(filter_entry) link_4t; + LIST_ENTRY(filter_entry) link_tid; + uint32_t valid:1; /* filter allocated and valid */ uint32_t locked:1; /* filter is administratively locked or busy */ uint32_t pending:1; /* filter action is pending firmware reply */ @@ -78,17 +82,54 @@ separate_hpfilter_region(struct adapter *sc) return (chip_id(sc) >= CHELSIO_T6); } +static inline uint32_t +hf_hashfn_4t(struct t4_filter_specification *fs) +{ + struct t4_filter_tuple *ft = &fs->val; + uint32_t hash; + + if (fs->type) { + /* IPv6 */ + hash = fnv_32_buf(&ft->sip[0], 16, FNV1_32_INIT); + hash = fnv_32_buf(&ft->dip[0], 16, hash); + } else { + hash = fnv_32_buf(&ft->sip[0], 4, FNV1_32_INIT); + hash = fnv_32_buf(&ft->dip[0], 4, hash); + } + hash = fnv_32_buf(&ft->sport, sizeof(ft->sport), hash); + hash = fnv_32_buf(&ft->dport, sizeof(ft->dport), hash); + + return (hash); +} + +static inline uint32_t +hf_hashfn_tid(int tid) +{ + + return (fnv_32_buf(&tid, sizeof(tid), FNV1_32_INIT)); +} + static int -alloc_hftid_tab(struct tid_info *t, int flags) +alloc_hftid_hash(struct tid_info *t, int flags) { + int n; MPASS(t->ntids > 0); - MPASS(t->hftid_tab == NULL); + MPASS(t->hftid_hash_4t == NULL); + MPASS(t->hftid_hash_tid == NULL); - t->hftid_tab = malloc(sizeof(*t->hftid_tab) * t->ntids, M_CXGBE, - M_ZERO | flags); - if (t->hftid_tab == NULL) + n = max(t->ntids / 1024, 16); + t->hftid_hash_4t = hashinit_flags(n, M_CXGBE, &t->hftid_4t_mask, flags); + if (t->hftid_hash_4t == NULL) return (ENOMEM); + t->hftid_hash_tid = hashinit_flags(n, M_CXGBE, &t->hftid_tid_mask, + flags); + if (t->hftid_hash_tid == NULL) { + hashdestroy(t->hftid_hash_4t, M_CXGBE, t->hftid_4t_mask); + t->hftid_hash_4t = NULL; + return (ENOMEM); + } + mtx_init(&t->hftid_lock, "T4 hashfilters", 0, MTX_DEF); cv_init(&t->hftid_cv, "t4hfcv"); @@ -96,22 +137,47 @@ alloc_hftid_tab(struct tid_info *t, int flags) } void -free_hftid_tab(struct tid_info *t) +free_hftid_hash(struct tid_info *t) { + struct filter_entry *f, *ftmp; + LIST_HEAD(, filter_entry) *head; int i; +#ifdef INVARIANTS + int n = 0; +#endif - if (t->hftid_tab != NULL) { - MPASS(t->ntids > 0); - for (i = 0; t->tids_in_use > 0 && i < t->ntids; i++) { - if (t->hftid_tab[i] == NULL) - continue; - free(t->hftid_tab[i], M_CXGBE); - t->tids_in_use--; + if (t->tids_in_use > 0) { + /* Remove everything from the tid hash. */ + head = t->hftid_hash_tid; + for (i = 0; i <= t->hftid_tid_mask; i++) { + LIST_FOREACH_SAFE(f, &head[i], link_tid, ftmp) { + LIST_REMOVE(f, link_tid); + } } - free(t->hftid_tab, M_CXGBE); - t->hftid_tab = NULL; + + /* Remove and then free each filter in the 4t hash. */ + head = t->hftid_hash_4t; + for (i = 0; i <= t->hftid_4t_mask; i++) { + LIST_FOREACH_SAFE(f, &head[i], link_4t, ftmp) { +#ifdef INVARIANTS + n += f->fs.type ? 2 : 1; +#endif + LIST_REMOVE(f, link_4t); + free(f, M_CXGBE); + } + } + MPASS(t->tids_in_use == n); + t->tids_in_use = 0; } + if (t->hftid_hash_4t) { + hashdestroy(t->hftid_hash_4t, M_CXGBE, t->hftid_4t_mask); + t->hftid_hash_4t = NULL; + } + if (t->hftid_hash_tid) { + hashdestroy(t->hftid_hash_tid, M_CXGBE, t->hftid_tid_mask); + t->hftid_hash_tid = NULL; + } if (mtx_initialized(&t->hftid_lock)) { mtx_destroy(&t->hftid_lock); cv_destroy(&t->hftid_cv); @@ -119,31 +185,146 @@ free_hftid_tab(struct tid_info *t) } static void -insert_hftid(struct adapter *sc, int tid, void *ctx, int ntids) +insert_hf(struct adapter *sc, struct filter_entry *f, uint32_t hash) { struct tid_info *t = &sc->tids; + LIST_HEAD(, filter_entry) *head = t->hftid_hash_4t; - t->hftid_tab[tid] = ctx; - atomic_add_int(&t->tids_in_use, ntids); + MPASS(head != NULL); + if (hash == 0) + hash = hf_hashfn_4t(&f->fs); + LIST_INSERT_HEAD(&head[hash & t->hftid_4t_mask], f, link_4t); + atomic_add_int(&t->tids_in_use, f->fs.type ? 2 : 1); } -static void * +static void +insert_hftid(struct adapter *sc, struct filter_entry *f) +{ + struct tid_info *t = &sc->tids; + LIST_HEAD(, filter_entry) *head = t->hftid_hash_tid; + uint32_t hash; + + MPASS(f->tid >= t->tid_base); + MPASS(f->tid - t->tid_base < t->ntids); + mtx_assert(&t->hftid_lock, MA_OWNED); + + hash = hf_hashfn_tid(f->tid); + LIST_INSERT_HEAD(&head[hash & t->hftid_tid_mask], f, link_tid); +} + +static bool +filter_eq(struct t4_filter_specification *fs1, + struct t4_filter_specification *fs2) +{ + int n; + + MPASS(fs1->hash && fs2->hash); + + if (fs1->type != fs2->type) + return (false); + + n = fs1->type ? 16 : 4; + if (bcmp(&fs1->val.sip[0], &fs2->val.sip[0], n) || + bcmp(&fs1->val.dip[0], &fs2->val.dip[0], n) || + fs1->val.sport != fs2->val.sport || + fs1->val.dport != fs2->val.dport) + return (false); + + /* + * We know the masks are the same because all hashfilter masks have to + * conform to the global tp->hash_filter_mask and the driver has + * verified that already. + */ + + if ((fs1->mask.pfvf_vld || fs1->mask.ovlan_vld) && + fs1->val.vnic != fs2->val.vnic) + return (false); + if (fs1->mask.vlan_vld && fs1->val.vlan != fs2->val.vlan) + return (false); + if (fs1->mask.macidx && fs1->val.macidx != fs2->val.macidx) + return (false); + if (fs1->mask.frag && fs1->val.frag != fs2->val.frag) + return (false); + if (fs1->mask.matchtype && fs1->val.matchtype != fs2->val.matchtype) + return (false); + if (fs1->mask.iport && fs1->val.iport != fs2->val.iport) + return (false); + if (fs1->mask.fcoe && fs1->val.fcoe != fs2->val.fcoe) + return (false); + if (fs1->mask.proto && fs1->val.proto != fs2->val.proto) + return (false); + if (fs1->mask.tos && fs1->val.tos != fs2->val.tos) + return (false); + if (fs1->mask.ethtype && fs1->val.ethtype != fs2->val.ethtype) + return (false); + + return (true); +} + +static struct filter_entry * +lookup_hf(struct adapter *sc, struct t4_filter_specification *fs, uint32_t hash) +{ + struct tid_info *t = &sc->tids; + LIST_HEAD(, filter_entry) *head = t->hftid_hash_4t; + struct filter_entry *f; + + mtx_assert(&t->hftid_lock, MA_OWNED); + MPASS(head != NULL); + + if (hash == 0) + hash = hf_hashfn_4t(fs); + + LIST_FOREACH(f, &head[hash & t->hftid_4t_mask], link_4t) { + if (filter_eq(&f->fs, fs)) + return (f); + } + + return (NULL); +} + +static struct filter_entry * lookup_hftid(struct adapter *sc, int tid) { struct tid_info *t = &sc->tids; + LIST_HEAD(, filter_entry) *head = t->hftid_hash_tid; + struct filter_entry *f; + uint32_t hash; - return (t->hftid_tab[tid]); + mtx_assert(&t->hftid_lock, MA_OWNED); + MPASS(head != NULL); + + hash = hf_hashfn_tid(tid); + LIST_FOREACH(f, &head[hash & t->hftid_tid_mask], link_tid) { + if (f->tid == tid) + return (f); + } + + return (NULL); } static void -remove_hftid(struct adapter *sc, int tid, int ntids) +remove_hf(struct adapter *sc, struct filter_entry *f) { struct tid_info *t = &sc->tids; - t->hftid_tab[tid] = NULL; - atomic_subtract_int(&t->tids_in_use, ntids); + mtx_assert(&t->hftid_lock, MA_OWNED); + + LIST_REMOVE(f, link_4t); + atomic_subtract_int(&t->tids_in_use, f->fs.type ? 2 : 1); } +static void +remove_hftid(struct adapter *sc, struct filter_entry *f) +{ +#ifdef INVARIANTS + struct tid_info *t = &sc->tids; + + mtx_assert(&t->hftid_lock, MA_OWNED); +#endif + + LIST_REMOVE(f, link_tid); +} + static uint32_t mode_to_fconf(uint32_t mode) { @@ -613,6 +794,25 @@ hashfilter_ntuple(struct adapter *sc, const struct t4_ return (0); } +static bool +is_4tuple_specified(struct t4_filter_specification *fs) +{ + int i; + const int n = fs->type ? 16 : 4; + + if (fs->mask.sport != 0xffff || fs->mask.dport != 0xffff) + return (false); + + for (i = 0; i < n; i++) { + if (fs->mask.sip[i] != 0xff) + return (false); + if (fs->mask.dip[i] != 0xff) + return (false); + } + + return (true); +} + int set_filter(struct adapter *sc, struct t4_filter *t) { @@ -635,6 +835,8 @@ set_filter(struct adapter *sc, struct t4_filter *t) /* T5 can't count hashfilter hits. */ if (is_t5(sc) && t->fs.hitcnts) return (EINVAL); + if (!is_4tuple_specified(&t->fs)) + return (EINVAL); rc = hashfilter_ntuple(sc, &t->fs, &ftuple); if (rc != 0) return (rc); @@ -689,8 +891,8 @@ set_filter(struct adapter *sc, struct t4_filter *t) return (rc); } if (t->fs.hash) { - if (__predict_false(ti->hftid_tab == NULL)) { - rc = alloc_hftid_tab(&sc->tids, M_NOWAIT); + if (__predict_false(ti->hftid_hash_4t == NULL)) { + rc = alloc_hftid_hash(&sc->tids, HASH_NOWAIT); if (rc != 0) goto done; } @@ -864,7 +1066,7 @@ del_filter(struct adapter *sc, struct t4_filter *t) * for are initialized. */ if (t->fs.hash) { - if (sc->tids.hftid_tab != NULL) + if (sc->tids.hftid_hash_4t != NULL) return (del_hashfilter(sc, t)); } else if (separate_hpfilter_region(sc) && t->fs.prio) { if (sc->tids.hpftid_tab != NULL) @@ -1018,18 +1220,9 @@ t4_hashfilter_ao_rpl(struct sge_iq *iq, const struct r KASSERT(f->tid == -1, ("%s: hashfilter[%p] has tid %d already.", __func__, f, f->tid)); if (status == CPL_ERR_NONE) { - struct filter_entry *f2; - f->tid = GET_TID(cpl); - MPASS(f->tid < sc->tids.ntids); - if (__predict_false((f2 = lookup_hftid(sc, f->tid)) != NULL)) { - /* XXX: avoid hash collisions in the first place. */ - MPASS(f2->tid == f->tid); - remove_hftid(sc, f2->tid, f2->fs.type ? 2 : 1); - free_filter_resources(f2); - free(f2, M_CXGBE); - } - insert_hftid(sc, f->tid, f, f->fs.type ? 2 : 1); + MPASS(lookup_hftid(sc, f->tid) == NULL); + insert_hftid(sc, f); /* * Leave the filter pending until it is fully set up, which will * be indicated by the reply to the last TCB update. No need to @@ -1046,6 +1239,7 @@ t4_hashfilter_ao_rpl(struct sge_iq *iq, const struct r if (act_open_has_tid(status)) release_tid(sc, GET_TID(cpl), &sc->sge.ctrlq[0]); free_filter_resources(f); + remove_hf(sc, f); if (f->locked == 0) free(f, M_CXGBE); } @@ -1080,7 +1274,8 @@ t4_hashfilter_tcb_rpl(struct sge_iq *iq, const struct f->tid = EIO; f->valid = 0; free_filter_resources(f); - remove_hftid(sc, tid, f->fs.type ? 2 : 1); + remove_hftid(sc, f); + remove_hf(sc, f); release_tid(sc, tid, &sc->sge.ctrlq[0]); if (f->locked == 0) free(f, M_CXGBE); @@ -1111,7 +1306,8 @@ t4_del_hashfilter_rpl(struct sge_iq *iq, const struct if (cpl->status == 0) { f->valid = 0; free_filter_resources(f); - remove_hftid(sc, tid, f->fs.type ? 2 : 1); + remove_hftid(sc, f); + remove_hf(sc, f); release_tid(sc, tid, &sc->sge.ctrlq[0]); if (f->locked == 0) free(f, M_CXGBE); @@ -1181,26 +1377,30 @@ done: static int get_hashfilter(struct adapter *sc, struct t4_filter *t) { - int i, nfilters = sc->tids.ntids; + struct tid_info *ti = &sc->tids; + int tid; struct filter_entry *f; + const int inv_tid = ti->ntids + ti->tid_base; MPASS(t->fs.hash); - if (sc->tids.tids_in_use == 0 || sc->tids.hftid_tab == NULL || - t->idx >= nfilters) { + if (ti->tids_in_use == 0 || ti->hftid_hash_tid == NULL || + t->idx >= inv_tid) { t->idx = 0xffffffff; return (0); } + if (t->idx < ti->tid_base) + t->idx = ti->tid_base; - mtx_lock(&sc->tids.hftid_lock); - for (i = t->idx; i < nfilters; i++) { - f = lookup_hftid(sc, i); + mtx_lock(&ti->hftid_lock); + for (tid = t->idx; tid < inv_tid; tid++) { + f = lookup_hftid(sc, tid); if (f != NULL && f->valid) { - t->idx = i; + t->idx = tid; t->l2tidx = f->l2te ? f->l2te->idx : 0; t->smtidx = f->smt ? f->smt->idx : 0; if (f->fs.hitcnts) - t->hits = get_filter_hits(sc, t->idx); + t->hits = get_filter_hits(sc, tid); else t->hits = UINT64_MAX; t->fs = f->fs; @@ -1210,7 +1410,7 @@ get_hashfilter(struct adapter *sc, struct t4_filter *t } t->idx = 0xffffffff; done: - mtx_unlock(&sc->tids.hftid_lock); + mtx_unlock(&ti->hftid_lock); return (0); } @@ -1335,20 +1535,21 @@ set_hashfilter(struct adapter *sc, struct t4_filter *t struct wrq_cookie cookie; struct filter_entry *f; int rc, atid = -1; + uint32_t hash; MPASS(t->fs.hash); /* Already validated against fconf, iconf */ MPASS((t->fs.val.pfvf_vld & t->fs.val.ovlan_vld) == 0); MPASS((t->fs.mask.pfvf_vld & t->fs.mask.ovlan_vld) == 0); + hash = hf_hashfn_4t(&t->fs); + mtx_lock(&sc->tids.hftid_lock); + if (lookup_hf(sc, &t->fs, hash) != NULL) { + rc = EEXIST; + goto done; + } - /* - * XXX: Check for hash collisions and insert in the hash based lookup - * table so that in-flight hashfilters are also considered when checking - * for collisions. - */ - f = malloc(sizeof(*f), M_CXGBE, M_ZERO | M_NOWAIT); if (__predict_false(f == NULL)) { if (l2te) @@ -1394,6 +1595,7 @@ set_hashfilter(struct adapter *sc, struct t4_filter *t f->locked = 1; /* ithread mustn't free f if ioctl is still around. */ f->pending = 1; f->tid = -1; + insert_hf(sc, f, hash); commit_wrq_wr(&sc->sge.ctrlq[0], wr, &cookie); for (;;) { @@ -1404,6 +1606,7 @@ set_hashfilter(struct adapter *sc, struct t4_filter *t f->locked = 0; t->idx = f->tid; } else { + remove_hf(sc, f); rc = f->tid; free(f, M_CXGBE); } @@ -1544,19 +1747,21 @@ mk_del_hashfilter_wr(int tid, struct work_request_hdr static int del_hashfilter(struct adapter *sc, struct t4_filter *t) { + struct tid_info *ti = &sc->tids; void *wr; struct filter_entry *f; struct wrq_cookie cookie; int rc; const int wrlen = del_hashfilter_wrlen(); + const int inv_tid = ti->ntids + ti->tid_base; - MPASS(sc->tids.hftid_tab != NULL); + MPASS(sc->tids.hftid_hash_4t != NULL); MPASS(sc->tids.ntids > 0); - if (t->idx >= sc->tids.ntids) + if (t->idx < sc->tids.tid_base || t->idx >= inv_tid) return (EINVAL); - mtx_lock(&sc->tids.hftid_lock); + mtx_lock(&ti->hftid_lock); f = lookup_hftid(sc, t->idx); if (f == NULL || f->valid == 0) { rc = EINVAL; @@ -1595,14 +1800,14 @@ del_hashfilter(struct adapter *sc, struct t4_filter *t } break; } - if (cv_wait_sig(&sc->tids.hftid_cv, &sc->tids.hftid_lock) != 0) { + if (cv_wait_sig(&ti->hftid_cv, &ti->hftid_lock) != 0) { f->locked = 0; rc = EINPROGRESS; break; } } done: - mtx_unlock(&sc->tids.hftid_lock); + mtx_unlock(&ti->hftid_lock); return (rc); } Modified: stable/11/sys/dev/cxgbe/t4_main.c ============================================================================== --- stable/11/sys/dev/cxgbe/t4_main.c Mon Apr 29 04:42:18 2019 (r346876) +++ stable/11/sys/dev/cxgbe/t4_main.c Mon Apr 29 04:51:30 2019 (r346877) @@ -1511,8 +1511,7 @@ t4_detach_common(device_t dev) free(sc->sge.eqmap, M_CXGBE); free(sc->tids.ftid_tab, M_CXGBE); free(sc->tids.hpftid_tab, M_CXGBE); - if (sc->tids.hftid_tab) - free_hftid_tab(&sc->tids); + free_hftid_hash(&sc->tids); free(sc->tids.atid_tab, M_CXGBE); free(sc->tids.tid_tab, M_CXGBE); free(sc->tt.tls_rx_ports, M_CXGBE);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201904290451.x3T4pUrc013151>