can: kvaser_usb: Update stats and state even if alloc_can_err_skb() fails

Ensure statistics, error counters, and CAN state are updated consistently,
even when alloc_can_err_skb() fails during state changes or error message
frame reception.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
Link: https://patch.msgid.link/20241230142645.128244-1-extja@kvaser.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Jimmy Assarsson 2024-12-30 15:26:42 +01:00 committed by Marc Kleine-Budde
parent 7e0c2f136d
commit 3749637b71
2 changed files with 60 additions and 95 deletions

View File

@ -926,6 +926,42 @@ kvaser_usb_hydra_bus_status_to_can_state(const struct kvaser_usb_net_priv *priv,
}
}
static void kvaser_usb_hydra_change_state(struct kvaser_usb_net_priv *priv,
const struct can_berr_counter *bec,
struct can_frame *cf,
enum can_state new_state)
{
struct net_device *netdev = priv->netdev;
enum can_state old_state = priv->can.state;
enum can_state tx_state, rx_state;
tx_state = (bec->txerr >= bec->rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
rx_state = (bec->txerr <= bec->rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
can_change_state(netdev, cf, tx_state, rx_state);
if (new_state == CAN_STATE_BUS_OFF && old_state < CAN_STATE_BUS_OFF) {
if (priv->can.restart_ms == 0)
kvaser_usb_hydra_send_simple_cmd_async(priv, CMD_STOP_CHIP_REQ);
can_bus_off(netdev);
}
if (priv->can.restart_ms &&
old_state >= CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF) {
priv->can.can_stats.restarts++;
if (cf)
cf->can_id |= CAN_ERR_RESTARTED;
}
if (cf && new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec->txerr;
cf->data[7] = bec->rxerr;
}
}
static void kvaser_usb_hydra_update_state(struct kvaser_usb_net_priv *priv,
u8 bus_status,
const struct can_berr_counter *bec)
@ -951,41 +987,11 @@ static void kvaser_usb_hydra_update_state(struct kvaser_usb_net_priv *priv,
return;
skb = alloc_can_err_skb(netdev, &cf);
if (skb) {
enum can_state tx_state, rx_state;
tx_state = (bec->txerr >= bec->rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
rx_state = (bec->txerr <= bec->rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
can_change_state(netdev, cf, tx_state, rx_state);
}
if (new_state == CAN_STATE_BUS_OFF && old_state < CAN_STATE_BUS_OFF) {
if (!priv->can.restart_ms)
kvaser_usb_hydra_send_simple_cmd_async
(priv, CMD_STOP_CHIP_REQ);
can_bus_off(netdev);
}
if (!skb) {
kvaser_usb_hydra_change_state(priv, bec, cf, new_state);
if (skb)
netif_rx(skb);
else
netdev_warn(netdev, "No memory left for err_skb\n");
return;
}
if (priv->can.restart_ms &&
old_state >= CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF)
priv->can.can_stats.restarts++;
if (new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec->txerr;
cf->data[7] = bec->rxerr;
}
netif_rx(skb);
}
static void kvaser_usb_hydra_state_event(const struct kvaser_usb *dev,
@ -1080,7 +1086,6 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
struct net_device_stats *stats = &netdev->stats;
struct can_frame *cf;
struct sk_buff *skb;
struct skb_shared_hwtstamps *shhwtstamps;
struct can_berr_counter bec;
enum can_state new_state, old_state;
u8 bus_status;
@ -1097,51 +1102,22 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
&new_state);
skb = alloc_can_err_skb(netdev, &cf);
if (new_state != old_state)
kvaser_usb_hydra_change_state(priv, &bec, cf, new_state);
if (new_state != old_state) {
if (skb) {
enum can_state tx_state, rx_state;
if (skb) {
struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
tx_state = (bec.txerr >= bec.rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
rx_state = (bec.txerr <= bec.rxerr) ?
new_state : CAN_STATE_ERROR_ACTIVE;
can_change_state(netdev, cf, tx_state, rx_state);
if (priv->can.restart_ms &&
old_state >= CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF)
cf->can_id |= CAN_ERR_RESTARTED;
}
if (new_state == CAN_STATE_BUS_OFF) {
if (!priv->can.restart_ms)
kvaser_usb_hydra_send_simple_cmd_async
(priv, CMD_STOP_CHIP_REQ);
can_bus_off(netdev);
}
}
if (!skb) {
stats->rx_dropped++;
netdev_warn(netdev, "No memory left for err_skb\n");
return;
}
shhwtstamps = skb_hwtstamps(skb);
shhwtstamps->hwtstamp = hwtstamp;
cf->can_id |= CAN_ERR_BUSERROR;
if (new_state != CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_CNT;
shhwtstamps->hwtstamp = hwtstamp;
cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_CNT;
cf->data[6] = bec.txerr;
cf->data[7] = bec.rxerr;
netif_rx(skb);
} else {
stats->rx_dropped++;
netdev_warn(netdev, "No memory left for err_skb\n");
}
netif_rx(skb);
priv->bec.txerr = bec.txerr;
priv->bec.rxerr = bec.rxerr;
}

View File

@ -1121,8 +1121,6 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
const struct kvaser_usb_err_summary *es)
{
struct can_frame *cf;
struct can_frame tmp_cf = { .can_id = CAN_ERR_FLAG,
.len = CAN_ERR_DLC };
struct sk_buff *skb;
struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
@ -1143,18 +1141,9 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
if (!netif_running(priv->netdev))
return;
/* Update all of the CAN interface's state and error counters before
* 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_leaf_rx_error_update_can_state(priv, es, &tmp_cf);
skb = alloc_can_err_skb(priv->netdev, &cf);
kvaser_usb_leaf_rx_error_update_can_state(priv, es, cf);
new_state = priv->can.state;
/* If there are errors, request status updates periodically as we do
@ -1168,13 +1157,6 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
schedule_delayed_work(&leaf->chip_state_req_work,
msecs_to_jiffies(500));
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
return;
}
memcpy(cf, &tmp_cf, sizeof(*cf));
if (new_state != old_state) {
if (es->status &
(M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
@ -1187,11 +1169,18 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
if (priv->can.restart_ms &&
old_state == CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_RESTARTED;
if (cf)
cf->can_id |= CAN_ERR_RESTARTED;
netif_carrier_on(priv->netdev);
}
}
if (!skb) {
stats->rx_dropped++;
netdev_warn(priv->netdev, "No memory left for err_skb\n");
return;
}
switch (dev->driver_info->family) {
case KVASER_LEAF:
if (es->leaf.error_factor) {