From: Tony Nguyen <anthony.l.nguyen@intel.com>
To: Julia Lawall <julia.lawall@inria.fr>,
Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <oe-kbuild-all@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
Jacob Keller <jacob.e.keller@intel.com>,
"Simon Horman" <horms@kernel.org>,
Dan Carpenter <dan.carpenter@linaro.org>
Subject: Re: drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c:678:6-11: ERROR: invalid reference to the index variable of the iterator on line 665 (fwd)
Date: Mon, 25 Nov 2024 09:53:58 -0800 [thread overview]
Message-ID: <224ee788-1371-4b57-b2e8-c595aa0a2b2a@intel.com> (raw)
In-Reply-To: <27f489a5-1758-c0a2-f977-bd17a26d6dbf@inria.fr>
On 11/23/2024 7:11 AM, Julia Lawall wrote:
> Please see lines 663-679. Setting entry to NULL is useless. The
> list_for_each_entry will overwrite it, even if the list is empty. The
> !empty test on line 678 is likewise useless, and the !entry->free est on
> the same line may return a meaningless value if the list is empty.
> Consider adding a flag variable to record whether the break was taken.
Hi Julia,
I believe this has already been addressed via commit a41654c3ed1d
("ixgbe: fix end of loop test in ixgbe_set_vf_macvlan()").
Thanks,
Tony
> julia
>
> ---------- Forwarded message ----------
> Date: Fri, 22 Nov 2024 14:07:03 +0800
> From: kernel test robot <lkp@intel.com>
> To: oe-kbuild@lists.linux.dev
> Cc: lkp@intel.com, Julia Lawall <julia.lawall@inria.fr>
> Subject: drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c:678:6-11: ERROR: invalid
> reference to the index variable of the iterator on line 665
>
> BCC: lkp@intel.com
> CC: oe-kbuild-all@lists.linux.dev
> CC: linux-kernel@vger.kernel.org
> TO: Jinjie Ruan <ruanjinjie@huawei.com>
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Jacob Keller <jacob.e.keller@intel.com>
> CC: Tony Nguyen <anthony.l.nguyen@intel.com>
> CC: Simon Horman <horms@kernel.org>
>
> Hi Jinjie,
>
> FYI, the error/warning was bisected to this commit, please ignore it if it's irrelevant.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 28eb75e178d389d325f1666e422bc13bbbb9804c
> commit: c1fec890458ad101ddbbc52cdd29f7bba6aa2b10 ethernet/intel: Use list_for_each_entry() helper
> date: 1 year, 2 months ago
> :::::: branch date: 7 hours ago
> :::::: commit date: 1 year, 2 months ago
> config: riscv-randconfig-r051-20241122 (https://download.01.org/0day-ci/archive/20241122/202411221358.yZAZKbuI-lkp@intel.com/config)
> compiler: riscv64-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 <lkp@intel.com>
> | Reported-by: Julia Lawall <julia.lawall@inria.fr>
> | Closes: https://lore.kernel.org/r/202411221358.yZAZKbuI-lkp@intel.com/
>
> cocci warnings: (new ones prefixed by >>)
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c:678:6-11: ERROR: invalid reference to the index variable of the iterator on line 665
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c:678:16-21: ERROR: invalid reference to the index variable of the iterator on line 665
>
> vim +678 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>
> 4c7f35f679f592 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Alexander Duyck 2015-11-02 637
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 638 static int ixgbe_set_vf_macvlan(struct ixgbe_adapter *adapter,
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 639 int vf, int index, unsigned char *mac_addr)
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 640 {
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 641 struct vf_macvlans *entry;
> 0e1ff3061cb529 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Tony Nguyen 2017-07-19 642 int retval = 0;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 643
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 644 if (index <= 1) {
> c1fec890458ad1 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Jinjie Ruan 2023-09-19 645 list_for_each_entry(entry, &adapter->vf_mvs.l, l) {
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 646 if (entry->vf == vf) {
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 647 entry->vf = -1;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 648 entry->free = true;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 649 entry->is_macvlan = false;
> 5d7daa35b9eb14 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Jacob Keller 2014-03-29 650 ixgbe_del_mac_filter(adapter,
> 5d7daa35b9eb14 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Jacob Keller 2014-03-29 651 entry->vf_macvlan, vf);
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 652 }
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 653 }
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 654 }
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 655
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 656 /*
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 657 * If index was zero then we were asked to clear the uc list
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 658 * for the VF. We're done.
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 659 */
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 660 if (!index)
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 661 return 0;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 662
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 663 entry = NULL;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 664
> c1fec890458ad1 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Jinjie Ruan 2023-09-19 @665 list_for_each_entry(entry, &adapter->vf_mvs.l, l) {
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 666 if (entry->free)
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 667 break;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 668 }
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 669
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 670 /*
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 671 * If we traversed the entire list and didn't find a free entry
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 672 * then we're out of space on the RAR table. Also entry may
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 673 * be NULL because the original memory allocation for the list
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 674 * failed, which is not fatal but does mean we can't support
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 675 * VF requests for MACVLAN because we couldn't allocate
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 676 * memory for the list management required.
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 677 */
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 @678 if (!entry || !entry->free)
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 679 return -ENOSPC;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 680
> 0e1ff3061cb529 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Tony Nguyen 2017-07-19 681 retval = ixgbe_add_mac_filter(adapter, mac_addr, vf);
> 0e1ff3061cb529 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Tony Nguyen 2017-07-19 682 if (retval < 0)
> 0e1ff3061cb529 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Tony Nguyen 2017-07-19 683 return retval;
> 0e1ff3061cb529 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c Tony Nguyen 2017-07-19 684
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 685 entry->free = false;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 686 entry->is_macvlan = true;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 687 entry->vf = vf;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 688 memcpy(entry->vf_macvlan, mac_addr, ETH_ALEN);
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 689
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 690 return 0;
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 691 }
> a1cbb15c13971b drivers/net/ixgbe/ixgbe_sriov.c Greg Rose 2011-05-13 692
>
> :::::: The code at line 678 was first introduced by commit
> :::::: a1cbb15c13971bd5d41626e9e5ced9f9de132c47 ixgbe: Add macvlan support for VF
>
> :::::: TO: Greg Rose <gregory.v.rose@intel.com>
> :::::: CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
next prev parent reply other threads:[~2024-11-25 17:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-23 15:11 Julia Lawall
2024-11-25 17:53 ` Tony Nguyen [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-10-30 18:09 Julia Lawall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=224ee788-1371-4b57-b2e8-c595aa0a2b2a@intel.com \
--to=anthony.l.nguyen@intel.com \
--cc=dan.carpenter@linaro.org \
--cc=horms@kernel.org \
--cc=jacob.e.keller@intel.com \
--cc=julia.lawall@inria.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=pabeni@redhat.com \
--cc=ruanjinjie@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®