Re: $pexp in re.subr(8)
On Thu, August 6, 2020 9:12 pm, 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.
>
> The present patch replaces instances of "regular expression" with
> "fixed expression".
>
I'm not sure what you mean by a "fixed expression". A string? But reread
the pgrep/pkill manpage. They take a pattern as an operand which is
described to be an extended regular expression.
This is true. Even with -x.
Try it.
$pexp in re.subr(8)
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.
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
Re: exFAT support
Hi, in addition to what Bryan said... This message is wildly off-topic on tech@. If you reply, please reply to misc@. Quoting from https://www.openbsd.org/mail.html (please read that!): Developer lists: [...] tech@openbsd.org Discussion of technical topics for OpenBSD developers and advanced users. This is _not_ a "tech support" forum - do not use it as such. jo...@armadilloaerospace.com wrote on Thu, Aug 06, 2020 at 02:16:11PM -0700: > I tried to mount a 12TB USB drive, and was getting an "Inappropriate > file type or format" error. Even on misc@, when asking questions, please state what you are actually doing, showing the exact commands you type and the exact output you get, together in the original order, for example like this: isnote# mount -t msdos /dev/sd1a /mnt mount_msdos: /dev/sd1a on /mnt: Inappropriate file type or format > It turned out to be due to exFAT formatting, but it took me some > investigating to figure that out. Would it be reasonable to have the > kernel print You didn't say where you saw the message, but i assume it was the output of mount(8), not kernel output on the system console or in /var/log/messages. The kernel doesn't print such messages. The application program (e.g. mount(8)) prints them. All the kernel does is return an errno(2) to the mount(2) syscall. So you can find an exhaustive list of the messages that could be printed in the errno(2) manual page. Also, which errno(2) is returned from syscalls in which error case is not arbitrary and cannot be changed arbitrarily. > a more informative warning like "exFAT filesystem not > supported" when you try to mount it with mount_msdos, or are additional > kernel prints considered bad form? I bet if the kernel had printed anything, you wouldn't even have noticed. Besides, yes indeed, the kernel is absolutely not supposed to print anything when users type wrong commands. Yours, Ingo
Re: exFAT support
On Thu, Aug 06, 2020 at 02:16:11PM -0700, jo...@armadilloaerospace.com wrote: > I tried to mount a 12TB USB drive, and was getting an "Inappropriate > file type or format" error. > > It turned out to be due to exFAT formatting, but it took me some > investigating to figure that out. Would it be reasonable to have the > kernel print a more informative warning like "exFAT filesystem not > supported" when you try to mount it with mount_msdos, or are additional > kernel prints considered bad form? Yes, EFTYPE error is pretty much the best you can do. mount_msdos is only for FAT12/16/32, in the same way you can't mount FFS with it or NTFS. More informative errors here would require writing a lot more parsing code in the kernel, which is already a sensitive area. > With Microsoft's release of the spec last year, is the path open for > kernel support now, when someone gets around to it? I don't know the details, but I believe one issue with exFAT has been Microsoft and software patents, not just available documentation. Linux may eventually get a kernel implementation but I'm not sure that helps us. https://www.zdnet.com/article/microsoft-readies-exfat-patents-for-linux-and-open-source/ Maybe that's not important, and it's just a lack of both interest and capable developers hacking on new filesystems. > I have seen some comments about the reliability of exfat-fuse, will I > be sad if I try to use it for something important? I mean, can mount the volume read-only if you don't intend to write. If interoperability isn't a concern, choosing a different filesystem (e.g. FFS2) might be alternative option. Hope that helps, -Bryan.
exFAT support
I tried to mount a 12TB USB drive, and was getting an "Inappropriate file type or format" error. It turned out to be due to exFAT formatting, but it took me some investigating to figure that out. Would it be reasonable to have the kernel print a more informative warning like "exFAT filesystem not supported" when you try to mount it with mount_msdos, or are additional kernel prints considered bad form? With Microsoft's release of the spec last year, is the path open for kernel support now, when someone gets around to it? I have seen some comments about the reliability of exfat-fuse, will I be sad if I try to use it for something important?
Re: mountd: Avoid reading one byte before buffer
Length checks should generally be done before the dereference :-) OK millert@ - todd
mountd: Avoid reading one byte before buffer
>From CheriBSD, via FreeBSD:
| Avoid reading one byte before the path buffer.
|
| This happens when there's only one component (e.g. "/foo"). This
| (mostly-harmless) bug has been present since June 1990 when it was
| commited to mountd.c SCCS version 5.9.
|
| Note: the bug is on the second changed line, the first line is changed
| for visual consistency.
https://svnweb.freebsd.org/base?view=revision=363435
You need to look at the surrounding loop to see the problem.
OK?
Index: sbin/mountd/mountd.c
===
RCS file: /cvs/src/sbin/mountd/mountd.c,v
retrieving revision 1.88
diff -u -p -r1.88 mountd.c
--- sbin/mountd/mountd.c24 Jan 2020 18:51:45 - 1.88
+++ sbin/mountd/mountd.c6 Aug 2020 14:41:16 -
@@ -2021,9 +2021,9 @@ do_mount(struct exportlist *ep, struct g
#endif
}
/* back up over the last component */
- while (*cp == '/' && cp > dirp)
+ while (cp > dirp && *cp == '/')
cp--;
- while (*(cp - 1) != '/' && cp > dirp)
+ while (cp > dirp && *(cp - 1) != '/')
cp--;
if (cp == dirp) {
if (debug)
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: pppx(4): move ifnet out of KERNEL_LOCK()
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
Re: pppx(4): move ifnet out of KERNEL_LOCK()
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
> ===
> 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 5 Aug 2020 09:34:50 -
> @@ -864,28 +863,23 @@ 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;
> -
> + while ((m = ifq_dequeue(ifq)) != NULL) {
> proto = *mtod(m, int *);
> m_adj(m, sizeof(proto));
>
> ifp->if_obytes += m->m_pkthdr.len;
> ifp->if_opackets++;
>
> + 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.
> }
> }
