[PATCH net-next 0/3] add skb_segment kunit coverage

[PATCH net-next 0/3] add skb_segment kunit coverage

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

As discussed at netconf last week. Some kernel code is exercised in
many different ways. skb_segment is a prime example. This ~350 line
function has 49 different patches in git blame with 28 different
authors.

When making a change, e.g., to fix a bug in one specific use case,
it is hard to establish through analysis alone that the change does
not break the many other paths through the code. It is impractical to
exercise all code paths through regression testing from userspace.

Add the minimal infrastructure needed to add KUnit tests to networking,
and add code coverage for this function.

Patch 1 adds the infra and the first simple test case: a linear skb
Patch 2 adds variants with frags[]
Patch 3 adds variants with frag_list skbs

Willem de Bruijn (3):
  net: add skb_segment kunit test
  net: parametrize skb_segment unit test to expand coverage
  net: expand skb_segment unit test with frag_list coverage

 net/Kconfig         |   9 ++
 net/core/Makefile   |   1 +
 net/core/gso_test.c | 274 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 284 insertions(+)
 create mode 100644 net/core/gso_test.c

-- 
2.42.0.582.g8ccd20d70d-goog

[PATCH net-next 1/3] net: add skb_segment kunit test

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Add unit testing for skb segment. This function is exercised by many
different code paths, such as GSO_PARTIAL or GSO_BY_FRAGS, linear
(with or without head_frag), frags or frag_list skbs, etc.

It is infeasible to manually run tests that cover all code paths when
making changes. The long and complex function also makes it hard to
establish through analysis alone that a patch has no unintended
side-effects.

Add code coverage through kunit regression testing. Introduce kunit
infrastructure for tests under net/core, and add this first test.

This first skb_segment test exercises a simple case: a linear skb.
Follow-on patches will parametrize the test and add more variants.

Tested: Built and ran the test with

    make ARCH=um mrproper

    ./tools/testing/kunit/kunit.py run \
        --kconfig_add CONFIG_NET=y \
        --kconfig_add CONFIG_DEBUG_KERNEL=y \
        --kconfig_add CONFIG_DEBUG_INFO=y \
        --kconfig_add=CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y \
        net_core_gso

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/Kconfig         |  9 +++++
 net/core/Makefile   |  1 +
 net/core/gso_test.c | 86 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 net/core/gso_test.c

diff --git a/net/Kconfig b/net/Kconfig
index d532ec33f1fed..17676dba10fbe 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -508,4 +508,13 @@ config NETDEV_ADDR_LIST_TEST
 	default KUNIT_ALL_TESTS
 	depends on KUNIT
 
+config NET_TEST
+	tristate "KUnit tests for networking" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  KUnit tests covering core networking infra, such as sk_buff.
+
+	  If unsure, say N.
+
 endif   # if NET
diff --git a/net/core/Makefile b/net/core/Makefile
index 731db2eaa6107..0cb734cbc24b2 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -40,3 +40,4 @@ obj-$(CONFIG_NET_SOCK_MSG) += skmsg.o
 obj-$(CONFIG_BPF_SYSCALL) += sock_map.o
 obj-$(CONFIG_BPF_SYSCALL) += bpf_sk_storage.o
 obj-$(CONFIG_OF)	+= of_net.o
