* [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
@ 2019-11-27 9:59 ` Po Liu
2019-11-27 11:00 ` Vladimir Oltean
2019-12-04 1:35 ` Ivan Khoronzhuk
2019-11-27 18:57 ` [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes David Miller
` (4 subsequent siblings)
5 siblings, 2 replies; 39+ messages in thread
From: Po Liu @ 2019-11-27 9:59 UTC (permalink / raw)
To: davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1, saeedm,
andrew, f.fainelli, alexandru.ardelean, jiri, ayal, pablo,
linux-kernel, netdev
Cc: vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Po Liu
The interface follow up the ethtool 'preemption' set/get.
Hardware features also need to set hw_features with
NETIF_F_PREEMPTION flag. So ethtool could check kernel
link features if there is preemption capability of port.
There are two MACs in ENETC. One is express MAC which traffic
classes in it are advanced transmition. Another is preemptable
MAC which traffic classes are frame preemptable.
The hardware need to initialize the MACs at initial stage.
And then set the preemption enable registers of traffic
classes when ethtool set .get_link_ksettings/.set_link_ksettings
stage.
To test the ENETC preemption capability, user need to set mqprio
or taprio to mapping the traffic classes with priorities. Then
use ethtool command to set 'preemption' with a 8 bits value.
MSB represent high number traffic class.
Signed-off-by: Po Liu <Po.Liu@nxp.com>
---
drivers/net/ethernet/freescale/enetc/enetc.c | 3 +
drivers/net/ethernet/freescale/enetc/enetc.h | 4 +
.../ethernet/freescale/enetc/enetc_ethtool.c | 142 ++++++++++++++++--
.../net/ethernet/freescale/enetc/enetc_hw.h | 17 +++
.../net/ethernet/freescale/enetc/enetc_pf.c | 15 +-
.../net/ethernet/freescale/enetc/enetc_qos.c | 4 +
6 files changed, 174 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 9db1b96ed9b9..be0d9916e6ea 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -750,6 +750,8 @@ void enetc_get_si_caps(struct enetc_si *si)
if (val & ENETC_SIPCAPR0_QBV)
si->hw_features |= ENETC_SI_F_QBV;
+ if (val & ENETC_SIPCAPR0_QBU)
+ si->hw_features |= ENETC_SI_F_QBU;
}
static int enetc_dma_alloc_bdr(struct enetc_bdr *r, size_t bd_size)
@@ -1448,6 +1450,7 @@ static int enetc_setup_tc_mqprio(struct net_device *ndev, void *type_data)
num_tc = mqprio->num_tc;
if (!num_tc) {
+ enetc_preemption_set(ndev, 0);
netdev_reset_tc(ndev);
netif_set_real_num_tx_queues(ndev, priv->num_tx_rings);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 7ee0da6d0015..cfa74fa326e8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -119,6 +119,7 @@ enum enetc_errata {
};
#define ENETC_SI_F_QBV BIT(0)
+#define ENETC_SI_F_QBU BIT(1)
/* PCI IEP device data */
struct enetc_si {
@@ -177,6 +178,7 @@ enum enetc_active_offloads {
ENETC_F_RX_TSTAMP = BIT(0),
ENETC_F_TX_TSTAMP = BIT(1),
ENETC_F_QBV = BIT(2),
+ ENETC_F_QBU = BIT(3),
};
struct enetc_ndev_priv {
@@ -261,3 +263,5 @@ int enetc_setup_tc_cbs(struct net_device *ndev, void *type_data);
#define enetc_sched_speed_set(ndev) (void)0
#define enetc_setup_tc_cbs(ndev, type_data) -EOPNOTSUPP
#endif
+
+int enetc_preemption_set(struct net_device *ndev, u32 ptvector);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
index 880a8ed8bb47..4c7425539280 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
@@ -183,6 +183,21 @@ static const struct {
{ ENETC_PICDR(3), "ICM DR3 discarded frames" },
};
+static const struct {
+ int reg;
+ char name[ETH_GSTRING_LEN];
+} enetc_pmac_counters[] = {
+ { ENETC_PM1_RFRM, "PMAC rx frames" },
+ { ENETC_PM1_RPKT, "PMAC rx packets" },
+ { ENETC_PM1_RDRP, "PMAC rx dropped packets" },
+ { ENETC_PM1_RFRG, "PMAC rx fragment packets" },
+ { ENETC_PM1_TFRM, "PMAC tx frames" },
+ { ENETC_PM1_TERR, "PMAC tx error frames" },
+ { ENETC_PM1_TPKT, "PMAC tx packets" },
+ { ENETC_MAC_MERGE_MMFCRXR, "MAC merge fragment rx counter" },
+ { ENETC_MAC_MERGE_MMFCTXR, "MAC merge fragment tx counter"},
+};
+
static const char rx_ring_stats[][ETH_GSTRING_LEN] = {
"Rx ring %2d frames",
"Rx ring %2d alloc errors",
@@ -195,15 +210,24 @@ static const char tx_ring_stats[][ETH_GSTRING_LEN] = {
static int enetc_get_sset_count(struct net_device *ndev, int sset)
{
struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ int len;
+
+ if (sset != ETH_SS_STATS)
+ return -EOPNOTSUPP;
- if (sset == ETH_SS_STATS)
- return ARRAY_SIZE(enetc_si_counters) +
- ARRAY_SIZE(tx_ring_stats) * priv->num_tx_rings +
- ARRAY_SIZE(rx_ring_stats) * priv->num_rx_rings +
- (enetc_si_is_pf(priv->si) ?
- ARRAY_SIZE(enetc_port_counters) : 0);
+ len = ARRAY_SIZE(enetc_si_counters) +
+ ARRAY_SIZE(tx_ring_stats) * priv->num_tx_rings +
+ ARRAY_SIZE(rx_ring_stats) * priv->num_rx_rings;
- return -EOPNOTSUPP;
+ if (!enetc_si_is_pf(priv->si))
+ return len;
+
+ len += ARRAY_SIZE(enetc_port_counters);
+
+ if (priv->active_offloads & ENETC_F_QBU)
+ len += ARRAY_SIZE(enetc_pmac_counters);
+
+ return len;
}
static void enetc_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
@@ -241,6 +265,16 @@ static void enetc_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
ETH_GSTRING_LEN);
p += ETH_GSTRING_LEN;
}
+
+ if (!(priv->active_offloads & ENETC_F_QBU))
+ break;
+
+ for (i = 0; i < ARRAY_SIZE(enetc_pmac_counters); i++) {
+ strlcpy(p, enetc_pmac_counters[i].name,
+ ETH_GSTRING_LEN);
+ p += ETH_GSTRING_LEN;
+ }
+
break;
}
}
@@ -268,6 +302,12 @@ static void enetc_get_ethtool_stats(struct net_device *ndev,
for (i = 0; i < ARRAY_SIZE(enetc_port_counters); i++)
data[o++] = enetc_port_rd(hw, enetc_port_counters[i].reg);
+
+ if (!(priv->active_offloads & ENETC_F_QBU))
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(enetc_pmac_counters); i++)
+ data[o++] = enetc_port_rd(hw, enetc_pmac_counters[i].reg);
}
#define ENETC_RSSHASH_L3 (RXH_L2DA | RXH_VLAN | RXH_L3_PROTO | RXH_IP_SRC | \
@@ -609,6 +649,90 @@ static int enetc_set_wol(struct net_device *dev,
return ret;
}
+static u8 enetc_get_tc_num(struct enetc_si *si)
+{
+ struct net_device *ndev = si->ndev;
+ u8 tc_num;
+
+ tc_num = (enetc_port_rd(&si->hw, ENETC_PCAPR1)
+ & ENETC_NUM_TCS_MASK) >> 4;
+
+ return min(netdev_get_num_tc(ndev), tc_num + 1);
+}
+
+int enetc_preemption_set(struct net_device *ndev, u32 ptvector)
+{
+ struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ u8 tc_num;
+ u32 temp;
+ int i;
+
+ if (ptvector & ~ENETC_QBU_TC_MASK)
+ return -EINVAL;
+
+ temp = enetc_rd(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET);
+ if (temp & ENETC_QBV_TGE)
+ enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET,
+ temp & (~ENETC_QBV_TGPE));
+
+ tc_num = enetc_get_tc_num(priv->si);
+
+ for (i = 0; i < tc_num; i++) {
+ temp = enetc_port_rd(&priv->si->hw, ENETC_PTCFPR(i));
+
+ if ((ptvector >> i) & 0x1)
+ enetc_port_wr(&priv->si->hw,
+ ENETC_PTCFPR(i),
+ temp | ENETC_FPE);
+ else
+ enetc_port_wr(&priv->si->hw,
+ ENETC_PTCFPR(i),
+ temp & ~ENETC_FPE);
+ }
+
+ return 0;
+}
+
+static u32 enetc_preemption_get(struct net_device *ndev)
+{
+ struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ u32 ptvector = 0;
+ u8 tc_num;
+ int i;
+
+ /* If preemptable MAC is not enable return 0 */
+ if (!(enetc_port_rd(&priv->si->hw, ENETC_PFPMR) & ENETC_PFPMR_PMACE))
+ return 0;
+
+ tc_num = enetc_get_tc_num(priv->si);
+
+ for (i = 0; i < tc_num; i++)
+ if (enetc_port_rd(&priv->si->hw, ENETC_PTCFPR(i)) & ENETC_FPE)
+ ptvector |= 1 << i;
+
+ return ptvector;
+}
+
+static int enetc_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd)
+{
+ cmd->base.preemption = enetc_preemption_get(ndev);
+
+ return phy_ethtool_get_link_ksettings(ndev, cmd);
+}
+
+static int enetc_set_link_ksettings(struct net_device *ndev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ int err;
+
+ err = enetc_preemption_set(ndev, cmd->base.preemption);
+ if (err)
+ return err;
+
+ return phy_ethtool_set_link_ksettings(ndev, cmd);
+}
+
static const struct ethtool_ops enetc_pf_ethtool_ops = {
.get_regs_len = enetc_get_reglen,
.get_regs = enetc_get_regs,
@@ -622,8 +746,8 @@ static const struct ethtool_ops enetc_pf_ethtool_ops = {
.get_rxfh = enetc_get_rxfh,
.set_rxfh = enetc_set_rxfh,
.get_ringparam = enetc_get_ringparam,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_link_ksettings = enetc_get_link_ksettings,
+ .set_link_ksettings = enetc_set_link_ksettings,
.get_link = ethtool_op_get_link,
.get_ts_info = enetc_get_ts_info,
.get_wol = enetc_get_wol,
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 51f543ef37a8..b609ec095710 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -19,6 +19,7 @@
#define ENETC_SICTR1 0x1c
#define ENETC_SIPCAPR0 0x20
#define ENETC_SIPCAPR0_QBV BIT(4)
+#define ENETC_SIPCAPR0_QBU BIT(3)
#define ENETC_SIPCAPR0_RSS BIT(8)
#define ENETC_SIPCAPR1 0x24
#define ENETC_SITGTGR 0x30
@@ -176,6 +177,7 @@ enum enetc_bdr_type {TX, RX};
#define ENETC_PCAPR0_RXBDR(val) ((val) >> 24)
#define ENETC_PCAPR0_TXBDR(val) (((val) >> 16) & 0xff)
#define ENETC_PCAPR1 0x0904
+#define ENETC_NUM_TCS_MASK GENMASK(6, 4)
#define ENETC_PSICFGR0(n) (0x0940 + (n) * 0xc) /* n = SI index */
#define ENETC_PSICFGR0_SET_TXBDR(val) ((val) & 0xff)
#define ENETC_PSICFGR0_SET_RXBDR(val) (((val) & 0xff) << 16)
@@ -223,6 +225,7 @@ enum enetc_bdr_type {TX, RX};
#define ENETC_SET_TX_MTU(val) ((val) << 16)
#define ENETC_SET_MAXFRM(val) ((val) & 0xffff)
#define ENETC_PM0_IF_MODE 0x8300
+#define ENETC_PM1_IF_MODE 0x9300
#define ENETC_PMO_IFM_RG BIT(2)
#define ENETC_PM0_IFM_RLP (BIT(5) | BIT(11))
#define ENETC_PM0_IFM_RGAUTO (BIT(15) | ENETC_PMO_IFM_RG | BIT(1))
@@ -276,6 +279,15 @@ enum enetc_bdr_type {TX, RX};
#define ENETC_PM0_TSCOL 0x82E0
#define ENETC_PM0_TLCOL 0x82E8
#define ENETC_PM0_TECOL 0x82F0
+#define ENETC_PM1_RFRM 0x9120
+#define ENETC_PM1_RDRP 0x9158
+#define ENETC_PM1_RPKT 0x9160
+#define ENETC_PM1_RFRG 0x91B8
+#define ENETC_PM1_TFRM 0x9220
+#define ENETC_PM1_TERR 0x9238
+#define ENETC_PM1_TPKT 0x9260
+#define ENETC_MAC_MERGE_MMFCRXR 0x1f14
+#define ENETC_MAC_MERGE_MMFCTXR 0x1f18
/* Port counters */
#define ENETC_PICDR(n) (0x0700 + (n) * 8) /* n = [0..3] */
@@ -615,3 +627,8 @@ struct enetc_cbd {
/* Port time gating capability register */
#define ENETC_QBV_PTGCAPR_OFFSET 0x11a08
#define ENETC_QBV_MAX_GCL_LEN_MASK GENMASK(15, 0)
+
+#define ENETC_QBU_TC_MASK GENMASK(7, 0)
+
+#define ENETC_PTCFPR(n) (0x1910 + (n) * 4)
+#define ENETC_FPE BIT(31)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index e7482d483b28..f1873c4da77f 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -523,10 +523,15 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
ENETC_PM0_CMD_TXP | ENETC_PM0_PROMISC |
ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
/* set auto-speed for RGMII */
- if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG)
+ if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG) {
enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_RGAUTO);
- if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII)
+ enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_RGAUTO);
+ }
+
+ if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII) {
enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
+ enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_XGMII);
+ }
}
static void enetc_configure_port_pmac(struct enetc_hw *hw)
@@ -745,6 +750,12 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
if (si->hw_features & ENETC_SI_F_QBV)
priv->active_offloads |= ENETC_F_QBV;
+ if (si->hw_features & ENETC_SI_F_QBU) {
+ ndev->hw_features |= NETIF_F_PREEMPTION;
+ ndev->features |= NETIF_F_PREEMPTION;
+ priv->active_offloads |= ENETC_F_QBU;
+ }
+
/* pick up primary MAC address from SI */
enetc_get_primary_mac_addr(&si->hw, ndev->dev_addr);
}
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 2e99438cb1bf..94dde847d052 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -169,6 +169,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
priv->tx_ring[i]->index,
taprio->enable ? 0 : i);
+ /* preemption off if TC priority is all 0 */
+ if ((err && taprio->enable) || !(err || taprio->enable))
+ enetc_preemption_set(ndev, 0);
+
return err;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function
2019-11-27 9:59 ` [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function Po Liu
@ 2019-11-27 11:00 ` Vladimir Oltean
2019-12-04 1:35 ` Ivan Khoronzhuk
1 sibling, 0 replies; 39+ messages in thread
From: Vladimir Oltean @ 2019-11-27 11:00 UTC (permalink / raw)
To: Po Liu
Cc: davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1, saeedm,
andrew, f.fainelli, alexandru.ardelean, jiri, ayal, pablo,
linux-kernel, netdev, vinicius.gomes, simon.horman,
Claudiu Manoil, Vladimir Oltean, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
Hi Po,
On Wed, 27 Nov 2019 at 12:01, Po Liu <po.liu@nxp.com> wrote:
>
> The interface follow up the ethtool 'preemption' set/get.
> Hardware features also need to set hw_features with
> NETIF_F_PREEMPTION flag. So ethtool could check kernel
> link features if there is preemption capability of port.
>
> There are two MACs in ENETC. One is express MAC which traffic
> classes in it are advanced transmition. Another is preemptable
> MAC which traffic classes are frame preemptable.
>
> The hardware need to initialize the MACs at initial stage.
> And then set the preemption enable registers of traffic
> classes when ethtool set .get_link_ksettings/.set_link_ksettings
> stage.
>
> To test the ENETC preemption capability, user need to set mqprio
> or taprio to mapping the traffic classes with priorities. Then
> use ethtool command to set 'preemption' with a 8 bits value.
> MSB represent high number traffic class.
>
> Signed-off-by: Po Liu <Po.Liu@nxp.com>
> ---
Just to clarify, the net-next tree is closed during the following 2
weeks or so, for the 5.5 merge window:
http://vger.kernel.org/~davem/net-next.html
Only bugfix and RFC patches (aka for the sake of discussion) patches
should be sent during this time.
So let's treat this series as RFC.
> drivers/net/ethernet/freescale/enetc/enetc.c | 3 +
> drivers/net/ethernet/freescale/enetc/enetc.h | 4 +
> .../ethernet/freescale/enetc/enetc_ethtool.c | 142 ++++++++++++++++--
> .../net/ethernet/freescale/enetc/enetc_hw.h | 17 +++
> .../net/ethernet/freescale/enetc/enetc_pf.c | 15 +-
> .../net/ethernet/freescale/enetc/enetc_qos.c | 4 +
> 6 files changed, 174 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 9db1b96ed9b9..be0d9916e6ea 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -750,6 +750,8 @@ void enetc_get_si_caps(struct enetc_si *si)
>
> if (val & ENETC_SIPCAPR0_QBV)
> si->hw_features |= ENETC_SI_F_QBV;
> + if (val & ENETC_SIPCAPR0_QBU)
> + si->hw_features |= ENETC_SI_F_QBU;
> }
>
> static int enetc_dma_alloc_bdr(struct enetc_bdr *r, size_t bd_size)
> @@ -1448,6 +1450,7 @@ static int enetc_setup_tc_mqprio(struct net_device *ndev, void *type_data)
> num_tc = mqprio->num_tc;
>
> if (!num_tc) {
> + enetc_preemption_set(ndev, 0);
> netdev_reset_tc(ndev);
> netif_set_real_num_tx_queues(ndev, priv->num_tx_rings);
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
> index 7ee0da6d0015..cfa74fa326e8 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -119,6 +119,7 @@ enum enetc_errata {
> };
>
> #define ENETC_SI_F_QBV BIT(0)
> +#define ENETC_SI_F_QBU BIT(1)
>
> /* PCI IEP device data */
> struct enetc_si {
> @@ -177,6 +178,7 @@ enum enetc_active_offloads {
> ENETC_F_RX_TSTAMP = BIT(0),
> ENETC_F_TX_TSTAMP = BIT(1),
> ENETC_F_QBV = BIT(2),
> + ENETC_F_QBU = BIT(3),
> };
>
> struct enetc_ndev_priv {
> @@ -261,3 +263,5 @@ int enetc_setup_tc_cbs(struct net_device *ndev, void *type_data);
> #define enetc_sched_speed_set(ndev) (void)0
> #define enetc_setup_tc_cbs(ndev, type_data) -EOPNOTSUPP
> #endif
> +
> +int enetc_preemption_set(struct net_device *ndev, u32 ptvector);
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index 880a8ed8bb47..4c7425539280 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -183,6 +183,21 @@ static const struct {
> { ENETC_PICDR(3), "ICM DR3 discarded frames" },
> };
>
> +static const struct {
> + int reg;
> + char name[ETH_GSTRING_LEN];
> +} enetc_pmac_counters[] = {
> + { ENETC_PM1_RFRM, "PMAC rx frames" },
> + { ENETC_PM1_RPKT, "PMAC rx packets" },
> + { ENETC_PM1_RDRP, "PMAC rx dropped packets" },
> + { ENETC_PM1_RFRG, "PMAC rx fragment packets" },
> + { ENETC_PM1_TFRM, "PMAC tx frames" },
> + { ENETC_PM1_TERR, "PMAC tx error frames" },
> + { ENETC_PM1_TPKT, "PMAC tx packets" },
> + { ENETC_MAC_MERGE_MMFCRXR, "MAC merge fragment rx counter" },
> + { ENETC_MAC_MERGE_MMFCTXR, "MAC merge fragment tx counter"},
> +};
> +
> static const char rx_ring_stats[][ETH_GSTRING_LEN] = {
> "Rx ring %2d frames",
> "Rx ring %2d alloc errors",
> @@ -195,15 +210,24 @@ static const char tx_ring_stats[][ETH_GSTRING_LEN] = {
> static int enetc_get_sset_count(struct net_device *ndev, int sset)
> {
> struct enetc_ndev_priv *priv = netdev_priv(ndev);
> + int len;
> +
> + if (sset != ETH_SS_STATS)
> + return -EOPNOTSUPP;
>
> - if (sset == ETH_SS_STATS)
> - return ARRAY_SIZE(enetc_si_counters) +
> - ARRAY_SIZE(tx_ring_stats) * priv->num_tx_rings +
> - ARRAY_SIZE(rx_ring_stats) * priv->num_rx_rings +
> - (enetc_si_is_pf(priv->si) ?
> - ARRAY_SIZE(enetc_port_counters) : 0);
> + len = ARRAY_SIZE(enetc_si_counters) +
> + ARRAY_SIZE(tx_ring_stats) * priv->num_tx_rings +
> + ARRAY_SIZE(rx_ring_stats) * priv->num_rx_rings;
>
> - return -EOPNOTSUPP;
> + if (!enetc_si_is_pf(priv->si))
> + return len;
> +
> + len += ARRAY_SIZE(enetc_port_counters);
> +
> + if (priv->active_offloads & ENETC_F_QBU)
> + len += ARRAY_SIZE(enetc_pmac_counters);
> +
> + return len;
> }
>
> static void enetc_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
> @@ -241,6 +265,16 @@ static void enetc_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
> ETH_GSTRING_LEN);
> p += ETH_GSTRING_LEN;
> }
> +
> + if (!(priv->active_offloads & ENETC_F_QBU))
> + break;
> +
> + for (i = 0; i < ARRAY_SIZE(enetc_pmac_counters); i++) {
> + strlcpy(p, enetc_pmac_counters[i].name,
> + ETH_GSTRING_LEN);
> + p += ETH_GSTRING_LEN;
> + }
> +
> break;
> }
> }
> @@ -268,6 +302,12 @@ static void enetc_get_ethtool_stats(struct net_device *ndev,
>
> for (i = 0; i < ARRAY_SIZE(enetc_port_counters); i++)
> data[o++] = enetc_port_rd(hw, enetc_port_counters[i].reg);
> +
> + if (!(priv->active_offloads & ENETC_F_QBU))
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(enetc_pmac_counters); i++)
> + data[o++] = enetc_port_rd(hw, enetc_pmac_counters[i].reg);
> }
>
> #define ENETC_RSSHASH_L3 (RXH_L2DA | RXH_VLAN | RXH_L3_PROTO | RXH_IP_SRC | \
> @@ -609,6 +649,90 @@ static int enetc_set_wol(struct net_device *dev,
> return ret;
> }
>
> +static u8 enetc_get_tc_num(struct enetc_si *si)
> +{
> + struct net_device *ndev = si->ndev;
> + u8 tc_num;
> +
> + tc_num = (enetc_port_rd(&si->hw, ENETC_PCAPR1)
> + & ENETC_NUM_TCS_MASK) >> 4;
> +
> + return min(netdev_get_num_tc(ndev), tc_num + 1);
> +}
> +
> +int enetc_preemption_set(struct net_device *ndev, u32 ptvector)
> +{
> + struct enetc_ndev_priv *priv = netdev_priv(ndev);
> + u8 tc_num;
> + u32 temp;
> + int i;
> +
> + if (ptvector & ~ENETC_QBU_TC_MASK)
> + return -EINVAL;
> +
> + temp = enetc_rd(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET);
> + if (temp & ENETC_QBV_TGE)
> + enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET,
> + temp & (~ENETC_QBV_TGPE));
> +
> + tc_num = enetc_get_tc_num(priv->si);
> +
> + for (i = 0; i < tc_num; i++) {
> + temp = enetc_port_rd(&priv->si->hw, ENETC_PTCFPR(i));
> +
> + if ((ptvector >> i) & 0x1)
> + enetc_port_wr(&priv->si->hw,
> + ENETC_PTCFPR(i),
> + temp | ENETC_FPE);
> + else
> + enetc_port_wr(&priv->si->hw,
> + ENETC_PTCFPR(i),
> + temp & ~ENETC_FPE);
> + }
> +
> + return 0;
> +}
> +
> +static u32 enetc_preemption_get(struct net_device *ndev)
> +{
> + struct enetc_ndev_priv *priv = netdev_priv(ndev);
> + u32 ptvector = 0;
> + u8 tc_num;
> + int i;
> +
> + /* If preemptable MAC is not enable return 0 */
> + if (!(enetc_port_rd(&priv->si->hw, ENETC_PFPMR) & ENETC_PFPMR_PMACE))
> + return 0;
> +
> + tc_num = enetc_get_tc_num(priv->si);
> +
> + for (i = 0; i < tc_num; i++)
> + if (enetc_port_rd(&priv->si->hw, ENETC_PTCFPR(i)) & ENETC_FPE)
> + ptvector |= 1 << i;
> +
> + return ptvector;
> +}
> +
> +static int enetc_get_link_ksettings(struct net_device *ndev,
> + struct ethtool_link_ksettings *cmd)
> +{
> + cmd->base.preemption = enetc_preemption_get(ndev);
> +
> + return phy_ethtool_get_link_ksettings(ndev, cmd);
> +}
> +
> +static int enetc_set_link_ksettings(struct net_device *ndev,
> + const struct ethtool_link_ksettings *cmd)
> +{
> + int err;
> +
> + err = enetc_preemption_set(ndev, cmd->base.preemption);
> + if (err)
> + return err;
> +
> + return phy_ethtool_set_link_ksettings(ndev, cmd);
> +}
> +
> static const struct ethtool_ops enetc_pf_ethtool_ops = {
> .get_regs_len = enetc_get_reglen,
> .get_regs = enetc_get_regs,
> @@ -622,8 +746,8 @@ static const struct ethtool_ops enetc_pf_ethtool_ops = {
> .get_rxfh = enetc_get_rxfh,
> .set_rxfh = enetc_set_rxfh,
> .get_ringparam = enetc_get_ringparam,
> - .get_link_ksettings = phy_ethtool_get_link_ksettings,
> - .set_link_ksettings = phy_ethtool_set_link_ksettings,
> + .get_link_ksettings = enetc_get_link_ksettings,
> + .set_link_ksettings = enetc_set_link_ksettings,
> .get_link = ethtool_op_get_link,
> .get_ts_info = enetc_get_ts_info,
> .get_wol = enetc_get_wol,
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index 51f543ef37a8..b609ec095710 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> @@ -19,6 +19,7 @@
> #define ENETC_SICTR1 0x1c
> #define ENETC_SIPCAPR0 0x20
> #define ENETC_SIPCAPR0_QBV BIT(4)
> +#define ENETC_SIPCAPR0_QBU BIT(3)
> #define ENETC_SIPCAPR0_RSS BIT(8)
> #define ENETC_SIPCAPR1 0x24
> #define ENETC_SITGTGR 0x30
> @@ -176,6 +177,7 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_PCAPR0_RXBDR(val) ((val) >> 24)
> #define ENETC_PCAPR0_TXBDR(val) (((val) >> 16) & 0xff)
> #define ENETC_PCAPR1 0x0904
> +#define ENETC_NUM_TCS_MASK GENMASK(6, 4)
> #define ENETC_PSICFGR0(n) (0x0940 + (n) * 0xc) /* n = SI index */
> #define ENETC_PSICFGR0_SET_TXBDR(val) ((val) & 0xff)
> #define ENETC_PSICFGR0_SET_RXBDR(val) (((val) & 0xff) << 16)
> @@ -223,6 +225,7 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_SET_TX_MTU(val) ((val) << 16)
> #define ENETC_SET_MAXFRM(val) ((val) & 0xffff)
> #define ENETC_PM0_IF_MODE 0x8300
> +#define ENETC_PM1_IF_MODE 0x9300
> #define ENETC_PMO_IFM_RG BIT(2)
> #define ENETC_PM0_IFM_RLP (BIT(5) | BIT(11))
> #define ENETC_PM0_IFM_RGAUTO (BIT(15) | ENETC_PMO_IFM_RG | BIT(1))
> @@ -276,6 +279,15 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_PM0_TSCOL 0x82E0
> #define ENETC_PM0_TLCOL 0x82E8
> #define ENETC_PM0_TECOL 0x82F0
> +#define ENETC_PM1_RFRM 0x9120
> +#define ENETC_PM1_RDRP 0x9158
> +#define ENETC_PM1_RPKT 0x9160
> +#define ENETC_PM1_RFRG 0x91B8
> +#define ENETC_PM1_TFRM 0x9220
> +#define ENETC_PM1_TERR 0x9238
> +#define ENETC_PM1_TPKT 0x9260
> +#define ENETC_MAC_MERGE_MMFCRXR 0x1f14
> +#define ENETC_MAC_MERGE_MMFCTXR 0x1f18
>
> /* Port counters */
> #define ENETC_PICDR(n) (0x0700 + (n) * 8) /* n = [0..3] */
> @@ -615,3 +627,8 @@ struct enetc_cbd {
> /* Port time gating capability register */
> #define ENETC_QBV_PTGCAPR_OFFSET 0x11a08
> #define ENETC_QBV_MAX_GCL_LEN_MASK GENMASK(15, 0)
> +
> +#define ENETC_QBU_TC_MASK GENMASK(7, 0)
> +
> +#define ENETC_PTCFPR(n) (0x1910 + (n) * 4)
> +#define ENETC_FPE BIT(31)
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index e7482d483b28..f1873c4da77f 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -523,10 +523,15 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
> ENETC_PM0_CMD_TXP | ENETC_PM0_PROMISC |
> ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
> /* set auto-speed for RGMII */
> - if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG)
> + if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG) {
> enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_RGAUTO);
> - if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII)
> + enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_RGAUTO);
> + }
> +
> + if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII) {
> enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
> + enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_XGMII);
> + }
> }
>
> static void enetc_configure_port_pmac(struct enetc_hw *hw)
> @@ -745,6 +750,12 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
> if (si->hw_features & ENETC_SI_F_QBV)
> priv->active_offloads |= ENETC_F_QBV;
>
> + if (si->hw_features & ENETC_SI_F_QBU) {
> + ndev->hw_features |= NETIF_F_PREEMPTION;
> + ndev->features |= NETIF_F_PREEMPTION;
> + priv->active_offloads |= ENETC_F_QBU;
> + }
> +
> /* pick up primary MAC address from SI */
> enetc_get_primary_mac_addr(&si->hw, ndev->dev_addr);
> }
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> index 2e99438cb1bf..94dde847d052 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> @@ -169,6 +169,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
> priv->tx_ring[i]->index,
> taprio->enable ? 0 : i);
>
> + /* preemption off if TC priority is all 0 */
> + if ((err && taprio->enable) || !(err || taprio->enable))
> + enetc_preemption_set(ndev, 0);
> +
> return err;
> }
>
> --
> 2.17.1
>
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function
2019-11-27 9:59 ` [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function Po Liu
2019-11-27 11:00 ` Vladimir Oltean
@ 2019-12-04 1:35 ` Ivan Khoronzhuk
1 sibling, 0 replies; 39+ messages in thread
From: Ivan Khoronzhuk @ 2019-12-04 1:35 UTC (permalink / raw)
To: Po Liu
Cc: davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1, saeedm,
andrew, f.fainelli, alexandru.ardelean, jiri, ayal, pablo,
linux-kernel, netdev, vinicius.gomes, simon.horman,
Claudiu Manoil, Vladimir Oltean, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
On Wed, Nov 27, 2019 at 09:59:30AM +0000, Po Liu wrote:
Hi, Po Liu
>The interface follow up the ethtool 'preemption' set/get.
>Hardware features also need to set hw_features with
>NETIF_F_PREEMPTION flag. So ethtool could check kernel
>link features if there is preemption capability of port.
>
>There are two MACs in ENETC. One is express MAC which traffic
>classes in it are advanced transmition. Another is preemptable
>MAC which traffic classes are frame preemptable.
>
>The hardware need to initialize the MACs at initial stage.
>And then set the preemption enable registers of traffic
>classes when ethtool set .get_link_ksettings/.set_link_ksettings
>stage.
>
>To test the ENETC preemption capability, user need to set mqprio
>or taprio to mapping the traffic classes with priorities. Then
>use ethtool command to set 'preemption' with a 8 bits value.
>MSB represent high number traffic class.
>
>Signed-off-by: Po Liu <Po.Liu@nxp.com>
>---
> drivers/net/ethernet/freescale/enetc/enetc.c | 3 +
> drivers/net/ethernet/freescale/enetc/enetc.h | 4 +
> .../ethernet/freescale/enetc/enetc_ethtool.c | 142 ++++++++++++++++--
> .../net/ethernet/freescale/enetc/enetc_hw.h | 17 +++
> .../net/ethernet/freescale/enetc/enetc_pf.c | 15 +-
> .../net/ethernet/freescale/enetc/enetc_qos.c | 4 +
[...]
>+
>+static u32 enetc_preemption_get(struct net_device *ndev)
>+{
>+ struct enetc_ndev_priv *priv = netdev_priv(ndev);
>+ u32 ptvector = 0;
>+ u8 tc_num;
>+ int i;
>+
>+ /* If preemptable MAC is not enable return 0 */
>+ if (!(enetc_port_rd(&priv->si->hw, ENETC_PFPMR) & ENETC_PFPMR_PMACE))
>+ return 0;
>+
>+ tc_num = enetc_get_tc_num(priv->si);
>+
>+ for (i = 0; i < tc_num; i++)
>+ if (enetc_port_rd(&priv->si->hw, ENETC_PTCFPR(i)) & ENETC_FPE)
>+ ptvector |= 1 << i;
Would be better to replace above on just priv var?
>+
>+ return ptvector;
>+}
>+
>+static int enetc_get_link_ksettings(struct net_device *ndev,
>+ struct ethtool_link_ksettings *cmd)
>+{
>+ cmd->base.preemption = enetc_preemption_get(ndev);
>+
>+ return phy_ethtool_get_link_ksettings(ndev, cmd);
>+}
>+
>+static int enetc_set_link_ksettings(struct net_device *ndev,
>+ const struct ethtool_link_ksettings *cmd)
>+{
>+ int err;
>+
>+ err = enetc_preemption_set(ndev, cmd->base.preemption);
>+ if (err)
>+ return err;
Shouldn't it be after
phy_ethtool_set_link_ksettings() ?
I mean after potential phy restart, does it have impact on it?
>+
>+ return phy_ethtool_set_link_ksettings(ndev, cmd);
>+}
>+
> static const struct ethtool_ops enetc_pf_ethtool_ops = {
> .get_regs_len = enetc_get_reglen,
> .get_regs = enetc_get_regs,
>@@ -622,8 +746,8 @@ static const struct ethtool_ops enetc_pf_ethtool_ops = {
> .get_rxfh = enetc_get_rxfh,
> .set_rxfh = enetc_set_rxfh,
> .get_ringparam = enetc_get_ringparam,
>- .get_link_ksettings = phy_ethtool_get_link_ksettings,
>- .set_link_ksettings = phy_ethtool_set_link_ksettings,
>+ .get_link_ksettings = enetc_get_link_ksettings,
>+ .set_link_ksettings = enetc_set_link_ksettings,
> .get_link = ethtool_op_get_link,
> .get_ts_info = enetc_get_ts_info,
> .get_wol = enetc_get_wol,
>diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
>index 51f543ef37a8..b609ec095710 100644
>--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
>+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
>@@ -19,6 +19,7 @@
> #define ENETC_SICTR1 0x1c
> #define ENETC_SIPCAPR0 0x20
> #define ENETC_SIPCAPR0_QBV BIT(4)
>+#define ENETC_SIPCAPR0_QBU BIT(3)
> #define ENETC_SIPCAPR0_RSS BIT(8)
> #define ENETC_SIPCAPR1 0x24
> #define ENETC_SITGTGR 0x30
>@@ -176,6 +177,7 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_PCAPR0_RXBDR(val) ((val) >> 24)
> #define ENETC_PCAPR0_TXBDR(val) (((val) >> 16) & 0xff)
> #define ENETC_PCAPR1 0x0904
>+#define ENETC_NUM_TCS_MASK GENMASK(6, 4)
> #define ENETC_PSICFGR0(n) (0x0940 + (n) * 0xc) /* n = SI index */
> #define ENETC_PSICFGR0_SET_TXBDR(val) ((val) & 0xff)
> #define ENETC_PSICFGR0_SET_RXBDR(val) (((val) & 0xff) << 16)
>@@ -223,6 +225,7 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_SET_TX_MTU(val) ((val) << 16)
> #define ENETC_SET_MAXFRM(val) ((val) & 0xffff)
> #define ENETC_PM0_IF_MODE 0x8300
>+#define ENETC_PM1_IF_MODE 0x9300
> #define ENETC_PMO_IFM_RG BIT(2)
> #define ENETC_PM0_IFM_RLP (BIT(5) | BIT(11))
> #define ENETC_PM0_IFM_RGAUTO (BIT(15) | ENETC_PMO_IFM_RG | BIT(1))
>@@ -276,6 +279,15 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_PM0_TSCOL 0x82E0
> #define ENETC_PM0_TLCOL 0x82E8
> #define ENETC_PM0_TECOL 0x82F0
>+#define ENETC_PM1_RFRM 0x9120
>+#define ENETC_PM1_RDRP 0x9158
>+#define ENETC_PM1_RPKT 0x9160
>+#define ENETC_PM1_RFRG 0x91B8
>+#define ENETC_PM1_TFRM 0x9220
>+#define ENETC_PM1_TERR 0x9238
>+#define ENETC_PM1_TPKT 0x9260
>+#define ENETC_MAC_MERGE_MMFCRXR 0x1f14
>+#define ENETC_MAC_MERGE_MMFCTXR 0x1f18
>
> /* Port counters */
> #define ENETC_PICDR(n) (0x0700 + (n) * 8) /* n = [0..3] */
>@@ -615,3 +627,8 @@ struct enetc_cbd {
> /* Port time gating capability register */
> #define ENETC_QBV_PTGCAPR_OFFSET 0x11a08
> #define ENETC_QBV_MAX_GCL_LEN_MASK GENMASK(15, 0)
>+
>+#define ENETC_QBU_TC_MASK GENMASK(7, 0)
>+
>+#define ENETC_PTCFPR(n) (0x1910 + (n) * 4)
>+#define ENETC_FPE BIT(31)
>diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>index e7482d483b28..f1873c4da77f 100644
>--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>@@ -523,10 +523,15 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
> ENETC_PM0_CMD_TXP | ENETC_PM0_PROMISC |
> ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
> /* set auto-speed for RGMII */
>- if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG)
>+ if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG) {
> enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_RGAUTO);
>- if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII)
>+ enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_RGAUTO);
>+ }
>+
>+ if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII) {
> enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
>+ enetc_port_wr(hw, ENETC_PM1_IF_MODE, ENETC_PM0_IFM_XGMII);
>+ }
> }
>
> static void enetc_configure_port_pmac(struct enetc_hw *hw)
>@@ -745,6 +750,12 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
> if (si->hw_features & ENETC_SI_F_QBV)
> priv->active_offloads |= ENETC_F_QBV;
>
>+ if (si->hw_features & ENETC_SI_F_QBU) {
>+ ndev->hw_features |= NETIF_F_PREEMPTION;
>+ ndev->features |= NETIF_F_PREEMPTION;
>+ priv->active_offloads |= ENETC_F_QBU;
>+ }
>+
> /* pick up primary MAC address from SI */
> enetc_get_primary_mac_addr(&si->hw, ndev->dev_addr);
> }
>diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
>index 2e99438cb1bf..94dde847d052 100644
>--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
>+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
>@@ -169,6 +169,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
> priv->tx_ring[i]->index,
> taprio->enable ? 0 : i);
>
>+ /* preemption off if TC priority is all 0 */
>+ if ((err && taprio->enable) || !(err || taprio->enable))
>+ enetc_preemption_set(ndev, 0);
>+
> return err;
> }
>
>--
>2.17.1
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
2019-11-27 9:59 ` [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function Po Liu
@ 2019-11-27 18:57 ` David Miller
2019-12-03 15:11 ` Ivan Khoronzhuk
` (3 subsequent siblings)
5 siblings, 0 replies; 39+ messages in thread
From: David Miller @ 2019-11-27 18:57 UTC (permalink / raw)
To: po.liu
Cc: hauke.mehrtens, gregkh, allison, tglx, hkallweit1, saeedm,
andrew, f.fainelli, alexandru.ardelean, jiri, ayal, pablo,
linux-kernel, netdev, vinicius.gomes, simon.horman,
claudiu.manoil, vladimir.oltean, alexandru.marginean,
xiaoliang.yang_1, roy.zang, mingkai.hu, jerry.huang, leoyang.li
net-next is closed, please repost this series when net-next opens back up.
Thank you.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
2019-11-27 9:59 ` [v1,net-next, 2/2] enetc: implement the enetc 802.1Qbu hardware function Po Liu
2019-11-27 18:57 ` [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes David Miller
@ 2019-12-03 15:11 ` Ivan Khoronzhuk
2019-12-11 2:52 ` Andre Guedes
` (2 subsequent siblings)
5 siblings, 0 replies; 39+ messages in thread
From: Ivan Khoronzhuk @ 2019-12-03 15:11 UTC (permalink / raw)
To: Po Liu
Cc: davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1, saeedm,
andrew, f.fainelli, alexandru.ardelean, jiri, ayal, pablo,
linux-kernel, netdev, vinicius.gomes, simon.horman,
Claudiu Manoil, Vladimir Oltean, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
On Wed, Nov 27, 2019 at 09:59:18AM +0000, Po Liu wrote:
Hi, Po Liu
>IEEE Std 802.1Qbu standard defined the frame preemption of port
>traffic classes. This patch introduce a method to set traffic
>classes preemption. Add a parameter 'preemption' in struct
>ethtool_link_settings. The value will be translated to a binary,
>each bit represent a traffic class. Bit "1" means preemptable
>traffic class. Bit "0" means express traffic class. MSB represent
>high number traffic class.
>
>If hardware support the frame preemption, driver could set the
>ethernet device with hw_features and features with NETIF_F_PREEMPTION
>when initializing the port driver.
>
>User can check the feature 'tx-preemption' by command 'ethtool -k
>devname'. If hareware set preemption feature. The property would
>be a fixed value 'on' if hardware support the frame preemption.
>Feature would show a fixed value 'off' if hardware don't support
>the frame preemption.
>
>Command 'ethtool devname' and 'ethtool -s devname preemption N'
>would show/set which traffic classes are frame preemptable.
>
>Port driver would implement the frame preemption in the function
>get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>
>Signed-off-by: Po Liu <Po.Liu@nxp.com>
>---
> include/linux/netdev_features.h | 5 ++++-
> include/uapi/linux/ethtool.h | 5 ++++-
> net/core/ethtool.c | 1 +
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>index 4b19c544c59a..299750a8b414 100644
>--- a/include/linux/netdev_features.h
>+++ b/include/linux/netdev_features.h
>@@ -80,6 +80,7 @@ enum {
>
> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
>+ NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>
> /*
> * Add your fresh new feature above and remember to update
>@@ -150,6 +151,7 @@ enum {
> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
>+#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>
> /* Finds the next feature with the highest number of the range of start till 0.
> */
>@@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
>- NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
>+ NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
>+ NETIF_F_PREEMPTION)
>
> /* remember that ((t)1 << t_BITS) is undefined in C99 */
> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
>diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>index d4591792f0b4..12ffb34afbfa 100644
>--- a/include/uapi/linux/ethtool.h
>+++ b/include/uapi/linux/ethtool.h
>@@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
> };
> #define ETH_RESET_SHARED_SHIFT 16
>
>+/* Disable preemtion. */
>+#define PREEMPTION_DISABLE 0x0
>
> /**
> * struct ethtool_link_settings - link control and status
>@@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
> __s8 link_mode_masks_nwords;
> __u8 transceiver;
> __u8 reserved1[3];
>- __u32 reserved[7];
>+ __u32 preemption;
Why 32 when only 8 is needed?
>+ __u32 reserved[6];
> __u32 link_mode_masks[0];
> /* layout of link_mode_masks fields:
> * __u32 map_supported[link_mode_masks_nwords];
>diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>index cd9bc67381b2..6ffcd8a602b8 100644
>--- a/net/core/ethtool.c
>+++ b/net/core/ethtool.c
>@@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
>+ [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
What about tx-frame-preempt? or frame-preemption?
> };
>
> static const char
>--
>2.17.1
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
` (2 preceding siblings ...)
2019-12-03 15:11 ` Ivan Khoronzhuk
@ 2019-12-11 2:52 ` Andre Guedes
2019-12-16 7:43 ` [EXT] " Po Liu
2020-01-18 0:03 ` Vinicius Costa Gomes
2020-02-21 21:43 ` Vinicius Costa Gomes
5 siblings, 1 reply; 39+ messages in thread
From: Andre Guedes @ 2019-12-11 2:52 UTC (permalink / raw)
To: alexandru.ardelean, allison, andrew, ayal, davem, f.fainelli,
gregkh, hauke.mehrtens, hkallweit1, jiri, linux-kernel, netdev,
pablo, saeedm, tglx, Po Liu
Cc: vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Po Liu
Hi Po,
Quoting Po Liu (2019-11-27 01:59:18)
> IEEE Std 802.1Qbu standard defined the frame preemption of port
> traffic classes. This patch introduce a method to set traffic
> classes preemption. Add a parameter 'preemption' in struct
> ethtool_link_settings. The value will be translated to a binary,
> each bit represent a traffic class. Bit "1" means preemptable
> traffic class. Bit "0" means express traffic class. MSB represent
> high number traffic class.
>
> If hardware support the frame preemption, driver could set the
> ethernet device with hw_features and features with NETIF_F_PREEMPTION
> when initializing the port driver.
>
> User can check the feature 'tx-preemption' by command 'ethtool -k
> devname'. If hareware set preemption feature. The property would
> be a fixed value 'on' if hardware support the frame preemption.
> Feature would show a fixed value 'off' if hardware don't support
> the frame preemption.
>
> Command 'ethtool devname' and 'ethtool -s devname preemption N'
> would show/set which traffic classes are frame preemptable.
>
> Port driver would implement the frame preemption in the function
> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
In an early RFC series [1], we proposed a way to support frame preemption. I'm not
sure if you have considered it before implementing this other proposal based on
ethtool interface so I thought it would be a good idea to bring that up to your
attention, just in case.
In that initial proposal, Frame Preemption feature is configured via taprio
qdisc. For example:
$ tc qdisc add dev IFACE parent root handle 100 taprio \
num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
queues 1@0 1@1 2@2 \
preemption 0 1 1 1 \
base-time 10000000 \
sched-entry S 01 300000 \
sched-entry S 02 300000 \
sched-entry S 04 400000 \
clockid CLOCK_TAI
It also aligns with the gate control operations Set-And-Hold-MAC and
Set-And-Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
802.1Q-2018 for further details.
Please share your thoughts on this.
Regards,
Andre
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-11 2:52 ` Andre Guedes
@ 2019-12-16 7:43 ` Po Liu
2019-12-16 21:44 ` Vinicius Costa Gomes
0 siblings, 1 reply; 39+ messages in thread
From: Po Liu @ 2019-12-16 7:43 UTC (permalink / raw)
To: Andre Guedes, alexandru.ardelean, allison, andrew, ayal, davem,
f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
Murali Karicheri, Ivan Khoronzhuk
Cc: vinicius.gomes, simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi Andre,
Br,
Po Liu
> -----Original Message-----
> From: Andre Guedes <andre.guedes@linux.intel.com>
> Sent: 2019年12月11日 10:53
> To: alexandru.ardelean@analog.com; allison@lohutok.net; andrew@lunn.ch;
> ayal@mellanox.com; davem@davemloft.net; f.fainelli@gmail.com;
> gregkh@linuxfoundation.org; hauke.mehrtens@intel.com;
> hkallweit1@gmail.com; jiri@mellanox.com; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; pablo@netfilter.org; saeedm@mellanox.com;
> tglx@linutronix.de; Po Liu <po.liu@nxp.com>
> Cc: vinicius.gomes@intel.com; simon.horman@netronome.com; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
> traffic classes
>
> Caution: EXT Email
>
> Hi Po,
>
> Quoting Po Liu (2019-11-27 01:59:18)
> > IEEE Std 802.1Qbu standard defined the frame preemption of port
> > traffic classes. This patch introduce a method to set traffic classes
> > preemption. Add a parameter 'preemption' in struct
> > ethtool_link_settings. The value will be translated to a binary, each
> > bit represent a traffic class. Bit "1" means preemptable traffic
> > class. Bit "0" means express traffic class. MSB represent high number
> > traffic class.
> >
> > If hardware support the frame preemption, driver could set the
> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
> > when initializing the port driver.
> >
> > User can check the feature 'tx-preemption' by command 'ethtool -k
> > devname'. If hareware set preemption feature. The property would be a
> > fixed value 'on' if hardware support the frame preemption.
> > Feature would show a fixed value 'off' if hardware don't support the
> > frame preemption.
> >
> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
> > would show/set which traffic classes are frame preemptable.
> >
> > Port driver would implement the frame preemption in the function
> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>
> In an early RFC series [1], we proposed a way to support frame preemption. I'm
> not sure if you have considered it before implementing this other proposal
> based on ethtool interface so I thought it would be a good idea to bring that up
> to your attention, just in case.
Sorry, I didn't notice the RFC proposal. Using ethtool set the preemption just thinking about 8021Qbu as standalone. And not limit to the taprio if user won't set 802.1Qbv.
As some feedback also want to set the MAC merge minimal fragment size and get some more information of 802.3br.
>
> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
> For example:
>
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> num_tc 3 \
> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> queues 1@0 1@1 2@2 \
> preemption 0 1 1 1 \
> base-time 10000000 \
> sched-entry S 01 300000 \
> sched-entry S 02 300000 \
> sched-entry S 04 400000 \
> clockid CLOCK_TAI
>
> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
> 802.1Q-2018 for further details.
I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it could be understand as guardband by hardware preemption. MAC should auto calculate the nano seconds before express entry slot start to break to two fragments. Set-And-Hold-MAC should minimal larger than the fragment-size oct times.
>
> Please share your thoughts on this.
I am good to see there is frame preemption proposal. Each way is ok for me but ethtool is more flexible. I've seen the RFC the code. The hardware offload is in the mainline, but preemption is not yet, I don't know why. Could you post it again?
> Regards,
>
> Andre
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-16 7:43 ` [EXT] " Po Liu
@ 2019-12-16 21:44 ` Vinicius Costa Gomes
2019-12-19 0:43 ` Ivan Khoronzhuk
2020-01-09 0:56 ` Andre Guedes
0 siblings, 2 replies; 39+ messages in thread
From: Vinicius Costa Gomes @ 2019-12-16 21:44 UTC (permalink / raw)
To: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
Murali Karicheri, Ivan Khoronzhuk
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi Po,
Po Liu <po.liu@nxp.com> writes:
> Hi Andre,
>
>
> Br,
> Po Liu
>
>> -----Original Message-----
>> From: Andre Guedes <andre.guedes@linux.intel.com>
>> Sent: 2019年12月11日 10:53
>> To: alexandru.ardelean@analog.com; allison@lohutok.net; andrew@lunn.ch;
>> ayal@mellanox.com; davem@davemloft.net; f.fainelli@gmail.com;
>> gregkh@linuxfoundation.org; hauke.mehrtens@intel.com;
>> hkallweit1@gmail.com; jiri@mellanox.com; linux-kernel@vger.kernel.org;
>> netdev@vger.kernel.org; pablo@netfilter.org; saeedm@mellanox.com;
>> tglx@linutronix.de; Po Liu <po.liu@nxp.com>
>> Cc: vinicius.gomes@intel.com; simon.horman@netronome.com; Claudiu Manoil
>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>> traffic classes
>>
>> Caution: EXT Email
>>
>> Hi Po,
>>
>> Quoting Po Liu (2019-11-27 01:59:18)
>> > IEEE Std 802.1Qbu standard defined the frame preemption of port
>> > traffic classes. This patch introduce a method to set traffic classes
>> > preemption. Add a parameter 'preemption' in struct
>> > ethtool_link_settings. The value will be translated to a binary, each
>> > bit represent a traffic class. Bit "1" means preemptable traffic
>> > class. Bit "0" means express traffic class. MSB represent high number
>> > traffic class.
>> >
>> > If hardware support the frame preemption, driver could set the
>> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
>> > when initializing the port driver.
>> >
>> > User can check the feature 'tx-preemption' by command 'ethtool -k
>> > devname'. If hareware set preemption feature. The property would be a
>> > fixed value 'on' if hardware support the frame preemption.
>> > Feature would show a fixed value 'off' if hardware don't support the
>> > frame preemption.
Having some knobs in ethtool to enable when/how Frame Preemption is
advertised on the wire makes sense. I also agree that it should be "on"
by default.
>> >
>> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
>> > would show/set which traffic classes are frame preemptable.
>> >
>> > Port driver would implement the frame preemption in the function
>> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>
>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>> not sure if you have considered it before implementing this other proposal
>> based on ethtool interface so I thought it would be a good idea to bring that up
>> to your attention, just in case.
>
> Sorry, I didn't notice the RFC proposal. Using ethtool set the
> preemption just thinking about 8021Qbu as standalone. And not limit to
> the taprio if user won't set 802.1Qbv.
I see your point of using frame-preemption "standalone", I have two
ideas:
1. add support in taprio to be configured without any schedule in the
"full offload" mode. In practice, allowing taprio to work somewhat
similar to (mqprio + frame-preemption), changes in the code should de
fairly small;
2. extend mqprio to support frame-preemption;
>
> As some feedback also want to set the MAC merge minimal fragment size
> and get some more information of 802.3br.
The minimal fragment size, I guess, also makes sense to be kept in
ethtool. That is we have a sane default, and allow the user to change
this setting for special cases.
>
>>
>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>> For example:
>>
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> num_tc 3 \
>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>> queues 1@0 1@1 2@2 \
>> preemption 0 1 1 1 \
>> base-time 10000000 \
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> sched-entry S 04 400000 \
>> clockid CLOCK_TAI
>>
>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>> 802.1Q-2018 for further details.
>
> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
> could be understand as guardband by hardware preemption. MAC should
> auto calculate the nano seconds before express entry slot start to
> break to two fragments. Set-And-Hold-MAC should minimal larger than
> the fragment-size oct times.
Another interesting point. My first idea is that when the schedule is
offloaded to the driver and the driver detects that the "entry" width is
smaller than the fragment side, the driver could reject that schedule
with a nice error message.
>
>>
>> Please share your thoughts on this.
>
> I am good to see there is frame preemption proposal. Each way is ok
> for me but ethtool is more flexible. I've seen the RFC the code. The
> hardware offload is in the mainline, but preemption is not yet, I
> don't know why. Could you post it again?
It's not mainline because this kind of stuff will not be accepted
upstream without in-tree users. And you are the first one to propose
such a thing :-)
It's just now that I have something that supports frame-preemption, the
code I have is approaching RFC-like quality. I will send another RFC
this week hopefully, and we can see how things look in practice.
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-16 21:44 ` Vinicius Costa Gomes
@ 2019-12-19 0:43 ` Ivan Khoronzhuk
2019-12-19 1:54 ` Vinicius Costa Gomes
2020-01-09 0:56 ` Andre Guedes
1 sibling, 1 reply; 39+ messages in thread
From: Ivan Khoronzhuk @ 2019-12-19 0:43 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
Murali Karicheri, simon.horman, Claudiu Manoil,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
On Mon, Dec 16, 2019 at 01:44:13PM -0800, Vinicius Costa Gomes wrote:
>Hi Po,
>
>Po Liu <po.liu@nxp.com> writes:
>
>> Hi Andre,
>>
>>
>> Br,
>> Po Liu
>>
>>> -----Original Message-----
>>> From: Andre Guedes <andre.guedes@linux.intel.com>
>>> Sent: 2019年12月11日 10:53
>>> To: alexandru.ardelean@analog.com; allison@lohutok.net; andrew@lunn.ch;
>>> ayal@mellanox.com; davem@davemloft.net; f.fainelli@gmail.com;
>>> gregkh@linuxfoundation.org; hauke.mehrtens@intel.com;
>>> hkallweit1@gmail.com; jiri@mellanox.com; linux-kernel@vger.kernel.org;
>>> netdev@vger.kernel.org; pablo@netfilter.org; saeedm@mellanox.com;
>>> tglx@linutronix.de; Po Liu <po.liu@nxp.com>
>>> Cc: vinicius.gomes@intel.com; simon.horman@netronome.com; Claudiu Manoil
>>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>>> traffic classes
>>>
>>> Caution: EXT Email
>>>
>>> Hi Po,
>>>
>>> Quoting Po Liu (2019-11-27 01:59:18)
>>> > IEEE Std 802.1Qbu standard defined the frame preemption of port
>>> > traffic classes. This patch introduce a method to set traffic classes
>>> > preemption. Add a parameter 'preemption' in struct
>>> > ethtool_link_settings. The value will be translated to a binary, each
>>> > bit represent a traffic class. Bit "1" means preemptable traffic
>>> > class. Bit "0" means express traffic class. MSB represent high number
>>> > traffic class.
>>> >
>>> > If hardware support the frame preemption, driver could set the
>>> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>> > when initializing the port driver.
>>> >
>>> > User can check the feature 'tx-preemption' by command 'ethtool -k
>>> > devname'. If hareware set preemption feature. The property would be a
>>> > fixed value 'on' if hardware support the frame preemption.
>>> > Feature would show a fixed value 'off' if hardware don't support the
>>> > frame preemption.
>
>Having some knobs in ethtool to enable when/how Frame Preemption is
>advertised on the wire makes sense. I also agree that it should be "on"
>by default.
>
>>> >
>>> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>> > would show/set which traffic classes are frame preemptable.
>>> >
>>> > Port driver would implement the frame preemption in the function
>>> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>
>>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>>> not sure if you have considered it before implementing this other proposal
>>> based on ethtool interface so I thought it would be a good idea to bring that up
>>> to your attention, just in case.
>>
>> Sorry, I didn't notice the RFC proposal. Using ethtool set the
>> preemption just thinking about 8021Qbu as standalone. And not limit to
>> the taprio if user won't set 802.1Qbv.
>
>I see your point of using frame-preemption "standalone", I have two
>ideas:
>
> 1. add support in taprio to be configured without any schedule in the
> "full offload" mode. In practice, allowing taprio to work somewhat
> similar to (mqprio + frame-preemption), changes in the code should de
> fairly small;
+
And if follow mqprio settings logic then preemption also can be enabled
immediately while configuring taprio first time, and similarly new ADMIN
can't change it and can be set w/o preemption option afterwards.
So that following is correct:
OPER
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 10000000 \
num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
queues 1@0 1@1 2@2 \
preemption 0 1 1 1
flags 1
then
ADMIN
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 12000000 \
num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
queues 1@0 1@1 2@2 \
preemption 0 1 1 1
sched-entry S 01 300000 \
sched-entry S 02 300000 \
flags 1
then
ADMIN
$ tc qdisc add dev IFACE parent root handle 100 taprio \
base-time 13000000 \
sched-entry S 01 300000 \
sched-entry S 02 300000 \
flags 1
BUT:
1) The question is only should it be in this way? I mean preemption to be
enabled immediately? Also should include other parameters like fragment size.
2) What if I want to use frame preemption with another "transmission selection
algorithm"? Say another one "time sensitive" - CBS? How is it going to be
stacked?
In this case ethtool looks better, allowing this "MAC level" feature, to be
configured separately.
>
> 2. extend mqprio to support frame-preemption;
>
>>
>> As some feedback also want to set the MAC merge minimal fragment size
>> and get some more information of 802.3br.
>
>The minimal fragment size, I guess, also makes sense to be kept in
>ethtool. That is we have a sane default, and allow the user to change
>this setting for special cases.
>
>>
>>>
>>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>>> For example:
>>>
>>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>>> num_tc 3 \
>>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>>> queues 1@0 1@1 2@2 \
>>> preemption 0 1 1 1 \
>>> base-time 10000000 \
>>> sched-entry S 01 300000 \
>>> sched-entry S 02 300000 \
>>> sched-entry S 04 400000 \
>>> clockid CLOCK_TAI
>>>
>>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>>> 802.1Q-2018 for further details.
>>
>> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
>> could be understand as guardband by hardware preemption. MAC should
>> auto calculate the nano seconds before express entry slot start to
>> break to two fragments. Set-And-Hold-MAC should minimal larger than
>> the fragment-size oct times.
>
>Another interesting point. My first idea is that when the schedule is
>offloaded to the driver and the driver detects that the "entry" width is
>smaller than the fragment side, the driver could reject that schedule
>with a nice error message.
Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
only if it contains express queues. And if for entry is expectable to have
interval shorter, the entry has to be marked as HOLD then.
But not every offload is able to support mac/hold per sched (there is
no HOLD/RELEASE commands in this case). For this case seems like here can
be 2 cases:
1) there is no “gate close” event for the preemptible traffic
2) there is "gate close" event for the preemptable traffic
And both can have the following impact, if assume the main reason to
this guard check is to guarantee the express queue cannot be blocked while
this "close to short" interval opening ofc:
If a preemption fragment is started before "express" frame, then interval
should allow to complete preemption fragment and has to have enough time
to insert express frame. So here situation when maximum packet size per
each queue can have place.
In case of TI am65 this queue MTU is configurable per queue (for similar
reasons and couple more (packet fill feature for instance)) and can be
used for guard check also, but not clear where it should be. Seems like
it should be done using ethtool also, but can be needed for taprio
interface....
>>
>>>
>>> Please share your thoughts on this.
>>
>> I am good to see there is frame preemption proposal. Each way is ok
>> for me but ethtool is more flexible. I've seen the RFC the code. The
>> hardware offload is in the mainline, but preemption is not yet, I
>> don't know why. Could you post it again?
>
>It's not mainline because this kind of stuff will not be accepted
>upstream without in-tree users. And you are the first one to propose
>such a thing :-)
>
>It's just now that I have something that supports frame-preemption, the
>code I have is approaching RFC-like quality. I will send another RFC
>this week hopefully, and we can see how things look in practice.
>
>
>Cheers,
>--
>Vinicius
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-19 0:43 ` Ivan Khoronzhuk
@ 2019-12-19 1:54 ` Vinicius Costa Gomes
2019-12-30 16:56 ` Murali Karicheri
2019-12-30 17:03 ` Murali Karicheri
0 siblings, 2 replies; 39+ messages in thread
From: Vinicius Costa Gomes @ 2019-12-19 1:54 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
Murali Karicheri, simon.horman, Claudiu Manoil,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi Ivan,
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> writes:
>>>> Quoting Po Liu (2019-11-27 01:59:18)
>>>> > IEEE Std 802.1Qbu standard defined the frame preemption of port
>>>> > traffic classes. This patch introduce a method to set traffic classes
>>>> > preemption. Add a parameter 'preemption' in struct
>>>> > ethtool_link_settings. The value will be translated to a binary, each
>>>> > bit represent a traffic class. Bit "1" means preemptable traffic
>>>> > class. Bit "0" means express traffic class. MSB represent high number
>>>> > traffic class.
>>>> >
>>>> > If hardware support the frame preemption, driver could set the
>>>> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>>> > when initializing the port driver.
>>>> >
>>>> > User can check the feature 'tx-preemption' by command 'ethtool -k
>>>> > devname'. If hareware set preemption feature. The property would be a
>>>> > fixed value 'on' if hardware support the frame preemption.
>>>> > Feature would show a fixed value 'off' if hardware don't support the
>>>> > frame preemption.
>>
>>Having some knobs in ethtool to enable when/how Frame Preemption is
>>advertised on the wire makes sense. I also agree that it should be "on"
>>by default.
>>
>>>> >
>>>> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>>> > would show/set which traffic classes are frame preemptable.
>>>> >
>>>> > Port driver would implement the frame preemption in the function
>>>> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>>
>>>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>>>> not sure if you have considered it before implementing this other proposal
>>>> based on ethtool interface so I thought it would be a good idea to bring that up
>>>> to your attention, just in case.
>>>
>>> Sorry, I didn't notice the RFC proposal. Using ethtool set the
>>> preemption just thinking about 8021Qbu as standalone. And not limit to
>>> the taprio if user won't set 802.1Qbv.
>>
>>I see your point of using frame-preemption "standalone", I have two
>>ideas:
>>
>> 1. add support in taprio to be configured without any schedule in the
>> "full offload" mode. In practice, allowing taprio to work somewhat
>> similar to (mqprio + frame-preemption), changes in the code should de
>> fairly small;
>
> +
>
> And if follow mqprio settings logic then preemption also can be enabled
> immediately while configuring taprio first time, and similarly new ADMIN
> can't change it and can be set w/o preemption option afterwards.
>
> So that following is correct:
>
> OPER
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 10000000 \
> num_tc 3 \
> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> queues 1@0 1@1 2@2 \
> preemption 0 1 1 1
> flags 1
>
> then
> ADMIN
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 12000000 \
> num_tc 3 \
> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> queues 1@0 1@1 2@2 \
> preemption 0 1 1 1
> sched-entry S 01 300000 \
> sched-entry S 02 300000 \
> flags 1
>
> then
> ADMIN
> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> base-time 13000000 \
> sched-entry S 01 300000 \
> sched-entry S 02 300000 \
> flags 1
>
> BUT:
>
> 1) The question is only should it be in this way? I mean preemption to be
> enabled immediately? Also should include other parameters like
> fragment size.
We can decide what things are allowed/useful here. For example, it might
make sense to allow "preemption" to be changed. We can extend taprio to
support changing the fragment size, if that makes sense.
>
> 2) What if I want to use frame preemption with another "transmission selection
> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> stacked?
I am not seeing any (conceptual*) problems when plugging a cbs (for
example) qdisc into one of taprio children. Or, are you talking about a
more general problem?
* here I am considering that support for taprio without an schedule is
added.
>
> In this case ethtool looks better, allowing this "MAC level" feature, to be
> configured separately.
My only issue with using ethtool is that then we would have two
different interfaces for "complementary" features. And it would make
things even harder to configure and debug. The fact that one talks about
traffic classes and the other transmission queues doesn't make me more
comfortable as well.
On the other hand, as there isn't a way to implement frame preemption in
software, I agree that it makes it kind of awkward to have it in the tc
subsystem.
At this point, I am slightly in favor of the taprio approach (yes, I am
biased :-), but I can be convinced otherwise. I will be only a little
sad if we choose to go with ethtool for now, and then add support up in
the stack, something similar to "ethtool -N" and "tc-flower".
>
>>
>> 2. extend mqprio to support frame-preemption;
>>
>>>
>>> As some feedback also want to set the MAC merge minimal fragment size
>>> and get some more information of 802.3br.
>>
>>The minimal fragment size, I guess, also makes sense to be kept in
>>ethtool. That is we have a sane default, and allow the user to change
>>this setting for special cases.
>>
>>>
>>>>
>>>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>>>> For example:
>>>>
>>>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>>>> num_tc 3 \
>>>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>>>> queues 1@0 1@1 2@2 \
>>>> preemption 0 1 1 1 \
>>>> base-time 10000000 \
>>>> sched-entry S 01 300000 \
>>>> sched-entry S 02 300000 \
>>>> sched-entry S 04 400000 \
>>>> clockid CLOCK_TAI
>>>>
>>>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>>>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>>>> 802.1Q-2018 for further details.
>>>
>>> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
>>> could be understand as guardband by hardware preemption. MAC should
>>> auto calculate the nano seconds before express entry slot start to
>>> break to two fragments. Set-And-Hold-MAC should minimal larger than
>>> the fragment-size oct times.
>>
>>Another interesting point. My first idea is that when the schedule is
>>offloaded to the driver and the driver detects that the "entry" width is
>>smaller than the fragment side, the driver could reject that schedule
>>with a nice error message.
>
> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
> only if it contains express queues. And if for entry is expectable to have
> interval shorter, the entry has to be marked as HOLD then.
>
> But not every offload is able to support mac/hold per sched (there is
> no HOLD/RELEASE commands in this case). For this case seems like here can
> be 2 cases:
Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
commands.
>
> 1) there is no “gate close” event for the preemptible traffic
> 2) there is "gate close" event for the preemptable traffic
>
> And both can have the following impact, if assume the main reason to
> this guard check is to guarantee the express queue cannot be blocked while
> this "close to short" interval opening ofc:
>
> If a preemption fragment is started before "express" frame, then interval
> should allow to complete preemption fragment and has to have enough time
> to insert express frame. So here situation when maximum packet size per
> each queue can have place.
>
> In case of TI am65 this queue MTU is configurable per queue (for similar
> reasons and couple more (packet fill feature for instance)) and can be
> used for guard check also, but not clear where it should be. Seems like
> it should be done using ethtool also, but can be needed for taprio
> interface....
For now, at least for the hardware I am working on, something like this
is configurable, but I wasn't planning on exposing it, using the maximum
ethernet frame size seemed a good default.
>
>>>
>>>>
>>>> Please share your thoughts on this.
>>>
>>> I am good to see there is frame preemption proposal. Each way is ok
>>> for me but ethtool is more flexible. I've seen the RFC the code. The
>>> hardware offload is in the mainline, but preemption is not yet, I
>>> don't know why. Could you post it again?
>>
>>It's not mainline because this kind of stuff will not be accepted
>>upstream without in-tree users. And you are the first one to propose
>>such a thing :-)
>>
>>It's just now that I have something that supports frame-preemption, the
>>code I have is approaching RFC-like quality. I will send another RFC
>>this week hopefully, and we can see how things look in practice.
>>
>>
>>Cheers,
>>--
>>Vinicius
>
> --
> Regards,
> Ivan Khoronzhuk
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-19 1:54 ` Vinicius Costa Gomes
@ 2019-12-30 16:56 ` Murali Karicheri
2020-01-17 23:47 ` Vinicius Costa Gomes
2019-12-30 17:03 ` Murali Karicheri
1 sibling, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2019-12-30 16:56 UTC (permalink / raw)
To: Vinicius Costa Gomes, Ivan Khoronzhuk
Cc: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
Hi Vinicius,
On 12/18/2019 08:54 PM, Vinicius Costa Gomes wrote:
> Hi Ivan,
>
> Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> writes:
>
>>>>> Quoting Po Liu (2019-11-27 01:59:18)
>>>>>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>>>>>> traffic classes. This patch introduce a method to set traffic classes
>>>>>> preemption. Add a parameter 'preemption' in struct
>>>>>> ethtool_link_settings. The value will be translated to a binary, each
>>>>>> bit represent a traffic class. Bit "1" means preemptable traffic
>>>>>> class. Bit "0" means express traffic class. MSB represent high number
>>>>>> traffic class.
>>>>>>
>>>>>> If hardware support the frame preemption, driver could set the
>>>>>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>>>>> when initializing the port driver.
>>>>>>
>>>>>> User can check the feature 'tx-preemption' by command 'ethtool -k
>>>>>> devname'. If hareware set preemption feature. The property would be a
>>>>>> fixed value 'on' if hardware support the frame preemption.
>>>>>> Feature would show a fixed value 'off' if hardware don't support the
>>>>>> frame preemption.
>>>
>>> Having some knobs in ethtool to enable when/how Frame Preemption is
>>> advertised on the wire makes sense. I also agree that it should be "on"
>>> by default.
>>>
>>>>>>
>>>>>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>>>>> would show/set which traffic classes are frame preemptable.
>>>>>>
>>>>>> Port driver would implement the frame preemption in the function
>>>>>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>>>
>>>>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>>>>> not sure if you have considered it before implementing this other proposal
>>>>> based on ethtool interface so I thought it would be a good idea to bring that up
>>>>> to your attention, just in case.
>>>>
>>>> Sorry, I didn't notice the RFC proposal. Using ethtool set the
>>>> preemption just thinking about 8021Qbu as standalone. And not limit to
>>>> the taprio if user won't set 802.1Qbv.
>>>
>>> I see your point of using frame-preemption "standalone", I have two
>>> ideas:
>>>
>>> 1. add support in taprio to be configured without any schedule in the
>>> "full offload" mode. In practice, allowing taprio to work somewhat
>>> similar to (mqprio + frame-preemption), changes in the code should de
>>> fairly small;
>>
>> +
>>
>> And if follow mqprio settings logic then preemption also can be enabled
>> immediately while configuring taprio first time, and similarly new ADMIN
>> can't change it and can be set w/o preemption option afterwards.
>>
>> So that following is correct:
>>
>> OPER
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 10000000 \
>> num_tc 3 \
>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>> queues 1@0 1@1 2@2 \
>> preemption 0 1 1 1
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 12000000 \
>> num_tc 3 \
>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>> queues 1@0 1@1 2@2 \
>> preemption 0 1 1 1
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 13000000 \
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> BUT:
>>
>> 1) The question is only should it be in this way? I mean preemption to be
>> enabled immediately? Also should include other parameters like
>> fragment size.
>
> We can decide what things are allowed/useful here. For example, it might
> make sense to allow "preemption" to be changed. We can extend taprio to
> support changing the fragment size, if that makes sense.
>
The point is it make sense to configure pre-emption related parameters
independently of taprio since they are not related. User may use just
pre-emption to reduce latency in the communication path for their end
application.. So there should be a way to configure it independently of
taprio. Right??
>>
>> 2) What if I want to use frame preemption with another "transmission selection
>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>> stacked?
>
> I am not seeing any (conceptual*) problems when plugging a cbs (for
> example) qdisc into one of taprio children. Or, are you talking about a
> more general problem?
>
If I understand it correctly problem is not stacking taprio with cbs,
but rather pre-emption with other qdiscs and allow configuring
the parameters such as frag size. How do I use frame pre-emption as
an independent feature and configure frag size? Ethool appears to be
better from this point of view as Ivan has mentioned below.
Murali
> * here I am considering that support for taprio without an schedule is
> added.
>
>>
>> In this case ethtool looks better, allowing this "MAC level" feature, to be
>> configured separately.
>
> My only issue with using ethtool is that then we would have two
> different interfaces for "complementary" features. And it would make
> things even harder to configure and debug. The fact that one talks about
> traffic classes and the other transmission queues doesn't make me more
> comfortable as well.
>
> On the other hand, as there isn't a way to implement frame preemption in
> software, I agree that it makes it kind of awkward to have it in the tc
> subsystem.
>
> At this point, I am slightly in favor of the taprio approach (yes, I am
> biased :-), but I can be convinced otherwise. I will be only a little
> sad if we choose to go with ethtool for now, and then add support up in
> the stack, something similar to "ethtool -N" and "tc-flower".
>
>>
>>>
>>> 2. extend mqprio to support frame-preemption;
>>>
>>>>
>>>> As some feedback also want to set the MAC merge minimal fragment size
>>>> and get some more information of 802.3br.
>>>
>>> The minimal fragment size, I guess, also makes sense to be kept in
>>> ethtool. That is we have a sane default, and allow the user to change
>>> this setting for special cases.
>>>
>>>>
>>>>>
>>>>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>>>>> For example:
>>>>>
>>>>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>>>>> num_tc 3 \
>>>>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>>>>> queues 1@0 1@1 2@2 \
>>>>> preemption 0 1 1 1 \
>>>>> base-time 10000000 \
>>>>> sched-entry S 01 300000 \
>>>>> sched-entry S 02 300000 \
>>>>> sched-entry S 04 400000 \
>>>>> clockid CLOCK_TAI
>>>>>
>>>>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>>>>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>>>>> 802.1Q-2018 for further details.
>>>>
>>>> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
>>>> could be understand as guardband by hardware preemption. MAC should
>>>> auto calculate the nano seconds before express entry slot start to
>>>> break to two fragments. Set-And-Hold-MAC should minimal larger than
>>>> the fragment-size oct times.
>>>
>>> Another interesting point. My first idea is that when the schedule is
>>> offloaded to the driver and the driver detects that the "entry" width is
>>> smaller than the fragment side, the driver could reject that schedule
>>> with a nice error message.
>>
>> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
>> only if it contains express queues. And if for entry is expectable to have
>> interval shorter, the entry has to be marked as HOLD then.
>>
>> But not every offload is able to support mac/hold per sched (there is
>> no HOLD/RELEASE commands in this case). For this case seems like here can
>> be 2 cases:
>
> Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
> commands.
>
>>
>> 1) there is no “gate close” event for the preemptible traffic
>> 2) there is "gate close" event for the preemptable traffic
>>
>> And both can have the following impact, if assume the main reason to
>> this guard check is to guarantee the express queue cannot be blocked while
>> this "close to short" interval opening ofc:
>>
>> If a preemption fragment is started before "express" frame, then interval
>> should allow to complete preemption fragment and has to have enough time
>> to insert express frame. So here situation when maximum packet size per
>> each queue can have place.
>>
>> In case of TI am65 this queue MTU is configurable per queue (for similar
>> reasons and couple more (packet fill feature for instance)) and can be
>> used for guard check also, but not clear where it should be. Seems like
>> it should be done using ethtool also, but can be needed for taprio
>> interface....
>
> For now, at least for the hardware I am working on, something like this
> is configurable, but I wasn't planning on exposing it, using the maximum
> ethernet frame size seemed a good default.
>
>>
>>>>
>>>>>
>>>>> Please share your thoughts on this.
>>>>
>>>> I am good to see there is frame preemption proposal. Each way is ok
>>>> for me but ethtool is more flexible. I've seen the RFC the code. The
>>>> hardware offload is in the mainline, but preemption is not yet, I
>>>> don't know why. Could you post it again?
>>>
>>> It's not mainline because this kind of stuff will not be accepted
>>> upstream without in-tree users. And you are the first one to propose
>>> such a thing :-)
>>>
>>> It's just now that I have something that supports frame-preemption, the
>>> code I have is approaching RFC-like quality. I will send another RFC
>>> this week hopefully, and we can see how things look in practice.
>>>
>>>
>>> Cheers,
>>> --
>>> Vinicius
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
>
> Cheers,
> --
> Vinicius
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-30 16:56 ` Murali Karicheri
@ 2020-01-17 23:47 ` Vinicius Costa Gomes
0 siblings, 0 replies; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-01-17 23:47 UTC (permalink / raw)
To: Murali Karicheri, Ivan Khoronzhuk
Cc: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
Hi,
Murali Karicheri <m-karicheri2@ti.com> writes:
[...]
>>>
>>> 2) What if I want to use frame preemption with another "transmission selection
>>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>>> stacked?
>>
>> I am not seeing any (conceptual*) problems when plugging a cbs (for
>> example) qdisc into one of taprio children. Or, are you talking about a
>> more general problem?
>>
>
> If I understand it correctly problem is not stacking taprio with cbs,
> but rather pre-emption with other qdiscs and allow configuring
> the parameters such as frag size. How do I use frame pre-emption as
> an independent feature and configure frag size? Ethool appears to be
> better from this point of view as Ivan has mentioned below.
>
Yeah, after all the discussion here, I think we came to the rough
consensus that ethtool is the best place we have to enable/disable frame
preemption and its configuration knobs.
I just have a few comments to add to the series (sorry for the long
delay).
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-19 1:54 ` Vinicius Costa Gomes
2019-12-30 16:56 ` Murali Karicheri
@ 2019-12-30 17:03 ` Murali Karicheri
2020-01-09 1:07 ` Andre Guedes
1 sibling, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2019-12-30 17:03 UTC (permalink / raw)
To: Vinicius Costa Gomes, Ivan Khoronzhuk
Cc: Po Liu, Andre Guedes, alexandru.ardelean, allison, andrew, ayal,
davem, f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
Hi Vinicius,
On 12/18/2019 08:54 PM, Vinicius Costa Gomes wrote:
> Hi Ivan,
>
> Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> writes:
>
>>>>> Quoting Po Liu (2019-11-27 01:59:18)
>>>>>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>>>>>> traffic classes. This patch introduce a method to set traffic classes
>>>>>> preemption. Add a parameter 'preemption' in struct
>>>>>> ethtool_link_settings. The value will be translated to a binary, each
>>>>>> bit represent a traffic class. Bit "1" means preemptable traffic
>>>>>> class. Bit "0" means express traffic class. MSB represent high number
>>>>>> traffic class.
>>>>>>
>>>>>> If hardware support the frame preemption, driver could set the
>>>>>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>>>>> when initializing the port driver.
>>>>>>
>>>>>> User can check the feature 'tx-preemption' by command 'ethtool -k
>>>>>> devname'. If hareware set preemption feature. The property would be a
>>>>>> fixed value 'on' if hardware support the frame preemption.
>>>>>> Feature would show a fixed value 'off' if hardware don't support the
>>>>>> frame preemption.
>>>
>>> Having some knobs in ethtool to enable when/how Frame Preemption is
>>> advertised on the wire makes sense. I also agree that it should be "on"
>>> by default.
>>>
>>>>>>
>>>>>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>>>>> would show/set which traffic classes are frame preemptable.
>>>>>>
>>>>>> Port driver would implement the frame preemption in the function
>>>>>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>>>
>>>>> In an early RFC series [1], we proposed a way to support frame preemption. I'm
>>>>> not sure if you have considered it before implementing this other proposal
>>>>> based on ethtool interface so I thought it would be a good idea to bring that up
>>>>> to your attention, just in case.
>>>>
>>>> Sorry, I didn't notice the RFC proposal. Using ethtool set the
>>>> preemption just thinking about 8021Qbu as standalone. And not limit to
>>>> the taprio if user won't set 802.1Qbv.
>>>
>>> I see your point of using frame-preemption "standalone", I have two
>>> ideas:
>>>
>>> 1. add support in taprio to be configured without any schedule in the
>>> "full offload" mode. In practice, allowing taprio to work somewhat
>>> similar to (mqprio + frame-preemption), changes in the code should de
>>> fairly small;
>>
>> +
>>
>> And if follow mqprio settings logic then preemption also can be enabled
>> immediately while configuring taprio first time, and similarly new ADMIN
>> can't change it and can be set w/o preemption option afterwards.
>>
>> So that following is correct:
>>
>> OPER
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 10000000 \
>> num_tc 3 \
>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>> queues 1@0 1@1 2@2 \
>> preemption 0 1 1 1
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 12000000 \
>> num_tc 3 \
>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>> queues 1@0 1@1 2@2 \
>> preemption 0 1 1 1
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> then
>> ADMIN
>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>> base-time 13000000 \
>> sched-entry S 01 300000 \
>> sched-entry S 02 300000 \
>> flags 1
>>
>> BUT:
>>
>> 1) The question is only should it be in this way? I mean preemption to be
>> enabled immediately? Also should include other parameters like
>> fragment size.
>
> We can decide what things are allowed/useful here. For example, it might
> make sense to allow "preemption" to be changed. We can extend taprio to
> support changing the fragment size, if that makes sense.
>
>>
>> 2) What if I want to use frame preemption with another "transmission selection
>> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
>> stacked?
>
> I am not seeing any (conceptual*) problems when plugging a cbs (for
> example) qdisc into one of taprio children. Or, are you talking about a
> more general problem?
>
> * here I am considering that support for taprio without an schedule is
> added.
>
>>
>> In this case ethtool looks better, allowing this "MAC level" feature, to be
>> configured separately.
>
> My only issue with using ethtool is that then we would have two
> different interfaces for "complementary" features. And it would make
> things even harder to configure and debug. The fact that one talks about
> traffic classes and the other transmission queues doesn't make me more
> comfortable as well.
>
> On the other hand, as there isn't a way to implement frame preemption in
> software, I agree that it makes it kind of awkward to have it in the tc
> subsystem.
Absolutely. I think frame pre-emption feature flag, per queue express/
pre-empt state, frag size, timers (hold/release) to be configured
independently (perhaps through ethtool) and then taprio should check
this with the lower device and then allow supporting additional Gate
operations such as Hold/release if supported by underlying device.
What do you think? Why to abuse tc for this?
Thanks and regards,
Murali
>
> At this point, I am slightly in favor of the taprio approach (yes, I am
> biased :-), but I can be convinced otherwise. I will be only a little
> sad if we choose to go with ethtool for now, and then add support up in
> the stack, something similar to "ethtool -N" and "tc-flower".
>
>>
>>>
>>> 2. extend mqprio to support frame-preemption;
>>>
>>>>
>>>> As some feedback also want to set the MAC merge minimal fragment size
>>>> and get some more information of 802.3br.
>>>
>>> The minimal fragment size, I guess, also makes sense to be kept in
>>> ethtool. That is we have a sane default, and allow the user to change
>>> this setting for special cases.
>>>
>>>>
>>>>>
>>>>> In that initial proposal, Frame Preemption feature is configured via taprio qdisc.
>>>>> For example:
>>>>>
>>>>> $ tc qdisc add dev IFACE parent root handle 100 taprio \
>>>>> num_tc 3 \
>>>>> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>>>>> queues 1@0 1@1 2@2 \
>>>>> preemption 0 1 1 1 \
>>>>> base-time 10000000 \
>>>>> sched-entry S 01 300000 \
>>>>> sched-entry S 02 300000 \
>>>>> sched-entry S 04 400000 \
>>>>> clockid CLOCK_TAI
>>>>>
>>>>> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
>>>>> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
>>>>> 802.1Q-2018 for further details.
>>>>
>>>> I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
>>>> could be understand as guardband by hardware preemption. MAC should
>>>> auto calculate the nano seconds before express entry slot start to
>>>> break to two fragments. Set-And-Hold-MAC should minimal larger than
>>>> the fragment-size oct times.
>>>
>>> Another interesting point. My first idea is that when the schedule is
>>> offloaded to the driver and the driver detects that the "entry" width is
>>> smaller than the fragment side, the driver could reject that schedule
>>> with a nice error message.
>>
>> Looks ok, if entry command is RELEASE or SET only, but not HOLD, and
>> only if it contains express queues. And if for entry is expectable to have
>> interval shorter, the entry has to be marked as HOLD then.
>>
>> But not every offload is able to support mac/hold per sched (there is
>> no HOLD/RELEASE commands in this case). For this case seems like here can
>> be 2 cases:
>
> Yeah, the hw I have in hand also doesn't support the HOLD/RELEASE
> commands.
>
>>
>> 1) there is no “gate close” event for the preemptible traffic
>> 2) there is "gate close" event for the preemptable traffic
>>
>> And both can have the following impact, if assume the main reason to
>> this guard check is to guarantee the express queue cannot be blocked while
>> this "close to short" interval opening ofc:
>>
>> If a preemption fragment is started before "express" frame, then interval
>> should allow to complete preemption fragment and has to have enough time
>> to insert express frame. So here situation when maximum packet size per
>> each queue can have place.
>>
>> In case of TI am65 this queue MTU is configurable per queue (for similar
>> reasons and couple more (packet fill feature for instance)) and can be
>> used for guard check also, but not clear where it should be. Seems like
>> it should be done using ethtool also, but can be needed for taprio
>> interface....
>
> For now, at least for the hardware I am working on, something like this
> is configurable, but I wasn't planning on exposing it, using the maximum
> ethernet frame size seemed a good default.
>
>>
>>>>
>>>>>
>>>>> Please share your thoughts on this.
>>>>
>>>> I am good to see there is frame preemption proposal. Each way is ok
>>>> for me but ethtool is more flexible. I've seen the RFC the code. The
>>>> hardware offload is in the mainline, but preemption is not yet, I
>>>> don't know why. Could you post it again?
>>>
>>> It's not mainline because this kind of stuff will not be accepted
>>> upstream without in-tree users. And you are the first one to propose
>>> such a thing :-)
>>>
>>> It's just now that I have something that supports frame-preemption, the
>>> code I have is approaching RFC-like quality. I will send another RFC
>>> this week hopefully, and we can see how things look in practice.
>>>
>>>
>>> Cheers,
>>> --
>>> Vinicius
>>
>> --
>> Regards,
>> Ivan Khoronzhuk
>
> Cheers,
> --
> Vinicius
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-30 17:03 ` Murali Karicheri
@ 2020-01-09 1:07 ` Andre Guedes
2020-01-09 8:59 ` Jose Abreu
2020-01-10 16:02 ` Vladimir Oltean
0 siblings, 2 replies; 39+ messages in thread
From: Andre Guedes @ 2020-01-09 1:07 UTC (permalink / raw)
To: Ivan Khoronzhuk, Murali Karicheri, Vinicius Costa Gomes
Cc: Po Liu, alexandru.ardelean, allison, andrew, ayal, davem,
f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li
Hi,
> >>> 1. add support in taprio to be configured without any schedule in the
> >>> "full offload" mode. In practice, allowing taprio to work somewhat
> >>> similar to (mqprio + frame-preemption), changes in the code should de
> >>> fairly small;
> >>
> >> +
> >>
> >> And if follow mqprio settings logic then preemption also can be enabled
> >> immediately while configuring taprio first time, and similarly new ADMIN
> >> can't change it and can be set w/o preemption option afterwards.
> >>
> >> So that following is correct:
> >>
> >> OPER
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 10000000 \
> >> num_tc 3 \
> >> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> >> queues 1@0 1@1 2@2 \
> >> preemption 0 1 1 1
> >> flags 1
> >>
> >> then
> >> ADMIN
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 12000000 \
> >> num_tc 3 \
> >> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> >> queues 1@0 1@1 2@2 \
> >> preemption 0 1 1 1
> >> sched-entry S 01 300000 \
> >> sched-entry S 02 300000 \
> >> flags 1
> >>
> >> then
> >> ADMIN
> >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> >> base-time 13000000 \
> >> sched-entry S 01 300000 \
> >> sched-entry S 02 300000 \
> >> flags 1
> >>
> >> BUT:
> >>
> >> 1) The question is only should it be in this way? I mean preemption to be
> >> enabled immediately? Also should include other parameters like
> >> fragment size.
> >
> > We can decide what things are allowed/useful here. For example, it might
> > make sense to allow "preemption" to be changed. We can extend taprio to
> > support changing the fragment size, if that makes sense.
> >
> >>
> >> 2) What if I want to use frame preemption with another "transmission selection
> >> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> >> stacked?
> >
> > I am not seeing any (conceptual*) problems when plugging a cbs (for
> > example) qdisc into one of taprio children. Or, are you talking about a
> > more general problem?
> >
> > * here I am considering that support for taprio without an schedule is
> > added.
> >
> >>
> >> In this case ethtool looks better, allowing this "MAC level" feature, to be
> >> configured separately.
> >
> > My only issue with using ethtool is that then we would have two
> > different interfaces for "complementary" features. And it would make
> > things even harder to configure and debug. The fact that one talks about
> > traffic classes and the other transmission queues doesn't make me more
> > comfortable as well.
> >
> > On the other hand, as there isn't a way to implement frame preemption in
> > software, I agree that it makes it kind of awkward to have it in the tc
> > subsystem.
> Absolutely. I think frame pre-emption feature flag, per queue express/
> pre-empt state, frag size, timers (hold/release) to be configured
> independently (perhaps through ethtool) and then taprio should check
> this with the lower device and then allow supporting additional Gate
> operations such as Hold/release if supported by underlying device.
>
> What do you think? Why to abuse tc for this?
>
After reading all this great discussion and revisiting the 802.1Q and 802.3br
specs, I'm now leaning towards to not coupling Frame Preemption support under
taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
foresees FP without EST so it makes me feel like we should keep them separate.
Regarding the FP configuration knobs, the following seems reasonable to me:
* Enable/disable FP feature
* Preemptable queue mapping
* Fragment size multiplier
I'm not sure about the knob 'timers (hold/release)' described in the quotes
above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
configurable. Do we know any hardware where they are configurable?
Regards,
Andre
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-09 1:07 ` Andre Guedes
@ 2020-01-09 8:59 ` Jose Abreu
2020-01-09 18:04 ` Andre Guedes
2020-01-10 16:02 ` Vladimir Oltean
1 sibling, 1 reply; 39+ messages in thread
From: Jose Abreu @ 2020-01-09 8:59 UTC (permalink / raw)
To: Andre Guedes, Ivan Khoronzhuk, Murali Karicheri, Vinicius Costa Gomes
Cc: Po Liu, alexandru.ardelean, allison, andrew, ayal, davem,
f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li,
Joao Pinto
From: Andre Guedes <andre.guedes@linux.intel.com>
Date: Jan/09/2020, 01:07:37 (UTC+00:00)
> After reading all this great discussion and revisiting the 802.1Q and 802.3br
> specs, I'm now leaning towards to not coupling Frame Preemption support under
> taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> foresees FP without EST so it makes me feel like we should keep them separate.
I agree that EST and FP can be used individually. But how can you
specify the hold and release commands for gates without changing taprio qdisc user space API ?
> Regarding the FP configuration knobs, the following seems reasonable to me:
> * Enable/disable FP feature
> * Preemptable queue mapping
> * Fragment size multiplier
>
> I'm not sure about the knob 'timers (hold/release)' described in the quotes
> above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> configurable. Do we know any hardware where they are configurable?
Synopsys' HW supports reconfiguring these parameters. They are, however,
fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-09 8:59 ` Jose Abreu
@ 2020-01-09 18:04 ` Andre Guedes
2020-01-10 14:35 ` Jose Abreu
0 siblings, 1 reply; 39+ messages in thread
From: Andre Guedes @ 2020-01-09 18:04 UTC (permalink / raw)
To: Ivan Khoronzhuk, Jose Abreu, Murali Karicheri, Vinicius Costa Gomes
Cc: Po Liu, alexandru.ardelean, allison, andrew, ayal, davem,
f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li,
Joao Pinto
Hi Jose,
Quoting Jose Abreu (2020-01-09 00:59:24)
> From: Andre Guedes <andre.guedes@linux.intel.com>
> Date: Jan/09/2020, 01:07:37 (UTC+00:00)
>
> > After reading all this great discussion and revisiting the 802.1Q and 802.3br
> > specs, I'm now leaning towards to not coupling Frame Preemption support under
> > taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> > foresees FP without EST so it makes me feel like we should keep them separate.
>
> I agree that EST and FP can be used individually. But how can you
> specify the hold and release commands for gates without changing taprio qdisc user space API ?
The 'hold' and 'release' are operations from the GCL, which is part of EST. So
they should still be specified via taprio. No changing in the user space API is
required since these operations are already supported in taprio API. What is
missing today is just the 'tc' side of it, which you already have a patch for
it.
> > Regarding the FP configuration knobs, the following seems reasonable to me:
> > * Enable/disable FP feature
> > * Preemptable queue mapping
> > * Fragment size multiplier
> >
> > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > configurable. Do we know any hardware where they are configurable?
>
> Synopsys' HW supports reconfiguring these parameters. They are, however,
> fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.
Good to know. Is the datasheet publicly available? If so, could you please
point me to it? I'd like to learn more about the FP knobs provided by
different HW.
Regards,
Andre
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-09 18:04 ` Andre Guedes
@ 2020-01-10 14:35 ` Jose Abreu
0 siblings, 0 replies; 39+ messages in thread
From: Jose Abreu @ 2020-01-10 14:35 UTC (permalink / raw)
To: Andre Guedes, Ivan Khoronzhuk, Murali Karicheri, Vinicius Costa Gomes
Cc: Po Liu, alexandru.ardelean, allison, andrew, ayal, davem,
f.fainelli, gregkh, hauke.mehrtens, hkallweit1, jiri,
linux-kernel, netdev, pablo, saeedm, tglx, Vladimir Oltean,
simon.horman, Claudiu Manoil, Alexandru Marginean,
Xiaoliang Yang, Roy Zang, Mingkai Hu, Jerry Huang, Leo Li,
Joao Pinto
From: Andre Guedes <andre.guedes@linux.intel.com>
Date: Jan/09/2020, 18:04:55 (UTC+00:00)
> Quoting Jose Abreu (2020-01-09 00:59:24)
> > From: Andre Guedes <andre.guedes@linux.intel.com>
> > Date: Jan/09/2020, 01:07:37 (UTC+00:00)
> >
> > > After reading all this great discussion and revisiting the 802.1Q and 802.3br
> > > specs, I'm now leaning towards to not coupling Frame Preemption support under
> > > taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> > > foresees FP without EST so it makes me feel like we should keep them separate.
> >
> > I agree that EST and FP can be used individually. But how can you
> > specify the hold and release commands for gates without changing taprio qdisc user space API ?
>
> The 'hold' and 'release' are operations from the GCL, which is part of EST. So
> they should still be specified via taprio. No changing in the user space API is
> required since these operations are already supported in taprio API. What is
> missing today is just the 'tc' side of it, which you already have a patch for
> it.
OK. Should we ask to merge it as-is then ?
> > > Regarding the FP configuration knobs, the following seems reasonable to me:
> > > * Enable/disable FP feature
> > > * Preemptable queue mapping
> > > * Fragment size multiplier
> > >
> > > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > > configurable. Do we know any hardware where they are configurable?
> >
> > Synopsys' HW supports reconfiguring these parameters. They are, however,
> > fixed independently of Queues. i.e. all queues will have same holdAdvance / releaseAdvance.
>
> Good to know. Is the datasheet publicly available? If so, could you please
> point me to it? I'd like to learn more about the FP knobs provided by
> different HW.
I'm afraid its not available unless you are a Synopsys customer. You
should however be able to figure out the behavior by reading my patch
that adds support for FPE in XGMAC and QoS cores. I can clarify any
doubts.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-09 1:07 ` Andre Guedes
2020-01-09 8:59 ` Jose Abreu
@ 2020-01-10 16:02 ` Vladimir Oltean
2020-01-10 20:59 ` Andre Guedes
1 sibling, 1 reply; 39+ messages in thread
From: Vladimir Oltean @ 2020-01-10 16:02 UTC (permalink / raw)
To: Andre Guedes
Cc: Ivan Khoronzhuk, Murali Karicheri, Vinicius Costa Gomes, Po Liu,
alexandru.ardelean, allison, andrew, ayal, davem, f.fainelli,
gregkh, hauke.mehrtens, hkallweit1, jiri, linux-kernel, netdev,
pablo, saeedm, tglx, Vladimir Oltean, simon.horman,
Claudiu Manoil, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Hi Andre,
On Thu, 9 Jan 2020 at 03:08, Andre Guedes <andre.guedes@linux.intel.com> wrote:
>
> Hi,
>
> > >>> 1. add support in taprio to be configured without any schedule in the
> > >>> "full offload" mode. In practice, allowing taprio to work somewhat
> > >>> similar to (mqprio + frame-preemption), changes in the code should de
> > >>> fairly small;
> > >>
> > >> +
> > >>
> > >> And if follow mqprio settings logic then preemption also can be enabled
> > >> immediately while configuring taprio first time, and similarly new ADMIN
> > >> can't change it and can be set w/o preemption option afterwards.
> > >>
> > >> So that following is correct:
> > >>
> > >> OPER
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 10000000 \
> > >> num_tc 3 \
> > >> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> > >> queues 1@0 1@1 2@2 \
> > >> preemption 0 1 1 1
> > >> flags 1
> > >>
> > >> then
> > >> ADMIN
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 12000000 \
> > >> num_tc 3 \
> > >> map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> > >> queues 1@0 1@1 2@2 \
> > >> preemption 0 1 1 1
> > >> sched-entry S 01 300000 \
> > >> sched-entry S 02 300000 \
> > >> flags 1
> > >>
> > >> then
> > >> ADMIN
> > >> $ tc qdisc add dev IFACE parent root handle 100 taprio \
> > >> base-time 13000000 \
> > >> sched-entry S 01 300000 \
> > >> sched-entry S 02 300000 \
> > >> flags 1
> > >>
> > >> BUT:
> > >>
> > >> 1) The question is only should it be in this way? I mean preemption to be
> > >> enabled immediately? Also should include other parameters like
> > >> fragment size.
> > >
> > > We can decide what things are allowed/useful here. For example, it might
> > > make sense to allow "preemption" to be changed. We can extend taprio to
> > > support changing the fragment size, if that makes sense.
> > >
> > >>
> > >> 2) What if I want to use frame preemption with another "transmission selection
> > >> algorithm"? Say another one "time sensitive" - CBS? How is it going to be
> > >> stacked?
> > >
> > > I am not seeing any (conceptual*) problems when plugging a cbs (for
> > > example) qdisc into one of taprio children. Or, are you talking about a
> > > more general problem?
> > >
> > > * here I am considering that support for taprio without an schedule is
> > > added.
> > >
> > >>
> > >> In this case ethtool looks better, allowing this "MAC level" feature, to be
> > >> configured separately.
> > >
> > > My only issue with using ethtool is that then we would have two
> > > different interfaces for "complementary" features. And it would make
> > > things even harder to configure and debug. The fact that one talks about
> > > traffic classes and the other transmission queues doesn't make me more
> > > comfortable as well.
> > >
> > > On the other hand, as there isn't a way to implement frame preemption in
> > > software, I agree that it makes it kind of awkward to have it in the tc
> > > subsystem.
> > Absolutely. I think frame pre-emption feature flag, per queue express/
> > pre-empt state, frag size, timers (hold/release) to be configured
> > independently (perhaps through ethtool) and then taprio should check
> > this with the lower device and then allow supporting additional Gate
> > operations such as Hold/release if supported by underlying device.
> >
> > What do you think? Why to abuse tc for this?
> >
>
> After reading all this great discussion and revisiting the 802.1Q and 802.3br
> specs, I'm now leaning towards to not coupling Frame Preemption support under
> taprio qdisc. Besides what have been discussed, Annex S.2 from 802.1Q-2018
> foresees FP without EST so it makes me feel like we should keep them separate.
>
> Regarding the FP configuration knobs, the following seems reasonable to me:
> * Enable/disable FP feature
> * Preemptable queue mapping
> * Fragment size multiplier
>
> I'm not sure about the knob 'timers (hold/release)' described in the quotes
> above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> configurable. Do we know any hardware where they are configurable?
>
On NXP LS1028A, HOLD_ADVANCE is configurable on both ENETC and the
Felix switch (its default value is 127 bytes). Same as Synopsys, it is
a global setting and not per queue or per GCL entry.
RELEASE_ADVANCE is not configurable.
Regardless, I am not sure if there is any value in configuring this
knob. Remember that the minimum guard band size still needs to be
twice as large as the minimum Ethernet frame size.
As for the main topic of tc-taprio vs ethtool for configuring frame
preemption, I think ethtool is the more natural place for configuring
the traffic class to pMAC/eMAC mapping, global enable bit, and
fragment size, while tc-taprio is the more natural place for
configuring hold/release on individual GCL entries. The tc-taprio
offload can check the netdev support of the ethtool feature, just as
it checks right now the support for PTP clock for the full offload
feature.
Introducing tc-taprio with a null schedule is not really natural, and
frame preemption is a hardware-only feature that cannot be emulated,
so it is odd to enable it through tc.
> Regards,
>
> Andre
Regards,
-Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-10 16:02 ` Vladimir Oltean
@ 2020-01-10 20:59 ` Andre Guedes
0 siblings, 0 replies; 39+ messages in thread
From: Andre Guedes @ 2020-01-10 20:59 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Ivan Khoronzhuk, Murali Karicheri, Vinicius Costa Gomes, Po Liu,
alexandru.ardelean, allison, andrew, ayal, davem, f.fainelli,
gregkh, hauke.mehrtens, hkallweit1, jiri, linux-kernel, netdev,
pablo, saeedm, tglx, Vladimir Oltean, simon.horman,
Claudiu Manoil, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Hi Vladimir,
Quoting Vladimir Oltean (2020-01-10 08:02:45)
> > I'm not sure about the knob 'timers (hold/release)' described in the quotes
> > above. I couldn't find a match in the specs. If it refers to 'holdAdvance' and
> > 'releaseAdvance' parameters described in 802.1Q-2018, I believe they are not
> > configurable. Do we know any hardware where they are configurable?
> >
>
> On NXP LS1028A, HOLD_ADVANCE is configurable on both ENETC and the
> Felix switch (its default value is 127 bytes). Same as Synopsys, it is
> a global setting and not per queue or per GCL entry.
> RELEASE_ADVANCE is not configurable.
> Regardless, I am not sure if there is any value in configuring this
> knob. Remember that the minimum guard band size still needs to be
> twice as large as the minimum Ethernet frame size.
Not adding this knob now sounds reasonable to me too, but let's consider it in
the design so we can easily add it later, in case we need it.
Regards,
Andre
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-12-16 21:44 ` Vinicius Costa Gomes
2019-12-19 0:43 ` Ivan Khoronzhuk
@ 2020-01-09 0:56 ` Andre Guedes
1 sibling, 0 replies; 39+ messages in thread
From: Andre Guedes @ 2020-01-09 0:56 UTC (permalink / raw)
To: alexandru.ardelean, allison, andrew, ayal, davem, f.fainelli,
gregkh, hauke.mehrtens, hkallweit1, jiri, linux-kernel, netdev,
pablo, saeedm, tglx, Ivan Khoronzhuk, Murali Karicheri, Po Liu,
Vinicius Costa Gomes, Vladimir Oltean
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi,
Quoting Vinicius Costa Gomes (2019-12-16 13:44:13)
> >> Quoting Po Liu (2019-11-27 01:59:18)
> >> > User can check the feature 'tx-preemption' by command 'ethtool -k
> >> > devname'. If hareware set preemption feature. The property would be a
> >> > fixed value 'on' if hardware support the frame preemption.
> >> > Feature would show a fixed value 'off' if hardware don't support the
> >> > frame preemption.
>
> Having some knobs in ethtool to enable when/how Frame Preemption is
> advertised on the wire makes sense. I also agree that it should be "on"
> by default.
Agreed. If Frame Preemption is supported by hardware and it can be
enabled/disabled, I think we should allow the user to configure it, instead of
having it fixed in 'on'.
> >> In an early RFC series [1], we proposed a way to support frame preemption. I'm
> >> not sure if you have considered it before implementing this other proposal
> >> based on ethtool interface so I thought it would be a good idea to bring that up
> >> to your attention, just in case.
> >
> > Sorry, I didn't notice the RFC proposal. Using ethtool set the
> > preemption just thinking about 8021Qbu as standalone. And not limit to
> > the taprio if user won't set 802.1Qbv.
>
> I see your point of using frame-preemption "standalone", I have two
> ideas:
>
> 1. add support in taprio to be configured without any schedule in the
> "full offload" mode. In practice, allowing taprio to work somewhat
> similar to (mqprio + frame-preemption), changes in the code should de
> fairly small;
>
> 2. extend mqprio to support frame-preemption;
I'm not sure 2) is a good way to support frame preemption "standalone"
functionality since mpqrio already looks overloaded. Besides its original goal
(map traffic flows to hardware queues), it also supports different modes and
traffic shaping. I rather not add another functionality to it.
> > As some feedback also want to set the MAC merge minimal fragment size
> > and get some more information of 802.3br.
>
> The minimal fragment size, I guess, also makes sense to be kept in
> ethtool. That is we have a sane default, and allow the user to change
> this setting for special cases.
Yes, the mim fragment size is another configuration knob we should have, and it
should probably live at the same place we land the enable/disable knob.
> >> It also aligns with the gate control operations Set-And-Hold-MAC and Set-And-
> >> Release-MAC that can be set via 'sched-entry' (see Table 8.7 from
> >> 802.1Q-2018 for further details.
> >
> > I am curious about Set-And-Hold-Mac via 'sched-entry'. Actually, it
> > could be understand as guardband by hardware preemption. MAC should
> > auto calculate the nano seconds before express entry slot start to
> > break to two fragments. Set-And-Hold-MAC should minimal larger than
> > the fragment-size oct times.
>
> Another interesting point. My first idea is that when the schedule is
> offloaded to the driver and the driver detects that the "entry" width is
> smaller than the fragment side, the driver could reject that schedule
> with a nice error message.
My understanding is that, if HOLD operation is supported, the hardware issues
an MM_CTL.request(HOLD) at 'holdAdvance' nsecs in advance to the point where
the 'sched-entry' with Set-And-Hold-MAC starts. This comes from the description
in Table 8-7 from 802.1Q-2018.
Best regards,
Andre
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
` (3 preceding siblings ...)
2019-12-11 2:52 ` Andre Guedes
@ 2020-01-18 0:03 ` Vinicius Costa Gomes
2020-01-22 18:10 ` Murali Karicheri
2020-02-21 21:43 ` Vinicius Costa Gomes
5 siblings, 1 reply; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-01-18 0:03 UTC (permalink / raw)
To: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Po Liu
Hi,
Po Liu <po.liu@nxp.com> writes:
> IEEE Std 802.1Qbu standard defined the frame preemption of port
> traffic classes. This patch introduce a method to set traffic
> classes preemption. Add a parameter 'preemption' in struct
> ethtool_link_settings. The value will be translated to a binary,
> each bit represent a traffic class. Bit "1" means preemptable
> traffic class. Bit "0" means express traffic class. MSB represent
> high number traffic class.
I think that we should keep the concept of 'traffic classes' outside of
ethtool. Ethtool should only care about queues (or similar concepts).
And unless I am missing something here, what you mean by 'traffic class'
here is really a hardware queue.
>
> If hardware support the frame preemption, driver could set the
> ethernet device with hw_features and features with NETIF_F_PREEMPTION
> when initializing the port driver.
>
> User can check the feature 'tx-preemption' by command 'ethtool -k
> devname'. If hareware set preemption feature. The property would
> be a fixed value 'on' if hardware support the frame preemption.
> Feature would show a fixed value 'off' if hardware don't support
> the frame preemption.
>
> Command 'ethtool devname' and 'ethtool -s devname preemption N'
> would show/set which traffic classes are frame preemptable.
>
> Port driver would implement the frame preemption in the function
> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>
> Signed-off-by: Po Liu <Po.Liu@nxp.com>
> ---
> include/linux/netdev_features.h | 5 ++++-
> include/uapi/linux/ethtool.h | 5 ++++-
> net/core/ethtool.c | 1 +
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 4b19c544c59a..299750a8b414 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -80,6 +80,7 @@ enum {
>
> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
> + NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>
> /*
> * Add your fresh new feature above and remember to update
> @@ -150,6 +151,7 @@ enum {
> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
> +#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>
> /* Finds the next feature with the highest number of the range of start till 0.
> */
> @@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
> - NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
> + NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
> + NETIF_F_PREEMPTION)
>
> /* remember that ((t)1 << t_BITS) is undefined in C99 */
> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index d4591792f0b4..12ffb34afbfa 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
> };
> #define ETH_RESET_SHARED_SHIFT 16
>
> +/* Disable preemtion. */
> +#define PREEMPTION_DISABLE 0x0
>
> /**
> * struct ethtool_link_settings - link control and status
> @@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
> __s8 link_mode_masks_nwords;
> __u8 transceiver;
> __u8 reserved1[3];
> - __u32 reserved[7];
> + __u32 preemption;
> + __u32 reserved[6];
> __u32 link_mode_masks[0];
> /* layout of link_mode_masks fields:
> * __u32 map_supported[link_mode_masks_nwords];
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index cd9bc67381b2..6ffcd8a602b8 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> + [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
> };
>
> static const char
> --
> 2.17.1
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-18 0:03 ` Vinicius Costa Gomes
@ 2020-01-22 18:10 ` Murali Karicheri
2020-01-23 13:30 ` Vladimir Oltean
0 siblings, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2020-01-22 18:10 UTC (permalink / raw)
To: Vinicius Costa Gomes, Po Liu, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi, Vinicius,
On 01/17/2020 07:03 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Po Liu <po.liu@nxp.com> writes:
>
>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>> traffic classes. This patch introduce a method to set traffic
>> classes preemption. Add a parameter 'preemption' in struct
>> ethtool_link_settings. The value will be translated to a binary,
>> each bit represent a traffic class. Bit "1" means preemptable
>> traffic class. Bit "0" means express traffic class. MSB represent
>> high number traffic class.
>
> I think that we should keep the concept of 'traffic classes' outside of
> ethtool. Ethtool should only care about queues (or similar concepts).
> And unless I am missing something here, what you mean by 'traffic class'
> here is really a hardware queue.
>
Agree.
So this is my understanding..
Once driver support FPE, then drive accepts tc gate operation,
Set-And-Hold MAC & Set-And-Release-MAC. Otherwise it can only offload
SetGateStates.
So candidates for ethtool command.
Set
- FPE enable/disable
- framePreemptionStatusTable of Table 12-30—Frame Preemption Parameter
table of 802.1Q 2018 which are defined as Read/Write. This is an array
of Hw queues and if they are preemptible or express.
- Additionally 802.3br defines Min fragment size that is a candidate
for ethtool command.
Show the following:-
- holdAdvance
- releaseAdvance
- preemptionActive
- holdRequest
There is also Table 12-29—The Gate Parameter Table for taprio. These
related to taprio schedule is already part of tc command. However there
are below parameters that requires configuration by user as well.
Can we use ethtool for the same as this
- queueMaxSDUTable
Below is what I read about this which is also for hw queues AFAIK.
==== from 802.1Q====================================================
A per-traffic class queue queueMaxSDU parameter defines the maximum
service data unit size for each queue; frames that exceed queueMaxSDU
are discarded. The value of queueMaxSDU for each queue is configurable
by management its default value is the maximum SDU size supported by the
MAC procedures employed on the LAN to which the frame is to be relayed
=======================================================================
I have question about the below parameters in The Gate Parameter Table
that are not currently supported by tc command. Looks like they need to
be shown to user for management.
- ConfigChange - Looks like this needs to be controlled by
user. After sending admin command, user send this trigger to start
copying admin schedule to operation schedule. Is this getting
added to tc command?
- ConfigChangeTime - The time at which the administrative variables
that determine the cycle are to be copied across to the
corresponding operational variables, expressed as a PTP timescale
- TickGranularity - the management parameters specified in Gate
Parameter Table allow a management station to discover the
characteristics of an implementation’s cycle timer clock
(TickGranularity) and to set the parameters for the gating cycle
accordingly.
- ConfigPending - A Boolean variable, set TRUE to indicate that
there is a new cycle configuration awaiting installation.
- ConfigChangeError - Error in configuration (AdminBaseTime <
CurrentTime)
- SupportedListMax - Maximum supported Admin/Open shed list.
Is there a plan to export these from driver through tc show or such
command? The reason being, there would be applications developed to
manage configuration/schedule of TSN nodes that would requires these
information from the node. So would need a support either in tc or
some other means to retrieve them from hardware or driver. That is my
understanding...
Regards,
Murali
>>
>> If hardware support the frame preemption, driver could set the
>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>> when initializing the port driver.
>>
>> User can check the feature 'tx-preemption' by command 'ethtool -k
>> devname'. If hareware set preemption feature. The property would
>> be a fixed value 'on' if hardware support the frame preemption.
>> Feature would show a fixed value 'off' if hardware don't support
>> the frame preemption.
>>
>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>> would show/set which traffic classes are frame preemptable.
>>
>> Port driver would implement the frame preemption in the function
>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>
>> Signed-off-by: Po Liu <Po.Liu@nxp.com>
>> ---
>> include/linux/netdev_features.h | 5 ++++-
>> include/uapi/linux/ethtool.h | 5 ++++-
>> net/core/ethtool.c | 1 +
>> 3 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index 4b19c544c59a..299750a8b414 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -80,6 +80,7 @@ enum {
>>
>> NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
>> NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */
>> + NETIF_F_HW_PREEMPTION_BIT, /* Hardware TC frame preemption */
>>
>> /*
>> * Add your fresh new feature above and remember to update
>> @@ -150,6 +151,7 @@ enum {
>> #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4)
>> #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
>> #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX)
>> +#define NETIF_F_PREEMPTION __NETIF_F(HW_PREEMPTION)
>>
>> /* Finds the next feature with the highest number of the range of start till 0.
>> */
>> @@ -175,7 +177,8 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
>> /* Features valid for ethtool to change */
>> /* = all defined minus driver/device-class-related */
>> #define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
>> - NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
>> + NETIF_F_LLTX | NETIF_F_NETNS_LOCAL | \
>> + NETIF_F_PREEMPTION)
>>
>> /* remember that ((t)1 << t_BITS) is undefined in C99 */
>> #define NETIF_F_ETHTOOL_BITS ((__NETIF_F_BIT(NETDEV_FEATURE_COUNT - 1) | \
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index d4591792f0b4..12ffb34afbfa 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -1776,6 +1776,8 @@ enum ethtool_reset_flags {
>> };
>> #define ETH_RESET_SHARED_SHIFT 16
>>
>> +/* Disable preemtion. */
>> +#define PREEMPTION_DISABLE 0x0
>>
>> /**
>> * struct ethtool_link_settings - link control and status
>> @@ -1886,7 +1888,8 @@ struct ethtool_link_settings {
>> __s8 link_mode_masks_nwords;
>> __u8 transceiver;
>> __u8 reserved1[3];
>> - __u32 reserved[7];
>> + __u32 preemption;
>> + __u32 reserved[6];
>> __u32 link_mode_masks[0];
>> /* layout of link_mode_masks fields:
>> * __u32 map_supported[link_mode_masks_nwords];
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index cd9bc67381b2..6ffcd8a602b8 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>> [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
>> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
>> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
>> + [NETIF_F_HW_PREEMPTION_BIT] = "tx-preemption",
>> };
>>
>> static const char
>> --
>> 2.17.1
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-22 18:10 ` Murali Karicheri
@ 2020-01-23 13:30 ` Vladimir Oltean
2020-01-23 17:50 ` Vinicius Costa Gomes
2020-02-10 20:17 ` Murali Karicheri
0 siblings, 2 replies; 39+ messages in thread
From: Vladimir Oltean @ 2020-01-23 13:30 UTC (permalink / raw)
To: Murali Karicheri
Cc: Vinicius Costa Gomes, Po Liu, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev,
simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi Murali,
On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <m-karicheri2@ti.com> wrote:
>
> I have question about the below parameters in The Gate Parameter Table
> that are not currently supported by tc command. Looks like they need to
> be shown to user for management.
>
> - ConfigChange - Looks like this needs to be controlled by
> user. After sending admin command, user send this trigger to start
> copying admin schedule to operation schedule. Is this getting
> added to tc command?
"The ConfigChange parameter signals the start of a
configuration change for the gate
when it is set to TRUE. This should only be done
when the various administrative parameters
are all set to appropriate values."
As far as my understanding goes, all tc-taprio commands currently
behave as though this boolean is implicitly set to TRUE after the
structures have been set up. I'm not sure there is any value in doing
otherwise.
> - ConfigChangeTime - The time at which the administrative variables
> that determine the cycle are to be copied across to the
> corresponding operational variables, expressed as a PTP timescale
This is the base-time of the admin schedule, no?
"The PTPtime at which the next config change is scheduled to occur.
The value is a representation of a PTPtime value,
consisting of a 48-bit integer
number of seconds and a 32-bit integer number of nanoseconds."
> - TickGranularity - the management parameters specified in Gate
> Parameter Table allow a management station to discover the
> characteristics of an implementation’s cycle timer clock
> (TickGranularity) and to set the parameters for the gating cycle
> accordingly.
Not sure who is going to use this and for what purpose, but ok.
> - ConfigPending - A Boolean variable, set TRUE to indicate that
> there is a new cycle configuration awaiting installation.
I had tried to export something like this (driver calls back into
sch_taprio.c when hw has applied the config, this would result in
ConfigPending = FALSE), but ultimately didn't finish the idea, and it
caused some problems too, due to incorrect RCU usage.
> - ConfigChangeError - Error in configuration (AdminBaseTime <
> CurrentTime)
This can be exported similarly.
> - SupportedListMax - Maximum supported Admin/Open shed list.
>
> Is there a plan to export these from driver through tc show or such
> command? The reason being, there would be applications developed to
> manage configuration/schedule of TSN nodes that would requires these
> information from the node. So would need a support either in tc or
> some other means to retrieve them from hardware or driver. That is my
> understanding...
>
Not sure what answer you expect to receive for "is there any plan".
You can go ahead and propose something, as long as it is reasonably
useful to have.
> Regards,
>
> Murali
>
> --
> Murali Karicheri
> Texas Instruments
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-23 13:30 ` Vladimir Oltean
@ 2020-01-23 17:50 ` Vinicius Costa Gomes
2020-02-10 20:30 ` Murali Karicheri
2020-02-10 20:17 ` Murali Karicheri
1 sibling, 1 reply; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-01-23 17:50 UTC (permalink / raw)
To: Vladimir Oltean, Murali Karicheri
Cc: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev, simon.horman, Claudiu Manoil,
Vladimir Oltean, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Hi,
Vladimir Oltean <olteanv@gmail.com> writes:
> Hi Murali,
>
> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <m-karicheri2@ti.com> wrote:
>>
>> I have question about the below parameters in The Gate Parameter Table
>> that are not currently supported by tc command. Looks like they need to
>> be shown to user for management.
>>
>> - ConfigChange - Looks like this needs to be controlled by
>> user. After sending admin command, user send this trigger to start
>> copying admin schedule to operation schedule. Is this getting
>> added to tc command?
>
> "The ConfigChange parameter signals the start of a
> configuration change for the gate
> when it is set to TRUE. This should only be done
> when the various administrative parameters
> are all set to appropriate values."
>
> As far as my understanding goes, all tc-taprio commands currently
> behave as though this boolean is implicitly set to TRUE after the
> structures have been set up. I'm not sure there is any value in doing
> otherwise.
>
>> - ConfigChangeTime - The time at which the administrative variables
>> that determine the cycle are to be copied across to the
>> corresponding operational variables, expressed as a PTP timescale
>
> This is the base-time of the admin schedule, no?
>
> "The PTPtime at which the next config change is scheduled to occur.
> The value is a representation of a PTPtime value,
> consisting of a 48-bit integer
> number of seconds and a 32-bit integer number of nanoseconds."
>
>> - TickGranularity - the management parameters specified in Gate
>> Parameter Table allow a management station to discover the
>> characteristics of an implementation’s cycle timer clock
>> (TickGranularity) and to set the parameters for the gating cycle
>> accordingly.
>
> Not sure who is going to use this and for what purpose, but ok.
>
>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>> there is a new cycle configuration awaiting installation.
>
> I had tried to export something like this (driver calls back into
> sch_taprio.c when hw has applied the config, this would result in
> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
> caused some problems too, due to incorrect RCU usage.
>
If this should be exported, this should be done from taprio, perhaps
adding a new field to what is exported via the dump() callback, which
should be quite easy.
>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>> CurrentTime)
>
> This can be exported similarly.
In my view, having this as a "runtime" error is not useful, as we can
verify this at configuration time.
>
>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>
>> Is there a plan to export these from driver through tc show or such
>> command? The reason being, there would be applications developed to
>> manage configuration/schedule of TSN nodes that would requires these
>> information from the node. So would need a support either in tc or
>> some other means to retrieve them from hardware or driver. That is my
>> understanding...
>>
Hm, now I understamd what you meant here...
>
> Not sure what answer you expect to receive for "is there any plan".
> You can go ahead and propose something, as long as it is reasonably
> useful to have.
... if this is indeed useful, perhaps one way to do is to add a subcommand
to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
from the driver. Similar to what cls_flower does.
>
>> Regards,
>>
>> Murali
>>
>> --
>> Murali Karicheri
>> Texas Instruments
>
> Thanks,
> -Vladimir
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-23 17:50 ` Vinicius Costa Gomes
@ 2020-02-10 20:30 ` Murali Karicheri
2020-02-11 19:22 ` Vinicius Costa Gomes
0 siblings, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2020-02-10 20:30 UTC (permalink / raw)
To: Vinicius Costa Gomes, Vladimir Oltean
Cc: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev, simon.horman, Claudiu Manoil,
Vladimir Oltean, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Hi,
On 01/23/2020 12:50 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Vladimir Oltean <olteanv@gmail.com> writes:
>
>> Hi Murali,
>>
>> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <m-karicheri2@ti.com> wrote:
>>>
>>> I have question about the below parameters in The Gate Parameter Table
>>> that are not currently supported by tc command. Looks like they need to
>>> be shown to user for management.
>>>
>>> - ConfigChange - Looks like this needs to be controlled by
>>> user. After sending admin command, user send this trigger to start
>>> copying admin schedule to operation schedule. Is this getting
>>> added to tc command?
>>
>> "The ConfigChange parameter signals the start of a
>> configuration change for the gate
>> when it is set to TRUE. This should only be done
>> when the various administrative parameters
>> are all set to appropriate values."
>>
>> As far as my understanding goes, all tc-taprio commands currently
>> behave as though this boolean is implicitly set to TRUE after the
>> structures have been set up. I'm not sure there is any value in doing
>> otherwise.
>>
>>> - ConfigChangeTime - The time at which the administrative variables
>>> that determine the cycle are to be copied across to the
>>> corresponding operational variables, expressed as a PTP timescale
>>
>> This is the base-time of the admin schedule, no?
>>
>> "The PTPtime at which the next config change is scheduled to occur.
>> The value is a representation of a PTPtime value,
>> consisting of a 48-bit integer
>> number of seconds and a 32-bit integer number of nanoseconds."
>>
>>> - TickGranularity - the management parameters specified in Gate
>>> Parameter Table allow a management station to discover the
>>> characteristics of an implementation’s cycle timer clock
>>> (TickGranularity) and to set the parameters for the gating cycle
>>> accordingly.
>>
>> Not sure who is going to use this and for what purpose, but ok.
>>
>>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>>> there is a new cycle configuration awaiting installation.
>>
>> I had tried to export something like this (driver calls back into
>> sch_taprio.c when hw has applied the config, this would result in
>> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
>> caused some problems too, due to incorrect RCU usage.
>>
>
> If this should be exported, this should be done from taprio, perhaps
> adding a new field to what is exported via the dump() callback, which
> should be quite easy.
>
We are still working to send a patch for taprio offload on our hardware
and it may take a while to get to this. So if someone can help to add
the required kernel/driver interface for this, that will be great!
>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>> CurrentTime)
>>
>> This can be exported similarly.
>
> In my view, having this as a "runtime" error is not useful, as we can
> verify this at configuration time.
Looks like this is not an error per 802.1Q standard if I understood it
correctly.
This is what I see.
=======================================================================
From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()
If AdminBaseTime is set to the same time in the past in all bridges and
end stations, OperBaseTime is always in the past, and all cycles start
synchronized. Using AdminBaseTime in the past is appropriate when you
can start schedules prior to starting the application that uses the
schedules. Use of AdminBaseTime in the future is intended to change a
currently running schedule in all bridges and end stations to a new
schedule at a future time. Using AdminBaseTime in the future is
appropriate when schedules must be changed without stopping the
application
========================================================================
>
>>
>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>
>>> Is there a plan to export these from driver through tc show or such
>>> command? The reason being, there would be applications developed to
>>> manage configuration/schedule of TSN nodes that would requires these
>>> information from the node. So would need a support either in tc or
>>> some other means to retrieve them from hardware or driver. That is my
>>> understanding...
>>>
>
> Hm, now I understamd what you meant here...
>
>>
>> Not sure what answer you expect to receive for "is there any plan".
>> You can go ahead and propose something, as long as it is reasonably
>> useful to have.
>
> ... if this is indeed useful, perhaps one way to do is to add a subcommand
> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
> from the driver. Similar to what cls_flower does.
>
What I understand is that there will be some work done to allow auto
configuration of TSN nodes from user space and that would need access to
all or some of the above parameters along with tc command to configure
the same. May be a open source project for this or some custom
application? Any such projects existing??
Regards,
Murali
>>
>>> Regards,
>>>
>>> Murali
>>>
>>> --
>>> Murali Karicheri
>>> Texas Instruments
>>
>> Thanks,
>> -Vladimir
>
> Cheers,
> --
> Vinicius
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-02-10 20:30 ` Murali Karicheri
@ 2020-02-11 19:22 ` Vinicius Costa Gomes
2020-02-25 17:55 ` Murali Karicheri
0 siblings, 1 reply; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-02-11 19:22 UTC (permalink / raw)
To: Murali Karicheri, Vladimir Oltean
Cc: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev, simon.horman, Claudiu Manoil,
Vladimir Oltean, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Murali Karicheri <m-karicheri2@ti.com> writes:
> We are still working to send a patch for taprio offload on our hardware
> and it may take a while to get to this. So if someone can help to add
> the required kernel/driver interface for this, that will be great!
Will add this to my todo list. But if anyone else has the spare cycles
feel free to have a go at it.
>
>>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>>> CurrentTime)
>>>
>>> This can be exported similarly.
>>
>> In my view, having this as a "runtime" error is not useful, as we can
>> verify this at configuration time.
>
> Looks like this is not an error per 802.1Q standard if I understood it
> correctly.
>
> This is what I see.
> =======================================================================
> From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()
>
> If AdminBaseTime is set to the same time in the past in all bridges and
> end stations, OperBaseTime is always in the past, and all cycles start
> synchronized. Using AdminBaseTime in the past is appropriate when you
> can start schedules prior to starting the application that uses the
> schedules. Use of AdminBaseTime in the future is intended to change a
> currently running schedule in all bridges and end stations to a new
> schedule at a future time. Using AdminBaseTime in the future is
> appropriate when schedules must be changed without stopping the
> application
> ========================================================================
>
What I meant here is the case that I already have an "oper" schedule
running, so my "oper->base_time" is in the past, and I try to add an
"admin" schedule with a "base_time" also in the past. What's the
expected behavior in this case? The text about stopping/starting
applications doesn't seem to apply to the way the tc subsystem interacts
with the applications.
>>
>>>
>>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>>
>>>> Is there a plan to export these from driver through tc show or such
>>>> command? The reason being, there would be applications developed to
>>>> manage configuration/schedule of TSN nodes that would requires these
>>>> information from the node. So would need a support either in tc or
>>>> some other means to retrieve them from hardware or driver. That is my
>>>> understanding...
>>>>
>>
>> Hm, now I understamd what you meant here...
>>
>>>
>>> Not sure what answer you expect to receive for "is there any plan".
>>> You can go ahead and propose something, as long as it is reasonably
>>> useful to have.
>>
>> ... if this is indeed useful, perhaps one way to do is to add a subcommand
>> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
>> from the driver. Similar to what cls_flower does.
>>
>
> What I understand is that there will be some work done to allow auto
> configuration of TSN nodes from user space and that would need access to
> all or some of the above parameters along with tc command to configure
> the same. May be a open source project for this or some custom
> application? Any such projects existing??
Yeah, this is a big missing piece for TSN. I've heard 'netopeer2' and
'sysrepo' mentioned when similar questions were asked, but I have still
to take a look at them and see what's missing. (Or if they are the right
tool for the job)
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-02-11 19:22 ` Vinicius Costa Gomes
@ 2020-02-25 17:55 ` Murali Karicheri
0 siblings, 0 replies; 39+ messages in thread
From: Murali Karicheri @ 2020-02-25 17:55 UTC (permalink / raw)
To: Vinicius Costa Gomes, Vladimir Oltean
Cc: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev, simon.horman, Claudiu Manoil,
Vladimir Oltean, Alexandru Marginean, Xiaoliang Yang, Roy Zang,
Mingkai Hu, Jerry Huang, Leo Li
Hi Vinicius,
On 02/11/2020 02:22 PM, Vinicius Costa Gomes wrote:
> Murali Karicheri <m-karicheri2@ti.com> writes:
>
>> We are still working to send a patch for taprio offload on our hardware
>> and it may take a while to get to this. So if someone can help to add
>> the required kernel/driver interface for this, that will be great!
>
> Will add this to my todo list. But if anyone else has the spare cycles
> feel free to have a go at it.
>
Thanks! We have made some progress in sending the base driver to netdev
list now https://lkml.org/lkml/2020/2/22/157
This device is taprio offload capable. Next step is to add taprio
offload to this driver. Then other features will follow.
>>
>>>>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>>>>> CurrentTime)
>>>>
>>>> This can be exported similarly.
>>>
>>> In my view, having this as a "runtime" error is not useful, as we can
>>> verify this at configuration time.
>>
>> Looks like this is not an error per 802.1Q standard if I understood it
>> correctly.
>>
>> This is what I see.
>> =======================================================================
>> From 802.1Q 2018, 8.6.9.1.1 SetCycleStartTime()
>>
>> If AdminBaseTime is set to the same time in the past in all bridges and
>> end stations, OperBaseTime is always in the past, and all cycles start
>> synchronized. Using AdminBaseTime in the past is appropriate when you
>> can start schedules prior to starting the application that uses the
>> schedules. Use of AdminBaseTime in the future is intended to change a
>> currently running schedule in all bridges and end stations to a new
>> schedule at a future time. Using AdminBaseTime in the future is
>> appropriate when schedules must be changed without stopping the
>> application
>> ========================================================================
>>
>
> What I meant here is the case that I already have an "oper" schedule
> running, so my "oper->base_time" is in the past, and I try to add an
> "admin" schedule with a "base_time" also in the past. What's the
> expected behavior in this case? The text about stopping/starting
> applications doesn't seem to apply to the way the tc subsystem interacts
> with the applications.
>
> I try to add an "admin" schedule with a "base_time" also in the past.
> What's the expected behavior in this case?
Ok got it. I don't think this behavior is explained in the spec. I would
assume a sane thing to do is to switch to admin schedule if
admin->base_time is newer than oper->base_time and flag
the ConfigChangeError to be compliant to the spec, but frankly speaking
I don't know how application is going to use this. It is a low priority
item IMO and can be added as needed.
Regards,
Murali
>>>
>>>>
>>>>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>>>>
>>>>> Is there a plan to export these from driver through tc show or such
>>>>> command? The reason being, there would be applications developed to
>>>>> manage configuration/schedule of TSN nodes that would requires these
>>>>> information from the node. So would need a support either in tc or
>>>>> some other means to retrieve them from hardware or driver. That is my
>>>>> understanding...
>>>>>
>>>
>>> Hm, now I understamd what you meant here...
>>>
>>>>
>>>> Not sure what answer you expect to receive for "is there any plan".
>>>> You can go ahead and propose something, as long as it is reasonably
>>>> useful to have.
>>>
>>> ... if this is indeed useful, perhaps one way to do is to add a subcommand
>>> to TC_SETUP_QDISC_TAPRIO, so we can retrieve the stats/information we want
>>> from the driver. Similar to what cls_flower does.
>>>
>>
>> What I understand is that there will be some work done to allow auto
>> configuration of TSN nodes from user space and that would need access to
>> all or some of the above parameters along with tc command to configure
>> the same. May be a open source project for this or some custom
>> application? Any such projects existing??
>
> Yeah, this is a big missing piece for TSN. I've heard 'netopeer2' and
> 'sysrepo' mentioned when similar questions were asked, but I have still
> to take a look at them and see what's missing. (Or if they are the right
> tool for the job)
>
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-01-23 13:30 ` Vladimir Oltean
2020-01-23 17:50 ` Vinicius Costa Gomes
@ 2020-02-10 20:17 ` Murali Karicheri
1 sibling, 0 replies; 39+ messages in thread
From: Murali Karicheri @ 2020-02-10 20:17 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Vinicius Costa Gomes, Po Liu, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev,
simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li
Hi Vladimir,
On 01/23/2020 08:30 AM, Vladimir Oltean wrote:
> Hi Murali,
>
> On Wed, 22 Jan 2020 at 20:04, Murali Karicheri <m-karicheri2@ti.com> wrote:
>>
>> I have question about the below parameters in The Gate Parameter Table
>> that are not currently supported by tc command. Looks like they need to
>> be shown to user for management.
>>
>> - ConfigChange - Looks like this needs to be controlled by
>> user. After sending admin command, user send this trigger to start
>> copying admin schedule to operation schedule. Is this getting
>> added to tc command?
>
> "The ConfigChange parameter signals the start of a
> configuration change for the gate
> when it is set to TRUE. This should only be done
> when the various administrative parameters
> are all set to appropriate values."
>
> As far as my understanding goes, all tc-taprio commands currently
> behave as though this boolean is implicitly set to TRUE after the
> structures have been set up. I'm not sure there is any value in doing
> otherwise.
>
That is my understanding as well. However I found this in the 802.1Q
and want to see if someone has insight into this parameter. So perhaps
we can ignore this for now and re-visit when there is a real need for
the same.
>> - ConfigChangeTime - The time at which the administrative variables
>> that determine the cycle are to be copied across to the
>> corresponding operational variables, expressed as a PTP timescale
>
> This is the base-time of the admin schedule, no?
I think there is cycle time extension possible and in that case,
ConfigChangeTime may be different from Admin base-time. So this gives
the actual time when the cycle configuration is actually applied.
>
> "The PTPtime at which the next config change is scheduled to occur.
> The value is a representation of a PTPtime value,
> consisting of a 48-bit integer
> number of seconds and a 32-bit integer number of nanoseconds."
>
>> - TickGranularity - the management parameters specified in Gate
>> Parameter Table allow a management station to discover the
>> characteristics of an implementation’s cycle timer clock
>> (TickGranularity) and to set the parameters for the gating cycle
>> accordingly.
>
> Not sure who is going to use this and for what purpose, but ok.
Same here. Not sure how this will be used by application.
>
>> - ConfigPending - A Boolean variable, set TRUE to indicate that
>> there is a new cycle configuration awaiting installation.
>
> I had tried to export something like this (driver calls back into
> sch_taprio.c when hw has applied the config, this would result in
> ConfigPending = FALSE), but ultimately didn't finish the idea, and it
> caused some problems too, due to incorrect RCU usage.
>
Ok. Hope you plan to send a patch for this in the future.
>> - ConfigChangeError - Error in configuration (AdminBaseTime <
>> CurrentTime)
>
> This can be exported similarly.
Ok.
>
>> - SupportedListMax - Maximum supported Admin/Open shed list.
>>
>> Is there a plan to export these from driver through tc show or such
>> command? The reason being, there would be applications developed to
>> manage configuration/schedule of TSN nodes that would requires these
>> information from the node. So would need a support either in tc or
>> some other means to retrieve them from hardware or driver. That is my
>> understanding...
>>
>
> Not sure what answer you expect to receive for "is there any plan".
To avoid duplicating the work if someone is already working on this.
> You can go ahead and propose something, as long as it is reasonably
> useful to have.
Ok. Will keep in mind.
>
>> Regards,
>>
>> Murali
>>
>> --
>> Murali Karicheri
>> Texas Instruments
>
> Thanks,
> -Vladimir
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2019-11-27 9:59 [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes Po Liu
` (4 preceding siblings ...)
2020-01-18 0:03 ` Vinicius Costa Gomes
@ 2020-02-21 21:43 ` Vinicius Costa Gomes
2020-02-22 3:26 ` [EXT] " Po Liu
5 siblings, 1 reply; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-02-21 21:43 UTC (permalink / raw)
To: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Po Liu
Hi,
Po Liu <po.liu@nxp.com> writes:
> IEEE Std 802.1Qbu standard defined the frame preemption of port
> traffic classes. This patch introduce a method to set traffic
> classes preemption. Add a parameter 'preemption' in struct
> ethtool_link_settings. The value will be translated to a binary,
> each bit represent a traffic class. Bit "1" means preemptable
> traffic class. Bit "0" means express traffic class. MSB represent
> high number traffic class.
>
> If hardware support the frame preemption, driver could set the
> ethernet device with hw_features and features with NETIF_F_PREEMPTION
> when initializing the port driver.
>
> User can check the feature 'tx-preemption' by command 'ethtool -k
> devname'. If hareware set preemption feature. The property would
> be a fixed value 'on' if hardware support the frame preemption.
> Feature would show a fixed value 'off' if hardware don't support
> the frame preemption.
>
> Command 'ethtool devname' and 'ethtool -s devname preemption N'
> would show/set which traffic classes are frame preemptable.
>
> Port driver would implement the frame preemption in the function
> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>
Any updates on this series? If you think that there's something that I
could help, just tell.
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-02-21 21:43 ` Vinicius Costa Gomes
@ 2020-02-22 3:26 ` Po Liu
2020-02-25 17:59 ` Murali Karicheri
2020-03-12 23:34 ` Vinicius Costa Gomes
0 siblings, 2 replies; 39+ messages in thread
From: Po Liu @ 2020-02-22 3:26 UTC (permalink / raw)
To: Vinicius Costa Gomes, davem, hauke.mehrtens, gregkh, allison,
tglx, hkallweit1, saeedm, andrew, f.fainelli, alexandru.ardelean,
jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Murali Karicheri, Ivan Khoronzhuk
Hi Vinicius,
Br,
Po Liu
> -----Original Message-----
> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Sent: 2020年2月22日 5:44
> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org; allison@lohutok.net;
> tglx@linutronix.de; hkallweit1@gmail.com; saeedm@mellanox.com;
> andrew@lunn.ch; f.fainelli@gmail.com; alexandru.ardelean@analog.com;
> jiri@mellanox.com; ayal@mellanox.com; pablo@netfilter.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Cc: simon.horman@netronome.com; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
> traffic classes
>
> Caution: EXT Email
>
> Hi,
>
> Po Liu <po.liu@nxp.com> writes:
>
> > IEEE Std 802.1Qbu standard defined the frame preemption of port
> > traffic classes. This patch introduce a method to set traffic classes
> > preemption. Add a parameter 'preemption' in struct
> > ethtool_link_settings. The value will be translated to a binary, each
> > bit represent a traffic class. Bit "1" means preemptable traffic
> > class. Bit "0" means express traffic class. MSB represent high number
> > traffic class.
> >
> > If hardware support the frame preemption, driver could set the
> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
> > when initializing the port driver.
> >
> > User can check the feature 'tx-preemption' by command 'ethtool -k
> > devname'. If hareware set preemption feature. The property would be a
> > fixed value 'on' if hardware support the frame preemption.
> > Feature would show a fixed value 'off' if hardware don't support the
> > frame preemption.
> >
> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
> > would show/set which traffic classes are frame preemptable.
> >
> > Port driver would implement the frame preemption in the function
> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
> >
>
> Any updates on this series? If you think that there's something that I could help,
> just tell.
Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
If you can take more about this preemption serial, that would be good.
I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
- Add config the fragment size, hold advance, release advance and flags;
My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
- " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
- The ethtool is the better choice to configure the preemption
I agree.
Thanks！
>
>
> Cheers,
> --
> Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
@ 2020-02-25 17:59 ` Murali Karicheri
0 siblings, 0 replies; 39+ messages in thread
From: Murali Karicheri @ 2020-02-25 17:59 UTC (permalink / raw)
To: Po Liu, Vinicius Costa Gomes, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
<<< No Message Collected >>>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
@ 2020-02-25 17:59 ` Murali Karicheri
0 siblings, 0 replies; 39+ messages in thread
From: Murali Karicheri @ 2020-02-25 17:59 UTC (permalink / raw)
To: Po Liu, Vinicius Costa Gomes, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
Hi Po,
On 02/21/2020 10:26 PM, Po Liu wrote:
> Hi Vinicius,
>
>
> Br,
> Po Liu
>
>> -----Original Message-----
>> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> Sent: 2020年2月22日 5:44
>> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
>> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org; allison@lohutok.net;
>> tglx@linutronix.de; hkallweit1@gmail.com; saeedm@mellanox.com;
>> andrew@lunn.ch; f.fainelli@gmail.com; alexandru.ardelean@analog.com;
>> jiri@mellanox.com; ayal@mellanox.com; pablo@netfilter.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Cc: simon.horman@netronome.com; Claudiu Manoil
>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>> traffic classes
>>
>> Caution: EXT Email
>>
>> Hi,
>>
>> Po Liu <po.liu@nxp.com> writes:
>>
>>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>>> traffic classes. This patch introduce a method to set traffic classes
>>> preemption. Add a parameter 'preemption' in struct
>>> ethtool_link_settings. The value will be translated to a binary, each
>>> bit represent a traffic class. Bit "1" means preemptable traffic
>>> class. Bit "0" means express traffic class. MSB represent high number
>>> traffic class.
>>>
>>> If hardware support the frame preemption, driver could set the
>>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>> when initializing the port driver.
>>>
>>> User can check the feature 'tx-preemption' by command 'ethtool -k
>>> devname'. If hareware set preemption feature. The property would be a
>>> fixed value 'on' if hardware support the frame preemption.
>>> Feature would show a fixed value 'off' if hardware don't support the
>>> frame preemption.
>>>
>>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>> would show/set which traffic classes are frame preemptable.
>>>
>>> Port driver would implement the frame preemption in the function
>>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>
>>
>> Any updates on this series? If you think that there's something that I could help,
>> just tell.
>
> Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
> If you can take more about this preemption serial, that would be good.
>
> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
It is Murali :)
> - Add config the fragment size, hold advance, release advance and flags;
- I believe hold advance, release advance values are read only
> My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
> 128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
> - " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
- Couldn't understand the above? Could you please explain a bit more
on what you meant here?
Murali
> Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
> - The ethtool is the better choice to configure the preemption
> I agree.
>
> Thanks！
>>
>>
>> Cheers,
>> --
>> Vinicius
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-02-25 17:59 ` Murali Karicheri
(?)
@ 2020-02-26 2:01 ` Po Liu
-1 siblings, 0 replies; 39+ messages in thread
From: Po Liu @ 2020-02-26 2:01 UTC (permalink / raw)
To: Murali Karicheri, Vinicius Costa Gomes, davem, hauke.mehrtens,
gregkh, allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
Br,
Po Liu
> -----Original Message-----
> From: Murali Karicheri <m-karicheri2@ti.com>
> Sent: 2020年2月26日 1:59
> To: Po Liu <po.liu@nxp.com>; Vinicius Costa Gomes
> <vinicius.gomes@intel.com>; davem@davemloft.net;
> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org;
> allison@lohutok.net; tglx@linutronix.de; hkallweit1@gmail.com;
> saeedm@mellanox.com; andrew@lunn.ch; f.fainelli@gmail.com;
> alexandru.ardelean@analog.com; jiri@mellanox.com; ayal@mellanox.com;
> pablo@netfilter.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Cc: simon.horman@netronome.com; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Subject: Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
> preemption of traffic classes
>
> Caution: EXT Email
>
> Hi Po,
>
> On 02/21/2020 10:26 PM, Po Liu wrote:
> > Hi Vinicius,
> >
> >
> > Br,
> > Po Liu
> >
> >> -----Original Message-----
> >> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> >> Sent: 2020年2月22日 5:44
> >> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
> >> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org;
> >> allison@lohutok.net; tglx@linutronix.de; hkallweit1@gmail.com;
> >> saeedm@mellanox.com; andrew@lunn.ch; f.fainelli@gmail.com;
> >> alexandru.ardelean@analog.com; jiri@mellanox.com;
> ayal@mellanox.com;
> >> pablo@netfilter.org; linux- kernel@vger.kernel.org;
> >> netdev@vger.kernel.org
> >> Cc: simon.horman@netronome.com; Claudiu Manoil
> >> <claudiu.manoil@nxp.com>; Vladimir Oltean
> <vladimir.oltean@nxp.com>;
> >> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang
> Yang
> >> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai
> Hu
> >> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> >> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
> >> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
> >> preemption of traffic classes
> >>
> >> Caution: EXT Email
> >>
> >> Hi,
> >>
> >> Po Liu <po.liu@nxp.com> writes:
> >>
> >>> IEEE Std 802.1Qbu standard defined the frame preemption of port
> >>> traffic classes. This patch introduce a method to set traffic
> >>> classes preemption. Add a parameter 'preemption' in struct
> >>> ethtool_link_settings. The value will be translated to a binary,
> >>> each bit represent a traffic class. Bit "1" means preemptable
> >>> traffic class. Bit "0" means express traffic class. MSB represent
> >>> high number traffic class.
> >>>
> >>> If hardware support the frame preemption, driver could set the
> >>> ethernet device with hw_features and features with
> >>> NETIF_F_PREEMPTION when initializing the port driver.
> >>>
> >>> User can check the feature 'tx-preemption' by command 'ethtool -k
> >>> devname'. If hareware set preemption feature. The property would be
> >>> a fixed value 'on' if hardware support the frame preemption.
> >>> Feature would show a fixed value 'off' if hardware don't support the
> >>> frame preemption.
> >>>
> >>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
> >>> would show/set which traffic classes are frame preemptable.
> >>>
> >>> Port driver would implement the frame preemption in the function
> >>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
> >>>
> >>
> >> Any updates on this series? If you think that there's something that
> >> I could help, just tell.
> >
> > Sorry for the long time not involve the discussion. I am focus on other
> tsn code for tc flower.
> > If you can take more about this preemption serial, that would be good.
> >
> > I summary some suggestions from Marali Karicheri and Ivan
> Khornonzhuk and by you and also others:
>
> It is Murali :)
>
> > - Add config the fragment size, hold advance, release advance and
> > flags;
>
> - I believe hold advance, release advance values are read only
>
> > My comments about the fragment size is in the Qbu spec limit the
> > fragment size " the minimum non-final fragment size is 64, 128, 192, or
> 256 octets " this setting would affect the guardband setting for Qbv. But
> the ethtool setting could not involve this issues but by the taprio side.
> > - " Furthermore, this setting could be extend for a serial setting for mac
> and traffic class." "Better not to using the traffic class concept."
> - Couldn't understand the above? Could you please explain a bit more
> on what you meant here?
Oh, I copy the original suggestion by you and other here. Just for more evaluation for next version.
>
> Murali
> > Could adding a serial setting by "ethtool --preemption xxx" or other
> name. I don' t think it is good to involve in the queue control since queues
> number may bigger than the TC number.
> > - The ethtool is the better choice to configure the preemption
> > I agree.
> >
> > Thanks！
> >>
> >>
> >> Cheers,
> >> --
> >> Vinicius
>
> --
> Murali Karicheri
> Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-02-22 3:26 ` [EXT] " Po Liu
2020-02-25 17:59 ` Murali Karicheri
@ 2020-03-12 23:34 ` Vinicius Costa Gomes
2020-03-13 6:00 ` Po Liu
2020-03-18 14:07 ` Murali Karicheri
1 sibling, 2 replies; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-03-12 23:34 UTC (permalink / raw)
To: Po Liu, davem, hauke.mehrtens, gregkh, allison, tglx, hkallweit1,
saeedm, andrew, f.fainelli, alexandru.ardelean, jiri, ayal,
pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Murali Karicheri, Ivan Khoronzhuk
Hi,
Po Liu <po.liu@nxp.com> writes:
> Hi Vinicius,
>
>
> Br,
> Po Liu
>
>> -----Original Message-----
>> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> Sent: 2020年2月22日 5:44
>> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
>> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org; allison@lohutok.net;
>> tglx@linutronix.de; hkallweit1@gmail.com; saeedm@mellanox.com;
>> andrew@lunn.ch; f.fainelli@gmail.com; alexandru.ardelean@analog.com;
>> jiri@mellanox.com; ayal@mellanox.com; pablo@netfilter.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Cc: simon.horman@netronome.com; Claudiu Manoil
>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>> traffic classes
>>
>> Caution: EXT Email
>>
>> Hi,
>>
>> Po Liu <po.liu@nxp.com> writes:
>>
>> > IEEE Std 802.1Qbu standard defined the frame preemption of port
>> > traffic classes. This patch introduce a method to set traffic classes
>> > preemption. Add a parameter 'preemption' in struct
>> > ethtool_link_settings. The value will be translated to a binary, each
>> > bit represent a traffic class. Bit "1" means preemptable traffic
>> > class. Bit "0" means express traffic class. MSB represent high number
>> > traffic class.
>> >
>> > If hardware support the frame preemption, driver could set the
>> > ethernet device with hw_features and features with NETIF_F_PREEMPTION
>> > when initializing the port driver.
>> >
>> > User can check the feature 'tx-preemption' by command 'ethtool -k
>> > devname'. If hareware set preemption feature. The property would be a
>> > fixed value 'on' if hardware support the frame preemption.
>> > Feature would show a fixed value 'off' if hardware don't support the
>> > frame preemption.
>> >
>> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
>> > would show/set which traffic classes are frame preemptable.
>> >
>> > Port driver would implement the frame preemption in the function
>> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>> >
>>
>> Any updates on this series? If you think that there's something that I could help,
>> just tell.
>
> Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
> If you can take more about this preemption serial, that would be good.
>
> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
> - Add config the fragment size, hold advance, release advance and flags;
> My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
> 128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
> - " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
> Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
> - The ethtool is the better choice to configure the preemption
> I agree.
Just a quick update. I was able to dedicate some time to this, and have
something aproaching RFC-quality, but it needs more testing.
So, question, what were you using for testing this? Anything special?
And btw, thanks for the summary of the discussion.
>
> Thanks！
>>
>>
>> Cheers,
>> --
>> Vinicius
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-03-12 23:34 ` Vinicius Costa Gomes
@ 2020-03-13 6:00 ` Po Liu
2020-03-18 14:07 ` Murali Karicheri
1 sibling, 0 replies; 39+ messages in thread
From: Po Liu @ 2020-03-13 6:00 UTC (permalink / raw)
To: Vinicius Costa Gomes, davem, hauke.mehrtens, gregkh, allison,
tglx, hkallweit1, saeedm, andrew, f.fainelli, alexandru.ardelean,
jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Murali Karicheri, Ivan Khoronzhuk
Br,
Po Liu
> -----Original Message-----
> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Sent: 2020年3月13日 7:35
> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org;
> allison@lohutok.net; tglx@linutronix.de; hkallweit1@gmail.com;
> saeedm@mellanox.com; andrew@lunn.ch; f.fainelli@gmail.com;
> alexandru.ardelean@analog.com; jiri@mellanox.com; ayal@mellanox.com;
> pablo@netfilter.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Cc: simon.horman@netronome.com; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Murali Karicheri <m-karicheri2@ti.com>; Ivan
> Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Subject: RE: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
> preemption of traffic classes
>
> Caution: EXT Email
>
> Hi,
>
> Po Liu <po.liu@nxp.com> writes:
>
> > Hi Vinicius,
> >
> >
> > Br,
> > Po Liu
> >
> >> -----Original Message-----
> >> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> >> Sent: 2020年2月22日 5:44
> >> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
> >> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org;
> >> allison@lohutok.net; tglx@linutronix.de; hkallweit1@gmail.com;
> >> saeedm@mellanox.com; andrew@lunn.ch; f.fainelli@gmail.com;
> >> alexandru.ardelean@analog.com; jiri@mellanox.com; ayal@mellanox.com;
> >> pablo@netfilter.org; linux- kernel@vger.kernel.org;
> >> netdev@vger.kernel.org
> >> Cc: simon.horman@netronome.com; Claudiu Manoil
> >> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> >> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
> >> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> >> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
> >> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
> >> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
> >> preemption of traffic classes
> >>
> >> Caution: EXT Email
> >>
> >> Hi,
> >>
> >> Po Liu <po.liu@nxp.com> writes:
> >>
> >> > IEEE Std 802.1Qbu standard defined the frame preemption of port
> >> > traffic classes. This patch introduce a method to set traffic
> >> > classes preemption. Add a parameter 'preemption' in struct
> >> > ethtool_link_settings. The value will be translated to a binary,
> >> > each bit represent a traffic class. Bit "1" means preemptable
> >> > traffic class. Bit "0" means express traffic class. MSB represent
> >> > high number traffic class.
> >> >
> >> > If hardware support the frame preemption, driver could set the
> >> > ethernet device with hw_features and features with
> >> > NETIF_F_PREEMPTION when initializing the port driver.
> >> >
> >> > User can check the feature 'tx-preemption' by command 'ethtool -k
> >> > devname'. If hareware set preemption feature. The property would be
> >> > a fixed value 'on' if hardware support the frame preemption.
> >> > Feature would show a fixed value 'off' if hardware don't support
> >> > the frame preemption.
> >> >
> >> > Command 'ethtool devname' and 'ethtool -s devname preemption N'
> >> > would show/set which traffic classes are frame preemptable.
> >> >
> >> > Port driver would implement the frame preemption in the function
> >> > get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
> >> >
> >>
> >> Any updates on this series? If you think that there's something that
> >> I could help, just tell.
> >
> > Sorry for the long time not involve the discussion. I am focus on other tsn
> code for tc flower.
> > If you can take more about this preemption serial, that would be good.
> >
> > I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk
> and by you and also others:
> > - Add config the fragment size, hold advance, release advance and flags;
> > My comments about the fragment size is in the Qbu spec limit the
> > fragment size " the minimum non-final fragment size is 64, 128, 192, or 256
> octets " this setting would affect the guardband setting for Qbv. But the
> ethtool setting could not involve this issues but by the taprio side.
> > - " Furthermore, this setting could be extend for a serial setting for mac and
> traffic class." "Better not to using the traffic class concept."
> > Could adding a serial setting by "ethtool --preemption xxx" or other name.
> I don' t think it is good to involve in the queue control since queues number
> may bigger than the TC number.
> > - The ethtool is the better choice to configure the preemption
> > I agree.
>
> Just a quick update. I was able to dedicate some time to this, and have
> something aproaching RFC-quality, but it needs more testing.
>
> So, question, what were you using for testing this? Anything special?
I tested on www.nxp.com/LS1028A. There is nothing special for preemption.
>
> And btw, thanks for the summary of the discussion.
>
> >
> > Thanks！
> >>
> >>
> >> Cheers,
> >> --
> >> Vinicius
>
>
> --
> Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-03-12 23:34 ` Vinicius Costa Gomes
2020-03-13 6:00 ` Po Liu
@ 2020-03-18 14:07 ` Murali Karicheri
2020-05-13 14:55 ` Murali Karicheri
1 sibling, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2020-03-18 14:07 UTC (permalink / raw)
To: Vinicius Costa Gomes, Po Liu, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
Hi Vinicius,
On 03/12/2020 07:34 PM, Vinicius Costa Gomes wrote:
> Hi,
>
> Po Liu <po.liu@nxp.com> writes:
>
>> Hi Vinicius,
>>
>>
>> Br,
>> Po Liu
>>
>>> -----Original Message-----
>>> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>>> Sent: 2020年2月22日 5:44
>>> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
>>> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org; allison@lohutok.net;
>>> tglx@linutronix.de; hkallweit1@gmail.com; saeedm@mellanox.com;
>>> andrew@lunn.ch; f.fainelli@gmail.com; alexandru.ardelean@analog.com;
>>> jiri@mellanox.com; ayal@mellanox.com; pablo@netfilter.org; linux-
>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>> Cc: simon.horman@netronome.com; Claudiu Manoil
>>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of
>>> traffic classes
>>>
>>> Caution: EXT Email
>>>
>>> Hi,
>>>
>>> Po Liu <po.liu@nxp.com> writes:
>>>
>>>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>>>> traffic classes. This patch introduce a method to set traffic classes
>>>> preemption. Add a parameter 'preemption' in struct
>>>> ethtool_link_settings. The value will be translated to a binary, each
>>>> bit represent a traffic class. Bit "1" means preemptable traffic
>>>> class. Bit "0" means express traffic class. MSB represent high number
>>>> traffic class.
>>>>
>>>> If hardware support the frame preemption, driver could set the
>>>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>>> when initializing the port driver.
>>>>
>>>> User can check the feature 'tx-preemption' by command 'ethtool -k
>>>> devname'. If hareware set preemption feature. The property would be a
>>>> fixed value 'on' if hardware support the frame preemption.
>>>> Feature would show a fixed value 'off' if hardware don't support the
>>>> frame preemption.
>>>>
>>>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>>> would show/set which traffic classes are frame preemptable.
>>>>
>>>> Port driver would implement the frame preemption in the function
>>>> get_link_ksettings() and set_link_ksettings() in the struct ethtool_ops.
>>>>
>>>
>>> Any updates on this series? If you think that there's something that I could help,
>>> just tell.
>>
>> Sorry for the long time not involve the discussion. I am focus on other tsn code for tc flower.
>> If you can take more about this preemption serial, that would be good.
>>
>> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk and by you and also others:
>> - Add config the fragment size, hold advance, release advance and flags;
>> My comments about the fragment size is in the Qbu spec limit the fragment size " the minimum non-final fragment size is 64,
>> 128, 192, or 256 octets " this setting would affect the guardband setting for Qbv. But the ethtool setting could not involve this issues but by the taprio side.
>> - " Furthermore, this setting could be extend for a serial setting for mac and traffic class." "Better not to using the traffic class concept."
>> Could adding a serial setting by "ethtool --preemption xxx" or other name. I don' t think it is good to involve in the queue control since queues number may bigger than the TC number.
>> - The ethtool is the better choice to configure the preemption
>> I agree.
>
> Just a quick update. I was able to dedicate some time to this, and have
> something aproaching RFC-quality, but it needs more testing.
>
Great! I have got my frame preemption working on my SoC. Currently I am
using some defaults. I test it by using statistics provided by the
SoC. I will be able to integrate and test your patch using my internal
version and will include it in my patch to upstream once I am ready.
Regards,
Murali
> So, question, what were you using for testing this? Anything special?
>
> And btw, thanks for the summary of the discussion.
>
>>
>> Thanks！
>>>
>>>
>>> Cheers,
>>> --
>>> Vinicius
>
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-03-18 14:07 ` Murali Karicheri
@ 2020-05-13 14:55 ` Murali Karicheri
2020-05-13 17:21 ` Vinicius Costa Gomes
0 siblings, 1 reply; 39+ messages in thread
From: Murali Karicheri @ 2020-05-13 14:55 UTC (permalink / raw)
To: Vinicius Costa Gomes, Po Liu, davem, hauke.mehrtens, gregkh,
allison, tglx, hkallweit1, saeedm, andrew, f.fainelli,
alexandru.ardelean, jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
Hi Vinicius,
On 3/18/20 10:07 AM, Murali Karicheri wrote:
> Hi Vinicius,
>
> On 03/12/2020 07:34 PM, Vinicius Costa Gomes wrote:
>> Hi,
>>
>> Po Liu <po.liu@nxp.com> writes:
>>
>>> Hi Vinicius,
>>>
>>>
>>> Br,
>>> Po Liu
>>>
>>>> -----Original Message-----
>>>> From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>>>> Sent: 2020年2月22日 5:44
>>>> To: Po Liu <po.liu@nxp.com>; davem@davemloft.net;
>>>> hauke.mehrtens@intel.com; gregkh@linuxfoundation.org;
>>>> allison@lohutok.net;
>>>> tglx@linutronix.de; hkallweit1@gmail.com; saeedm@mellanox.com;
>>>> andrew@lunn.ch; f.fainelli@gmail.com; alexandru.ardelean@analog.com;
>>>> jiri@mellanox.com; ayal@mellanox.com; pablo@netfilter.org; linux-
>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>> Cc: simon.horman@netronome.com; Claudiu Manoil
>>>> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
>>>> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang
>>>> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
>>>> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li
>>>> <leoyang.li@nxp.com>; Po Liu <po.liu@nxp.com>
>>>> Subject: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame
>>>> preemption of
>>>> traffic classes
>>>>
>>>> Caution: EXT Email
>>>>
>>>> Hi,
>>>>
>>>> Po Liu <po.liu@nxp.com> writes:
>>>>
>>>>> IEEE Std 802.1Qbu standard defined the frame preemption of port
>>>>> traffic classes. This patch introduce a method to set traffic classes
>>>>> preemption. Add a parameter 'preemption' in struct
>>>>> ethtool_link_settings. The value will be translated to a binary, each
>>>>> bit represent a traffic class. Bit "1" means preemptable traffic
>>>>> class. Bit "0" means express traffic class. MSB represent high number
>>>>> traffic class.
>>>>>
>>>>> If hardware support the frame preemption, driver could set the
>>>>> ethernet device with hw_features and features with NETIF_F_PREEMPTION
>>>>> when initializing the port driver.
>>>>>
>>>>> User can check the feature 'tx-preemption' by command 'ethtool -k
>>>>> devname'. If hareware set preemption feature. The property would be a
>>>>> fixed value 'on' if hardware support the frame preemption.
>>>>> Feature would show a fixed value 'off' if hardware don't support the
>>>>> frame preemption.
>>>>>
>>>>> Command 'ethtool devname' and 'ethtool -s devname preemption N'
>>>>> would show/set which traffic classes are frame preemptable.
>>>>>
>>>>> Port driver would implement the frame preemption in the function
>>>>> get_link_ksettings() and set_link_ksettings() in the struct
>>>>> ethtool_ops.
>>>>>
>>>>
>>>> Any updates on this series? If you think that there's something that
>>>> I could help,
>>>> just tell.
>>>
>>> Sorry for the long time not involve the discussion. I am focus on
>>> other tsn code for tc flower.
>>> If you can take more about this preemption serial, that would be good.
>>>
>>> I summary some suggestions from Marali Karicheri and Ivan Khornonzhuk
>>> and by you and also others:
>>> - Add config the fragment size, hold advance, release advance and flags;
>>> My comments about the fragment size is in the Qbu spec limit the
>>> fragment size " the minimum non-final fragment size is 64,
>>> 128, 192, or 256 octets " this setting would affect the guardband
>>> setting for Qbv. But the ethtool setting could not involve this
>>> issues but by the taprio side.
>>> - " Furthermore, this setting could be extend for a serial setting
>>> for mac and traffic class." "Better not to using the traffic class
>>> concept."
>>> Could adding a serial setting by "ethtool --preemption xxx" or
>>> other name. I don' t think it is good to involve in the queue control
>>> since queues number may bigger than the TC number.
>>> - The ethtool is the better choice to configure the preemption
>>> I agree.
>>
>> Just a quick update. I was able to dedicate some time to this, and have
>> something aproaching RFC-quality, but it needs more testing.
>>
> Great! I have got my frame preemption working on my SoC. Currently I am
> using some defaults. I test it by using statistics provided by the
> SoC. I will be able to integrate and test your patch using my internal
> version and will include it in my patch to upstream once I am ready.
>
Any progress on your side for a patch for the support?
I have posted my EST offload series for AM65x CPSW to netdev list today
at
https://marc.info/?l=linux-netdev&m=158937640015582&w=2
https://marc.info/?l=linux-netdev&m=158937639515579&w=2
https://marc.info/?l=linux-netdev&m=158937638315573&w=2
Next on my list of things to do is the IET FPE support for which I need
to have ethtool interface to allow configuring the express/preemptible
queues and feature enable/disable. Currently I am using a ethtool
priv-flags and some defaults. If you can post a patch, I will be able
to integrate and test it on AM65x CPSW driver and provide my comments/
Tested-by:
Regards,
Murali
> Regards,
>
> Murali
>> So, question, what were you using for testing this? Anything special?
>>
>> And btw, thanks for the summary of the discussion.
>>
>>>
>>> Thanks！
>>>>
>>>>
>>>> Cheers,
>>>> --
>>>> Vinicius
>>
>>
>
--
Murali Karicheri
Texas Instruments
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [EXT] Re: [v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes
2020-05-13 14:55 ` Murali Karicheri
@ 2020-05-13 17:21 ` Vinicius Costa Gomes
0 siblings, 0 replies; 39+ messages in thread
From: Vinicius Costa Gomes @ 2020-05-13 17:21 UTC (permalink / raw)
To: Murali Karicheri, Po Liu, davem, hauke.mehrtens, gregkh, allison,
tglx, hkallweit1, saeedm, andrew, f.fainelli, alexandru.ardelean,
jiri, ayal, pablo, linux-kernel, netdev
Cc: simon.horman, Claudiu Manoil, Vladimir Oltean,
Alexandru Marginean, Xiaoliang Yang, Roy Zang, Mingkai Hu,
Jerry Huang, Leo Li, Ivan Khoronzhuk
Hi Murali,
Murali Karicheri <m-karicheri2@ti.com> writes:
> Any progress on your side for a patch for the support?
>
Sorry for the delay, things got a bit crazy here for some time.
I have a RFC-quality series that I am finishing testing, I'll try to
post it this week.
> I have posted my EST offload series for AM65x CPSW to netdev list today
> at
>
> https://marc.info/?l=linux-netdev&m=158937640015582&w=2
> https://marc.info/?l=linux-netdev&m=158937639515579&w=2
> https://marc.info/?l=linux-netdev&m=158937638315573&w=2
I'll try to have a look.
>
> Next on my list of things to do is the IET FPE support for which I need
> to have ethtool interface to allow configuring the express/preemptible
> queues and feature enable/disable. Currently I am using a ethtool
> priv-flags and some defaults. If you can post a patch, I will be able
> to integrate and test it on AM65x CPSW driver and provide my comments/
> Tested-by:
Understood. Thanks.
--
Vinicius
^ permalink raw reply [flat|nested] 39+ messages in thread