- User Since
- Aug 24 2017, 3:21 PM (150 w, 5 d)
Jan 3 2020
Update ice driver code
Update ice driver review to address feedback
It looks like similar work was done via r355942, with slightly different (better) function names, and various style changes. This can be closed as essentially resolved/committed now.
Dec 20 2019
Fix typo of local variable name
Dec 19 2019
Add missing break statement
Refactor check as suggested by review comments
I mean ixl_if_priv_ioctl(). I'm not sure what you mean by "that" ioctl, since ixl_if_priv_ioctl() handles multiple ioctls by virtue of not looking at the command argument. In particular, iflib passes driver-specific ioctls to ixl_if_priv_ioctl() and so would not seem to be the right place to check for privileges for exactly the reason that you state.
Dec 18 2019
I put the check directly in ixl_handle_nvmupd_cmd, similar to the check that we have in the ice driver code.
Dec 3 2019
Dec 2 2019
Hmm. I think I understand the change.. but it seems less intuitive.
Hah, thanks for fixing!
Nov 20 2019
Update test cases per review feedback
fix test cases for bit_ffc_at
dereference _result properly before trying to assign it
Nov 19 2019
add test cases to help cover the expected behavior
Updated the review to include additional test cases and fixed the manual page.
Update review based on feedback from Alan Somers
Nov 18 2019
Ok, will add tests and fix the comments.
Nov 16 2019
We use this implementation in the ice driver (current review at https://reviews.freebsd.org/D21959, will be updated to reflect moving the function here soon).
Nov 15 2019
Nov 5 2019
You should be able to verify this leak by doing something like:
I'm really not sure this is the correct fix. Possibly we should just add an ifa_free after the rtrequest1_fib()?
I found this while debugging an extra reference on the ifa refcount. I think I found a "solution" that isn't quite right, but this comment is misleading.
Nov 4 2019
Remove ETHER_IS_ZERO definitions from USB networking drivers.
Nov 1 2019
We're in agreement that the refactor is a better solution, so I'm going to abandon this one.
add an IFNET_WLOCK_ASSERT in if_freegroup
I think the refactored version to avoid code duplication is probably a better fix. If you go with this one, perhaps remove the whitespace changes and only commit the added free().
Also, yay.. yet another memory leak fixed while playing whack-a-mole trying to fix leaks during driver load/unload testing. I think we still have at least one more on M_IFADDR somewhere.
This is likely the better approach to solving the leak described in https://reviews.freebsd.org/D22215
This is the simpler straight forward fix to the same issue described in https://reviews.freebsd.org/D22214
Oct 31 2019
The fact that the map pointer can be NULL is weird, but we found this to be true in several places, and without the removal of the NULL checks we end up leaking ~200 bytes of M_DEVBUF, and associated other DMA memory allocated outside of malloc.
Oct 25 2019
We should be able to MFC this to 12-STABLE and 11-STABLE. Since the issue has existed since the beginning of iflib in-tree, I don't really think it's worth trying to rush it into 12.1... although it is a relatively small fix.
Oct 23 2019
Oct 18 2019
Updated the review to add vlan event handler unregister calls before iflib_stop as well. I kept the call in iflib_deregister as well so that they get cleaned up properly during error flows. Now it uses the "unregister -> assign NULL" pattern so that only one of the flows will actually perform an unregister check.
also move VLAN event handlers
Oct 17 2019
Assuming this fix makes sense, this should be MFC'd to 11-STABLE and 12-STABLE too, IMHO.
I did manage to eventually trigger another stale ifp pointer, though it was not reliable:
Oct 10 2019
Oct 9 2019
Use the correct register value for the received pause frames
Oct 1 2019
Sep 9 2019
I agree with Eric, we should depend on the device to be configured properly, and push to get the images for the device updated if they're wrong.
Sep 5 2019
This is specifically a fix for 11-STABLE. It's already fixed in CURRENT and STABLE-12. It looks like it was an accidental miss in MFC for STABLE-11 a few months ago.
Aug 13 2019
Use bitwise OR instead of bitwise AND to correct implementation
Aug 12 2019
Jul 31 2019
Move the kobject refcount removal to a separate patch
Jul 23 2019
Jul 19 2019
This is built on top of https://reviews.freebsd.org/D21003
May 10 2019
We discovered this as part of our rename from ixlv to iavf.