net: dsa: mxl862xx: statistics, PCS state and conduit TX queue fixes - #214
Open
meehien wants to merge 3 commits into
Open
net: dsa: mxl862xx: statistics, PCS state and conduit TX queue fixes#214meehien wants to merge 3 commits into
meehien wants to merge 3 commits into
Conversation
mxl862xx_stats_poll() adds the delta of tx_dropped_pkts to both rtnl_link_stats64.tx_errors and rtnl_link_stats64.tx_dropped, so every frame the switch drops on egress is reported twice, once under a field that is meant for a different thing. A drop is not an error. tx_errors is defined as the number of packets that could not be transmitted because of a fault, while tx_dropped covers packets discarded for lack of a resource - a full queue being the normal case. Conflating them makes a congested port look like a failing one, and inflates the total error count that monitoring tools key on. Drop the tx_errors contribution and leave the counter in tx_dropped, where the hardware's own naming puts it. tx_errors then has no contributor; the natural source for it would be tx_excess_coll_count, which the poll does not currently read, and adding that is a separate change. The mxl862xx driver is not upstream, so there is no commit to name in a Fixes: tag. Signed-off-by: Mihai Ordean <research@mihaiordean.com>
phylink_mac_pcs_get_state() presets state->link = 1 before invoking the pcs_get_state() callback. Both the legacy and the XPCS implementations return early when the firmware mailbox read fails (or, in the XPCS path, when the interface mode is unknown), leaving the preset link-up in place with SPEED_UNKNOWN and no negotiated pause. A transient mailbox timeout is then reported to phylink as a valid link. Initialize state->link to false on entry in both implementations so any early exit reports link down, which phylink handles correctly by retrying on the next poll. Signed-off-by: Mihai Ordean <research@mihaiordean.com>
mtk_eth_soc gives each DSA user port its own QDMA transmit queue: mtk_select_queue() maps a switch-bound frame onto queue skb_get_queue_mapping(skb) + 3, and mtk_device_event() programs that queue's scheduler entry (rate and WRR weight) from the user port's resolved link speed on every NETDEV_CHANGE. The contract, established by tag_mtk.c, is that the tagger stamps the originating port index into the skb queue mapping on xmit. Neither MxL862xx tagger does, so every frame reaches the conduit with the queue mapping still 0 from the single-queue user netdev: - all switch-bound traffic from every user port serialises onto QDMA queue 3 regardless of destination port or tagger; - queue 3 belongs to port index 0, which is not a user port on any MxL862xx board, so its scheduler entry is never programmed after mtk_dma_tx_alloc() and runs with a WRR weight of zero and no configured rate; - the per-port queues that mtk_device_event() does program (queues 4-7 for lan1-lan4 on the MxL86252C) carry no traffic at all. Stamp dp->index in both taggers, as tag_mtk does. Ports 0-12 then use their programmed per-port queues. Ports 13 and above still fold back to queue 0 in mtk_select_queue() because their queue index would exceed the 16-entry array; that is unchanged behaviour and a mtk_eth_soc limitation, not a tagger one. Signed-off-by: Mihai Ordean <research@mihaiordean.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent mxl862xx fixes, unrelated to each other and to the firmware
version. None is required to boot; each removes a real defect. They apply
cleanly on
7.1-mainin either order relative to thetag_8021qforwardingseries, so this can be merged before or after it.
The series
do not count transmit drops as transmit errorsget_stats64folds the GDM transmit-drop counter intotx_errors. Drops anderrors are separate conditions and
rtnl_link_stats64has a field for each,so a congested port currently reports errors that never occurred. One line,
ethtool -Sandip -s linkonly.report link down when the PCS state read failsphylink_mac_pcs_get_state()presetsstate->link = 1before invokingpcs_get_state(). Both the legacy and XPCS implementations return early whenthe firmware mailbox read fails — or, in the XPCS path, when the interface
mode is unknown — leaving the preset link-up in place with
SPEED_UNKNOWNand no negotiated pause. A transient mailbox timeout is then reported to
phylink as a valid link. Initialising
state->linkto false on entry makesany early exit report link down, which phylink handles by retrying on the
next poll.
tag_mxl862xx: assign per-port conduit TX queuesmtk_eth_socgives each DSA user port its own QDMA transmit queue:mtk_select_queue()maps a switch-bound frame ontoskb_get_queue_mapping(skb) + 3, andmtk_device_event()programs thatqueue's rate and WRR weight from the user port's resolved link speed. The
contract, established by
tag_mtk.c, is that the tagger stamps theoriginating port index into the skb queue mapping on xmit. Neither MxL862xx
tagger does, so every frame arrives with mapping 0 and:
regardless of destination port or tagger;
board, so its scheduler entry is never programmed after
mtk_dma_tx_alloc()— it runs with a WRR weight of zero and no configuredrate;
mtk_device_event()does program (queues 4–7 forlan1–lan4 on the MxL86252C) carry no traffic at all.
Ports 13 and above still fold back to queue 0 in
mtk_select_queue()because their queue index would exceed the 16-entry array. That is unchanged
behaviour and an
mtk_eth_soclimitation, not a tagger one.