[PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[sundeep.lkml]

From: Subbaraya Sundeep <sbhatta@marvell.com>

Hi,

There are problems in the existing Octeontx2
netdev drivers like missing cancel_work for the
reset task, missing lock in reset task and
missing unergister_netdev in driver remove.
This patch set fixes the above problems.

Thanks,
Sundeep

Subbaraya Sundeep (3):
  octeontx2-pf: Fix reset_task bugs
  octeontx2-pf: cancel reset_task work
  octeontx2-pf: Unregister netdev at driver remove

 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 +++
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 2 ++
 2 files changed, 5 insertions(+)

-- 
2.7.4

[PATCH net 1/3] octeontx2-pf: Fix reset_task bugs

[sundeep.lkml]

From: Subbaraya Sundeep <sbhatta@marvell.com>

Two bugs exist in the code related to reset_task
in PF driver one is the missing protection
against network stack ndo_open and ndo_close.
Other one is the missing cancel_work.
This patch fixes those problems.

Fixes: 4ff7d1488a84 ("octeontx2-pf: Error handling support")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 6478656..75a8c40 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -1730,10 +1730,12 @@ static void otx2_reset_task(struct work_struct *work)
 	if (!netif_running(pf->netdev))
 		return;
 
+	rtnl_lock();
 	otx2_stop(pf->netdev);
 	pf->reset_count++;
 	otx2_open(pf->netdev);
 	netif_trans_update(pf->netdev);
+	rtnl_unlock();
 }
 
 static const struct net_device_ops otx2_netdev_ops = {
@@ -2111,6 +2113,7 @@ static void otx2_remove(struct pci_dev *pdev)
 
 	pf = netdev_priv(netdev);
 
+	cancel_work_sync(&pf->reset_task);
 	/* Disable link notifications */
 	otx2_cgx_config_linkevents(pf, false);
 
-- 
2.7.4

[PATCH net 2/3] octeontx2-pf: cancel reset_task work

[sundeep.lkml]

From: Subbaraya Sundeep <sbhatta@marvell.com>

During driver exit cancel the queued
reset_task work in VF driver.

Fixes: 3184fb5ba96e ("octeontx2-vf: Virtual function driver support")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index f422751..c1c263d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -617,6 +617,7 @@ static void otx2vf_remove(struct pci_dev *pdev)
 
 	vf = netdev_priv(netdev);
 
+	cancel_work_sync(&vf->reset_task);
 	otx2vf_disable_mbox_intr(vf);
 
 	otx2_detach_resources(&vf->mbox);
-- 
2.7.4

[PATCH net 3/3] octeontx2-pf: Unregister netdev at driver remove

[sundeep.lkml]

From: Subbaraya Sundeep <sbhatta@marvell.com>

unregister_netdev is missing in the VF driver
remove function. Hence add it.

Fixes: 3184fb5ba96e ("octeontx2-vf: Virtual function driver support")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index c1c263d..92a3db6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -618,6 +618,7 @@ static void otx2vf_remove(struct pci_dev *pdev)
 	vf = netdev_priv(netdev);
 
 	cancel_work_sync(&vf->reset_task);
+	unregister_netdev(netdev);
 	otx2vf_disable_mbox_intr(vf);
 
 	otx2_detach_resources(&vf->mbox);
-- 
2.7.4

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[David Miller]

From: sundeep.lkml@gmail.com
Date: Tue, 21 Jul 2020 22:44:05 +0530

> Subbaraya Sundeep (3):
>   octeontx2-pf: Fix reset_task bugs
>   octeontx2-pf: cancel reset_task work
>   octeontx2-pf: Unregister netdev at driver remove

I think you should shut down all the interrupts and other state
before unregistering the vf network device.

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[sundeep subbaraya]

Hi David,

On Wed, Jul 22, 2020 at 4:47 AM David Miller <davem@davemloft.net> wrote:
>
> From: sundeep.lkml@gmail.com
> Date: Tue, 21 Jul 2020 22:44:05 +0530
>
> > Subbaraya Sundeep (3):
> >   octeontx2-pf: Fix reset_task bugs
> >   octeontx2-pf: cancel reset_task work
> >   octeontx2-pf: Unregister netdev at driver remove
>
> I think you should shut down all the interrupts and other state
> before unregistering the vf network device.

Okay will change it and send v2.

Thanks,
Sundeep

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[sundeep subbaraya]

Hi David,

On Wed, Jul 22, 2020 at 7:34 PM sundeep subbaraya
<sundeep.lkml@gmail.com> wrote:
>
> Hi David,
>
> On Wed, Jul 22, 2020 at 4:47 AM David Miller <davem@davemloft.net> wrote:
> >
> > From: sundeep.lkml@gmail.com
> > Date: Tue, 21 Jul 2020 22:44:05 +0530
> >
> > > Subbaraya Sundeep (3):
> > >   octeontx2-pf: Fix reset_task bugs
> > >   octeontx2-pf: cancel reset_task work
> > >   octeontx2-pf: Unregister netdev at driver remove
> >
> > I think you should shut down all the interrupts and other state
> > before unregistering the vf network device.
>
> Okay will change it and send v2.
>

For our case interrupts need to be ON when unregister_netdev is called.
If driver remove is called when the interface is up then
otx2_stop(called by unregister_netdev)
needs mailbox interrupts to communicate with PF to release its resources.


Thanks,
Sundeep
> Thanks,
> Sundeep

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[David Miller]

From: sundeep subbaraya <sundeep.lkml@gmail.com>
Date: Thu, 23 Jul 2020 20:29:03 +0530

> Hi David,
> 
> On Wed, Jul 22, 2020 at 7:34 PM sundeep subbaraya
> <sundeep.lkml@gmail.com> wrote:
>>
>> Hi David,
>>
>> On Wed, Jul 22, 2020 at 4:47 AM David Miller <davem@davemloft.net> wrote:
>> >
>> > From: sundeep.lkml@gmail.com
>> > Date: Tue, 21 Jul 2020 22:44:05 +0530
>> >
>> > > Subbaraya Sundeep (3):
>> > >   octeontx2-pf: Fix reset_task bugs
>> > >   octeontx2-pf: cancel reset_task work
>> > >   octeontx2-pf: Unregister netdev at driver remove
>> >
>> > I think you should shut down all the interrupts and other state
>> > before unregistering the vf network device.
>>
>> Okay will change it and send v2.
>>
> 
> For our case interrupts need to be ON when unregister_netdev is called.
> If driver remove is called when the interface is up then
> otx2_stop(called by unregister_netdev)
> needs mailbox interrupts to communicate with PF to release its resources.

If you leave interrupts on then an interrupt can arrive after the software
state has been released by unregister_netdev.

Sounds like you need to resolve this some other way.

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[sundeep subbaraya]

Hi David,

On Fri, Jul 24, 2020 at 12:43 AM David Miller <davem@davemloft.net> wrote:
>
> From: sundeep subbaraya <sundeep.lkml@gmail.com>
> Date: Thu, 23 Jul 2020 20:29:03 +0530
>
> > Hi David,
> >
> > On Wed, Jul 22, 2020 at 7:34 PM sundeep subbaraya
> > <sundeep.lkml@gmail.com> wrote:
> >>
> >> Hi David,
> >>
> >> On Wed, Jul 22, 2020 at 4:47 AM David Miller <davem@davemloft.net> wrote:
> >> >
> >> > From: sundeep.lkml@gmail.com
> >> > Date: Tue, 21 Jul 2020 22:44:05 +0530
> >> >
> >> > > Subbaraya Sundeep (3):
> >> > >   octeontx2-pf: Fix reset_task bugs
> >> > >   octeontx2-pf: cancel reset_task work
> >> > >   octeontx2-pf: Unregister netdev at driver remove
> >> >
> >> > I think you should shut down all the interrupts and other state
> >> > before unregistering the vf network device.
> >>
> >> Okay will change it and send v2.
> >>
> >
> > For our case interrupts need to be ON when unregister_netdev is called.
> > If driver remove is called when the interface is up then
> > otx2_stop(called by unregister_netdev)
> > needs mailbox interrupts to communicate with PF to release its resources.
>
> If you leave interrupts on then an interrupt can arrive after the software
> state has been released by unregister_netdev.
>
> Sounds like you need to resolve this some other way.

Only mailbox interrupts can arrive after unregister_netdev since
otx2_stop disables
the packet I/O and its interrupts as the first step.
And mbox interrupts are turned off after unregister_neetdev.
unregister_netdev(netdev);
otx2vf_disable_mbox_intr(vf);

Thanks,
Sundeep

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[David Miller]

From: sundeep subbaraya <sundeep.lkml@gmail.com>
Date: Fri, 24 Jul 2020 08:40:44 +0530

> On Fri, Jul 24, 2020 at 12:43 AM David Miller <davem@davemloft.net> wrote:
>>
>> If you leave interrupts on then an interrupt can arrive after the software
>> state has been released by unregister_netdev.
>>
>> Sounds like you need to resolve this some other way.
> 
> Only mailbox interrupts can arrive after unregister_netdev since
> otx2_stop disables
> the packet I/O and its interrupts as the first step.
> And mbox interrupts are turned off after unregister_neetdev.
> unregister_netdev(netdev);
> otx2vf_disable_mbox_intr(vf);

Please explain this in your commit message.

Thank you.

Re: [PATCH net 0/3] Fix bugs in Octeontx2 netdev driver

[sundeep subbaraya]

Hi David,

On Sat, Jul 25, 2020 at 8:40 AM David Miller <davem@davemloft.net> wrote:
>
> From: sundeep subbaraya <sundeep.lkml@gmail.com>
> Date: Fri, 24 Jul 2020 08:40:44 +0530
>
> > On Fri, Jul 24, 2020 at 12:43 AM David Miller <davem@davemloft.net> wrote:
> >>
> >> If you leave interrupts on then an interrupt can arrive after the software
> >> state has been released by unregister_netdev.
> >>
> >> Sounds like you need to resolve this some other way.
> >
> > Only mailbox interrupts can arrive after unregister_netdev since
> > otx2_stop disables
> > the packet I/O and its interrupts as the first step.
> > And mbox interrupts are turned off after unregister_neetdev.
> > unregister_netdev(netdev);
> > otx2vf_disable_mbox_intr(vf);
>
> Please explain this in your commit message.
>
> Thank you.
Sure will change the commit message.

Thanks,
Sundeep