From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.chopps.org (smtp.chopps.org [54.88.81.56]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 83C221A8C06 for ; Wed, 31 Jul 2024 09:16:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=54.88.81.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722417421; cv=none; b=F8iiaqhAS1l6TKILqBjQnpSsYmdSd6sF4hvP0PQGSmgscFiLO8Bass+uq6SKxXiophPv3HSDO0I0adjUoZMQsHEq6X3R6jGKt5embU4XH9F3giAPPu6RbmN1AecRcA9t+vnHmnIlEwpqcJ/vPXhM5qnJJH/lNY1mxamU4hgGI3I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722417421; c=relaxed/simple; bh=8pe7bMqvwyxniR00yf0UDCmwaivCv9A2LElfGIRgcDQ=; h=References:From:To:Cc:Subject:Date:In-reply-to:Message-ID: MIME-Version:Content-Type; b=Egwr2Xa1czKe7jcB0Zv30Y0Mmk/EvzdkBOhafYTlG6Qjpl5cOnI145Bij9SXmyM6rZV3ONXJD+Z9uVDFGngfpNaZ/JbXh52MD33ng2ryWgFwz2KMXYeIWJMJiskonjLQlqyH7hcb0HUZkybmdZxxJrumsYGHpJLJSDvF1asggPs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=chopps.org; spf=fail smtp.mailfrom=chopps.org; arc=none smtp.client-ip=54.88.81.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=chopps.org Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=chopps.org Received: from ja.int.chopps.org.chopps.org (syn-172-222-091-149.res.spectrum.com [172.222.91.149]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) by smtp.chopps.org (Postfix) with ESMTPSA id B67967D052; Wed, 31 Jul 2024 09:16:52 +0000 (UTC) References: <20240714202246.1573817-1-chopps@chopps.org> <20240714202246.1573817-7-chopps@chopps.org> User-agent: mu4e 1.8.14; emacs 28.3 From: Christian Hopps To: Sabrina Dubroca Cc: Christian Hopps , devel@linux-ipsec.org, Steffen Klassert , netdev@vger.kernel.org, Christian Hopps Subject: Re: [PATCH ipsec-next v5 06/17] xfrm: add mode_cbs module functionality Date: Tue, 30 Jul 2024 17:29:06 -0400 In-reply-to: Message-ID: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; format=flowed Sabrina Dubroca writes: > 2024-07-14, 16:22:34 -0400, Christian Hopps wrote: >> +struct xfrm_mode_cbs { > > It would be nice to add kdoc for the whole thing. Ok, I'll move the inline comments to a kdoc. FWIW, all the other structs in this header, including the main `xfrm_state` struct use the same inline comment documentation style I copied. >> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c >> index 7cee9c0a2cdc..6ff05604f973 100644 >> --- a/net/xfrm/xfrm_input.c >> +++ b/net/xfrm/xfrm_input.c >> @@ -494,6 +497,10 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type) >> >> family = x->props.family; >> >> + /* An encap_type of -3 indicates reconstructed inner packet */ > > And I think it's time to document all the encap_types above the > function (and in particular, how xfrm_inner_mode_input/encap_type=-3 > pair together), and/or define some constants. Also, is -2 used > anywhere (I only see -1 and -3)? If not, then why -3? At the time this was added ISTR that there was some belief that -2 was used perhaps in an upcoming patch, so I picked -3. I can't find a -2 use case though so I will switch to -2 instead. Re documentation: I think the inline comments where encap_type is used is sufficient documentation for the 2 negative values. There's a lot going on in this function and someone wishing to change (or understand) something is going to have to walk the code and use cases regardless of a bit of extra verbiage on the encap_value beyond what's already there. Fully documenting how xfrm_input works (in all it's use cases) seems beyond the scope of this patch to me. Thanks, Chris.