fuse_parse_cmd_line.3 patch

2018-11-27 Thread Edgar Pettijohn III

remove a bunch of `_' where there shouldn't be `_'s

Index: fuse_parse_cmd_line.3
===
RCS file: /cvs/src/lib/libfuse/fuse_parse_cmd_line.3,v
retrieving revision 1.2
diff -u -p -u -r1.2 fuse_parse_cmd_line.3
--- fuse_parse_cmd_line.3    8 Jul 2018 06:17:10 -    1.2
+++ fuse_parse_cmd_line.3    28 Nov 2018 02:54:13 -
@@ -15,25 +15,25 @@
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
 .Dd $Mdocdate: July 8 2018 $
-.Dt FUSE_PARSE_CMD_LINE 3
+.Dt FUSE_PARSE_CMDLINE 3
 .Os
 .Sh NAME
-.Nm fuse_parse_cmd_line
+.Nm fuse_parse_cmdline
 .Nd FUSE helper function to parse command line arguments
 .Sh SYNOPSIS
 .In fuse.h
 .Ft int
-.Fn fuse_parse_cmd_line "struct fuse_args *args" "char **mp" \
+.Fn fuse_parse_cmdline "struct fuse_args *args" "char **mp" \
 "int *mt" "int *fg"
 .Sh DESCRIPTION
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 is a helper function to parse standard FUSE arguments.
 .Fa args
 can be constructed using the
 .Xr FUSE_ARGS_INIT 3
 macro.
 .Pp
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 supports the following arguments.
 .Bl -tag -width Ds
 .It Fl d , Fl odebug
@@ -65,13 +65,13 @@ Print the FUSE library version to stderr
 .El
 .Pp
 If the first argument not recognised by
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 is a valid directory then
 .Fa mp
 will be set to the canonicalized absolute pathname of this directory.
 .Sh RETURN VALUES
 The
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 function will return 0 on success and -1 if
 .Fl h , Fl -help , Fl ho , Fl v
 or
@@ -88,11 +88,11 @@ does not exist or is not a directory.
 .Xr fuse_setup 3
 .Sh STANDARDS
 The
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 function conforms to FUSE 2.6.
 .Sh HISTORY
 The
-.Fn fuse_parse_cmd_line
+.Fn fuse_parse_cmdline
 function first appeared in
 .Ox 5.4 .
 .Sh AUTHORS



Re: option kcov + GENERIC.MP -> silent crash

2018-11-27 Thread Greg Steuck
I booted the patched kernel and it seems to have gone farther and I believe
reached init before crashing.

boot> b bsd.anton
booting hd0a:bsd.anton: 12380226+2360336+270368+0+675840
[684182+128+754752+529898]=0x10d8f48
entry point at 0x1001000
[ using 1969992 bytes of bsd ELF symbol table ]
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2018 OpenBSD. All rights reserved.
https://www.OpenBSD.org
OpenBSD 6.4-current (SYZKALLER) #0: Tue Nov 27 17:40:55 PST 2018
syzkaller@ci-openbsd.syzkaller
:/home/syzkaller/src/sys/arch/amd64/compile/SYZKALLER
real mem = 17163079680 (16367MB)
avail mem = 16632164352 (15861MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xbcf0 (20 entries)
bios0: vendor Google version "Google" date 01/01/2011
bios0: Google Google Compute Engine
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP SSDT APIC WAET SRAT
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: Intel(R) Xeon(R) CPU @ 2.30GHz, 2300.42 MHz, 06-3f-00
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 999MHz
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.55 MHz, 06-3f-00
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.53 MHz, 06-3f-00
cpu2:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.54 MHz, 06-3f-00
cpu3:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
cpu4 at mainbus0: apid 1 (application processor)
cpu4: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.57 MHz, 06-3f-00
cpu4:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu4: 256KB 64b/line 8-way L2 cache
cpu4: smt 1, core 0, package 0
cpu5 at mainbus0: apid 3 (application processor)
cpu5: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.56 MHz, 06-3f-00
cpu5:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu5: 256KB 64b/line 8-way L2 cache
cpu5: smt 1, core 1, package 0
cpu6 at mainbus0: apid 5 (application processor)
cpu6: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.54 MHz, 06-3f-00
cpu6:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN
cpu6: 256KB 64b/line 8-way L2 cache
cpu6: smt 1, core 2, package 0
cpu7 at mainbus0: apid 7 (application processor)
cpu7: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.57 MHz, 06-3f-00
cpu7:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4.
2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF

Re: top: allow reverse sort order

2018-11-27 Thread Klemens Nanni
On Wed, Nov 28, 2018 at 12:07:37AM +0100, Klemens Nanni wrote:
> Note how an empty field is silently treated as the default field
> "state", but that's an independent issue I'd like to address in a
> separate diff for string_index().
Not a problem of string_index() actually.

We consistently handle empty input for all commands silently as no-op.



Re: top: allow reverse sort order

2018-11-27 Thread Klemens Nanni
On Tue, Nov 27, 2018 at 03:52:52PM -0600, Scott Cheloha wrote:
> > >  static int
> > > +getorder(char *field)
> > > +{
> > > + rev_order = field[0] == '-';
> > > +
> > > + return string_index(rev_order ? field + 1 : field, statics.order_names);
> > > +}
> > > +
> 
> You need to check that the field has an ordering before potentially
> modifying rev_order, otherwise a bogus input will have a side effect.
> 
> For example, with the current code the input "o -notafield" reverses
> the ordering.
Good catch.

Updated diff to fix that, rest unchanged.

Note how an empty field is silently treated as the default field
"state", but that's an independent issue I'd like to address in a
separate diff for string_index().

OK?

Index: display.c
===
RCS file: /cvs/src/usr.bin/top/display.c,v
retrieving revision 1.57
diff -u -p -r1.57 display.c
--- display.c   17 Nov 2018 23:10:08 -  1.57
+++ display.c   27 Nov 2018 21:09:56 -
@@ -817,7 +817,8 @@ show_help(void)
"I | i- toggle the display of idle processes\n"
"k [-sig] pid - send signal `-sig' to process `pid'\n"
"n|# count- show `count' processes\n"
-   "o field  - specify sort order (size, res, cpu, time, pri, pid, 
command)\n"
+   "o [-]field   - specify sort order (size, res, cpu, time, pri, pid, 
command)\n"
+   "   (o -field sorts in reverse)\n"
"P pid- highlight process `pid' (P+ switches highlighting 
off)\n"
"p pid- display process by `pid' (p+ selects all 
processes)\n"
"q- quit\n"
Index: machine.c
===
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.95
diff -u -p -r1.95 machine.c
--- machine.c   17 Nov 2018 23:10:08 -  1.95
+++ machine.c   27 Nov 2018 21:09:56 -
@@ -602,6 +602,8 @@ static unsigned char sorted_state[] =
1   /* zombie*/
 };
 
