From 7193c1f1e9e4a5e6fb52d00b965417c613e7b126 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 15:30:24 +0000 Subject: [PATCH 1/6] Fix wrong memory order and switch bpf to atomic_load/store_*. --- sys/net/bpf.c | 23 ++++++++--------------- sys/net/bpfjit.c | 5 ++--- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/sys/net/bpf.c b/sys/net/bpf.c index b1b646f6c5cc..b6c2a10cd752 100644 --- a/sys/net/bpf.c +++ b/sys/net/bpf.c @@ -301,10 +301,13 @@ const struct cdevsw bpf_cdevsw = { bpfjit_func_t bpf_jit_generate(bpf_ctx_t *bc, void *code, size_t size) { + struct bpfjit_ops *ops = &bpfjit_module_ops; + bpfjit_func_t (*generate_code)(const bpf_ctx_t *, + const struct bpf_insn *, size_t); - membar_consumer(); - if (bpfjit_module_ops.bj_generate_code != NULL) { - return bpfjit_module_ops.bj_generate_code(bc, code, size); + generate_code = atomic_load_acquire(&ops->bj_generate_code); + if (generate_code != NULL) { + return generate_code(bc, code, size); } return NULL; } @@ -1289,7 +1292,6 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp) kmem_free(fcode, size); return EINVAL; } - membar_consumer(); if (bpf_jit) jcode = bpf_jit_generate(NULL, fcode, flen); } else { @@ -1306,8 +1308,7 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp) mutex_enter(&bpf_mtx); mutex_enter(d->bd_mtx); oldf = d->bd_filter; - d->bd_filter = newf; - membar_producer(); + atomic_store_release(&d->bd_filter, newf); reset_d(d); pserialize_perform(bpf_psz); mutex_exit(d->bd_mtx); @@ -1607,8 +1608,7 @@ bpf_deliver(struct bpf_if *bp, void *(*cpfn)(void *, const void *, size_t), atomic_inc_ulong(&d->bd_rcount); BPF_STATINC(recv); - filter = d->bd_filter; - membar_datadep_consumer(); + filter = atomic_load_consume(&d->bd_filter); if (filter != NULL) { if (filter->bf_jitcode != NULL) slen = filter->bf_jitcode(NULL, &args); @@ -2308,13 +2308,6 @@ sysctl_net_bpf_jit(SYSCTLFN_ARGS) return error; bpf_jit = newval; - - /* - * Do a full sync to publish new bpf_jit value and - * update bpfjit_module_ops.bj_generate_code variable. - */ - membar_sync(); - if (newval && bpfjit_module_ops.bj_generate_code == NULL) { printf("JIT compilation is postponed " "until after bpfjit module is loaded\n"); diff --git a/sys/net/bpfjit.c b/sys/net/bpfjit.c index f250cccfd822..ee88f9113156 100644 --- a/sys/net/bpfjit.c +++ b/sys/net/bpfjit.c @@ -231,9 +231,8 @@ bpfjit_modcmd(modcmd_t cmd, void *arg) switch (cmd) { case MODULE_CMD_INIT: bpfjit_module_ops.bj_free_code = &bpfjit_free_code; - membar_producer(); - bpfjit_module_ops.bj_generate_code = &bpfjit_generate_code; - membar_producer(); + atomic_store_release(&bpfjit_module_ops.bj_generate_code, + &bpfjit_generate_code); return 0; case MODULE_CMD_FINI: From f71fa6de65a95a81229de8db0dfaec774a47f637 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 15:36:53 +0000 Subject: [PATCH 2/6] Fix wrong memory order and switch pfil to atomic_load/store_*. --- sys/net/pfil.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/sys/net/pfil.c b/sys/net/pfil.c index 677f8b586251..0e71f9127050 100644 --- a/sys/net/pfil.c +++ b/sys/net/pfil.c @@ -232,8 +232,7 @@ pfil_list_add(pfil_listset_t *phlistset, pfil_polyfunc_t func, void *arg, pfh->pfil_arg = arg; /* switch from oldlist to newlist */ - membar_producer(); - phlistset->active = newlist; + atomic_store_release(&phlistset->active, newlist); #ifdef NET_MPSAFE pserialize_perform(pfil_psz); #endif @@ -336,8 +335,7 @@ pfil_list_remove(pfil_listset_t *phlistset, pfil_polyfunc_t func, void *arg) newlist->nhooks--; /* switch from oldlist to newlist */ - phlistset->active = newlist; - membar_producer(); + atomic_store_release(&phlistset->active, newlist); #ifdef NET_MPSAFE pserialize_perform(pfil_psz); #endif @@ -406,8 +404,7 @@ pfil_run_hooks(pfil_head_t *ph, struct mbuf **mp, ifnet_t *ifp, int dir) bound = curlwp_bind(); s = pserialize_read_enter(); - phlist = phlistset->active; - membar_datadep_consumer(); + phlist = atomic_load_consume(&phlistset->active); psref_acquire(&psref, &phlist->psref, pfil_psref_class); pserialize_read_exit(s); for (u_int i = 0; i < phlist->nhooks; i++) { @@ -436,8 +433,7 @@ pfil_run_arg(pfil_listset_t *phlistset, u_long cmd, void *arg) bound = curlwp_bind(); s = pserialize_read_enter(); - phlist = phlistset->active; - membar_datadep_consumer(); + phlist = atomic_load_consume(&phlistset->active); psref_acquire(&psref, &phlist->psref, pfil_psref_class); pserialize_read_exit(s); for (u_int i = 0; i < phlist->nhooks; i++) { From 53e37c4e68dabbeb5a71e85aaeeeaeb71e80cdec Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 15:44:42 +0000 Subject: [PATCH 3/6] Switch if_gif to atomic_load/store_*. --- sys/net/if_gif.c | 5 ++--- sys/net/if_gif.h | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sys/net/if_gif.c b/sys/net/if_gif.c index 06f34c04bd73..ed09a73543ef 100644 --- a/sys/net/if_gif.c +++ b/sys/net/if_gif.c @@ -40,6 +40,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_gif.c,v 1.150 2019/10/30 03:45:59 knakahara Exp $ #include #include +#include #include #include #include @@ -1130,7 +1131,6 @@ gif_set_tunnel(struct ifnet *ifp, struct sockaddr *src, struct sockaddr *dst) if (error) goto out; psref_target_init(&nvar->gv_psref, gv_psref_class); - membar_producer(); gif_update_variant(sc, nvar); mutex_exit(&sc->gif_lock); @@ -1207,7 +1207,6 @@ gif_delete_tunnel(struct ifnet *ifp) nvar->gv_encap_cookie6 = NULL; nvar->gv_output = NULL; psref_target_init(&nvar->gv_psref, gv_psref_class); - membar_producer(); gif_update_variant(sc, nvar); mutex_exit(&sc->gif_lock); @@ -1240,7 +1239,7 @@ gif_update_variant(struct gif_softc *sc, struct gif_variant *nvar) KASSERT(mutex_owned(&sc->gif_lock)); - sc->gif_var = nvar; + atomic_store_release(&sc->gif_var, nvar); pserialize_perform(sc->gif_psz); psref_target_destroy(&ovar->gv_psref, gv_psref_class); diff --git a/sys/net/if_gif.h b/sys/net/if_gif.h index ae46cb7e2a15..7f522a37527c 100644 --- a/sys/net/if_gif.h +++ b/sys/net/if_gif.h @@ -101,9 +101,8 @@ gif_getref_variant(struct gif_softc *sc, struct psref *psref) int s; s = pserialize_read_enter(); - var = sc->gif_var; + var = atomic_load_consume(&sc->gif_var); KASSERT(var != NULL); - membar_datadep_consumer(); psref_acquire(psref, &var->gv_psref, gv_psref_class); pserialize_read_exit(s); From b600b6b7212784cf5c738541534db0aaadedb8ee Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 15:53:00 +0000 Subject: [PATCH 4/6] Fix order in rollback case; switch if_ipsec to atomic_load/store_*. --- sys/net/if_ipsec.c | 10 ++++------ sys/net/if_ipsec.h | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sys/net/if_ipsec.c b/sys/net/if_ipsec.c index 481ae7adbc99..6da2f50f30b5 100644 --- a/sys/net/if_ipsec.c +++ b/sys/net/if_ipsec.c @@ -34,6 +34,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_ipsec.c,v 1.25 2019/11/01 04:28:14 knakahara Exp #endif #include +#include #include #include #include @@ -1132,7 +1133,6 @@ if_ipsec_set_tunnel(struct ifnet *ifp, if_ipsec_copy_variant(nullvar, ovar); if_ipsec_clear_config(nullvar); psref_target_init(&nullvar->iv_psref, iv_psref_class); - membar_producer(); /* * (2-3) Swap variant include its SPs. */ @@ -1238,7 +1238,6 @@ if_ipsec_delete_tunnel(struct ifnet *ifp) if_ipsec_copy_variant(nullvar, ovar); if_ipsec_clear_config(nullvar); psref_target_init(&nullvar->iv_psref, iv_psref_class); - membar_producer(); /* * (2-3) Swap variant include its SPs. */ @@ -1325,7 +1324,6 @@ if_ipsec_ensure_flags(struct ifnet *ifp, u_short oflags) if_ipsec_copy_variant(nullvar, ovar); if_ipsec_clear_config(nullvar); psref_target_init(&nullvar->iv_psref, iv_psref_class); - membar_producer(); /* * (2-3) Swap variant include its SPs. */ @@ -1896,16 +1894,16 @@ if_ipsec_update_variant(struct ipsec_softc *sc, struct ipsec_variant *nvar, * we stop packet processing while replacing SPs, that is, we set * "null" config variant to sc->ipsec_var. */ - sc->ipsec_var = nullvar; + atomic_store_release(&sc->ipsec_var, nullvar); pserialize_perform(sc->ipsec_psz); psref_target_destroy(&ovar->iv_psref, iv_psref_class); error = if_ipsec_replace_sp(sc, ovar, nvar); if (!error) - sc->ipsec_var = nvar; + atomic_store_release(&sc->ipsec_var, nvar); else { - sc->ipsec_var = ovar; /* rollback */ psref_target_init(&ovar->iv_psref, iv_psref_class); + atomic_store_release(&sc->ipsec_var, ovar); /* rollback */ } pserialize_perform(sc->ipsec_psz); diff --git a/sys/net/if_ipsec.h b/sys/net/if_ipsec.h index 25529ccf95eb..0db5010af991 100644 --- a/sys/net/if_ipsec.h +++ b/sys/net/if_ipsec.h @@ -165,9 +165,8 @@ if_ipsec_getref_variant(struct ipsec_softc *sc, struct psref *psref) int s; s = pserialize_read_enter(); - var = sc->ipsec_var; + var = atomic_load_consume(&sc->ipsec_var); KASSERT(var != NULL); - membar_datadep_consumer(); psref_acquire(psref, &var->iv_psref, iv_psref_class); pserialize_read_exit(s); From 6c7d7b0ea4d7171afcfa215c3ab8e37b6860acd3 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 15:58:35 +0000 Subject: [PATCH 5/6] Switch if_l2tp to atomic_load/store_*. Fix missing membar_datadep_consumer -- now atomic_load_consume -- in l2tp_lookup_session_ref. --- sys/net/if_l2tp.c | 20 ++++---------------- sys/net/if_l2tp.h | 3 +-- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/sys/net/if_l2tp.c b/sys/net/if_l2tp.c index f4bf2d9648a4..e71d78fd406a 100644 --- a/sys/net/if_l2tp.c +++ b/sys/net/if_l2tp.c @@ -1062,7 +1062,6 @@ l2tp_set_tunnel(struct ifnet *ifp, struct sockaddr *src, struct sockaddr *dst) encap_lock_exit(); goto error; } - membar_producer(); l2tp_variant_update(sc, nvar); mutex_exit(&sc->l2tp_lock); @@ -1111,7 +1110,6 @@ l2tp_delete_tunnel(struct ifnet *ifp) psref_target_init(&nvar->lv_psref, lv_psref_class); nvar->lv_psrc = NULL; nvar->lv_pdst = NULL; - membar_producer(); l2tp_variant_update(sc, nvar); mutex_exit(&sc->l2tp_lock); @@ -1186,7 +1184,6 @@ l2tp_set_session(struct l2tp_softc *sc, uint32_t my_sess_id, psref_target_init(&nvar->lv_psref, lv_psref_class); nvar->lv_my_sess_id = my_sess_id; nvar->lv_peer_sess_id = peer_sess_id; - membar_producer(); mutex_enter(&l2tp_hash.lock); if (ovar->lv_my_sess_id > 0 && ovar->lv_peer_sess_id > 0) { @@ -1227,7 +1224,6 @@ l2tp_clear_session(struct l2tp_softc *sc) psref_target_init(&nvar->lv_psref, lv_psref_class); nvar->lv_my_sess_id = 0; nvar->lv_peer_sess_id = 0; - membar_producer(); mutex_enter(&l2tp_hash.lock); if (ovar->lv_my_sess_id > 0 && ovar->lv_peer_sess_id > 0) { @@ -1254,7 +1250,7 @@ l2tp_lookup_session_ref(uint32_t id, struct psref *psref) s = pserialize_read_enter(); PSLIST_READER_FOREACH(sc, &l2tp_hash.lists[idx], struct l2tp_softc, l2tp_hash) { - struct l2tp_variant *var = sc->l2tp_var; + struct l2tp_variant *var = atomic_load_consume(&sc->l2tp_var); if (var == NULL) continue; if (var->lv_my_sess_id != id) @@ -1283,17 +1279,12 @@ l2tp_variant_update(struct l2tp_softc *sc, struct l2tp_variant *nvar) KASSERT(mutex_owned(&sc->l2tp_lock)); - sc->l2tp_var = nvar; + atomic_store_release(&sc->l2tp_var, nvar); pserialize_perform(sc->l2tp_psz); psref_target_destroy(&ovar->lv_psref, lv_psref_class); - /* - * In the manual of atomic_swap_ptr(3), there is no mention if 2nd - * argument is rewrite or not. So, use sc->l2tp_var instead of nvar. - */ - if (sc->l2tp_var != NULL) { - if (sc->l2tp_var->lv_psrc != NULL - && sc->l2tp_var->lv_pdst != NULL) + if (nvar != NULL) { + if (nvar->lv_psrc != NULL && nvar->lv_pdst != NULL) ifp->if_flags |= IFF_RUNNING; else ifp->if_flags &= ~IFF_RUNNING; @@ -1324,7 +1315,6 @@ l2tp_set_cookie(struct l2tp_softc *sc, uint64_t my_cookie, u_int my_cookie_len, nvar->lv_peer_cookie = peer_cookie; nvar->lv_peer_cookie_len = peer_cookie_len; nvar->lv_use_cookie = L2TP_COOKIE_ON; - membar_producer(); l2tp_variant_update(sc, nvar); mutex_exit(&sc->l2tp_lock); @@ -1358,7 +1348,6 @@ l2tp_clear_cookie(struct l2tp_softc *sc) nvar->lv_peer_cookie = 0; nvar->lv_peer_cookie_len = 0; nvar->lv_use_cookie = L2TP_COOKIE_OFF; - membar_producer(); l2tp_variant_update(sc, nvar); mutex_exit(&sc->l2tp_lock); @@ -1377,7 +1366,6 @@ l2tp_set_state(struct l2tp_softc *sc, int state) *nvar = *sc->l2tp_var; psref_target_init(&nvar->lv_psref, lv_psref_class); nvar->lv_state = state; - membar_producer(); l2tp_variant_update(sc, nvar); if (nvar->lv_state == L2TP_STATE_UP) { diff --git a/sys/net/if_l2tp.h b/sys/net/if_l2tp.h index cbadbbcd93cc..bd9afadc654c 100644 --- a/sys/net/if_l2tp.h +++ b/sys/net/if_l2tp.h @@ -131,12 +131,11 @@ l2tp_getref_variant(struct l2tp_softc *sc, struct psref *psref) int s; s = pserialize_read_enter(); - var = sc->l2tp_var; + var = atomic_load_consume(&sc->l2tp_var); if (var == NULL) { pserialize_read_exit(s); return NULL; } - membar_datadep_consumer(); psref_acquire(psref, &var->lv_psref, lv_psref_class); pserialize_read_exit(s); From 01bc27c096a1054d6993a5e5de755f5f43f8a7e9 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 29 Jan 2020 16:01:55 +0000 Subject: [PATCH 6/6] Switch if_vlan to atomic_load/store_*. Fix missing membar_datadep_consumer -- now atomic_load_consume -- in vlan_lookup_tag_psref. --- sys/net/if_vlan.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index 9bae536569a1..2769c4da12a8 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -780,12 +780,11 @@ vlan_getref_linkmib(struct ifvlan *sc, struct psref *psref) int s; s = pserialize_read_enter(); - mib = sc->ifv_mib; + mib = atomic_load_consume(&sc->ifv_mib); if (mib == NULL) { pserialize_read_exit(s); return NULL; } - membar_datadep_consumer(); psref_acquire(psref, &mib->ifvm_psref, ifvm_psref_class); pserialize_read_exit(s); @@ -812,7 +811,7 @@ vlan_lookup_tag_psref(struct ifnet *ifp, uint16_t tag, struct psref *psref) s = pserialize_read_enter(); PSLIST_READER_FOREACH(sc, &ifv_hash.lists[idx], struct ifvlan, ifv_hash) { - struct ifvlan_linkmib *mib = sc->ifv_mib; + struct ifvlan_linkmib *mib = atomic_load_consume(&sc->ifv_mib); if (mib == NULL) continue; if (mib->ifvm_tag != tag) @@ -835,8 +834,7 @@ vlan_linkmib_update(struct ifvlan *ifv, struct ifvlan_linkmib *nmib) KASSERT(mutex_owned(&ifv->ifv_lock)); - membar_producer(); - ifv->ifv_mib = nmib; + atomic_store_release(&ifv->ifv_mib, nmib); pserialize_perform(ifv->ifv_psz); psref_target_destroy(&omib->ifvm_psref, ifvm_psref_class);