From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-a6-smtp.messagingengine.com (fhigh-a6-smtp.messagingengine.com [103.168.172.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 103683D7D7F; Thu, 26 Feb 2026 16:59:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.157 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772125201; cv=none; b=TrfkovIgAL0FuTFmPbT2NNUMD0Z05VllwaoluN0FN4quPK+OLUZ6eZwqh1OI3UZC6IH74n7IuWJGFjVuJ1q8LWCB7dRS0tB5W4DmrxDc2F4wTnDP6InKNGuVNuqxYBYvlDO3lV+t19h75WkJvHF8n+xpph753GVfaj8PIYjBEHM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772125201; c=relaxed/simple; bh=fnh5iSifVccC2P+7Bw9Ravyrahm0sqQFHCjFTlbqT1U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=D6gdCEJwjqcdWeK1eOTDdnopiZyG0JwLalFBMaQljmIqqCnDIbbKvsmKlcGK2JCHZ776wO1LIQjLcY6cwqNhX4LGcBiIjD8T+UBjhXNO9Brf1v45Y/A0Lrl11hiI2rIoHhAXtdI/ejh/cFeHV94zn2TKUODXteuNmJ3d5AedDwM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=queasysnail.net; spf=pass smtp.mailfrom=queasysnail.net; dkim=pass (2048-bit key) header.d=queasysnail.net header.i=@queasysnail.net header.b=LSI6TqCT; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=j2L9eAxr; arc=none smtp.client-ip=103.168.172.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=queasysnail.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=queasysnail.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=queasysnail.net header.i=@queasysnail.net header.b="LSI6TqCT"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="j2L9eAxr" Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfhigh.phl.internal (Postfix) with ESMTP id 197BD140019C; Thu, 26 Feb 2026 11:59:56 -0500 (EST) Received: from phl-frontend-03 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Thu, 26 Feb 2026 11:59:56 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=queasysnail.net; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1772125195; x= 1772211595; bh=rMpdeGppPamOfXcrHEpeM2JvhdgBbq/Wcs0zQ8hwPjg=; b=L SI6TqCTNh56+j7MsNGyoGyawIIG+05JEh6GU0T1QKUBgAOS9qAfqzi8bjQ6RPOBC E5E7pGXLLyNJv1wTazVwnfYLF+5bWrcam5xypVgZyxSLLo95LP6pA155BmnD7br8 dIcZ3CqTUP7UH0iJJpFE58dqXPY78HK8ymheLtqQcf598i9Opo76aR4vIxzbdyer Niez65EhzVZXlF9KNya6ztCUFwh1MUBs6QfLaJEOEZlTjnwesoHkaJowAcwoF/OB jnrSGXf7WuYtRDXRDKmxebx/G/yyJpH7OXDwqpXT6QmUPM9rnKZPKWK1O99sfc/e ccK+4zA5rAAefHkAoj8iw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1772125195; x=1772211595; bh=rMpdeGppPamOfXcrHEpeM2JvhdgBbq/Wcs0 zQ8hwPjg=; b=j2L9eAxr7OtIwWGTypc/mioZ2HJAZxQRgSIcxTUjD3Vf1myc0ak GKppsGXbvz/ILqscqe6d6NKvC1HaZ5LTwMC44PxLlCmQ+6oijk1tJ2R+qMBFP83A zFDojXzwgPhT1nOjMnpbAAd4E+cFOhzYEZZUBXWjwLvhzapiz/GnmcdtUM6s1fZx PuztRQ3PT302E8jqEp8fDmFoUQt5ew1LjIRKCNDm6bT3VruyvqugMkuKxN+tpeTS 5UI01kNGpo2Frg3k6iQr+GKq82n8Y0+Z4FI3SbU5sWVQEDhBSOGTYZXIuoihUzMl a6SCw53tOLWN4/TdehXICRK6X4ho20n6HMg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefgedrtddtgddvgeeiheekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhepfffhvfevuffkfhggtggujgesthdtredttddtjeenucfhrhhomhepufgrsghrihhn rgcuffhusghrohgtrgcuoehsugesqhhuvggrshihshhnrghilhdrnhgvtheqnecuggftrf grthhtvghrnhepuefhhfffgfffhfefueeiudegtdefhfekgeetheegheeifffguedvueff fefgudffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomh epshgusehquhgvrghshihsnhgrihhlrdhnvghtpdhnsggprhgtphhtthhopedugedpmhho uggvpehsmhhtphhouhhtpdhrtghpthhtoheprghnthhonhihsehphhgvnhhomhgvrdhorh hgpdhrtghpthhtoheprghnthhonhihrdgrnhhtohhnhiesshgvtghunhgvthdrtghomhdp rhgtphhtthhopehsthgvfhhfvghnrdhklhgrshhsvghrthesshgvtghunhgvthdrtghomh dprhgtphhtthhopehhvghrsggvrhhtsehgohhnughorhdrrghprghnrgdrohhrghdrrghu pdhrtghpthhtohepnhgvthguvghvsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpth htohepuggrvhgvmhesuggrvhgvmhhlohhfthdrnhgvthdprhgtphhtthhopegvughumhgr iigvthesghhoohhglhgvrdgtohhmpdhrtghpthhtohepkhhusggrsehkvghrnhgvlhdroh hrghdprhgtphhtthhopehprggsvghnihesrhgvughhrghtrdgtohhm X-ME-Proxy: Feedback-ID: i934648bf:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 26 Feb 2026 11:59:53 -0500 (EST) Date: Thu, 26 Feb 2026 17:59:52 +0100 From: Sabrina Dubroca To: Antony Antony Cc: Antony Antony , Steffen Klassert , Herbert Xu , netdev@vger.kernel.org, "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Chiachang Wang , Yan Yan , devel@linux-ipsec.org, Simon Horman , linux-kernel@vger.kernel.org Subject: Re: [devel-ipsec] Re: [PATCH ipsec-next v5 7/8] xfrm: add error messages to state migration Message-ID: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 2026-02-26, 16:43:22 +0100, Antony Antony wrote: > On Fri, Jan 30, 2026 at 01:14:39PM +0100, Sabrina Dubroca via Devel wrote: > > 2026-01-27, 11:43:42 +0100, Antony Antony wrote: > > > Add descriptive(extack) error messages for all error paths > > > in state migration. This improves diagnostics by > > > providing clear feedback when migration fails. > > > > > > Signed-off-by: Antony Antony > > > --- > > > v4->v5: - added this patch > > > --- > > > net/xfrm/xfrm_state.c | 13 ++++++++++--- > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > > > index 88a362e46972..2e03871ae872 100644 > > > --- a/net/xfrm/xfrm_state.c > > > +++ b/net/xfrm/xfrm_state.c > > > @@ -2129,15 +2129,21 @@ struct xfrm_state *xfrm_state_migrate_create(struct xfrm_state *x, > > > struct xfrm_state *xc; > > > > > > xc = xfrm_state_clone_and_setup(x, encap, m); > > > - if (!xc) > > > + if (!xc) { > > > + NL_SET_ERR_MSG(extack, "Failed to clone and setup state"); > > > > When xfrm_state_clone_and_setup fails it's because some allocation > > failed and the user won't be able to do much about this, right? I > > don't feel extack in those situations is super helpful. > > I felt it was usefaul to know, and to log this happened. May not a great > idea. I don't have a super strong opinion. IIRC that was the approach I picked when I added extack (no extack for kernel events that the user can't do anything about and don't result from an invalid netlink message), but maybe that kind of stuff deserves an extack too. Also, I thought that something that ends up returning ENOMEM to userspace is explicit enough, without adding a string "failed to allocate memory for $object" in extack. But I don't work on *swan, so maybe it's more useful than I think. (Steffen has the final word, and you're closer to him than I am :)) > > > return NULL; > > > + } > > > > > > - if (xfrm_init_state(xc) < 0) > > > + if (xfrm_init_state(xc) < 0) { > > > + NL_SET_ERR_MSG(extack, "Failed to initialize migrated state"); > > > > xfrm_init_state itself doesn't handle extack, but it's just a wrapper > > around functions that do. Maybe better to make xfrm_init_state > > propagate extack? > > That is a great idea. May be in a future patch set. For now, I will drop > this patch from this series. To move forward quickly. Ok. Or keep the patch with just the fixup right below this, I'm not NACKing it. > > > goto error; > > > + } > > > > > > /* configure the hardware if offload is requested */ > > > - if (xuo && xfrm_dev_state_add(net, xc, xuo, extack)) > > > + if (xuo && xfrm_dev_state_add(net, xc, xuo, extack)) { > > > + NL_SET_ERR_MSG(extack, "Failed to initialize state offload"); > > > > We already set an extack in xfrm_dev_state_add, this chunk should be > > dropped to avoid overwriting the more specific info we got. > > > > > goto error; > > > + } > > > > > > return xc; > > > error: > > > @@ -2161,6 +2167,7 @@ int xfrm_state_migrate_install(const struct xfrm_state *x, > > > xfrm_state_insert(xc); > > > } else { > > > if (xfrm_state_add(xc) < 0) { > > > + NL_SET_ERR_MSG(extack, "Failed to add migrated state"); > > > > Not a strong objection, but this case would be the EEXIST situation > > from xfrm_state_add, and there's not much the user can do about this? > > Fair point, but logging it still has value too, userspace can track these > over time and adapt. Let's revisit when we add extack to xfrm_init_state. Ok. > > > if (xuo) > > > xfrm_dev_state_delete(xc); > > > xc->km.state = XFRM_STATE_DEAD; > > -- Sabrina