On Mon, Feb 10, 2025 at 02:01:59AM -0500, Faizal Rahim 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 | 61 +++++
> net/ethtool/mm.c | 222 ++++++++++++++++++
> 7 files changed, 327 insertions(+), 200 deletions(-)
So I'm not exactly sure how this is supposed to work, but this is moving
a non-negligible portion of code from stmmac_fpe.c which has Furong Xu's
copyright, to ethtool/mm.c which doesn't. That being said, commit
8d43e99a5a03 ("net: stmmac: refactor FPE verification process") which
added that logic said that it was co-developed together with me (NXP),
and I clearly remember both of us contributing to it. So, how about
expanding NXP's current 2022-2023 copyright in ethtool/mm.c to
2022-2025 (date of commit 8d43e99a5a03 plus date of this patch), and
also copy Furong's copyright 2024 line?
> @@ -710,6 +714,63 @@ struct ethtool_mm_stats {
> u64 MACMergeHoldCount;
> };
>
> +enum ethtool_mmsv_event {
> + ETHTOOL_MMSV_LP_SENT_VERIFY_MPACKET,
> + ETHTOOL_MMSV_LD_SENT_VERIFY_MPACKET,
> + ETHTOOL_MMSV_LP_SENT_RESPONSE_MPACKET,
> +};
> +
> +/* MAC Merge verification mPacket type */
> +enum ethtool_mpacket {
> + ETHTOOL_MPACKET_VERIFY,
> + ETHTOOL_MPACKET_RESPONSE,
> +};
> +
> +struct ethtool_mmsv;
> +
> +struct ethtool_mmsv_ops {
Since these are driver-facing API, how about a kernel-doc? The content
is subject to further review comments, of course.
/**
* struct ethtool_mmsv_ops - Operations for MAC Merge Software Verification
* @configure_tx: Driver callback for the event where the preemptible TX
* becomes active or inactive. Preemptible traffic
* classes must be committed to hardware only while
* preemptible TX is active.
* @configure_pmac: Driver callback for the event where the pMAC state
* changes as result of an administrative setting
* (ethtool) or a call to ethtool_mmsv_link_state_handle().
* @send_mpacket: Driver-provided method for sending a Verify or a Response
* mPacket.
*/
> + void (*configure_tx)(struct ethtool_mmsv *mmsv, bool tx_active);
> + void (*configure_pmac)(struct ethtool_mmsv *mmsv, bool pmac_enabled);
> + void (*send_mpacket)(struct ethtool_mmsv *mmsv, enum ethtool_mpacket mpacket);
> +};
> +
> +/**
> + * struct ethtool_mmsv - MAC Merge Software Verification
> + * @ops: operations for MAC Merge Software Verification
> + * @dev: pointer to net_device structure
> + * @lock: serialize access to MAC Merge state between
> + * ethtool requests and link state updates.
> + * @status: current verification FSM state
> + * @verify_timer: timer for verification in local TX direction
> + * @verify_enabled: indicates if verification is enabled
> + * @verify_retries: number of retries for verification
> + * @pmac_enabled: indicates if the preemptible MAC is enabled
> + * @verify_time: time for verification in milliseconds
> + * @tx_enabled: indicates if transmission is enabled
> + */
> +struct ethtool_mmsv {
> + const struct ethtool_mmsv_ops *ops;
> + struct net_device *dev;
> + spinlock_t lock;
> + enum ethtool_mm_verify_status status;
> + struct timer_list verify_timer;
> + bool verify_enabled;
> + int verify_retries;
> + bool pmac_enabled;
> + u32 verify_time;
> + bool tx_enabled;
> +};
> +
/**
* ethtool_mmsv_stop() - Stop MAC Merge Software Verification
* @mmsv: MAC Merge Software Verification state
*
* Drivers should call this method in a state where the hardware is
* about to lose state, like ndo_stop() or suspend(), and turning off
* MAC Merge features would be superfluous. Otherwise, prefer
* ethtool_mmsv_link_state_handle() with up=false.
*/
> +void ethtool_mmsv_stop(struct ethtool_mmsv *mmsv);
/**
* ethtool_mmsv_link_state_handle() - Inform MAC Merge Software Verification
* of link state changes
* @mmsv: MAC Merge Software Verification state
* @up: True if device carrier is up and able to pass verification packets
*
* Calling context is expected to be from a thread, interrupts enabled.
*/
> +void ethtool_mmsv_link_state_handle(struct ethtool_mmsv *mmsv, bool up);
/**
* ethtool_mmsv_event_handle() - Inform MAC Merge Software Verification
* of interrupt-based events
* @mmsv: MAC Merge Software Verification state
* @event: Event which took place (packet transmission or reception)
*
* Calling context expects to have interrupts disabled.
*/
> +void ethtool_mmsv_event_handle(struct ethtool_mmsv *mmsv,
> + enum ethtool_mmsv_event event);
/**
* ethtool_mmsv_get_mm() - get_mm() hook for MAC Merge Software Verification
* @mmsv: MAC Merge Software Verification state
* @state: see struct ethtool_mm_state
*
* Drivers are expected to call this from their ethtool_ops :: get_mm()
* method.
*/
> +void ethtool_mmsv_get_mm(struct ethtool_mmsv *mmsv,
> + struct ethtool_mm_state *state);
/**
* ethtool_mmsv_set_mm() - set_mm() hook for MAC Merge Software Verification
* @mmsv: MAC Merge Software Verification state
* @state: see struct ethtool_mm_state
*
* Drivers are expected to call this from their ethtool_ops :: set_mm()
* method.
*/
> +void ethtool_mmsv_set_mm(struct ethtool_mmsv *mmsv, struct ethtool_mm_cfg *cfg);
/**
* ethtool_mmsv_init() - Initialize MAC Merge Software Verification state
* @mmsv: MAC Merge Software Verification state
* @dev: Pointer to network interface
* @ops: Methods for implementing the generic functionality
*
* The MAC Merge Software Verification is a timer- and event-based state
* machine intended for network interfaces which lack a hardware-based
* TX verification process (as per IEEE 802.3 clause 99.4.3). The timer
* is managed by the core code, whereas events are supplied by the
* driver explicitly calling one of the other API functions.
*/
> +void ethtool_mmsv_init(struct ethtool_mmsv *mmsv, struct net_device *dev,
> + const struct ethtool_mmsv_ops *ops);
> +
> /**
> * struct ethtool_rxfh_param - RXFH (RSS) parameters
> * @hfunc: Defines the current RSS hash function used by HW (or to be set to).
