Re: CVS commit: src

[Greg Troxel]

Martin Husemann mar...@duskware.de writes:

 On Mon, Jan 12, 2015 at 02:14:17PM -0500, Christos Zoulas wrote:
 macppc? I don't think that the iso image produced there is bootable.
 The question is how many of those images are bootable?

 Ma68k and macppc need hybrid HFS+/ISO9660 images, which mkimage can
 not create currently.

On a 1 Ghz ish macppc laptop, I booted the iso created by build.sh
release on the netbsd-7 branch within the last few months.   Perhaps
it's the older macppcs that need the hybrid images


pgp9qbFrtzezr.pgp
Description: PGP signature

Re: CVS commit: src

[Greg Troxel]

Alan Barrett a...@cequrux.com writes:

 On Sat, 07 Mar 2015, Martin Husemann wrote:
 Anything that has no PCI.

 Could we rename LEGACY to something more descriptive?  The problem
 with the name LEGACY is that it leaves people wondering whether or
 not particular hardware is old enough to be classified as legacy.
 If the test is Does it have an ISA bus without a PCI bus? then I'd
 suggest a name like ISA_NO_PCI.

I agree about ISA_NO_PCI.   In addition, legacy is a really
problematic word in technical English, because it usually means how
people do things now, compared to the new thing I am proposing that I
think everyone should do instead.

I'm also not sure we need a new kernel config.  It seems the systems of
interest are limited to 486 machines without PCI (and thus EISA
probably), and that's a pretty narrow window around 1993-1994.  So
leaving the lines commented out with a comment explaining it in the
kernel config file is probably entirely adequate for the handful of
people who still have such hardware.


pgpVKVcCocu7L.pgp
Description: PGP signature

Re: CVS commit: src

[Greg Troxel]

David Laight da...@l8s.co.uk writes:

 I'm also not sure we need a new kernel config.  It seems the systems of
 interest are limited to 486 machines without PCI (and thus EISA
 probably), and that's a pretty narrow window around 1993-1994.  So
 leaving the lines commented out with a comment explaining it in the
 kernel config file is probably entirely adequate for the handful of
 people who still have such hardware.

 It is probably almost all 486 systems, and no pentium ones.
 They probably need a 'small' kernel anyway.

I had a 486DX4 motherboard, now broken, that had PCI.  A good point
about GENERIC not being ok.

 More interesting might be the embedded 486-like systems
 from soekris (etc). Not sure if any of those have graphics
 but they will normally run a generic kernel.

The Soekris net5501 and net6501 don't have any VGA, and I think the rest
also lack it.  But I see the point about the other SOC stuff.

So just NO_PCI is descriptive enough; we should rename the file is
going to stay.



pgpLvZZzmoZsm.pgp
Description: PGP signature

Re: CVS commit: src

[Greg Troxel]

Manuel Bouyer bou...@antioche.eu.org writes:

 On Sat, Mar 07, 2015 at 07:28:37AM +, matthew green wrote:
 Module Name: src
 Committed By:mrg
 Date:Sat Mar  7 07:28:37 UTC 2015
 
 Modified Files:
  src/distrib/notes/i386: contents
  src/etc/etc.i386: Makefile.inc
  src/sys/arch/i386/conf: GENERIC
 Added Files:
  src/sys/arch/i386/conf: LEGACY
 
 Log Message:
 remove vga@isa and pcdisplay@isa from i386 GENERIC, and create a new
 LEGACY kernel that includes them instead.  now radeon@pci is able to
 properly claim wsdisplay0 on i386 systems, and radeondrmkms has a good
 chance of working.
 
 this fixes PR#49290.

 This should also fix a problem I noticed with an intel device but
 didn't report yet:
 an intelfb0 is attached to i915drmkms0@pci, but a vga@isa is also
 attached, ending up with 2 wsdisplays. The console keyboard is then
 attached to wsdisplay1, leaving the console (wsdisplay0) without
 keyboard ...

Sorry, I missed the discussion on this.

Will LEGACY be built by default?

What's the set of computers that one has to use LEGACY on?
Is it just systems that are so old that vga is not via PCI?  Are there
thought to be any that have a 486 and could have worked?


pgp1UG8z4tTD0.pgp
Description: PGP signature

Re: CVS commit: src

[Greg Troxel]

Martin Husemann mar...@duskware.de writes:

 On Sat, Mar 07, 2015 at 08:13:17AM -0500, Greg Troxel wrote:
 Will LEGACY be built by default?

 I would vote no on that one.

 What's the set of computers that one has to use LEGACY on?

 Anything that has no PCI.

That sounds ok, then.

 Is it just systems that are so old that vga is not via PCI?  Are there
 thought to be any that have a 486 and could have worked?

 I have a working one of those, EISA, 486DX, basically sun-lamp lookalike
 (if you remember that). I plan to add it to my regular test schedule, maybe
 once a month.

I see - that is pretty ancient, and the number of people wanting to run
NetBSD (or anything) on those is probably tiny.

So this all seems fine to me - I just couldn't tell right away and had
wondered for a second if it was going to affect all machines that didn't
have the spiffy drm2-capacble hw.


pgpWRiUnVNExo.pgp
Description: PGP signature

Re: CVS commit: src/usr.bin/pwait

[Greg Troxel]

chris...@astron.com (Christos Zoulas) writes:

 If we want to make every single change to go through tech-userlevel,
 we should institute a rule to do so. To my knowledge we don't have yet
 such a rule. We already have the rest of the p* programs which originated
 in Solaris, we were missing this one, so it made sense to me to add it.

We do sort of have a rule, which is that significant changes need
discussion.  I would say adding programs to base always counts.   Other
than that, it's trickier, but if there's a reasonable likelihood of a
valid objection, I'd say it's over the line into signficant/discuss.


pgp7H4fsvfMbX.pgp
Description: PGP signature

Re: CVS commit: src/usr.bin/pwait

[Greg Troxel]

Marc Balmer m...@msys.ch writes:

 Am 03.03.15 um 14:35 schrieb Greg Troxel:
 
 chris...@astron.com (Christos Zoulas) writes:
 
 If we want to make every single change to go through
 tech-userlevel, we should institute a rule to do so. To my
 knowledge we don't have yet such a rule. We already have the rest
 of the p* programs which originated in Solaris, we were missing
 this one, so it made sense to me to add it.
 
 We do sort of have a rule, which is that significant changes need 
 discussion.  I would say adding programs to base always counts.
 Other than that, it's trickier, but if there's a reasonable
 likelihood of a valid objection, I'd say it's over the line into
 signficant/discuss.

 Adding new stuff bears a low risk of breaking existing stuff, so I
 think it does not need discussion all the time.

I meant that adding to base was discuss-worthy because there's a bloat
or necessary question, not because of risk of breakage.


pgpSkT3o7R8yK.pgp
Description: PGP signature

Re: CVS commit: src/usr.bin/pwait

[Greg Troxel]

Marc Balmer m...@msys.ch writes:

 I meant that adding to base was discuss-worthy because there's a
 bloat or necessary question, not because of risk of breakage.

 Sure.  So how much bloat is pwait?  Is it a huge piece of software
 or a small utility?  I think that matters a bit.

Posting a note that says I would like to add X, and here's why, and my
comments on the bloat/necessary tradeoff. takes only a few minutes
(assuming well-formed thoughts already).  If there's no grumbling in
48-72h, that's fine.  It's not like this sort of review/comment costs a
lot or really gets in the way - new programs in base are pretty rare.

I don't think pwait is a big deal.  I do think that in general we have
too much commit first, argue about appropriate later.


pgpjC8D8gagfc.pgp
Description: PGP signature

Re: CVS commit: src/distrib/atari/floppies/install

[Greg Troxel]

Izumi Tsutsui tsut...@ceres.dti.ne.jp writes:

 joerg@ wrote:

  Please don't commit untested and broken fix.
  Please file a PR instead if you can't test it.
 
 I agree with Christos that leaving the build broken is worse.  The
 alternative band aids are much more involved, too. That shouldn't stop
 the second point from that list.

 If you claim leaving the build broken is worse than commiting untested code,
 you should ask to update our commit guideline first:

 http://www.netbsd.org/developers/commit-guidelines.html

Perhaps we should.  The problems are:

  A broken build is evidence that the prior commits were not tested, and
  thus need fixing.  Objecting to fixing the build without testing
  should be a far lower priority than objecting to commits that break
  the build.

  A broken build, or a build that succeeds but fails to work is a
  serious problem because it prevents bisecting to find bugs.  I've seen
  this in current/i386 pretty often, less so recently.  During a ~week
  that the build was broken many commits happened, and some of those
  were trouble, and it was a mess to sort out.  So arguably no commits
  should be allowed at all during a time when the build is broken or
  there are sudden significant new test failures, other than fixing the
  build.

  


pgpPVycMH2F2d.pgp
Description: PGP signature

Re: CVS commit: src/distrib/atari/floppies/install

[Greg Troxel]

Joerg Sonnenberger jo...@britannica.bec.de writes:

 On Sun, Apr 26, 2015 at 10:07:32AM -0400, Greg Troxel wrote:

 A broken build is evidence that the prior commits were not tested,
 and thus need fixing.  Objecting to fixing the build without testing
 should be a far lower priority than objecting to commits that break
 the build.

 The problem here is that the original commit that triggered the
 overflow can and often enough is perfectly reasonable on all platforms
 without artifically low size constraints. What is needed to get at
 least basic regression testing done in an emulator?

Fair enough and a good point about emulator testing being really
helpful.


pgpGjBHUeCXLj.pgp
Description: PGP signature

Re: CVS commit: src/share/misc

[Greg Troxel]

S.P.Zeidler s...@netbsd.org writes:

 Thus wrote Paul Goyette (p...@vps1.whooppee.com):

 At the very les, if we're going to have these acronyms, they should be
 listed in a separate file which is not searched by default.  Similar to what
 is done with fortune(6).

 But that might not serve to indicate to these unwanted elements (women)
 that they are not welcome here and might face violence if they obtrude anyway.

Indeed.  This kind of content has no place in NetBSD or any other
open-source project.  It should just be removed.


pgpBXTvGTCS_1.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev

[Greg Troxel]

Michael van Elst mlel...@netbsd.org writes:

 Modified Files:
   src/sys/dev: dksubr.c

 Log Message:
 warn about labels only when built with DIAGNOSTIC

This feels like an abuse of DIAGNOSTIC, which is supposed to be about
asserts for detecting internal inconsistencies, not changing the
behavior of how the kernel responds to external inputs.

I don't have any problem with warnings for dodgy external input being
generally enabled, or having a separate verbose option.   I can't see
any reason not to have them, as it's not like new labels appearing is a
high-rate event that would dos the log.


pgpI40F0feW4P.pgp
Description: PGP signature

Re: CVS commit: src/external/cddl/osnet/dist/lib/libdtrace/common

[Greg Troxel]

chris...@zoulas.com (Christos Zoulas) writes:

> On Sep 25,  9:30am, m...@eterna.com.au (matthew green) wrote:
> -- Subject: re: CVS commit: src/external/cddl/osnet/dist/lib/libdtrace/common
>
> | when you sync things, can you also include what version you sunc with?
> | so for the next person to update, they know what baseline we have in
> | our tree.
>
> In this case all the source files have __FBSDID's... Do you mean add in
> the commit message the date and the time I did the last svn sync?

