Re: recursive splsoftnet()
On 2 January 2017 at 12:39, Martin Pieuchot <m...@openbsd.org> wrote: > Kill two simple recursive splsoftnet(). > > - pr_usrreq() functions are always called under NET_LOCK(). > - tcp_ident() is called by tcp_sysctl() only which asserts for the NET_LOCK() > > ok? > OK
recursive splsoftnet()
Kill two simple recursive splsoftnet().
- pr_usrreq() functions are always called under NET_LOCK().
- tcp_ident() is called by tcp_sysctl() only which asserts for the NET_LOCK()
ok?
Index: netinet/ip_gre.c
===
RCS file: /cvs/src/sys/netinet/ip_gre.c,v
retrieving revision 1.59
diff -u -p -r1.59 ip_gre.c
--- netinet/ip_gre.c4 Mar 2016 22:38:23 - 1.59
+++ netinet/ip_gre.c2 Jan 2017 11:34:01 -
@@ -411,12 +411,10 @@ gre_usrreq(struct socket *so, int req, s
struct inpcb *inp = sotoinpcb(so);
if (inp != NULL && inp->inp_pipex && req == PRU_SEND) {
- int s;
struct sockaddr_in *sin4;
struct in_addr *ina_dst;
struct pipex_session *session;
- s = splsoftnet();
ina_dst = NULL;
if ((so->so_state & SS_ISCONNECTED) != 0) {
inp = sotoinpcb(so);
@@ -432,7 +430,6 @@ gre_usrreq(struct socket *so, int req, s
(session = pipex_pptp_userland_lookup_session_ipv4(m,
*ina_dst)))
m = pipex_pptp_userland_output(m, session);
- splx(s);
if (m == NULL)
return (ENOMEM);
Index: netinet/tcp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.140
diff -u -p -r1.140 tcp_usrreq.c
--- netinet/tcp_usrreq.c26 Dec 2016 21:30:10 - 1.140
+++ netinet/tcp_usrreq.c2 Jan 2017 11:30:56 -
@@ -732,7 +732,7 @@ tcp_usrclosed(struct tcpcb *tp)
int
tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop)
{
- int error = 0, s;
+ int error = 0;
struct tcp_ident_mapping tir;
struct inpcb *inp;
struct tcpcb *tp = NULL;
@@ -741,6 +741,9 @@ tcp_ident(void *oldp, size_t *oldlenp, v
struct sockaddr_in6 *fin6, *lin6;
struct in6_addr f6, l6;
#endif
+
+ splsoftassert(IPL_SOFTNET);
+
if (dodrop) {
if (oldp != NULL || *oldlenp != 0)
return (EINVAL);
@@ -781,7 +784,6 @@ tcp_ident(void *oldp, size_t *oldlenp, v
return (EINVAL);
}
- s = splsoftnet();
switch (tir.faddr.ss_family) {
#ifdef INET6
case AF_INET6:
@@ -803,7 +805,6 @@ tcp_ident(void *oldp, size_t *oldlenp, v
tp = tcp_drop(tp, ECONNABORTED);
else
error = ESRCH;
- splx(s);
return (error);
}
@@ -830,7 +831,6 @@ tcp_ident(void *oldp, size_t *oldlenp, v
tir.ruid = -1;
tir.euid = -1;
}
- splx(s);
*oldlenp = sizeof (tir);
error = copyout((void *), oldp, sizeof (tir));
Re: Recursive splsoftnet() in in6_ifattach_linklocal()
On Thu, Dec 29, 2016 at 09:15:54AM +0100, Martin Pieuchot wrote:
> Get rid of them, ok?
OK bluhm@
>
> Index: netinet6/in6_ifattach.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 in6_ifattach.c
> --- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100
> +++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 -
> @@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp
> {
> struct in6_aliasreq ifra;
> struct in6_ifaddr *ia6;
> - int s, error, flags;
> + int error, flags;
> +
> + splsoftassert(IPL_SOFTNET);
>
> /*
>* configure link-local address.
> @@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp
> if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0))
> ifra.ifra_flags |= IN6_IFF_TENTATIVE;
>
> - s = splsoftnet();
> error = in6_update_ifa(ifp, , in6ifa_ifpforlinklocal(ifp, 0));
> - splx(s);
> if (error != 0)
> return (error);
>
> @@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp
> if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
> flags |= RTF_CLONING;
>
> - s = splsoftnet();
> error = rt_ifa_add(>ia_ifa, flags, ia6->ia_ifa.ifa_addr);
> if (error) {
> in6_purgeaddr(>ia_ifa);
> - splx(s);
> return (error);
> }
> dohooks(ifp->if_addrhooks, 0);
> - splx(s);
>
> return (0);
> }
Recursive splsoftnet() in in6_ifattach_linklocal()
Get rid of them, ok?
Index: netinet6/in6_ifattach.c
===
RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
retrieving revision 1.100
diff -u -p -r1.100 in6_ifattach.c
--- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100
+++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 -
@@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp
{
struct in6_aliasreq ifra;
struct in6_ifaddr *ia6;
- int s, error, flags;
+ int error, flags;
+
+ splsoftassert(IPL_SOFTNET);
/*
* configure link-local address.
@@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp
if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0))
ifra.ifra_flags |= IN6_IFF_TENTATIVE;
- s = splsoftnet();
error = in6_update_ifa(ifp, , in6ifa_ifpforlinklocal(ifp, 0));
- splx(s);
if (error != 0)
return (error);
@@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp
if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
flags |= RTF_CLONING;
- s = splsoftnet();
error = rt_ifa_add(>ia_ifa, flags, ia6->ia_ifa.ifa_addr);
if (error) {
in6_purgeaddr(>ia_ifa);
- splx(s);
return (error);
}
dohooks(ifp->if_addrhooks, 0);
- splx(s);
return (0);
}
Re: recursive splsoftnet() in ND6
On Mon, Nov 28, 2016 at 12:31:39PM +0100, Martin Pieuchot wrote:
> This kill multiple recursive splsoftnet(), ok?
OK bluhm@
>
> Index: netinet6/nd6.h
> ===
> RCS file: /cvs/src/sys/netinet6/nd6.h,v
> retrieving revision 1.64
> diff -u -p -r1.64 nd6.h
> --- netinet6/nd6.h21 Nov 2016 10:52:08 - 1.64
> +++ netinet6/nd6.h28 Nov 2016 11:19:49 -
> @@ -288,13 +288,10 @@ void nd6_rs_detach(struct ifnet *);
> void nd6_rs_input(struct mbuf *, int, int);
>
> void prelist_del(struct nd_prefix *);
> -void defrouter_addreq(struct nd_defrouter *);
> void defrouter_reset(void);
> void defrouter_select(void);
> void defrtrlist_del(struct nd_defrouter *);
> void prelist_remove(struct nd_prefix *);
> -int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
> - struct nd_prefix **);
> void pfxlist_onlink_check(void);
> struct nd_defrouter *defrouter_lookup(struct in6_addr *, unsigned int);
>
> Index: netinet6/nd6_rtr.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
> retrieving revision 1.151
> diff -u -p -r1.151 nd6_rtr.c
> --- netinet6/nd6_rtr.c21 Nov 2016 10:56:26 - 1.151
> +++ netinet6/nd6_rtr.c28 Nov 2016 11:25:24 -
> @@ -70,6 +70,9 @@ int nd6_prefix_onlink(struct nd_prefix *
> int nd6_prefix_offlink(struct nd_prefix *);
> void in6_init_address_ltimes(struct nd_prefix *, struct in6_addrlifetime *);
> int prelist_update(struct nd_prefix *, struct nd_defrouter *, struct mbuf *);
> +int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
> + struct nd_prefix **);
> +void defrouter_addreq(struct nd_defrouter *);
> int rt6_deleteroute(struct rtentry *, void *, unsigned int);
>
> void nd6_addr_add(void *);
> @@ -578,7 +581,6 @@ defrouter_addreq(struct nd_defrouter *ne
> struct rt_addrinfo info;
> struct sockaddr_in6 def, mask, gate;
> struct rtentry *rt;
> - int s;
> int error;
>
> memset(, 0, sizeof(def));
> @@ -597,7 +599,6 @@ defrouter_addreq(struct nd_defrouter *ne
> info.rti_info[RTAX_GATEWAY] = sin6tosa();
> info.rti_info[RTAX_NETMASK] = sin6tosa();
>
> - s = splsoftnet();
> error = rtrequest(RTM_ADD, , RTP_DEFAULT, ,
> new->ifp->if_rdomain);
> if (error == 0) {
> @@ -605,8 +606,6 @@ defrouter_addreq(struct nd_defrouter *ne
> rtfree(rt);
> new->installed = 1;
> }
> - splx(s);
> - return;
> }
>
> struct nd_defrouter *
> @@ -663,7 +662,7 @@ defrtrlist_del(struct nd_defrouter *dr)
>
> ext->ndefrouters--;
> if (ext->ndefrouters < 0) {
> - log(LOG_WARNING, "defrtrlist_del: negative count on %s\n",
> + log(LOG_WARNING, "%s: negative count on %s\n", __func__,
> dr->ifp->if_xname);
> }
>
> @@ -758,14 +757,12 @@ defrouter_select(void)
> struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL;
> struct rtentry *rt = NULL;
> struct llinfo_nd6 *ln = NULL;
> - int s = splsoftnet();
>
> /*
>* Let's handle easy case (3) first:
>* If default router list is empty, there's nothing to be done.
>*/
> if (TAILQ_EMPTY(_defrouter)) {
> - splx(s);
> return;
> }
>
> @@ -830,9 +827,6 @@ defrouter_select(void)
> defrouter_delreq(installed_dr);
> defrouter_addreq(selected_dr);
> }
> -
> - splx(s);
> - return;
> }
>
> /*
> @@ -1049,14 +1043,14 @@ nd6_prelist_add(struct nd_prefix *pr, st
> struct nd_prefix **newp)
> {
> struct nd_prefix *new = NULL;
> - int i, s;
> struct in6_ifextra *ext = pr->ndpr_ifp->if_afdata[AF_INET6];
> + int i;
> +
> + splsoftassert(IPL_SOFTNET);
>
> if (ip6_maxifprefixes >= 0) {
> if (ext->nprefixes >= ip6_maxifprefixes / 2) {
> - s = splsoftnet();
> purge_detached(pr->ndpr_ifp);
> - splx(s);
> }
> if (ext->nprefixes >= ip6_maxifprefixes)
> return(ENOMEM);
> @@ -1079,7 +1073,6 @@ nd6_prelist_add(struct nd_prefix *pr, st
>
> task_set(>ndpr_task, nd6_addr_add, new);
>
> - s = splsoftnet();
> /* link ndpr_entry to nd_prefix list */
> LIST_INSERT_HEAD(_prefix, new, ndpr_entry);
>
> @@ -1089,8 +1082,8 @@ nd6_prelist_add(struc
recursive splsoftnet() in ND6
This kill multiple recursive splsoftnet(), ok?
Index: netinet6/nd6.h
===
RCS file: /cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.64
diff -u -p -r1.64 nd6.h
--- netinet6/nd6.h 21 Nov 2016 10:52:08 - 1.64
+++ netinet6/nd6.h 28 Nov 2016 11:19:49 -
@@ -288,13 +288,10 @@ void nd6_rs_detach(struct ifnet *);
void nd6_rs_input(struct mbuf *, int, int);
void prelist_del(struct nd_prefix *);
-void defrouter_addreq(struct nd_defrouter *);
void defrouter_reset(void);
void defrouter_select(void);
void defrtrlist_del(struct nd_defrouter *);
void prelist_remove(struct nd_prefix *);
-int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
- struct nd_prefix **);
void pfxlist_onlink_check(void);
struct nd_defrouter *defrouter_lookup(struct in6_addr *, unsigned int);
Index: netinet6/nd6_rtr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.151
diff -u -p -r1.151 nd6_rtr.c
--- netinet6/nd6_rtr.c 21 Nov 2016 10:56:26 - 1.151
+++ netinet6/nd6_rtr.c 28 Nov 2016 11:25:24 -
@@ -70,6 +70,9 @@ int nd6_prefix_onlink(struct nd_prefix *
int nd6_prefix_offlink(struct nd_prefix *);
void in6_init_address_ltimes(struct nd_prefix *, struct in6_addrlifetime *);
int prelist_update(struct nd_prefix *, struct nd_defrouter *, struct mbuf *);
+int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
+ struct nd_prefix **);
+void defrouter_addreq(struct nd_defrouter *);
int rt6_deleteroute(struct rtentry *, void *, unsigned int);
void nd6_addr_add(void *);
@@ -578,7 +581,6 @@ defrouter_addreq(struct nd_defrouter *ne
struct rt_addrinfo info;
struct sockaddr_in6 def, mask, gate;
struct rtentry *rt;
- int s;
int error;
memset(, 0, sizeof(def));
@@ -597,7 +599,6 @@ defrouter_addreq(struct nd_defrouter *ne
info.rti_info[RTAX_GATEWAY] = sin6tosa();
info.rti_info[RTAX_NETMASK] = sin6tosa();
- s = splsoftnet();
error = rtrequest(RTM_ADD, , RTP_DEFAULT, ,
new->ifp->if_rdomain);
if (error == 0) {
@@ -605,8 +606,6 @@ defrouter_addreq(struct nd_defrouter *ne
rtfree(rt);
new->installed = 1;
}
- splx(s);
- return;
}
struct nd_defrouter *
@@ -663,7 +662,7 @@ defrtrlist_del(struct nd_defrouter *dr)
ext->ndefrouters--;
if (ext->ndefrouters < 0) {
- log(LOG_WARNING, "defrtrlist_del: negative count on %s\n",
+ log(LOG_WARNING, "%s: negative count on %s\n", __func__,
dr->ifp->if_xname);
}
@@ -758,14 +757,12 @@ defrouter_select(void)
struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL;
struct rtentry *rt = NULL;
struct llinfo_nd6 *ln = NULL;
- int s = splsoftnet();
/*
* Let's handle easy case (3) first:
* If default router list is empty, there's nothing to be done.
*/
if (TAILQ_EMPTY(_defrouter)) {
- splx(s);
return;
}
@@ -830,9 +827,6 @@ defrouter_select(void)
defrouter_delreq(installed_dr);
defrouter_addreq(selected_dr);
}
-
- splx(s);
- return;
}
/*
@@ -1049,14 +1043,14 @@ nd6_prelist_add(struct nd_prefix *pr, st
struct nd_prefix **newp)
{
struct nd_prefix *new = NULL;
- int i, s;
struct in6_ifextra *ext = pr->ndpr_ifp->if_afdata[AF_INET6];
+ int i;
+
+ splsoftassert(IPL_SOFTNET);
if (ip6_maxifprefixes >= 0) {
if (ext->nprefixes >= ip6_maxifprefixes / 2) {
- s = splsoftnet();
purge_detached(pr->ndpr_ifp);
- splx(s);
}
if (ext->nprefixes >= ip6_maxifprefixes)
return(ENOMEM);
@@ -1079,7 +1073,6 @@ nd6_prelist_add(struct nd_prefix *pr, st
task_set(>ndpr_task, nd6_addr_add, new);
- s = splsoftnet();
/* link ndpr_entry to nd_prefix list */
LIST_INSERT_HEAD(_prefix, new, ndpr_entry);
@@ -1089,8 +1082,8 @@ nd6_prelist_add(struct nd_prefix *pr, st
int e;
if ((e = nd6_prefix_onlink(new)) != 0) {
- nd6log((LOG_ERR, "nd6_prelist_add: failed to make "
- "the prefix %s/%d on-link on %s (errno=%d)\n",
+ nd6log((LOG_ERR, "%s: failed to make the prefix %s/%d"
+ " on-link on %s (errno=%d)\n", __func__,
inet_ntop(AF_INET6, >ndpr_prefix.sin6_addr,
addr, sizeof(addr)),
pr->nd
Re: tcp_input.c: recursive splsoftnet()
On Wed, Nov 16, 2016 at 09:58:00AM +0100, Martin Pieuchot wrote:
> On 15/11/16(Tue) 16:36, Alexander Bluhm wrote:
> > [...]
> > You are a bit inconsistent wether you remove the {} from one line
> > if blocks.
>
> That should fix my inconsistencies.
OK bluhm@
>
> Index: netinet/tcp_input.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.332
> diff -u -p -r1.332 tcp_input.c
> --- netinet/tcp_input.c 16 Nov 2016 08:50:32 - 1.332
> +++ netinet/tcp_input.c 16 Nov 2016 08:57:03 -
> @@ -3678,10 +3678,9 @@ syn_cache_get(struct sockaddr *src, stru
>
> splsoftassert(IPL_SOFTNET);
>
> - if ((sc = syn_cache_lookup(src, dst, ,
> - sotoinpcb(so)->inp_rtableid)) == NULL) {
> + sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid);
> + if (sc == NULL)
> return (NULL);
> - }
>
> /*
>* Verify the sequence and ack numbers. Try getting the correct
> @@ -3910,9 +3909,8 @@ syn_cache_reset(struct sockaddr *src, st
> if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
> return;
> if (SEQ_LT(th->th_seq, sc->sc_irs) ||
> - SEQ_GT(th->th_seq, sc->sc_irs+1)) {
> + SEQ_GT(th->th_seq, sc->sc_irs + 1))
> return;
> - }
> syn_cache_rm(sc);
> tcpstat.tcps_sc_reset++;
> syn_cache_put(sc);
> @@ -3927,9 +3925,8 @@ syn_cache_unreach(struct sockaddr *src,
>
> splsoftassert(IPL_SOFTNET);
>
> - if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL) {
> + if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
> return;
> - }
> /* If the sequence number != sc_iss, then it's a bogus ICMP msg */
> if (ntohl (th->th_seq) != sc->sc_iss) {
> return;
> @@ -4030,8 +4027,8 @@ syn_cache_add(struct sockaddr *src, stru
>* If we do, resend the SYN,ACK. We do not count this
>* as a retransmission (XXX though maybe we should).
>*/
> - if ((sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid))
> - != NULL) {
> + sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid);
> + if (sc != NULL) {
> tcpstat.tcps_sc_dupesyn++;
> if (ipopts) {
> /*
Re: tcp_input.c: recursive splsoftnet()
On 15/11/16(Tue) 16:36, Alexander Bluhm wrote:
> [...]
> You are a bit inconsistent wether you remove the {} from one line
> if blocks.
That should fix my inconsistencies.
Index: netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.332
diff -u -p -r1.332 tcp_input.c
--- netinet/tcp_input.c 16 Nov 2016 08:50:32 - 1.332
+++ netinet/tcp_input.c 16 Nov 2016 08:57:03 -
@@ -3678,10 +3678,9 @@ syn_cache_get(struct sockaddr *src, stru
splsoftassert(IPL_SOFTNET);
- if ((sc = syn_cache_lookup(src, dst, ,
- sotoinpcb(so)->inp_rtableid)) == NULL) {
+ sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid);
+ if (sc == NULL)
return (NULL);
- }
/*
* Verify the sequence and ack numbers. Try getting the correct
@@ -3910,9 +3909,8 @@ syn_cache_reset(struct sockaddr *src, st
if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
return;
if (SEQ_LT(th->th_seq, sc->sc_irs) ||
- SEQ_GT(th->th_seq, sc->sc_irs+1)) {
+ SEQ_GT(th->th_seq, sc->sc_irs + 1))
return;
- }
syn_cache_rm(sc);
tcpstat.tcps_sc_reset++;
syn_cache_put(sc);
@@ -3927,9 +3925,8 @@ syn_cache_unreach(struct sockaddr *src,
splsoftassert(IPL_SOFTNET);
- if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL) {
+ if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
return;
- }
/* If the sequence number != sc_iss, then it's a bogus ICMP msg */
if (ntohl (th->th_seq) != sc->sc_iss) {
return;
@@ -4030,8 +4027,8 @@ syn_cache_add(struct sockaddr *src, stru
* If we do, resend the SYN,ACK. We do not count this
* as a retransmission (XXX though maybe we should).
*/
- if ((sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid))
- != NULL) {
+ sc = syn_cache_lookup(src, dst, , sotoinpcb(so)->inp_rtableid);
+ if (sc != NULL) {
tcpstat.tcps_sc_dupesyn++;
if (ipopts) {
/*
Re: pf_ioctl.c: recursive splsoftnet()
On Tue, Nov 15, 2016 at 03:41:48PM +0100, Martin Pieuchot wrote:
> Kill a bunch of recursive splsoftnet()s. pf_ioctl() already call
> splsoftnet() and for the functions that are also called from pf_test()
> the spl level is already IPL_SOFTNET.
>
> ok?
OK bluhm@
>
> Index: net/pf_if.c
> ===
> RCS file: /cvs/src/sys/net/pf_if.c,v
> retrieving revision 1.86
> diff -u -p -r1.86 pf_if.c
> --- net/pf_if.c 27 Sep 2016 04:57:17 - 1.86
> +++ net/pf_if.c 15 Nov 2016 14:27:47 -
> @@ -356,7 +356,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
> struct pfi_dynaddr *dyn;
> char tblname[PF_TABLE_NAME_SIZE];
> struct pf_ruleset *ruleset = NULL;
> - int s, rv = 0;
> + int rv = 0;
>
> if (aw->type != PF_ADDR_DYNIFTL)
> return (0);
> @@ -364,7 +364,6 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
> == NULL)
> return (1);
>
> - s = splsoftnet();
> if (!strcmp(aw->v.ifname, "self"))
> dyn->pfid_kif = pfi_kif_get(IFG_ALL);
> else
> @@ -407,7 +406,6 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
> TAILQ_INSERT_TAIL(>pfid_kif->pfik_dynaddrs, dyn, entry);
> aw->p.dyn = dyn;
> pfi_kif_update(dyn->pfid_kif);
> - splx(s);
> return (0);
>
> _bad:
> @@ -418,7 +416,6 @@ _bad:
> if (dyn->pfid_kif != NULL)
> pfi_kif_unref(dyn->pfid_kif, PFI_KIF_REF_RULE);
> pool_put(_addr_pl, dyn);
> - splx(s);
> return (rv);
> }
>
> @@ -589,13 +586,10 @@ pfi_address_add(struct sockaddr *sa, sa_
> void
> pfi_dynaddr_remove(struct pf_addr_wrap *aw)
> {
> - int s;
> -
> if (aw->type != PF_ADDR_DYNIFTL || aw->p.dyn == NULL ||
> aw->p.dyn->pfid_kif == NULL || aw->p.dyn->pfid_kt == NULL)
> return;
>
> - s = splsoftnet();
> TAILQ_REMOVE(>p.dyn->pfid_kif->pfik_dynaddrs, aw->p.dyn, entry);
> pfi_kif_unref(aw->p.dyn->pfid_kif, PFI_KIF_REF_RULE);
> aw->p.dyn->pfid_kif = NULL;
> @@ -603,7 +597,6 @@ pfi_dynaddr_remove(struct pf_addr_wrap *
> aw->p.dyn->pfid_kt = NULL;
> pool_put(_addr_pl, aw->p.dyn);
> aw->p.dyn = NULL;
> - splx(s);
> }
>
> void
> @@ -640,23 +633,20 @@ pfi_update_status(const char *name, stru
> struct pfi_kif_cmp key;
> struct ifg_memberp_member, *ifgm;
> TAILQ_HEAD(, ifg_member) ifg_members;
> - int i, j, k, s;
> + int i, j, k;
>
> - s = splsoftnet();
> if (*name == '\0' && pfs == NULL) {
> RB_FOREACH(p, pfi_ifhead, _ifs) {
> bzero(p->pfik_packets, sizeof(p->pfik_packets));
> bzero(p->pfik_bytes, sizeof(p->pfik_bytes));
> p->pfik_tzero = time_second;
> }
> - splx(s);
> return;
> }
>
> strlcpy(key.pfik_name, name, sizeof(key.pfik_name));
> p = RB_FIND(pfi_ifhead, _ifs, (struct pfi_kif *));
> if (p == NULL) {
> - splx(s);
> return;
> }
> if (p->pfik_group != NULL) {
> @@ -694,16 +684,14 @@ pfi_update_status(const char *name, stru
> p->pfik_bytes[i][j][k];
> }
> }
> - splx(s);
> }
>
> int
> pfi_get_ifaces(const char *name, struct pfi_kif *buf, int *size)
> {
> struct pfi_kif *p, *nextp;
> - int s, n = 0;
> + int n = 0;
>
> - s = splsoftnet();
> for (p = RB_MIN(pfi_ifhead, _ifs); p; p = nextp) {
> nextp = RB_NEXT(pfi_ifhead, _ifs, p);
> if (pfi_skip_if(name, p))
> @@ -714,14 +702,12 @@ pfi_get_ifaces(const char *name, struct
> pfi_kif_ref(p, PFI_KIF_REF_RULE);
> if (copyout(p, buf++, sizeof(*buf))) {
> pfi_kif_unref(p, PFI_KIF_REF_RULE);
> - splx(s);
> return (EFAULT);
> }
> nextp = RB_NEXT(pfi_ifhead, _ifs, p);
> pfi_kif_unref(p, PFI_KIF_REF_RULE);
> }
> }
> - splx(s);
> *size = n;
> return (0);
> }
> @@ -752,15 +738,12 @@ int
> pfi_set_flags(const char *
Re: tcp_input.c: recursive splsoftnet()
On Tue, Nov 15, 2016 at 04:03:40PM +0100, Martin Pieuchot wrote:
> Assert that the sync cache functions are always called under the correct
> IPL.
>
> This kill some more recursive splsoftnet().
>
> While here keep local declarations local, this makes my life easier.
> This way I know I don't need to grep the whole tree to look where the
> functions are called.
You are a bit inconsistent wether you remove the {} from one line
if blocks.
OK bluhm@
>
> Index: netinet/tcp_input.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.331
> diff -u -p -r1.331 tcp_input.c
> --- netinet/tcp_input.c 15 Nov 2016 14:30:59 - 1.331
> +++ netinet/tcp_input.c 15 Nov 2016 14:54:42 -
> @@ -186,8 +186,22 @@ do { \
> if_put(ifp); \
> } while (0)
>
> -void syn_cache_put(struct syn_cache *);
> -void syn_cache_rm(struct syn_cache *);
> +void syn_cache_put(struct syn_cache *);
> +void syn_cache_rm(struct syn_cache *);
> +int syn_cache_respond(struct syn_cache *, struct mbuf *);
> +void syn_cache_timer(void *);
> +void syn_cache_reaper(void *);
> +void syn_cache_insert(struct syn_cache *, struct tcpcb *);
> +void syn_cache_reset(struct sockaddr *, struct sockaddr *,
> + struct tcphdr *, u_int);
> +int syn_cache_add(struct sockaddr *, struct sockaddr *, struct tcphdr *,
> + unsigned int, struct socket *, struct mbuf *, u_char *, int,
> + struct tcp_opt_info *, tcp_seq *);
> +struct socket *syn_cache_get(struct sockaddr *, struct sockaddr *,
> + struct tcphdr *, unsigned int, unsigned int, struct socket *,
> + struct mbuf *);
> +struct syn_cache *syn_cache_lookup(struct sockaddr *, struct sockaddr *,
> + struct syn_cache_head **, u_int);
>
> /*
> * Insert segment ti into reassembly queue of tcp with
> @@ -3382,9 +3396,9 @@ syn_cache_insert(struct syn_cache *sc, s
> struct syn_cache_set *set = _syn_cache[tcp_syn_cache_active];
> struct syn_cache_head *scp;
> struct syn_cache *sc2;
> - int i, s;
> + int i;
>
> - s = splsoftnet();
> + splsoftassert(IPL_SOFTNET);
>
> /*
>* If there are no entries in the hash table, reinitialize
> @@ -3507,8 +3521,6 @@ syn_cache_insert(struct syn_cache *sc, s
> if (set->scs_use <= 0 &&
> tcp_syn_cache[!tcp_syn_cache_active].scs_count == 0)
> tcp_syn_cache_active = !tcp_syn_cache_active;
> -
> - splx(s);
> }
>
> /*
> @@ -3576,9 +3588,8 @@ void
> syn_cache_cleanup(struct tcpcb *tp)
> {
> struct syn_cache *sc, *nsc;
> - int s;
>
> - s = splsoftnet();
> + splsoftassert(IPL_SOFTNET);
>
> LIST_FOREACH_SAFE(sc, >t_sc, sc_tpq, nsc) {
> #ifdef DIAGNOSTIC
> @@ -3590,8 +3601,6 @@ syn_cache_cleanup(struct tcpcb *tp)
> }
> /* just for safety */
> LIST_INIT(>t_sc);
> -
> - splx(s);
> }
>
> /*
> @@ -3662,16 +3671,15 @@ syn_cache_get(struct sockaddr *src, stru
> struct inpcb *inp, *oldinp;
> struct tcpcb *tp = NULL;
> struct mbuf *am;
> - int s;
> struct socket *oso;
> #if NPF > 0
> struct pf_divert *divert = NULL;
> #endif
>
> - s = splsoftnet();
> + splsoftassert(IPL_SOFTNET);
> +
> if ((sc = syn_cache_lookup(src, dst, ,
> sotoinpcb(so)->inp_rtableid)) == NULL) {
> - splx(s);
> return (NULL);
> }
>
> @@ -3683,13 +3691,11 @@ syn_cache_get(struct sockaddr *src, stru
> SEQ_LEQ(th->th_seq, sc->sc_irs) ||
> SEQ_GT(th->th_seq, sc->sc_irs + 1 + sc->sc_win)) {
> (void) syn_cache_respond(sc, m);
> - splx(s);
> return ((struct socket *)(-1));
> }
>
> /* Remove this cache entry */
> syn_cache_rm(sc);
> - splx(s);
>
> /*
>* Ok, create the full blown connection, and set things up
> @@ -3898,19 +3904,16 @@ syn_cache_reset(struct sockaddr *src, st
> {
> struct syn_cache *sc;
> struct syn_cache_head *scp;
> - int s = splsoftnet();
>
> - if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL) {
> - splx(s);
> + splsoftassert(IPL_SOFTNET);
> +
> + if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
> return;
> - }
> if (SEQ_LT(th->th_seq, sc->sc_irs) ||
> SEQ_GT(th->th_seq, sc->sc_irs+1)) {
> - splx(s);
> return;
>
tcp_input.c: recursive splsoftnet()
Assert that the sync cache functions are always called under the correct
IPL.
This kill some more recursive splsoftnet().
While here keep local declarations local, this makes my life easier.
This way I know I don't need to grep the whole tree to look where the
functions are called.
Index: netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.331
diff -u -p -r1.331 tcp_input.c
--- netinet/tcp_input.c 15 Nov 2016 14:30:59 - 1.331
+++ netinet/tcp_input.c 15 Nov 2016 14:54:42 -
@@ -186,8 +186,22 @@ do { \
if_put(ifp); \
} while (0)
-void syn_cache_put(struct syn_cache *);
-void syn_cache_rm(struct syn_cache *);
+voidsyn_cache_put(struct syn_cache *);
+voidsyn_cache_rm(struct syn_cache *);
+int syn_cache_respond(struct syn_cache *, struct mbuf *);
+voidsyn_cache_timer(void *);
+voidsyn_cache_reaper(void *);
+voidsyn_cache_insert(struct syn_cache *, struct tcpcb *);
+voidsyn_cache_reset(struct sockaddr *, struct sockaddr *,
+ struct tcphdr *, u_int);
+int syn_cache_add(struct sockaddr *, struct sockaddr *, struct tcphdr *,
+ unsigned int, struct socket *, struct mbuf *, u_char *, int,
+ struct tcp_opt_info *, tcp_seq *);
+struct socket *syn_cache_get(struct sockaddr *, struct sockaddr *,
+ struct tcphdr *, unsigned int, unsigned int, struct socket *,
+ struct mbuf *);
+struct syn_cache *syn_cache_lookup(struct sockaddr *, struct sockaddr *,
+ struct syn_cache_head **, u_int);
/*
* Insert segment ti into reassembly queue of tcp with
@@ -3382,9 +3396,9 @@ syn_cache_insert(struct syn_cache *sc, s
struct syn_cache_set *set = _syn_cache[tcp_syn_cache_active];
struct syn_cache_head *scp;
struct syn_cache *sc2;
- int i, s;
+ int i;
- s = splsoftnet();
+ splsoftassert(IPL_SOFTNET);
/*
* If there are no entries in the hash table, reinitialize
@@ -3507,8 +3521,6 @@ syn_cache_insert(struct syn_cache *sc, s
if (set->scs_use <= 0 &&
tcp_syn_cache[!tcp_syn_cache_active].scs_count == 0)
tcp_syn_cache_active = !tcp_syn_cache_active;
-
- splx(s);
}
/*
@@ -3576,9 +3588,8 @@ void
syn_cache_cleanup(struct tcpcb *tp)
{
struct syn_cache *sc, *nsc;
- int s;
- s = splsoftnet();
+ splsoftassert(IPL_SOFTNET);
LIST_FOREACH_SAFE(sc, >t_sc, sc_tpq, nsc) {
#ifdef DIAGNOSTIC
@@ -3590,8 +3601,6 @@ syn_cache_cleanup(struct tcpcb *tp)
}
/* just for safety */
LIST_INIT(>t_sc);
-
- splx(s);
}
/*
@@ -3662,16 +3671,15 @@ syn_cache_get(struct sockaddr *src, stru
struct inpcb *inp, *oldinp;
struct tcpcb *tp = NULL;
struct mbuf *am;
- int s;
struct socket *oso;
#if NPF > 0
struct pf_divert *divert = NULL;
#endif
- s = splsoftnet();
+ splsoftassert(IPL_SOFTNET);
+
if ((sc = syn_cache_lookup(src, dst, ,
sotoinpcb(so)->inp_rtableid)) == NULL) {
- splx(s);
return (NULL);
}
@@ -3683,13 +3691,11 @@ syn_cache_get(struct sockaddr *src, stru
SEQ_LEQ(th->th_seq, sc->sc_irs) ||
SEQ_GT(th->th_seq, sc->sc_irs + 1 + sc->sc_win)) {
(void) syn_cache_respond(sc, m);
- splx(s);
return ((struct socket *)(-1));
}
/* Remove this cache entry */
syn_cache_rm(sc);
- splx(s);
/*
* Ok, create the full blown connection, and set things up
@@ -3898,19 +3904,16 @@ syn_cache_reset(struct sockaddr *src, st
{
struct syn_cache *sc;
struct syn_cache_head *scp;
- int s = splsoftnet();
- if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL) {
- splx(s);
+ splsoftassert(IPL_SOFTNET);
+
+ if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL)
return;
- }
if (SEQ_LT(th->th_seq, sc->sc_irs) ||
SEQ_GT(th->th_seq, sc->sc_irs+1)) {
- splx(s);
return;
}
syn_cache_rm(sc);
- splx(s);
tcpstat.tcps_sc_reset++;
syn_cache_put(sc);
}
@@ -3921,16 +3924,14 @@ syn_cache_unreach(struct sockaddr *src,
{
struct syn_cache *sc;
struct syn_cache_head *scp;
- int s;
- s = splsoftnet();
+ splsoftassert(IPL_SOFTNET);
+
if ((sc = syn_cache_lookup(src, dst, , rtableid)) == NULL) {
- splx(s);
return;
}
/* If the sequence number != sc_iss, then it's a bogus ICMP msg */
if (ntohl (th->th_seq) != sc->sc_iss) {
- splx(s);
return;
}
@@ -3944,12 +3945,10 @@ syn_cache_unreach(s
pf_ioctl.c: recursive splsoftnet()
Kill a bunch of recursive splsoftnet()s. pf_ioctl() already call
splsoftnet() and for the functions that are also called from pf_test()
the spl level is already IPL_SOFTNET.
ok?
Index: net/pf_if.c
===
RCS file: /cvs/src/sys/net/pf_if.c,v
retrieving revision 1.86
diff -u -p -r1.86 pf_if.c
--- net/pf_if.c 27 Sep 2016 04:57:17 - 1.86
+++ net/pf_if.c 15 Nov 2016 14:27:47 -
@@ -356,7 +356,7 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
struct pfi_dynaddr *dyn;
char tblname[PF_TABLE_NAME_SIZE];
struct pf_ruleset *ruleset = NULL;
- int s, rv = 0;
+ int rv = 0;
if (aw->type != PF_ADDR_DYNIFTL)
return (0);
@@ -364,7 +364,6 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
== NULL)
return (1);
- s = splsoftnet();
if (!strcmp(aw->v.ifname, "self"))
dyn->pfid_kif = pfi_kif_get(IFG_ALL);
else
@@ -407,7 +406,6 @@ pfi_dynaddr_setup(struct pf_addr_wrap *a
TAILQ_INSERT_TAIL(>pfid_kif->pfik_dynaddrs, dyn, entry);
aw->p.dyn = dyn;
pfi_kif_update(dyn->pfid_kif);
- splx(s);
return (0);
_bad:
@@ -418,7 +416,6 @@ _bad:
if (dyn->pfid_kif != NULL)
pfi_kif_unref(dyn->pfid_kif, PFI_KIF_REF_RULE);
pool_put(_addr_pl, dyn);
- splx(s);
return (rv);
}
@@ -589,13 +586,10 @@ pfi_address_add(struct sockaddr *sa, sa_
void
pfi_dynaddr_remove(struct pf_addr_wrap *aw)
{
- int s;
-
if (aw->type != PF_ADDR_DYNIFTL || aw->p.dyn == NULL ||
aw->p.dyn->pfid_kif == NULL || aw->p.dyn->pfid_kt == NULL)
return;
- s = splsoftnet();
TAILQ_REMOVE(>p.dyn->pfid_kif->pfik_dynaddrs, aw->p.dyn, entry);
pfi_kif_unref(aw->p.dyn->pfid_kif, PFI_KIF_REF_RULE);
aw->p.dyn->pfid_kif = NULL;
@@ -603,7 +597,6 @@ pfi_dynaddr_remove(struct pf_addr_wrap *
aw->p.dyn->pfid_kt = NULL;
pool_put(_addr_pl, aw->p.dyn);
aw->p.dyn = NULL;
- splx(s);
}
void
@@ -640,23 +633,20 @@ pfi_update_status(const char *name, stru
struct pfi_kif_cmp key;
struct ifg_memberp_member, *ifgm;
TAILQ_HEAD(, ifg_member) ifg_members;
- int i, j, k, s;
+ int i, j, k;
- s = splsoftnet();
if (*name == '\0' && pfs == NULL) {
RB_FOREACH(p, pfi_ifhead, _ifs) {
bzero(p->pfik_packets, sizeof(p->pfik_packets));
bzero(p->pfik_bytes, sizeof(p->pfik_bytes));
p->pfik_tzero = time_second;
}
- splx(s);
return;
}
strlcpy(key.pfik_name, name, sizeof(key.pfik_name));
p = RB_FIND(pfi_ifhead, _ifs, (struct pfi_kif *));
if (p == NULL) {
- splx(s);
return;
}
if (p->pfik_group != NULL) {
@@ -694,16 +684,14 @@ pfi_update_status(const char *name, stru
p->pfik_bytes[i][j][k];
}
}
- splx(s);
}
int
pfi_get_ifaces(const char *name, struct pfi_kif *buf, int *size)
{
struct pfi_kif *p, *nextp;
- int s, n = 0;
+ int n = 0;
- s = splsoftnet();
for (p = RB_MIN(pfi_ifhead, _ifs); p; p = nextp) {
nextp = RB_NEXT(pfi_ifhead, _ifs, p);
if (pfi_skip_if(name, p))
@@ -714,14 +702,12 @@ pfi_get_ifaces(const char *name, struct
pfi_kif_ref(p, PFI_KIF_REF_RULE);
if (copyout(p, buf++, sizeof(*buf))) {
pfi_kif_unref(p, PFI_KIF_REF_RULE);
- splx(s);
return (EFAULT);
}
nextp = RB_NEXT(pfi_ifhead, _ifs, p);
pfi_kif_unref(p, PFI_KIF_REF_RULE);
}
}
- splx(s);
*size = n;
return (0);
}
@@ -752,15 +738,12 @@ int
pfi_set_flags(const char *name, int flags)
{
struct pfi_kif *p;
- int s;
- s = splsoftnet();
RB_FOREACH(p, pfi_ifhead, _ifs) {
if (pfi_skip_if(name, p))
continue;
p->pfik_flags_new = p->pfik_flags | flags;
}
- splx(s);
return (0);
}
@@ -768,15 +751,12 @@ int
pfi_clear_flags(const char *name, int flags)
{
struct pfi_kif *p;
- int s;
- s = splsoftnet();
RB_FOREACH(p, pfi_ifhead, _ifs) {
if (pfi_skip_if(na
Recursive splsoftnet() in PMTU
Timers configured with rt_timer_add(9) are always run under splsoftnet()
so no need to take it recursively.
ok?
Index: netinet/ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.152
diff -u -p -r1.152 ip_icmp.c
--- netinet/ip_icmp.c 22 Aug 2016 15:37:23 - 1.152
+++ netinet/ip_icmp.c 7 Nov 2016 09:23:47 -
@@ -1046,7 +1046,8 @@ void
icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
struct ifnet *ifp;
- int s;
+
+ splsoftassert(IPL_SOFTNET);
ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
@@ -1058,7 +1059,6 @@ icmp_mtudisc_timeout(struct rtentry *rt,
sin = *satosin(rt_key(rt));
- s = splsoftnet();
rtdeletemsg(rt, ifp, r->rtt_tableid);
/* Notify TCP layer of increased Path MTU estimate */
@@ -1066,7 +1066,6 @@ icmp_mtudisc_timeout(struct rtentry *rt,
if (ctlfunc)
(*ctlfunc)(PRC_MTUINC, sintosa(),
r->rtt_tableid, NULL);
- splx(s);
} else {
if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
rt->rt_rmx.rmx_mtu = 0;
@@ -1097,16 +1096,15 @@ void
icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
struct ifnet *ifp;
- int s;
+
+ splsoftassert(IPL_SOFTNET);
ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
return;
if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
- s = splsoftnet();
rtdeletemsg(rt, ifp, r->rtt_tableid);
- splx(s);
}
if_put(ifp);
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.190
diff -u -p -r1.190 icmp6.c
--- netinet6/icmp6.c24 Aug 2016 09:38:29 - 1.190
+++ netinet6/icmp6.c7 Nov 2016 09:24:37 -
@@ -1947,16 +1947,15 @@ void
icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
struct ifnet *ifp;
- int s;
+
+ splsoftassert(IPL_SOFTNET);
ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
return;
if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
- s = splsoftnet();
rtdeletemsg(rt, ifp, r->rtt_tableid);
- splx(s);
} else {
if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
rt->rt_rmx.rmx_mtu = 0;
@@ -1969,16 +1968,15 @@ void
icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
struct ifnet *ifp;
- int s;
+
+ splsoftassert(IPL_SOFTNET);
ifp = if_get(rt->rt_ifidx);
if (ifp == NULL)
return;
if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
- s = splsoftnet();
rtdeletemsg(rt, ifp, r->rtt_tableid);
- splx(s);
}
if_put(ifp);
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.333
diff -u -p -r1.333 route.c
--- net/route.c 6 Oct 2016 19:09:08 - 1.333
+++ net/route.c 7 Nov 2016 09:22:11 -
@@ -1498,6 +1498,8 @@ rt_timer_queue_destroy(struct rttimer_qu
{
struct rttimer *r;
+ splsoftassert(IPL_SOFTNET);
+
while ((r = TAILQ_FIRST(>rtq_head)) != NULL) {
LIST_REMOVE(r, rtt_link);
TAILQ_REMOVE(>rtq_head, r, rtt_next);
