Page MenuHomeFreeBSD

mjoras (Matt Joras)
User

Projects

User Details

User Since
Mar 18 2016, 8:59 PM (142 w, 3 d)

Recent Activity

Sep 14 2018

mjoras accepted D16808: fix vlan locking to permit sx acquisition in ioctl calls.

Epoch, dude.

Sep 14 2018, 4:59 PM

Aug 20 2018

mjoras added a comment to D16808: fix vlan locking to permit sx acquisition in ioctl calls.

Nice. This overall looks like a pretty great idea to me.

Aug 20 2018, 8:07 PM

Aug 16 2018

mjoras added inline comments to D16752: Add the ability to lookup the 3b PCP of a VLAN interface. Use it in toe_l2_resolve to fill up the complete vtag and not just the vid..
Aug 16 2018, 10:15 PM

May 10 2018

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

This wasn't not making progress due to the blocking review, it was not making progress since the current diff probably can't land as is. It seems everyone is in favor of the listening socket stashing approach, but I just haven't looked into it yet.

May 10 2018, 2:58 AM

Mar 29 2018

mjoras committed rS331727: MFC r325621, r325622, r331227.
MFC r325621, r325622, r331227
Mar 29 2018, 4:42 AM

Mar 19 2018

mjoras committed rS331227: Fix initialization of eventhandler mutex..
Fix initialization of eventhandler mutex.
Mar 19 2018, 10:43 PM
mjoras closed D14764: Fix initialization of eventhandler mutex..
Mar 19 2018, 10:43 PM
mjoras created D14764: Fix initialization of eventhandler mutex..
Mar 19 2018, 10:16 PM

Mar 8 2018

mjoras added a comment to D14604: Add CTASSERTs for where CTLFLAG_MPSAFE is redundant..
In D14604#307002, @jtl wrote:

imp actually just added stating not to use it to the man page in r330371, is that good enough? :) I agree that it is basically a style problem, but I don't know how else we are supposed to catch these other than periodically grepping the tree.

IMHO, the man page should also state that there is a CTASSERT for it. And, the man page should also explain why this is serious enough that we are going to CTASSERT on style violations.

Ideally, this would be in some sort of linter that we encourage people to run before they commit. Or, you can make the CTASSERT only be active for LINT builds. Or, we can deal with it the same way we deal with most other style violations: peer pressure by emailing the offender after a commit.

A CTASSERT just seems like too heavy of a hammer for this. But, maybe I'm missing the importance?

Mar 8 2018, 1:34 AM
mjoras added a comment to D14604: Add CTASSERTs for where CTLFLAG_MPSAFE is redundant..
In D14604#306835, @jtl wrote:

I agree with eliminating the pointless uses of the flag; however, I don't understand why we need a CTASSERT to catch this. It doesn't break anything. At most, it seems like it is simply a style violation. And, I don't think CTASSERTs are necessary to enforce style.

If you think there is a really good reason to add CTASSERTs, please update the man page to indicate this. At least, that will document it for developers (like me :-) ) who regularly add them out of habit.

Mar 8 2018, 12:11 AM

Mar 7 2018

mjoras created D14604: Add CTASSERTs for where CTLFLAG_MPSAFE is redundant..
Mar 7 2018, 4:42 PM
mjoras added a comment to D12050: Fix mlx5en(4) to properly call m_defrag..

Hans, did you ever get a chance to test this?

Mar 7 2018, 2:24 AM

Mar 6 2018

mjoras added a comment to D14547: Allow IPv4 subnet routes to move to a different ifa.

Care to elaborate what are you trying to achieve by moving subnet route between interfaces?

My customer is using it as a lame form of failover. If the interface with address A goes down, we can fail over the subnet route to address B (on a different interface) and address B remains functional (address A remains down, of course. I did same it was a lame form of failover).

I have no ability to push back on the customer on this point. Their position is that this configuration was supported on previous versions and therefore must remain supported.

In which "previous version" was it "supported"?

