iflib-enabled ixgbe driver update, including:
- Initial iflib conversion provided by mmacy & sbruno (D5213)
- Support for X550EM devices.
- Support for Bypass adapters.
- Flow Director code moved to separate files
- SR-IOV code moved to separate files
Details
Details
Compile-tested with minimal touch-testing for the PF driver (none for VF). Respectfully request waiting for Jeff's team to perform a validation pass before committing.
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 8709 Build 9046: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
- Moved code around to match if_ix.c and/or the OOT driver
- Wrapped KTR code with ifdef for those using GENERIC-NODEBUG
Comment Actions
- Updated interrupt setup/servicing/detection to match OOT. This fixes the issue of the interface not coming back up after unplugging/re-plugging in the module/cable.
Comment Actions
Wow that's a lot of changes.
Done an initial pass, mostly style nits but would be nice to get them fixed.
| sys/dev/ixgbe/if_bypass.c | ||
|---|---|---|
| 49 | Might be nice to extract the while loop to a little helper so you can do: atomic_cmpset_int_ensure(&adapter->bypass.low, 0, 1, 3000); | |
| 129 | Bracing issue, I would expect this to be: if ((error = hw->mac.ops.bypass_rw(hw, cmd, &version)) != 0) | |
| 133 | Bracing issue | |
| 174 | excessive bracing | |
| 187 | Bracing | |
| 195 | Why the delay? If ixgbe_bypass_mutex_clear needs time to settle then would it not be better for it to do that instead of the caller? | |
| 235 | bracing | |
| 254 | As above | |
| 273 | Should this be earlier? | |
| 277 | bracing | |
| 296 | As above | |
| 319 | bracing | |
| 338 | as above | |
| 361 | bracing | |
| 380 | as above | |
| 402 | bracing | |
| 422 | as above | |
| 457 | bracing | |
| 515 | bracing | |
| 565 | bracing | |
| 619 | IXGBE_ERR_INVALID_ARGUMENT is used above should they be the same? | |
| 622 | 0 as the value is not bool | |
| sys/dev/ixgbe/if_fdir.c | ||
| 60 | 0 | |
| sys/dev/ixgbe/if_ixv.c | ||
| 292 | == NULL | |
| 310 | Long line | |
| 345 | == NULL | |
| 352 | formatting | |
| 394 | I thought the kernel free checked this already? Same above | |
| 396 | Move above free. | |
| 434 | does this return a bool or an int if int != 0 check. | |
| 562 | Given end only does return (error); just do that here and then return (0); at the end as that makes it clearer whats happening | |
| 573 | brace return | |
| 605 | return (EINVAL); and then return (0); at the end eliminating the else and error var makes the flow clearer | |
| 855 | Indentation | |
| 891–895 | Change to an if and return (0) then followed by the default case to make flow clearer. | |
| 1204 | no need for return on void func | |
| 1226–1227 | brace | |
| 1330 | what is the 3, need a constant? | |
| 1337 | != 0 | |
| 1415 | != 0 | |
| 1416 | Long line. | |
| 1422 | != 0 or == 1? | |
| 1423 | Long line | |
| 1652 | As this always return 0 should it be a void? | |
| 1899 | return not needed | |
| sys/dev/ixgbe/if_sriov.c | ||
| 282 | break not needed | |
| 452 | == 0 | |
| 596 | == 0 | |
| 751 | == 0 | |
| 847 | always returns 0, should it be a void | |
| sys/dev/ixgbe/ix_txrx.c | ||
| 90 | != 0 | |
| 318 | != 0 | |
| 370 | Commented out code | |
| 373 | != 0 | |
| 421 | != 0 | |
| sys/dev/ixgbe/ixgbe_api.c | ||
| 1706 | != NULL | |
| sys/dev/ixgbe/ixgbe_common.c | ||
| 424 | bracing | |
| 433 | != 0 | |
| 446 | bracing | |
| 1194 | bracing more below. | |
| sys/dev/ixgbe/ixgbe_x550.c | ||
| 2809 | Return early to avoid all the indentation? | |
Comment Actions
- Building the drivers fails because of the malloc-type change I performed in a previous commit. Reverting that change.
Comment Actions
- Applied same ring accounting bug fix to ixv as was committed to the PF driver in previous commit.
- Modified ixv to indicate that we don't support full frame sizes when VLANs are enabled if the hardware is 82599 and jumbo frames is disabled.
Comment Actions
- Removed code that is already performed in the iflib layer. This should help Tx function in the VF driver.