net: stmmac: FPE via ethtool + tc
Subsystems (4)
v8 cover net: stmmac: FPE via ethtool + tc
Move the Frame Preemption(FPE) over to the new standard API which uses ethtool-mm/tc-mqprio/tc-taprio.
Changes in v8:
1. use timer_delete_sync() instead of deprecated del_timer_sync()
2. check netif_running() to guarantee synchronization rules between mod_timer() and timer_delete_sync()
3. split up stmmac_tc_ops of dwmac4, dwmac4+ and dwxgmac to give user more descriptive error message
4. fix wrong indentation about switch-case
5. delete more unbalanced logs
Changes in v7:
1. code style fixes and clean up warnings reported by patchwork netdev checks, no functional change intended
Changes in v6:
1. new FPE verification process based on Vladimir Oltean's proposal
2. embed ethtool_mm_state into stmmac_fpe_cfg
3. convert some bit ops to u32_replace_bits
4. register name and function name update to be more descriptive
5. split up stmmac_tc_ops of dwmac4+ and dwxgmac, they have different implementations about mqprio
6. some code style fixes
Changes in v5:
1. fix typo in commit message
2. drop FPE capability check in tc-mqprio/tc-taprio
Changes in v4:
1. reorder FPE-related declarations and definitions into clean groups
2. move mm_lock to stmmac_fpe_cfg.lock
3. protect user configurations across NIC up/down
4. block stmmac_set_mm() when fpe_task is in progress to finish
5. convert to ethtool_dev_mm_supported() to check FPE capability in tc-mqprio/tc-taprio
6. silence FPE workqueue start/stop logs
Changes in v3:
1. avoid races among ISR, workqueue, link update and register configuration.
2. update FPE verification retry logic, so it retries and fails as expected.
Changes in v2:
1. refactor FPE verification process
2. suspend/resume and kselftest-ethtool_mm, all test cases passed
3. handle TC:TXQ remapping for DWMAC CORE4+
Furong Xu (7):
net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
net: stmmac: drop stmmac_fpe_handshake
net: stmmac: refactor FPE verification process
net: stmmac: configure FPE via ethtool-mm
net: stmmac: support fp parameter of tc-mqprio
net: stmmac: support fp parameter of tc-taprio
net: stmmac: silence FPE kernel logs
.../net/ethernet/stmicro/stmmac/dwmac4_core.c | 10 +- drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 96 +++++- drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 12 +- .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/hwif.c | 6 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 22 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 30 +- .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 91 ++++++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 276 ++++++++---------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 153 +++++++--- include/linux/stmmac.h | 28 -- 11 files changed, 491 insertions(+), 242 deletions(-)
Signature · 1 line
2.34.1
PATCH v8 1/7 net: stmmac: move stmmac_fpe_cfg to stmmac_priv data
By moving the fpe_cfg field to the stmmac_priv data, stmmac_fpe_cfg becomes platform-data eventually, instead of a run-time config.
--- drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 ++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 30 ++++++++++++++++++- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 ++++++------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 16 ++-------- include/linux/stmmac.h | 28 ----------------- 5 files changed, 44 insertions(+), 52 deletions(-)
drivers/net/ethernet/stmicro/stmmac/hwif.h +2 -0
@@ -28,6 +28,8 @@struct stmmac_extra_stats ;struct stmmac_priv ;struct stmmac_safety_stats ; +struct stmmac_fpe_cfg ; +enum stmmac_mpacket_type ;struct dma_desc ;struct dma_extended_desc ;struct dma_edesc ;
drivers/net/ethernet/stmicro/stmmac/stmmac.h +29 -1
@@ -146,6 +146,33 @@ struct stmmac_channel {u32 index; }; +/* FPE link state */ +enum stmmac_fpe_state { +FPE_STATE_OFF =0 , +FPE_STATE_CAPABLE =1 , +FPE_STATE_ENTERING_ON =2 , +FPE_STATE_ON =3 , +}; + +/* FPE link-partner hand-shaking mPacket type */ +enum stmmac_mpacket_type { +MPACKET_VERIFY =0 , +MPACKET_RESPONSE =1 , +}; + +enum stmmac_fpe_task_state_t { + __FPE_REMOVING, + __FPE_TASK_SCHED, +}; + +struct stmmac_fpe_cfg { +bool enable;/* FPE enable */ +bool hs_enable;/* FPE handshake enable */ +enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ +enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ +u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ +}; +struct stmmac_tc_entry {bool in_use;bool in_hw;
@@ -339,11 +366,12 @@ struct stmmac_priv {struct workqueue_struct *wq;struct work_struct service_task; -/* Workqueue for handling FPE hand-shaking */ +/* Frame Preemption feature (FPE) */ unsigned long fpe_task_state;struct workqueue_struct *fpe_wq;struct work_struct fpe_task;char wq_name[IFNAMSIZ +4 ]; +struct stmmac_fpe_cfg fpe_cfg;/* TC Handling */ unsigned int tc_entries_max;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10 -10
@@ -968,7 +968,7 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,static void stmmac_fpe_link_state_handle (struct stmmac_priv *priv,bool is_up) { -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -3536,7 +3536,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)if (priv->dma_cap.fpesel) {stmmac_fpe_start_wq (priv); -if (priv->plat->fpe_cfg->enable) +if (priv->fpe_cfg.enable)stmmac_fpe_handshake (priv, true); }
@@ -5982,7 +5982,7 @@ static int stmmac_set_features(struct net_device *netdev,static void stmmac_fpe_event_status (struct stmmac_priv *priv,int status) { -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -7381,7 +7381,7 @@ static void stmmac_fpe_lp_task(struct work_struct *work) { struct stmmac_priv *priv = container_of(work, struct stmmac_priv, fpe_task); -struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;enum stmmac_fpe_state *lo_state = &fpe_cfg->lo_fpe_state;enum stmmac_fpe_state *lp_state = &fpe_cfg->lp_fpe_state;bool *hs_enable = &fpe_cfg->hs_enable;
@@ -7427,17 +7427,17 @@ static void stmmac_fpe_lp_task(struct work_struct *work)void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable) { -if (priv->plat->fpe_cfg->hs_enable != enable) { +if (priv->fpe_cfg.hs_enable != enable) {if (enable) {stmmac_fpe_send_mpacket (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg,MPACKET_VERIFY ); }else { - priv->plat->fpe_cfg->lo_fpe_state =FPE_STATE_OFF ; - priv->plat->fpe_cfg->lp_fpe_state =FPE_STATE_OFF ; + priv->fpe_cfg.lo_fpe_state =FPE_STATE_OFF ; + priv->fpe_cfg.lp_fpe_state =FPE_STATE_OFF ; } - priv->plat->fpe_cfg->hs_enable = enable; + priv->fpe_cfg.hs_enable = enable; } }
@@ -7898,7 +7898,7 @@ int stmmac_suspend(struct device *dev)if (priv->dma_cap.fpesel) {/* Disable FPE */ stmmac_fpe_configure (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false);
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +3 -13
@@ -282,16 +282,6 @@ static int tc_init(struct stmmac_priv *priv)if (ret)return -ENOMEM ; -if (!priv->plat->fpe_cfg) { - priv->plat->fpe_cfg =devm_kzalloc (priv->device, -sizeof (*priv->plat->fpe_cfg), -GFP_KERNEL ); -if (!priv->plat->fpe_cfg) -return -ENOMEM ; - }else { -memset (priv->plat->fpe_cfg,0 ,sizeof (*priv->plat->fpe_cfg)); - } -/* Fail silently as we can still use remaining features, e.g. CBS */ if (!dma_cap->frpsel)return 0 ;
@@ -1076,7 +1066,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,/* Actual FPE register configuration will be done after FPE handshake * is success. */ - priv->plat->fpe_cfg->enable = fpe; + priv->fpe_cfg.enable = fpe; ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);
@@ -1109,9 +1099,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } - priv->plat->fpe_cfg->enable = false; + priv->fpe_cfg.enable = false;stmmac_fpe_configure (priv, priv->ioaddr, - priv->plat->fpe_cfg, + &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false);
include/linux/stmmac.h +0 -28
@@ -138,33 +138,6 @@ struct stmmac_txq_cfg {int tbs_en; }; -/* FPE link state */ -enum stmmac_fpe_state { -FPE_STATE_OFF =0 , -FPE_STATE_CAPABLE =1 , -FPE_STATE_ENTERING_ON =2 , -FPE_STATE_ON =3 , -}; - -/* FPE link-partner hand-shaking mPacket type */ -enum stmmac_mpacket_type { -MPACKET_VERIFY =0 , -MPACKET_RESPONSE =1 , -}; - -enum stmmac_fpe_task_state_t { - __FPE_REMOVING, - __FPE_TASK_SCHED, -}; - -struct stmmac_fpe_cfg { -bool enable;/* FPE enable */ -bool hs_enable;/* FPE handshake enable */ -enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ -enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ -u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ -}; -struct stmmac_safety_feature_cfg {u32 tsoee;u32 mrxpee;
@@ -232,7 +205,6 @@ struct plat_stmmacenet_data {struct fwnode_handle *port_node;struct device_node *mdio_node;struct stmmac_dma_cfg *dma_cfg; -struct stmmac_fpe_cfg *fpe_cfg;struct stmmac_safety_feature_cfg *safety_feat_cfg;int clk_csr;int has_gmac;
Signature · 1 line
2.34.1
PATCH v8 2/7 net: stmmac: drop stmmac_fpe_handshake
In reply to Furong Xuethtool --set-mm can trigger FPE verification process by calling stmmac_fpe_send_mpacket, stmmac_fpe_handshake should be gone.
--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 - .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +------------------ .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 ------- 3 files changed, 1 insertion(+), 31 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 -1
@@ -425,7 +425,6 @@ bool stmmac_eee_init(struct stmmac_priv *priv);int stmmac_reinit_queues (struct net_device *dev,u32 rx_cnt,u32 tx_cnt);int stmmac_reinit_ringparam (struct net_device *dev,u32 rx_size,u32 tx_size);int stmmac_bus_clks_config (struct stmmac_priv *priv,bool enabled); -void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable);static inline bool stmmac_xdp_is_enabled (struct stmmac_priv *priv) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -22
@@ -3533,13 +3533,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)stmmac_set_hw_vlan_mode (priv, priv->hw); -if (priv->dma_cap.fpesel) { +if (priv->dma_cap.fpesel)stmmac_fpe_start_wq (priv); -if (priv->fpe_cfg.enable) -stmmac_fpe_handshake (priv, true); - } -return 0 ; }
@@ -7425,22 +7421,6 @@ static void stmmac_fpe_lp_task(struct work_struct *work)clear_bit (__FPE_TASK_SCHED, &priv->fpe_task_state); } -void stmmac_fpe_handshake (struct stmmac_priv *priv,bool enable) -{ -if (priv->fpe_cfg.hs_enable != enable) { -if (enable) { -stmmac_fpe_send_mpacket (priv, priv->ioaddr, - &priv->fpe_cfg, -MPACKET_VERIFY ); - }else { - priv->fpe_cfg.lo_fpe_state =FPE_STATE_OFF ; - priv->fpe_cfg.lp_fpe_state =FPE_STATE_OFF ; - } - - priv->fpe_cfg.hs_enable = enable; - } -} -static int stmmac_xdp_rx_timestamp (const struct xdp_md *_ctx,u64 *timestamp) {const struct stmmac_xdp_buff *ctx = (void *)_ctx;
@@ -7902,7 +7882,6 @@ int stmmac_suspend(struct device *dev) priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, false); -stmmac_fpe_handshake (priv, false);stmmac_fpe_stop_wq (priv); }
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +0 -8
@@ -1078,11 +1078,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,netdev_info (priv->dev,"configured EST\n" ); -if (fpe) { -stmmac_fpe_handshake (priv, true); -netdev_info (priv->dev,"start FPE handshake\n" ); - } -return 0 ; disable:
@@ -1107,9 +1102,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv, false);netdev_info (priv->dev,"disabled FPE\n" ); -stmmac_fpe_handshake (priv, false); -netdev_info (priv->dev,"stop FPE handshake\n" ); -return ret; }
Signature · 1 line
2.34.1
PATCH v8 3/7 net: stmmac: refactor FPE verification process
In reply to Furong XuDrop driver defined stmmac_fpe_state, and switch to common ethtool_mm_verify_status for local TX verification status.
Local side and remote side verification processes are completely independent. There is no reason at all to keep a local state and a remote state.
Add a spinlock to avoid races among ISR, timer, link update and register configuration.
This patch is based on Vladimir Oltean's proposal.
Vladimir Oltean says:
====================
In the INITIAL state, the timer sends MPACKET_VERIFY. Eventually the stmmac_fpe_event_status() IRQ fires and advances the state to VERIFYING, then rearms the timer after verify_time ms. If a subsequent IRQ comes in and modifies the state to SUCCEEDED after getting MPACKET_RESPONSE, the timer sees this. It must enable the EFPE bit now. Otherwise, it decrements the verify_limit counter and tries again. Eventually it moves the status to FAILED, from which the IRQ cannot move it anywhere else, except for another stmmac_fpe_apply() call.
====================
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 4 - drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 19 +- drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 2 +- .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 9 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 31 +-- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 259 +++++++++--------- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 8 +- 8 files changed, 165 insertions(+), 169 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +0 -4
@@ -58,10 +58,6 @@ static void dwmac4_core_init(struct mac_device_info *hw,if (hw->pcs) value |=GMAC_PCS_IRQ_DEFAULT ; -/* Enable FPE interrupt */ -if ((GMAC_HW_FEAT_FPESEL &readl (ioaddr +GMAC_HW_FEATURE3 )) >>26 ) - value |=GMAC_INT_FPE_EN ; -writel (value, ioaddr +GMAC_INT_EN );if (GMAC_INT_DEFAULT_ENABLE &GMAC_INT_TSIE )
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +17 -2
@@ -575,11 +575,11 @@ int dwmac5_flex_pps_config(void __iomem *ioaddr, int index,void dwmac5_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable) +bool tx_enable,bool pmac_enable) {u32 value; -if (enable) { +if (tx_enable) { cfg->fpe_csr =EFPE ; value =readl (ioaddr +GMAC_RXQ_CTRL1 ); value &= ~GMAC_RXQCTRL_FPRQ ;
@@ -589,6 +589,21 @@ void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg, cfg->fpe_csr =0 ; }writel (cfg->fpe_csr, ioaddr +MAC_FPE_CTRL_STS ); + + value =readl (ioaddr +GMAC_INT_EN ); + +if (pmac_enable) { +if (!(value &GMAC_INT_FPE_EN )) { +/* Dummy read to clear any pending masked interrupts */ +readl (ioaddr +MAC_FPE_CTRL_STS ); + + value |=GMAC_INT_FPE_EN ; + } + }else { + value &= ~GMAC_INT_FPE_EN ; + } + +writel (value, ioaddr +GMAC_INT_EN ); }int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev)
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +1 -1
@@ -104,7 +104,7 @@ int dwmac5_flex_pps_config(void __iomem *ioaddr, int index, u32 sub_second_inc, u32 systime_flags);void dwmac5_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable); +bool tx_enable,bool pmac_enable);void dwmac5_fpe_send_mpacket (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,enum stmmac_mpacket_type type);
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +5 -4
@@ -1504,13 +1504,14 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,writel (value, ioaddr +XGMAC_RX_CONFIG ); } -static void dwxgmac3_fpe_configure (void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg, -u32 num_txq, -u32 num_rxq,bool enable) +static void dwxgmac3_fpe_configure (void __iomem *ioaddr, +struct stmmac_fpe_cfg *cfg, +u32 num_txq,u32 num_rxq, +bool tx_enable,bool pmac_enable) {u32 value; -if (!enable) { +if (!tx_enable) { value =readl (ioaddr +XGMAC_FPE_CTRL_STS ); value &= ~XGMAC_EFPE ;
drivers/net/ethernet/stmicro/stmmac/hwif.h +1 -1
@@ -421,7 +421,7 @@ struct stmmac_ops {void (*set_arp_offload)(struct mac_device_info *hw,bool en,u32 addr);void (*fpe_configure)(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,u32 num_txq,u32 num_rxq, -bool enable); +bool tx_enable,bool pmac_enable);void (*fpe_send_mpacket)(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg,enum stmmac_mpacket_type type);
drivers/net/ethernet/stmicro/stmmac/stmmac.h +11 -20
@@ -146,31 +146,25 @@ struct stmmac_channel {u32 index; }; -/* FPE link state */ -enum stmmac_fpe_state { -FPE_STATE_OFF =0 , -FPE_STATE_CAPABLE =1 , -FPE_STATE_ENTERING_ON =2 , -FPE_STATE_ON =3 , -}; -/* FPE link-partner hand-shaking mPacket type */ enum stmmac_mpacket_type {MPACKET_VERIFY =0 ,MPACKET_RESPONSE =1 , }; -enum stmmac_fpe_task_state_t { - __FPE_REMOVING, - __FPE_TASK_SCHED, -}; +#define STMMAC_FPE_MM_MAX_VERIFY_RETRIES 3 +#define STMMAC_FPE_MM_MAX_VERIFY_TIME_MS 128struct stmmac_fpe_cfg { -bool enable;/* FPE enable */ -bool hs_enable;/* FPE handshake enable */ -enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ -enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ +/* Serialize access to MAC Merge state between ethtool requests +* and link state updates. +*/ +spinlock_t lock; +u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ +struct timer_list verify_timer; +struct ethtool_mm_state state; +int verify_retries; };struct stmmac_tc_entry {
@@ -367,10 +361,6 @@ struct stmmac_priv {struct work_struct service_task;/* Frame Preemption feature (FPE) */ -unsigned long fpe_task_state; -struct workqueue_struct *fpe_wq; -struct work_struct fpe_task; -char wq_name[IFNAMSIZ +4 ];struct stmmac_fpe_cfg fpe_cfg;/* TC Handling */
@@ -425,6 +415,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv);int stmmac_reinit_queues (struct net_device *dev,u32 rx_cnt,u32 tx_cnt);int stmmac_reinit_ringparam (struct net_device *dev,u32 rx_size,u32 tx_size);int stmmac_bus_clks_config (struct stmmac_priv *priv,bool enabled); +void stmmac_fpe_apply (struct stmmac_priv *priv);static inline bool stmmac_xdp_is_enabled (struct stmmac_priv *priv) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +129 -130
338 lines not on this page · raw message
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +1 -7
@@ -1063,11 +1063,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,return -EOPNOTSUPP ; } -/* Actual FPE register configuration will be done after FPE handshake -* is success. -*/ - priv->fpe_cfg.enable = fpe; - ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);mutex_unlock (&priv->est_lock);
@@ -1094,12 +1089,11 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } - priv->fpe_cfg.enable = false;stmmac_fpe_configure (priv, priv->ioaddr, &priv->fpe_cfg, priv->plat->tx_queues_to_use, priv->plat->rx_queues_to_use, - false); + false, false);netdev_info (priv->dev,"disabled FPE\n" );return ret;
Signature · 1 line
2.34.1
Re: [PATCH net-next v8 3/7] net: stmmac: refactor FPE verification process
Quoted 12 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12
-0+void stmmac_fpe_apply (struct stmmac_priv *priv) +{ +struct ethtool_mm_state *state = &priv->fpe_cfg.state; +struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg; + +/* If verification is disabled, configure FPE right away. +* Otherwise let the timer code do it. +*/ +if (!state->verify_enabled) { + stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg, + priv->plat->tx_queues_to_use, + priv->plat->rx_queues_to_use,
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size) · line 7423
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +9
-0+ state->tx_enabled, + state->pmac_enabled); + }else { + state->verify_status =ETHTOOL_MM_VERIFY_STATUS_INITIAL ; + fpe_cfg->verify_retries =STMMAC_FPE_MM_MAX_VERIFY_RETRIES ; + +if (netif_device_present (priv->dev) &&netif_running (priv->dev)) +stmmac_fpe_verify_timer_arm (fpe_cfg); + } }
In the cover letter, you say:
2. check netif_running() to guarantee synchronization rules between mod_timer() and timer_delete_sync()
[ by the way, it would be nice if you could list the changes in
individual patches as well ]
but I guess this helps with something other than what you say it helps with.
netif_running() essentially checks that __dev_open() has been called, aka "ip link set dev eth0 up". And I don't see the ethtool_ops :: begin() implemented by the driver any longer, so I think you've done this in order to accept stmmac_set_mm() calls even before the netdev has been brought operationally up. Okay.
As for netif_device_present(), I don't know, maybe the intention was to suppress stmmac_set_mm() calls made after stmmac_suspend(). But ethnl_ops_begin() has its own netif_device_present() call, so I'm not sure why it is needed - they should already be suppressed.
But in v7, I was thinking about the concurrency issues here:
static int stmmac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
struct netlink_ext_ack *extack)
{
/* Wait for the verification that's currently in progress to finish */
del_timer_sync(&fpe_cfg->verify_timer);
<- Concurrent code can run here:
stmmac_fpe_link_state_handle(),
called from phylink_resolve()
workqueue context, rtnl_lock()
not held.
spin_lock_irqsave(&fpe_cfg->lock, flags);
stmmac_fpe_apply(priv);
spin_unlock_irqrestore(&fpe_cfg->lock, flags);
}static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
{
struct stmmac_fpe_cfg *fpe_cfg = &priv->fpe_cfg;
unsigned long flags;
timer_delete_sync(&fpe_cfg->verify_timer);
<- Concurrent code can run here:
stmmac_set_mm()
spin_lock_irqsave(&fpe_cfg->lock, flags);
if (is_up && fpe_cfg->state.pmac_enabled) {
/* VERIFY process requires pmac enabled when NIC comes up */
stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
false, true);
/* New link => maybe new partner => new verification process */
stmmac_fpe_apply(priv);
} else {
/* No link => turn off EFPE */
stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
false, false);
}
spin_unlock_irqrestore(&fpe_cfg->lock, flags);
}[ oh btw, you forgot to replace the del_timer_sync() instance from
stmmac_set_mm() to timer_delete_sync() ]
Because the timer can be restarted right after the timer_delete_sync() call, this is a half-baked implementation.
I think at the end of the day, we need to ask ourselves: what is the timer_delete_sync() call even supposed to accomplish? What if the verify timer is allowed to run concurrently with us changing the settings?
Well, for example, if it runs concurrently with stmmac_fpe_link_state_handle(is_down==false), it will not learn that the link is down, it will send an MPACKET_VERIFY, get no response, and fail. So, not very bad.
And the other way around: stmmac_set_mm() stops the verify timer, but the link comes up, the timer is armed with the old settings, it does whatever (succeeds, fails), and only afterwards does stmmac_set_mm() manage to grab &fpe_cfg->lock, change the settings to the new ones, and re-trigger the verify timer once again, if needed.
So bottom line, I think timer_delete_sync() is to avoid some useless work, but otherwise, it is not critical to have it. The choice is between removing the timer_delete_sync() calls from these 2 functions altogether, or implementing an actually effective mechanism to stop the timer for a while.
I _think_ that the simplest way to stop it is to hold one more lock for the verify_timer when we call timer_delete_sync() and stmmac_fpe_verify_timer_arm(), lock which _is_ IRQ-safe, unlike &fpe_cfg->lock.
static int stmmac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
struct netlink_ext_ack *extack)
{
spin_lock(&fpe_cfg->verify_timer_lock);
timer_delete_sync(&fpe_cfg->verify_timer);
spin_lock_irqsave(&fpe_cfg->lock, flags);
stmmac_fpe_apply(priv);
spin_unlock_irqrestore(&fpe_cfg->lock, flags);
spin_unlock(&fpe_cfg->verify_timer_lock);
}static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
{
spin_lock(&fpe_cfg->verify_timer_lock);
timer_delete_sync(&fpe_cfg->verify_timer);
spin_lock_irqsave(&fpe_cfg->lock, flags);
if (is_up && fpe_cfg->state.pmac_enabled) {
/* VERIFY process requires pmac enabled when NIC comes up */
stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
false, true);
/* New link => maybe new partner => new verification process */
stmmac_fpe_apply(priv);
} else {
/* No link => turn off EFPE */
stmmac_fpe_configure(priv, priv->ioaddr, fpe_cfg,
priv->plat->tx_queues_to_use,
priv->plat->rx_queues_to_use,
false, false);
}
spin_unlock_irqrestore(&fpe_cfg->lock, flags);
spin_unlock(&fpe_cfg->verify_timer_lock);
}Looking at the __timer_delete_sync() implementation, I don't think verify_timer_lock needs to be sleepable and hence a mutex (except on PREEMPT_RT where spinlocks are sleepable no matter what you do).
But I think the implementation would be simpler without timer_delete_sync() in these 2 functions, and this overengineered mechanism.
I would expect a comment in stmmac_release() here:
if (priv->dma_cap.fpesel) timer_delete_sync(&priv->fpe_cfg.verify_timer);
that timer restarts are not possible, because we have rtnl_lock() held and a concurrent stmmac_set_mm() cannot run now, and the earlier phylink_stop() has also ensured stmmac_fpe_link_state_handle() cannot run any longer.
Similarly, I would like to see an explanation in the form of a comment for why timer restarts are not possible after the same pattern in stmmac_suspend(). The explanation is different there, I think.
Re: [PATCH net-next v8 3/7] net: stmmac: refactor FPE verification process
In reply to Furong Xudrivers/net/ethernet/stmicro/stmmac/stmmac.h struct stmmac_channel { · line 166
Quoted 13 lines · Furong Xu
struct
stmmac_fpe_cfg{drivers/net/ethernet/stmicro/stmmac/stmmac.h +7
-4-bool enable;/* FPE enable */ -bool hs_enable;/* FPE handshake enable */ -enum stmmac_fpe_state lp_fpe_state;/* Link Partner FPE state */ -enum stmmac_fpe_state lo_fpe_state;/* Local station FPE state */ +/* Serialize access to MAC Merge state between ethtool requests +* and link state updates. +*/ +spinlock_t lock; +u32 fpe_csr;/* MAC_FPE_CTRL_STS reg cache */ +struct timer_list verify_timer; +struct ethtool_mm_state state;
I don't know what to say about keeping a full-blown struct ethtool_mm_state copy in struct stmmac_fpe_cfg.
You don't populate two of its members: tx_active and tx_min_frag_size, and thus they would be invalid if anyone tried to access them. And two more of its member variables are populated with driver-constant values:max_verify_time and rx_min_frag_size.
This leaves only verify_time, verify_status, pmac_enabled, tx_enabled, verify_enabled. Maybe it would be better to just open-code these variables directly in struct stmmac_fpe_cfg.
PATCH v8 4/7 net: stmmac: configure FPE via ethtool-mm
In reply to Furong XuImplement ethtool --show-mm and --set-mm callbacks.
NIC up/down, link up/down, suspend/resume, kselftest-ethtool_mm, all tested okay.
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 4 + drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 14 +++ drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 6 ++ drivers/net/ethernet/stmicro/stmmac/hwif.h | 6 ++ .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 91 +++++++++++++++++++ 5 files changed, 121 insertions(+)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +4 -0
@@ -1264,6 +1264,8 @@ const struct stmmac_ops dwmac410_ops = { .fpe_configure = dwmac5_fpe_configure, .fpe_send_mpacket = dwmac5_fpe_send_mpacket, .fpe_irq_status = dwmac5_fpe_irq_status, + .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, + .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
@@ -1316,6 +1318,8 @@ const struct stmmac_ops dwmac510_ops = { .fpe_configure = dwmac5_fpe_configure, .fpe_send_mpacket = dwmac5_fpe_send_mpacket, .fpe_irq_status = dwmac5_fpe_irq_status, + .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, + .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +14 -0
@@ -653,3 +653,17 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,writel (value, ioaddr +MAC_FPE_CTRL_STS ); } + +int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr) +{ +return FIELD_GET (DWMAC5_ADD_FRAG_SZ ,readl (ioaddr +MTL_FPE_CTRL_STS )); +} + +void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size) +{ +u32 value; + + value =readl (ioaddr +MTL_FPE_CTRL_STS ); +writel (u32_replace_bits (value, add_frag_size,DWMAC5_ADD_FRAG_SZ ), + ioaddr +MTL_FPE_CTRL_STS ); +}
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +6 -0
@@ -39,6 +39,10 @@#define MAC_PPSx_INTERVAL (x) (0x00000b88 + ((x) * 0x10))#define MAC_PPSx_WIDTH (x) (0x00000b8c + ((x) * 0x10)) +#define MTL_FPE_CTRL_STS 0x00000c90 +/* Additional Fragment Size of preempted frames */ +#define DWMAC5_ADD_FRAG_SZ GENMASK(1, 0) +#define MTL_RXP_CONTROL_STATUS 0x00000ca0#define RXPI BIT(31)#define NPE GENMASK(23, 16)
@@ -109,5 +113,7 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,struct stmmac_fpe_cfg *cfg, enum stmmac_mpacket_type type);int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev); +int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr); +void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size);#endif /* __DWMAC5_H__ */
drivers/net/ethernet/stmicro/stmmac/hwif.h +6 -0
@@ -426,6 +426,8 @@ struct stmmac_ops {struct stmmac_fpe_cfg *cfg, enum stmmac_mpacket_type type);int (*fpe_irq_status)(void __iomem *ioaddr,struct net_device *dev); +int (*fpe_get_add_frag_size)(const void __iomem *ioaddr); +void (*fpe_set_add_frag_size)(void __iomem *ioaddr,u32 add_frag_size); }; #define stmmac_core_init(__priv, __args...) \
@@ -530,6 +532,10 @@ struct stmmac_ops { stmmac_do_void_callback(__priv, mac, fpe_send_mpacket, __args) #define stmmac_fpe_irq_status(__priv, __args...) \ stmmac_do_callback(__priv, mac, fpe_irq_status, __args) +#define stmmac_fpe_get_add_frag_size(__priv, __args...) \ + stmmac_do_callback(__priv, mac, fpe_get_add_frag_size, __args) +#define stmmac_fpe_set_add_frag_size(__priv, __args...) \ + stmmac_do_void_callback(__priv, mac, fpe_set_add_frag_size, __args)/* PTP and HW Timer helpers */ struct stmmac_hwtimestamp {
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +91 -0
109 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v8 5/7 net: stmmac: support fp parameter of tc-mqprio
In reply to Furong Xutc-mqprio can select whether traffic classes are express or preemptible.
After some traffic tests, MAC merge layer statistics are all good.
Local device:
ethtool --include-statistics --json --show-mm eth1
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 35105,
"MACMergeHoldCount": 0
}
} ]Remote device:
ethtool --include-statistics --json --show-mm end1
[ {
"ifname": "end1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 35105,
"MACMergeFragCountRx": 35105,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]Tested on DWMAC CORE 5.10a
--- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 + drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 55 +++++++++++ drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 4 + drivers/net/ethernet/stmicro/stmmac/hwif.c | 6 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 12 +++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 + .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 94 +++++++++++++++++++ 7 files changed, 172 insertions(+), 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +2 -0
@@ -1266,6 +1266,7 @@ const struct stmmac_ops dwmac410_ops = { .fpe_irq_status = dwmac5_fpe_irq_status, .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, + .fpe_map_preemption_class = dwmac5_fpe_map_preemption_class, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
@@ -1320,6 +1321,7 @@ const struct stmmac_ops dwmac510_ops = { .fpe_irq_status = dwmac5_fpe_irq_status, .fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size, .fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size, + .fpe_map_preemption_class = dwmac5_fpe_map_preemption_class, .add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr, .del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr, .restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +55 -0
58 lines not on this page · raw message
drivers/net/ethernet/stmicro/stmmac/dwmac5.h +4 -0
@@ -40,6 +40,8 @@#define MAC_PPSx_WIDTH (x) (0x00000b8c + ((x) * 0x10))#define MTL_FPE_CTRL_STS 0x00000c90 +/* Preemption Classification */ +#define DWMAC5_PREEMPTION_CLASS GENMASK(15, 8)/* Additional Fragment Size of preempted frames */ #define DWMAC5_ADD_FRAG_SZ GENMASK(1, 0)
@@ -115,5 +117,7 @@ void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,int dwmac5_fpe_irq_status (void __iomem *ioaddr,struct net_device *dev);int dwmac5_fpe_get_add_frag_size (const void __iomem *ioaddr);void dwmac5_fpe_set_add_frag_size (void __iomem *ioaddr,u32 add_frag_size); +int dwmac5_fpe_map_preemption_class (struct net_device *ndev, +struct netlink_ext_ack *extack,u32 pclass);#endif /* __DWMAC5_H__ */
drivers/net/ethernet/stmicro/stmmac/hwif.c +3 -3
@@ -171,7 +171,7 @@ static const struct stmmac_hwif_entry { .mac = &dwmac4_ops, .hwtimestamp = &stmmac_ptp, .mode = NULL, - .tc = &dwmac510_tc_ops, + .tc = &dwmac4_tc_ops, .mmc = &dwmac_mmc_ops, .est = &dwmac510_est_ops, .setup = dwmac4_setup,
@@ -252,7 +252,7 @@ static const struct stmmac_hwif_entry { .mac = &dwxgmac210_ops, .hwtimestamp = &stmmac_ptp, .mode = NULL, - .tc = &dwmac510_tc_ops, + .tc = &dwxgmac_tc_ops, .mmc = &dwxgmac_mmc_ops, .est = &dwmac510_est_ops, .setup = dwxgmac2_setup,
@@ -273,7 +273,7 @@ static const struct stmmac_hwif_entry { .mac = &dwxlgmac2_ops, .hwtimestamp = &stmmac_ptp, .mode = NULL, - .tc = &dwmac510_tc_ops, + .tc = &dwxgmac_tc_ops, .mmc = &dwxgmac_mmc_ops, .est = &dwmac510_est_ops, .setup = dwxlgmac2_setup,
drivers/net/ethernet/stmicro/stmmac/hwif.h +12 -0
@@ -7,6 +7,7 @@#include <linux/netdevice.h> #include <linux/stmmac.h> +#include <net/pkt_cls.h> #define stmmac_do_void_callback(__priv, __module, __cname, __arg0, __args...) \ ({ \
@@ -428,6 +429,9 @@ struct stmmac_ops {int (*fpe_irq_status)(void __iomem *ioaddr,struct net_device *dev);int (*fpe_get_add_frag_size)(const void __iomem *ioaddr);void (*fpe_set_add_frag_size)(void __iomem *ioaddr,u32 add_frag_size); +int (*fpe_map_preemption_class)(struct net_device *ndev, +struct netlink_ext_ack *extack, +u32 pclass); }; #define stmmac_core_init(__priv, __args...) \
@@ -536,6 +540,8 @@ struct stmmac_ops { stmmac_do_callback(__priv, mac, fpe_get_add_frag_size, __args) #define stmmac_fpe_set_add_frag_size(__priv, __args...) \ stmmac_do_void_callback(__priv, mac, fpe_set_add_frag_size, __args) +#define stmmac_fpe_map_preemption_class(__priv, __args...) \ + stmmac_do_void_callback(__priv, mac, fpe_map_preemption_class, __args)/* PTP and HW Timer helpers */ struct stmmac_hwtimestamp {
@@ -623,6 +629,8 @@ struct stmmac_tc_ops { struct tc_etf_qopt_offload *qopt);int (*query_caps)(struct stmmac_priv *priv,struct tc_query_caps_base *base); +int (*setup_mqprio)(struct stmmac_priv *priv, +struct tc_mqprio_qopt_offload *qopt); }; #define stmmac_tc_init(__priv, __args...) \
@@ -639,6 +647,8 @@ struct stmmac_tc_ops { stmmac_do_callback(__priv, tc, setup_etf, __args) #define stmmac_tc_query_caps(__priv, __args...) \ stmmac_do_callback(__priv, tc, query_caps, __args) +#define stmmac_tc_setup_mqprio(__priv, __args...) \ + stmmac_do_callback(__priv, tc, setup_mqprio, __args)struct stmmac_counters ;
@@ -682,7 +692,9 @@ extern const struct stmmac_dma_ops dwmac4_dma_ops;extern const struct stmmac_ops dwmac410_ops;extern const struct stmmac_dma_ops dwmac410_dma_ops;extern const struct stmmac_ops dwmac510_ops; +extern const struct stmmac_tc_ops dwmac4_tc_ops;extern const struct stmmac_tc_ops dwmac510_tc_ops; +extern const struct stmmac_tc_ops dwxgmac_tc_ops;extern const struct stmmac_ops dwxgmac210_ops;extern const struct stmmac_ops dwxlgmac2_ops;extern const struct stmmac_dma_ops dwxgmac210_dma_ops;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -0
@@ -6216,6 +6216,8 @@ static int stmmac_setup_tc(struct net_device *ndev, enum tc_setup_type type,switch (type) {case TC_QUERY_CAPS :return stmmac_tc_query_caps (priv, priv, type_data); +case TC_SETUP_QDISC_MQPRIO : +return stmmac_tc_setup_mqprio (priv, priv, type_data);case TC_SETUP_BLOCK : return flow_block_cb_setup_simple(type_data, &stmmac_block_cb_list,
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +94 -0
110 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH net-next v8 5/7] net: stmmac: support fp parameter of tc-mqprio
Quoted 77 lines · Furong Xu
tc-mqprio can select whether traffic classes are express or preemptible.
After some traffic tests, MAC merge layer statistics are all good.
Local device:
ethtool --include-statistics --json --show-mm eth1[ { "ifname": "eth1", "pmac-enabled": true, "tx-enabled": true, "tx-active": true, "tx-min-frag-size": 60, "rx-min-frag-size": 60, "verify-enabled": true, "verify-time": 100, "max-verify-time": 128, "verify-status": "SUCCEEDED", "statistics": { "MACMergeFrameAssErrorCount": 0, "MACMergeFrameSmdErrorCount": 0, "MACMergeFrameAssOkCount": 0, "MACMergeFragCountRx": 0, "MACMergeFragCountTx": 35105, "MACMergeHoldCount": 0 } } ]Remote device:
ethtool --include-statistics --json --show-mm end1[ { "ifname": "end1", "pmac-enabled": true, "tx-enabled": true, "tx-active": true, "tx-min-frag-size": 60, "rx-min-frag-size": 60, "verify-enabled": true, "verify-time": 100, "max-verify-time": 128, "verify-status": "SUCCEEDED", "statistics": { "MACMergeFrameAssErrorCount": 0, "MACMergeFrameSmdErrorCount": 0, "MACMergeFrameAssOkCount": 35105, "MACMergeFragCountRx": 35105, "MACMergeFragCountTx": 0, "MACMergeHoldCount": 0 } } ]Tested on DWMAC CORE 5.10a
Signed-off-by Furong Xu <[email protected]>---
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +23
-0+int dwmac5_fpe_map_preemption_class (struct net_device *ndev, +struct netlink_ext_ack *extack,u32 pclass) +{ +u32 offset, count, val, preemptible_txqs =0 ; +struct stmmac_priv *priv =netdev_priv (ndev); +u32 num_tc = ndev->num_tc; + +if (!pclass) + goto update_mapping; + +/* DWMAC CORE4+ can not program TC:TXQ mapping to hardware. +* +* Synopsys Databook: +* "The number of Tx DMA channels is equal to the number of Tx queues, +* and is direct one-to-one mapping." +*/ +for (u32 tc =0 ; tc < num_tc; tc++) { + count = ndev->tc_to_txq[tc].count; + offset = ndev->tc_to_txq[tc].offset; + +if (pclass &BIT (tc)) + preemptible_txqs |=GENMASK (offset + count -1 , offset); +
drivers/net/ethernet/stmicro/stmmac/dwmac5.c void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size) · line 706
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +10
-0+/* This is 1:1 mapping, go to next TC */ +if (count ==1 ) +continue ; + +if (priv->plat->tx_sched_algorithm ==MTL_TX_ALGORITHM_SP ) { +NL_SET_ERR_MSG_MOD (extack,ALG_ERR_MSG ); +return -EINVAL ; + } + +u32 queue_weight = priv->plat->tx_queues_cfg[offset].weight;
Please do not put variable declarations in the middle of the scope. Declare "u32 queue_weight" separately and just keep the assignment here.
Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +8
-0+ +for (u32 i =1 ; i < count; i++) { +if (priv->plat->tx_queues_cfg[offset + i].weight != + queue_weight) { +NL_SET_ERR_MSG_FMT_MOD (extack,WEIGHT_ERR_MSG , + queue_weight, tc); +return -EINVAL ; + }
drivers/net/ethernet/stmicro/stmmac/dwmac5.c void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size) · line 724
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +10
-0+ } + } + +update_mapping: + val =readl (priv->ioaddr +MTL_FPE_CTRL_STS ); +writel (u32_replace_bits (val, preemptible_txqs,DWMAC5_PREEMPTION_CLASS ), + priv->ioaddr +MTL_FPE_CTRL_STS ); + +return 0 ; +}
Otherwise:
PATCH v8 6/7 net: stmmac: support fp parameter of tc-taprio
In reply to Furong Xutc-taprio can select whether traffic classes are express or preemptible.
0) tc qdisc add dev eth1 parent root handle 100 taprio \
num_tc 4 \
map 0 1 2 3 2 2 2 2 2 2 2 2 2 2 2 3 \
queues 1@0 1@1 1@2 1@3 \
base-time 1000000000 \
sched-entry S 03 10000000 \
sched-entry S 0e 10000000 \
flags 0x2 fp P E E E1) After some traffic tests, MAC merge layer statistics are all good.
Local device:
[ {
"ifname": "eth1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 0,
"MACMergeFragCountRx": 0,
"MACMergeFragCountTx": 17837,
"MACMergeHoldCount": 18639
}
} ]Remote device:
[ {
"ifname": "end1",
"pmac-enabled": true,
"tx-enabled": true,
"tx-active": true,
"tx-min-frag-size": 60,
"rx-min-frag-size": 60,
"verify-enabled": true,
"verify-time": 100,
"max-verify-time": 128,
"verify-status": "SUCCEEDED",
"statistics": {
"MACMergeFrameAssErrorCount": 0,
"MACMergeFrameSmdErrorCount": 0,
"MACMergeFrameAssOkCount": 17189,
"MACMergeFragCountRx": 17837,
"MACMergeFragCountTx": 0,
"MACMergeHoldCount": 0
}
} ]Tested on DWMAC CORE 5.10a
--- .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +22 -17
@@ -931,9 +931,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv, struct tc_taprio_qopt_offload *qopt) {u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep; +struct netlink_ext_ack *extack = qopt->mqprio.extack;struct timespec64 time, current_time, qopt_time;ktime_t current_time_ns; -bool fpe = false;int i, ret =0 ;u64 ctr;
@@ -1018,16 +1018,12 @@ static int tc_taprio_configure(struct stmmac_priv *priv,switch (qopt->entries[i].command) {case TC_TAPRIO_CMD_SET_GATES : -if (fpe) -return -EINVAL ;break ;case TC_TAPRIO_CMD_SET_AND_HOLD : gates |=BIT (0 ); - fpe = true;break ;case TC_TAPRIO_CMD_SET_AND_RELEASE : gates &= ~BIT (0 ); - fpe = true;break ;default :return -EOPNOTSUPP ;
@@ -1058,11 +1054,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,tc_taprio_map_maxsdu_txq (priv, qopt); -if (fpe && !priv->dma_cap.fpesel) { -mutex_unlock (&priv->est_lock); -return -EOPNOTSUPP ; - } - ret =stmmac_est_configure (priv, priv, priv->est, priv->plat->clk_ptp_rate);mutex_unlock (&priv->est_lock);
@@ -1071,6 +1062,11 @@ static int tc_taprio_configure(struct stmmac_priv *priv, goto disable; } + ret =stmmac_fpe_map_preemption_class (priv, priv->dev, extack, + qopt->mqprio.preemptible_tcs); +if (ret) + goto disable; +netdev_info (priv->dev,"configured EST\n" );return 0 ;
@@ -1089,11 +1085,8 @@ static int tc_taprio_configure(struct stmmac_priv *priv,mutex_unlock (&priv->est_lock); } -stmmac_fpe_configure (priv, priv->ioaddr, - &priv->fpe_cfg, - priv->plat->tx_queues_to_use, - priv->plat->rx_queues_to_use, - false, false); +stmmac_fpe_map_preemption_class (priv, priv->dev, extack,0 ); +netdev_info (priv->dev,"disabled FPE\n" );return ret;
@@ -1150,6 +1143,18 @@ static int tc_setup_taprio(struct stmmac_priv *priv,return err; } +static int tc_setup_taprio_without_fpe (struct stmmac_priv *priv, +struct tc_taprio_qopt_offload *qopt) +{ +if (!qopt->mqprio.preemptible_tcs) +return tc_setup_taprio (priv, qopt); + +NL_SET_ERR_MSG_MOD (qopt->mqprio.extack, +"taprio with FPE is not implemented for this MAC" ); + +return -EOPNOTSUPP ; +} +static int tc_setup_etf (struct stmmac_priv *priv,struct tc_etf_qopt_offload *qopt) {
@@ -1266,7 +1271,7 @@ const struct stmmac_tc_ops dwmac4_tc_ops = { .setup_cls_u32 = tc_setup_cls_u32, .setup_cbs = tc_setup_cbs, .setup_cls = tc_setup_cls, - .setup_taprio = tc_setup_taprio, + .setup_taprio = tc_setup_taprio_without_fpe, .setup_etf = tc_setup_etf, .query_caps = tc_query_caps, .setup_mqprio = tc_setup_mqprio_unimplemented,
@@ -1288,7 +1293,7 @@ const struct stmmac_tc_ops dwxgmac_tc_ops = { .setup_cls_u32 = tc_setup_cls_u32, .setup_cbs = tc_setup_cbs, .setup_cls = tc_setup_cls, - .setup_taprio = tc_setup_taprio, + .setup_taprio = tc_setup_taprio_without_fpe, .setup_etf = tc_setup_etf, .query_caps = tc_query_caps, .setup_mqprio = tc_setup_mqprio_unimplemented,
Signature · 1 line
2.34.1
Re: [PATCH net-next v8 6/7] net: stmmac: support fp parameter of tc-taprio
Quoted 57 lines · Furong Xu
tc-taprio can select whether traffic classes are express or preemptible.
0) tc qdisc add dev eth1 parent root handle 100 taprio \
num_tc 4 \ map 0 1 2 3 2 2 2 2 2 2 2 2 2 2 2 3 \ queues 1@0 1@1 1@2 1@3 \ base-time 1000000000 \ sched-entry S 03 10000000 \ sched-entry S 0e 10000000 \ flags 0x2 fp P E E E1) After some traffic tests, MAC merge layer statistics are all good.
Local device:
[ { "ifname": "eth1", "pmac-enabled": true, "tx-enabled": true, "tx-active": true, "tx-min-frag-size": 60, "rx-min-frag-size": 60, "verify-enabled": true, "verify-time": 100, "max-verify-time": 128, "verify-status": "SUCCEEDED", "statistics": { "MACMergeFrameAssErrorCount": 0, "MACMergeFrameSmdErrorCount": 0, "MACMergeFrameAssOkCount": 0, "MACMergeFragCountRx": 0, "MACMergeFragCountTx": 17837, "MACMergeHoldCount": 18639 } } ]Remote device:
[ { "ifname": "end1", "pmac-enabled": true, "tx-enabled": true, "tx-active": true, "tx-min-frag-size": 60, "rx-min-frag-size": 60, "verify-enabled": true, "verify-time": 100, "max-verify-time": 128, "verify-status": "SUCCEEDED", "statistics": { "MACMergeFrameAssErrorCount": 0, "MACMergeFrameSmdErrorCount": 0, "MACMergeFrameAssOkCount": 17189, "MACMergeFragCountRx": 17837, "MACMergeFragCountTx": 0, "MACMergeHoldCount": 0 } } ]
Quoted 4 lines
Tested on DWMAC CORE 5.10a
Signed-off-by Furong Xu <[email protected]>---
PATCH v8 7/7 net: stmmac: silence FPE kernel logs
In reply to Furong Xuethtool --show-mm can get real-time state of FPE.fpe_irq_status logs should keep quiet.
tc-taprio can always query driver state, delete unbalanced logs.
--- drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 8 ++++---- drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 4 ---- 2 files changed, 4 insertions(+), 8 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac5.c +4 -4
@@ -620,22 +620,22 @@ int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)if (value &TRSP ) { status |=FPE_EVENT_TRSP ; -netdev_info (dev,"FPE: Respond mPacket is transmitted\n" ); +netdev_dbg (dev,"FPE: Respond mPacket is transmitted\n" ); }if (value &TVER ) { status |=FPE_EVENT_TVER ; -netdev_info (dev,"FPE: Verify mPacket is transmitted\n" ); +netdev_dbg (dev,"FPE: Verify mPacket is transmitted\n" ); }if (value &RRSP ) { status |=FPE_EVENT_RRSP ; -netdev_info (dev,"FPE: Respond mPacket is received\n" ); +netdev_dbg (dev,"FPE: Respond mPacket is received\n" ); }if (value &RVER ) { status |=FPE_EVENT_RVER ; -netdev_info (dev,"FPE: Verify mPacket is received\n" ); +netdev_dbg (dev,"FPE: Verify mPacket is received\n" ); }return status;
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +0 -4
@@ -1067,8 +1067,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,if (ret) goto disable; -netdev_info (priv->dev,"configured EST\n" ); -return 0 ; disable:
@@ -1087,8 +1085,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,stmmac_fpe_map_preemption_class (priv, priv->dev, extack,0 ); -netdev_info (priv->dev,"disabled FPE\n" ); -return ret; }
Signature · 1 line
2.34.1
Re: [PATCH net-next v8 7/7] net: stmmac: silence FPE kernel logs
Quoted 7 lines · Furong Xu
ethtool --show-mm can get real-time state of FPE.
fpe_irq_statuslogs should keep quiet.tc-taprio can always query driver state, delete unbalanced logs.
Signed-off-by Furong Xu <[email protected]>---