On Mon, 4 Sept 2023 at 21:22, benjamin@sipsolutions.net wrote:
From: Benjamin Berg benjamin.berg@intel.com
Add a simple convenience helper to allocate and zero fill an SKB for the use by a kunit test. Also provide a way to free it again in case that may be desirable.
This simply mirrors the kunit_kmalloc API.
Signed-off-by: Benjamin Berg benjamin.berg@intel.com
This _looks_ pretty good to me, but again, I'd like to see something use it, be it a simple test for these helpers, or a real-world test which takes advantage of them. Particularly since I've not had to use sk_buffs before, personally, so I'd love to see it actually working.
Otherwise, this seems okay to me. I'll hold off a final judgement until I've had a chance to actually run it, but I've left a few minor notes (mostly to myself) below.
Cheers, -- David
include/kunit/skbuff.h | 51 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 include/kunit/skbuff.h
diff --git a/include/kunit/skbuff.h b/include/kunit/skbuff.h new file mode 100644 index 000000000000..947fc8b5b48f --- /dev/null +++ b/include/kunit/skbuff.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- Base unit test (KUnit) API.
- Copyright (C) 2023 Intel Corporation
- */
+#ifndef _KUNIT_SKBUFF_H +#define _KUNIT_SKBUFF_H
Is it worth us hiding this behind #if IS_ENABLED(CONFIG_KUNIT)? I suspect not (we haven't bothered for resource.h, and only really do this for hooks/stubs).
+#include <kunit/resource.h> +#include <linux/skbuff.h>
+/**
- kunit_zalloc_skb() - Allocate and initialize a resource managed skb.
- @test: The test case to which the skb belongs
- @len: size to allocate
- Allocate a new struct sk_buff with GFP_KERNEL, zero fill the give length
- and add it as a resource to the kunit test for automatic cleanup.
- Returns: newly allocated SKB, or %NULL on error
- */
+static inline struct sk_buff *kunit_zalloc_skb(struct kunit *test, int len,
gfp_t gfp)+{
struct sk_buff *res = alloc_skb(len, GFP_KERNEL);if (!res || skb_pad(res, len))return NULL;
From a quick look, skb_pad() will free 'res' if it fails? If so, this is fine, if not we may need to move the add_action call below to prevent a leak.
if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree_skb, res))return NULL;
Just be warned that, while casting to kunit_action_t* is fine by me, some versions of clang are warning on any function pointer casts. So you can expect some whinge-y automatic emails from the kernel test robot and similar.
return res;+}
+/**
- kunit_kfree_skb() - Like kfree_skb except for allocations managed by KUnit.
- @test: The test case to which the resource belongs.
- @skb: The SKB to free.
- */
+static inline void kunit_kfree_skb(struct kunit *test, struct sk_buff *skb) +{
if (!skb)return;kunit_release_action(test, (kunit_action_t *)kfree_skb, (void *)skb);
As above, note that the kunit_action_t cast may cause warnings on some versions of clang. I'm personally okay with it, but if you want to write a wrapper to avoid it, that's fine by me, too.
+}
+#endif /* _KUNIT_SKBUFF_H */
2.41.0
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230904132139.103140-2-benjamin....