Page MenuHomeFreeBSD

jmg (John-Mark Gurney)
User

Projects

User Details

User Since
Sep 2 2014, 10:55 PM (215 w, 8 h)

Recent Activity

Fri, Oct 12

jmg created D17541: update ZFS to note support of extended attributes.
Fri, Oct 12, 10:38 PM

Sep 16 2018

jmg committed rS338698: Significantly improve pf purge cpu usage by only taking locks.
Significantly improve pf purge cpu usage by only taking locks
Sep 16 2018, 12:44 AM
jmg closed D17097: only lock row in pf purge thread when work to do.
Sep 16 2018, 12:44 AM

Sep 10 2018

jmg added a comment to D17097: only lock row in pf purge thread when work to do.

For reference, on an active pf firewall, before the patch, ~1.56% cpu core was used on the purge thread, after the patch, .427% cpu core. over a third less CPU. This is with the same 128k hash table size.

Sep 10 2018, 5:18 PM

Sep 9 2018

jmg added a comment to D17097: only lock row in pf purge thread when work to do.

CPU usage calculated by taking the time pf purge ran and dividing it by the difference between now, and lstart:

ps -ax -o lstart,time,command | grep pf; date

Sun Sep 9 16:31:23 2018 0:30.95 [pf purge]
Sun Sep 9 19:33:53 2018 0:00.02 grep pf
Sun Sep 9 19:33:53 UTC 2018

Sep 9 2018, 7:35 PM
jmg added a comment to D17097: only lock row in pf purge thread when work to do.

note, if you apply the diff and check it w/ svn diff -x -wb, you'll see that the only change is the comment, the if, and the braces.

Sep 9 2018, 4:54 PM
jmg created D17097: only lock row in pf purge thread when work to do.
Sep 9 2018, 4:52 PM

Sep 1 2018

jmg added a comment to D15713: Bug 182518 - [login.conf] Better Password Hashes .

All comments are minor.

Sep 1 2018, 6:40 PM · security

Aug 29 2018

jmg updated the diff for D16690: make device_printf use sbuf.

ok, this should be ready to land.. Not going to update printf(9),
that'll be a different patch, and there are missing vprintf and vlog
from the body of the man page...

Aug 29 2018, 12:39 AM

Aug 28 2018

jmg added inline comments to D16690: make device_printf use sbuf.
Aug 28 2018, 9:27 PM
jmg updated the diff for D16690: make device_printf use sbuf.

add fix for when printf returns an error, pass the error up
correctly.. prf_buf does not return an error..

Aug 28 2018, 5:19 AM
jmg planned changes to D16690: make device_printf use sbuf.

All but the printf error is addressed, and the next patch will address this issue.

Aug 28 2018, 5:17 AM
jmg added a comment to D16690: make device_printf use sbuf.

Fix most of the issues. I will address the cast to int w/ a KASSERT in another update.

Aug 28 2018, 5:05 AM
jmg updated the diff for D16690: make device_printf use sbuf.

Update and address various comments.

Aug 28 2018, 5:00 AM

Aug 24 2018

jmg added a comment to D16873: Limit the harvest rate of "fast" entropy for random(4) so as not to overload the system..

simple fix, limit local_read_rate to be 0 or 1. Even more simple that the fix that was committed.

Aug 24 2018, 9:12 PM
jmg added a comment to D16873: Limit the harvest rate of "fast" entropy for random(4) so as not to overload the system..

@delphij this is my comment copied over from https://reviews.freebsd.org/D16866?id=47165 that was unaddressed.

Aug 24 2018, 5:23 PM
jmg planned changes to D16690: make device_printf use sbuf.

I'll update the patch shortly. Thanks for the review.

Aug 24 2018, 5:02 PM

Aug 23 2018

jmg added a comment to D16690: make device_printf use sbuf.

This does add some tests that were missing. I can commit those and the MLINK for _putbuf separately if people would like. I'm also fine breaking out the sys/kern/subr_bus.c changes as well.

Aug 23 2018, 4:47 PM
jmg added inline comments to D16690: make device_printf use sbuf.
Aug 23 2018, 4:46 PM
jmg updated the diff for D16690: make device_printf use sbuf.