This worked on older versions of our product, which were based roughly on FreeBSD 7.x

Mar 6 2018, 4:16 AM

Feb 16 2018

mjoras added a comment to D14385: Wrap a vlan's parent's if_output in a separate function..
In D14385#301730, @ae wrote:

You can't just state it has "significant performance penalties" by looking at the code. Consider also that the path if_output takes isn't as direct as you might think anyway. When a vlan's if_output gets called you end up back in ether_output_frame. That ends up calling the interface's if_transmit, which puts you back in vlan_transmit. So the old path would have been:

Reducing inbound call path improves forwarding performance for up to 20%. Additional entries can hit performance. This is not noticeable for stock FreeBSD, that can't do more that 3-5Mpps due to lock contention. But when packet rate is about 10-12Mpps it will be significant. We can ask Olivier to try test with our patches.

Feb 16 2018, 9:32 PM

Feb 15 2018

mjoras added a comment to D14385: Wrap a vlan's parent's if_output in a separate function..

Why don't we just reject creation of vlan interface on lagg member (as opposed to lagg itself)?

Feb 15 2018, 7:01 PM
mjoras added a comment to D14385: Wrap a vlan's parent's if_output in a separate function..

The issue which this change is trying to solve is control plane issue. Dealing with it in the data path code seem to be a wrong approach. Additionally, it imposes significant performance penalties.
The better way of doing this is to have a "solver" function which is able to handle such cases. Calls to this function can be triggered by ifnet/lagg change events subscription.

Feb 15 2018, 6:58 PM
mjoras created D14385: Wrap a vlan's parent's if_output in a separate function..
Feb 15 2018, 6:18 PM

Feb 8 2018

mjoras added a comment to D11725: Add optional TCP logging on sonewconn failures..
In D11725#298967, @ae wrote:

It would be good have some progress with this review.

Feb 8 2018, 6:11 PM

Jan 26 2018

mjoras accepted D14059: Set the correct state for new neighbor cache entries.

Thanks for this. I've noticed this behavior when debugging an unrelated ND6 issue, but never dug into where it happens in the code.

Jan 26 2018, 5:24 PM

Jan 25 2018

mjoras added a comment to D14051: Remove K&R stuff: never use it in new code..

Only question I have before we remove this, is the tree void of code in this format?

If it is not then I think it would be unwise to remove this,if it is then yes, absolutely, remove this.

Jan 25 2018, 4:44 PM
mjoras accepted D14051: Remove K&R stuff: never use it in new code..
Jan 25 2018, 4:34 PM

Jan 18 2018

mjoras accepted D13975: Add info about c99 designationed initializers..
Jan 18 2018, 9:35 PM

Nov 9 2017

mjoras committed rS325622: Correct mistake in manpage..
Correct mistake in manpage.
Nov 9 2017, 11:36 PM
mjoras committed rS325621: Introduce EVENTHANDLER_LIST and some users..
Introduce EVENTHANDLER_LIST and some users.
Nov 9 2017, 10:52 PM
mjoras closed D12814: Introduce EVENTHANDLER_LIST_* and some users..
Nov 9 2017, 10:52 PM
mjoras added a comment to D12973: bnxt: While VLAN TCP Tx is progress, "ifconfig destroy" caused kernel Crash in Iflib code.

First off, please submit future patches with arc diff directly, or with a diff generated with full context.

Nov 9 2017, 8:41 PM

Nov 3 2017

mjoras updated the diff for D12814: Introduce EVENTHANDLER_LIST_* and some users..
  • in order to be MFCable we can't change the eventhandler_list layout.
Nov 3 2017, 4:47 PM

Nov 1 2017

mjoras added inline comments to D12814: Introduce EVENTHANDLER_LIST_* and some users..
Nov 1 2017, 8:41 PM
mjoras updated the diff for D12814: Introduce EVENTHANDLER_LIST_* and some users..
  • style fixups.
