Re: [PATCH v9 net-next 13/14] pds_core: publish events to the clients

From: Leon Romanovsky <leon@kernel.org>
To: Shannon Nelson <shannon.nelson@amd.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	brett.creeley@amd.com, davem@davemloft.net,
	netdev@vger.kernel.org, drivers@pensando.io, jiri@resnulli.us
Subject: Re: [PATCH v9 net-next 13/14] pds_core: publish events to the clients
Date: Fri, 14 Apr 2023 11:51:25 +0300	[thread overview]
Message-ID: <20230414085125.GY17993@unreal> (raw)
In-Reply-To: <d6a65f08-4494-8d54-7799-a819f6f2e566@amd.com>

On Thu, Apr 13, 2023 at 04:42:08PM -0700, Shannon Nelson wrote:
> On 4/13/23 10:10 AM, Jakub Kicinski wrote:
> > 
> > On Thu, 13 Apr 2023 20:07:04 +0300 Leon Romanovsky wrote:
> > > > Hm, my memory may be incorrect and I didn't look at the code but
> > > > I thought that knob came from the "hit-less upgrade" effort.
> > > > And for "hit-less upgrade" not respawning the devices was the whole
> > > > point.
> > > > 
> > > > Which is not to disagree with you. What I'm trying to get at is that
> > > > there are different types of reset which deserve different treatment.
> > > 
> > > I don't disagree with you either, just have a feeling that proposed
> > > behaviour is wrong.
> > 
> > Shannon, can you elaborate on what the impact of the reset is?
> > What loss of state and/or configuration is possible?
> 
> The device has a couple different cases that might generate the RESET
> message:
>  - crashed and recovered, no state saved
>  - FW restarted, some or all state saved
> There are some variations on this, but these are the two general cases.
> 
> We can see in the existing ionic driver there already is some handling of
> this where the driver sees the FW come back and is able to replay the Rx
> filters and Rx mode configurations.  If and when we are able to add an Eth
> client through pds_core it will want this message so that it can replay
> configuration in the same way.  This case will also want the Link Down event
> so that it can do the right thing with the netdev.

I don't see how you can replay ALL (ethtool, devlink, ip, e.t.c) states
without net core involvement. The real fun will be with many offloaded
features, where you must preserve everything while keeping upper layer
in-sync with HW.

Thanks

> 
> For the VFio/Migration support (pds_vfio) the RESET message is essentially a
> no-op if nothing is happening.  But if the system is in the middle of a
> migration it offers the ability to "cleanly" fail the migration and let the
> system get ready to try again.

> 
> For the vDPA case (pds_vdpa) we can trigger the config_cb callback to get
> the attention of the stack above us to suggest it look at current status and
> respond as needed, whether that is a Link Change or a Reset.
> 
> sln