Page MenuHomeFreeBSD

markj (Mark Johnston)
User

Projects (6)

User Details

User Since
Mar 12 2014, 1:00 AM (341 w, 4 d)

Recent Activity

Fri, Sep 25

markj committed rS366170: MFC r353027 (by glebius):.
MFC r353027 (by glebius):
Fri, Sep 25, 7:11 PM
markj committed rS366167: ng_l2tp: Fix callout synchronization in the rexmit timeout handler.
ng_l2tp: Fix callout synchronization in the rexmit timeout handler
Fri, Sep 25, 6:56 PM
markj closed D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.
Fri, Sep 25, 6:56 PM
markj added a comment to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

Indeed, there seems to be a race in ng_l2tp_seq_xack_timeout() as well.

Fri, Sep 25, 6:10 PM
markj added a comment to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

My problem is, that I do not understand, how this race condition can happen at all.

Fri, Sep 25, 5:41 PM
markj accepted D26536: col(1): fix incorrect output and segfault.

Thanks for writing tests.

Fri, Sep 25, 5:32 PM
markj added a comment to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

Can you please point me to the bug report?

The PR that Aleksandr noted, https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241133

A pfsense developer also reported seeing a null pointer dereference in ng_l2tp_seq_rack_timeout(), apparently because seq->xwin[0] == 0. Apparently the panic was observed even after this commit: https://svnweb.freebsd.org/base?view=revision&revision=353027 .

Thank you for the info.

We are running this node in a large scale production environment (with remote LAC at different carriers) and did not observe such an issue before.

How common are retransmits in this environment?

We had no problems with the L2TP tunnel since PR146082 was solved. There we run into long time stability problems with L2TP sequence numbers.
Yes we have retransmits, but I did not set up an special monitoring for this, because we use sequence numbers in L2TP only for the control channel, but not for the data channel.
Typically we see a permanent difference in sequence numbers of more than one (always sending data). May be the heavy traffic hides the problem, because we usually do not run into any timeout.
Of course, we have loss of connectivity between LAC and LNS, but never had a panic with a similar stack trace.

Fri, Sep 25, 5:22 PM
markj committed rS366160: MFS r366154:.
MFS r366154:
Fri, Sep 25, 4:35 PM
markj added a comment to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

Is the bug reproducible in a test environment?

Fri, Sep 25, 4:29 PM
markj committed rS366158: MFC r365879, r365976:.
MFC r365879, r365976:
Fri, Sep 25, 2:00 PM
markj committed rS366157: MFC r365889:.
MFC r365889:
Fri, Sep 25, 1:53 PM
markj committed rD54530: Update my key, reminded by philip@..
Update my key, reminded by philip@.
Fri, Sep 25, 1:43 PM
markj committed rS366155: MFC r366005:.
MFC r366005:
Fri, Sep 25, 1:29 PM
markj committed rS366154: MFC r366005:.
MFC r366005:
Fri, Sep 25, 1:25 PM
markj accepted D26549: Enabling SO_LINGER to the so_reuseport_lb_test regression tests.
Fri, Sep 25, 1:20 PM
markj added a comment to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

Can you please point me to the bug report?

Fri, Sep 25, 1:15 PM
markj updated the diff for D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.

Fix lock leak.

Fri, Sep 25, 1:10 PM

Thu, Sep 24

markj added inline comments to D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.
Thu, Sep 24, 10:28 PM
markj updated the test plan for D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.
Thu, Sep 24, 6:41 PM
markj requested review of D26548: ng_l2tp: Fix callout synchronization in the RACK timeout handler.
Thu, Sep 24, 6:40 PM
markj added a comment to D26538: Add SIOCGIFDATA.

Looks ok to me, thanks. This seems like it would be useful for programs that run under Capsicum.

Note that pledge does not allow SIOCGIFDATA - at all.

Thu, Sep 24, 1:50 PM
markj added a comment to D26542: [ggated/ggatec] Simplify Gate Handshake..

