Re: [PATCH net-next v2 06/10] handshake: Add a kunit test for the completion gate

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] handshake: unrelated sessiontags KUnit changes squashed into
  commit
--

commit b879c6737fc8615a84b7321f23d468457911d409
Author: Chuck Lever <cel@kernel.org>

handshake: Add a kunit test for the completion gate

This commit exports handshake_try_complete() and handshake_finish_complete()
for KUnit and adds a test case verifying completion gate exclusivity, along
with updates to the session-tag duplicate KUnit tests.

> diff --git a/net/handshake/handshake-test.c b/net/handshake/handshake-test.c
> index 21fccfa94d897..b6992d1fa4ce4 100644
> --- a/net/handshake/handshake-test.c
> +++ b/net/handshake/handshake-test.c

[ ... ]

> @@ -651,7 +729,7 @@ static void handshake_sessiontags_duplicate(struct kunit *test)
>  	info.nlhdr = nlh;
>  
>  	KUNIT_EXPECT_EQ(test, handshake_get_sessiontags(&tags, &info), 0);
> -	KUNIT_EXPECT_EQ(test, stringset_count(&tags), 2);
> +	KUNIT_EXPECT_EQ(test, stringset_count(&tags), 1);
>  	stringset_finalize(&tags);
>  	KUNIT_EXPECT_EQ(test, stringset_count(&tags), 1);
>  	KUNIT_EXPECT_TRUE(test, stringset_is_member(&tags, "dup"));
> @@ -660,6 +738,30 @@ static void handshake_sessiontags_duplicate(struct kunit *test)
>  	kfree_skb(skb);
>  }
>  
> +static void handshake_sessiontags_duplicate_at_cap(struct kunit *test)
> +{
> +	struct genl_info info = {};
> +	struct nlmsghdr *nlh;
> +	struct sk_buff *skb;
> +	DEFINE_STRINGSET(tags);
> +	unsigned int i;
> +
> +	skb = handshake_done_msg(test, &nlh);
> +	for (i = 0; i < HANDSHAKE_MAX_SESSIONTAGS; i++)
> +		nla_put_string(skb, HANDSHAKE_A_DONE_TAG, "dup");
> +	nla_put_string(skb, HANDSHAKE_A_DONE_TAG, "last");
> +	nlmsg_end(skb, nlh);
> +	info.nlhdr = nlh;
> +
> +	KUNIT_EXPECT_EQ(test, handshake_get_sessiontags(&tags, &info), 0);
> +	KUNIT_EXPECT_EQ(test, stringset_count(&tags), 2);
> +	stringset_finalize(&tags);
> +	KUNIT_EXPECT_TRUE(test, stringset_is_member(&tags, "last"));
> +
> +	stringset_destroy(&tags);
> +	kfree_skb(skb);
> +}

[ ... ]

> @@ -755,6 +861,10 @@ static struct kunit_case handshake_api_test_cases[] = {
>  		.name			= "sessiontags duplicate",
>  		.run_case		= handshake_sessiontags_duplicate,
>  	},
> +	{
> +		.name			= "sessiontags duplicates at cap",
> +		.run_case		= handshake_sessiontags_duplicate_at_cap,
> +	},
>  	{
>  		.name			= "sessiontags truncate at cap",
>  		.run_case		= handshake_sessiontags_truncate,

[Severity: Low]
Were these session-tag duplicate KUnit changes meant to be folded into
the preceding commit "handshake: Pick up session tags passed during the
DONE downcall"?

In that commit, handshake_get_sessiontags() already skips duplicate tags
via handshake_sessiontag_is_dup(), so handshake_sessiontags_duplicate()
fails its stringset_count(&tags) == 2 check until this commit updates it,
and the commit message here only describes the completion gate test.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-tls-session-tags-v2-0-a587a50de7fc@kernel.org?part=6