mbox series

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

Message ID 1595351648-10794-1-git-send-email-sundeep.lkml@gmail.com
Headers show
Series Fix bugs in Octeontx2 netdev driver | expand

Message

sundeep subbaraya July 21, 2020, 5:14 p.m. UTC
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(+)

Comments

David Miller July 21, 2020, 11:17 p.m. UTC | #1
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.
sundeep subbaraya July 22, 2020, 2:04 p.m. UTC | #2
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
sundeep subbaraya July 23, 2020, 2:59 p.m. UTC | #3
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
David Miller July 23, 2020, 7:13 p.m. UTC | #4
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.
sundeep subbaraya July 24, 2020, 3:10 a.m. UTC | #5
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
David Miller July 25, 2020, 3:10 a.m. UTC | #6
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.
sundeep subbaraya July 25, 2020, 4:41 a.m. UTC | #7
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