+obj-$(CONFIG_NET_TEST) += gso_test.o
diff --git a/net/core/gso_test.c b/net/core/gso_test.c
new file mode 100644
index 0000000000000..9a4828b20203a
--- /dev/null
+++ b/net/core/gso_test.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <kunit/test.h>
+#include <linux/skbuff.h>
+
+static const char hdr[] = "abcdefgh";
+static const int gso_size = 1000, last_seg_size = 1;
+
+/* default: create 3 segment gso packet */
+static const int payload_len = (2 * gso_size) + last_seg_size;
+
+static void __init_skb(struct sk_buff *skb)
+{
+	skb_reset_mac_header(skb);
+	memcpy(skb_mac_header(skb), hdr, sizeof(hdr));
+
+	/* skb_segment expects skb->data at start of payload */
+	skb_pull(skb, sizeof(hdr));
+	skb_reset_network_header(skb);
+	skb_reset_transport_header(skb);
+
+	/* proto is arbitrary, as long as not ETH_P_TEB or vlan */
+	skb->protocol = htons(ETH_P_ATALK);
+	skb_shinfo(skb)->gso_size = gso_size;
+}
+
+static void gso_test_func(struct kunit *test)
+{
+	const int shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+	struct sk_buff *skb, *segs, *cur;
+	struct page *page;
+
+	page = alloc_page(GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, page);
+	skb = build_skb(page_address(page), sizeof(hdr) + payload_len + shinfo_size);
+	KUNIT_ASSERT_NOT_NULL(test, skb);
+	__skb_put(skb, sizeof(hdr) + payload_len);
+
+	__init_skb(skb);
+
+	segs = skb_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM);
+	if (IS_ERR(segs)) {
+		KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
+		goto free_gso_skb;
+	} else if (!segs) {
+		KUNIT_FAIL(test, "no segments");
+		goto free_gso_skb;
+	}
+
+	for (cur = segs; cur; cur = cur->next) {
+		/* segs have skb->data pointing to the mac header */
+		KUNIT_ASSERT_PTR_EQ(test, skb_mac_header(cur), cur->data);
+		KUNIT_ASSERT_PTR_EQ(test, skb_network_header(cur), cur->data + sizeof(hdr));
+
+		/* header was copied to all segs */
+		KUNIT_ASSERT_EQ(test, memcmp(skb_mac_header(cur), hdr, sizeof(hdr)), 0);
+
+		/* all segs are gso_size, except for last */
+		if (cur->next) {
+			KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + gso_size);
+		} else {
+			KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + last_seg_size);
+
+			/* last seg can be found through segs->prev pointer */
+			KUNIT_ASSERT_PTR_EQ(test, cur, segs->prev);
+		}
+	}
+
+	consume_skb(segs);
+free_gso_skb:
+	consume_skb(skb);
+}
+
+static struct kunit_case gso_test_cases[] = {
+	KUNIT_CASE(gso_test_func),
+	{}
+};
+
+static struct kunit_suite gso_test_suite = {
+	.name = "net_core_gso",
+	.test_cases = gso_test_cases,
+};
+
+kunit_test_suite(gso_test_suite);
+
+MODULE_LICENSE("GPL");
-- 
2.42.0.582.g8ccd20d70d-goog

[PATCH net-next 2/3] net: parametrize skb_segment unit test to expand coverage

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Expand the test with variants

- GSO_TEST_NO_GSO:      payload size less than or equal to gso_size
- GSO_TEST_FRAGS:       payload in both linear and page frags
- GSO_TEST_FRAGS_PURE:  payload exclusively in page frags
- GSO_TEST_GSO_PARTIAL: produce one gso segment of multiple of gso_size,
                        plus optionally one non-gso trailer segment

Define a test struct that encodes the input gso skb and output segs.
Input in terms of linear and fragment lengths. Output as length of
each segment.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/core/gso_test.c | 129 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 112 insertions(+), 17 deletions(-)

diff --git a/net/core/gso_test.c b/net/core/gso_test.c
index 9a4828b20203a..41d1daa20831e 100644
--- a/net/core/gso_test.c
+++ b/net/core/gso_test.c
@@ -4,10 +4,7 @@
 #include <linux/skbuff.h>
 
 static const char hdr[] = "abcdefgh";
-static const int gso_size = 1000, last_seg_size = 1;
-
-/* default: create 3 segment gso packet */
-static const int payload_len = (2 * gso_size) + last_seg_size;
+static const int gso_size = 1000;
 
 static void __init_skb(struct sk_buff *skb)
 {
@@ -24,21 +21,121 @@ static void __init_skb(struct sk_buff *skb)
 	skb_shinfo(skb)->gso_size = gso_size;
 }
 
