| Message ID | 20230626154003.3153076-4-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix PTP packet drops with ocelot-8021q DSA tag protocol | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 12 of 12 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Mon, Jun 26, 2023 at 06:40:03PM +0300, Vladimir Oltean wrote: > drivers/net/dsa/ocelot/felix.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c > index 80861ac090ae..7b494d975073 100644 > --- a/drivers/net/dsa/ocelot/felix.c > +++ b/drivers/net/dsa/ocelot/felix.c > @@ -1725,6 +1725,9 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port, > u32 tstamp_hi; > u64 tstamp; > > + if (ocelot->ports[port]->ptp_rx_filter == HWTSTAMP_FILTER_NONE) > + return false; > + > /* If the "no XTR IRQ" workaround is in use, tell DSA to defer this skb > * for RX timestamping. Then free it, and poll for its copy through > * MMIO in the CPU port module, and inject that into the stack from > -- > 2.34.1 > > This is still not as good as I had wanted it, because simply checking for HWTSTAMP_FILTER_NONE does not distinguish between L2 and L4 timestamping filters, and a port configured just with L2 traps will still drop L4 PTP packets. Preparing a v2.
On Tue, 27 Jun 2023 18:12:22 +0300 Vladimir Oltean wrote: > This is still not as good as I had wanted it, because simply checking > for HWTSTAMP_FILTER_NONE does not distinguish between L2 and L4 > timestamping filters, and a port configured just with L2 traps will > still drop L4 PTP packets. Out of curiosity - quick survey on why your reply does not contain: pw-bot: changes-requested a) your email address is different and the bot doesn't understand aliases b) commands are hard to remember c) don't care about patchwork d) laziness e) other
On Tue, Jun 27, 2023 at 08:46:51AM -0700, Jakub Kicinski wrote: > On Tue, 27 Jun 2023 18:12:22 +0300 Vladimir Oltean wrote: > > This is still not as good as I had wanted it, because simply checking > > for HWTSTAMP_FILTER_NONE does not distinguish between L2 and L4 > > timestamping filters, and a port configured just with L2 traps will > > still drop L4 PTP packets. > > Out of curiosity - quick survey on why your reply does not contain: > > pw-bot: changes-requested > > a) your email address is different and the bot doesn't understand > aliases > b) commands are hard to remember > c) don't care about patchwork > d) laziness > e) other hmm, I'll tick e) unslept...
On Tue, 27 Jun 2023 18:51:47 +0300 Vladimir Oltean wrote: > > pw-bot: changes-requested > > > > a) your email address is different and the bot doesn't understand > > aliases > > b) commands are hard to remember > > c) don't care about patchwork > > d) laziness > > e) other > > hmm, I'll tick e) unslept... Ah, good, I was worried it was the aliases and I don't have a great plan yet for how to deal with that :)
On Tue, Jun 27, 2023 at 08:53:33AM -0700, Jakub Kicinski wrote: > On Tue, 27 Jun 2023 18:51:47 +0300 Vladimir Oltean wrote: > > > pw-bot: changes-requested > > > > > > a) your email address is different and the bot doesn't understand > > > aliases > > > b) commands are hard to remember > > > c) don't care about patchwork > > > d) laziness > > > e) other > > > > hmm, I'll tick e) unslept... > > Ah, good, I was worried it was the aliases and I don't have a great > plan yet for how to deal with that :) No, I can easily switch to the work email, I just wasn't paying enough attention to which screen I was looking at.
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index 80861ac090ae..7b494d975073 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1725,6 +1725,9 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port, u32 tstamp_hi; u64 tstamp; + if (ocelot->ports[port]->ptp_rx_filter == HWTSTAMP_FILTER_NONE) + return false; + /* If the "no XTR IRQ" workaround is in use, tell DSA to defer this skb * for RX timestamping. Then free it, and poll for its copy through * MMIO in the CPU port module, and inject that into the stack from
The driver implements a workaround for the fact that it doesn't have an IRQ source to tell it whether PTP frames are available through the extraction registers, for those frames to be processed and passed towards the network stack. That workaround is to configure the switch, through felix_hwtstamp_set() -> felix_update_trapping_destinations(), to create two copies of PTP packets: one sent over Ethernet to the DSA master, and one to be consumed through the aforementioned CPU extraction queue registers. The reason why we want PTP packets to be consumed through the CPU extraction registers in the first place is because we want to see their hardware RX timestamp. With tag_8021q, that is only visible that way, and it isn't visible with the copy of the packet that's transmitted over Ethernet. The problem with the workaround implementation is that it drops the packet received over Ethernet, in expectation of its copy being present in the CPU extraction registers. However, if felix_hwtstamp_set() hasn't run (aka PTP RX timestamping is disabled), the driver will drop the original PTP frame and there will be no copy of it in the CPU extraction registers. So, the network stack will simply not see any PTP frame. Look at the port's ptp_rx_filter to see whether the driver has previously enabled the CPU extraction registers. If it hasn't, just don't RX timestamp the frame and let it be passed up the stack by DSA, which is absolutely perfectly fine. Fixes: 0a6f17c6ae21 ("net: dsa: tag_ocelot_8021q: add support for PTP timestamping") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/dsa/ocelot/felix.c | 3 +++ 1 file changed, 3 insertions(+)