Update. Use a new created printf drain function that works in both
userland and kernel. Document the function, and add tests for it.

Aug 23 2018, 4:45 PM
jmg added a comment to D16866: Fix excessive reseeding in random(4)..

Please commit these patches separately. They deal w/ different issues.

Aug 23 2018, 4:16 PM
jmg accepted D16860: Use arc4rand() instead of read_random().

IMO, looks good to me.

Aug 23 2018, 3:22 PM

Aug 21 2018

jmg committed rS338140: minor grammar nit, to what? between them...
minor grammar nit, to what? between them..
Aug 21 2018, 3:11 PM

Aug 20 2018

jmg planned changes to D16690: make device_printf use sbuf.

Working on testing sbuf, and adding new drain function from printing, w/ both a userland and kernel versions of the function.

Aug 20 2018, 12:32 AM

Aug 19 2018

jmg committed rS338075: use sbuf so that lines are printed together... As aarch64 often.
use sbuf so that lines are printed together... As aarch64 often
Aug 19 2018, 9:38 PM
jmg closed D16689: make arm64 identcpu use an sbuf.
Aug 19 2018, 9:38 PM

Aug 17 2018

jmg added a comment to D16690: make device_printf use sbuf.

Thanks for the comments, you solved some of the issues I knew about and didn't know how to solve best.

Aug 17 2018, 3:41 PM

Aug 16 2018

jmg added a comment to D16740: GPT is standard in x86 and arm64 land. Add it to DEFAULTS with the others..

Should powerpc64 be added too?

Aug 16 2018, 4:49 PM

Aug 12 2018

jmg created D16690: make device_printf use sbuf.
Aug 12 2018, 3:07 AM
jmg updated the summary of D16689: make arm64 identcpu use an sbuf.
Aug 12 2018, 2:59 AM
jmg created D16689: make arm64 identcpu use an sbuf.
Aug 12 2018, 2:57 AM

Aug 10 2018

jmg committed rD52098: update key now that mine expired....
update key now that mine expired...
Aug 10 2018, 3:12 AM

Aug 9 2018

jmg added a comment to D16552: random: Add PowerPC 'darn' instruction entropy source.

looks fine. Agree that conditioned output should be used. We do our own conditioning so it wouldn't be a major problem to use the raw, but the raw will likely have less entropy.

Aug 9 2018, 12:24 PM

Jul 18 2018

jmg added a comment to D16316: Consistently use explicit_bzero() for _Final methods..

looks good. Thanks.

Jul 18 2018, 10:35 PM

Jun 25 2018

jmg added a comment to D15993: change rcmds removal.

I cannot comment on this as __pure2 is not documented, so I don't know the expected semantics of adding that define.

Jun 25 2018, 11:26 PM
jmg added a comment to D15679: Remove potential identifier conflict in the EV_SET macro..

no comments...

Jun 25 2018, 11:22 PM

Jun 2 2018

jmg added a comment to D15526: reduce overhead of entropy collection.

Oh, note that the above are my opinions, and they differ significantly from most of the rest of the FreeBSD team. Mainly why I haven't spent much time working on the rng system,

Jun 2 2018, 8:45 PM
jmg added a comment to D15526: reduce overhead of entropy collection.

Collecting the first 10 packets or so every second is not a great idea. With things like tcp off-load which bursts packets, it's common for bursts of packets to be correlated.

Yes. True. Nonetheless, I'm much more interested in the questions I raised in the review. All of these implementation details are secondary to those.

Jun 2 2018, 8:42 PM
jmg added a comment to D15526: reduce overhead of entropy collection.

While certainly better than what we were doing, global pps accounting would not be efficient multi-core. I'd much rather have it collect the first 10 packets or so every second.

Jun 2 2018, 6:45 PM
jmg added a comment to D15526: reduce overhead of entropy collection.

If people want to use ethernet entropy harvesting, can we do something to make it more useful in a separate change? I was going to initially suggest the ether dst addr, but that's not very random either.

