On Fri, Nov 10, 2023 at 3:13 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> My brain is slightly fried after trying to catch up on the thread
> for close to 2h. So forgive me if I'm missing something.
> This applies to all emails I'm about to send :)
>
> On Sun, 5 Nov 2023 18:44:11 -0800 Mina Almasry wrote:
> > + trigger_device_reset();
>
> The user space must not be responsible for the reset.
> We can add some temporary "recreate page pools" ndo
> until the queue API is ready.
>
Thanks for the clear requirement. I clearly had something different in mind.
Might be dumb suggestions, but instead of creating a new ndo that we
maybe end up wanting to deprecate once the queue API is ready, how
about we use either of those existing APIs?
+void netdev_reset(struct net_device *dev)
+{
+ int flags = ETH_RESET_ALL;
+ int err;
+
+#if 1
+ __dev_close(dev);
+ err = __dev_open(dev, NULL);
+#else
+ err = dev->ethtool_ops->reset(dev, &flags);
+#endif
+}
+
I've tested both of these to work with GVE on both bind via the
netlink API and unbind via the netlink socket close, but I'm not
enough of an expert to tell if there is some bad side effect that can
happen or something.
> But it should not be visible to the user in any way.
>
> And then the kernel can issue the same reset when the netlink
> socket dies to flush device free lists.
>
Sure thing, I can do that.
> Maybe we should also add a "allow device/all-queues reload" flag
> to the netlink API to differentiate drivers which can't implement
> full queue API later on. We want to make sure the defaults work well
> in our "target design", rather than at the first stage. And target
> design will reload queues one by one.
I can add a flag, yes.
--
Thanks,
Mina
