Commit ebb4c6a9 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by Jakub Kicinski

net: mscc: ocelot: cross-check the sequence id from the timestamp FIFO with the skb PTP header

The sad reality is that when a PTP frame with a TX timestamping request
is transmitted, it isn't guaranteed that it will make it all the way to
the wire (due to congestion inside the switch), and that a timestamp
will be taken by the hardware and placed in the timestamp FIFO where an
IRQ will be raised for it.

The implication is that if enough PTP frames are silently dropped by the
hardware such that the timestamp ID has rolled over, it is possible to
match a timestamp to an old skb.

Furthermore, nobody will match on the real skb corresponding to this
timestamp, since we stupidly matched on a previous one that was stale in
the queue, and stopped there.

So PTP timestamping will be broken and there will be no way to recover.

It looks like the hardware parses the sequenceID from the PTP header,
and also provides that metadata for each timestamp. The driver currently
ignores this, but it shouldn't.

As an extra resiliency measure, do the following:

- check whether the PTP sequenceID also matches between the skb and the
  timestamp, treat the skb as stale otherwise and free it

- if we see a stale skb, don't stop there and try to match an skb one
  more time, chances are there's one more skb in the queue with the same
  timestamp ID, otherwise we wouldn't have ever found the stale one (it
  is by timestamp ID that we matched it).

While this does not prevent PTP packet drops, it at least prevents
the catastrophic consequences of incorrect timestamp matching.

Since we already call ptp_classify_raw in the TX path, save the result
in the skb->cb of the clone, and just use that result in the interrupt
code path.

Fixes: 4e3b0468 ("net: mscc: PTP Hardware Clock (PHC) support")
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent fba01283
...@@ -675,6 +675,7 @@ int ocelot_port_txtstamp_request(struct ocelot *ocelot, int port, ...@@ -675,6 +675,7 @@ int ocelot_port_txtstamp_request(struct ocelot *ocelot, int port,
return err; return err;
OCELOT_SKB_CB(skb)->ptp_cmd = ptp_cmd; OCELOT_SKB_CB(skb)->ptp_cmd = ptp_cmd;
OCELOT_SKB_CB(*clone)->ptp_class = ptp_class;
} }
return 0; return 0;
...@@ -708,6 +709,17 @@ static void ocelot_get_hwtimestamp(struct ocelot *ocelot, ...@@ -708,6 +709,17 @@ static void ocelot_get_hwtimestamp(struct ocelot *ocelot,
spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags); spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
} }
static bool ocelot_validate_ptp_skb(struct sk_buff *clone, u16 seqid)
{
struct ptp_header *hdr;
hdr = ptp_parse_header(clone, OCELOT_SKB_CB(clone)->ptp_class);
if (WARN_ON(!hdr))
return false;
return seqid == ntohs(hdr->sequence_id);
}
void ocelot_get_txtstamp(struct ocelot *ocelot) void ocelot_get_txtstamp(struct ocelot *ocelot)
{ {
int budget = OCELOT_PTP_QUEUE_SZ; int budget = OCELOT_PTP_QUEUE_SZ;
...@@ -715,10 +727,10 @@ void ocelot_get_txtstamp(struct ocelot *ocelot) ...@@ -715,10 +727,10 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
while (budget--) { while (budget--) {
struct sk_buff *skb, *skb_tmp, *skb_match = NULL; struct sk_buff *skb, *skb_tmp, *skb_match = NULL;
struct skb_shared_hwtstamps shhwtstamps; struct skb_shared_hwtstamps shhwtstamps;
u32 val, id, seqid, txport;
struct ocelot_port *port; struct ocelot_port *port;
struct timespec64 ts; struct timespec64 ts;
unsigned long flags; unsigned long flags;
u32 val, id, txport;
val = ocelot_read(ocelot, SYS_PTP_STATUS); val = ocelot_read(ocelot, SYS_PTP_STATUS);
...@@ -731,6 +743,7 @@ void ocelot_get_txtstamp(struct ocelot *ocelot) ...@@ -731,6 +743,7 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
/* Retrieve the ts ID and Tx port */ /* Retrieve the ts ID and Tx port */
id = SYS_PTP_STATUS_PTP_MESS_ID_X(val); id = SYS_PTP_STATUS_PTP_MESS_ID_X(val);
txport = SYS_PTP_STATUS_PTP_MESS_TXPORT_X(val); txport = SYS_PTP_STATUS_PTP_MESS_TXPORT_X(val);
seqid = SYS_PTP_STATUS_PTP_MESS_SEQ_ID(val);
port = ocelot->ports[txport]; port = ocelot->ports[txport];
...@@ -740,6 +753,7 @@ void ocelot_get_txtstamp(struct ocelot *ocelot) ...@@ -740,6 +753,7 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
spin_unlock(&ocelot->ts_id_lock); spin_unlock(&ocelot->ts_id_lock);
/* Retrieve its associated skb */ /* Retrieve its associated skb */
try_again:
spin_lock_irqsave(&port->tx_skbs.lock, flags); spin_lock_irqsave(&port->tx_skbs.lock, flags);
skb_queue_walk_safe(&port->tx_skbs, skb, skb_tmp) { skb_queue_walk_safe(&port->tx_skbs, skb, skb_tmp) {
...@@ -755,6 +769,14 @@ void ocelot_get_txtstamp(struct ocelot *ocelot) ...@@ -755,6 +769,14 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
if (WARN_ON(!skb_match)) if (WARN_ON(!skb_match))
continue; continue;
if (!ocelot_validate_ptp_skb(skb_match, seqid)) {
dev_err_ratelimited(ocelot->dev,
"port %d received stale TX timestamp for seqid %d, discarding\n",
txport, seqid);
dev_kfree_skb_any(skb);
goto try_again;
}
/* Get the h/w timestamp */ /* Get the h/w timestamp */
ocelot_get_hwtimestamp(ocelot, &ts); ocelot_get_hwtimestamp(ocelot, &ts);
......
...@@ -697,6 +697,7 @@ struct ocelot_policer { ...@@ -697,6 +697,7 @@ struct ocelot_policer {
struct ocelot_skb_cb { struct ocelot_skb_cb {
struct sk_buff *clone; struct sk_buff *clone;
unsigned int ptp_class; /* valid only for clones */
u8 ptp_cmd; u8 ptp_cmd;
u8 ts_id; u8 ts_id;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment