| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150120214537.GB16828@linux>
Date: Tue, 20 Jan 2015 16:45:37 -0500
From: "Ahmed S. Darwish" <darwish.07@...il.com>
To: Olivier Sobrie <olivier@...rie.be>,
Oliver Hartkopp <socketcan@...tkopp.net>,
Wolfgang Grandegger <wg@...ndegger.com>,
Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Andri Yngvason <andri.yngvason@...el.com>,
Linux-CAN <linux-can@...r.kernel.org>,
netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change
handling
From: Ahmed S. Darwish <ahmed.darwish@...eo.com>
Replace most of the can interface's state and error counters
handling with the new can-dev can_change_state() mechanism.
Suggested-by: Andri Yngvason <andri.yngvason@...el.com>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@...eo.com>
---
drivers/net/can/usb/kvaser_usb.c | 114 +++++++++++++++++++--------------------
1 file changed, 55 insertions(+), 59 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 971c5f9..0386d3f 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -620,40 +620,43 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
}
static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
- const struct kvaser_usb_error_summary *es)
+ const struct kvaser_usb_error_summary *es,
+ struct can_frame *cf)
{
struct net_device_stats *stats;
- enum can_state new_state;
-
- stats = &priv->netdev->stats;
- new_state = priv->can.state;
+ enum can_state cur_state, new_state, tx_state, rx_state;
netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
- if (es->status & M16C_STATE_BUS_OFF) {
- priv->can.can_stats.bus_off++;
+ stats = &priv->netdev->stats;
+ new_state = cur_state = priv->can.state;
+
+ if (es->status & M16C_STATE_BUS_OFF)
new_state = CAN_STATE_BUS_OFF;
- } else if (es->status & M16C_STATE_BUS_PASSIVE) {
- if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
- priv->can.can_stats.error_passive++;
+ else if (es->status & M16C_STATE_BUS_PASSIVE)
new_state = CAN_STATE_ERROR_PASSIVE;
- }
if (es->status == M16C_STATE_BUS_ERROR) {
- if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((es->txerr >= 96) || (es->rxerr >= 96))) {
- priv->can.can_stats.error_warning++;
+ if ((cur_state < CAN_STATE_ERROR_WARNING) &&
+ ((es->txerr >= 96) || (es->rxerr >= 96)))
new_state = CAN_STATE_ERROR_WARNING;
- } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
+ else if (cur_state > CAN_STATE_ERROR_ACTIVE)
new_state = CAN_STATE_ERROR_ACTIVE;
- }
}
if (!es->status)
new_state = CAN_STATE_ERROR_ACTIVE;
+ if (new_state != cur_state) {
+ tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
+ rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
+
+ can_change_state(priv->netdev, cf, tx_state, rx_state);
+ new_state = priv->can.state;
+ }
+
if (priv->can.restart_ms &&
- (priv->can.state >= CAN_STATE_BUS_OFF) &&
+ (cur_state >= CAN_STATE_BUS_OFF) &&
(new_state < CAN_STATE_BUS_OFF)) {
priv->can.can_stats.restarts++;
}
@@ -665,18 +668,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
priv->bec.txerr = es->txerr;
priv->bec.rxerr = es->rxerr;
- priv->can.state = new_state;
}
static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
const struct kvaser_msg *msg)
{
- struct can_frame *cf;
+ struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
struct sk_buff *skb;
struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
struct kvaser_usb_error_summary es = { };
- enum can_state old_state;
+ enum can_state old_state, new_state;
switch (msg->id) {
case CMD_CAN_ERROR_EVENT:
@@ -721,60 +723,54 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
/* Update all of the can interface's state and error counters before
- * trying any skb allocation that can actually fail with -ENOMEM.
+ * trying any memory allocation that can actually fail with -ENOMEM.
+ *
+ * We send a temporary stack-allocated error can frame to
+ * can_change_state() for the very same reason.
+ *
+ * TODO: Split can_change_state() responsibility between updating the
+ * can interface's state and counters, and the setting up of can error
+ * frame ID and data to userspace. Remove stack allocation afterwards.
*/
old_state = priv->can.state;
- kvaser_usb_rx_error_update_can_state(priv, &es);
+ kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
+ new_state = priv->can.state;
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
return;
}
+ memcpy(cf, &tmp_cf, sizeof(*cf));
- if (es.status & M16C_STATE_BUS_OFF) {
- cf->can_id |= CAN_ERR_BUSOFF;
-
- if (!priv->can.restart_ms)
- kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
- netif_carrier_off(priv->netdev);
- } else if (es.status & M16C_STATE_BUS_PASSIVE) {
- if (old_state != CAN_STATE_ERROR_PASSIVE) {
- cf->can_id |= CAN_ERR_CRTL;
-
- if (es.txerr || es.rxerr)
- cf->data[1] = (es.txerr > es.rxerr)
- ? CAN_ERR_CRTL_TX_PASSIVE
- : CAN_ERR_CRTL_RX_PASSIVE;
- else
- cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
- CAN_ERR_CRTL_RX_PASSIVE;
+ if (new_state != old_state) {
+ if (es.status & M16C_STATE_BUS_OFF) {
+ if (!priv->can.restart_ms)
+ kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
+ netif_carrier_off(priv->netdev);
+ }
+
+ if (es.status == M16C_STATE_BUS_ERROR) {
+ if ((old_state >= CAN_STATE_ERROR_WARNING) ||
+ (es.txerr < 96 && es.rxerr < 96)) {
+ if (old_state > CAN_STATE_ERROR_ACTIVE) {
+ cf->can_id |= CAN_ERR_PROT;
+ cf->data[2] = CAN_ERR_PROT_ACTIVE;
+ }
+ }
}
- }
- if (es.status == M16C_STATE_BUS_ERROR) {
- if ((old_state < CAN_STATE_ERROR_WARNING) &&
- ((es.txerr >= 96) || (es.rxerr >= 96))) {
- cf->can_id |= CAN_ERR_CRTL;
- cf->data[1] = (es.txerr > es.rxerr)
- ? CAN_ERR_CRTL_TX_WARNING
- : CAN_ERR_CRTL_RX_WARNING;
- } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
+ if (!es.status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
}
- }
- if (!es.status) {
- cf->can_id |= CAN_ERR_PROT;
- cf->data[2] = CAN_ERR_PROT_ACTIVE;
- }
-
- if (priv->can.restart_ms &&
- (old_state >= CAN_STATE_BUS_OFF) &&
- (priv->can.state < CAN_STATE_BUS_OFF)) {
- cf->can_id |= CAN_ERR_RESTARTED;
- netif_carrier_on(priv->netdev);
+ if (priv->can.restart_ms &&
+ (old_state >= CAN_STATE_BUS_OFF) &&
+ (new_state < CAN_STATE_BUS_OFF)) {
+ cf->can_id |= CAN_ERR_RESTARTED;
+ netif_carrier_on(priv->netdev);
+ }
}
if (es.error_factor) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists