Re: PATCH: better error return for exFAT filesystem
On Fri, Aug 07, 2020 at 12:59:00PM -0700, jo...@armadilloaerospace.com wrote: > Perform an explicit check for the unsupported exFAT MSDOS filesystem > instead of letting it fail mysteriously when it gets cluster sizes > of 0 from the normal fields. > > This causes mount_msdos to report: > mount_msdos: /dev/sd1i on /root/mnt: filesystem not supported by kernel > > Instead of the more obscure: > mount_msdos: /dev/sd1i on /root/mnt: Inapropriate file type or format > > Index: msdosfs_vfsops.c > === > RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v > retrieving revision 1.93 > diff -u -p -r1.93 msdosfs_vfsops.c > --- msdosfs_vfsops.c 24 Jan 2020 03:49:34 - 1.93 > +++ msdosfs_vfsops.c 7 Aug 2020 19:52:04 - > @@ -298,6 +298,12 @@ msdosfs_mountfs(struct vnode *devvp, str > b50 = (struct byte_bpb50 *)bsp->bs50.bsBPB; > b710 = (struct byte_bpb710 *)bsp->bs710.bsBPB; > > + /* No support for exFAT filesystems */ > + if (!strncmp("EXFAT", bsp->bs33.bsOemName, 5)) { > + error = EOPNOTSUPP; > + goto error_exit; > + } > + > pmp = malloc(sizeof *pmp, M_MSDOSFSMNT, M_WAITOK | M_ZERO); > pmp->pm_mountp = mp; The microsoft specification states it is EXFAT followed by three spaces if (!memcmp("EXFAT ", bsp->bs33.bsOemName, 8)) { may be more suitable here. fsck_msdos(8) likely needs a change to bail early if exFAT is found as well.
Re: brconfig: strto*l -> strtonum()
On Wed, Jul 29, 2020 at 07:39:43PM +0200, Klemens Nanni wrote: > > Poking and testing around in brconfig.c for tpmr(4) stuff, I noticed a > lot of old code around strto*l(3). > > Many pass unbounded `long' values into the `[u]int32_t' struct members > without limiting them to at least the type size the value is stored in, > some report wrong commands in error messages, e.g. > > # ifconfig switch1 portno vether1 9223372036854775808 # LONG_MAX + 1 > ifconfig: invalid arg for portidx: 9223372036854775808 > > some pass values to the kernel that are above limits documented in > ifconfig(8), e.g. > > # ifconfig bridge0 maxage 50 > ifconfig: bridge0: Invalid argument > > and others overflow in-kernel due to that (causing persistent damage > that causes deleting and adding bridge members to fix it). > > I'm aware that moving to strtonum(3) drops the ability to pass numbers > in bases others than ten, e.g. `ifconfig bridge0 ifcost vether0 0x3' > mut now be `... 15', but all of the options really want a decimal number > as far as I can judge after reading ifconfig(8) and testing them. > > The only exception to this is switch(4)'s `datapath' which is printed in > hexadecimal, so I left it untouched such that it can be copy-pasted and > set as is. > > Diff below applies converts all places to the very same strtonum() idiom > with proper range checks based on documented or type limits. > > I've tested each option manually: valid values are still valid and but > more out of range values are catched now, many of them up front in > ifconfig rather than the kerne's ioctl() path. > > I've also checked most of the ioctl() paths to see what those really > check for. One or two diffs for the kernel should follow soon. > > Feedback? OK? Ping. Alternatively, we can avoid duplicating the ioctl specific min/max values in strtonum(3) calls, just use the struct member type's *_MAX defines and rely on the kernel for appropiate boundary checks - this is what the code does now. Feedback? OK? Index: brconfig.c === RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v retrieving revision 1.28 diff -u -p -r1.28 brconfig.c --- brconfig.c 5 Aug 2020 06:22:11 - 1.28 +++ brconfig.c 8 Aug 2020 03:04:30 - @@ -419,18 +419,13 @@ void bridge_timeout(const char *arg, int d) { struct ifbrparam bp; - long newtime; - char *endptr; + const char *errstr; - errno = 0; - newtime = strtol(arg, , 0); - if (arg[0] == '\0' || endptr[0] != '\0' || - (newtime & ~INT_MAX) != 0L || - (errno == ERANGE && newtime == LONG_MAX)) - errx(1, "invalid arg for timeout: %s", arg); + bp.ifbrp_ctime = strtonum(arg, 0, UINT32_MAX, ); + if (errstr) + err(1, "timeout %s is: %s", arg, errstr); strlcpy(bp.ifbrp_name, ifname, sizeof(bp.ifbrp_name)); - bp.ifbrp_ctime = newtime; if (ioctl(sock, SIOCBRDGSTO, (caddr_t)) == -1) err(1, "%s", ifname); } @@ -439,17 +434,13 @@ void bridge_maxage(const char *arg, int d) { struct ifbrparam bp; - unsigned long v; - char *endptr; + const char *errstr; - errno = 0; - v = strtoul(arg, , 0); - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xffUL || - (errno == ERANGE && v == ULONG_MAX)) - errx(1, "invalid arg for maxage: %s", arg); + bp.ifbrp_maxage = strtonum(arg, 0, UINT8_MAX, ); + if (errstr) + errx(1, "maxage %s is: %s", arg, errstr); strlcpy(bp.ifbrp_name, ifname, sizeof(bp.ifbrp_name)); - bp.ifbrp_maxage = v; if (ioctl(sock, SIOCBRDGSMA, (caddr_t)) == -1) err(1, "%s", ifname); } @@ -458,17 +449,13 @@ void bridge_priority(const char *arg, int d) { struct ifbrparam bp; - unsigned long v; - char *endptr; + const char *errstr; - errno = 0; - v = strtoul(arg, , 0); - if (arg[0] == '\0' || endptr[0] != '\0' || v > 0xUL || - (errno == ERANGE && v == ULONG_MAX)) - errx(1, "invalid arg for spanpriority: %s", arg); + bp.ifbrp_prio = strtonum(arg, 0, UINT16_MAX, ); + if (errstr) + errx(1, "spanpriority %s is: %s", arg, errstr); strlcpy(bp.ifbrp_name, ifname, sizeof(bp.ifbrp_name)); - bp.ifbrp_prio = v; if (ioctl(sock, SIOCBRDGSPRI, (caddr_t)) == -1) err(1, "%s", ifname); } @@ -479,7 +466,7 @@ bridge_protect(const char *ifsname, cons struct ifbreq breq; unsigned long v; char *optlist, *str; - char *endptr; + const char *errstr; strlcpy(breq.ifbr_name, ifname, sizeof(breq.ifbr_name)); strlcpy(breq.ifbr_ifsname, ifsname, sizeof(breq.ifbr_ifsname)); @@ -492,11 +479,9 @@ bridge_protect(const char *ifsname, cons str = strtok(optlist, ","); while
Re: PATCH: better error return for exFAT filesystem
On Fri, Aug 07, 2020 at 12:59:00PM -0700, jo...@armadilloaerospace.com wrote: > Perform an explicit check for the unsupported exFAT MSDOS filesystem > instead of letting it fail mysteriously when it gets cluster sizes > of 0 from the normal fields. > > This causes mount_msdos to report: > mount_msdos: /dev/sd1i on /root/mnt: filesystem not supported by kernel > > Instead of the more obscure: > mount_msdos: /dev/sd1i on /root/mnt: Inapropriate file type or format Much better. I think this check for the complete string as per https://github.com/MicrosoftDocs/win32/blob/docs/desktop-src/FileIO/exfat-specification.md#312-filesystemname-field The FileSystemName field shall contain the name of the file system on the volume. The valid value for this field is, in ASCII characters, "EXFAT ", which includes three trailing white spaces. /* exFAT specification, 3.1.2 FileSystemName Field */ if (strcmp(bsp->bs33.bsOemName, "EXFAT ") != 0) { ... } > Index: msdosfs_vfsops.c > === > RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v > retrieving revision 1.93 > diff -u -p -r1.93 msdosfs_vfsops.c > --- msdosfs_vfsops.c 24 Jan 2020 03:49:34 - 1.93 > +++ msdosfs_vfsops.c 7 Aug 2020 19:52:04 - > @@ -298,6 +298,12 @@ msdosfs_mountfs(struct vnode *devvp, str > b50 = (struct byte_bpb50 *)bsp->bs50.bsBPB; > b710 = (struct byte_bpb710 *)bsp->bs710.bsBPB; > > + /* No support for exFAT filesystems */ > + if (!strncmp("EXFAT", bsp->bs33.bsOemName, 5)) { > + error = EOPNOTSUPP; > + goto error_exit; > + } > + > pmp = malloc(sizeof *pmp, M_MSDOSFSMNT, M_WAITOK | M_ZERO); > pmp->pm_mountp = mp; > > >
Re: PATCH: iostat spacing
On Fri, Aug 07, 2020 at 12:04:59PM -0700, jo...@armadilloaerospace.com wrote: > IO rates above 100 MB/s are common with SSD; this patch expands the > column so it stays neatly printed. This is OK with me as it fixes the default view, but I think other views need fixing as well, e.g. $ iostat -I ttysd0 sd1 cpu tin tout KB/t xfr MB KB/t xfr MB us ni sy sp in id 178524 22139320 26.99 10698431 281987.61 11.78 7582117 87218.82 7 0 3 1 0 88 I'm testing with `dd if=/dev/rsd0c of=/dev/null bs=1m' on a X230 with sd0 at scsibus1 targ 0 lun 0: naa.5002538d410a7bce > An argument can be made for expanding it one more for fast M.2 drives. I don't have that fast disks at hand, but expanding it by another column makes sense to me and could further simplify some of the code.
Re: Proxy-arp behavior change between 5.9 and 6.0
On 2020-07-29 21:47, Andrew Hewus Fresh wrote: > I am helping some folks with some OpenBSD stuff, including at some > point being more proactive about updating, but they currently have an > OpenBSD 5.9 machine that has been routing traffic happily for a while. > Unfortunately, they can't currently update because the proxy arp > configuration that they are using on 5.9 doesn't work on anything newer. > > > Here is the excellent description from someone there, and I was able to > reproduce the issue on my laptop, so I don't believe a dmesg will be > particularly helpful, but here's one for my laptop that's not terribly > old. > > https://dmesgd.nycbug.org/index.cgi?do=view=5279 > > As I haven't ever actually used proxy arp on OpenBSD, so don't actually > know whether this was a glitch that was fixed or added. > > I do see that 60.html says the routing table is now based on ART, so > that seems a possible culprit. > > --- > > > We've found that a proxy-arp configuration we've relied on (which works > perfectly on OpenBSD 5.9 and earlier) no longer works on OpenBSD 6.0 thru 6.7. > > On OpenBSD 5.9, we can add an "arp -s" entry for a public IP on our > public-facing gateway router's external interface, followed by a host-route > for that same public IP (pointing to an internal subnet/interface). This > allows us to route public IPs to clients through our internal private subnets, > so a client can have a public IP assigned on their router, despite actually > being several hops deep inside our private network. > > With this proxy-arp configuration, from both the client router's perspective > and from an external perspective, it looks as if the client router is in the > public IP subnet. > > Unfortunately this method doesn't work in OpenBSD 6.0 and later. > > > Here's how it looks when you configure this on OpenBSD 5.9 > (working as expected with no errors): > > root@openbsd-59-pub-gw-rtr~ # arp -s 1.2.3.4 > [PUB-GW_em0_EXT-IF_MAC-ADDR] pub > root@openbsd-59-pub-gw-rtr~ # route add 1.2.3.4/32 10.0.2.47 > add host 1.2.3.4/32: gateway 10.0.2.47 > root@openbsd-59-pub-gw-rtr~ # netstat -rn -finet|grep 1.2.3.4 > 1.2.3.4 10.0.2.47 UGHS 0 0 - 8 em1 > 1.2.3.4/32 [PUB-GW_em0_EXT-IF_MAC-ADDR] ULS2 0 0 - > 8 em0 > # ^ Success: Routing table shows both the host-route and the "arp > -s" entry. > > > That same setup fails on OpenBSD 6.0 and later > (tested on OpenBSD 6.0, 6.2, 6.6, and 6.7). > > > If you add an arp -s entry before adding a host route for the same IP, the > "route add" fails with "File exists" error: > > root@openbsd-67-pub-gw-rtr~ # arp -s 1.2.3.4 > [PUB-GW_em0_EXT-IF_MAC-ADDR] pub > root@openbsd-67-pub-gw-rtr~ # route add 1.2.3.4/32 10.0.2.47 > !-> add host 1.2.3.4/32: gateway 10.0.2.47: File exists > root@openbsd-67-pub-gw-rtr~ # netstat -rn -finet|grep 1.2.3.4 > 1.2.3.4/32 [PUB-GW_em0_EXT-IF_MAC-ADDR] ULS2 0 0 - > 8 em0 > # ^ Failure: Routing table only shows the "arp -s" entry (failed to > add host-route) > > > If you add the route before the arp -s entry, "arp -s" fails with error > "set: proxy entry exists for non 802 device": > > root@openbsd-67-pub-gw-rtr~ # route add 1.2.3.4/32 10.0.2.47 > add host 1.2.3.4/32: gateway 10.0.2.47 > root@openbsd-67-pub-gw-rtr~ # arp -s 1.2.3.4 > [PUB-GW_em0_EXT-IF_MAC-ADDR] pub > !-> set: proxy entry exists for non 802 device > root@openbsd-67-pub-gw-rtr~ # netstat -rn -finet|grep 1.2.3.4 > 1.2.3.4 10.0.2.47 UGHS 0 0 - 8 em1 > # ^ Failure: Routing table only shows the host-route (failed to add > arp -s entry) > > > For most of our clients who request a dedicated public IP, we're able to > assign their router a private IP, which we then bi-NAT to a dedicated public > IP on our gateway router. Unfortunately the bi-NAT method doesn't work for > certain routers' built-in VPNs, so we're looking for a way to re-create the > functionality we had with proxy-arp on OpenBSD v5.9. I imagine there's some > workaround or alternate method to achieve the same thing on OpenBSD 6.x, but I > haven't found it yet. You can avoid this problem by first setting the ARP entry, and then adding the host route with a lower (better) priority. On my system (which runs OpenBSD-CURRENT), ARP entries have priority 8, so your host route should have priority below 8. Of course, you need to do this before interfaces come up, as otherwise packets might be routed to the wrong destination. If you do this, you won’t be able to use arp(8) to delete the proxy ARP entry. This is due to a limitation in arp(8), which is fixed by the following patch. This patch has been tested on my -CURRENT system and also allows creating an entry even if a route entry (with a priority other than 8) already exists. I would appreciate
Re: allow TCP connections to IPv6 anycast addresses
On Fri, Aug 07 2020, Florian Obser wrote: > No longer prevent TCP connections to IPv6 anycast addresses. > > RFC 4291 dropped this requirement from RFC 3513: >o An anycast address must not be used as the source address of an > IPv6 packet. > > And from that requirement draft-itojun-ipv6-tcp-to-anycast rightly > concluded that TCP connections must be prevented. > > The draft also states: > > The proposed method MUST be removed when one of the following events > happens in the future: > > o Restriction imposed on IPv6 anycast address is loosened, so that >anycast address can be placed into source address field of the IPv6 >header[...] Also worth reading: https://tools.ietf.org/html/rfc4942#section-2.1.6 > OK? ok jca@ If you don't want to remove M_ACAST from sys/mbuf.h, can you please at least change the comment? /* obsolete */ or something. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: describe 'idle-timeout' exception in npppd.conf man page
On Fri, Aug 07, 2020 at 11:56:09PM +0300, Vitaliy Makkoveev wrote: > On Fri, Aug 07, 2020 at 09:29:13PM +0100, Jason McIntyre wrote: > > On Fri, Aug 07, 2020 at 10:19:05PM +0300, Vitaliy Makkoveev wrote: > > > Some times ago we disabled in-kernel timeout for pppx(4) related > > > pipex(4) sessions. We did this for prevent use after free issue caused > > > by pipex_timer [1]. By default "idle-timeout" is not set in > > > npppd.conf(5) and I guess this is reason for we forgot to describe this > > > exception in npppd.conf(5). > > > > > > But looks like one user caught this [2]. So I propose to describe this > > > in BUGS section of npppd.conf(5). > > > > > > Also current "idle-timeout" description looks incorrect. If this option > > > is missing, there is not in-kernel timeout for this session, but > > > npppd(8) uses it's own timeout for. And we can't configure this value. > > > > > > YASUOKA, what do you think? May be we can kill in-kernel timeout feature > > > for pipex(4)?, and make npppd(8)'s idle timeout configurable by this > > > option? > > > > > > 1. > > > https://cvsweb.openbsd.org/src/sys/net/if_pppx.c?rev=1.78=text/x-cvsweb-markup > > > 2. https://marc.info/?l=openbsd-misc=159655468504864=2 > > > > > > > > > Index: usr.sbin/npppd/npppd/npppd.conf.5 > > > === > > > RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v > > > retrieving revision 1.27 > > > diff -u -p -r1.27 npppd.conf.5 > > > --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - > > > 1.27 > > > +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 19:17:00 - > > > @@ -699,3 +699,9 @@ The current version of > > > .Xr npppd 8 > > > does not support adding or removing tunnel settings or changing listener > > > settings (listen address, port and l2tp-ipsec-require). > > > +.Pp > > > +This time > > > +.Xr pppx 4 > > > +does not allow to create sessions with non null > > > +.Ic idle-timeout > > > +option. > > > > > > > Thanks for your feedback. My English is bad, so thanks for fixing. > > > is this an actual bug? i'm just asking - it might be that the > > idle-timeout text is the best place to warn users, and not BUGS. > > It is pppx(4) related bug. Unfortunately it wasn't solved and we just > disabled this feature to avoid panics. May be pipex(4) man page is the > best place to describe this issue in BUGS section. > > > > > regarding your text: > > > > - "this time" is better written as "At this time" or "currently". > > - "allow to create" is not good sentence structure > > > > i think the text would read better something like: > > > > .Xr pppx 4 > > does not allow sessions with > > .Ic idle-timeout > > set to any value other than 0. > > > > I added this to pipex(4) BUGS section. > > > if the text was better placed in the idle-timeout section: > > > > This value must be 0 for > > .Xr pppx 4 > > sessions. > > And this to npppd.conf(5) idle-timeout section. > i think that's fine. jmc > > Index: share/man/man4/pipex.4 > === > RCS file: /cvs/src/share/man/man4/pipex.4,v > retrieving revision 1.12 > diff -u -p -r1.12 pipex.4 > --- share/man/man4/pipex.43 Apr 2020 07:46:04 - 1.12 > +++ share/man/man4/pipex.47 Aug 2020 20:54:32 - > @@ -288,3 +288,8 @@ The > .Nm > was written by > .An Internet Initiative Japan Inc . > +.Sh BUGS > +.Xr pppx 4 > +does not allow sessions with > +.Ic pr_timeout_sec > +set to any value other than 0. > Index: usr.sbin/npppd/npppd/npppd.conf.5 > === > RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v > retrieving revision 1.27 > diff -u -p -r1.27 npppd.conf.5 > --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - 1.27 > +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 20:54:32 - > @@ -325,6 +325,9 @@ The link is disconnected if there are no > for more than the amount of the > .Ar idle-timeout . > The default is 0, which disables the idle timer. > +This value must be 0 for > +.Xr pppx 4 > +sessions. > .It Ic tcp-mss-adjust Ar yes | no > If > .Dq yes >
Re: describe 'idle-timeout' exception in npppd.conf man page
On Fri, Aug 07, 2020 at 09:29:13PM +0100, Jason McIntyre wrote: > On Fri, Aug 07, 2020 at 10:19:05PM +0300, Vitaliy Makkoveev wrote: > > Some times ago we disabled in-kernel timeout for pppx(4) related > > pipex(4) sessions. We did this for prevent use after free issue caused > > by pipex_timer [1]. By default "idle-timeout" is not set in > > npppd.conf(5) and I guess this is reason for we forgot to describe this > > exception in npppd.conf(5). > > > > But looks like one user caught this [2]. So I propose to describe this > > in BUGS section of npppd.conf(5). > > > > Also current "idle-timeout" description looks incorrect. If this option > > is missing, there is not in-kernel timeout for this session, but > > npppd(8) uses it's own timeout for. And we can't configure this value. > > > > YASUOKA, what do you think? May be we can kill in-kernel timeout feature > > for pipex(4)?, and make npppd(8)'s idle timeout configurable by this > > option? > > > > 1. > > https://cvsweb.openbsd.org/src/sys/net/if_pppx.c?rev=1.78=text/x-cvsweb-markup > > 2. https://marc.info/?l=openbsd-misc=159655468504864=2 > > > > > > Index: usr.sbin/npppd/npppd/npppd.conf.5 > > === > > RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v > > retrieving revision 1.27 > > diff -u -p -r1.27 npppd.conf.5 > > --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - > > 1.27 > > +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 19:17:00 - > > @@ -699,3 +699,9 @@ The current version of > > .Xr npppd 8 > > does not support adding or removing tunnel settings or changing listener > > settings (listen address, port and l2tp-ipsec-require). > > +.Pp > > +This time > > +.Xr pppx 4 > > +does not allow to create sessions with non null > > +.Ic idle-timeout > > +option. > > > Thanks for your feedback. My English is bad, so thanks for fixing. > is this an actual bug? i'm just asking - it might be that the > idle-timeout text is the best place to warn users, and not BUGS. It is pppx(4) related bug. Unfortunately it wasn't solved and we just disabled this feature to avoid panics. May be pipex(4) man page is the best place to describe this issue in BUGS section. > > regarding your text: > > - "this time" is better written as "At this time" or "currently". > - "allow to create" is not good sentence structure > > i think the text would read better something like: > > .Xr pppx 4 > does not allow sessions with > .Ic idle-timeout > set to any value other than 0. > I added this to pipex(4) BUGS section. > if the text was better placed in the idle-timeout section: > > This value must be 0 for > .Xr pppx 4 > sessions. And this to npppd.conf(5) idle-timeout section. Index: share/man/man4/pipex.4 === RCS file: /cvs/src/share/man/man4/pipex.4,v retrieving revision 1.12 diff -u -p -r1.12 pipex.4 --- share/man/man4/pipex.4 3 Apr 2020 07:46:04 - 1.12 +++ share/man/man4/pipex.4 7 Aug 2020 20:54:32 - @@ -288,3 +288,8 @@ The .Nm was written by .An Internet Initiative Japan Inc . +.Sh BUGS +.Xr pppx 4 +does not allow sessions with +.Ic pr_timeout_sec +set to any value other than 0. Index: usr.sbin/npppd/npppd/npppd.conf.5 === RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v retrieving revision 1.27 diff -u -p -r1.27 npppd.conf.5 --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - 1.27 +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 20:54:32 - @@ -325,6 +325,9 @@ The link is disconnected if there are no for more than the amount of the .Ar idle-timeout . The default is 0, which disables the idle timer. +This value must be 0 for +.Xr pppx 4 +sessions. .It Ic tcp-mss-adjust Ar yes | no If .Dq yes
Re: describe 'idle-timeout' exception in npppd.conf man page
On Fri, Aug 07, 2020 at 10:19:05PM +0300, Vitaliy Makkoveev wrote: > Some times ago we disabled in-kernel timeout for pppx(4) related > pipex(4) sessions. We did this for prevent use after free issue caused > by pipex_timer [1]. By default "idle-timeout" is not set in > npppd.conf(5) and I guess this is reason for we forgot to describe this > exception in npppd.conf(5). > > But looks like one user caught this [2]. So I propose to describe this > in BUGS section of npppd.conf(5). > > Also current "idle-timeout" description looks incorrect. If this option > is missing, there is not in-kernel timeout for this session, but > npppd(8) uses it's own timeout for. And we can't configure this value. > > YASUOKA, what do you think? May be we can kill in-kernel timeout feature > for pipex(4)?, and make npppd(8)'s idle timeout configurable by this > option? > > 1. > https://cvsweb.openbsd.org/src/sys/net/if_pppx.c?rev=1.78=text/x-cvsweb-markup > 2. https://marc.info/?l=openbsd-misc=159655468504864=2 > > > Index: usr.sbin/npppd/npppd/npppd.conf.5 > === > RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v > retrieving revision 1.27 > diff -u -p -r1.27 npppd.conf.5 > --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - 1.27 > +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 19:17:00 - > @@ -699,3 +699,9 @@ The current version of > .Xr npppd 8 > does not support adding or removing tunnel settings or changing listener > settings (listen address, port and l2tp-ipsec-require). > +.Pp > +This time > +.Xr pppx 4 > +does not allow to create sessions with non null > +.Ic idle-timeout > +option. > is this an actual bug? i'm just asking - it might be that the idle-timeout text is the best place to warn users, and not BUGS. regarding your text: - "this time" is better written as "At this time" or "currently". - "allow to create" is not good sentence structure i think the text would read better something like: .Xr pppx 4 does not allow sessions with .Ic idle-timeout set to any value other than 0. if the text was better placed in the idle-timeout section: This value must be 0 for .Xr pppx 4 sessions. jmc
PATCH: better error return for exFAT filesystem
Perform an explicit check for the unsupported exFAT MSDOS filesystem instead of letting it fail mysteriously when it gets cluster sizes of 0 from the normal fields. This causes mount_msdos to report: mount_msdos: /dev/sd1i on /root/mnt: filesystem not supported by kernel Instead of the more obscure: mount_msdos: /dev/sd1i on /root/mnt: Inapropriate file type or format Index: msdosfs_vfsops.c === RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v retrieving revision 1.93 diff -u -p -r1.93 msdosfs_vfsops.c --- msdosfs_vfsops.c24 Jan 2020 03:49:34 - 1.93 +++ msdosfs_vfsops.c7 Aug 2020 19:52:04 - @@ -298,6 +298,12 @@ msdosfs_mountfs(struct vnode *devvp, str b50 = (struct byte_bpb50 *)bsp->bs50.bsBPB; b710 = (struct byte_bpb710 *)bsp->bs710.bsBPB; + /* No support for exFAT filesystems */ + if (!strncmp("EXFAT", bsp->bs33.bsOemName, 5)) { + error = EOPNOTSUPP; + goto error_exit; + } + pmp = malloc(sizeof *pmp, M_MSDOSFSMNT, M_WAITOK | M_ZERO); pmp->pm_mountp = mp;
describe 'idle-timeout' exception in npppd.conf man page
Some times ago we disabled in-kernel timeout for pppx(4) related pipex(4) sessions. We did this for prevent use after free issue caused by pipex_timer [1]. By default "idle-timeout" is not set in npppd.conf(5) and I guess this is reason for we forgot to describe this exception in npppd.conf(5). But looks like one user caught this [2]. So I propose to describe this in BUGS section of npppd.conf(5). Also current "idle-timeout" description looks incorrect. If this option is missing, there is not in-kernel timeout for this session, but npppd(8) uses it's own timeout for. And we can't configure this value. YASUOKA, what do you think? May be we can kill in-kernel timeout feature for pipex(4)?, and make npppd(8)'s idle timeout configurable by this option? 1. https://cvsweb.openbsd.org/src/sys/net/if_pppx.c?rev=1.78=text/x-cvsweb-markup 2. https://marc.info/?l=openbsd-misc=159655468504864=2 Index: usr.sbin/npppd/npppd/npppd.conf.5 === RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v retrieving revision 1.27 diff -u -p -r1.27 npppd.conf.5 --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 - 1.27 +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 19:17:00 - @@ -699,3 +699,9 @@ The current version of .Xr npppd 8 does not support adding or removing tunnel settings or changing listener settings (listen address, port and l2tp-ipsec-require). +.Pp +This time +.Xr pppx 4 +does not allow to create sessions with non null +.Ic idle-timeout +option.
PATCH: iostat spacing
IO rates above 100 MB/s are common with SSD; this patch expands the column so it stays neatly printed. An argument can be made for expanding it one more for fast M.2 drives. ? dkstats.d ? iostat ? iostat.d Index: iostat.c === RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v retrieving revision 1.42 diff -u -p -r1.42 iostat.c --- iostat.c14 Oct 2019 19:22:17 - 1.42 +++ iostat.c7 Aug 2020 19:00:45 - @@ -227,7 +227,7 @@ header(void) if (ISSET(todo, SHOW_TOTALS)) printf(" %18.18s ", cur.dk_name[i]); else - printf(" %16.16s ", cur.dk_name[i]); + printf(" %17.17s ", cur.dk_name[i]); } if (ISSET(todo, SHOW_STATS_2)) for (i = 0; i < dk_ndrive; i++) @@ -252,7 +252,7 @@ header(void) if (ISSET(todo, SHOW_TOTALS)) printf(" KB/t xfr MB "); else - printf(" KB/t t/s MB/s "); + printf(" KB/t t/s MB/s "); } if (ISSET(todo, SHOW_STATS_2)) for (i = 0; i < dk_ndrive; i++) @@ -299,10 +299,7 @@ disk_stats(double etime) (double)(1024 * 1024); else mbps = 0; - if (ISSET(todo, SHOW_TOTALS)) - printf(" %6.2f ", mbps / etime); - else - printf(" %5.2f ", mbps / etime); + printf(" %6.2f ", mbps / etime); } }
Fwd: pppx(4): move ifnet out of KERNEL_LOCK()
-- Forwarded message - From: Vitaliy Makkoveev Date: Fri, Aug 7, 2020 at 2:11 PM Subject: Re: pppx(4): move ifnet out of KERNEL_LOCK() To: Sven F. What reaction do you expect? Look, you did something and you got panic with *not* modified system. What is expected you will do? At least you described your actions which caused issue. It's great if you know how to reproduce it and you also describe required actions. Also dmesg(8) output welcomed. That's all. It's not expected you have C or OpenBSD internals knowledge. It's not expected you have a solution to fix. But without things I described above your problem report is *useless*. Also we have bugs@ list to report bugs. I guess posting to tech@ is welcomed if you understand the problem you caught and you have solution. Did you anything of things above? No you *didn't*. You have system modified by yourself. You caught panic. Who knows what did you modify? May be you are the person who introduced issue? Who ever knows? Do you expect someone of developers will spend his time and do your work instead of you? Are you serious? The second your appearance. You did something and you caught panic with generic system. You posted dmesg(8) output. Ok, but where is the description of actions you did? Imagine, you are the first person who caught this issue. We have no telepathy skills, your report is useless. But well, I looked in your dmesg(8) output and the problem looks the same problem with I work. I answered you what works is in progress. Also I said you there is no quick solution for. And my solution can be rejected by other developers. So there is nothing to share yet. The problem is not secret, but you looks not the person who understand the reasons which caused problems. Also your posts about netlock show you as person which don't understand haw this subsystem works. Do you remember I asked you what is the data you propose to protect by netlock? Looks like you don't understand the meaning of rwlocks. What is the reason for me or other developers to have discussion with you? To teach you how to code? What is my interest? And don't assume my reply as "fuck off stupid user". It's *not*. I spend my time to explain you obvious things. If you are skilled enough, if you understand what you do - you are welcomed. But you should *describe* the problem and *describe* what you are doing. That's enough. If you are wrong you will be corrected. But unfortunately it's looks like it's not about you. -- -- - Knowing is not enough; we must apply. Willing is not enough; we must do
current.html: i586 requirement for i386 architecture
Now that i386 platform requires i586 CPU, I guess we should mention it in current.html (the page i386.html should be updated accordingly at 6.8 release) Index: current.html === RCS file: /home/reposync/www/faq/current.html,v retrieving revision 1.1048 diff -u -p -r1.1048 current.html --- current.html11 Jul 2020 16:53:47 - 1.1048 +++ current.html7 Aug 2020 13:43:25 - @@ -57,6 +57,16 @@ use a snapshot to recover. Most of these changes will have to be performed as root. +2020/08/03 - i386 requirements raised to i586 + +Due to llvm upgrade to version 10 in base system, the minimal +CPU instruction sets requirements for i386 architecture has been +raised from i386 to i586. + +In concrete terms for Intel and AMD CPUs, Intel Pentium and AMD K5 +are now the minimal requirements for i386 architecture. + + 2020/05/29 - [ports] net/isc-bind example files dropped Outdated example configuration (including some "standard" zone files and
allow TCP connections to IPv6 anycast addresses
No longer prevent TCP connections to IPv6 anycast addresses. RFC 4291 dropped this requirement from RFC 3513: o An anycast address must not be used as the source address of an IPv6 packet. And from that requirement draft-itojun-ipv6-tcp-to-anycast rightly concluded that TCP connections must be prevented. The draft also states: The proposed method MUST be removed when one of the following events happens in the future: o Restriction imposed on IPv6 anycast address is loosened, so that anycast address can be placed into source address field of the IPv6 header[...] OK? diff --git share/man/man9/mbuf.9 share/man/man9/mbuf.9 index 6f798945437..ab02c36798f 100644 --- share/man/man9/mbuf.9 +++ share/man/man9/mbuf.9 @@ -306,8 +306,6 @@ protocol-specific. variable is valid. .It Dv M_LOOP packet has been sent from local machine. -.It Dv M_ACAST -received as IPv6 anycast. .It Dv M_BCAST packet sent/received as link-level broadcast. .It Dv M_MCAST diff --git sys/netinet/ip_input.c sys/netinet/ip_input.c index 1b511d14a4b..40c2f675959 100644 --- sys/netinet/ip_input.c +++ sys/netinet/ip_input.c @@ -619,20 +619,6 @@ ip_deliver(struct mbuf **mp, int *offp, int nxt, int af) goto bad; } -#ifdef INET6 - /* draft-itojun-ipv6-tcp-to-anycast */ - if (af == AF_INET6 && - ISSET((*mp)->m_flags, M_ACAST) && (nxt == IPPROTO_TCP)) { - if ((*mp)->m_len >= sizeof(struct ip6_hdr)) { - icmp6_error(*mp, ICMP6_DST_UNREACH, - ICMP6_DST_UNREACH_ADDR, - offsetof(struct ip6_hdr, ip6_dst)); - *mp = NULL; - } - goto bad; - } -#endif /* INET6 */ - #ifdef IPSEC if (ipsec_in_use) { if (ipsec_local_check(*mp, *offp, nxt, af) != 0) { diff --git sys/netinet6/ip6_input.c sys/netinet6/ip6_input.c index 64489f53b48..6c1beb6866c 100644 --- sys/netinet6/ip6_input.c +++ sys/netinet6/ip6_input.c @@ -424,8 +424,6 @@ ip6_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp) */ if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL)) { struct in6_ifaddr *ia6 = ifatoia6(rt->rt_ifa); - if (ia6->ia6_flags & IN6_IFF_ANYCAST) - m->m_flags |= M_ACAST; if (ip6_forwarding == 0 && rt->rt_ifidx != ifp->if_index && !((ifp->if_flags & IFF_LOOPBACK) || -- I'm not entirely sure you are real.
Re: pppx(4): move ifnet out of KERNEL_LOCK()
On Thu, Aug 6, 2020 at 8:11 AM Vitaliy Makkoveev wrote: > > On Thu, Aug 06, 2020 at 01:25:14PM +0200, Martin Pieuchot wrote: > > On 05/08/20(Wed) 12:50, Vitaliy Makkoveev wrote: > > > pipex(4) and pppx(4) are ready to became a little bit more MP capable. > > > Diff below moves pppx(4) related `ifnet' out of KERNEL_LOCK(). > > > > Nice, one comment below. > > > > > Index: sys/net/if_pppx.c > > > > > > [skip] > > > > > > + NET_LOCK(); > > > pipex_ppp_output(m, pxi->pxi_session, proto); > > > + NET_UNLOCK(); > > > > This means the lock is taken and released for every packet. It would be > > better to grab it outside the loop. > > Ok, fixed. > > Index: sys/net/if_pppx.c > === > RCS file: /cvs/src/sys/net/if_pppx.c,v > retrieving revision 1.98 > diff -u -p -r1.98 if_pppx.c > --- sys/net/if_pppx.c 28 Jul 2020 09:53:36 - 1.98 > +++ sys/net/if_pppx.c 6 Aug 2020 11:54:44 - > @@ -191,7 +191,7 @@ int pppx_set_session_descr(struct pppx_ > struct pipex_session_descr_req *); > > void pppx_if_destroy(struct pppx_dev *, struct pppx_if *); > -void pppx_if_start(struct ifnet *); > +void pppx_if_qstart(struct ifqueue *); > intpppx_if_output(struct ifnet *, struct mbuf *, > struct sockaddr *, struct rtentry *); > intpppx_if_ioctl(struct ifnet *, u_long, caddr_t); > @@ -683,13 +683,12 @@ pppx_add_session(struct pppx_dev *pxd, s > snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit); > ifp->if_mtu = req->pr_peer_mru; /* XXX */ > ifp->if_flags = IFF_POINTOPOINT | IFF_MULTICAST | IFF_UP; > - ifp->if_xflags = IFXF_CLONED; > - ifp->if_start = pppx_if_start; > + ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE; > + ifp->if_qstart = pppx_if_qstart; > ifp->if_output = pppx_if_output; > ifp->if_ioctl = pppx_if_ioctl; > ifp->if_rtrequest = p2p_rtrequest; > ifp->if_type = IFT_PPP; > - ifq_set_maxlen(>if_snd, 1); > ifp->if_softc = pxi; > /* ifp->if_rdomain = req->pr_rdomain; */ > > @@ -864,21 +863,15 @@ pppx_if_destroy(struct pppx_dev *pxd, st > } > > void > -pppx_if_start(struct ifnet *ifp) > +pppx_if_qstart(struct ifqueue *ifq) > { > + struct ifnet *ifp = ifq->ifq_if; > struct pppx_if *pxi = (struct pppx_if *)ifp->if_softc; > struct mbuf *m; > int proto; > > - if (!ISSET(ifp->if_flags, IFF_RUNNING)) > - return; > - > - for (;;) { > - m = ifq_dequeue(>if_snd); > - > - if (m == NULL) > - break; > - > + NET_LOCK(); > + while ((m = ifq_dequeue(ifq)) != NULL) { > proto = *mtod(m, int *); > m_adj(m, sizeof(proto)); > > @@ -887,6 +880,7 @@ pppx_if_start(struct ifnet *ifp) > > pipex_ppp_output(m, pxi->pxi_session, proto); > } > + NET_UNLOCK(); > } > > int > Hey , I think putting things out of locks while there is known race condition in code, is hurting. I was candidly asking polite questions because I create those races sometimes, but apparently the work in progress must be kept secret . Because it is a work in progress reading code may just mislead into : <> but I think it is hurting the project to have known issues like that, in the network stack. Never saw OpenBSD do DDB> outside some strange drivers or snapshots, since like 3.5 ? But way more recently. Currently I'm asking myself why each call of if_hooks_run in if.c are surrounded by NET_LOCK and then do another mutex : mtx_enter(_hooks_mtx); But then call (*func)(arg); While some deletion of func are not protected by any lock like here, in XX_detach which is not NET_LOCK nor _hooks_mtx locked: int lacp_detach(struct trunk_softc *sc) { struct lacp_softc *lsc = LACP_SOFTC(sc); KASSERT(TAILQ_EMPTY(>lsc_aggregators)); KASSERT(lsc->lsc_active_aggregator == NULL); sc->tr_psc = NULL; Most enthusiastic users would suffer a lot from a failing PPP driver ( not me ). I understand the work is complex ( because as usual OpenBSD is trying to do the right thing ). Maybe a define 'MP_NET_STACK' could activate/deactivate optimization for MP architecture so users can easily switch between tests and stable for this complex task. Because validating Races is not an easy task especially on modern processors. Thanks for the great MP optimization work. Have a good weekend y'all.
Re: current.html: i586 requirement for i386 architecture
On Fri, Aug 07, 2020 at 09:57:37AM -0400, Bryan Steele wrote: > On Fri, Aug 07, 2020 at 03:49:32PM +0200, Solene Rapenne wrote: > > Now that i386 platform requires i586 CPU, I guess we should mention > > it in current.html (the page i386.html should be updated accordingly > > at 6.8 release) > > > > Index: current.html > > === > > RCS file: /home/reposync/www/faq/current.html,v > > retrieving revision 1.1048 > > diff -u -p -r1.1048 current.html > > --- current.html11 Jul 2020 16:53:47 - 1.1048 > > +++ current.html7 Aug 2020 13:43:25 - > > @@ -57,6 +57,16 @@ use a snapshot to recover. > > Most of these changes will have to be performed as root. > > > > > > +2020/08/03 - i386 requirements raised to i586 > > + > > +Due to llvm upgrade to version 10 in base system, the minimal > > +CPU instruction sets requirements for i386 architecture has been > > +raised from i386 to i586. > > I can't speak as to whether or not this is warrented or not, but this I think it's worthwhile to have this since it will likely be picked up for the upgrade notes from here as well. Please add new entries to the end of the file, not the beginning. > part is at least wrong. i386 CPU support was dropped years ago in the > kernel, only 486-class processors and higher (with a math-coprocessor) > are supported. > > The real hint here might be that cpu0: must contain "CX8" to reliably > work from now on, which indicates support for the CMPXCHG8 instruction. > > > +In concrete terms for Intel and AMD CPUs, Intel Pentium and AMD K5 > > +are now the minimal requirements for i386 architecture. > > + > > + > > 2020/05/29 - [ports] net/isc-bind example files > > dropped > > > > Outdated example configuration (including some "standard" zone files and > > > > > -- I'm not entirely sure you are real.
Reflect recent wsfontload(8) changes in the man page
Hi tech@, We are now getting the default values for font height and width using the WSDISPLAYIO_GETSCREENTYPE ioctl, so they always match the currently loaded font metrics. The following diff reflects that in the man page. As text-mode VGA compatible displays are increasingly uncommon, I took the liberty to switch the order and to mention raster displays first. Comments? OK? Index: usr.sbin/wsfontload/wsfontload.8 === RCS file: /cvs/src/usr.sbin/wsfontload/wsfontload.8,v retrieving revision 1.20 diff -u -p -r1.20 wsfontload.8 --- usr.sbin/wsfontload/wsfontload.83 Apr 2017 18:43:41 - 1.20 +++ usr.sbin/wsfontload/wsfontload.87 Aug 2020 13:40:26 - @@ -85,8 +85,8 @@ Default is .Pa /dev/ttyCcfg . .It Fl h Ar height Sets the height of a font character in pixels. -Default is 16 for text-mode VGA compatible displays, -and 22 for raster displays. +Default is to match the currently loaded font height for raster displays, +and 16 for text-mode VGA compatible displays. .It Fl l Specifies to print out a list of loaded fonts, no other arguments should be specified. @@ -97,8 +97,8 @@ If none is given, the name is used to create one. .It Fl w Ar width Sets the width of a font character in pixels. -Default is 8 for text-mode VGA compatible displays, -and 12 for raster displays. +Default is to match the currently loaded font width for raster displays, +and 8 for text-mode VGA compatible displays. .El .Pp .\" Typically, the
Re: current.html: i586 requirement for i386 architecture
On Fri, Aug 07, 2020 at 03:49:32PM +0200, Solene Rapenne wrote: > Now that i386 platform requires i586 CPU, I guess we should mention > it in current.html (the page i386.html should be updated accordingly > at 6.8 release) > > Index: current.html > === > RCS file: /home/reposync/www/faq/current.html,v > retrieving revision 1.1048 > diff -u -p -r1.1048 current.html > --- current.html 11 Jul 2020 16:53:47 - 1.1048 > +++ current.html 7 Aug 2020 13:43:25 - > @@ -57,6 +57,16 @@ use a snapshot to recover. > Most of these changes will have to be performed as root. > > > +2020/08/03 - i386 requirements raised to i586 > + > +Due to llvm upgrade to version 10 in base system, the minimal > +CPU instruction sets requirements for i386 architecture has been > +raised from i386 to i586. I can't speak as to whether or not this is warrented or not, but this part is at least wrong. i386 CPU support was dropped years ago in the kernel, only 486-class processors and higher (with a math-coprocessor) are supported. The real hint here might be that cpu0: must contain "CX8" to reliably work from now on, which indicates support for the CMPXCHG8 instruction. > +In concrete terms for Intel and AMD CPUs, Intel Pentium and AMD K5 > +are now the minimal requirements for i386 architecture. > + > + > 2020/05/29 - [ports] net/isc-bind example files > dropped > > Outdated example configuration (including some "standard" zone files and > >
Re: $pexp in re.subr(8)
On 2020/08/06 18:12, Thomas Levine wrote: > The present patch changes the rc.subr(8) manual page to match > the implementation. > > The current manual page for rc.subr(8) says that $pexp is "A regular > expression to be passed to pgrep(1) in order to find the desired process > or to be passed to pkill(1) to stop it." > > The file /etc/rc.d/rc.subr currently uses $pexp only as a fixed > expression, with the -xf flags. > > > grep pexp /etc/rc.d/rc.subr > pexp=${pexp} > multicast nfs_server pexp pf pkg_scripts shlib_dirs spamd_black > pgrep -T "${daemon_rtable}" -q -xf "${pexp}" > pkill -HUP -T "${daemon_rtable}" -xf "${pexp}" > pkill -T "${daemon_rtable}" -xf "${pexp}" > # make sure pexp matches the process (i.e. doesn't include the quotes) > pexp="$(eval echo ${daemon}${daemon_flags:+ ${daemon_flags}})" > > The -xf flags are explained in the pgrep(1) and pkill(1) man page. > > -x Require an exact match of the process name, or argument > list if -f is given. The default is to match any substring. This means that the regular expression must match the full process string. Equivalent to providing an expression with ^ at the start and $ at the end of the. > > The present patch replaces instances of "regular expression" with > "fixed expression". > > I also think a better phrasing would be to explain in the rc.subr(8) > man page that $pexp is a substring of the process name with flags and > that it uses pgrep with -xf. I might eventually do that in a separate > patch. > > > Index: rc.subr.8 > === > RCS file: /cvs/src/share/man/man8/rc.subr.8,v > retrieving revision 1.37 > diff -u -p -r1.37 rc.subr.8 > --- rc.subr.8 21 Feb 2020 00:47:21 - 1.37 > +++ rc.subr.8 7 Aug 2020 00:46:34 - > @@ -184,7 +184,7 @@ call > .It Ic rc_check > Search for processes of the service with > .Xr pgrep 1 > -using the regular expression given in the > +using the fixed expression given in the > .Va pexp > variable. > .It Ic rc_start > @@ -199,7 +199,7 @@ Send a > .Dv SIGTERM > signal using > .Xr pkill 1 > -on the regular expression given in the > +on the fixed expression given in the > .Va pexp > variable. > .It Ic rc_reload > @@ -207,7 +207,7 @@ Send a > .Dv SIGHUP > signal using > .Xr pkill 1 > -on the regular expression given in the > +on the fixed expression given in the > .Va pexp > variable. > One has to make sure that sending > @@ -267,7 +267,7 @@ functions. > User to run the daemon as, using > .Xr su 1 . > .It Va pexp > -A regular expression to be passed to > +A fixed expression to be passed to > .Xr pgrep 1 > in order to find the desired process or to be passed to > .Xr pkill 1 >