[Intel-wired-lan] [tnguy-next-queue:dev-queue 20/95] drivers/net/ethernet/intel/ice/ice_parser.c:971 __ice_pg_nm_cam_match() error: memcmp() '&item->key.val' too small (9 vs 13)
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
dev-queue
head: 2746dc17cbf54ef20cc09a7ec6862477a326fae1
commit: 678aebbf361736b67e2d83264fb1f2b49ecf6bc0 [20/95] ice: add parser
internal helper functions
config: x86_64-randconfig-161-20240609
(https://download.01.org/0day-ci/archive/20240610/202406100753.38qaqzo9-...@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202406100753.38qaqzo9-...@intel.com/
smatch warnings:
drivers/net/ethernet/intel/ice/ice_parser.c:971 __ice_pg_nm_cam_match() error:
memcmp() '&item->key.val' too small (9 vs 13)
vim +971 drivers/net/ethernet/intel/ice/ice_parser.c
678aebbf361736 Junfeng Guo 2024-05-27 967 static bool
__ice_pg_nm_cam_match(struct ice_pg_nm_cam_item *item,
678aebbf361736 Junfeng Guo 2024-05-27 968
struct ice_pg_cam_key *key)
678aebbf361736 Junfeng Guo 2024-05-27 969 {
678aebbf361736 Junfeng Guo 2024-05-27 970 return (item->key.valid &&
678aebbf361736 Junfeng Guo 2024-05-27 @971 !memcmp(&item->key.val,
&key->val, sizeof(key->val)));
This will read beyond the end of the "item->key.val". They're similar
structs but key->val has u32 next_proto; at the end.
678aebbf361736 Junfeng Guo 2024-05-27 972 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[Intel-wired-lan] [tnguy-next-queue:dev-queue 12/92] drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c:875 ice_vc_fdir_parse_raw() warn: missing error code 'status'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
dev-queue
head: d051d5283b530af3f9627c8e600aa581bc6c1958
commit: 23eb6f4d18624231cd3f57f322814591f38b3837 [12/92] ice: enable FDIR
filters from raw binary patterns for VFs
config: x86_64-randconfig-161-20240706
(https://download.01.org/0day-ci/archive/20240707/202407070634.atz9naa1-...@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202407070634.atz9naa1-...@intel.com/
smatch warnings:
drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c:875 ice_vc_fdir_parse_raw()
warn: missing error code 'status'
vim +/status +875 drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
23eb6f4d186242 Junfeng Guo 2024-05-27 831 static int
23eb6f4d186242 Junfeng Guo 2024-05-27 832 ice_vc_fdir_parse_raw(struct ice_vf
*vf,
23eb6f4d186242 Junfeng Guo 2024-05-27 833struct
virtchnl_proto_hdrs *proto,
23eb6f4d186242 Junfeng Guo 2024-05-27 834struct
virtchnl_fdir_fltr_conf *conf)
23eb6f4d186242 Junfeng Guo 2024-05-27 835 {
23eb6f4d186242 Junfeng Guo 2024-05-27 836 u8 *pkt_buf, *msk_buf
__free(kfree);
23eb6f4d186242 Junfeng Guo 2024-05-27 837 struct ice_parser_result rslt;
23eb6f4d186242 Junfeng Guo 2024-05-27 838 struct ice_pf *pf = vf->pf;
23eb6f4d186242 Junfeng Guo 2024-05-27 839 struct ice_parser *psr;
23eb6f4d186242 Junfeng Guo 2024-05-27 840 int status = -ENOMEM;
23eb6f4d186242 Junfeng Guo 2024-05-27 841 struct ice_hw *hw;
23eb6f4d186242 Junfeng Guo 2024-05-27 842 u16 udp_port = 0;
23eb6f4d186242 Junfeng Guo 2024-05-27 843
23eb6f4d186242 Junfeng Guo 2024-05-27 844 pkt_buf =
kzalloc(proto->raw.pkt_len, GFP_KERNEL);
23eb6f4d186242 Junfeng Guo 2024-05-27 845 msk_buf =
kzalloc(proto->raw.pkt_len, GFP_KERNEL);
23eb6f4d186242 Junfeng Guo 2024-05-27 846 if (!pkt_buf || !msk_buf)
23eb6f4d186242 Junfeng Guo 2024-05-27 847 goto err_mem_alloc;
23eb6f4d186242 Junfeng Guo 2024-05-27 848
23eb6f4d186242 Junfeng Guo 2024-05-27 849 memcpy(pkt_buf,
proto->raw.spec, proto->raw.pkt_len);
23eb6f4d186242 Junfeng Guo 2024-05-27 850 memcpy(msk_buf,
proto->raw.mask, proto->raw.pkt_len);
23eb6f4d186242 Junfeng Guo 2024-05-27 851
23eb6f4d186242 Junfeng Guo 2024-05-27 852 hw = &pf->hw;
23eb6f4d186242 Junfeng Guo 2024-05-27 853
23eb6f4d186242 Junfeng Guo 2024-05-27 854 /* Get raw profile info via
Parser Lib */
23eb6f4d186242 Junfeng Guo 2024-05-27 855 psr = ice_parser_create(hw);
23eb6f4d186242 Junfeng Guo 2024-05-27 856 if (IS_ERR(psr)) {
23eb6f4d186242 Junfeng Guo 2024-05-27 857 status = PTR_ERR(psr);
23eb6f4d186242 Junfeng Guo 2024-05-27 858 goto err_mem_alloc;
23eb6f4d186242 Junfeng Guo 2024-05-27 859 }
23eb6f4d186242 Junfeng Guo 2024-05-27 860
23eb6f4d186242 Junfeng Guo 2024-05-27 861 ice_parser_dvm_set(psr,
ice_is_dvm_ena(hw));
23eb6f4d186242 Junfeng Guo 2024-05-27 862
23eb6f4d186242 Junfeng Guo 2024-05-27 863 if
(ice_get_open_tunnel_port(hw, &udp_port, TNL_VXLAN))
23eb6f4d186242 Junfeng Guo 2024-05-27 864
ice_parser_vxlan_tunnel_set(psr, udp_port, true);
23eb6f4d186242 Junfeng Guo 2024-05-27 865
23eb6f4d186242 Junfeng Guo 2024-05-27 866 status = ice_parser_run(psr,
pkt_buf, proto->raw.pkt_len, &rslt);
23eb6f4d186242 Junfeng Guo 2024-05-27 867 if (status)
23eb6f4d186242 Junfeng Guo 2024-05-27 868 goto err_parser_destroy;
23eb6f4d186242 Junfeng Guo 2024-05-27 869
23eb6f4d186242 Junfeng Guo 2024-05-27 870 if (hw->debug_mask &
ICE_DBG_PARSER)
23eb6f4d186242 Junfeng Guo 2024-05-27 871
ice_parser_result_dump(hw, &rslt);
23eb6f4d186242 Junfeng Guo 2024-05-27 872
23eb6f4d186242 Junfeng Guo 2024-05-27 873 conf->prof =
kzalloc(sizeof(*conf->prof), GFP_KERNEL);
23eb6f4d186242 Junfeng Guo 2024-05-27 874 if (!conf->prof)
23eb6f4d186242 Junfeng Guo 2024-05-27 @875 goto err_parser_destroy;
status = -ENOMEM;
23eb6f4d186242 Junfeng Guo 2024-05-27 876
23eb6f4d186242 Junfeng Guo 2024-05-27 877 status =
ice_parser_profile_init(&rslt, pkt_buf, msk_buf,
23eb6f4d186242 Junfeng Guo 2024-05-27 878
proto->raw.pkt_len, ICE_BLK_FD,
23eb6f4d186242 Junfeng Guo 2024-05-27 879
conf->prof);
23eb6f4d186242 Junfeng Guo 2024-05-27 880 if (status)
23eb6f4d186242 Junfeng Guo 2024-05-27 881 goto
err_parser_profile_init;
23eb6f4d186242 Junfeng Guo 2024-05-27 882
23eb6f4d186242 Junfeng Guo 2024-05-27 883 i
[Intel-wired-lan] [bug report] idpf: add core init and interrupt request
Hello Pavan Kumar Linga,
Commit 4930fbf419a7 ("idpf: add core init and interrupt request")
from Aug 7, 2023 (linux-next), leads to the following Smatch static
checker warning:
drivers/net/ethernet/intel/idpf/idpf_lib.c:417 idpf_intr_req()
error: we previously assumed 'adapter->req_vec_chunks' could be null
(see line 360)
drivers/net/ethernet/intel/idpf/idpf_lib.c
315 int idpf_intr_req(struct idpf_adapter *adapter)
316 {
317 u16 default_vports = idpf_get_default_vports(adapter);
318 int num_q_vecs, total_vecs, num_vec_ids;
319 int min_vectors, v_actual, err;
320 unsigned int vector;
321 u16 *vecids;
322
323 total_vecs = idpf_get_reserved_vecs(adapter);
324 num_q_vecs = total_vecs - IDPF_MBX_Q_VEC;
325
326 err = idpf_send_alloc_vectors_msg(adapter, num_q_vecs);
327 if (err) {
328 dev_err(&adapter->pdev->dev,
329 "Failed to allocate %d vectors: %d\n",
num_q_vecs, err);
330
331 return -EAGAIN;
332 }
333
334 min_vectors = IDPF_MBX_Q_VEC + IDPF_MIN_Q_VEC * default_vports;
335 v_actual = pci_alloc_irq_vectors(adapter->pdev, min_vectors,
336 total_vecs, PCI_IRQ_MSIX);
337 if (v_actual < min_vectors) {
338 dev_err(&adapter->pdev->dev, "Failed to allocate MSIX
vectors: %d\n",
339 v_actual);
340 err = -EAGAIN;
341 goto send_dealloc_vecs;
342 }
343
344 adapter->msix_entries = kcalloc(v_actual, sizeof(struct
msix_entry),
345 GFP_KERNEL);
346
347 if (!adapter->msix_entries) {
348 err = -ENOMEM;
349 goto free_irq;
350 }
351
352 idpf_set_mb_vec_id(adapter);
353
354 vecids = kcalloc(total_vecs, sizeof(u16), GFP_KERNEL);
355 if (!vecids) {
356 err = -ENOMEM;
357 goto free_msix;
358 }
359
360 if (adapter->req_vec_chunks) {
^^^
If ->req_vec_chunks is NULL the error handling will crash
361 struct virtchnl2_vector_chunks *vchunks;
362 struct virtchnl2_alloc_vectors *ac;
363
364 ac = adapter->req_vec_chunks;
365 vchunks = &ac->vchunks;
366
367 num_vec_ids = idpf_get_vec_ids(adapter, vecids,
total_vecs,
368vchunks);
369 if (num_vec_ids < v_actual) {
370 err = -EINVAL;
371 goto free_vecids;
372 }
373 } else {
374 int i;
375
376 for (i = 0; i < v_actual; i++)
377 vecids[i] = i;
378 }
379
380 for (vector = 0; vector < v_actual; vector++) {
381 adapter->msix_entries[vector].entry = vecids[vector];
382 adapter->msix_entries[vector].vector =
383 pci_irq_vector(adapter->pdev, vector);
384 }
385
386 adapter->num_req_msix = total_vecs;
387 adapter->num_msix_entries = v_actual;
388 /* 'num_avail_msix' is used to distribute excess vectors to the
vports
389 * after considering the minimum vectors required per each
default
390 * vport
391 */
392 adapter->num_avail_msix = v_actual - min_vectors;
393
394 /* Fill MSIX vector lifo stack with vector indexes */
395 err = idpf_init_vector_stack(adapter);
396 if (err)
397 goto free_vecids;
398
399 err = idpf_mb_intr_init(adapter);
400 if (err)
401 goto deinit_vec_stack;
402 idpf_mb_irq_enable(adapter);
403 kfree(vecids);
404
405 return 0;
406
407 deinit_vec_stack:
408 idpf_deinit_vector_stack(adapter);
409 free_vecids:
410 kfree(vecids);
411 free_msix:
412 kfree(adapter->msix_entries);
413 adapter->msix_entries = NULL;
414 free_irq:
415 pci_free_irq_vectors(adapter->pdev);
416 send_dealloc_vecs:
--> 417 idpf_send_dealloc_vectors_msg(adapter);
^^^
Inside this function
418
419 return err;
420 }
regards,
dan carpenter
[Intel-wired-lan] [bug report] ice: add parser execution main loop
Hello Junfeng Guo,
Commit 9a4c07aaa0f5 ("ice: add parser execution main loop") from Jul
25, 2024 (linux-next), leads to the following Smatch static checker
warning:
drivers/net/ethernet/intel/ice/ice_parser_rt.c:124 ice_bst_key_init() error:
buffer overflow 'key' 10 <= 19
drivers/net/ethernet/intel/ice/ice_parser_rt.c:126 ice_bst_key_init() error:
buffer overflow 'key' 10 <= 19
drivers/net/ethernet/intel/ice/ice_parser_rt.c:134 ice_bst_key_init() error:
buffer overflow 'key' 10 <= 18
drivers/net/ethernet/intel/ice/ice_parser_rt.c:136 ice_bst_key_init() error:
buffer overflow 'key' 10 <= 18
drivers/net/ethernet/intel/ice/ice_parser_rt.c
114 static void ice_bst_key_init(struct ice_parser_rt *rt,
115 struct ice_imem_item *imem)
116 {
117 u8 tsr = (u8)rt->gpr[ICE_GPR_TSR_IDX];
118 u16 ho = rt->gpr[ICE_GPR_HO_IDX];
119 u8 *key = rt->bst_key;
120 int idd, i;
121
122 idd = ICE_BST_TCAM_KEY_SIZE - 1;
The key array has ICE_BST_KEY_SIZE (10) elements, but this code is using
TCAM key size which is 20.
123 if (imem->b_kb.tsr_ctrl)
--> 124 key[idd] = tsr;
It results in memory corruption
125 else
126 key[idd] = imem->b_kb.prio;
127
128 idd = ICE_BST_KEY_TCAM_SIZE - 1;
Same thing again. This size is 19 instead of 20 but still larger than 10.
129 for (i = idd; i >= 0; i--) {
130 int j;
131
132 j = ho + idd - i;
133 if (j < ICE_PARSER_MAX_PKT_LEN)
134 key[i] = rt->pkt_buf[ho + idd - i];
135 else
136 key[i] = 0;
^^^
Corrupt
137 }
138
139 ice_debug(rt->psr->hw, ICE_DBG_PARSER, "Generated Boost TCAM
Key:\n");
140 ice_debug(rt->psr->hw, ICE_DBG_PARSER, "%02X %02X %02X %02X
%02X %02X %02X %02X %02X %02X\n",
141 key[0], key[1], key[2], key[3], key[4],
142 key[5], key[6], key[7], key[8], key[9]);
143 ice_debug(rt->psr->hw, ICE_DBG_PARSER, "\n");
144 }
regards,
dan carpenter
[Intel-wired-lan] [PATCH net-next] ice: Fix a 32bit bug
BIT() is unsigned long but ->pu.flg_msk and ->pu.flg_val are u64 type.
On 32 bit systems, unsigned long is a u32 and the mismatch between u32
and u64 will break things for the high 32 bits.
Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_parser_rt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
index d5bcc266b01e..dedf5e854e4b 100644
--- a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
+++ b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
@@ -377,11 +377,11 @@ static void ice_pg_exe(struct ice_parser_rt *rt)
static void ice_flg_add(struct ice_parser_rt *rt, int idx, bool val)
{
- rt->pu.flg_msk |= BIT(idx);
+ rt->pu.flg_msk |= BIT_ULL(idx);
if (val)
- rt->pu.flg_val |= BIT(idx);
+ rt->pu.flg_val |= BIT_ULL(idx);
else
- rt->pu.flg_val &= ~BIT(idx);
+ rt->pu.flg_val &= ~BIT_ULL(idx);
ice_debug(rt->psr->hw, ICE_DBG_PARSER, "Pending update for flag %d
value %d\n",
idx, val);
--
2.43.0
Re: [Intel-wired-lan] [PATCH net-next] idpf: Slightly simplify memory management in idpf_add_del_mac_filters()
On Fri, Aug 23, 2024 at 08:23:29AM +0200, Christophe JAILLET wrote:
> In idpf_add_del_mac_filters(), filters are chunked up into multiple
> messages to avoid sending a control queue message buffer that is too large.
>
> Each chunk has up to IDPF_NUM_FILTERS_PER_MSG entries. So except for the
> last iteration which can be smaller, space for exactly
> IDPF_NUM_FILTERS_PER_MSG entries is allocated.
>
> There is no need to free and reallocate a smaller array just for the last
> iteration.
>
> This slightly simplifies the code and avoid an (unlikely) memory allocation
> failure.
>
> Signed-off-by: Christophe JAILLET
> ---
> drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 70986e12da28..b6f4b58e1094 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -3669,12 +3669,15 @@ int idpf_add_del_mac_filters(struct idpf_vport *vport,
> entries_size = sizeof(struct virtchnl2_mac_addr) * num_entries;
> buf_size = struct_size(ma_list, mac_addr_list, num_entries);
>
> - if (!ma_list || num_entries != IDPF_NUM_FILTERS_PER_MSG) {
> - kfree(ma_list);
> + if (!ma_list) {
> ma_list = kzalloc(buf_size, GFP_ATOMIC);
> if (!ma_list)
> return -ENOMEM;
> } else {
> + /* ma_list was allocated in the first iteration
> + * so IDPF_NUM_FILTERS_PER_MSG entries are
> + * available
> + */
> memset(ma_list, 0, buf_size);
> }
It would be even nicer to move the ma_list allocation outside the loop:
buf_size = struct_size(ma_list, mac_addr_list,
IDPF_NUM_FILTERS_PER_MSG);
ma_list = kmalloc(buf_size, GFP_ATOMIC);
regards,
dan carpenter
[Intel-wired-lan] [PATCH net] igc: Unlock on error in igc_io_resume()
Call rtnl_unlock() on this error path, before returning.
Fixes: bc23aa949aeb ("igc: Add pcie error handler support")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/igc/igc_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
b/drivers/net/ethernet/intel/igc/igc_main.c
index dfd6c00b4205..0a095cdea4fb 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7413,6 +7413,7 @@ static void igc_io_resume(struct pci_dev *pdev)
rtnl_lock();
if (netif_running(netdev)) {
if (igc_open(netdev)) {
+ rtnl_unlock();
netdev_err(netdev, "igc_open failed after reset\n");
return;
}
--
2.45.2
Re: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: Convert ret val type from s32 to int
Hi Jedrzej,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Jedrzej-Jagielski/ixgbe-Convert-ret-val-type-from-s32-to-int/20240103-182213
base: https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link:
https://lore.kernel.org/r/20240103101135.386891-1-jedrzej.jagielski%40intel.com
patch subject: [PATCH iwl-next v1] ixgbe: Convert ret val type from s32 to int
config: i386-randconfig-141-20240104
(https://download.01.org/0day-ci/archive/20240104/202401041701.6qktszmx-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202401041701.6qktszmx-...@intel.com/
New smatch warnings:
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:2884 ixgbe_get_lcd_t_x550em()
warn: missing error code? 'status'
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:3130 ixgbe_enter_lplu_t_x550em()
warn: missing error code? 'status'
Old smatch warnings:
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:2890 ixgbe_get_lcd_t_x550em()
warn: missing error code? 'status'
vim +/status +2884 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
9ea222bfe41f87 Jedrzej Jagielski 2024-01-03 2866 static int
ixgbe_get_lcd_t_x550em(struct ixgbe_hw *hw,
6ac7439459606a Don Skidmore 2015-06-17 2867
ixgbe_link_speed *lcd_speed)
6ac7439459606a Don Skidmore 2015-06-17 2868 {
6ac7439459606a Don Skidmore 2015-06-17 2869 u16 an_lp_status;
9ea222bfe41f87 Jedrzej Jagielski 2024-01-03 2870 int status;
6ac7439459606a Don Skidmore 2015-06-17 2871 u16 word =
hw->eeprom.ctrl_word_3;
6ac7439459606a Don Skidmore 2015-06-17 2872
6ac7439459606a Don Skidmore 2015-06-17 2873 *lcd_speed =
IXGBE_LINK_SPEED_UNKNOWN;
6ac7439459606a Don Skidmore 2015-06-17 2874
6ac7439459606a Don Skidmore 2015-06-17 2875 status =
hw->phy.ops.read_reg(hw, IXGBE_AUTO_NEG_LP_STATUS,
4dc4000b35119f Emil Tantilov 2016-09-26 2876
MDIO_MMD_AN,
6ac7439459606a Don Skidmore 2015-06-17 2877
&an_lp_status);
6ac7439459606a Don Skidmore 2015-06-17 2878 if (status)
6ac7439459606a Don Skidmore 2015-06-17 2879 return status;
6ac7439459606a Don Skidmore 2015-06-17 2880
6ac7439459606a Don Skidmore 2015-06-17 2881 /* If link partner
advertised 1G, return 1G */
6ac7439459606a Don Skidmore 2015-06-17 2882 if (an_lp_status &
IXGBE_AUTO_NEG_LP_1000BASE_CAP) {
6ac7439459606a Don Skidmore 2015-06-17 2883 *lcd_speed =
IXGBE_LINK_SPEED_1GB_FULL;
6ac7439459606a Don Skidmore 2015-06-17 @2884 return status;
Smatch only warns about missing error codes when the function returns an
int. :P The bug predates your patch obvoiusly.
6ac7439459606a Don Skidmore 2015-06-17 2885 }
6ac7439459606a Don Skidmore 2015-06-17 2886
6ac7439459606a Don Skidmore 2015-06-17 2887 /* If 10G disabled for
LPLU via NVM D10GMP, then return no valid LCD */
6ac7439459606a Don Skidmore 2015-06-17 2888 if ((hw->bus.lan_id &&
(word & NVM_INIT_CTRL_3_D10GMP_PORT1)) ||
6ac7439459606a Don Skidmore 2015-06-17 2889 (word &
NVM_INIT_CTRL_3_D10GMP_PORT0))
6ac7439459606a Don Skidmore 2015-06-17 2890 return status;
6ac7439459606a Don Skidmore 2015-06-17 2891
6ac7439459606a Don Skidmore 2015-06-17 2892 /* Link partner not
capable of lower speeds, return 10G */
6ac7439459606a Don Skidmore 2015-06-17 2893 *lcd_speed =
IXGBE_LINK_SPEED_10GB_FULL;
6ac7439459606a Don Skidmore 2015-06-17 2894 return status;
6ac7439459606a Don Skidmore 2015-06-17 2895 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [Intel-wired-lan] [PATCH iwl-next v3 2/3] ixgbe: Fix smatch warnings after type convertion
Thanks for this patch!
On Thu, Jan 18, 2024 at 02:43:31PM +0100, Jedrzej Jagielski wrote:
> Converting s32 functions to regular int in the patch 8035560dbfaf caused
> trigerring smatch warnings about missing error code. The bug predates
> the mentioned patch.
It's not really a bug, just some suspicous code. Especially the
"If 10G disabled for LPLU via NVM D10GMP, then return no valid LCD"
return. But it's actually all fine so this patch is really just a
cleanup.
>
> New smatch warnings:
> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:2884 ixgbe_get_lcd_t_x550em()
> warn: missing error code? 'status'
> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:3130
> ixgbe_enter_lplu_t_x550em() warn: missing error code? 'status'
>
> Old smatch warnings:
> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:2890 ixgbe_get_lcd_t_x550em()
> warn: missing error code? 'status'
>
> Fix it by clearly stating returning error code as 0.
>
> Reported-by: kernel test robot
> Reported-by: Dan Carpenter
> Closes: https://lore.kernel.org/r/202401041701.6qktszmx-...@intel.com/
> Fixes: 6ac743945960 ("ixgbe: Add support for entering low power link up
> state")
No need for a Fixes tag.
> Signed-off-by: Jedrzej Jagielski
Thanks, again. I do think this makes it a lot more clear.
regards,
dan carpenter
[Intel-wired-lan] [bug report] ice: implement dpll interface to control cgu
Hello Arkadiusz Kubalewski,
The patch d7999f5ea64b: "ice: implement dpll interface to control
cgu" from Sep 13, 2023 (linux-next), leads to the following Smatch
static checker warning:
drivers/net/ethernet/intel/ice/ice_dpll.c:1590 ice_dpll_init_rclk_pins()
warn: inconsistent refcounting
'pf->dplls.rclk.pin->refcount.refs.counter':
drivers/net/ethernet/intel/ice/ice_dpll.c
1554 static int
1555 ice_dpll_init_rclk_pins(struct ice_pf *pf, struct ice_dpll_pin *pin,
1556 int start_idx, const struct dpll_pin_ops *ops)
1557 {
1558 struct ice_vsi *vsi = ice_get_main_vsi(pf);
1559 struct dpll_pin *parent;
1560 int ret, i;
1561
1562 ret = ice_dpll_get_pins(pf, pin, start_idx,
ICE_DPLL_RCLK_NUM_PER_PF,
1563 pf->dplls.clock_id);
1564 if (ret)
1565 return ret;
1566 for (i = 0; i < pf->dplls.rclk.num_parents; i++) {
1567 parent =
pf->dplls.inputs[pf->dplls.rclk.parent_idx[i]].pin;
1568 if (!parent) {
1569 ret = -ENODEV;
1570 goto unregister_pins;
1571 }
1572 ret = dpll_pin_on_pin_register(parent,
pf->dplls.rclk.pin,
1573ops, &pf->dplls.rclk);
1574 if (ret)
1575 goto unregister_pins;
1576 }
1577 if (WARN_ON((!vsi || !vsi->netdev)))
1578 return -EINVAL;
goto unregister_pins?
1579 netdev_dpll_pin_set(vsi->netdev, pf->dplls.rclk.pin);
1580
1581 return 0;
1582
1583 unregister_pins:
1584 while (i) {
1585 parent =
pf->dplls.inputs[pf->dplls.rclk.parent_idx[--i]].pin;
1586 dpll_pin_on_pin_unregister(parent, pf->dplls.rclk.pin,
1587&ice_dpll_rclk_ops,
&pf->dplls.rclk);
1588 }
1589 ice_dpll_release_pins(pin, ICE_DPLL_RCLK_NUM_PER_PF);
--> 1590 return ret;
1591 }
regards,
dan carpenter
[Intel-wired-lan] [bug report] ixgbe: add VF IPsec management
Hello Shannon Nelson,
The patch eda0333ac293: "ixgbe: add VF IPsec management" from Aug 13,
2018 (linux-next), leads to the following Smatch static checker
warning:
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c:917
ixgbe_ipsec_vf_add_sa()
warn: sleeping in IRQ context
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
890 int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf,
u32 vf)
891 {
892 struct ixgbe_ipsec *ipsec = adapter->ipsec;
893 struct xfrm_algo_desc *algo;
894 struct sa_mbx_msg *sam;
895 struct xfrm_state *xs;
896 size_t aead_len;
897 u16 sa_idx;
898 u32 pfsa;
899 int err;
900
901 sam = (struct sa_mbx_msg *)(&msgbuf[1]);
902 if (!adapter->vfinfo[vf].trusted ||
903 !(adapter->flags2 & IXGBE_FLAG2_VF_IPSEC_ENABLED)) {
904 e_warn(drv, "VF %d attempted to add an IPsec SA\n", vf);
905 err = -EACCES;
906 goto err_out;
907 }
908
909 /* Tx IPsec offload doesn't seem to work on this
910 * device, so block these requests for now.
911 */
912 if (sam->dir != XFRM_DEV_OFFLOAD_IN) {
913 err = -EOPNOTSUPP;
914 goto err_out;
915 }
916
--> 917 xs = kzalloc(sizeof(*xs), GFP_KERNEL);
^^
Sleeping allocation.
The call tree that Smatch is worried about is:
ixgbe_msix_other() <- IRQ handler
-> ixgbe_msg_task()
-> ixgbe_rcv_msg_from_vf()
-> ixgbe_ipsec_vf_add_sa()
This is a fairly new warning and those have a higher risk of false
positives. Plus the longer the call tree the higher the chance of
false positives. However, I did review it and the warning looks
reasonable.
regards,
dan carpenter
[Intel-wired-lan] [PATCH net] ice: Fix freeing uninitialized pointers
Automatically cleaned up pointers need to be initialized before exiting
their scope. In this case, they need to be initialized to NULL before
any return statement.
Fixes: 90f821d72e11 ("ice: avoid unnecessary devm_ usage")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_common.c | 4 ++--
drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 4d8111aeb0ff..4b27d2bc2912 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1002,8 +1002,8 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
*/
int ice_init_hw(struct ice_hw *hw)
{
- struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
- void *mac_buf __free(kfree);
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
+ void *mac_buf __free(kfree) = NULL;
u16 mac_buf_len;
int status;
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 255a9c8151b4..78b833b3e1d7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -941,11 +941,11 @@ static u64 ice_loopback_test(struct net_device *netdev)
struct ice_netdev_priv *np = netdev_priv(netdev);
struct ice_vsi *orig_vsi = np->vsi, *test_vsi;
struct ice_pf *pf = orig_vsi->back;
+ u8 *tx_frame __free(kfree) = NULL;
u8 broadcast[ETH_ALEN], ret = 0;
int num_frames, valid_frames;
struct ice_tx_ring *tx_ring;
struct ice_rx_ring *rx_ring;
- u8 *tx_frame __free(kfree);
int i;
netdev_info(netdev, "loopback test\n");
--
2.43.0
Re: [Intel-wired-lan] [PATCH net] ice: Fix freeing uninitialized pointers
On Mon, Mar 18, 2024 at 08:58:24AM +0100, Jiri Pirko wrote:
> Sat, Mar 16, 2024 at 10:44:40AM CET, dan.carpen...@linaro.org wrote:
> >Automatically cleaned up pointers need to be initialized before exiting
> >their scope. In this case, they need to be initialized to NULL before
> >any return statement.
> >
> >Fixes: 90f821d72e11 ("ice: avoid unnecessary devm_ usage")
> >Signed-off-by: Dan Carpenter
> >---
> > drivers/net/ethernet/intel/ice/ice_common.c | 4 ++--
> > drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> >b/drivers/net/ethernet/intel/ice/ice_common.c
> >index 4d8111aeb0ff..4b27d2bc2912 100644
> >--- a/drivers/net/ethernet/intel/ice/ice_common.c
> >+++ b/drivers/net/ethernet/intel/ice/ice_common.c
> >@@ -1002,8 +1002,8 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
> > */
> > int ice_init_hw(struct ice_hw *hw)
> > {
> >-struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
> >-void *mac_buf __free(kfree);
> >+struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
> >+void *mac_buf __free(kfree) = NULL;
> > u16 mac_buf_len;
> > int status;
> >
>
> How about similar issues in:
> ice_set_fc()
> ice_cfg_phy_fec()
> ?
Yeah. Sorry, I'll resend. Smatch didn't warn about those bugs because
the sanity checks are the begining of the functions:
if (!pi || !aq_failures)
return -EINVAL;
are never true... It's the first time I've run into this as an issue.
regards,
dan carpenter
Re: [Intel-wired-lan] [PATCH net] ice: Fix freeing uninitialized pointers
On Tue, Mar 19, 2024 at 12:43:17PM -0700, Jakub Kicinski wrote: > On Sat, 16 Mar 2024 12:44:40 +0300 Dan Carpenter wrote: > > - struct ice_aqc_get_phy_caps_data *pcaps __free(kfree); > > - void *mac_buf __free(kfree); > > + struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL; > > + void *mac_buf __free(kfree) = NULL; > > This is just trading one kind of bug for another, and the __free() > magic is at a cost of readability. > > I think we should ban the use of __free() in all of networking, > until / unless it cleanly handles the NULL init case. Free handles the NULL init case, it doesn't handle the uninitialized case. I had previously argued that checkpatch should complain about every __free() pointer if the declaration doesn't have an assignment. The = NULL assignment is unnecessary if the pointer is assigned to something else before the first return, so this might cause "unused assignment" warnings? I don't know if there are any tools which complain about that in that situation. I think probably we should just make that an exception and do the checkpatch thing because it's such a simple rule to implement. regards, dan carpenter
Re: [Intel-wired-lan] [PATCH net] ice: Fix freeing uninitialized pointers
On Thu, Mar 21, 2024 at 10:59:42AM +0100, Przemek Kitszel wrote: > Simplest solution would be to add a macro wrapper, especially that there > are only a few deallocation methods. > > in cleanup.h: > +#define auto_kfree __free(kfree) = NULL > > and similar macros for auto vfree(), etc. > > then in the drivers: > -struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL, > *othercaps __free(kfree) = NULL; > +struct ice_aqc_get_phy_caps_data *pcaps auto_kfree, > *othercaps auto_kfree; The auto_kfree looks like a variable to my eyes. I'd prefer something like: #define __FREE(p) p __free(kfree) = NULL struct ice_aqc_get_phy_caps_data *__FREE(pcaps); regards, dan carpenter
[Intel-wired-lan] [PATCH v2 net] ice: Fix freeing uninitialized pointers
Automatically cleaned up pointers need to be initialized before exiting
their scope. In this case, they need to be initialized to NULL before
any return statement.
Fixes: 90f821d72e11 ("ice: avoid unnecessary devm_ usage")
Signed-off-by: Dan Carpenter
---
v2: I missed a couple pointers in v1.
The change to ice_update_link_info() isn't required because it's
assigned on the very next line... But I did that because it's harmless
and makes __free() stuff easier to verify. I felt like moving the
declarations into the code would be controversial and it also ends up
making the lines really long.
goto goto err_unroll_sched;
struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) =
kzalloc(sizeof(*pcaps), GFP_KERNEL);
drivers/net/ethernet/intel/ice/ice_common.c | 10 +-
drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
2 file changed, 6 insertion(+), 6 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 4d8111aeb0ff..6f2db603b36e 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1002,8 +1002,8 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
*/
int ice_init_hw(struct ice_hw *hw)
{
- struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
- void *mac_buf __free(kfree);
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
+ void *mac_buf __free(kfree) = NULL;
u16 mac_buf_len;
int status;
@@ -3272,7 +3272,7 @@ int ice_update_link_info(struct ice_port_info *pi)
return status;
if (li->link_info & ICE_AQ_MEDIA_AVAILABLE) {
- struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
if (!pcaps)
@@ -3420,7 +3420,7 @@ ice_cfg_phy_fc(struct ice_port_info *pi, struct
ice_aqc_set_phy_cfg_data *cfg,
int
ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool
ena_auto_link_update)
{
- struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
struct ice_aqc_set_phy_cfg_data cfg = { 0 };
struct ice_hw *hw;
int status;
@@ -3561,7 +3561,7 @@ int
ice_cfg_phy_fec(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
enum ice_fec_mode fec)
{
- struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = NULL;
struct ice_hw *hw;
int status;
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 255a9c8151b4..78b833b3e1d7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -941,11 +941,11 @@ static u64 ice_loopback_test(struct net_device *netdev)
struct ice_netdev_priv *np = netdev_priv(netdev);
struct ice_vsi *orig_vsi = np->vsi, *test_vsi;
struct ice_pf *pf = orig_vsi->back;
+ u8 *tx_frame __free(kfree) = NULL;
u8 broadcast[ETH_ALEN], ret = 0;
int num_frames, valid_frames;
struct ice_tx_ring *tx_ring;
struct ice_rx_ring *rx_ring;
- u8 *tx_frame __free(kfree);
int i;
netdev_info(netdev, "loopback test\n");
--
2.43.0
Re: [Intel-wired-lan] [PATCH v2 net] ice: Fix freeing uninitialized pointers
On Thu, Mar 21, 2024 at 04:34:37PM +0100, Jiri Pirko wrote: > >The change to ice_update_link_info() isn't required because it's > >assigned on the very next line... But I did that because it's harmless > >and makes __free() stuff easier to verify. I felt like moving the > >declarations into the code would be controversial and it also ends up > >making the lines really long. > > > > goto goto err_unroll_sched; > > > > struct ice_aqc_get_phy_caps_data *pcaps __free(kfree) = > > kzalloc(sizeof(*pcaps), GFP_KERNEL); > > Yeah, that is why I'm proposing KZALLOC_FREE helper: > https://lore.kernel.org/all/20240315132249.2515468-1-j...@resnulli.us/ > I like the idea, but I'm not keen on the format. What about something like? #define __ALLOC(p) p __free(kfree) = kzalloc(sizeof(*p), GFP_KERNEL) struct ice_aqc_get_phy_caps_data *__ALLOC(pcaps); I'm not a huge fan of putting functions which can fail into the declaration block but I feel like we're going to officially say that small allocations can't fail. https://lwn.net/Articles/964793/ https://lore.kernel.org/all/170925937840.24797.2167230750547152...@noble.neil.brown.name/ Normally we would try to delay the allocations until after all the sanity checks have run but that's optimizing for the failure case. In the normal case we're going to want these allocations. regards, damn carpenter
Re: [Intel-wired-lan] [PATCH net] ice: Fix freeing uninitialized pointers
On Thu, Mar 21, 2024 at 04:20:09PM -0400, Julia Lawall wrote: > Does one prefer an initialization of null at the top of the function > or an initialization to a meaningful value in the middle of the > function? I prefer at the top, but it will be interesting to see where the consensus is. Kent Overstreet has said we should move away from declarations at the top generally. I don't know if anyone else agrees with him though. regards, dan carpenter
Re: [Intel-wired-lan] [PATCH v2 net] ice: Fix freeing uninitialized pointers
Markus please don't do this. Don't take a controversial opinion and start trying to force it on everyone via review comments and an automatic converstion script. regards, dan carpenter
Re: [Intel-wired-lan] [PATCH v2 net] ice: Fix freeing uninitialized pointers
On Sat, Mar 23, 2024 at 05:56:29PM +0100, Markus Elfring wrote: > > Automatically cleaned up pointers need to be initialized before exiting > > their scope. In this case, they need to be initialized to NULL before > > any return statement. > > * May we expect that compilers should report that affected variables > were only declared here instead of appropriately defined > (despite of attempts for scope-based resource management)? > We disabled GCC's check for uninitialized variables a long time ago because it had too many false positives. > * Did you extend detection support in the source code analysis tool “Smatch” > for a questionable implementation detail? Yes. Smatch detects this as an uninitialized variable. regards, dan carpenter
Re: [Intel-wired-lan] [PATCH net-next v6 15/21] ip_tunnel: use a separate struct to store tunnel params in the kernel
On Wed, Mar 27, 2024 at 04:23:52PM +0100, Alexander Lobakin wrote:
> +bool ip_tunnel_parm_to_user(void __user *data, struct ip_tunnel_parm_kern
> *kp)
> +{
> + struct ip_tunnel_parm p;
> +
> + strscpy(p.name, kp->name);
We need to clear out p before copying to user space to avoid an
information leak. So this strscpy() needs to be strcpy_pad()
> + p.link = kp->link;
> + p.i_flags = kp->i_flags;
> + p.o_flags = kp->o_flags;
> + p.i_key = kp->i_key;
> + p.o_key = kp->o_key;
> + memcpy(&p.iph, &kp->iph, min(sizeof(p.iph), sizeof(kp->iph)));
And this memcpy() doesn't necessarily clear the whole of p.iph.
> +
> + return !copy_to_user(data, &p, sizeof(p));
> +}
regards,
dan carpenter
[Intel-wired-lan] [PATCH net-next] ice: Fix a couple NULL vs IS_ERR() bugs
The ice_repr_create() function returns error pointers. It never returns
NULL. Fix the callers to check for IS_ERR().
Fixes: 977514fb0fa8 ("ice: create port representor for SF")
Fixes: 415db8399d06 ("ice: make representor code generic")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_repr.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_repr.c
b/drivers/net/ethernet/intel/ice/ice_repr.c
index 00d4a9125dfa..970a99a52bf1 100644
--- a/drivers/net/ethernet/intel/ice/ice_repr.c
+++ b/drivers/net/ethernet/intel/ice/ice_repr.c
@@ -452,8 +452,8 @@ struct ice_repr *ice_repr_create_vf(struct ice_vf *vf)
return ERR_PTR(-EINVAL);
repr = ice_repr_create(vsi);
- if (!repr)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(repr))
+ return repr;
repr->type = ICE_REPR_TYPE_VF;
repr->vf = vf;
@@ -501,8 +501,8 @@ struct ice_repr *ice_repr_create_sf(struct ice_dynamic_port
*sf)
{
struct ice_repr *repr = ice_repr_create(sf->vsi);
- if (!repr)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(repr))
+ return repr;
repr->type = ICE_REPR_TYPE_SF;
repr->sf = sf;
--
2.45.2
[Intel-wired-lan] [PATCH net-next] ice: Fix a NULL vs IS_ERR() check in probe()
The ice_allocate_sf() function returns error pointers on error. It
doesn't return NULL. Update the check to match.
Fixes: 177ef7f1e2a0 ("ice: base subfunction aux driver")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_sf_eth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
index d00389c405c4..75d7147e1c01 100644
--- a/drivers/net/ethernet/intel/ice/ice_sf_eth.c
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.c
@@ -108,9 +108,9 @@ static int ice_sf_dev_probe(struct auxiliary_device *adev,
vsi->flags = ICE_VSI_FLAG_INIT;
priv = ice_allocate_sf(&adev->dev, pf);
- if (!priv) {
+ if (IS_ERR(priv)) {
dev_err(dev, "Subfunction devlink alloc failed");
- return -ENOMEM;
+ return PTR_ERR(priv);
}
priv->dev = sf_dev;
--
2.45.2
Re: [Intel-wired-lan] [PATCH net-next] iavf: Avoid a memory allocation in iavf_print_link_message()
On Tue, Oct 03, 2023 at 04:01:18PM -0700, Jesse Brandeburg wrote: > On 10/3/2023 1:33 PM, Christophe JAILLET wrote: > > kasprintf() is much better. > > cool! I just sent the patches and cc'd you earlier today. > > > > > > > your patch still shows these errors > > > > I built-tested the patch before sending, so this is strange. > > > > However, I got a similar feedback from Greg KH and the "kernel test > > robot" for another similar patch. > > > > What version of gcc do you use? > > I use 12.3.0, and I suspect that the value range algorithm or how the > > diagnostic is done has been improved in recent gcc. > > Fedora gcc 12.3.1, with W=1 flag > > gcc version 12.3.1 20230508 (Red Hat 12.3.1-1) (GCC) > > [linux]$ make W=1 M=drivers/net/ethernet/intel/iavf > CC [M] drivers/net/ethernet/intel/iavf/iavf_main.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_ethtool.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_virtchnl.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_fdir.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_adv_rss.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_txrx.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_common.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_adminq.o > CC [M] drivers/net/ethernet/intel/iavf/iavf_client.o > drivers/net/ethernet/intel/iavf/iavf_virtchnl.c: In function > ‘iavf_virtchnl_completion’: > drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1446:60: warning: ‘%s’ > directive output may be truncated writing 4 bytes into a region of size > between 1 and 11 [-Wformat-truncation=] > 1446 | snprintf(speed, IAVF_MAX_SPEED_STRLEN, "%d %s", > |^~ > 1447 | link_speed_mbps, "Mbps"); > | ~~ GCC is kind of crap at static analysis, right? Smatch would know that this at most 11 characters long. It's kind of laziness for GCC to print this warning. If you complained to me about a false positive like this in Smatch I would at least think about various ways to silence it. But I probably wouldn't write a check for this anyway because I don't view truncating strings as a note worthy bug... Smatch also gets stuff wrong, but in that case I just always encourage people to mark the warning as old news and move on. Only new warnings are interesting. I feel like as we incorporate more and more static analysis into our processes we're going to have to give up on trying to keep every static checker happy. regards, dan carpenter ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH net] ixgbe: fix crash with empty VF macvlan list
The adapter->vf_mvs.l list needs to be initialized even if the list is
empty. Otherwise it will lead to crashes.
Fixes: c6bda30a06d9 ("ixgbe: Reconfigure SR-IOV Init")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index a703ba975205..9cfdfa8a4355 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -28,6 +28,9 @@ static inline void ixgbe_alloc_vf_macvlans(struct
ixgbe_adapter *adapter,
struct vf_macvlans *mv_list;
int num_vf_macvlans, i;
+ /* Initialize list of VF macvlans */
+ INIT_LIST_HEAD(&adapter->vf_mvs.l);
+
num_vf_macvlans = hw->mac.num_rar_entries -
(IXGBE_MAX_PF_MACVLANS + 1 + num_vfs);
if (!num_vf_macvlans)
@@ -36,8 +39,6 @@ static inline void ixgbe_alloc_vf_macvlans(struct
ixgbe_adapter *adapter,
mv_list = kcalloc(num_vf_macvlans, sizeof(struct vf_macvlans),
GFP_KERNEL);
if (mv_list) {
- /* Initialize list of VF macvlans */
- INIT_LIST_HEAD(&adapter->vf_mvs.l);
for (i = 0; i < num_vf_macvlans; i++) {
mv_list[i].vf = -1;
mv_list[i].free = true;
--
2.39.2
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH net-next 1/2] igb: Fix an end of loop test
When we exit a list_for_each_entry() without hitting a break statement,
the list iterator isn't NULL, it just point to an offset off the
list_head. In that situation, it wouldn't be too surprising for
entry->free to be true and we end up corrupting memory.
The way to test for these is to just set a flag.
Fixes: c1fec890458a ("ethernet/intel: Use list_for_each_entry() helper")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/igb/igb_main.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
b/drivers/net/ethernet/intel/igb/igb_main.c
index 2ac9dffd0bf8..c45b1e7cde58 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7857,7 +7857,8 @@ static int igb_set_vf_mac_filter(struct igb_adapter
*adapter, const int vf,
{
struct pci_dev *pdev = adapter->pdev;
struct vf_data_storage *vf_data = &adapter->vf_data[vf];
- struct vf_mac_filter *entry = NULL;
+ struct vf_mac_filter *entry;
+ bool found = false;
int ret = 0;
if ((vf_data->flags & IGB_VF_FLAG_PF_SET_MAC) &&
@@ -7888,11 +7889,13 @@ static int igb_set_vf_mac_filter(struct igb_adapter
*adapter, const int vf,
case E1000_VF_MAC_FILTER_ADD:
/* try to find empty slot in the list */
list_for_each_entry(entry, &adapter->vf_macs.l, l) {
- if (entry->free)
+ if (entry->free) {
+ found = true;
break;
+ }
}
- if (entry && entry->free) {
+ if (found) {
entry->free = false;
entry->vf = vf;
ether_addr_copy(entry->vf_mac, addr);
--
2.39.2
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH net-next 2/2] ixgbe: fix end of loop test in ixgbe_set_vf_macvlan()
The list iterator in a list_for_each_entry() loop can never be NULL.
If the loop exits without hitting a break then the iterator points
to an offset off the list head and dereferencing it is an out of
bounds access.
Before we transitioned to using list_for_each_entry() loops, then
it was possible for "entry" to be NULL and the comments mention
this. I have updated the comments to match the new code.
Fixes: c1fec890458a ("ethernet/intel: Use list_for_each_entry() helper")
Signed-off-by: Dan Carpenter
---
.../net/ethernet/intel/ixgbe/ixgbe_sriov.c| 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 4c6e2a485d8e..a703ba975205 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -639,6 +639,7 @@ static int ixgbe_set_vf_macvlan(struct ixgbe_adapter
*adapter,
int vf, int index, unsigned char *mac_addr)
{
struct vf_macvlans *entry;
+ bool found = false;
int retval = 0;
if (index <= 1) {
@@ -660,22 +661,22 @@ static int ixgbe_set_vf_macvlan(struct ixgbe_adapter
*adapter,
if (!index)
return 0;
- entry = NULL;
-
list_for_each_entry(entry, &adapter->vf_mvs.l, l) {
- if (entry->free)
+ if (entry->free) {
+ found = true;
break;
+ }
}
/*
* If we traversed the entire list and didn't find a free entry
-* then we're out of space on the RAR table. Also entry may
-* be NULL because the original memory allocation for the list
-* failed, which is not fatal but does mean we can't support
-* VF requests for MACVLAN because we couldn't allocate
-* memory for the list management required.
+* then we're out of space on the RAR table. It's also possible
+* for the &adapter->vf_mvs.l list to be empty because the original
+* memory allocation for the list failed, which is not fatal but does
+* mean we can't support VF requests for MACVLAN because we couldn't
+* allocate memory for the list management required.
*/
- if (!entry || !entry->free)
+ if (!found)
return -ENOSPC;
retval = ixgbe_add_mac_filter(adapter, mac_addr, vf);
--
2.39.2
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH net] ixgbe: fix crash with empty VF macvlan list
On Fri, Oct 06, 2023 at 01:16:27PM +0200, Simon Horman wrote:
> On Thu, Oct 05, 2023 at 04:57:02PM +0300, Dan Carpenter wrote:
> > The adapter->vf_mvs.l list needs to be initialized even if the list is
> > empty. Otherwise it will lead to crashes.
> >
> > Fixes: c6bda30a06d9 ("ixgbe: Reconfigure SR-IOV Init")
>
> Hi Dan,
>
> I see that the patch cited above added the line you are changing.
> But it also seems to me that patch was moving it from elsewhere.
>
> Perhaps I am mistaken, but I wonder if this is a better tag.
>
> Fixes: a1cbb15c1397 ("ixgbe: Add macvlan support for VF")
>
Yeah. You're right. I'll resend.
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > index a703ba975205..9cfdfa8a4355 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > @@ -28,6 +28,9 @@ static inline void ixgbe_alloc_vf_macvlans(struct
> > ixgbe_adapter *adapter,
> > struct vf_macvlans *mv_list;
> > int num_vf_macvlans, i;
> >
> > + /* Initialize list of VF macvlans */
> > + INIT_LIST_HEAD(&adapter->vf_mvs.l);
> > +
> > num_vf_macvlans = hw->mac.num_rar_entries -
> > (IXGBE_MAX_PF_MACVLANS + 1 + num_vfs);
> > if (!num_vf_macvlans)
> > @@ -36,8 +39,6 @@ static inline void ixgbe_alloc_vf_macvlans(struct
> > ixgbe_adapter *adapter,
> > mv_list = kcalloc(num_vf_macvlans, sizeof(struct vf_macvlans),
> > GFP_KERNEL);
> > if (mv_list) {
>
> I'm not sure it it is worth it, but perhaps more conventional error
> handling could be used here:
>
> if (!mv_list)
> return;
>
> for (i = 0; i < num_vf_macvlans; i++) {
> ...
I mean error handling is always cleaner than success handling but it's
probably not worth cleaning up in old code. I say it's not worth
cleaning up old code and yet I secretly reversed two if statements like
this yesterday. :P
https://lore.kernel.org/all/d9da4c97-0da9-499f-9a21-1f8e3f148dc1@moroto.mountain/
It really is nicer, yes. But it just makes the patch too noisy.
regards,
dan carpenter
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [PATCH net v2] ixgbe: fix crash with empty VF macvlan list
The adapter->vf_mvs.l list needs to be initialized even if the list is
empty. Otherwise it will lead to crashes.
Fixes: a1cbb15c1397 ("ixgbe: Add macvlan support for VF")
Signed-off-by: Dan Carpenter
---
v2: Use the correct fixes tag. Thanks, Simon.
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index a703ba975205..9cfdfa8a4355 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -28,6 +28,9 @@ static inline void ixgbe_alloc_vf_macvlans(struct
ixgbe_adapter *adapter,
struct vf_macvlans *mv_list;
int num_vf_macvlans, i;
+ /* Initialize list of VF macvlans */
+ INIT_LIST_HEAD(&adapter->vf_mvs.l);
+
num_vf_macvlans = hw->mac.num_rar_entries -
(IXGBE_MAX_PF_MACVLANS + 1 + num_vfs);
if (!num_vf_macvlans)
@@ -36,8 +39,6 @@ static inline void ixgbe_alloc_vf_macvlans(struct
ixgbe_adapter *adapter,
mv_list = kcalloc(num_vf_macvlans, sizeof(struct vf_macvlans),
GFP_KERNEL);
if (mv_list) {
- /* Initialize list of VF macvlans */
- INIT_LIST_HEAD(&adapter->vf_mvs.l);
for (i = 0; i < num_vf_macvlans; i++) {
mv_list[i].vf = -1;
mv_list[i].free = true;
--
2.39.2
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH] i40e: add an error code check in i40e_vsi_setup
On Thu, Oct 19, 2023 at 04:42:42PM +0800, Su Hui wrote:
> check the value of 'ret' after calling 'i40e_vsi_config_rss'.
>
> Signed-off-by: Su Hui
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index de7fd43dc11c..9205090e5017 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -14567,6 +14567,8 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf,
> u8 type,
> if ((pf->hw_features & I40E_HW_RSS_AQ_CAPABLE) &&
> (vsi->type == I40E_VSI_VMDQ2)) {
> ret = i40e_vsi_config_rss(vsi);
> + if (ret)
> + goto err_rings;
This function uses Come From label names. Instead of telling you what
the function/goto does it tells you where it is called from. Here
"err_rings" means that the rings allocation failed. However, the rings
allocation actually succeeded and we need to free it
with i40e_vsi_clear_rings().
What clean up we need to do depends on vsi->type as well but here
we know the type is vsi->type == I40E_VSI_VMDQ2 so it's needs to call
the i40e_vsi_clear_rings().
regards,
dan carpenter
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH v2] i40e: add an error code check in i40e_vsi_setup
On Fri, Oct 20, 2023 at 10:43:09AM +0800, Su Hui wrote: > check the value of 'ret' after calling 'i40e_vsi_config_rss'. > > Signed-off-by: Su Hui > --- > v2: > - call i40e_vsi_clear_rings() to free rings(thank dan carpenter for > pointing out this). Looks okay now. Reviewed-by: Dan Carpenter regards, dan carpenter ___ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH v5 4/5] i40e: Fix broken support for floating VEBs
Hi Ivan,
kernel test robot noticed the following build warnings:
url:
https://github.com/intel-lab-lkp/linux/commits/Ivan-Vecera/i40e-Use-existing-helper-to-find-flow-director-VSI/20231124-230616
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
dev-queue
patch link:
https://lore.kernel.org/r/20231124150343.81520-5-ivecera%40redhat.com
patch subject: [PATCH v5 4/5] i40e: Fix broken support for floating VEBs
config: x86_64-randconfig-161-20231127
(https://download.01.org/0day-ci/archive/20231127/202311270851.ie6aegcs-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20231127/202311270851.ie6aegcs-...@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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202311270851.ie6aegcs-...@intel.com/
New smatch warnings:
drivers/net/ethernet/intel/i40e/i40e_main.c:14743 i40e_veb_release() error:
potentially dereferencing uninitialized 'vsi'.
Old smatch warnings:
[ low confidence ]
drivers/net/ethernet/intel/i40e/i40e_main.c:5966 i40e_set_bw_limit() warn:
error code type promoted to positive: 'speed'
drivers/net/ethernet/intel/i40e/i40e_main.c:13476
i40e_queue_pair_toggle_rings() warn: missing error code? 'ret'
drivers/net/ethernet/intel/i40e/i40e_main.c:14272 i40e_vsi_setup_vectors()
warn: missing error code? 'ret'
drivers/net/ethernet/intel/i40e/i40e_main.c:15566 i40e_init_recovery_mode()
warn: 'vsi->netdev' from register_netdev() not released on lines: 15566.
vim +/vsi +14743 drivers/net/ethernet/intel/i40e/i40e_main.c
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14715 void
i40e_veb_release(struct i40e_veb *veb)
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14716 {
0aab77d67d37d09 Ivan Vecera 2023-11-24 14717 struct i40e_vsi *vsi,
*vsi_it;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14718 struct i40e_pf *pf;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14719 int i, n = 0;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14720
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14721 pf = veb->pf;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14722
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14723 /* find the remaining
VSI and check for extras */
0aab77d67d37d09 Ivan Vecera 2023-11-24 14724
i40e_pf_for_each_vsi(pf, i, vsi_it)
0aab77d67d37d09 Ivan Vecera 2023-11-24 14725 if
(vsi_it->uplink_seid == veb->seid) {
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14726 if
(vsi_it->flags & I40E_VSI_FLAG_VEB_OWNER)
0aab77d67d37d09 Ivan Vecera 2023-11-24 14727
vsi = vsi_it;
Do we always find a vsi? Presumably, yes, but it's not obvious just
from reading this function.
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14728 n++;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14729 }
0aab77d67d37d09 Ivan Vecera 2023-11-24 14730
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14731 /* Floating VEB has to
be empty and regular one must have
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14732 * single owner VSI.
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14733 */
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14734 if ((veb->uplink_seid
&& n != 1) || (!veb->uplink_seid && n != 0)) {
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14735
dev_info(&pf->pdev->dev,
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14736 "can't
remove VEB %d with %d VSIs left\n",
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14737
veb->seid, n);
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14738 return;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14739 }
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14740
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 14741 /* For regular VEB move
the owner VSI to uplink VEB */
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14742 if (veb->uplink_seid) {
93a1bc91a1ccc5a Ivan Vecera 2023-11-24 @14743 vsi->flags &=
~I40E_VSI_FLAG_VEB_OWNER;
^^
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14744
vsi->uplink_seid = veb->uplink_seid;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14745 if
(veb->uplink_seid == pf->mac_seid)
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14746
vsi->veb_idx = I40E_NO_VEB;
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14747 else
41c445ff0f482bb Jesse Brandeburg 2013-09-11 14748
vsi->veb_idx = veb->veb_idx;
41c445
[Intel-wired-lan] [PATCH net-next] ice: fix error code in ice_eswitch_attach()
Set the "err" variable on this error path.
Fixes: fff292b47ac1 ("ice: add VF representors one by one")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_eswitch.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c
b/drivers/net/ethernet/intel/ice/ice_eswitch.c
index 3f80e2081e5d..ca118bc37e44 100644
--- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
+++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
@@ -669,8 +669,10 @@ ice_eswitch_attach(struct ice_pf *pf, struct ice_vf *vf)
ice_eswitch_stop_reprs(pf);
repr = ice_repr_add_vf(vf);
- if (IS_ERR(repr))
+ if (IS_ERR(repr)) {
+ err = PTR_ERR(repr);
goto err_create_repr;
+ }
err = ice_eswitch_setup_repr(pf, repr);
if (err)
--
2.42.0
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
[Intel-wired-lan] [tnguy-next-queue:dev-queue 50/51] drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2391 ixgbe_clean_rx_irq() error: uninitialized symbol 'xdp_res'.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
dev-queue
head: 02a4f6b6bf8145719b1318ef112758c889660044
commit: 25892a448d275daf1dd6714aa5ffa51ad78e9dda [50/51] ixgbe: Fix passing 0
to ERR_PTR in ixgbe_run_xdp()
config: x86_64-randconfig-161-20241020
(https://download.01.org/0day-ci/archive/20241021/202410210921.qwq9giqr-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202410210921.qwq9giqr-...@intel.com/
New smatch warnings:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2391 ixgbe_clean_rx_irq() error:
uninitialized symbol 'xdp_res'.
vim +/xdp_res +2391 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
5a85e737f30ce7 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Eliezer Tamir
2013-06-10 2328 static int ixgbe_clean_rx_irq(struct ixgbe_q_vector
*q_vector,
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2329 struct ixgbe_ring *rx_ring,
f4de00ed58df50 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-09-25 2330 const int budget)
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2331 {
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2332unsigned int total_rx_bytes = 0, total_rx_packets = 0,
frame_sz = 0;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2333struct ixgbe_adapter *adapter = q_vector->adapter;
33fdc82f08835d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c John Fastabend
2017-04-24 2334 #ifdef IXGBE_FCOE
4ffdf91a5feae6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Mark Rustad
2012-07-18 2335int ddp_bytes;
4ffdf91a5feae6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Mark Rustad
2012-07-18 2336unsigned int mss = 0;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2337 #endif /* IXGBE_FCOE */
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2338u16 cleaned_count = ixgbe_desc_unused(rx_ring);
c0d4e9d223c5f4 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Maciej Fijalkowski
2021-01-18 2339unsigned int offset = rx_ring->rx_offset;
ad088ec4807688 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-06-26 2340unsigned int xdp_xmit = 0;
99ffc5ade4e870 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-01-03 2341struct xdp_buff xdp;
25892a448d275d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Yue Haibing
2024-10-18 2342int xdp_res;
99ffc5ade4e870 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-01-03 2343
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2344/* Frame size depend on rx_ring setup when
PAGE_SIZE=4K */
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2345 #if (PAGE_SIZE < 8192)
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2346frame_sz = ixgbe_rx_frame_truesize(rx_ring, 0);
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2347 #endif
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2348xdp_init_buff(&xdp, frame_sz, &rx_ring->xdp_rxq);
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2349
fdabfc8a74c713 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Eric W. Biederman
2014-03-14 2350while (likely(total_rx_packets < budget)) {
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2351union ixgbe_adv_rx_desc *rx_desc;
3fd218767fa498 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2017-01-17 2352struct ixgbe_rx_buffer *rx_buffer;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2353struct sk_buff *skb;
a06316dc87bdc0 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Björn Töpel
2020-08-25 2354int rx_buffer_pgcnt;
3fd218767fa498 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2017-01-17 2355unsigned int size;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2356
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2357/* return some bu
[Intel-wired-lan] [tnguy-net-queue:dev-queue 9/14] drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2377 ixgbe_clean_rx_irq() error: uninitialized symbol 'xdp_res'.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git
dev-queue
head: 278dfaa171a0061a341f6b5d44c2c9913a2b7fa8
commit: c919a57ea9cdd6aa02e0e411d8fdb3e3485353cc [9/14] ixgbe: Fix passing 0 to
ERR_PTR in ixgbe_run_xdp()
config: powerpc64-randconfig-r071-20241104
(https://download.01.org/0day-ci/archive/20241105/202411052110.vjxfpzue-...@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project
639a7ac648f1e50ccd2556e17d401c04f9cce625)
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202411052110.vjxfpzue-...@intel.com/
New smatch warnings:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2377 ixgbe_clean_rx_irq() error:
uninitialized symbol 'xdp_res'.
vim +/xdp_res +2377 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
5a85e737f30ce7 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Eliezer Tamir
2013-06-10 2314 static int ixgbe_clean_rx_irq(struct ixgbe_q_vector
*q_vector,
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2315 struct ixgbe_ring *rx_ring,
f4de00ed58df50 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-09-25 2316 const int budget)
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2317 {
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2318unsigned int total_rx_bytes = 0, total_rx_packets = 0,
frame_sz = 0;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2319struct ixgbe_adapter *adapter = q_vector->adapter;
33fdc82f08835d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c John Fastabend
2017-04-24 2320 #ifdef IXGBE_FCOE
4ffdf91a5feae6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Mark Rustad
2012-07-18 2321int ddp_bytes;
4ffdf91a5feae6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Mark Rustad
2012-07-18 2322unsigned int mss = 0;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2323 #endif /* IXGBE_FCOE */
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2324u16 cleaned_count = ixgbe_desc_unused(rx_ring);
c0d4e9d223c5f4 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Maciej Fijalkowski
2021-01-18 2325unsigned int offset = rx_ring->rx_offset;
ad088ec4807688 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-06-26 2326unsigned int xdp_xmit = 0;
99ffc5ade4e870 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-01-03 2327struct xdp_buff xdp;
c919a57ea9cdd6 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Yue Haibing
2024-10-18 2328int xdp_res;
99ffc5ade4e870 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2018-01-03 2329
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2330/* Frame size depend on rx_ring setup when
PAGE_SIZE=4K */
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2331 #if (PAGE_SIZE < 8192)
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2332frame_sz = ixgbe_rx_frame_truesize(rx_ring, 0);
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2333 #endif
43b5169d8355cc drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Lorenzo Bianconi
2020-12-22 2334xdp_init_buff(&xdp, frame_sz, &rx_ring->xdp_rxq);
cf02512899805d drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Jesper Dangaard
Brouer 2020-05-14 2335
fdabfc8a74c713 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Eric W. Biederman
2014-03-14 2336while (likely(total_rx_packets < budget)) {
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2337union ixgbe_adv_rx_desc *rx_desc;
3fd218767fa498 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2017-01-17 2338struct ixgbe_rx_buffer *rx_buffer;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2339struct sk_buff *skb;
a06316dc87bdc0 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Björn Töpel
2020-08-25 2340int rx_buffer_pgcnt;
3fd218767fa498 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2017-01-17 2341unsigned int size;
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c Alexander Duyck
2012-07-20 2342
18806c9ea28320 drivers/net/ethernet/intel/ixgbe/ixgbe_ma
[Intel-wired-lan] [tnguy-next-queue:dev-queue 16/69] drivers/net/ethernet/intel/ice/devlink/health.c:95 ice_tx_hang_reporter_dump() warn: variable dereferenced before check 'event' (see line 93)
tree: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
dev-queue
head: 4b8b07968dc902d751d23a775c1a02220db267c9
commit: 9f889e8ffb48536622852fbe2501dc8b75887239 [16/69] ice: dump ethtool
stats and skb by Tx hang devlink health reporter
config: microblaze-randconfig-r072-20241124
(https://download.01.org/0day-ci/archive/20241124/202411242101.qlvmbflb-...@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 14.2.0
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202411242101.qlvmbflb-...@intel.com/
smatch warnings:
drivers/net/ethernet/intel/ice/devlink/health.c:95 ice_tx_hang_reporter_dump()
warn: variable dereferenced before check 'event' (see line 93)
vim +/event +95 drivers/net/ethernet/intel/ice/devlink/health.c
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 86 static int
ice_tx_hang_reporter_dump(struct devlink_health_reporter *reporter,
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 87
struct devlink_fmsg *fmsg, void *priv_ctx,
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 88
struct netlink_ext_ack *extack)
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 89 {
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 90 struct
ice_tx_hang_event *event = priv_ctx;
9f889e8ffb4853 Przemek Kitszel 2024-09-30 91 struct sk_buff *skb;
9f889e8ffb4853 Przemek Kitszel 2024-09-30 92
9f889e8ffb4853 Przemek Kitszel 2024-09-30 @93 skb =
event->tx_ring->tx_buf->skb;
^^^
Dereference
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 94
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 @95 if (!event)
^^
Too late
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 96 return 0;
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 97
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 98
devlink_fmsg_obj_nest_start(fmsg);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 99
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, head);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 100
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, intr);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 101
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, vsi_num);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 102
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, queue);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 103
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_clean);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 104
ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_use);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 105 devlink_fmsg_put(fmsg,
"irq-mapping", event->tx_ring->q_vector->name);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 106 ice_fmsg_put_ptr(fmsg,
"desc-ptr", event->tx_ring->desc);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 107 ice_fmsg_put_ptr(fmsg,
"dma-ptr", (void *)(long)event->tx_ring->dma);
9f889e8ffb4853 Przemek Kitszel 2024-09-30 108 ice_fmsg_put_ptr(fmsg,
"skb-ptr", skb);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 109
devlink_fmsg_binary_pair_put(fmsg, "desc", event->tx_ring->desc,
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 110
event->tx_ring->count * sizeof(struct ice_tx_desc));
9f889e8ffb4853 Przemek Kitszel 2024-09-30 111
devlink_fmsg_dump_skb(fmsg, skb);
9f889e8ffb4853 Przemek Kitszel 2024-09-30 112
ice_dump_ethtool_stats_to_fmsg(fmsg, event->tx_ring->vsi->netdev);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 113
devlink_fmsg_obj_nest_end(fmsg);
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 114
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 115 return 0;
f19878b2a2e6e4 Przemek Kitszel 2024-09-30 116 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [Intel-wired-lan] [PATCH net-next v3 3/6] net: napi: add CPU affinity to napi_config
Hi Ahmed,
kernel test robot noticed the following build warnings:
url:
https://github.com/intel-lab-lkp/linux/commits/Ahmed-Zaki/net-move-ARFS-rmap-management-to-core/20250104-084501
base: net-next/main
patch link:
https://lore.kernel.org/r/20250104004314.208259-4-ahmed.zaki%40intel.com
patch subject: [Intel-wired-lan] [PATCH net-next v3 3/6] net: napi: add CPU
affinity to napi_config
config: i386-randconfig-141-20250104
(https://download.01.org/0day-ci/archive/20250105/202501050625.ny1c97ex-...@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project
ab51eccf88f5321e7c60591c5546b254b6afab99)
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
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202501050625.ny1c97ex-...@intel.com/
smatch warnings:
net/core/dev.c:6835 napi_restore_config() warn: variable dereferenced before
check 'n->config' (see line 6831)
net/core/dev.c:6855 napi_save_config() warn: variable dereferenced before check
'n->config' (see line 6850)
vim +6835 net/core/dev.c
86e25f40aa1e9e5 Joe Damato 2024-10-11 6829 static void
napi_restore_config(struct napi_struct *n)
86e25f40aa1e9e5 Joe Damato 2024-10-11 6830 {
86e25f40aa1e9e5 Joe Damato 2024-10-11 @6831 n->defer_hard_irqs =
n->config->defer_hard_irqs;
86e25f40aa1e9e5 Joe Damato 2024-10-11 6832 n->gro_flush_timeout =
n->config->gro_flush_timeout;
5dc51ec86df6e22 Martin Karsten 2024-11-09 6833 n->irq_suspend_timeout
= n->config->irq_suspend_timeout;
^
These lines all dereference n->config.
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6834
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 @6835 if (n->irq > 0 &&
n->config && n->dev->irq_affinity_auto)
^
This code assumes it can be NULL
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6836
irq_set_affinity(n->irq, &n->config->affinity_mask);
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6837
86e25f40aa1e9e5 Joe Damato 2024-10-11 6838 /* a NAPI ID might be
stored in the config, if so use it. if not, use
86e25f40aa1e9e5 Joe Damato 2024-10-11 6839 * napi_hash_add to
generate one for us. It will be saved to the config
86e25f40aa1e9e5 Joe Damato 2024-10-11 6840 * in napi_disable.
86e25f40aa1e9e5 Joe Damato 2024-10-11 6841 */
86e25f40aa1e9e5 Joe Damato 2024-10-11 6842 if (n->config->napi_id)
86e25f40aa1e9e5 Joe Damato 2024-10-11 6843
napi_hash_add_with_id(n, n->config->napi_id);
86e25f40aa1e9e5 Joe Damato 2024-10-11 6844 else
86e25f40aa1e9e5 Joe Damato 2024-10-11 6845
napi_hash_add(n);
86e25f40aa1e9e5 Joe Damato 2024-10-11 6846 }
86e25f40aa1e9e5 Joe Damato 2024-10-11 6847
86e25f40aa1e9e5 Joe Damato 2024-10-11 6848 static void
napi_save_config(struct napi_struct *n)
86e25f40aa1e9e5 Joe Damato 2024-10-11 6849 {
86e25f40aa1e9e5 Joe Damato 2024-10-11 @6850
n->config->defer_hard_irqs = n->defer_hard_irqs;
86e25f40aa1e9e5 Joe Damato 2024-10-11 6851
n->config->gro_flush_timeout = n->gro_flush_timeout;
5dc51ec86df6e22 Martin Karsten 2024-11-09 6852
n->config->irq_suspend_timeout = n->irq_suspend_timeout;
86e25f40aa1e9e5 Joe Damato 2024-10-11 6853 n->config->napi_id =
n->napi_id;
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6854
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 @6855 if (n->irq > 0 &&
n->config && n->dev->irq_affinity_auto)
Same
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6856
irq_set_affinity_notifier(n->irq, NULL);
d6b43b8a2e5297b Ahmed Zaki 2025-01-03 6857
86e25f40aa1e9e5 Joe Damato 2024-10-11 6858 napi_hash_del(n);
86e25f40aa1e9e5 Joe Damato 2024-10-11 6859 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
[Intel-wired-lan] [bug report] ixgbe: Add link management support for E610 device
Hello Piotr Kwapulinski,
Commit 23c0e5a16bcc ("ixgbe: Add link management support for E610
device") from Dec 5, 2024 (linux-next), leads to the following Smatch
static checker warning:
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:1125
ixgbe_is_media_cage_present()
warn: signedness bug returning '(-95)'
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
1103 static bool ixgbe_is_media_cage_present(struct ixgbe_hw *hw)
1104 {
1105 struct ixgbe_aci_cmd_get_link_topo *cmd;
1106 struct ixgbe_aci_desc desc;
1107
1108 cmd = &desc.params.get_link_topo;
1109
1110 ixgbe_fill_dflt_direct_cmd_desc(&desc,
ixgbe_aci_opc_get_link_topo);
1112 cmd->addr.topo_params.node_type_ctx =
1113 FIELD_PREP(IXGBE_ACI_LINK_TOPO_NODE_CTX_M,
1114IXGBE_ACI_LINK_TOPO_NODE_CTX_PORT);
1115
1116 /* Set node type. */
1117 cmd->addr.topo_params.node_type_ctx |=
1118 FIELD_PREP(IXGBE_ACI_LINK_TOPO_NODE_TYPE_M,
1119IXGBE_ACI_LINK_TOPO_NODE_TYPE_CAGE);
1120
1121 /* Node type cage can be used to determine if cage is present.
If AQC
1122 * returns error (ENOENT), then no cage present. If no cage
present then
^^
1123 * connection type is backplane or BASE-T.
1124 */
--> 1125 return ixgbe_aci_get_netlist_node(hw, cmd, NULL, NULL);
This is a bool function. Based on the name, it should return true for
present and false for not but it does the reverse. I don't know the code
well enough to say if the returns should be changed or the function name.
The comment says that ixgbe_aci_get_netlist_node() returns -ENOENT but
actually the only error code it returns is -EOPNOTSUPP.
1126 }
regards,
dan carpenter
[Intel-wired-lan] [PATCH next] ice: Fix signedness bug in ice_init_interrupt_scheme()
If pci_alloc_irq_vectors() can't allocate the minimum number of vectors
then it returns -ENOSPC so there is no need to check for that in the
caller. In fact, because pf->msix.min is an unsigned int, it means that
any negative error codes are type promoted to high positive values and
treated as success. So here the "return -ENOMEM;" is unreachable code.
Check for negatives instead.
Fixes: 79d97b8cf9a8 ("ice: remove splitting MSI-X between features")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/intel/ice/ice_irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c
b/drivers/net/ethernet/intel/ice/ice_irq.c
index cbae3d81f0f1..b1fdad154203 100644
--- a/drivers/net/ethernet/intel/ice/ice_irq.c
+++ b/drivers/net/ethernet/intel/ice/ice_irq.c
@@ -149,7 +149,7 @@ int ice_init_interrupt_scheme(struct ice_pf *pf)
vectors = pci_alloc_irq_vectors(pf->pdev, pf->msix.min, vectors,
PCI_IRQ_MSIX);
- if (vectors < pf->msix.min)
+ if (vectors < 0)
return -ENOMEM;
ice_init_irq_tracker(pf, pf->msix.max, vectors);
--
2.47.2
[Intel-wired-lan] [bug report] ixgbe: Fix passing 0 to ERR_PTR in ixgbe_run_xdp()
Hello Yue Haibing,
Commit c824125cbb18 ("ixgbe: Fix passing 0 to ERR_PTR in
ixgbe_run_xdp()") from Jan 6, 2025 (linux-next), leads to the
following Smatch static checker warning:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2108 ixgbe_put_rx_buffer()
warn: possible NULL dereference of 'skb'
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
2099 static void ixgbe_put_rx_buffer(struct ixgbe_ring *rx_ring,
2100 struct ixgbe_rx_buffer *rx_buffer,
2101 struct sk_buff *skb,
2102 int rx_buffer_pgcnt)
2103 {
2104 if (ixgbe_can_reuse_rx_page(rx_buffer, rx_buffer_pgcnt)) {
2105 /* hand second half of page back to the ring */
2106 ixgbe_reuse_rx_page(rx_ring, rx_buffer);
2107 } else {
--> 2108 if (!IS_ERR(skb) && IXGBE_CB(skb)->dma ==
rx_buffer->dma) {
^^^
This can't be an error pointer and probably it should be a NULL check.
I'm not sure if your patch introduced this issue or just exposed it.
2109 /* the page has been released from the ring */
2110 IXGBE_CB(skb)->page_released = true;
2111 } else {
2112 /* we are not reusing the buffer so unmap it */
2113 dma_unmap_page_attrs(rx_ring->dev,
rx_buffer->dma,
2114 ixgbe_rx_pg_size(rx_ring),
2115 DMA_FROM_DEVICE,
2116 IXGBE_RX_DMA_ATTR);
2117 }
2118 __page_frag_cache_drain(rx_buffer->page,
2119 rx_buffer->pagecnt_bias);
2120 }
2121
2122 /* clear contents of rx_buffer */
2123 rx_buffer->page = NULL;
2124 rx_buffer->skb = NULL;
2125 }
regards,
dan carpenter
[Intel-wired-lan] [PATCH v2 net-next] ice: Fix signedness bug in ice_init_interrupt_scheme()
If pci_alloc_irq_vectors() can't allocate the minimum number of vectors
then it returns -ENOSPC so there is no need to check for that in the
caller. In fact, because pf->msix.min is an unsigned int, it means that
any negative error codes are type promoted to high positive values and
treated as success. So here, the "return -ENOMEM;" is unreachable code.
Check for negatives instead.
Now that we're only dealing with error codes, it's easier to propagate
the error code from pci_alloc_irq_vectors() instead of hardcoding
-ENOMEM.
Fixes: 79d97b8cf9a8 ("ice: remove splitting MSI-X between features")
Signed-off-by: Dan Carpenter
---
v2: Fix my scripts to say [PATCH net-next]
Propagate the error code.
drivers/net/ethernet/intel/ice/ice_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c
b/drivers/net/ethernet/intel/ice/ice_irq.c
index cbae3d81f0f1..30801fd375f0 100644
--- a/drivers/net/ethernet/intel/ice/ice_irq.c
+++ b/drivers/net/ethernet/intel/ice/ice_irq.c
@@ -149,8 +149,8 @@ int ice_init_interrupt_scheme(struct ice_pf *pf)
vectors = pci_alloc_irq_vectors(pf->pdev, pf->msix.min, vectors,
PCI_IRQ_MSIX);
- if (vectors < pf->msix.min)
- return -ENOMEM;
+ if (vectors < 0)
+ return vectors;
ice_init_irq_tracker(pf, pf->msix.max, vectors);
--
2.47.2
Re: [Intel-wired-lan] [PATCH next] ice: Fix signedness bug in ice_init_interrupt_scheme()
On Wed, Feb 12, 2025 at 05:59:01PM -0800, Jakub Kicinski wrote:
> On Wed, 12 Feb 2025 17:46:54 +0100 Alexander Lobakin wrote:
> > > [PATCH next] ice: Fix signedness bug in ice_init_interrupt_scheme()
> >
> > I believe it should be "PATCH net" with
> >
> > > If pci_alloc_irq_vectors() can't allocate the minimum number of vectors
> > > then it returns -ENOSPC so there is no need to check for that in the
> > > caller. In fact, because pf->msix.min is an unsigned int, it means that
> > > any negative error codes are type promoted to high positive values and
> > > treated as success. So here the "return -ENOMEM;" is unreachable code.
> > > Check for negatives instead.
> > >
> > > Fixes: 79d97b8cf9a8 ("ice: remove splitting MSI-X between features")
> >
> > a 'Stable:' tag here.
>
> Bug only exists in net-next if it comes from commit under Fixes.
> So I think the patch is good as is.
I want to resen this. My scripts should have put a net-next in the
subject and I think that changing:
- return -ENOMEM;
+ return vectors;
actually does fall within the scope of the patch so I want to change
that as well. There is no point in really breaking that into a separate
patch from a practical perspective.
regards,
dan carpenter
[Intel-wired-lan] [bug report] igc: add lock preventing multiple simultaneous PTM transactions
Hello Christopher S M Hall,
Commit 1a931c4f5e68 ("igc: add lock preventing multiple simultaneous
PTM transactions") from Apr 1, 2025 (linux-next), leads to the
following Smatch static checker warning:
drivers/net/ethernet/intel/igc/igc_ptp.c:1311 igc_ptp_reset()
warn: sleeping in atomic context
drivers/net/ethernet/intel/igc/igc_ptp.c
1280 void igc_ptp_reset(struct igc_adapter *adapter)
1281 {
1282 struct igc_hw *hw = &adapter->hw;
1283 u32 cycle_ctrl, ctrl, stat;
1284 unsigned long flags;
1285 u32 timadj;
1286
1287 if (!(adapter->ptp_flags & IGC_PTP_ENABLED))
1288 return;
1289
1290 /* reset the tstamp_config */
1291 igc_ptp_set_timestamp_mode(adapter, &adapter->tstamp_config);
1292
1293 spin_lock_irqsave(&adapter->tmreg_lock, flags);
^
Holding a spin_lock
1294
1295 switch (adapter->hw.mac.type) {
1296 case igc_i225:
1297 timadj = rd32(IGC_TIMADJ);
1298 timadj |= IGC_TIMADJ_ADJUST_METH;
1299 wr32(IGC_TIMADJ, timadj);
1300
1301 wr32(IGC_TSAUXC, 0x0);
1302 wr32(IGC_TSSDP, 0x0);
1303 wr32(IGC_TSIM,
1304 IGC_TSICR_INTERRUPTS |
1305 (adapter->pps_sys_wrap_on ? IGC_TSICR_SYS_WRAP :
0));
1306 wr32(IGC_IMS, IGC_IMS_TS);
1307
1308 if (!igc_is_crosststamp_supported(adapter))
1309 break;
1310
--> 1311 mutex_lock(&adapter->ptm_lock);
^^
So we can't take a mutex.
1312 wr32(IGC_PCIE_DIG_DELAY, IGC_PCIE_DIG_DELAY_DEFAULT);
1313 wr32(IGC_PCIE_PHY_DELAY, IGC_PCIE_PHY_DELAY_DEFAULT);
1314
1315 cycle_ctrl =
IGC_PTM_CYCLE_CTRL_CYC_TIME(IGC_PTM_CYC_TIME_DEFAULT);
1316
1317 wr32(IGC_PTM_CYCLE_CTRL, cycle_ctrl);
1318
1319 ctrl = IGC_PTM_CTRL_EN |
1320 IGC_PTM_CTRL_START_NOW |
1321
IGC_PTM_CTRL_SHRT_CYC(IGC_PTM_SHORT_CYC_DEFAULT) |
1322 IGC_PTM_CTRL_PTM_TO(IGC_PTM_TIMEOUT_DEFAULT);
1323
1324 wr32(IGC_PTM_CTRL, ctrl);
1325
1326 /* Force the first cycle to run. */
1327 igc_ptm_trigger(hw);
1328
1329 if (readx_poll_timeout_atomic(rd32, IGC_PTM_STAT, stat,
1330 stat, IGC_PTM_STAT_SLEEP,
1331 IGC_PTM_STAT_TIMEOUT))
1332 netdev_err(adapter->netdev, "Timeout reading
IGC_PTM_STAT register\n");
1333
1334 igc_ptm_reset(hw);
1335 mutex_unlock(&adapter->ptm_lock);
1336 break;
1337 default:
1338 /* No work to do. */
1339 goto out;
1340 }
1341
1342 /* Re-initialize the timer. */
1343 if (hw->mac.type == igc_i225) {
1344 igc_ptp_time_restore(adapter);
1345 } else {
1346 timecounter_init(&adapter->tc, &adapter->cc,
1347 ktime_to_ns(ktime_get_real()));
1348 }
1349 out:
1350 spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
1351
1352 wrfl();
1353 }
regards,
dan carpenter
