* [Intel-wired-lan] [PATCH net-next v3 0/7] introduce DEFINE_FLEX() macro
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
with trailing flex array member.
Expose __struct_size() macro which reads size of data allocated
by DEFINE_FLEX().
Accompany new macros introduction with actual usage,
in the ice driver (hence targeting for netdev tree).
Obvious benefits include simpler resulting code, less heap usage,
less error checking. Less obvious is the fact that compiler has
more room to optimize, and as a whole, even with more stuff on the stack,
we end up with overall better (smaller) report from bloat-o-meter:
add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
(individual results in each patch).
v3: tidy up 1st patch
v2: Kees: reusing __struct_size() instead of doubling it as a new macro
Przemek Kitszel (7):
overflow: add DEFINE_FLEX() for on-stack allocs
ice: ice_sched_remove_elems: replace 1 elem array param by u32
ice: drop two params of ice_aq_move_sched_elems()
ice: make use of DEFINE_FLEX() in ice_ddp.c
ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
ice: make use of DEFINE_FLEX() in ice_switch.c
drivers/net/ethernet/intel/ice/ice_common.c | 20 ++-----
drivers/net/ethernet/intel/ice/ice_ddp.c | 39 ++++---------
drivers/net/ethernet/intel/ice/ice_lag.c | 48 ++++------------
drivers/net/ethernet/intel/ice/ice_lib.c | 23 ++------
drivers/net/ethernet/intel/ice/ice_sched.c | 56 ++++++------------
drivers/net/ethernet/intel/ice/ice_sched.h | 6 +-
drivers/net/ethernet/intel/ice/ice_switch.c | 63 +++++----------------
drivers/net/ethernet/intel/ice/ice_xsk.c | 22 +++----
include/linux/compiler_types.h | 32 +++++++----
include/linux/fortify-string.h | 4 --
include/linux/overflow.h | 20 +++++++
11 files changed, 115 insertions(+), 218 deletions(-)
base-commit: 479b322ee6feaff612285a0e7f22c022e8cd84eb
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH net-next v3 0/7] introduce DEFINE_FLEX() macro
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
with trailing flex array member.
Expose __struct_size() macro which reads size of data allocated
by DEFINE_FLEX().
Accompany new macros introduction with actual usage,
in the ice driver (hence targeting for netdev tree).
Obvious benefits include simpler resulting code, less heap usage,
less error checking. Less obvious is the fact that compiler has
more room to optimize, and as a whole, even with more stuff on the stack,
we end up with overall better (smaller) report from bloat-o-meter:
add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
(individual results in each patch).
v3: tidy up 1st patch
v2: Kees: reusing __struct_size() instead of doubling it as a new macro
Przemek Kitszel (7):
overflow: add DEFINE_FLEX() for on-stack allocs
ice: ice_sched_remove_elems: replace 1 elem array param by u32
ice: drop two params of ice_aq_move_sched_elems()
ice: make use of DEFINE_FLEX() in ice_ddp.c
ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
ice: make use of DEFINE_FLEX() in ice_switch.c
drivers/net/ethernet/intel/ice/ice_common.c | 20 ++-----
drivers/net/ethernet/intel/ice/ice_ddp.c | 39 ++++---------
drivers/net/ethernet/intel/ice/ice_lag.c | 48 ++++------------
drivers/net/ethernet/intel/ice/ice_lib.c | 23 ++------
drivers/net/ethernet/intel/ice/ice_sched.c | 56 ++++++------------
drivers/net/ethernet/intel/ice/ice_sched.h | 6 +-
drivers/net/ethernet/intel/ice/ice_switch.c | 63 +++++----------------
drivers/net/ethernet/intel/ice/ice_xsk.c | 22 +++----
include/linux/compiler_types.h | 32 +++++++----
include/linux/fortify-string.h | 4 --
include/linux/overflow.h | 20 +++++++
11 files changed, 115 insertions(+), 218 deletions(-)
base-commit: 479b322ee6feaff612285a0e7f22c022e8cd84eb
--
2.40.1
^ permalink raw reply [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Add DEFINE_FLEX() macro for on-stack allocations of structs with
flexible array member.
Expose __struct_size() macro outside of fortify-string.h, as it could be
used to read size of structs allocated by DEFINE_FLEX().
Move __member_size() alongside it.
-Kees
Using underlying array for on-stack storage lets us to declare
known-at-compile-time structures without kzalloc().
Actual usage for ice driver is in following patches of the series.
Missing __has_builtin() workaround is moved up to serve also assembly
compilation with m68k-linux-gcc, see [1].
Error was (note the .S file extension):
In file included from ../include/linux/linkage.h:5,
from ../arch/m68k/fpsp040/skeleton.S:40:
../include/linux/compiler_types.h:331:5: warning: "__has_builtin" is not defined, evaluates to 0 [-Wundef]
331 | #if __has_builtin(__builtin_dynamic_object_size)
| ^~~~~~~~~~~~~
../include/linux/compiler_types.h:331:18: error: missing binary operator before token "("
331 | #if __has_builtin(__builtin_dynamic_object_size)
| ^
[1] https://lore.kernel.org/netdev/202308112122.OuF0YZqL-lkp@intel.com/
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v3: remove old macro needlessly kept in v2; fix build warning;
reword doc comment;
v2: Kees: reuse __struct_size() instead of adding new macro
(adding Kees as Co-dev here);
v1: change macro name; add macro for size read;
accept struct type instead of ptr to it; change alignment;
---
include/linux/compiler_types.h | 32 +++++++++++++++++++++-----------
include/linux/fortify-string.h | 4 ----
include/linux/overflow.h | 20 ++++++++++++++++++++
3 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 547ea1ff806e..f706691fc5b9 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -2,6 +2,15 @@
#ifndef __LINUX_COMPILER_TYPES_H
#define __LINUX_COMPILER_TYPES_H
+/*
+ * __has_builtin is supported on gcc >= 10, clang >= 3 and icc >= 21.
+ * In the meantime, to support gcc < 10, we implement __has_builtin
+ * by hand.
+ */
+#ifndef __has_builtin
+#define __has_builtin(x) (0)
+#endif
+
#ifndef __ASSEMBLY__
/*
@@ -106,17 +115,6 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
#define __cold
#endif
-/* Builtins */
-
-/*
- * __has_builtin is supported on gcc >= 10, clang >= 3 and icc >= 21.
- * In the meantime, to support gcc < 10, we implement __has_builtin
- * by hand.
- */
-#ifndef __has_builtin
-#define __has_builtin(x) (0)
-#endif
-
/* Compiler specific macros. */
#ifdef __clang__
#include <linux/compiler-clang.h>
@@ -324,6 +322,18 @@ struct ftrace_likely_data {
# define __realloc_size(x, ...)
#endif
+/*
+ * When the size of an allocated object is needed, use the best available
+ * mechanism to find it. (For cases where sizeof() cannot be used.)
+ */
+#if __has_builtin(__builtin_dynamic_object_size)
+#define __struct_size(p) __builtin_dynamic_object_size(p, 0)
+#define __member_size(p) __builtin_dynamic_object_size(p, 1)
+#else
+#define __struct_size(p) __builtin_object_size(p, 0)
+#define __member_size(p) __builtin_object_size(p, 1)
+#endif
+
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
#endif
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index da51a83b2829..1e7711185ec6 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -93,13 +93,9 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
#if __has_builtin(__builtin_dynamic_object_size)
#define POS __pass_dynamic_object_size(1)
#define POS0 __pass_dynamic_object_size(0)
-#define __struct_size(p) __builtin_dynamic_object_size(p, 0)
-#define __member_size(p) __builtin_dynamic_object_size(p, 1)
#else
#define POS __pass_object_size(1)
#define POS0 __pass_object_size(0)
-#define __struct_size(p) __builtin_object_size(p, 0)
-#define __member_size(p) __builtin_object_size(p, 1)
#endif
#define __compiletime_lessthan(bounds, length) ( \
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index f9b60313eaea..34de97ea9e8e 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -309,4 +309,24 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
#define struct_size_t(type, member, count) \
struct_size((type *)NULL, member, count)
+/**
+ * DEFINE_FLEX() - Define an on-stack instance of structure with a trailing
+ * flexible array member.
+ *
+ * @type: structure type name, including "struct" keyword.
+ * @name: Name for a variable to define.
+ * @member: Name of the array member.
+ * @count: Number of elements in the array; must be compile-time const.
+ *
+ * Define a zeroed, on-stack, instance of @type structure with a trailing
+ * flexible array member.
+ * Use __struct_size(@name) to get compile-time size of it afterwards.
+ */
+#define DEFINE_FLEX(type, name, member, count) \
+ union { \
+ u8 bytes[struct_size_t(type, member, count)]; \
+ type obj; \
+ } name##_u __aligned(_Alignof(type)) = {}; \
+ type *name = (type *)&name##_u
+
#endif /* __LINUX_OVERFLOW_H */
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Add DEFINE_FLEX() macro for on-stack allocations of structs with
flexible array member.
Expose __struct_size() macro outside of fortify-string.h, as it could be
used to read size of structs allocated by DEFINE_FLEX().
Move __member_size() alongside it.
-Kees
Using underlying array for on-stack storage lets us to declare
known-at-compile-time structures without kzalloc().
Actual usage for ice driver is in following patches of the series.
Missing __has_builtin() workaround is moved up to serve also assembly
compilation with m68k-linux-gcc, see [1].
Error was (note the .S file extension):
In file included from ../include/linux/linkage.h:5,
from ../arch/m68k/fpsp040/skeleton.S:40:
../include/linux/compiler_types.h:331:5: warning: "__has_builtin" is not defined, evaluates to 0 [-Wundef]
331 | #if __has_builtin(__builtin_dynamic_object_size)
| ^~~~~~~~~~~~~
../include/linux/compiler_types.h:331:18: error: missing binary operator before token "("
331 | #if __has_builtin(__builtin_dynamic_object_size)
| ^
[1] https://lore.kernel.org/netdev/202308112122.OuF0YZqL-lkp@intel.com/
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
v3: remove old macro needlessly kept in v2; fix build warning;
reword doc comment;
v2: Kees: reuse __struct_size() instead of adding new macro
(adding Kees as Co-dev here);
v1: change macro name; add macro for size read;
accept struct type instead of ptr to it; change alignment;
---
include/linux/compiler_types.h | 32 +++++++++++++++++++++-----------
include/linux/fortify-string.h | 4 ----
include/linux/overflow.h | 20 ++++++++++++++++++++
3 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 547ea1ff806e..f706691fc5b9 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -2,6 +2,15 @@
#ifndef __LINUX_COMPILER_TYPES_H
#define __LINUX_COMPILER_TYPES_H
+/*
+ * __has_builtin is supported on gcc >= 10, clang >= 3 and icc >= 21.
+ * In the meantime, to support gcc < 10, we implement __has_builtin
+ * by hand.
+ */
+#ifndef __has_builtin
+#define __has_builtin(x) (0)
+#endif
+
#ifndef __ASSEMBLY__
/*
@@ -106,17 +115,6 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
#define __cold
#endif
-/* Builtins */
-
-/*
- * __has_builtin is supported on gcc >= 10, clang >= 3 and icc >= 21.
- * In the meantime, to support gcc < 10, we implement __has_builtin
- * by hand.
- */
-#ifndef __has_builtin
-#define __has_builtin(x) (0)
-#endif
-
/* Compiler specific macros. */
#ifdef __clang__
#include <linux/compiler-clang.h>
@@ -324,6 +322,18 @@ struct ftrace_likely_data {
# define __realloc_size(x, ...)
#endif
+/*
+ * When the size of an allocated object is needed, use the best available
+ * mechanism to find it. (For cases where sizeof() cannot be used.)
+ */
+#if __has_builtin(__builtin_dynamic_object_size)
+#define __struct_size(p) __builtin_dynamic_object_size(p, 0)
+#define __member_size(p) __builtin_dynamic_object_size(p, 1)
+#else
+#define __struct_size(p) __builtin_object_size(p, 0)
+#define __member_size(p) __builtin_object_size(p, 1)
+#endif
+
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
#endif
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index da51a83b2829..1e7711185ec6 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -93,13 +93,9 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
#if __has_builtin(__builtin_dynamic_object_size)
#define POS __pass_dynamic_object_size(1)
#define POS0 __pass_dynamic_object_size(0)
-#define __struct_size(p) __builtin_dynamic_object_size(p, 0)
-#define __member_size(p) __builtin_dynamic_object_size(p, 1)
#else
#define POS __pass_object_size(1)
#define POS0 __pass_object_size(0)
-#define __struct_size(p) __builtin_object_size(p, 0)
-#define __member_size(p) __builtin_object_size(p, 1)
#endif
#define __compiletime_lessthan(bounds, length) ( \
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index f9b60313eaea..34de97ea9e8e 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -309,4 +309,24 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
#define struct_size_t(type, member, count) \
struct_size((type *)NULL, member, count)
+/**
+ * DEFINE_FLEX() - Define an on-stack instance of structure with a trailing
+ * flexible array member.
+ *
+ * @type: structure type name, including "struct" keyword.
+ * @name: Name for a variable to define.
+ * @member: Name of the array member.
+ * @count: Number of elements in the array; must be compile-time const.
+ *
+ * Define a zeroed, on-stack, instance of @type structure with a trailing
+ * flexible array member.
+ * Use __struct_size(@name) to get compile-time size of it afterwards.
+ */
+#define DEFINE_FLEX(type, name, member, count) \
+ union { \
+ u8 bytes[struct_size_t(type, member, count)]; \
+ type obj; \
+ } name##_u __aligned(_Alignof(type)) = {}; \
+ type *name = (type *)&name##_u
+
#endif /* __LINUX_OVERFLOW_H */
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 2/7] ice: ice_sched_remove_elems: replace 1 elem array param by u32
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Replace array+size params of ice_sched_remove_elems:() by just single u32,
as all callers are using it with "1".
This enables moving from heap-based, to stack-based allocation, what is also
more elegant thanks to DEFINE_FLEX() macro.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 2/2 grow/shrink: 0/2 up/down: 252/-388 (-136)
---
drivers/net/ethernet/intel/ice/ice_sched.c | 26 ++++++++--------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index f4677704b95e..ca37252cac03 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -229,37 +229,29 @@ ice_aq_delete_sched_elems(struct ice_hw *hw, u16 grps_req,
* ice_sched_remove_elems - remove nodes from HW
* @hw: pointer to the HW struct
* @parent: pointer to the parent node
- * @num_nodes: number of nodes
- * @node_teids: array of node teids to be deleted
+ * @node_teid: node teid to be deleted
*
* This function remove nodes from HW
*/
static int
ice_sched_remove_elems(struct ice_hw *hw, struct ice_sched_node *parent,
- u16 num_nodes, u32 *node_teids)
+ u32 node_teid)
{
- struct ice_aqc_delete_elem *buf;
- u16 i, num_groups_removed = 0;
- u16 buf_size;
+ DEFINE_FLEX(struct ice_aqc_delete_elem, buf, teid, 1);
+ u16 buf_size = __struct_size(buf);
+ u16 num_groups_removed = 0;
int status;
- buf_size = struct_size(buf, teid, num_nodes);
- buf = devm_kzalloc(ice_hw_to_dev(hw), buf_size, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->hdr.parent_teid = parent->info.node_teid;
- buf->hdr.num_elems = cpu_to_le16(num_nodes);
- for (i = 0; i < num_nodes; i++)
- buf->teid[i] = cpu_to_le32(node_teids[i]);
+ buf->hdr.num_elems = cpu_to_le16(1);
+ buf->teid[0] = cpu_to_le32(node_teid);
status = ice_aq_delete_sched_elems(hw, 1, buf, buf_size,
&num_groups_removed, NULL);
if (status || num_groups_removed != 1)
ice_debug(hw, ICE_DBG_SCHED, "remove node failed FW error %d\n",
hw->adminq.sq_last_status);
- devm_kfree(ice_hw_to_dev(hw), buf);
return status;
}
@@ -326,7 +318,7 @@ void ice_free_sched_node(struct ice_port_info *pi, struct ice_sched_node *node)
node->info.data.elem_type != ICE_AQC_ELEM_TYPE_LEAF) {
u32 teid = le32_to_cpu(node->info.node_teid);
- ice_sched_remove_elems(hw, node->parent, 1, &teid);
+ ice_sched_remove_elems(hw, node->parent, teid);
}
parent = node->parent;
/* root has no parent */
@@ -1193,7 +1185,7 @@ static void ice_rm_dflt_leaf_node(struct ice_port_info *pi)
int status;
/* remove the default leaf node */
- status = ice_sched_remove_elems(pi->hw, node->parent, 1, &teid);
+ status = ice_sched_remove_elems(pi->hw, node->parent, teid);
if (!status)
ice_free_sched_node(pi, node);
}
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 2/7] ice: ice_sched_remove_elems: replace 1 elem array param by u32
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Replace array+size params of ice_sched_remove_elems:() by just single u32,
as all callers are using it with "1".
This enables moving from heap-based, to stack-based allocation, what is also
more elegant thanks to DEFINE_FLEX() macro.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 2/2 grow/shrink: 0/2 up/down: 252/-388 (-136)
---
drivers/net/ethernet/intel/ice/ice_sched.c | 26 ++++++++--------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index f4677704b95e..ca37252cac03 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -229,37 +229,29 @@ ice_aq_delete_sched_elems(struct ice_hw *hw, u16 grps_req,
* ice_sched_remove_elems - remove nodes from HW
* @hw: pointer to the HW struct
* @parent: pointer to the parent node
- * @num_nodes: number of nodes
- * @node_teids: array of node teids to be deleted
+ * @node_teid: node teid to be deleted
*
* This function remove nodes from HW
*/
static int
ice_sched_remove_elems(struct ice_hw *hw, struct ice_sched_node *parent,
- u16 num_nodes, u32 *node_teids)
+ u32 node_teid)
{
- struct ice_aqc_delete_elem *buf;
- u16 i, num_groups_removed = 0;
- u16 buf_size;
+ DEFINE_FLEX(struct ice_aqc_delete_elem, buf, teid, 1);
+ u16 buf_size = __struct_size(buf);
+ u16 num_groups_removed = 0;
int status;
- buf_size = struct_size(buf, teid, num_nodes);
- buf = devm_kzalloc(ice_hw_to_dev(hw), buf_size, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->hdr.parent_teid = parent->info.node_teid;
- buf->hdr.num_elems = cpu_to_le16(num_nodes);
- for (i = 0; i < num_nodes; i++)
- buf->teid[i] = cpu_to_le32(node_teids[i]);
+ buf->hdr.num_elems = cpu_to_le16(1);
+ buf->teid[0] = cpu_to_le32(node_teid);
status = ice_aq_delete_sched_elems(hw, 1, buf, buf_size,
&num_groups_removed, NULL);
if (status || num_groups_removed != 1)
ice_debug(hw, ICE_DBG_SCHED, "remove node failed FW error %d\n",
hw->adminq.sq_last_status);
- devm_kfree(ice_hw_to_dev(hw), buf);
return status;
}
@@ -326,7 +318,7 @@ void ice_free_sched_node(struct ice_port_info *pi, struct ice_sched_node *node)
node->info.data.elem_type != ICE_AQC_ELEM_TYPE_LEAF) {
u32 teid = le32_to_cpu(node->info.node_teid);
- ice_sched_remove_elems(hw, node->parent, 1, &teid);
+ ice_sched_remove_elems(hw, node->parent, teid);
}
parent = node->parent;
/* root has no parent */
@@ -1193,7 +1185,7 @@ static void ice_rm_dflt_leaf_node(struct ice_port_info *pi)
int status;
/* remove the default leaf node */
- status = ice_sched_remove_elems(pi->hw, node->parent, 1, &teid);
+ status = ice_sched_remove_elems(pi->hw, node->parent, teid);
if (!status)
ice_free_sched_node(pi, node);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 3/7] ice: drop two params of ice_aq_move_sched_elems()
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Remove two arguments of ice_aq_move_sched_elems().
Last of them was always NULL, and @grps_req was always 1.
Assuming @grps_req to be one, allows us to use DEFINE_FLEX() macro,
what removes some need for heap allocations.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/0 grow/shrink: 1/6 up/down: 46/-261 (-215)
---
drivers/net/ethernet/intel/ice/ice_lag.c | 48 ++++++----------------
drivers/net/ethernet/intel/ice/ice_sched.c | 30 ++++----------
drivers/net/ethernet/intel/ice/ice_sched.h | 6 +--
3 files changed, 23 insertions(+), 61 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index 36b7044717e8..0f3d01765c05 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -432,10 +432,11 @@ static void
ice_lag_move_vf_node_tc(struct ice_lag *lag, u8 oldport, u8 newport,
u16 vsi_num, u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
struct ice_hw *new_hw = NULL;
__le32 teid, parent_teid;
@@ -507,26 +508,17 @@ ice_lag_move_vf_node_tc(struct ice_lag *lag, u8 oldport, u8 newport,
goto resume_traffic;
/* Move Vf's VSI node for this TC to newport's scheduler tree */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc memory for VF node failover\n");
- goto resume_traffic;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for failover\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_traffic;
qbuf_err:
@@ -757,10 +749,11 @@ static void
ice_lag_reclaim_vf_tc(struct ice_lag *lag, struct ice_hw *src_hw, u16 vsi_num,
u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
__le32 teid, parent_teid;
struct ice_vsi_ctx *ctx;
@@ -822,26 +815,17 @@ ice_lag_reclaim_vf_tc(struct ice_lag *lag, struct ice_hw *src_hw, u16 vsi_num,
goto resume_reclaim;
/* Move node to new parent */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc memory for VF node failover\n");
- goto resume_reclaim;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for LAG reclaim\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_reclaim;
reclaim_qerr:
@@ -1797,10 +1781,11 @@ static void
ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw,
u16 vsi_num, u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
__le32 teid, parent_teid;
struct ice_vsi_ctx *ctx;
@@ -1858,26 +1843,17 @@ ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw,
goto resume_sync;
/* Move node to new parent */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc for VF node move in reset rebuild\n");
- goto resume_sync;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for LAG reset rebuild\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_sync;
sync_qerr:
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index ca37252cac03..df3d6e279de6 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -429,24 +429,20 @@ ice_aq_cfg_sched_elems(struct ice_hw *hw, u16 elems_req,
}
/**
- * ice_aq_move_sched_elems - move scheduler elements
+ * ice_aq_move_sched_elems - move scheduler element (just 1 group)
* @hw: pointer to the HW struct
- * @grps_req: number of groups to move
* @buf: pointer to buffer
* @buf_size: buffer size in bytes
* @grps_movd: returns total number of groups moved
- * @cd: pointer to command details structure or NULL
*
* Move scheduling elements (0x0408)
*/
int
-ice_aq_move_sched_elems(struct ice_hw *hw, u16 grps_req,
- struct ice_aqc_move_elem *buf, u16 buf_size,
- u16 *grps_movd, struct ice_sq_cd *cd)
+ice_aq_move_sched_elems(struct ice_hw *hw, struct ice_aqc_move_elem *buf,
+ u16 buf_size, u16 *grps_movd)
{
return ice_aqc_send_sched_elem_cmd(hw, ice_aqc_opc_move_sched_elems,
- grps_req, (void *)buf, buf_size,
- grps_movd, cd);
+ 1, buf, buf_size, grps_movd, NULL);
}
/**
@@ -2224,12 +2220,12 @@ int
ice_sched_move_nodes(struct ice_port_info *pi, struct ice_sched_node *parent,
u16 num_items, u32 *list)
{
- struct ice_aqc_move_elem *buf;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
+ u16 buf_len = __struct_size(buf);
struct ice_sched_node *node;
u16 i, grps_movd = 0;
struct ice_hw *hw;
int status = 0;
- u16 buf_len;
hw = pi->hw;
@@ -2241,35 +2237,27 @@ ice_sched_move_nodes(struct ice_port_info *pi, struct ice_sched_node *parent,
hw->max_children[parent->tx_sched_layer])
return -ENOSPC;
- buf_len = struct_size(buf, teid, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
for (i = 0; i < num_items; i++) {
node = ice_sched_find_node_by_teid(pi->root, list[i]);
if (!node) {
status = -EINVAL;
- goto move_err_exit;
+ break;
}
buf->hdr.src_parent_teid = node->info.parent_teid;
buf->hdr.dest_parent_teid = parent->info.node_teid;
buf->teid[0] = node->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
- status = ice_aq_move_sched_elems(hw, 1, buf, buf_len,
- &grps_movd, NULL);
+ status = ice_aq_move_sched_elems(hw, buf, buf_len, &grps_movd);
if (status && grps_movd != 1) {
status = -EIO;
- goto move_err_exit;
+ break;
}
/* update the SW DB */
ice_sched_update_parent(parent, node);
}
-move_err_exit:
- kfree(buf);
return status;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.h b/drivers/net/ethernet/intel/ice/ice_sched.h
index 8bd26353d76a..dc24bf55ff05 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.h
+++ b/drivers/net/ethernet/intel/ice/ice_sched.h
@@ -165,10 +165,8 @@ ice_sched_add_nodes_to_layer(struct ice_port_info *pi,
u16 *num_nodes_added);
void ice_sched_replay_agg_vsi_preinit(struct ice_hw *hw);
void ice_sched_replay_agg(struct ice_hw *hw);
-int
-ice_aq_move_sched_elems(struct ice_hw *hw, u16 grps_req,
- struct ice_aqc_move_elem *buf, u16 buf_size,
- u16 *grps_movd, struct ice_sq_cd *cd);
+int ice_aq_move_sched_elems(struct ice_hw *hw, struct ice_aqc_move_elem *buf,
+ u16 buf_size, u16 *grps_movd);
int ice_replay_vsi_agg(struct ice_hw *hw, u16 vsi_handle);
int ice_sched_replay_q_bw(struct ice_port_info *pi, struct ice_q_ctx *q_ctx);
#endif /* _ICE_SCHED_H_ */
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 3/7] ice: drop two params of ice_aq_move_sched_elems()
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Remove two arguments of ice_aq_move_sched_elems().
Last of them was always NULL, and @grps_req was always 1.
Assuming @grps_req to be one, allows us to use DEFINE_FLEX() macro,
what removes some need for heap allocations.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/0 grow/shrink: 1/6 up/down: 46/-261 (-215)
---
drivers/net/ethernet/intel/ice/ice_lag.c | 48 ++++++----------------
drivers/net/ethernet/intel/ice/ice_sched.c | 30 ++++----------
drivers/net/ethernet/intel/ice/ice_sched.h | 6 +--
3 files changed, 23 insertions(+), 61 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index 36b7044717e8..0f3d01765c05 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -432,10 +432,11 @@ static void
ice_lag_move_vf_node_tc(struct ice_lag *lag, u8 oldport, u8 newport,
u16 vsi_num, u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
struct ice_hw *new_hw = NULL;
__le32 teid, parent_teid;
@@ -507,26 +508,17 @@ ice_lag_move_vf_node_tc(struct ice_lag *lag, u8 oldport, u8 newport,
goto resume_traffic;
/* Move Vf's VSI node for this TC to newport's scheduler tree */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc memory for VF node failover\n");
- goto resume_traffic;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for failover\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_traffic;
qbuf_err:
@@ -757,10 +749,11 @@ static void
ice_lag_reclaim_vf_tc(struct ice_lag *lag, struct ice_hw *src_hw, u16 vsi_num,
u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
__le32 teid, parent_teid;
struct ice_vsi_ctx *ctx;
@@ -822,26 +815,17 @@ ice_lag_reclaim_vf_tc(struct ice_lag *lag, struct ice_hw *src_hw, u16 vsi_num,
goto resume_reclaim;
/* Move node to new parent */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc memory for VF node failover\n");
- goto resume_reclaim;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for LAG reclaim\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_reclaim;
reclaim_qerr:
@@ -1797,10 +1781,11 @@ static void
ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw,
u16 vsi_num, u8 tc)
{
- u16 numq, valq, buf_size, num_moved, qbuf_size;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
struct device *dev = ice_pf_to_dev(lag->pf);
+ u16 numq, valq, num_moved, qbuf_size;
+ u16 buf_size = __struct_size(buf);
struct ice_aqc_cfg_txqs_buf *qbuf;
- struct ice_aqc_move_elem *buf;
struct ice_sched_node *n_prt;
__le32 teid, parent_teid;
struct ice_vsi_ctx *ctx;
@@ -1858,26 +1843,17 @@ ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw,
goto resume_sync;
/* Move node to new parent */
- buf_size = struct_size(buf, teid, 1);
- buf = kzalloc(buf_size, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failure to alloc for VF node move in reset rebuild\n");
- goto resume_sync;
- }
-
buf->hdr.src_parent_teid = parent_teid;
buf->hdr.dest_parent_teid = n_prt->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
buf->teid[0] = teid;
- if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
- NULL))
+ if (ice_aq_move_sched_elems(&lag->pf->hw, buf, buf_size, &num_moved))
dev_warn(dev, "Failure to move VF nodes for LAG reset rebuild\n");
else
ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
- kfree(buf);
goto resume_sync;
sync_qerr:
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index ca37252cac03..df3d6e279de6 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -429,24 +429,20 @@ ice_aq_cfg_sched_elems(struct ice_hw *hw, u16 elems_req,
}
/**
- * ice_aq_move_sched_elems - move scheduler elements
+ * ice_aq_move_sched_elems - move scheduler element (just 1 group)
* @hw: pointer to the HW struct
- * @grps_req: number of groups to move
* @buf: pointer to buffer
* @buf_size: buffer size in bytes
* @grps_movd: returns total number of groups moved
- * @cd: pointer to command details structure or NULL
*
* Move scheduling elements (0x0408)
*/
int
-ice_aq_move_sched_elems(struct ice_hw *hw, u16 grps_req,
- struct ice_aqc_move_elem *buf, u16 buf_size,
- u16 *grps_movd, struct ice_sq_cd *cd)
+ice_aq_move_sched_elems(struct ice_hw *hw, struct ice_aqc_move_elem *buf,
+ u16 buf_size, u16 *grps_movd)
{
return ice_aqc_send_sched_elem_cmd(hw, ice_aqc_opc_move_sched_elems,
- grps_req, (void *)buf, buf_size,
- grps_movd, cd);
+ 1, buf, buf_size, grps_movd, NULL);
}
/**
@@ -2224,12 +2220,12 @@ int
ice_sched_move_nodes(struct ice_port_info *pi, struct ice_sched_node *parent,
u16 num_items, u32 *list)
{
- struct ice_aqc_move_elem *buf;
+ DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1);
+ u16 buf_len = __struct_size(buf);
struct ice_sched_node *node;
u16 i, grps_movd = 0;
struct ice_hw *hw;
int status = 0;
- u16 buf_len;
hw = pi->hw;
@@ -2241,35 +2237,27 @@ ice_sched_move_nodes(struct ice_port_info *pi, struct ice_sched_node *parent,
hw->max_children[parent->tx_sched_layer])
return -ENOSPC;
- buf_len = struct_size(buf, teid, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
for (i = 0; i < num_items; i++) {
node = ice_sched_find_node_by_teid(pi->root, list[i]);
if (!node) {
status = -EINVAL;
- goto move_err_exit;
+ break;
}
buf->hdr.src_parent_teid = node->info.parent_teid;
buf->hdr.dest_parent_teid = parent->info.node_teid;
buf->teid[0] = node->info.node_teid;
buf->hdr.num_elems = cpu_to_le16(1);
- status = ice_aq_move_sched_elems(hw, 1, buf, buf_len,
- &grps_movd, NULL);
+ status = ice_aq_move_sched_elems(hw, buf, buf_len, &grps_movd);
if (status && grps_movd != 1) {
status = -EIO;
- goto move_err_exit;
+ break;
}
/* update the SW DB */
ice_sched_update_parent(parent, node);
}
-move_err_exit:
- kfree(buf);
return status;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.h b/drivers/net/ethernet/intel/ice/ice_sched.h
index 8bd26353d76a..dc24bf55ff05 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.h
+++ b/drivers/net/ethernet/intel/ice/ice_sched.h
@@ -165,10 +165,8 @@ ice_sched_add_nodes_to_layer(struct ice_port_info *pi,
u16 *num_nodes_added);
void ice_sched_replay_agg_vsi_preinit(struct ice_hw *hw);
void ice_sched_replay_agg(struct ice_hw *hw);
-int
-ice_aq_move_sched_elems(struct ice_hw *hw, u16 grps_req,
- struct ice_aqc_move_elem *buf, u16 buf_size,
- u16 *grps_movd, struct ice_sq_cd *cd);
+int ice_aq_move_sched_elems(struct ice_hw *hw, struct ice_aqc_move_elem *buf,
+ u16 buf_size, u16 *grps_movd);
int ice_replay_vsi_agg(struct ice_hw *hw, u16 vsi_handle);
int ice_sched_replay_q_bw(struct ice_port_info *pi, struct ice_q_ctx *q_ctx);
#endif /* _ICE_SCHED_H_ */
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 4/7] ice: make use of DEFINE_FLEX() in ice_ddp.c
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Use DEFINE_FLEX() macro for constant-num-of-elems (4)
flex array members of ice_ddp.c
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 4/0 grow/shrink: 0/1 up/down: 1195/-878 (317)
---
drivers/net/ethernet/intel/ice/ice_ddp.c | 39 +++++++-----------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index d71ed210f9c4..0e64ecd6f7c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1558,21 +1558,14 @@ static enum ice_ddp_state ice_init_pkg_info(struct ice_hw *hw,
*/
static enum ice_ddp_state ice_get_pkg_info(struct ice_hw *hw)
{
- enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
- struct ice_aqc_get_pkg_info_resp *pkg_info;
- u16 size;
+ DEFINE_FLEX(struct ice_aqc_get_pkg_info_resp, pkg_info, pkg_info,
+ ICE_PKG_CNT);
+ u16 size = __struct_size(pkg_info);
u32 i;
- size = struct_size(pkg_info, pkg_info, ICE_PKG_CNT);
- pkg_info = kzalloc(size, GFP_KERNEL);
- if (!pkg_info)
+ if (ice_aq_get_pkg_info_list(hw, pkg_info, size, NULL))
return ICE_DDP_PKG_ERR;
- if (ice_aq_get_pkg_info_list(hw, pkg_info, size, NULL)) {
- state = ICE_DDP_PKG_ERR;
- goto init_pkg_free_alloc;
- }
-
for (i = 0; i < le32_to_cpu(pkg_info->count); i++) {
#define ICE_PKG_FLAG_COUNT 4
char flags[ICE_PKG_FLAG_COUNT + 1] = { 0 };
@@ -1602,10 +1595,7 @@ static enum ice_ddp_state ice_get_pkg_info(struct ice_hw *hw)
pkg_info->pkg_info[i].name, flags);
}
-init_pkg_free_alloc:
- kfree(pkg_info);
-
- return state;
+ return ICE_DDP_PKG_SUCCESS;
}
/**
@@ -1620,9 +1610,10 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
struct ice_pkg_hdr *ospkg,
struct ice_seg **seg)
{
- struct ice_aqc_get_pkg_info_resp *pkg;
+ DEFINE_FLEX(struct ice_aqc_get_pkg_info_resp, pkg, pkg_info,
+ ICE_PKG_CNT);
+ u16 size = __struct_size(pkg);
enum ice_ddp_state state;
- u16 size;
u32 i;
/* Check package version compatibility */
@@ -1641,15 +1632,8 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
}
/* Check if FW is compatible with the OS package */
- size = struct_size(pkg, pkg_info, ICE_PKG_CNT);
- pkg = kzalloc(size, GFP_KERNEL);
- if (!pkg)
- return ICE_DDP_PKG_ERR;
-
- if (ice_aq_get_pkg_info_list(hw, pkg, size, NULL)) {
- state = ICE_DDP_PKG_LOAD_ERROR;
- goto fw_ddp_compat_free_alloc;
- }
+ if (ice_aq_get_pkg_info_list(hw, pkg, size, NULL))
+ return ICE_DDP_PKG_LOAD_ERROR;
for (i = 0; i < le32_to_cpu(pkg->count); i++) {
/* loop till we find the NVM package */
@@ -1666,8 +1650,7 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
/* done processing NVM package so break */
break;
}
-fw_ddp_compat_free_alloc:
- kfree(pkg);
+
return state;
}
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 4/7] ice: make use of DEFINE_FLEX() in ice_ddp.c
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Use DEFINE_FLEX() macro for constant-num-of-elems (4)
flex array members of ice_ddp.c
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 4/0 grow/shrink: 0/1 up/down: 1195/-878 (317)
---
drivers/net/ethernet/intel/ice/ice_ddp.c | 39 +++++++-----------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index d71ed210f9c4..0e64ecd6f7c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1558,21 +1558,14 @@ static enum ice_ddp_state ice_init_pkg_info(struct ice_hw *hw,
*/
static enum ice_ddp_state ice_get_pkg_info(struct ice_hw *hw)
{
- enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS;
- struct ice_aqc_get_pkg_info_resp *pkg_info;
- u16 size;
+ DEFINE_FLEX(struct ice_aqc_get_pkg_info_resp, pkg_info, pkg_info,
+ ICE_PKG_CNT);
+ u16 size = __struct_size(pkg_info);
u32 i;
- size = struct_size(pkg_info, pkg_info, ICE_PKG_CNT);
- pkg_info = kzalloc(size, GFP_KERNEL);
- if (!pkg_info)
+ if (ice_aq_get_pkg_info_list(hw, pkg_info, size, NULL))
return ICE_DDP_PKG_ERR;
- if (ice_aq_get_pkg_info_list(hw, pkg_info, size, NULL)) {
- state = ICE_DDP_PKG_ERR;
- goto init_pkg_free_alloc;
- }
-
for (i = 0; i < le32_to_cpu(pkg_info->count); i++) {
#define ICE_PKG_FLAG_COUNT 4
char flags[ICE_PKG_FLAG_COUNT + 1] = { 0 };
@@ -1602,10 +1595,7 @@ static enum ice_ddp_state ice_get_pkg_info(struct ice_hw *hw)
pkg_info->pkg_info[i].name, flags);
}
-init_pkg_free_alloc:
- kfree(pkg_info);
-
- return state;
+ return ICE_DDP_PKG_SUCCESS;
}
/**
@@ -1620,9 +1610,10 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
struct ice_pkg_hdr *ospkg,
struct ice_seg **seg)
{
- struct ice_aqc_get_pkg_info_resp *pkg;
+ DEFINE_FLEX(struct ice_aqc_get_pkg_info_resp, pkg, pkg_info,
+ ICE_PKG_CNT);
+ u16 size = __struct_size(pkg);
enum ice_ddp_state state;
- u16 size;
u32 i;
/* Check package version compatibility */
@@ -1641,15 +1632,8 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
}
/* Check if FW is compatible with the OS package */
- size = struct_size(pkg, pkg_info, ICE_PKG_CNT);
- pkg = kzalloc(size, GFP_KERNEL);
- if (!pkg)
- return ICE_DDP_PKG_ERR;
-
- if (ice_aq_get_pkg_info_list(hw, pkg, size, NULL)) {
- state = ICE_DDP_PKG_LOAD_ERROR;
- goto fw_ddp_compat_free_alloc;
- }
+ if (ice_aq_get_pkg_info_list(hw, pkg, size, NULL))
+ return ICE_DDP_PKG_LOAD_ERROR;
for (i = 0; i < le32_to_cpu(pkg->count); i++) {
/* loop till we find the NVM package */
@@ -1666,8 +1650,7 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,
/* done processing NVM package so break */
break;
}
-fw_ddp_compat_free_alloc:
- kfree(pkg);
+
return state;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 5/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Use DEFINE_FLEX() macro for 1-elem flex array use case
of struct ice_aqc_add_tx_qgrp.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/2 grow/shrink: 2/2 up/down: 220/-255 (-35)
---
drivers/net/ethernet/intel/ice/ice_lib.c | 23 +++++------------------
drivers/net/ethernet/intel/ice/ice_xsk.c | 22 ++++++++--------------
2 files changed, 13 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 927518fcad51..c005ee1006f1 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1821,21 +1821,14 @@ int ice_vsi_cfg_single_rxq(struct ice_vsi *vsi, u16 q_idx)
int ice_vsi_cfg_single_txq(struct ice_vsi *vsi, struct ice_tx_ring **tx_rings, u16 q_idx)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
- int err;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
if (q_idx >= vsi->alloc_txq || !tx_rings || !tx_rings[q_idx])
return -EINVAL;
- qg_buf = kzalloc(struct_size(qg_buf, txqs, 1), GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
-
qg_buf->num_txqs = 1;
- err = ice_vsi_cfg_txq(vsi, tx_rings[q_idx], qg_buf);
- kfree(qg_buf);
- return err;
+ return ice_vsi_cfg_txq(vsi, tx_rings[q_idx], qg_buf);
}
/**
@@ -1877,24 +1870,18 @@ int ice_vsi_cfg_rxqs(struct ice_vsi *vsi)
static int
ice_vsi_cfg_txqs(struct ice_vsi *vsi, struct ice_tx_ring **rings, u16 count)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
- u16 q_idx = 0;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
int err = 0;
-
- qg_buf = kzalloc(struct_size(qg_buf, txqs, 1), GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
+ u16 q_idx;
qg_buf->num_txqs = 1;
for (q_idx = 0; q_idx < count; q_idx++) {
err = ice_vsi_cfg_txq(vsi, rings[q_idx], qg_buf);
if (err)
- goto err_cfg_txqs;
+ break;
}
-err_cfg_txqs:
- kfree(qg_buf);
return err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 2a3f0834e139..99954508184f 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -217,61 +217,55 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx)
*/
static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
+ u16 size = __struct_size(qg_buf);
struct ice_q_vector *q_vector;
struct ice_tx_ring *tx_ring;
struct ice_rx_ring *rx_ring;
- u16 size;
int err;
if (q_idx >= vsi->num_rxq || q_idx >= vsi->num_txq)
return -EINVAL;
- size = struct_size(qg_buf, txqs, 1);
- qg_buf = kzalloc(size, GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
-
qg_buf->num_txqs = 1;
tx_ring = vsi->tx_rings[q_idx];
rx_ring = vsi->rx_rings[q_idx];
q_vector = rx_ring->q_vector;
err = ice_vsi_cfg_txq(vsi, tx_ring, qg_buf);
if (err)
- goto free_buf;
+ return err;
if (ice_is_xdp_ena_vsi(vsi)) {
struct ice_tx_ring *xdp_ring = vsi->xdp_rings[q_idx];
memset(qg_buf, 0, size);
qg_buf->num_txqs = 1;
err = ice_vsi_cfg_txq(vsi, xdp_ring, qg_buf);
if (err)
- goto free_buf;
+ return err;
ice_set_ring_xdp(xdp_ring);
ice_tx_xsk_pool(vsi, q_idx);
}
err = ice_vsi_cfg_rxq(rx_ring);
if (err)
- goto free_buf;
+ return err;
ice_qvec_cfg_msix(vsi, q_vector);
err = ice_vsi_ctrl_one_rx_ring(vsi, true, q_idx, true);
if (err)
- goto free_buf;
+ return err;
clear_bit(ICE_CFG_BUSY, vsi->state);
ice_qvec_toggle_napi(vsi, q_vector, true);
ice_qvec_ena_irq(vsi, q_vector);
netif_tx_start_queue(netdev_get_tx_queue(vsi->netdev, q_idx));
-free_buf:
- kfree(qg_buf);
- return err;
+
+ return 0;
}
/**
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 5/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Use DEFINE_FLEX() macro for 1-elem flex array use case
of struct ice_aqc_add_tx_qgrp.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/2 grow/shrink: 2/2 up/down: 220/-255 (-35)
---
drivers/net/ethernet/intel/ice/ice_lib.c | 23 +++++------------------
drivers/net/ethernet/intel/ice/ice_xsk.c | 22 ++++++++--------------
2 files changed, 13 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 927518fcad51..c005ee1006f1 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1821,21 +1821,14 @@ int ice_vsi_cfg_single_rxq(struct ice_vsi *vsi, u16 q_idx)
int ice_vsi_cfg_single_txq(struct ice_vsi *vsi, struct ice_tx_ring **tx_rings, u16 q_idx)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
- int err;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
if (q_idx >= vsi->alloc_txq || !tx_rings || !tx_rings[q_idx])
return -EINVAL;
- qg_buf = kzalloc(struct_size(qg_buf, txqs, 1), GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
-
qg_buf->num_txqs = 1;
- err = ice_vsi_cfg_txq(vsi, tx_rings[q_idx], qg_buf);
- kfree(qg_buf);
- return err;
+ return ice_vsi_cfg_txq(vsi, tx_rings[q_idx], qg_buf);
}
/**
@@ -1877,24 +1870,18 @@ int ice_vsi_cfg_rxqs(struct ice_vsi *vsi)
static int
ice_vsi_cfg_txqs(struct ice_vsi *vsi, struct ice_tx_ring **rings, u16 count)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
- u16 q_idx = 0;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
int err = 0;
-
- qg_buf = kzalloc(struct_size(qg_buf, txqs, 1), GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
+ u16 q_idx;
qg_buf->num_txqs = 1;
for (q_idx = 0; q_idx < count; q_idx++) {
err = ice_vsi_cfg_txq(vsi, rings[q_idx], qg_buf);
if (err)
- goto err_cfg_txqs;
+ break;
}
-err_cfg_txqs:
- kfree(qg_buf);
return err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 2a3f0834e139..99954508184f 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -217,61 +217,55 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx)
*/
static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx)
{
- struct ice_aqc_add_tx_qgrp *qg_buf;
+ DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);
+ u16 size = __struct_size(qg_buf);
struct ice_q_vector *q_vector;
struct ice_tx_ring *tx_ring;
struct ice_rx_ring *rx_ring;
- u16 size;
int err;
if (q_idx >= vsi->num_rxq || q_idx >= vsi->num_txq)
return -EINVAL;
- size = struct_size(qg_buf, txqs, 1);
- qg_buf = kzalloc(size, GFP_KERNEL);
- if (!qg_buf)
- return -ENOMEM;
-
qg_buf->num_txqs = 1;
tx_ring = vsi->tx_rings[q_idx];
rx_ring = vsi->rx_rings[q_idx];
q_vector = rx_ring->q_vector;
err = ice_vsi_cfg_txq(vsi, tx_ring, qg_buf);
if (err)
- goto free_buf;
+ return err;
if (ice_is_xdp_ena_vsi(vsi)) {
struct ice_tx_ring *xdp_ring = vsi->xdp_rings[q_idx];
memset(qg_buf, 0, size);
qg_buf->num_txqs = 1;
err = ice_vsi_cfg_txq(vsi, xdp_ring, qg_buf);
if (err)
- goto free_buf;
+ return err;
ice_set_ring_xdp(xdp_ring);
ice_tx_xsk_pool(vsi, q_idx);
}
err = ice_vsi_cfg_rxq(rx_ring);
if (err)
- goto free_buf;
+ return err;
ice_qvec_cfg_msix(vsi, q_vector);
err = ice_vsi_ctrl_one_rx_ring(vsi, true, q_idx, true);
if (err)
- goto free_buf;
+ return err;
clear_bit(ICE_CFG_BUSY, vsi->state);
ice_qvec_toggle_napi(vsi, q_vector, true);
ice_qvec_ena_irq(vsi, q_vector);
netif_tx_start_queue(netdev_get_tx_queue(vsi->netdev, q_idx));
-free_buf:
- kfree(qg_buf);
- return err;
+
+ return 0;
}
/**
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 6/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Use DEFINE_FLEX() macro for 1-elem flex array use case
of struct ice_aqc_dis_txq_item.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/0 grow/shrink: 1/1 up/down: 9/-18 (-9)
---
drivers/net/ethernet/intel/ice/ice_common.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index a86255b529a0..158931424283 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4734,11 +4734,11 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
enum ice_disq_rst_src rst_src, u16 vmvf_num,
struct ice_sq_cd *cd)
{
- struct ice_aqc_dis_txq_item *qg_list;
+ DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);
+ u16 i, buf_size = __struct_size(qg_list);
struct ice_q_ctx *q_ctx;
int status = -ENOENT;
struct ice_hw *hw;
- u16 i, buf_size;
if (!pi || pi->port_state != ICE_SCHED_PORT_STATE_READY)
return -EIO;
@@ -4756,11 +4756,6 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
return -EIO;
}
- buf_size = struct_size(qg_list, q_id, 1);
- qg_list = kzalloc(buf_size, GFP_KERNEL);
- if (!qg_list)
- return -ENOMEM;
-
mutex_lock(&pi->sched_lock);
for (i = 0; i < num_queues; i++) {
@@ -4793,7 +4788,6 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
q_ctx->q_teid = ICE_INVAL_TEID;
}
mutex_unlock(&pi->sched_lock);
- kfree(qg_list);
return status;
}
@@ -4962,22 +4956,17 @@ int
ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid,
u16 *q_id)
{
- struct ice_aqc_dis_txq_item *qg_list;
+ DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);
+ u16 qg_size = __struct_size(qg_list);
struct ice_hw *hw;
int status = 0;
- u16 qg_size;
int i;
if (!pi || pi->port_state != ICE_SCHED_PORT_STATE_READY)
return -EIO;
hw = pi->hw;
- qg_size = struct_size(qg_list, q_id, 1);
- qg_list = kzalloc(qg_size, GFP_KERNEL);
- if (!qg_list)
- return -ENOMEM;
-
mutex_lock(&pi->sched_lock);
for (i = 0; i < count; i++) {
@@ -5002,7 +4991,6 @@ ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid,
}
mutex_unlock(&pi->sched_lock);
- kfree(qg_list);
return status;
}
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 6/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Use DEFINE_FLEX() macro for 1-elem flex array use case
of struct ice_aqc_dis_txq_item.
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 0/0 grow/shrink: 1/1 up/down: 9/-18 (-9)
---
drivers/net/ethernet/intel/ice/ice_common.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index a86255b529a0..158931424283 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4734,11 +4734,11 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
enum ice_disq_rst_src rst_src, u16 vmvf_num,
struct ice_sq_cd *cd)
{
- struct ice_aqc_dis_txq_item *qg_list;
+ DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);
+ u16 i, buf_size = __struct_size(qg_list);
struct ice_q_ctx *q_ctx;
int status = -ENOENT;
struct ice_hw *hw;
- u16 i, buf_size;
if (!pi || pi->port_state != ICE_SCHED_PORT_STATE_READY)
return -EIO;
@@ -4756,11 +4756,6 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
return -EIO;
}
- buf_size = struct_size(qg_list, q_id, 1);
- qg_list = kzalloc(buf_size, GFP_KERNEL);
- if (!qg_list)
- return -ENOMEM;
-
mutex_lock(&pi->sched_lock);
for (i = 0; i < num_queues; i++) {
@@ -4793,7 +4788,6 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues,
q_ctx->q_teid = ICE_INVAL_TEID;
}
mutex_unlock(&pi->sched_lock);
- kfree(qg_list);
return status;
}
@@ -4962,22 +4956,17 @@ int
ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid,
u16 *q_id)
{
- struct ice_aqc_dis_txq_item *qg_list;
+ DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);
+ u16 qg_size = __struct_size(qg_list);
struct ice_hw *hw;
int status = 0;
- u16 qg_size;
int i;
if (!pi || pi->port_state != ICE_SCHED_PORT_STATE_READY)
return -EIO;
hw = pi->hw;
- qg_size = struct_size(qg_list, q_id, 1);
- qg_list = kzalloc(qg_size, GFP_KERNEL);
- if (!qg_list)
- return -ENOMEM;
-
mutex_lock(&pi->sched_lock);
for (i = 0; i < count; i++) {
@@ -5002,7 +4991,6 @@ ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid,
}
mutex_unlock(&pi->sched_lock);
- kfree(qg_list);
return status;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [Intel-wired-lan] [PATCH net-next v3 7/7] ice: make use of DEFINE_FLEX() in ice_switch.c
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 14:06 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Przemek Kitszel, Steven Zou, intel-wired-lan, linux-hardening
Use DEFINE_FLEX() macro for 1-elem flex array members of ice_switch.c
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 2/2 grow/shrink: 3/6 up/down: 489/-470 (19)
---
drivers/net/ethernet/intel/ice/ice_switch.c | 63 +++++----------------
1 file changed, 14 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index a7afb612fe32..b5a1445ed256 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -1812,15 +1812,11 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
enum ice_sw_lkup_type lkup_type,
enum ice_adminq_opc opc)
{
- struct ice_aqc_alloc_free_res_elem *sw_buf;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
+ u16 buf_len = __struct_size(sw_buf);
struct ice_aqc_res_elem *vsi_ele;
- u16 buf_len;
int status;
- buf_len = struct_size(sw_buf, elem, 1);
- sw_buf = devm_kzalloc(ice_hw_to_dev(hw), buf_len, GFP_KERNEL);
- if (!sw_buf)
- return -ENOMEM;
sw_buf->num_elems = cpu_to_le16(1);
if (lkup_type == ICE_SW_LKUP_MAC ||
@@ -1840,25 +1836,22 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
sw_buf->res_type =
cpu_to_le16(ICE_AQC_RES_TYPE_VSI_LIST_PRUNE);
} else {
- status = -EINVAL;
- goto ice_aq_alloc_free_vsi_list_exit;
+ return -EINVAL;
}
if (opc == ice_aqc_opc_free_res)
sw_buf->elem[0].e.sw_resp = cpu_to_le16(*vsi_list_id);
status = ice_aq_alloc_free_res(hw, 1, sw_buf, buf_len, opc, NULL);
if (status)
- goto ice_aq_alloc_free_vsi_list_exit;
+ return status;
if (opc == ice_aqc_opc_alloc_res) {
vsi_ele = &sw_buf->elem[0];
*vsi_list_id = le16_to_cpu(vsi_ele->e.sw_resp);
}
-ice_aq_alloc_free_vsi_list_exit:
- devm_kfree(ice_hw_to_dev(hw), sw_buf);
- return status;
+ return 0;
}
/**
@@ -2088,24 +2081,18 @@ ice_aq_get_recipe_to_profile(struct ice_hw *hw, u32 profile_id, u8 *r_bitmap,
*/
int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
{
- struct ice_aqc_alloc_free_res_elem *sw_buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
+ u16 buf_len = __struct_size(sw_buf);
int status;
- buf_len = struct_size(sw_buf, elem, 1);
- sw_buf = kzalloc(buf_len, GFP_KERNEL);
- if (!sw_buf)
- return -ENOMEM;
-
sw_buf->num_elems = cpu_to_le16(1);
sw_buf->res_type = cpu_to_le16((ICE_AQC_RES_TYPE_RECIPE <<
ICE_AQC_RES_TYPE_S) |
ICE_AQC_RES_TYPE_FLAG_SHARED);
status = ice_aq_alloc_free_res(hw, 1, sw_buf, buf_len,
ice_aqc_opc_alloc_res, NULL);
if (!status)
*rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp);
- kfree(sw_buf);
return status;
}
@@ -4482,29 +4469,20 @@ int
ice_alloc_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
u16 *counter_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- /* Allocate resource */
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(num_items);
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
ICE_AQC_RES_TYPE_M) | alloc_shared);
status = ice_aq_alloc_free_res(hw, 1, buf, buf_len,
ice_aqc_opc_alloc_res, NULL);
if (status)
- goto exit;
+ return status;
*counter_id = le16_to_cpu(buf->elem[0].e.sw_resp);
-
-exit:
- kfree(buf);
return status;
}
@@ -4520,16 +4498,10 @@ int
ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
u16 counter_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- /* Free resource */
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(num_items);
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
ICE_AQC_RES_TYPE_M) | alloc_shared);
@@ -4540,7 +4512,6 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
if (status)
ice_debug(hw, ICE_DBG_SW, "counter resource could not be freed\n");
- kfree(buf);
return status;
}
@@ -4558,15 +4529,10 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
*/
int ice_share_res(struct ice_hw *hw, u16 type, u8 shared, u16 res_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(1);
if (shared)
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
@@ -4584,7 +4550,6 @@ int ice_share_res(struct ice_hw *hw, u16 type, u8 shared, u16 res_id)
ice_debug(hw, ICE_DBG_SW, "Could not set resource type %u id %u to %s\n",
type, res_id, shared ? "SHARED" : "DEDICATED");
- kfree(buf);
return status;
}
--
2.40.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH net-next v3 7/7] ice: make use of DEFINE_FLEX() in ice_switch.c
@ 2023-08-16 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-16 14:06 UTC (permalink / raw)
To: Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou, Przemek Kitszel
Use DEFINE_FLEX() macro for 1-elem flex array members of ice_switch.c
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
add/remove: 2/2 grow/shrink: 3/6 up/down: 489/-470 (19)
---
drivers/net/ethernet/intel/ice/ice_switch.c | 63 +++++----------------
1 file changed, 14 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index a7afb612fe32..b5a1445ed256 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -1812,15 +1812,11 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
enum ice_sw_lkup_type lkup_type,
enum ice_adminq_opc opc)
{
- struct ice_aqc_alloc_free_res_elem *sw_buf;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
+ u16 buf_len = __struct_size(sw_buf);
struct ice_aqc_res_elem *vsi_ele;
- u16 buf_len;
int status;
- buf_len = struct_size(sw_buf, elem, 1);
- sw_buf = devm_kzalloc(ice_hw_to_dev(hw), buf_len, GFP_KERNEL);
- if (!sw_buf)
- return -ENOMEM;
sw_buf->num_elems = cpu_to_le16(1);
if (lkup_type == ICE_SW_LKUP_MAC ||
@@ -1840,25 +1836,22 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
sw_buf->res_type =
cpu_to_le16(ICE_AQC_RES_TYPE_VSI_LIST_PRUNE);
} else {
- status = -EINVAL;
- goto ice_aq_alloc_free_vsi_list_exit;
+ return -EINVAL;
}
if (opc == ice_aqc_opc_free_res)
sw_buf->elem[0].e.sw_resp = cpu_to_le16(*vsi_list_id);
status = ice_aq_alloc_free_res(hw, 1, sw_buf, buf_len, opc, NULL);
if (status)
- goto ice_aq_alloc_free_vsi_list_exit;
+ return status;
if (opc == ice_aqc_opc_alloc_res) {
vsi_ele = &sw_buf->elem[0];
*vsi_list_id = le16_to_cpu(vsi_ele->e.sw_resp);
}
-ice_aq_alloc_free_vsi_list_exit:
- devm_kfree(ice_hw_to_dev(hw), sw_buf);
- return status;
+ return 0;
}
/**
@@ -2088,24 +2081,18 @@ ice_aq_get_recipe_to_profile(struct ice_hw *hw, u32 profile_id, u8 *r_bitmap,
*/
int ice_alloc_recipe(struct ice_hw *hw, u16 *rid)
{
- struct ice_aqc_alloc_free_res_elem *sw_buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
+ u16 buf_len = __struct_size(sw_buf);
int status;
- buf_len = struct_size(sw_buf, elem, 1);
- sw_buf = kzalloc(buf_len, GFP_KERNEL);
- if (!sw_buf)
- return -ENOMEM;
-
sw_buf->num_elems = cpu_to_le16(1);
sw_buf->res_type = cpu_to_le16((ICE_AQC_RES_TYPE_RECIPE <<
ICE_AQC_RES_TYPE_S) |
ICE_AQC_RES_TYPE_FLAG_SHARED);
status = ice_aq_alloc_free_res(hw, 1, sw_buf, buf_len,
ice_aqc_opc_alloc_res, NULL);
if (!status)
*rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp);
- kfree(sw_buf);
return status;
}
@@ -4482,29 +4469,20 @@ int
ice_alloc_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
u16 *counter_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- /* Allocate resource */
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(num_items);
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
ICE_AQC_RES_TYPE_M) | alloc_shared);
status = ice_aq_alloc_free_res(hw, 1, buf, buf_len,
ice_aqc_opc_alloc_res, NULL);
if (status)
- goto exit;
+ return status;
*counter_id = le16_to_cpu(buf->elem[0].e.sw_resp);
-
-exit:
- kfree(buf);
return status;
}
@@ -4520,16 +4498,10 @@ int
ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
u16 counter_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- /* Free resource */
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(num_items);
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
ICE_AQC_RES_TYPE_M) | alloc_shared);
@@ -4540,7 +4512,6 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
if (status)
ice_debug(hw, ICE_DBG_SW, "counter resource could not be freed\n");
- kfree(buf);
return status;
}
@@ -4558,15 +4529,10 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
*/
int ice_share_res(struct ice_hw *hw, u16 type, u8 shared, u16 res_id)
{
- struct ice_aqc_alloc_free_res_elem *buf;
- u16 buf_len;
+ DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);
+ u16 buf_len = __struct_size(buf);
int status;
- buf_len = struct_size(buf, elem, 1);
- buf = kzalloc(buf_len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
buf->num_elems = cpu_to_le16(1);
if (shared)
buf->res_type = cpu_to_le16(((type << ICE_AQC_RES_TYPE_S) &
@@ -4584,7 +4550,6 @@ int ice_share_res(struct ice_hw *hw, u16 type, u8 shared, u16 res_id)
ice_debug(hw, ICE_DBG_SW, "Could not set resource type %u id %u to %s\n",
type, res_id, shared ? "SHARED" : "DEDICATED");
- kfree(buf);
return status;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 16:35 ` kernel test robot
-1 siblings, 0 replies; 42+ messages in thread
From: kernel test robot @ 2023-08-16 16:35 UTC (permalink / raw)
To: Przemek Kitszel, Kees Cook, netdev
Cc: llvm, oe-kbuild-all, Jacob Keller, intel-wired-lan,
Alexander Lobakin, linux-hardening, Steven Zou, Przemek Kitszel
Hi Przemek,
kernel test robot noticed the following build errors:
[auto build test ERROR on 479b322ee6feaff612285a0e7f22c022e8cd84eb]
url: https://github.com/intel-lab-lkp/linux/commits/Przemek-Kitszel/overflow-add-DEFINE_FLEX-for-on-stack-allocs/20230816-221402
base: 479b322ee6feaff612285a0e7f22c022e8cd84eb
patch link: https://lore.kernel.org/r/20230816140623.452869-2-przemyslaw.kitszel%40intel.com
patch subject: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308170000.YqabIR9D-lkp@intel.com/
All errors (new ones prefixed by >>):
>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
stack backtrace:
0: rust_begin_unwind
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
1: core::panicking::panic_fmt
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
2: proc_macro2::fallback::Ident::_new
3: proc_macro2::Ident::new
4: bindgen::ir::context::BindgenContext::rust_ident
5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
13: bindgen::ir::context::BindgenContext::gen
14: bindgen::Builder::generate
15: bindgen::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
make[3]: *** [rust/Makefile:316: rust/uapi/uapi_generated.rs] Error 1
make[3]: *** Deleting file 'rust/uapi/uapi_generated.rs'
>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
stack backtrace:
0: rust_begin_unwind
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
1: core::panicking::panic_fmt
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
2: proc_macro2::fallback::Ident::_new
3: proc_macro2::Ident::new
4: bindgen::ir::context::BindgenContext::rust_ident
5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
13: bindgen::ir::context::BindgenContext::gen
14: bindgen::Builder::generate
15: bindgen::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
make[3]: *** [rust/Makefile:310: rust/bindings/bindings_generated.rs] Error 1
make[3]: *** Deleting file 'rust/bindings/bindings_generated.rs'
make[3]: Target 'rust/' not remade because of errors.
make[2]: *** [Makefile:1293: prepare] Error 2
make[1]: *** [Makefile:234: __sub-make] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:234: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-16 16:35 ` kernel test robot
0 siblings, 0 replies; 42+ messages in thread
From: kernel test robot @ 2023-08-16 16:35 UTC (permalink / raw)
To: Przemek Kitszel, Kees Cook, netdev
Cc: Przemek Kitszel, llvm, Steven Zou, intel-wired-lan,
linux-hardening, oe-kbuild-all
Hi Przemek,
kernel test robot noticed the following build errors:
[auto build test ERROR on 479b322ee6feaff612285a0e7f22c022e8cd84eb]
url: https://github.com/intel-lab-lkp/linux/commits/Przemek-Kitszel/overflow-add-DEFINE_FLEX-for-on-stack-allocs/20230816-221402
base: 479b322ee6feaff612285a0e7f22c022e8cd84eb
patch link: https://lore.kernel.org/r/20230816140623.452869-2-przemyslaw.kitszel%40intel.com
patch subject: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308170000.YqabIR9D-lkp@intel.com/
All errors (new ones prefixed by >>):
>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
stack backtrace:
0: rust_begin_unwind
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
1: core::panicking::panic_fmt
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
2: proc_macro2::fallback::Ident::_new
3: proc_macro2::Ident::new
4: bindgen::ir::context::BindgenContext::rust_ident
5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
13: bindgen::ir::context::BindgenContext::gen
14: bindgen::Builder::generate
15: bindgen::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
make[3]: *** [rust/Makefile:316: rust/uapi/uapi_generated.rs] Error 1
make[3]: *** Deleting file 'rust/uapi/uapi_generated.rs'
>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
stack backtrace:
0: rust_begin_unwind
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
1: core::panicking::panic_fmt
at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
2: proc_macro2::fallback::Ident::_new
3: proc_macro2::Ident::new
4: bindgen::ir::context::BindgenContext::rust_ident
5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
13: bindgen::ir::context::BindgenContext::gen
14: bindgen::Builder::generate
15: bindgen::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
make[3]: *** [rust/Makefile:310: rust/bindings/bindings_generated.rs] Error 1
make[3]: *** Deleting file 'rust/bindings/bindings_generated.rs'
make[3]: Target 'rust/' not remade because of errors.
make[2]: *** [Makefile:1293: prepare] Error 2
make[1]: *** [Makefile:234: __sub-make] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:234: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-16 20:38 ` Kees Cook
-1 siblings, 0 replies; 42+ messages in thread
From: Kees Cook @ 2023-08-16 20:38 UTC (permalink / raw)
To: Przemek Kitszel
Cc: netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
On Wed, Aug 16, 2023 at 10:06:17AM -0400, Przemek Kitszel wrote:
> Add DEFINE_FLEX() macro for on-stack allocations of structs with
> flexible array member.
>
> Expose __struct_size() macro outside of fortify-string.h, as it could be
> used to read size of structs allocated by DEFINE_FLEX().
> Move __member_size() alongside it.
> -Kees
>
> Using underlying array for on-stack storage lets us to declare
> known-at-compile-time structures without kzalloc().
>
> Actual usage for ice driver is in following patches of the series.
>
> Missing __has_builtin() workaround is moved up to serve also assembly
> compilation with m68k-linux-gcc, see [1].
> Error was (note the .S file extension):
> In file included from ../include/linux/linkage.h:5,
> from ../arch/m68k/fpsp040/skeleton.S:40:
> ../include/linux/compiler_types.h:331:5: warning: "__has_builtin" is not defined, evaluates to 0 [-Wundef]
> 331 | #if __has_builtin(__builtin_dynamic_object_size)
> | ^~~~~~~~~~~~~
> ../include/linux/compiler_types.h:331:18: error: missing binary operator before token "("
> 331 | #if __has_builtin(__builtin_dynamic_object_size)
> | ^
Looks good to me! Thanks for working on this. :)
Acked-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-16 20:38 ` Kees Cook
0 siblings, 0 replies; 42+ messages in thread
From: Kees Cook @ 2023-08-16 20:38 UTC (permalink / raw)
To: Przemek Kitszel; +Cc: netdev, Steven Zou, intel-wired-lan, linux-hardening
On Wed, Aug 16, 2023 at 10:06:17AM -0400, Przemek Kitszel wrote:
> Add DEFINE_FLEX() macro for on-stack allocations of structs with
> flexible array member.
>
> Expose __struct_size() macro outside of fortify-string.h, as it could be
> used to read size of structs allocated by DEFINE_FLEX().
> Move __member_size() alongside it.
> -Kees
>
> Using underlying array for on-stack storage lets us to declare
> known-at-compile-time structures without kzalloc().
>
> Actual usage for ice driver is in following patches of the series.
>
> Missing __has_builtin() workaround is moved up to serve also assembly
> compilation with m68k-linux-gcc, see [1].
> Error was (note the .S file extension):
> In file included from ../include/linux/linkage.h:5,
> from ../arch/m68k/fpsp040/skeleton.S:40:
> ../include/linux/compiler_types.h:331:5: warning: "__has_builtin" is not defined, evaluates to 0 [-Wundef]
> 331 | #if __has_builtin(__builtin_dynamic_object_size)
> | ^~~~~~~~~~~~~
> ../include/linux/compiler_types.h:331:18: error: missing binary operator before token "("
> 331 | #if __has_builtin(__builtin_dynamic_object_size)
> | ^
Looks good to me! Thanks for working on this. :)
Acked-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-16 14:06 ` Przemek Kitszel
@ 2023-08-17 14:35 ` David Laight
-1 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-17 14:35 UTC (permalink / raw)
To: 'Przemek Kitszel', Kees Cook, netdev
Cc: Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
From: Przemek Kitszel
> Sent: Wednesday, August 16, 2023 3:06 PM
>
> Using underlying array for on-stack storage lets us to declare
> known-at-compile-time structures without kzalloc().
Isn't DEFINE_FLEX() a bit misleading?
One thing it isn't is 'flexible' since it has a fixed size.
> +#define DEFINE_FLEX(type, name, member, count) \
> + union { \
> + u8 bytes[struct_size_t(type, member, count)]; \
> + type obj; \
> + } name##_u __aligned(_Alignof(type)) = {}; \
You shouldn't need the _Alignof() it is the default.
I'm not sure you should be forcing the memset() either.
> + type *name = (type *)&name##_u
How about?
type *const name = &name_##_u.obj;
You might want to add:
Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-17 14:35 ` David Laight
0 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-17 14:35 UTC (permalink / raw)
To: 'Przemek Kitszel', Kees Cook, netdev
Cc: intel-wired-lan, linux-hardening, Steven Zou
From: Przemek Kitszel
> Sent: Wednesday, August 16, 2023 3:06 PM
>
> Using underlying array for on-stack storage lets us to declare
> known-at-compile-time structures without kzalloc().
Isn't DEFINE_FLEX() a bit misleading?
One thing it isn't is 'flexible' since it has a fixed size.
> +#define DEFINE_FLEX(type, name, member, count) \
> + union { \
> + u8 bytes[struct_size_t(type, member, count)]; \
> + type obj; \
> + } name##_u __aligned(_Alignof(type)) = {}; \
You shouldn't need the _Alignof() it is the default.
I'm not sure you should be forcing the memset() either.
> + type *name = (type *)&name##_u
How about?
type *const name = &name_##_u.obj;
You might want to add:
Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-17 14:35 ` [Intel-wired-lan] " David Laight
@ 2023-08-17 17:00 ` Kees Cook
-1 siblings, 0 replies; 42+ messages in thread
From: Kees Cook @ 2023-08-17 17:00 UTC (permalink / raw)
To: David Laight
Cc: 'Przemek Kitszel',
netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
> From: Przemek Kitszel
> > Sent: Wednesday, August 16, 2023 3:06 PM
> >
> > Using underlying array for on-stack storage lets us to declare
> > known-at-compile-time structures without kzalloc().
>
> Isn't DEFINE_FLEX() a bit misleading?
> One thing it isn't is 'flexible' since it has a fixed size.
It works only on flex array structs, and defines a specific instance. I
think naming is okay here.
>
> > +#define DEFINE_FLEX(type, name, member, count) \
> > + union { \
> > + u8 bytes[struct_size_t(type, member, count)]; \
> > + type obj; \
> > + } name##_u __aligned(_Alignof(type)) = {}; \
>
> You shouldn't need the _Alignof() it is the default.
In the sense that since "type" is in the union, it's okay?
> I'm not sure you should be forcing the memset() either.
This already got discussed: better to fail safe.
>
> > + type *name = (type *)&name##_u
>
> How about?
> type *const name = &name_##_u.obj;
This is by design (see earlier threads) so that
__builtin_object_size(name, 1) will get the correct size. Otherwise it
doesn't include the FAM elements in the size.
>
> You might want to add:
> Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
That would be nice, though can Static_assert()s live in the middle of
variable definitions?
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-17 17:00 ` Kees Cook
0 siblings, 0 replies; 42+ messages in thread
From: Kees Cook @ 2023-08-17 17:00 UTC (permalink / raw)
To: David Laight
Cc: 'Przemek Kitszel',
Steven Zou, intel-wired-lan, linux-hardening, netdev
On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
> From: Przemek Kitszel
> > Sent: Wednesday, August 16, 2023 3:06 PM
> >
> > Using underlying array for on-stack storage lets us to declare
> > known-at-compile-time structures without kzalloc().
>
> Isn't DEFINE_FLEX() a bit misleading?
> One thing it isn't is 'flexible' since it has a fixed size.
It works only on flex array structs, and defines a specific instance. I
think naming is okay here.
>
> > +#define DEFINE_FLEX(type, name, member, count) \
> > + union { \
> > + u8 bytes[struct_size_t(type, member, count)]; \
> > + type obj; \
> > + } name##_u __aligned(_Alignof(type)) = {}; \
>
> You shouldn't need the _Alignof() it is the default.
In the sense that since "type" is in the union, it's okay?
> I'm not sure you should be forcing the memset() either.
This already got discussed: better to fail safe.
>
> > + type *name = (type *)&name##_u
>
> How about?
> type *const name = &name_##_u.obj;
This is by design (see earlier threads) so that
__builtin_object_size(name, 1) will get the correct size. Otherwise it
doesn't include the FAM elements in the size.
>
> You might want to add:
> Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
That would be nice, though can Static_assert()s live in the middle of
variable definitions?
-Kees
--
Kees Cook
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-17 17:00 ` [Intel-wired-lan] " Kees Cook
@ 2023-08-18 7:14 ` David Laight
-1 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-18 7:14 UTC (permalink / raw)
To: 'Kees Cook'
Cc: 'Przemek Kitszel',
netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
From: Kees Cook
> Sent: Thursday, August 17, 2023 6:00 PM
>
> On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
> > From: Przemek Kitszel
> > > Sent: Wednesday, August 16, 2023 3:06 PM
...
> > > +#define DEFINE_FLEX(type, name, member, count) \
> > > + union { \
> > > + u8 bytes[struct_size_t(type, member, count)]; \
> > > + type obj; \
> > > + } name##_u __aligned(_Alignof(type)) = {}; \
> >
> > You shouldn't need the _Alignof() it is the default.
>
> In the sense that since "type" is in the union, it's okay?
The alignment of the union is the larger of the alignments
of all its members.
Which is what you want.
> > I'm not sure you should be forcing the memset() either.
>
> This already got discussed: better to fail safe.
Perhaps call it DEFINE_FLEX_Z() to make this clear and
give the option for a non-zeroing version later.
Not everyone wants the expense of zeroing everything.
..
> > You might want to add:
> > Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
>
> That would be nice, though can Static_assert()s live in the middle of
> variable definitions?
I checked and it is fine.
(I double-checked by adding a statement and getting an error.)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 7:14 ` David Laight
0 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-18 7:14 UTC (permalink / raw)
To: 'Kees Cook'
Cc: 'Przemek Kitszel',
Steven Zou, intel-wired-lan, linux-hardening, netdev
From: Kees Cook
> Sent: Thursday, August 17, 2023 6:00 PM
>
> On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
> > From: Przemek Kitszel
> > > Sent: Wednesday, August 16, 2023 3:06 PM
...
> > > +#define DEFINE_FLEX(type, name, member, count) \
> > > + union { \
> > > + u8 bytes[struct_size_t(type, member, count)]; \
> > > + type obj; \
> > > + } name##_u __aligned(_Alignof(type)) = {}; \
> >
> > You shouldn't need the _Alignof() it is the default.
>
> In the sense that since "type" is in the union, it's okay?
The alignment of the union is the larger of the alignments
of all its members.
Which is what you want.
> > I'm not sure you should be forcing the memset() either.
>
> This already got discussed: better to fail safe.
Perhaps call it DEFINE_FLEX_Z() to make this clear and
give the option for a non-zeroing version later.
Not everyone wants the expense of zeroing everything.
..
> > You might want to add:
> > Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
>
> That would be nice, though can Static_assert()s live in the middle of
> variable definitions?
I checked and it is fine.
(I double-checked by adding a statement and getting an error.)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 7:14 ` [Intel-wired-lan] " David Laight
@ 2023-08-18 10:28 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-18 10:28 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Steven Zou, intel-wired-lan, linux-hardening
On 8/18/23 09:14, David Laight wrote:
> From: Kees Cook
>> Sent: Thursday, August 17, 2023 6:00 PM
>>
>> On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
>>> From: Przemek Kitszel
>>>> Sent: Wednesday, August 16, 2023 3:06 PM
> ...
>>>> +#define DEFINE_FLEX(type, name, member, count) \
>>>> + union { \
>>>> + u8 bytes[struct_size_t(type, member, count)]; \
>>>> + type obj; \
>>>> + } name##_u __aligned(_Alignof(type)) = {}; \
>>>
>>> You shouldn't need the _Alignof() it is the default.
>>
>> In the sense that since "type" is in the union, it's okay?
>
> The alignment of the union is the larger of the alignments
> of all its members.
> Which is what you want.
>
>>> I'm not sure you should be forcing the memset() either.
>>
>> This already got discussed: better to fail safe.
>
> Perhaps call it DEFINE_FLEX_Z() to make this clear and
> give the option for a non-zeroing version later.
> Not everyone wants the expense of zeroing everything.
per Kees, zeroing should be removed by compiler when not needed:
https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
Thanks for education on alignment!
>
> ..
>>> You might want to add:
>>> Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
>>
>> That would be nice, though can Static_assert()s live in the middle of
>> variable definitions?
>
> I checked and it is fine.
> (I double-checked by adding a statement and getting an error.)
Static_assert with nice wording definitively would make it better,
thanks again!
>
> David >
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 10:28 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-18 10:28 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
On 8/18/23 09:14, David Laight wrote:
> From: Kees Cook
>> Sent: Thursday, August 17, 2023 6:00 PM
>>
>> On Thu, Aug 17, 2023 at 02:35:23PM +0000, David Laight wrote:
>>> From: Przemek Kitszel
>>>> Sent: Wednesday, August 16, 2023 3:06 PM
> ...
>>>> +#define DEFINE_FLEX(type, name, member, count) \
>>>> + union { \
>>>> + u8 bytes[struct_size_t(type, member, count)]; \
>>>> + type obj; \
>>>> + } name##_u __aligned(_Alignof(type)) = {}; \
>>>
>>> You shouldn't need the _Alignof() it is the default.
>>
>> In the sense that since "type" is in the union, it's okay?
>
> The alignment of the union is the larger of the alignments
> of all its members.
> Which is what you want.
>
>>> I'm not sure you should be forcing the memset() either.
>>
>> This already got discussed: better to fail safe.
>
> Perhaps call it DEFINE_FLEX_Z() to make this clear and
> give the option for a non-zeroing version later.
> Not everyone wants the expense of zeroing everything.
per Kees, zeroing should be removed by compiler when not needed:
https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
Thanks for education on alignment!
>
> ..
>>> You might want to add:
>>> Static_assert(is_constexpr(count), "DEFINE_FLEX: non-constant count " #count);
>>
>> That would be nice, though can Static_assert()s live in the middle of
>> variable definitions?
>
> I checked and it is fine.
> (I double-checked by adding a statement and getting an error.)
Static_assert with nice wording definitively would make it better,
thanks again!
>
> David >
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-16 16:35 ` [Intel-wired-lan] " kernel test robot
@ 2023-08-18 10:37 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-18 10:37 UTC (permalink / raw)
To: kernel test robot, rust-for-linux
Cc: llvm, oe-kbuild-all, Jacob Keller, intel-wired-lan,
Alexander Lobakin, linux-hardening, Kees Cook, netdev
On 8/16/23 18:35, kernel test robot wrote:
> Hi Przemek,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 479b322ee6feaff612285a0e7f22c022e8cd84eb]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Przemek-Kitszel/overflow-add-DEFINE_FLEX-for-on-stack-allocs/20230816-221402
> base: 479b322ee6feaff612285a0e7f22c022e8cd84eb
> patch link: https://lore.kernel.org/r/20230816140623.452869-2-przemyslaw.kitszel%40intel.com
> patch subject: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/config)
Rust folks, could you please tell me if this is something I should fix,
or I just uncovered some existing bug in "unstable" thing?
Perhaps it is worth to mention, diff of v3 vs v2 is:
move dummy implementation of __has_builtin() macro to the top of
compiler_types.h, just before `#ifndef ASSEMBLY`
Przemek
> compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
> reproduce: (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202308170000.YqabIR9D-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
> stack backtrace:
> 0: rust_begin_unwind
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
> 1: core::panicking::panic_fmt
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
> 2: proc_macro2::fallback::Ident::_new
> 3: proc_macro2::Ident::new
> 4: bindgen::ir::context::BindgenContext::rust_ident
> 5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
> 12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 13: bindgen::ir::context::BindgenContext::gen
> 14: bindgen::Builder::generate
> 15: bindgen::main
> note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
> make[3]: *** [rust/Makefile:316: rust/uapi/uapi_generated.rs] Error 1
> make[3]: *** Deleting file 'rust/uapi/uapi_generated.rs'
>>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
> stack backtrace:
> 0: rust_begin_unwind
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
> 1: core::panicking::panic_fmt
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
> 2: proc_macro2::fallback::Ident::_new
> 3: proc_macro2::Ident::new
> 4: bindgen::ir::context::BindgenContext::rust_ident
> 5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
> 12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 13: bindgen::ir::context::BindgenContext::gen
> 14: bindgen::Builder::generate
> 15: bindgen::main
> note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
> make[3]: *** [rust/Makefile:310: rust/bindings/bindings_generated.rs] Error 1
> make[3]: *** Deleting file 'rust/bindings/bindings_generated.rs'
> make[3]: Target 'rust/' not remade because of errors.
> make[2]: *** [Makefile:1293: prepare] Error 2
> make[1]: *** [Makefile:234: __sub-make] Error 2
> make[1]: Target 'prepare' not remade because of errors.
> make: *** [Makefile:234: __sub-make] Error 2
> make: Target 'prepare' not remade because of errors.
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 10:37 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-18 10:37 UTC (permalink / raw)
To: kernel test robot, rust-for-linux
Cc: Kees Cook, netdev, llvm, intel-wired-lan, linux-hardening, oe-kbuild-all
On 8/16/23 18:35, kernel test robot wrote:
> Hi Przemek,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 479b322ee6feaff612285a0e7f22c022e8cd84eb]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Przemek-Kitszel/overflow-add-DEFINE_FLEX-for-on-stack-allocs/20230816-221402
> base: 479b322ee6feaff612285a0e7f22c022e8cd84eb
> patch link: https://lore.kernel.org/r/20230816140623.452869-2-przemyslaw.kitszel%40intel.com
> patch subject: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/config)
Rust folks, could you please tell me if this is something I should fix,
or I just uncovered some existing bug in "unstable" thing?
Perhaps it is worth to mention, diff of v3 vs v2 is:
move dummy implementation of __has_builtin() macro to the top of
compiler_types.h, just before `#ifndef ASSEMBLY`
Przemek
> compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
> reproduce: (https://download.01.org/0day-ci/archive/20230817/202308170000.YqabIR9D-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202308170000.YqabIR9D-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
> stack backtrace:
> 0: rust_begin_unwind
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
> 1: core::panicking::panic_fmt
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
> 2: proc_macro2::fallback::Ident::_new
> 3: proc_macro2::Ident::new
> 4: bindgen::ir::context::BindgenContext::rust_ident
> 5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
> 12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 13: bindgen::ir::context::BindgenContext::gen
> 14: bindgen::Builder::generate
> 15: bindgen::main
> note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
> make[3]: *** [rust/Makefile:316: rust/uapi/uapi_generated.rs] Error 1
> make[3]: *** Deleting file 'rust/uapi/uapi_generated.rs'
>>> thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at_include/linux/compiler_types_h_144_2)" is not a valid Ident', /opt/cross/rustc-1.68.2-bindgen-0.56.0/cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-1.0.24/src/fallback.rs:693:9
> stack backtrace:
> 0: rust_begin_unwind
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
> 1: core::panicking::panic_fmt
> at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
> 2: proc_macro2::fallback::Ident::_new
> 3: proc_macro2::Ident::new
> 4: bindgen::ir::context::BindgenContext::rust_ident
> 5: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 6: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 7: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 8: <bindgen::ir::comp::CompInfo as bindgen::codegen::CodeGenerator>::codegen
> 9: <bindgen::ir::ty::Type as bindgen::codegen::CodeGenerator>::codegen
> 10: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen
> 12: <bindgen::ir::item::Item as bindgen::codegen::CodeGenerator>::codegen
> 13: bindgen::ir::context::BindgenContext::gen
> 14: bindgen::Builder::generate
> 15: bindgen::main
> note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
> make[3]: *** [rust/Makefile:310: rust/bindings/bindings_generated.rs] Error 1
> make[3]: *** Deleting file 'rust/bindings/bindings_generated.rs'
> make[3]: Target 'rust/' not remade because of errors.
> make[2]: *** [Makefile:1293: prepare] Error 2
> make[1]: *** [Makefile:234: __sub-make] Error 2
> make[1]: Target 'prepare' not remade because of errors.
> make: *** [Makefile:234: __sub-make] Error 2
> make: Target 'prepare' not remade because of errors.
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 10:28 ` Przemek Kitszel
@ 2023-08-18 10:49 ` David Laight
-1 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-18 10:49 UTC (permalink / raw)
To: 'Przemek Kitszel', 'Kees Cook'
Cc: netdev, Steven Zou, intel-wired-lan, linux-hardening
From: Przemek Kitszel
> Sent: Friday, August 18, 2023 11:28 AM
...
> >>> I'm not sure you should be forcing the memset() either.
> >>
> >> This already got discussed: better to fail safe.
> >
> > Perhaps call it DEFINE_FLEX_Z() to make this clear and
> > give the option for a non-zeroing version later.
> > Not everyone wants the expense of zeroing everything.
>
> per Kees, zeroing should be removed by compiler when not needed:
> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
Expect in the most trivial cases the compiler is pretty much never
going to remove the zeroing of the data[] part.
I'm also not at all sure what happens if there is a function
call between the initialisation and any assignments.
With a bit of effort you should be able to pass the '= {}'
through into an inner #define.
Possibly with the alternative of a caller-provider
'= { .obj = call_supplied_initialiser }'
The 'not _Z' form would pass an empty argument.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 10:49 ` David Laight
0 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2023-08-18 10:49 UTC (permalink / raw)
To: 'Przemek Kitszel', 'Kees Cook'
Cc: netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
From: Przemek Kitszel
> Sent: Friday, August 18, 2023 11:28 AM
...
> >>> I'm not sure you should be forcing the memset() either.
> >>
> >> This already got discussed: better to fail safe.
> >
> > Perhaps call it DEFINE_FLEX_Z() to make this clear and
> > give the option for a non-zeroing version later.
> > Not everyone wants the expense of zeroing everything.
>
> per Kees, zeroing should be removed by compiler when not needed:
> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
Expect in the most trivial cases the compiler is pretty much never
going to remove the zeroing of the data[] part.
I'm also not at all sure what happens if there is a function
call between the initialisation and any assignments.
With a bit of effort you should be able to pass the '= {}'
through into an inner #define.
Possibly with the alternative of a caller-provider
'= { .obj = call_supplied_initialiser }'
The 'not _Z' form would pass an empty argument.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 10:37 ` [Intel-wired-lan] " Przemek Kitszel
@ 2023-08-18 11:10 ` Miguel Ojeda
-1 siblings, 0 replies; 42+ messages in thread
From: Miguel Ojeda @ 2023-08-18 11:10 UTC (permalink / raw)
To: Przemek Kitszel, kernel test robot, Yujie Liu, Philip Li, Greg KH
Cc: rust-for-linux, llvm, oe-kbuild-all, Jacob Keller,
intel-wired-lan, Alexander Lobakin, linux-hardening, Kees Cook,
netdev
On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
> Rust folks, could you please tell me if this is something I should fix,
> or I just uncovered some existing bug in "unstable" thing?
>
> Perhaps it is worth to mention, diff of v3 vs v2 is:
> move dummy implementation of __has_builtin() macro to the top of
> compiler_types.h, just before `#ifndef ASSEMBLY`
Nothing you need to worry about, it is an issue with old `bindgen` and
LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
0.65.1") which is in `rust-next` at the moment. Sorry about that, and
thanks for pinging us!
LKP / Yujie / Philip: since we got a few reports on this, would it be
possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
that does not include the new `bindgen` or at least 08ab786556ff? Or,
if Greg is OK with that, I guess we could also backport the upgrade,
but perhaps it is a bit too much for stable?
Cheers,
Miguel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 11:10 ` Miguel Ojeda
0 siblings, 0 replies; 42+ messages in thread
From: Miguel Ojeda @ 2023-08-18 11:10 UTC (permalink / raw)
To: Przemek Kitszel, kernel test robot, Yujie Liu, Philip Li, Greg KH
Cc: Kees Cook, rust-for-linux, netdev, llvm, intel-wired-lan,
linux-hardening, oe-kbuild-all
On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
> Rust folks, could you please tell me if this is something I should fix,
> or I just uncovered some existing bug in "unstable" thing?
>
> Perhaps it is worth to mention, diff of v3 vs v2 is:
> move dummy implementation of __has_builtin() macro to the top of
> compiler_types.h, just before `#ifndef ASSEMBLY`
Nothing you need to worry about, it is an issue with old `bindgen` and
LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
0.65.1") which is in `rust-next` at the moment. Sorry about that, and
thanks for pinging us!
LKP / Yujie / Philip: since we got a few reports on this, would it be
possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
that does not include the new `bindgen` or at least 08ab786556ff? Or,
if Greg is OK with that, I guess we could also backport the upgrade,
but perhaps it is a bit too much for stable?
Cheers,
Miguel
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 11:10 ` [Intel-wired-lan] " Miguel Ojeda
@ 2023-08-18 12:07 ` Philip Li
-1 siblings, 0 replies; 42+ messages in thread
From: Philip Li @ 2023-08-18 12:07 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Przemek Kitszel, kernel test robot, Yujie Liu, Greg KH,
rust-for-linux, llvm, oe-kbuild-all, Jacob Keller,
intel-wired-lan, Alexander Lobakin, linux-hardening, Kees Cook,
netdev
On Fri, Aug 18, 2023 at 01:10:07PM +0200, Miguel Ojeda wrote:
> On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
> <przemyslaw.kitszel@intel.com> wrote:
> >
> > Rust folks, could you please tell me if this is something I should fix,
> > or I just uncovered some existing bug in "unstable" thing?
> >
> > Perhaps it is worth to mention, diff of v3 vs v2 is:
> > move dummy implementation of __has_builtin() macro to the top of
> > compiler_types.h, just before `#ifndef ASSEMBLY`
>
> Nothing you need to worry about, it is an issue with old `bindgen` and
> LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
> 0.65.1") which is in `rust-next` at the moment. Sorry about that, and
> thanks for pinging us!
>
> LKP / Yujie / Philip: since we got a few reports on this, would it be
> possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
> that does not include the new `bindgen` or at least 08ab786556ff? Or,
Got it, we will update the bot to handle this to avoid further false
positive.
> if Greg is OK with that, I guess we could also backport the upgrade,
> but perhaps it is a bit too much for stable?
>
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-18 12:07 ` Philip Li
0 siblings, 0 replies; 42+ messages in thread
From: Philip Li @ 2023-08-18 12:07 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Yujie Liu, rust-for-linux, Przemek Kitszel, llvm, oe-kbuild-all,
netdev, intel-wired-lan, linux-hardening, Greg KH, Kees Cook
On Fri, Aug 18, 2023 at 01:10:07PM +0200, Miguel Ojeda wrote:
> On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
> <przemyslaw.kitszel@intel.com> wrote:
> >
> > Rust folks, could you please tell me if this is something I should fix,
> > or I just uncovered some existing bug in "unstable" thing?
> >
> > Perhaps it is worth to mention, diff of v3 vs v2 is:
> > move dummy implementation of __has_builtin() macro to the top of
> > compiler_types.h, just before `#ifndef ASSEMBLY`
>
> Nothing you need to worry about, it is an issue with old `bindgen` and
> LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
> 0.65.1") which is in `rust-next` at the moment. Sorry about that, and
> thanks for pinging us!
>
> LKP / Yujie / Philip: since we got a few reports on this, would it be
> possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
> that does not include the new `bindgen` or at least 08ab786556ff? Or,
Got it, we will update the bot to handle this to avoid further false
positive.
> if Greg is OK with that, I guess we could also backport the upgrade,
> but perhaps it is a bit too much for stable?
>
> Cheers,
> Miguel
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 11:10 ` [Intel-wired-lan] " Miguel Ojeda
@ 2023-08-19 10:06 ` Greg KH
-1 siblings, 0 replies; 42+ messages in thread
From: Greg KH @ 2023-08-19 10:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Przemek Kitszel, kernel test robot, Yujie Liu, Philip Li,
rust-for-linux, llvm, oe-kbuild-all, Jacob Keller,
intel-wired-lan, Alexander Lobakin, linux-hardening, Kees Cook,
netdev
On Fri, Aug 18, 2023 at 01:10:07PM +0200, Miguel Ojeda wrote:
> On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
> <przemyslaw.kitszel@intel.com> wrote:
> >
> > Rust folks, could you please tell me if this is something I should fix,
> > or I just uncovered some existing bug in "unstable" thing?
> >
> > Perhaps it is worth to mention, diff of v3 vs v2 is:
> > move dummy implementation of __has_builtin() macro to the top of
> > compiler_types.h, just before `#ifndef ASSEMBLY`
>
> Nothing you need to worry about, it is an issue with old `bindgen` and
> LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
> 0.65.1") which is in `rust-next` at the moment. Sorry about that, and
> thanks for pinging us!
>
> LKP / Yujie / Philip: since we got a few reports on this, would it be
> possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
> that does not include the new `bindgen` or at least 08ab786556ff? Or,
> if Greg is OK with that, I guess we could also backport the upgrade,
> but perhaps it is a bit too much for stable?
Commit is tiny enough for stable backports if it fixes a real issue that
everyone needs to address, so I have no objection to taking it for
stable releases once it hits Linus's tree.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-19 10:06 ` Greg KH
0 siblings, 0 replies; 42+ messages in thread
From: Greg KH @ 2023-08-19 10:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Yujie Liu, rust-for-linux, Przemek Kitszel, llvm, Philip Li,
netdev, intel-wired-lan, linux-hardening, oe-kbuild-all,
Kees Cook
On Fri, Aug 18, 2023 at 01:10:07PM +0200, Miguel Ojeda wrote:
> On Fri, Aug 18, 2023 at 12:38 PM Przemek Kitszel
> <przemyslaw.kitszel@intel.com> wrote:
> >
> > Rust folks, could you please tell me if this is something I should fix,
> > or I just uncovered some existing bug in "unstable" thing?
> >
> > Perhaps it is worth to mention, diff of v3 vs v2 is:
> > move dummy implementation of __has_builtin() macro to the top of
> > compiler_types.h, just before `#ifndef ASSEMBLY`
>
> Nothing you need to worry about, it is an issue with old `bindgen` and
> LLVM >= 16, fixed in commit 08ab786556ff ("rust: bindgen: upgrade to
> 0.65.1") which is in `rust-next` at the moment. Sorry about that, and
> thanks for pinging us!
>
> LKP / Yujie / Philip: since we got a few reports on this, would it be
> possible to avoid LLVM >= 16 for Rust-enabled builds for any branch
> that does not include the new `bindgen` or at least 08ab786556ff? Or,
> if Greg is OK with that, I guess we could also backport the upgrade,
> but perhaps it is a bit too much for stable?
Commit is tiny enough for stable backports if it fixes a real issue that
everyone needs to address, so I have no objection to taking it for
stable releases once it hits Linus's tree.
thanks,
greg k-h
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-18 10:49 ` David Laight
@ 2023-08-23 20:52 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-23 20:52 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Steven Zou, intel-wired-lan, linux-hardening
On 8/18/23 12:49, David Laight wrote:
> From: Przemek Kitszel
>> Sent: Friday, August 18, 2023 11:28 AM
> ...
>>>>> I'm not sure you should be forcing the memset() either.
>>>>
>>>> This already got discussed: better to fail safe.
>>>
>>> Perhaps call it DEFINE_FLEX_Z() to make this clear and
>>> give the option for a non-zeroing version later.
>>> Not everyone wants the expense of zeroing everything.
>>
>> per Kees, zeroing should be removed by compiler when not needed:
>> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
>
> Expect in the most trivial cases the compiler is pretty much never
> going to remove the zeroing of the data[] part.
>
> I'm also not at all sure what happens if there is a function
> call between the initialisation and any assignments.
>
> With a bit of effort you should be able to pass the '= {}'
> through into an inner #define.
> Possibly with the alternative of a caller-provider
> '= { .obj = call_supplied_initialiser }'
> The 'not _Z' form would pass an empty argument.
>
> David
Thanks, makes sense, there could be also DEFINE_FLEX_COUNTED
(or DEFINE_FLEX_BOUNDED) to cover Kees's __counted_by() cases.
Would you like me to cover/convert any existing code/use cases (as with
other patches in the series, to have some examples/actual usage of newly
introduced macros)?
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-23 20:52 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-23 20:52 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
On 8/18/23 12:49, David Laight wrote:
> From: Przemek Kitszel
>> Sent: Friday, August 18, 2023 11:28 AM
> ...
>>>>> I'm not sure you should be forcing the memset() either.
>>>>
>>>> This already got discussed: better to fail safe.
>>>
>>> Perhaps call it DEFINE_FLEX_Z() to make this clear and
>>> give the option for a non-zeroing version later.
>>> Not everyone wants the expense of zeroing everything.
>>
>> per Kees, zeroing should be removed by compiler when not needed:
>> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
>
> Expect in the most trivial cases the compiler is pretty much never
> going to remove the zeroing of the data[] part.
>
> I'm also not at all sure what happens if there is a function
> call between the initialisation and any assignments.
>
> With a bit of effort you should be able to pass the '= {}'
> through into an inner #define.
> Possibly with the alternative of a caller-provider
> '= { .obj = call_supplied_initialiser }'
> The 'not _Z' form would pass an empty argument.
>
> David
Thanks, makes sense, there could be also DEFINE_FLEX_COUNTED
(or DEFINE_FLEX_BOUNDED) to cover Kees's __counted_by() cases.
Would you like me to cover/convert any existing code/use cases (as with
other patches in the series, to have some examples/actual usage of newly
introduced macros)?
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
2023-08-23 20:52 ` Przemek Kitszel
@ 2023-08-28 14:41 ` Przemek Kitszel
-1 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-28 14:41 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Steven Zou, intel-wired-lan, linux-hardening
On 8/23/23 22:52, Przemek Kitszel wrote:
> On 8/18/23 12:49, David Laight wrote:
>> From: Przemek Kitszel
>>> Sent: Friday, August 18, 2023 11:28 AM
>> ...
>>>>>> I'm not sure you should be forcing the memset() either.
>>>>>
>>>>> This already got discussed: better to fail safe.
>>>>
>>>> Perhaps call it DEFINE_FLEX_Z() to make this clear and
>>>> give the option for a non-zeroing version later.
>>>> Not everyone wants the expense of zeroing everything.
>>>
>>> per Kees, zeroing should be removed by compiler when not needed:
>>> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
>>
>> Expect in the most trivial cases the compiler is pretty much never
>> going to remove the zeroing of the data[] part.
>>
>> I'm also not at all sure what happens if there is a function
>> call between the initialisation and any assignments.
>>
>> With a bit of effort you should be able to pass the '= {}'
>> through into an inner #define.
>> Possibly with the alternative of a caller-provider
>> '= { .obj = call_supplied_initialiser }'
>> The 'not _Z' form would pass an empty argument.
>>
>> David
>
> Thanks, makes sense, there could be also DEFINE_FLEX_COUNTED
> (or DEFINE_FLEX_BOUNDED) to cover Kees's __counted_by() cases.
>
> Would you like me to cover/convert any existing code/use cases (as with
> other patches in the series, to have some examples/actual usage of newly
> introduced macros)?
I did some manual searches and found no obvious candidate :/
will post next version/RFC without _NOINIT() variant.
>
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
>> MK1 1PT, UK
>> Registration No: 1397386 (Wales)
>
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs
@ 2023-08-28 14:41 ` Przemek Kitszel
0 siblings, 0 replies; 42+ messages in thread
From: Przemek Kitszel @ 2023-08-28 14:41 UTC (permalink / raw)
To: David Laight, 'Kees Cook'
Cc: netdev, Jacob Keller, intel-wired-lan, Alexander Lobakin,
linux-hardening, Steven Zou
On 8/23/23 22:52, Przemek Kitszel wrote:
> On 8/18/23 12:49, David Laight wrote:
>> From: Przemek Kitszel
>>> Sent: Friday, August 18, 2023 11:28 AM
>> ...
>>>>>> I'm not sure you should be forcing the memset() either.
>>>>>
>>>>> This already got discussed: better to fail safe.
>>>>
>>>> Perhaps call it DEFINE_FLEX_Z() to make this clear and
>>>> give the option for a non-zeroing version later.
>>>> Not everyone wants the expense of zeroing everything.
>>>
>>> per Kees, zeroing should be removed by compiler when not needed:
>>> https://lore.kernel.org/intel-wired-lan/202308101128.C4F0FA235@keescook/
>>
>> Expect in the most trivial cases the compiler is pretty much never
>> going to remove the zeroing of the data[] part.
>>
>> I'm also not at all sure what happens if there is a function
>> call between the initialisation and any assignments.
>>
>> With a bit of effort you should be able to pass the '= {}'
>> through into an inner #define.
>> Possibly with the alternative of a caller-provider
>> '= { .obj = call_supplied_initialiser }'
>> The 'not _Z' form would pass an empty argument.
>>
>> David
>
> Thanks, makes sense, there could be also DEFINE_FLEX_COUNTED
> (or DEFINE_FLEX_BOUNDED) to cover Kees's __counted_by() cases.
>
> Would you like me to cover/convert any existing code/use cases (as with
> other patches in the series, to have some examples/actual usage of newly
> introduced macros)?
I did some manual searches and found no obvious candidate :/
will post next version/RFC without _NOINIT() variant.
>
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
>> MK1 1PT, UK
>> Registration No: 1397386 (Wales)
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2023-08-28 14:41 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-16 14:06 [Intel-wired-lan] [PATCH net-next v3 0/7] introduce DEFINE_FLEX() macro Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 1/7] overflow: add DEFINE_FLEX() for on-stack allocs Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 16:35 ` kernel test robot
2023-08-16 16:35 ` [Intel-wired-lan] " kernel test robot
2023-08-18 10:37 ` Przemek Kitszel
2023-08-18 10:37 ` [Intel-wired-lan] " Przemek Kitszel
2023-08-18 11:10 ` Miguel Ojeda
2023-08-18 11:10 ` [Intel-wired-lan] " Miguel Ojeda
2023-08-18 12:07 ` Philip Li
2023-08-18 12:07 ` [Intel-wired-lan] " Philip Li
2023-08-19 10:06 ` Greg KH
2023-08-19 10:06 ` [Intel-wired-lan] " Greg KH
2023-08-16 20:38 ` Kees Cook
2023-08-16 20:38 ` [Intel-wired-lan] " Kees Cook
2023-08-17 14:35 ` David Laight
2023-08-17 14:35 ` [Intel-wired-lan] " David Laight
2023-08-17 17:00 ` Kees Cook
2023-08-17 17:00 ` [Intel-wired-lan] " Kees Cook
2023-08-18 7:14 ` David Laight
2023-08-18 7:14 ` [Intel-wired-lan] " David Laight
2023-08-18 10:28 ` Przemek Kitszel
2023-08-18 10:28 ` Przemek Kitszel
2023-08-18 10:49 ` [Intel-wired-lan] " David Laight
2023-08-18 10:49 ` David Laight
2023-08-23 20:52 ` [Intel-wired-lan] " Przemek Kitszel
2023-08-23 20:52 ` Przemek Kitszel
2023-08-28 14:41 ` [Intel-wired-lan] " Przemek Kitszel
2023-08-28 14:41 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 2/7] ice: ice_sched_remove_elems: replace 1 elem array param by u32 Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 3/7] ice: drop two params of ice_aq_move_sched_elems() Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 4/7] ice: make use of DEFINE_FLEX() in ice_ddp.c Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 5/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 6/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
2023-08-16 14:06 ` [Intel-wired-lan] [PATCH net-next v3 7/7] ice: make use of DEFINE_FLEX() in ice_switch.c Przemek Kitszel
2023-08-16 14:06 ` Przemek Kitszel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.