Re: [PATCH net-next v8 06/12] gve: simplify reset logic

[Joshua Washington]

On Tue, Sep 29, 2026 at 2:27 AM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
>
> On 9/25/26 20:56, netdev-bot+sashiko@xxxxxxxxxx wrote:
> > Thank you for your contribution! Sashiko AI review found 7 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 2 (1) · Medium: 2 (2) · Low: 3 (1)
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [High] After a failed recovery, an unprivileged user can crash the
> >    kernel with an ethtool query.
> > - [High] When gve_recover() fails during a reset while the interface is
> >    up (service-task reset, ethtool reset, flow-rule timeout, or…
> > - [Medium] gve_teardown_control_plane_resources() (gve_main.c) now calls
> >    gve_teardown_clock().
> > - [Medium] The commit message does not mention several user-visible
> >    changes to error handling, and one of its design statements is not…
> > - [Low] This patch removed gve_queues_mem_remove() from
> >    gve_queues_start()'s error path.
> > - [Low] The commit fixes a real unbounded recursive reset but has no
> >    Fixes: tag.
> > - [Low] Item 2 of the commit message describes the old bug this way:
> >    gve_close() during gve_shutdown triggers a hardware reset and then…
> Low prio remarks could be ignored and we prefer no fixes tag in net-next
> patches, but the first 3 ones in the list look relevant and should be
> addressed.
>
> Also please note you are supposed to address clashiko comments proactively.
>
> /P
>

My apologies for not replying directly. Harshitha ultimately replied
to the cover letter stating that we would be sending a v9 to address
high/medium issues, so I thought that was sufficient.

In any case, I will add a response here for the planned fixes for v9:

> - [High] After a failed recovery, an unprivileged user can crash the
>    kernel with an ethtool query.
> - [High] When gve_recover() fails during a reset while the interface is
>    up (service-task reset, ethtool reset, flow-rule timeout, or…

These first two are a result of the change to teardown when recover
fails. For that, we will avoid tearing down when only queue creation
fails, and call device_detach when the control plane itself cannot be
set up. This should prevent any ethtool IOCTLs from going through.

> - [Medium] gve_teardown_control_plane_resources() (gve_main.c) now calls
>    gve_teardown_clock().

gve_teardown_clock() will be called as part of
gve_free_control_plane_resources() instead. Since it is only freeing
memory, and is idempotent, this should be a safe change.

For the commit message issue, the commit message will be updated to
reflect the latest changes. This code has changed quite a lot since
v1, but the commit messages has not seen many of those updates.

-- Josh