On Wed, 19 Feb 2025 21:53:41 -0500, Faizal Rahim <faizal.abdul.rahim@xxxxxxxxxxxxxxx> wrote:
> From: Vladimir Oltean <vladimir.oltean@xxxxxxx>
>
> It appears that stmmac is not the only hardware which requires a
> software-driven verification state machine for the MAC Merge layer.
>
> While on the one hand it's good to encourage hardware implementations,
> on the other hand it's quite difficult to tolerate multiple drivers
> implementing independently fairly non-trivial logic.
>
> Extract the hardware-independent logic from stmmac into library code and
> put it in ethtool. Name the state structure "mmsv" for MAC Merge
> Software Verification. Let this expose an operations structure for
> executing the hardware stuff: sync hardware with the tx_active boolean
> (result of verification process), enable/disable the pMAC, send mPackets,
> notify library of external events (reception of mPackets), as well as
> link state changes.
>
> Note that it is assumed that the external events are received in hardirq
> context. If they are not, it is probably a good idea to disable hardirqs
> when calling ethtool_mmsv_event_handle(), because the library does not
> do so.
>
> Also, the MM software verification process has no business with the
> tx_min_frag_size, that is all the driver's to handle.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> Co-developed-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx>
> Co-developed-by: Faizal Rahim <faizal.abdul.rahim@xxxxxxxxxxxxxxx>
> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@xxxxxxxxxxxxxxx>
> Tested-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 16 +-
> .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 41 +---
> .../net/ethernet/stmicro/stmmac/stmmac_fpe.c | 174 +++-----------
> .../net/ethernet/stmicro/stmmac/stmmac_fpe.h | 5 -
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +-
> include/linux/ethtool.h | 131 ++++++++++
> net/ethtool/mm.c | 225 +++++++++++++++++-
> 7 files changed, 399 insertions(+), 201 deletions(-)
>
[...]
> +void ethtool_mmsv_link_state_handle(struct ethtool_mmsv *mmsv, bool up)
> +{
> + unsigned long flags;
> +
> + ethtool_mmsv_stop(mmsv);
> +
> + spin_lock_irqsave(&mmsv->lock, flags);
> +
> + if (up && mmsv->pmac_enabled) {
> + /* VERIFY process requires pMAC enabled when NIC comes up */
> + ethtool_mmsv_configure_pmac(mmsv, true);
> +
> + /* New link => maybe new partner => new verification process */
> + ethtool_mmsv_apply(mmsv);
> + } else {
> + mmsv->status = ETHTOOL_MM_VERIFY_STATUS_INITIAL;
Tested this patch on my side, everything works well, but the verify-status
is a little weird:
# kernel booted, check initial states:
ethtool --include-statistics --json --show-mm eth1
[ {
"ifname": "eth1",
"pmac-enabled": false,
"tx-enabled": false,
"tx-active": false,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": false,
"verify-time": 128,
"max-verify-time": 128,
"verify-status": "INITIAL",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]
# Enable pMAC by: ethtool --set-mm eth1 pmac-enabled on
ethtool --include-statistics --json --show-mm eth1
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": false,
"tx-active": false,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": false,
"verify-time": 128,
"max-verify-time": 128,
"verify-status": "DISABLED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]
# Disable pMAC by: ethtool --set-mm eth1 pmac-enabled off
ethtool --include-statistics --json --show-mm eth1
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": false,
"tx-active": false,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": false,
"verify-time": 128,
"max-verify-time": 128,
"verify-status": "DISABLED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]
verify-status always normal on other cases.
@Vladimir, maybe we shouldn't update mmsv->status in ethtool_mmsv_link_state_handle()?
Or, update mmsv->status like below:
mmsv->status = mmsv->pmac_enabled ?
ETHTOOL_MM_VERIFY_STATUS_INITIAL :
ETHTOOL_MM_VERIFY_STATUS_DISABLED;
Anyway, this is too minor, so:
Tested-by: Furong Xu <0x1207@xxxxxxxxx>
> + mmsv->verify_retries = ETHTOOL_MM_MAX_VERIFY_RETRIES;
> +
> + /* No link or pMAC not enabled */
> + ethtool_mmsv_configure_pmac(mmsv, false);
> + ethtool_mmsv_configure_tx(mmsv, false);
> + }
> +
> + spin_unlock_irqrestore(&mmsv->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(ethtool_mmsv_link_state_handle);