In any cases, I think these changes are a huge win. Especially moving the collection mask outside a cacheline that is written in the hot path.

Jun 2 2018, 5:45 PM
jmg added a comment to D15446: AES CCM-CBC cryptography code.
In D15446#330696, @sef wrote:

Yes, we have done this before. When I did the GCM work under contract for the FreeBSD Foundation, they paid for a third party reviewer to go over the code and make sure it was correct. So, yes, we have done this before.

Oh, so *I* don't have to go find one and pay for it. That was not clear. Phew.

Jun 2 2018, 5:23 PM
jmg added a comment to D15446: AES CCM-CBC cryptography code.
In D15446#329285, @sef wrote:

I was unable to review that the code matches an implementation, as the code does not state what implementation it implements. Even if I review it, a professional cryptographer needs to be paid to review the code before it is committed/enabled for general use.

Then there is no point to responding to anything in this, since I cannot afford to hire a cryptographer, and all my attempts to get someone with actual crypto experience to work on it were ignored or responded with "it's easy, you can just see what the aes-gcm code does."

Jun 2 2018, 5:21 PM
jmg added a comment to D15446: AES CCM-CBC cryptography code.
In D15446#329550, @mav wrote:
In D15446#329261, @jmg wrote:

Even if I review it, a professional cryptographer needs to be paid to review the code before it is committed/enabled for general use.

That's something new to me. Did we ever do that before? Who and how shall do that? I can understand when secteam@ review is requested for crypto code, that is reasonable, but who is a "professional cryptographer"? I think it was kind of agreed before that every review in this area usually starts from "I am not a cryptographer, but ..." disclaimer.

Jun 2 2018, 5:17 PM

May 27 2018

jmg added a comment to D15446: AES CCM-CBC cryptography code.

Per comments, there are a number of improvements that should be made.

May 27 2018, 10:24 PM
jmg added a comment to D15389: aesni(4): improve session lookup performance.

I agree w/ cem that we need a general solution to this. I'm fine w/ this patch, but I do question the wisdom in making this code aesni only. This code really needs to be turned into a library which any OCF driver can use, as this performance problem is not limited to aesni.

May 27 2018, 9:25 PM · secteam

Apr 23 2018

jmg added a comment to D15147: Implement 32-bit atomic_fcmpset() in userland for armv4/v5..
In D15147#319660, @jhb wrote:
In D15147#319396, @jmg wrote:

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

The code looks correct, sans all the type changes. I'd recommend committing the type changes separately from fcmpset change.

The BBB has an armv7 CPU so it uses atomic-v6.h, not this file. Only a few specific armv4/v5 boards are supported by FreeBSD and they aren't the common BBB, ${FRUIT}Pi type boards which are all based on armv6 (original RPi) or armv7/v8.

Apr 23 2018, 10:34 PM

Apr 22 2018

jmg added a comment to D15147: Implement 32-bit atomic_fcmpset() in userland for armv4/v5..

I have an spare BBB, and I can test this if you send me an image + test case. I'm not setup for building images right now.

Apr 22 2018, 6:27 PM

Mar 24 2018

jmg committed rS331478: minor work smithing....
minor work smithing...
Mar 24 2018, 4:21 AM

Mar 8 2018

jmg added a comment to D14541: Make Raspberry Pi RNG compatible with upstream DTBs.

I only briefly reviewed changes, and I don't see any issues. I did not do a security review.

Mar 8 2018, 12:51 AM

Dec 23 2017

jmg added a comment to D12813: Ensure random_source_descr[] does not get out of sync.

I don't see an issue with this change, looks good to me.

Dec 23 2017, 1:54 AM
jmg added a comment to D12889: Remove entropy harvesting from tmpfs atime.

Are you sure this is even a problem? You have to enable FS_ATIME harvest flag before it'll even be a problem, from sys/dev/random/random_harvestq.c https://svnweb.freebsd.org/base/head/sys/dev/random/random_harvestq.c?annotate=324394#l456 :

if (!(harvest_context.hc_source_mask & (1 << origin)))
        return;
Dec 23 2017, 1:53 AM

Dec 10 2017