Could you explain why using a single TCP connection improves performance? I find it easy to believe that it does, but I'm fairly ignorant of TCP internals and would like to understand the reason for it.

Thu, Sep 24, 1:25 PM
markj added a comment to D26536: col(1): fix incorrect output and segfault.

Would you be willing to add some simple regression tests for these fixes based on the examples in the code review description? There are some tests already in usr.bin/col/tests/col_test.sh. The atf-sh-api man page has info on the shell functions you can use for writing tests, and there are lots of examples in the tree.

Thu, Sep 24, 1:23 PM

Wed, Sep 23

markj added a comment to D26538: Add SIOCGIFDATA.

I'll give other folks a couple of days to have a look, and will commit if there are no objections.

Wed, Sep 23, 11:05 PM
markj added a reviewer for D26538: Add SIOCGIFDATA: network.
Wed, Sep 23, 11:05 PM
markj added a comment to D26538: Add SIOCGIFDATA.

So AFAICT NetBSD populates ifdr->ifdr_data and is properly _IOWR, OpenBSD matches this patch (and is unnecessarily _IOWR), and DragonFlyBSD has added ifru_data to struct ifreq and matches the NetBSD approach other than not having a separate struct ifdatareq

Wed, Sep 23, 10:21 PM
markj accepted D26538: Add SIOCGIFDATA.

Looks ok to me, thanks. This seems like it would be useful for programs that run under Capsicum.

Wed, Sep 23, 9:35 PM
markj accepted D26526: Add a new option (-H) to daemon(8) to catch SIGHUP and re-open output_file file when received..

Looks ok to me, my comments are just nitpicking.

Wed, Sep 23, 9:30 PM
markj added inline comments to D26538: Add SIOCGIFDATA.
Wed, Sep 23, 9:22 PM
markj added inline comments to D26538: Add SIOCGIFDATA.
Wed, Sep 23, 9:02 PM
markj added inline comments to D26538: Add SIOCGIFDATA.
Wed, Sep 23, 8:52 PM
markj added a comment to D26427: UMA: Use the bucket cache for cross-domain allocations..

Any objections to this change? I'd like to commit soon if not.

Wed, Sep 23, 7:51 PM
markj added a comment to D26426: UMA: Use LIFO for non-SMR bucket caches..

Any objections to this change? I'd like to commit soon if not.

Wed, Sep 23, 7:51 PM
markj committed rS366091: Flag vm_reserv and vm_phys sysctls as MPSAFE..
Flag vm_reserv and vm_phys sysctls as MPSAFE.
Wed, Sep 23, 7:36 PM
markj committed rS366090: Add a vmparam.h constant indicating pmap support for large pages..
Add a vmparam.h constant indicating pmap support for large pages.
Wed, Sep 23, 7:34 PM
markj closed D26467: Enable largepage support for arm64..
Wed, Sep 23, 7:34 PM
markj committed rS366089: Add largepage support to the arm64 pmap..
Add largepage support to the arm64 pmap.
Wed, Sep 23, 7:34 PM
markj closed D26466: Add largepage support to the arm64 pmap..
Wed, Sep 23, 7:34 PM
markj accepted D26525: Do not leak oldvmspace if image activation failed.
Wed, Sep 23, 4:14 PM
markj closed D26510: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3()..

Committed in r365976, I omitted the phabricator link by accident.

Wed, Sep 23, 4:12 PM
markj added a comment to D26041: install(1): Avoid unncessary fstatfs() calls and use mmap() based on size.

I have no objection to the change, it brings install(1)'s logic closer to that used in cp(1).

Do you have a benchmark which demonstrates a difference when read() is used instead of mmap() for small files?

No I don't and I doubt I'll have time to test this in the near future. My guess is that it probably doesn't make much of a difference for small files.

Wed, Sep 23, 1:52 PM
markj added a comment to D26041: install(1): Avoid unncessary fstatfs() calls and use mmap() based on size.

