* [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
@ 2022-06-28 16:53 Tony Nguyen
2022-06-28 17:27 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Tony Nguyen @ 2022-06-28 16:53 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet
Cc: Piotr Skajewski, netdev, anthony.l.nguyen, Konrad Jankowski
From: Piotr Skajewski <piotrx.skajewski@intel.com>
It is possible to disable VFs while the PF driver is processing requests
from the VF driver. This can result in a panic.
BUG: unable to handle kernel paging request at 000000000000106c
PGD 0 P4D 0
Oops: 0000 [#1] SMP NOPTI
CPU: 8 PID: 0 Comm: swapper/8 Kdump: loaded Tainted: G I --------- -
Hardware name: Dell Inc. PowerEdge R740/06WXJT, BIOS 2.8.2 08/27/2020
RIP: 0010:ixgbe_msg_task+0x4c8/0x1690 [ixgbe]
Code: 00 00 48 8d 04 40 48 c1 e0 05 89 7c 24 24 89 fd 48 89 44 24 10 83 ff
01 0f 84 b8 04 00 00 4c 8b 64 24 10 4d 03 a5 48 22 00 00 <41> 80 7c 24 4c
00 0f 84 8a 03 00 00 0f b7 c7 83 f8 08 0f 84 8f 0a
RSP: 0018:ffffb337869f8df8 EFLAGS: 00010002
RAX: 0000000000001020 RBX: 0000000000000000 RCX: 000000000000002b
RDX: 0000000000000002 RSI: 0000000000000008 RDI: 0000000000000006
RBP: 0000000000000006 R08: 0000000000000002 R09: 0000000000029780
R10: 00006957d8f42832 R11: 0000000000000000 R12: 0000000000001020
R13: ffff8a00e8978ac0 R14: 000000000000002b R15: ffff8a00e8979c80
FS: 0000000000000000(0000) GS:ffff8a07dfd00000(0000) knlGS:00000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000106c CR3: 0000000063e10004 CR4: 00000000007726e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<IRQ>
? ttwu_do_wakeup+0x19/0x140
? try_to_wake_up+0x1cd/0x550
? ixgbevf_update_xcast_mode+0x71/0xc0 [ixgbevf]
ixgbe_msix_other+0x17e/0x310 [ixgbe]
__handle_irq_event_percpu+0x40/0x180
handle_irq_event_percpu+0x30/0x80
handle_irq_event+0x36/0x53
handle_edge_irq+0x82/0x190
handle_irq+0x1c/0x30
do_IRQ+0x49/0xd0
common_interrupt+0xf/0xf
This can be eventually be reproduced with the following script:
while :
do
echo 63 > /sys/class/net/<devname>/device/sriov_numvfs
sleep 1
echo 0 > /sys/class/net/<devname>/device/sriov_numvfs
sleep 1
done
Add lock when disabling SR-IOV to prevent process VF mailbox communication.
Fixes: d773d1310625 ("ixgbe: Fix memory leak when SR-IOV VFs are direct assigned")
Signed-off-by: Piotr Skajewski <piotrx.skajewski@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 ++
.../net/ethernet/intel/ixgbe/ixgbe_sriov.c | 30 ++++++++++++-------
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 921a4d977d65..8813b4dd6872 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -779,6 +779,7 @@ struct ixgbe_adapter {
#ifdef CONFIG_IXGBE_IPSEC
struct ixgbe_ipsec *ipsec;
#endif /* CONFIG_IXGBE_IPSEC */
+ spinlock_t vfs_lock;
};
static inline int ixgbe_determine_xdp_q_idx(int cpu)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 77c2e70b0860..55f91c9ff047 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6403,6 +6403,9 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
/* n-tuple support exists, always init our spinlock */
spin_lock_init(&adapter->fdir_perfect_lock);
+ /* init spinlock to avoid concurrency of VF resources */
+ spin_lock_init(&adapter->vfs_lock);
+
#ifdef CONFIG_IXGBE_DCB
ixgbe_init_dcb(adapter);
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index d4e63f0644c3..7f132322e2e3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -205,8 +205,11 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter, unsigned int max_vfs)
int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
{
unsigned int num_vfs = adapter->num_vfs, vf;
+ unsigned long flags;
int rss;
+ spin_lock_irqsave(&adapter->vfs_lock, flags);
+
/* set num VFs to 0 to prevent access to vfinfo */
adapter->num_vfs = 0;
@@ -228,6 +231,8 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
kfree(adapter->mv_list);
adapter->mv_list = NULL;
+ spin_unlock_irqrestore(&adapter->vfs_lock, flags);
+
/* if SR-IOV is already disabled then there is nothing to do */
if (!(adapter->flags & IXGBE_FLAG_SRIOV_ENABLED))
return 0;
@@ -1355,21 +1360,26 @@ static void ixgbe_rcv_ack_from_vf(struct ixgbe_adapter *adapter, u32 vf)
void ixgbe_msg_task(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
+ unsigned long flags;
u32 vf;
- for (vf = 0; vf < adapter->num_vfs; vf++) {
- /* process any reset requests */
- if (!ixgbe_check_for_rst(hw, vf))
- ixgbe_vf_reset_event(adapter, vf);
+ spin_lock_irqsave(&adapter->vfs_lock, flags);
+ if (adapter->vfinfo) {
+ for (vf = 0; vf < adapter->num_vfs; vf++) {
+ /* process any reset requests */
+ if (!ixgbe_check_for_rst(hw, vf))
+ ixgbe_vf_reset_event(adapter, vf);
- /* process any messages pending */
- if (!ixgbe_check_for_msg(hw, vf))
- ixgbe_rcv_msg_from_vf(adapter, vf);
+ /* process any messages pending */
+ if (!ixgbe_check_for_msg(hw, vf))
+ ixgbe_rcv_msg_from_vf(adapter, vf);
- /* process any acks */
- if (!ixgbe_check_for_ack(hw, vf))
- ixgbe_rcv_ack_from_vf(adapter, vf);
+ /* process any acks */
+ if (!ixgbe_check_for_ack(hw, vf))
+ ixgbe_rcv_ack_from_vf(adapter, vf);
+ }
}
+ spin_unlock_irqrestore(&adapter->vfs_lock, flags);
}
static inline void ixgbe_ping_vf(struct ixgbe_adapter *adapter, int vf)
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2022-06-28 16:53 [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero Tony Nguyen
@ 2022-06-28 17:27 ` Jakub Kicinski
2022-06-30 10:08 ` Piotr Skajewski
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2022-06-28 17:27 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, pabeni, edumazet, Piotr Skajewski, netdev, Konrad Jankowski
On Tue, 28 Jun 2022 09:53:46 -0700 Tony Nguyen wrote:
> + spin_lock_irqsave(&adapter->vfs_lock, flags);
> +
> /* set num VFs to 0 to prevent access to vfinfo */
> adapter->num_vfs = 0;
>
> @@ -228,6 +231,8 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
> kfree(adapter->mv_list);
> adapter->mv_list = NULL;
>
> + spin_unlock_irqrestore(&adapter->vfs_lock, flags);
There's a pci_dev_put() in there, are you sure it won't sleep?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2022-06-28 17:27 ` Jakub Kicinski
@ 2022-06-30 10:08 ` Piotr Skajewski
2022-06-30 15:11 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Piotr Skajewski @ 2022-06-30 10:08 UTC (permalink / raw)
To: kuba
Cc: anthony.l.nguyen, davem, edumazet, konrad0.jankowski, netdev,
pabeni, piotrx.skajewski
On Tue, 28 Jun 2022 10:27:07 -0700 Jakub Kicinski wrote:
> On Tue, 28 Jun 2022 09:53:46 -0700 Tony Nguyen wrote:
> > + spin_lock_irqsave(&adapter->vfs_lock, flags);
> > +
> > /* set num VFs to 0 to prevent access to vfinfo */
> > adapter->num_vfs = 0;
> >
> > @@ -228,6 +231,8 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
> > kfree(adapter->mv_list);
> > adapter->mv_list = NULL;
> >
> > + spin_unlock_irqrestore(&adapter->vfs_lock, flags);
>
> There's a pci_dev_put() in there, are you sure it won't sleep?
Thank Jakub for your notice, during development we were aware about this
and tests we've made on this particular case, did not report any problems
that could be related to might_sleep in conjunction with spinlock.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2022-06-30 10:08 ` Piotr Skajewski
@ 2022-06-30 15:11 ` Jakub Kicinski
2022-07-01 8:31 ` Piotr Skajewski
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2022-06-30 15:11 UTC (permalink / raw)
To: Piotr Skajewski
Cc: anthony.l.nguyen, davem, edumazet, konrad0.jankowski, netdev, pabeni
On Thu, 30 Jun 2022 12:08:39 +0200 Piotr Skajewski wrote:
> On Tue, 28 Jun 2022 10:27:07 -0700 Jakub Kicinski wrote:
> > On Tue, 28 Jun 2022 09:53:46 -0700 Tony Nguyen wrote:
> > > + spin_lock_irqsave(&adapter->vfs_lock, flags);
> > > +
> > > /* set num VFs to 0 to prevent access to vfinfo */
> > > adapter->num_vfs = 0;
> > >
> > > @@ -228,6 +231,8 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
> > > kfree(adapter->mv_list);
> > > adapter->mv_list = NULL;
> > >
> > > + spin_unlock_irqrestore(&adapter->vfs_lock, flags);
> >
> > There's a pci_dev_put() in there, are you sure it won't sleep?
>
> Thank Jakub for your notice, during development we were aware about this
> and tests we've made on this particular case, did not report any problems
> that could be related to might_sleep in conjunction with spinlock.
To be on the safe side how about we protect adapter->num_vfs instead
of adapter->vfinfo ?
You can hold the lock just around setting adapter->num_vfs to zero,
and then inside ixgbe_msg_task() you don't have to add the new if()
because the loop bound is already adapter->num_vfs.
Smaller change, and safer.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2022-06-30 15:11 ` Jakub Kicinski
@ 2022-07-01 8:31 ` Piotr Skajewski
0 siblings, 0 replies; 9+ messages in thread
From: Piotr Skajewski @ 2022-07-01 8:31 UTC (permalink / raw)
To: kuba
Cc: anthony.l.nguyen, davem, edumazet, konrad0.jankowski, netdev,
pabeni, piotrx.skajewski
On Thu, 30 Jun 2022 08:11:34 -0700 Jakub Kicinski wrote:
> On Thu, 30 Jun 2022 12:08:39 +0200 Piotr Skajewski wrote:
> > On Tue, 28 Jun 2022 10:27:07 -0700 Jakub Kicinski wrote:
> > > On Tue, 28 Jun 2022 09:53:46 -0700 Tony Nguyen wrote:
> > > > + spin_lock_irqsave(&adapter->vfs_lock, flags);
> > > > +
> > > > /* set num VFs to 0 to prevent access to vfinfo */
> > > > adapter->num_vfs = 0;
> > > >
> > > > @@ -228,6 +231,8 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
> > > > kfree(adapter->mv_list);
> > > > adapter->mv_list = NULL;
> > > >
> > > > + spin_unlock_irqrestore(&adapter->vfs_lock, flags);
> > >
> > > There's a pci_dev_put() in there, are you sure it won't sleep?
> >
> > Thank Jakub for your notice, during development we were aware about this
> > and tests we've made on this particular case, did not report any problems
> > that could be related to might_sleep in conjunction with spinlock.
>
> To be on the safe side how about we protect adapter->num_vfs instead
> of adapter->vfinfo ?
>
> You can hold the lock just around setting adapter->num_vfs to zero,
> and then inside ixgbe_msg_task() you don't have to add the new if()
> because the loop bound is already adapter->num_vfs.
>
> Smaller change, and safer.
Yes sounds good, I will test it and prepare the patch.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
@ 2021-08-12 17:18 Tony Nguyen
2021-08-14 0:20 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Tony Nguyen @ 2021-08-12 17:18 UTC (permalink / raw)
To: davem, kuba
Cc: Ken Cox, netdev, anthony.l.nguyen, Anirudh Venkataramanan, Marek Szlosek
From: Ken Cox <jkc@redhat.com>
It is possible to disable VFs while the PF driver is processing requests
from the VF driver. This can result in a panic.
BUG: unable to handle kernel paging request at 000000000000106c
PGD 0 P4D 0
Oops: 0000 [#1] SMP NOPTI
CPU: 8 PID: 0 Comm: swapper/8 Kdump: loaded Tainted: G I --------- - -
Hardware name: Dell Inc. PowerEdge R740/06WXJT, BIOS 2.8.2 08/27/2020
RIP: 0010:ixgbe_msg_task+0x4c8/0x1690 [ixgbe]
Code: 00 00 48 8d 04 40 48 c1 e0 05 89 7c 24 24 89 fd 48 89 44 24 10 83 ff 01 0f 84 b8 04 00 00 4c 8b 64 24 10 4d 03 a5 48 22 00 00 <41> 80 7c 24 4c 00 0f 84 8a 03 00 00 0f b7 c7 83 f8 08 0f 84 8f 0a
RSP: 0018:ffffb337869f8df8 EFLAGS: 00010002
RAX: 0000000000001020 RBX: 0000000000000000 RCX: 000000000000002b
RDX: 0000000000000002 RSI: 0000000000000008 RDI: 0000000000000006
RBP: 0000000000000006 R08: 0000000000000002 R09: 0000000000029780
R10: 00006957d8f42832 R11: 0000000000000000 R12: 0000000000001020
R13: ffff8a00e8978ac0 R14: 000000000000002b R15: ffff8a00e8979c80
FS: 0000000000000000(0000) GS:ffff8a07dfd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000106c CR3: 0000000063e10004 CR4: 00000000007726e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<IRQ>
? ttwu_do_wakeup+0x19/0x140
? try_to_wake_up+0x1cd/0x550
? ixgbevf_update_xcast_mode+0x71/0xc0 [ixgbevf]
ixgbe_msix_other+0x17e/0x310 [ixgbe]
__handle_irq_event_percpu+0x40/0x180
handle_irq_event_percpu+0x30/0x80
handle_irq_event+0x36/0x53
handle_edge_irq+0x82/0x190
handle_irq+0x1c/0x30
do_IRQ+0x49/0xd0
common_interrupt+0xf/0xf
This can be eventually be reproduced with the following script:
while :
do
echo 63 > /sys/class/net/ens3f0/device/sriov_numvfs
sleep 1
echo 0 > /sys/class/net/ens3f0/device/sriov_numvfs
sleep 1
done
Fixes: da36b64736cf ("ixgbe: Implement PCI SR-IOV sysfs callback operation")
Signed-off-by: Ken Cox <jkc@redhat.com>
Acked-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Marek Szlosek <marek.szlosek@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index a604552fa634..696bb2a61ea7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -807,6 +807,7 @@ enum ixgbe_state_t {
__IXGBE_PTP_RUNNING,
__IXGBE_PTP_TX_IN_PROGRESS,
__IXGBE_RESET_REQUESTED,
+ __IXGBE_DISABLING_VFS,
};
struct ixgbe_cb {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 214a38de3f41..0a1a8756f1fd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -206,8 +206,12 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
unsigned int num_vfs = adapter->num_vfs, vf;
int rss;
+ while (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
+ usleep_range(1000, 2000);
+
/* set num VFs to 0 to prevent access to vfinfo */
adapter->num_vfs = 0;
+ clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
/* put the reference to all of the vf devices */
for (vf = 0; vf < num_vfs; ++vf) {
@@ -1307,6 +1311,9 @@ void ixgbe_msg_task(struct ixgbe_adapter *adapter)
struct ixgbe_hw *hw = &adapter->hw;
u32 vf;
+ if (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
+ return;
+
for (vf = 0; vf < adapter->num_vfs; vf++) {
/* process any reset requests */
if (!ixgbe_check_for_rst(hw, vf))
@@ -1320,6 +1327,7 @@ void ixgbe_msg_task(struct ixgbe_adapter *adapter)
if (!ixgbe_check_for_ack(hw, vf))
ixgbe_rcv_ack_from_vf(adapter, vf);
}
+ clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
}
void ixgbe_disable_tx_rx(struct ixgbe_adapter *adapter)
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2021-08-12 17:18 Tony Nguyen
@ 2021-08-14 0:20 ` Jakub Kicinski
2021-08-16 17:52 ` Nguyen, Anthony L
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-08-14 0:20 UTC (permalink / raw)
To: Tony Nguyen; +Cc: davem, Ken Cox, netdev, Anirudh Venkataramanan, Marek Szlosek
On Thu, 12 Aug 2021 10:18:56 -0700 Tony Nguyen wrote:
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 214a38de3f41..0a1a8756f1fd 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -206,8 +206,12 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
> unsigned int num_vfs = adapter->num_vfs, vf;
> int rss;
>
> + while (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
> + usleep_range(1000, 2000);
> +
> /* set num VFs to 0 to prevent access to vfinfo */
> adapter->num_vfs = 0;
> + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>
> /* put the reference to all of the vf devices */
> for (vf = 0; vf < num_vfs; ++vf) {
> @@ -1307,6 +1311,9 @@ void ixgbe_msg_task(struct ixgbe_adapter *adapter)
> struct ixgbe_hw *hw = &adapter->hw;
> u32 vf;
>
> + if (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
> + return;
> +
> for (vf = 0; vf < adapter->num_vfs; vf++) {
> /* process any reset requests */
> if (!ixgbe_check_for_rst(hw, vf))
> @@ -1320,6 +1327,7 @@ void ixgbe_msg_task(struct ixgbe_adapter *adapter)
> if (!ixgbe_check_for_ack(hw, vf))
> ixgbe_rcv_ack_from_vf(adapter, vf);
> }
> + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
Like I've already said two or three times. No flag based locking.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2021-08-14 0:20 ` Jakub Kicinski
@ 2021-08-16 17:52 ` Nguyen, Anthony L
2021-08-17 10:23 ` Ken Cox
0 siblings, 1 reply; 9+ messages in thread
From: Nguyen, Anthony L @ 2021-08-16 17:52 UTC (permalink / raw)
To: kuba
Cc: jkc, Szlosek, Marek, Yang, Lihong, davem, netdev, Venkataramanan,
Anirudh
On Fri, 2021-08-13 at 17:20 -0700, Jakub Kicinski wrote:
> On Thu, 12 Aug 2021 10:18:56 -0700 Tony Nguyen wrote:
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > index 214a38de3f41..0a1a8756f1fd 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > @@ -206,8 +206,12 @@ int ixgbe_disable_sriov(struct ixgbe_adapter
> > *adapter)
> > unsigned int num_vfs = adapter->num_vfs, vf;
> > int rss;
> >
> > + while (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter-
> > >state))
> > + usleep_range(1000, 2000);
> > +
> > /* set num VFs to 0 to prevent access to vfinfo */
> > adapter->num_vfs = 0;
> > + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
> >
> > /* put the reference to all of the vf devices */
> > for (vf = 0; vf < num_vfs; ++vf) {
> > @@ -1307,6 +1311,9 @@ void ixgbe_msg_task(struct ixgbe_adapter
> > *adapter)
> > struct ixgbe_hw *hw = &adapter->hw;
> > u32 vf;
> >
> > + if (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
> > + return;
> > +
> > for (vf = 0; vf < adapter->num_vfs; vf++) {
> > /* process any reset requests */
> > if (!ixgbe_check_for_rst(hw, vf))
> > @@ -1320,6 +1327,7 @@ void ixgbe_msg_task(struct ixgbe_adapter
> > *adapter)
> > if (!ixgbe_check_for_ack(hw, vf))
> > ixgbe_rcv_ack_from_vf(adapter, vf);
> > }
> > + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>
> Like I've already said two or three times. No flag based locking.
Ken,
Did you want to make this change or did you want Intel to do it?
Thanks,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero
2021-08-16 17:52 ` Nguyen, Anthony L
@ 2021-08-17 10:23 ` Ken Cox
0 siblings, 0 replies; 9+ messages in thread
From: Ken Cox @ 2021-08-17 10:23 UTC (permalink / raw)
To: Nguyen, Anthony L, kuba
Cc: Szlosek, Marek, Yang, Lihong, davem, netdev, Venkataramanan, Anirudh
On 8/16/21 12:52 PM, Nguyen, Anthony L wrote:
> On Fri, 2021-08-13 at 17:20 -0700, Jakub Kicinski wrote:
>> On Thu, 12 Aug 2021 10:18:56 -0700 Tony Nguyen wrote:
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> index 214a38de3f41..0a1a8756f1fd 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> @@ -206,8 +206,12 @@ int ixgbe_disable_sriov(struct ixgbe_adapter
>>> *adapter)
>>> unsigned int num_vfs = adapter->num_vfs, vf;
>>> int rss;
>>>
>>> + while (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter-
>>>> state))
>>> + usleep_range(1000, 2000);
>>> +
>>> /* set num VFs to 0 to prevent access to vfinfo */
>>> adapter->num_vfs = 0;
>>> + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>>>
>>> /* put the reference to all of the vf devices */
>>> for (vf = 0; vf < num_vfs; ++vf) {
>>> @@ -1307,6 +1311,9 @@ void ixgbe_msg_task(struct ixgbe_adapter
>>> *adapter)
>>> struct ixgbe_hw *hw = &adapter->hw;
>>> u32 vf;
>>>
>>> + if (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
>>> + return;
>>> +
>>> for (vf = 0; vf < adapter->num_vfs; vf++) {
>>> /* process any reset requests */
>>> if (!ixgbe_check_for_rst(hw, vf))
>>> @@ -1320,6 +1327,7 @@ void ixgbe_msg_task(struct ixgbe_adapter
>>> *adapter)
>>> if (!ixgbe_check_for_ack(hw, vf))
>>> ixgbe_rcv_ack_from_vf(adapter, vf);
>>> }
>>> + clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>>
>> Like I've already said two or three times. No flag based locking.
>
> Ken,
>
> Did you want to make this change or did you want Intel to do it?
Hi Tony,
It would be great if Intel could make the change for the locking.
Thanks,
Ken
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-01 8:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 16:53 [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov_numvfs to zero Tony Nguyen
2022-06-28 17:27 ` Jakub Kicinski
2022-06-30 10:08 ` Piotr Skajewski
2022-06-30 15:11 ` Jakub Kicinski
2022-07-01 8:31 ` Piotr Skajewski
-- strict thread matches above, loose matches on Subject: below --
2021-08-12 17:18 Tony Nguyen
2021-08-14 0:20 ` Jakub Kicinski
2021-08-16 17:52 ` Nguyen, Anthony L
2021-08-17 10:23 ` Ken Cox
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.