* [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors
@ 2019-03-27 18:38 Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 1/2] nfp: validate the return code from dev_queue_xmit() Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jakub Kicinski @ 2019-03-27 18:38 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, Jakub Kicinski
Hi!
This series avoids a potential crash on nfp representor devices
when netpoll is in use. If transmitting the frame through underlying
vNIC fails we'd return an error code (by passing on error code from
__dev_queue_xmit()) and cause double free in netpoll code.
Fix the error code and disable netpoll on reprs altogether.
IRQ-safety of locking the queues and calling __dev_queue_xmit()
is questionable.
Big thanks to John Hurley for debugging and narrowing down
the trace log after I gave up! :)
Jakub Kicinski (2):
nfp: validate the return code from dev_queue_xmit()
nfp: disable netpoll on representors
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.19.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net 1/2] nfp: validate the return code from dev_queue_xmit()
2019-03-27 18:38 [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors Jakub Kicinski
@ 2019-03-27 18:38 ` Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 2/2] nfp: disable netpoll on representors Jakub Kicinski
2019-03-29 0:04 ` [PATCH net 0/2] nfp: fix retcode and " David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2019-03-27 18:38 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, Jakub Kicinski, John Hurley, Simon Horman
dev_queue_xmit() may return error codes as well as netdev_tx_t,
and it always consumes the skb. Make sure we always return a
correct netdev_tx_t value.
Fixes: eadfa4c3be99 ("nfp: add stats and xmit helpers for representors")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index d2c803bb4e56..7b46fce2e81e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -195,7 +195,7 @@ static netdev_tx_t nfp_repr_xmit(struct sk_buff *skb, struct net_device *netdev)
ret = dev_queue_xmit(skb);
nfp_repr_inc_tx_stats(netdev, len, ret);
- return ret;
+ return NETDEV_TX_OK;
}
static int nfp_repr_stop(struct net_device *netdev)
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/2] nfp: disable netpoll on representors
2019-03-27 18:38 [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 1/2] nfp: validate the return code from dev_queue_xmit() Jakub Kicinski
@ 2019-03-27 18:38 ` Jakub Kicinski
2019-03-29 0:04 ` [PATCH net 0/2] nfp: fix retcode and " David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2019-03-27 18:38 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, Jakub Kicinski, John Hurley, Simon Horman
NFP reprs are software device on top of the PF's vNIC.
The comment above __dev_queue_xmit() sayeth:
When calling this method, interrupts MUST be enabled. This is because
the BH enable code must have IRQs enabled so that it will not deadlock.
For netconsole we can't guarantee IRQ state, let's just
disable netpoll on representors to be on the safe side.
When the initial implementation of NFP reprs was added by the
commit 5de73ee46704 ("nfp: general representor implementation")
.ndo_poll_controller was required for netpoll to be enabled.
Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 7b46fce2e81e..94d228c04496 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -383,7 +383,7 @@ int nfp_repr_init(struct nfp_app *app, struct net_device *netdev,
netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
netdev->gso_max_segs = NFP_NET_LSO_MAX_SEGS;
- netdev->priv_flags |= IFF_NO_QUEUE;
+ netdev->priv_flags |= IFF_NO_QUEUE | IFF_DISABLE_NETPOLL;
netdev->features |= NETIF_F_LLTX;
if (nfp_app_has_tc(app)) {
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors
2019-03-27 18:38 [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 1/2] nfp: validate the return code from dev_queue_xmit() Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 2/2] nfp: disable netpoll on representors Jakub Kicinski
@ 2019-03-29 0:04 ` David Miller
2019-03-29 0:27 ` Jakub Kicinski
2 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-03-29 0:04 UTC (permalink / raw)
To: jakub.kicinski; +Cc: oss-drivers, netdev
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 27 Mar 2019 11:38:37 -0700
> This series avoids a potential crash on nfp representor devices
> when netpoll is in use. If transmitting the frame through underlying
> vNIC fails we'd return an error code (by passing on error code from
> __dev_queue_xmit()) and cause double free in netpoll code.
>
> Fix the error code and disable netpoll on reprs altogether.
> IRQ-safety of locking the queues and calling __dev_queue_xmit()
> is questionable.
>
> Big thanks to John Hurley for debugging and narrowing down
> the trace log after I gave up! :)
Series applied, thanks Jakub.
Want me to queue these up for -stable?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors
2019-03-29 0:04 ` [PATCH net 0/2] nfp: fix retcode and " David Miller
@ 2019-03-29 0:27 ` Jakub Kicinski
2019-03-29 0:30 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2019-03-29 0:27 UTC (permalink / raw)
To: David Miller; +Cc: oss-drivers, netdev
On Thu, 28 Mar 2019 17:04:53 -0700 (PDT), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed, 27 Mar 2019 11:38:37 -0700
>
> > This series avoids a potential crash on nfp representor devices
> > when netpoll is in use. If transmitting the frame through underlying
> > vNIC fails we'd return an error code (by passing on error code from
> > __dev_queue_xmit()) and cause double free in netpoll code.
> >
> > Fix the error code and disable netpoll on reprs altogether.
> > IRQ-safety of locking the queues and calling __dev_queue_xmit()
> > is questionable.
> >
> > Big thanks to John Hurley for debugging and narrowing down
> > the trace log after I gave up! :)
>
> Series applied, thanks Jakub.
>
> Want me to queue these up for -stable?
Mm.. yes, I think so, patch 1 can lead to a hard to catch double free.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors
2019-03-29 0:27 ` Jakub Kicinski
@ 2019-03-29 0:30 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-03-29 0:30 UTC (permalink / raw)
To: jakub.kicinski; +Cc: oss-drivers, netdev
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 28 Mar 2019 17:27:50 -0700
> On Thu, 28 Mar 2019 17:04:53 -0700 (PDT), David Miller wrote:
>> From: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Date: Wed, 27 Mar 2019 11:38:37 -0700
>>
>> > This series avoids a potential crash on nfp representor devices
>> > when netpoll is in use. If transmitting the frame through underlying
>> > vNIC fails we'd return an error code (by passing on error code from
>> > __dev_queue_xmit()) and cause double free in netpoll code.
>> >
>> > Fix the error code and disable netpoll on reprs altogether.
>> > IRQ-safety of locking the queues and calling __dev_queue_xmit()
>> > is questionable.
>> >
>> > Big thanks to John Hurley for debugging and narrowing down
>> > the trace log after I gave up! :)
>>
>> Series applied, thanks Jakub.
>>
>> Want me to queue these up for -stable?
>
> Mm.. yes, I think so, patch 1 can lead to a hard to catch double free.
Ok, queued up.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-29 0:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-27 18:38 [PATCH net 0/2] nfp: fix retcode and disable netpoll on representors Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 1/2] nfp: validate the return code from dev_queue_xmit() Jakub Kicinski
2019-03-27 18:38 ` [PATCH net 2/2] nfp: disable netpoll on representors Jakub Kicinski
2019-03-29 0:04 ` [PATCH net 0/2] nfp: fix retcode and " David Miller
2019-03-29 0:27 ` Jakub Kicinski
2019-03-29 0:30 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox