OSPFD

[S.Gopinath SundaraRajan]

Hi,

I just started using OpenBSD's ospfd.

1. I would like to have a direct Ethernet link
between OpenBSD box and Cisco/Juniper router.

I would like to specify the link type as point to point.

Which configuration directive I have to use ?.

2. I would like to use SHA1/SHA2 Message digest
authentication. Whether ospfd supports this ?.


3. I would like to crypt the authentication keys in the
ospfd.conf file instead of plain text keys. Is it possible ?.


Thanks,
S.Gopinatyh

remove splsoftnet from *_clone_destroy

[David Hill]

Hello -

I believe this is correct.

After mikeb@ committed destroying cloneable interfaces under
splsoftnet, these aren't needed anymore.

Index: if_mpe.c
===
RCS file: /cvs/src/sys/net/if_mpe.c,v
retrieving revision 1.54
diff -u -p -r1.54 if_mpe.c
--- if_mpe.c13 Apr 2016 11:41:15 -  1.54
+++ if_mpe.c21 Sep 2016 01:30:27 -
@@ -117,15 +117,12 @@ int
 mpe_clone_destroy(struct ifnet *ifp)
 {
struct mpe_softc*mpeif = ifp->if_softc;
-   int s;
 
LIST_REMOVE(mpeif, sc_list);
 
if (mpeif->sc_smpls.smpls_label) {
-   s = splsoftnet();
rt_ifa_del(>sc_ifa, RTF_MPLS,
smplstosa(>sc_smpls));
-   splx(s);
}
 
if_detach(ifp);
Index: if_mpw.c
===
RCS file: /cvs/src/sys/net/if_mpw.c,v
retrieving revision 1.14
diff -u -p -r1.14 if_mpw.c
--- if_mpw.c13 Apr 2016 11:41:15 -  1.14
+++ if_mpw.c21 Sep 2016 01:30:27 -
@@ -120,15 +120,12 @@ int
 mpw_clone_destroy(struct ifnet *ifp)
 {
struct mpw_softc *sc = ifp->if_softc;
-   int s;
 
ifp->if_flags &= ~IFF_RUNNING;
 
if (sc->sc_smpls.smpls_label) {
-   s = splsoftnet();
rt_ifa_del(>sc_ifa, RTF_MPLS,
smplstosa(>sc_smpls));
-   splx(s);
}
 
if_ih_remove(ifp, mpw_input, NULL);
Index: if_pfsync.c
===
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.231
diff -u -p -r1.231 if_pfsync.c
--- if_pfsync.c 15 Sep 2016 02:00:18 -  1.231
+++ if_pfsync.c 21 Sep 2016 01:30:27 -
@@ -353,9 +353,7 @@ pfsync_clone_destroy(struct ifnet *ifp)
 {
struct pfsync_softc *sc = ifp->if_softc;
struct pfsync_deferral *pd;
-   int s;
 
-   s = splsoftnet();
timeout_del(>sc_bulkfail_tmo);
timeout_del(>sc_bulk_tmo);
timeout_del(>sc_tmo);
@@ -384,7 +382,6 @@ pfsync_clone_destroy(struct ifnet *ifp)
free(sc, M_DEVBUF, sizeof(*sc));
 
pfsyncif = NULL;
-   splx(s);
 
return (0);
 }

Re: work-in-progress iwm performance fix

[Stefan Sperling]

On Tue, Sep 20, 2016 at 02:35:18AM +0200, Stefan Sperling wrote:
> I'd like to know if this diff helps with iwm(4) performance issues
> some people have been reporting.
> 
> This is not done yet and some details don't really make sense, especially
> the #if 0 hiding of what should be correct code -- yet, enabling that code
> makes the problem come back.
> 
> But hopefully this is generally going in the right direction.
> 
> Thank you, Edward Wandasiewicz, for pointing out that reverting if_iwm.c
> r1.85 restores performance. That was the door to this rabbit hole :-)

Here's a new version. Requires bleeding edge -current and also this
uncommitted patch I posted earlier today:
http://marc.info/?l=openbsd-tech=147440006608035=raw

It already seems to behave better. But the problem seems to be
load dependent and I can't trigger it very reliably.

Thanks to all who provided feedback for the previous version.

Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.132
diff -u -p -r1.132 if_iwm.c
--- if_iwm.c12 Sep 2016 10:18:26 -  1.132
+++ if_iwm.c20 Sep 2016 21:52:57 -
@@ -301,6 +301,7 @@ voidiwm_init_channel_map(struct iwm_sof
 void   iwm_setup_ht_rates(struct iwm_softc *);
 void   iwm_htprot_task(void *);
 void   iwm_update_htprot(struct ieee80211com *, struct ieee80211_node *);
+void   iwm_updateedca(struct ieee80211com *);
 intiwm_ampdu_rx_start(struct ieee80211com *, struct ieee80211_node *,
uint8_t);
 void   iwm_ampdu_rx_stop(struct ieee80211com *, struct ieee80211_node *,
@@ -402,10 +403,10 @@ int   iwm_config_umac_scan(struct iwm_soft
 intiwm_umac_scan(struct iwm_softc *);
 void   iwm_ack_rates(struct iwm_softc *, struct iwm_node *, int *, int *);
 void   iwm_mac_ctxt_cmd_common(struct iwm_softc *, struct iwm_node *,
-   struct iwm_mac_ctx_cmd *, uint32_t);
+   struct iwm_mac_ctx_cmd *, uint32_t, int);
 void   iwm_mac_ctxt_cmd_fill_sta(struct iwm_softc *, struct iwm_node *,
struct iwm_mac_data_sta *, int);
-intiwm_mac_ctxt_cmd(struct iwm_softc *, struct iwm_node *, uint32_t);
+intiwm_mac_ctxt_cmd(struct iwm_softc *, struct iwm_node *, uint32_t, int);
 intiwm_update_quotas(struct iwm_softc *, struct iwm_node *);
 intiwm_auth(struct iwm_softc *);
 intiwm_assoc(struct iwm_softc *);
@@ -2379,13 +2380,16 @@ void
 iwm_sta_rx_agg(struct iwm_softc *sc, struct ieee80211_node *ni, uint8_t tid,
 uint16_t ssn, int start)
 {
+   struct ieee80211com *ic = >sc_ic;
struct iwm_add_sta_cmd_v7 cmd;
struct iwm_node *in = (void *)ni;
int err, s;
uint32_t status;
 
-   if (start && sc->sc_rx_ba_sessions >= IWM_MAX_RX_BA_SESSIONS)
+   if (start && sc->sc_rx_ba_sessions >= IWM_MAX_RX_BA_SESSIONS) {
+   ieee80211_addba_req_refuse(ic, ni, tid);
return;
+   }
 
memset(, 0, sizeof(cmd));
 
@@ -2406,17 +2410,18 @@ iwm_sta_rx_agg(struct iwm_softc *sc, str
status = IWM_ADD_STA_SUCCESS;
err = iwm_send_cmd_pdu_status(sc, IWM_ADD_STA, sizeof(cmd), ,
);
-   if (err)
-   return;
 
-   if (status == IWM_ADD_STA_SUCCESS) {
-   s = splnet();
-   if (start)
+   s = splnet();
+   if (err == 0 && status == IWM_ADD_STA_SUCCESS) {
+   if (start) {
sc->sc_rx_ba_sessions++;
-   else if (sc->sc_rx_ba_sessions > 0)
+   ieee80211_addba_req_accept(ic, ni, tid);
+   } else if (sc->sc_rx_ba_sessions > 0)
sc->sc_rx_ba_sessions--;
-   splx(s);
-   }
+   } else if (start)
+   ieee80211_addba_req_refuse(ic, ni, tid);
+
+   splx(s);
 }
 
 void
@@ -2428,13 +2433,29 @@ iwm_htprot_task(void *arg)
int err;
 
/* This call updates HT protection based on in->in_ni.ni_htop1. */
-   err = iwm_mac_ctxt_cmd(sc, in, IWM_FW_CTXT_ACTION_MODIFY);
+   err = iwm_mac_ctxt_cmd(sc, in, IWM_FW_CTXT_ACTION_MODIFY, 1);
if (err)
printf("%s: could not change HT protection: error %d\n",
DEVNAME(sc), err);
 }
 
 /*
+ * This function is called by upper layer when EDCA settings in
+ * beacons have changed.
+ */
+void
+iwm_updateedca(struct ieee80211com *ic)
+{
+   struct iwm_softc *sc = ic->ic_softc;
+
+   if (ic->ic_state != IEEE80211_S_RUN)
+   return;
+
+   /* XXX htprot_task is also used here and should be renamed */
+   task_add(systq, >htprot_task);
+}
+
+/*
  * This function is called by upper layer when HT protection settings in
  * beacons have changed.
  */
@@ -2479,7 +2500,7 @@ iwm_ampdu_rx_start(struct ieee80211com *
sc->ba_ssn = htole16(ba->ba_winstart);
task_add(systq, >ba_task);
 
-   return 0; /* XXX firmware may still fail to add BA agreement... */
+   return 

ownership fix for /usr/lib/locate/src.db

[Martin Natano]

Without this diff src.db is installed as the build user when makeing a
noperm release.  is required for BINOWN/BINGRP. Ok?

natano


Index: Makefile
===
RCS file: /cvs/src/distrib/sets/Makefile,v
retrieving revision 1.1
diff -u -p -r1.1 Makefile
--- Makefile9 Jul 2014 19:23:28 -   1.1
+++ Makefile20 Sep 2016 19:55:10 -
@@ -4,5 +4,8 @@ DB = /usr/lib/locate/src.db
 
 makedb:
/bin/sh ${.CURDIR}/makelocatedb ${OSrev} >${DESTDIR}${DB}
+   chown ${BINOWN}:${BINGRP} ${DESTDIR}${DB}
 
 .PHONY: makedb
+
+.include 

net80211: allow ADDBA confirmation by driver

[Stefan Sperling]

When processing an ADDBA request, iwm(4) runs a task which sends a
command to the firmware and waits for confirmation. This command can
fail and there's currently no way we can recover from such an error.

With the diff below, drivers can return EBUSY from their ic_ampdu_rx_start()
handler which tells the stack not to send a confirmation just yet. The stack
provides functions which the driver can call to accept or refuse the request.

There is no functional change yet. This just shuffles code around so
drivers may insert themselves into the process.

ok?

Index: ieee80211_input.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.179
diff -u -p -r1.179 ieee80211_input.c
--- ieee80211_input.c   20 Sep 2016 13:24:42 -  1.179
+++ ieee80211_input.c   20 Sep 2016 19:23:47 -
@@ -2424,8 +2424,9 @@ ieee80211_recv_addba_req(struct ieee8021
const struct ieee80211_frame *wh;
const u_int8_t *frm;
struct ieee80211_rx_ba *ba;
-   u_int16_t params, ssn, bufsz, timeout, status;
+   u_int16_t params, ssn, bufsz, timeout;
u_int8_t token, tid;
+   int err;
 
if (!(ni->ni_flags & IEEE80211_NODE_HT)) {
DPRINTF(("received ADDBA req from non-HT STA %s\n",
@@ -2467,32 +2468,33 @@ ieee80211_recv_addba_req(struct ieee8021
ieee80211_ba_move_window(ic, ni, tid, ssn);
return;
}
+
+   /* If the driver does not support A-MPDU, refuse the request. */
+   if (ic->ic_ampdu_rx_start == NULL)
+   goto refuse;
+
/* if PBAC required but RA does not support it, refuse request */
if ((ic->ic_flags & IEEE80211_F_PBAR) &&
(!(ni->ni_flags & IEEE80211_NODE_MFP) ||
-!(ni->ni_rsncaps & IEEE80211_RSNCAP_PBAC))) {
-   status = IEEE80211_STATUS_REFUSED;
-   goto resp;
-   }
+!(ni->ni_rsncaps & IEEE80211_RSNCAP_PBAC)))
+   goto refuse;
/*
 * If the TID for which the Block Ack agreement is requested is
 * configured with a no-ACK policy, refuse the agreement.
 */
-   if (ic->ic_tid_noack & (1 << tid)) {
-   status = IEEE80211_STATUS_REFUSED;
-   goto resp;
-   }
+   if (ic->ic_tid_noack & (1 << tid))
+   goto refuse;
+
/* check that we support the requested Block Ack Policy */
if (!(ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) &&
-   !(params & IEEE80211_ADDBA_BA_POLICY)) {
-   status = IEEE80211_STATUS_INVALID_PARAM;
-   goto resp;
-   }
+   !(params & IEEE80211_ADDBA_BA_POLICY))
+   goto refuse;
 
/* setup Block Ack agreement */
ba->ba_state = IEEE80211_BA_INIT;
ba->ba_timeout_val = timeout * IEEE80211_DUR_TU;
ba->ba_ni = ni;
+   ba->ba_token = token;
timeout_set(>ba_to, ieee80211_rx_ba_timeout, ba);
timeout_set(>ba_gap_to, ieee80211_input_ba_gap_timeout, ba);
ba->ba_winsize = bufsz;
@@ -2506,31 +2508,61 @@ ieee80211_recv_addba_req(struct ieee8021
/* allocate and setup our reordering buffer */
ba->ba_buf = malloc(IEEE80211_BA_MAX_WINSZ * sizeof(*ba->ba_buf),
M_DEVBUF, M_NOWAIT | M_ZERO);
-   if (ba->ba_buf == NULL) {
-   status = IEEE80211_STATUS_REFUSED;
-   goto resp;
-   }
+   if (ba->ba_buf == NULL)
+   goto refuse;
+
ba->ba_head = 0;
 
/* notify drivers of this new Block Ack agreement */
-   if (ic->ic_ampdu_rx_start == NULL ||
-   ic->ic_ampdu_rx_start(ic, ni, tid) != 0) {
+   err = ic->ic_ampdu_rx_start(ic, ni, tid);
+   if (err == EBUSY) {
+   /* driver will accept or refuse agreement when done */
+   return;
+   } else if (err) {
/* driver failed to setup, rollback */
-   free(ba->ba_buf, M_DEVBUF, 0);
-   ba->ba_buf = NULL;
-   status = IEEE80211_STATUS_REFUSED;
-   goto resp;
-   }
+   ieee80211_addba_req_refuse(ic, ni, tid);
+   } else
+   ieee80211_addba_req_accept(ic, ni, tid);
+   return;
+
+ refuse:
+   /* MLME-ADDBA.response */
+   IEEE80211_SEND_ACTION(ic, ni, IEEE80211_CATEG_BA,
+   IEEE80211_ACTION_ADDBA_RESP,
+   IEEE80211_STATUS_REFUSED << 16 | token << 8 | tid);
+}
+
+void
+ieee80211_addba_req_accept(struct ieee80211com *ic, struct ieee80211_node *ni,
+uint8_t tid)
+{
+   struct ieee80211_rx_ba *ba = >ni_rx_ba[tid];
+
ba->ba_state = IEEE80211_BA_AGREED;
ic->ic_stats.is_ht_rx_ba_agreements++;
/* start Block Ack inactivity timer */
if (ba->ba_timeout_val != 0)
timeout_add_usec(>ba_to, ba->ba_timeout_val);
-   status = IEEE80211_STATUS_SUCCESS;
- resp:
+
+   /* MLME-ADDBA.response 

Re: share/: install ownership fixes

[Martin Natano]

Ping. Ok?


On Sat, Sep 10, 2016 at 10:16:20PM +0200, Martin Natano wrote:
> Another diff I typoed, also found by rpe@. Ok?
> 
> Index: share/misc/pcvtfonts/Makefile
> ===
> RCS file: /cvs/src/share/misc/pcvtfonts/Makefile,v
> retrieving revision 1.6
> diff -u -p -r1.6 Makefile
> --- share/misc/pcvtfonts/Makefile 13 May 2002 15:27:58 -  1.6
> +++ share/misc/pcvtfonts/Makefile 8 Sep 2016 20:54:08 -
> @@ -16,12 +16,9 @@ FONTDIR =  ${BINDIR}/misc/pcvtfonts
>  all: $(FONTS)
>  
>  install: ${FONTS}
> - @if [ ! -d ${DESTDIR}${FONTDIR} ]; then mkdir ${DESTDIR}${FONTDIR};fi
> - @for i in ${FONTS}; do \
> - echo "installing font $$i into ${DESTDIR}${FONTDIR}"; \
> - install -c -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
> - $$i ${DESTDIR}${FONTDIR}; \
> - done
> + ${INSTALL} -d -o root -g wheel ${DESTDIR}${FONTDIR}
> + ${INSTALL} ${INSTALL_COPY} -m ${LIBMODE} -o ${LIBOWN} -g ${LIBGRP} \
> + ${FONTS} ${DESTDIR}${FONTDIR}
>  
>  clean:
>   rm -f ${CLEANFILES}
> Index: share/snmp/Makefile
> ===
> RCS file: /cvs/src/share/snmp/Makefile,v
> retrieving revision 1.4
> diff -u -p -r1.4 Makefile
> --- share/snmp/Makefile   29 Jan 2016 03:06:00 -  1.4
> +++ share/snmp/Makefile   8 Sep 2016 21:02:02 -
> @@ -8,6 +8,7 @@ FILES+= OPENBSD-RELAYD-MIB.txt
>  all clean cleandir depend lint obj tags: _SUBDIRUSE
>  
>  realinstall:
> - ${INSTALL} -c -m 0444 ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
> + ${INSTALL} ${INSTALL_COPY} -o root -g wheel -m 0444 \
> + ${FILES} ${DESTDIR}${BINDIR}/snmp/mibs
>  
>  .include 
> Index: share/termtypes/Makefile
> ===
> RCS file: /cvs/src/share/termtypes/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- share/termtypes/Makefile  3 Dec 2015 11:30:46 -   1.24
> +++ share/termtypes/Makefile  10 Sep 2016 20:11:58 -
> @@ -14,12 +14,14 @@ termcap: termtypes.master
>   @[ -s ${.TARGET} ] || exit 1
>  
>  realinstall:
> + ${INSTALL} -d -o root -g wheel ${DESTDIR}${BINDIR}/terminfo
>   find terminfo -type f -exec \
>${INSTALL} -D ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 \
>{} ${DESTDIR}${BINDIR}/{} \;
>   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 termcap \
>${DESTDIR}${BINDIR}/misc/termcap
>   ln -fs ${BINDIR}/misc/termcap ${DESTDIR}/etc/termcap
> + chown -h root:wheel ${DESTDIR}/etc/termcap
>  
>  clean:
>   rm -f termcap

Re: dl_unwind_find_exidx prototype

[Philip Guenther]

On Tue, Sep 20, 2016 at 11:08 AM, Mark Kettenis  wrote:
> So my diff that switched libunwind to use dl_unwind_find_exidx() was
> incomplete.  I forgot that I had put its prototype in  and
> missed that file when generating the diff.  However,  isn't
> really the right place to put the prototype.  We have
> dl_iterate_phdr() in , and dl_unwind_find_exidx() is
> closely realted to that function.  In addition to that FreeBSD has it
> there as well.  Well actually, they have ..  So that's
> why I include  to get the prototype, which include
>  on OpenBSD and NetBSD and  on FreeBSD.
>
> ok?

I can believe that story.  ok guenther@

dl_unwind_find_exidx prototype

[Mark Kettenis]

So my diff that switched libunwind to use dl_unwind_find_exidx() was
incomplete.  I forgot that I had put its prototype in  and
missed that file when generating the diff.  However,  isn't
really the right place to put the prototype.  We have
dl_iterate_phdr() in , and dl_unwind_find_exidx() is
closely realted to that function.  In addition to that FreeBSD has it
there as well.  Well actually, they have ..  So that's
why I include  to get the prototype, which include
 on OpenBSD and NetBSD and  on FreeBSD.

ok?


Index: include/link_elf.h
===
RCS file: /cvs/src/include/link_elf.h,v
retrieving revision 1.6
diff -u -p -r1.6 link_elf.h
--- include/link_elf.h  6 Feb 2006 16:51:50 -   1.6
+++ include/link_elf.h  20 Sep 2016 17:59:37 -
@@ -36,6 +36,7 @@ struct dl_phdr_info {
 __BEGIN_DECLS
 intdl_iterate_phdr (int (*)(struct dl_phdr_info *, size_t, void *),
   void *);
+void   *dl_unwind_find_exidx(const void *, int *);
 __END_DECLS
 
 #endif /* !_LINK_ELF_H */
Index: lib/libunwind/src/AddressSpace.hpp
===
RCS file: /cvs/src/lib/libunwind/src/AddressSpace.hpp,v
retrieving revision 1.4
diff -u -p -r1.4 AddressSpace.hpp
--- lib/libunwind/src/AddressSpace.hpp  18 Sep 2016 11:18:28 -  1.4
+++ lib/libunwind/src/AddressSpace.hpp  20 Sep 2016 17:59:37 -
@@ -37,6 +37,7 @@ namespace libunwind {
 #if _LIBUNWIND_ARM_EHABI
 #if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
 
+#include 
 typedef void *_Unwind_Ptr;
 
 #elif defined(__linux__)

Re: bluhm's splsoftassert

[Mike Belopuhov]

On Tue, Sep 20, 2016 at 17:20 +0200, Martin Pieuchot wrote:
> On 20/09/16(Tue) 17:04, Mike Belopuhov wrote:
> > On Tue, Sep 20, 2016 at 10:51 -0400, David Hill wrote:
> > > On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > > > More...
> > > > 
> > > > splassert: sorwakeup: want 5 have 0
> > > > Starting stack trace...
> > > > splassert_check() at splassert_check+0x78
> > > > sorwakeup() at sorwakeup+0x27
> > > > route_input() at route_input+0x284
> > > > pflog_clone_create() at pflog_clone_create+0xa4
> > > > if_clone_create() at if_clone_create+0x7f
> > > > ifioctl() at ifioctl+0x35a
> > > > sys_ioctl() at sys_ioctl+0x196
> > > > syscall() at syscall+0x27b
> > > > --- syscall (number 54) ---
> > > > end of kernel
> > > > end trace frame: 0x7f7c7930, count: 249
> > > > 0x1aeaedf1af1a:
> > > > End of stack trace.
> > > > 
> > > >
> > > 
> > > Another similar...
> > > 
> > > Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> > > Sep 20 10:33:25 olive /bsd: Starting stack trace...
> > > Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> > > Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> > > Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> > > Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> > > vether_clone_create+0xd9
> > > Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> > > Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> > > Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> > > Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> > > Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> > > Sep 20 10:33:25 olive /bsd: end of kernel
> > > Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> > > Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> > > Sep 20 10:33:25 olive /bsd: End of stack trace.
> > >  
> > > 
> > 
> > It's all the same.  I'm not certain what would be the best way to
> > go around this, but a cautious approach would be something like this:
> > just wrapping ifc_create, ifc_destroy in splsoftnet.
> 
> ok mpi@
> 

Heh, we can commit the version that has both features :)

Index: if.c
===
RCS file: /home/cvs/src/sys/net/if.c,v
retrieving revision 1.448
diff -u -p -r1.448 if.c
--- if.c13 Sep 2016 08:15:01 -  1.448
+++ if.c20 Sep 2016 15:52:51 -
@@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
 {
struct if_clone *ifc;
struct ifnet *ifp;
-   int unit, ret;
+   int unit, ret, s;
 
ifc = if_clone_lookup(name, );
if (ifc == NULL)
@@ -1050,8 +1050,11 @@ if_clone_create(const char *name, int rd
if (ifunit(name) != NULL)
return (EEXIST);
 
-   if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
-   (ifp = ifunit(name)) == NULL)
+   s = splsoftnet();
+   ret = (*ifc->ifc_create)(ifc, unit);
+   splx(s);
+
+   if (ret != 0 || (ifp = ifunit(name)) == NULL)
return (ret);
 
if_addgroup(ifp, ifc->ifc_name);
@@ -1069,7 +1072,7 @@ if_clone_destroy(const char *name)
 {
struct if_clone *ifc;
struct ifnet *ifp;
-   int s;
+   int error, s;
 
ifc = if_clone_lookup(name, NULL);
if (ifc == NULL)
@@ -1088,7 +1091,10 @@ if_clone_destroy(const char *name)
splx(s);
}
 
-   return ((*ifc->ifc_destroy)(ifp));
+   s = splsoftnet();
+   error = (*ifc->ifc_destroy)(ifp);
+   splx(s);
+   return (error);
 }
 
 /*

Re: bluhm's splsoftassert

[Mike Belopuhov]

On 20 September 2016 at 17:09, Martin Pieuchot  wrote:
> On 20/09/16(Tue) 10:51, David Hill wrote:
>> On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
>> > More...
>> >
>> > splassert: sorwakeup: want 5 have 0
>> > Starting stack trace...
>> > splassert_check() at splassert_check+0x78
>> > sorwakeup() at sorwakeup+0x27
>> > route_input() at route_input+0x284
>> > pflog_clone_create() at pflog_clone_create+0xa4
>> > if_clone_create() at if_clone_create+0x7f
>> > ifioctl() at ifioctl+0x35a
>> > sys_ioctl() at sys_ioctl+0x196
>> > syscall() at syscall+0x27b
>> > --- syscall (number 54) ---
>> > end of kernel
>> > end trace frame: 0x7f7c7930, count: 249
>> > 0x1aeaedf1af1a:
>> > End of stack trace.
>> >
>> >
>>
>> Another similar...
>>
>> Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
>> Sep 20 10:33:25 olive /bsd: Starting stack trace...
>> Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
>> Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
>> Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
>> Sep 20 10:33:25 olive /bsd: vether_clone_create() at
>> vether_clone_create+0xd9
>> Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
>> Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
>> Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
>> Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
>> Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
>> Sep 20 10:33:25 olive /bsd: end of kernel
>> Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
>> Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
>> Sep 20 10:33:25 olive /bsd: End of stack trace.
>
> Diff below should fix that.  I'd rather keep the splsoftnet() close
> to ifioctl() because that's where the lock is going to be taken.
>
> ok?
>

I'm fine with this too.  Don't you want to splsoftnet around ifc_destroy
for the symmetry?

Re: bluhm's splsoftassert

[Martin Pieuchot]

On 20/09/16(Tue) 17:04, Mike Belopuhov wrote:
> On Tue, Sep 20, 2016 at 10:51 -0400, David Hill wrote:
> > On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > > More...
> > > 
> > > splassert: sorwakeup: want 5 have 0
> > > Starting stack trace...
> > > splassert_check() at splassert_check+0x78
> > > sorwakeup() at sorwakeup+0x27
> > > route_input() at route_input+0x284
> > > pflog_clone_create() at pflog_clone_create+0xa4
> > > if_clone_create() at if_clone_create+0x7f
> > > ifioctl() at ifioctl+0x35a
> > > sys_ioctl() at sys_ioctl+0x196
> > > syscall() at syscall+0x27b
> > > --- syscall (number 54) ---
> > > end of kernel
> > > end trace frame: 0x7f7c7930, count: 249
> > > 0x1aeaedf1af1a:
> > > End of stack trace.
> > > 
> > >
> > 
> > Another similar...
> > 
> > Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> > Sep 20 10:33:25 olive /bsd: Starting stack trace...
> > Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> > Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> > Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> > Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> > vether_clone_create+0xd9
> > Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> > Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> > Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> > Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> > Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> > Sep 20 10:33:25 olive /bsd: end of kernel
> > Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> > Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> > Sep 20 10:33:25 olive /bsd: End of stack trace.
> >  
> > 
> 
> It's all the same.  I'm not certain what would be the best way to
> go around this, but a cautious approach would be something like this:
> just wrapping ifc_create, ifc_destroy in splsoftnet.

ok mpi@

> 
> David, can you give this a spin?
> 
> Index: sys/net/if.c
> ===
> RCS file: /home/cvs/src/sys/net/if.c,v
> retrieving revision 1.448
> diff -u -p -r1.448 if.c
> --- sys/net/if.c  13 Sep 2016 08:15:01 -  1.448
> +++ sys/net/if.c  20 Sep 2016 14:58:57 -
> @@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int unit, ret;
> + int unit, ret, s;
>  
>   ifc = if_clone_lookup(name, );
>   if (ifc == NULL)
> @@ -1050,9 +1050,13 @@ if_clone_create(const char *name, int rd
>   if (ifunit(name) != NULL)
>   return (EEXIST);
>  
> + s = splsoftnet();
>   if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
> - (ifp = ifunit(name)) == NULL)
> + (ifp = ifunit(name)) == NULL) {
> + splx(s);
>   return (ret);
> + }
> + splx(s);
>  
>   if_addgroup(ifp, ifc->ifc_name);
>   if (rdomain != 0)
> @@ -1069,7 +1073,7 @@ if_clone_destroy(const char *name)
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int s;
> + int error, s;
>  
>   ifc = if_clone_lookup(name, NULL);
>   if (ifc == NULL)
> @@ -1088,7 +1092,10 @@ if_clone_destroy(const char *name)
>   splx(s);
>   }
>  
> - return ((*ifc->ifc_destroy)(ifp));
> + s = splsoftnet();
> + error = (*ifc->ifc_destroy)(ifp);
> + splx(s);
> + return (error);
>  }
>  
>  /*
> 

Re: bluhm's splsoftassert

[David Hill]

On Tue, Sep 20, 2016 at 05:04:02PM +0200, Mike Belopuhov wrote:
> On Tue, Sep 20, 2016 at 10:51 -0400, David Hill wrote:
> > On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > > More...
> > > 
> > > splassert: sorwakeup: want 5 have 0
> > > Starting stack trace...
> > > splassert_check() at splassert_check+0x78
> > > sorwakeup() at sorwakeup+0x27
> > > route_input() at route_input+0x284
> > > pflog_clone_create() at pflog_clone_create+0xa4
> > > if_clone_create() at if_clone_create+0x7f
> > > ifioctl() at ifioctl+0x35a
> > > sys_ioctl() at sys_ioctl+0x196
> > > syscall() at syscall+0x27b
> > > --- syscall (number 54) ---
> > > end of kernel
> > > end trace frame: 0x7f7c7930, count: 249
> > > 0x1aeaedf1af1a:
> > > End of stack trace.
> > > 
> > >
> > 
> > Another similar...
> > 
> > Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> > Sep 20 10:33:25 olive /bsd: Starting stack trace...
> > Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> > Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> > Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> > Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> > vether_clone_create+0xd9
> > Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> > Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> > Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> > Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> > Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> > Sep 20 10:33:25 olive /bsd: end of kernel
> > Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> > Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> > Sep 20 10:33:25 olive /bsd: End of stack trace.
> >  
> > 
> 
> It's all the same.  I'm not certain what would be the best way to
> go around this, but a cautious approach would be something like this:
> just wrapping ifc_create, ifc_destroy in splsoftnet.
> 
> David, can you give this a spin?
> 
> Index: sys/net/if.c
> ===
> RCS file: /home/cvs/src/sys/net/if.c,v
> retrieving revision 1.448
> diff -u -p -r1.448 if.c
> --- sys/net/if.c  13 Sep 2016 08:15:01 -  1.448
> +++ sys/net/if.c  20 Sep 2016 14:58:57 -
> @@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int unit, ret;
> + int unit, ret, s;
>  
>   ifc = if_clone_lookup(name, );
>   if (ifc == NULL)
> @@ -1050,9 +1050,13 @@ if_clone_create(const char *name, int rd
>   if (ifunit(name) != NULL)
>   return (EEXIST);
>  
> + s = splsoftnet();
>   if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
> - (ifp = ifunit(name)) == NULL)
> + (ifp = ifunit(name)) == NULL) {
> + splx(s);
>   return (ret);
> + }
> + splx(s);
>  
>   if_addgroup(ifp, ifc->ifc_name);
>   if (rdomain != 0)
> @@ -1069,7 +1073,7 @@ if_clone_destroy(const char *name)
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int s;
> + int error, s;
>  
>   ifc = if_clone_lookup(name, NULL);
>   if (ifc == NULL)
> @@ -1088,7 +1092,10 @@ if_clone_destroy(const char *name)
>   splx(s);
>   }
>  
> - return ((*ifc->ifc_destroy)(ifp));
> + s = splsoftnet();
> + error = (*ifc->ifc_destroy)(ifp);
> + splx(s);
> + return (error);
>  }
>  
>  /*
>

Works.  No more assert and functions normally. 

Re: bluhm's splsoftassert

[Theo Buehler]

On Tue, Sep 20, 2016 at 05:09:41PM +0200, Martin Pieuchot wrote:
> On 20/09/16(Tue) 10:51, David Hill wrote:
> > On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > > More...
> > > 
> > > splassert: sorwakeup: want 5 have 0
> > > Starting stack trace...
> > > splassert_check() at splassert_check+0x78
> > > sorwakeup() at sorwakeup+0x27
> > > route_input() at route_input+0x284
> > > pflog_clone_create() at pflog_clone_create+0xa4
> > > if_clone_create() at if_clone_create+0x7f
> > > ifioctl() at ifioctl+0x35a
> > > sys_ioctl() at sys_ioctl+0x196
> > > syscall() at syscall+0x27b
> > > --- syscall (number 54) ---
> > > end of kernel
> > > end trace frame: 0x7f7c7930, count: 249
> > > 0x1aeaedf1af1a:
> > > End of stack trace.
> > > 
> > >
> > 
> > Another similar...
> > 
> > Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> > Sep 20 10:33:25 olive /bsd: Starting stack trace...
> > Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> > Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> > Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> > Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> > vether_clone_create+0xd9
> > Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> > Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> > Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> > Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> > Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> > Sep 20 10:33:25 olive /bsd: end of kernel
> > Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> > Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> > Sep 20 10:33:25 olive /bsd: End of stack trace.
> 
> Diff below should fix that.  I'd rather keep the splsoftnet() close
> to ifioctl() because that's where the lock is going to be taken.
> 

This one fixes that problem as well.

> ok?
> 
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.448
> diff -u -p -r1.448 if.c
> --- net/if.c  13 Sep 2016 08:15:01 -  1.448
> +++ net/if.c  20 Sep 2016 15:07:57 -
> @@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int unit, ret;
> + int unit, ret, s;
>  
>   ifc = if_clone_lookup(name, );
>   if (ifc == NULL)
> @@ -1050,8 +1050,11 @@ if_clone_create(const char *name, int rd
>   if (ifunit(name) != NULL)
>   return (EEXIST);
>  
> - if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
> - (ifp = ifunit(name)) == NULL)
> + s = splsoftnet();
> + ret = (*ifc->ifc_create)(ifc, unit);
> + splx(s);
> +
> + if (ret != 0 || (ifp = ifunit(name)) == NULL)
>   return (ret);
>  
>   if_addgroup(ifp, ifc->ifc_name);
> 

Re: bluhm's splsoftassert

[Theo Buehler]

On Tue, Sep 20, 2016 at 05:04:02PM +0200, Mike Belopuhov wrote:
> On Tue, Sep 20, 2016 at 10:51 -0400, David Hill wrote:
> > On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > > More...
> > > 
> > > splassert: sorwakeup: want 5 have 0
> > > Starting stack trace...
> > > splassert_check() at splassert_check+0x78
> > > sorwakeup() at sorwakeup+0x27
> > > route_input() at route_input+0x284
> > > pflog_clone_create() at pflog_clone_create+0xa4
> > > if_clone_create() at if_clone_create+0x7f
> > > ifioctl() at ifioctl+0x35a
> > > sys_ioctl() at sys_ioctl+0x196
> > > syscall() at syscall+0x27b
> > > --- syscall (number 54) ---
> > > end of kernel
> > > end trace frame: 0x7f7c7930, count: 249
> > > 0x1aeaedf1af1a:
> > > End of stack trace.
> > > 
> > >
> > 
> > Another similar...
> > 
> > Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> > Sep 20 10:33:25 olive /bsd: Starting stack trace...
> > Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> > Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> > Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> > Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> > vether_clone_create+0xd9
> > Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> > Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> > Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> > Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> > Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> > Sep 20 10:33:25 olive /bsd: end of kernel
> > Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> > Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> > Sep 20 10:33:25 olive /bsd: End of stack trace.
> >  
> > 
> 
> It's all the same.  I'm not certain what would be the best way to
> go around this, but a cautious approach would be something like this:
> just wrapping ifc_create, ifc_destroy in splsoftnet.
> 
> David, can you give this a spin?

This fixes the one with pflog_clone_create() on my box. I haven't seen
the vether one.

> 
> Index: sys/net/if.c
> ===
> RCS file: /home/cvs/src/sys/net/if.c,v
> retrieving revision 1.448
> diff -u -p -r1.448 if.c
> --- sys/net/if.c  13 Sep 2016 08:15:01 -  1.448
> +++ sys/net/if.c  20 Sep 2016 14:58:57 -
> @@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int unit, ret;
> + int unit, ret, s;
>  
>   ifc = if_clone_lookup(name, );
>   if (ifc == NULL)
> @@ -1050,9 +1050,13 @@ if_clone_create(const char *name, int rd
>   if (ifunit(name) != NULL)
>   return (EEXIST);
>  
> + s = splsoftnet();
>   if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
> - (ifp = ifunit(name)) == NULL)
> + (ifp = ifunit(name)) == NULL) {
> + splx(s);
>   return (ret);
> + }
> + splx(s);
>  
>   if_addgroup(ifp, ifc->ifc_name);
>   if (rdomain != 0)
> @@ -1069,7 +1073,7 @@ if_clone_destroy(const char *name)
>  {
>   struct if_clone *ifc;
>   struct ifnet *ifp;
> - int s;
> + int error, s;
>  
>   ifc = if_clone_lookup(name, NULL);
>   if (ifc == NULL)
> @@ -1088,7 +1092,10 @@ if_clone_destroy(const char *name)
>   splx(s);
>   }
>  
> - return ((*ifc->ifc_destroy)(ifp));
> + s = splsoftnet();
> + error = (*ifc->ifc_destroy)(ifp);
> + splx(s);
> + return (error);
>  }
>  
>  /*
> 

Re: bluhm's splsoftassert

[Martin Pieuchot]

On 20/09/16(Tue) 10:51, David Hill wrote:
> On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > More...
> > 
> > splassert: sorwakeup: want 5 have 0
> > Starting stack trace...
> > splassert_check() at splassert_check+0x78
> > sorwakeup() at sorwakeup+0x27
> > route_input() at route_input+0x284
> > pflog_clone_create() at pflog_clone_create+0xa4
> > if_clone_create() at if_clone_create+0x7f
> > ifioctl() at ifioctl+0x35a
> > sys_ioctl() at sys_ioctl+0x196
> > syscall() at syscall+0x27b
> > --- syscall (number 54) ---
> > end of kernel
> > end trace frame: 0x7f7c7930, count: 249
> > 0x1aeaedf1af1a:
> > End of stack trace.
> > 
> >
> 
> Another similar...
> 
> Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> Sep 20 10:33:25 olive /bsd: Starting stack trace...
> Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> vether_clone_create+0xd9
> Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> Sep 20 10:33:25 olive /bsd: end of kernel
> Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> Sep 20 10:33:25 olive /bsd: End of stack trace.

Diff below should fix that.  I'd rather keep the splsoftnet() close
to ifioctl() because that's where the lock is going to be taken.

ok?

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.448
diff -u -p -r1.448 if.c
--- net/if.c13 Sep 2016 08:15:01 -  1.448
+++ net/if.c20 Sep 2016 15:07:57 -
@@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
 {
struct if_clone *ifc;
struct ifnet *ifp;
-   int unit, ret;
+   int unit, ret, s;
 
ifc = if_clone_lookup(name, );
if (ifc == NULL)
@@ -1050,8 +1050,11 @@ if_clone_create(const char *name, int rd
if (ifunit(name) != NULL)
return (EEXIST);
 
-   if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
-   (ifp = ifunit(name)) == NULL)
+   s = splsoftnet();
+   ret = (*ifc->ifc_create)(ifc, unit);
+   splx(s);
+
+   if (ret != 0 || (ifp = ifunit(name)) == NULL)
return (ret);
 
if_addgroup(ifp, ifc->ifc_name);

Re: libc: remove pointless off_t, size_t, void* and char* casts

[Todd C. Miller]

On Mon, 19 Sep 2016 20:27:56 -0700, Philip Guenther wrote:

> I'm pretty sure I got all of the "no effect" casts to off_t and size_t.  
> I picked up the void* and char* casts with a less comprehensize search, so 
> there may be some that slipped through.
> 
> This diff was checked by comparing the .o files before and after, building 
> on both an LP64 arch (amd64) and an ILP32 arch (i386).  For a handful of 
> files, the joining of lines meant the debug info was altered; for those, 
> they still compared as unchanged when the 'before' and 'after' files were 
> stripped before comparing them.

Cast removal looks fine.  Did you intend to include this line?

DEF_STRONG(_malloc_init)

 - todd

Re: bluhm's splsoftassert

[Mike Belopuhov]

On Tue, Sep 20, 2016 at 10:51 -0400, David Hill wrote:
> On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> > More...
> > 
> > splassert: sorwakeup: want 5 have 0
> > Starting stack trace...
> > splassert_check() at splassert_check+0x78
> > sorwakeup() at sorwakeup+0x27
> > route_input() at route_input+0x284
> > pflog_clone_create() at pflog_clone_create+0xa4
> > if_clone_create() at if_clone_create+0x7f
> > ifioctl() at ifioctl+0x35a
> > sys_ioctl() at sys_ioctl+0x196
> > syscall() at syscall+0x27b
> > --- syscall (number 54) ---
> > end of kernel
> > end trace frame: 0x7f7c7930, count: 249
> > 0x1aeaedf1af1a:
> > End of stack trace.
> > 
> >
> 
> Another similar...
> 
> Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
> Sep 20 10:33:25 olive /bsd: Starting stack trace...
> Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
> Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
> Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
> Sep 20 10:33:25 olive /bsd: vether_clone_create() at
> vether_clone_create+0xd9
> Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
> Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
> Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
> Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
> Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
> Sep 20 10:33:25 olive /bsd: end of kernel
> Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
> Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
> Sep 20 10:33:25 olive /bsd: End of stack trace.
>  
> 

It's all the same.  I'm not certain what would be the best way to
go around this, but a cautious approach would be something like this:
just wrapping ifc_create, ifc_destroy in splsoftnet.

David, can you give this a spin?

Index: sys/net/if.c
===
RCS file: /home/cvs/src/sys/net/if.c,v
retrieving revision 1.448
diff -u -p -r1.448 if.c
--- sys/net/if.c13 Sep 2016 08:15:01 -  1.448
+++ sys/net/if.c20 Sep 2016 14:58:57 -
@@ -1041,7 +1041,7 @@ if_clone_create(const char *name, int rd
 {
struct if_clone *ifc;
struct ifnet *ifp;
-   int unit, ret;
+   int unit, ret, s;
 
ifc = if_clone_lookup(name, );
if (ifc == NULL)
@@ -1050,9 +1050,13 @@ if_clone_create(const char *name, int rd
if (ifunit(name) != NULL)
return (EEXIST);
 
+   s = splsoftnet();
if ((ret = (*ifc->ifc_create)(ifc, unit)) != 0 ||
-   (ifp = ifunit(name)) == NULL)
+   (ifp = ifunit(name)) == NULL) {
+   splx(s);
return (ret);
+   }
+   splx(s);
 
if_addgroup(ifp, ifc->ifc_name);
if (rdomain != 0)
@@ -1069,7 +1073,7 @@ if_clone_destroy(const char *name)
 {
struct if_clone *ifc;
struct ifnet *ifp;
-   int s;
+   int error, s;
 
ifc = if_clone_lookup(name, NULL);
if (ifc == NULL)
@@ -1088,7 +1092,10 @@ if_clone_destroy(const char *name)
splx(s);
}
 
-   return ((*ifc->ifc_destroy)(ifp));
+   s = splsoftnet();
+   error = (*ifc->ifc_destroy)(ifp);
+   splx(s);
+   return (error);
 }
 
 /*

Re: bluhm's splsoftassert

[Mike Belopuhov]

On 20 September 2016 at 16:50, Martin Pieuchot  wrote:
> On 20/09/16(Tue) 16:37, Alexander Bluhm wrote:
>> On Tue, Sep 20, 2016 at 04:17:37PM +0200, Mike Belopuhov wrote:
>> > Can we assert that *_usrreq is always called under splsoftnet?
>>
>> I think that is the way to go.  Long time back the spl moved from
>> inet to socket.  We need it in the socket layer to fix various
>> races.
>
> *_ussreq() is a can of worm.  It is recursive and we'll have to split
> per operation if if we want to avoid releasing/grabbing the lock again.
>

This means it would be better to grab a lock before calling usrreq and
assert that it's taken there, no? In other words we can remove splsoftnet
from *_usrreq and assert IPL_SOFTNET now.

> That said, assert are good putting more splsoftnet() should be avoided.
>

s/assert are good putting/assert are good, but putting/  ?

Re: bluhm's splsoftassert

[David Hill]

On Tue, Sep 20, 2016 at 09:53:02AM -0400, David Hill wrote:
> More...
> 
> splassert: sorwakeup: want 5 have 0
> Starting stack trace...
> splassert_check() at splassert_check+0x78
> sorwakeup() at sorwakeup+0x27
> route_input() at route_input+0x284
> pflog_clone_create() at pflog_clone_create+0xa4
> if_clone_create() at if_clone_create+0x7f
> ifioctl() at ifioctl+0x35a
> sys_ioctl() at sys_ioctl+0x196
> syscall() at syscall+0x27b
> --- syscall (number 54) ---
> end of kernel
> end trace frame: 0x7f7c7930, count: 249
> 0x1aeaedf1af1a:
> End of stack trace.
> 
>

Another similar...

Sep 20 10:33:25 olive /bsd: splassert: sorwakeup: want 5 have 0
Sep 20 10:33:25 olive /bsd: Starting stack trace...
Sep 20 10:33:25 olive /bsd: splassert_check() at splassert_check+0x78
Sep 20 10:33:25 olive /bsd: sorwakeup() at sorwakeup+0x27
Sep 20 10:33:25 olive /bsd: route_input() at route_input+0x284
Sep 20 10:33:25 olive /bsd: vether_clone_create() at
vether_clone_create+0xd9
Sep 20 10:33:25 olive /bsd: if_clone_create() at if_clone_create+0x7f
Sep 20 10:33:25 olive /bsd: ifioctl() at ifioctl+0x35a
Sep 20 10:33:25 olive /bsd: sys_ioctl() at sys_ioctl+0x196
Sep 20 10:33:25 olive /bsd: syscall() at syscall+0x27b
Sep 20 10:33:25 olive /bsd: --- syscall (number 54) ---
Sep 20 10:33:25 olive /bsd: end of kernel
Sep 20 10:33:25 olive /bsd: end trace frame: 0x7f7d0df1, count: 249
Sep 20 10:33:25 olive /bsd: 0x119ca1c1af1a:
Sep 20 10:33:25 olive /bsd: End of stack trace.
 

Re: bluhm's splsoftassert

[Martin Pieuchot]

On 20/09/16(Tue) 16:37, Alexander Bluhm wrote:
> On Tue, Sep 20, 2016 at 04:17:37PM +0200, Mike Belopuhov wrote:
> > Can we assert that *_usrreq is always called under splsoftnet?
> 
> I think that is the way to go.  Long time back the spl moved from
> inet to socket.  We need it in the socket layer to fix various
> races.

*_ussreq() is a can of worm.  It is recursive and we'll have to split
per operation if if we want to avoid releasing/grabbing the lock again.

That said, assert are good putting more splsoftnet() should be avoided.

Re: bluhm's splsoftassert

[Alexander Bluhm]

On Tue, Sep 20, 2016 at 04:17:37PM +0200, Mike Belopuhov wrote:
> Can we assert that *_usrreq is always called under splsoftnet?

I think that is the way to go.  Long time back the spl moved from
inet to socket.  We need it in the socket layer to fix various
races.

> I recall fixing some of them for raw sockets and some such and
> was wondering if the spl has to be raised before we end up there.

To fix every protocol individually makes not much sense.  We need
the lock in the socket layer, otherwise all the assertions cannot
work.  It would be difficult if we run the socket code with more
or less locks depending on the socket type.

I would say move most the splsoftnet() into uipc_socket.c and replace
it with an propper rwlock/mutex/whatever later.

bluhm

Re: bluhm's splsoftassert

[Mike Belopuhov]

On 20 September 2016 at 15:55, Alexander Bluhm  wrote:
> On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
>> With bluhm's r1.160 uipc_socket.c.
>
> With splsoftnet() in soshutdown() I can fix this one.
>
> splassert: sowwakeup: want 5 have 0
> Starting stack trace...
> splassert_check() at splassert_check+0x78
> sowwakeup() at sowwakeup+0x27
> uipc_usrreq() at uipc_usrreq+0xfd
> sys_shutdown() at sys_shutdown+0x67
> syscall() at syscall+0x27b
> --- syscall (number 134) ---
> end of kernel
> end trace frame: 0xe8f2cba5e80, count: 252
> 0xe8f305dc16a:
> End of stack trace.
>
> ok?
>

OK mikeb

Can we assert that *_usrreq is always called under splsoftnet?
I recall fixing some of them for raw sockets and some such and
was wondering if the spl has to be raised before we end up there.

Re: bluhm's splsoftassert

[Alexander Bluhm]

On Tue, Sep 20, 2016 at 03:39:17PM +0200, Mike Belopuhov wrote:
> On Tue, Sep 20, 2016 at 09:19 -0400, David Hill wrote:
> > Another...
> > 
> > splassert: sorwakeup: want 5 have 4
> > Starting stack trace...
> > splassert_check() at splassert_check+0x78
> > sorwakeup() at sorwakeup+0x27
> > pfkey_sendup() at pfkey_sendup+0x99
> > pfkeyv2_sendmessage() at pfkeyv2_sendmessage+0x226
> > pfkeyv2_expire() at pfkeyv2_expire+0x18d
> > tdb_timeout() at tdb_timeout+0x2f
> > softclock() at softclock+0x144
> > softintr_dispatch() at softintr_dispatch+0x8b
> > Xsoftclock() at Xsoftclock+0x1f
> > --- interrupt ---
> > end of kernel
> > end trace frame: 0x20, count: 248
> > 0x8:
> > End of stack trace.
> > 
> 
> All TDB timeouts can do an splsoftnet.  OK?

OK bluhm@

> 
> Index: netinet/ip_ipsp.c
> ===
> RCS file: /home/cvs/src/sys/netinet/ip_ipsp.c,v
> retrieving revision 1.216
> diff -u -p -r1.216 ip_ipsp.c
> --- netinet/ip_ipsp.c 19 Sep 2016 18:09:22 -  1.216
> +++ netinet/ip_ipsp.c 20 Sep 2016 13:34:16 -
> @@ -504,55 +504,67 @@ void
>  tdb_timeout(void *v)
>  {
>   struct tdb *tdb = v;
> + int s;
>  
>   if (!(tdb->tdb_flags & TDBF_TIMER))
>   return;
>  
> + s = splsoftnet();
>   /* If it's an "invalid" TDB do a silent expiration. */
>   if (!(tdb->tdb_flags & TDBF_INVALID))
>   pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
>   tdb_delete(tdb);
> + splx(s);
>  }
>  
>  void
>  tdb_firstuse(void *v)
>  {
>   struct tdb *tdb = v;
> + int s;
>  
>   if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
>   return;
>  
> + s = splsoftnet();
>   /* If the TDB hasn't been used, don't renew it. */
>   if (tdb->tdb_first_use != 0)
>   pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
>   tdb_delete(tdb);
> + splx(s);
>  }
>  
>  void
>  tdb_soft_timeout(void *v)
>  {
>   struct tdb *tdb = v;
> + int s;
>  
>   if (!(tdb->tdb_flags & TDBF_SOFT_TIMER))
>   return;
>  
> + s = splsoftnet();
>   /* Soft expirations. */
>   pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
>   tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
> + splx(s);
>  }
>  
>  void
>  tdb_soft_firstuse(void *v)
>  {
>   struct tdb *tdb = v;
> + int s;
>  
>   if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
>   return;
>  
> + s = splsoftnet();
>   /* If the TDB hasn't been used, don't renew it. */
>   if (tdb->tdb_first_use != 0)
>   pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
>   tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
> + splx(s);
>  }
>  
>  /*

Re: bluhm's splsoftassert

[Alexander Bluhm]

On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
> With bluhm's r1.160 uipc_socket.c.

With splsoftnet() in soshutdown() I can fix this one.

splassert: sowwakeup: want 5 have 0
Starting stack trace...
splassert_check() at splassert_check+0x78
sowwakeup() at sowwakeup+0x27
uipc_usrreq() at uipc_usrreq+0xfd
sys_shutdown() at sys_shutdown+0x67
syscall() at syscall+0x27b
--- syscall (number 134) ---
end of kernel
end trace frame: 0xe8f2cba5e80, count: 252
0xe8f305dc16a:
End of stack trace.

ok?

bluhm

Index: kern/uipc_socket.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.160
diff -u -p -r1.160 uipc_socket.c
--- kern/uipc_socket.c  20 Sep 2016 11:11:44 -  1.160
+++ kern/uipc_socket.c  20 Sep 2016 13:43:19 -
@@ -1018,20 +1018,26 @@ int
 soshutdown(struct socket *so, int how)
 {
struct protosw *pr = so->so_proto;
+   int s, error = 0;
 
+   s = splsoftnet();
switch (how) {
case SHUT_RD:
case SHUT_RDWR:
sorflush(so);
if (how == SHUT_RD)
-   return (0);
+   break;
/* FALLTHROUGH */
case SHUT_WR:
-   return (*pr->pr_usrreq)(so, PRU_SHUTDOWN, NULL, NULL, NULL,
+   error = (*pr->pr_usrreq)(so, PRU_SHUTDOWN, NULL, NULL, NULL,
curproc);
+   break;
default:
-   return (EINVAL);
+   error = EINVAL;
+   break;
}
+   splx(s);
+   return (error);
 }
 
 void

Re: bluhm's splsoftassert

[David Hill]

More...

splassert: sorwakeup: want 5 have 0
Starting stack trace...
splassert_check() at splassert_check+0x78
sorwakeup() at sorwakeup+0x27
route_input() at route_input+0x284
pflog_clone_create() at pflog_clone_create+0xa4
if_clone_create() at if_clone_create+0x7f
ifioctl() at ifioctl+0x35a
sys_ioctl() at sys_ioctl+0x196
syscall() at syscall+0x27b
--- syscall (number 54) ---
end of kernel
end trace frame: 0x7f7c7930, count: 249
0x1aeaedf1af1a:
End of stack trace.

Re: bluhm's splsoftassert

[David Hill]

Another

splassert: sowwakeup: want 5 have 0
Starting stack trace...
splassert_check() at splassert_check+0x78
sowwakeup() at sowwakeup+0x27
uipc_usrreq() at uipc_usrreq+0xfd
sys_shutdown() at sys_shutdown+0x67
syscall() at syscall+0x27b
--- syscall (number 134) ---
end of kernel
end trace frame: 0xfd39b922600, count: 252
0xfd2c6bbcffa:
End of stack trace.

Re: bluhm's splsoftassert

[Mike Belopuhov]

On Tue, Sep 20, 2016 at 09:19 -0400, David Hill wrote:
> Another...
> 
> splassert: sorwakeup: want 5 have 4
> Starting stack trace...
> splassert_check() at splassert_check+0x78
> sorwakeup() at sorwakeup+0x27
> pfkey_sendup() at pfkey_sendup+0x99
> pfkeyv2_sendmessage() at pfkeyv2_sendmessage+0x226
> pfkeyv2_expire() at pfkeyv2_expire+0x18d
> tdb_timeout() at tdb_timeout+0x2f
> softclock() at softclock+0x144
> softintr_dispatch() at softintr_dispatch+0x8b
> Xsoftclock() at Xsoftclock+0x1f
> --- interrupt ---
> end of kernel
> end trace frame: 0x20, count: 248
> 0x8:
> End of stack trace.
> 

All TDB timeouts can do an splsoftnet.  OK?

Index: netinet/ip_ipsp.c
===
RCS file: /home/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.216
diff -u -p -r1.216 ip_ipsp.c
--- netinet/ip_ipsp.c   19 Sep 2016 18:09:22 -  1.216
+++ netinet/ip_ipsp.c   20 Sep 2016 13:34:16 -
@@ -504,55 +504,67 @@ void
 tdb_timeout(void *v)
 {
struct tdb *tdb = v;
+   int s;
 
if (!(tdb->tdb_flags & TDBF_TIMER))
return;
 
+   s = splsoftnet();
/* If it's an "invalid" TDB do a silent expiration. */
if (!(tdb->tdb_flags & TDBF_INVALID))
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
+   splx(s);
 }
 
 void
 tdb_firstuse(void *v)
 {
struct tdb *tdb = v;
+   int s;
 
if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
return;
 
+   s = splsoftnet();
/* If the TDB hasn't been used, don't renew it. */
if (tdb->tdb_first_use != 0)
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
+   splx(s);
 }
 
 void
 tdb_soft_timeout(void *v)
 {
struct tdb *tdb = v;
+   int s;
 
if (!(tdb->tdb_flags & TDBF_SOFT_TIMER))
return;
 
+   s = splsoftnet();
/* Soft expirations. */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
+   splx(s);
 }
 
 void
 tdb_soft_firstuse(void *v)
 {
struct tdb *tdb = v;
+   int s;
 
if (!(tdb->tdb_flags & TDBF_SOFT_FIRSTUSE))
return;
 
+   s = splsoftnet();
/* If the TDB hasn't been used, don't renew it. */
if (tdb->tdb_first_use != 0)
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
+   splx(s);
 }
 
 /*

Re: bluhm's splsoftassert

[Alexander Bluhm]

On Tue, Sep 20, 2016 at 09:27:57AM -0400, David Hill wrote:
> On Tue, Sep 20, 2016 at 03:16:50PM +0200, Alexander Bluhm wrote:
> > On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
> > > With bluhm's r1.160 uipc_socket.c.
> > > Here are the first ones that were detected.
> > 
> > Thanks for the fast report.
> > 
> > So fifo works around the socket layer.  Better call soconnect2()
> > instead of unp_connect2().  This adds the missing splsoftnet().
> > 
> > I think we should demand that socantsendmore() and socantrcvmore()
> > in uipc_socket2.c should be called with splsoftnet().
> 
> Should both socantsendmore() and socantrcvmore() get a splsoftassert()
> then, for now? 

No, not more asserts please :-)

The goal is to find one entry layer in the kernel where we can put
the network lock.  I is pretty much where we have the splsoftnet()
now.

The fifo, unix, pfkey and routing sockets don't work well with this
idea now.  Maybe I backout the splsoftasserts in the wakeup code
if I cannot fix it fast.

bluhm

Re: bluhm's splsoftassert

[David Hill]

On Tue, Sep 20, 2016 at 03:16:50PM +0200, Alexander Bluhm wrote:
> On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
> > With bluhm's r1.160 uipc_socket.c.
> > Here are the first ones that were detected.
> 
> Thanks for the fast report.
> 
> So fifo works around the socket layer.  Better call soconnect2()
> instead of unp_connect2().  This adds the missing splsoftnet().
> 
> I think we should demand that socantsendmore() and socantrcvmore()
> in uipc_socket2.c should be called with splsoftnet().

Should both socantsendmore() and socantrcvmore() get a splsoftassert()
then, for now? 

> 
> ok?
> 
> bluhm
> 
> Index: miscfs/fifofs/fifo_vnops.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 fifo_vnops.c
> --- miscfs/fifofs/fifo_vnops.c7 Jun 2016 06:12:37 -   1.51
> +++ miscfs/fifofs/fifo_vnops.c20 Sep 2016 13:04:56 -
> @@ -48,7 +48,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  
>  #include 
> @@ -143,7 +142,7 @@ fifo_open(void *v)
>   return (error);
>   }
>   fip->fi_writesock = wso;
> - if ((error = unp_connect2(wso, rso)) != 0) {
> + if ((error = soconnect2(wso, rso)) != 0) {
>   (void)soclose(wso);
>   (void)soclose(rso);
>   free(fip, M_VNODE, sizeof *fip);
> @@ -350,20 +349,25 @@ fifo_close(void *v)
>   struct vop_close_args *ap = v;
>   struct vnode *vp = ap->a_vp;
>   struct fifoinfo *fip = vp->v_fifoinfo;
> - int error1 = 0, error2 = 0;
> + int s, error1 = 0, error2 = 0;
>  
>   if (fip == NULL)
>   return (0);
>  
>   if (ap->a_fflag & FREAD) {
> - if (--fip->fi_readers == 0)
> + if (--fip->fi_readers == 0) {
> + s = splsoftnet();
>   socantsendmore(fip->fi_writesock);
> + splx(s);
> + }
>   }
>   if (ap->a_fflag & FWRITE) {
>   if (--fip->fi_writers == 0) {
> + s = splsoftnet();
>   /* SS_ISDISCONNECTED will result in POLLHUP */
>   fip->fi_readsock->so_state |= SS_ISDISCONNECTED;
>   socantrcvmore(fip->fi_readsock);
> + splx(s);
>   }
>   }
>   if (fip->fi_readers == 0 && fip->fi_writers == 0) {
> 

Re: bluhm's splsoftassert

[Mike Belopuhov]

On 20 September 2016 at 15:16, Alexander Bluhm  wrote:
> On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
>> With bluhm's r1.160 uipc_socket.c.
>> Here are the first ones that were detected.
>
> Thanks for the fast report.
>
> So fifo works around the socket layer.  Better call soconnect2()
> instead of unp_connect2().  This adds the missing splsoftnet().
>
> I think we should demand that socantsendmore() and socantrcvmore()
> in uipc_socket2.c should be called with splsoftnet().
>
> ok?
>
> bluhm
>

OK

Re: bluhm's splsoftassert

[David Hill]

And another.

splassert: sorwakeup: want 5 have 4
Starting stack trace...
splassert_check() at splassert_check+0x78
sorwakeup() at sorwakeup+0x27
pfkey_sendup() at pfkey_sendup+0x99
pfkeyv2_sendmessage() at pfkeyv2_sendmessage+0x226
pfkeyv2_expire() at pfkeyv2_expire+0x18d
tdb_soft_timeout() at tdb_soft_timeout+0x1c
softclock() at softclock+0x144
softintr_dispatch() at softintr_dispatch+0x8b
Xsoftclock() at Xsoftclock+0x1f
--- interrupt ---
end of kernel
end trace frame: 0x20, count: 248
0x8:
End of stack trace.

Re: bluhm's splsoftassert

[David Hill]

Another...

splassert: sorwakeup: want 5 have 4
Starting stack trace...
splassert_check() at splassert_check+0x78
sorwakeup() at sorwakeup+0x27
pfkey_sendup() at pfkey_sendup+0x99
pfkeyv2_sendmessage() at pfkeyv2_sendmessage+0x226
pfkeyv2_expire() at pfkeyv2_expire+0x18d
tdb_timeout() at tdb_timeout+0x2f
softclock() at softclock+0x144
softintr_dispatch() at softintr_dispatch+0x8b
Xsoftclock() at Xsoftclock+0x1f
--- interrupt ---
end of kernel
end trace frame: 0x20, count: 248
0x8:
End of stack trace.

Re: net80211: parse DTIM count and period from TIM IE

[Peter Hessler]

OK


On 2016 Sep 20 (Tue) at 14:14:49 +0200 (+0200), Stefan Sperling wrote:
:Parse the DTIM count and period advertised in beacons and store them
:in the node structure. This should be useful for iwm(4) in the future.
:The TIM IE is documented in 802.11-2012 section "8.4.2.7 TIM element".
:
:ok?
:
:There used to be code in iwm(4) which read these values from the ic
:(ic->ic_dtim_count and ic->ic_dtim_period) and did math based on them.
:An obvious bug in hindsight. The device needs live values from the AP
:and not some static values we set in our stack. The driver code that did
:this has already been deleted because it didn't allow 8k devices to work.
:
:Index: net80211/ieee80211_input.c
:===
:RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
:retrieving revision 1.178
:diff -u -p -r1.178 ieee80211_input.c
:--- net80211/ieee80211_input.c 18 May 2016 08:15:28 -  1.178
:+++ net80211/ieee80211_input.c 20 Sep 2016 12:02:11 -
:@@ -1393,7 +1393,7 @@ ieee80211_recv_probe_resp(struct ieee802
:   const u_int8_t *tstamp, *ssid, *rates, *xrates, *edcaie, *wmmie;
:   const u_int8_t *rsnie, *wpaie, *htcaps, *htop;
:   u_int16_t capinfo, bintval;
:-  u_int8_t chan, bchan, erp;
:+  u_int8_t chan, bchan, erp, dtim_count, dtim_period;
:   int is_new;
: 
:   /*
:@@ -1436,6 +1436,7 @@ ieee80211_recv_probe_resp(struct ieee802
:   bchan = ieee80211_chan2ieee(ic, ic->ic_bss->ni_chan);
:   chan = bchan;
:   erp = 0;
:+  dtim_count = dtim_period = 0;
:   while (frm + 2 <= efrm) {
:   if (frm + 2 + frm[1] > efrm) {
:   ic->ic_stats.is_rx_elem_toosmall++;
:@@ -1477,6 +1478,12 @@ ieee80211_recv_probe_resp(struct ieee802
:   case IEEE80211_ELEMID_HTOP:
:   htop = frm;
:   break;
:+  case IEEE80211_ELEMID_TIM:
:+  if (frm[1] > 3) {
:+  dtim_count = frm[2];
:+  dtim_period = frm[3];
:+  }
:+  break;
:   case IEEE80211_ELEMID_VENDOR:
:   if (frm[1] < 4) {
:   ic->ic_stats.is_rx_elem_toosmall++;
:@@ -1567,6 +1574,9 @@ ieee80211_recv_probe_resp(struct ieee802
:   ieee80211_setup_htcaps(ni, htcaps + 2, htcaps[1]);
:   if (htop && !ieee80211_setup_htop(ni, htop + 2, htop[1]))
:   htop = NULL; /* invalid HTOP */
:+
:+  ni->ni_dtimcount = dtim_count;
:+  ni->ni_dtimperiod = dtim_period;
: 
:   /*
:* When operating in station mode, check for state updates
:Index: net80211/ieee80211_node.h
:===
:RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
:retrieving revision 1.60
:diff -u -p -r1.60 ieee80211_node.h
:--- net80211/ieee80211_node.h  28 Apr 2016 08:18:10 -  1.60
:+++ net80211/ieee80211_node.h  20 Sep 2016 12:01:37 -
:@@ -188,9 +188,10 @@ struct ieee80211_node {
:   struct ieee80211_channel *ni_chan;
:   u_int8_tni_erp; /* 11g only */
: 
:-#ifdef notyet
:   /* DTIM and contention free period (CFP) */
:+  u_int8_tni_dtimcount;
:   u_int8_tni_dtimperiod;
:+#ifdef notyet
:   u_int8_tni_cfpperiod;   /* # of DTIMs between CFPs */
:   u_int16_t   ni_cfpduremain; /* remaining cfp duration */
:   u_int16_t   ni_cfpmaxduration;/* max CFP duration in TU */
:

-- 
An architect fellow named Yoric
Could, when feeling euphoric,
Display for selection
Three kinds of erection --
Corinthian, ionic, and doric.

Re: bluhm's splsoftassert

[Alexander Bluhm]

On Tue, Sep 20, 2016 at 08:21:55AM -0400, David Hill wrote:
> With bluhm's r1.160 uipc_socket.c.
> Here are the first ones that were detected.

Thanks for the fast report.

So fifo works around the socket layer.  Better call soconnect2()
instead of unp_connect2().  This adds the missing splsoftnet().

I think we should demand that socantsendmore() and socantrcvmore()
in uipc_socket2.c should be called with splsoftnet().

ok?

bluhm

Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.51
diff -u -p -r1.51 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c  7 Jun 2016 06:12:37 -   1.51
+++ miscfs/fifofs/fifo_vnops.c  20 Sep 2016 13:04:56 -
@@ -48,7 +48,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -143,7 +142,7 @@ fifo_open(void *v)
return (error);
}
fip->fi_writesock = wso;
-   if ((error = unp_connect2(wso, rso)) != 0) {
+   if ((error = soconnect2(wso, rso)) != 0) {
(void)soclose(wso);
(void)soclose(rso);
free(fip, M_VNODE, sizeof *fip);
@@ -350,20 +349,25 @@ fifo_close(void *v)
struct vop_close_args *ap = v;
struct vnode *vp = ap->a_vp;
struct fifoinfo *fip = vp->v_fifoinfo;
-   int error1 = 0, error2 = 0;
+   int s, error1 = 0, error2 = 0;
 
if (fip == NULL)
return (0);
 
if (ap->a_fflag & FREAD) {
-   if (--fip->fi_readers == 0)
+   if (--fip->fi_readers == 0) {
+   s = splsoftnet();
socantsendmore(fip->fi_writesock);
+   splx(s);
+   }
}
if (ap->a_fflag & FWRITE) {
if (--fip->fi_writers == 0) {
+   s = splsoftnet();
/* SS_ISDISCONNECTED will result in POLLHUP */
fip->fi_readsock->so_state |= SS_ISDISCONNECTED;
socantrcvmore(fip->fi_readsock);
+   splx(s);
}
}
if (fip->fi_readers == 0 && fip->fi_writers == 0) {

bluhm's splsoftassert

[David Hill]

Hello -

With bluhm's r1.160 uipc_socket.c.

Here are the first ones that were detected.

splassert: sowwakeup: want 5 have 0
Starting stack trace...
splassert_check() at splassert_check+0x78
sowwakeup() at sowwakeup+0x27
unp_connect2() at unp_connect2+0x62
fifo_open() at fifo_open+0x244
VOP_OPEN() at VOP_OPEN+0x3f
vn_open() at vn_open+0x16f
doopenat() at doopenat+0x187
syscall() at syscall+0x27b
--- syscall (number 5) ---
end of kernel
end trace frame: 0x199f2ba48898, count: 249
0x199ebb5c6d0a:
End of stack trace.
splassert: sorwakeup: want 5 have 0
Starting stack trace...
splassert_check() at splassert_check+0x78
sorwakeup() at sorwakeup+0x27
soisconnected() at soisconnected+0x46
unp_connect2() at unp_connect2+0x5a
fifo_open() at fifo_open+0x244
VOP_OPEN() at VOP_OPEN+0x3f
vn_open() at vn_open+0x16f
doopenat() at doopenat+0x187
syscall() at syscall+0x27b
--- syscall (number 5) ---
end of kernel
end trace frame: 0x199e63a704a8, count: 248
0x199ebb5c6d0a:
End of stack trace.
--- Begin Message ---
CVSROOT:/cvs
Module name:src
Changes by: bl...@cvs.openbsd.org   2016/09/20 05:11:44

Modified files:
sys/kern   : uipc_socket.c 

Log message:
Add some spl softnet assertions that will help us to find the right
places for the upcoming network lock.  This might trigger some
asserts, but we have to find the missing code paths.
OK mpi@

--- End Message ---

net80211: parse DTIM count and period from TIM IE

[Stefan Sperling]

Parse the DTIM count and period advertised in beacons and store them
in the node structure. This should be useful for iwm(4) in the future.
The TIM IE is documented in 802.11-2012 section "8.4.2.7 TIM element".

ok?

There used to be code in iwm(4) which read these values from the ic
(ic->ic_dtim_count and ic->ic_dtim_period) and did math based on them.
An obvious bug in hindsight. The device needs live values from the AP
and not some static values we set in our stack. The driver code that did
this has already been deleted because it didn't allow 8k devices to work.

Index: net80211/ieee80211_input.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.178
diff -u -p -r1.178 ieee80211_input.c
--- net80211/ieee80211_input.c  18 May 2016 08:15:28 -  1.178
+++ net80211/ieee80211_input.c  20 Sep 2016 12:02:11 -
@@ -1393,7 +1393,7 @@ ieee80211_recv_probe_resp(struct ieee802
const u_int8_t *tstamp, *ssid, *rates, *xrates, *edcaie, *wmmie;
const u_int8_t *rsnie, *wpaie, *htcaps, *htop;
u_int16_t capinfo, bintval;
-   u_int8_t chan, bchan, erp;
+   u_int8_t chan, bchan, erp, dtim_count, dtim_period;
int is_new;
 
/*
@@ -1436,6 +1436,7 @@ ieee80211_recv_probe_resp(struct ieee802
bchan = ieee80211_chan2ieee(ic, ic->ic_bss->ni_chan);
chan = bchan;
erp = 0;
+   dtim_count = dtim_period = 0;
while (frm + 2 <= efrm) {
if (frm + 2 + frm[1] > efrm) {
ic->ic_stats.is_rx_elem_toosmall++;
@@ -1477,6 +1478,12 @@ ieee80211_recv_probe_resp(struct ieee802
case IEEE80211_ELEMID_HTOP:
htop = frm;
break;
+   case IEEE80211_ELEMID_TIM:
+   if (frm[1] > 3) {
+   dtim_count = frm[2];
+   dtim_period = frm[3];
+   }
+   break;
case IEEE80211_ELEMID_VENDOR:
if (frm[1] < 4) {
ic->ic_stats.is_rx_elem_toosmall++;
@@ -1567,6 +1574,9 @@ ieee80211_recv_probe_resp(struct ieee802
ieee80211_setup_htcaps(ni, htcaps + 2, htcaps[1]);
if (htop && !ieee80211_setup_htop(ni, htop + 2, htop[1]))
htop = NULL; /* invalid HTOP */
+
+   ni->ni_dtimcount = dtim_count;
+   ni->ni_dtimperiod = dtim_period;
 
/*
 * When operating in station mode, check for state updates
Index: net80211/ieee80211_node.h
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
retrieving revision 1.60
diff -u -p -r1.60 ieee80211_node.h
--- net80211/ieee80211_node.h   28 Apr 2016 08:18:10 -  1.60
+++ net80211/ieee80211_node.h   20 Sep 2016 12:01:37 -
@@ -188,9 +188,10 @@ struct ieee80211_node {
struct ieee80211_channel *ni_chan;
u_int8_tni_erp; /* 11g only */
 
-#ifdef notyet
/* DTIM and contention free period (CFP) */
+   u_int8_tni_dtimcount;
u_int8_tni_dtimperiod;
+#ifdef notyet
u_int8_tni_cfpperiod;   /* # of DTIMs between CFPs */
u_int16_t   ni_cfpduremain; /* remaining cfp duration */
u_int16_t   ni_cfpmaxduration;/* max CFP duration in TU */

Re: teach BFD how to send route messages, again

[Peter Hessler]

On 2016 Sep 17 (Sat) at 12:05:54 +0200 (+0200), Peter Hessler wrote:
:This is a 2nd pass at having BFD send route messages.
:
:I fixed the things pointed out in the first thread, with the following
:comments:
:
: route(8) for the ramdisks is not built with SMALL, so adding SMALL
:won't help.
:
: rt_bfdmsg() is named in the same style, and is in the same place, as
:the other functions that send route messages
:
:OK?
:

SMALL has been added to route(8) for the ramdisks, and I have improved
printing some of the bfd fields.

this diff depends on rev 1.9 of bfd.h I just committed.

OK?


Index: sys/net/rtsock.c
===
RCS file: /cvs/openbsd/src/sys/net/rtsock.c,v
retrieving revision 1.205
diff -u -p -u -p -r1.205 rtsock.c
--- sys/net/rtsock.c17 Sep 2016 07:35:05 -  1.205
+++ sys/net/rtsock.c19 Sep 2016 08:15:50 -
@@ -1100,6 +1100,11 @@ rt_msg1(int type, struct rt_addrinfo *rt
case RTM_IFANNOUNCE:
len = sizeof(struct if_announcemsghdr);
break;
+#ifdef BFD
+   case RTM_BFD:
+   len = sizeof(struct bfd_msghdr);
+   break;
+#endif
default:
len = sizeof(struct rt_msghdr);
break;
@@ -1332,6 +1337,47 @@ rt_ifannouncemsg(struct ifnet *ifp, int 
route_proto.sp_protocol = 0;
route_input(m, _proto, _src, _dst);
 }
+
+#ifdef BFD
+/*
+ * This is used to generate routing socket messages indicating
+ * the state of a BFD session.
+ */
+void
+rt_bfdmsg(struct bfd_config *bfd)
+{
+   struct bfd_msghdr   *bfdm;
+   struct mbuf *m;
+
+   if (route_cb.any_count == 0)
+   return;
+   m = rt_msg1(RTM_BFD, NULL);
+   if (m == NULL)
+   return;
+   bfdm = mtod(m, struct bfd_msghdr *);
+
+   bfdm->bm_mode = bfd->bc_mode;
+   bfdm->bm_mintx = bfd->bc_mintx;
+   bfdm->bm_minrx = bfd->bc_minrx;
+   bfdm->bm_minecho = bfd->bc_minecho;
+   bfdm->bm_multiplier = bfd->bc_multiplier;
+
+   bfdm->bm_uptime = bfd->bc_time->tv_sec;
+   bfdm->bm_lastuptime = bfd->bc_lastuptime;
+   bfdm->bm_state = bfd->bc_state;
+   bfdm->bm_remotestate = bfd->bc_neighbor->bn_rstate;
+   bfdm->bm_laststate = bfd->bc_laststate;
+   bfdm->bm_error = bfd->bc_error;
+
+   bfdm->bm_localdiscr = bfd->bc_neighbor->bn_ldiscr;
+   bfdm->bm_localdiag = bfd->bc_neighbor->bn_ldiag;
+   bfdm->bm_remotediscr = bfd->bc_neighbor->bn_rdiscr;
+   bfdm->bm_remotediag = bfd->bc_neighbor->bn_rdiag;
+
+   route_proto.sp_protocol = 0;
+   route_input(m, _proto, _src, _dst);
+}
+#endif /* BFD */
 
 /*
  * This is used in dumping the kernel table via sysctl().
Index: sys/net/route.h
===
RCS file: /cvs/openbsd/src/sys/net/route.h,v
retrieving revision 1.147
diff -u -p -u -p -r1.147 route.h
--- sys/net/route.h 4 Sep 2016 10:32:01 -   1.147
+++ sys/net/route.h 17 Sep 2016 09:33:22 -
@@ -356,6 +356,7 @@ struct mbuf;
 struct socket;
 struct ifnet;
 struct sockaddr_in6;
+struct bfd_config;
 
 voidroute_init(void);
 int route_output(struct mbuf *, ...);
@@ -363,6 +364,7 @@ int  route_usrreq(struct socket *, int, 
   struct mbuf *, struct mbuf *, struct proc *);
 voidrt_ifmsg(struct ifnet *);
 voidrt_ifannouncemsg(struct ifnet *, int);
+voidrt_bfdmsg(struct bfd_config *);
 voidrt_maskedcopy(struct sockaddr *,
struct sockaddr *, struct sockaddr *);
 struct sockaddr *rt_plen2mask(struct rtentry *, struct sockaddr_in6 *);
Index: sbin/route/route.c
===
RCS file: /cvs/openbsd/src/sbin/route/route.c,v
retrieving revision 1.191
diff -u -p -u -p -r1.191 route.c
--- sbin/route/route.c  15 Sep 2016 12:51:20 -  1.191
+++ sbin/route/route.c  20 Sep 2016 10:46:16 -
@@ -41,6 +41,11 @@
 #include 
 #include 
 
+#ifndef SMALL
+#include 
+#include 
+#endif
+
 #include 
 #include 
 
@@ -90,6 +95,12 @@ void  sodump(sup, char *);
 char   *priorityname(uint8_t);
 uint8_t getpriority(char *);
 voidprint_getmsg(struct rt_msghdr *, int);
+#ifndef SMALL
+const char *bfd_printstate(unsigned int);
+const char *bfd_printdiag(unsigned int);
+const char *bfd_calc_uptime(time_t);
+voidprint_bfdmsg(struct rt_msghdr *);
+#endif
 const char *get_linkstate(int, int);
 voidprint_rtmsg(struct rt_msghdr *, int);
 voidpmsg_common(struct rt_msghdr *);
@@ -1334,7 +1345,9 @@ print_rtmsg(struct rt_msghdr *rtm, int m
printf("\n");
break;
case RTM_BFD:
-   printf("bfd\n");/* XXX - expand*/
+#ifndef SMALL
+   print_bfdmsg(rtm);
+#endif
break;
default:
printf(", priority %d, table %u, ifidx %u, ",
@@ -1526,6 +1539,98 @@ print_getmsg(struct rt_msghdr *rtm, int 
  

Re: teach BFD how to send route messages, again

[Peter Hessler]

On 2016 Sep 18 (Sun) at 20:29:57 +0200 (+0200), Peter Hessler wrote:
:On 2016 Sep 17 (Sat) at 13:15:56 +0200 (+0200), Peter Hessler wrote:
::Tested on an amd64 bsd.rd. dhclient, route add, route del, route show,
::all still work, and it's slightly smaller.
::
:
:All options, commands, modifiers, and special names for networks have
:been tested.  Some modifiers are already disabled in a SMALL kernel (esp
:mpath and mpls).
:

ping.  OK?


:
::textdatabss dec hex
::213397  889636896   259189  3f475 obj/route
::212677  889636896   258469  3f1a5 obj/route-SMALL
::
::
::OK?
::
::Index: distrib/special/route/Makefile
::===
::RCS file: /cvs/openbsd/src/distrib/special/route/Makefile,v
::retrieving revision 1.1
::diff -u -p -u -p -r1.1 Makefile
::--- distrib/special/route/Makefile23 Dec 2014 17:16:03 -  1.1
::+++ distrib/special/route/Makefile17 Sep 2016 11:09:59 -
::@@ -4,7 +4,7 @@ PROG= route
:: MAN= route.8
:: SRCS=route.c show.c
:: 
::-CFLAGS+= -Wall
::+CFLAGS+= -Wall -DSMALL
:: 
:: route.o .depend lint tags: keywords.h
:: 
::

Re: libc: remove pointless off_t, size_t, void* and char* casts

[Alexander Bluhm]

On Mon, Sep 19, 2016 at 08:27:56PM -0700, Philip Guenther wrote:
> --- stdlib/malloc.c   18 Sep 2016 13:46:28 -  1.196
> +++ stdlib/malloc.c   19 Sep 2016 11:55:20 -
> @@ -1231,6 +1231,7 @@ _malloc_init(int from_rthreads)
>   mprotect(_readonly, sizeof(malloc_readonly), PROT_READ);
>   _MALLOC_UNLOCK(0);
>  }
> +DEF_STRONG(_malloc_init);
>  
>  void *
>  malloc(size_t size)

This chunk looks unrelated.

OK bluhm@ for the cast cleanup.

Re: timeout_set_proc(9)

[Martin Pieuchot]

On 15/09/16(Thu) 16:29, Martin Pieuchot wrote:
> After discussing with a few people about a new "timed task" API I came
> to the conclusion that mixing timeouts and tasks will result in:
> 
>   - always including a 'struct timeout' in a 'struct task', or the other
> the way around
> or
>   
>   - introducing a new data structure, hence API.
> 
> Since I'd like to keep the change as small as possible when converting
> existing timeout_set(9), neither option seem a good fit.  So I decided
> to add a new kernel thread, curiously named "softclock", that will
> offer his stack to the poor timeout handlers that need one. 
> 
> With this approach, converting a timeout is just a matter of doing:
> 
>   s/timeout_set/timeout_set_proc/
> 
> 
> Diff below includes the conversions I need for the "netlock".  I'm
> waiting for feedbacks and a better name to document the new function.

Updated diff using  CPU_INFO_FOREACH() as requested by kettenis@ and
including manpage bits.

Should I just move forward?  Objections?  Oks?


Index: sys/kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.258
diff -u -p -r1.258 init_main.c
--- sys/kern/init_main.c18 Sep 2016 12:36:28 -  1.258
+++ sys/kern/init_main.c19 Sep 2016 09:18:49 -
@@ -144,6 +144,7 @@ voidprof_init(void);
 void   init_exec(void);
 void   kqueue_init(void);
 void   taskq_init(void);
+void   timeout_proc_init(void);
 void   pool_gc_pages(void *);
 
 extern char sigcode[], esigcode[], sigcoderet[];
@@ -335,6 +336,9 @@ main(void *framep)
sleep_queue_init();
sched_init_cpu(curcpu());
p->p_cpu->ci_randseed = (arc4random() & 0x7fff) + 1;
+
+   /* Initialize timeouts in process context. */
+   timeout_proc_init();
 
/* Initialize task queues */
taskq_init();
Index: sys/kern/kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.48
diff -u -p -r1.48 kern_timeout.c
--- sys/kern/kern_timeout.c 6 Jul 2016 15:53:01 -   1.48
+++ sys/kern/kern_timeout.c 20 Sep 2016 08:37:48 -
@@ -27,7 +27,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -54,6 +54,7 @@
 
 struct circq timeout_wheel[BUCKETS];   /* Queues of timeouts */
 struct circq timeout_todo; /* Worklist */
+struct circq timeout_proc; /* Due timeouts needing proc. context */
 
 #define MASKWHEEL(wheel, time) (((time) >> ((wheel)*WHEELBITS)) & WHEELMASK)
 
@@ -127,6 +128,9 @@ struct mutex timeout_mutex = MUTEX_INITI
 
 #define CIRCQ_EMPTY(elem) (CIRCQ_FIRST(elem) == (elem))
 
+void softclock_thread(void *);
+void softclock_create_thread(void *);
+
 /*
  * Some of the "math" in here is a bit tricky.
  *
@@ -147,11 +151,18 @@ timeout_startup(void)
int b;
 
CIRCQ_INIT(_todo);
+   CIRCQ_INIT(_proc);
for (b = 0; b < nitems(timeout_wheel); b++)
CIRCQ_INIT(_wheel[b]);
 }
 
 void
+timeout_proc_init(void)
+{
+   kthread_create_deferred(softclock_create_thread, NULL);
+}
+
+void
 timeout_set(struct timeout *new, void (*fn)(void *), void *arg)
 {
new->to_func = fn;
@@ -159,6 +170,12 @@ timeout_set(struct timeout *new, void (*
new->to_flags = TIMEOUT_INITIALIZED;
 }
 
+void
+timeout_set_proc(struct timeout *new, void (*fn)(void *), void *arg)
+{
+   timeout_set(new, fn, arg);
+   new->to_flags |= TIMEOUT_NEEDPROCCTX;
+}
 
 int
 timeout_add(struct timeout *new, int to_ticks)
@@ -334,38 +351,90 @@ timeout_hardclock_update(void)
 }
 
 void
+timeout_run(struct timeout *to)
+{
+   void (*fn)(void *);
+   void *arg;
+
+   MUTEX_ASSERT_LOCKED(_mutex);
+
+   to->to_flags &= ~TIMEOUT_ONQUEUE;
+   to->to_flags |= TIMEOUT_TRIGGERED;
+
+   fn = to->to_func;
+   arg = to->to_arg;
+
+   mtx_leave(_mutex);
+   fn(arg);
+   mtx_enter(_mutex);
+}
+
+void
 softclock(void *arg)
 {
int delta;
struct circq *bucket;
struct timeout *to;
-   void (*fn)(void *);
 
mtx_enter(_mutex);
while (!CIRCQ_EMPTY(_todo)) {
to = timeout_from_circq(CIRCQ_FIRST(_todo));
CIRCQ_REMOVE(>to_list);
 
-   /* If due run it, otherwise insert it into the right bucket. */
+   /*
+* If due run it or defer execution to the thread,
+* otherwise insert it into the right bucket.
+*/
delta = to->to_time - ticks;
if (delta > 0) {
bucket = (delta, to->to_time);
CIRCQ_INSERT(>to_list, bucket);
+   } else if (to->to_flags & TIMEOUT_NEEDPROCCTX) {
+   CIRCQ_INSERT(>to_list, _proc);
+   wakeup(_proc);
} else {
 #ifdef DEBUG
 

Re: db_{interface,trace}.c: ansify function definitions

[Markus Hennecke]

Am 19.09.2016 um 22:30 schrieb Philip Guenther:

On Mon, Sep 19, 2016 at 11:02 AM, Jasper Lievisse Adriaanse
 wrote:

OK?

Index: alpha/alpha/db_trace.c

...

ok guenther@



There is one too many closing brace in the arm code:

Index: sys/arch/arm/arm/db_trace.c
===
RCS file: /cvs/src/sys/arch/arm/arm/db_trace.c,v
retrieving revision 1.8
diff -u -p -r1.8 db_trace.c
--- sys/arch/arm/arm/db_trace.c 19 Sep 2016 21:18:35 -  1.8
+++ sys/arch/arm/arm/db_trace.c 20 Sep 2016 07:59:02 -
@@ -82,7 +82,7 @@ db_regs_t ddb_regs;

 void
 db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
-char *modif, int (*pr)(const char *, ...)))
+char *modif, int (*pr)(const char *, ...))
 {
u_int32_t   *frame, *lastframe;
char c, *cp = modif;

Re: libc: remove pointless off_t, size_t, void* and char* casts

[Martin Natano]

Reads fine to me. OK.