Page MenuHomeFreeBSD

wollman (Garrett Wollman)
User

Projects

User Details

User Since
May 14 2014, 10:19 PM (287 w, 5 d)

Recent Activity

Oct 4 2019

wollman added a comment to D21880: fstat(1) -s option.

Did not review the code but I love the change. One less reason to install lsof!

Oct 4 2019, 5:56 PM

Sep 16 2018

wollman committed rD52262: Remove my obsolete and long-expired DSA key 0x0b92faea and add my.
Remove my obsolete and long-expired DSA key 0x0b92faea and add my
Sep 16 2018, 2:56 AM

May 23 2018

wollman accepted D15530: Add time2posix and posix2time to time.h.

Seems pretty sensible to me. These functions are of the same ilk (tzcode inventions) as timelocal (standardized as mktime()) and timegm (standardized as that stupid formula in the POSIX spec).

May 23 2018, 7:23 PM
wollman committed rS334071: Whoops, forgot to add this file in r334070..
Whoops, forgot to add this file in r334070.
May 23 2018, 2:54 AM
wollman committed rS334070: Move unsigned limits to a separate table/recognizer and display them.
Move unsigned limits to a separate table/recognizer and display them
May 23 2018, 2:52 AM

May 12 2018

wollman accepted D15329: Improvement for MAC address uniqueness of if_epair(4).
May 12 2018, 12:56 AM

May 11 2018

wollman added inline comments to D10600: Revision of LDAP section of the FreeBSD Handbook.
May 11 2018, 1:40 AM · Doc Committers, network

May 10 2018

wollman added a comment to D15358: Address memory leak in new reno cc module.

As an aside, @jtl privately questioned the use of ENOMEM vs ENOBUFS. The TCP stack uses ENOBUFS and ENOMEM interchangeably for malloc failures, and @jtl is wondering if we should standardise on one or the other as far as user space app error handling is concerned to aid portability. This LKML thread and the Linux socket(2) man page suggest that returning both is kosher from both a POSIX point of view and as established precedent. I haven't consulted the standards myself, but it seems plausible that we should add ENOMEM to our tcp(4) - here is a revised patch that does so. @emaste, your were singled out as someone with a good working knowledge of POSIX... do you have any comment on the subject of socket API ENOMEM vs ENOBUFS errno usage?

May 10 2018, 2:47 AM

May 9 2018

wollman added a comment to D11725: Add optional TCP logging on sonewconn failures..

It would be really nice to see this fixed sooner rather than later. I can't tell from the phab UI who's put a block on this, but could we maybe move forward a bit? The current messages are both annoying and unhelpful.

May 9 2018, 2:13 AM
wollman accepted D15278: Add IFCAP_LINKSTATE support to if_loop(4).

Looks good to me.

May 9 2018, 2:10 AM
wollman added a comment to D10600: Revision of LDAP section of the FreeBSD Handbook.

Is it planned to address the client side (e.g., nss-pam-ldapd) or just the server?

May 9 2018, 2:09 AM · Doc Committers, network
wollman added a comment to D15329: Improvement for MAC address uniqueness of if_epair(4).

It's probably worth a little bit of thought as to what is the more common case, a zillion epair interfaces on one host (read: half a zillion vnet jails) or a much smaller number of epairs on a larger number of hosts. It sounds like you are well placed to assign your own addresses, whatever the default may be.

Well, this is not network hot path and we now have jenkins_hash32() kernel function in all supported branches, so we could just allocate additional uint32_t key[3] array on stack of this function, copy there all 64 bits of (long)hostid plus (uint32_t)if_index and make use of jenkins_hash32() to get them mixed to 32-bit hash value. Then use it as-is for eaddr[1]-eaddr[4] similar to current manner or switch to using 58:9C:FC for upper half of MAC and use lower 24 bits of the hash for the rest of MAC resetting lowest bit to 0 for first interface and to 1 for second one.

May 9 2018, 1:55 AM

May 8 2018

wollman added inline comments to D15329: Improvement for MAC address uniqueness of if_epair(4).
May 8 2018, 2:12 AM
wollman added a comment to D15329: Improvement for MAC address uniqueness of if_epair(4).

