Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
able to report statistics (which is future work), it seems that we need
to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
multiplexing, and pass the command as part of the common portion of the
muxed structure.
Since we already have an "enable" variable in tc_taprio_qopt_offload,
refactor all drivers to check for "cmd" instead of "enable", and reject
every other command except "replace" and "destroy" - to be future proof.
Signed-off-by: Vladimir Oltean <[email protected]>
---
drivers/net/dsa/hirschmann/hellcreek.c | 14 +++++++++-----
drivers/net/dsa/ocelot/felix_vsc9959.c | 4 +++-
drivers/net/dsa/sja1105/sja1105_tas.c | 7 +++++--
drivers/net/ethernet/engleder/tsnep_selftests.c | 12 ++++++------
drivers/net/ethernet/engleder/tsnep_tc.c | 4 +++-
drivers/net/ethernet/freescale/enetc/enetc_qos.c | 6 +++++-
drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++++++--
.../net/ethernet/microchip/lan966x/lan966x_tc.c | 10 ++++++++--
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 7 +++++--
drivers/net/ethernet/ti/am65-cpsw-qos.c | 11 ++++++++---
include/net/pkt_sched.h | 7 ++++++-
net/sched/sch_taprio.c | 4 ++--
12 files changed, 71 insertions(+), 28 deletions(-)
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 595a548bb0a8..af50001ccdd4 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1885,13 +1885,17 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
case TC_SETUP_QDISC_TAPRIO: {
struct tc_taprio_qopt_offload *taprio = type_data;
- if (!hellcreek_validate_schedule(hellcreek, taprio))
- return -EOPNOTSUPP;
+ switch (taprio->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ if (!hellcreek_validate_schedule(hellcreek, taprio))
+ return -EOPNOTSUPP;
- if (taprio->enable)
return hellcreek_port_set_schedule(ds, port, taprio);
-
- return hellcreek_port_del_schedule(ds, port);
+ case TAPRIO_CMD_DESTROY:
+ return hellcreek_port_del_schedule(ds, port);
+ default:
+ return -EOPNOTSUPP;
+ }
}
default:
return -EOPNOTSUPP;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 030738fef60e..5de6a27052fc 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1411,7 +1411,7 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
mutex_lock(&ocelot->tas_lock);
- if (!taprio->enable) {
+ if (taprio->cmd == TAPRIO_CMD_DESTROY) {
ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
ocelot_rmw_rix(ocelot, 0, QSYS_TAG_CONFIG_ENABLE,
QSYS_TAG_CONFIG, port);
@@ -1423,6 +1423,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
mutex_unlock(&ocelot->tas_lock);
return 0;
+ } else if (taprio->cmd != TAPRIO_CMD_REPLACE) {
+ return -EOPNOTSUPP;
}
ret = ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
diff --git a/drivers/net/dsa/sja1105/sja1105_tas.c b/drivers/net/dsa/sja1105/sja1105_tas.c
index e6153848a950..d7818710bc02 100644
--- a/drivers/net/dsa/sja1105/sja1105_tas.c
+++ b/drivers/net/dsa/sja1105/sja1105_tas.c
@@ -516,10 +516,11 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
/* Can't change an already configured port (must delete qdisc first).
* Can't delete the qdisc from an unconfigured port.
*/
- if (!!tas_data->offload[port] == admin->enable)
+ if ((!!tas_data->offload[port] && admin->cmd == TAPRIO_CMD_REPLACE) ||
+ (!tas_data->offload[port] && admin->cmd == TAPRIO_CMD_DESTROY))
return -EINVAL;
- if (!admin->enable) {
+ if (admin->cmd == TAPRIO_CMD_DESTROY) {
taprio_offload_free(tas_data->offload[port]);
tas_data->offload[port] = NULL;
@@ -528,6 +529,8 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
return rc;
return sja1105_static_config_reload(priv, SJA1105_SCHEDULING);
+ } else if (admin->cmd != TAPRIO_CMD_REPLACE) {
+ return -EOPNOTSUPP;
}
/* The cycle time extension is the amount of time the last cycle from
diff --git a/drivers/net/ethernet/engleder/tsnep_selftests.c b/drivers/net/ethernet/engleder/tsnep_selftests.c
index 1581d6b22232..8a9145f93147 100644
--- a/drivers/net/ethernet/engleder/tsnep_selftests.c
+++ b/drivers/net/ethernet/engleder/tsnep_selftests.c
@@ -329,7 +329,7 @@ static bool disable_taprio(struct tsnep_adapter *adapter)
int retval;
memset(&qopt, 0, sizeof(qopt));
- qopt.enable = 0;
+ qopt.cmd = TAPRIO_CMD_DESTROY;
retval = tsnep_tc_setup(adapter->netdev, TC_SETUP_QDISC_TAPRIO, &qopt);
if (retval)
return false;
@@ -360,7 +360,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
for (i = 0; i < 255; i++)
qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
- qopt->enable = 1;
+ qopt->cmd = TAPRIO_CMD_REPLACE;
qopt->base_time = ktime_set(0, 0);
qopt->cycle_time = 1500000;
qopt->cycle_time_extension = 0;
@@ -382,7 +382,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
if (!run_taprio(adapter, qopt, 100))
goto failed;
- qopt->enable = 1;
+ qopt->cmd = TAPRIO_CMD_REPLACE;
qopt->base_time = ktime_set(0, 0);
qopt->cycle_time = 411854;
qopt->cycle_time_extension = 0;
@@ -406,7 +406,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
if (!run_taprio(adapter, qopt, 100))
goto failed;
- qopt->enable = 1;
+ qopt->cmd = TAPRIO_CMD_REPLACE;
qopt->base_time = ktime_set(0, 0);
delay_base_time(adapter, qopt, 12);
qopt->cycle_time = 125000;
@@ -457,7 +457,7 @@ static bool tsnep_test_taprio_change(struct tsnep_adapter *adapter)
for (i = 0; i < 255; i++)
qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
- qopt->enable = 1;
+ qopt->cmd = TAPRIO_CMD_REPLACE;
qopt->base_time = ktime_set(0, 0);
qopt->cycle_time = 100000;
qopt->cycle_time_extension = 0;
@@ -610,7 +610,7 @@ static bool tsnep_test_taprio_extension(struct tsnep_adapter *adapter)
for (i = 0; i < 255; i++)
qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
- qopt->enable = 1;
+ qopt->cmd = TAPRIO_CMD_REPLACE;
qopt->base_time = ktime_set(0, 0);
qopt->cycle_time = 100000;
qopt->cycle_time_extension = 50000;
diff --git a/drivers/net/ethernet/engleder/tsnep_tc.c b/drivers/net/ethernet/engleder/tsnep_tc.c
index d083e6684f12..745b191a5540 100644
--- a/drivers/net/ethernet/engleder/tsnep_tc.c
+++ b/drivers/net/ethernet/engleder/tsnep_tc.c
@@ -325,7 +325,7 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
if (!adapter->gate_control)
return -EOPNOTSUPP;
- if (!qopt->enable) {
+ if (qopt->cmd == TAPRIO_CMD_DESTROY) {
/* disable gate control if active */
mutex_lock(&adapter->gate_control_lock);
@@ -337,6 +337,8 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
mutex_unlock(&adapter->gate_control_lock);
return 0;
+ } else if (qopt->cmd != TAPRIO_CMD_REPLACE) {
+ return -EOPNOTSUPP;
}
retval = tsnep_validate_gcl(qopt);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 83c27bbbc6ed..7aad824f4da7 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -65,7 +65,7 @@ static int enetc_setup_taprio(struct net_device *ndev,
gcl_len = admin_conf->num_entries;
tge = enetc_rd(hw, ENETC_PTGCR);
- if (!admin_conf->enable) {
+ if (admin_conf->cmd == TAPRIO_CMD_DESTROY) {
enetc_wr(hw, ENETC_PTGCR, tge & ~ENETC_PTGCR_TGE);
enetc_reset_ptcmsdur(hw);
@@ -138,6 +138,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
struct enetc_ndev_priv *priv = netdev_priv(ndev);
int err, i;
+ if (taprio->cmd != TAPRIO_CMD_REPLACE &&
+ taprio->cmd != TAPRIO_CMD_DESTROY)
+ return -EOPNOTSUPP;
+
/* TSD and Qbv are mutually exclusive in hardware */
for (i = 0; i < priv->num_tx_rings; i++)
if (priv->tx_ring[i]->tsd_enable)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index c5ef1edcf548..88145c30c919 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6113,9 +6113,18 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
size_t n;
int i;
- adapter->qbv_enable = qopt->enable;
+ switch (qopt->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ adapter->qbv_enable = true;
+ break;
+ case TAPRIO_CMD_DESTROY:
+ adapter->qbv_enable = false;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
- if (!qopt->enable)
+ if (!adapter->qbv_enable)
return igc_tsn_clear_schedule(adapter);
if (qopt->base_time < 0)
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
index cf0cc7562d04..ee652f2d2359 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
@@ -21,8 +21,14 @@ static int lan966x_tc_setup_qdisc_mqprio(struct lan966x_port *port,
static int lan966x_tc_setup_qdisc_taprio(struct lan966x_port *port,
struct tc_taprio_qopt_offload *taprio)
{
- return taprio->enable ? lan966x_taprio_add(port, taprio) :
- lan966x_taprio_del(port);
+ switch (taprio->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ return lan966x_taprio_add(port, taprio);
+ case TAPRIO_CMD_DESTROY:
+ return lan966x_taprio_del(port);
+ default:
+ return -EOPNOTSUPP;
+ }
}
static int lan966x_tc_setup_qdisc_tbf(struct lan966x_port *port,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 9d55226479b4..ac41ef4cbd2f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -966,8 +966,11 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
return -EOPNOTSUPP;
}
- if (!qopt->enable)
+ if (qopt->cmd == TAPRIO_CMD_DESTROY)
goto disable;
+ else if (qopt->cmd != TAPRIO_CMD_REPLACE)
+ return -EOPNOTSUPP;
+
if (qopt->num_entries >= dep)
return -EINVAL;
if (!qopt->cycle_time)
@@ -988,7 +991,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
mutex_lock(&priv->plat->est->lock);
priv->plat->est->gcl_size = size;
- priv->plat->est->enable = qopt->enable;
+ priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE;
mutex_unlock(&priv->plat->est->lock);
for (i = 0; i < size; i++) {
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 3a908db6e5b2..eced87fa261c 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -450,7 +450,7 @@ static int am65_cpsw_configure_taprio(struct net_device *ndev,
am65_cpsw_est_update_state(ndev);
- if (!est_new->taprio.enable) {
+ if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
am65_cpsw_stop_est(ndev);
return ret;
}
@@ -476,7 +476,7 @@ static int am65_cpsw_configure_taprio(struct net_device *ndev,
am65_cpsw_est_set_sched_list(ndev, est_new);
am65_cpsw_port_est_assign_buf_num(ndev, est_new->buf);
- am65_cpsw_est_set(ndev, est_new->taprio.enable);
+ am65_cpsw_est_set(ndev, est_new->taprio.cmd == TAPRIO_CMD_REPLACE);
if (tact == TACT_PROG) {
ret = am65_cpsw_timer_set(ndev, est_new);
@@ -520,7 +520,7 @@ static int am65_cpsw_set_taprio(struct net_device *ndev, void *type_data)
am65_cpsw_cp_taprio(taprio, &est_new->taprio);
ret = am65_cpsw_configure_taprio(ndev, est_new);
if (!ret) {
- if (taprio->enable) {
+ if (taprio->cmd == TAPRIO_CMD_REPLACE) {
devm_kfree(&ndev->dev, port->qos.est_admin);
port->qos.est_admin = est_new;
@@ -564,8 +564,13 @@ static void am65_cpsw_est_link_up(struct net_device *ndev, int link_speed)
static int am65_cpsw_setup_taprio(struct net_device *ndev, void *type_data)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+ struct tc_taprio_qopt_offload *taprio = type_data;
struct am65_cpsw_common *common = port->common;
+ if (taprio->cmd != TAPRIO_CMD_REPLACE &&
+ taprio->cmd != TAPRIO_CMD_DESTROY)
+ return -EOPNOTSUPP;
+
if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
return -ENODEV;
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index f436688b6efc..f5fb11da357b 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -185,6 +185,11 @@ struct tc_taprio_caps {
bool broken_mqprio:1;
};
+enum tc_taprio_qopt_cmd {
+ TAPRIO_CMD_REPLACE,
+ TAPRIO_CMD_DESTROY,
+};
+
struct tc_taprio_sched_entry {
u8 command; /* TC_TAPRIO_CMD_* */
@@ -196,7 +201,7 @@ struct tc_taprio_sched_entry {
struct tc_taprio_qopt_offload {
struct tc_mqprio_qopt_offload mqprio;
struct netlink_ext_ack *extack;
- u8 enable;
+ enum tc_taprio_qopt_cmd cmd;
ktime_t base_time;
u64 cycle_time;
u64 cycle_time_extension;
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index d29e6785854d..06bf4c6355a5 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1524,7 +1524,7 @@ static int taprio_enable_offload(struct net_device *dev,
"Not enough memory for enabling offload mode");
return -ENOMEM;
}
- offload->enable = 1;
+ offload->cmd = TAPRIO_CMD_REPLACE;
offload->extack = extack;
mqprio_qopt_reconstruct(dev, &offload->mqprio.qopt);
offload->mqprio.extack = extack;
@@ -1572,7 +1572,7 @@ static int taprio_disable_offload(struct net_device *dev,
"Not enough memory to disable offload mode");
return -ENOMEM;
}
- offload->enable = 0;
+ offload->cmd = TAPRIO_CMD_DESTROY;
err = ops->ndo_setup_tc(dev, TC_SETUP_QDISC_TAPRIO, offload);
if (err < 0) {
--
2.34.1
The 05/30/2023 12:19, Vladimir Oltean wrote:
>
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
> able to report statistics (which is future work), it seems that we need
> to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload,
> refactor all drivers to check for "cmd" instead of "enable", and reject
> every other command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]> # for lan966x
> ---
> drivers/net/dsa/hirschmann/hellcreek.c | 14 +++++++++-----
> drivers/net/dsa/ocelot/felix_vsc9959.c | 4 +++-
> drivers/net/dsa/sja1105/sja1105_tas.c | 7 +++++--
> drivers/net/ethernet/engleder/tsnep_selftests.c | 12 ++++++------
> drivers/net/ethernet/engleder/tsnep_tc.c | 4 +++-
> drivers/net/ethernet/freescale/enetc/enetc_qos.c | 6 +++++-
> drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++++++--
> .../net/ethernet/microchip/lan966x/lan966x_tc.c | 10 ++++++++--
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 7 +++++--
> drivers/net/ethernet/ti/am65-cpsw-qos.c | 11 ++++++++---
> include/net/pkt_sched.h | 7 ++++++-
> net/sched/sch_taprio.c | 4 ++--
> 12 files changed, 71 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 595a548bb0a8..af50001ccdd4 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -1885,13 +1885,17 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
> case TC_SETUP_QDISC_TAPRIO: {
> struct tc_taprio_qopt_offload *taprio = type_data;
>
> - if (!hellcreek_validate_schedule(hellcreek, taprio))
> - return -EOPNOTSUPP;
> + switch (taprio->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + if (!hellcreek_validate_schedule(hellcreek, taprio))
> + return -EOPNOTSUPP;
>
> - if (taprio->enable)
> return hellcreek_port_set_schedule(ds, port, taprio);
> -
> - return hellcreek_port_del_schedule(ds, port);
> + case TAPRIO_CMD_DESTROY:
> + return hellcreek_port_del_schedule(ds, port);
> + default:
> + return -EOPNOTSUPP;
> + }
> }
> default:
> return -EOPNOTSUPP;
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 030738fef60e..5de6a27052fc 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -1411,7 +1411,7 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
>
> mutex_lock(&ocelot->tas_lock);
>
> - if (!taprio->enable) {
> + if (taprio->cmd == TAPRIO_CMD_DESTROY) {
> ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
> ocelot_rmw_rix(ocelot, 0, QSYS_TAG_CONFIG_ENABLE,
> QSYS_TAG_CONFIG, port);
> @@ -1423,6 +1423,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
>
> mutex_unlock(&ocelot->tas_lock);
> return 0;
> + } else if (taprio->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> ret = ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
> diff --git a/drivers/net/dsa/sja1105/sja1105_tas.c b/drivers/net/dsa/sja1105/sja1105_tas.c
> index e6153848a950..d7818710bc02 100644
> --- a/drivers/net/dsa/sja1105/sja1105_tas.c
> +++ b/drivers/net/dsa/sja1105/sja1105_tas.c
> @@ -516,10 +516,11 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
> /* Can't change an already configured port (must delete qdisc first).
> * Can't delete the qdisc from an unconfigured port.
> */
> - if (!!tas_data->offload[port] == admin->enable)
> + if ((!!tas_data->offload[port] && admin->cmd == TAPRIO_CMD_REPLACE) ||
> + (!tas_data->offload[port] && admin->cmd == TAPRIO_CMD_DESTROY))
> return -EINVAL;
>
> - if (!admin->enable) {
> + if (admin->cmd == TAPRIO_CMD_DESTROY) {
> taprio_offload_free(tas_data->offload[port]);
> tas_data->offload[port] = NULL;
>
> @@ -528,6 +529,8 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int port,
> return rc;
>
> return sja1105_static_config_reload(priv, SJA1105_SCHEDULING);
> + } else if (admin->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> /* The cycle time extension is the amount of time the last cycle from
> diff --git a/drivers/net/ethernet/engleder/tsnep_selftests.c b/drivers/net/ethernet/engleder/tsnep_selftests.c
> index 1581d6b22232..8a9145f93147 100644
> --- a/drivers/net/ethernet/engleder/tsnep_selftests.c
> +++ b/drivers/net/ethernet/engleder/tsnep_selftests.c
> @@ -329,7 +329,7 @@ static bool disable_taprio(struct tsnep_adapter *adapter)
> int retval;
>
> memset(&qopt, 0, sizeof(qopt));
> - qopt.enable = 0;
> + qopt.cmd = TAPRIO_CMD_DESTROY;
> retval = tsnep_tc_setup(adapter->netdev, TC_SETUP_QDISC_TAPRIO, &qopt);
> if (retval)
> return false;
> @@ -360,7 +360,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 1500000;
> qopt->cycle_time_extension = 0;
> @@ -382,7 +382,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
> if (!run_taprio(adapter, qopt, 100))
> goto failed;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 411854;
> qopt->cycle_time_extension = 0;
> @@ -406,7 +406,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter *adapter)
> if (!run_taprio(adapter, qopt, 100))
> goto failed;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> delay_base_time(adapter, qopt, 12);
> qopt->cycle_time = 125000;
> @@ -457,7 +457,7 @@ static bool tsnep_test_taprio_change(struct tsnep_adapter *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 100000;
> qopt->cycle_time_extension = 0;
> @@ -610,7 +610,7 @@ static bool tsnep_test_taprio_extension(struct tsnep_adapter *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 100000;
> qopt->cycle_time_extension = 50000;
> diff --git a/drivers/net/ethernet/engleder/tsnep_tc.c b/drivers/net/ethernet/engleder/tsnep_tc.c
> index d083e6684f12..745b191a5540 100644
> --- a/drivers/net/ethernet/engleder/tsnep_tc.c
> +++ b/drivers/net/ethernet/engleder/tsnep_tc.c
> @@ -325,7 +325,7 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
> if (!adapter->gate_control)
> return -EOPNOTSUPP;
>
> - if (!qopt->enable) {
> + if (qopt->cmd == TAPRIO_CMD_DESTROY) {
> /* disable gate control if active */
> mutex_lock(&adapter->gate_control_lock);
>
> @@ -337,6 +337,8 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
> mutex_unlock(&adapter->gate_control_lock);
>
> return 0;
> + } else if (qopt->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> retval = tsnep_validate_gcl(qopt);
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> index 83c27bbbc6ed..7aad824f4da7 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> @@ -65,7 +65,7 @@ static int enetc_setup_taprio(struct net_device *ndev,
> gcl_len = admin_conf->num_entries;
>
> tge = enetc_rd(hw, ENETC_PTGCR);
> - if (!admin_conf->enable) {
> + if (admin_conf->cmd == TAPRIO_CMD_DESTROY) {
> enetc_wr(hw, ENETC_PTGCR, tge & ~ENETC_PTGCR_TGE);
> enetc_reset_ptcmsdur(hw);
>
> @@ -138,6 +138,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
> struct enetc_ndev_priv *priv = netdev_priv(ndev);
> int err, i;
>
> + if (taprio->cmd != TAPRIO_CMD_REPLACE &&
> + taprio->cmd != TAPRIO_CMD_DESTROY)
> + return -EOPNOTSUPP;
> +
> /* TSD and Qbv are mutually exclusive in hardware */
> for (i = 0; i < priv->num_tx_rings; i++)
> if (priv->tx_ring[i]->tsd_enable)
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index c5ef1edcf548..88145c30c919 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6113,9 +6113,18 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
> size_t n;
> int i;
>
> - adapter->qbv_enable = qopt->enable;
> + switch (qopt->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + adapter->qbv_enable = true;
> + break;
> + case TAPRIO_CMD_DESTROY:
> + adapter->qbv_enable = false;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
>
> - if (!qopt->enable)
> + if (!adapter->qbv_enable)
> return igc_tsn_clear_schedule(adapter);
>
> if (qopt->base_time < 0)
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> index cf0cc7562d04..ee652f2d2359 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> @@ -21,8 +21,14 @@ static int lan966x_tc_setup_qdisc_mqprio(struct lan966x_port *port,
> static int lan966x_tc_setup_qdisc_taprio(struct lan966x_port *port,
> struct tc_taprio_qopt_offload *taprio)
> {
> - return taprio->enable ? lan966x_taprio_add(port, taprio) :
> - lan966x_taprio_del(port);
> + switch (taprio->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + return lan966x_taprio_add(port, taprio);
> + case TAPRIO_CMD_DESTROY:
> + return lan966x_taprio_del(port);
> + default:
> + return -EOPNOTSUPP;
> + }
> }
>
> static int lan966x_tc_setup_qdisc_tbf(struct lan966x_port *port,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 9d55226479b4..ac41ef4cbd2f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -966,8 +966,11 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
> return -EOPNOTSUPP;
> }
>
> - if (!qopt->enable)
> + if (qopt->cmd == TAPRIO_CMD_DESTROY)
> goto disable;
> + else if (qopt->cmd != TAPRIO_CMD_REPLACE)
> + return -EOPNOTSUPP;
> +
> if (qopt->num_entries >= dep)
> return -EINVAL;
> if (!qopt->cycle_time)
> @@ -988,7 +991,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
>
> mutex_lock(&priv->plat->est->lock);
> priv->plat->est->gcl_size = size;
> - priv->plat->est->enable = qopt->enable;
> + priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE;
> mutex_unlock(&priv->plat->est->lock);
>
> for (i = 0; i < size; i++) {
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c b/drivers/net/ethernet/ti/am65-cpsw-qos.c
> index 3a908db6e5b2..eced87fa261c 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
> @@ -450,7 +450,7 @@ static int am65_cpsw_configure_taprio(struct net_device *ndev,
>
> am65_cpsw_est_update_state(ndev);
>
> - if (!est_new->taprio.enable) {
> + if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
> am65_cpsw_stop_est(ndev);
> return ret;
> }
> @@ -476,7 +476,7 @@ static int am65_cpsw_configure_taprio(struct net_device *ndev,
> am65_cpsw_est_set_sched_list(ndev, est_new);
> am65_cpsw_port_est_assign_buf_num(ndev, est_new->buf);
>
> - am65_cpsw_est_set(ndev, est_new->taprio.enable);
> + am65_cpsw_est_set(ndev, est_new->taprio.cmd == TAPRIO_CMD_REPLACE);
>
> if (tact == TACT_PROG) {
> ret = am65_cpsw_timer_set(ndev, est_new);
> @@ -520,7 +520,7 @@ static int am65_cpsw_set_taprio(struct net_device *ndev, void *type_data)
> am65_cpsw_cp_taprio(taprio, &est_new->taprio);
> ret = am65_cpsw_configure_taprio(ndev, est_new);
> if (!ret) {
> - if (taprio->enable) {
> + if (taprio->cmd == TAPRIO_CMD_REPLACE) {
> devm_kfree(&ndev->dev, port->qos.est_admin);
>
> port->qos.est_admin = est_new;
> @@ -564,8 +564,13 @@ static void am65_cpsw_est_link_up(struct net_device *ndev, int link_speed)
> static int am65_cpsw_setup_taprio(struct net_device *ndev, void *type_data)
> {
> struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> + struct tc_taprio_qopt_offload *taprio = type_data;
> struct am65_cpsw_common *common = port->common;
>
> + if (taprio->cmd != TAPRIO_CMD_REPLACE &&
> + taprio->cmd != TAPRIO_CMD_DESTROY)
> + return -EOPNOTSUPP;
> +
> if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
> return -ENODEV;
>
> diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
> index f436688b6efc..f5fb11da357b 100644
> --- a/include/net/pkt_sched.h
> +++ b/include/net/pkt_sched.h
> @@ -185,6 +185,11 @@ struct tc_taprio_caps {
> bool broken_mqprio:1;
> };
>
> +enum tc_taprio_qopt_cmd {
> + TAPRIO_CMD_REPLACE,
> + TAPRIO_CMD_DESTROY,
> +};
> +
> struct tc_taprio_sched_entry {
> u8 command; /* TC_TAPRIO_CMD_* */
>
> @@ -196,7 +201,7 @@ struct tc_taprio_sched_entry {
> struct tc_taprio_qopt_offload {
> struct tc_mqprio_qopt_offload mqprio;
> struct netlink_ext_ack *extack;
> - u8 enable;
> + enum tc_taprio_qopt_cmd cmd;
> ktime_t base_time;
> u64 cycle_time;
> u64 cycle_time_extension;
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index d29e6785854d..06bf4c6355a5 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -1524,7 +1524,7 @@ static int taprio_enable_offload(struct net_device *dev,
> "Not enough memory for enabling offload mode");
> return -ENOMEM;
> }
> - offload->enable = 1;
> + offload->cmd = TAPRIO_CMD_REPLACE;
> offload->extack = extack;
> mqprio_qopt_reconstruct(dev, &offload->mqprio.qopt);
> offload->mqprio.extack = extack;
> @@ -1572,7 +1572,7 @@ static int taprio_disable_offload(struct net_device *dev,
> "Not enough memory to disable offload mode");
> return -ENOMEM;
> }
> - offload->enable = 0;
> + offload->cmd = TAPRIO_CMD_DESTROY;
>
> err = ops->ndo_setup_tc(dev, TC_SETUP_QDISC_TAPRIO, offload);
> if (err < 0) {
> --
> 2.34.1
>
>
--
/Horatiu
On Tue May 30 2023, Vladimir Oltean wrote:
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
> able to report statistics (which is future work), it seems that we need
> to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload,
> refactor all drivers to check for "cmd" instead of "enable", and reject
> every other command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
[...]
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 595a548bb0a8..af50001ccdd4 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -1885,13 +1885,17 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
> case TC_SETUP_QDISC_TAPRIO: {
> struct tc_taprio_qopt_offload *taprio = type_data;
>
> - if (!hellcreek_validate_schedule(hellcreek, taprio))
> - return -EOPNOTSUPP;
> + switch (taprio->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + if (!hellcreek_validate_schedule(hellcreek, taprio))
> + return -EOPNOTSUPP;
>
> - if (taprio->enable)
> return hellcreek_port_set_schedule(ds, port, taprio);
> -
> - return hellcreek_port_del_schedule(ds, port);
> + case TAPRIO_CMD_DESTROY:
> + return hellcreek_port_del_schedule(ds, port);
> + default:
> + return -EOPNOTSUPP;
> + }
> }
> default:
> return -EOPNOTSUPP;
Uhm, seems like the current code validates the schedule even for
removing a schedule which seems a bit odd. With your changes it looks
correct.
Acked-by: Kurt Kanzenbach <[email protected]> # hellcreek
Anyway, the hellcreek device has Tx overrun counters per TC. Even though
they should be zero, simply because the hardware Length Aware Shaper is
enabled by default.
Thanks,
Kurt
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be able to
> report statistics (which is future work), it seems that we need to drill one step
> further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload, refactor
> all drivers to check for "cmd" instead of "enable", and reject every other
> command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Muhammad Husaini Zulkifli <[email protected]>
> ---
> drivers/net/dsa/hirschmann/hellcreek.c | 14 +++++++++-----
> drivers/net/dsa/ocelot/felix_vsc9959.c | 4 +++-
> drivers/net/dsa/sja1105/sja1105_tas.c | 7 +++++--
> drivers/net/ethernet/engleder/tsnep_selftests.c | 12 ++++++------
> drivers/net/ethernet/engleder/tsnep_tc.c | 4 +++-
> drivers/net/ethernet/freescale/enetc/enetc_qos.c | 6 +++++-
> drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++++++--
> .../net/ethernet/microchip/lan966x/lan966x_tc.c | 10 ++++++++--
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 7 +++++--
> drivers/net/ethernet/ti/am65-cpsw-qos.c | 11 ++++++++---
> include/net/pkt_sched.h | 7 ++++++-
> net/sched/sch_taprio.c | 4 ++--
> 12 files changed, 71 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c
> b/drivers/net/dsa/hirschmann/hellcreek.c
> index 595a548bb0a8..af50001ccdd4 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -1885,13 +1885,17 @@ static int hellcreek_port_setup_tc(struct
> dsa_switch *ds, int port,
> case TC_SETUP_QDISC_TAPRIO: {
> struct tc_taprio_qopt_offload *taprio = type_data;
>
> - if (!hellcreek_validate_schedule(hellcreek, taprio))
> - return -EOPNOTSUPP;
> + switch (taprio->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + if (!hellcreek_validate_schedule(hellcreek, taprio))
> + return -EOPNOTSUPP;
>
> - if (taprio->enable)
> return hellcreek_port_set_schedule(ds, port, taprio);
> -
> - return hellcreek_port_del_schedule(ds, port);
> + case TAPRIO_CMD_DESTROY:
> + return hellcreek_port_del_schedule(ds, port);
> + default:
> + return -EOPNOTSUPP;
> + }
> }
> default:
> return -EOPNOTSUPP;
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c
> b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 030738fef60e..5de6a27052fc 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -1411,7 +1411,7 @@ static int vsc9959_qos_port_tas_set(struct ocelot
> *ocelot, int port,
>
> mutex_lock(&ocelot->tas_lock);
>
> - if (!taprio->enable) {
> + if (taprio->cmd == TAPRIO_CMD_DESTROY) {
> ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
> ocelot_rmw_rix(ocelot, 0, QSYS_TAG_CONFIG_ENABLE,
> QSYS_TAG_CONFIG, port);
> @@ -1423,6 +1423,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot
> *ocelot, int port,
>
> mutex_unlock(&ocelot->tas_lock);
> return 0;
> + } else if (taprio->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> ret = ocelot_port_mqprio(ocelot, port, &taprio->mqprio); diff --git
> a/drivers/net/dsa/sja1105/sja1105_tas.c
> b/drivers/net/dsa/sja1105/sja1105_tas.c
> index e6153848a950..d7818710bc02 100644
> --- a/drivers/net/dsa/sja1105/sja1105_tas.c
> +++ b/drivers/net/dsa/sja1105/sja1105_tas.c
> @@ -516,10 +516,11 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds,
> int port,
> /* Can't change an already configured port (must delete qdisc first).
> * Can't delete the qdisc from an unconfigured port.
> */
> - if (!!tas_data->offload[port] == admin->enable)
> + if ((!!tas_data->offload[port] && admin->cmd ==
> TAPRIO_CMD_REPLACE) ||
> + (!tas_data->offload[port] && admin->cmd ==
> TAPRIO_CMD_DESTROY))
> return -EINVAL;
>
> - if (!admin->enable) {
> + if (admin->cmd == TAPRIO_CMD_DESTROY) {
> taprio_offload_free(tas_data->offload[port]);
> tas_data->offload[port] = NULL;
>
> @@ -528,6 +529,8 @@ int sja1105_setup_tc_taprio(struct dsa_switch *ds, int
> port,
> return rc;
>
> return sja1105_static_config_reload(priv,
> SJA1105_SCHEDULING);
> + } else if (admin->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> /* The cycle time extension is the amount of time the last cycle from diff
> --git a/drivers/net/ethernet/engleder/tsnep_selftests.c
> b/drivers/net/ethernet/engleder/tsnep_selftests.c
> index 1581d6b22232..8a9145f93147 100644
> --- a/drivers/net/ethernet/engleder/tsnep_selftests.c
> +++ b/drivers/net/ethernet/engleder/tsnep_selftests.c
> @@ -329,7 +329,7 @@ static bool disable_taprio(struct tsnep_adapter
> *adapter)
> int retval;
>
> memset(&qopt, 0, sizeof(qopt));
> - qopt.enable = 0;
> + qopt.cmd = TAPRIO_CMD_DESTROY;
> retval = tsnep_tc_setup(adapter->netdev, TC_SETUP_QDISC_TAPRIO,
> &qopt);
> if (retval)
> return false;
> @@ -360,7 +360,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter
> *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 1500000;
> qopt->cycle_time_extension = 0;
> @@ -382,7 +382,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter
> *adapter)
> if (!run_taprio(adapter, qopt, 100))
> goto failed;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 411854;
> qopt->cycle_time_extension = 0;
> @@ -406,7 +406,7 @@ static bool tsnep_test_taprio(struct tsnep_adapter
> *adapter)
> if (!run_taprio(adapter, qopt, 100))
> goto failed;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> delay_base_time(adapter, qopt, 12);
> qopt->cycle_time = 125000;
> @@ -457,7 +457,7 @@ static bool tsnep_test_taprio_change(struct
> tsnep_adapter *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 100000;
> qopt->cycle_time_extension = 0;
> @@ -610,7 +610,7 @@ static bool tsnep_test_taprio_extension(struct
> tsnep_adapter *adapter)
> for (i = 0; i < 255; i++)
> qopt->entries[i].command = TC_TAPRIO_CMD_SET_GATES;
>
> - qopt->enable = 1;
> + qopt->cmd = TAPRIO_CMD_REPLACE;
> qopt->base_time = ktime_set(0, 0);
> qopt->cycle_time = 100000;
> qopt->cycle_time_extension = 50000;
> diff --git a/drivers/net/ethernet/engleder/tsnep_tc.c
> b/drivers/net/ethernet/engleder/tsnep_tc.c
> index d083e6684f12..745b191a5540 100644
> --- a/drivers/net/ethernet/engleder/tsnep_tc.c
> +++ b/drivers/net/ethernet/engleder/tsnep_tc.c
> @@ -325,7 +325,7 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
> if (!adapter->gate_control)
> return -EOPNOTSUPP;
>
> - if (!qopt->enable) {
> + if (qopt->cmd == TAPRIO_CMD_DESTROY) {
> /* disable gate control if active */
> mutex_lock(&adapter->gate_control_lock);
>
> @@ -337,6 +337,8 @@ static int tsnep_taprio(struct tsnep_adapter *adapter,
> mutex_unlock(&adapter->gate_control_lock);
>
> return 0;
> + } else if (qopt->cmd != TAPRIO_CMD_REPLACE) {
> + return -EOPNOTSUPP;
> }
>
> retval = tsnep_validate_gcl(qopt);
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> index 83c27bbbc6ed..7aad824f4da7 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> @@ -65,7 +65,7 @@ static int enetc_setup_taprio(struct net_device *ndev,
> gcl_len = admin_conf->num_entries;
>
> tge = enetc_rd(hw, ENETC_PTGCR);
> - if (!admin_conf->enable) {
> + if (admin_conf->cmd == TAPRIO_CMD_DESTROY) {
> enetc_wr(hw, ENETC_PTGCR, tge & ~ENETC_PTGCR_TGE);
> enetc_reset_ptcmsdur(hw);
>
> @@ -138,6 +138,10 @@ int enetc_setup_tc_taprio(struct net_device *ndev,
> void *type_data)
> struct enetc_ndev_priv *priv = netdev_priv(ndev);
> int err, i;
>
> + if (taprio->cmd != TAPRIO_CMD_REPLACE &&
> + taprio->cmd != TAPRIO_CMD_DESTROY)
> + return -EOPNOTSUPP;
> +
> /* TSD and Qbv are mutually exclusive in hardware */
> for (i = 0; i < priv->num_tx_rings; i++)
> if (priv->tx_ring[i]->tsd_enable)
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index c5ef1edcf548..88145c30c919 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6113,9 +6113,18 @@ static int igc_save_qbv_schedule(struct
> igc_adapter *adapter,
> size_t n;
> int i;
>
> - adapter->qbv_enable = qopt->enable;
> + switch (qopt->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + adapter->qbv_enable = true;
> + break;
> + case TAPRIO_CMD_DESTROY:
> + adapter->qbv_enable = false;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
>
> - if (!qopt->enable)
> + if (!adapter->qbv_enable)
> return igc_tsn_clear_schedule(adapter);
>
> if (qopt->base_time < 0)
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> index cf0cc7562d04..ee652f2d2359 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc.c
> @@ -21,8 +21,14 @@ static int lan966x_tc_setup_qdisc_mqprio(struct
> lan966x_port *port, static int lan966x_tc_setup_qdisc_taprio(struct
> lan966x_port *port,
> struct tc_taprio_qopt_offload
> *taprio) {
> - return taprio->enable ? lan966x_taprio_add(port, taprio) :
> - lan966x_taprio_del(port);
> + switch (taprio->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + return lan966x_taprio_add(port, taprio);
> + case TAPRIO_CMD_DESTROY:
> + return lan966x_taprio_del(port);
> + default:
> + return -EOPNOTSUPP;
> + }
> }
>
> static int lan966x_tc_setup_qdisc_tbf(struct lan966x_port *port, diff --git
> a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 9d55226479b4..ac41ef4cbd2f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -966,8 +966,11 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
> return -EOPNOTSUPP;
> }
>
> - if (!qopt->enable)
> + if (qopt->cmd == TAPRIO_CMD_DESTROY)
> goto disable;
> + else if (qopt->cmd != TAPRIO_CMD_REPLACE)
> + return -EOPNOTSUPP;
> +
> if (qopt->num_entries >= dep)
> return -EINVAL;
> if (!qopt->cycle_time)
> @@ -988,7 +991,7 @@ static int tc_setup_taprio(struct stmmac_priv *priv,
>
> mutex_lock(&priv->plat->est->lock);
> priv->plat->est->gcl_size = size;
> - priv->plat->est->enable = qopt->enable;
> + priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE;
> mutex_unlock(&priv->plat->est->lock);
>
> for (i = 0; i < size; i++) {
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c
> b/drivers/net/ethernet/ti/am65-cpsw-qos.c
> index 3a908db6e5b2..eced87fa261c 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
> @@ -450,7 +450,7 @@ static int am65_cpsw_configure_taprio(struct
> net_device *ndev,
>
> am65_cpsw_est_update_state(ndev);
>
> - if (!est_new->taprio.enable) {
> + if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
> am65_cpsw_stop_est(ndev);
> return ret;
> }
> @@ -476,7 +476,7 @@ static int am65_cpsw_configure_taprio(struct
> net_device *ndev,
> am65_cpsw_est_set_sched_list(ndev, est_new);
> am65_cpsw_port_est_assign_buf_num(ndev, est_new->buf);
>
> - am65_cpsw_est_set(ndev, est_new->taprio.enable);
> + am65_cpsw_est_set(ndev, est_new->taprio.cmd ==
> TAPRIO_CMD_REPLACE);
>
> if (tact == TACT_PROG) {
> ret = am65_cpsw_timer_set(ndev, est_new); @@ -520,7
> +520,7 @@ static int am65_cpsw_set_taprio(struct net_device *ndev, void
> *type_data)
> am65_cpsw_cp_taprio(taprio, &est_new->taprio);
> ret = am65_cpsw_configure_taprio(ndev, est_new);
> if (!ret) {
> - if (taprio->enable) {
> + if (taprio->cmd == TAPRIO_CMD_REPLACE) {
> devm_kfree(&ndev->dev, port->qos.est_admin);
>
> port->qos.est_admin = est_new;
> @@ -564,8 +564,13 @@ static void am65_cpsw_est_link_up(struct net_device
> *ndev, int link_speed) static int am65_cpsw_setup_taprio(struct net_device
> *ndev, void *type_data) {
> struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> + struct tc_taprio_qopt_offload *taprio = type_data;
> struct am65_cpsw_common *common = port->common;
>
> + if (taprio->cmd != TAPRIO_CMD_REPLACE &&
> + taprio->cmd != TAPRIO_CMD_DESTROY)
> + return -EOPNOTSUPP;
> +
> if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
> return -ENODEV;
>
> diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h index
> f436688b6efc..f5fb11da357b 100644
> --- a/include/net/pkt_sched.h
> +++ b/include/net/pkt_sched.h
> @@ -185,6 +185,11 @@ struct tc_taprio_caps {
> bool broken_mqprio:1;
> };
>
> +enum tc_taprio_qopt_cmd {
> + TAPRIO_CMD_REPLACE,
> + TAPRIO_CMD_DESTROY,
> +};
> +
> struct tc_taprio_sched_entry {
> u8 command; /* TC_TAPRIO_CMD_* */
>
> @@ -196,7 +201,7 @@ struct tc_taprio_sched_entry { struct
> tc_taprio_qopt_offload {
> struct tc_mqprio_qopt_offload mqprio;
> struct netlink_ext_ack *extack;
> - u8 enable;
> + enum tc_taprio_qopt_cmd cmd;
> ktime_t base_time;
> u64 cycle_time;
> u64 cycle_time_extension;
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index
> d29e6785854d..06bf4c6355a5 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -1524,7 +1524,7 @@ static int taprio_enable_offload(struct net_device
> *dev,
> "Not enough memory for enabling offload mode");
> return -ENOMEM;
> }
> - offload->enable = 1;
> + offload->cmd = TAPRIO_CMD_REPLACE;
> offload->extack = extack;
> mqprio_qopt_reconstruct(dev, &offload->mqprio.qopt);
> offload->mqprio.extack = extack;
> @@ -1572,7 +1572,7 @@ static int taprio_disable_offload(struct net_device
> *dev,
> "Not enough memory to disable offload mode");
> return -ENOMEM;
> }
> - offload->enable = 0;
> + offload->cmd = TAPRIO_CMD_DESTROY;
>
> err = ops->ndo_setup_tc(dev, TC_SETUP_QDISC_TAPRIO, offload);
> if (err < 0) {
> --
> 2.34.1
On 30.05.23 11:19, Vladimir Oltean wrote:
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
> able to report statistics (which is future work), it seems that we need
> to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload,
> refactor all drivers to check for "cmd" instead of "enable", and reject
> every other command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> drivers/net/dsa/hirschmann/hellcreek.c | 14 +++++++++-----
> drivers/net/dsa/ocelot/felix_vsc9959.c | 4 +++-
> drivers/net/dsa/sja1105/sja1105_tas.c | 7 +++++--
> drivers/net/ethernet/engleder/tsnep_selftests.c | 12 ++++++------
> drivers/net/ethernet/engleder/tsnep_tc.c | 4 +++-
> drivers/net/ethernet/freescale/enetc/enetc_qos.c | 6 +++++-
> drivers/net/ethernet/intel/igc/igc_main.c | 13 +++++++++++--
> .../net/ethernet/microchip/lan966x/lan966x_tc.c | 10 ++++++++--
> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 7 +++++--
> drivers/net/ethernet/ti/am65-cpsw-qos.c | 11 ++++++++---
> include/net/pkt_sched.h | 7 ++++++-
> net/sched/sch_taprio.c | 4 ++--
> 12 files changed, 71 insertions(+), 28 deletions(-)
Reviewed-by: Gerhard Engleder <[email protected]>
On Wed, May 31, 2023 at 07:08:10PM +0200, Simon Horman wrote:
> > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
>
> ...
>
> > @@ -1423,6 +1423,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> >
> > mutex_unlock(&ocelot->tas_lock);
> > return 0;
> > + } else if (taprio->cmd != TAPRIO_CMD_REPLACE) {
>
> Hi Vladimir,
>
> Do you need to 'mutex_unlock(&ocelot->tas_lock)' here?
Yup. Nice catch, thanks. I reviewed this a couple of times myself but I
missed it. I'll send a fixup patch later today, hopefully.
> > + return -EOPNOTSUPP;
> > }
> >
> > ret = ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
>
> ...
On Tue, May 30, 2023 at 12:19:45PM +0300, Vladimir Oltean wrote:
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
> able to report statistics (which is future work), it seems that we need
> to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload,
> refactor all drivers to check for "cmd" instead of "enable", and reject
> every other command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
...
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
...
> @@ -1423,6 +1423,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
>
> mutex_unlock(&ocelot->tas_lock);
> return 0;
> + } else if (taprio->cmd != TAPRIO_CMD_REPLACE) {
Hi Vladimir,
Do you need to 'mutex_unlock(&ocelot->tas_lock)' here?
> + return -EOPNOTSUPP;
> }
>
> ret = ocelot_port_mqprio(ocelot, port, &taprio->mqprio);
...