What I'd like to see is the commit message specify which FreeBSD branch
it came from (head presumably, but not entirely clear) and the date.
Your point about _FBSDID is fair, but doing a cvs up -D or the equiv in
FreeBSD before importing would make it clearer to do later updates
(since we can't/shouldn't tag FreeBSD's tree for exports to us).


pgpqZLL_BEH71.pgp
Description: PGP signature

Re: CVS commit: src/sys/net

[Greg Troxel]

chris...@astron.com (Christos Zoulas) writes:

> In article <20151210081103.e0fbbf...@cvs.netbsd.org>,
> Kengo NAKAHARA  wrote:
>>-=-=-=-=-=-
>>
>>Module Name:  src
>>Committed By: knakahara
>>Date: Thu Dec 10 08:11:03 UTC 2015
>>
>>Modified Files:
>>  src/sys/net: if_gif.c
>>
>>Log Message:
>>kmem_zalloc(, KM_SLEEP) must not return NULL.
>
> I would like to solicit opinions about this change and form a general
> policy.
>
> 1. I would like to reduce the use of KASSERT in the kernel, specially
> in situations like thee above where the test can be centralized (inside
> kmem_alloc) and avoided without being fatal.
>
> 2. Static analyzer models understand allocators, but they are not
> smart enough to determine under which situations they can fail. I
> believe even kmem_alloc with KM_SLEEP can fail when the size is
> large enough.
>
> So I propose to always check the return value of allocators with
> an 'if' and not a KASSERT.

I think that a function needs to have a contract specified in a contract
(and perhaps static analyzer markup).  Code should never KASSERT for
anything that can not be argued (statically shown) to be always true
given contracts.  So I agree with you.


signature.asc
Description: PGP signature

Re: CVS commit: src/tests/lib/libutil

[Greg Troxel]

"David A. Holland"  writes:

> Module Name:  src
> Committed By: dholland
> Date: Thu Dec 31 10:18:00 UTC 2015
>
> Modified Files:
>   src/tests/lib/libutil: t_parsedate.c
>
> Log Message:
> When evaluated on a Sunday, "next Sunday" means 7 days in the future,
> not 14. When evaluated on a Monday, it apparently means 13 days in the
> future. There's not exactly a spec for parsedate.y, so conform to the
> implementation.
>
> PR 50574.
>
> XXX: to me at least this is an odd notion of "next Sunday", but whatever...

It's clearly a bug.

On a Monday, "next Sunday" is 6 days away.  "Sunday week" is 13.

So perhaps the test should be correct, and the implementation fixed.



signature.asc
Description: PGP signature

Re: CVS commit: src/sys/dev/usb

[Greg Troxel]

Nick Hudson  writes:

> On 09/10/16 16:46, Jonathan A. Kollasch wrote:
>> Module Name: src
>> Committed By:jakllsch
>> Date:Sat Sep 10 15:46:36 UTC 2016
>>
>> Modified Files:
>>  src/sys/dev/usb: usbdevices.config
>>
>> Log Message:
>> Make usscanner(4) useful by also attaching its children.
>>
> I was hoping to remove u{s,}scanner(4) in favour of userland drivers...

I think that's a good long-term plan, but it seems that usscanner(4),
when configured and enabled, should also work in the interrim.  But
maybe it doesn't work well enough to use at all anyway.



signature.asc
Description: PGP signature

Re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

[Greg Troxel]

matthew green  writes:

>> In short, this is because -munaligned-access is enabled on ARMv6+ by
>> default for GCC. As the unaligned memory access is forbidden in the
>> supervisor mode unlike in the user mode, we need to explicitly specify
>> -mno-unaligned-access for kernel on ARMv6+.
>
> i think this seems like the right thing to do here.
>
> othewise we'd have to patch this all over..

Why do we generate code with unaligned access in user space?  That seems
surprising, if the processor isn't happy about it.

Re: CVS commit: src/etc

[Greg Troxel]

matthew green  writes:

> (i wouldn't pick 'wheel' as this group -- i would invent a
> new group either called 'net' or 'wpa', with no underscore
> since they're designed to be assigned, unlike the groups
> for specific programs security models.)

Are you saying that you are ok with the following:

   add a new group "net"

   by default, nobody is in it

   it's ok for things that modify networking config to allow this to be
   done by users in group net, in addition to root

   (so therefore, absent configuration by root, there are no additional
   privileges compared to now)

?

If so, that seems like a reasonable compromise compared to letting wheel
modify networking, and calling it "net" lets this be a logical privilege
in general, even if wpa config is the only thing right now.

Re: CVS commit: src/etc

[Greg Troxel]

Roy Marples  writes:

> On 13/01/2019 10:20, matthew green wrote:
>> shouldn't one need to be root to modify network configuration?
>> i shouldn't be able to tell wpa_supplicant to do something as
>> non-root, in a default install.
>
> In a default install the only member of wheel is root and
> wpa_supplicant is not started.
>
> I suppose the real question is do we want to allow group access to
> wpa_supplicant and if so which group if not wheel?

That is indeed the real question.  As I see it wheel has historically
been a group for users that are system administrators, given how "su"
only allows users in wheel to su.  So it seems reasonable to allow
various configuration changes by users in wheel.

It seems the only point in putting somebody in wheel now is if you tell
them the root pw, to let them su.  Are there other reasons?

Another approach is to create a wpa_supplicant group, and allow wpa
changes by those in that group.  I can't see any reasonable objection to
this, other than group bloat.

> If we don't want to allow group access I may as well revert my changes
> and setup is then as before - the user is expected to configure
> everything themselves and wpa_cli won't work by default. This would be
> a shame as I've had a lot of positive feedback on this change already.

Even if you have to be root, these changes are still hugely useful.
"sudo wpa_cli" is not that hard, even if it seems like it should not be
necessary.

Re: CVS commit: src/etc

[Greg Troxel]

Jason Thorpe  writes:

>> On Jan 13, 2019, at 5:21 AM, Greg Troxel  wrote:
>> 
>> Even if you have to be root, these changes are still hugely useful.
>> "sudo wpa_cli" is not that hard, even if it seems like it should not be
>> necessary.
>
> ...but made slightly more annoying seeing as how sudo is not part of the base 
> OS.

s/sudo wpa_cli/su root -c wpa_cli/

But yes, it is harder.  I had to read the su man page (back when I was
young, we didn't have sudo and had to use su uphill both ways after
toggling in the boot loader).

Re: CVS commit: src/share/mk

[Greg Troxel]

David Brownlee  writes:

> On Thu, 14 Nov 2019 at 21:05, Christos Zoulas  wrote:
>>
>> The first issue is that I prefer to have tar respect existing
>> symlinks (ones that it did not create by default -- without having
>> to specify extra flags) since to do this (in my opinion) does not
>> pose a security risk. Yes my implementation is Q+D, but it works.
>>
>> https://www.netbsd.org/~christos/track-symlinks.diff
>>
>> The second issue is that the way libarchive-tar overwrites existing
>> files is by removing them first, and writing them directly to the
>> final destination pathname. This creates a significant amount of
>> time where the file is either not available or not completely
>> written. Imagine trying to replace shared libraries or programs
>> frequently used. Pax-as-tar wrote the file to a temporary one first
>> and used rename(2) to atomically replace it. I've written a patch
>> to do the same for libarchive-tar:
>>
>> https://www.netbsd.org/~christos/libarchive-atomic.diff
>>
>> With those two patches we can put libarchive as tar back and we don't
>> need to make any other changes since behavioraly the old pax-as-tar
>> and libarchive-tar behave the same for those two cases that bothered us.
>>
>> I am inclined to just commit them and flip the default again.
>
> I could see an argument for having an option to turn off the
> extract-to-temp-and-rename behaviour (not that I'd use it), but I'd be
> very happy to see both above changes in as defaults and us back onto
> libarchive-tar

Me too.

My sense of the list is that

  most people want unpacking sets over a system to work as well as it
  used to work

  some people really want a tar that handles newer formats, and probably
  everything thinks this would be good

  some concern has been expresseed over performance, but more people
  have expressed the notion that things working correctly (which does
  not have a universal definition) is more important.

so Christos's proposal seems to steer very well to this rough consensus.

> Thanks for working on this!

Seconded!

Re: CVS commit: src/sys/sys

[Greg Troxel]

Kamil Rytarowski  writes:

> On 23.12.2019 01:54, Roy Marples wrote:
>> On 22/12/2019 22:24, Andrew Doran wrote:
>>> NetBSD 9.99.29 - struct mount changed.
>> 
>> Just curious - does our build software cope with 3 digit for the last
>> number?
>> 
>> Roy
>
> At least from the __NetBSD_Version__ point of view there are 4 digits
> unused, but it is more valuable to branch -10 earlier than going to
> 3-digit patchlevel.

Well, we are coming up on a year since netbsd-9 was branched, or at
least will arrive there before this discussion resolves.   So cutting
-10 before we hit 100 works for me :-)

Re: CVS commit: src/sys/sys

[Greg Troxel]

Martin Husemann  writes:

> On Mon, Dec 23, 2019 at 09:02:50AM -0500, Greg Troxel wrote:
>> Well, we are coming up on a year since netbsd-9 was branched, or at
>> least will arrive there before this discussion resolves.   So cutting
>> -10 before we hit 100 works for me :-)
>
> Nitpicking (and I don't know for the discussion resolving), but netbsd-9
> was branched on 2019-07-30 (so not even 1/2 a year yes).
>
> The branch for netbsd-10 can happen soon after Andrew is done (we need
> 10.0 on the build cluster ASAP).

I will admit that my comment was partly humor.

Thanks for pointing out the -9 branch date.  Given that we have had an
RC, this branch is going much better than recent previous ones.  I
realize it's always difficult, but I think we (mostly you, perhaps) are
doing better this time.

I did mean to be somewhat serious in saying it was going to be time to
start 10, just based on calendar, because I believe releases should be
no more than 18 months apart, and I think 12 months is ideal.  Thus I am
in favor of starting a new branch 12 months after the last one was
started.

(I see the merits of points about lots of improvements in current vs 9
and the reasonableness of branching late spring and releasing fall, even
if that seems a bit aspirational.)

Re: CVS commit: src/sys/sys

[Greg Troxel]

Warner Losh  writes:

> Just a quick note: when FreeBSD when from 9 to 10, we had to play 'hunt the
> wumpus' for all the autoconfig scripts that suddenly thought they were
> configuring for FreeBSD 1.0.
>
> If you can arrange it, it might make sense to do a pkgsrc run on an
> experimental system that has the version as 10.0 rather than 9.9.xx before
> committing to a schedule given the experience of your sister BSD project.

Thanks, that's a really good point.

Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs

[Greg Troxel]

J. Hannken-Illjes  writes:

>> Forgot to add in the commit log, the changes have been pulled in from
>> upstream openzfs.
>> 
>> https://github.com/openzfs/zfs/commit/928e8ad47d3478a3d5d01f0dd6ae74a9371af65e#diff-9fd6b453f8153161abe0728c449e6505R4386
>
> This is NOT our upstream --

This seems to be hard to figure out.  Is there someplace in the tree
that says what our upsream is, and what theirs is, and how all of the
various trees out there relate?  And how we should be sending things,
and getting things?

I tried to figure this out, and ended up with

http://wiki.netbsd.org/zfs/

which has lots of \todo statements.

Re: CVS commit: src/external/gpl3

[Greg Troxel]

Taylor R Campbell  writes:

>> Do we insist on this patch? Can we remove it from local sources?
>
> We should keep the change.  There is no semantic justification for
> putting build-time temporary files in the directory for temporary
> files that are meant to persist across reboot.  These temporary files
> _cannot_ be used if interrupted -- let alone by a reboot.

I'll fourth this sentiment.

Plus, I'll observe that on my main development system /var/tmp has 6.4G
free while /tmp has 12G.  That's of course just arbitrary choices on my
part, and if either is an issue for a compiler there's something wrong
anyway.

Re: CVS commit: src/external/gpl3

[Greg Troxel]

Great to hear of progress.

I don't see why we don't care about 8.  Given our release policy, that
is supported until 10 is released.

Re: CVS commit: src/external/gpl3

[Greg Troxel]

Kamil Rytarowski  writes:

> On 26.03.2020 15:01, Martin Husemann wrote:
>> On Thu, Mar 26, 2020 at 02:57:40PM +0100, Kamil Rytarowski wrote:
>>> The build of tools could be fixed independently.
>> The problem is that we build the whole system with the tools gcc, and that
>> gcc misbehaves (or so I understood).
>> 
>> So pointing TMPDIR anywhere does not help.
>> 
>
> Right. Addressing tools build independently (honoring TMPDIR?) and
> defining TMPDIR in /etc shell profile for common use inside the
> distribution.

It seem really obvious to me, and obvious that it is netbsd consensus,
that a tool that needs tmp (without needing persistence) should default
to /tmp.  So I think it's unreasonable to follow upstream here, and
unreasonable to ask everybody to either inherit some system profile or
adjust their own to make tmp files be put in /tmp.

Certainly if something in the tools build doesn't honor TMPDIR, that's a
bug to be fixed -- but that seems uncontroversial.


People who really want gcc tmp files to be put in /var/tmp can set
TMPDIR.  As I understand it people that actually want that are a
minority, and we haven't had a single person express that they want it
during this discussion.

What is the real problem here?  I think it's great that NetBSD-specific
fixes are being upstreamed, and that we are reducing gratuitous changes
from upstream.  But upstream's position that the default tmp should be
/var/tmp is at odds with our and traditional norms, and really seems to
just not make sense.  (Perhaps it does make sense on typical Linux, and
perhaps upstream should have OS-specific tmp defaults.)   Adding
complexity to NetBSD config files and users for the sake of reducing a
diff seems like a bad tradeoff.

Re: CVS commit: src/external/gpl3

[Greg Troxel]

Greg Troxel  writes:

> It seem really obvious to me, and obvious that it is netbsd consensus,
> that a tool that needs tmp (without needing persistence) should default
> to /tmp.  So I think it's unreasonable to follow upstream here, and
> unreasonable to ask everybody to either inherit some system profile or
> adjust their own to make tmp files be put in /tmp.
>
> Certainly if something in the tools build doesn't honor TMPDIR, that's a
> bug to be fixed -- but that seems uncontroversial.
>
>
> People who really want gcc tmp files to be put in /var/tmp can set
> TMPDIR.  As I understand it people that actually want that are a
> minority, and we haven't had a single person express that they want it
> during this discussion.
>
> What is the real problem here?  I think it's great that NetBSD-specific
> fixes are being upstreamed, and that we are reducing gratuitous changes
> from upstream.  But upstream's position that the default tmp should be
> /var/tmp is at odds with our and traditional norms, and really seems to
> just not make sense.  (Perhaps it does make sense on typical Linux, and
> perhaps upstream should have OS-specific tmp defaults.)   Adding
> complexity to NetBSD config files and users for the sake of reducing a
> diff seems like a bad tradeoff.

If you were only talking about fixing tools build, and have withdrawn
the notion of removing our patch to fix the default, I apologize for
writing this way.   My note is based on a perception that you are still
pushing to remove our patch to fix the tmp behavior.

Re: CVS commit: src/external/gpl3

[Greg Troxel]

Generally speaking divergence with upstream is a problem and we lost
these changes anyway in pkgsrc and every standalone GNU toolchain copy.


Agreed that divergence is a problem, but so is having bugs.  It's a 
question of the right balance in each case.



Finding a creative way to make this at least tunable is challenging with
the GCC people.


I think this is the real issue.   As I understand it, upstream is making 
a choice that is inconsistent with longstanding norms and against the 
consensus of an OS community.   So if the best that can happen is that 
we have to patch, that's how it is.  It would be really nice if there 
were a --default-tmpdir= configure argument, but it sounds like they are 
unwilling to do that.


Basically, I don't think we can go from "upstream is unwilling to do X" 
to "impose pain on NetBSD" by making "divergence bad" be the 
highest-weighted concern.   We have to say "what is the minimally 
painful way to cope".

Re: CVS commit: src/tests/lib/libarchive

[Greg Troxel]

Jason Thorpe  writes:

>> On Jun 16, 2020, at 8:43 AM, Martin Husemann  wrote:
>> 
>> No, that is definitively not OK, which is what the PR is about.
>> 
>> It is not OK for a regular atf run to cause a reboot of the test machine
>> though, so this is a temporary hack around the issue (and admitedly a very
>> ugly hack).
>
> At the very least, the user-land watchdog tickler should wire itself down.

My impression of the point is that it be normal, so that the system
reboots if normal processes cannot be run.It seems like once
whatever bug exists is fixed, wiring is probably not necessary anyway.

Re: CVS commit: src/sys/arch/amd64/conf

[Greg Troxel]

matthew green  writes:

> could this be done with include and "no foo" statement?
> eg, like sys/arch/sparc/conf/INSTALL does.

Maybe, but I'm not sure it will end up working.  Right now we don't know
if any of the missing things will be trouble, and even if we do move to
include/no I'd like to do that via an intermediate step of a config with
small differences.

Also I think we should also consider extracting lots of things into
includable files, similar to how

  include "dev/usb/usbdevices.config"

is used now in GENERIC.   That will raise interesting cross-arch issues
about value vs kernel memory usage probably.

These include files would allow a simplification of XEN3_DOMU which as I
see it should have ~no drivers but all the rest of the options.


signature.asc
Description: PGP signature

Re: CVS commit: src/sbin/fsck_ffs

[Greg Troxel]

  (or just use FFSv2 (UFS2) etc.) in some man pages and related docs.
  (and also add the UFS2 paper into the SEE ALSO section?)

I see both points, and think this is an excellent way to address both of
them.



pgpV50T6scYJU.pgp
Description: PGP signature

CVS commit: src/usr.bin/passwd

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Tue Mar  2 16:19:13 UTC 2010

Modified Files:
src/usr.bin/passwd: local_passwd.c pam_passwd.c

Log Message:
Log successful and unsuccessful attempts to change passwords, via -l
or pam, to ease IT audit guideline compliance.  Patch from Richard
Hansen of BBN in private mail.

Proposed on tech-kern with positive comments, except a suggestion I
didn't implement:

A possible future enhancement is refraining from logging if the old
password is empty, as some people abort password changing that way.
However, it's not clear if this complies with most guidelines that
require password change logging, and at first glance that appears to
be a fairly difficult change.


To generate a diff of this commit:
cvs rdiff -u -r1.33 -r1.34 src/usr.bin/passwd/local_passwd.c
cvs rdiff -u -r1.4 -r1.5 src/usr.bin/passwd/pam_passwd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/passwd/local_passwd.c
diff -u src/usr.bin/passwd/local_passwd.c:1.33 src/usr.bin/passwd/local_passwd.c:1.34
--- src/usr.bin/passwd/local_passwd.c:1.33	Fri Apr 17 20:25:08 2009
+++ src/usr.bin/passwd/local_passwd.c	Tue Mar  2 16:19:13 2010
@@ -1,4 +1,4 @@
-/*	$NetBSD: local_passwd.c,v 1.33 2009/04/17 20:25:08 dyoung Exp $	*/
+/*	$NetBSD: local_passwd.c,v 1.34 2010/03/02 16:19:13 gdt Exp $	*/
 
 /*-
  * Copyright (c) 1990, 1993, 1994
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = from: @(#)local_passwd.c8.3 (Berkeley) 4/2/94;
 #else
-__RCSID($NetBSD: local_passwd.c,v 1.33 2009/04/17 20:25:08 dyoung Exp $);
+__RCSID($NetBSD: local_passwd.c,v 1.34 2010/03/02 16:19:13 gdt Exp $);
 #endif
 #endif /* not lint */
 
@@ -53,6 +53,7 @@
 #include unistd.h
 #include util.h
 #include login_cap.h
+#include syslog.h
 
 #include extern.h
 
@@ -72,6 +73,10 @@
 	strcmp(crypt(getpass(Old password:), pw-pw_passwd),
 	pw-pw_passwd)) {
 		errno = EACCES;
+		syslog(LOG_AUTH | LOG_NOTICE,
+		   user %s (UID %lu) failed to change the 
+		   local password of user %s: %m,
+		   pw-pw_name, (unsigned long)uid, pw-pw_name);
 		pw_error(NULL, 1, 1);
 	}
 
