Re: hashfree: sizes for free(9) when using hashinit.
On Wed, Sep 21, 2016 at 1:30 AM, Mathieu - wrote: > Ted Unangst wrote: >> Mathieu - wrote: >> > Hello list, >> > >> > I'm introducing hashfree, a counterpart to hashinit in order to pass the >> > size to free(9) while hiding the implementation details. >> > Most of the api users are converted in the patch below, those not >> > included just simply do not free the memory (pid hash table etc). All, >> > except for one case, the input hashtbl in in_pcb, because at free time >> > the size is really not known, so it needs more moving of things around >> > and is out the scope of this patch. >> > >> > Manpage diff courtesy of natano@ on an old version of the diff! >> >> looks good > > Anyone ? I don't understand the rename to hashfree() from the NetBSD name of hashdone(). Yes, it has different args...but so does our hashinit()! tedu, were you going to commit this? Philip Guenther
Re: timeout_set_proc(9)
On Fri, 23 Sep 2016, Christiano F. Haesbaert wrote:
...
> The diff as it is will deadlock against SCHED_LOCK.
> tsleep() calls sleep_setup(), which grabs SCHED_LOCK,
> Then sleep_setup_timeout() will grab timeout_mutex in timeout_add()
>
> On softclock() you have the opposite:
> Grabs timeout_mutex then does a wakeup, which grabs SCHED_LOCK.
>
Hmm, yes. And softclock_thread() has the same problem: msleep() needs to
grab SCHED_LOCK while the passed in mutex is held, so it'll hold
timeout_mutex while grabbing SCHED_LOCK too.
I just played around with using a separate mutex for protecting
timeout_proc, a mutex which would be 'outside' SCHED_LOCK, unlike
timeout_mutex which is 'inside' SCHED_LOCK. The catch is that supporting
all the functionality of timeout_add() and timeout_del() becomes ugly and
fragile: they would need to check whether the mutex has
TIMEOUT_NEEDPROCCTX set and, if so, grab the new mutex before grabbing
timeout_mutex. That's "safe" from being a lock loop from tsleep() because
the thread's p_sleep_to *can't* have that flag set, so the 'outside' mutex
wouldn't be neededbut geez is this ugly. I'm also unsure what defined
semantics, if any, timeout_triggered() should have for NEEDPROCCTX
timeouts. Should it return true once the timeout has been dequeued from
the timeout wheel, or should it only be set once softclock_thread is
actually going to run it?
...or maybe this makes people think we should toss this out and go
directly to dlg@'s proposal...
Philip Guenther
Index: kern_timeout.c
===
RCS file: /data/src/openbsd/src/sys/kern/kern_timeout.c,v
retrieving revision 1.49
diff -u -p -r1.49 kern_timeout.c
--- kern_timeout.c 22 Sep 2016 12:55:24 - 1.49
+++ kern_timeout.c 24 Sep 2016 05:10:00 -
@@ -87,9 +87,15 @@ timeout_from_circq(struct circq *p)
* All wheels are locked with the same mutex.
*
* We need locking since the timeouts are manipulated from hardclock that's
- * not behind the big lock.
+ * not behind the big lock. timeout_mutex protects the actual timeout
+ * wheel and queue of new timeouts. timeout_proc_mutex proctects the
+ * queue of "need proc context" timeouts that have triggered. Since those
+ * timeouts can reside in either location and are moved between them by
+ * softclock(), calls that might run when it's uncertain where the timeout
+ * is queued must hold *both* mutexes when queueing or dequeueing them.
*/
struct mutex timeout_mutex = MUTEX_INITIALIZER(IPL_HIGH);
+struct mutex timeout_proc_mutex = MUTEX_INITIALIZER(IPL_NONE);
/*
* Circular queue definitions.
@@ -190,6 +196,8 @@ timeout_add(struct timeout *new, int to_
panic("timeout_add: to_ticks (%d) < 0", to_ticks);
#endif
+ if (new->to_flags & TIMEOUT_NEEDPROCCTX)
+ mtx_enter(&timeout_proc_mutex);
mtx_enter(&timeout_mutex);
/* Initialize the time here, it won't change. */
old_time = new->to_time;
@@ -212,6 +220,8 @@ timeout_add(struct timeout *new, int to_
CIRCQ_INSERT(&new->to_list, &timeout_todo);
}
mtx_leave(&timeout_mutex);
+ if (new->to_flags & TIMEOUT_NEEDPROCCTX)
+ mtx_leave(&timeout_proc_mutex);
return (ret);
}
@@ -312,6 +322,8 @@ timeout_del(struct timeout *to)
{
int ret = 0;
+ if (to->to_flags & TIMEOUT_NEEDPROCCTX)
+ mtx_enter(&timeout_proc_mutex);
mtx_enter(&timeout_mutex);
if (to->to_flags & TIMEOUT_ONQUEUE) {
CIRCQ_REMOVE(&to->to_list);
@@ -320,6 +332,8 @@ timeout_del(struct timeout *to)
}
to->to_flags &= ~TIMEOUT_TRIGGERED;
mtx_leave(&timeout_mutex);
+ if (to->to_flags & TIMEOUT_NEEDPROCCTX)
+ mtx_leave(&timeout_proc_mutex);
return (ret);
}
@@ -351,56 +365,66 @@ timeout_hardclock_update(void)
}
void
-timeout_run(struct timeout *to)
+timeout_run(struct timeout *to, struct mutex *mtx)
{
void (*fn)(void *);
void *arg;
- MUTEX_ASSERT_LOCKED(&timeout_mutex);
+ MUTEX_ASSERT_LOCKED(mtx);
to->to_flags &= ~TIMEOUT_ONQUEUE;
- to->to_flags |= TIMEOUT_TRIGGERED;
fn = to->to_func;
arg = to->to_arg;
- mtx_leave(&timeout_mutex);
+ mtx_leave(mtx);
fn(arg);
- mtx_enter(&timeout_mutex);
+ mtx_enter(mtx);
}
void
softclock(void *arg)
{
+ struct circq need_proc;
int delta;
struct circq *bucket;
struct timeout *to;
+ int need_wakeup;
+ CIRCQ_INIT(&need_proc);
mtx_enter(&timeout_mutex);
while (!CIRCQ_EMPTY(&timeout_todo)) {
to = timeout_from_circq(CIRCQ_FIRST(&timeout_todo));
CIRCQ_REMOVE(&to->to_list);
- /*
-* If due run it or defer execution to the thread,
-* otherwise insert it into the right bucket.
-
Re: Some arm cleanups suggested by clang
On Fri, Sep 23, 2016 at 01:43:04PM +0200, Mark Kettenis wrote:
> Most of these are warnings about static symbols that aren't used. The
> pmap_get_pde_pte() bit fixes:
>
> ../../../../arch/arm/arm/pmap7.c:2220:10: warning: comparison of array
> 'pm->pm_l2' equal to a null pointer is always false
> [-Wtautological-pointer-compare]
> if (pm->pm_l2 == NULL)
>
> ok?
>
>
> Index: arch/arm/arm/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/arm/arm/cpu.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 cpu.c
> --- arch/arm/arm/cpu.c14 Aug 2016 11:30:54 - 1.32
> +++ arch/arm/arm/cpu.c23 Sep 2016 11:32:51 -
> @@ -103,16 +103,6 @@ static const char * const pxa2x0_steppin
> "rev 12", "rev 13", "rev 14", "rev 15"
> };
>
> -/* Steppings for PXA255/26x.
> - * rev 5: PXA26x B0, rev 6: PXA255 A0
> - */
> -static const char * const pxa255_steppings[16] = {
> - "rev 0","rev 1","rev 2","step A-0",
> - "rev 4","step B-0", "step A-0", "rev 7",
> - "rev 8","rev 9","rev 10", "rev 11",
> - "rev 12", "rev 13", "rev 14", "rev 15"
> -};
Why not just remove all the pxa/xscale bits from cpu/cpufunc?
Anyway removing this and the making the other changes is ok jsg@
> -
> /* Steppings for PXA270 */
> static const char * const pxa27x_steppings[16] = {
> "step A-0", "step A-1", "step B-0", "step B-1",
> Index: arch/arm/arm/pmap7.c
> ===
> RCS file: /cvs/src/sys/arch/arm/arm/pmap7.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 pmap7.c
> --- arch/arm/arm/pmap7.c 15 Sep 2016 02:00:17 - 1.52
> +++ arch/arm/arm/pmap7.c 23 Sep 2016 11:32:51 -
> @@ -446,26 +446,12 @@ pmap_tlb_flushID_SE(pmap_t pm, vaddr_t v
> }
>
> static __inline void
> -pmap_tlb_flushD_SE(pmap_t pm, vaddr_t va)
> -{
> - if (pmap_is_current(pm))
> - cpu_tlb_flushD_SE(va);
> -}
> -
> -static __inline void
> pmap_tlb_flushID(pmap_t pm)
> {
> if (pmap_is_current(pm))
> cpu_tlb_flushID();
> }
>
> -static __inline void
> -pmap_tlb_flushD(pmap_t pm)
> -{
> - if (pmap_is_current(pm))
> - cpu_tlb_flushD();
> -}
> -
> /*
> * Returns a pointer to the L2 bucket associated with the specified pmap
> * and VA, or NULL if no L2 bucket exists for the address.
> @@ -2217,11 +2203,7 @@ pmap_get_pde_pte(pmap_t pm, vaddr_t va,
> return (TRUE);
> }
>
> - if (pm->pm_l2 == NULL)
> - return (FALSE);
> -
> l2 = pm->pm_l2[L2_IDX(l1idx)];
> -
> if (l2 == NULL ||
> (ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL) {
> return (FALSE);
> Index: arch/arm/cortex/agtimer.c
> ===
> RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 agtimer.c
> --- arch/arm/cortex/agtimer.c 10 Aug 2016 06:51:57 - 1.7
> +++ arch/arm/cortex/agtimer.c 23 Sep 2016 11:32:51 -
> @@ -126,16 +126,6 @@ agtimer_set_ctrl(uint32_t val)
> }
>
> static inline int
> -agtimer_get_tval(void)
> -{
> - uint32_t val;
> -
> - __asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
> -
> - return (val);
> -}
> -
> -static inline int
> agtimer_set_tval(uint32_t val)
> {
> __asm volatile("mcr p15, 0, %[val], c14, c2, 0" : :
>
Re: dwc2 cleanup diff
On Fri, Sep 23, 2016 at 01:48:39PM +0200, Mark Kettenis wrote: > Essentially fixes for clang warnings. Clang doesn't like the > > sc = sc; > > statements in dwc2_shutdown(), dwc2_childdet() and dwc2_activate(). > Those functions aren't actually used. This diff moves them inside the > #if 0 that's already there for dwc2_suspend() and dwc2_resume(). But > perhaps I should just remove the code altogether? I think these can be removed. They seem to be parts of netbsd's driver interface that we don't have, and keeping them around doesn't really make updating dwc2 any easier. Either this diff or one that removes them is ok by me.
correct option listing in switchd(8)
Sync the option list with reality. Don't document the internal -I and -P options that set the instance number and title on purpose. Index: switchd.8 === RCS file: /cvs/src/usr.sbin/switchd/switchd.8,v retrieving revision 1.1 diff -u -p -r1.1 switchd.8 --- switchd.8 19 Jul 2016 16:54:26 - 1.1 +++ switchd.8 16 Sep 2016 11:01:27 - @@ -22,13 +22,52 @@ .Nd software-defined networking (SDN) sflow controller .Sh SYNOPSIS .Nm switchd -.Op Fl 6dnSTtv +.Op Fl dnv +.Op Fl c Ar cachesize .Op Fl D Ar macro Ns = Ns Ar value .Op Fl f Ar file +.Op Fl t Ar timeout .Sh DESCRIPTION .Nm is an controller for software-defined networking (SDN) and is compatible with the OpenFlow protocol. +.Pp +The options are as follows: +.Bl -tag -width Dssmacro=value +.It Fl D Ar macro Ns = Ns Ar value +Set a +.Ar macro +to a +.Ar value . +Macros can be referenced in the configuration files. +.It Fl c Ar cachesize +Number of MAC addresses to cache. +The default is 4096. +.It Fl d +Debug mode. +Don't detach or become a daemon. +This allows for easy monitoring of +.Nm . +.It Fl f Ar file +Specifies the configuration file. +The default is +.Pa /etc/switchd.conf . +.It Fl n +Check that the configuration is valid, but don't start the daemon. +.It Fl t Ar timeout +Timeout in seconds for learned MAC addresses. +The default is 240 seconds. +.It Fl v +Verbose mode. +Multiple +.Fl v +options increase the verbosity. +.El +.Sh FILES +.Bl -tag -width "/etc/switchd.conf" -compact +.It Pa /etc/switchd.conf +Default configuration file. +.El .Sh STANDARDS .Rs .%A Open Networking Foundation (ONF)
etherip alignment issues
Here's another alignment issue. I have configured two machines as follows. ifconfig vether0 inet ... ifconfig etherip0 tunnel ... ... ifconfig bridge0 add vether0 add etherip0 An amd64 machine works fine in this configuration, however armv7 and sparc64 both have trouble. This happens very quickly after setting up the interfaces and attempting to ping. The armv7 is running -current and the sparc64 is running 6.0. Some quick research points at the M_PREPEND calls in ip_etherip_output, since etherip headers are only two bytes. I can reproduce and get more information if necessary. Thanks, Martin Here's armv7 Fatal kernel mode data abort: 'Alignment fault' trapframe: 0xcc3c2c90 DFSR=0801, DFAR=ca1829e6, spsr=2113 r0 =ca182900, r1 =08c4, r2 =030a, r3 =7800 r4 =ca1829da, r5 =c56d92e8, r6 =c56d9000, r7 =c56d93e8 r8 =, r9 =ca182900, r10=0002, r11=cc3c2d24 r12=c06987d8, ssp=cc3c2ce0, slr=c0524ae8, pc =c040a13c Stopped at ip_etherip_output+0x158:str r2, [r4, #0x00c] ddb> trace ip_etherip_output+0x10 scp=0xc0409ff4 rlv=0xc040a688 (etherip_start+0xe8) rsp=0xcc3c2d28 rfp=0xcc3c2d4c r10=0xc5726800 r9=0xc56f8000 r8=0xc56f8000 r7=0xc56d9198 r6=0xc56d9000 r5=0xc56d9000 r4=0xca182900 etherip_start+0xc scp=0xc040a5ac rlv=0xc0403aac (if_start_locked+0x34) rsp=0xcc3c2d50 rfp=0xcc3c2d74 r7=0x r6=0xc069a840 r5=0x0003 r4=0xc56d9000 if_start_locked+0xc scp=0xc0403a84 rlv=0xc0407b14 (if_enqueue+0x88) rsp=0xcc3c2d78 rfp=0xcc3c2d9c r6=0x0062 r5=0x0012 r4=0xc56d9000 if_enqueue+0xc scp=0xc0407a98 rlv=0xc041a138 (bridge_ifenqueue+0x30) rsp=0xcc3c2da0 rfp=0xcc3c2dcc r7=0x r6=0x0062 r5=0x r4=0xc5727900 bridge_ifenqueue+0xc scp=0xc041a114 rlv=0xc041a6fc (bridge_output+0x278) rsp=0xcc3c2dd0 rfp=0xcc3c2e0c r8=0xca182900 r7=0x r6=0xc56d9000 r5=0x r4=0xc5727900 bridge_output+0xc scp=0xc041a490 rlv=0xc0409768 ($a+0xb0) rsp=0xcc3c2e10 rfp=0xcc3c2e54 r10=0xcc3c2e1e r9=0xc5726aa0 r8=0xca182f00 r7=0xc5726800 r6=0x0008 r5=0xca182900 r4=0xca1829f0 ether_output+0xc scp=0xc04095a8 rlv=0xc04510a4 (arpcache+0x130) rsp=0xcc3c2e58 rfp=0xcc3c2eac r10=0xc5726800 r9=0xca182c50 r8=0xca3ed08c r7=0xca0497f8 r6=0xca3ed09c r5=0xc07044b0 r4=0x0001 arpcache+0xc scp=0xc0450f80 rlv=0xc0451578 (in_arpinput+0x280) rsp=0xcc3c2eb0 rfp=0xcc3c2f14 r10=0x r9=0xca182c50 r8=0x0001 r7=0xc5726800 r6=0xca182c00 r5=0xca0497f8 r4=0xca182c48 in_arpinput+0xc scp=0xc0451304 rlv=0xc0451660 (arpintr+0x28) rsp=0xcc3c2f18 rfp=0xcc3c2f44 r10=0xc069a840 r9=0x r8=0x r7=0xc0703a98 r6=0xcc3c2f18 r5=0xc5726800 r4=0xca182c00 arpintr+0x10 scp=0xc0451648 rlv=0xc0406e2c (if_netisr+0xc8) rsp=0xcc3c2f48 rfp=0xcc3c2f74 r6=0x r5=0xc0703a98 r4=0x0004 if_netisr+0x10 scp=0xc0406d74 rlv=0xc03b85cc (taskq_thread+0x78) rsp=0xcc3c2f78 rfp=0xcc3c2fac r10=0xc06fbc98 r8=0xc06fbe78 r7=0xcc3c2f78 r6=0x0001 r5=0xc54c0040 r4=0xc03b4434 taskq_thread+0xc scp=0xc03b8560 rlv=0xc052b088 (proc_trampoline+0x18) rsp=0xcc3c2fb0 rfp=0xc0815ec4 r7=0x r6=0x r5=0xc54c0040 r4=0xc03b8554 Bad frame pointer: 0xc0815ec4 ddb> and here's sparc64 panic: trap type 0x34 (mem address not aligned): pc=11ffa0c npc=11ffa 10 pstate=820006 Stopped at Debugger+0x8: nop TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *45795 45795 0 0x14000 0x2101 softnet trap(4007849d370, 34, 11ffa0c, 820006, 4007849db10, 0) at trap+0x360 Lslowtrap_reenter(40014464400, 14, 2, 0, 88d8, 6) at Lslowtrap_reenter+0xf8 ip_etherip_output(4000a85c800, 40014464400, 62, 4007849dc08, 4007849db10, 0) at ip_etherip_output+0xd4 etherip_start(4000a85c800, 62, 0, 0, 88d8, 6) at etherip_start+0xc4 if_start_locked(4000a85c800, 40014464400, 62, 4007849dc08, 4007849db10, 0) at if_start_locked+0x24 if_enqueue(0, 62, 0, 0, 88d8, 6) at if_enqueue+0x6c bridge_ifenqueue(4000ae12000, 4000a85c800, 62, 4007849dc08, 4007849db10, 0) at bridge_ifenqueue+0x30 bridge_output(0, 40014464400, 0, 0, 88d8, 6) at bridge_output+0x260 if_enqueue(4000a85d800, 40014464400, 40014464400, 4007849dc08, 4007849db10, 0) at if_enqueue+0x90 ether_output(37, 40014464b00, 4007849dc08, 40014b04e70, 88d8, ) at ether_output+0x214 ip_output(40014464b00, 40014464b68, 4007849dbf8, 0, 0, 0) at ip_output+0x7a0 ip_send_dispatch(0, 4007849dde0, 119bfe0, 0, 4, 2000) at ip_send_dispatch+0x64 taskq_thread(40008b18080, 40014bded80, 168fe98, 168fc10, 0, 3b9ac800) at taskq_thread+0x6c proc_trampoline(0, 0, 0, 0, 0, 0) at proc_trampoline+0x14
attach SR drive by force even if not all chunks provide native metadata
Hello,
following patch fixes issue while attempting to attach SR RAID1 drive
where not all chunks provide native metadata. I.e. one chunk is dd
zeroed. The complain of SR is good one, but I'd think that force
parameter should overcome it and really enforce SR to attach such
drive.
Thanks,
Karel
diff -u -p -u -r1.377 softraid.c
--- softraid.c 20 Jul 2016 20:45:13 - 1.377
+++ softraid.c 23 Sep 2016 22:06:55 -
@@ -1658,7 +1661,7 @@ sr_meta_native_attach(struct sr_discipli
not_sr++;
}
- if (sr && not_sr) {
+ if (sr && not_sr && !force) {
sr_error(sc, "not all chunks are of the native metadata "
"format");
goto bad;
libcrypto/Makefile CLEANFILES
There are two lines in the Makefile setting CLEANFILES with the latter
one overriding the former. The result: libcrypto.pc is not removed on make
clean. Ok?
natano
Index: Makefile
===
RCS file: /cvs/src/lib/libcrypto/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- Makefile14 Sep 2016 06:26:02 - 1.6
+++ Makefile23 Sep 2016 21:17:34 -
@@ -395,7 +395,7 @@ includes: obj_mac.h
CFLAGS+= -I${.OBJDIR}
GENERATED=obj_mac.h obj_dat.h
-CLEANFILES=${GENERATED} obj_mac.num.tmp
+CLEANFILES+=${GENERATED} obj_mac.num.tmp
SSL_OBJECTS=${LCRYPTO_SRC}/objects
obj_mac.h: ${SSL_OBJECTS}/objects.h ${SSL_OBJECTS}/obj_mac.num
${SSL_OBJECTS}/objects.txt
Re: Remove more duplicated includes
Frederic Cambus writes: > Hi tech@, > > A few remaining duplicated includes to remove. > > Comments? OK? ok except for ixgbe.h > Index: sys/arch/sparc64/include/asm.h > === > RCS file: /cvs/src/sys/arch/sparc64/include/asm.h,v > retrieving revision 1.10 > diff -u -p -r1.10 asm.h > --- sys/arch/sparc64/include/asm.h27 May 2016 16:32:39 - 1.10 > +++ sys/arch/sparc64/include/asm.h22 Sep 2016 14:08:50 - > @@ -41,11 +41,6 @@ > #ifndef _MACHINE_ASM_H_ > #define _MACHINE_ASM_H_ > > -#ifndef _LOCORE > -#define _LOCORE > -#endif > -#include > - > /* Pull in CCFSZ, CC64FSZ, and BIAS from frame.h */ > #ifndef _LOCORE > #define _LOCORE > Index: sys/dev/pci/ixgbe.h > === > RCS file: /cvs/src/sys/dev/pci/ixgbe.h,v > retrieving revision 1.22 > diff -u -p -r1.22 ixgbe.h > --- sys/dev/pci/ixgbe.h 15 Mar 2016 16:45:52 - 1.22 > +++ sys/dev/pci/ixgbe.h 22 Sep 2016 14:08:51 - > @@ -62,10 +62,6 @@ > #include > #include > > -#if NBPFILTER > 0 > -#include > -#endif Better remove the unguarded #include, I think. Even if a kernel configured without BPFILTER will not build... Index: ixgbe.h === RCS file: /cvs/src/sys/dev/pci/ixgbe.h,v retrieving revision 1.22 diff -u -p -p -u -r1.22 ixgbe.h --- ixgbe.h 15 Mar 2016 16:45:52 - 1.22 +++ ixgbe.h 23 Sep 2016 17:51:44 - @@ -55,7 +55,6 @@ #include #include -#include #include #include > - > typedef int boolean_t; > #define TRUE 1 > #define FALSE0 > Index: usr.sbin/snmpd/snmpd.h > === > RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v > retrieving revision 1.67 > diff -u -p -r1.67 snmpd.h > --- usr.sbin/snmpd/snmpd.h16 Aug 2016 18:41:57 - 1.67 > +++ usr.sbin/snmpd/snmpd.h22 Sep 2016 14:08:51 - > @@ -24,7 +24,6 @@ > #include > #include > #include > -#include > #include > #include > #include > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
ftp5.usa.openbsd.org going down Saturday September 24th at 10pm - Back Sunday morning
Due to a power outage happening this Sunday morning, ftp5.usa.openbsd.org will be going down around 10pm EDT (UTC-4) on Saturday September 24th. I will bring it back up when the power comes back at 9:30am EDT on Sunday September 25th, so it should be back up by 11am EDT. FYI --Kurt Mosiejczuk
Re: switchd(8): set the pktbuf for packet_in messages
On Thu, Sep 22, 2016 at 09:48:48PM +0200, Rafael Zalamena wrote:
> The pkt_buf variable is never set in incoming packet_in messages and this
> diff fixes it.
>
> ok?
>
I wonder how this happened, thanks.
Skip the space before the cast,
> + pkt->pkt_buf = (uint8_t *) eh;
pkt->pkt_buf = (uint8_t *)eh;
otherwise OK.
Reyk
>
> Index: packet.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/packet.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 packet.c
> --- packet.c 21 Jul 2016 08:39:23 - 1.3
> +++ packet.c 21 Sep 2016 11:33:44 -
> @@ -63,7 +63,7 @@ packet_input(struct switchd *sc, struct
> return (-1);
>
> pkt->pkt_len = ibuf_dataleft(ibuf);
> - if ((pkt->pkt_eh = eh = ibuf_getdata(ibuf, sizeof(*eh))) == NULL) {
> + if ((eh = ibuf_getdata(ibuf, sizeof(*eh))) == NULL) {
> log_debug("short packet");
> return (-1);
> }
> @@ -86,6 +86,9 @@ packet_input(struct switchd *sc, struct
>
> if (dstport)
> *dstport = dst == NULL ? OFP_PORT_ANY : dst->mac_port;
> +
> + pkt->pkt_eh = eh;
> + pkt->pkt_buf = (uint8_t *) eh;
>
> return (0);
> }
--
Re: switchd(8): fix memory leak and loop
On Thu, Sep 22, 2016 at 09:46:50PM +0200, Rafael Zalamena wrote:
> This diff fixes a memory leak in ofp_read() that happens in every message
> and a infinite loop that happens when the remote switch closes the
> connection.
>
> ok?
>
I'm planning to replace this part of the code (network I/O).
but OK reyk@
> Index: ofp.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 ofp.c
> --- ofp.c 14 Sep 2016 13:46:51 - 1.7
> +++ ofp.c 21 Sep 2016 11:59:46 -
> @@ -146,6 +146,7 @@ void
> ofp_close(struct switch_connection *con)
> {
> log_info("%s: connection %u closed", __func__, con->con_id);
> + event_del(&con->con_ev);
> switch_remove(con->con_sc, con->con_switch);
> close(con->con_fd);
> TAILQ_REMOVE(&conn_head, con, con_next);
> @@ -203,7 +204,7 @@ ofp_read(int fd, short event, void *arg)
> if ((len = read(fd, buf, sizeof(buf))) == -1)
> goto fail;
> if (len == 0)
> - return;
> + goto fail;
>
> if ((ibuf = ibuf_new(buf, len)) == NULL)
> goto fail;
> @@ -236,6 +237,7 @@ ofp_read(int fd, short event, void *arg)
> goto fail;
> }
>
> + ibuf_release(ibuf);
> return;
>
> fail:
--
Re: switchd(8): more debug messages
On Thu, Sep 22, 2016 at 09:45:05PM +0200, Rafael Zalamena wrote: > Enable more debug messages to help developing the flow modification messages. > > ok? > OK, sure. Reyk > Index: ofp13.c > === > RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v > retrieving revision 1.5 > diff -u -p -r1.5 ofp13.c > --- ofp13.c 21 Jul 2016 14:25:36 - 1.5 > +++ ofp13.c 22 Sep 2016 18:35:50 - > @@ -291,6 +347,15 @@ ofp13_validate_error(struct switchd *sc, > case OFP_ERRTYPE_FLOW_MOD_FAILED: > code = print_map(ntohs(err->err_code), ofp_errflowmod_map); > break; > + case OFP_ERRTYPE_BAD_MATCH: > + code = print_map(ntohs(err->err_code), ofp_errmatch_map); > + break; > + case OFP_ERRTYPE_BAD_INSTRUCTION: > + code = print_map(ntohs(err->err_code), ofp_errinst_map); > + break; > + case OFP_ERRTYPE_BAD_REQUEST: > + code = print_map(ntohs(err->err_code), ofp_errreq_map); > + break; > default: > code = NULL; > break; > Index: ofp_map.h > === > RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp_map.h,v > retrieving revision 1.3 > diff -u -p -r1.3 ofp_map.h > --- ofp_map.h 20 Jul 2016 19:57:54 - 1.3 > +++ ofp_map.h 21 Sep 2016 13:43:30 - > @@ -52,5 +52,8 @@ extern struct constmap ofp_flowcmd_map[] > extern struct constmap ofp_flowflag_map[]; > extern struct constmap ofp_errtype_map[]; > extern struct constmap ofp_errflowmod_map[]; > +extern struct constmap ofp_errmatch_map[]; > +extern struct constmap ofp_errinst_map[]; > +extern struct constmap ofp_errreq_map[]; > > #endif /* _SWITCHD_OFP_MAP_H */ > --
Remove more duplicated includes
Hi tech@, A few remaining duplicated includes to remove. Comments? OK? Index: sys/arch/sparc64/include/asm.h === RCS file: /cvs/src/sys/arch/sparc64/include/asm.h,v retrieving revision 1.10 diff -u -p -r1.10 asm.h --- sys/arch/sparc64/include/asm.h 27 May 2016 16:32:39 - 1.10 +++ sys/arch/sparc64/include/asm.h 22 Sep 2016 14:08:50 - @@ -41,11 +41,6 @@ #ifndef _MACHINE_ASM_H_ #define _MACHINE_ASM_H_ -#ifndef _LOCORE -#define _LOCORE -#endif -#include - /* Pull in CCFSZ, CC64FSZ, and BIAS from frame.h */ #ifndef _LOCORE #define _LOCORE Index: sys/dev/pci/ixgbe.h === RCS file: /cvs/src/sys/dev/pci/ixgbe.h,v retrieving revision 1.22 diff -u -p -r1.22 ixgbe.h --- sys/dev/pci/ixgbe.h 15 Mar 2016 16:45:52 - 1.22 +++ sys/dev/pci/ixgbe.h 22 Sep 2016 14:08:51 - @@ -62,10 +62,6 @@ #include #include -#if NBPFILTER > 0 -#include -#endif - typedef intboolean_t; #define TRUE 1 #define FALSE 0 Index: usr.sbin/snmpd/snmpd.h === RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v retrieving revision 1.67 diff -u -p -r1.67 snmpd.h --- usr.sbin/snmpd/snmpd.h 16 Aug 2016 18:41:57 - 1.67 +++ usr.sbin/snmpd/snmpd.h 22 Sep 2016 14:08:51 - @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include
switchd(8): set the pktbuf for packet_in messages
The pkt_buf variable is never set in incoming packet_in messages and this
diff fixes it.
ok?
Index: packet.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/packet.c,v
retrieving revision 1.3
diff -u -p -r1.3 packet.c
--- packet.c21 Jul 2016 08:39:23 - 1.3
+++ packet.c21 Sep 2016 11:33:44 -
@@ -63,7 +63,7 @@ packet_input(struct switchd *sc, struct
return (-1);
pkt->pkt_len = ibuf_dataleft(ibuf);
- if ((pkt->pkt_eh = eh = ibuf_getdata(ibuf, sizeof(*eh))) == NULL) {
+ if ((eh = ibuf_getdata(ibuf, sizeof(*eh))) == NULL) {
log_debug("short packet");
return (-1);
}
@@ -86,6 +86,9 @@ packet_input(struct switchd *sc, struct
if (dstport)
*dstport = dst == NULL ? OFP_PORT_ANY : dst->mac_port;
+
+ pkt->pkt_eh = eh;
+ pkt->pkt_buf = (uint8_t *) eh;
return (0);
}
switchd(8): fix memory leak and loop
This diff fixes a memory leak in ofp_read() that happens in every message
and a infinite loop that happens when the remote switch closes the
connection.
ok?
Index: ofp.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp.c,v
retrieving revision 1.7
diff -u -p -r1.7 ofp.c
--- ofp.c 14 Sep 2016 13:46:51 - 1.7
+++ ofp.c 21 Sep 2016 11:59:46 -
@@ -146,6 +146,7 @@ void
ofp_close(struct switch_connection *con)
{
log_info("%s: connection %u closed", __func__, con->con_id);
+ event_del(&con->con_ev);
switch_remove(con->con_sc, con->con_switch);
close(con->con_fd);
TAILQ_REMOVE(&conn_head, con, con_next);
@@ -203,7 +204,7 @@ ofp_read(int fd, short event, void *arg)
if ((len = read(fd, buf, sizeof(buf))) == -1)
goto fail;
if (len == 0)
- return;
+ goto fail;
if ((ibuf = ibuf_new(buf, len)) == NULL)
goto fail;
@@ -236,6 +237,7 @@ ofp_read(int fd, short event, void *arg)
goto fail;
}
+ ibuf_release(ibuf);
return;
fail:
switchd(8): more debug messages
Enable more debug messages to help developing the flow modification messages. ok? Index: ofp13.c === RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v retrieving revision 1.5 diff -u -p -r1.5 ofp13.c --- ofp13.c 21 Jul 2016 14:25:36 - 1.5 +++ ofp13.c 22 Sep 2016 18:35:50 - @@ -291,6 +347,15 @@ ofp13_validate_error(struct switchd *sc, case OFP_ERRTYPE_FLOW_MOD_FAILED: code = print_map(ntohs(err->err_code), ofp_errflowmod_map); break; + case OFP_ERRTYPE_BAD_MATCH: + code = print_map(ntohs(err->err_code), ofp_errmatch_map); + break; + case OFP_ERRTYPE_BAD_INSTRUCTION: + code = print_map(ntohs(err->err_code), ofp_errinst_map); + break; + case OFP_ERRTYPE_BAD_REQUEST: + code = print_map(ntohs(err->err_code), ofp_errreq_map); + break; default: code = NULL; break; Index: ofp_map.h === RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp_map.h,v retrieving revision 1.3 diff -u -p -r1.3 ofp_map.h --- ofp_map.h 20 Jul 2016 19:57:54 - 1.3 +++ ofp_map.h 21 Sep 2016 13:43:30 - @@ -52,5 +52,8 @@ extern struct constmap ofp_flowcmd_map[] extern struct constmap ofp_flowflag_map[]; extern struct constmap ofp_errtype_map[]; extern struct constmap ofp_errflowmod_map[]; +extern struct constmap ofp_errmatch_map[]; +extern struct constmap ofp_errinst_map[]; +extern struct constmap ofp_errreq_map[]; #endif /* _SWITCHD_OFP_MAP_H */
dwc2 cleanup diff
Essentially fixes for clang warnings. Clang doesn't like the
sc = sc;
statements in dwc2_shutdown(), dwc2_childdet() and dwc2_activate().
Those functions aren't actually used. This diff moves them inside the
#if 0 that's already there for dwc2_suspend() and dwc2_resume(). But
perhaps I should just remove the code altogether?
Index: dev/usb/dwc2/dwc2.c
===
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2.c,v
retrieving revision 1.37
diff -u -p -r1.37 dwc2.c
--- dev/usb/dwc2/dwc2.c 15 Sep 2016 02:00:18 - 1.37
+++ dev/usb/dwc2/dwc2.c 23 Sep 2016 11:43:30 -
@@ -1493,6 +1493,7 @@ dwc2_detach(struct dwc2_softc *sc, int f
return rv;
}
+#if 0
bool
dwc2_shutdown(struct device *self, int flags)
{
@@ -1521,7 +1522,6 @@ dwc2_activate(struct device *self, int a
return 0;
}
-#if 0
bool
dwc2_resume(struct device *dv, const pmf_qual_t *qual)
{
Index: dev/usb/dwc2/dwc2var.h
===
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2var.h,v
retrieving revision 1.16
diff -u -p -r1.16 dwc2var.h
--- dev/usb/dwc2/dwc2var.h 3 Sep 2015 14:22:27 - 1.16
+++ dev/usb/dwc2/dwc2var.h 23 Sep 2016 11:43:30 -
@@ -120,10 +120,10 @@ int dwc2_dma_config(struct dwc2_softc *
struct dwc2_core_dma_config *);
intdwc2_intr(void *);
intdwc2_detach(dwc2_softc_t *, int);
+#if 0
bool dwc2_shutdown(struct device *, int);
void dwc2_childdet(struct device *, struct device *);
intdwc2_activate(struct device *, int);
-#if 0
bool dwc2_resume(struct device *, const pmf_qual_t *);
bool dwc2_suspend(struct device *, const pmf_qual_t *);
#endif
Some arm cleanups suggested by clang
Most of these are warnings about static symbols that aren't used. The
pmap_get_pde_pte() bit fixes:
../../../../arch/arm/arm/pmap7.c:2220:10: warning: comparison of array
'pm->pm_l2' equal to a null pointer is always false
[-Wtautological-pointer-compare]
if (pm->pm_l2 == NULL)
ok?
Index: arch/arm/arm/cpu.c
===
RCS file: /cvs/src/sys/arch/arm/arm/cpu.c,v
retrieving revision 1.32
diff -u -p -r1.32 cpu.c
--- arch/arm/arm/cpu.c 14 Aug 2016 11:30:54 - 1.32
+++ arch/arm/arm/cpu.c 23 Sep 2016 11:32:51 -
@@ -103,16 +103,6 @@ static const char * const pxa2x0_steppin
"rev 12", "rev 13", "rev 14", "rev 15"
};
-/* Steppings for PXA255/26x.
- * rev 5: PXA26x B0, rev 6: PXA255 A0
- */
-static const char * const pxa255_steppings[16] = {
- "rev 0","rev 1","rev 2","step A-0",
- "rev 4","step B-0", "step A-0", "rev 7",
- "rev 8","rev 9","rev 10", "rev 11",
- "rev 12", "rev 13", "rev 14", "rev 15"
-};
-
/* Steppings for PXA270 */
static const char * const pxa27x_steppings[16] = {
"step A-0", "step A-1", "step B-0", "step B-1",
Index: arch/arm/arm/pmap7.c
===
RCS file: /cvs/src/sys/arch/arm/arm/pmap7.c,v
retrieving revision 1.52
diff -u -p -r1.52 pmap7.c
--- arch/arm/arm/pmap7.c15 Sep 2016 02:00:17 - 1.52
+++ arch/arm/arm/pmap7.c23 Sep 2016 11:32:51 -
@@ -446,26 +446,12 @@ pmap_tlb_flushID_SE(pmap_t pm, vaddr_t v
}
static __inline void
-pmap_tlb_flushD_SE(pmap_t pm, vaddr_t va)
-{
- if (pmap_is_current(pm))
- cpu_tlb_flushD_SE(va);
-}
-
-static __inline void
pmap_tlb_flushID(pmap_t pm)
{
if (pmap_is_current(pm))
cpu_tlb_flushID();
}
-static __inline void
-pmap_tlb_flushD(pmap_t pm)
-{
- if (pmap_is_current(pm))
- cpu_tlb_flushD();
-}
-
/*
* Returns a pointer to the L2 bucket associated with the specified pmap
* and VA, or NULL if no L2 bucket exists for the address.
@@ -2217,11 +2203,7 @@ pmap_get_pde_pte(pmap_t pm, vaddr_t va,
return (TRUE);
}
- if (pm->pm_l2 == NULL)
- return (FALSE);
-
l2 = pm->pm_l2[L2_IDX(l1idx)];
-
if (l2 == NULL ||
(ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL) {
return (FALSE);
Index: arch/arm/cortex/agtimer.c
===
RCS file: /cvs/src/sys/arch/arm/cortex/agtimer.c,v
retrieving revision 1.7
diff -u -p -r1.7 agtimer.c
--- arch/arm/cortex/agtimer.c 10 Aug 2016 06:51:57 - 1.7
+++ arch/arm/cortex/agtimer.c 23 Sep 2016 11:32:51 -
@@ -126,16 +126,6 @@ agtimer_set_ctrl(uint32_t val)
}
static inline int
-agtimer_get_tval(void)
-{
- uint32_t val;
-
- __asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
-
- return (val);
-}
-
-static inline int
agtimer_set_tval(uint32_t val)
{
__asm volatile("mcr p15, 0, %[val], c14, c2, 0" : :
Re: timeout_set_proc(9)
Am Mittwoch, 21. September 2016 schrieb Martin Pieuchot : > On 21/09/16(Wed) 16:29, David Gwynne wrote: > > [...] > > the point i was trying to make was that the existing stuff (tasks, > timeouts) can be used together to get the effect we want. my point was very > poorly made though. > > > > i think your point is that you can make a clever change to timeouts and > not have to do a ton of flow on code changes to take advantage of it. > > I'm trying to fix one problem at a time. > > > [...] > > if timeouts are the way to schedule stuff to run in the future, then > we're going to get head-of-line blocking problems. pending timeouts will > end up stuck behind work that is waiting on an arbitrary lock, because > there's an implicit single thread that will run them all. right now that is > mitigated by timeouts being an interrupt context, we just dont put a lot of > work like that in there right now. > > Really? Is it worth than it is now with the KERNEL_LOCK()? > > > the benefit of taskqs is that you explicitly steer work to threads that > can sleep independently of each other. they lack being able to schedule > work to run in the future though. > > > > it turns out it isnt that hard to make taskqs use a priority queue > internally instead of a tailq. this allows you to specify that tasks get > executed in the future (or right now, like the current behaviour) in an > explicit thread (or pool of threads). it does mean a lot of changes to code > thats using timeouts now though. > > I agree with you, but these thoughts are IMHO too far ahead. Everything > is still serialized in our kernel. > > The diff as it is will deadlock against SCHED_LOCK. tsleep() calls sleep_setup(), which grabs SCHED_LOCK, Then sleep_setup_timeout() will grab timeout_mutex in timeout_add() On softclock() you have the opposite: Grabs timeout_mutex then does a wakeup, which grabs SCHED_LOCK.