Nov 1 2017, 7:01 PM
mjoras updated the diff for D12814: Introduce EVENTHANDLER_LIST_* and some users..
  • manpage style and extraneous use of "relative"
  • remove the flags field as it is not needed.
Nov 1 2017, 6:57 PM
mjoras added inline comments to D12814: Introduce EVENTHANDLER_LIST_* and some users..
Nov 1 2017, 6:50 PM
mjoras updated the diff for D12814: Introduce EVENTHANDLER_LIST_* and some users..
  • style nit
Nov 1 2017, 6:12 PM
mjoras added a reviewer for D12814: Introduce EVENTHANDLER_LIST_* and some users.: ian.
Nov 1 2017, 6:04 PM
mjoras retitled D12814: Introduce EVENTHANDLER_LIST_* and some users. from Introduce EVENTHANDLER_STATIC_* and some users. to Introduce EVENTHANDLER_LIST_* and some users..
Nov 1 2017, 5:57 PM
mjoras updated the diff for D12814: Introduce EVENTHANDLER_LIST_* and some users..
  • use a pointer as the global reference to the pre-defined list, allowing for these lists to be created on-register (e.g. from a module) before they are defined. This was the approach suggested and taken by ian in his revision.
  • refactor API usage to reflect this change. LISTS are now defined and declared separately from the eventhandlers. The corresponding invoke is EVENTHANDLER_DIRECT_INVOKE since I think EVENTHANDLER_LIST_INVOKE is too confusing (whereas DIRECT can reasonably imply direct disaptch-ish semantics).
  • Now you must use EVENTHANDLER_LIST_DEFINE to create the reference to the list. EVENTHANDLER_LIST_DECLARE is only needed to declare the list pointer extern, so for callers of EVENTHANDLER_DIRECT_INVOKE that are not in the same compilation unit as the DEFINE.
  • small change to make the eventhandler list name stored at the end of the struct, since the storage is always allocated at the end of the struct anyway.
  • remove EHL_INITTED since it is no longer a relevant state (the lists are initialized either by the SYSINIT stage for eventhandler or when the first handler is registered).
  • update EVENTHANDLER(9) to reflect the API addition, with the recommendation to use the EVENTHANDLER_LIST/DIRECT macros.
Nov 1 2017, 5:47 PM

Oct 31 2017

mjoras added a comment to D12814: Introduce EVENTHANDLER_LIST_* and some users..

Seems ok to me. Do you plan to update this further in light of Ian's similar patch?

+1 to the man page update.

Oct 31 2017, 3:24 PM

Oct 30 2017

mjoras accepted D12831: bluetooth: Default to discoverable off.

Plz

Oct 30 2017, 4:50 AM

Oct 28 2017

mjoras created D12814: Introduce EVENTHANDLER_LIST_* and some users..
Oct 28 2017, 3:40 AM

Oct 23 2017

mjoras committed rS324921: Move clear_unrhdr to tmpfs_free_tmp..
Move clear_unrhdr to tmpfs_free_tmp.
Oct 23 2017, 3:43 PM
mjoras closed D12749: Move clear_unrhdr to tmpfs_free_tmp..
Oct 23 2017, 3:43 PM

Oct 22 2017

mjoras added inline comments to D12753: kernel macro cleanliness, first pass.
Oct 22 2017, 8:24 PM
mjoras added inline comments to D12753: kernel macro cleanliness, first pass.
Oct 22 2017, 3:48 PM
mjoras accepted D12753: kernel macro cleanliness, first pass.

I'd rather not turn my (perhaps radical) parentheses opinions into a bikeshed. At the least I request not adding new ones that aren't needed, but if kib or someone else prefers they stay I won't get in the way.

Oct 22 2017, 3:37 PM
mjoras added inline comments to D12753: kernel macro cleanliness, first pass.
Oct 22 2017, 3:29 PM
mjoras updated the test plan for D12749: Move clear_unrhdr to tmpfs_free_tmp..
Oct 22 2017, 1:47 AM
mjoras requested changes to D12753: kernel macro cleanliness, first pass.