+enum gso_test_nr {
+	GSO_TEST_LINEAR,
+	GSO_TEST_NO_GSO,
+	GSO_TEST_FRAGS,
+	GSO_TEST_FRAGS_PURE,
+	GSO_TEST_GSO_PARTIAL,
+};
+
+struct gso_test_case {
+	enum gso_test_nr id;
+	const char *name;
+
+	/* input */
+	unsigned int linear_len;
+	unsigned int nr_frags;
+	const unsigned int *frags;
+
+	/* output as expected */
+	unsigned int nr_segs;
+	const unsigned int *segs;
+};
+
+static struct gso_test_case cases[] = {
+	{
+		.id = GSO_TEST_NO_GSO,
+		.name = "no_gso",
+		.linear_len = gso_size,
+		.nr_segs = 1,
+		.segs = (const unsigned int[]) { gso_size },
+	},
+	{
+		.id = GSO_TEST_LINEAR,
+		.name = "linear",
+		.linear_len = gso_size + gso_size + 1,
+		.nr_segs = 3,
+		.segs = (const unsigned int[]) { gso_size, gso_size, 1 },
+	},
+	{
+		.id = GSO_TEST_FRAGS,
+		.name = "frags",
+		.linear_len = gso_size,
+		.nr_frags = 2,
+		.frags = (const unsigned int[]) { gso_size, 1 },
+		.nr_segs = 3,
+		.segs = (const unsigned int[]) { gso_size, gso_size, 1 },
+	},
+	{
+		.id = GSO_TEST_FRAGS_PURE,
+		.name = "frags_pure",
+		.nr_frags = 3,
+		.frags = (const unsigned int[]) { gso_size, gso_size, 2 },
+		.nr_segs = 3,
+		.segs = (const unsigned int[]) { gso_size, gso_size, 2 },
+	},
+	{
+		.id = GSO_TEST_GSO_PARTIAL,
+		.name = "gso_partial",
+		.linear_len = gso_size,
+		.nr_frags = 2,
+		.frags = (const unsigned int[]) { gso_size, 3 },
+		.nr_segs = 2,
+		.segs = (const unsigned int[]) { 2 * gso_size, 3 },
+	},
+};
+
+static void gso_test_case_to_desc(struct gso_test_case *t, char *desc)
+{
+	sprintf(desc, "%s", t->name);
+}
+
+KUNIT_ARRAY_PARAM(gso_test, cases, gso_test_case_to_desc);
+
 static void gso_test_func(struct kunit *test)
 {
 	const int shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+	const struct gso_test_case *tcase;
 	struct sk_buff *skb, *segs, *cur;
+	netdev_features_t features;
 	struct page *page;
+	int i;
+
+	tcase = test->param_value;
 
 	page = alloc_page(GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, page);
-	skb = build_skb(page_address(page), sizeof(hdr) + payload_len + shinfo_size);
+	skb = build_skb(page_address(page), sizeof(hdr) + tcase->linear_len + shinfo_size);
 	KUNIT_ASSERT_NOT_NULL(test, skb);
-	__skb_put(skb, sizeof(hdr) + payload_len);
+	__skb_put(skb, sizeof(hdr) + tcase->linear_len);
 
 	__init_skb(skb);
 
-	segs = skb_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM);
+	if (tcase->nr_frags) {
+		unsigned int pg_off = 0;
+
+		page = alloc_page(GFP_KERNEL);
+		KUNIT_ASSERT_NOT_NULL(test, page);
+		page_ref_add(page, tcase->nr_frags - 1);
+
+		for (i = 0; i < tcase->nr_frags; i++) {
+			skb_fill_page_desc(skb, i, page, pg_off, tcase->frags[i]);
+			pg_off += tcase->frags[i];
+		}
+
+		KUNIT_ASSERT_LE(test, pg_off, PAGE_SIZE);
+
+		skb->data_len = pg_off;
+		skb->len += skb->data_len;
+		skb->truesize += skb->data_len;
+	}
+
+	features = NETIF_F_SG | NETIF_F_HW_CSUM;
+	if (tcase->id == GSO_TEST_GSO_PARTIAL)
+		features |= NETIF_F_GSO_PARTIAL;
+
+	segs = skb_segment(skb, features);
 	if (IS_ERR(segs)) {
 		KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
 		goto free_gso_skb;
@@ -47,7 +144,9 @@ static void gso_test_func(struct kunit *test)
 		goto free_gso_skb;
 	}
 
