Re: n_time in trpt(8)

[Stefan Sperling]

On Wed, Dec 21, 2016 at 10:01:28PM -0500, Lawrence Teo wrote:
> In 2014, mpi@ substituted n_time, n_long, and n_short with their equivalent
> u_int_* types throughout the network stack to remove the dependency on
> :
> 
> http://marc.info/?l=openbsd-tech=140523875001860=2
> 
> As mentioned in his mail, trpt(8) is the only program in userland that uses
> n_time.  The following diff does the final cleanup for trpt(8).
> 
> ok?

OK stsp@

I believe we prefer uint32_t over u_int32_t in new code.
I'm fine with either form in this case.


> 
> 
> Index: trpt.c
> ===
> RCS file: /cvs/src/usr.sbin/trpt/trpt.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 trpt.c
> --- trpt.c27 Aug 2016 01:50:07 -  1.33
> +++ trpt.c1 Dec 2016 19:17:38 -
> @@ -73,7 +73,6 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -112,7 +111,7 @@ int   tcp_debx;
>  struct   tcp_debug tcp_debug[TCP_NDEBUG];
>  
>  static caddr_t tcp_pcbs[TCP_NDEBUG];
> -static n_time ntime;
> +static u_int32_t ntime;
>  static int aflag, follow, sflag, tflag;
>  
>  extern   char *__progname;
> 

n_time in trpt(8)

[Lawrence Teo]

In 2014, mpi@ substituted n_time, n_long, and n_short with their equivalent
u_int_* types throughout the network stack to remove the dependency on
:

http://marc.info/?l=openbsd-tech=140523875001860=2

As mentioned in his mail, trpt(8) is the only program in userland that uses
n_time.  The following diff does the final cleanup for trpt(8).

ok?


Index: trpt.c
===
RCS file: /cvs/src/usr.sbin/trpt/trpt.c,v
retrieving revision 1.33
diff -u -p -r1.33 trpt.c
--- trpt.c  27 Aug 2016 01:50:07 -  1.33
+++ trpt.c  1 Dec 2016 19:17:38 -
@@ -73,7 +73,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -112,7 +111,7 @@ int tcp_debx;
 struct tcp_debug tcp_debug[TCP_NDEBUG];
 
 static caddr_t tcp_pcbs[TCP_NDEBUG];
-static n_time ntime;
+static u_int32_t ntime;
 static int aflag, follow, sflag, tflag;
 
 extern char *__progname;

Re: Minor improvement to socket(2) man page

[Ingo Schwarze]

Hi,

Michael McConville wrote on Wed, Dec 21, 2016 at 08:17:20PM -0500:

> Before I do the busy-work: do the man page gurus have a preferred
> phrasing? I prefer fhopen(2)'s (listed above), which seems to be the
> most common.

The preferred version is something close to what .Rv produces:

  schwarze@isnote $ echo ".Rv\n.Pp\n.Rv foo" | mandoc -mdoc

  Upon successful completion, the value 0 is returned; otherwise the
  value -1 is returned and the global variable errno is set to indicate
  the error.

  The foo() function returns the value 0 if successful; otherwise the
  value -1 is returned and the global variable errno is set to indicate
  the error.

That gives, for a page with one function, or if all functions have
the same return values, taking accept(2) as an example:

  Upon successful completion, a non-negative integer representing
  the file descriptor for the accepted socket is returned;
  otherwise the value -1 is returned and the global variable
  .Va errno
  is set to indicate the error.

I think fhopen(3) can remain as it is, even though it differs in a
few very small details.

Yours,
  Ingo

Re: snmpd improvements

[Emille Blanc]

On 16-12-21 9:41 AM, Jan Klemkow wrote:

On Wed, Dec 21, 2016 at 10:40:48AM +0100, Franco Fichtner wrote:
The snmpd.conf can contain static values. If these values

are rewritten/changed over time by rewriting the config,
snmpd needs to be restarted.  Is there a technical reason
for not supporting e.g. reload using HUP?

I'm only looking for input, can provide patches with a bit
of help.  :)

The HUP feature isn't that import for the OpenSNMPd because it does not
cost that much time to reload the information out of the kernel and
the daemon don't keep stat from it's clients.  In the case of the
OpenOSPF its much more important because it take a while to restart.
And during the starting time the router would not be available.


As a frequent user of SNMP, and OpenBSD's snmpd in particular, I could 
see the HUP feature being immensely useful.  Restarting the snmpd daemon 
resets the timeticks value to zero, which appears as a "reboot" to the 
outsider.
However, in lieu of graceful restart to pick up static changes in 
snmpd.conf, I could see perhaps changing timeticks to reference the host 
system uptime counter instead as an alternative, then restarting it 
would be less of an issue on my end.
However, I'm sure there are legitimate reasons for things to be the way 
they are now, such as portability since everyone's uptime will be 
somewhere different.


Something that's been on my list of things to look at, should I ever 
find my coder hat ever again (it's been missing somewhere for quite some 
time now).


--
http://blog.sarlok.com/
Sometimes all the left hand needs to know is where the right hand is, so it 
knows where to point the blame.

Re: Minor improvement to socket(2) man page

[Michael McConville]

Philip Guenther wrote:
> On Wed, Sep 21, 2016 at 6:23 PM, Michael McConville  wrote:
> > The current version is somewhat awkward and forgets to mention that
> > errno is set. I adapted the paragraph found in most other system call
> > man pages.
> 
> There are six syscalls that return an fd on success...and their
> manpages all have totally different wording for the return value.
> Whee.
> 
> accept
>  The call returns -1 on error.  If it succeeds, it returns a non-negative
>  integer that is a descriptor for the accepted socket.
> 
> fhopen
>  Upon successful completion, fhopen() returns the file descriptor for the
>  opened file, while fhstat() and fhstatfs() return 0.  Otherwise, -1 is
>  returned and errno is set to indicate the error.
> 
> kqueue
>  kqueue() creates a new kernel event queue and returns a file descriptor.
>  If there was an error creating the kernel event queue, a value of -1 is
>  returned and errno set.
> 
> open
>  If successful, open() returns a non-negative integer, termed a file
>  descriptor.  Otherwise, a value of -1 is returned and errno is set to
>  indicate the error.
> 
> openat: same manpage as open but totally unmentioned in the RETURN
> VALUES section
> 
> socket
>  A -1 is returned if an error occurs, otherwise the return value is a
>  descriptor referencing the socket.
> 
>   Some consistency that doesn't sacrifice clarity would be nice...

Before I do the busy-work: do the man page gurus have a preferred
phrasing? I prefer fhopen(2)'s (listed above), which seems to be the
most common.

Re: NET_LOCK() pflow panic

[Darren Tucker]

On Thu, Dec 22, 2016 at 11:41 AM, Alexander Bluhm
 wrote:
> On Thu, Dec 22, 2016 at 11:32:26AM +1100, Darren Tucker wrote:
>> > I don't have a solution for the moment and I want to be sure we know all
>> > recursions before trying to write a fix.  So here's a diff that mark the
>> > recursions with a XXXSMP like in the NFS case.
>
> Looks like visa@'s crash.  I have already OKed his fix.

Thanks, I'll give that diff a go.  (The "XXXSMP" comment mislead me
since my problem was observed on a single processor machine.)

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.

Re: NET_LOCK() pflow panic

[Alexander Bluhm]

On Thu, Dec 22, 2016 at 11:32:26AM +1100, Darren Tucker wrote:
> > I don't have a solution for the moment and I want to be sure we know all
> > recursions before trying to write a fix.  So here's a diff that mark the
> > recursions with a XXXSMP like in the NFS case.

Looks like visa@'s crash.  I have already OKed his fix.

bluhm

> login: panic: rw_enter: netlock locking against myself
> Stopped at  Debugger+0x9:   leave
>TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
> *389277  87361500 0x3  00  ssh
> Debugger() at Debugger+0x9
> panic() at panic+0xfe
> rw_enter() at rw_enter+0x1c1
> sosend() at sosend+0x114
> nfs_send() at nfs_send+0x60
> nfs_request() at nfs_request+0x408
> nfs_lookup() at nfs_lookup+0x28c
> VOP_LOOKUP() at VOP_LOOKUP+0x39
> vfs_lookup() at vfs_lookup+0x2fe
> namei() at namei+0x346
> unp_connect() at unp_connect+0xd4
> uipc_usrreq() at uipc_usrreq+0x26f
> soconnect() at soconnect+0xb1
> sys_connect() at sys_connect+0x1c2

Re: NET_LOCK() pr_sysctl

[Mateusz Guzik]

