v1->v2:
- define PSEC_PER_NSEC in include/linux/time64.h rather than
include/vdso/time64.h
- add missing #include <linux/time.h> to users of PSEC_PER_NSEC
- move the PSEC_PER_NSEC consolidation to the last patch
Richie Pearn reports that if we install a tc-taprio schedule on a Felix
switch port, and that schedule has at least one gate that never opens
(for example TC0 below):
tc qdisc add dev swp1 root taprio num_tc 8 map 0 1 2 3 4 5 6 7 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \
base-time 0 sched-entry S fe 1000000 flags 0x2
then packets classified to the permanently closed traffic class will not
be dequeued by the egress port. They will just remain in the queue
system, to consume resources. Frame aging does not trigger either,
because in order for that to happen, the packets need to be eligible for
egress scheduling in the first place, which they aren't. If that port is
allowed to consume the entire shared buffer of the switch (as we
configure things by default using devlink-sb), then eventually, by
sending enough packets, the entire switch will hang.
If we think enough about the problem, we realize that this is only a
special case of a more general issue, and can also be reproduced with
gates that aren't permanently closed, but are not large enough to send
an entire frame. In that sense, a permanently closed gate is simply a
case where all frames are oversized.
The ENETC has logic to reject transmitted packets that would overrun the
time window - see commit 285e8dedb4bd ("net: enetc: count the tc-taprio
window drops").
The Felix switch has no such thing on a per-packet basis, but it has a
register replicated per {egress port, TC} which essentially limits the
max MTU. A packet which exceeds the per-port-TC MTU is immediately
discarded and therefore will not hang the port anymore (albeit, sadly,
this only bumps a generic drop hardware counter and we cannot really
infer the reason such as to offer a dedicated counter for these events).
This patch set calculates the max MTU per {port, TC} when the tc-taprio
config, or link speed, or port-global MTU values change. This solves the
larger "gate too small for packet" problem, but also the original issue
with the gate permanently closed that was reported by Richie.
Q: Bug fix patch sent to net-next?
A: Yeah, after Xiaoliang started sending bug fixes to net-next himself
(see https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/)
there is absolutely no gain in targeting "net" here - I am modifying
the same areas of code, that have already diverged from "net", 5.19
and earlier. So this is why I am also taking the opportunity to
introduce cleanup patches, to leave things as clean as possible after
the rework. I'd be interested if there is a better approach to this.
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vincenzo Frascino <[email protected]>
Vladimir Oltean (4):
net: dsa: felix: keep reference on entire tc-taprio config
net: dsa: felix: keep QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) out of rmw
net: dsa: felix: drop oversized frames with tc-taprio instead of
hanging the port
time64.h: consolidate uses of PSEC_PER_NSEC
drivers/net/dsa/ocelot/felix.c | 9 +
drivers/net/dsa/ocelot/felix.h | 1 +
drivers/net/dsa/ocelot/felix_vsc9959.c | 242 ++++++++++++++++++++++---
include/linux/time64.h | 3 +
include/soc/mscc/ocelot.h | 5 +-
net/sched/sch_taprio.c | 5 +-
6 files changed, 238 insertions(+), 27 deletions(-)
--
2.25.1
Currently, sending a packet into a time gate too small for it (or always
closed) causes the queue system to hold the frame forever. Even worse,
this frame isn't subject to aging either, because for that to happen, it
needs to be scheduled for transmission in the first place. But the frame
will consume buffer memory and frame references while it is forever held
in the queue system.
Before commit a4ae997adcbd ("net: mscc: ocelot: initialize watermarks to
sane defaults"), this behavior was somewhat subtle, as the switch had a
more intricately tuned default watermark configuration out of reset,
which did not allow any single port and tc to consume the entire switch
buffer space. Nonetheless, the held frames are still there, and they
reduce the total backplane capacity of the switch.
However, after the aforementioned commit, the behavior can be very
clearly seen, since we deliberately allow each {port, tc} to consume the
entire shared buffer of the switch minus the reservations (and we
disable all reservations by default). That is to say, we allow a
permanently closed tc-taprio gate to hang the entire switch.
A careful inspection of the documentation shows that the QSYS:Q_MAX_SDU
per-port-tc registers serve 2 purposes: one is for guard band calculation
(when zero, this falls back to QSYS:PORT_MAX_SDU), and the other is to
enable oversized frame dropping (when non-zero).
Currently the QSYS:Q_MAX_SDU registers are all zero, so oversized frame
dropping is disabled. The goal of the change is to enable it seamlessly.
For that, we need to hook into the MTU change, tc-taprio change, and
port link speed change procedures, since we depend on these variables.
Frames are not dropped on egress due to a queue system oversize
condition, instead that egress port is simply excluded from the mask of
valid destination ports for the packet. If there are no destination
ports at all, the ingress counter that increments is the generic
"drop_tail" in ethtool -S.
The issue exists in various forms since the tc-taprio offload was introduced.
Fixes: de143c0e274b ("net: dsa: felix: Configure Time-Aware Scheduler via taprio offload")
Reported-by: Richie Pearn <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2:
- reword commit message regarding "drop_tail" counter
- hardcode 1000 rather than PSEC_PER_NSEC to eliminate dependency on
include/linux/time64.h change (to be fixed up later)
drivers/net/dsa/ocelot/felix.c | 9 ++
drivers/net/dsa/ocelot/felix.h | 1 +
drivers/net/dsa/ocelot/felix_vsc9959.c | 201 +++++++++++++++++++++++++
3 files changed, 211 insertions(+)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 3e07dc39007a..859196898a7d 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1553,9 +1553,18 @@ static void felix_txtstamp(struct dsa_switch *ds, int port,
static int felix_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
{
struct ocelot *ocelot = ds->priv;
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
+ struct felix *felix = ocelot_to_felix(ocelot);
ocelot_port_set_maxlen(ocelot, port, new_mtu);
+ mutex_lock(&ocelot->tas_lock);
+
+ if (ocelot_port->taprio && felix->info->tas_guard_bands_update)
+ felix->info->tas_guard_bands_update(ocelot, port);
+
+ mutex_unlock(&ocelot->tas_lock);
+
return 0;
}
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 9e07eb7ee28d..deb8dde1fc19 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -53,6 +53,7 @@ struct felix_info {
struct phylink_link_state *state);
int (*port_setup_tc)(struct dsa_switch *ds, int port,
enum tc_setup_type type, void *type_data);
+ void (*tas_guard_bands_update)(struct ocelot *ocelot, int port);
void (*port_sched_speed_set)(struct ocelot *ocelot, int port,
u32 speed);
struct regmap *(*init_regmap)(struct ocelot *ocelot,
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 7573254274b3..27d8b56cc21c 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1127,9 +1127,199 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
mdiobus_free(felix->imdio);
}
+/* Extract shortest continuous gate open intervals in ns for each traffic class
+ * of a cyclic tc-taprio schedule. If a gate is always open, the duration is
+ * considered U64_MAX. If the gate is always closed, it is considered 0.
+ */
+static void vsc9959_tas_min_gate_lengths(struct tc_taprio_qopt_offload *taprio,
+ u64 min_gate_len[OCELOT_NUM_TC])
+{
+ struct tc_taprio_sched_entry *entry;
+ u64 gate_len[OCELOT_NUM_TC];
+ int tc, i, n;
+
+ /* Initialize arrays */
+ for (tc = 0; tc < OCELOT_NUM_TC; tc++) {
+ min_gate_len[tc] = U64_MAX;
+ gate_len[tc] = 0;
+ }
+
+ /* If we don't have taprio, consider all gates as permanently open */
+ if (!taprio)
+ return;
+
+ n = taprio->num_entries;
+
+ /* Walk through the gate list twice to determine the length
+ * of consecutively open gates for a traffic class, including
+ * open gates that wrap around. We are just interested in the
+ * minimum window size, and this doesn't change what the
+ * minimum is (if the gate never closes, min_gate_len will
+ * remain U64_MAX).
+ */
+ for (i = 0; i < 2 * n; i++) {
+ entry = &taprio->entries[i % n];
+
+ for (tc = 0; tc < OCELOT_NUM_TC; tc++) {
+ if (entry->gate_mask & BIT(tc)) {
+ gate_len[tc] += entry->interval;
+ } else {
+ /* Gate closes now, record a potential new
+ * minimum and reinitialize length
+ */
+ if (min_gate_len[tc] > gate_len[tc])
+ min_gate_len[tc] = gate_len[tc];
+ gate_len[tc] = 0;
+ }
+ }
+ }
+}
+
+/* Update QSYS_PORT_MAX_SDU to make sure the static guard bands added by the
+ * switch (see the ALWAYS_GUARD_BAND_SCH_Q comment) are correct at all MTU
+ * values (the default value is 1518). Also, for traffic class windows smaller
+ * than one MTU sized frame, update QSYS_QMAXSDU_CFG to enable oversized frame
+ * dropping, such that these won't hang the port, as they will never be sent.
+ */
+static void vsc9959_tas_guard_bands_update(struct ocelot *ocelot, int port)
+{
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
+ u64 min_gate_len[OCELOT_NUM_TC];
+ int speed, picos_per_byte;
+ u64 needed_bit_time_ps;
+ u32 val, maxlen;
+ u8 tas_speed;
+ int tc;
+
+ lockdep_assert_held(&ocelot->tas_lock);
+
+ val = ocelot_read_rix(ocelot, QSYS_TAG_CONFIG, port);
+ tas_speed = QSYS_TAG_CONFIG_LINK_SPEED_X(val);
+
+ switch (tas_speed) {
+ case OCELOT_SPEED_10:
+ speed = SPEED_10;
+ break;
+ case OCELOT_SPEED_100:
+ speed = SPEED_100;
+ break;
+ case OCELOT_SPEED_1000:
+ speed = SPEED_1000;
+ break;
+ case OCELOT_SPEED_2500:
+ speed = SPEED_2500;
+ break;
+ default:
+ return;
+ }
+
+ picos_per_byte = (USEC_PER_SEC * 8) / speed;
+
+ val = ocelot_port_readl(ocelot_port, DEV_MAC_MAXLEN_CFG);
+ /* MAXLEN_CFG accounts automatically for VLAN. We need to include it
+ * manually in the bit time calculation, plus the preamble and SFD.
+ */
+ maxlen = val + 2 * VLAN_HLEN;
+ /* Consider the standard Ethernet overhead of 8 octets preamble+SFD,
+ * 4 octets FCS, 12 octets IFG.
+ */
+ needed_bit_time_ps = (maxlen + 24) * picos_per_byte;
+
+ dev_dbg(ocelot->dev,
+ "port %d: max frame size %d needs %llu ps at speed %d\n",
+ port, maxlen, needed_bit_time_ps, speed);
+
+ vsc9959_tas_min_gate_lengths(ocelot_port->taprio, min_gate_len);
+
+ for (tc = 0; tc < OCELOT_NUM_TC; tc++) {
+ u32 max_sdu;
+
+ if (min_gate_len[tc] == U64_MAX /* Gate always open */ ||
+ min_gate_len[tc] * 1000 > needed_bit_time_ps) {
+ /* Setting QMAXSDU_CFG to 0 disables oversized frame
+ * dropping.
+ */
+ max_sdu = 0;
+ dev_dbg(ocelot->dev,
+ "port %d tc %d min gate len %llu"
+ ", sending all frames\n",
+ port, tc, min_gate_len[tc]);
+ } else {
+ /* If traffic class doesn't support a full MTU sized
+ * frame, make sure to enable oversize frame dropping
+ * for frames larger than the smallest that would fit.
+ */
+ max_sdu = div_u64(min_gate_len[tc] * 1000,
+ picos_per_byte);
+ /* A TC gate may be completely closed, which is a
+ * special case where all packets are oversized.
+ * Any limit smaller than 64 octets accomplishes this
+ */
+ if (!max_sdu)
+ max_sdu = 1;
+ /* Take L1 overhead into account, but just don't allow
+ * max_sdu to go negative or to 0. Here we use 20
+ * because QSYS_MAXSDU_CFG_* already counts the 4 FCS
+ * octets as part of packet size.
+ */
+ if (max_sdu > 20)
+ max_sdu -= 20;
+ dev_info(ocelot->dev,
+ "port %d tc %d min gate length %llu"
+ " ns not enough for max frame size %d at %d"
+ " Mbps, dropping frames over %d"
+ " octets including FCS\n",
+ port, tc, min_gate_len[tc], maxlen, speed,
+ max_sdu);
+ }
+
+ /* ocelot_write_rix is a macro that concatenates
+ * QSYS_MAXSDU_CFG_* with _RSZ, so we need to spell out
+ * the writes to each traffic class
+ */
+ switch (tc) {
+ case 0:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_0,
+ port);
+ break;
+ case 1:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_1,
+ port);
+ break;
+ case 2:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_2,
+ port);
+ break;
+ case 3:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_3,
+ port);
+ break;
+ case 4:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_4,
+ port);
+ break;
+ case 5:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_5,
+ port);
+ break;
+ case 6:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_6,
+ port);
+ break;
+ case 7:
+ ocelot_write_rix(ocelot, max_sdu, QSYS_QMAXSDU_CFG_7,
+ port);
+ break;
+ }
+ }
+
+ ocelot_write_rix(ocelot, maxlen, QSYS_PORT_MAX_SDU, port);
+}
+
static void vsc9959_sched_speed_set(struct ocelot *ocelot, int port,
u32 speed)
{
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
u8 tas_speed;
switch (speed) {
@@ -1154,6 +1344,13 @@ static void vsc9959_sched_speed_set(struct ocelot *ocelot, int port,
QSYS_TAG_CONFIG_LINK_SPEED(tas_speed),
QSYS_TAG_CONFIG_LINK_SPEED_M,
QSYS_TAG_CONFIG, port);
+
+ mutex_lock(&ocelot->tas_lock);
+
+ if (ocelot_port->taprio)
+ vsc9959_tas_guard_bands_update(ocelot, port);
+
+ mutex_unlock(&ocelot->tas_lock);
}
static void vsc9959_new_base_time(struct ocelot *ocelot, ktime_t base_time,
@@ -1210,6 +1407,8 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
taprio_offload_free(ocelot_port->taprio);
ocelot_port->taprio = NULL;
+ vsc9959_tas_guard_bands_update(ocelot, port);
+
mutex_unlock(&ocelot->tas_lock);
return 0;
}
@@ -1284,6 +1483,7 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
goto err;
ocelot_port->taprio = taprio_offload_get(taprio);
+ vsc9959_tas_guard_bands_update(ocelot, port);
err:
mutex_unlock(&ocelot->tas_lock);
@@ -2303,6 +2503,7 @@ static const struct felix_info felix_info_vsc9959 = {
.port_modes = vsc9959_port_modes,
.port_setup_tc = vsc9959_port_setup_tc,
.port_sched_speed_set = vsc9959_sched_speed_set,
+ .tas_guard_bands_update = vsc9959_tas_guard_bands_update,
.init_regmap = ocelot_regmap_init,
};
--
2.25.1
Time-sensitive networking code needs to work with PTP times expressed in
nanoseconds, and with packet transmission times expressed in
picoseconds, since those would be fractional at higher than gigabit
speed when expressed in nanoseconds.
Convert the existing uses in tc-taprio and the ocelot/felix DSA driver
to a PSEC_PER_NSEC macro. This macro is placed in include/linux/time64.h
as opposed to its relatives (PSEC_PER_SEC etc) from include/vdso/time64.h
because the vDSO library does not (yet) need/use it.
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vincenzo Frascino <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2:
- move PSEC_PER_NSEC to include/linux/time64.h
- add missing include of linux/time.h
drivers/net/dsa/ocelot/felix_vsc9959.c | 5 +++--
include/linux/time64.h | 3 +++
net/sched/sch_taprio.c | 5 +++--
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 27d8b56cc21c..28bd4892c30a 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -16,6 +16,7 @@
#include <linux/iopoll.h>
#include <linux/mdio.h>
#include <linux/pci.h>
+#include <linux/time.h>
#include "felix.h"
#define VSC9959_NUM_PORTS 6
@@ -1235,7 +1236,7 @@ static void vsc9959_tas_guard_bands_update(struct ocelot *ocelot, int port)
u32 max_sdu;
if (min_gate_len[tc] == U64_MAX /* Gate always open */ ||
- min_gate_len[tc] * 1000 > needed_bit_time_ps) {
+ min_gate_len[tc] * PSEC_PER_NSEC > needed_bit_time_ps) {
/* Setting QMAXSDU_CFG to 0 disables oversized frame
* dropping.
*/
@@ -1249,7 +1250,7 @@ static void vsc9959_tas_guard_bands_update(struct ocelot *ocelot, int port)
* frame, make sure to enable oversize frame dropping
* for frames larger than the smallest that would fit.
*/
- max_sdu = div_u64(min_gate_len[tc] * 1000,
+ max_sdu = div_u64(min_gate_len[tc] * PSEC_PER_NSEC,
picos_per_byte);
/* A TC gate may be completely closed, which is a
* special case where all packets are oversized.
diff --git a/include/linux/time64.h b/include/linux/time64.h
index 81b9686a2079..2fb8232cff1d 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -20,6 +20,9 @@ struct itimerspec64 {
struct timespec64 it_value;
};
+/* Parameters used to convert the timespec values: */
+#define PSEC_PER_NSEC 1000L
+
/* Located here for timespec[64]_valid_strict */
#define TIME64_MAX ((s64)~((u64)1 << 63))
#define TIME64_MIN (-TIME64_MAX - 1)
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index b9c71a304d39..0b941dd63d26 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/spinlock.h>
#include <linux/rcupdate.h>
+#include <linux/time.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
#include <net/pkt_cls.h>
@@ -176,7 +177,7 @@ static ktime_t get_interval_end_time(struct sched_gate_list *sched,
static int length_to_duration(struct taprio_sched *q, int len)
{
- return div_u64(len * atomic64_read(&q->picos_per_byte), 1000);
+ return div_u64(len * atomic64_read(&q->picos_per_byte), PSEC_PER_NSEC);
}
/* Returns the entry corresponding to next available interval. If
@@ -551,7 +552,7 @@ static struct sk_buff *taprio_peek(struct Qdisc *sch)
static void taprio_set_budget(struct taprio_sched *q, struct sched_entry *entry)
{
atomic_set(&entry->budget,
- div64_u64((u64)entry->interval * 1000,
+ div64_u64((u64)entry->interval * PSEC_PER_NSEC,
atomic64_read(&q->picos_per_byte)));
}
--
2.25.1
In vsc9959_tas_clock_adjust(), the INIT_GATE_STATE field is not changed,
only the ENABLE field. Similarly for the disabling of the time-aware
shaper in vsc9959_qos_port_tas_set().
To reflect this, keep the QSYS_TAG_CONFIG_INIT_GATE_STATE_M mask out of
the read-modify-write procedure to make it clearer what is the intention
of the code.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: none
drivers/net/dsa/ocelot/felix_vsc9959.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 44bbbba4d528..7573254274b3 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1204,10 +1204,7 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
mutex_lock(&ocelot->tas_lock);
if (!taprio->enable) {
- ocelot_rmw_rix(ocelot,
- QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
- QSYS_TAG_CONFIG_ENABLE |
- QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
+ ocelot_rmw_rix(ocelot, 0, QSYS_TAG_CONFIG_ENABLE,
QSYS_TAG_CONFIG, port);
taprio_offload_free(ocelot_port->taprio);
@@ -1315,10 +1312,8 @@ static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
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,
+ /* Disable time-aware shaper */
+ ocelot_rmw_rix(ocelot, 0, QSYS_TAG_CONFIG_ENABLE,
QSYS_TAG_CONFIG, port);
vsc9959_new_base_time(ocelot, taprio->base_time,
@@ -1337,11 +1332,9 @@ static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
QSYS_TAS_PARAM_CFG_CTRL);
- ocelot_rmw_rix(ocelot,
- QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) |
+ /* Re-enable time-aware shaper */
+ ocelot_rmw_rix(ocelot, QSYS_TAG_CONFIG_ENABLE,
QSYS_TAG_CONFIG_ENABLE,
- QSYS_TAG_CONFIG_ENABLE |
- QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
QSYS_TAG_CONFIG, port);
}
mutex_unlock(&ocelot->tas_lock);
--
2.25.1
In a future change we will need to remember the entire tc-taprio config
on all ports rather than just the base time, so use the
taprio_offload_get() helper function to replace ocelot_port->base_time
with ocelot_port->taprio.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: none
drivers/net/dsa/ocelot/felix_vsc9959.c | 23 +++++++++++++----------
include/soc/mscc/ocelot.h | 5 ++---
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index dd9085ae0922..44bbbba4d528 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1210,6 +1210,9 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
QSYS_TAG_CONFIG, port);
+ taprio_offload_free(ocelot_port->taprio);
+ ocelot_port->taprio = NULL;
+
mutex_unlock(&ocelot->tas_lock);
return 0;
}
@@ -1258,8 +1261,6 @@ 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);
@@ -1282,6 +1283,10 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
ret = readx_poll_timeout(vsc9959_tas_read_cfg_status, ocelot, val,
!(val & QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
10, 100000);
+ if (ret)
+ goto err;
+
+ ocelot_port->taprio = taprio_offload_get(taprio);
err:
mutex_unlock(&ocelot->tas_lock);
@@ -1291,17 +1296,18 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
{
+ struct tc_taprio_qopt_offload *taprio;
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))
+ ocelot_port = ocelot->ports[port];
+ taprio = ocelot_port->taprio;
+ if (!taprio)
continue;
ocelot_rmw(ocelot,
@@ -1315,11 +1321,8 @@ static void vsc9959_tas_clock_adjust(struct ocelot *ocelot)
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);
+ 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);
ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec),
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 3737570116c3..ac151ecc7f19 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -670,6 +670,8 @@ struct ocelot_port {
/* VLAN that untagged frames are classified to, on ingress */
const struct ocelot_bridge_vlan *pvid_vlan;
+ struct tc_taprio_qopt_offload *taprio;
+
phy_interface_t phy_mode;
unsigned int ptp_skbs_in_flight;
@@ -692,9 +694,6 @@ struct ocelot_port {
int bridge_num;
int speed;
-
- /* Store the AdminBaseTime of EST fetched from userspace. */
- s64 base_time;
};
struct ocelot {
--
2.25.1
On 6/28/22 15:52, Vladimir Oltean wrote:
> Time-sensitive networking code needs to work with PTP times expressed in
> nanoseconds, and with packet transmission times expressed in
> picoseconds, since those would be fractional at higher than gigabit
> speed when expressed in nanoseconds.
>
> Convert the existing uses in tc-taprio and the ocelot/felix DSA driver
> to a PSEC_PER_NSEC macro. This macro is placed in include/linux/time64.h
> as opposed to its relatives (PSEC_PER_SEC etc) from include/vdso/time64.h
> because the vDSO library does not (yet) need/use it.
>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Vincenzo Frascino <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Vincenzo Frascino <[email protected]> # for the vDSO parts
> ---
> v1->v2:
> - move PSEC_PER_NSEC to include/linux/time64.h
> - add missing include of linux/time.h
>
> drivers/net/dsa/ocelot/felix_vsc9959.c | 5 +++--
> include/linux/time64.h | 3 +++
> net/sched/sch_taprio.c | 5 +++--
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 27d8b56cc21c..28bd4892c30a 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -16,6 +16,7 @@
> #include <linux/iopoll.h>
> #include <linux/mdio.h>
> #include <linux/pci.h>
> +#include <linux/time.h>
> #include "felix.h"
>
> #define VSC9959_NUM_PORTS 6
> @@ -1235,7 +1236,7 @@ static void vsc9959_tas_guard_bands_update(struct ocelot *ocelot, int port)
> u32 max_sdu;
>
> if (min_gate_len[tc] == U64_MAX /* Gate always open */ ||
> - min_gate_len[tc] * 1000 > needed_bit_time_ps) {
> + min_gate_len[tc] * PSEC_PER_NSEC > needed_bit_time_ps) {
> /* Setting QMAXSDU_CFG to 0 disables oversized frame
> * dropping.
> */
> @@ -1249,7 +1250,7 @@ static void vsc9959_tas_guard_bands_update(struct ocelot *ocelot, int port)
> * frame, make sure to enable oversize frame dropping
> * for frames larger than the smallest that would fit.
> */
> - max_sdu = div_u64(min_gate_len[tc] * 1000,
> + max_sdu = div_u64(min_gate_len[tc] * PSEC_PER_NSEC,
> picos_per_byte);
> /* A TC gate may be completely closed, which is a
> * special case where all packets are oversized.
> diff --git a/include/linux/time64.h b/include/linux/time64.h
> index 81b9686a2079..2fb8232cff1d 100644
> --- a/include/linux/time64.h
> +++ b/include/linux/time64.h
> @@ -20,6 +20,9 @@ struct itimerspec64 {
> struct timespec64 it_value;
> };
>
> +/* Parameters used to convert the timespec values: */
> +#define PSEC_PER_NSEC 1000L
> +
> /* Located here for timespec[64]_valid_strict */
> #define TIME64_MAX ((s64)~((u64)1 << 63))
> #define TIME64_MIN (-TIME64_MAX - 1)
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index b9c71a304d39..0b941dd63d26 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/spinlock.h>
> #include <linux/rcupdate.h>
> +#include <linux/time.h>
> #include <net/netlink.h>
> #include <net/pkt_sched.h>
> #include <net/pkt_cls.h>
> @@ -176,7 +177,7 @@ static ktime_t get_interval_end_time(struct sched_gate_list *sched,
>
> static int length_to_duration(struct taprio_sched *q, int len)
> {
> - return div_u64(len * atomic64_read(&q->picos_per_byte), 1000);
> + return div_u64(len * atomic64_read(&q->picos_per_byte), PSEC_PER_NSEC);
> }
>
> /* Returns the entry corresponding to next available interval. If
> @@ -551,7 +552,7 @@ static struct sk_buff *taprio_peek(struct Qdisc *sch)
> static void taprio_set_budget(struct taprio_sched *q, struct sched_entry *entry)
> {
> atomic_set(&entry->budget,
> - div64_u64((u64)entry->interval * 1000,
> + div64_u64((u64)entry->interval * PSEC_PER_NSEC,
> atomic64_read(&q->picos_per_byte)));
> }
>
--
Regards,
Vincenzo
Hello:
This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <[email protected]>:
On Tue, 28 Jun 2022 17:52:34 +0300 you wrote:
> v1->v2:
> - define PSEC_PER_NSEC in include/linux/time64.h rather than
> include/vdso/time64.h
> - add missing #include <linux/time.h> to users of PSEC_PER_NSEC
> - move the PSEC_PER_NSEC consolidation to the last patch
>
> Richie Pearn reports that if we install a tc-taprio schedule on a Felix
> switch port, and that schedule has at least one gate that never opens
> (for example TC0 below):
>
> [...]
Here is the summary with links:
- [v2,net-next,1/4] net: dsa: felix: keep reference on entire tc-taprio config
https://git.kernel.org/netdev/net-next/c/1c9017e44af2
- [v2,net-next,2/4] net: dsa: felix: keep QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) out of rmw
https://git.kernel.org/netdev/net-next/c/d68a373bfbf4
- [v2,net-next,3/4] net: dsa: felix: drop oversized frames with tc-taprio instead of hanging the port
https://git.kernel.org/netdev/net-next/c/55a515b1f5a9
- [v2,net-next,4/4] time64.h: consolidate uses of PSEC_PER_NSEC
https://git.kernel.org/netdev/net-next/c/837ced3a1a5d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html