The Davinci EMAC driver does two things incorrectly:
1. it does not use the budget value during transmission, so it can
process more than the allowed number of packets. It should either honour
the budget, or lie about the number of packets that it processed.
2. it manipulates the NAPI list in the rare case that it consumes its
entire budget during the poll, and at the end there are no more packets
pending. This is not allowed, according to the comment in
net/core/dev.c, function net_rx_action()
/* Drivers must not modify the NAPI state if they
* consume the entire weight. In such cases this code
* still "owns" the NAPI instance and therefore can
* move the instance around on the list at-will.
*/
and in fact causes a kernel panic in net_rx_action.
In addition, netpoll.c function poll_napi() is susceptible to buggy
drivers that consume more than their allocated budget. It tests the
remaining budget using
if (!budget)
This is identical to
if (budget == 0)
However, poll_one_napi() returns (budget - work), so a buggy driver
could return work > budget, which will fail the test with budget
suddenly being a negative number... unless the same buggy driver
correctly does no work when the budget is negative...
Function net_rx_action() considers this situation with a warning
WARN_ON_ONCE(work > weight);
and also stops polling when the budget is exactly or over-consumed.
if (unlikely(budget <= 0 || jiffies != start_time))
goto softnet_break;
I hope these patches are usable: they are based on 2.6.27-davinci1 (+
some patches), not the latest head, and were generated using svn diff...
Stephen Irons
Senior Engineer
Tait Electronics Ltd
Index: drivers/net/davinci_emac.c
===================================================================
--- drivers/net/davinci_emac.c (revision 17242)
+++ drivers/net/davinci_emac.c (working copy)
@@ -2348,7 +2348,7 @@
if (status & mask) {
num_pkts = emac_tx_bdproc(priv, EMAC_DEF_TX_CH,
- EMAC_DEF_TX_MAX_SERVICE,
+ budget,
&txpending);
} /* TX processing */
@@ -2364,8 +2364,10 @@
__netif_rx_schedule(ndev, &priv->napi);
}
} else {
- netif_rx_complete(ndev, napi);
- emac_int_enable(priv);
+ if (num_pkts < budget) {
+ netif_rx_complete(ndev, napi);
+ emac_int_enable(priv);
+ }
}
if (unlikely(status & EMAC_DM644X_MAC_IN_VECTOR_HOST_INT)) {
Index: net/core/netpoll.c
===================================================================
--- net/core/netpoll.c (revision 17242)
+++ net/core/netpoll.c (working copy)
@@ -136,6 +136,8 @@
work = napi->poll(napi, budget);
+ WARN_ON_ONCE(work > budget);
+
atomic_dec(&trapped);
npinfo->rx_flags &= ~NETPOLL_RX_DROP;
@@ -153,7 +155,7 @@
budget = poll_one_napi(dev->npinfo, napi, budget);
spin_unlock(&napi->poll_lock);
- if (!budget)
+ if (budget <= 0)
break;
}
}
=======================================================================
This email, including any attachments, is only for the intended
addressee. It is subject to copyright, is confidential and may be
the subject of legal or other privilege, none of which is waived or
lost by reason of this transmission.
If the receiver is not the intended addressee, please accept our
apologies, notify us by return, delete all copies and perform no
other act on the email.
Unfortunately, we cannot warrant that the email has not been
altered or corrupted during transmission.
=======================================================================
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