The superfluous parentheses stuff is sort of nitpicky, but I do feel that at least in the case where the expansion is surrounded by commas then it should be dropped since comma operators / separators have the lowest possible precedence.

Oct 22 2017, 1:19 AM

Oct 20 2017

mjoras updated the diff for D12749: Move clear_unrhdr to tmpfs_free_tmp..
  • whitespace
Oct 20 2017, 10:48 PM
mjoras created D12749: Move clear_unrhdr to tmpfs_free_tmp..
Oct 20 2017, 10:47 PM
mjoras edited P149 Masterwork From Distant Lands.
Oct 20 2017, 2:35 AM
mjoras edited P148 Masterwork From Distant Lands.
Oct 20 2017, 1:47 AM

Oct 19 2017

mjoras edited P147 Masterwork From Distant Lands.
Oct 19 2017, 7:03 PM

Oct 16 2017

mjoras closed D12662: Properly reset fields in clean_unrhdr..
Oct 16 2017, 4:23 PM
mjoras committed rS324666: Properly reset the fields in clean_unrhdr..
Properly reset the fields in clean_unrhdr.
Oct 16 2017, 4:15 PM

Oct 14 2017

mjoras updated the diff for D12662: Properly reset fields in clean_unrhdr..
  • Might as well use TAILQ_FOREACH_SAFE while we are here.
Oct 14 2017, 11:58 PM

Oct 13 2017

mjoras updated the diff for D12662: Properly reset fields in clean_unrhdr..
  • use init_unrhdr instead of duplicating it
Oct 13 2017, 8:30 PM
mjoras created D12662: Properly reset fields in clean_unrhdr..
Oct 13 2017, 5:17 PM

Oct 11 2017

mjoras committed rS324541: Add clearing function for unr(9)..
Add clearing function for unr(9).
Oct 11 2017, 9:54 PM
mjoras committed rS324542: When unmounting a tmpfs, do not call free_unr..
When unmounting a tmpfs, do not call free_unr.
Oct 11 2017, 9:54 PM
mjoras closed D12591: Add clearing function for unr(9), call it from tmpfs..
Oct 11 2017, 9:54 PM

Oct 4 2017

mjoras updated the diff for D12591: Add clearing function for unr(9), call it from tmpfs..
  • Add comment explaining the conditional free_unr
Oct 4 2017, 8:05 PM
mjoras updated the diff for D12591: Add clearing function for unr(9), call it from tmpfs..
  • Fixup manpage.
  • Move KASSERT.
Oct 4 2017, 8:02 PM
mjoras added inline comments to D12591: Add clearing function for unr(9), call it from tmpfs..
Oct 4 2017, 7:43 PM
mjoras updated the summary of D12591: Add clearing function for unr(9), call it from tmpfs..
Oct 4 2017, 7:03 PM
mjoras created D12591: Add clearing function for unr(9), call it from tmpfs..
Oct 4 2017, 6:38 PM
mjoras created P145 (An Untitled Masterwork).
Oct 4 2017, 3:18 PM
mjoras edited P144 Masterwork From Distant Lands.
Oct 4 2017, 3:06 PM

Sep 16 2017

mjoras committed rS323633: MFC r323513:.
MFC r323513:
Sep 16 2017, 2:10 AM

Sep 13 2017

mjoras closed D12191: Allow vlan interfaces to rx through netmap(4).
Sep 13 2017, 12:25 AM
mjoras committed rS323513: Allow vlan interfaces to rx through netmap(4)..
Allow vlan interfaces to rx through netmap(4).
Sep 13 2017, 12:25 AM

Sep 12 2017

mjoras accepted D12267: Fix an infinite loop in tcp_tw_2msl_scan() when an INP_TIMEWAIT inp has been destroyed before its tcptw with INVARIANTS undefined..

For anyone wondering about the history here, see this thread from -net: https://lists.freebsd.org/pipermail/freebsd-net/2017-August/048621.html. TL;DR is this: https://lists.freebsd.org/pipermail/freebsd-net/2017-September/048826.html