I have no objection to the change, it brings install(1)'s logic closer to that used in cp(1).

Wed, Sep 23, 1:35 PM
markj added inline comments to D26526: Add a new option (-H) to daemon(8) to catch SIGHUP and re-open output_file file when received..
Wed, Sep 23, 1:25 PM
markj added inline comments to D26525: Do not leak oldvmspace if image activation failed.
Wed, Sep 23, 12:58 PM

Tue, Sep 22

markj accepted D25886: Add O_RESOLVE_BENEATH and AT_RESOLVE_BENEATH to mimic Linux' RESOLVE_BENEATH.

Looks ok to me aside from the man pages.

Tue, Sep 22, 5:58 PM
markj added inline comments to D25886: Add O_RESOLVE_BENEATH and AT_RESOLVE_BENEATH to mimic Linux' RESOLVE_BENEATH.
Tue, Sep 22, 5:33 PM
markj committed rS366005: udf: Validate the full file entry length.
udf: Validate the full file entry length
Tue, Sep 22, 5:05 PM
markj updated the test plan for D26522: newlocale(3): Fix a memory leak..
Tue, Sep 22, 4:46 PM
markj requested review of D26522: newlocale(3): Fix a memory leak..
Tue, Sep 22, 4:45 PM
markj committed rS366003: ftpd: Add missing braces around a statfd check.
ftpd: Add missing braces around a statfd check
Tue, Sep 22, 4:00 PM
markj committed rS366002: tftpd: Check for errors from chdir().
tftpd: Check for errors from chdir()
Tue, Sep 22, 4:00 PM
markj updated the diff for D26467: Enable largepage support for arm64..

Assert PMAP_HAS_LARGEPAGES in vm_fault_populate().

Tue, Sep 22, 2:05 PM
markj accepted D26456: syslogd: Support locale when removing unsafe characters.

Seems reasonable to me.

Tue, Sep 22, 1:39 PM
markj added a comment to D26467: Enable largepage support for arm64..
In D26467#589671, @kib wrote:
In D26467#589425, @kib wrote:

I did not do it this way because my feel was that requires asserts in other pmap_enter() that PMAP_ENTER_LARGEPAGE is not set. It might be fine as is.

Why does the current scheme not require such assertions? I don't mind adding them to this diff, or simply extending the ifdef in kern_shm_open2(). I just prefer having a central place to indicate PMAP_ENTER_LARGEPAGE support.

I am not advocating keeping the current scheme, but for me its advantage was that there is simple place where the support for LARGEPAGE is specified per-arch. With PMAP_HAS_LARGEPAGE I need to inspect all <arch>/include/vmparam.h.

If switching to this scheme, we should provide a pattern for pmap_enter() where somebody copying current implementation to a new architecture clearly see that the flag is handled or not. My scheme makes it correct without any action from the porter.

Tue, Sep 22, 1:35 PM
markj closed D26238: Include the psind in data returned by mincore(2)..
Tue, Sep 22, 12:43 PM
markj accepted D26238: Include the psind in data returned by mincore(2)..
Tue, Sep 22, 12:43 PM
markj accepted D26513: amd64 pmap: More unification for psind = 1 vs 2 in pmap_enter_largepage()..
Tue, Sep 22, 12:16 PM
markj committed rS365994: MFC r365749:.
MFC r365749:
Tue, Sep 22, 12:15 PM
markj committed rS365985: MFS r365979:.
MFS r365979:
Tue, Sep 22, 1:36 AM

Mon, Sep 21

markj committed rS365979: Mark sysctls added in r365689 as MPSAFE..
Mark sysctls added in r365689 as MPSAFE.
Mon, Sep 21, 10:29 PM
markj closed D26507: Flag kstat sysctls as MPSAFE..
Mon, Sep 21, 10:22 PM
markj committed rS365976: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3()..
Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3().
Mon, Sep 21, 10:19 PM
markj updated the diff for D26510: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3()..

