Re: [PATCH net-next 12/15] net: test the frag_list 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
> 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@xxxxxxxxxxxxxx>

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

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
>