Re: pfctl should allow administrator to flush _anchors
Alexandr Nedvedicky wrote: > it is, however -Fall operates on main ruleset only. -Fall also does > not reset limits and timeouts. Hence my first idea was to introduce > '-FNuke', which kills all rulesets and tables. > > I don't want to change behaviour of existing option ('-Fall'), therefore > I'm in favor to introduce a new option. Either '-FNuke' or '-U' works > for me. I'm the most concerned about flushing all rulesets. Is the existing behavior intentional or an oversight? I don't know when I would want to use -Fall, but keep the old timeouts, and depend on that. I'd guess most people using -Fall are keeping old timeout only by happen stance, and not because they desire that. In any case, if you're seeking input on the name, something like -Freset says to me that it resets pf back to its initial state.
New driver apupgio(4): interface for LEDs and button of APU1. (Advices needed)
Hi tech@, I wrote a driver to control the APU1 LEDs and read the state of the pushbutton switch, like skgpio(4). I would like some reviews and hints about some project decisions that I have to make. I am cooking a diff to support APU{2,3,4}, but it is an extension of this code. Setup: Machine: APU 1 (dmesg with path applied bellow). GPIOs: Pushbutton Switch (GPIO 187), LED1 (GPIO 189), LED2 (GPIO 190), LED3 (GPIO 191). Implementation: One gpio(4) for pushbutton (gpio0) and other for the LEDs (gpio1). apugpio0 at isa0 gpio0 at apugpio0 (You can read, but can't write.) gpio1 at apugpio0 (You can read and write.) Questions: 1. It attaches at isabus using iomem. Is it okay? Kettenis said (here: https://marc.info/?l=openbsd-tech=147023610319035=2) that isa drivers is a risk to probe after hibernate, but is it relevant with these machines, and considering that it address is fixed and known? Alternatives are: + I could add the code directly to piixpm(4): gpio0 at piixpm(4). + Make piixpm(4) like a pcib(4), so I can attach apugpio to it (gpio0 at apupgio at piixpm). I think it is intrusive. 2. It has two handlers to map the non contiguous addresses. It has two handlers, one for the button and the other for the LEDs (their addresses are contiguous). I did this because I assume that we must not map GPIOs that we won't use, because we don't know what these GPIOs does, and if another driver bus_space_map(9) it too the kernel will panic(4). Is this approach correct? 3. Is the bus_space_unmap(9) necessary when some of the mappings fails? The driver is intended to work as a whole (button and leds) or doesn't work at all. The bus_space_unmap(9) lines in case of failed maps are necessary? If so, if the second bus_space_map(9) fails, I must unmap the first one in apugpio_match(), like I am doing inside the attach function apugpio_attach(). 4. I allow only reading from button, not setting to. I allow only reading to button, because I think that doesn't make sense to set its value, and when I tried it the GPIO does not hold the state that I set. Is that ok? 5. Are there a preferred ordering between the LEDs gpio(4) and the switches one? Now the button attaches first, but to me it is perfectly natural that the LEDs, that people may use more, get the gpio0 driver. Any thoughts about it? 6. Any other comments or suggestions? Regards, Rafael If you have an APU1 and want to test the patch, before using the gpio(4) devices trough gpioctl(8), you must configure them before the securelevel is raised. For example, with the /etc/rc.securelevel below: #!/bin/sh # # $OpenBSD: rc.securelevel,v 1.3 2014/07/14 10:15:33 ajacoutot Exp $ # # site-specific startup actions, daemons, and other things which # can be done BEFORE your system goes into securemode. For actions # which should be done AFTER your system has gone into securemode # please see /etc/rc.local. # /usr/sbin/gpioctl -q gpio0 0 set /usr/sbin/gpioctl -q gpio1 0 set /usr/sbin/gpioctl -q gpio1 1 set /usr/sbin/gpioctl -q gpio1 2 set dmesg (with patch): OpenBSD 6.5-beta (GENERIC.MP) #24: Mon Mar 25 00:34:29 WAT 2019 rne...@orus.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 4246003712 (4049MB) avail mem = 4106997760 (3916MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xdf16d820 (7 entries) bios0: vendor coreboot version "4.0" date 07/08/2014 bios0: PC Engines APU acpi0 at bios0: rev 0 acpi0: sleep states S0 S1 S3 S4 S5 acpi0: tables DSDT FACP SPCR HPET APIC HEST SSDT SSDT SSDT acpi0: wakeup devices AGPB(S4) HDMI(S4) PBR4(S4) PBR5(S4) PBR6(S4) PBR7(S4) PE20(S4) PE21(S4) PE22(S4) PE23(S4) PIBR(S4) UOH1(S3) UOH2(S3) UOH3(S3) UOH4(S3) UOH5(S3) [...] acpitimer0 at acpi0: 3579545 Hz, 32 bits acpihpet0 at acpi0: 14318180 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD G-T40E Processor, 1000.15 MHz, 14-02-00 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 16-way L2 cache cpu0: 8 4MB entries fully associative cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 199MHz cpu0: mwait min=64, max=64, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: AMD G-T40E Processor, 1000.00 MHz, 14-02-00 cpu1:
Re: pfctl should allow administrator to flush _anchors
Alexandr Nedvedicky wrote: > how about making the '-U' (or whatever name we agree) undocumented. We can > also make the option available if pfctl will get compiled with 'DEBUG' > option (assuming we are doing regress on debug bits anyway). no, it should be documented. but few people will use it, and that's fine
Re: pfctl should allow administrator to flush _anchors
Hello, > > > > Isn't -U pretty close to -Fall ? > > > > > > > > > > it is, however -Fall operates on main ruleset only. -Fall also does > > > not reset limits and timeouts. Hence my first idea was to introduce > > > '-FNuke', which kills all rulesets and tables. > > > > > > I don't want to change behaviour of existing option ('-Fall'), > > > therefore > > > I'm in favor to introduce a new option. Either '-FNuke' or '-U' works > > > for me. I'm the most concerned about flushing all rulesets. > > > > > > Also making "pfctl -a '_1/_2' -Fr" to remove PF 'private' rulesets > > > works > > > for me. Actually this is the most important thing I'd like to achieve. > > > > whatever gets done here, the initial-raw-state-forcing should be 1 > > operation. > > not multiple operations acting on aspects of pf. > > > > I think if it is multiple operations, people won't ever get comfortable > > using it. > > Not sure about that: I wont be comfortable anyway, as it can cause all sorts > of problems on a running system. > > When i reset things to the boot state, i would expect thats not a simple > thing and not without issues. > > I consider this as a cleanup op, most useful for regress tests, developers > testing stuff etc. In normal sysadmin work i never needed it. I think this is a good point. I don't expect experienced admins, who maintain production systems to use an unconfigure operation. It's indeed more useful for regression tests and people who are configuring their PF in sandbox/testing environment. how about making the '-U' (or whatever name we agree) undocumented. We can also make the option available if pfctl will get compiled with 'DEBUG' option (assuming we are doing regress on debug bits anyway). sashan
Re: ospfd: Warn when the router ID changes during config reload
On Mon, Mar 25 2019, Remi Locherer wrote: [...] > This works and it makes sense to me. > > The log message is a bit lengthy compared to other log messages produced > by ospfd. Maybe something like this: "router-id changed: restart required" Yep, fine with me. > But the patch is also OK remi@ as it is now. ok jca@ for your wording instead -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: pfctl should allow administrator to flush _anchors
Theo de Raadt(dera...@openbsd.org) on 2019.03.24 10:22:25 -0600: > Alexandr Nedvedicky wrote: > > > On Sun, Mar 24, 2019 at 09:51:13AM +0100, Denis Fondras wrote: > > > On Sun, Mar 24, 2019 at 09:24:34AM +0100, Alexandr Nedvedicky wrote: > > > > I think all the above calls for a new standalone option, which I named > > > > as > > > > 'Unconfigure'. Patch below suggest unconfigure behavior for PF. > > > > Doing 'pfctl -U' will bring PF back to its initial state (e.g. right > > > > before > > > > pf.conf got processed during the system boot). In case of PF the > > > > proposed -U > > > > will do following: > > > > - remove all rulesets and tables > > > > - remove all states and source nodes > > > > - remove all OS fingerprints > > > > - set all limits, timeouts and options to their defaults > > > > > > > > > > Isn't -U pretty close to -Fall ? > > > > > > > it is, however -Fall operates on main ruleset only. -Fall also does > > not reset limits and timeouts. Hence my first idea was to introduce > > '-FNuke', which kills all rulesets and tables. > > > > I don't want to change behaviour of existing option ('-Fall'), therefore > > I'm in favor to introduce a new option. Either '-FNuke' or '-U' works > > for me. I'm the most concerned about flushing all rulesets. > > > > Also making "pfctl -a '_1/_2' -Fr" to remove PF 'private' rulesets works > > for me. Actually this is the most important thing I'd like to achieve. > > whatever gets done here, the initial-raw-state-forcing should be 1 operation. > not multiple operations acting on aspects of pf. > > I think if it is multiple operations, people won't ever get comfortable > using it. Not sure about that: I wont be comfortable anyway, as it can cause all sorts of problems on a running system. When i reset things to the boot state, i would expect thats not a simple thing and not without issues. I consider this as a cleanup op, most useful for regress tests, developers testing stuff etc. In normal sysadmin work i never needed it.
unbound 1.9.1
this has a couple of fixes, nothing particularly major. we already had the fix for "#4225: clients seem to erroneously receive no answer with DNS-over-TLS and qname-minimisation" backported. working here, anyone else want to test? Index: doc/Changelog === RCS file: /cvs/src/usr.sbin/unbound/doc/Changelog,v retrieving revision 1.31 diff -u -p -r1.31 Changelog --- doc/Changelog 8 Feb 2019 10:29:08 - 1.31 +++ doc/Changelog 25 Mar 2019 20:42:08 - @@ -1,5 +1,85 @@ -5 February 2019: Wouter - - Fix tls-ciphers spelling in example.conf +1 March 2019: Wouter + - output forwarder log in ssl_req_order test. + +28 February 2019: Wouter + - Remove memory leak on pythonmod python2 script file init. + - Remove swig gcc8 python function cast warnings, they are ignored. + - Print correct module that failed when module-config is wrong. + +27 February 2019: Wouter + - Fix #4229: Unbound man pages lack information, about access-control + order and local zone tags, and elements in views. + - Fix #14: contrib/unbound.init: Fix wrong comparison judgment + before copying. + - Fix for python module on Windows, fix fopen. + +25 February 2019: Wouter + - Fix #4227: pair event del and add for libevent for tcp_req_info. + +21 February 2019: Wouter + - Fix the error for unknown module in module-config is understandable, + and explains it was not compiled in and where to see the list. + - In example.conf explain where to put cachedb module in module-config. + - In man page and example config explain that most modules have to + be listed at the start of module-config. + +20 February 2019: Wouter + - Fix pythonmod include and sockaddr_un ifdefs for compile on + Windows, and for libunbound. + +18 February 2019: Wouter + - Print query name with ip_ratelimit exceeded log lines. + - Spaces instead of tabs in that log message. + - Print query name and IP address when domain rate limit exceeded. + +14 February 2019: Wouter + - Fix capsforid canonical sort qsort callback. + +11 February 2019: Wouter + - Note default for module-config in man page. + - Fix recursion lame test for qname minimisation asked queries, + that were not present in the set of prepared answers. + - Fix #13: Remove left-over requirements on OpenSSL >= 1.1.0 for + cert name matching, from man page. + - make depend, with newer gcc, nicer layout. + +7 February 2019: Wouter + - Fix #4206: OpenSSL 1.0.2 hostname verification for FreeBSD 11.2. + - Fix that qname minimisation does not skip a label when missing + nameserver targets need to be fetched. + - Fix #4225: clients seem to erroneously receive no answer with + DNS-over-TLS and qname-minimisation. + +4 February 2019: Wouter + - Fix that log-replies prints the correct name for local-alias + names, for names that have a CNAME in local-data configuration. + It logs the original query name, not the target of the CNAME. + - Add local-zone type inform_redirect, which logs like type inform, + and redirects like type redirect. + - Perform canonical sort for 0x20 capsforid compare of replies, + this sorts rrsets in the authority and additional section before + comparison, so that out of order rrsets do not cause failure. + +31 January 2019: Wouter + - Set ub_ctx_set_tls call signature in ltrace config file for + libunbound in contrib/libunbound.so.conf. + - improve documentation for tls-service-key and forward-first. + - #10: fixed pkg-config operations, PKG_PROG_PKG_CONFIG moved out of + conditional section, fixes systemd builds, from Enrico Scholz. + - #9: For openssl 1.0.2 use the CRYPTO_THREADID locking callbacks, + still supports the set_id_callback previous API. And for 1.1.0 + no locking callbacks are needed. + - #8: Fix OpenSSL without ENGINE support compilation. + - Wipe TLS session key data from memory on exit. + +30 January 2019: Ralph + - Fix case in which query timeout can result in marking delegation + as edns_lame_known. + +29 January 2019: Wouter + - Fix spelling of tls-ciphers in example.conf.in. + - Fix #4224: auth_xfr_notify.rpl test broken due to typo + - Fix locking for libunbound context setup with broken port config. 28 January 2019: Wouter - ub_ctx_set_tls call for libunbound that enables DoT for the machines @@ -8,7 +88,9 @@ - List example config for root zone copy locally hosted with auth-zone as suggested from draft-ietf-dnsop-7706-bis-02. But with updated B root address. - - set version to 1.9.0 for release. + - set version to 1.9.0 for release. And this was released with the + spelling
Re: chflagsat(2): fix function argument
On Mon, Mar 25 2019, Anton Lindqvist wrote: > Hi, > My guess is that flag actually refers to atflags. > > Comments? OK? ok jca@ > Index: chflags.2 > === > RCS file: /cvs/src/lib/libc/sys/chflags.2,v > retrieving revision 1.27 > diff -u -p -r1.27 chflags.2 > --- chflags.2 19 Jan 2015 15:54:11 - 1.27 > +++ chflags.2 25 Mar 2019 20:38:23 - > @@ -113,12 +113,12 @@ in the > .Fa fd > parameter, the current working directory is used. > If > -.Fa flag > +.Fa atflags > is also zero, the behavior is identical to a call to > .Fn chflags . > .Pp > The > -.Fa flag > +.Fa atflags > argument is the bitwise OR of zero or more of the following values: > .Pp > .Bl -tag -width AT_SYMLINK_NOFOLLOW -offset indent -compact > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: chflagsat(2): fix function argument
On Mon, 25 Mar 2019 21:44:45 +0100, Anton Lindqvist wrote: > My guess is that flag actually refers to atflags. Yes, looks like copy pasta from fchmodat. OK millert@ - todd
Re: ospfd: Warn when the router ID changes during config reload
On Mon, Mar 25, 2019 at 02:43:26PM +0100, Jeremie Courreges-Anglas wrote: > On Sun, Mar 24 2019, Mitchell Krome wrote: > > On 24/03/2019 7:23 am, Theo de Raadt wrote: > >> Sebastian Benoit wrote: > >> > >>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000: > Was messing around with ospf and got myself into a situation where the > router ID's were the same on two boxes because I only did a reload on > one of them when I changed the loopback IP's. > >>> > >>> Thats sub optimal i believe... > >>> > This adds a warning when reloading if the router ID changes (there was > already a comment saying as much). Same patch can probably be applied to > ospf6d if people think it's useful. > > ospf6d currently doesn't support config reloads at all. It might be > worth adding an XXX comment there. > > >>> I think it would be better to abort the reload if the router-id is > >>> changed, > >>> i.e. not load the new config at all. > >> > >> That's the right approach in all our other daemons: > >> > >> if the configuration change cannot be installed correctly, consider > >> it invalid and abort. Someone would need to write code to make it > >> valid.. > >> > > > > That makes sense. I checked the manuals for the routers I use at work > > and they also required the ospf process to be restarted for the config > > to take effect after changing the router id. > > > > I moved the check up into ospf_reload because it doesn't make sense > > sending the config to all the children if we know we're going to abort. > > Your patch was mangled (long line wrapped) but the changes looked good. > Here's an updated version which tweaks punctuation and case (to match > the router-id keyword). Works for me in my simple test setup. > > Comments/oks? This works and it makes sense to me. The log message is a bit lengthy compared to other log messages produced by ospfd. Maybe something like this: "router-id changed: restart required" But the patch is also OK remi@ as it is now. > > > Index: ospfd.c > === > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v > retrieving revision 1.105 > diff -u -p -r1.105 ospfd.c > --- ospfd.c 15 Jan 2019 22:18:10 - 1.105 > +++ ospfd.c 25 Mar 2019 13:33:43 - > @@ -642,6 +642,13 @@ ospf_reload(void) > if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL) > return (-1); > > + /* Abort the reload if rtr_id changed */ > + if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) { > + log_warnx("router-id changed in new configuration, " > + "this requires ospfd to be restarted."); > + return (-1); > + } > + > /* send config to childs */ > if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1) > return (-1); > @@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st > struct redistribute *r; > int rchange = 0; > > - /* change of rtr_id needs a restart */ > conf->flags = xconf->flags; > conf->spf_delay = xconf->spf_delay; > conf->spf_hold_time = xconf->spf_hold_time; > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE >
chflagsat(2): fix function argument
Hi, My guess is that flag actually refers to atflags. Comments? OK? Index: chflags.2 === RCS file: /cvs/src/lib/libc/sys/chflags.2,v retrieving revision 1.27 diff -u -p -r1.27 chflags.2 --- chflags.2 19 Jan 2015 15:54:11 - 1.27 +++ chflags.2 25 Mar 2019 20:38:23 - @@ -113,12 +113,12 @@ in the .Fa fd parameter, the current working directory is used. If -.Fa flag +.Fa atflags is also zero, the behavior is identical to a call to .Fn chflags . .Pp The -.Fa flag +.Fa atflags argument is the bitwise OR of zero or more of the following values: .Pp .Bl -tag -width AT_SYMLINK_NOFOLLOW -offset indent -compact
Re: ospfd: Warn when the router ID changes during config reload
ok Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2019.03.25 14:43:26 +0100: > On Sun, Mar 24 2019, Mitchell Krome wrote: > > On 24/03/2019 7:23 am, Theo de Raadt wrote: > >> Sebastian Benoit wrote: > >> > >>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000: > Was messing around with ospf and got myself into a situation where the > router ID's were the same on two boxes because I only did a reload on > one of them when I changed the loopback IP's. > >>> > >>> Thats sub optimal i believe... > >>> > This adds a warning when reloading if the router ID changes (there was > already a comment saying as much). Same patch can probably be applied to > ospf6d if people think it's useful. > > ospf6d currently doesn't support config reloads at all. It might be > worth adding an XXX comment there. > > >>> I think it would be better to abort the reload if the router-id is > >>> changed, > >>> i.e. not load the new config at all. > >> > >> That's the right approach in all our other daemons: > >> > >> if the configuration change cannot be installed correctly, consider > >> it invalid and abort. Someone would need to write code to make it > >> valid.. > >> > > > > That makes sense. I checked the manuals for the routers I use at work > > and they also required the ospf process to be restarted for the config > > to take effect after changing the router id. > > > > I moved the check up into ospf_reload because it doesn't make sense > > sending the config to all the children if we know we're going to abort. > > Your patch was mangled (long line wrapped) but the changes looked good. > Here's an updated version which tweaks punctuation and case (to match > the router-id keyword). Works for me in my simple test setup. > > Comments/oks? > > > Index: ospfd.c > === > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v > retrieving revision 1.105 > diff -u -p -r1.105 ospfd.c > --- ospfd.c 15 Jan 2019 22:18:10 - 1.105 > +++ ospfd.c 25 Mar 2019 13:33:43 - > @@ -642,6 +642,13 @@ ospf_reload(void) > if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL) > return (-1); > > + /* Abort the reload if rtr_id changed */ > + if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) { > + log_warnx("router-id changed in new configuration, " > + "this requires ospfd to be restarted."); > + return (-1); > + } > + > /* send config to childs */ > if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1) > return (-1); > @@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st > struct redistribute *r; > int rchange = 0; > > - /* change of rtr_id needs a restart */ > conf->flags = xconf->flags; > conf->spf_delay = xconf->spf_delay; > conf->spf_hold_time = xconf->spf_hold_time; > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE >
Re: ssl(8): change example key size to 4096 bits
On Sat, Mar 23, 2019 at 11:30:15PM -0600, Randy Hartman wrote: > ssl(8) man page changed example key size from 2048 to 4096 > in 2012 but reverted after two days. smtpd.conf(5)'s man page > and acme-client both use 4096. is it time to un-revert? > fixed, thanks. jmc > Index: ssl.8 > === > RCS file: /cvs/src/share/man/man8/ssl.8,v > retrieving revision 1.66 > diff -u -p -u -p -r1.66 ssl.8 > --- ssl.8 8 Jul 2017 17:52:44 - 1.66 > +++ ssl.8 24 Mar 2019 04:41:31 - > @@ -65,13 +65,13 @@ To support HTTPS transactions in > .Xr httpd 8 > you will need to generate an RSA certificate. > .Bd -literal -offset indent > -# openssl genrsa -out /etc/ssl/private/server.key 2048 > +# openssl genrsa -out /etc/ssl/private/server.key 4096 > .Ed > .Pp > Or, if you wish the key to be encrypted with a passphrase that you will > have to type in when starting servers > .Bd -literal -offset indent > -# openssl genrsa -aes256 -out /etc/ssl/private/server.key 2048 > +# openssl genrsa -aes256 -out /etc/ssl/private/server.key 4096 > .Ed > .Pp > The next step is to generate a Certificate Signing Request (CSR) which is >
Re: divert(4): increment divs_errors on if_get failure
On 24/03/19(Sun) 20:16, Lawrence Teo wrote: > This diff modifies divert_packet() to increment the divs_errors counter > on if_get failure so that users will be aware of it via netstat(1). If if_get(9) fails that means the interface no longer exists. Is it an error? Or a delayed free? Did we write an incorrect interface index? > The same thing is done for divert6_packet(). Is it the correct things to do? > While here, it also modifies divert_output() to move m_freem(m) below > divstat_inc(divs_errors). This is purely for consistency since all the > other divert_* functions increment the divstat counters before m_freem. > divert6_output() already does this so there is no need to make the same > change there. > > ok? > > > Index: netinet/ip_divert.c > === > RCS file: /cvs/src/sys/netinet/ip_divert.c,v > retrieving revision 1.61 > diff -u -p -r1.61 ip_divert.c > --- netinet/ip_divert.c 4 Feb 2019 21:40:52 - 1.61 > +++ netinet/ip_divert.c 24 Mar 2019 23:39:58 - > @@ -163,8 +163,8 @@ divert_output(struct inpcb *inp, struct > return (error); > > fail: > - m_freem(m); > divstat_inc(divs_errors); > + m_freem(m); > return (error ? error : EINVAL); > } > > @@ -199,6 +199,7 @@ divert_packet(struct mbuf *m, int dir, u > > ifp = if_get(m->m_pkthdr.ph_ifidx); > if (ifp == NULL) { > + divstat_inc(divs_errors); > m_freem(m); > return (0); > } > Index: netinet6/ip6_divert.c > === > RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v > retrieving revision 1.59 > diff -u -p -r1.59 ip6_divert.c > --- netinet6/ip6_divert.c 4 Feb 2019 21:40:52 - 1.59 > +++ netinet6/ip6_divert.c 24 Mar 2019 23:39:03 - > @@ -205,6 +205,7 @@ divert6_packet(struct mbuf *m, int dir, > > ifp = if_get(m->m_pkthdr.ph_ifidx); > if (ifp == NULL) { > + div6stat_inc(div6s_errors); > m_freem(m); > return (0); > } >
Re: diff: reboot(8): document the -l option
When you see a comment like this, it means it is intentionally undocumented. It is the current mechanism shutdown uses. It could use a different mechanism in the future. It is done this way so that noone else depends on it. If you document this user-visible, they will come to depend on it, then we cannot be agile and change the mechanism later. I don't think we don't want that, so no. If you persist at going through the tree you'll find about 20 other instances of this. It is a fairly common pattern. - case 'l': /* Undocumented; used by shutdown. */ > Hi, > > The following diff adds missing documentation for the -l option of > reboot(8) as its done in NetBSD. > > Bye, > Jan > > Index: reboot.8 > === > RCS file: /cvs/src/sbin/reboot/reboot.8,v > retrieving revision 1.50 > diff -u -p -r1.50 reboot.8 > --- reboot.8 3 Sep 2016 14:25:05 - 1.50 > +++ reboot.8 25 Mar 2019 14:12:48 - > @@ -30,7 +30,7 @@ > .\" > .\" @(#)reboot.88.1 (Berkeley) 6/9/93 > .\" > -.Dd $Mdocdate: September 3 2016 $ > +.Dd $Mdocdate: March 25 2019 $ > .Dt REBOOT 8 > .Os > .Sh NAME > @@ -39,9 +39,9 @@ > .Nd stopping and restarting the system > .Sh SYNOPSIS > .Nm halt > -.Op Fl dnpq > +.Op Fl dlnpq > .Nm reboot > -.Op Fl dnq > +.Op Fl dlnq > .Sh DESCRIPTION > The > .Nm halt > @@ -70,6 +70,10 @@ capturing the state of a corrupted or mi > See > .Xr savecore 8 > for information on how to recover this dump. > +.It Fl l > +Suppress sending a message via > +.Xr syslog 3 > +before halting or restarting. > .It Fl n > Prevent file system cache from being flushed. > This option should probably not be used. > Index: reboot.c > === > RCS file: /cvs/src/sbin/reboot/reboot.c,v > retrieving revision 1.38 > diff -u -p -r1.38 reboot.c > --- reboot.c 22 Aug 2017 00:30:16 - 1.38 > +++ reboot.c 25 Mar 2019 14:03:43 - > @@ -94,7 +94,7 @@ main(int argc, char *argv[]) > case 'd': > howto |= RB_DUMP; > break; > - case 'l': /* Undocumented; used by shutdown. */ > + case 'l': > lflag = 1; > break; > case 'n': > @@ -272,7 +272,7 @@ restart: > void > usage(void) > { > - fprintf(stderr, "usage: %s [-dn%sq]\n", __progname, > + fprintf(stderr, "usage: %s [-dln%sq]\n", __progname, > dohalt ? "p" : ""); > exit(1); > } >
Re: [patch] Re: Possible sasyncd memory leak ?
On Sat, Mar 23, 2019 at 06:07:02PM +0100, Michał Koc wrote: > ... [snip] This is almost good. You might fold host_ip() into net_set_sa(). the double malloc and copy isn't really needed. -Otto
diff: reboot(8): document the -l option
Hi, The following diff adds missing documentation for the -l option of reboot(8) as its done in NetBSD. Bye, Jan Index: reboot.8 === RCS file: /cvs/src/sbin/reboot/reboot.8,v retrieving revision 1.50 diff -u -p -r1.50 reboot.8 --- reboot.83 Sep 2016 14:25:05 - 1.50 +++ reboot.825 Mar 2019 14:12:48 - @@ -30,7 +30,7 @@ .\" .\"@(#)reboot.88.1 (Berkeley) 6/9/93 .\" -.Dd $Mdocdate: September 3 2016 $ +.Dd $Mdocdate: March 25 2019 $ .Dt REBOOT 8 .Os .Sh NAME @@ -39,9 +39,9 @@ .Nd stopping and restarting the system .Sh SYNOPSIS .Nm halt -.Op Fl dnpq +.Op Fl dlnpq .Nm reboot -.Op Fl dnq +.Op Fl dlnq .Sh DESCRIPTION The .Nm halt @@ -70,6 +70,10 @@ capturing the state of a corrupted or mi See .Xr savecore 8 for information on how to recover this dump. +.It Fl l +Suppress sending a message via +.Xr syslog 3 +before halting or restarting. .It Fl n Prevent file system cache from being flushed. This option should probably not be used. Index: reboot.c === RCS file: /cvs/src/sbin/reboot/reboot.c,v retrieving revision 1.38 diff -u -p -r1.38 reboot.c --- reboot.c22 Aug 2017 00:30:16 - 1.38 +++ reboot.c25 Mar 2019 14:03:43 - @@ -94,7 +94,7 @@ main(int argc, char *argv[]) case 'd': howto |= RB_DUMP; break; - case 'l': /* Undocumented; used by shutdown. */ + case 'l': lflag = 1; break; case 'n': @@ -272,7 +272,7 @@ restart: void usage(void) { - fprintf(stderr, "usage: %s [-dn%sq]\n", __progname, + fprintf(stderr, "usage: %s [-dln%sq]\n", __progname, dohalt ? "p" : ""); exit(1); }
Re: ospfd: Warn when the router ID changes during config reload
On Sun, Mar 24 2019, Mitchell Krome wrote: > On 24/03/2019 7:23 am, Theo de Raadt wrote: >> Sebastian Benoit wrote: >> >>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000: Was messing around with ospf and got myself into a situation where the router ID's were the same on two boxes because I only did a reload on one of them when I changed the loopback IP's. >>> >>> Thats sub optimal i believe... >>> This adds a warning when reloading if the router ID changes (there was already a comment saying as much). Same patch can probably be applied to ospf6d if people think it's useful. ospf6d currently doesn't support config reloads at all. It might be worth adding an XXX comment there. >>> I think it would be better to abort the reload if the router-id is changed, >>> i.e. not load the new config at all. >> >> That's the right approach in all our other daemons: >> >> if the configuration change cannot be installed correctly, consider >> it invalid and abort. Someone would need to write code to make it >> valid.. >> > > That makes sense. I checked the manuals for the routers I use at work > and they also required the ospf process to be restarted for the config > to take effect after changing the router id. > > I moved the check up into ospf_reload because it doesn't make sense > sending the config to all the children if we know we're going to abort. Your patch was mangled (long line wrapped) but the changes looked good. Here's an updated version which tweaks punctuation and case (to match the router-id keyword). Works for me in my simple test setup. Comments/oks? Index: ospfd.c === RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v retrieving revision 1.105 diff -u -p -r1.105 ospfd.c --- ospfd.c 15 Jan 2019 22:18:10 - 1.105 +++ ospfd.c 25 Mar 2019 13:33:43 - @@ -642,6 +642,13 @@ ospf_reload(void) if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL) return (-1); + /* Abort the reload if rtr_id changed */ + if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) { + log_warnx("router-id changed in new configuration, " + "this requires ospfd to be restarted."); + return (-1); + } + /* send config to childs */ if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1) return (-1); @@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st struct redistribute *r; int rchange = 0; - /* change of rtr_id needs a restart */ conf->flags = xconf->flags; conf->spf_delay = xconf->spf_delay; conf->spf_hold_time = xconf->spf_hold_time; -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: MPLSv6 2/2 : bgpd diff
On Sun, Mar 24, 2019 at 10:03:15PM +1300, Richard Procter wrote: > The ldpd issue might merit a hint in the man page as I found it difficult to > diagnose > as a newbie (see attached patch), and the man page, while not wrong, threw me > by > stating that GTSM is mandatory for LDPv6; it is now but wasn’t in the past. > > ok for the patch? > Thank you very much for testing and the report. I like the idea of being more specific in the manual. OK denis@, I would however remove the parenthesis. > Are there any other tests you’d be interested in while I have the machine > configured? > > best, > Richard. > > Here’s the version info of JUNOS I was using (note the git -dirty commit > (!!!)) > > uname -a: FreeBSD mx480-lab-re0 JNPR-10.3-20171207.04b87e3_buil FreeBSD > JNPR-10.3-20171207.04b87e3_builder_stable_10 #0 r356532+04b87e3(HEAD)-dirty: > Thu Dec 7 09:13:19 PST 2017 > buil...@basith.juniper.net:/volume/build/junos/occam/freebsd/stable_10/20171116.200501_builder_stable_10.04b87e3/obj/amd64/juniper/kernels/JNPR-AMD64-PRD/kernel > amd64 > > Model: mx480 > Family: junos > Junos: 17.2R2-S2.1 > > RFC5036 (2007) "LDP Specification" > > RFC6720 (2012) "The Generalized TTL Security Mechanism (GTSM) for LDP” - 1 > >> GTSM specifies that "it SHOULD NOT be enabled by default in order to > >> remain backward compatible with the unmodified protocol" (see > >> Section 3 of [RFC5082]). > >> > >> This document specifies a "built-in dynamic GTSM capability negotiation" > >> for > >> LDP to suggest the use of GTSM. GTSM will be used as specified in this > >> document provided both peers on an LDP session can detect each others' > >> support > >> for GTSM procedures and agree to use it. That is, the desire to use GTSM > >> (i.e., its negotiation mechanics) is enabled by default without any > >> configuration. > > RFC7552 (2015) "Updates to LDP for IPv6” - 5.1 > >> Also, the LDP Link Hello packets MUST have their IPv6 Hop Limit set > >> to 255, be checked for the same upon receipt (before any LDP-specific > >> processing), and be handled as specified in Section 3 of [RFC5082]. > > > cvs server: Diffing . > Index: ldpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/ldpd/ldpd.conf.5,v > retrieving revision 1.37 > diff -u -p -u -r1.37 ldpd.conf.5 > --- ldpd.conf.5 23 Jan 2019 02:02:04 - 1.37 > +++ ldpd.conf.5 24 Mar 2019 08:36:14 - > @@ -177,7 +177,8 @@ and RFC 7552 (for the IPv6 address-famil > Since GTSM is mandatory for LDPv6, the only effect of disabling GTSM for the > IPv6 address-family is that > .Xr ldpd 8 > -will not check the incoming packets' hop limit. > +will not check the incoming packets' hop limit. (This may be necessary to > +interoperate with implementations lacking RFC 7552 (2015) compliance.) > Outgoing packets will still be sent using a hop limit of 255 to guarantee > interoperability. > .Pp > >