On Wed, 29 May 2019 at 07:49, Richard Cochran [off-list ref] wrote:

On Wed, May 29, 2019 at 02:56:25AM +0300, Vladimir Oltean wrote:

quoted

The newly introduced function is called on both the RX and TX paths.

NAK on this patch.

quoted

The boolean returned by port_txtstamp should only return false if the
driver tried to timestamp the skb but failed.

So you say.

quoted

Currently there is some logic in the mv88e6xxx driver that determines
whether it should timestamp frames or not.

This is wasteful, because if the decision is to not timestamp them, then
DSA will have cloned an skb and freed it immediately afterwards.

No, it isn't wasteful.  Look at the tests in that driver to see why.

quoted

Additionally other drivers (sja1105) may have other hardware criteria
for timestamping frames on RX, and the default conditions for
timestamping a frame are too restrictive.

I'm sorry, but we won't change the frame just for one device that has
design issues.

Please put device specific workarounds into its driver.

Thanks,
Richard

Hi Richard,

I removed this patch and my RX timestamping path still works, apparently.
It's just that now I'm not holding up in the RX timestamping queue
anything else except what the PTP classifier requested me to.
What I'm concerned about is that I'm using the skb->cb as a
communication space between the tagger waiting for the meta frame, and
the RX timestamping queue waiting to be notified that the meta frame
arrived.
If I'm not holding up all frames that I know will have a follow-up
come after them, then I'm letting them free up the stack, and who
knows whose skb->cb the tagger will overwrite.
By asking me to remove this patch you're basically asking the state
machine inside the tagger to guess whether the previous frame is one
that DSA cares about w.r.t. RX timestamping, and based on that info
write to its skb->cb or not.
I would like to avoid keeping meta frames in their own RX queue,
because then I'm complicating (or rather put, making impossible) the
association between a meta frame and the frame it holds a timestamp
of.

Regards,
-Vladimir