| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 16 Jun 2022 10:05:04 +0300
From: Maxim Kochetkov <fido_max@...ox.ru>
To: Xiaoliang Yang <xiaoliang.yang_1@....com>, davem@...emloft.net,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: allan.nielsen@...rochip.com, joergen.andreasen@...rochip.com,
horatiu.vultur@...rochip.com, UNGLinuxDriver@...rochip.com,
alexandre.belloni@...tlin.com, f.fainelli@...il.com,
vivien.didelot@...il.com, pabeni@...hat.com, kuba@...nel.org,
edumazet@...gle.com, andrew@...n.ch, alexandru.marginean@....com,
claudiu.manoil@....com, vladimir.oltean@....com,
leoyang.li@....com, yangbo.lu@....com
Subject: Re: [PATCH v1 net-next] net: dsa: felix: update base time of
time-aware shaper when adjusting PTP time
On 16.06.2022 09:46, Xiaoliang Yang wrote:
> When adjusting the PTP clock, the base time of the TAS configuration
> will become unreliable. We need reset the TAS configuration by using a
> new base time.
>
> For example, if the driver gets a base time 0 of Qbv configuration from
> user, and current time is 20000. The driver will set the TAS base time
> to be 20000. After the PTP clock adjustment, the current time becomes
> 10000. If the TAS base time is still 20000, it will be a future time,
> and TAS entry list will stop running. Another example, if the current
> time becomes to be 10000000 after PTP clock adjust, a large time offset
> can cause the hardware to hang.
>
> This patch introduces a tas_clock_adjust() function to reset the TAS
> module by using a new base time after the PTP clock adjustment. This can
> avoid issues above.
>
> Due to PTP clock adjustment can occur at any time, it may conflict with
> the TAS configuration. We introduce a new TAS lock to serialize the
> access to the TAS registers.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@....com>
> ---
> drivers/net/dsa/ocelot/felix_vsc9959.c | 83 ++++++++++++++++++++++++--
> drivers/net/ethernet/mscc/ocelot.c | 1 +
> drivers/net/ethernet/mscc/ocelot_ptp.c | 12 +++-
> include/soc/mscc/ocelot.h | 7 +++
> 4 files changed, 95 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 570d0204b7be..dd9085ae0922 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -1196,10 +1196,13 @@ static void vsc9959_tas_gcl_set(struct ocelot *ocelot, const u32 gcl_ix,
> static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> struct tc_taprio_qopt_offload *taprio)
> {
> + struct ocelot_port *ocelot_port = ocelot->ports[port];
> struct timespec64 base_ts;
> int ret, i;
> u32 val;
>
> + mutex_lock(&ocelot->tas_lock);
> +
> if (!taprio->enable) {
> ocelot_rmw_rix(ocelot,
> QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> @@ -1207,15 +1210,20 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> QSYS_TAG_CONFIG, port);
>
> + mutex_unlock(&ocelot->tas_lock);
> return 0;
> }
>
> if (taprio->cycle_time > NSEC_PER_SEC ||
> - taprio->cycle_time_extension >= NSEC_PER_SEC)
> - return -EINVAL;
> + taprio->cycle_time_extension >= NSEC_PER_SEC) {
> + ret = -EINVAL;
> + goto err;
> + }
>
> - if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX)
> - return -ERANGE;
> + if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX) {
> + ret = -ERANGE;
> + goto err;
> + }
>
> /* Enable guard band. The switch will schedule frames without taking
> * their length into account. Thus we'll always need to enable the
> @@ -1236,8 +1244,10 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> * config is pending, need reset the TAS module
> */
> val = ocelot_read(ocelot, QSYS_PARAM_STATUS_REG_8);
> - if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING)
> - return -EBUSY;
> + if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING) {
> + ret = -EBUSY;
> + goto err;
> + }
>
> ocelot_rmw_rix(ocelot,
> QSYS_TAG_CONFIG_ENABLE |
> @@ -1248,6 +1258,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES_M,
> QSYS_TAG_CONFIG, port);
>
> + ocelot_port->base_time = taprio->base_time;
> +
> vsc9959_new_base_time(ocelot, taprio->base_time,
> taprio->cycle_time, &base_ts);
> ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
> @@ -1271,9 +1283,67 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
> !(val & QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
> 10, 100000);
>
> +err:
> + mutex_unlock(&ocelot->tas_lock);
> +
> return ret;
> }
>
> +static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
> +{
> + struct ocelot_port *ocelot_port;
> + struct timespec64 base_ts;
> + u64 cycletime;
> + int port;
> + u32 val;
> +
> + mutex_lock(&ocelot->tas_lock);
> +
> + for (port = 0; port < ocelot->num_phys_ports; port++) {
> + val = ocelot_read_rix(ocelot, QSYS_TAG_CONFIG, port);
> + if (!(val & QSYS_TAG_CONFIG_ENABLE))
> + continue;
> +
> + ocelot_rmw(ocelot,
> + QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM(port),
> + QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM_M,
> + QSYS_TAS_PARAM_CFG_CTRL);
> +
> + ocelot_rmw_rix(ocelot,
> + QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
> + QSYS_TAG_CONFIG_ENABLE |
> + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> + QSYS_TAG_CONFIG, port);
> +
> + cycletime = ocelot_read(ocelot, QSYS_PARAM_CFG_REG_4);
> + ocelot_port = ocelot->ports[port];
> +
> + vsc9959_new_base_time(ocelot, ocelot_port->base_time,
> + cycletime, &base_ts);
> +
> + ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
> + ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec),
> + QSYS_PARAM_CFG_REG_2);
> + val = upper_32_bits(base_ts.tv_sec);
> + ocelot_rmw(ocelot,
> + QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB(val),
> + QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB_M,
> + QSYS_PARAM_CFG_REG_3);
> +
> + ocelot_rmw(ocelot, QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> + QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
> + QSYS_TAS_PARAM_CFG_CTRL);
> +
> + ocelot_rmw_rix(ocelot,
> + QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) |
> + QSYS_TAG_CONFIG_ENABLE,
> + QSYS_TAG_CONFIG_ENABLE |
> + QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
> + QSYS_TAG_CONFIG, port);
> + }
> + mutex_unlock(&ocelot->tas_lock);
> +}
> +
> static int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> struct tc_cbs_qopt_offload *cbs_qopt)
> {
> @@ -2210,6 +2280,7 @@ static const struct ocelot_ops vsc9959_ops = {
> .psfp_filter_del = vsc9959_psfp_filter_del,
> .psfp_stats_get = vsc9959_psfp_stats_get,
> .cut_through_fwd = vsc9959_cut_through_fwd,
> + .tas_clock_adjust = vsc9959_tas_clock_adjust,
> };
>
> static const struct felix_info felix_info_vsc9959 = {
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
> index 8da7e25a47c9..d4649e4ee0e7 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -3367,6 +3367,7 @@ int ocelot_init(struct ocelot *ocelot)
> mutex_init(&ocelot->ptp_lock);
> mutex_init(&ocelot->mact_lock);
> mutex_init(&ocelot->fwd_domain_lock);
> + mutex_init(&ocelot->tas_lock);
> spin_lock_init(&ocelot->ptp_clock_lock);
> spin_lock_init(&ocelot->ts_id_lock);
> snprintf(queue_name, sizeof(queue_name), "%s-stats",
> diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
> index 87ad2137ba06..522fdc38d4d0 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ptp.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
> @@ -72,15 +72,19 @@ int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
>
> spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +
> + if (ocelot->ops->tas_clock_adjust)
> + ocelot->ops->tas_clock_adjust(ocelot);
> +
> return 0;
> }
> EXPORT_SYMBOL(ocelot_ptp_settime64);
>
> int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> {
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +
> if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> - struct ocelot *ocelot = container_of(ptp, struct ocelot,
> - ptp_info);
> unsigned long flags;
> u32 val;
>
> @@ -117,6 +121,10 @@ int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>
> ocelot_ptp_settime64(ptp, &ts);
> }
> +
> + if (ocelot->ops->tas_clock_adjust)
> + ocelot->ops->tas_clock_adjust(ocelot);
> +
Here tas_clock_adjust() may be called twice (first from
ocelot_ptp_settime64() )
> return 0;
> }
> EXPORT_SYMBOL(ocelot_ptp_adjtime);
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index 5f88385a7748..3737570116c3 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -575,6 +575,7 @@ struct ocelot_ops {
> int (*psfp_stats_get)(struct ocelot *ocelot, struct flow_cls_offload *f,
> struct flow_stats *stats);
> void (*cut_through_fwd)(struct ocelot *ocelot);
> + void (*tas_clock_adjust)(struct ocelot *ocelot);
> };
>
> struct ocelot_vcap_policer {
> @@ -691,6 +692,9 @@ struct ocelot_port {
> int bridge_num;
>
> int speed;
> +
> + /* Store the AdminBaseTime of EST fetched from userspace. */
> + s64 base_time;
> };
>
> struct ocelot {
> @@ -757,6 +761,9 @@ struct ocelot {
> /* Lock for serializing forwarding domain changes */
> struct mutex fwd_domain_lock;
>
> + /* Lock for serializing Time-Aware Shaper changes */
> + struct mutex tas_lock;
> +
Why do we need extra mutex? ocelot_ptp_settime64() and
ocelot_ptp_adjtime() already uses
spin_lock_irqsave(&ocelot->ptp_clock_lock, flags)
> struct workqueue_struct *owq;
>
> u8 ptp:1;
Powered by blists - more mailing lists