Re: tftp: sort commands, add help to manual
On Sun, Oct 02, 2022 at 06:05:44PM +, Klemens Nanni wrote:
> Searching for a command in help output is much simpler when sorted.
> The strings can be inlined into the struct while staying under 80 chars.
>
> Now manual and help output are in the same order, except help folded
> into ? in the manual to avoid duplicate text there.
>
> Or should help appear at the same spot to make manual and help output
> an exact match order-wise?
>
> Feedback? OK?
>
- i'm fine with ordering the output
- as to the order of ? and help, isn;t ? already in the correct place
being first? so then adding "help" wouldn;t break that?
- i bet most people who use "help" do not single out specific commands,
especially since the output to screen is minimal. so it would be
helpful to show "command " as optional:
? | help [command ...]
- i think "command" would be better than "command-name": shorter and
saner.
jmc
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/tftp/main.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 main.c
> --- main.c20 Sep 2018 11:42:42 - 1.43
> +++ main.c1 Oct 2022 19:18:12 -
> @@ -109,23 +109,6 @@ int opt_tsize = 0;
> int opt_tout = 0;
> int opt_blksize = 0;
>
> -char vhelp[] = "toggle verbose mode";
> -char thelp[] = "toggle packet tracing";
> -char chelp[] = "connect to remote tftp";
> -char qhelp[] = "exit tftp";
> -char hhelp[] = "print help information";
> -char shelp[] = "send file";
> -char rhelp[] = "receive file";
> -char mhelp[] = "set file transfer mode";
> -char sthelp[] = "show current status";
> -char xhelp[] = "set per-packet retransmission timeout";
> -char ihelp[] = "set total retransmission timeout";
> -char ashelp[] = "set mode to netascii";
> -char bnhelp[] = "set mode to octet";
> -char oshelp[] = "toggle tsize option";
> -char othelp[] = "toggle timeout option";
> -char obhelp[] = "set alternative blksize option";
> -
> struct cmd {
> char*name;
> char*help;
> @@ -133,23 +116,23 @@ struct cmd {
> };
>
> struct cmd cmdtab[] = {
> - { "connect",chelp, parsearg },
> - { "mode", mhelp, modecmd },
> - { "put",shelp, put },
> - { "get",rhelp, get },
> - { "quit", qhelp, quit },
> - { "verbose",vhelp, setverbose },
> - { "trace", thelp, settrace },
> - { "status", sthelp, status },
> - { "binary", bnhelp, setbinary },
> - { "ascii", ashelp, setascii },
> - { "rexmt", xhelp, setrexmt },
> - { "timeout",ihelp, settimeout },
> - { "tsize", oshelp, settsize },
> - { "tout", othelp, settout },
> - { "blksize",obhelp, setblksize },
> - { "help", hhelp, help },
> - { "?", hhelp, help },
> + { "?", "print help information", help },
> + { "ascii", "set mode to netascii", setascii },
> + { "binary", "set mode to octet",setbinary },
> + { "blksize","set alternative blksize option", setblksize },
> + { "connect","connect to remote tftp", parsearg },
> + { "get","receive file", get },
> + { "help", "print help information", help },
> + { "mode", "set file transfer mode", modecmd },
> + { "put","send file",put },
> + { "quit", "exit tftp",quit },
> + { "rexmt", "set per-packet retransmission timeout", setrexmt },
> + { "status", "show current status", status },
> + { "timeout","set total retransmission timeout", settimeout },
> + { "tout", "toggle timeout option",settout },
> + { "trace", "toggle packet tracing",settrace },
> + { "tsize", "toggle tsize option", settsize },
> + { "verbose","toggle verbose mode", setverbose },
> { NULL, NULL, NULL }
> };
>
> Index: tftp.1
> ===
> RCS file: /cvs/src/usr.bin/tftp/tftp.1,v
> retrieving revision 1.20
> diff -u -p -r1.20 tftp.1
> --- tftp.11 May 2012 04:23:21 - 1.20
> +++ tftp.11 Oct 2022 19:18:14 -
> @@ -62,7 +62,7 @@ is running, it issues the prompt
> and recognizes the following commands:
> .Pp
> .Bl -tag -width verbose -compact
> -.It Ic \&? Ar command-name ...
> +.It Ic \&? Ns | Ns Ic help Ar command-name ...
> Print help information.
> .Pp
> .It Ic ascii
>
Re: Softraid crypto with keydisk and installboot, skip on the same disk
On Tue, Sep 06, 2022 at 09:06:41PM +, Klemens Nanni wrote:
> On Sun, Sep 04, 2022 at 07:08:51PM +, Mikolaj Kucharski wrote:
> > Hi,
> >
> > I have strange setup on some of my machines, when I want to encrypt disk
> > where OpenBSD is installed, but still be able to boot them up without
> > any user interaction, like passphrase entry for CRYPTO softraid(4). I
> > have this so I can with little time spent lock out access to the disk,
> > by wiping beginning of the disk, instead of entire disk. I do recognise
> > magnitute of limitations of this. I still try to wipe entire disk, when
> > it's time for a machine decommission, but first I break CRYPTO softraid
> > by wiping beginning and then switch to proper full disk wipe.
> >
> > All in all that brings me to the below diff. I was only able to test on
> > amd64, as this is the only type of machine which I have.
>
> Thanks, although the setup seems a bit strange, your diff makes sense
> and works as advertised on amd64, arm64 and sparc64.
>
> I have adjusted our installboot regress tests to install onto softraid
> RAID 1C with a keydisk so it must a) iterate over multiple chunks and
> b) ignore the key-disk, which is a nice combined exercise.
>
> Here is your diff with tweaked wording so it is clearer; this also
> nicely aligns the "- skipping..." for both offline and keydisk cases.
>
> With this diff, regress/usr.sbin/installboot passes on amd64, arm64 and
> sparc64 using the above mentioned softraid.
>
> regress uses a separate device for the keydisk but that does not effect
> the skip logic.
>
> Feedback? OK?
Ping.
Index: efi_softraid.c
===
RCS file: /cvs/src/usr.sbin/installboot/efi_softraid.c,v
retrieving revision 1.2
diff -u -p -r1.2 efi_softraid.c
--- efi_softraid.c 29 Aug 2022 18:54:43 - 1.2
+++ efi_softraid.c 2 Oct 2022 21:08:14 -
@@ -54,6 +54,13 @@ sr_install_bootblk(int devfd, int vol, i
return;
}
+ /* Keydisks always has as size of zero. */
+ if (bd.bd_size == 0) {
+ fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
+ disk);
+ return;
+ }
+
if (strlen(bd.bd_vendor) < 1)
errx(1, "invalid disk name");
part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
Index: i386_softraid.c
===
RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
retrieving revision 1.19
diff -u -p -r1.19 i386_softraid.c
--- i386_softraid.c 29 Aug 2022 18:54:43 - 1.19
+++ i386_softraid.c 2 Oct 2022 21:08:15 -
@@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
return;
}
+ /* Keydisks always has as size of zero. */
+ if (bd.bd_size == 0) {
+ fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
+ disk);
+ return;
+ }
+
if (strlen(bd.bd_vendor) < 1)
errx(1, "invalid disk name");
part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
Index: sparc64_softraid.c
===
RCS file: /cvs/src/usr.sbin/installboot/sparc64_softraid.c,v
retrieving revision 1.6
diff -u -p -r1.6 sparc64_softraid.c
--- sparc64_softraid.c 29 Aug 2022 18:54:43 - 1.6
+++ sparc64_softraid.c 2 Oct 2022 21:08:16 -
@@ -55,6 +55,13 @@ sr_install_bootblk(int devfd, int vol, i
return;
}
+ /* Keydisks always has as size of zero. */
+ if (bd.bd_size == 0) {
+ fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
+ disk);
+ return;
+ }
+
if (strlen(bd.bd_vendor) < 1)
errx(1, "invalid disk name");
part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
tftpd: add -R for read-only mode/reduced pledges
diskless(8) just needs tftpd(8) to deliver files, none of the possibly
untrusted clients are supposed to ever write anything.
Either way, even when run without -c, a single file writable by _tftpd
might be enough for a malicious client to fill up the server's disk.
A proper read-only mode ("stdio rpath dns inet") seems much safer.
diskless(8) setup and manual testing runs fine with this, clients get
proper error responses (just like trying to write /etc/random.seed) and
the server keeps running without any pledge violations:
$ doas ./obj/tfpd -dvR ./_tftpd
$ touch file
$ echo put file | tftp localhost
tftp> Error code 2: Access violation
tftpd: 127.0.0.1: write request for 'file'
tftpd: 127.0.0.1: nak: Access violation
tftpd(8) is nicely written such that all file access goes through a
single validation function, so adding read-only logic seems trivial.
Did I miss anything?
Feedback? OK?
This applies cleanly to -current but conflicts with the -c/cpath diff
on tech@, so one needs rebasing after the other lands.
Index: tftpd.8
===
RCS file: /cvs/src/usr.sbin/tftpd/tftpd.8,v
retrieving revision 1.8
diff -u -p -r1.8 tftpd.8
--- tftpd.8 4 Mar 2019 01:06:03 - 1.8
+++ tftpd.8 1 Oct 2022 17:30:27 -
@@ -37,7 +37,7 @@
.Nd Trivial File Transfer Protocol daemon
.Sh SYNOPSIS
.Nm tftpd
-.Op Fl 46cdiv
+.Op Fl 46cdivR
.Op Fl l Ar address
.Op Fl p Ar port
.Op Fl r Ar socket
@@ -56,6 +56,8 @@ will allow only publicly readable files
Files may be written only if they already exist and are publicly writable,
unless the
.Fl c
+or
+.Fl R
flag is specified
.Pq see below .
Note that this extends the concept of
@@ -145,6 +147,8 @@ to
on startup;
the remote host is not expected to pass the directory
as part of the file name to transfer.
+.Fl R
+Prevent creating or writing to files.
.El
.Sh SEE ALSO
.Xr tftp 1 ,
Index: tftpd.c
===
RCS file: /cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.47
diff -u -p -r1.47 tftpd.c
--- tftpd.c 24 Oct 2021 21:24:19 - 1.47
+++ tftpd.c 2 Oct 2022 18:12:25 -
@@ -289,6 +289,7 @@ usage(void)
}
int cancreate = 0;
+int readonly = 0;
int verbose = 0;
int debug = 0;
int iflag = 0;
@@ -309,7 +310,7 @@ main(int argc, char *argv[])
int family = AF_UNSPEC;
int devnull = -1;
- while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) {
+ while ((c = getopt(argc, argv, "46cdil:p:r:vR")) != -1) {
switch (c) {
case '4':
family = AF_INET;
@@ -342,6 +343,9 @@ main(int argc, char *argv[])
case 'v':
verbose = 1;
break;
+ case 'R':
+ readonly = 1;
+ break;
default:
usage();
/* NOTREACHED */
@@ -351,6 +355,9 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
+ if (cancreate && readonly)
+ errx(1, "options -c and -R are incompatible");
+
if (argc != 1)
usage();
@@ -391,8 +398,13 @@ main(int argc, char *argv[])
if (!debug && rdaemon(devnull) == -1)
err(1, "unable to daemonize");
- if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) == -1)
- lerr(1, "pledge");
+ if (readonly) {
+ if (pledge("stdio rpath dns inet", NULL) == -1)
+ lerr(1, "pledge");
+ } else {
+ if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) ==
-1)
+ lerr(1, "pledge");
+ }
event_init();
@@ -964,6 +976,9 @@ validate_access(struct tftp_client *clie
int fd, wmode;
const char *errstr, *filename;
char rewritten[PATH_MAX];
+
+ if (readonly && mode != RRQ)
+ return (EACCESS);
if (strcmp(requested, SEEDPATH) == 0) {
char *buf;
Re: rarpd: clarify synopsis
Klemens Nanni wrote: > rarpd(8) is small enough where my impression is that refining it a > little would be good, but it quickly comes down to personal taste. And I continue to disagree. Another example of the same pattern is ifconfig. Here you will see it is not documented that [-a] is incompatible with [interface], but the program does actually enforce it (refuses to act). Even later in the man page, this is not actually described. Yet noone is dying from this. The synopsis / usage language simply cannot describe this, and I argue that it SHOULD NOT describe the situation, because past attempts at being complete precise have made the SYNOPSIS / usage cluttered, and thus less valuable to the users. Another way of looking at it is that the grammer is permissive, rather than proscriptive. So I suggest you get over it.
tftp: sort commands, add help to manual
Searching for a command in help output is much simpler when sorted.
The strings can be inlined into the struct while staying under 80 chars.
Now manual and help output are in the same order, except help folded
into ? in the manual to avoid duplicate text there.
Or should help appear at the same spot to make manual and help output
an exact match order-wise?
Feedback? OK?
Index: main.c
===
RCS file: /cvs/src/usr.bin/tftp/main.c,v
retrieving revision 1.43
diff -u -p -r1.43 main.c
--- main.c 20 Sep 2018 11:42:42 - 1.43
+++ main.c 1 Oct 2022 19:18:12 -
@@ -109,23 +109,6 @@ int opt_tsize = 0;
int opt_tout = 0;
int opt_blksize = 0;
-char vhelp[] = "toggle verbose mode";
-char thelp[] = "toggle packet tracing";
-char chelp[] = "connect to remote tftp";
-char qhelp[] = "exit tftp";
-char hhelp[] = "print help information";
-char shelp[] = "send file";
-char rhelp[] = "receive file";
-char mhelp[] = "set file transfer mode";
-char sthelp[] = "show current status";
-char xhelp[] = "set per-packet retransmission timeout";
-char ihelp[] = "set total retransmission timeout";
-char ashelp[] = "set mode to netascii";
-char bnhelp[] = "set mode to octet";
-char oshelp[] = "toggle tsize option";
-char othelp[] = "toggle timeout option";
-char obhelp[] = "set alternative blksize option";
-
struct cmd {
char*name;
char*help;
@@ -133,23 +116,23 @@ struct cmd {
};
struct cmd cmdtab[] = {
- { "connect",chelp, parsearg },
- { "mode", mhelp, modecmd },
- { "put",shelp, put },
- { "get",rhelp, get },
- { "quit", qhelp, quit },
- { "verbose",vhelp, setverbose },
- { "trace", thelp, settrace },
- { "status", sthelp, status },
- { "binary", bnhelp, setbinary },
- { "ascii", ashelp, setascii },
- { "rexmt", xhelp, setrexmt },
- { "timeout",ihelp, settimeout },
- { "tsize", oshelp, settsize },
- { "tout", othelp, settout },
- { "blksize",obhelp, setblksize },
- { "help", hhelp, help },
- { "?", hhelp, help },
+ { "?", "print help information", help },
+ { "ascii", "set mode to netascii", setascii },
+ { "binary", "set mode to octet",setbinary },
+ { "blksize","set alternative blksize option", setblksize },
+ { "connect","connect to remote tftp", parsearg },
+ { "get","receive file", get },
+ { "help", "print help information", help },
+ { "mode", "set file transfer mode", modecmd },
+ { "put","send file",put },
+ { "quit", "exit tftp",quit },
+ { "rexmt", "set per-packet retransmission timeout", setrexmt },
+ { "status", "show current status", status },
+ { "timeout","set total retransmission timeout", settimeout },
+ { "tout", "toggle timeout option",settout },
+ { "trace", "toggle packet tracing",settrace },
+ { "tsize", "toggle tsize option", settsize },
+ { "verbose","toggle verbose mode", setverbose },
{ NULL, NULL, NULL }
};
Index: tftp.1
===
RCS file: /cvs/src/usr.bin/tftp/tftp.1,v
retrieving revision 1.20
diff -u -p -r1.20 tftp.1
--- tftp.1 1 May 2012 04:23:21 - 1.20
+++ tftp.1 1 Oct 2022 19:18:14 -
@@ -62,7 +62,7 @@ is running, it issues the prompt
and recognizes the following commands:
.Pp
.Bl -tag -width verbose -compact
-.It Ic \&? Ar command-name ...
+.It Ic \&? Ns | Ns Ic help Ar command-name ...
Print help information.
.Pp
.It Ic ascii
Re: rarpd: clarify synopsis
On Sun, Oct 02, 2022 at 02:52:37PM +0100, Jason McIntyre wrote: > On Sun, Oct 02, 2022 at 01:07:04PM +, Klemens Nanni wrote: > > rarpd(8) either "Listen[s] on all the Ethernets attached to the system" > > or requires an explicit list, not both: > > > > $ rarpd -a em0 > > usage: rarpd [-adflt] if0 [... ifN] > > $ ./obj/rarpd -a em0 > > usage: rarpd [-dflt] -a | if ... > > > > Or would this be better? > > rarpd [-dflt] if ... > > rarpd -a [-dflt] > > > > Feedback? OK? > > > > please don't! Let's just leave it as is, then. > > - if you are specifying -a, you would hardly expect to give a list of > interfaces too. the manual documents it clearly. why single out -a? Yes, the manual makes that clear, but the usage/synopsis does not. I personally prefer such things to be visible right away, but that might just be me. > - with the newer version, it is unclear whether you can specify any > flags at all with "if ...". we could tighten that, but it's still > messy. You mean it could mean either '[-dflt] -a' or 'if ...'? Fair point, that's not better. > - and splitting such a simple SYNOPSIS in two would be nuts. Not a big deal to me; cut(1) seems a bit similar in that it is clearly split into three rather than an ambiguous one. > > - if anything, i think we could trim "if0 [... ifN]" to just "if ...". > so i agree with that part. That's a general simplification we could do in a few more manuals.
Re: rarpd: clarify synopsis
On Sun, Oct 02, 2022 at 10:09:32AM -0600, Theo de Raadt wrote: > The getopt language is imprecise, and attempts to be precise with it > usually go poorly. > > For example, > > SYNOPSIS > ls [-1AaCcdFfgHhikLlmnopqRrSsTtux] [file ...] > > % ls -1AaCcdFfgHhikLlmnopqRrSsTtux > > The result may seem surprising. I claim the result is not surprising. > > It is unsurprising because the getopt langauge makes no claim about > which options are prioritized over other options, nor does it make > claims about which option combinations will be rejected and result > in error. > > It only lists possibilities. Sometimes the words in the manual page > describe the subset that will work, but sometimes it doesn't. Sometimes > a program enforces a strict set of combinations, and somethings there > are surprises. You point about getopt not being that precise (by design) makes sense. Many manuals differ in this regard and yet I still consider each of them fine as they are. > And that's fine. Imagine trying to change the ls SYNOPSIS into 15 valid > combination so options, and then enforcing the priority and overlap > inside ls.c, and describing the reason why the priorites are chosen as > such (and probably introducing incompatibilities with other systems in > the process). I think that would be a gigantic waste of time to do just > in ls, and I think we should only try to improve this in extremely > narrow cases when it really matters. I agree, ls(1) seems like the ultimate case where splitting things up is a bad idea. rarpd(8) is small enough where my impression is that refining it a little would be good, but it quickly comes down to personal taste.
Re: Data Independent Timing on arm64
ok, let's give it a shot then. And watch for behaviour changes... Mark Kettenis wrote: > > From: "Theo de Raadt" > > Date: Sat, 01 Oct 2022 09:37:01 -0600 > > > > Mark Kettenis wrote: > > > > > Armv8.4 introduced a feature that provides data independent timing for > > > data processing instructions. This feature is obviously introduced to > > > mitigate timing side-channel attacks. Presumably enabling the feature > > > has some impact on performance as it would disable certain > > > optimizations to guarantee constant-time execution of instructions. > > > > But what impact does it have on all regular code? I cannot find that > > answer in the quoted email thread. And I don't even see the question > > being asked, because those people aren't talking about changing the > > cpu to this mode permanently & completely. > > > > > The only hardware that implements this feature that I've seen so far > > > is Apple's Icestorm/Firestorm (M1) and Avalanche/Blizzard (M2) cores. > > > I ran some benchmarks on an M2 Macbook Air. In particular, I ran > > > "eopenssl30 speed" bound to a "performance" core. > > > > That is testing the performance of a program which uses a very narrow > > set of cpu behaviours. For example, it has almost no crossings in & out > > of the kernel: system calls and page faults. The class of operations > > being tested are mostly pure compute against the register set rather > > than memory, and when it does perform memory loads, it does so in quite > > linear fashion. It also does relatively few memory writes. > > I also tested kernel builds. I don't see any evidence of any > significant impact on those. > > > It is a program that would be slower if they implimented the feature > > poorly, but using such a small subset of system behaviours, I don't > > think it can identify things which might be slower in part, and thus > > have an effect on whole system performance. > > The ARM ARM is rather explicit on the instructions that might be > affected by those flags. That list makes me believe any performance > impact would show up most prominantly in code that uses the ARMv8 > crypto instructions. > > > > I could not detect a significant slowdown with this feature enabled. > > > > Then why did they make it a chicken bit? Why did the cpu makers not > > simply make the cpus always act this way? There must be a reason, > > probably undisclosed. They have been conservative for some reasons. > > I wouldn't call it a chicken bit. But obviously those in charge of > the architecture spec anticipated some significant speedup from having > instructions that have data-dependent timings. It appears that > Apple's implementation doesn't though. > > What might be going on here is that Apple is just ticking boxes to > make their implementation spec compliant. The feature is required for > ARMv8.4 and above. But if none of their instructions have > data-dependent timing, they could implement the "chicken bit" without > it actually having an effect. > > > Is there an impact on the performance of building a full snapshot? That > > at least has a richer use of all code flows, with lots of kernel crossings, > > as opposed to the openssl speed command. > > I didn't test full snapshot builds. I think the kernel builds I did > should be representative enough. But I certainly can do more > benchmarking if you think that would be desirable. > > > > Therefore I think we should enable this feature by default on OpenBSD. > > > > Maybe. But I am a bit unconvinced. > > > > > The use of this feature is still being discussed in the wider > > > comminity. See for example the following thread: > > > > > > > > > https://lore.kernel.org/linux-arm-kernel/ywgcrqutxmx0w...@gmail.com/T/#mfcba14511c69461bd8921fef758baae120d090dc > > > > > > > That discussion is talking about providing the ability for programs to > > request that mode of operation. They are not talking about switching > > into that mode for the kernel and all processes. > > > > That seems like a big difference. > > > > >From a security perspective it might make some sense, but this isn't > > like some x86 catastrophy level speculation. I'm not sure there is > > enough evidence yet that this is required for all modes of operation. > > In principle this should be only necessary for code that is sensitive > to timing side-channel attacks. But the way I see it, the ecosystem > will need (a lot of) time to figure out how to enable this mode around > the bits of code where it *does* matter. > > > > On arm64, the feature can be controlled from userland, so even if we > > > turn it on by default, userland code can still make its own decisions > > > on whether it wants the feature enabled or disabled. We may have to > > > expose the ID_AA64PFR0_EL1 register to userland when code shows uo > > > that attempts to do that. > > > > I suspect that is this will go. Programs with specific libraries would > > then request the
Re: rarpd: clarify synopsis
The getopt language is imprecise, and attempts to be precise with it
usually go poorly.
For example,
SYNOPSIS
ls [-1AaCcdFfgHhikLlmnopqRrSsTtux] [file ...]
% ls -1AaCcdFfgHhikLlmnopqRrSsTtux
The result may seem surprising. I claim the result is not surprising.
It is unsurprising because the getopt langauge makes no claim about
which options are prioritized over other options, nor does it make
claims about which option combinations will be rejected and result
in error.
It only lists possibilities. Sometimes the words in the manual page
describe the subset that will work, but sometimes it doesn't. Sometimes
a program enforces a strict set of combinations, and somethings there
are surprises.
And that's fine. Imagine trying to change the ls SYNOPSIS into 15 valid
combination so options, and then enforcing the priority and overlap
inside ls.c, and describing the reason why the priorites are chosen as
such (and probably introducing incompatibilities with other systems in
the process). I think that would be a gigantic waste of time to do just
in ls, and I think we should only try to improve this in extremely
narrow cases when it really matters.
Klemens Nanni wrote:
> rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
> or requires an explicit list, not both:
>
> $ rarpd -a em0
> usage: rarpd [-adflt] if0 [... ifN]
> $ ./obj/rarpd -a em0
> usage: rarpd [-dflt] -a | if ...
>
> Or would this be better?
> rarpd [-dflt] if ...
> rarpd -a [-dflt]
>
> Feedback? OK?
>
> Index: rarpd.8
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 rarpd.8
> --- rarpd.8 28 Oct 2015 10:02:59 - 1.21
> +++ rarpd.8 29 Sep 2022 21:04:29 -
> @@ -29,8 +29,8 @@
> .Nd reverse ARP daemon
> .Sh SYNOPSIS
> .Nm rarpd
> -.Op Fl adflt
> -.Ar if0 Op Ar ... ifN
> +.Op Fl dflt
> +.Fl a | Ar if ...
> .Sh DESCRIPTION
> .Nm
> services Reverse ARP requests on the Ethernet connected to
> Index: rarpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 rarpd.c
> --- rarpd.c 15 Nov 2021 15:14:24 - 1.79
> +++ rarpd.c 29 Sep 2022 21:05:35 -
> @@ -217,7 +217,7 @@ init_all(void)
> __dead void
> usage(void)
> {
> - (void) fprintf(stderr, "usage: rarpd [-adflt] if0 [... ifN]\n");
> + fprintf(stderr, "usage: rarpd [-dflt] -a | if ...\n");
> exit(1);
> }
>
>
Re: rarpd: unveil /tftpboot only if needed
On Sun, 02 Oct 2022 12:33:21 -, Klemens Nanni wrote: > -t Only honour a request if the server (the host that rarpd is > running on) can "boot" the target; that is, if a file or > directory called /tftpboot/ipaddr exists, where ipaddr is the > target IP address expressed in uppercase hexadecimal (only the > first 8 characters of filenames are checked). > > Unless -t is used, this directory is not accessed in any way; TFTP_DIR > is used once in rarp_bootable() which gets called once: > > if (tflag == 0 || rarp_bootable(htonl(target_ipaddr))) > rarp_reply(ii, ia, ep, target_ipaddr, hp); OK millert@ - todd
Re: rarpd: clarify synopsis
On Sun, Oct 02, 2022 at 01:07:04PM +, Klemens Nanni wrote:
> rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
> or requires an explicit list, not both:
>
> $ rarpd -a em0
> usage: rarpd [-adflt] if0 [... ifN]
> $ ./obj/rarpd -a em0
> usage: rarpd [-dflt] -a | if ...
>
> Or would this be better?
> rarpd [-dflt] if ...
> rarpd -a [-dflt]
>
> Feedback? OK?
>
please don't!
- if you are specifying -a, you would hardly expect to give a list of
interfaces too. the manual documents it clearly. why single out -a?
- with the newer version, it is unclear whether you can specify any
flags at all with "if ...". we could tighten that, but it's still
messy.
- and splitting such a simple SYNOPSIS in two would be nuts.
- if anything, i think we could trim "if0 [... ifN]" to just "if ...".
so i agree with that part.
jmc
> Index: rarpd.8
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 rarpd.8
> --- rarpd.8 28 Oct 2015 10:02:59 - 1.21
> +++ rarpd.8 29 Sep 2022 21:04:29 -
> @@ -29,8 +29,8 @@
> .Nd reverse ARP daemon
> .Sh SYNOPSIS
> .Nm rarpd
> -.Op Fl adflt
> -.Ar if0 Op Ar ... ifN
> +.Op Fl dflt
> +.Fl a | Ar if ...
> .Sh DESCRIPTION
> .Nm
> services Reverse ARP requests on the Ethernet connected to
> Index: rarpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 rarpd.c
> --- rarpd.c 15 Nov 2021 15:14:24 - 1.79
> +++ rarpd.c 29 Sep 2022 21:05:35 -
> @@ -217,7 +217,7 @@ init_all(void)
> __dead void
> usage(void)
> {
> - (void) fprintf(stderr, "usage: rarpd [-adflt] if0 [... ifN]\n");
> + fprintf(stderr, "usage: rarpd [-dflt] -a | if ...\n");
> exit(1);
> }
>
>
rarpd: clarify synopsis
rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
or requires an explicit list, not both:
$ rarpd -a em0
usage: rarpd [-adflt] if0 [... ifN]
$ ./obj/rarpd -a em0
usage: rarpd [-dflt] -a | if ...
Or would this be better?
rarpd [-dflt] if ...
rarpd -a [-dflt]
Feedback? OK?
Index: rarpd.8
===
RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
retrieving revision 1.21
diff -u -p -r1.21 rarpd.8
--- rarpd.8 28 Oct 2015 10:02:59 - 1.21
+++ rarpd.8 29 Sep 2022 21:04:29 -
@@ -29,8 +29,8 @@
.Nd reverse ARP daemon
.Sh SYNOPSIS
.Nm rarpd
-.Op Fl adflt
-.Ar if0 Op Ar ... ifN
+.Op Fl dflt
+.Fl a | Ar if ...
.Sh DESCRIPTION
.Nm
services Reverse ARP requests on the Ethernet connected to
Index: rarpd.c
===
RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
retrieving revision 1.79
diff -u -p -r1.79 rarpd.c
--- rarpd.c 15 Nov 2021 15:14:24 - 1.79
+++ rarpd.c 29 Sep 2022 21:05:35 -
@@ -217,7 +217,7 @@ init_all(void)
__dead void
usage(void)
{
- (void) fprintf(stderr, "usage: rarpd [-adflt] if0 [... ifN]\n");
+ fprintf(stderr, "usage: rarpd [-dflt] -a | if ...\n");
exit(1);
}
tftp: only print prompt in interactive usage
Scripting tftp(1) makes it non-interactive, yet the prompt is still
printed and may mess up the shell's PS1:
$ echo put nonexistent | tftp localhost
tftp> tftp: open: nonexistent: No such file or directory
tftp> $
The fix seems easy and works as expected for multiple commands as well:
$ echo 'verbose\nput nonexistent' | ./obj/tftp localhost
Verbose mode on.
tftp: open: nonexistent: No such file or directory
$
I think the manual's wording can just stay as-is:
COMMANDS
Once tftp is running, it issues the prompt ‘tftp>’ and recognizes
the
following commands:
Feedback? OK?
Index: main.c
===
RCS file: /cvs/src/usr.bin/tftp/main.c,v
retrieving revision 1.43
diff -u -p -r1.43 main.c
--- main.c 20 Sep 2018 11:42:42 - 1.43
+++ main.c 2 Oct 2022 12:39:04 -
@@ -602,7 +602,8 @@ command(void)
struct cmd *c;
for (;;) {
- printf("%s> ", prompt);
+ if (isatty(STDIN_FILENO))
+ printf("%s> ", prompt);
if (readcmd(line, LBUFLEN, stdin) < 1)
continue;
if ((line[0] == 0) || (line[0] == '\n'))
rarpd: unveil /tftpboot only if needed
-t Only honour a request if the server (the host that rarpd is
running on) can "boot" the target; that is, if a file or
directory called /tftpboot/ipaddr exists, where ipaddr is the
target IP address expressed in uppercase hexadecimal (only the
first 8 characters of filenames are checked).
Unless -t is used, this directory is not accessed in any way; TFTP_DIR
is used once in rarp_bootable() which gets called once:
if (tflag == 0 || rarp_bootable(htonl(target_ipaddr)))
rarp_reply(ii, ia, ep, target_ipaddr, hp);
Feedback? OK?
Index: rarpd.c
===
RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
retrieving revision 1.79
diff -u -p -r1.79 rarpd.c
--- rarpd.c 15 Nov 2021 15:14:24 - 1.79
+++ rarpd.c 1 Oct 2022 20:05:52 -
@@ -339,8 +339,9 @@ rarp_loop(void)
arptab_init();
- if (unveil(TFTP_DIR, "r") == -1)
- error("unveil %s", TFTP_DIR);
+ if (tflag)
+ if (unveil(TFTP_DIR, "r") == -1)
+ error("unveil %s", TFTP_DIR);
if (unveil("/etc/ethers", "r") == -1)
error("unveil /etc/ethers");
if (pledge("stdio rpath dns", NULL) == -1)
tftpd: drop cpath promise unless file creation is allowed
-c Allow new files to be created; otherwise uploaded files must
already exist. Files are created with default permissions
allowing anyone to read or write to them.
Works for me in diskless(8) usage and manual tftp(1) get/put testing
with existing and new files.
Feedback? OK?
Index: tftpd.c
===
RCS file: /cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.47
diff -u -p -r1.47 tftpd.c
--- tftpd.c 24 Oct 2021 21:24:19 - 1.47
+++ tftpd.c 1 Oct 2022 16:40:56 -
@@ -391,8 +391,13 @@ main(int argc, char *argv[])
if (!debug && rdaemon(devnull) == -1)
err(1, "unable to daemonize");
- if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) == -1)
- lerr(1, "pledge");
+ if (cancreate) {
+ if (pledge("stdio rpath wpath cpath fattr dns inet", NULL) ==
-1)
+ lerr(1, "pledge");
+ } else {
+ if (pledge("stdio rpath wpath fattr dns inet", NULL) == -1)
+ lerr(1, "pledge");
+ }
event_init();
rc.d/dhcpd: enable configtest
dhcpd(8) has
-n Only test configuration, do not run dhcpd.
rc_configtest() taken from other rc.d scripts.
Feedback? OK?
Index: dhcpd
===
RCS file: /cvs/src/etc/rc.d/dhcpd,v
retrieving revision 1.3
diff -u -p -r1.3 dhcpd
--- dhcpd 11 Jan 2018 19:52:12 - 1.3
+++ dhcpd 29 Sep 2022 20:52:28 -
@@ -6,6 +6,11 @@ daemon="/usr/sbin/dhcpd"
. /etc/rc.d/rc.subr
+rc_configtest() {
+ # use rc_exec here since daemon_flags may contain arguments with spaces
+ rc_exec "${daemon} -n ${daemon_flags}"
+}
+
rc_reload=NO
rc_pre() {
Re: problem with interrupts on machines with many cores and multiqueue nics
On 1.10.2022. 23:28, Mark Kettenis wrote: > At least on some of these machines, you're simply running out of > kernel malloc space. The machines "hang" because the M_WAITOK flag is > used for the allocations, and malloc(9) waits in the hope someone else > gives up the memory. Maybe we need to allow for more malloc space. > But to be sure this isn't the drivers being completely stupid, vmstat > -m output would be helpful. Do you mean vmstat -m when machines are fine or show all pools or some other command when in ddb console ?