-	for (cur = segs; cur; cur = cur->next) {
+	for (cur = segs, i = 0; cur; cur = cur->next, i++) {
+		KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + tcase->segs[i]);
+
 		/* segs have skb->data pointing to the mac header */
 		KUNIT_ASSERT_PTR_EQ(test, skb_mac_header(cur), cur->data);
 		KUNIT_ASSERT_PTR_EQ(test, skb_network_header(cur), cur->data + sizeof(hdr));
@@ -55,24 +154,20 @@ static void gso_test_func(struct kunit *test)
 		/* header was copied to all segs */
 		KUNIT_ASSERT_EQ(test, memcmp(skb_mac_header(cur), hdr, sizeof(hdr)), 0);
 
-		/* all segs are gso_size, except for last */
-		if (cur->next) {
-			KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + gso_size);
-		} else {
-			KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + last_seg_size);
-
-			/* last seg can be found through segs->prev pointer */
+		/* last seg can be found through segs->prev pointer */
+		if (!cur->next)
 			KUNIT_ASSERT_PTR_EQ(test, cur, segs->prev);
-		}
 	}
 
+	KUNIT_ASSERT_EQ(test, i, tcase->nr_segs);
+
 	consume_skb(segs);
 free_gso_skb:
 	consume_skb(skb);
 }
 
 static struct kunit_case gso_test_cases[] = {
-	KUNIT_CASE(gso_test_func),
+	KUNIT_CASE_PARAM(gso_test_func, gso_test_gen_params),
 	{}
 };
 
-- 
2.42.0.582.g8ccd20d70d-goog

[PATCH net-next 3/3] net: expand skb_segment unit test with frag_list coverage

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Expand the test with these variants that use skb->frag_list:

- GSO_TEST_FRAG_LIST:             frag_skb length is gso_size
- GSO_TEST_FRAG_LIST_PURE:        same, data exclusively in frag skbs
- GSO_TEST_FRAG_LIST_NON_UNIFORM: frag_skb length may vary
- GSO_TEST_GSO_BY_FRAGS:          frag_skb length defines gso_size,
                                  i.e., segs may have varying sizes.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/core/gso_test.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/net/core/gso_test.c b/net/core/gso_test.c
