- User Since
- Jun 22 2015, 5:21 PM (134 w, 2 d)
Oct 31 2017
I think you'll get a lot less pushback if you serialize the multicast stuff in the stack, rather than the driver framework. This will allow you to put warnings / asserts into all the ioctl entry points above the drivers, so as to lock in the "you can't hold a lock while calling into a driver" rule.
Oct 23 2017
So let me try to sum up in my own words what's going on here:
Oct 17 2017
I patched this into our netflix tree. I confirmed that, even with rx timestamps enabled, the new functionality does not cause a significant performance loss in our case. There seems to be roughly one or two cache misses per mlx5e_poll_rx_cq() to check the p->priv->clbr_done > 2 condition, and roughly 8x as many for the timestamp handling itself. This pushes the cost of mlx5e_build_rx_mbuf() higher, but not horribly so.
Oct 11 2017
I have not tested it yet, but this looks fantastic. Thank you for this!
Oct 9 2017
Sep 25 2017
I think this is intended to piggyback on the recent iflib change which claims a speedup from chaining the packets. However, I'm afraid that I don't understand where this speedup is coming from. The stated reason to allow chaining in ether_input() is to allow drivers to amortize the release/acquire of the rx lock. However, no decent driver even uses an rx lock anymore, certainly not iflib or mlx5. So is there a benefit? If yes, then can you explain where it is coming from?
Sep 6 2017
Aug 31 2017
Aug 28 2017
We currently disable entropy collection (or hoarding, as you aptly describe it) for NET_ETHER. It looks like if the IFF_ NO_ENTROPY flag was present for our 100G nics, this might be slightly cheaper for us, as it will avoid the function call, and the load of the entropy mask, since if_flags will already be hot in cache from the IFF_UP check. As it is, it will be no worse .
Aug 27 2017
Aug 24 2017
Aug 11 2017
It has been years since I've had to think about this, but I remember that the problem that all drivers fight is that you wind up being called in your ioctl routine with potentially some lock held by something that is calling you, so you cannot sleep. Is that still the issue? Can you remind me what lock is held?
Aug 3 2017
Aug 1 2017
Thanks for the feedback. I'll reach out to wblock about the doc changes. I'm quite weak in man page fu, and this is a slightly odd one at that..
- Feedback from jbh regarding wording
Jul 31 2017
Jul 13 2017
I finally tried this, and sadly it does not seem to speed things up at all.
Jul 10 2017
Jul 7 2017
Jul 6 2017
Jul 5 2017
Address kib's feedback
Jul 4 2017
This seems reasonable to me
Jul 3 2017
May 18 2017
I really hate the idea of spreading the linux kpi, but I totally understand why you used it. We should probably move a lot of that stuff to a native interface.
May 15 2017
May 11 2017
I added Scott, as he's been quite involved in the busdma code. I want to make sure this looks OK to him too.
Apr 24 2017
Mar 28 2017
Feb 17 2017
Feb 15 2017
- Add sanity checking to reject bad configs.
- Make original physical domain count available
Thanks again for all the style fixes (we really need a checkpatch, a-la linux, or a c-style like opensolaris -- having hopped between OSes so much, i'm terrible at style(9). And thanks for catching the uninitialized variable.
- style fixes, as well as making sure to initialize the domain.
Feb 10 2017
Thanks for the feedback
- Address Kib's feedback
Jan 23 2017
Jan 17 2017
Nov 30 2016
Nov 18 2016
Oct 20 2016
Oct 19 2016
- we could add another 4 bytes to mbuf pkthdr and it still wouldn't fill a cacheline;
Oct 14 2016
This seems like a great idea. I don't see any downside.
Oct 13 2016
Yep.. i missed it. So nevermind then. Darn, I was hoping this would be easy.
I'm not sure where ipi_flags is initialized in the main iflib.c routine; it appears that it is used just for v4/v6 determination and those bits are OR'ed in. So having stack garbage in the flags could lead to IPI_TX_IPV4 being set, and the ixgbe driver taking the wrong offload path. Perhaps the first thing to try would be to change those OR's to simple assignments:
TSO implies checksum offload. If you could reverse the order of your test, that would be helpful. Eg, does +txcsum6 -tso6 work?
Oct 11 2016
Oct 3 2016
Sep 21 2016
Aug 18 2016
Mostly looks great. A few general comments:
I can't prove my paranoia, so I think we should get this in, as it will be an improvement on balance.
Aug 16 2016
Aug 15 2016
I tend to think type / range checking something like this is overkill.. I'm OK with it as-is..
Aug 11 2016
Aug 4 2016
Aug 2 2016
Aug 1 2016
Jul 28 2016
Fix LINT-NOINET and a few line width problems
Jul 27 2016
Update with some cleanups in reaction to comments from lstewart@
Jul 26 2016
Jul 23 2016
Jul 22 2016
- Address ae's feedback
OK, the diff is back to the way it should be. Now to try to make arc apply my change to the correct, dependant review.
- Revert other files touched in mis-placed arc diff
- Revert mistplaced arc diff
OK, so my attempt at properly using arc has failed miserably.
address feedback from ae
Thanks for the valuable input. I will uprev with my fixes later today.
Jul 21 2016
Jul 20 2016
The code looks good to me; thanks for noticing that this was not in place.
Jul 19 2016
Jul 13 2016
In addition to helping the callout correctness, this seems to improve lock contention on tcbinfo, since this lock is now taken only when needed, not when entering every routine (thereby blocking writers, or blocking when a writer has the lock).
Jun 29 2016
Jun 7 2016
Looks good in terms of not killing perf. for the sorted case, and I'm fine with it as-is. However, maybe an else would be better than a goto?