Received-SPF: pass (google.com: domain of linux-kernel+bounces-186357-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Wed, 22 May 2024 07:22:12 -0700
From: Jakub Kicinski <kuba@kernel.org>
To: Danielle Ratson <danieller@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>, "netdev@vger.kernel.org"
 <netdev@vger.kernel.org>, "davem@davemloft.net" <davem@davemloft.net>,
 "edumazet@google.com" <edumazet@google.com>, "pabeni@redhat.com"
 <pabeni@redhat.com>, "corbet@lwn.net" <corbet@lwn.net>,
 "linux@armlinux.org.uk" <linux@armlinux.org.uk>, "sdf@google.com"
 <sdf@google.com>, "kory.maincent@bootlin.com" <kory.maincent@bootlin.com>,
 "maxime.chevallier@bootlin.com" <maxime.chevallier@bootlin.com>,
 "vladimir.oltean@nxp.com" <vladimir.oltean@nxp.com>,
 "przemyslaw.kitszel@intel.com" <przemyslaw.kitszel@intel.com>,
 "ahmed.zaki@intel.com" <ahmed.zaki@intel.com>, "richardcochran@gmail.com"
 <richardcochran@gmail.com>, "shayagr@amazon.com" <shayagr@amazon.com>,
 "paul.greenwalt@intel.com" <paul.greenwalt@intel.com>, "jiri@resnulli.us"
 <jiri@resnulli.us>, "linux-doc@vger.kernel.org"
 <linux-doc@vger.kernel.org>, "linux-kernel@vger.kernel.org"
 <linux-kernel@vger.kernel.org>, mlxsw <mlxsw@nvidia.com>, Petr Machata
 <petrm@nvidia.com>
Subject: Re: [PATCH net-next v5 04/10] ethtool: Add flashing transceiver
 modules' firmware notifications ability
Message-ID: <20240522072212.7a21c84b@kernel.org>
In-Reply-To: <DM6PR12MB451677DBA41EA8A622D3D446D8EB2@DM6PR12MB4516.namprd12.prod.outlook.com>
References: <20240424133023.4150624-1-danieller@nvidia.com>
	<20240424133023.4150624-5-danieller@nvidia.com>
	<20240429201130.5fad6d05@kernel.org>
	<DM6PR12MB45168DC7D9D9D7A5AE3E2B2DD81A2@DM6PR12MB4516.namprd12.prod.outlook.com>
	<20240430130302.235d612d@kernel.org>
	<ZjH1DCu0rJTL_RYz@shredder>
	<20240501073758.3da76601@kernel.org>
	<DM6PR12MB451687C3C54323473716621ED8EB2@DM6PR12MB4516.namprd12.prod.outlook.com>
	<20240522064519.3e980390@kernel.org>
	<DM6PR12MB451677DBA41EA8A622D3D446D8EB2@DM6PR12MB4516.namprd12.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, 22 May 2024 13:56:11 +0000 Danielle Ratson wrote:
> > > 4. Add a new netlink notifier that when the relevant event takes plac=
e, =20
> > deletes the node from the list, wait until the end of the work item, wi=
th
> > cancel_work_sync() and free allocations.
> >=20
> > What's the "relevant event" in this case? Closing of the socket that us=
er had
> > issued the command on? =20
>=20
> The event should match the below:
> event =3D=3D NETLINK_URELEASE && notify->protocol =3D=3D NETLINK_GENERIC
>=20
> Then iterate over the list to look for work that matches the dev and port=
id.
> The socket doesn=E2=80=99t close until the work is done in that case.=20

Okay, good, yes. I think you can use one of the callbacks I mentioned
below to achieve the same thing with less complexity than the notifier.

> > Easiest way to "notice" the socket got closed would probably be to add =
some
> > info to genl_sk_priv_*(). ->sock_priv_destroy() will get called. But yo=
u can also
> > get a close notification in the family =20
> > ->unbind callback. =20
> >=20
> > I'm on the fence whether we should cancel the work. We could just mark =
the
> > command as 'no socket present' and stop sending notifications.
> > Not sure which is better.. =20
>=20
> Is there a scenario that we hit this event and won't intend to cancel the=
 work?=20

I think it's up to us. I don't see any legit reason for user space to
intentionally cancel the flashing. So the only option is that user space
is either buggy or has crashed, and the socket got closed before
flashing finished. Right?