From 26dd68d293fd1c5ac966fb5dd5f6d89de322a541 Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:31 -0400 Subject: [PATCH 1/7] overflow: add DEFINE_FLEX() for on-stack allocs 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 Signed-off-by: Kees Cook Signed-off-by: Przemek Kitszel Link: https://lore.kernel.org/r/20230912115937.1645707-2-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- include/linux/compiler_types.h | 32 ++++++++++++++++++++----------- include/linux/fortify-string.h | 4 ---- include/linux/overflow.h | 35 ++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index c523c6683789..6f1ca49306d2 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__ /* @@ -134,17 +143,6 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { } # define __preserve_most #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 @@ -352,6 +350,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..7b5cf4a5cd19 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -309,4 +309,39 @@ 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() - helper macro for DEFINE_FLEX() family. + * Enables caller macro to pass (different) initializer. + * + * @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. + * @initializer: initializer expression (could be empty for no init). + */ +#define _DEFINE_FLEX(type, name, member, count, initializer) \ + _Static_assert(__builtin_constant_p(count), \ + "onstack flex array members require compile-time const count"); \ + union { \ + u8 bytes[struct_size_t(type, member, count)]; \ + type obj; \ + } name##_u initializer; \ + type *name = (type *)&name##_u + +/** + * 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) \ + _DEFINE_FLEX(type, name, member, count, = {}) + #endif /* __LINUX_OVERFLOW_H */ From ece285af77d00f7f4f9dd311f5cfa866a51c010f Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:32 -0400 Subject: [PATCH 2/7] ice: ice_sched_remove_elems: replace 1 elem array param by u32 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 Link: https://lore.kernel.org/r/20230912115937.1645707-3-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 c0533d7b66b9..efa5cb202eac 100644 --- a/drivers/net/ethernet/intel/ice/ice_sched.c +++ b/drivers/net/ethernet/intel/ice/ice_sched.c @@ -229,29 +229,22 @@ 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); @@ -259,7 +252,6 @@ ice_sched_remove_elems(struct ice_hw *hw, struct ice_sched_node *parent, 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); } From a034fcdbeaf7478ccbdec03650b07feaa6ecd51a Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:33 -0400 Subject: [PATCH 3/7] ice: drop two params of ice_aq_move_sched_elems() 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 Link: https://lore.kernel.org/r/20230912115937.1645707-4-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 4f39863b5537..2c96d1883e19 100644 --- a/drivers/net/ethernet/intel/ice/ice_lag.c +++ b/drivers/net/ethernet/intel/ice/ice_lag.c @@ -430,10 +430,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; @@ -505,26 +506,17 @@ qbuf_none: 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: @@ -755,10 +747,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; @@ -820,26 +813,17 @@ reclaim_none: 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: @@ -1792,10 +1776,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; @@ -1853,26 +1838,17 @@ sync_none: 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 efa5cb202eac..2f4a621254e8 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 0055d9330c07..1aef05ea5a57 100644 --- a/drivers/net/ethernet/intel/ice/ice_sched.h +++ b/drivers/net/ethernet/intel/ice/ice_sched.h @@ -161,10 +161,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_ */ From 230064baa43d7e3f13c9d5471350194e74f83601 Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:34 -0400 Subject: [PATCH 4/7] ice: make use of DEFINE_FLEX() in ice_ddp.c Use DEFINE_FLEX() macro for constant-num-of-elems (4) flex array members of ice_ddp.c Signed-off-by: Przemek Kitszel Link: https://lore.kernel.org/r/20230912115937.1645707-5-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 b27ec93638b6..78ed909745fe 100644 --- a/drivers/net/ethernet/intel/ice/ice_ddp.c +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c @@ -1560,21 +1560,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 }; @@ -1604,10 +1597,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; } /** @@ -1622,9 +1612,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 */ @@ -1643,15 +1634,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 */ @@ -1668,8 +1652,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; } From 43bba3b1664d303bfed3382bd3e6b6b48b8e5039 Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:35 -0400 Subject: [PATCH 5/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp Use DEFINE_FLEX() macro for 1-elem flex array use case of struct ice_aqc_add_tx_qgrp. Signed-off-by: Przemek Kitszel Link: https://lore.kernel.org/r/20230912115937.1645707-6-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 1549890a3cbf..aed84d369daf 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -1832,21 +1832,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); } /** @@ -1888,24 +1881,18 @@ setup_rings: 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,21 +217,16 @@ 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]; @@ -240,7 +235,7 @@ static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx) 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]; @@ -249,29 +244,28 @@ static int ice_qp_ena(struct ice_vsi *vsi, u16 q_idx) 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; } /** From 11dee3d611dd2bba0d9c91d8a8960e602b9944c0 Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:36 -0400 Subject: [PATCH 6/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item Use DEFINE_FLEX() macro for 1-elem flex array use case of struct ice_aqc_dis_txq_item. Signed-off-by: Przemek Kitszel Link: https://lore.kernel.org/r/20230912115937.1645707-7-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 5ed266fdc01b..963e2ffc1e2a 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -4790,11 +4790,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; @@ -4812,11 +4812,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++) { @@ -4849,7 +4844,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; } @@ -5018,10 +5012,10 @@ 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) @@ -5029,11 +5023,6 @@ ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid, 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++) { @@ -5058,7 +5047,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; } From e268b972270567e0414836919c5b9f8bc01651a4 Mon Sep 17 00:00:00 2001 From: Przemek Kitszel Date: Tue, 12 Sep 2023 07:59:37 -0400 Subject: [PATCH 7/7] ice: make use of DEFINE_FLEX() in ice_switch.c Use DEFINE_FLEX() macro for 1-elem flex array members of ice_switch.c Signed-off-by: Przemek Kitszel Link: https://lore.kernel.org/r/20230912115937.1645707-8-przemyslaw.kitszel@intel.com Signed-off-by: Jakub Kicinski --- 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 2f77b684ff76..ee19f3aa3d19 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,8 +1836,7 @@ 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) @@ -1849,16 +1844,14 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id, status = ice_aq_alloc_free_res(hw, sw_buf, buf_len, opc); 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,15 +2081,10 @@ 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) | @@ -2105,7 +2093,6 @@ int ice_alloc_recipe(struct ice_hw *hw, u16 *rid) ice_aqc_opc_alloc_res); if (!status) *rid = le16_to_cpu(sw_buf->elem[0].e.sw_resp); - kfree(sw_buf); return status; } @@ -4434,28 +4421,19 @@ 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, buf, buf_len, ice_aqc_opc_alloc_res); if (status) - goto exit; + return status; *counter_id = le16_to_cpu(buf->elem[0].e.sw_resp); - -exit: - kfree(buf); return status; } @@ -4471,16 +4449,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); @@ -4490,7 +4462,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; } @@ -4508,15 +4479,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) & @@ -4534,7 +4500,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; }