From: Simon Horman <horms@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>,
Anthony Nguyen <anthony.l.nguyen@intel.com>,
Jesse Brandeburg <jesse.brandeburg@intel.com>,
netdev@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-net 1/3] ice: remove ptp_tx ring parameter flag
Date: Sat, 4 Nov 2023 11:23:41 -0400 [thread overview]
Message-ID: <20231104152341.GG891380@kernel.org> (raw)
In-Reply-To: <20231103234658.511859-2-jacob.e.keller@intel.com>
On Fri, Nov 03, 2023 at 04:46:56PM -0700, Jacob Keller wrote:
> Before performing a Tx timestamp in ice_stamp(), the driver checks a ptp_tx
> ring variable to see if timestamping is enabled on that ring. This value is
> set for all rings whenever userspace configures Tx timestamping.
>
> Ostensibly this was done to avoid wasting cycles checking other fields when
> timestamping has not been enabled. However, for Tx timestamps we already
> get an individual per-SKB flag indicating whether userspace wants to
> request a timestamp on that packet. We do not gain much by also having
> a separate flag to check for whether timestamping was enabled.
>
> In fact, the driver currently fails to restore the field after a PF reset.
> Because of this, if a PF reset occurs, timestamps will be disabled.
>
> Since this flag doesn't add value in the hotpath, remove it and always
> provide a timestamp if the SKB flag has been set.
>
> A following change will fix the reset path to properly restore user
> timestamping configuration completely.
>
> This went unnoticed for some time because one of the most common
> applications using Tx timestamps, ptp4l, will reconfigure the socket as
> part of its fault recovery logic.
>
> Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: Anthony Nguyen <anthony.l.nguyen@intel.com>,
Intel Wired LAN <intel-wired-lan@lists.osuosl.org>,
netdev@vger.kernel.org,
Jesse Brandeburg <jesse.brandeburg@intel.com>
Subject: Re: [PATCH iwl-net 1/3] ice: remove ptp_tx ring parameter flag
Date: Sat, 4 Nov 2023 11:23:41 -0400 [thread overview]
Message-ID: <20231104152341.GG891380@kernel.org> (raw)
In-Reply-To: <20231103234658.511859-2-jacob.e.keller@intel.com>
On Fri, Nov 03, 2023 at 04:46:56PM -0700, Jacob Keller wrote:
> Before performing a Tx timestamp in ice_stamp(), the driver checks a ptp_tx
> ring variable to see if timestamping is enabled on that ring. This value is
> set for all rings whenever userspace configures Tx timestamping.
>
> Ostensibly this was done to avoid wasting cycles checking other fields when
> timestamping has not been enabled. However, for Tx timestamps we already
> get an individual per-SKB flag indicating whether userspace wants to
> request a timestamp on that packet. We do not gain much by also having
> a separate flag to check for whether timestamping was enabled.
>
> In fact, the driver currently fails to restore the field after a PF reset.
> Because of this, if a PF reset occurs, timestamps will be disabled.
>
> Since this flag doesn't add value in the hotpath, remove it and always
> provide a timestamp if the SKB flag has been set.
>
> A following change will fix the reset path to properly restore user
> timestamping configuration completely.
>
> This went unnoticed for some time because one of the most common
> applications using Tx timestamps, ptp4l, will reconfigure the socket as
> part of its fault recovery logic.
>
> Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
next prev parent reply other threads:[~2023-11-04 15:24 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-03 23:46 [Intel-wired-lan] [PATCH iwl-net 0/3] ice: restore timestamp config after reset Jacob Keller
2023-11-03 23:46 ` Jacob Keller
2023-11-03 23:46 ` [Intel-wired-lan] [PATCH iwl-net 1/3] ice: remove ptp_tx ring parameter flag Jacob Keller
2023-11-03 23:46 ` Jacob Keller
2023-11-04 15:23 ` Simon Horman [this message]
2023-11-04 15:23 ` Simon Horman
2023-11-16 8:12 ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
2023-11-16 8:12 ` Pucha, HimasekharX Reddy
2023-11-03 23:46 ` [Intel-wired-lan] [PATCH iwl-net 2/3] ice: unify logic for programming PFINT_TSYN_MSK Jacob Keller
2023-11-03 23:46 ` Jacob Keller
2023-11-04 15:24 ` [Intel-wired-lan] " Simon Horman
2023-11-04 15:24 ` Simon Horman
2023-11-16 8:12 ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
2023-11-16 8:12 ` Pucha, HimasekharX Reddy
2023-11-03 23:46 ` [Intel-wired-lan] [PATCH iwl-net 3/3] ice: restore timestamp configuration after device reset Jacob Keller
2023-11-03 23:46 ` Jacob Keller
2023-11-04 15:25 ` [Intel-wired-lan] " Simon Horman
2023-11-04 15:25 ` Simon Horman
2023-11-16 8:12 ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
2023-11-16 8:12 ` Pucha, HimasekharX Reddy
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=20231104152341.GG891380@kernel.org \
--to=horms@kernel.org \
--cc=anthony.l.nguyen@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=netdev@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.