From: Stephen Hemminger <stephen@networkplumber.org>
To: Mark Blasko <blasko@google.com>
Cc: dev@dpdk.org, joshwash@google.com, jtranoleary@google.com
Subject: Re: [PATCH v4 0/6] net/gve: add hardware timestamping support
Date: Mon, 15 Jun 2026 14:29:27 -0700 [thread overview]
Message-ID: <20260615142927.525f01dd@phoenix.local> (raw)
In-Reply-To: <CANULgnLQ6E-JFEJoB6N5DEcKcFsJ3Vj3qJMmAB=2OvBfbPWKFw@mail.gmail.com>
On Mon, 15 Jun 2026 14:01:42 -0700
Mark Blasko <blasko@google.com> wrote:
> That appears to be the AI feedback from V3 (which was addressed in V4).
> I also looked at the AI feedback in the mail archive for V4 and it does not
> look like there's anything actionable.
>
> Do you have any other feedback for the V4 patches?
Let me re-run it.
Sorry got wrong part of long AI thread.
Re-reviewed v4 against current main. The two correctness errors and the
interrupt-thread warning from v3 are all resolved:
- 4/6: the periodic read now runs on a dedicated control thread via
rte_thread_create_internal_control(), sleeping in 10ms increments
with a relaxed stop flag, instead of blocking the shared EAL
interrupt thread. Teardown sets the stop flag, joins the thread,
then frees the memzone, so the join keeps the thread off the freed
memzone. Good.
- 5/6: nic_ts_lock is now initialized before gve_init_priv() (so the
setup-time gve_read_nic_clock() locks an initialized mutex) and
destroyed only after gve_teardown_device_resources() has joined the
sync thread. Both ordering bugs are fixed, and the gve_init_priv()
failure path destroys both mutexes correctly.
One item remains:
[PATCH v4 4/6] net/gve: add periodic NIC clock synchronization
Warning: the commit message body still describes the old implementation.
It says the sync "runs every 250ms using rte_alarm" and "the alarm is
still rescheduled", but the code no longer uses rte_alarm - it is a
dedicated control thread (gve_nic_ts_thread) that polls with
rte_delay_us_sleep() and a stop flag. The v4 changelog notes the switch,
but the commit message body itself was not updated. Please reword it to
match the thread-based implementation.
Patches 1, 2, 3, 5, and 6 look good. With the 4/6 commit message
corrected, the series is ready.
prev parent reply other threads:[~2026-06-15 21:29 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-11 22:43 [PATCH 0/6] net/gve: add hardware timestamping support mark-blasko
2026-05-11 22:43 ` [PATCH 1/6] net/gve: add thread safety to admin queue mark-blasko
2026-05-11 22:43 ` [PATCH 2/6] net/gve: add device option support for HW timestamps mark-blasko
2026-05-11 22:43 ` [PATCH 3/6] net/gve: add AdminQ command for NIC timestamps mark-blasko
2026-05-11 22:43 ` [PATCH 4/6] net/gve: add periodic NIC clock synchronization mark-blasko
2026-05-11 22:43 ` [PATCH 5/6] net/gve: support read clock ethdev op mark-blasko
2026-05-11 22:43 ` [PATCH 6/6] net/gve: reconstruct HW timestamps from DQO mark-blasko
2026-05-12 7:14 ` [PATCH 0/6] net/gve: add hardware timestamping support Stephen Hemminger
2026-05-15 23:19 ` [PATCH v2 " Mark Blasko
2026-05-15 23:19 ` [PATCH v2 1/6] net/gve: add thread safety to admin queue Mark Blasko
2026-05-15 23:19 ` [PATCH v2 2/6] net/gve: add device option support for HW timestamps Mark Blasko
2026-05-15 23:19 ` [PATCH v2 3/6] net/gve: add AdminQ command for NIC timestamps Mark Blasko
2026-05-15 23:19 ` [PATCH v2 4/6] net/gve: add periodic NIC clock synchronization Mark Blasko
2026-05-15 23:19 ` [PATCH v2 5/6] net/gve: support read clock ethdev op Mark Blasko
2026-05-15 23:19 ` [PATCH v2 6/6] net/gve: reconstruct HW timestamps from DQO Mark Blasko
2026-05-17 23:15 ` [PATCH v2 0/6] net/gve: add hardware timestamping support Stephen Hemminger
2026-05-18 18:43 ` Mark Blasko
2026-05-21 16:10 ` Joshua Washington
2026-06-05 21:29 ` [PATCH v3 " Mark Blasko
2026-06-05 21:29 ` [PATCH v3 1/6] net/gve: add thread safety to admin queue Mark Blasko
2026-06-05 21:29 ` [PATCH v3 2/6] net/gve: add device option support for HW timestamps Mark Blasko
2026-06-05 21:29 ` [PATCH v3 3/6] net/gve: add AdminQ command for NIC timestamps Mark Blasko
2026-06-05 21:29 ` [PATCH v3 4/6] net/gve: add periodic NIC clock synchronization Mark Blasko
2026-06-05 21:29 ` [PATCH v3 5/6] net/gve: support read clock ethdev op Mark Blasko
2026-06-05 21:29 ` [PATCH v3 6/6] net/gve: reconstruct HW timestamps from DQO Mark Blasko
2026-06-13 4:22 ` [PATCH v4 0/6] net/gve: add hardware timestamping support Mark Blasko
2026-06-13 4:22 ` [PATCH v4 1/6] net/gve: add thread safety to admin queue Mark Blasko
2026-06-13 4:22 ` [PATCH v4 2/6] net/gve: add device option support for HW timestamps Mark Blasko
2026-06-13 4:22 ` [PATCH v4 3/6] net/gve: add AdminQ command for NIC timestamps Mark Blasko
2026-06-13 4:22 ` [PATCH v4 4/6] net/gve: add periodic NIC clock synchronization Mark Blasko
2026-06-13 4:22 ` [PATCH v4 5/6] net/gve: support read clock ethdev op Mark Blasko
2026-06-13 4:22 ` [PATCH v4 6/6] net/gve: reconstruct HW timestamps from DQO Mark Blasko
2026-06-15 18:53 ` [PATCH v4 0/6] net/gve: add hardware timestamping support Stephen Hemminger
2026-06-15 21:01 ` Mark Blasko
2026-06-15 21:29 ` Stephen Hemminger [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=20260615142927.525f01dd@phoenix.local \
--to=stephen@networkplumber.org \
--cc=blasko@google.com \
--cc=dev@dpdk.org \
--cc=joshwash@google.com \
--cc=jtranoleary@google.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 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.