On Tue, Dec 20, 2016 at 05:37:20PM +, Alexander Bluhm wrote:
> Obviosly a NET_LOCK() is missing in tcp_sysctl().
> 
> I think it is better to place the lock into net_sysctl() where all
> the protocol sysctls are called via pr_sysctl.  Then we don't have
> to decide each case individually.  As calling sysctl(2) is in the
> slow path, doing fine grained locking has no benefit.  Many sysctl
> cases copy out a struct.  Having a lock around that keeps the struct
> consistent.
> 

Holding locks across copyout/copyin is always fishy.

In this particular case, what happens if the access results in a page
fault and the area comes from a nfs mapped file? If network i/o is done
from the same context, this should result in 'locking against myself'
assertion failure.

That said, I'm not exactly familiar with the area, so maybe that's a
false alarm.

-- 
Mateusz Guzik 

Re: NET_LOCK() pflow panic

[Darren Tucker]

Hi.

On Tue, Dec 20, 2016 at 03:28:38PM +0100, Martin Pieuchot wrote:
[...]
> I don't have a solution for the moment and I want to be sure we know all
> recursions before trying to write a fix.  So here's a diff that mark the
> recursions with a XXXSMP like in the NFS case.

I think I have another case of this.  (Note that it's a single processor
VM, not SMP).  So far it's reproduceable 2/2 so if you need any
additional info or testing please let me know.

panic: rw_enter: netlock locking against myself
Stopped at  Debugger+0x9:   leave
   TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
*389277  87361500 0x3  00  ssh
Debugger() at Debugger+0x9
panic() at panic+0xfe
rw_enter() at rw_enter+0x1c1
sosend() at sosend+0x114