@@ -213,6 +218,11 @@
 
 	if (pw_mkdb(username, old_change == pw-pw_change)  0)
 		pw_error((char *)NULL, 0, 1);
+
+	syslog(LOG_AUTH | LOG_INFO,
+	   user %s (UID %lu) successfully changed 
+	   the local password of user %s,
+	   uid ? username : root, (unsigned long)uid, username);
 }
 
 #else /* ! USE_PAM */
@@ -319,6 +329,12 @@
 
 	if (pw_mkdb(uname, old_change == pw-pw_change)  0)
 		pw_error((char *)NULL, 0, 1);
+
+	syslog(LOG_AUTH | LOG_INFO,
+	   user %s (UID %lu) successfully changed 
+	   the local password of user %s,
+	   uid ? uname : root, (unsigned long)uid, uname);
+
 	return (0);
 }
 

Index: src/usr.bin/passwd/pam_passwd.c
diff -u src/usr.bin/passwd/pam_passwd.c:1.4 src/usr.bin/passwd/pam_passwd.c:1.5
--- src/usr.bin/passwd/pam_passwd.c:1.4	Sun May  6 09:19:44 2007
+++ src/usr.bin/passwd/pam_passwd.c	Tue Mar  2 16:19:13 2010
@@ -1,4 +1,4 @@
-/*	$NetBSD: pam_passwd.c,v 1.4 2007/05/06 09:19:44 jnemeth Exp $	*/
+/*	$NetBSD: pam_passwd.c,v 1.5 2010/03/02 16:19:13 gdt Exp $	*/
 
 /*-
  * Copyright (c) 2002 Networks Associates Technologies, Inc.
@@ -38,7 +38,7 @@
 #ifdef __FreeBSD__
 __FBSDID($FreeBSD: src/usr.bin/passwd/passwd.c,v 1.23 2003/04/18 21:27:09 nectar Exp $);
 #else
-__RCSID($NetBSD: pam_passwd.c,v 1.4 2007/05/06 09:19:44 jnemeth Exp $);
+__RCSID($NetBSD: pam_passwd.c,v 1.5 2010/03/02 16:19:13 gdt Exp $);
 #endif
 
 #include sys/param.h
@@ -75,6 +75,12 @@
 	int ch, pam_err;
 	char hostname[MAXHOSTNAMELEN + 1];
 
+	/* details about the invoking user for logging */
+	const uid_t i_uid = getuid();
+	const struct passwd *const i_pwd = getpwuid(i_uid);
+	const char *const i_username = (i_pwd  i_pwd-pw_name)
+		? i_pwd-pw_name : (null);
+
 	while ((ch = getopt(argc, argv, )) != -1) {
 		switch (ch) {
 		default:
@@ -116,9 +122,22 @@
 
 	/* set new password */
 	pam_err = pam_chauthtok(pamh, 0);
-	if (pam_err != PAM_SUCCESS)
+	if (pam_err != PAM_SUCCESS) {
+		if (pam_err == PAM_PERM_DENIED) {
+			syslog(LOG_AUTH | LOG_NOTICE,
+			   user %s (UID %lu) failed to change the 
+			   PAM authentication token of user %s: %s,
+			   i_username, (unsigned long)i_uid, username,
+			   pam_strerror(pamh, pam_err));
+		}
 		printf(Unable to change auth token: %s\n,
 		pam_strerror(pamh, pam_err));
+	} else {
+		syslog(LOG_AUTH | LOG_INFO,
+		   user %s (UID %lu) successfully changed the 
+		   PAM authentication token of user %s,
+		   i_username, (unsigned long)i_uid, username);
+	}
 
  end:
 	pam_end(pamh, pam_err);

CVS commit: src/usr.bin/passwd

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Tue Mar  2 16:19:13 UTC 2010

Modified Files:
src/usr.bin/passwd: local_passwd.c pam_passwd.c

Log Message:
Log successful and unsuccessful attempts to change passwords, via -l
or pam, to ease IT audit guideline compliance.  Patch from Richard
Hansen of BBN in private mail.

Proposed on tech-kern with positive comments, except a suggestion I
didn't implement:

A possible future enhancement is refraining from logging if the old
password is empty, as some people abort password changing that way.
However, it's not clear if this complies with most guidelines that
require password change logging, and at first glance that appears to
be a fairly difficult change.


To generate a diff of this commit:
cvs rdiff -u -r1.33 -r1.34 src/usr.bin/passwd/local_passwd.c
cvs rdiff -u -r1.4 -r1.5 src/usr.bin/passwd/pam_passwd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/sys/net

[Greg Troxel]

Roy Marples r...@netbsd.org writes:

 Module Name:  src
 Committed By: roy
 Date: Fri Nov 12 16:30:27 UTC 2010

 Modified Files:
   src/sys/net: link_proto.c route.h rtsock.c

 Log Message:
 Add RTM_CHGADDR to signal that an address on the interface has changed.
 This is mainly used for notifying userland about active link address changes.

Is there an IETF draft or some other precedent in BSD for this?  I'm not
objecting, but I haven't seen any discussion, and there were no
documentation files changed.


pgpNPSfYnGRrv.pgp
Description: PGP signature

Re: CVS commit: src/lib/libc/gen

[Greg Troxel]

David Laight da...@l8s.co.uk writes:

 On Sun, Mar 13, 2011 at 07:40:45AM +, matthew green wrote:
  src/lib/libc/gen: unvis.c
 
 Log Message:
 cast ~0 to (size_t) when passing to a size_t taking function.
 fixes lint build errors.

 Is that right? My C promotion rules are getting rusty...
 ~0 is of type 'int', 'size_t' is an unsigned type, if 'size_t' is
 larger than 'int' isn't the convertion 'value preserving' rather
 than 'sign preserving'?
 So you need ~(size_t)0 if you want to generate the all 1's bit pattern?

That sounds right to me, and if all-1s size_t means something, perhaps
there should be a #define.


pgpv2LNWvhpkO.pgp
Description: PGP signature

Re: CVS commit: src/etc/skel

[Greg Troxel]

Jared McNeill jmcne...@invisible.ca writes:

 Please restore EDITOR=vi, otherwise the following message is going to
 haunt me for the rest of time:

 $ svn commit
 svn: Commit failed (details follow):
 svn: Could not use external editor to fetch log message; consider
 setting the $SVN_EDITOR environment variable or using the --message
 (-m) or --file (-F) options
 svn: None of the environment variables SVN_EDITOR, VISUAL or EDITOR
 are set, and no 'editor-cmd' run-time configuration option was found


I thought the historical default was that EDITOR should be ex, and
VISUAL vi.  But again this is preference; others like emacs.  So you're
really asking that the default for all users be set the way you like it,
instead of making your own choices and setting them.  To me this is an
argument for not having these values set in global default files.


pgpvv4xnkvShB.pgp
Description: PGP signature

Re: CVS commit: src/distrib/utils/sysinst

[Greg Troxel]

  If TNF can provide a open resolver on a stable IPv6 address for use
  by sysinst, that'd be great.

Do we need only a resolver that answers for things in netbsd.org, to
find mirrors, or really a full resolver?


pgpkxsOKL8CZH.pgp
Description: PGP signature

Re: CVS commit: src/sys/arch

[Greg Troxel]

David Holland dholland-sourcechan...@netbsd.org writes:

 On Fri, Jan 06, 2012 at 03:15:28PM +, Cherry G. Mathew wrote:
   Modified Files:
  src/sys/arch/x86/x86: pmap.c
  src/sys/arch/xen/x86: cpu.c
   
   Log Message:
   Address those pesky DIAGNOSTIC messages. \n
   Take a performance hit at fork() for not DTRT. \n
   Note: Only applicable for kernels built with options DIAGNOSTIC \n
  
   [...]
   +#ifdef DIAGNOSTIC
   +   pmap_kremove(object, PAGE_SIZE);
   +#endif /* DIAGNOSTIC */
   [plus two more like that]

 Uh... even if that's correct, which doesn't seem too likely on the
 surface of things, it doesn't seem desirable.

I'm not sure if it's what David meant, but it seems wrong to have
different behavior based on DIAGNOSTIC.  I think DIAGNOSTIC is supposed
to be about enabling inexpensive consistency checks, only.  So if it's
necessary to have consistency to have that call, it should be
unconditional.  And if not, it shouldn't exist in any case.


Do you understand precisely what's going on?  Is this about omitting
steps necessary for consistency, but in the case where the new state
will be immediately discarded?



pgp5Nd9a0VKjT.pgp
Description: PGP signature

Re: CVS commit: src/lib/libc

[Greg Troxel]

  Code that is using gets(3) is almost always broken. It doesn't just

No argument there, but that's not the question.   The questions are

  should NetBSD provide a compliant C99 environment?  (obviously yes)

  is moving gets(3) to libcompat consistent with the above (it seems
  not, since your goal seems to be to force other people to make changes
  because you don't like what they are doing)

  should NetBSD be in the business of making it difficult to do things
  which are bad practice?   (Here I would say warnings are fine, and
  that standards compliance is more important.)


pgpedoZHN3TCz.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev/sdmmc

[Greg Troxel]

  Modified Files:
  src/sys/dev/sdmmc: sdhc.c

  Log Message:
  Handle interrupt acknowledgement in the SDHC_FLAG_32BIT_ACCESS case in
  the same way as non-SDHC_FLAG_32BIT_ACCESS case.

  To generate a diff of this commit:
  cvs rdiff -u -r1.20 -r1.21 src/sys/dev/sdmmc/sdhc.c

Two questions, one of which isn't about your change.

It seems that the HREAD4 of SDHC_NINTR_STATUS is reading both that and
SDHC_EINTR_STATUS, but it seems odd that those are addresses 0x30 and
0x32, which don't seem 4-byte aligned.  So is it really ok to do an
HREAD4 across boundaries like that?  It seems odd, but maybe it's about
the bus and the chip is fine with it.

In the non-32 case, it seems that the EINTR register is read and then
written back exactly if the error bit is set in the NINTR register.  In
the 32 case, it seems that the SDHC_ERROR_INTERRUPT bit is set in NINTR
if any bits are set in EINTR, in addition to writing both.

So while I see the point that the |= into status is effectively a dead
store,

  it seems odd to have bits set in SDHC_EINTR_STATUS without
  SDHC_ERROR_INTERRUPT set in SDHC_NINTR_STATUS, and

  the two code paths seem different still


I'm curious if you've been seeing bits in EINTR without ERROR in NINTR,
and what the symptoms are that provoked this fix - I have a system with
an sdhc (on evbppc/P2020) that mostly works but has occasional write
errors.

Thanks,
Greg


pgpnAXFg7lo2L.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev/sdmmc

[Greg Troxel]

On Jul 17, 2012, at 0925 , Matt Thomas wrote:

 
 On Jul 17, 2012, at 5:39 AM, Greg Troxel wrote:
 
 
 
 0x30 is aligned and that is the address that is read from a 32-bit access.

Not enough coffee - I was reading hex as decimal :-)

 In the non-32 case, it seems that the EINTR register is read and then
 written back exactly if the error bit is set in the NINTR register.  In
 the 32 case, it seems that the SDHC_ERROR_INTERRUPT bit is set in NINTR
 if any bits are set in EINTR, in addition to writing both.
 
 So while I see the point that the |= into status is effectively a dead
 store,
 
 it seems odd to have bits set in SDHC_EINTR_STATUS without
 SDHC_ERROR_INTERRUPT set in SDHC_NINTR_STATUS, and
 the two code paths seem different still
 
 This is because the ESDHC doesn't set SDHC_ERROR_INTERRUPT in its
 register since you've read SDHC_EINTR_STATUS and can see those bits
 directly.  So, for the sdhc driver, it needs to be emulated.

So the 4-byte read causes the SDHC_ERROR_INTERRUPT not to get set because
the same read reads EINTR_STATUS, or the chip that's in systems that need
4-byte reads is different (ESDHC vs SDHC?)?

 I'm curious if you've been seeing bits in EINTR without ERROR in NINTR,
 and what the symptoms are that provoked this fix - I have a system with
 an sdhc (on evbppc/P2020) that mostly works but has occasional write
 errors.
 
 That is exactly what the ESDHC on the P2020 does.

Thanks; that could be our issue then.

Re: CVS commit: src/sys/dev/sdmmc

[Greg Troxel]

it seems odd to have bits set in SDHC_EINTR_STATUS without
SDHC_ERROR_INTERRUPT set in SDHC_NINTR_STATUS, and
the two code paths seem different still

  This is because the ESDHC doesn't set SDHC_ERROR_INTERRUPT in its
  register since you've read SDHC_EINTR_STATUS and can see those bits
  directly.  So, for the sdhc driver, it needs to be emulated.

Don't we need to look at the error bits and take that into account
before the continue?  As I read the code below, if SDHC_NINTR_STATUS
doesn't have SDHC_ERROR_INTERRUPT set, but some bits are set in
SDHC_EINTR_STATUS, then the continue (none for us) will still be taken.
Don't we still need to set the bit in status before that test?  Isn't
that what the code did before?  If I understand, the bug was not putting
SDHC_ERROR_INTERRUPT into xstatus for write-back/ack, and how the
synthetic SDHC_ERROR_INTERRUPT was generated for local consumption was
ok?  (Sorry if I'm being dense - I'm finding this all a little hard to
follow.)

uint32_t xstatus = HREAD4(hp, SDHC_NINTR_STATUS);
status = xstatus;
error = xstatus  16;
if (!ISSET(status, SDHC_NINTR_STATUS_MASK))
continue; /* no interrupt for us */
/* Acknowledge the interrupts we are about to handle. */
xstatus |= (error ? SDHC_ERROR_INTERRUPT : 0);
HWRITE4(hp, SDHC_NINTR_STATUS, xstatus);


pgp6bMiZ67Qzv.pgp
Description: PGP signature

Re: CVS commit: src/external/public-domain/sqlite/dist

[Greg Troxel]

chris...@astron.com (Christos Zoulas) writes:

 In article 20121216202507.ga21...@britannica.bec.de,
 Joerg Sonnenberger  jo...@britannica.bec.de wrote:

Can you please not add random crappy files with unclear license?

 Every other distribution that contains sqlite3 (including commercial ones
 like MacOS/X), distributes this exact manpage, and we are going to be
 different? What is unclear about:

   This manual page was originally written by Andreas Rottmann
   ro...@debian.org, for the Debian GNU/Linux system (but may be used by
   others). It was subsequently revised by Bill Bumgarner b...@mac.com.

 If the license changes, we can always use this free copy or decide to remove
 it.

That is indeed unclear.  No permission is given to copy, distribute or
create derivative works.  One can imply that it's intended to be a free
license because it was for Debian, and because Andreas is known to be an
entirely reasonable member of the free software community.  But that's
implication, not reading the text of a license that has been blessed by
OSI or FSF.

However, it is easy enough to write to the authors and ask for
permission under CC0 (as would be appropriate for sqlite) or some other
non-copyleft free license.


pgpuJib3ABv57.pgp
Description: PGP signature

Re: CVS commit: src/sbin/ping

[Greg Troxel]

  3. Always send ICMP_MINLEN packets; this is what everyone else does. Makes
 ping -s n where n  8 work.

Shouldn't that be an error?  Otherwise people will think that something
is happening when it isn't.


pgpsqNepA4SMC.pgp
Description: PGP signature

Re: CVS commit: src/usr.sbin/mtree

[Greg Troxel]

chris...@zoulas.com (Christos Zoulas) writes:

 On Jan 2, 10:19am, mar...@homeworld.netbsd.org (Martin Husemann) wrote:
 -- Subject: Re: CVS commit: src/usr.sbin/mtree

 | On Thu, Dec 20, 2012 at 11:43:17AM -0500, Christos Zoulas wrote:
 |  Module Name:  src
 |  Committed By: christos
 |  Date: Thu Dec 20 16:43:17 UTC 2012
 |  
 |  Modified Files:
 |src/usr.sbin/mtree: create.c extern.h mtree.8 mtree.c spec.c
 |  
 |  Log Message:
 |  Implement the flavor output discussed in tech-userlevel@, by Brooks 
 Davis
 | This broke the mtree atf tests. Output now differs by additional size
 | values for symlinks and directories:
 | 
 | -2   type=dir nlink=2
 | +2   type=dir nlink=2 size=0
 | 
 | See for example:
 | 
 http://www.netbsd.org/~martin/sparc64-atf/147_atf.html#usr.sbin_mtree_t_mtree_create

 Yes, we agreed to change the format for the default and if you want
 the backwards
 compatible one, we you need to say netbsd flavor.

Then it seems that the tests should have been changed in the same
commit, and new tests added for netbsd flavor!


pgpwIjxnuv4aW.pgp
Description: PGP signature

Re: CVS commit: src/sbin/ping

[Greg Troxel]

chris...@zoulas.com (Christos Zoulas) writes:

 ping -s 1 localhost

 linux:
 PING localhost (127.0.0.1) 1(29) bytes of data.
 9 bytes from localhost (127.0.0.1): icmp_seq=1 ttl=64

so linux sends n bytes more than the header.
 macosx
 PING localhost (127.0.0.1): 1 data bytes
 9 bytes from 127.0.0.1: icmp_seq=0 ttl=64

as does macosx

 netbsd-current
 PING localhost (127.0.0.1): 1 data bytes
 8 bytes from 127.0.0.1: icmp_seq=0 ttl=255

The man page says:

 -s packetsize
 Specifies the number of data bytes to be sent.  The default is
 56, which translates into 64 ICMP data bytes when combined with
 the 8 bytes of ICMP header data.  The maximum allowed value is
 65467 bytes.

so clearly -s 0 should lead to 8-byte packets, and -s 56 to 64-byte
packets.  Then we would match the man page, NetBSD 5_STABLE, Linux and
mac.


pgphta0ZAO7qe.pgp
Description: PGP signature

Re: CVS commit: src/sbin/ping

[Greg Troxel]

I am not understanding why '-s argument is the number of data bytes
beyond the standard 8-byte icmp header' is complicated.   People who use
-s are trying to control packet size, and the rule of 8 header + data
seems to be longstanding.

Looking at -current:

  the size of the timespec should be given, and it's endianness.

  the man page for -C should say that these compat timestamps are
  instead of the timespec.

so 

ping -s 8: should send 16-byte packets in compat format
ping -s 16: should send 24-byte packets in new format

It's at best odd to have the packet length change for a given -s
argument because of timestamp format.  This points out that the original
decision should have been '-s X means X bytes of IP payload', but that's
not what was decided, and it's easy enough to stick with 'X bytes beyond
the 8-byte ICMP header'.

  [10:58am] 2277ping  quasar
  PING quasar.astron.com (192.168.2.4): 48 data bytes
  64 bytes from 192.168.2.4: icmp_seq=0 ttl=255 time=0.027 ms

That's a bug.  All bytes after the 8-byte header are data bytes, even if
some of them have timestamps.


pgpaKLBiDpb_j.pgp
Description: PGP signature

Re: CVS commit: src/sys/netinet

[Greg Troxel]

Christos Zoulas chris...@netbsd.org writes:

 Module Name:  src
 Committed By: christos
 Date: Tue Mar 12 21:54:36 UTC 2013

 Modified Files:
   src/sys/netinet: in4_cksum.c

 Log Message:
 - Don't panic on short packets unless DIAGNOSTIC. In general we should try
   to make the kernel survive errors...

This seems odd.  Either there's an invariant that there can't be a short
packet, or there isn't.  DIAGNOSTIC is basically supposed to add checks
for invariants, but that's not about not crashing on not DIAGNOSTIC but
avoiding the run-time costs of the checks.
So is a short packet a can't happen case?


pgp6M39ybliFu.pgp
Description: PGP signature

Re: CVS commit: src/tests/fs/common

[Greg Troxel]

Nicolas Joly nj...@pasteur.fr writes:

 The zfs tests are failing when run as an unprivileged user in
 thread_create() ... because default limit for threads (160) is too
 low ! When run as root, the limit value (2500) seems to be high enough.

So should we increase the default limits?  It's always a tough call to
set limits for a large ensemble of hardware, big and small, but it also
seems that limits should be big enough to run our test cases.



pgpmmHx2QH_Pf.pgp
Description: PGP signature

Re: CVS commit: src/sys/netinet6

[Greg Troxel]

David Laight da...@l8s.co.uk writes:

 On Mon, Mar 18, 2013 at 07:31:39PM +, Greg Troxel wrote:
 Module Name: src
 Committed By:gdt
 Date:Mon Mar 18 19:31:39 UTC 2013
 
 Modified Files:
  src/sys/netinet6: ip6_output.c
 
 Log Message:
 Initialize variable used as (conditional) result parameter.
 
 ip6_insertfraghdr either sets a result parameter or returns an error.
 While the caller only uses the result parameter in the non-error case,
 knowing that requires cross-module static analysis, and that's not
 robust against distant code changes.  Therfore, set ip6f to NULL
 before the function call that maybe sets it, avoiding a spuruious
 warning and changing the future possible bug from an unitialized
 dereference to a NULL deferrence.

 'If it returns fail it hasn't changed anything' is quite a common
 property of functions. In fact I'd tend to expect it.

Sure, but the property that's needed is the trickier one: being sure
that a function that doesn't return an error must have set a result
parameter.

 Cross module analysis isn't really a big issue, the actual problem
 is when a compiler does a deeper analysis of a local function and
 fails to spot the relationship.

That may actually be the issue; I had a hard time convincing myself that
ip6_insertfraghdr follows it's own invariant.  The problem shows up if
one adds IPSEC to NET4501.

So if the compiler is sure, then the new NULL is a dead store and will
be optimized out.


pgpjjczgZWuoy.pgp
Description: PGP signature

Re: CVS commit: src/sys

[Greg Troxel]

Jukka Ruohonen jruoho...@iki.fi writes:

 On Wed, Jun 05, 2013 at 10:24:34AM -0400, Greg Troxel wrote:
 INET is really INET4.

 Sure; but see below.

 #ifdef INET
   ...
 #ifdef INET6
 
 That's a bug; in theory one should be able to have INET6 without INET.
 I did try it once several years ago, and had some trouble that I didn't
 solve.

 Not a single instance. In any case, these again cast a shadow on the
 usefulness or quality of some of the #ifdefs, which are perhaps not even
 expected to work... or at least contribute to the bitrot that christos also
 noted in the commit log.

I'm not sure what you're trying to get it.  It's clear that there are
ifdef bugs, but there may very well not be that many.  I suspect there
is some code that is needed for INET or INET6 but is only compiled if
INET is defined.

As for bitrot, it wasn't clear that there actually was any, just a
concern that it might be there.  I have been compiling with IPSEC_NAT_T
enabled on -5, -6 and -current or a long time, and my -current builds do
not seem to break more than a few times a week ;-) But seriously, I have
not found any build problems in the NAT_T code.

The INET and INET6 conditionals do not contribute much to the code
bitrot problem, unless there is code that is enabled when they aren't
defined and that code has issues.  I expect that it's fairly commnon to
omit INET6, so we're really talking about !INET.

The real decision is how many conditional options to have.  The
IPSEC_NAT_T code is small, and NAT is regrettably common, so that seems
reasoable to fold into IPSEC.  But I don't think we should decide to
make INET mandatory.  (Of course, I don't expect you or anyone else to
fix these bugs unless you are in the mood.)


pgpBTysglnFd6.pgp
Description: PGP signature

Re: CVS commit: src/sys/netinet

[Greg Troxel]

Christos Zoulas chris...@netbsd.org writes:

 Module Name:  src
 Committed By: christos
 Date: Wed Jun  5 00:48:32 UTC 2013

 Modified Files:
   src/sys/netinet: udp_usrreq.c

 Log Message:
 conditionalize the net traversal code on FAST_IPSEC to make rump build.

FAST_IPSEC does not appear in -current; it's just IPSEC.  Does adding
ifdef FAST_IPSEC really make a full release build work?

The previous commit should have been two - correctness fixes are
candidates for pulling up to -6, and rototilling ifdefs are not.

(While here, do X is always best avoided, in my opinion.)

Re: CVS commit: src/sys

[Greg Troxel]

Christos Zoulas chris...@netbsd.org writes:

 Module Name:  src
 Committed By: christos
 Date: Wed Jun  5 19:01:26 UTC 2013

 Modified Files:
   src/sys/kern: init_main.c
   src/sys/netinet: in_pcb.c in_proto.c ip_icmp.c ip_input.c ip_mroute.c
   ip_output.c raw_ip.c tcp_input.c tcp_output.c tcp_subr.c
   udp_usrreq.c
   src/sys/netinet6: icmp6.c in6_pcb.c in6_proto.c ip6_forward.c
   ip6_input.c ip6_output.c raw_ip6.c
   src/sys/netipsec: files.netipsec key.c xform_ipip.c

 Log Message:
 IPSEC has not come in two speeds for a long time now (IPSEC == kame,
 FAST_IPSEC). Make everything refer to IPSEC to avoid confusion.

Besides s/FAST_IPSEC/IPSEC/g (fine) and some whitespace cleanup, I
found:

Index: src/sys/netinet/tcp_input.c
diff -u src/sys/netinet/tcp_input.c:1.325 src/sys/netinet/tcp_input.c:1.326
--- src/sys/netinet/tcp_input.c:1.325   Fri Jun 22 15:09:36 2012
+++ src/sys/netinet/tcp_input.c Wed Jun  5 19:01:26 2013
@@ -3421,11 +3417,7 @@
tcp_fields_to_host(th);
if (sav == NULL)
return (-1);
-#ifdef FAST_IPSEC
-   KEY_FREESAV(sav);
-#else
-   key_freesav(sav);
-#endif
+   KEY_FREESAV(sav);
return (-1);
}
tcp_fields_to_host(th);

