netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Cochran <richardcochran@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Keller, Jacob E" <jacob.e.keller@intel.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"jakub.kicinski@netronome.com" <jakub.kicinski@netronome.com>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH net] net: dsa: sja1105: Fix double delivery of TX timestamps to socket error queue
Date: Fri, 27 Dec 2019 09:39:50 -0800	[thread overview]
Message-ID: <20191227173950.GD1435@localhost> (raw)
In-Reply-To: <CA+h21hoq2-CAVfv7zQkQCsaTFmOTrxj-93MbHpJLPAt7jbOgbg@mail.gmail.com>

On Fri, Dec 27, 2019 at 05:30:09PM +0200, Vladimir Oltean wrote:
> But in the case that I _do_ care about, it _is_ caused by DSA. The
> gianfar driver is not expecting anybody else to set SKBTX_IN_PROGRESS,
> which in itself is not illegal, whether or not you argue that it is
> needed or not.

Ok, so I've reviewed the whole SKBTX_IN_PROGRESS thing again, and I'm
starting to see it your way.  (Or maybe not ;^)

The purpose of SKBTX_IN_PROGRESS is to prevent a SW transmit time
stamp from being delivered (unless SOF_TIMESTAMPING_OPT_TX_SWHW is set
on the socket).

So DSA drivers (and PHY drivers) should indeed set this flag.

However, most (or all?) MAC drivers do not expect anyone else to have
set this flag.

In the case of DSA, the DSA driver has the chance to set the flag
first, before the skb is passed to the MAC driver.  (PHY drivers don't
have first dibs, but let's concentrate on DSA for now.)

So, you could introduce a new rule that MAC drivers must check to see
if the flag is already set, and if so leave it alone.

MAC drivers should in any case respect their current SIOCSHWTSTAMP
configuration and not attempt time stamping when it is not enabled.

So I'm afraid that, as I've said before, you'll have to patch the MAC
drivers one by.  It will be lots of work.

But if and when you submit patches for the MAC drivers, please
remember that DSA time stamping is the new kid on the block, and
yelling that the MAC drivers are broken won't make you any friends.

Thanks,
Richard


      reply	other threads:[~2019-12-27 17:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-16 22:33 [PATCH net] net: dsa: sja1105: Fix double delivery of TX timestamps to socket error queue Vladimir Oltean
2019-12-17 19:57 ` Keller, Jacob E
2019-12-17 20:07   ` Vladimir Oltean
2019-12-17 22:21     ` Keller, Jacob E
2019-12-24 19:05       ` Richard Cochran
2019-12-26 18:24         ` Vladimir Oltean
2019-12-27  1:52           ` Richard Cochran
2019-12-27 15:19             ` Richard Cochran
2019-12-27 15:30               ` Vladimir Oltean
2019-12-27 17:39                 ` Richard Cochran [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191227173950.GD1435@localhost \
    --to=richardcochran@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).