Full dmesg and trace below.

Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2016 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.0-current (GENERIC) #61: Tue Dec 20 15:30:01 MST 2016
bu...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC
real mem = 2130698240 (2031MB)
avail mem = 2061586432 (1966MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf64e0 (10 entries)
bios0: vendor Bochs version "Bochs" date 01/01/2011
bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP SSDT APIC HPET
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: QEMU Virtual CPU version 2.5+, 2400.57 MHz
cpu0: 
FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,VMX,CX16,x2APIC,HV,NXE,LONG,LAHF
cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 999MHz
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
acpihpet0 at acpi0: 1 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
"ACPI0006" at acpi0 not configured
"PNP0303" at acpi0 not configured
"PNP0F13" at acpi0 not configured
"PNP0700" at acpi0 not configured
"PNP0501" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
pvbus0 at mainbus0: KVM
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel 82441FX" rev 0x02
pcib0 at pci0 dev 1 function 0 "Intel 82371SB ISA" rev 0x00
pciide0 at pci0 dev 1 function 1 "Intel 82371SB IDE" rev 0x00: DMA, channel 0 
wired to compatibility, channel 1 wired to compatibility
pciide0: channel 0 disabled (no drives)
atapiscsi0 at pciide0 channel 1 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  ATAPI 5/cdrom removable
cd0(pciide0:1:0): using PIO mode 4, DMA mode 2
piixpm0 at pci0 dev 1 function 3 "Intel 82371AB Power" rev 0x03: apic 0 int 9
iic0 at piixpm0
vga1 at pci0 dev 2 function 0 "Cirrus Logic CL-GD5446" rev 0x00
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
virtio0 at pci0 dev 3 function 0 "Qumranet Virtio RNG" rev 0x00
viornd0 at virtio0
virtio0: apic 0 int 11
virtio1 at pci0 dev 4 function 0 "Qumranet Virtio Network" rev 0x00
vio0 at virtio1: address 52:54:00:f6:02:ea
virtio1: msix shared
virtio2 at pci0 dev 5 function 0 "Qumranet Virtio Storage" rev 0x00
vioblk0 at virtio2
scsibus2 at vioblk0: 2 targets
sd0 at scsibus2 targ 0 lun 0:  SCSI3 0/direct fixed
sd0: 16384MB, 512 bytes/sector, 33554432 sectors
virtio2: msix shared
virtio3 at pci0 dev 6 function 0 "Qumranet Virtio Memory" rev 0x00
viomb0 at virtio3
virtio3: apic 0 int 10
virtio4 at pci0 dev 7 function 0 "Qumranet Virtio Storage" rev 0x00
vioblk1 at virtio4
scsibus3 at vioblk1: 2 targets
sd1 at scsibus3 targ 0 lun 0:  SCSI3 0/direct fixed
sd1: 16384MB, 512 bytes/sector, 33554432 sectors
virtio4: msix shared
isa0 at pcib0
isadma0 at isa0
fdc0 at isa0 port 0x3f0/6 irq 6 drq 2
fd0 at fdc0 drive 1: density unknown
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
com0: console
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
vmm0 at mainbus0: VMX/EPT
vscsi0 at root
scsibus4 at vscsi0: 256 targets
softraid0 at root
scsibus5 at softraid0: 256 targets
root on sd0a (1afc9f32ece695a9.a) swap on sd0b dump on sd0b
Automatic boot in progress: starting file system checks.
/dev/rsd0a: file system is clean; not 

crtbegin.o and .ctors

[Mark Kettenis]

Our crtbegin.o has:

[ 8] .ctorsPROGBITS 000248 04 00   A  0   0  4

whereas crtend.o has:

[ 8] .ctorsPROGBITS 48 04 00  WA  0   0  4

So in crtbegin.o the .ctors section is read-only while in crtend.o it
is writable.  Our current linker concatenates those sections as
intended into a writable section in the output binary.  But lld, the
llvm linker, creates two separate sections, which causes the output
binary to crash as the code that runs constructors doesn't encounter
the sentinel from crtend.o.  The Solaris linker exhibits the same
behaviour.

Turns out that the ELF spec actually suggests that lld is right here
as the section flags are different.  Therefore I think we should apply
the following diff.

ok?


Index: lib/csu/crtbegin.c
===
RCS file: /cvs/src/lib/csu/crtbegin.c,v
retrieving revision 1.20
diff -u -p -r1.20 crtbegin.c
--- lib/csu/crtbegin.c  10 Nov 2015 04:14:03 -  1.20
+++ lib/csu/crtbegin.c  21 Dec 2016 23:36:09 -
@@ -84,9 +84,9 @@ __asm(".hidden  __dso_handle");
 long __guard_local __dso_hidden 
__attribute__((section(".openbsd.randomdata")));
 
 
-static const init_f __CTOR_LIST__[1]
+static init_f __CTOR_LIST__[1]
 __attribute__((section(".ctors"))) = { (void *)-1 };   /* XXX */
-static const init_f __DTOR_LIST__[1]
+static init_f __DTOR_LIST__[1]
 __attribute__((section(".dtors"))) = { (void *)-1 };   /* XXX */
 
 static void__dtors(void) __used;

splassert: ip_output: want 1 have 0

[Mark Kettenis]

splassert: ip_output: want 1 have 0
Starting stack trace...
ip_output() at ip_output+0x7d
ipsp_process_done() at ipsp_process_done+0x2ad
esp_output_cb() at esp_output_cb+0x135
taskq_thread() at taskq_thread+0x6c
end trace frame: 0x0, count: 253
End of stack trace.

This makes no sense to me since esp_output_cb() calls
ipsp_process_done() while at splsoftnet.  What am I missing?

Re: ospf6d: handle interface MTU changes

[Sebastian Benoit]

i think this is ok

Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2016.12.21 12:08:23 +0100:
> 
> Hi,
> 
> After ospfd here's a diff to make ospf6d refresh his view of an
> interface's MTU at runtime.  This needs a fresh kernel.
> 
> The parent should pass the IFINFO message to its children first, and
> then decide to react to a possible interface change.  Like for ospfd,
> the engine runs the FSM only if the interface state has actually
> changed.
> 
> ok?
> 
> 
> Index: ospf6ctl/ospf6ctl.c
> ===
> RCS file: /d/cvs/src/usr.sbin/ospf6ctl/ospf6ctl.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 ospf6ctl.c
> --- ospf6ctl/ospf6ctl.c   5 Dec 2015 13:12:40 -   1.43
> +++ ospf6ctl/ospf6ctl.c   5 Dec 2016 22:40:53 -
> @@ -409,9 +409,10 @@ show_interface_detail_msg(struct imsg *i
>   iface->name, print_link(iface->flags));
>   printf("  Internet address %s Area %s\n",
>   log_in6addr(>addr), inet_ntoa(iface->area));
> - printf("  Link type %s, state %s",
> + printf("  Link type %s, state %s, mtu %d",
>   get_media_descr(get_ifms_type(iface->if_type)),
> - get_linkstate(iface->if_type, iface->linkstate));
> + get_linkstate(iface->if_type, iface->linkstate),
> + iface->mtu);
>   if (iface->linkstate != LINK_STATE_DOWN &&
>   iface->baudrate > 0) {
>   printf(", ");
> Index: ospf6d/kroute.c
> ===
> RCS file: /d/cvs/src/usr.sbin/ospf6d/kroute.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 kroute.c
> --- ospf6d/kroute.c   17 Jul 2015 20:12:38 -  1.48
> +++ ospf6d/kroute.c   20 Dec 2016 16:09:14 -
> @@ -729,12 +729,6 @@ if_change(u_short ifindex, int flags, st
>   return;
>   }
>  
> - isvalid = (iface->flags & IFF_UP) &&
> - LINK_STATE_IS_UP(iface->linkstate);
> -
> - if (wasvalid == isvalid)
> - return; /* nothing changed wrt validity */
> -
>   /* inform engine and rde about state change if interface is used */
>   if (iface->cflags & F_IFACE_CONFIGURED) {
>   main_imsg_compose_ospfe(IMSG_IFINFO, 0, iface,
> @@ -742,6 +736,12 @@ if_change(u_short ifindex, int flags, st
>   main_imsg_compose_rde(IMSG_IFINFO, 0, iface,
>   sizeof(struct iface));
>   }
> +
> + isvalid = (iface->flags & IFF_UP) &&
> + LINK_STATE_IS_UP(iface->linkstate);
> +
> + if (wasvalid == isvalid)
> + return; /* nothing changed wrt validity */
>  
>   /* update redistribute list */
>   RB_FOREACH(kr, kroute_tree, ) {
> Index: ospf6d/ospfe.c
> ===
> RCS file: /d/cvs/src/usr.sbin/ospf6d/ospfe.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 ospfe.c
> --- ospf6d/ospfe.c3 Sep 2016 10:25:36 -   1.49
> +++ ospf6d/ospfe.c21 Dec 2016 10:55:11 -
> @@ -260,7 +260,7 @@ ospfe_dispatch_main(int fd, short event,
>   struct imsg  imsg;
>   struct imsgev   *iev = bula;
>   struct imsgbuf  *ibuf = >ibuf;
> - int  n, stub_changed, shut = 0;
> + int  n, stub_changed, shut = 0, isvalid, wasvalid;
>   unsigned int ifindex;
>  
>   if (event & EV_READ) {
> @@ -293,11 +293,19 @@ ospfe_dispatch_main(int fd, short event,
>   if (iface == NULL)
>   fatalx("interface lost in ospfe");
>  
> + wasvalid = (iface->flags & IFF_UP) &&
> + LINK_STATE_IS_UP(iface->linkstate);
> +
>   if_update(iface, ifp->mtu, ifp->flags, ifp->if_type,
>   ifp->linkstate, ifp->baudrate);
>  
> - if ((iface->flags & IFF_UP) &&
> - LINK_STATE_IS_UP(iface->linkstate)) {
> + isvalid = (iface->flags & IFF_UP) &&
> + LINK_STATE_IS_UP(iface->linkstate);
> +
> + if (wasvalid == isvalid)
> + break;
> +
> + if (isvalid) {
>   if_fsm(iface, IF_EVT_UP);
>   log_warnx("interface %s up", iface->name);
>   } else {
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

Re: BFD: route get and route monitor

[Sebastian Benoit]

Hrvoje Popovski(hrv...@srce.hr) on 2016.12.21 22:03:56 +0100:
> On 17.12.2016. 14:05, Peter Hessler wrote:
> > Updated output, requested by Theo.  A normal get will show just the bfd
> > state, use "-bfd" to get all of the information.
> > 
> > OK?
> > 
> > $ route -n get 203.0.113.9
> >route to: 203.0.113.9
> > destination: 203.0.113.9
> >mask: 255.255.255.255
> >   interface: em1
> >  if address: 203.0.113.1
> >priority: 4 (connected)
> >   flags: 
> > BFD: async state up remote up
> >  use   mtuexpire
> >83924 0   133 
> > sockaddrs: 
> > 
> > $ route -n get 203.0.113.9 -bfd
> >route to: 203.0.113.9
> > destination: 203.0.113.9
> >mask: 255.255.255.255
> >   interface: em1
> >  if address: 203.0.113.1
> >priority: 4 (connected)
> >   flags: 
> > BFD: async state up remote up laststate down error 0
> >  diag none remote neighbor-down
> >  discr 186919089 remote 55
> >  uptime 05d 2h07m29s
> >  mintx 100 minrx 100 minecho 0 multiplier 3
> >  use   mtuexpire
> >83923 0   229 
> > sockaddrs: 
> 
> 
> Hi,
> 
> it seems that bfd is working with Force10 S4810 and Extreme Networks
> x460 switches. I can test it with cisco c6k5 if you want?

Hei,

i'm sure phessler (who might not read this for a couple of days) is happy
about any test you can do.

And thanks for doing these tests!

/Benno

Re: clang amd64 libm: declare copysign() correctly

[Philip Guenther]

On Wed, Dec 21, 2016 at 4:49 AM, Mark Kettenis  wrote:
>> Date: Wed, 21 Dec 2016 13:28:26 +0100
>> From: Christian Weisgerber 
>>
>> libm uses copysign() and copysignf() internally, but fails to declare
>> the amd64 assembly versions that way.  When built with clang, this
>> results in undefined references to _libm_copysign etc.
>>
>> Presumably gcc replaces those calls to copysign with a builtin, but
>> clang doesn't.
>
> Looks correct to me.  Hopefully guenther@ can confirm?

Looks right.  Might as well make the same change in libm/arch/i387/ so
that they stay in sync for the symbols that they provide.
ok guenther@

(Hmm, copysign(3) doesn't mention that it's in C99)

Re: BFD: route get and route monitor

[Hrvoje Popovski]

On 17.12.2016. 14:05, Peter Hessler wrote:
> Updated output, requested by Theo.  A normal get will show just the bfd
> state, use "-bfd" to get all of the information.
> 
> OK?
> 
> $ route -n get 203.0.113.9
>route to: 203.0.113.9
> destination: 203.0.113.9
>mask: 255.255.255.255
>   interface: em1
>  if address: 203.0.113.1
>priority: 4 (connected)
>   flags: 
> BFD: async state up remote up
>  use   mtuexpire
>83924 0   133 
> sockaddrs: 
> 
> $ route -n get 203.0.113.9 -bfd
>route to: 203.0.113.9
> destination: 203.0.113.9
>mask: 255.255.255.255
>   interface: em1
>  if address: 203.0.113.1
>priority: 4 (connected)
>   flags: 
> BFD: async state up remote up laststate down error 0
>  diag none remote neighbor-down
>  discr 186919089 remote 55
>  uptime 05d 2h07m29s
>  mintx 100 minrx 100 minecho 0 multiplier 3
>  use   mtuexpire
>83923 0   229 
> sockaddrs: 


Hi,

it seems that bfd is working with Force10 S4810 and Extreme Networks
x460 switches. I can test it with cisco c6k5 if you want?
Thank you.

bsdtest box is connected in this way:

192.168.11.1 - bsdtest ix0
192.168.11.2 - Dell Force10 S4810 9.11(0.0)

192.168.112.1 - bsdtest ix1
192.168.112.2 - Extreme Networks X460-48t 15.5.3.4


BFD between OpenBSD - Force10 S4810

root@bsdtest:~
# route -n get 192.168.11.2 -bfd
   route to: 192.168.11.2
destination: 192.168.11.2
   mask: 255.255.255.255
  interface: ix0
 if address: 192.168.11.1
   priority: 3 ()
  flags: 
BFD: async state up remote up laststate down error 0
 diag none remote none
 discr 2137079205 remote 4
 uptime 07m26s last state time 01s
 mintx 100 minrx 100 minecho 0 multiplier 3
 use   mtuexpire
 113 0   628
sockaddrs: 

root@bsdtest:~
# route -n monitor

sockaddrs: 
 192.168.11.2 192.168.11.1
got message of size 128 on Wed Dec 21 21:55:15 2016
RTM_BFD: bidirectional forwarding detection: len 128
BFD: async state up remote up


S4810#sh bfd neighbors detail

Session Discriminator: 4
Neighbor Discriminator: 0
Local Addr: 192.168.11.2
Local MAC Addr: 00:01:e8:8a:ea:53
Remote Addr: 192.168.11.1
Remote MAC Addr: 00:25:90:5d:ca:38
Int: TenGigabitEthernet 0/3
State: Up
Configured parameters:
 TX:  200ms, RX:  200ms, Multiplier: 3
Neighbor parameters:
 TX:0ms, RX:0ms, Multiplier: 0
Actual parameters:
 TX: 1000ms, RX: 1000ms, Multiplier: 3
Role: Active
Delete session on Down: False
Client Registered: RTM
Uptime: 00:06:49
Statistics:
 Number of packets received from neighbor: 514
 Number of packets sent to neighbor: 465
 Number of state changes: 2
 Number of messages from IFA about port state change: 0
 Number of messages communicated b/w Manager and Agent: 4



BFD between OpenBSD and Extreme

root@bsdtest:~
# route -n get 192.168.112.2 -bfd
   route to: 192.168.112.2
destination: 192.168.112.2
   mask: 255.255.255.255
  interface: ix1
 if address: 192.168.112.1
   priority: 3 ()
  flags: 
BFD: async state up remote up laststate down error 0
 diag none remote none
 discr 3953390566 remote 2
 uptime 10m17s last state time 01s
 mintx 100 minrx 100 minecho 0 multiplier 3
 use   mtuexpire
  87 0   511
sockaddrs: 

root@bsdtest:~
# route -n monitor

sockaddrs: 
 192.168.112.2 192.168.112.1
got message of size 128 on Wed Dec 21 21:56:30 2016
RTM_BFD: bidirectional forwarding detection: len 128
BFD: async state up remote up



ExtTestFW.30 # sh bfd session detail
Neighbor   : 192.168.112.1   Local   : 192.168.112.2
VR-Name: VR-Default  Interface   : obfd
Session Type   : Single Hop  State   : Up
Detect Time: 3000 ms Age : 550 ms
Discriminator (local/remote): 2 / 3953390566
Demand Mode (local/remote)  : Off / Off
Poll (local/remote) : Off / Off
Tx Interval (local/remote)  : 1000 / 1000 ms
Rx Interval (local/remote)  : 1000 / 1000 ms
oper Tx Interval: 1000 ms
oper Rx Interval: 1000 ms
Multiplier (local/remote)   : 3 / 3
Local Diag  : 0 (No Diagnostic)
Remote Diag : 0 (No Diagnostic)
Authentication  : None
Clients : STATIC
Uptime  : 00 days 00 hours 10 minutes 29 seconds
Up Count: 2
Last Valid Packet Rx: 20:52:44.173792
Last Packet Tx  : 

Re: ND6 and splsoftnet()

[Martin Pieuchot]

On 21/12/16(Wed) 18:22, Alexander Bluhm wrote:
> On Wed, Dec 21, 2016 at 05:21:53PM +0100, Alexander Bluhm wrote:
> > > @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
> > >  void
> > >  nd6_timer_work(void *null)
> > >  {
> > > - int s;
> > >   struct nd_defrouter *dr, *ndr;
> > >   struct nd_prefix *pr, *npr;
> > >   struct in6_ifaddr *ia6, *nia6;
> > > + int s;
> > >  
> > > - s = splsoftnet();
> > >   timeout_set(_timer_ch, nd6_timer, NULL);
> > >   timeout_add_sec(_timer_ch, nd6_prune);
> > >  
> > > + NET_LOCK(s);
> > > +
> > 
> > Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
> > task construct could be replaced with a timeout_set_proc(), I will
> > make a diff.
> 
> This diff replaces the nd6_timer timeout and task with a timeout
> proc.

Can we wait until the experiment is complete?  Until then I'd like
to keep timeout_set_proc() only for timeout converted because they
need the NET_LOCK().  

If we discover a flaw and need to "back out" we can simply do

#define timeout_set_proc(x) timeout_set

and

#define NET_LOCK(s) s = splsoftnet()

But if we start to convert other stuff, like the diff below does, this
wont work.

> 
> ok?
> 
> bluhm
> 
> Index: netinet6/ip6_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.172
> diff -u -p -r1.172 ip6_input.c
> --- netinet6/ip6_input.c  20 Dec 2016 18:33:43 -  1.172
> +++ netinet6/ip6_input.c  21 Dec 2016 16:49:17 -
> @@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
>  
>  struct ip6stat ip6stat;
>  
> -void ip6_init2(void *);
>  int ip6_check_rh0hdr(struct mbuf *, int *);
>  
>  int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
> @@ -157,19 +156,8 @@ ip6_init(void)
>   ip6_randomid_init();
>   nd6_init();
>   frag6_init();
> - ip6_init2(NULL);
>  
>   mq_init(_mq, 64, IPL_SOFTNET);
> -}
> -
> -void
> -ip6_init2(void *dummy)
> -{
> -
> - /* nd6_timer_init */
> - bzero(_timer_ch, sizeof(nd6_timer_ch));
> - timeout_set(_timer_ch, nd6_timer, NULL);
> - timeout_add_sec(_timer_ch, 1);
>  }
>  
>  /*
> Index: netinet6/nd6.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.199
> diff -u -p -r1.199 nd6.c
> --- netinet6/nd6.c20 Dec 2016 18:33:43 -  1.199
> +++ netinet6/nd6.c21 Dec 2016 17:21:12 -
> @@ -93,14 +93,13 @@ struct nd_prhead nd_prefix = { 0 };
>  int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
>  
>  void nd6_slowtimo(void *);
> +void nd6_timer(void *);
>  void nd6_invalidate(struct rtentry *);
>  struct llinfo_nd6 *nd6_free(struct rtentry *, int);
>  void nd6_llinfo_timer(void *);
>  
>  struct timeout nd6_slowtimo_ch;
>  struct timeout nd6_timer_ch;
> -struct task nd6_timer_task;
> -void nd6_timer_work(void *);
>  
>  int fill_drlist(void *, size_t *, size_t);
>  int fill_prlist(void *, size_t *, size_t);
> @@ -122,13 +121,13 @@ nd6_init(void)
>   /* initialization of the default router list */
>   TAILQ_INIT(_defrouter);
>  
> - task_set(_timer_task, nd6_timer_work, NULL);
> -
>   nd6_init_done = 1;
>  
>   /* start timer */
>   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
>   timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> + timeout_set_proc(_timer_ch, nd6_timer, NULL);
> + timeout_add_sec(_timer_ch, nd6_prune);
>  
>   nd6_rs_init();
>  }
> @@ -428,7 +427,7 @@ nd6_llinfo_timer(void *arg)
>   * ND6 timer routine to expire default route list and prefix list
>   */
>  void
> -nd6_timer_work(void *null)
> +nd6_timer(void *arg)
>  {
>   int s;
>   struct nd_defrouter *dr, *ndr;
> @@ -436,7 +435,6 @@ nd6_timer_work(void *null)
>   struct in6_ifaddr *ia6, *nia6;
>  
>   s = splsoftnet();
> - timeout_set(_timer_ch, nd6_timer, NULL);
>   timeout_add_sec(_timer_ch, nd6_prune);
>  
>   /* expire default router list */
> @@ -483,12 +481,6 @@ nd6_timer_work(void *null)
>   }
>   }
>   splx(s);
> -}
> -
> -void
> -nd6_timer(void *ignored_arg)
> -{
> - task_add(systq, _timer_task);
>  }
>  
>  /*
> Index: netinet6/nd6.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
> retrieving revision 1.65
> diff -u -p -r1.65 nd6.h
> --- netinet6/nd6.h28 Nov 2016 13:59:51 -  1.65
> +++ netinet6/nd6.h21 Dec 2016 16:48:46 -
> @@ -223,8 +223,6 @@ extern int nd6_debug;
>  
>  #define nd6log(x)do { if (nd6_debug) log x; } while (0)
>  
> -extern struct timeout nd6_timer_ch;
> -
>  union nd_opts {
>   struct nd_opt_hdr *nd_opt_array[9];
>   struct {
> @@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
>  struct   rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, 
> u_int);
>  

Re: snmpd improvements

[Jan Klemkow]

On Wed, Dec 21, 2016 at 10:40:48AM +0100, Franco Fichtner wrote:
> Hi,
> 
> Switching from net-snmp to OpenBSD's snmpd raised two
> issues and I'd like to know if they make sense to address:
> 
> A pid file is missing.  Would a patch for this be accepted?

As far as I know, the OpenBSD project avoids pid files for a longer
time.  Because of race conditions and other reasons.

> The snmpd.conf can contain static values.  If these values
> are rewritten/changed over time by rewriting the config,
> snmpd needs to be restarted.  Is there a technical reason
> for not supporting e.g. reload using HUP?
> 
> I'm only looking for input, can provide patches with a bit
> of help.  :)

The HUP feature isn't that import for the OpenSNMPd because it does not
cost that much time to reload the information out of the kernel and
the daemon don't keep stat from it's clients.  In the case of the
OpenOSPF its much more important because it take a while to restart.
And during the starting time the router would not be available.

Just my two cents.

bye,
Jan

Re: NET_LOCK() recursion during netboot

[Alexander Bluhm]

On Wed, Dec 21, 2016 at 04:52:32PM +, Visa Hankala wrote:
> Network boot triggers a netlock recursion:
> 
> panic
> rw_enter
> sosend  <- NET_LOCK()
> nfs_send
> nfs_request
> nfs_lookup
> VOP_LOOKUP
> vfs_lookup
> namei
> unp_connect
> uipc_usrreq
> soconnect  <- NET_LOCK()
> sys_connect
> 
> An XXXSMP workaround seems appropriate here.
> 
> OK?

I was wondering wether the unp or the nfs layer is the right place
for the workaround.  As we have another XXXSMP in uipc_usrreq()
this seems correct.

OK bluhm@

> 
> Index: kern/uipc_usrreq.c
> ===
> RCS file: src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 uipc_usrreq.c
> --- kern/uipc_usrreq.c19 Dec 2016 08:36:49 -  1.104
> +++ kern/uipc_usrreq.c21 Dec 2016 16:44:03 -
> @@ -143,7 +143,10 @@ uipc_usrreq(struct socket *so, int req, 
>   break;
>  
>   case PRU_CONNECT:
> + /* XXXSMP breaks atomicity */
> + rw_exit_write();
>   error = unp_connect(so, nam, p);
> + rw_enter_write();
>   break;
>  
>   case PRU_CONNECT2:

Re: ND6 and splsoftnet()

[Alexander Bluhm]

On Wed, Dec 21, 2016 at 05:21:53PM +0100, Alexander Bluhm wrote:
> > @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
> >  void
> >  nd6_timer_work(void *null)
> >  {
> > -   int s;
> > struct nd_defrouter *dr, *ndr;
> > struct nd_prefix *pr, *npr;
> > struct in6_ifaddr *ia6, *nia6;
> > +   int s;
> >  
> > -   s = splsoftnet();
> > timeout_set(_timer_ch, nd6_timer, NULL);
> > timeout_add_sec(_timer_ch, nd6_prune);
> >  
> > +   NET_LOCK(s);
> > +
> 
> Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
> task construct could be replaced with a timeout_set_proc(), I will
> make a diff.

This diff replaces the nd6_timer timeout and task with a timeout
proc.

ok?

bluhm

Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.172
diff -u -p -r1.172 ip6_input.c
--- netinet6/ip6_input.c20 Dec 2016 18:33:43 -  1.172
+++ netinet6/ip6_input.c21 Dec 2016 16:49:17 -
@@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
 
 struct ip6stat ip6stat;
 
-void ip6_init2(void *);
 int ip6_check_rh0hdr(struct mbuf *, int *);
 
 int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
@@ -157,19 +156,8 @@ ip6_init(void)
ip6_randomid_init();
nd6_init();
frag6_init();
-   ip6_init2(NULL);
 
mq_init(_mq, 64, IPL_SOFTNET);
-}
-
-void
-ip6_init2(void *dummy)
-{
-
-   /* nd6_timer_init */
-   bzero(_timer_ch, sizeof(nd6_timer_ch));
-   timeout_set(_timer_ch, nd6_timer, NULL);
-   timeout_add_sec(_timer_ch, 1);
 }
 
 /*
Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.199
diff -u -p -r1.199 nd6.c
--- netinet6/nd6.c  20 Dec 2016 18:33:43 -  1.199
+++ netinet6/nd6.c  21 Dec 2016 17:21:12 -
@@ -93,14 +93,13 @@ struct nd_prhead nd_prefix = { 0 };
 int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
 
 void nd6_slowtimo(void *);
+void nd6_timer(void *);
 void nd6_invalidate(struct rtentry *);
 struct llinfo_nd6 *nd6_free(struct rtentry *, int);
 void nd6_llinfo_timer(void *);
 
 struct timeout nd6_slowtimo_ch;
 struct timeout nd6_timer_ch;
-struct task nd6_timer_task;
-void nd6_timer_work(void *);
 
 int fill_drlist(void *, size_t *, size_t);
 int fill_prlist(void *, size_t *, size_t);
@@ -122,13 +121,13 @@ nd6_init(void)
/* initialization of the default router list */
TAILQ_INIT(_defrouter);
 
-   task_set(_timer_task, nd6_timer_work, NULL);
-
nd6_init_done = 1;
 
/* start timer */
timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+   timeout_set_proc(_timer_ch, nd6_timer, NULL);
+   timeout_add_sec(_timer_ch, nd6_prune);
 
nd6_rs_init();
 }
@@ -428,7 +427,7 @@ nd6_llinfo_timer(void *arg)
  * ND6 timer routine to expire default route list and prefix list
  */
 void
-nd6_timer_work(void *null)
+nd6_timer(void *arg)
 {
int s;
struct nd_defrouter *dr, *ndr;
@@ -436,7 +435,6 @@ nd6_timer_work(void *null)
struct in6_ifaddr *ia6, *nia6;
 
s = splsoftnet();
-   timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
/* expire default router list */
@@ -483,12 +481,6 @@ nd6_timer_work(void *null)
}
}
splx(s);
-}
-
-void
-nd6_timer(void *ignored_arg)
-{
-   task_add(systq, _timer_task);
 }
 
 /*
Index: netinet6/nd6.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.65
diff -u -p -r1.65 nd6.h
--- netinet6/nd6.h  28 Nov 2016 13:59:51 -  1.65
+++ netinet6/nd6.h  21 Dec 2016 16:48:46 -
@@ -223,8 +223,6 @@ extern int nd6_debug;
 
 #define nd6log(x)  do { if (nd6_debug) log x; } while (0)
 
-extern struct timeout nd6_timer_ch;
-
 union nd_opts {
struct nd_opt_hdr *nd_opt_array[9];
struct {
@@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
 struct rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, u_int);
 void nd6_setmtu(struct ifnet *);
 void nd6_llinfo_settimer(struct llinfo_nd6 *, int);
-void nd6_timer(void *);
 void nd6_purge(struct ifnet *);
 void nd6_nud_hint(struct rtentry *);
 void nd6_rtrequest(struct ifnet *, int, struct rtentry *);

Re: if attach/detach netlocks

[Alexander Bluhm]

On Tue, Dec 20, 2016 at 07:33:35PM +0100, Martin Pieuchot wrote:
> On 20/12/16(Tue) 18:47, Mike Belopuhov wrote:
> > On Tue, Dec 20, 2016 at 17:06 +0100, Martin Pieuchot wrote:
> > > 
> > > You'll need to release the lock before calling ifc->ifc_create in
> > > if_clone_create() and do the same dance in if_clone_destroy().
> > >
> > 
> > Indeed.
> > 
> > > But I think that's the way to go.  If you can create/destroy
> > > pseudo-interface without panicing we're good.
> > >
> > 
> > Seems to work here, but it feels wrong.
> 
> I don't think we can do much better, let's go with that we'll refine
> later.

And another one.

splassert: ip_output: want 1 have 0
Starting stack trace...
ip_output(1,0,d09e32c1,d05129ab,d1001000) at ip_output+0x6d
ip_output(d9766c00,d9804200,0,0,f5770c10) at ip_output+0x6d
igmp_sendpkt(d3e9d800,d3e24640,17,2e0,d3704560) at igmp_sendpkt+0x119
igmp_leavegroup(d3e24640,d09e29de,d0bbe520,d03cdcb6,0) at igmp_leavegroup+0x65
in_delmulti(d3e24640,d3704560,c0186943,f5770e74,f5770e79) at in_delmulti+0x6a
vxlan_multicast_cleanup(d3e8f000,f5770e74,770d4c,d046b999,d3e8f000) at 
vxlan_multicast_cleanup+0xbd
vxlan_clone_destroy(d3e8f000,0,c0186943,d055d8cd,d961d7a0) at 
vxlan_clone_destroy+0x2a
if_clone_destroy(f5770e74,0,d3e8f000,1,d3e8f000) at if_clone_destroy+0x7b
ifioctl(d8e6beb0,80206979,f5770e74,d96ffcc0,d96f113c) at ifioctl+0x1ed
soo_ioctl(d961ca08,80206979,f5770e74,d96ffcc0,d97b1f64) at soo_ioctl+0x21c
sys_ioctl(d96ffcc0,f5770f5c,f5770f7c,0,f5770fa8) at sys_ioctl+0x19f
syscall() at syscall+0x250

This time vxlan_multicast_cleanup() expects that it holds the lock.
vxlan_clone_destroy() does an splnet(), but ip_output() expects to
have the netlock also.

bluhm

NET_LOCK() recursion during netboot

[Visa Hankala]

Network boot triggers a netlock recursion:

panic
rw_enter
sosend  <- NET_LOCK()
nfs_send
nfs_request
nfs_lookup
VOP_LOOKUP
vfs_lookup
namei
unp_connect
uipc_usrreq
soconnect  <- NET_LOCK()
sys_connect

An XXXSMP workaround seems appropriate here.

OK?

Index: kern/uipc_usrreq.c
===
RCS file: src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.104
diff -u -p -r1.104 uipc_usrreq.c
--- kern/uipc_usrreq.c  19 Dec 2016 08:36:49 -  1.104
+++ kern/uipc_usrreq.c  21 Dec 2016 16:44:03 -
@@ -143,7 +143,10 @@ uipc_usrreq(struct socket *so, int req, 
break;
 
case PRU_CONNECT:
+   /* XXXSMP breaks atomicity */
+   rw_exit_write();
error = unp_connect(so, nam, p);
+   rw_enter_write();
break;
 
case PRU_CONNECT2:

Re: ND6 and splsoftnet()

[Alexander Bluhm]

On Wed, Dec 21, 2016 at 01:52:45PM +0100, Martin Pieuchot wrote:
> ok?

OK bluhm@, but comments inline

> @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
>  void
>  nd6_timer_work(void *null)
>  {
> - int s;
>   struct nd_defrouter *dr, *ndr;
>   struct nd_prefix *pr, *npr;
>   struct in6_ifaddr *ia6, *nia6;
> + int s;
>  
> - s = splsoftnet();
>   timeout_set(_timer_ch, nd6_timer, NULL);
>   timeout_add_sec(_timer_ch, nd6_prune);
>  
> + NET_LOCK(s);
> +

Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
task construct could be replaced with a timeout_set_proc(), I will
make a diff.


> @@ -1120,7 +1121,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
>   struct in6_nbrinfo *nbi = (struct in6_nbrinfo *)data;
>   struct rtentry *rt;
>   int error = 0;
> - int s;
>  

Could you add an assert here when you remove the splsoftnet().  This
makes further review easier and we see bugs during runtime.

> @@ -1478,12 +1471,14 @@ fail:
>  void
>  nd6_slowtimo(void *ignored_arg)
>  {
> - int s = splsoftnet();
>   struct nd_ifinfo *nd6if;
>   struct ifnet *ifp;
> + int s;
>  
>   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
>   timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> +
> + NET_LOCK(s);

Again timeout_set() without lock feels wrong.  It has been done in
nd6_init() already.  And there it is a timeout_set_proc() now.
Remove the line.

> @@ -825,7 +825,6 @@ nd6_na_input(struct mbuf *m, int off, in
>*/
>   struct nd_defrouter *dr;
>   struct in6_addr *in6;
> - int s;

Please add an assert.

>  nd6_dad_starttimer(struct dadq *dp, int ticks)
>  {
>  
> - timeout_set(>dad_timer_ch, nd6_dad_timer, dp->dad_ifa);
> + timeout_set_proc(>dad_timer_ch, nd6_dad_timer, dp->dad_ifa);
>   timeout_add(>dad_timer_ch, ticks);

Are we sure that timeout_set() is not called when the timer is
already running?  I will have a look.  You diff does not make it
worse.

> @@ -202,7 +202,7 @@ nd6_rs_output(struct ifnet* ifp, struct 
>   struct nd_router_solicit *rs;
>   struct ip6_moptions im6o;
>   caddr_t mac;
> - int icmp6len, maxlen, s;
> + int icmp6len, maxlen;

Can you put an assert here?

> @@ -1592,9 +1592,8 @@ pfxlist_onlink_check(void)
>   if ((pr->ndpr_stateflags & NDPRF_DETACHED) != 0 &&
>   (pr->ndpr_stateflags & NDPRF_ONLINK) != 0) {
>   if ((e = nd6_prefix_offlink(pr)) != 0) {
> - nd6log((LOG_ERR,
> - "pfxlist_onlink_check: failed to "
> - "make %s/%d offlink, errno=%d\n",
> + nd6log((LOG_ERR, "%s: failed to make %s/%d"
> + " offlink, errno=%d\n", __func__,
>   inet_ntop(AF_INET6,
>   >ndpr_prefix.sin6_addr,
>   addr, sizeof(addr)),
> @@ -1605,9 +1604,8 @@ pfxlist_onlink_check(void)
>   (pr->ndpr_stateflags & NDPRF_ONLINK) == 0 &&
>   pr->ndpr_raf_onlink) {
>   if ((e = nd6_prefix_onlink(pr)) != 0) {
> - nd6log((LOG_ERR,
> - "pfxlist_onlink_check: failed to "
> - "make %s/%d offlink, errno=%d\n",
> + nd6log((LOG_ERR, "%s: failed to make %s/%d"
> + " offlink, errno=%d\n", __func__,
>   inet_ntop(AF_INET6,
>   >ndpr_prefix.sin6_addr,
>   addr, sizeof(addr)),
> @@ -2000,7 +1998,7 @@ in6_init_address_ltimes(struct nd_prefix

Plaese keep the style that the space is before the " at the end
of the line.

ND6 and splsoftnet()

[Martin Pieuchot]

This diff get rids of all splsoftet() in nd6 code.

I converted timers to the NET_LOCK() even if ip6_output() doesn't assert
for it for the moment.  That's simply the way to go.  Also because these
functions iterates on global data structures that will need the NET_LOCK()
or a rewrite.

ok?

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.199
diff -u -p -r1.199 nd6.c
--- netinet6/nd6.c  20 Dec 2016 18:33:43 -  1.199
+++ netinet6/nd6.c  21 Dec 2016 12:47:31 -
@@ -127,7 +127,7 @@ nd6_init(void)
nd6_init_done = 1;
 
/* start timer */
-   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
+   timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
 
nd6_rs_init();
@@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
 void
 nd6_timer_work(void *null)
 {
-   int s;
struct nd_defrouter *dr, *ndr;
struct nd_prefix *pr, *npr;
struct in6_ifaddr *ia6, *nia6;
+   int s;
 
-   s = splsoftnet();
timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
+   NET_LOCK(s);
+
/* expire default router list */
TAILQ_FOREACH_SAFE(dr, _defrouter, dr_entry, ndr)
if (dr->expire && dr->expire < time_uptime)
@@ -482,7 +483,7 @@ nd6_timer_work(void *null)
prelist_remove(pr);
}
}
-   splx(s);
+   NET_UNLOCK(s);
 }
 
 void
@@ -1120,7 +1121,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
struct in6_nbrinfo *nbi = (struct in6_nbrinfo *)data;
struct rtentry *rt;
int error = 0;
-   int s;
 
switch (cmd) {
case SIOCGIFINFO_IN6:
@@ -1142,7 +1142,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
/* flush all the prefix advertised by routers */
struct nd_prefix *pr, *npr;
 
-   s = splsoftnet();
/* First purge the addresses referenced by a prefix. */
LIST_FOREACH_SAFE(pr, _prefix, ndpr_entry, npr) {
struct in6_ifaddr *ia6, *ia6_next;
@@ -1170,7 +1169,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
 
prelist_remove(pr);
}
-   splx(s);
break;
}
case SIOCSRTRFLUSH_IN6:
@@ -1178,12 +1176,10 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
/* flush all the default routers */
struct nd_defrouter *dr, *ndr;
 
-   s = splsoftnet();
defrouter_reset();
TAILQ_FOREACH_SAFE(dr, _defrouter, dr_entry, ndr)
defrtrlist_del(dr);
defrouter_select();
-   splx(s);
break;
}
case SIOCGNBRINFO_IN6:
@@ -1204,13 +1200,11 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
*idp = htons(ifp->if_index);
}
 
-   s = splsoftnet();
rt = nd6_lookup(_addr, 0, ifp, ifp->if_rdomain);
if (rt == NULL ||
(ln = (struct llinfo_nd6 *)rt->rt_llinfo) == NULL) {
error = EINVAL;
rtfree(rt);
-   splx(s);
break;
}
expire = ln->ln_rt->rt_expire;
@@ -1224,7 +1218,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
nbi->isrouter = ln->ln_router;
nbi->expire = expire;
rtfree(rt);
-   splx(s);
 
break;
}
@@ -1478,12 +1471,14 @@ fail:
 void
 nd6_slowtimo(void *ignored_arg)
 {
-   int s = splsoftnet();
struct nd_ifinfo *nd6if;
struct ifnet *ifp;
+   int s;
 
timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+
+   NET_LOCK(s);
TAILQ_FOREACH(ifp, , if_list) {
nd6if = ND_IFINFO(ifp);
if (nd6if->basereachable && /* already initialized */
@@ -1498,7 +1493,7 @@ nd6_slowtimo(void *ignored_arg)
nd6if->reachable = 
ND_COMPUTE_RTIME(nd6if->basereachable);
}
}
-   splx(s);
+   NET_UNLOCK(s);
 }
 
 int
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.112
diff -u -p -r1.112 nd6_nbr.c
--- netinet6/nd6_nbr.c  21 Dec 2016 12:30:19 -  1.112
+++ netinet6/nd6_nbr.c  21 Dec 2016 12:47:31 -
@@ -825,7 +825,6 @@ nd6_na_input(struct mbuf *m, int off, in
 */
struct nd_defrouter *dr;
struct in6_addr *in6;
-   int s;
 
in6 = (rt_key(rt))->sin6_addr;
 
@@ -835,7 

Re: clang amd64 libm: declare copysign() correctly

[Mark Kettenis]

> Date: Wed, 21 Dec 2016 13:28:26 +0100
> From: Christian Weisgerber 
> 
> libm uses copysign() and copysignf() internally, but fails to declare
> the amd64 assembly versions that way.  When built with clang, this
> results in undefined references to _libm_copysign etc.
> 
> Presumably gcc replaces those calls to copysign with a builtin, but
> clang doesn't.

Looks correct to me.  Hopefully guenther@ can confirm?

> Index: arch/amd64/s_copysign.S
> ===
> RCS file: /cvs/src/lib/libm/arch/amd64/s_copysign.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 s_copysign.S
> --- arch/amd64/s_copysign.S   12 Sep 2016 19:47:01 -  1.5
> +++ arch/amd64/s_copysign.S   21 Dec 2016 11:46:53 -
> @@ -6,6 +6,8 @@
>  
>  #include 
>  
> +#include "abi.h"
> +
>  .Lpos:
>   .quad   0x8000
>  .Lneg:
> @@ -18,4 +20,4 @@ ENTRY(copysign)
>   pand%xmm3,%xmm0
>   por %xmm1,%xmm0
>   ret
> -END(copysign)
> +END_STD(copysign)
> Index: arch/amd64/s_copysignf.S
> ===
> RCS file: /cvs/src/lib/libm/arch/amd64/s_copysignf.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 s_copysignf.S
> --- arch/amd64/s_copysignf.S  12 Sep 2016 19:47:01 -  1.5
> +++ arch/amd64/s_copysignf.S  21 Dec 2016 11:48:56 -
> @@ -6,6 +6,8 @@
>  
>  #include 
>  
> +#include "abi.h"
> +
>  .Lneg:
>   .long 0x7fff
>  .Lpos:
> @@ -18,4 +20,4 @@ ENTRY(copysignf)
>   pand%xmm3,%xmm0
>   por %xmm1,%xmm0
>   ret
> -END(copysignf)
> +END_STD(copysignf)
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
> 

clang amd64 libm: declare copysign() correctly

[Christian Weisgerber]

libm uses copysign() and copysignf() internally, but fails to declare
the amd64 assembly versions that way.  When built with clang, this
results in undefined references to _libm_copysign etc.

Presumably gcc replaces those calls to copysign with a builtin, but
clang doesn't.

Index: arch/amd64/s_copysign.S
===
RCS file: /cvs/src/lib/libm/arch/amd64/s_copysign.S,v
retrieving revision 1.5
diff -u -p -r1.5 s_copysign.S
--- arch/amd64/s_copysign.S 12 Sep 2016 19:47:01 -  1.5
+++ arch/amd64/s_copysign.S 21 Dec 2016 11:46:53 -
@@ -6,6 +6,8 @@
 
 #include 
 
+#include "abi.h"
+
 .Lpos:
.quad   0x8000
 .Lneg:
@@ -18,4 +20,4 @@ ENTRY(copysign)
pand%xmm3,%xmm0
por %xmm1,%xmm0
ret
-END(copysign)
+END_STD(copysign)
Index: arch/amd64/s_copysignf.S
===
RCS file: /cvs/src/lib/libm/arch/amd64/s_copysignf.S,v
retrieving revision 1.5
diff -u -p -r1.5 s_copysignf.S
--- arch/amd64/s_copysignf.S12 Sep 2016 19:47:01 -  1.5
+++ arch/amd64/s_copysignf.S21 Dec 2016 11:48:56 -
@@ -6,6 +6,8 @@
 
 #include 
 
+#include "abi.h"
+
 .Lneg:
.long 0x7fff
 .Lpos:
@@ -18,4 +20,4 @@ ENTRY(copysignf)
pand%xmm3,%xmm0
por %xmm1,%xmm0
ret
-END(copysignf)
+END_STD(copysignf)
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: sleep.1: "while true;" -> "while :;"

[Christian Weisgerber]

On 2016-12-20, Jason McIntyre  wrote:

> i'm not sure why "while :" is better in this example, but "while true"
> is clearer, i think. allowing for differences in preference, is their a
> good reason to change what's there?

I agree that "true" is clearer and I see no reason to change this.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: if attach/detach netlocks

[Alexander Bluhm]

On Wed, Dec 21, 2016 at 12:45:37PM +0100, Mike Belopuhov wrote:
> Anyways, OK for the diff below?

OK bluhm@

> diff --git sys/net/if_pflow.c sys/net/if_pflow.c
> index 0df0b69fd8a..8592d98a743 100644
> --- sys/net/if_pflow.c
> +++ sys/net/if_pflow.c
> @@ -265,14 +265,11 @@ pflow_clone_destroy(struct ifnet *ifp)
>   if (timeout_initialized(>sc_tmo_tmpl))
>   timeout_del(>sc_tmo_tmpl);
>   pflow_flush(sc);
>   m_freem(sc->send_nam);
>   if (sc->so != NULL) {
> - /* XXXSMP breaks atomicity */
> - rw_exit_write();
>   error = soclose(sc->so);
> - rw_enter_write();
>   sc->so = NULL;
>   }
>   if (sc->sc_flowdst != NULL)
>   free(sc->sc_flowdst, M_DEVBUF, sc->sc_flowdst->sa_len);
>   if (sc->sc_flowsrc != NULL)

Re: if attach/detach netlocks

[Mike Belopuhov]

On Wed, Dec 21, 2016 at 00:13 +0100, Alexander Bluhm wrote:
> On Tue, Dec 20, 2016 at 06:47:31PM +0100, Mike Belopuhov wrote:
> > @@ -1109,11 +1115,16 @@ if_clone_destroy(const char *name)
> > s = splnet();
> > if_down(ifp);
> > splx(s);
> > }
> >  
> > -   return ((*ifc->ifc_destroy)(ifp));
> > +   /* XXXSMP breaks atomicity */
> > +   rw_exit_write();
> > +   ret = (*ifc->ifc_destroy)(ifp);
> > +   rw_enter_write();
> > +
> > +   return (ret);
> >  }
> 
> This broke pflow again.
> 
> panic: netlock: lock not held
> Stopped at  Debugger+0x7:   leave
>TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
> *440943   4323  0 0x2  01  ifconfig
> Debugger(d09facfd,f57a2ca8,d09d23c6,f57a2ca8,d76fd000) at Debugger+0x7
> panic(d09d23c6,d09dc32f,f57a2cec,d76fd000,0) at panic+0x71
> rw_assert_wrlock(d0b56f38,40,f57a2cec,d03e506b,d8e1eb00) at 
> rw_assert_wrlock+0x
> 3d
> rw_exit_write(d0b56f38,0,7a2d4c,d046b949,d76fd000) at rw_exit_write+0x19
> pflow_clone_destroy(d76fd000,0,c0186943,d055d8ed,d96ad7a0) at 
> pflow_clone_destr
> oy+0x71
> if_clone_destroy(f57a2e74,0,d76fd000,1,d76fd000) at if_clone_destroy+0x7b
> ifioctl(d9587648,80206979,f57a2e74,d97e22d8,d956d26c) at ifioctl+0x1ed
> soo_ioctl(d96405a8,80206979,f57a2e74,d97e22d8,d97b1f64) at soo_ioctl+0x21c
> sys_ioctl(d97e22d8,f57a2f5c,f57a2f7c,0,f57a2fa8) at sys_ioctl+0x19f
> syscall() at syscall+0x250
> --- syscall (number 9) ---
> 0x14:
> https://www.openbsd.org/ddb.html describes the minimum info required in bug
> reports.  Insufficient info makes it difficult to find and fix bugs.
> ddb{1}> 
> 
> Now we have two conflicting XXXSMP workarounds in if_clone_destroy()
> and pflow_clone_destroy().
> 
> bluhm

With the diff above, the workaround in pflow_clone_destroy is not
needed.  The diff below fixes this for me.  However, I start doubting
this approach.  A lot of code is now run with the lock released.
While I agree that the amount of lock recursions should be minimized,
I don't think that eliminating them completely is a necessity at this
point.

Recursive rwlock would work just fine.  We can add a flag to make
rrw_enter fail to recurse for the purpose of minimizing the amount of
recursions.  Haesbaert has experimented extensively with (r)rwlocks
instead of KERNEL_LOCKs (which is what we're doing, sort of) and
found out (r)rwlocks to be about 25% slower in real life applications
(make build) and produce a ton of IPI calls due to sleep/wakeup cycles.
He then moved on to turnstile exclusive recursive locks (bmtx) [1] with
better semantics to regain performance and lower down the number of
IPIs.

Anyways, OK for the diff below?

[1] https://github.com/bitrig/bitrig/blob/smpns/sys/kern/kern_bmtx.c

diff --git sys/net/if_pflow.c sys/net/if_pflow.c
index 0df0b69fd8a..8592d98a743 100644
--- sys/net/if_pflow.c
+++ sys/net/if_pflow.c
@@ -265,14 +265,11 @@ pflow_clone_destroy(struct ifnet *ifp)
if (timeout_initialized(>sc_tmo_tmpl))
timeout_del(>sc_tmo_tmpl);
pflow_flush(sc);
m_freem(sc->send_nam);
if (sc->so != NULL) {
-   /* XXXSMP breaks atomicity */
-   rw_exit_write();
error = soclose(sc->so);
-   rw_enter_write();
sc->so = NULL;
}
if (sc->sc_flowdst != NULL)
free(sc->sc_flowdst, M_DEVBUF, sc->sc_flowdst->sa_len);
if (sc->sc_flowsrc != NULL)

ospf6d: handle interface MTU changes

[Jeremie Courreges-Anglas]

Hi,

After ospfd here's a diff to make ospf6d refresh his view of an
interface's MTU at runtime.  This needs a fresh kernel.

The parent should pass the IFINFO message to its children first, and
then decide to react to a possible interface change.  Like for ospfd,
the engine runs the FSM only if the interface state has actually
changed.

ok?


Index: ospf6ctl/ospf6ctl.c
===
RCS file: /d/cvs/src/usr.sbin/ospf6ctl/ospf6ctl.c,v
retrieving revision 1.43
diff -u -p -r1.43 ospf6ctl.c
--- ospf6ctl/ospf6ctl.c 5 Dec 2015 13:12:40 -   1.43
+++ ospf6ctl/ospf6ctl.c 5 Dec 2016 22:40:53 -
@@ -409,9 +409,10 @@ show_interface_detail_msg(struct imsg *i
iface->name, print_link(iface->flags));
printf("  Internet address %s Area %s\n",
log_in6addr(>addr), inet_ntoa(iface->area));
-   printf("  Link type %s, state %s",
+   printf("  Link type %s, state %s, mtu %d",
get_media_descr(get_ifms_type(iface->if_type)),
-   get_linkstate(iface->if_type, iface->linkstate));
+   get_linkstate(iface->if_type, iface->linkstate),
+   iface->mtu);
if (iface->linkstate != LINK_STATE_DOWN &&
iface->baudrate > 0) {
printf(", ");
Index: ospf6d/kroute.c
===
RCS file: /d/cvs/src/usr.sbin/ospf6d/kroute.c,v
retrieving revision 1.48
diff -u -p -r1.48 kroute.c
--- ospf6d/kroute.c 17 Jul 2015 20:12:38 -  1.48
+++ ospf6d/kroute.c 20 Dec 2016 16:09:14 -
@@ -729,12 +729,6 @@ if_change(u_short ifindex, int flags, st
return;
}
 
-   isvalid = (iface->flags & IFF_UP) &&
-   LINK_STATE_IS_UP(iface->linkstate);
-
-   if (wasvalid == isvalid)
-   return; /* nothing changed wrt validity */
-
/* inform engine and rde about state change if interface is used */
if (iface->cflags & F_IFACE_CONFIGURED) {
main_imsg_compose_ospfe(IMSG_IFINFO, 0, iface,
@@ -742,6 +736,12 @@ if_change(u_short ifindex, int flags, st
main_imsg_compose_rde(IMSG_IFINFO, 0, iface,
sizeof(struct iface));
}
+
+   isvalid = (iface->flags & IFF_UP) &&
+   LINK_STATE_IS_UP(iface->linkstate);
+
+   if (wasvalid == isvalid)
+   return; /* nothing changed wrt validity */
 
/* update redistribute list */
RB_FOREACH(kr, kroute_tree, ) {
Index: ospf6d/ospfe.c
===
RCS file: /d/cvs/src/usr.sbin/ospf6d/ospfe.c,v
retrieving revision 1.49
diff -u -p -r1.49 ospfe.c
--- ospf6d/ospfe.c  3 Sep 2016 10:25:36 -   1.49
+++ ospf6d/ospfe.c  21 Dec 2016 10:55:11 -
@@ -260,7 +260,7 @@ ospfe_dispatch_main(int fd, short event,
struct imsg  imsg;
struct imsgev   *iev = bula;
struct imsgbuf  *ibuf = >ibuf;
-   int  n, stub_changed, shut = 0;
+   int  n, stub_changed, shut = 0, isvalid, wasvalid;
unsigned int ifindex;
 
if (event & EV_READ) {
@@ -293,11 +293,19 @@ ospfe_dispatch_main(int fd, short event,
if (iface == NULL)
fatalx("interface lost in ospfe");
 
+   wasvalid = (iface->flags & IFF_UP) &&
+   LINK_STATE_IS_UP(iface->linkstate);
+
if_update(iface, ifp->mtu, ifp->flags, ifp->if_type,
ifp->linkstate, ifp->baudrate);
 
-   if ((iface->flags & IFF_UP) &&
-   LINK_STATE_IS_UP(iface->linkstate)) {
+   isvalid = (iface->flags & IFF_UP) &&
+   LINK_STATE_IS_UP(iface->linkstate);
+
+   if (wasvalid == isvalid)
+   break;
+
+   if (isvalid) {
if_fsm(iface, IF_EVT_UP);
log_warnx("interface %s up", iface->name);
} else {


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

snmpd improvements

[Franco Fichtner]

Hi,

Switching from net-snmp to OpenBSD's snmpd raised two
issues and I'd like to know if they make sense to address:

A pid file is missing.  Would a patch for this be accepted?

The snmpd.conf can contain static values.  If these values
are rewritten/changed over time by rewriting the config,
snmpd needs to be restarted.  Is there a technical reason
for not supporting e.g. reload using HUP?

I'm only looking for input, can provide patches with a bit
of help.  :)


Cheers,
Franco

vndcompress et al import?

[Franco Fichtner]

Hi,

Is anyone aware or interested in porting vndcompress et al
from NetBSD to OpenBSD?

Is there any technical reason against inclusion?

We have a budget for this.  If anyone is interested please
let me know.


Cheers,
Franco

Re: sleep.1: "while true;" -> "while :;"

[Jason McIntyre]

On Wed, Dec 21, 2016 at 09:59:18AM +0100, Michal Mazurek wrote:
> On 22:49:38, 20.12.16, Jason McIntyre wrote:
> > On Tue, Dec 20, 2016 at 10:58:40PM +0100, Michal Mazurek wrote:
> > > While there is nothing wrong with "while true", "while :" is better
> > > and used a lot more often in the source tree.
> > > 
> > > OK?
> > > 
> > 
> > i'm not sure why "while :" is better in this example, but "while true"
> > is clearer, i think. allowing for differences in preference, is their a
> > good reason to change what's there?
> 
> ":" is a builtin, and appears to be the proper idiom as evidenced by the
> source tree:
> 
> $ grep -r 'while true' . | wc -l
>   11
> 
> $ grep -r 'while :' . | wc -l
>  156
> 

morning.

"true" is also a built-in. the only difference i can see really is that
":" is classed as a special built-in, but in the sleep example i don;t
see that being relevant.

it is maybe not used so much in the tree but it is correct. i still
don;t see the point of the change.

jmc

Re: sleep.1: "while true;" -> "while :;"

[Michal Mazurek]

On 22:49:38, 20.12.16, Jason McIntyre wrote:
> On Tue, Dec 20, 2016 at 10:58:40PM +0100, Michal Mazurek wrote:
> > While there is nothing wrong with "while true", "while :" is better
> > and used a lot more often in the source tree.
> > 
> > OK?
> > 
> 
> i'm not sure why "while :" is better in this example, but "while true"
> is clearer, i think. allowing for differences in preference, is their a
> good reason to change what's there?

":" is a builtin, and appears to be the proper idiom as evidenced by the
source tree:

$ grep -r 'while true' . | wc -l
  11

$ grep -r 'while :' . | wc -l
 156

-- 
Michal Mazurek