which looks like either a logic bug or something that won't build (note
disappearing in argument to KEY_FREESAV.  Did this pass a build.sh
release?


pgp5iMEjNJMML.pgp
Description: PGP signature

Re: CVS commit: src/bin/hostname

[Greg Troxel]

Erik Fair f...@netbsd.org writes:

 On Jul 19, 2013, at 03:34, Roy Marples r...@netbsd.org wrote:

 Module Name: src
 Committed By:roy
 Date:Fri Jul 19 10:34:51 UTC 2013
 
 Modified Files:
  src/bin/hostname: hostname.1 hostname.c
 
 Log Message:
 Add the following options
 -A Display the FQDN of each address on all interfaces.
 -a Display alias name(s) of the host.
 -d Display the DNS domain.
 -f Display the FQDN for the hostname.
 -I Display each IP address on all interfaces.
 -i Display the IP address(es) for the hostname.
 

 Not to go all Rob Pike on you (cf. cat -v considered harmful), but
 what the heck is all this for? The system's hostname is supposed to be
 the FQDN, not the short form (Sun got this wrong), and what the hell
 is hostname doing groveling around in network interfaces? Or talking
 to the DNS?

 hostname(1) has one job: set/get the system hostname.

 Does some other (*cough* Linux) system do these other things that we
 maybe might need to be … compatible with it for scripts?