Sep 12 2017, 4:41 PM
mjoras committed rS323477: MFC r322548: Rework vlan(4) locking..
MFC r322548: Rework vlan(4) locking.
Sep 12 2017, 3:54 AM

Sep 5 2017

mjoras added a comment to D12217: Add temperature sensor support for AMD Zen processors; two separate commits included this review..

Now, no one needs an amdsmn.
If some module in future will use SMN then it will be easy to share this code into module.

That's true, but what's so offensive about having another module? Why do we need to wait for abundant reasons to have logical code separation as opposed to copy paste code sharing?

Sep 5 2017, 1:00 AM

Sep 4 2017

mjoras added a comment to D12217: Add temperature sensor support for AMD Zen processors; two separate commits included this review..
In D12217#253620, @avg wrote:

I dont know where did you get info about Ryzen thermal interface, but for fam 15h 0x60 named "Miscellaneous Index" and I dont know about any renaming registers before.

Why amdsmn in new driver module?
(I dont think than any one want to do something with SMU or Miscellaneous registers)

Why do you think that there is any direction relation between SMU and Miscellaneous registers in family 15h and SMN in family 17h?
SMN is a completely new thing in family 17h (or so it seems), the only similarity between SMU and SMN is "SM" :-)
Family 17h is a big step from families 10h - 16h and even between those families there were incompatible changes in PCI register definitions (I know that for sure about registers that describe DRAM configuration).

Given the lack of documentation we can resort -- again! -- to using Linux commits by AMD employees as a guidance.
For example, https://patchwork.kernel.org/patch/9432511/

Sep 4 2017, 9:09 PM

Sep 2 2017

mjoras accepted D12217: Add temperature sensor support for AMD Zen processors; two separate commits included this review..
In D12217#253379, @cem wrote:

Yeah, that is an unfortunate problem of AMD's temperature scale. On the threadripper part, the offset is 27°C. I don't know how smart we want this thing to be. Is there a good way of determining specific model numbers? For what it's worth, I believe amdtemp had exactly the same problem on previous CPU generations.

My thought is that this is outside the scope of this revision since it's an existing problem in amdtemp. It's not totally clear to me what the best way to do it since the different parts have different offsets, but it's something someone should probably explore. If we can reliably map the offsets for every known part in every generation it would be a nice convenience feature.

Sep 2 2017, 11:51 PM
mjoras requested changes to D12217: Add temperature sensor support for AMD Zen processors; two separate commits included this review..

Also missing the sys/modules/amdsmn directory, so doesn't work with buildkernel at the moment.

Sep 2 2017, 5:49 PM

Aug 31 2017

mjoras created D12191: Allow vlan interfaces to rx through netmap(4).
Aug 31 2017, 9:35 PM
mjoras added a comment to D12188: Fix LACP with extended media..
In D12188#252958, @np wrote:

When you commit can you be more explicit about why this fixes it with the extended media types? For people not familiar with if_lagg some more specifics in the message would be helpful.

I updated the review summary, which is going to be the commit message.

Aug 31 2017, 9:08 PM
mjoras added a comment to D12188: Fix LACP with extended media..

Also unless I'm mistaken it looks like IF_BRIDGE(4) has the same bug. Should we fix that as well?

Upon further inspection, it doesn't actually matter for bridge interfaces.

Aug 31 2017, 9:02 PM
mjoras accepted D12188: Fix LACP with extended media..

When you commit can you be more explicit about why this fixes it with the extended media types? For people not familiar with if_lagg some more specifics in the message would be helpful.

Aug 31 2017, 8:52 PM

Aug 24 2017

mjoras accepted D12119: Fix 32-bit overflow on latency measurements.

The integer handling looks correct. It is definitely better than the defined-but-bad behavior of truncating the non-fractional bits of sbintime_t on ILP32. Using unsigned values everywhere makes all the operations well-defined and reasonable.