jmg committed rD51283: switch to d/l'ing source of freebsd-update-server to be more.
switch to d/l'ing source of freebsd-update-server to be more
Dec 10 2017, 8:54 PM

Oct 28 2017

jmg added inline comments to D10680: IPSec performance increase in single flow mode by making crypto(9) multi thread.
Oct 28 2017, 8:04 AM

Sep 24 2017

jmg added a comment to D12451: crypto(9): Use a more specific error code when a capable driver is not found.
In D12451#258622, @cem wrote:
In D12451#258497, @jmg wrote:

It would be good to get a proper errors section added to the documentation.

Do you consider that an essential part of this change, or could that go in as a separate revision?

Sep 24 2017, 5:50 PM

Sep 23 2017

jmg added a comment to D10680: IPSec performance increase in single flow mode by making crypto(9) multi thread.

Only got part way through the patch. I'll continue reviewing it another time.

Sep 23 2017, 7:09 AM
jmg added a comment to D12451: crypto(9): Use a more specific error code when a capable driver is not found.

It would be good to get a proper errors section added to the documentation.

Sep 23 2017, 6:17 AM
jmg added a comment to D12452: aesni(4): Add support for x86 SHA intrinsics.
In D12452#258449, @jhb wrote:

On a general note, I would almost consider making this part of aesni.ko instead if the effective sets of CPUs that support both instruction sets is the same (or nearly so). In particular, the existing aesni.ko code does not accelerate things that combine AES with an HMAC (like an IPSec session using AES-CBC with a SHA HMAC). Having a single module that registers support for whatever algorithms the CPU supports (only SHA if AESNI isn't available for example) would permit the driver to handle these chained operations by combining AESNI with accelerated SHA hashing. (In this case aesni.ko would perhaps be better named "x86_crypto.ko" or some such to mean "accelerated software crypto for x86".)

Sep 23 2017, 6:12 AM

Sep 8 2017

jmg added a comment to D10680: IPSec performance increase in single flow mode by making crypto(9) multi thread.

I don't see any man page updates for this code. Please make sure any new capabilities, flags and features are properly documented in crypto(9). I cannot provide review on the patch till documentation is written.

Sep 8 2017, 2:04 PM

Sep 7 2017

jmg added a comment to D12132: Avoid spinning in random_harvest_queue.

So, you'll still suffer terribly with this, as random_harvest_fast is not PCPU, so you'll have the cache line bouncing around.

Sep 7 2017, 3:46 PM

Apr 28 2017

jmg abandoned D3932: PCIe HotPlug support.
Apr 28 2017, 5:42 PM
jmg added a comment to D10517: Use const with some read-only buffers in opencrypto APIs..

looks fine, have you verified that the tests in tests/sys/opencrypto pass? they are not present in your test plan.

Apr 28 2017, 5:31 PM

Apr 19 2017

jmg added a comment to D10384: Make crypto(9) multi thread.

Eventually the goal for us is to use crypto(9) from IPsec to accelerate single flows processing. Indeed IPsec does not guarantee packet ordering (neither does IP), but it would be for sure quite harmful for some end user applications if packets are not ordered.
A same crypto session may be used for several flows coming from the nic on several CPUs. It would be needed to keep the packets ordered for each flow on each CPU but it does not really matter to loss the ESP packet order in ouput, as the anti replay window handles that on the remote host.
That's why I think it would be nice for crypto(9) users to keep ordering when dispatching the jobs.

No, this is a requirement of the IPsec layer, not all users of crypto(9) require this. For example, disk encryption does not need this, as the upper layers ensures that writes are ordered correctly (ZFS and UFS both do this). And by forcing order, you are increasing latency unnecessarily for other consumers.

This isn't hard to handle at the IPsec layer. You use a TAILQ to enqueue the packets w/ a simple data structure w/ a flag that gets set when the packet is completed, then each completed packet, while the head of the tailq is ready, send it. It's not hard, and keeps the ordering logic where it belongs, or you add a flag to crypto(9) and the logic there, but you need to allow non-ordered operation.

This is maybe a bit more difficult, since we would need to reorder packets only within the flows that may share the same crypto session, but I get your idea. Maybe a reording queue per CPU would do the job, since we expect each flow to be pinned on the same CPU.

Apr 19 2017, 4:27 PM
jmg added a comment to D7876: Add Security System/Crypto (PRNG) driver for Allwinner A10/A20 SoC.

If this is truly a PRNG which it appears it is, It is not an effective source of entropy and should not be added. I'd be happy to review more information on the PRNG if you have it.

Apr 19 2017, 4:14 PM

Apr 15 2017

jmg added a comment to D10384: Make crypto(9) multi thread.
In D10384#215403, @jmg wrote:

as per other comments in the code, ordering does not have to be maintained.. w/ the async nature of callbacks, it is already assumed that the callers can handle this.

Thanks for your comments!

Eventually the goal for us is to use crypto(9) from IPsec to accelerate single flows processing. Indeed IPsec does not guarantee packet ordering (neither does IP), but it would be for sure quite harmful for some end user applications if packets are not ordered.
A same crypto session may be used for several flows coming from the nic on several CPUs. It would be needed to keep the packets ordered for each flow on each CPU but it does not really matter to loss the ESP packet order in ouput, as the anti replay window handles that on the remote host.
That's why I think it would be nice for crypto(9) users to keep ordering when dispatching the jobs.

Apr 15 2017, 6:01 AM

Apr 14 2017

jmg added a comment to D10384: Make crypto(9) multi thread.

as per other comments in the code, ordering does not have to be maintained.. w/ the async nature of callbacks, it is already assumed that the callers can handle this.

Apr 14 2017, 4:04 PM

Mar 20 2017

jmg added a comment to D10048: Replace the kernel RC4 with Chacha20..

markm, I've pointed out where the issue is.

Mar 20 2017, 1:04 AM

Mar 19 2017

jmg added a comment to D10048: Replace the kernel RC4 with Chacha20..

This will cause issues on platforms that do not use loader. We do not require loader on all of our platforms, and those that don't will have issues w/ the way chacha is started. As there is not an error (continues), this creates divergent behavior.

Mar 19 2017, 5:38 PM

Mar 5 2017

jmg committed rS314707: add missing MLINKS for functions that this man page documents..
add missing MLINKS for functions that this man page documents.
Mar 5 2017, 6:37 PM

Nov 16 2015

jmg committed rS290900: If you backup a large file that is mostly holes, previously we'd issue.
If you backup a large file that is mostly holes, previously we'd issue
Nov 16 2015, 1:30 AM
jmg retitled D4173: remove artifical tcp send/recv buffer limit, and add threads for io.. from to remove artifical tcp send/recv buffer limit, and add threads for io...
Nov 16 2015, 1:16 AM

Nov 5 2015

jmg committed rS290421: I'm still maintaining these....
I'm still maintaining these...
Nov 5 2015, 10:03 PM

Nov 4 2015

jmg updated the diff for D3933: Add /boot/entropy at install time, and be more careful with permissions.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
  • dteske says the parens are not needed.. remove them..
  • pull in dteske's version..
Nov 4 2015, 12:55 AM
jmg updated the diff for D3933: Add /boot/entropy at install time, and be more careful with permissions.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
  • dteske says the parens are not needed.. remove them..
Nov 4 2015, 12:50 AM
jmg commandeered D3933: Add /boot/entropy at install time, and be more careful with permissions.

take back so I can update and confirm my change is same..

Nov 4 2015, 12:49 AM
jmg added a comment to D1503: Use explicitly specified ivsize instead of blocksize, when we mean IV size..

has this been tested? This looks good otherwise.

Nov 4 2015, 12:37 AM
jmg added a comment to D3933: Add /boot/entropy at install time, and be more careful with permissions.

mark various parts done

Nov 4 2015, 12:05 AM
jmg updated the diff for D3933: Add /boot/entropy at install time, and be more careful with permissions.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
Nov 4 2015, 12:04 AM
jmg added a comment to D3933: Add /boot/entropy at install time, and be more careful with permissions.

for jails we don't need to create the file, but I don't see any harm in having this file created... I looked briefly at the jail script, and I don't see a good way to turn it off in the jail case...

Nov 4 2015, 12:00 AM

Oct 20 2015

jmg added inline comments to D3929: Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c.
Oct 20 2015, 11:34 PM
jmg added a comment to D3654: Avoid EINTR when debugging..

I have no issue w/ the kern_event.c change. I am have not reviewed anything else.

Oct 20 2015, 11:30 PM

Oct 19 2015

jmg added a comment to D3933: Add /boot/entropy at install time, and be more careful with permissions.

though using dd to put it in an image works, it doesn't change the file size, so I needed to delete some comments, and add white space... once I did that, I was able to verify that the script ran fine...

Oct 19 2015, 1:14 AM
jmg updated the diff for D3933: Add /boot/entropy at install time, and be more careful with permissions.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
Oct 19 2015, 1:13 AM
jmg added a comment to D3933: Add /boot/entropy at install time, and be more careful with permissions.

Well, I attempted to test this, and I don't believe that this file is being called at the end of install.

Oct 19 2015, 12:23 AM
jmg updated subscribers of D3933: Add /boot/entropy at install time, and be more careful with permissions.

I had thought that we weren't writing out even /entropy files, but @op pointed me to this file.

Oct 19 2015, 12:19 AM

Oct 18 2015

jmg retitled D3933: Add /boot/entropy at install time, and be more careful with permissions from to Add /boot/entropy at install time, and be more careful with permissions.
Oct 18 2015, 8:10 PM
jmg added a reviewer for D3932: PCIe HotPlug support: gavin.
Oct 18 2015, 7:14 PM
jmg added a comment to D3932: PCIe HotPlug support.

address some comments.

Oct 18 2015, 7:14 PM
jmg retitled D3932: PCIe HotPlug support from to PCIe HotPlug support.
Oct 18 2015, 9:13 AM
jmg committed rS289494: drop a bunch of white space at end of lines and end of files....
drop a bunch of white space at end of lines and end of files...
Oct 18 2015, 8:14 AM
jmg committed rS289492: page sized is not spelled 4096 on all arches....
page sized is not spelled 4096 on all arches...
Oct 18 2015, 8:08 AM

Oct 17 2015

jmg added a comment to D3929: Replace sys/crypto/sha2/sha2.c with lib/libmd/sha512c.c.

Just a few comments.

Oct 17 2015, 9:14 PM

Oct 12 2015

jmg added a comment to V6: Should /usr/local be included in FreeBSD's toolchain paths?.

I said Yes, but I think that we need to clean up our base libraries first. We need to make most/all libs private. If it isn't part of posix, it needs a man page which links to functions provided.

Oct 12 2015, 10:27 PM

Aug 27 2015

jmg committed rS287218: add documentation for timers that silby added in r197244, almost 6 years.
add documentation for timers that silby added in r197244, almost 6 years
Aug 27 2015, 7:12 PM

Aug 16 2015

jmg added a comment to D3390: improvements to kproc/kthread man pages.

Thanks. changed.

Aug 16 2015, 4:31 AM
jmg updated the diff for D3390: improvements to kproc/kthread man pages.
  • change a needs to to a must to be more correct.. If there are threads
Aug 16 2015, 4:30 AM

Aug 15 2015

jmg added a comment to D3390: improvements to kproc/kthread man pages.

Address all but /The/That/ change.. I understand why you suggested it, but it reads not as easily for me... We are only referencing the once example.

Aug 15 2015, 11:53 PM
jmg updated the diff for D3390: improvements to kproc/kthread man pages.
  • address comments from bjk...
Aug 15 2015, 11:51 PM
jmg updated the diff for D3390: improvements to kproc/kthread man pages.

hopefully update the diff w/ complete change..

Aug 15 2015, 11:47 PM
jmg abandoned D3394: improvements to kproc/kthread man pages.

ok, this is an issue w/ git and arc locally, not with phabric...

Aug 15 2015, 11:40 PM
jmg retitled D3394: improvements to kproc/kthread man pages from to improvements to kproc/kthread man pages.
Aug 15 2015, 11:39 PM