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

[Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>]

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@xxxxxxxxxxxxxx>

Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>

> -     /* 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;
> +     }
> +