Aug 24 2017, 8:04 PM
mjoras edited P137 Masterwork From Distant Lands.
Aug 24 2017, 7:35 PM
mjoras created P136 (An Untitled Masterwork).
Aug 24 2017, 7:33 PM
mjoras edited P135 Masterwork From Distant Lands.
Aug 24 2017, 7:30 PM
mjoras added inline comments to D12119: Fix 32-bit overflow on latency measurements.
Aug 24 2017, 7:01 PM

Aug 23 2017

mjoras edited P133 Masterwork From Distant Lands.
Aug 23 2017, 5:01 PM
mjoras edited P132 Masterwork From Distant Lands.
Aug 23 2017, 12:15 AM
mjoras edited P131 Masterwork From Distant Lands.
Aug 23 2017, 12:13 AM

Aug 22 2017

mjoras edited P130 Masterwork From Distant Lands.
Aug 22 2017, 11:42 PM

Aug 20 2017

mjoras accepted D12019: subr_smp: Clean up topology analysis, add additional layers.
mjoras@icarium ~> sysctl hw.model
hw.model: AMD Ryzen Threadripper 1950X 16-Core Processor
Aug 20 2017, 5:08 AM
mjoras accepted D12082: Add powerd(8) support for AMD family 17h (Zen) CPUs.

So I will note that on my system hwpstate ends up opting to get these from acpi_perf:

kernel: hwpstate0: going to fetch info from acpi_perf
...
mjoras@icarium ~> sysctl dev.hwpstate.0.freq_settings
dev.hwpstate.0.freq_settings: 3400/3825 2800/2765 2200/1952
Aug 20 2017, 12:32 AM

Aug 16 2017

mjoras updated the diff for D12050: Fix mlx5en(4) to properly call m_defrag..
  • braces
Aug 16 2017, 7:37 PM
mjoras added inline comments to D12050: Fix mlx5en(4) to properly call m_defrag..
Aug 16 2017, 7:36 PM
mjoras created D12050: Fix mlx5en(4) to properly call m_defrag..
Aug 16 2017, 7:01 PM

Aug 15 2017

mjoras committed rS322548: Rework vlan(4) locking..
Rework vlan(4) locking.
Aug 15 2017, 5:53 PM
mjoras closed D11370: Rework vlan(4) locking. by committing rS322548: Rework vlan(4) locking..
Aug 15 2017, 5:53 PM
mjoras added a member for transport: mjoras.
Aug 15 2017, 4:52 PM
mjoras added a member for network: mjoras.
Aug 15 2017, 4:48 PM
mjoras added a comment to D11370: Rework vlan(4) locking..
In D11370#249572, @ae wrote:

I also did the same test with vlans created on top of lagg with 2x25G mellanox adapters.
I didn't see measurable performance drop there. It is able to forward 14Mpps with our RX direct vlan handling patch.

I think it is acceptable.

Aug 15 2017, 4:14 PM
mjoras added a comment to D11370: Rework vlan(4) locking..
In D11370#249511, @ae wrote:

I have tested your patch in our test environment against forwarding performance.
[packet generator] -> [ switch ] -> [ix.10 -> ix.100]

So, the FreeBSD 12 receives tagged by vlan10 packets on ixgbe(4) and then sends them into vlan100 through the same interface.
With used traffic distribution this test machine is able to forward about 1.3Mpps with and without your patch.
Then I applied our local patch to reduce RX overhead using direct vlan handling in the ixgbe(4). With this patch the same machine is able to forward 3Mpps without packet loss. With your patch this value is lowered to 2.9Mpps. Thus the locking overhead cost is about ~100kpps.
Also I think the possible panic in the vlan_input() due to the race now fixed.

Aug 15 2017, 3:15 PM

Aug 4 2017

mjoras committed rS322062: Selectively print "hwaddr" from ifconfig(8)..
Selectively print "hwaddr" from ifconfig(8).
Aug 4 2017, 9:07 PM