Assert that ATTR_DESCR_VALID is set if va < VM_MAX_USER_ADDRESS.

Mon, Sep 21, 9:26 PM
markj updated the diff for D26510: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3()..

Add comments.

Mon, Sep 21, 9:13 PM
markj added a comment to D26507: Flag kstat sysctls as MPSAFE..

This needs to go to stable/12 then be merged to releng/12.2 (requires approval from re@)

Mon, Sep 21, 4:00 PM
markj accepted D26132: arm64/pmap: Sparsify pv_table.
Mon, Sep 21, 3:30 PM
markj added inline comments to D26456: syslogd: Support locale when removing unsafe characters.
Mon, Sep 21, 3:28 PM
markj requested review of D26510: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3()..
Mon, Sep 21, 3:06 PM
markj added reviewers for D26510: Weaken assertions in pmap_l1_to_l2() and pmap_l2_to_l3().: mmel, kib, alc, andrew.
Mon, Sep 21, 3:06 PM
markj added a reviewer for D26507: Flag kstat sysctls as MPSAFE.: allanjude.
Mon, Sep 21, 12:48 PM
markj requested review of D26507: Flag kstat sysctls as MPSAFE..
Mon, Sep 21, 12:48 PM
markj accepted D26499: amd64 pmap: only calculate page table page when needed..
Mon, Sep 21, 1:32 AM
markj updated the diff for D26466: Add largepage support to the arm64 pmap..

Address feedback:

  • Fix assertion message.
  • Correct wired page accounting.
Mon, Sep 21, 12:33 AM
markj committed rS365933: MFS r365928:.
MFS r365928:
Mon, Sep 21, 12:31 AM

Sun, Sep 20

markj updated the diff for D26466: Add largepage support to the arm64 pmap..

Address feedback:

  • When mapping a large page, don't look up the PTP unless we need it.
  • Simplify pmap wired count updates.
  • Unconditionally issue dsb after updating a large page PTE.
Sun, Sep 20, 5:24 PM
markj added a comment to D26467: Enable largepage support for arm64..
In D26467#589425, @kib wrote:

I did not do it this way because my feel was that requires asserts in other pmap_enter() that PMAP_ENTER_LARGEPAGE is not set. It might be fine as is.

Sun, Sep 20, 5:06 PM
markj added inline comments to D26463: Fix some nits in 1G page handling..
Sun, Sep 20, 5:00 PM
markj committed rS365928: MFC r365841:.
MFC r365841:
Sun, Sep 20, 4:50 PM
markj accepted D26492: amd64 pmap: handle cases where pml4 page table page is not allocated..
Sun, Sep 20, 4:31 PM

Sat, Sep 19

markj updated the diff for D26466: Add largepage support to the arm64 pmap..

Address feedback:

  • Use pmap_unuse_pt().
  • Fix resident page accounting, second attempt.
Sat, Sep 19, 7:44 PM
markj added inline comments to D26466: Add largepage support to the arm64 pmap..
Sat, Sep 19, 7:19 PM
markj accepted D26204: fix integer underflow in getgrnam_r and getpwnam_r.

Seems reasonable to me.

Sat, Sep 19, 7:04 PM
markj added inline comments to D26204: fix integer underflow in getgrnam_r and getpwnam_r.
Sat, Sep 19, 6:33 PM
markj added inline comments to D26204: fix integer underflow in getgrnam_r and getpwnam_r.
Sat, Sep 19, 6:27 PM
markj added inline comments to D26204: fix integer underflow in getgrnam_r and getpwnam_r.
Sat, Sep 19, 6:27 PM
markj committed rS365907: Address compiler warnings in C code used by the DTrace test suite..
Address compiler warnings in C code used by the DTrace test suite.
Sat, Sep 19, 4:15 PM
markj committed rS365906: Fix some nits in 1G page support in the amd64 pmap..
Fix some nits in 1G page support in the amd64 pmap.
Sat, Sep 19, 3:22 PM
markj closed D26463: Fix some nits in 1G page handling..
Sat, Sep 19, 3:22 PM
markj added inline comments to D26466: Add largepage support to the arm64 pmap..
Sat, Sep 19, 3:16 PM
markj updated the diff for D26466: Add largepage support to the arm64 pmap..