+extern int rev_order;
+
 /*
  *  proc_compares - comparison functions for "qsort"
  */
@@ -631,6 +633,17 @@ static unsigned char sorted_state[] =
 #define ORDERKEY_CMD \
if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0)
 
+/* remove one level of indirection and set sort order */
+#define SETORDER do { \
+   if (rev_order) { \
+   p1 = *(struct kinfo_proc **) pp2; \
+   p2 = *(struct kinfo_proc **) pp1; \
+   } else { \
+   p1 = *(struct kinfo_proc **) pp1; \
+   p2 = *(struct kinfo_proc **) pp2; \
+   } \
+   } while (0)
+
 /* compare_cpu - the comparison function for sorting by cpu percentage */
 static int
 compare_cpu(const void *v1, const void *v2)
@@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void *
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_PCTCPU
ORDERKEY_CPUTIME
@@ -663,9 +674,7 @@ compare_size(const void *v1, const void 
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_MEM
ORDERKEY_RSSIZE
@@ -686,9 +695,7 @@ compare_res(const void *v1, const void *
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_RSSIZE
ORDERKEY_MEM
@@ -709,9 +716,7 @@ compare_time(const void *v1, const void 
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_CPUTIME
ORDERKEY_PCTCPU
@@ -732,9 +737,7 @@ compare_prio(const void *v1, const void 
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_PRIO
ORDERKEY_PCTCPU
@@ -754,9 +757,7 @@ compare_pid(const void *v1, const void *
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_proc **) pp2;
+   SETORDER;
 
ORDERKEY_PID
ORDERKEY_PCTCPU
@@ -777,9 +778,7 @@ compare_cmd(const void *v1, const void *
struct kinfo_proc *p1, *p2;
int result;
 
-   /* remove one level of indirection */
-   p1 = *(struct kinfo_proc **) pp1;
-   p2 = *(struct kinfo_

Re: top: allow reverse sort order

2018-11-27 Thread Klemens Nanni
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote:
> No objections here to the feature in general.  We already support reversing
> select orderings in systat(1), which I've found useful in practice, so this
> is not without precedent and is consistent with at least one other monitoring
> tool in userland.
> 
> I think it's a bit unfortunate that 'r' in interactive mode is already 
> assigned
> to renice the process.  If we did that here it'd be consistent with 
> systat(1)'s
> interactive keybindings and a boon for overall UX.  But 'r' has mean "renice"
> since before top(1) was even imported, so at this late date I don't think we 
> can
> change the binding for 'r'. However, adding an 'R' shortcut to reverse the 
> ordering
> for the current primary key might be a reasonable addition in a later diff.
I thought about a toggle as well but went with the "-" prefix semantics
since the existing toggles we currently have (`C', `h', `H') are
distinct features not related to filters like `g' or `o'. That is,
currently you cannot toggle something like "only user kn/all but kn".

> > Index: display.c
> > ===
> > RCS file: /cvs/src/usr.bin/top/display.c,v
> > retrieving revision 1.57
> > diff -u -p -r1.57 display.c
> > --- display.c   17 Nov 2018 23:10:08 -  1.57
> > +++ display.c   24 Nov 2018 14:09:38 -
> > @@ -817,7 +817,8 @@ show_help(void)
> > "I | i- toggle the display of idle processes\n"
> > "k [-sig] pid - send signal `-sig' to process `pid'\n"
> > "n|# count- show `count' processes\n"
> > -   "o field  - specify sort order (size, res, cpu, time, pri, pid, 
> > command)\n"
> > +   "o [-]field   - specify sort order (size, res, cpu, time, pri, pid, 
> > command)\n"
> > +   "   (o -field sorts in reverse)\n"
> 
> "o" isn't needed, as contextually you're talking about the 'o' shortcut.
Keeping "o" is consistent with the rest: "g+ selects all commands" and
"u -user hides user" for example.

> > @@ -879,10 +877,10 @@ rundisplay(void)
> > new_message(MT_standout,
> > "Order to sort: ");
> > if (readline(tempbuf, sizeof(tempbuf)) > 0) {
> > -   if ((i = string_index(tempbuf,
> > -   statics.order_names)) == -1) {
> > +   if ((i = getorder(tempbuf)) == -1) {
> > new_message(MT_standout,
> > " %s: unrecognized sorting order",
> > +   tempbuf[0] == '-' ? tempbuf + 1 :
> > tempbuf);
> 
> You aren't ducking the '-' during init so I don't think you should hide it 
> here.
> You could even refactor the two into something like
We're also not "ducking" it for `u' in the option parsing. Admittedly,
this is inconsistent but I'd like to keep both in the command loop. The
command line options are parsed once so with an erroneous `-U --kn'
users might see "--kn: unknown user" only once on startup when in fact
"-kn" is the unknown user.

Stripping the dash in error messages from the command line which is
likely used repeatedly at runtime ensures that the correct username
is printed.

> void setorder(const char *);
> 
> to keep the messages in sync, but that should be done in a subsequent diff.
So I if at all, I suggest stripping the dash in the option parsing
routine as well for consistency in separate diff.



Re: top: allow reverse sort order

2018-11-27 Thread Scott Cheloha
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote:
> On Sat, Nov 24, 2018 at 04:06:34PM +0100, Klemens Nanni wrote:
> > Sometimes I want to see certain programs with least amount of memory,
> > so this diff implements `o -field' to sort in reverse order.
> > 
> > The logic is straight forward:
> > 
> > 1. merge common code from argument and command loops into new setorder()
> > 2. introduce global state `rev_order' (set in the helper)
> > 3. move identical code to set up process objects from compare_*()
> >functions into SETORDER macro using global boolean
> > 
> > compare_*() are used by qsort(3). To sort in reverse, the macro simply
> > swaps the objects used by the ORDERKEY_* macros. That is it inverts the
> > comparison from `p1 > p2' into `p2 > p1' respectively `p1 < p2'.
> > 
> > Works fine for all available fields on amd64, no behaviour change for
> > "normal" order.
> > 
> > Feedback? Objections?
> 
> [...]

After lingering on this I missed a bug in my initial review.  See inline.

> > Index: display.c
> > ===
> > RCS file: /cvs/src/usr.bin/top/display.c,v
> > retrieving revision 1.57
> > diff -u -p -r1.57 display.c
> > --- display.c   17 Nov 2018 23:10:08 -  1.57
> > +++ display.c   24 Nov 2018 14:09:38 -
> > @@ -817,7 +817,8 @@ show_help(void)
> > "I | i- toggle the display of idle processes\n"
> > "k [-sig] pid - send signal `-sig' to process `pid'\n"
> > "n|# count- show `count' processes\n"
> > -   "o field  - specify sort order (size, res, cpu, time, pri, pid, 
> > command)\n"
> > +   "o [-]field   - specify sort order (size, res, cpu, time, pri, pid, 
> > command)\n"
> > +   "   (o -field sorts in reverse)\n"
> 
> "o" isn't needed, as contextually you're talking about the 'o' shortcut.
> Maybe "-field reverses order" or "'-' prefix reverses sort order"?
> 
> > "P pid- highlight process `pid' (P+ switches highlighting 
> > off)\n"
> > "p pid- display process by `pid' (p+ selects all 
> > processes)\n"
> > "q- quit\n"
> > Index: machine.c
> > ===
> > RCS file: /cvs/src/usr.bin/top/machine.c,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 machine.c
> > --- machine.c   17 Nov 2018 23:10:08 -  1.95
> > +++ machine.c   24 Nov 2018 14:47:32 -
> > @@ -602,6 +602,8 @@ static unsigned char sorted_state[] =
> > 1   /* zombie*/
> >  };
> >  
> > +extern int rev_order;
> > +
> >  /*
> >   *  proc_compares - comparison functions for "qsort"
> >   */
> > @@ -631,6 +633,17 @@ static unsigned char sorted_state[] =
> >  #define ORDERKEY_CMD \
> > if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0)
> >  
> > +/* remove one level of indirection and set sort order */
> > +#define SETORDER do { \
> > +   if (rev_order) { \
> > +   p1 = *(struct kinfo_proc **) pp2; \
> > +   p2 = *(struct kinfo_proc **) pp1; \
> > +   } else { \
> > +   p1 = *(struct kinfo_proc **) pp1; \
> > +   p2 = *(struct kinfo_proc **) pp2; \
> > +   } \
> > +   } while (0)
> > +
> >  /* compare_cpu - the comparison function for sorting by cpu percentage */
> >  static int
> >  compare_cpu(const void *v1, const void *v2)
> > @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void *
> > struct kinfo_proc *p1, *p2;
> > int result;
> >  
> > -   /* remove one level of indirection */
> > -   p1 = *(struct kinfo_proc **) pp1;
> > -   p2 = *(struct kinfo_proc **) pp2;
> > +   SETORDER;
> >  
> > ORDERKEY_PCTCPU
> > ORDERKEY_CPUTIME
> > @@ -663,9 +674,7 @@ compare_size(const void *v1, const void 
> > struct kinfo_proc *p1, *p2;
> > int result;
> >  
> > -   /* remove one level of indirection */
> > -   p1 = *(struct kinfo_proc **) pp1;
> > -   p2 = *(struct kinfo_proc **) pp2;
> > +   SETORDER;
> >  
> > ORDERKEY_MEM
> > ORDERKEY_RSSIZE
> > @@ -686,9 +695,7 @@ compare_res(const void *v1, const void *
> > struct kinfo_proc *p1, *p2;
> > int result;
> >  
> > -   /* remove one level of indirection */
> > -   p1 = *(struct kinfo_proc **) pp1;
> > -   p2 = *(struct kinfo_proc **) pp2;
> > +   SETORDER;
> >  
> > ORDERKEY_RSSIZE
> > ORDERKEY_MEM
> > @@ -709,9 +716,7 @@ compare_time(const void *v1, const void 
> > struct kinfo_proc *p1, *p2;
> > int result;
> >  
> > -   /* remove one level of indirection */
> > -   p1 = *(struct kinfo_proc **) pp1;
> > -   p2 = *(struct kinfo_proc **) pp2;
> > +   SETORDER;
> >  
> > ORDERKEY_CPUTIME
> > ORDERKEY_PCTCPU
> > @@ -732,9 +737,7 @@ compare_prio(const void *v1, const void 
> > struct kinfo_proc *p1, *p2;
> > int result;
> >  
> > -   /* remove one level of indirection */
> > -   p1 = *(struct kinfo_proc **)

mg: toggle-read-only-all

2018-11-27 Thread Mark Lumsden

Allow all non-ephemeral buffers to be toggled write/read-only. This
toggle works while mg is running, unlike the -R option which is only
during mg startup.

ok?

Index: buffer.c
===
RCS file: /cvs/src/usr.bin/mg/buffer.c,v
retrieving revision 1.104
diff -u -p -u -p -r1.104 buffer.c
--- buffer.c6 Aug 2017 04:39:45 -   1.104
+++ buffer.c27 Nov 2018 19:52:41 -
@@ -31,6 +31,28 @@ extern int globalwd;

 /* ARGSUSED */
 int
+togglereadonlyall(int f, int n)
+{
+   struct buffer *bp = NULL;
+   int len = 0;
+
+   allbro = !allbro;
+   for (bp = bheadp; bp != NULL; bp = bp->b_bufp) {
+   len = strlen(bp->b_bname);
+   if (bp->b_bname[0] != '*' && bp->b_bname[len - 1] != '*') {
+   if (allbro)
+   bp->b_flag |= BFREADONLY;
+   else
+   bp->b_flag &= ~BFREADONLY;
+   }
+   }
+   curwp->w_rflag |= WFMODE;
+
+   return (TRUE);
+}
+
+/* ARGSUSED */
+int
 togglereadonly(int f, int n)
 {
int s;
Index: def.h
===
RCS file: /cvs/src/usr.bin/mg/def.h,v
retrieving revision 1.156
diff -u -p -u -p -r1.156 def.h
--- def.h   29 Aug 2018 07:50:16 -  1.156
+++ def.h   27 Nov 2018 19:52:41 -
@@ -417,6 +417,7 @@ int  delwind(int, int);

 /* buffer.c */
 int togglereadonly(int, int);
+int togglereadonlyall(int, int);
 struct buffer   *bfind(const char *, int);
 int poptobuffer(int, int);
 int killbuffer(struct buffer *);
@@ -737,6 +738,7 @@ extern int   defb_flag;
 extern int  doaudiblebell;
 extern int  dovisiblebell;
 extern int  dblspace;
+extern int  allbro;
 extern char cinfo[];
 extern char*keystrings[];
 extern char pat[NPAT];
Index: funmap.c
===
RCS file: /cvs/src/usr.bin/mg/funmap.c,v
retrieving revision 1.54
diff -u -p -u -p -r1.54 funmap.c
--- funmap.c29 Aug 2018 07:50:16 -  1.54
+++ funmap.c27 Nov 2018 19:52:41 -
@@ -201,6 +201,7 @@ static struct funmap functnames[] = {
{usebuffer, "switch-to-buffer",},
{poptobuffer, "switch-to-buffer-other-window",},
{togglereadonly, "toggle-read-only" },
+   {togglereadonlyall, "toggle-read-only-all" },
{twiddle, "transpose-chars",},
{transposepara, "transpose-paragraphs",},
{transposeword, "transpose-words",},
Index: main.c
===
RCS file: /cvs/src/usr.bin/mg/main.c,v
retrieving revision 1.84
diff -u -p -u -p -r1.84 main.c
--- main.c  16 Sep 2016 17:17:40 -  1.84
+++ main.c  27 Nov 2018 19:52:41 -
@@ -28,6 +28,7 @@ intstartrow;  /* row to start 
*/
 int doaudiblebell; /* audible bell toggle  */
 int dovisiblebell; /* visible bell toggle  */
 int dblspace;  /* sentence end #spaces */
+int allbro;/* all buffs read-only  */
 struct buffer  *curbp; /* current buffer   */
 struct buffer  *bheadp;/* BUFFER list head */
 struct mgwin   *curwp; /* current window   */
@@ -65,6 +66,7 @@ main(int argc, char **argv)
switch (o) {
case 'R':
bro = 1;
+   allbro = 1;
break;
case 'n':
nobackups = 1;
Index: mg.1
===
RCS file: /cvs/src/usr.bin/mg/mg.1,v
retrieving revision 1.108
diff -u -p -u -p -r1.108 mg.1
--- mg.118 Nov 2018 07:57:28 -  1.108
+++ mg.127 Nov 2018 19:52:41 -
@@ -884,6 +884,10 @@ Prompt and switch to a new buffer in the
 Switch to buffer in another window.
 .It toggle-read-only
 Toggle the read-only flag on the current buffer.
+.It toggle-read-only-all
+Toggle the read-only flag on all non-ephemeral buffers.
+A simple toggle that switches a global read-only flag either on
+or off.
 .It transpose-chars
 Transpose the two characters in front of and under dot,
 then move forward one character.



Re: top: allow reverse sort order

2018-11-27 Thread Scott Cheloha
On Tue, Nov 27, 2018 at 07:38:59PM +, Jason McIntyre wrote:
> On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote:
> > > @@ -137,13 +137,16 @@ mode.
> > >  This is identical to
> > >  .Em batch
> > >  mode.
> > > -.It Fl o Ar field
> > > +.It Fl o Oo - Oc Ns Ar field
> > >  Sort the process display area using the specified
> > >  .Ar field
> > >  as the primary key.
> > >  The field name is the name of the column as seen in the output,
> > >  but in lower case.
> > >  The
> > > +.Sq -
> > > +prefix reverses the order.
> > 
> > My understanding is that an "order" is the actual progression of
> > a sequence, while an "ordering" refers to the underlying function
> > or rule that gives rise to a given "order" when applied to a set.
> > 
> > So, I *think* you want
> > 
> > "reverses the ordering for the field"
> > 
> > or something similar, but I'm not positive.
> > 
> 
> hi.
> 
> i've never heard of "ordering" as a noun like this. i think klemens'
> language was fine (and simpler).

I can live with that.

kn: just make sure the two error messages match up.



Re: top: allow reverse sort order

2018-11-27 Thread Jason McIntyre
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote:
> > @@ -137,13 +137,16 @@ mode.
> >  This is identical to
> >  .Em batch
> >  mode.
> > -.It Fl o Ar field
> > +.It Fl o Oo - Oc Ns Ar field
> >  Sort the process display area using the specified
> >  .Ar field
> >  as the primary key.
> >  The field name is the name of the column as seen in the output,
> >  but in lower case.
> >  The
> > +.Sq -
> > +prefix reverses the order.
> 
> My understanding is that an "order" is the actual progression of
> a sequence, while an "ordering" refers to the underlying function
> or rule that gives rise to a given "order" when applied to a set.
> 
> So, I *think* you want
> 
>   "reverses the ordering for the field"
> 
> or something similar, but I'm not positive.
> 

hi.

i've never heard of "ordering" as a noun like this. i think klemens'
language was fine (and simpler).

jmc



rad(8): auto discovered prefixes should not overwrite explicitly configured prefixes

2018-11-27 Thread Florian Obser


say you have

2001:db8:23::1/64 and fdac:23::1/64 configured on ix0 and the following 
rad.conf:

interface ix0 {
prefix fdac:23::/64 { autonomous address-configuration no }
auto prefix { autonomous address-configuration yes }
}

rad should announce 2001:db8:23::/64 with the A flag set and
fdac:23::/64 without the A flag.

Currently it announces both prefixes with the A flag because auto
prefix wins. I believe that is the wrong way around.

Ross Richardson pointed a use case out in private where you get a
dynamic global prefix via dhcpv6-pd and you want a stable ULA prefix
for your infrastructure but don't want to use the ULA for slaac.
currently that's not possible.

OK?

diff --git frontend.c frontend.c
index 152f13a2738..a02b3ef37ff 100644
--- frontend.c
+++ frontend.c
@@ -791,10 +791,6 @@ merge_ra_interfaces(void)
ra_iface_conf = find_ra_iface_conf(
&frontend_conf->ra_iface_list, ra_iface->conf_name);
 
-   if (ra_iface_conf->autoprefix)
-   get_interface_prefixes(ra_iface,
-   ra_iface_conf->autoprefix);
-
log_debug("add static prefixes for %s", ra_iface->name);
 
SIMPLEQ_FOREACH(ra_prefix_conf, &ra_iface_conf->ra_prefix_list,
@@ -803,6 +799,11 @@ merge_ra_interfaces(void)
&ra_prefix_conf->prefix,
ra_prefix_conf->prefixlen, ra_prefix_conf);
}
+
+   if (ra_iface_conf->autoprefix)
+   get_interface_prefixes(ra_iface,
+   ra_iface_conf->autoprefix);
+
build_packet(ra_iface);
}
 }


-- 
I'm not entirely sure you are real.



Re: bgpd refactor aspath_match a bit

2018-11-27 Thread Job Snijders
On Tue, Nov 27, 2018 at 06:23:53PM +0100, Claudio Jeker wrote:
> On Tue, Nov 27, 2018 at 04:21:53PM +0100, Job Snijders wrote:
> > On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote:
> > > For origin validation I chacked the source_as in struct rde_aspath
> > > this is not really the right place. It should be in struct aspath
> > > since that holds all the ASPATH related stuff. Change this, move
> > > aspath_match out of util.c back into rde_attr.c and adjust code to use
> > > the cached value also in match from any source-as XYZ rules.
> > > This last bit causes a minor behavioural change since the old code
> > > extracted the last non AS_SET asnumber. The new code follows the ROA
> > > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for
> > > empty paths and AS_NONE (which is 0) for everything else.
> > > So now 'match from any source-as 0' will return all paths that do not
> > > have a final AS_SEQUENCE segment.
> > > 
> > > The reason for this change is that I don't want to have two different
> > > behaviours for what we call source-as (the one in roa-set and the one on a
> > > filter).
> > 
> > Something is off, it seems 'source-as 0' is matching anything that has
> > an AS_SET attribute set:
> > 
> > $ bgpctl show rib source-as 0 | head
> > flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
> >S = Stale, E = Error
> > origin validation state: N = not-found, V = valid, ! = invalid
> > origin: i = IGP, e = EGP, ? = Incomplete
> > 
> > flags ovs destination  gateway  lpref   med aspath 
> > origin
> > I*> N 5.39.176.0/21192.147.168.1  100 0 2914 8530 { 
> > 198753 } ?
> > I*> N 5.101.110.0/24   192.147.168.1  100 0 2914 14061 
> > { 46652 } i
> > I*> N 5.175.0.0/19 192.147.168.1  100 0 2914 1299 
> > 20773 { 8972 } i
> > I*> N 8.41.202.0/24192.147.168.1  100 0 2914 13789 
> > 30372 { 40179 } i
> > 
> > Similarly, this should return at least 5.39.176.0/21:
> > 
> > $ bgpctl show rib source-as 8530
> > flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
> >S = Stale, E = Error
> > origin validation state: N = not-found, V = valid, ! = invalid
> > origin: i = IGP, e = EGP, ? = Incomplete
> > 
> > flags ovs destination  gateway  lpref   med aspath 
> > origin
> > I*> N 80.87.16.0/20192.147.168.1  100 0 2914 8530 ?
> > I*> N 87.236.128.0/21  192.147.168.1  100 0 2914 8530 ?
> > I*> N 88.151.152.0/21  192.147.168.1  100 0 2914 8530 ?
> > I*> N 89.38.120.0/21   192.147.168.1  100 0 2914 8530 i
> > I*> N 93.115.176.0/20  192.147.168.1  100 0 2914 8530 i
> > I*> N 185.52.144.0/22  192.147.168.1  100 0 2914 8530 ?
> > 
> 
> I implemented source-as the way ROA is defining it. So anything which ends
> with a AS_SET will return AS_NONE (which is 0). OpenBGPD has no way to
> have an AS_PATH that has a real 0 in the AS_PATH (those UPDATES are
> treated as withdraw). Because of this also the 5.39.176.0/21 is no longer
> matching in 'bgpctl show rib source-as 8530'.

I'm not sure it should behave that way.

'bgpctl show rib source-as 8530' really ought to return prefixes like
80.87.16.0/20 but also 5.39.176.0/21.

> I'm a bit on the edge here about where to go and currently prefer to
> follow a RFC (which in this case is RFC6811).
> 
>  o  Route Origin ASN: The origin AS number derived from a Route as
> follows:
> 
> *  the rightmost AS in the final segment of the AS_PATH attribute
>  in the Route if that segment is of type AS_SEQUENCE, or
> 
> *  the BGP speaker's own AS number if that segment is of type
>AS_CONFED_SEQUENCE or AS_CONFED_SET or if the AS_PATH is empty,
>or
> 
> *  the distinguished value "NONE" if the final segment of the
>AS_PATH attribute is of any other type.
> 
> As mentioned above I found it strange when behaviour is different because
> of where it is used.

RFC 6811 describes how BGP Origin Validation should be performed, but is
not a guideline how to treat AS_SETs in general or how CLIs should
function. Perhaps 'source-as 0' should just throw an error in all
contexts (both bgpd.conf and bgpctl), since 0 can never be in an AS_SET
or AS_SEQUENCE. Perhaps we shouldn't overload the number.

1/ Maybe 'source-as self' or 'source-as none' can be used to match all
routes originated by the AS in which this bgpd instance runs.

2/ Similarly, the program could be made so that 'AS 15562' (lets use
15562 as example) from the Global Configuration passed on to bgpctl and
used in the filter ruleset means both "what is originated by 15562" and
also what is originated in the own AS (and doesn't have AS_PATH yet, but
we know the router runs in 15562 because of the global config parameter).

Kind regards,

Job



Re: top: allow reverse sort order

2018-11-27 Thread Scott Cheloha
On Sat, Nov 24, 2018 at 04:06:34PM +0100, Klemens Nanni wrote:
> Sometimes I want to see certain programs with least amount of memory,
> so this diff implements `o -field' to sort in reverse order.
> 
> The logic is straight forward:
> 
> 1. merge common code from argument and command loops into new setorder()
> 2. introduce global state `rev_order' (set in the helper)
> 3. move identical code to set up process objects from compare_*()
>functions into SETORDER macro using global boolean
> 
> compare_*() are used by qsort(3). To sort in reverse, the macro simply
> swaps the objects used by the ORDERKEY_* macros. That is it inverts the
> comparison from `p1 > p2' into `p2 > p1' respectively `p1 < p2'.
> 
> Works fine for all available fields on amd64, no behaviour change for
> "normal" order.
> 
> Feedback? Objections?

I mentioned in private that I disliked the lack of parentheses in the macro
invocations, but after rereading I see that that's the style in use in the
code already, so you're fine, my bad.

No objections here to the feature in general.  We already support reversing
select orderings in systat(1), which I've found useful in practice, so this
is not without precedent and is consistent with at least one other monitoring
tool in userland.

I think it's a bit unfortunate that 'r' in interactive mode is already assigned
to renice the process.  If we did that here it'd be consistent with systat(1)'s
interactive keybindings and a boon for overall UX.  But 'r' has mean "renice"
since before top(1) was even imported, so at this late date I don't think we can
change the binding for 'r'. However, adding an 'R' shortcut to reverse the 
ordering
for the current primary key might be a reasonable addition in a later diff.

Some nits inline for documentation, error messages.  Otherwise ok cheloha@.

... and I might be wrong about those nits, too.  Definitely have jmc@ weigh in
on the nits and the docs changes in general.

> Index: display.c
> ===
> RCS file: /cvs/src/usr.bin/top/display.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 display.c
> --- display.c 17 Nov 2018 23:10:08 -  1.57
> +++ display.c 24 Nov 2018 14:09:38 -
> @@ -817,7 +817,8 @@ show_help(void)
>   "I | i- toggle the display of idle processes\n"
>   "k [-sig] pid - send signal `-sig' to process `pid'\n"
>   "n|# count- show `count' processes\n"
> - "o field  - specify sort order (size, res, cpu, time, pri, pid, 
> command)\n"
> + "o [-]field   - specify sort order (size, res, cpu, time, pri, pid, 
> command)\n"
> + "   (o -field sorts in reverse)\n"

"o" isn't needed, as contextually you're talking about the 'o' shortcut.
Maybe "-field reverses order" or "'-' prefix reverses sort order"?

>   "P pid- highlight process `pid' (P+ switches highlighting 
> off)\n"
>   "p pid- display process by `pid' (p+ selects all 
> processes)\n"
>   "q- quit\n"
> Index: machine.c
> ===
> RCS file: /cvs/src/usr.bin/top/machine.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 machine.c
> --- machine.c 17 Nov 2018 23:10:08 -  1.95
> +++ machine.c 24 Nov 2018 14:47:32 -
> @@ -602,6 +602,8 @@ static unsigned char sorted_state[] =
>   1   /* zombie*/
>  };
>  
> +extern int rev_order;
> +
>  /*
>   *  proc_compares - comparison functions for "qsort"
>   */
> @@ -631,6 +633,17 @@ static unsigned char sorted_state[] =
>  #define ORDERKEY_CMD \
>   if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0)
>  
> +/* remove one level of indirection and set sort order */
> +#define SETORDER do { \
> + if (rev_order) { \
> + p1 = *(struct kinfo_proc **) pp2; \
> + p2 = *(struct kinfo_proc **) pp1; \
> + } else { \
> + p1 = *(struct kinfo_proc **) pp1; \
> + p2 = *(struct kinfo_proc **) pp2; \
> + } \
> + } while (0)
> +
>  /* compare_cpu - the comparison function for sorting by cpu percentage */
>  static int
>  compare_cpu(const void *v1, const void *v2)
> @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void *
>   struct kinfo_proc *p1, *p2;
>   int result;
>  
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>  
>   ORDERKEY_PCTCPU
>   ORDERKEY_CPUTIME
> @@ -663,9 +674,7 @@ compare_size(const void *v1, const void 
>   struct kinfo_proc *p1, *p2;
>   int result;
>  
> - /* remove one level of indirection */
> - p1 = *(struct kinfo_proc **) pp1;
> - p2 = *(struct kinfo_proc **) pp2;
> + SETORDER;
>  
>   ORDERKEY_MEM
>   ORDERKEY_RSSIZE
> @@ -686,9 +695,7 @@ compare_res(const void *

Re: bgpd refactor aspath_match a bit

2018-11-27 Thread Claudio Jeker
On Tue, Nov 27, 2018 at 04:21:53PM +0100, Job Snijders wrote:
> Hi Claudio,
> 
> On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote:
> > For origin validation I chacked the source_as in struct rde_aspath
> > this is not really the right place. It should be in struct aspath
> > since that holds all the ASPATH related stuff. Change this, move
> > aspath_match out of util.c back into rde_attr.c and adjust code to use
> > the cached value also in match from any source-as XYZ rules.
> > This last bit causes a minor behavioural change since the old code
> > extracted the last non AS_SET asnumber. The new code follows the ROA
> > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for
> > empty paths and AS_NONE (which is 0) for everything else.
> > So now 'match from any source-as 0' will return all paths that do not
> > have a final AS_SEQUENCE segment.
> > 
> > The reason for this change is that I don't want to have two different
> > behaviours for what we call source-as (the one in roa-set and the one on a
> > filter).
> 
> Something is off, it seems 'source-as 0' is matching anything that has
> an AS_SET attribute set:
> 
> $ bgpctl show rib source-as 0 | head
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
>S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
> 
> flags ovs destination  gateway  lpref   med aspath origin
> I*> N 5.39.176.0/21192.147.168.1  100 0 2914 8530 { 
> 198753 } ?
> I*> N 5.101.110.0/24   192.147.168.1  100 0 2914 14061 { 
> 46652 } i
> I*> N 5.175.0.0/19 192.147.168.1  100 0 2914 1299 
> 20773 { 8972 } i
> I*> N 8.41.202.0/24192.147.168.1  100 0 2914 13789 
> 30372 { 40179 } i
> 
> Similarly, this should return at least 5.39.176.0/21:
> 
> $ bgpctl show rib source-as 8530
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
>S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
> 
> flags ovs destination  gateway  lpref   med aspath origin
> I*> N 80.87.16.0/20192.147.168.1  100 0 2914 8530 ?
> I*> N 87.236.128.0/21  192.147.168.1  100 0 2914 8530 ?
> I*> N 88.151.152.0/21  192.147.168.1  100 0 2914 8530 ?
> I*> N 89.38.120.0/21   192.147.168.1  100 0 2914 8530 i
> I*> N 93.115.176.0/20  192.147.168.1  100 0 2914 8530 i
> I*> N 185.52.144.0/22  192.147.168.1  100 0 2914 8530 ?
> 

I implemented source-as the way ROA is defining it. So anything which ends
with a AS_SET will return AS_NONE (which is 0). OpenBGPD has no way to
have an AS_PATH that has a real 0 in the AS_PATH (those UPDATES are
treated as withdraw). Because of this also the 5.39.176.0/21 is no longer
matching in 'bgpctl show rib source-as 8530'.

I'm a bit on the edge here about where to go and currently prefer to
follow a RFC (which in this case is RFC6811).

 o  Route Origin ASN: The origin AS number derived from a Route as
follows:

*  the rightmost AS in the final segment of the AS_PATH attribute
 in the Route if that segment is of type AS_SEQUENCE, or

*  the BGP speaker's own AS number if that segment is of type
   AS_CONFED_SEQUENCE or AS_CONFED_SET or if the AS_PATH is empty,
   or

*  the distinguished value "NONE" if the final segment of the
   AS_PATH attribute is of any other type.

As mentioned above I found it strange when behaviour is different because
of where it is used.

-- 
:wq Claudio



Re: bgpd refactor aspath_match a bit

2018-11-27 Thread Job Snijders
Hi Claudio,

On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote:
> For origin validation I chacked the source_as in struct rde_aspath
> this is not really the right place. It should be in struct aspath
> since that holds all the ASPATH related stuff. Change this, move
> aspath_match out of util.c back into rde_attr.c and adjust code to use
> the cached value also in match from any source-as XYZ rules.
> This last bit causes a minor behavioural change since the old code
> extracted the last non AS_SET asnumber. The new code follows the ROA
> RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for
> empty paths and AS_NONE (which is 0) for everything else.
> So now 'match from any source-as 0' will return all paths that do not
> have a final AS_SEQUENCE segment.
> 
> The reason for this change is that I don't want to have two different
> behaviours for what we call source-as (the one in roa-set and the one on a
> filter).

Something is off, it seems 'source-as 0' is matching anything that has
an AS_SET attribute set:

$ bgpctl show rib source-as 0 | head
flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
   S = Stale, E = Error
origin validation state: N = not-found, V = valid, ! = invalid
origin: i = IGP, e = EGP, ? = Incomplete

flags ovs destination  gateway  lpref   med aspath origin
I*> N 5.39.176.0/21192.147.168.1  100 0 2914 8530 { 
198753 } ?
I*> N 5.101.110.0/24   192.147.168.1  100 0 2914 14061 { 
46652 } i
I*> N 5.175.0.0/19 192.147.168.1  100 0 2914 1299 20773 
{ 8972 } i
I*> N 8.41.202.0/24192.147.168.1  100 0 2914 13789 
30372 { 40179 } i

Similarly, this should return at least 5.39.176.0/21:

$ bgpctl show rib source-as 8530
flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
   S = Stale, E = Error
origin validation state: N = not-found, V = valid, ! = invalid
origin: i = IGP, e = EGP, ? = Incomplete

flags ovs destination  gateway  lpref   med aspath origin
I*> N 80.87.16.0/20192.147.168.1  100 0 2914 8530 ?
I*> N 87.236.128.0/21  192.147.168.1  100 0 2914 8530 ?
I*> N 88.151.152.0/21  192.147.168.1  100 0 2914 8530 ?
I*> N 89.38.120.0/21   192.147.168.1  100 0 2914 8530 i
I*> N 93.115.176.0/20  192.147.168.1  100 0 2914 8530 i
I*> N 185.52.144.0/22  192.147.168.1  100 0 2914 8530 ?

Kind regards,

Job



Re: bgpd refactor community code

2018-11-27 Thread Job Snijders
On Thu, Nov 22, 2018 at 05:56:20PM +0100, Claudio Jeker wrote:
> On Tue, Nov 13, 2018 at 06:53:55PM +0100, Claudio Jeker wrote:
> > This is a large diff that changes the way communities are stored in
> > filters and filter_sets. Both standard communities and large communities
> > now share the same data structure for lookups and at the same time the
> > filters are extended to allow more then one community to match per rule
> > (currently the maximum is 3). I did leave extended communities outside for
> > now since this diff is already big enough but they will follow in a second
> > step.
> > 
> > So now filters like
> > deny to any large-community 196618:0:666 large-community 196618:0:3
> > deny to any community 13030:1016 community 13030:5
> > will work and match only if both communities are matched.
> > 
> > As a side effect the bgpctl show rib code is changed and there is no
> > longer a limitation that only one of the filter is allowed to be used.
> > In other words 'bgpctl show rib as 13030 community 1303:1036' will
> > work now.
> > 
> > Apart from that there should be no other visible change.
> > 
> > Please test and report back

OK job@



Re: refactor malloc a bit

2018-11-27 Thread Theo Buehler
On Mon, Nov 26, 2018 at 07:36:56AM +0100, Otto Moerbeek wrote:
> Hi,
> 
> this refactors the code that find an existsing allocation into a
> separate function. The code is currently repeated in three spots.
> 
> Prepatory work to get a somehwat more efficient version of the
> "not-my-pool" case.
> 
> Please review and test,

works and reads fine.

ok



Re: diff: ftpd(8): fix for sign-compare compiler warnings

2018-11-27 Thread Theo Buehler
On Sun, Nov 25, 2018 at 12:32:23AM +0100, Jan Klemkow wrote:
> Hi,
> 
> This diff fixes some -Wsign-compare compiler warnings in ftpd(8) by
> using the right types for 'i' and 'len'.  One warning is left, but I
> don't see that it's fixable without suppressing the warning by a cast of
> len to size_t.  And casting might be controversial in this case?!

The diff looks correct to me. If anyone wants to commit

ok tb

> /usr/src/libexec/ftpd/ftpd.c:2781:11: warning: comparison of integers of
> different signs: 'int' and
>   'unsigned long' [-Wsign-compare]
> if (len >= sizeof(buf) || len == -1) {
> ~~~ ^  ~~~

This test is the wrong way around: compare CAVEATS in snprintf(3).

There's a ton of unchecked snprintfs in this code. Did you take a look
at those?