Good points and I'm curious too.  I've always viewed hostname(1) to be a
thin wrapper about {get,set}hostname, and *not connected at all* to the
IP networking stack.

Also, IP means 4, or 6, or both, and what about other protocols?


pgpSiod5Ukky7.pgp
Description: PGP signature

Re: CVS commit: src/bin/csh

[Greg Troxel]

Alan Barrett a...@cequrux.com writes:

 On Tue, 06 Aug 2013, Christos Zoulas wrote:
Module Name:  src
Committed By: christos
Date: Tue Aug  6 05:42:43 UTC 2013

Modified Files:
  src/bin/csh: lex.c

Log Message:
CID 1060854: Wrong sizeof argument (SIZEOF_MISMATCH)

 Does everybody know what CID means?  I'd be inclined to say
 Coverity CID  instead of just CID  in these log messages.

 --apb (Alan Barrett)

Also, the CIDs don't seem to be a stable hash of the bug :-), so really
this is an id from a particular instatiation of the tool, and a name
for that instatiation belongs too.


pgpN9UT2xrBi8.pgp
Description: PGP signature

Re: CVS commit: src/etc

[Greg Troxel]

Alan Barrett a...@netbsd.org writes:

 If you have restrict default nopeer noquery (the uncommented line in
 my commit), then time service will still work, but the configured
 servers will be denied query permission.

 If you use restrict default ignore, then time service does not work.

I have found the ntp restrict situation very confusing.  I think that
all we need to do is something like:

restrict default noquery nomodify notrap
restrict -6 default noquery nomodify notrap
restrict 127.0.0.1
restrict -6 ::1

and leave it at that.  The real issue is amplification via monlist.  I
don't understand the apparent leap from that to almost completely
firewalling ntp.

Why do you think the configured servers should be given query
permission?  Is that a sense of courtesy to the pool operators that they
should be able to run ntpdc -c monlist and ntpq -p at machines that
are syncing from them?


pgpfSjBwi1PQ5.pgp
Description: PGP signature

Re: CVS commit: src/share/mk

[Greg Troxel]

chris...@zoulas.com (Christos Zoulas) writes:

 On Jan 22,  7:29am, m...@3am-software.com (Matt Thomas) wrote:
 -- Subject: Re: CVS commit: src/share/mk

 | I always wondered why we don't use ln -sf=20
 | and avoid the race.

 That does not work because if the destnation is a directory it will
 try to link in the destination directory... (I tried). This is why
 I suggested that it needs to be done differently.

I feel like I'm completely missing something.  A make rule is triggered,
and it does something.  There are not in general requirements for make
rules to be atomic, and it seems like the problem is parallelism in make
which is improper, not the rule.


pgpT7UAvMAynh.pgp
Description: PGP signature

Re: CVS commit: src/sys/miscfs/genfs

[Greg Troxel]

Taylor R Campbell campbell+netbsd-source-change...@mumble.net writes:

Date: Wed, 12 Mar 2014 16:16:32 +0200
From: Jukka Ruohonen jruoho...@iki.fi

On Wed, Mar 12, 2014 at 09:39:23AM +, Juergen Hannken-Illjes wrote:
 Restructure layer_lock() to always lock before testing for dead node.
 Use ISSET() to test flags, add assertions.

As I wrote in the manual page, I'd rather see ISSET(3) et. al. disappear,
i.e. these obscure rather than clarify...

 I disagree.  Phrases like `(vp-v_iflag  (VI_XLOCK | VI_CLEAN)) == 0'
 make my head's parser stumble -- there are just enough complements to
 juggle that it overwhelms my brain registers for the fast path.  I'd
 rather read `!ISSET(vp-v_iflag, (VI_XLOCK | VI_CLEAN))'.

FWIW, I agree with Taylor.


pgpSzcZOop3yS.pgp
Description: PGP signature

Re: CVS commit: src/sys/miscfs/genfs

[Greg Troxel]

Paul Goyette p...@whooppee.com writes:

 Me, too. But I'd rather that we had the equivalent ISCLR() macro, too,
 to remove another negation/complement.

So is ISCLR when passed two bits true if both are clear, or if it's just
not the case that both are set?

Arguably this points out that the ISSET docs should explain about using
2 bits at once.


pgpZNQF7rHjXV.pgp
Description: PGP signature

Re: CVS commit: src/sys/arch/amd64/conf

[Greg Troxel]

John Nemeth jnem...@cue.bc.ca writes:

 On Mar 15,  1:50pm, Jonathan A. Kollasch wrote:
 } 
 } Module Name:src
 } Committed By:   jakllsch
 } Date:   Sat Mar 15 13:50:01 UTC 2014
 } 
 } Modified Files:
 } src/sys/arch/amd64/conf: XEN3_DOMU
 } 
 } Log Message:
 } Enable PCI support in amd64 XEN3_DOMU config to match i386 XEN3_DOMU config.

  PCI support in a DOMU kernel would only be useful when the
 DOM0 is setup with pci passthrough for some device, which isn't
 the norm.  My answer would be to go the opposite way and disable
 PCI support in both kernels.  Also, simply enabling PCI support
 without adding a whole schwak of PCI devices isn't very useful.
 As well, I don't think PCI passthrough is working with modern
 versions of Xen.

John's comments seem sensible to me.  Also, I find using PCI passthrough
to be very unusual.


pgpt4NlQQp6MV.pgp
Description: PGP signature

Re: CVS commit: src/sys/arch/amd64/conf

[Greg Troxel]

John Nemeth jnem...@cue.bc.ca writes:

 On Mar 15,  1:50pm, Jonathan A. Kollasch wrote:
 } 
 } Module Name:src
 } Committed By:   jakllsch
 } Date:   Sat Mar 15 13:50:01 UTC 2014
 } 
 } Modified Files:
 } src/sys/arch/amd64/conf: XEN3_DOMU
 } 
 } Log Message:
 } Enable PCI support in amd64 XEN3_DOMU config to match i386 XEN3_DOMU config.

  PCI support in a DOMU kernel would only be useful when the
 DOM0 is setup with pci passthrough for some device, which isn't
 the norm.  My answer would be to go the opposite way and disable
 PCI support in both kernels.  Also, simply enabling PCI support
 without adding a whole schwak of PCI devices isn't very useful.
 As well, I don't think PCI passthrough is working with modern
 versions of Xen.

John's comments seem sensible to me.  Also, I find using PCI passthrough
to be very unusual.


pgp8mWlUycFXS.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev/raidframe

[Greg Troxel]

I seemed to have missed the discussion for this change.

It seems there are 3 behaviors for root

  1) don't change the root device (old behavior with -A yes)

  2) if root is on a component, change root to the raid, otherwise don't
  change.

  3) force the root device to be the raid (old behavior with -A root)

It seems to me that the behavior 1 (not in case 2) isn't useful, and
that we could make behavior 2 the default behavior, with 3 with -A yes.

Is there any reason why someone would not want to use the raid for root
when a) the system booted from a component and b) it's marked
autoconfigurable?  If anything, I think there should be a way to disable
autoconfiguration.


pgpH2Q2GRCSi_.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev/raidframe

[Greg Troxel]

I think the most important thing is that people who have a system that
boots now (and aren't doing anything super sick) should continue to have
that system boot after they just replace the kernel.  And we should
discuss how to do this before the change is committed :-)

 On Apr 2, 10:33am, g...@ir.bbn.com (Greg Troxel) wrote:
 -- Subject: Re: CVS commit: src/sys/dev/raidframe

 | It seems there are 3 behaviors for root
 | 
 |   1) don't change the root device (old behavior with -A yes)

 That does autoconfig for raid and does not deal with root at all.

Right, but it is a way that some systems might be set up now.

 |   2) if root is on a component, change root to the raid, otherwise don't
 |   change.

 This behavior did not exist before my commit. It is now only turned on with
 -A root. 

 |   3) force the root device to be the raid (old behavior with -A root)

 This does not exist anymore. You can currently:

 1. boot -a
 2. make a kernel that has root on raidx

so this is the real concern, that the straightforward update will make
systems unbootable.

 Or:
 1. add the ability to pass the root name through the bootblocks/userconf
 2. add a raidctl -A forceroot and obey that.

 | It seems to me that the behavior 1 (not in case 2) isn't useful, and
 | that we could make behavior 2 the default behavior, with 3 with -A yes.

 Yes, I think you are right. There could be an issue with having wd0a being
 the real root and then a raid component on wd0e... In that case do we want
 to make wd0e the root device because it is on the same drive?

I don't think so.  The case that matters for automatically making the
raid root is when one has a RAID1 and the bootblocks boot off of half of
it.   So I think the narrow if the boot device is a component of an
autoconfigured raid, switch the boot device to that raid really is what
we want to do.

 | Is there any reason why someone would not want to use the raid for root
 | when a) the system booted from a component and b) it's marked
 | autoconfigurable?  If anything, I think there should be a way to disable
 | autoconfiguration.

 See above... There is a way to disable autoconfiguration, but there is
 currently no easy way to get the old behavior? What do you think we should
 do?

I guess the idea that you boot from half a raid and then you use root on
that half - doesn't make sense.  That breaks the raid set.  I think
people either set things up as (typical examples) one of the following
ways:

  boot a kernel from wd0a, which is nonraid, and have a RAID1 on
  wd0e/wd1e marked -A root.  Here, wd0a is merely as a rescue partition
  and a way to load the kernel.  This is what you used to have to do
  before RAID1 boot support, which means I think you still do have to do
  it on some platforms.

  have wd0a/wd1a be a RAID1.  Boot from wd0a (or wd1a) with boot blocks
  that skip over the raid header, and then -A root is used to make that
  the root device.

So the first way needs -A root to keep its current semantics.   The 2nd
way works with -A root, but I think it's fine to make it work with
merely -A yes.


pgpagpzE8AyBe.pgp
Description: PGP signature

Re: CVS commit: src/sys/dev/raidframe

[Greg Troxel]

chris...@zoulas.com (Christos Zoulas) writes:

 On Apr 3,  7:57am, m...@eterna.com.au (matthew green) wrote:
 -- Subject: re: CVS commit: src/sys/dev/raidframe

 | kernel configuration changes are not solutions, so 2 and 3 are out.  
 | 
 | if we do 4, we should instead add an option to mark something as a
 | 'soft root', and leave the current semantics alone.  the machines i
 | have that are now not going to reboot properly are both used
 | remotely, so changing semantics about how they work seems like a
 | bad idea.  i'm pretty sure i'm not the only one who does this.
 | i think i like this the best.

 Sure, we can add -A softroot. Do we want to rename the current option
 to -A hardroot? If that's the consensus, I can go ahead.

Why don't you just leave the current one alone, and not change the name?
The names only mean what the docs say, and -A root says and make this
root, period in the docs, which is not unsurprising for -A root.
Having -A softroot or -A condroot to mean make this root if the
existing root is a component sounds good.  This way the only people
that will see new behavior are those that configure -A softroot, and I
think that's a good goal.


pgpdgzG97Nfh6.pgp
Description: PGP signature

Re: CVS commit: src/sys/netinet

[Greg Troxel]

Erik Fair f...@netbsd.org writes:

 On Apr 12, 2014, at 05:24, Greg Troxel g...@netbsd.org wrote:

 Module Name: src
 Committed By:gdt
 Date:Sat Apr 12 12:24:50 UTC 2014
 
 Modified Files:
  src/sys/netinet: if_arp.c
 
 Log Message:
 revarprequest: Avoid leaking mbuf.
 
 In revarprequest, an mbuf could perhaps be leaked in an error path.
 My reading of the code is that this is not possible, because ar_pro is
 set to ETHERNET_IP, and ar_tha can only be null in the 1394 case.
 But, better to have the free call anyway; ar_tha does not have a
 documented interface contract :-)
 
 Pointed out by Maxime Villard.


 This should be pulled up to netbsd-6

I don't think so; I was able to convince myself by manual static
analysis that the leak condition could never happen.  So I think the
gain/effort ratio isn't high enough to make it worthwhile.  As I see it,
the fix is more about avoiding a future leak than fixing a current one.





pgpS0Z7aKtswx.pgp
Description: PGP signature

Re: CVS commit: src

[Greg Troxel]

Nick Hudson sk...@netbsd.org writes:

 On 07/25/14 17:13, Greg Troxel wrote:
 Module Name: src
 Committed By:gdt
 Date:Fri Jul 25 16:13:21 UTC 2014

 Modified Files:
  src/share/man/man4: ucom.4
  src/sys/dev/usb: ucom.c

 Log Message:
 Add PPS support to ucom(4).

 This is basically cribbed from regular serial ports, and just adds
 hooks to call the pps support routines.

 Also, note in the ucom(4) man page that there is about 1 ms of
 latency.  Discussed on tech-kern in October of 2013, with the only
 concern being that someone who didn't know what they were doing might
 set up a stratum 1 server, and that somehow might have worse
 timekeeping than whatever else that person might have done; the man
 page comment is a mitigation for this.

 latency of 1ms and the same jitter - only ucycom uses interrupt
 endpoints for data transfer

I was abstracting on purpose, because no other man page describes
performance characteristics in such detail.  (When I tested, my best
guess was that the delay from PPS edge capture ranged from 400us to
1400us as the USB frame clock shifted.)



pgpzewA67T2cg.pgp
Description: PGP signature

Re: CVS commit: src/sys

[Greg Troxel]

Manuel Bouyer  writes:

> On Mon, Oct 02, 2017 at 02:39:47PM +0200, Maxime Villard wrote:
>> > Actually I did suggest to make the default dependant on MODULAR.
>> 
>> what's the point exactly?
>
> that if I build a non-modular kernel with an emulation option explicitely
> selected, it works at boot. Even in single-user mode.

I think the real point is hidden behind this statement.

If I just run GENERIC, because I'm not paying attention to details, then
I am not "building a non-modular kernel with an emulation option
explicitly selected".  I'm accepting a default.

As I understand it, the real debate is about whether the distributed
GENERIC kernel will run Linux emulation by default, or whether it should
require some enabling of that emulation.   All the rest is details
flowing from that main decision.

