Re: [PATCH net-next 12/15] net: test the frag_list layouts skb_segment() refuses

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Josef Bacik wrote:
> The GSO KUnit test only checks layouts skb_segment() can segment.  Add
> two it has to refuse with -EINVAL: the non-uniform frag_list with SG
> on, which used to hit the BUG_ON(i >= nfrags) the existing test steps
> around, and a frag_list member that has its own frag_list.
> 
> Both warn on CONFIG_DEBUG_NET kernels, so run them with the warning
> backtrace suppressed.
> 
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Thanks!

> ---
>  net/core/net_test.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index bc2ef7612e7d..12a175268a01 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c
> @@ -39,6 +39,8 @@ enum gso_test_nr {
>  	GSO_TEST_RESEGMENT_MULTI,
>  	GSO_TEST_RESEGMENT_TAIL,
>  	GSO_TEST_RESEGMENT_ONE_MSS,
> +	GSO_TEST_FRAG_LIST_NON_UNIFORM_SG,
> +	GSO_TEST_FRAG_LIST_NESTED,
>  };
>  
>  struct gso_test_case {
> @@ -57,6 +59,9 @@ struct gso_test_case {
>  	unsigned int nr_segs;
>  	const unsigned int *segs;
>  	const unsigned int *gso_segs;
> +
> +	/* skb_segment() is expected to fail with this error */
> +	int err;
>  };
>  
>  static struct gso_test_case cases[] = {
> @@ -204,6 +209,28 @@ static struct gso_test_case cases[] = {
>  		},
>  		.gso_segs = (const unsigned int[]) { 0, 0, 0, 0 },
>  	},
> +	{
> +		/* frag_list members that don't end on gso_size boundaries
> +		 * can't be segmented with SG
> +		 */
> +		.id = GSO_TEST_FRAG_LIST_NON_UNIFORM_SG,
> +		.name = "frag_list_non_uniform_sg",
> +		.linear_len = GSO_TEST_SIZE,
> +		.nr_frag_skbs = 4,
> +		.frag_skbs = (const unsigned int[]) { GSO_TEST_SIZE, 1, GSO_TEST_SIZE, 2 },
> +		.err = -EINVAL,
> +	},
> +	{
> +		/* the second frag_list member hangs off the first one's
> +		 * frag_list instead of off the head's
> +		 */
> +		.id = GSO_TEST_FRAG_LIST_NESTED,
> +		.name = "frag_list_nested",
> +		.linear_len = GSO_TEST_SIZE,
> +		.nr_frag_skbs = 2,
> +		.frag_skbs = (const unsigned int[]) { GSO_TEST_SIZE, GSO_TEST_SIZE },
> +		.err = -EINVAL,
> +	},
>  };
>  
>  static void gso_test_case_to_desc(struct gso_test_case *t, char *desc)
> @@ -283,6 +310,15 @@ static void gso_test_func(struct kunit *test)
>  
>  		if (tcase->id == GSO_TEST_GSO_BY_FRAGS)
>  			skb_shinfo(skb)->gso_size = GSO_BY_FRAGS;
> +
> +		if (tcase->id == GSO_TEST_FRAG_LIST_NESTED) {
> +			struct sk_buff *first = skb_shinfo(skb)->frag_list;
> +
> +			skb_shinfo(first)->frag_list = first->next;
> +			first->next = NULL;
> +			first->len += tcase->frag_skbs[1];
> +			first->data_len += tcase->frag_skbs[1];
> +		}
>  	}
>  
>  	features = NETIF_F_SG | NETIF_F_HW_CSUM;
> @@ -296,6 +332,18 @@ static void gso_test_func(struct kunit *test)
>  		features &= ~NETIF_F_SG;
>  
>  	SKB_GSO_CB(skb)->max_segs = tcase->max_segs;
> +
> +	if (tcase->err) {
> +		/* the suppression block doesn't run if it can't be set up */
> +		segs = NULL;
> +		kunit_warning_suppress(test)
> +			segs = skb_segment(skb, features);

nit: if respinning, add braces


> +		KUNIT_EXPECT_EQ(test, PTR_ERR_OR_ZERO(segs), tcase->err);
> +		if (!IS_ERR_OR_NULL(segs))
> +			kfree_skb_list(segs);
> +		goto free_gso_skb;
> +	}
> +
>  	segs = skb_segment(skb, features);
>  	if (IS_ERR(segs)) {
>  		KUNIT_FAIL(test, "segs error %pe", segs);
> 
> -- 
> 2.55.0
>