| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250221174249.000000cc@gmail.com>
Date: Fri, 21 Feb 2025 17:42:49 +0800
From: Furong Xu <0x1207@...il.com>
To: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel
<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Maxime
Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
<alexandre.torgue@...s.st.com>, Simon Horman <horms@...nel.org>, Russell
King <linux@...linux.org.uk>, Alexei Starovoitov <ast@...nel.org>, Daniel
Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>, Russell King
<rmk+kernel@...linux.org.uk>, Vladimir Oltean <vladimir.oltean@....com>,
Serge Semin <fancer.lancer@...il.com>, Xiaolei Wang
<xiaolei.wang@...driver.com>, Suraj Jaiswal <quic_jsuraj@...cinc.com>, Kory
Maincent <kory.maincent@...tlin.com>, Gal Pressman <gal@...dia.com>, Jesper
Nilsson <jesper.nilsson@...s.com>, Andrew Halaney <ahalaney@...hat.com>,
Choong Yong Liang <yong.liang.choong@...ux.intel.com>, Kunihiko Hayashi
<hayashi.kunihiko@...ionext.com>, Vinicius Costa Gomes
<vinicius.gomes@...el.com>, intel-wired-lan@...ts.osuosl.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org
Subject: Re: [PATCH iwl-next v5 1/9] net: ethtool: mm: extract stmmac
verification logic into common library
On Wed, 19 Feb 2025 21:53:41 -0500, Faizal Rahim <faizal.abdul.rahim@...ux.intel.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
>
> 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@....com>
> Co-developed-by: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
> Co-developed-by: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
> Tested-by: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
> ---
> 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@...il.com>
> + 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);
Powered by blists - more mailing lists