index 41d1daa20831e..dc3ca31d11292 100644
--- a/net/core/gso_test.c
+++ b/net/core/gso_test.c
@@ -27,6 +27,10 @@ enum gso_test_nr {
 	GSO_TEST_FRAGS,
 	GSO_TEST_FRAGS_PURE,
 	GSO_TEST_GSO_PARTIAL,
+	GSO_TEST_FRAG_LIST,
+	GSO_TEST_FRAG_LIST_PURE,
+	GSO_TEST_FRAG_LIST_NON_UNIFORM,
+	GSO_TEST_GSO_BY_FRAGS,
 };
 
 struct gso_test_case {
@@ -37,6 +41,8 @@ struct gso_test_case {
 	unsigned int linear_len;
 	unsigned int nr_frags;
 	const unsigned int *frags;
+	unsigned int nr_frag_skbs;
+	const unsigned int *frag_skbs;
 
 	/* output as expected */
 	unsigned int nr_segs;
@@ -84,6 +90,48 @@ static struct gso_test_case cases[] = {
 		.nr_segs = 2,
 		.segs = (const unsigned int[]) { 2 * gso_size, 3 },
 	},
+	{
+		/* commit 89319d3801d1: frag_list on mss boundaries */
+		.id = GSO_TEST_FRAG_LIST,
+		.name = "frag_list",
+		.linear_len = gso_size,
+		.nr_frag_skbs = 2,
+		.frag_skbs = (const unsigned int[]) { gso_size, gso_size },
+		.nr_segs = 3,
+		.segs = (const unsigned int[]) { gso_size, gso_size, gso_size },
+	},
+	{
+		.id = GSO_TEST_FRAG_LIST_PURE,
+		.name = "frag_list_pure",
+		.nr_frag_skbs = 2,
+		.frag_skbs = (const unsigned int[]) { gso_size, gso_size },
+		.nr_segs = 2,
+		.segs = (const unsigned int[]) { gso_size, gso_size },
+	},
+	{
+		/* commit 43170c4e0ba7: GRO of frag_list trains */
+		.id = GSO_TEST_FRAG_LIST_NON_UNIFORM,
+		.name = "frag_list_non_uniform",
+		.linear_len = gso_size,
+		.nr_frag_skbs = 4,
+		.frag_skbs = (const unsigned int[]) { gso_size, 1, gso_size, 2 },
+		.nr_segs = 4,
+		.segs = (const unsigned int[]) { gso_size, gso_size, gso_size, 3 },
+	},
+	{
+		/* commit 3953c46c3ac7 ("sk_buff: allow segmenting based on frag sizes") and
+		 * commit 90017accff61 ("sctp: Add GSO support")
+		 *
+		 * "there will be a cover skb with protocol headers and
+		 *  children ones containing the actual segments"
+		 */
+		.id = GSO_TEST_GSO_BY_FRAGS,
+		.name = "gso_by_frags",
+		.nr_frag_skbs = 4,
+		.frag_skbs = (const unsigned int[]) { 100, 200, 300, 400 },
+		.nr_segs = 4,
+		.segs = (const unsigned int[]) { 100, 200, 300, 400 },
+	},
 };
 
 static void gso_test_case_to_desc(struct gso_test_case *t, char *desc)
@@ -131,10 +179,55 @@ static void gso_test_func(struct kunit *test)
 		skb->truesize += skb->data_len;
 	}
 
+	if (tcase->frag_skbs) {
+		unsigned int total_size = 0, total_true_size = 0;
+		unsigned int alloc_size = 0;
+		struct sk_buff *frag_skb, *prev = NULL;
+
+		page = alloc_page(GFP_KERNEL);
+		KUNIT_ASSERT_NOT_NULL(test, page);
+		page_ref_add(page, tcase->nr_frag_skbs - 1);
+
+		for (i = 0; i < tcase->nr_frag_skbs; i++) {
+			unsigned int frag_size;
+
+			frag_size = tcase->frag_skbs[i];
+			frag_skb = build_skb(page_address(page) + alloc_size,
+					     frag_size + shinfo_size);
+			KUNIT_ASSERT_NOT_NULL(test, frag_skb);
+			__skb_put(frag_skb, frag_size);
+
+			if (prev)
+				prev->next = frag_skb;
+			else
+				skb_shinfo(skb)->frag_list = frag_skb;
+			prev = frag_skb;
+
+			total_size += frag_size;
+			total_true_size += frag_skb->truesize;
+			alloc_size += frag_size + shinfo_size;
+		}
+
+		KUNIT_ASSERT_LE(test, alloc_size, PAGE_SIZE);
+
+		skb->len += total_size;
+		skb->data_len += total_size;
+		skb->truesize += total_true_size;
+
+		if (tcase->id == GSO_TEST_GSO_BY_FRAGS)
+			skb_shinfo(skb)->gso_size = GSO_BY_FRAGS;
+	}
+
 	features = NETIF_F_SG | NETIF_F_HW_CSUM;
 	if (tcase->id == GSO_TEST_GSO_PARTIAL)
 		features |= NETIF_F_GSO_PARTIAL;
 
