Re: [PATCH net-next v3 12/15] net: test the skb 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
> the ones 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.
>  - A frag_list member that has its own frag_list.
>  - An skb whose len counts bytes it doesn't hold.  The data runs out on
>    a segment boundary, in the middle of a segment, after the last
>    frag_list member, and with GSO_BY_FRAGS.
> 
> They warn on CONFIG_DEBUG_NET kernels, so run them with the warning
> backtrace suppressed.  Also update the comment on the non-uniform case,
> which still says SG would hit that BUG_ON().
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

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

> -	/* TODO: this should also work with SG,
> -	 * rather than hit BUG_ON(i >= nfrags)
> +	/* TODO: this should also work with SG, rather than fail the
> +	 * i >= nfrags check in skb_segment()
>  	 */
>  	if (tcase->id == GSO_TEST_FRAG_LIST_NON_UNIFORM)
>  		features &= ~NETIF_F_SG;

The previous patch also addresses non-SG.

Do you want to run the new tests in both SG and non-SG mode?
>  
>  	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);
> +		}
> +		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;
> +	}
> +