Certainly there could be a kernel option to default the sysctl to on.
People who wnt to "explicitly select an emulation optoon" can turn on
that define, and have it working from boot.   Then, I think the debate
reduces to "should the checked-in GENERIC enable the emulation sysctl".


signature.asc
Description: PGP signature

Re: CVS commit: src/external

[Greg Troxel]

m...@netbsd.org writes:

> On Sun, Apr 08, 2018 at 04:57:07PM +, Jared D. McNeill wrote:
>> @@ -82,6 +82,10 @@ The licenses currently used are:
>>  mpl Mozilla Public license.
>>  https://opensource.org/licenses/MPL-2.0
>>  
>> +nvidia-firmware NVIDIA firmware license permitting redistribution for
>> +use on operating systems distributed under the terms
>> +of an OSI-approved open source license.
>> +
>>  public-domain   Non-license for code that has been explicitly put
>>  into the Public Domain.
>> 
>
> Can we talk to someone with legal expertise before importing code under
> this license?

One issue is that NetBSD's license permits proprietary derived works
(that's the BSD point).  But this really means that people doing that
have to drop the nvidia firmware, and probably other firmware (or get a
license).

To be amusingly pedantic, adding this means that NetBSD's license is
some combination of BSD, GPL (omitting details, MPL, etc. for brevity)
and the nvidia license.  However the nvidia license is not open source,
so NetBSD with nvidia fails to be OSI-approved.  But it seems unlikely
that NVIDIA means this interpretation.

Re: CVS commit: src/etc/etc.evbarm

[Greg Troxel]

"Herbert J. Skuhra"  writes:

> On Sun, 04 Nov 2018 22:41:12 +0100, "Nick Hudson" wrote:
>> 
>> Module Name: src
>> Committed By:skrll
>> Date:Sun Nov  4 21:41:12 UTC 2018
>> 
>> Modified Files:
>>  src/etc/etc.evbarm: Makefile.inc
>> 
>> Log Message:
>> Only add GENERIC to earmv6 and earmv7 builds
>
> This change obviously breaks "./build.sh -m evbarm -a earmv7hf -U release".

That seems like an unusual invocation.   Do you really mean it that way,
instead of -a evbarm?   Passing earmv7hf to -a is an alias which sets
both a amd m, and then the line is also setting m.Why do you include
'-m evbarm' - what cpu type are you trying to build for?

Re: CVS commit: src/etc/etc.evbarm

[Greg Troxel]

Greg Troxel  writes:

> "Herbert J. Skuhra"  writes:
>
>> On Sun, 04 Nov 2018 22:41:12 +0100, "Nick Hudson" wrote:
>>> 
>>> Module Name:src
>>> Committed By:   skrll
>>> Date:   Sun Nov  4 21:41:12 UTC 2018
>>> 
>>> Modified Files:
>>> src/etc/etc.evbarm: Makefile.inc
>>> 
>>> Log Message:
>>> Only add GENERIC to earmv6 and earmv7 builds
>>
>> This change obviously breaks "./build.sh -m evbarm -a earmv7hf -U release".
>
> That seems like an unusual invocation.   Do you really mean it that way,
> instead of -a evbarm?   Passing earmv7hf to -a is an alias which sets
> both a amd m, and then the line is also setting m.Why do you include
> '-m evbarm' - what cpu type are you trying to build for?

Sorry, I was confused and wrote from memory and got it wrong.  You are
entirely right about your invocation.

I just actually read build.sh and adjusted the wording in the wiki.

Re: CVS commit: src/sys/arch/evbarm/conf

[Greg Troxel]

matthew green  writes:

> Nick Hudson writes:
>> On 13/11/2018 08:21, matthew green wrote:
>> >> Modified Files:
>> >>   src/sys/arch/evbarm/conf: std.generic64
>> >>
>> >> Log Message:
>> >> turn on MODULAR by default on aarch64
>> > 
>> > optional things should not be in "std.foo".  that should be
>> > things that are necessary for basic function.  stuff that
>> > a user would never want to remove.
>> 
>> I thought core wanted MODULAR everywhere? If so, it's in the right 
>> place, I think.
>
> it's still "optional", even if we want it default everywhere.
> std.foo is for things that are not optional, that all users
> should have, no matter what choices they have.
>
> i should never have to look in std.foo to decide if an option
> should be removed for my config or not.

Agreed on both points.

Re: CVS commit: src/sys

[Greg Troxel]

Kamil Rytarowski  writes:

> On 08.05.2019 11:34, Ryota Ozaki wrote:
>> On Sat, Apr 20, 2019 at 6:45 PM Ryota Ozaki  wrote:
>>>
>>> On Fri, Apr 19, 2019 at 6:49 PM Kamil Rytarowski  wrote:

 On 19.04.2019 11:41, J. Hannken-Illjes wrote:
>> On 19. Apr 2019, at 03:52, Ryota Ozaki  wrote:
>>
>> Module Name: src
>> Committed By:ozaki-r
>> Date:Fri Apr 19 01:52:56 UTC 2019
>>
>> Modified Files:
>>  src/sys/kern: kern_lwp.c kern_softint.c subr_psref.c
>>  src/sys/rump/kern/lib/libsysproxy: sysproxy.c
>>  src/sys/sys: lwp.h userret.h
>>
>> Log Message:
>> Implement a simple psref leak detector
>>
>> It detects leaks by counting up the number of held psref by an LWP and 
>> checking
>> its zeroness at the end of syscalls and softint handlers.  For the 
>> counter, a
>> unused field of struct lwp is reused.
>
> For DIAGNOSTIC-only operations LWP specific data
> (see kern/subr_lwp_specificdata.c) is a better choice.
>

 I wanted to propose the same. An exampe of this is in KCOV.
>>>
>>> Thanks.  I'll try it.  (I'll be AFK for the next few days...)
>> 
>> I'm sorry for delaying this task.  Finally I have benchmarked a revised patch
>> (our benchmarking setups have been out of service for a couple of weeks...).
>> 
>> Performance degradation of IP forwarding is 3%.  Is it acceptable as
>> DIAGNOSTIC?
>> 
>
> For DIAGNOSTIC should be fine.

I think 3% is too much for DIAGNOSTIC.   DEBUG, sure, and a specific
option to turn it on seems fine.  DIAGNOSTIC historically has been only
for things that check invariants and assert, such that if you don't mind
the crashes when detecting things, there is no performance reason to
avoid it.

Re: CVS commit: src/sys

[Greg Troxel]

Ryota Ozaki  writes:

> On Sat, May 11, 2019 at 10:49 AM Greg Troxel  wrote:
>> >> I'm sorry for delaying this task.  Finally I have benchmarked a revised 
>> >> patch
>> >> (our benchmarking setups have been out of service for a couple of 
>> >> weeks...).
>> >>
>> >> Performance degradation of IP forwarding is 3%.  Is it acceptable as
>> >> DIAGNOSTIC?
>> >
>> > For DIAGNOSTIC should be fine.
>>
>> I think 3% is too much for DIAGNOSTIC.   DEBUG, sure, and a specific
>> option to turn it on seems fine.  DIAGNOSTIC historically has been only
>> for things that check invariants and assert, such that if you don't mind
>> the crashes when detecting things, there is no performance reason to
>> avoid it.
>
> So we have two options:
> 1. Keep the original (enabled on DIAGNOSTIC)
>   - Its performance impact is negligible
> 2. Migrate to use lwp_specificadata and enable the feature on DEBUG
> (and something)

I am not following two things:

1) You said that IP forwarding speed is reduced by 3%, and also that it
is a negligble performance impact.  In my view, 3% is not negligible.
DIAGNOSTIC should be sufficiently small performance impact that
essentially nobody wants to disable it to get better performance.  I
could see 0.3% as negligible.  But everything adds up, so it's not just
this, but the overall system with and without DIAGNOSTIC.

2) Your option 2 seems to involve two things at once:

  - migration to lwp_specificadata
  - using DEBUG instead of DIAGNOSTIC to control the leak check feature

I do not understand why changing the nature of the implementation is
linked to how it is enabled.

Overall, I think checking features (other than simple KASSSERTs) should
have their own ifdef, so people can individually enable them, and then
additionally there would be "#ifdef DEBUG // #define PSREFLEAKDETECT" or
something like that.

> I prefer to 1. because I want the feature to be enabled by many users

Sure, but many users prefer that their systems not be slowed down, too.
If we add 10 more checks that each cause 3% slowdown, then we have a
real problem.  DIAGNOSTIC has always had the sense that it should turn
undefined behavior into a panic, but not slow the system down, so that
the only reason to avoid it is not liking the panics.

> (I assume that users (of -current) tend to enable DIAGNOSTIC and not DEBUG).

On -current, DIAGNOSTIC is on by default.  When we branch 9, I expect
that it will remain on until very close to release.

I agree that most people do not have DEBUG on.

> If the detector is not enabled, a psref leak appears as a form that is
> difficult to know where the leak occurs; a fatal page fault occurs on
> psref_target_destroy that is a completely different context.  If
> enabled, we can at least know a suspect LWP or softint handler and may
> find a cause in luck.

How often do these leaks happen?  If DIAGNOSTIC without the new code
will assert or crash on a leak, but in a non-specific way, we know they
are fairly rare.  People who encounter them can add the define to enable
the more specific detector and continue running to catch the next leak.

It seems that if leaks are common, then it should be easy to find them
with a few people enabling the new detector.  If they are very rare,
then 3% forwarding speed is a large price to pay for finding them more
specifically the first time, on systems where they are not expected to
happen.

Another possibility  (that I don't advocate) is to set this up so that
it is enabled on DIAGNOSTIC with current, but not enabled with
DIAGNOSTIC on release branches.   I am much more concerned with
performance loss on releaes branches.

Another option is to enable it in current, until the bugs are fixed and
we don't see leaks, and then turn it off.

Please don't take this as criticism of your new leak detector.  We are
getting lots of checking code to look for many kinds of problems.  Some
of it is expensive to varying degrees.  Everyone benefits from having
the bugs fixed once found, and it's hard to draw the line about what
should be enabled by default.

Re: CVS commit: src/sys

[Greg Troxel]

Ryota Ozaki  writes:

>> And, if someone is inclined, having better checks with meaurable
>> slowdown under DEBUG sounds ok too, but my revised understanding is
>> unclear on whether that's helpful.  (But I am only trying to keep
>> performance under DIAGNOSTIC high; I am unconcerned about DEBUG and
>> don't need to understand.)
>
> I've proposed another implementation a few days ago (*).  That provides
> useful information for debugging in exchange for expensive overhead.
> It is enabled by PSREF_DEBUG.
>
> (*) https://mail-index.netbsd.org/tech-kern/2019/05/10/msg025049.html

That sounds useful when needed.  I don't mind if DEBUG turns on
PSREF_DEBUG, but it might be that turning it on only when there is a
known reproduction recipe for a leak is sensible.

Thanks all for the useful and rational discussion.

Re: CVS commit: src/sys

[Greg Troxel]

Ryota Ozaki  writes:

> On Tue, May 14, 2019 at 2:31 AM Jason Thorpe  wrote:
>>
>>
>>
>> > On May 13, 2019, at 7:17 AM, Greg Troxel  wrote:
>> >
>> > 2) Your option 2 seems to involve two things at once:
>> >
>> >  - migration to lwp_specificadata
>> >  - using DEBUG instead of DIAGNOSTIC to control the leak check feature
>> >
>> > I do not understand why changing the nature of the implementation is
>> > linked to how it is enabled.
>>
>> I think Ozaki-san saying that the 3% performance hit only happens
>> when lwp_specificdata is used, and hence that it would need to be
>> wrapped in DEBUG rather than DIAGNOSTIC.

Now this is all making sense.

>> The original negligible-impact implementation did NOT use
>> lwp_specificdata, and thus was fine for DIAGNOSTIC.  I believe
>> Ozaki-san's preference is to use *this* implementation so that it
>> can be exposed to a wider audience.  The lwp_specificdata approach
>> was only explored after someone else suggested a preference for it.
>>
>> At least, that's my understanding of the situation.
>
> Yes, your understanding is correct.  Thank you for the clarification.

So having a check under DIAGNOSTIC that you more or less can't measure
sounds just fine to me.  I only meant to object to a 3% slowdown under
DIAGNOSTIC.

And, if someone is inclined, having better checks with meaurable
slowdown under DEBUG sounds ok too, but my revised understanding is
unclear on whether that's helpful.  (But I am only trying to keep
performance under DIAGNOSTIC high; I am unconcerned about DEBUG and
don't need to understand.)

Re: CVS commit: src

[Greg Troxel]

m...@netbsd.org writes:

> These seem to be lowercase versions of the same names. We're going to
> have trouble with the people who use case insensitive filesystems and
> CVS.

With any luck the multicase stuff is just dups to prune and not useful.


> (Or maybe it's not so bad because they're files, not directories?)

Doesn't matter - still bad.

What happens is that cvs creates one of the files, and then when it
creates the other finds a file already there, or overwrites it, so the
other one shows up modified.   Probably there is a bigger explosion with
directories...

CVS commit: src/sys/dev/ic

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Mon Jul 29 12:07:57 UTC 2019

Modified Files:
src/sys/dev/ic: mfi.c

Log Message:
sys/dev/ic/mfi.c: Add missing break in switch

(The entire switch is guarded by MFI_DEBUG and is known not to build.)
Reported by Oskar.