+	/* TODO: this should also work with SG,
+	 * rather than hit BUG_ON(i >= nfrags)
+	 */
+	if (tcase->id == GSO_TEST_FRAG_LIST_NON_UNIFORM)
+		features &= ~NETIF_F_SG;
+
 	segs = skb_segment(skb, features);
 	if (IS_ERR(segs)) {
 		KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
-- 
2.42.0.582.g8ccd20d70d-goog

Re: [PATCH net-next 3/3] net: expand skb_segment unit test with frag_list coverage

[Florian Westphal]

Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> +	/* TODO: this should also work with SG,
> +	 * rather than hit BUG_ON(i >= nfrags)
> +	 */
> +	if (tcase->id == GSO_TEST_FRAG_LIST_NON_UNIFORM)
> +		features &= ~NETIF_F_SG;

Out of curiosity, this can't be hit outside of this test
because GRO is only source and won't generate skbs that
would trigger this, correct?

Re: [PATCH net-next 0/3] add skb_segment kunit coverage

[Florian Westphal]

Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> As discussed at netconf last week. Some kernel code is exercised in
> many different ways. skb_segment is a prime example. This ~350 line
> function has 49 different patches in git blame with 28 different
> authors.

Thanks for this work, and especially for adding commit ids
for relevant features that are tested.

Reviewed-by: Florian Westphal <fw@strlen.de>

Re: [PATCH net-next 0/3] add skb_segment kunit coverage

[Jakub Kicinski]

On Thu,  5 Oct 2023 09:48:54 -0400 Willem de Bruijn wrote:
> As discussed at netconf last week. Some kernel code is exercised in
> many different ways. skb_segment is a prime example. This ~350 line
> function has 49 different patches in git blame with 28 different
> authors.
> 
> When making a change, e.g., to fix a bug in one specific use case,
> it is hard to establish through analysis alone that the change does
> not break the many other paths through the code. It is impractical to
> exercise all code paths through regression testing from userspace.
> 
> Add the minimal infrastructure needed to add KUnit tests to networking,
> and add code coverage for this function.

Apparently we're supposed add descriptions to all modules now:

WARNING: modpost: missing MODULE_DESCRIPTION() in net/core/gso_test.o

Re: [PATCH net-next 3/3] net: expand skb_segment unit test with frag_list coverage

[Willem de Bruijn]

On Fri, Oct 6, 2023 at 5:14 PM Florian Westphal <fw@strlen.de> wrote:
>
> Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > +     /* TODO: this should also work with SG,
> > +      * rather than hit BUG_ON(i >= nfrags)
> > +      */
> > +     if (tcase->id == GSO_TEST_FRAG_LIST_NON_UNIFORM)
> > +             features &= ~NETIF_F_SG;
>
> Out of curiosity, this can't be hit outside of this test
> because GRO is only source and won't generate skbs that
> would trigger this, correct?

My understanding from commit 43170c4e0ba7 is that this happens when a
device allocates non-uniform strides in its rxq. If packets cover two
or more such strides, but otherwise are of gso_size, then GRO will
create GSO packets with a repeating stride pattern. That's a
generalization of the initial frag_list support, which required equal
size of gso_size.

That said, I may have misunderstood the behavior (given that that
seems to cause a panic without _SG). That's one of the difficulties of
defining tests after the fact: reverse engineering what it is that
this function actually is intended to cover, and no more.

Thanks for reviewing the series, Florian.

Re: [PATCH net-next 0/3] add skb_segment kunit coverage

[Willem de Bruijn]

On Fri, Oct 6, 2023 at 7:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu,  5 Oct 2023 09:48:54 -0400 Willem de Bruijn wrote:
> > As discussed at netconf last week. Some kernel code is exercised in
> > many different ways. skb_segment is a prime example. This ~350 line
> > function has 49 different patches in git blame with 28 different
> > authors.
> >
> > When making a change, e.g., to fix a bug in one specific use case,
> > it is hard to establish through analysis alone that the change does
> > not break the many other paths through the code. It is impractical to
> > exercise all code paths through regression testing from userspace.
> >
> > Add the minimal infrastructure needed to add KUnit tests to networking,
> > and add code coverage for this function.
>
> Apparently we're supposed add descriptions to all modules now:
>
> WARNING: modpost: missing MODULE_DESCRIPTION() in net/core/gso_test.o

Will fix and resend. Thanks.