df(1): formatting adjustments and -T support
I noticed that large disk volumes cause problems with the formatting of
numerical columns in df(1), particularly when using -i. Here's a patch
that pads out their width a bit and raises the maximum width of
numerical columns before fields start running into each other.
I also added support for the -T argument, which is present in many
other df(1) implementations and shows the filesystem's type.
POSIX-style -P output is not affected by this since it has a specific
format. The affected flags don't support -P anyway.
I believe this follows the KNF style (at least that's what my editor
says) but if there are any problems please let me know.
diff --git bin/df/df.1 bin/df/df.1
index 89dd51aac8d..4a9a549c492 100644
--- bin/df/df.1
+++ bin/df/df.1
@@ -100,6 +100,10 @@ with the possibly stale statistics that were
previously obtained.
.It Fl P
Print out information in a stricter format designed to be parsed
by portable scripts.
+.It Fl T
+Include the filesystem type. This option is incompatible with the
+.Fl P
+option.
.It Fl t Ar type
Indicate the actions should only be taken on
file systems of the specified
diff --git bin/df/df.c bin/df/df.c
index fd51f906f89..427d774e802 100644
--- bin/df/df.c
+++ bin/df/df.c
@@ -63,7 +63,7 @@ extern int e2fs_df(int, char *, struct statfs
*);
extern int ffs_df(int, char *, struct statfs *);
static int raw_df(char *, struct statfs *);
-inthflag, iflag, kflag, lflag, nflag, Pflag;
+inthflag, iflag, kflag, lflag, nflag, Pflag, Tflag;
char **typelist = NULL;
int
@@ -79,7 +79,7 @@ main(int argc, char *argv[])
if (pledge("stdio rpath", NULL) == -1)
err(1, "pledge");
- while ((ch = getopt(argc, argv, "hiklnPt:")) != -1)
+ while ((ch = getopt(argc, argv, "hiklnPTt:")) != -1)
switch (ch) {
case 'h':
hflag = 1;
@@ -106,14 +106,17 @@ main(int argc, char *argv[])
errx(1, "only one -t option may be
specified.");
maketypelist(optarg);
break;
+ case 'T':
+ Tflag = 1;
+ break;
default:
usage();
}
argc -= optind;
argv += optind;
- if ((iflag || hflag) && Pflag) {
- warnx("-h and -i are incompatible with -P");
+ if ((iflag || hflag || Tflag) && Pflag) {
+ warnx("-h, -i, and -T are incompatible with -P");
usage();
}
@@ -159,13 +162,17 @@ main(int argc, char *argv[])
}
if (mntsize) {
- maxwidth = 11;
+ maxwidth = 13;
for (i = 0; i < mntsize; i++) {
width = strlen(mntbuf[i].f_mntfromname);
if (width > maxwidth)
maxwidth = width;
+ if (Tflag) {
+ width =
strlen(mntbuf[i].f_fstypename);
+ if (width > maxwidth)
+ maxwidth = width;
+ }
}
-
if (Pflag)
posixprint(mntbuf, mntsize, maxwidth);
else
@@ -189,6 +196,20 @@ getmntpt(char *name)
return (0);
}
+char *
+getfsname(char *name)
+{
+ long mntsize, i;
+ struct statfs *mntbuf;
+
+ mntsize = getmntinfo(, MNT_NOWAIT);
+ for (i = 0; i < mntsize; i++) {
+ if (!strcmp(mntbuf[i].f_mntfromname, name))
+ return (mntbuf[i].f_fstypename);
+ }
+ return (0);
+}
+
static enum { IN_LIST, NOT_IN_LIST } which;
static int
@@ -319,20 +340,23 @@ prtstat(struct statfs *sfsp, int maxwidth, int
headerlen, int blocksize)
if (hflag)
prthuman(sfsp, used);
else
- (void)printf(" %*llu %9llu %9lld", headerlen,
+ (void)printf(" %*llu %10llu %10lld", headerlen,
fsbtoblk(sfsp->f_blocks, sfsp->f_bsize,
blocksize),
fsbtoblk(used, sfsp->f_bsize, blocksize),
fsbtoblk(sfsp->f_bavail, sfsp->f_bsize,
blocksize));
(void)printf(" %5.0f%%",
availblks == 0 ? 100.0 : (double)used / (double)availblks
* 100.0);
+ if (Tflag) {
+ (void)printf("%4s", sfsp->f_fstypename);
+ }
if (iflag) {
inodes = sfsp->f_files;
used = inodes - sfsp->f_ffree;
- (void)printf(" %7llu %7llu %5.0f%% ", used, sfsp-
>f_ffree,
+ (void)printf(" %9llu %9llu %5.0f%% ", used, sfsp-
>f_ffree,
inodes == 0 ? 100.0 : (double)used / (double)inodes
* 100.0);
} else
(void)printf(" ");
- (void)printf(" %s\n", sfsp->f_mntonname);
+ (void)printf("%s\n", sfsp->f_mntonname);
}
/*
@@ -359,12 +383,14 @@
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 02:26:34PM -0500, Bryan Steele wrote: > On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote: > > On Fri, 15 Jan 2021 22:41:13 +0100 > > Marcus Glocker wrote: > > > > > On Fri, 15 Jan 2021 11:37:47 -0500 > > > Bryan Steele wrote: > > > > > > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > > > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > > > > > > > > > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote: > > > > > > > > > > > > > > I have heard from others who tried the diff that the PS4 > > > > > > > > controller is causing problems with the way it attaches. I > > > > > > > > ordered one to trial-and- error this myself at home. Could > > > > > > > > you share output of lsusb -vv? Thanks for giving it a try! > > > > > > > > > > > > > > > > > > > > > > Sure, here we go. > > > > > > > If I can find anything myself in the meantime I let you know. > > > > > > > > > > > > > > > > > > > So the problem doesn't seem to be in your new ujoy(4) code, but > > > > > > how the dev/hid/hid.c:hid_is_collection() function tries to cope > > > > > > with the PS4 controller. > > > > > > > > > > So with the hid_is_collection() problem not easy to mitigate [1], > > > > > should we table the ujoy(4) proposal for now pending a fix for the > > > > > problems with the PS4 controller? Or is this interesting enough > > > > > for some to work on moving forward despite this issue and finding > > > > > a solution for this specific (and in some ways unusual) device > > > > > later? > > > > > > > > Considering the hid_is_collection() fix from NetBSD proposed by > > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers > > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now? > > > > It's fairly small helper function. Like hid_is_joy_collection()? > > > > > > I think that could be an XXX workaround until somebody finds a proper, > > > generic fix for hid_is_collection() ... > > > > That's what you meant, yes? > > Yep, this is what I meant. Hopefully it won't have to stay for long, but > t would be nice to get ujoy(4) working as many controllers as possible > so that it can go in. > > Can you send a new full diff thfr@? I still need to test with my USB > SNES controller, but tentatively this is still ok brynet@ > > -Bryan. Yep, works fine with my generic USB SNES controller, tested with usbhidctl -f /dev/ujoy/0 -l, shows all button presses. uhidev4 at uhub0 port 12 configuration 1 interface 0 "vendor 0x0810 usb gamepad" rev 1.00/1.06 addr 2 uhidev4: iclass 3/0, 1 report id ujoy0 at uhidev4 reportid 1: input=7, output=0, feature=0 I think this is ready to go in. :-) -Bryan.
Patch for crypt(3) man page.
Hello everybody. I sent an email to the misc@ list yesterday regarding a patch proposal for a man page. Upon request, I am submitting the same proposal here with some changes. I reproduce the original email here for those who don follow misc@. / I have been porting a stupid old program to OpenBSD. I hit a bit or a road block because this program uses crypt() but the man page at OpenBSD is not clear enough regarding a couple of details. Specifically: the man page does not provide a clear explanation of the format in which the _setting_ parameter is to be fed to crypt(). In fact, it says: "The second, setting, currently supports a single form. If it begins with a string character (‘$’) and a number then a different algorithm is used depending on the number." So far, so good. What it doesn't say is that if you don't identify the algorithm as either "2a" or "2b" you get an errno and no hash. It also does not specify that you need to provide the log2 of rounds with the _setting_ param, and that such log2 of rounds must be expressed as a two digit number. I am pasting a proposed patch for the crypt(3) man page. Suggestions and ideas are welcome. I am aware the crypt() family is deprecated. --- crypt.3 Thu Jan 21 16:59:05 2021 +++ crypt.new Fri Jan 22 21:33:15 2021 @@ -75,9 +75,13 @@ with a string character .Pq Ql $ and a number then a different algorithm is used depending on the number. -At the moment -.Ql $2 -chooses Blowfish hashing; see below for more information. +Some algorithms use additional bytes to set parameters. +.Pp +At the moment, only Blowfish hashing is supported; see below for more +information. +The algorithm should be +.Ql $2b +unless compatibility with OpenBSD versions earlier than 5.5 is needed. .Ss Blowfish crypt The Blowfish version of crypt has 128 bits of .Fa salt @@ -103,6 +107,18 @@ An encoded .Sq 8 would specify 256 rounds. +The logarithm of the number of rounds must be specified as a two-digit +number; therefore, +.Ql 8 +would be encoded as +.Ql 08 +.Pp +A valid Blowfish +.Fa setting +with no password looks like this: +.Pp +.Dq $2b$12$FPWWO2RJ3CK4FINTw0Hi8O . +.Pp A valid Blowfish password looks like this: .Pp .Dq $2b$12$FPWWO2RJ3CK4FINTw0Hi8OiPKJcX653gzSS.jqltHFMxyDmmQ0Hqq . -- OpenPGP Key Fingerprint: BB5A C2A2 2CAD ACB7 D50D C081 1DB9 6FC4 5AB7 92FA
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 01:52:24PM -0700, Thomas Frohwein wrote:
> On Fri, Jan 22, 2021 at 02:26:31PM -0500, Bryan Steele wrote:
> > On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
>
> [...]
>
> > > > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > > > It's fairly small helper function. Like hid_is_joy_collection()?
> > > >
> > > > I think that could be an XXX workaround until somebody finds a proper,
> > > > generic fix for hid_is_collection() ...
> > >
> > > That's what you meant, yes?
> >
> > Yep, this is what I meant. Hopefully it won't have to stay for long, but
> > t would be nice to get ujoy(4) working as many controllers as possible
> > so that it can go in.
> >
> > Can you send a new full diff thfr@? I still need to test with my USB
> > SNES controller, but tentatively this is still ok brynet@
>
> Below is the updated diff, containing the original ujoy(4) diff and
> Marcus' ujoy.c that includes ujoy_hid_is_collection().
>
> [...]
>
> > > Can this been updated in the overall diff and re-verified if it still
> > > works fine with the other controllers? It (obviously) works with my
> > > PS4 controller.
>
> I built the kernel and device nodes anew with this diff and tested all the
> joystick and gamecontroller devices that I have access to with
> usbhidctl(1) (XBox 360 gamepad, Logitech F310 in D and X mode, Logitech
> Dual Action, Microsoft Sidewinder Precision 2) and all work as expected
> via /dev/ujoy/0.
Thanks.
Can you please also reset the tags of the new files to
/* $OpenBSD$ */
just to be sure that nothing nasty happens with the versions during
the import?
Otherwise this is ok mglocker@ under the condition that you guys take
care about the ports adaption :-)
> Index: etc/MAKEDEV.common
> ===
> RCS file: /cvs/src/etc/MAKEDEV.common,v
> retrieving revision 1.111
> diff -u -p -r1.111 MAKEDEV.common
> --- etc/MAKEDEV.common6 Jul 2020 06:11:26 - 1.111
> +++ etc/MAKEDEV.common22 Jan 2021 20:04:39 -
> @@ -181,6 +181,7 @@ dnl
> target(usb, usb, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> target(usb, uhid, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> twrget(usb, fido, fido)dnl
> +twrget(usb, ujoy, ujoy)dnl
> target(usb, ulpt, 0, 1)dnl
> target(usb, ugen, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> target(usb, ttyU, 0, 1, 2, 3)dnl
> @@ -365,6 +366,10 @@ __devitem(fido, fido, fido/* nodes, fido
> _mkdev(fido, fido, {-RMlist[${#RMlist[*]}]=";mkdir -p fido;rm -f" n=0
> while [ $n -lt 4 ];do M fido/$n c major_fido_c $n 666;n=Add($n, 1);done
> MKlist[${#MKlist[*]}]=";chmod 555 fido"-})dnl
> +__devitem(ujoy, ujoy, ujoy/* nodes, ujoy)dnl
> +_mkdev(ujoy, ujoy, {-RMlist[${#RMlist[*]}]=";mkdir -p ujoy;rm -f" n=0
> + while [ $n -lt 4 ];do M ujoy/$n c major_ujoy_c $n 444;n=Add($n, 1);done
> + MKlist[${#MKlist[*]}]=";chmod 555 ujoy"-})dnl
> __devitem(ulpt, ulpt*, Printer devices)dnl
> _mcdev({-ulpt-}, ulpt*, {-ulpt-}, {-major_ulpt_c-}, 600)dnl
> __devitem(ttyU, ttyU*, USB serial ports,ucom)dnl
> Index: etc/etc.alpha/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.alpha/MAKEDEV.md,v
> retrieving revision 1.76
> diff -u -p -r1.76 MAKEDEV.md
> --- etc/etc.alpha/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.76
> +++ etc/etc.alpha/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -56,6 +56,7 @@ _DEV(uall)
> _DEV(ugen, 48)
> _DEV(uhid, 46)
> _DEV(fido, 70)
> +_DEV(ujoy, 72)
> _DEV(ulpt, 47)
> _DEV(usb, 45)
> _TITLE(spec)
> Index: etc/etc.amd64/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.amd64/MAKEDEV.md,v
> retrieving revision 1.76
> diff -u -p -r1.76 MAKEDEV.md
> --- etc/etc.amd64/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.76
> +++ etc/etc.amd64/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -60,6 +60,7 @@ _DEV(uall)
> _DEV(ugen, 63)
> _DEV(uhid, 62)
> _DEV(fido, 98)
> +_DEV(ujoy, 100)
> _DEV(ulpt, 64)
> _DEV(usb, 61)
> _TITLE(spec)
> Index: etc/etc.arm64/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.arm64/MAKEDEV.md,v
> retrieving revision 1.10
> diff -u -p -r1.10 MAKEDEV.md
> --- etc/etc.arm64/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.10
> +++ etc/etc.arm64/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -52,6 +52,7 @@ _DEV(uall)
> _DEV(ugen, 63)
> _DEV(uhid, 62)
> _DEV(fido, 98)
> +_DEV(ujoy, 100)
> _DEV(ulpt, 64)
> _DEV(usb, 61)
> _TITLE(spec)
> Index: etc/etc.armv7/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.armv7/MAKEDEV.md,v
> retrieving revision 1.18
> diff -u -p -r1.18 MAKEDEV.md
> --- etc/etc.armv7/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.18
> +++
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 02:26:31PM -0500, Bryan Steele wrote:
> On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
[...]
> > > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > > It's fairly small helper function. Like hid_is_joy_collection()?
> > >
> > > I think that could be an XXX workaround until somebody finds a proper,
> > > generic fix for hid_is_collection() ...
> >
> > That's what you meant, yes?
>
> Yep, this is what I meant. Hopefully it won't have to stay for long, but
> t would be nice to get ujoy(4) working as many controllers as possible
> so that it can go in.
>
> Can you send a new full diff thfr@? I still need to test with my USB
> SNES controller, but tentatively this is still ok brynet@
Below is the updated diff, containing the original ujoy(4) diff and
Marcus' ujoy.c that includes ujoy_hid_is_collection().
[...]
> > Can this been updated in the overall diff and re-verified if it still
> > works fine with the other controllers? It (obviously) works with my
> > PS4 controller.
I built the kernel and device nodes anew with this diff and tested all the
joystick and gamecontroller devices that I have access to with
usbhidctl(1) (XBox 360 gamepad, Logitech F310 in D and X mode, Logitech
Dual Action, Microsoft Sidewinder Precision 2) and all work as expected
via /dev/ujoy/0.
Index: etc/MAKEDEV.common
===
RCS file: /cvs/src/etc/MAKEDEV.common,v
retrieving revision 1.111
diff -u -p -r1.111 MAKEDEV.common
--- etc/MAKEDEV.common 6 Jul 2020 06:11:26 - 1.111
+++ etc/MAKEDEV.common 22 Jan 2021 20:04:39 -
@@ -181,6 +181,7 @@ dnl
target(usb, usb, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, uhid, 0, 1, 2, 3, 4, 5, 6, 7)dnl
twrget(usb, fido, fido)dnl
+twrget(usb, ujoy, ujoy)dnl
target(usb, ulpt, 0, 1)dnl
target(usb, ugen, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, ttyU, 0, 1, 2, 3)dnl
@@ -365,6 +366,10 @@ __devitem(fido, fido, fido/* nodes, fido
_mkdev(fido, fido, {-RMlist[${#RMlist[*]}]=";mkdir -p fido;rm -f" n=0
while [ $n -lt 4 ];do M fido/$n c major_fido_c $n 666;n=Add($n, 1);done
MKlist[${#MKlist[*]}]=";chmod 555 fido"-})dnl
+__devitem(ujoy, ujoy, ujoy/* nodes, ujoy)dnl
+_mkdev(ujoy, ujoy, {-RMlist[${#RMlist[*]}]=";mkdir -p ujoy;rm -f" n=0
+ while [ $n -lt 4 ];do M ujoy/$n c major_ujoy_c $n 444;n=Add($n, 1);done
+ MKlist[${#MKlist[*]}]=";chmod 555 ujoy"-})dnl
__devitem(ulpt, ulpt*, Printer devices)dnl
_mcdev({-ulpt-}, ulpt*, {-ulpt-}, {-major_ulpt_c-}, 600)dnl
__devitem(ttyU, ttyU*, USB serial ports,ucom)dnl
Index: etc/etc.alpha/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.alpha/MAKEDEV.md,v
retrieving revision 1.76
diff -u -p -r1.76 MAKEDEV.md
--- etc/etc.alpha/MAKEDEV.md6 Jul 2020 06:11:26 - 1.76
+++ etc/etc.alpha/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -56,6 +56,7 @@ _DEV(uall)
_DEV(ugen, 48)
_DEV(uhid, 46)
_DEV(fido, 70)
+_DEV(ujoy, 72)
_DEV(ulpt, 47)
_DEV(usb, 45)
_TITLE(spec)
Index: etc/etc.amd64/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.amd64/MAKEDEV.md,v
retrieving revision 1.76
diff -u -p -r1.76 MAKEDEV.md
--- etc/etc.amd64/MAKEDEV.md6 Jul 2020 06:11:26 - 1.76
+++ etc/etc.amd64/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -60,6 +60,7 @@ _DEV(uall)
_DEV(ugen, 63)
_DEV(uhid, 62)
_DEV(fido, 98)
+_DEV(ujoy, 100)
_DEV(ulpt, 64)
_DEV(usb, 61)
_TITLE(spec)
Index: etc/etc.arm64/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.arm64/MAKEDEV.md,v
retrieving revision 1.10
diff -u -p -r1.10 MAKEDEV.md
--- etc/etc.arm64/MAKEDEV.md6 Jul 2020 06:11:26 - 1.10
+++ etc/etc.arm64/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -52,6 +52,7 @@ _DEV(uall)
_DEV(ugen, 63)
_DEV(uhid, 62)
_DEV(fido, 98)
+_DEV(ujoy, 100)
_DEV(ulpt, 64)
_DEV(usb, 61)
_TITLE(spec)
Index: etc/etc.armv7/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.armv7/MAKEDEV.md,v
retrieving revision 1.18
diff -u -p -r1.18 MAKEDEV.md
--- etc/etc.armv7/MAKEDEV.md6 Jul 2020 06:11:26 - 1.18
+++ etc/etc.armv7/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -61,6 +61,7 @@ _DEV(uall)
_DEV(ugen, 70)
_DEV(uhid, 65)
_DEV(fido, 106)
+_DEV(ujoy, 108)
_DEV(ulpt, 66)
_DEV(usb, 64)
_TITLE(spec)
Index: etc/etc.hppa/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.hppa/MAKEDEV.md,v
retrieving revision 1.65
diff -u -p -r1.65 MAKEDEV.md
--- etc/etc.hppa/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.65
+++ etc/etc.hppa/MAKEDEV.md 22 Jan 2021 20:04:39 -
@@ -54,6 +54,7 @@ _DEV(uall)
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
> On Fri, 15 Jan 2021 22:41:13 +0100
> Marcus Glocker wrote:
>
> > On Fri, 15 Jan 2021 11:37:47 -0500
> > Bryan Steele wrote:
> >
> > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > > >
> > > > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > > > >
> > > > > > > I have heard from others who tried the diff that the PS4
> > > > > > > controller is causing problems with the way it attaches. I
> > > > > > > ordered one to trial-and- error this myself at home. Could
> > > > > > > you share output of lsusb -vv? Thanks for giving it a try!
> > > > > > >
> > > > > >
> > > > > > Sure, here we go.
> > > > > > If I can find anything myself in the meantime I let you know.
> > > > > >
> > > > >
> > > > > So the problem doesn't seem to be in your new ujoy(4) code, but
> > > > > how the dev/hid/hid.c:hid_is_collection() function tries to cope
> > > > > with the PS4 controller.
> > > >
> > > > So with the hid_is_collection() problem not easy to mitigate [1],
> > > > should we table the ujoy(4) proposal for now pending a fix for the
> > > > problems with the PS4 controller? Or is this interesting enough
> > > > for some to work on moving forward despite this issue and finding
> > > > a solution for this specific (and in some ways unusual) device
> > > > later?
> > >
> > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > It's fairly small helper function. Like hid_is_joy_collection()?
> >
> > I think that could be an XXX workaround until somebody finds a proper,
> > generic fix for hid_is_collection() ...
>
> That's what you meant, yes?
Yep, this is what I meant. Hopefully it won't have to stay for long, but
t would be nice to get ujoy(4) working as many controllers as possible
so that it can go in.
Can you send a new full diff thfr@? I still need to test with my USB
SNES controller, but tentatively this is still ok brynet@
-Bryan.
> Can this been updated in the overall diff and re-verified if it still
> works fine with the other controllers? It (obviously) works with my
> PS4 controller.
>
>
> Index: sys/dev/usb/ujoy.c
> ===
> RCS file: sys/dev/usb/ujoy.c
> diff -N sys/dev/usb/ujoy.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sys/dev/usb/ujoy.c22 Jan 2021 17:54:46 -
> @@ -0,0 +1,149 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2020 Thomas Frohwein
> + * Copyright (c) 2020 Bryan Steele
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +int ujoy_match(struct device *, void *, void *);
> +
> +struct cfdriver ujoy_cd = {
> + NULL, "ujoy", DV_DULL
> +};
> +
> +const struct cfattach ujoy_ca = {
> + sizeof(struct uhid_softc),
> + ujoy_match,
> + uhid_attach,
> + uhid_detach,
> +};
> +
> +/*
> + * XXX workaround:
> + *
> + * This is a copy of sys/dev/hid/hid.c:hid_is_collection(), synced up to the
> + * NetBSD version. Our current hid_is_collection() is not playing nice with
> + * all HID devices like the PS4 controller. But applying this version
> + * globally breaks other HID devices like ims(4) and imt(4). Until our
> global
> + * hid_is_collection() can't be fixed to play nice with all HID devices, we
> + * go for this dedicated ujoy(4) version.
> + */
> +int
> +ujoy_hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> + uint32_t coll_usage = ~0;
> +
> + hd = hid_start_parse(desc, size, hid_input);
> + if (hd == NULL)
> + return (0);
> +
> + while (hid_get_item(hd, )) {
> + if (hi.kind == hid_collection &&
Re: [PATCH v2] tee: Add -q, --quiet, --silent option to not write to stdout
I added a few FreeBSD CCs that I found on their tee.c's git blame,
because the freebsd list rejects external mail.
Please have a look at the proposal below, and the discussion that
started on gnu coreutil's list.
On 1/22/21 12:12 AM, Alejandro Colomar wrote:
This is useful for using tee to just write to a file,
at the end of a pipeline,
without having to redirect to /dev/null.
Example:
echo 'foo' | sudo tee -q /etc/foo;
is equivalent to the old (and ugly)
echo 'foo' | sudo tee /etc/foo >/dev/null;
Tools with a similar interface: grep
Signed-off-by: Alejandro Colomar
---
v2: Add --silent synonym to --quiet, per GNU guidelines.
I tested tee --silent with success.
src/tee.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/tee.c b/src/tee.c
index c81faea91..68ace983a 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -45,6 +45,9 @@ static bool append;
/* If true, ignore interrupts. */
static bool ignore_interrupts;
+/* Don't write to stdout */
+static bool quiet;
+
enum output_error
{
output_error_sigpipe, /* traditional behavior, sigpipe enabled. */
@@ -61,6 +64,8 @@ static struct option const long_options[] =
{"append", no_argument, NULL, 'a'},
{"ignore-interrupts", no_argument, NULL, 'i'},
{"output-error", optional_argument, NULL, 'p'},
+ {"quiet", no_argument, NULL, 'q'},
+ {"silent", no_argument, NULL, 'q'},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{NULL, 0, NULL, 0}
@@ -93,6 +98,7 @@ Copy standard input to each FILE, and also to standard
output.\n\
"), stdout);
fputs (_("\
-pdiagnose errors writing to non pipes\n\
+ -q, --quiet, --silent don't write to standard output\n\
--output-error[=MODE] set behavior on write error. See MODE below\n\
"), stdout);
fputs (HELP_OPTION_DESCRIPTION, stdout);
@@ -130,6 +136,7 @@ main (int argc, char **argv)
append = false;
ignore_interrupts = false;
+ quiet = false;
while ((optc = getopt_long (argc, argv, "aip", long_options, NULL)) != -1)
{
@@ -151,6 +158,10 @@ main (int argc, char **argv)
output_error = output_error_warn_nopipe;
break;
+case 'q':
+ quiet = true;
+ break;
+
case_GETOPT_HELP_CHAR;
case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
@@ -235,8 +246,9 @@ tee_files (int nfiles, char **files)
break;
/* Write to all NFILES + 1 descriptors.
- Standard output is the first one. */
- for (i = 0; i <= nfiles; i++)
+ Standard output is the first one.
+ If 'quiet' is true, write to descriptors 1 and above (omit stdout) */
+ for (i = quiet; i <= nfiles; i++)
if (descriptors[i]
&& fwrite (buffer, bytes_read, 1, descriptors[i]) != 1)
{
--
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Re: New ujoy(4) device for USB gamecontrollers
On Fri, 15 Jan 2021 22:41:13 +0100
Marcus Glocker wrote:
> On Fri, 15 Jan 2021 11:37:47 -0500
> Bryan Steele wrote:
>
> > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > >
> > > > On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > > >
> > > > > > I have heard from others who tried the diff that the PS4
> > > > > > controller is causing problems with the way it attaches. I
> > > > > > ordered one to trial-and- error this myself at home. Could
> > > > > > you share output of lsusb -vv? Thanks for giving it a try!
> > > > > >
> > > > >
> > > > > Sure, here we go.
> > > > > If I can find anything myself in the meantime I let you know.
> > > > >
> > > >
> > > > So the problem doesn't seem to be in your new ujoy(4) code, but
> > > > how the dev/hid/hid.c:hid_is_collection() function tries to cope
> > > > with the PS4 controller.
> > >
> > > So with the hid_is_collection() problem not easy to mitigate [1],
> > > should we table the ujoy(4) proposal for now pending a fix for the
> > > problems with the PS4 controller? Or is this interesting enough
> > > for some to work on moving forward despite this issue and finding
> > > a solution for this specific (and in some ways unusual) device
> > > later?
> >
> > Considering the hid_is_collection() fix from NetBSD proposed by
> > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > It's fairly small helper function. Like hid_is_joy_collection()?
>
> I think that could be an XXX workaround until somebody finds a proper,
> generic fix for hid_is_collection() ...
That's what you meant, yes?
Can this been updated in the overall diff and re-verified if it still
works fine with the other controllers? It (obviously) works with my
PS4 controller.
Index: sys/dev/usb/ujoy.c
===
RCS file: sys/dev/usb/ujoy.c
diff -N sys/dev/usb/ujoy.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ sys/dev/usb/ujoy.c 22 Jan 2021 17:54:46 -
@@ -0,0 +1,149 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2020 Thomas Frohwein
+ * Copyright (c) 2020 Bryan Steele
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+
+int ujoy_match(struct device *, void *, void *);
+
+struct cfdriver ujoy_cd = {
+ NULL, "ujoy", DV_DULL
+};
+
+const struct cfattach ujoy_ca = {
+ sizeof(struct uhid_softc),
+ ujoy_match,
+ uhid_attach,
+ uhid_detach,
+};
+
+/*
+ * XXX workaround:
+ *
+ * This is a copy of sys/dev/hid/hid.c:hid_is_collection(), synced up to the
+ * NetBSD version. Our current hid_is_collection() is not playing nice with
+ * all HID devices like the PS4 controller. But applying this version
+ * globally breaks other HID devices like ims(4) and imt(4). Until our global
+ * hid_is_collection() can't be fixed to play nice with all HID devices, we
+ * go for this dedicated ujoy(4) version.
+ */
+int
+ujoy_hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
+{
+ struct hid_data *hd;
+ struct hid_item hi;
+ uint32_t coll_usage = ~0;
+
+ hd = hid_start_parse(desc, size, hid_input);
+ if (hd == NULL)
+ return (0);
+
+ while (hid_get_item(hd, )) {
+ if (hi.kind == hid_collection &&
+ hi.collection == HCOLL_APPLICATION)
+ coll_usage = hi.usage;
+
+ if (hi.kind == hid_endcollection)
+ coll_usage = ~0;
+
+ if (hi.kind == hid_input &&
+ coll_usage == usage &&
+ hi.report_ID == id) {
+ hid_end_parse(hd);
+ return (1);
+ }
+ }
+ hid_end_parse(hd);
+
+ return (0);
+}
+
+int
+ujoy_match(struct device *parent, void *match, void *aux)
+{
+ struct uhidev_attach_arg *uha = (struct
uhidpp(4): logitech hid++ device driver
Hi, Here's a new driver for Logitech HID++ devices, currently limited to exposing battery sensors. Here's an example using a Logitech M330 mouse: $ dmesg | grep uhidpp uhidpp0 at uhidev1 device 1 mouse "B330/M330/M3" serial c7-2f-a8-33 $ sysctl hw.sensors.uhidpp0 hw.sensors.uhidpp0.raw0=2 (battery levels) hw.sensors.uhidpp0.percent0=70.00% (battery level), OK The raw0 sensor reflects number of available battery levels, the resolution on this device is not great... Most of the code is derived from the hid-logitech-hidpp Linux driver. Some assorted notes: * In order to communicate with the device inside the attach routine, I had to wire up the interrupt handler as this by default is done first once the same attach routine has returned. Hence the introduction of uhidev_set_intr(). If this is an acceptable approach, this can go in as a separate commit. * I kept using the `return -errno' convention from the Linux driver in order to distingush errors from the hardware, which are always positive. I you happen to have a Logitech HID++ device and run into any problem(s), please enable UHIDPP_DEBUG and send me the output. Comments? OK? diff --git share/man/man4/Makefile share/man/man4/Makefile index 02af7a47a44..74a4e17d7dc 100644 --- share/man/man4/Makefile +++ share/man/man4/Makefile @@ -83,8 +83,8 @@ MAN= aac.4 abcrtc.4 abl.4 ac97.4 acphy.4 acrtc.4 \ txp.4 txphy.4 uaudio.4 uark.4 uath.4 ubcmtp.4 uberry.4 ubsa.4 \ ubsec.4 ucom.4 uchcom.4 ucrcom.4 ucycom.4 ukspan.4 uslhcom.4 \ udav.4 udcf.4 udl.4 udp.4 udsbr.4 \ - uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uipaq.4 \ - uk.4 ukbd.4 \ + uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uhidpp.4 \ + uipaq.4 uk.4 ukbd.4 \ ukphy.4 ulpt.4 umass.4 umb.4 umbg.4 umcs.4 umct.4 umidi.4 umodem.4 \ ums.4 umsm.4 umstc.4 umt.4 unix.4 uonerng.4 uow.4 uoaklux.4 uoakrh.4 \ uoakv.4 upd.4 upgt.4 upl.4 uplcom.4 ural.4 ure.4 url.4 urlphy.4 \ diff --git share/man/man4/uhidev.4 share/man/man4/uhidev.4 index f0a6776a27b..d264935a65c 100644 --- share/man/man4/uhidev.4 +++ share/man/man4/uhidev.4 @@ -40,6 +40,7 @@ .Cd "ucycom* at uhidev?" .Cd "ugold* at uhidev?" .Cd "uhid*at uhidev?" +.Cd "uhidpp* at uhidev?" .Cd "ukbd*at uhidev?" .Cd "ums* at uhidev?" .Cd "umstc* at uhidev?" @@ -73,6 +74,7 @@ only dispatches data to them based on the report id. .Xr ucycom 4 , .Xr ugold 4 , .Xr uhid 4 , +.Xr uhidpp 4 , .Xr ukbd 4 , .Xr ums 4 , .Xr umstc 4 , diff --git share/man/man4/uhidpp.4 share/man/man4/uhidpp.4 new file mode 100644 index 000..4c78380c35b --- /dev/null +++ share/man/man4/uhidpp.4 @@ -0,0 +1,48 @@ +.\"$OpenBSD$ +.\" +.\" Copyright (c) 2021 Anton Lindqvsit +.\" +.\" Permission to use, copy, modify, and distribute this software for any +.\" purpose with or without fee is hereby granted, provided that the above +.\" copyright notice and this permission notice appear in all copies. +.\" +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +.\" +.Dd $Mdocdate$ +.Dt UHIDPP 4 +.Os +.Sh NAME +.Nm uhidpp +.Nd Logitech HID++ devices +.Sh SYNOPSIS +.Cd "uhidpp* at uhidev?" +.Sh DESCRIPTION +The +.Nm +driver provides support for Logitech HID++ devices. +It exposes a collection of battery sensor values which are made available +through the +.Xr sysctl 8 +interface. +.Sh SEE ALSO +.Xr intro 4 , +.Xr uhidev 4 , +.Xr usb 4 , +.Xr sensorsd 8 , +.Xr sysctl 8 +.Sh HISTORY +The +.Nm +driver first appeared in +.Ox 6.9 . +.Sh AUTHORS +The +.Nm +driver was written by +.An Anton Lindqvist Aq Mt an...@opensd.org . diff --git share/man/man4/usb.4 share/man/man4/usb.4 index 520f46265e0..b58190539e3 100644 --- share/man/man4/usb.4 +++ share/man/man4/usb.4 @@ -255,6 +255,8 @@ TEMPer gold HID thermometer and hygrometer Generic driver for Human Interface Devices .It Xr uhidev 4 Base driver for all Human Interface Devices +.It Xr uhidpp 4 +Logitech HID++ devices .It Xr ukbd 4 USB keyboards that follow the boot protocol .It Xr ums 4 diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC index 45b3a9b6e66..00ac52adcd6 100644 --- sys/arch/amd64/conf/GENERIC +++ sys/arch/amd64/conf/GENERIC @@ -288,6 +288,7 @@ uhid* at uhidev? # USB generic HID support fido* at uhidev? # FIDO/U2F security key support upd* at uhidev? # USB Power Devices sensors umstc* at uhidev? # Microsoft Surface Type
Re: grep: add --null flag
quasi three-weekly ping.
Is this such a bad idea?
(TBH: I have still to look at how to write a regression test for this)
Omar Polo writes:
> Hello,
>
> There are various program, especially the one written with GNU grep in
> mind, that expects various flags that grep in base doesn't have. While
> some of the flags (like --color) can be easily worked out (i.e. by
> patching/customising these programs) one thing that it isn't easily
> workable is --null, because it alters the way grep outputs its data.
>
> --null makes grep output an ASCII NUL byte after the file name, so a
> program can parse the output of grep unambiguously even when the file
> names contains "funny" characters, such as a newline.
>
> GNU grep isn't the only one with a --null flag, also FreeBSD and NetBSD
> grep do (at least by looking at their manpages), so it's somewhat
> widespread.
>
> I searched the archives on marc.info but I haven't seen a previous
> discussion about this.
>
> The following patch was tried against GNU grep (installed from packages)
> and seem to behave consistently.
>
> I used the same text of the FreeBSD/NetBSD manpage for the description
> of the --null option, but I really dislike it: it feels way to verbose
> for what it's trying to say, but I wasn't able to come up with something
> better.
>
> I'm not familiar at all with the grep codebase, so I hope I'm not
> missing something. If this has some chances of being accepted, I guess
> I should also add some regress test; I'll try to work on them soon, but
> in the meantime I'm sending this.
>
> Thanks,
>
> Omar Polo
diff e992327fc31d0277a6f8518613a7db1b9face78b /home/op/w/openbsd-src
blob - 5cc228df222c54a0553f289b5da8bbbe6afd171e
file + usr.bin/grep/grep.1
--- usr.bin/grep/grep.1
+++ usr.bin/grep/grep.1
@@ -49,6 +49,7 @@
.Op Fl -context Ns Op = Ns Ar num
.Op Fl -label Ns = Ns Ar name
.Op Fl -line-buffered
+.Op Fl -null
.Op Ar pattern
.Op Ar
.Ek
@@ -297,6 +298,25 @@ instead of the filename before lines.
Force output to be line buffered.
By default, output is line buffered when standard output is a terminal
and block buffered otherwise.
+.It Fl -null
+Output a zero byte (the ASCII NUL character) instead of the character
+that normally follows a file name.
+For example,
+.Nm Fl l Fl -null
+outputs a zero byte after each file name instead of the usual newline.
+This option makes the output unambiguous, even in the presence of file
+names containing unusual characters like newlines.
+This option can be used with commands like
+.Xr find 1
+.Fl print0 Ns ,
+.Xr perl 1
+.Fl 0 Ns ,
+.Xr sort 1
+.Fl z Ns , and
+.Xr args 1
+.Fl 0
+to process arbitrary file names, even those that contain newline
+characters.
.El
.Sh EXIT STATUS
The
blob - f41b5e20ca68c9e9a36d2f7dd3c44329c621f29b
file + usr.bin/grep/grep.c
--- usr.bin/grep/grep.c
+++ usr.bin/grep/grep.c
@@ -80,6 +80,7 @@ intvflag; /* -v: only show non-matching lines */
int wflag; /* -w: pattern must start and end on word boundaries */
int xflag; /* -x: pattern must match entire line */
int lbflag;/* --line-buffered */
+int nullflag; /* --null */
const char *labelname; /* --label=name */
int binbehave = BIN_FILE_BIN;
@@ -89,6 +90,7 @@ enum {
HELP_OPT,
MMAP_OPT,
LINEBUF_OPT,
+ NULL_OPT,
LABEL_OPT,
};
@@ -134,6 +136,7 @@ static const struct option long_options[] =
{"mmap",no_argument,NULL, MMAP_OPT},
{"label", required_argument, NULL, LABEL_OPT},
{"line-buffered", no_argument,NULL, LINEBUF_OPT},
+ {"null",no_argument,NULL, NULL_OPT},
{"after-context", required_argument, NULL, 'A'},
{"before-context", required_argument, NULL, 'B'},
{"context", optional_argument, NULL, 'C'},
@@ -436,6 +439,9 @@ main(int argc, char *argv[])
case LINEBUF_OPT:
lbflag = 1;
break;
+ case NULL_OPT:
+ nullflag = 1;
+ break;
case HELP_OPT:
default:
usage();
blob - b3d24ae662beb72c5632190c5c819bcc92f0389a
file + usr.bin/grep/grep.h
--- usr.bin/grep/grep.h
+++ usr.bin/grep/grep.h
@@ -68,7 +68,7 @@ extern int cflags, eflags;
extern int Aflag, Bflag, Eflag, Fflag, Hflag, Lflag,
Rflag, Zflag,
bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
-sflag, vflag, wflag, xflag;
+sflag, vflag, wflag, xflag, nullflag;
extern int binbehave;
extern const char *labelname;
blob - e16d08e7d859609c2ccbc0ac4bba670188f81abf
file + usr.bin/grep/util.c
--- usr.bin/grep/util.c
+++ usr.bin/grep/util.c
@@ -172,13 +172,13 @@ procfile(char *fn)
if (cflag) {
if (!hflag)
-
Re: bgpd fix route decision for strict med
On Thu, Jan 14, 2021 at 12:27:54PM +0100, Claudio Jeker wrote:
> Currently bgpd does not properly handle strict med route decisions.
> The problem is that the strict MED check only matters for aspaths with the
> same neighbor as. The route decision process currently stops as soon as
> the current prefix is better then the one checked in the list of prefixes.
> Now in some cases this results in unstable decisions because the order of
> insertions matter. Depending on the order any route may be selected.
> The med.sh regress test I added shows this issue. Depending on the order
> any of the 3 routes can be selected as best:
>
> 1:
> flags ovs destination gateway lpref med aspath origin
> *> N 10.12.1.0/24 10.12.57.410050 64501 64510 i
> * N 10.12.1.0/24 10.12.57.2100 100 64501 64510 i
> * N 10.12.1.0/24 10.12.57.3100 100 64502 64510 i
>
> 2:
> flags ovs destination gateway lpref med aspath origin
> *> N 10.12.1.0/24 10.12.57.2100 100 64501 64510 i
> * N 10.12.1.0/24 10.12.57.3100 100 64502 64510 i
> * N 10.12.1.0/24 10.12.57.410050 64501 64510 i
>
> 3 (and the actual expected result):
> flags ovs destination gateway lpref med aspath origin
> *> N 10.12.1.0/24 10.12.57.3100 100 64502 64510 i
> * N 10.12.1.0/24 10.12.57.410050 64501 64510 i
> * N 10.12.1.0/24 10.12.57.2100 100 64501 64510 i
>
> Additionally removing a route requires to reevaluate part of the routes in
> some cases. For examle in case 3 if the middle route (with med 50) is
> removed then the last route actually becomes best (bgpid is lower).
>
> The following diff fixes this issue hopefully. On insertion if decisions
> happen at or after the MED check (step 5) then all remaining routes need
> to be checked (until a check before step 5 happens). Routes matching on
> med that need to be re-evaluated are put on a redo queue and at the end of
> the decision process are put back to get their order right.
>
> Something similar happens when removing a prefix. If the next prefix
> differ on a check after the MED check then again all those prefixes need
> to be rechecked and maybe re-evaluated.
>
> This change is important but also rather complex. Please test and if
> possible validate that it does not cause troubles in your setup.
> Btw. this only matters for 'rde med compare strict' (default).
I would really like some feedback on this. This is a major issue in bgpd
that finally needs to be fixed. Please help by testing this out.
--
:wq Claudio
Index: rde_decide.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v
retrieving revision 1.80
diff -u -p -r1.80 rde_decide.c
--- rde_decide.c14 Jan 2021 08:29:26 - 1.80
+++ rde_decide.c14 Jan 2021 10:16:30 -
@@ -26,7 +26,9 @@
#include "rde.h"
#include "log.h"
-intprefix_cmp(struct prefix *, struct prefix *);
+intprefix_cmp(struct prefix *, struct prefix *, int *);
+void prefix_insert(struct prefix *, struct prefix *, struct rib_entry *);
+void prefix_remove(struct prefix *, struct rib_entry *);
/*
* Decision Engine RFC implementation:
* Phase 1:
@@ -107,7 +109,7 @@ int prefix_cmp(struct prefix *, struct p
* already added prefix.
*/
int
-prefix_cmp(struct prefix *p1, struct prefix *p2)
+prefix_cmp(struct prefix *p1, struct prefix *p2, int *testall)
{
struct rde_aspath *asp1, *asp2;
struct rde_peer *peer1, *peer2;
@@ -115,6 +117,16 @@ prefix_cmp(struct prefix *p1, struct pre
u_int32_tp1id, p2id;
int p1cnt, p2cnt, i;
+ /*
+* If a match happens before the MED check then the list is
+* correctly sorted. If a match happens after MED then it
+* may further elements need to be checked to make sure that
+* all path are considered that could affect this path.
+* If the check happens to be on MED signal this by setting
+* testall to 2.
+*/
+ *testall = 0;
+
if (p1 == NULL)
return -1;
if (p2 == NULL)
@@ -166,10 +178,14 @@ prefix_cmp(struct prefix *p1, struct pre
/*
* 5. MED decision
* Only comparable between the same neighboring AS or if
-* 'rde med compare always' is set.
+* 'rde med compare always' is set. In the first case
+* set the testall flag since further elements need to be
+* evaluated as well.
*/
if ((rde_decisionflags() & BGPD_FLAG_DECISION_MED_ALWAYS) ||
aspath_neighbor(asp1->aspath) == aspath_neighbor(asp2->aspath)) {
+ if (!(rde_decisionflags() & BGPD_FLAG_DECISION_MED_ALWAYS))
+ *testall = 2;
Re: pf route-to issues
On Fri, Jan 08, 2021 at 04:43:39PM +0100, Alexandr Nedvedicky wrote: > Hello, > > > > > > revision 1.294 > > date: 2003/01/02 01:56:56; author: dhartmei; state: Exp; lines: +27 -49; > > When route-to/reply-to is used in combination with address translation, > > pf_test() may be called twice for the same packet. In this case, make > > sure the translation is only applied in the second call. This solves > > the problem with state insert failures where the second pf_test() call > > tried to insert another state entry after the first call's translation. > > ok henning@, mcbride@, thanks to Joe Nall for additional testing. > > > > > > I have tested your diffs in my setup, they all pass. I have not > > tested the scenario mentioned in the commit message. Note that the > > address translation implementation in 2003 was different from what > > we have now. And sasha@'s analysis shows that the current code is > > wrong in other use cases. > > > > I've completely forgot there was a change in NAT. Therefore I could > not understand the commit message. > > > > > > > The only way to find out is to commit it. It reduces comlexity that > > noone understands. > > > > OK bluhm@ to remove the check > > > > Please leave the "if (pd->kif->pfik_ifp != ifp)" around pf_test() > > in pf_route() as it is for now. > > I agree with bluhm@ here. we should proceed with small steps in such > case and let things to settle down before making next move. ok. i don't know how to split up the rest of the change though. here's an updated diff that includes the rest of the kernel changes and the pfctl and pf.conf tweaks. it's probably useful for me to try and explain at a high level what i think the semantics should be, otherwise we might end up arguing about which bits of the current config i broke. so, from an extremely high level point of view, and apologies if this is condescending, pf sits between the network stack and an interface that a packet travels on. for connections handled by the local box, this means packets come from the stack and get an output interface selected by a route lookup, then pf checks it, and then it goes out the selected interface. replies come into an interface, get checked by pf, and then enter the stack. when forwarding, a packet comes into an interface, pf checks it, the stack does a route lookup to pick an interface, pf checks it again, and then it goes out the interface. so what does it mean when route-to (or reply-to) gets involved? i'm saying that when route-to is applied to a packet, pf takes the packet away from the stack and immediately forwards it toward to specified destination address. for a packet entering the system, ie, when the packet is going from the interface into the stack, route-to should pretend that it is forwarding the packet and basically push it straight out an interface. however, like normal forwarding via the stack, there might be some policy on packets leaving that interface that you want to apply, so pf should run pf_test in that situation so the policy can be applied. this is especially useful if you need to apply nat-to when packets leave a particular interface. however, if you route-to when a packet is on the way out of the stack, i'm arguing that pf should not run again against that packet. currently route-to rules run pf_test again if the interface the packet is routed out of changes, which means pf runs multiple times against a packet if rules keep changing which interface it goes out. this means there's loop prevention in pf to mitigate against this, and weird potentials for multiple states to be created when nat gets involved. for simplicity, both in terms of reasoning and code i think pf should only be run once when a packet enters the system, and only once when it leaves the system. the only reason i can come up with for running pf_test multiple times when route-to changes the outgoing interface is so you can check the packet with "pass out on $new_if" type rules. we don't rerun pf again when nat/rdr changes addresses, so this feels inconsistent to me. i also don't think route-to is used much. getting basic functionality working is surprisingly hard, so the complicated possibilities in the current code are almost certainly not taken advantage of. we're going to break existing configurations anyway, so if we can agree that pf only runs twice even if route-to gets involved, then i'm not going to feel bad about breaking something this complicated anyway. this also breaks the ability to do route-to without states. is there a reason to do that apart from the DSR type things? did we agree that those use cases could be handled by sloppy states instead? lastly, the "argument" or address specified with route-to (and reply-to and dup-to) is a destination address, not a next-hop. this has been discussed on the lists a couple of times before, so i won't go over it again, except to