Well, I have many hosts having over housand of ngXXX interfaces, so yes, two bytes are needed at least.

May 8 2018, 2:05 AM

May 6 2018

wollman requested changes to D15329: Improvement for MAC address uniqueness of if_epair(4).

I'd like to see a few more bits of uniqueness here; I don't trust that two bytes of hostid is going to be sufficient. Nonetheless, this is a good thing. (We probably won't use it -- we embed the host's canonical IPv4 address into the MAC -- but it's important that the default out-of-the-box config be usable!)

May 6 2018, 9:56 PM

Jul 25 2017

wollman added a comment to D11725: Add optional TCP logging on sonewconn failures..

Yes, you're touching on the underlying awkwardness here which is that the original overflow logging happens in sonewconn, which is at the generic socket layer. I toyed with the idea of simply changing the message at the generic socket layer, but everything I came up with either had gross layering violations (e.g. having a socket layer function with knowlege of all the potential so_pcb types) or got fairly complicated with arguably poor abstractions (e.g. adding a protosw call for something like char *pr_getsockdescr). I'm definitely open to suggestions here though.

Jul 25 2017, 2:21 AM
wollman added a comment to D11725: Add optional TCP logging on sonewconn failures..

Have to say, I cannot imagine any circumstances in which the message that's on by default would be useful when the message that's off by default isn't printed. The PCB address is useless debugging information here, whereas the 4-tuple of the dropped connection is actually meaningful -- so I'd reverse them (or better yet, figure out a way to combine the two messages while hiding the PCB address under a debug flag).

Jul 25 2017, 2:05 AM

Aug 24 2016

wollman accepted D7618: Add non-TRUSTEDBSD prefixed knobs for _PC_ACL* and {CAP,INF,MAC}_PRESENT knobs.

Looks good to me.

Aug 24 2016, 1:19 AM

May 27 2016

wollman added a comment to D1626: Add iostat-like NFS server statistics .

I've been meaning to do something like this over the past year (since I made my last comment) but haven't had the time. But after attending a few conferences I became convinced that what I really wanted was to capture exact timing for all RPCs and have a usermode program to analyze the data via something like ALQ. In particular, I want to measure median and tail latency (which is going to be different for different operations) and also queue-residence time in the various queues a request moves through. This may be too expensive for some (many?) applications, so I don't expect to see that sort of thing here.

May 27 2016, 1:50 AM

Mar 8 2016

wollman retitled D5588: libprocstat: handle race condition while retrieving thread kernel stacks from to libprocstat: handle race condition while retrieving thread kernel stacks.
Mar 8 2016, 10:45 PM

Feb 22 2016

wollman added a comment to D5396: Store mbuf cluster reference count in the first mbuf..

Haven't read the diffs but I like this very much in concept.

Feb 22 2016, 11:30 PM

Feb 14 2016

wollman added a comment to D5255: Replace all resource occurrences of '0UL/~0UL' with '0/~0'..

I agree that this gives correct results as is, but (as the original perpetrator) I'd rather see an explicit unsigned type called out here -- e.g., ~(some_type_t)0 -- rather than depending on two's-complement sign extension behavior, if you're not going to use a macro. Using bitwise operators on signed values makes me uncomfortable, and I wouldn't want to rely on WG14's good graces....

Feb 14 2016, 7:26 PM

Jan 7 2016

wollman committed rS293358: MFH r292836:.
MFH r292836:
Jan 7 2016, 8:44 PM

Dec 28 2015

wollman committed rS292836: in6_if2idlen: treat bridge(4) interfaces like other Ethernet interfaces.
in6_if2idlen: treat bridge(4) interfaces like other Ethernet interfaces
Dec 28 2015, 6:30 PM

Oct 30 2015

wollman committed rS290203: Long-overdue MFC of r280930:.
Long-overdue MFC of r280930:
Oct 30 2015, 7:27 PM

Sep 7 2015

wollman added a comment to D3551: sysrc: Add support for ``rc.conf.d'' service(8) configuration file(s).
In D3551#73903, @dteske wrote:

Can we add some unit tests, please?

Did you see the "Test Plan" section in this review? Or did you mean a test harness?

Sep 7 2015, 12:55 AM

Sep 6 2015

wollman added a comment to D3343: Add "Logical Block Mode" to geom multipath.

For what it's worth, in all my testing -A and -R modes have been a small but significant lose, so I'll be interested to see if this mode fixes that.

Sep 6 2015, 8:43 PM
wollman accepted D3541: Eliminate setgid leftover in w(1)..

Makes sense to me.

Sep 6 2015, 8:28 PM
wollman added a comment to D3551: sysrc: Add support for ``rc.conf.d'' service(8) configuration file(s).

Can we add some unit tests, please?

Sep 6 2015, 8:20 PM
wollman added a comment to D3567: ignore ICMP need frag with equal or larger MTU offer.

Agreed that the current behavior is a bug and we should just fix it. I think pkelsey is right as far as the correct fix.

Sep 6 2015, 8:19 PM
wollman requested changes to D1858: Improvement for MAC address uniqueness of if_epair(4).
In D1858#35577, @bz wrote:

Also I'd rather have a random local part with the slight chance of collisions rather than something deterministic as the moment you bridge them out to a real ethernet you get a lot of conflicts if you have multiple net machines on the same VLAN.
The only real solution to avoid long-term conflicts for people is to have their own management and statically assign the ether address to the jail to avoid us having to guess one.

Sep 6 2015, 8:00 PM
wollman resigned from D3391: Prefer pciids if available as a database for pciconf(8).

No objection in principle but it's been far too long since I had anything to do with this code to express an opinion on the change itself.

Sep 6 2015, 7:55 PM
wollman abandoned D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.

This is now overtaken by events in mckusick's commit r287497.

Sep 6 2015, 7:51 PM

Apr 10 2015

wollman added a comment to D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.

See this image for a demonstration of what a difference correctly sized hash tables makes. (You can ignore the red pluses, that's just the default sizing for everything.) This is the metadata-intensive SWBUILD workload of the SPEC SFS 2014 benchmark; I'm collecting data for the other workloads presently.

Apr 10 2015, 6:38 PM
wollman added a comment to D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.
In D2265#14, @kostikbel wrote:

The hash which is not resized is the vfs_hash (vfs_hash.c), most likely. It is very much innocent to not adjust the hash size after desiredvnodes change at runtime.

Apr 10 2015, 3:03 AM
wollman added a comment to D2266: I can find no reason to allow packets with both SYN and FIN bits set past this point in the code. The packet should be dropped and not massaged as it is here..

There's nothing actually wrong with a SYN+FIN packet; the TCP spec unambiguously defines how it's supposed to be handled, and the code does it correctly as is (by turning off the FIN bit), which certainly seems to be within the spirit of what "scrub" is supposed to do. I'm not sure I see what the problem this is trying to solve is.

Apr 10 2015, 2:55 AM

Apr 9 2015

wollman updated the diff for D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.

This version actually works, and restores the run-time modifiability. The way I've implemented it preserves the compiled-in limit of MAXVNODES_MAX at boot time only; probably it should be bypassed or made into a louder warning when set as a tunable, or else enforced at run time as well. (It's not clear whether MAXVNODES_MAX is intended to be a true limit or just an additional sanity check for the auto-tuning implementation.)

Apr 9 2015, 11:03 PM
wollman added a comment to D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.
In D2265#8, @mav wrote:

I just hate when I have to reboot system to do some tuning. I think that read-write sysctl that is somewhat suboptimal is better then read-only. I think making sysctl read-only would be a step into wrong direction.

Apr 9 2015, 10:13 PM
wollman added a comment to D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.
In D2265#6, @mav wrote:

I don't see harm from making it tunable, that is good, but I don't like idea of making it read-only. I believe that loader-only tunables are too uncomfortable and should be avoided, when possible.

Apr 9 2015, 10:01 PM
wollman updated the diff for D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.

Don't overwrite the value specified in the tunable when we initialize desiredvnodes.

Apr 9 2015, 5:29 PM
wollman retitled D2265: kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable from to kern.maxvnodes should be a tunable, and probably shouldn't be run-time modifiable.
Apr 9 2015, 5:27 PM

Apr 1 2015

wollman closed D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

Closed by commit rS280930 (authored by @wollman).

Apr 1 2015, 12:46 AM
wollman added a comment to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
In D2165#39, @mav wrote:

svc_change_space_used() call out of the loop was kind of optimization when modifying variable shared by many threads, but that may be not so important.

Apr 1 2015, 12:16 AM

Mar 31 2015

wollman added a comment to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

This code is running on a production fileserver now. Waiting for the all-arches compile test before I'm ready to commit.

Mar 31 2015, 2:26 AM

Mar 30 2015

wollman updated the diff for D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

Previous patch had a use-after-free bug (that should have been obvious). This version reintroduces the "sz" variable to prevent this.

Mar 30 2015, 2:38 AM
wollman added a comment to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

NB: This diff is relative to 10.1. Context lines have changed in 11-current so the diff won't apply directly.

Mar 30 2015, 2:17 AM

Mar 29 2015

wollman added a comment to D1626: Add iostat-like NFS server statistics .

Speaking as a user, I would really love to see some form of this functionality happen.

Mar 29 2015, 6:15 AM
wollman updated the diff for D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

Remove the redundant overflow check and expand the comment to make it clear what's going on when we compute the request buffer limits. This should be the last iteration of this patch. This version isn't running on a production server yet; hopefully that will happen on Monday.

Mar 29 2015, 6:00 AM
wollman added a comment to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
In D2165#20, @rmacklem wrote:

It looks ok to me. I might have used int64_t instead of long, so that they're
64bits on all arches, but I don't think it will matter, since nmbclusters won't
be that large for 32bit arches, I think?

Mar 29 2015, 5:15 AM

Mar 28 2015

wollman retitled D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code from Remove antique hard-coded limit in kernel RPC request throttling code to Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 9:51 PM
wollman updated the diff for D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

This version eliminates the variable "sz" in svc_run_internal() entirely by not batching updates to the available request space.

Mar 28 2015, 9:50 PM
wollman added inline comments to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 9:23 PM
wollman updated the test plan for D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 9:13 PM
wollman added inline comments to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 9:11 PM
wollman added inline comments to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 8:49 PM
wollman updated the diff for D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

In the previous diff, arithmetic overflow resulted in incorrect values being computed. This version changes all of the data types to be longs rather than ints, as is more appropriate for 64-bit machines. (I haven't compile-tested on an ILP32 machine to see if it even still builds -- I suspect not, and I'll need to figure out a way to work around the lack of atomic 64-bit ops on 32-bit architectures.)

Mar 28 2015, 8:44 PM
wollman added a comment to D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.

This simple change actually failed even worse than the original code. I suspect signedness problems surfaced by the switch from gcc 4.2 to clang (we just upgraded from 9.3 to 10.1 on our NFS srevers), and I'm preparing a new diff that tries to deal with that.

Mar 28 2015, 8:34 PM
wollman updated D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code.
Mar 28 2015, 8:03 PM
wollman retitled D2165: Remove antique hard-coded limit and integer overflow bugs in kernel RPC request throttling code from to Remove antique hard-coded limit in kernel RPC request throttling code.
Mar 28 2015, 8:02 PM

Jan 24 2015

wollman added a comment to D1426: Add futimens and utimensat system calls..

Linux emulator support? I'm assuming it's already stubbed out (haven't looked) and just needs to be switched to the new internal interface.

Jan 24 2015, 10:07 PM
wollman accepted D1426: Add futimens and utimensat system calls..
Jan 24 2015, 10:05 PM

Aug 24 2014

wollman added a comment to D638: Add bsearch_b to the libc map and the stdlib header..

Shouldn't all of these BLOCKS declarations be moved down to the main BSD_VISIBLE block, since they are non-standard interfaces?

Aug 24 2014, 10:32 PM
wollman added a comment to D678: Validate the mode argument in access, eaccess, and faccessat for optional POSIX compliance and to improve compatibility with Linux and NetBSD.

As Jilles says, there's no requirement to have this check. But since access() is not called very frequently, adding one branch seems unlikely to hurt.

Aug 24 2014, 10:16 PM