axgbe: Various stability improvements #1103
Conversation
Hook in RSS glue. Default to "off" for the split header feature to ensure netmap compatibility. Change the PCS indirection register values based on hardware type (ported from Linux). Move tunable settings to sysctl_init() and set the defaults there. Ensure it's called at the right time by moving it back. Reset PHY RX data path when mailbox command times out (Ported from Linux). Check if VLAN HW tagging is enabled before assuming a VLAN tag is present in a descriptor. Disable the hardware filter since multicast traffic is dropped in promisc mode. Remove unnecessary return statement. Missing sfp_get_mux, causing a race between ports to read SFP(+) sideband signals. Validate and fix incorrectly initialized polarity/configuration registers. Remove unnecessary SFP reset. axgbe_isc_rxd_pkt_get has no error state, remove unnecessary big packet check. Enable RSF to prevent zero-length packets while in Netmap mode. DMA cache coherency update (ported from Linux).
|
Did you test this on a system with armv8, like an Overdrive 3000? |
|
The patches were designed and implemented for operational problems on amd64 systems… so no. |
sys/dev/axgbe/if_axgbe_pci.c
Outdated
| #include "opt_rss.h" | ||
|
|
||
| #ifdef RSS | ||
|
|
There was a problem hiding this comment.
We don't normally have blank lines like this after the ifdef line, before the extra includes (or before the final endif.
| @@ -431,8 +441,15 @@ axgbe_if_attach_pre(if_ctx_t ctx) | |||
| sc->pdata.xpcs_res = mac_res[1]; | |||
|
|
|||
| /* Set the PCS indirect addressing definition registers*/ | |||
| pdata->xpcs_window_def_reg = PCS_V2_WINDOW_DEF; | |||
| pdata->xpcs_window_sel_reg = PCS_V2_WINDOW_SELECT; | |||
| rdev = pci_find_dbsf(0, 0, 0, 0); | |||
There was a problem hiding this comment.
So we're finding the root complex? and then seeing if it matches something?
That needs a comment explaining what is going on, even if this exact same code is in Linux (there will be a commit that we can draw from).
sys/dev/axgbe/xgbe-dev.c
Outdated
| @@ -1451,7 +1451,8 @@ xgbe_dev_read(struct xgbe_channel *channel) | |||
|
|
|||
| if (!err || !etlt) { | |||
| /* No error if err is 0 or etlt is 0 */ | |||
| if (etlt == 0x09) { | |||
| if (etlt == 0x09 && | |||
| (if_getcapenable(pdata->netdev) & IFCAP_VLAN_HWTAGGING)) { | |||
sys/dev/axgbe/xgbe-phy-v2.c
Outdated
| XGBE_PCS_PSEQ_STATE_MASK); | ||
|
|
||
| if (reg == XGBE_PCS_PSEQ_STATE_POWER_GOOD) { | ||
| /* Mailbox command timed out, reset of RX block is required. |
There was a problem hiding this comment.
The /* should be on its own line.
|
A few nits... this looks decent... |
|
I should be clear: if this is tested working on amd64, then fixing this few nits will suffice. new architecture support, while desirable, shouldn't gate it (though a known regression would, so I'll be doing a universe kernel build before landing). All the cross build tests, though, suggest it will be fine. |
|
FWIW, this code has been running fine for at least a year now with customers on OPNsense with the relevant hardware on amd64. Part of the batched changes even for a couple of years ever since the driver was extended to 10G. |
Excellent report. The only magic that needs an explanation is the root complex check... |
Hook in RSS glue. Default to "off" for the split header feature to ensure netmap compatibility. Change the PCS indirection register values based on hardware type (ported from Linux). Move tunable settings to sysctl_init() and set the defaults there. Ensure it's called at the right time by moving it back. Reset PHY RX data path when mailbox command times out (Ported from Linux). Check if VLAN HW tagging is enabled before assuming a VLAN tag is present in a descriptor. Disable the hardware filter since multicast traffic is dropped in promisc mode. Remove unnecessary return statement. Missing sfp_get_mux, causing a race between ports to read SFP(+) sideband signals. Validate and fix incorrectly initialized polarity/configuration registers. Remove unnecessary SFP reset. axgbe_isc_rxd_pkt_get has no error state, remove unnecessary big packet check. Enable RSF to prevent zero-length packets while in Netmap mode. DMA cache coherency update (ported from Linux). Reviewed by: imp Pull Request: #1103
|
Merged. I went ahead and folded the second commit you made. Generally, we squash style commits that fix errors in the other commits in the series. Thanks for updating this driver. Sorry it's taken me so long to get to closure. |
Note: this is a continuation of https://reviews.freebsd.org/D40725. First half containing only link and module compatibility changes went in with 445bed5