To generate a diff of this commit:
cvs rdiff -u -r1.60 -r1.61 src/sys/dev/ic/mfi.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/ic/mfi.c
diff -u src/sys/dev/ic/mfi.c:1.60 src/sys/dev/ic/mfi.c:1.61
--- src/sys/dev/ic/mfi.c:1.60	Sat Nov 24 18:10:29 2018
+++ src/sys/dev/ic/mfi.c	Mon Jul 29 12:07:57 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: mfi.c,v 1.60 2018/11/24 18:10:29 bouyer Exp $ */
+/* $NetBSD: mfi.c,v 1.61 2019/07/29 12:07:57 gdt Exp $ */
 /* $OpenBSD: mfi.c,v 1.66 2006/11/28 23:59:45 dlg Exp $ */
 
 /*
@@ -73,7 +73,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: mfi.c,v 1.60 2018/11/24 18:10:29 bouyer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mfi.c,v 1.61 2019/07/29 12:07:57 gdt Exp $");
 
 #include "bio.h"
 
@@ -869,6 +869,7 @@ mfi_get_bbu(struct mfi_softc *sc, struct
 		stat->detail.bbu.remaining_capacity ,
 		stat->detail.bbu.full_charge_capacity ,
 		stat->detail.bbu.is_SOH_good);
+		break;
 	default:
 		printf("\n");
 	}

CVS commit: src/sys/dev/ic

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Mon Jul 29 12:07:57 UTC 2019

Modified Files:
src/sys/dev/ic: mfi.c

Log Message:
sys/dev/ic/mfi.c: Add missing break in switch

(The entire switch is guarded by MFI_DEBUG and is known not to build.)
Reported by Oskar.


To generate a diff of this commit:
cvs rdiff -u -r1.60 -r1.61 src/sys/dev/ic/mfi.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/etc

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Mon Jul 29 17:53:20 UTC 2019

Modified Files:
src/etc: MAKEDEV.tmpl

Log Message:
MAKEDEV.tmpl: Create nodes for 16 USB hubs

As proposed on current-users, but with better formatting.


To generate a diff of this commit:
cvs rdiff -u -r1.204 -r1.205 src/etc/MAKEDEV.tmpl

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/etc/MAKEDEV.tmpl
diff -u src/etc/MAKEDEV.tmpl:1.204 src/etc/MAKEDEV.tmpl:1.205
--- src/etc/MAKEDEV.tmpl:1.204	Fri May 31 13:15:00 2019
+++ src/etc/MAKEDEV.tmpl	Mon Jul 29 17:53:20 2019
@@ -1,5 +1,5 @@
 #!/bin/sh -
-#	$NetBSD: MAKEDEV.tmpl,v 1.204 2019/05/31 13:15:00 nia Exp $
+#	$NetBSD: MAKEDEV.tmpl,v 1.205 2019/07/29 17:53:20 gdt Exp $
 #
 # Copyright (c) 2003,2007,2008 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -914,6 +914,7 @@ ramdisk)
 
 usbs)
 	makedev usb usb0 usb1 usb2 usb3 usb4 usb5 usb6 usb7
+	makedev usb8 usb9 usb10 usb11 usb12 usb13 usb14 usb15
 	makedev uhid0 uhid1 uhid2 uhid3 uhid4 uhid5
 	makedev uhid6 uhid7 uhid8 uhid9 uhid10 uhid11
 	makedev uhid12 uhid13 uhid14 uhid15

CVS commit: src/etc

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Mon Jul 29 17:53:20 UTC 2019

Modified Files:
src/etc: MAKEDEV.tmpl

Log Message:
MAKEDEV.tmpl: Create nodes for 16 USB hubs

As proposed on current-users, but with better formatting.


To generate a diff of this commit:
cvs rdiff -u -r1.204 -r1.205 src/etc/MAKEDEV.tmpl

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/distrib/amd64/liveimage/emuimage

[Greg Troxel]

Andreas Gustafsson  writes:

> I really don't like the general idea of introducing differences
> between images and systems installed through sysinst.  It's confusing
> for users, and also means that testing of one is less likely to apply
> to the other.

Agreed strongly.

In addition, I don't like it that images have stuff in /boot in the DOS
partition that can't be found in some obvious place, like /usr/mdec.
But I haven't gotten around to trying to fix it either so I get it that
ENOPATCH.

Re: CVS commit: src/distrib/amd64/liveimage/emuimage

[Greg Troxel]

Martin Husemann  writes:

> On Thu, Aug 08, 2019 at 08:09:19PM -0400, Greg Troxel wrote:
>> In addition, I don't like it that images have stuff in /boot in the DOS
>> partition that can't be found in some obvious place, like /usr/mdec.
>> But I haven't gotten around to trying to fix it either so I get it that
>> ENOPATCH.
>
> Yes, but in this case the bug is that sysinst does not know how to properly
> fill the /boot things - which is on my TODO list.
>
> (and also, as you mention, that there is no obvious easy source for the 
> content)

I was talking about the first bug, that these things aren't in the
destdir of a build.  The following belong on /usr/mdec/RPI, or similar,
either for all arm build (or when MKRPI=yes if we want to split that
out).

-rwxr-xr-x  1 root  wheel 1494 Oct 26  2017 LICENCE.broadcom
-rwxr-xr-x  1 root  wheel17932 Oct 26  2017 bootcode.bin
-rwxr-xr-x  1 root  wheel  105 Nov  9  2018 cmdline.txt
-rwxr-xr-x  1 root  wheel 6624 Oct 26  2017 fixup.dat
-rwxr-xr-x  1 root  wheel 2533 Oct 26  2017 fixup_cd.dat
-rwxr-xr-x  1 root  wheel  2823716 Oct 26  2017 start.elf
-rwxr-xr-x  1 root  wheel   634948 Oct 26  2017 start_cd.elf

Arguably installboot should be taught to deal with this; it's a FAT
partition instead of blocks 0-15, but it strikes me as the same thing
logically.

Re: CVS commit: src

[Greg Troxel]

Thomas Klausner  writes:

> P.S.: The file is 44k, so I'm not convinced by the "/ is small"
> argument. ;)

100 * 44k is 4400k :-)   The / and /usr distinction is longstanding, I
don't think we should give up on it easily.

Re: CVS commit: src/etc

[Greg Troxel]

Simon Burge  writes:

> I'm running with a complete ZFS-only setup with no legacy mounts.  This
> is my basic ZFS layout (leaving out a few mounts that don't add any more
> value to this discussion):
>
>   NAME  MOUNTPOINT
>   pool0 /pool0
>   pool0/ROOTnone
>   pool0/ROOT/default/
>   pool0/home/home
>   pool0/home/simonb /home/simonb
>   pool0/usr /usr
>   pool0/usr/obj /usr/obj
>   pool0/usr/pkg /usr/pkg
>   pool0/var /var
>   pool0/var/crash   /var/crash
>   pool0/var/log /var/log
>   pool0/var/mail/var/mail
>   pool0/var/tmp /var/tmp
>
> and I then have this grot in my mountcritlocal:
>
>   # XX
>   zfs mount pool0/var
>   zfs mount pool0/var/crash
>   zfs mount pool0/var/log
>   zfs mount pool0/var/mail
>   zfs mount pool0/var/tmp

[I am guessing you have a boot partition that isn't zfs, just to load
the kernel, but I haven't tried to read about that, and it seems a
separate topic.]

I will assume that the / (zfs as you say above) is the only mounted fs
as init starts.

> I have been trying to think of better solutions to this before you added
> this new critical_filesystems_zfs rc.conf variable.  One idea was to add
> a user defined property (eg "netbsd:mountcrit=yes") and use that in a
> similar way to how you implemented mount_critical_filesystems_zfs .  Then
> another idea hit me.
>
> Why don't we just mount all the ZFS filesystems in mountcritlocal?  I
> can't think of any negative reasons for not mounting all non-legacy
> mountable ZFS filesystems early in the boot process. This would be as
> simple as moving this chunk from mountall to mountcritlocal:
>
> # Mount ZFS filesystems first because fstab
> # may try and null mount paths on ZFS.
> if checkyesno zfs; then
> zfs mount -a
> zfs share -a
> fi
>
> and the unmount stuff from mountall to a new mountcritlocal_stop
> function.

[I don't really know what you mean by legacy (other than non-zfs, but
you didn't say that, so perhaps you mean something different).]

I think the big point here  is why do we have a notion of critical
filesystems, and if we are going to mount all zfs filesystems in
mountcritlocal, why would we then not mount all local filesystems?

AIUI, the critical notion comes from the netbooting days and sequencing
of bringing up networking to mount filesystems, which can involve
running programs that aren't on root.

I have used mountcritlocal to mount /var and /usr so that route6d could
start at the time when it is invoked by rc.d, I think, but the details
have been paged out.  I don't see that being a different issue with zfs
for / /var /usr.  Although I see it hitting more people as having
multiple filesystems is a more sensible thing to do with zfs because of
shared pool space.


If we are going to keep the critical concept, then it seems like it
should apply to all filesystems.  If we are going to abandon it, that's
a big discussion for tech-userlevel.

> For people using critical early legacy mounts, they can still be added
> via critical_filesystems_local as that looks up via /etc/fstab (right?
> I haven't tested this).

If you mean /var or /usr as ffs, I would expect so and certainly that's
necessary to support.

> Anyone see a reason for not using this much simpler approach to
> having ZFS filesystems available?  I don't think we'd need to keep
> critical_filesystems_zfs if we change mountcritlocal as I suggest, as
> that explicitly only deals with non-legacy filesystems.

I don't think it's good to treat zfs and non-zfs differently and I don't
think there are any good reasons to do so.  I also don't think its a
good idea to make ffs 2nd class.

If the big thing is to avoid having to manually mark zfs filesystems as
critical in rc.conf, fstab or some other file, and instead use a
property, that sounds like a perfectly fine solution, as the mountpoint
is already in zfs properties, and it makes sense to keep the critical
bit in the same place as the mountpoint.

However, I don't know how it works if you have a 2nd pool with a
critical filesystem and that pool doesn't come up.  (I could see
somebody having RAID1 for / /var and swap and RAIDZ for /usr and
everything else.)  From that viewpoint listing the critical filesystems
in rc.conf, leading to failure if they are not available, seems better.

It also seems like perhaps pools could be marked critical vs not so that
zfs startup can attach the critical pools first, and then the others,
but I suspect that is a lot of work for no gain, and the rc.conf
approach avoids the need.

I am really uncomfortable with a plan that comes from a place of
declaring anything but zfs as "legacy, that 

Re: CVS commit: src/etc

[Greg Troxel]

I apologize for failing to understand "zfs legacy mount" and incorrectly
associating it with how I usually encounter the word legacy.

I now understand that you meant to separate:

  zfs's preferred path of having mountpoints stored as volume
  properties, which is a different way of specifying what is mounted
  where than everything else, but makes sense in a world with many zfs
  volumes

  having a zfs volume where instead of the normal zfs way, there is an
  fstab entry

So having re-thought I would say:

  It makes sense to have a boolean "critical" property (the name you
  suggested is fine) for zfs volumes that specify a mount point, so that
  such volumes would be mounted in mountcritlocal.  I am 100% happy for
  someone to add that and see no big problems.

  It makes sense to just put zfs volume mountpoints in
  critical_filesystems_local, if those volumes use the fstab method
  instead of mountpoint properties (i.e., are "zfs legacy mounts").

  I think this is tricky if there are multiple pools and some don't come
  up.  But I think it's ok if the main path is that one should have all
  critical zfs filesytems on the same pool as root, with root on zfs.
  With root not on zfs but say /usr and /var on zfs, there needs to be
  some way for the boot to fail if they aren't mountable, just like if
  they were in fstab, if the pool doesn't attach and thus the critical
  variable aren't readable.  That might mean "if root is not zfs, and
  something in critical_fileystems_{local,remote} is in zfs, then those
  things have to use zfs legacy mounts".  It might mean having
  "critical_zfs_pools_{local,remote}" which will fail the boot if they
  are not attached at the mountcritlocal/mountcritremote stage, so that
  the critical properties are reliably there.

  I am opposed to deciding that all zfs filesystems should be treated as
  critical (in a world where we have not abolished the notion).

  We could have a discussion about why we even have the concept of
  critical filesystems, but if so that should not be about zfs and
  should be in a new thread on tech-userlevel.  And, I think it really
  isn't strongly releated to this discussion.


for background, I used to understand the critical filesystem scheme
better, but I'll briefly say (projecting to modern rc.d) that I think
it's about sequencing getting enough filesystems mounted to be able to
hit the NETWORKING rc.d barrier.  Consider a diskless workstation with
separate / and /usr (because /usr is shared across all 10 Sun 3/50s :-)
that also needs to mount some other filesystem from a server beyond the
LAN, which requires running routed.  Sorry if that gives yuo bad
flashbacks to the 80s :-)

In modern NetBSD I put /var and /usr in critical_fileystems_local,
because I want route6d to start, and that's in /usr/sbin.  Arguably
route6d should be in /sbin, or NETWORKING should be split into things
needed to talk to the local link vs the broader network, but I have
avoided digging in because adding a line to rc.conf is easy, and moving
route6d would be actual work.

Greg


signature.asc
Description: PGP signature

Re: CVS commit: src/etc

[Greg Troxel]

Brad Spencer  writes:

> What is referred to here is a specific ZFS idea and should not be
> confused with any sort of global notion.  Specifically, ZFS, by default
> and in common use, does not use anything like a /etc/fstab or
> /etc/vfstab to specify where the filesets get mounted.  It also does not
> usually use the usual mount command either to mount them, although that
> would work in Solaris if I remember correctly (at least in some cases).
> What happens is that the mount point is specified in the fileset meta
> data and "zfs mount " takes care of getting it mounted.  If you
> wanted to use /etc/fstab or /etc/vfstab with ZFS you need to mark the
> mountpoint in the fileset as being legacy, then you could fill in your
> /etc/... files as you wanted.  This, of course, strongly suggests that
> Sun was going to make all of the other filesystems except ZFS second
> class, but Sun fell apart before that really progressed to anything
> final and Oracle never really ran to that conclusion either.  No one is

Somehow I am not shocked that this usage (by Sun) had that lurking.

> at all suggesting that any filesystem become second class in the NetBSD
> realization of ZFS.
>
> So the term legacy when speaking of ZFS is used to refer to a filesystem
> that uses whatever /etc/... files you use and uses the usual mount
> command.

Thanks, and Taylor also sent me a clue privately.A superceding
comment by me crossed in the mail.


signature.asc
Description: PGP signature

Re: null-terminated vs. nul-terminated

[Greg Troxel]

"David H. Gutteridge"  writes:

Thanks for the history and it is  all sensible.

> "nul-terminated" and "null-terminated" seemed more common in man pages
> that originated from historical BSD sources, so, lacking any style
> guide, I inferred the lowercase "nul" was more "correct" as "BSD style"
> (excepting modern OpenBSD), even though that looks a bit odd to me. I
> then examined where "nul-terminated" came from, and found these bulk
> commits, which imply a standard.

> date: 2005-01-02 18:38:04 +;  author: wiz;
> Mark up NULL, and replace null by nul where appropriate.
>
> date: 2006-10-16 08:48:45 +;  author: wiz;
> nul/null/NULL cleanup:
> when talking about characters/bytes, use "nul" and "nul-terminate"
> when talking about pointers, use "null pointer" or ".Dv NULL"
>
> So that seemed to me the established style.

It may have been BSD style, but I think it's wrong to use lowercase for
an ASCII codepoint.  And therefore it is confusing to people who know
that the ASCII zero byte is written NUL.




signature.asc
Description: PGP signature

Re: null-terminated vs. nul-terminated

[Greg Troxel]

Taylor R Campbell  writes:

>> Date: Sat, 26 Mar 2022 16:53:19 +0100
>> From: Roland Illig 
>> 
>> The term "null-terminated string" is quite common when talking about C.
>>   In contrast, the word "nul" in "nul-terminated" always reminds me of
>> the character abbreviation in ASCII, which has a narrower scope than C.
>>   I prefer to keep "null-terminated" here.
>
> I feel like I've usually seen it as NUL-terminated.  I thought it was
> in /usr/share/misc/style but I must have been thinking of a different
> style guide.
>
> `NUL' is better than `null' or `NULL' here because it's not a null
> pointer, unlike, e.g., the execve argv terminator.  Even if the string
> isn't US-ASCII, what character encoding calls a nonzero byte `NUL'?
>
> `NUL' is better than `zero' or `0' here because it's unambiguously the
> all-bits-zero byte, not the US-ASCII encoding of `0' (i.e., decimal 48
> or 0x30).

For background I'm a native en_US speaker whose second computer language
was K C from the pre-ANSI edition.

There are three separate concepts.

NULLRefers to a pointer, never to a character

NUL ASCII codepoint, 7 zero bits, and 8 zero bits when stored in an
8-bit byte.  NUL is never properly written nul; the ASCII
codepoints are upper case in formal usage.

nullAn English word that can mean various things, including

   null pointer => NULL

   null character => NUL in ASCII

   null character => 0 in something else, theoretically maybe,
   but C just cannot deal with a character set that uses 0 to
   represent something that gets used in strings.


So one can talk about a "null-terminated string" implying "null
character" which means NUL, and one could also write "NUL-terminated
string".  I find the from NUL-terminated to be artificial.

I perceive "nul-terminated" as an error due to the lower case nul.


signature.asc
Description: PGP signature

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Fri Oct 20 12:45:17 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig: Add caution to man page about cache bypassing behavior


To generate a diff of this commit:
cvs rdiff -u -r1.46 -r1.47 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Fri Oct 20 12:45:17 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig: Add caution to man page about cache bypassing behavior


To generate a diff of this commit:
cvs rdiff -u -r1.46 -r1.47 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.sbin/vnconfig/vnconfig.8
diff -u src/usr.sbin/vnconfig/vnconfig.8:1.46 src/usr.sbin/vnconfig/vnconfig.8:1.47
--- src/usr.sbin/vnconfig/vnconfig.8:1.46	Sat Jan  9 23:54:26 2021
+++ src/usr.sbin/vnconfig/vnconfig.8	Fri Oct 20 12:45:17 2023
@@ -1,4 +1,4 @@
-.\"	$NetBSD: vnconfig.8,v 1.46 2021/01/09 23:54:26 wiz Exp $
+.\"	$NetBSD: vnconfig.8,v 1.47 2023/10/20 12:45:17 gdt Exp $
 .\"
 .\" Copyright (c) 1997 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -100,6 +100,12 @@ The
 is a special file of raw partition or name of vnode disk like
 .Pa vnd0 .
 .Pp
+By default, accesses to the file bypass normal mechanisms.  This
+behavior results in not updating the modification timestamp of the
+file.  Also, file contents read through the filesystem may not be the
+correct values of ciphertext, which can lead to corrupted backups of
+the backing file.
+.Pp
 Options indicate an action to be performed:
 .Bl -tag -width indent
 .It Fl c

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Sat Oct 21 23:42:03 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig.8: Don't mention ciphertext

After all, this is not cgd(4).


To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.sbin/vnconfig/vnconfig.8
diff -u src/usr.sbin/vnconfig/vnconfig.8:1.49 src/usr.sbin/vnconfig/vnconfig.8:1.50
--- src/usr.sbin/vnconfig/vnconfig.8:1.49	Sat Oct 21 23:38:26 2023
+++ src/usr.sbin/vnconfig/vnconfig.8	Sat Oct 21 23:42:03 2023
@@ -1,4 +1,4 @@
-.\"	$NetBSD: vnconfig.8,v 1.49 2023/10/21 23:38:26 gdt Exp $
+.\"	$NetBSD: vnconfig.8,v 1.50 2023/10/21 23:42:03 gdt Exp $
 .\"
 .\" Copyright (c) 1997 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -110,8 +110,8 @@ accesses separated in time, this is gene
 This bypassing behavior results in not updating the modification
 timestamp of the file.
 Also, file contents read through the filesystem (and thus the
-filesystem's caching layer) may not be the correct values of
-ciphertext, so caution is in order for backups.
+filesystem's caching layer) may not be the contents written via this
+interface, so caution is in order for backups.
 The
 .Fl i
 option may be useful if it is necessary to avoid inconsistent caching.

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Sat Oct 21 23:42:03 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig.8: Don't mention ciphertext

After all, this is not cgd(4).


To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Sat Oct 21 23:38:26 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig: Improve recent caching edit to man page

Explain that typical usage patterns don't run into consistency issues.
Xref the -i flag.

Leave ambiguous the nature of cache inconsistency, because I don't
undersstand it and it's best not to make it a defined interface
anyway.  I am unclear on whether a buffer cache read before a vnd
session might persist after a vnd is configured/used/unconfigured, and
whether a buffer cache write might be delayed until after a vnd
configure/write.


To generate a diff of this commit:
cvs rdiff -u -r1.48 -r1.49 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.sbin/vnconfig/vnconfig.8
diff -u src/usr.sbin/vnconfig/vnconfig.8:1.48 src/usr.sbin/vnconfig/vnconfig.8:1.49
--- src/usr.sbin/vnconfig/vnconfig.8:1.48	Fri Oct 20 13:04:21 2023
+++ src/usr.sbin/vnconfig/vnconfig.8	Sat Oct 21 23:38:26 2023
@@ -1,4 +1,4 @@
-.\"	$NetBSD: vnconfig.8,v 1.48 2023/10/20 13:04:21 wiz Exp $
+.\"	$NetBSD: vnconfig.8,v 1.49 2023/10/21 23:38:26 gdt Exp $
 .\"
 .\" Copyright (c) 1997 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -100,12 +100,21 @@ The
 is a special file of raw partition or name of vnode disk like
 .Pa vnd0 .
 .Pp
-By default, accesses to the file bypass normal mechanisms.
-This behavior results in not updating the modification timestamp
-of the file.
-Also, file contents read through the filesystem may not be the
-correct values of ciphertext, which can lead to corrupted backups of
-the backing file.
+By default, accesses to the file bypass normal mechanisms and thus
+do not read from or fill the filesystem cache.
+Because the typical approach is to access the file only via
+.Xr vnd 4 ,
+or at least to have regular accesses and
+.Xr vnd 4
+accesses separated in time, this is generally not problematic.
+This bypassing behavior results in not updating the modification
+timestamp of the file.
+Also, file contents read through the filesystem (and thus the
+filesystem's caching layer) may not be the correct values of
+ciphertext, so caution is in order for backups.
+The
+.Fl i
+option may be useful if it is necessary to avoid inconsistent caching.
 .Pp
 Options indicate an action to be performed:
 .Bl -tag -width indent

CVS commit: src/usr.sbin/vnconfig

[Greg Troxel]

Module Name:src
Committed By:   gdt
Date:   Sat Oct 21 23:38:26 UTC 2023

Modified Files:
src/usr.sbin/vnconfig: vnconfig.8

Log Message:
vnconfig: Improve recent caching edit to man page

Explain that typical usage patterns don't run into consistency issues.
Xref the -i flag.

Leave ambiguous the nature of cache inconsistency, because I don't
undersstand it and it's best not to make it a defined interface
anyway.  I am unclear on whether a buffer cache read before a vnd
session might persist after a vnd is configured/used/unconfigured, and
whether a buffer cache write might be delayed until after a vnd
configure/write.


To generate a diff of this commit:
cvs rdiff -u -r1.48 -r1.49 src/usr.sbin/vnconfig/vnconfig.8

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/sbin/newfs

[Greg Troxel]

Taylor R Campbell  writes:

> Well, what happened is:
>
> 1. I was drafting posix_memalign and aligned_alloc for libbsdmalloc in
>response to the thread about static program size yesterday, and
>carefully reading the specs -- POSIX 2018, C11 -- to make sure I
>got all the details right.
>
> 2. I saw a silly restriction in C11, that aligned_alloc(A, S) requires
>A to divide S.
>
> 3. For the sake of encouraging portable code, I figured we should
>enforce that restriction, so I went ahead and did that in both the
>libbsdmalloc front end I was writing and in jemalloc.  (This caused
>a bunch of tests to start failing.)
>
> 4. I audited all the uses of aligned_alloc in tree to make sury they
>meet the restriction (which fixed the tests).
>
> 5. Someone pointed out that the silly restriction that was imposed in
>C11 had actually been lifted in C17:
>
> (C11) The value of alignment shall be a valid alignment
> supported by the implementation and the value of size shall be
> an integral multiple of alignment.
>
> (C17) If the value of alignment is not a valid alignment
> supported by the implementation the function shall fail by
> returning a null pointer.
>
> 6. So I reverted the jemalloc enforcement of the C11 restriction, and
>reverted the fixes made to conform to it, and removed the
>restriction in the new libbsdmalloc code, and we're more or less
>back to where we started.
>
> Except I am now reminded that I updated t_posix_memalign.c to verify
> this silly restriction of aligned_alloc, so I'd better go fix that
> before a bunch of tests start failing again!

Thanks for taking the time to explain so thoroughly.  So we are back to
status quo ante-fixum, which is by definition always ok :-)

Re: CVS commit: src/usr.bin/chflags

[Greg Troxel]

Jan Schaumann  writes:

> Valery Ushakov  wrote:
>> On Wed, May 24, 2023 at 22:33:17 +, Jan Schaumann wrote:
>
>> > Briefly describe the 'arch' and 'nodump' flags.
>> 
>> What makes them special and why is that these two have to be described
>> here and not in chflags(2), where the flags are actually defined?
>
> Unlike the other flags, they are not intuitive.  As
> they are exposed to the end-user (versus the
> programmer), it seems useful to describe them in
> section 1 of the manual pages.

FWIW, I agree that the flags are user-facing rather than (only)
programmer-facing, and thus should be described in a 1/8 manpage vs only
a 2 manpage.

Re: CVS commit: src/usr.bin/chflags

[Greg Troxel]

Taylor R Campbell  writes:

> jschauma@'s change did help clarify (1) and (2).  Let's try to work
> together to make it better?

Well said.


I can shed a little light on some of the questions.

> 1. What is `arch' or `archived' supposed to mean?  Who sets it, who
>pays attention to it, and what consequences does it have?  Is it
>obsolete?  What was it used for?  From the current text, I have no
>idea!

>From reading sources, I think it is exactly "the archive bit is set in
the foreign filesystem entry that this vnode is representting" and that
it is basically an MS-DOS thing that is likely not important (even to
people that do not dismiss all things Windows!) any more.

> 2. What is `nodump' supposed to mean?  Who sets it, who pays attention
>to it, and what consequences does it have?  Does the kernel do
>anything about it, or is it just an extra bit of storage that
>programs like dump(8) can choose to examine?  From the current
>text, I have no idea!

I believe it is just a bit that other programs can query, and that the
intent is that dump(8) will decline to dump files with the bit set.

(I intend, in my Copious Spare Time, to extend pkgsrc/sysutils/bup to
respect this flag.  I have previously made extensive use of nodump with
dump.  Partly because of lack of the bit in bup and partly for
organizational sanity, I use /u0 and /n0 for things that should and
should not be backed up.  But that's me in case it helps motivate why
nodump is useful, not about flag docs.)

I believe that nodump continues to be relevant today.

> 3. What is the difference between `system' and `user' immutable or
>append-only?  If I set uchg, does that mean the _owner_ can't
>modify the file, or that _nobody_ can modify the file?

Not useful for a 1 man page, but

  egrep '[SU]F_' /usr/include/sys/stat.h

is illuminating to programmers.

I believe, but would have to read the code to be 100% sure, that

  only root can modify schg and sappnd flags (and archive)

  either root or the owner can modify other flags (except on
  device files, on which only root may modify flags, for reasons that
  are unclear, but the rationale seems unimportant)

  etiher schg or uchg means the file can't be modified, probably, but
  it's possible that uchg applies to everybody but root and schg applies
  to everybody.  A quick experiment shows that root cannot write to a
  gdt-owned file with the uchg bit set.

Fundamentally there are 16 flag bits for user and 16 for system and I
think it is baked in hard that only root can modify the system set.

> jschauma@'s change did help clarify (1) and (2).  Let's try to work
> together to make it better?

The fundamental issue is that the table attempts to

  - map the tokens used by chflags(1) (good)

  - to names that sort of represent the flags (which are fundamentally C
preprocessor token) but are a third namespace that exists only here
(bad)

  - describe the flags permissions as if they are individual, instead of
simly saying that schg and sappend may be changed by superuser
only (confusing)

  - does not give semantics (ok, because that is not consistent with a
table)

Also, the man page does not explain that because "nodump" is the name of
a flag, one does "chflags dump foo" to remove the nodump flag.

(I don't think it needs explanation that the design decision is that
nodump is a flag, instead of dump, because the norms are that files tend
not to have any flags set and that all files should be backed up.)



I would recommend:

  Noting that archive flag represents something in some foreign
  filesystems, currently known to exist only for some MSDOS filesystems,
  and that the specific filesystem should document that.

  Explaining that the nodump flag is a request that backup programs omit
  the file from backups and to see dump(8).

  Be clearer about permissions.

  Drop the restatement of tokens into a third only-here namespace.
  Instead, use paragraphs following to describe that each token means.

Re: CVS commit: src/usr.bin/chflags

[Greg Troxel]

Valery Ushakov  writes:

> On Thu, May 25, 2023 at 07:17:52 -0400, Greg Troxel wrote:
>
>> Jan Schaumann  writes:
>> 
>> > Valery Ushakov  wrote:
>> >> On Wed, May 24, 2023 at 22:33:17 +, Jan Schaumann wrote:
>> >
>> >> > Briefly describe the 'arch' and 'nodump' flags.
>> >> 
>> >> What makes them special and why is that these two have to be described
>> >> here and not in chflags(2), where the flags are actually defined?
>> >
>> > Unlike the other flags, they are not intuitive.  As
>> > they are exposed to the end-user (versus the
>> > programmer), it seems useful to describe them in
>> > section 1 of the manual pages.
>> 
>> FWIW, I agree that the flags are user-facing rather than (only)
>> programmer-facing, and thus should be described in a 1/8 manpage vs only
>> a 2 manpage.
>
> Then *all* of them should be described in a proper narration.  I'm not
> objecting to that.  I'm objecting to sticking the description of two
> of them chosen seemingly at random in a place selected seemingly at
> random.

OK, that's a perfectly reasonable objection.