Re: [PATCH net] net: mvneta: do not redirect frames during reconfiguration

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

> On Tue, Jun 09, 2020 at 09:41:10AM +0200, Lorenzo Bianconi wrote:
> > > On Tue, Jun 09, 2020 at 12:02:39AM +0200, Lorenzo Bianconi wrote:
> > > > Disable frames injection in mvneta_xdp_xmit routine during hw
> > > > re-configuration in order to avoid hardware hangs
> > > 
> > > Hi Lorenzo
> > > 
> > > Why does mvneta_tx() also not need the same protection?
> > > 
> > >     Andrew
> > 
> > Hi Andrew,
> > 
> > So far I have not been able to trigger the issue in the legacy tx path.
> 
> Even if you have not hit the issue, do you still think it is possible?
> If it is hard to trigger, maybe it is worth protecting against it,
> just in case.

The issue occurs putting the device down while it is still transmitting. In
particular mvneta_port_down() fails to stop tx (TIMEOUT for TX stopped status=...)
and the device is not able to recover.
The above pattern can occur with XDP because if we remove the program from a
running interface, we will put the interface down for DMA buffers
reconfiguration while mvneta_xdp_xmit() is concurrently running on a remote
cpu.
Looking at the code I do not think it can occurs in the legacy tx path
(mvneta_tx()) since __dev_close() (trigger by userspace) will run
dev_deactivate_many() before running mvneta_stop().

> 
> > I hit the problem adding the capability to attach an eBPF program to CPUMAP
> > entries [1]. In particular I am redirecting traffic to mvneta and concurrently
> > attaching/removing a XDP program to/from it.
> > I am not sure this can occur running mvneta_tx().
> > Moreover it seems a common pattern for .ndo_xdp_xmit() in other drivers
> > (e.g ixgbe, bnxt, mlx5)
> 
> I was wondering if this should be solved at a higher level. And if you
> say there are more MAC drivers with this issue, maybe it should. Not
> sure how though. It seems like MTU change and rx mode change wound
> need to be protected, which at a higher level is harder to do. What
> exactly do you need to protect, in a generic way?

Yes, we can think about it but I guess we should fix the issue first since it
is already there and it will be easy to backport the fix, agree?

Regards,
Lorenzo

> 
>      Andrew

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]