Address review feedback.

Sat, Sep 19, 3:15 PM

Fri, Sep 18

markj committed rS365889: Install library symlinks atomically..
Install library symlinks atomically.
Fri, Sep 18, 7:03 PM
markj closed D26453: Hack to install symlinks atomically..
Fri, Sep 18, 7:03 PM
markj added a comment to D26463: Fix some nits in 1G page handling..
In D26463#589164, @kib wrote:

Hm, unless I miss something, does pmap_copy() break MINHERIT_ZERO ?

Fri, Sep 18, 6:09 PM
markj updated the diff for D26466: Add largepage support to the arm64 pmap..
  • Adjust assertion messages.
  • Implement pmap_copy() support.
Fri, Sep 18, 5:20 PM
markj updated the diff for D26463: Fix some nits in 1G page handling..

Drop a test printf().

Fri, Sep 18, 4:57 PM
markj updated the diff for D26463: Fix some nits in 1G page handling..

Implement handling for 1G pages in pmap_copy().

Fri, Sep 18, 4:52 PM
markj added a comment to D26453: Hack to install symlinks atomically..

I plan to commit this later today if there are no objections.

Fri, Sep 18, 1:26 PM
markj added a comment to D26453: Hack to install symlinks atomically..

It looks like library targets depend on .PHONY, which might explain this. I'm no make wizard though.

Fri, Sep 18, 1:26 PM
markj added a comment to D26453: Hack to install symlinks atomically..

Regarding installing it multiple times, I suspect that something is going wrong in the target up to date logic for the library.

Poking through a (meta-mode against already-built world to reduce number of rebuilt targets, but I assume the same problem exists in a "normal" build) build with "-d m" to dump out logic, I see the following:

Examining libgcc_s.so.1.full...
modified 19:54:21 Sep 16, 2020...out-of-date
Building /usr/obj/root/bdragon28-freebsd/powerpc.powerpc64/lib/libgcc_s/libgcc_s.so.1.full
--- libgcc_s.so.1.full ---
building shared library libgcc_s.so.1
Make_Update: libgcc_s.so.1.full
 recheck(libgcc_s.so.1.full): current update time: 19:55:47 Sep 16, 2020
inspect parent libgcc_s.so.1: flags 9, type b020001, made 1, unmade 1 - unmade children
inspect parent libgcc_s.so.1.debug: flags 9, type b020001, made 1, unmade 0 - libgcc_s.so.1.full made, schedule libgcc_s.so.1.debug (made 1)

....

Examining libgcc_s.so.1.full...
modified 19:55:47 Sep 16, 2020...out-of-date
Building /usr/obj/root/bdragon28-freebsd/powerpc.powerpc64/lib/libgcc_s/libgcc_s.so.1.full
--- libgcc_s.so.1.full ---
building shared library libgcc_s.so.1
Make_Update: libgcc_s.so.1.full
 recheck(libgcc_s.so.1.full): current update time: 19:55:53 Sep 16, 2020
inspect parent libgcc_s.so.1: flags 9, type b020001, made 1, unmade 1 - unmade children
inspect parent libgcc_s.so.1.debug: flags 9, type b020001, made 1, unmade 0 - libgcc_s.so.1.full made, schedule libgcc_s.so.1.debug (made 1)
NOTE: This is without this phab, I have not retested things to see if there are any differences with it.

so I wonder if make itself is getting confused about the state of the target.

Not that I understand much about how make works internally. It does make me wonder though.

Fri, Sep 18, 1:19 PM
markj closed D26465: Assert we are not traversing through superpages in the arm64 pmap..
Fri, Sep 18, 12:38 PM