* Re: [PATCH net-next 1/3] skbuff: Add skb_list_linearize()
From: Eric Dumazet @ 2015-01-20 21:43 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: davem, netdev
In-Reply-To: <1421785543-19823-1-git-send-email-pshelar@nicira.com>
On Tue, 2015-01-20 at 12:25 -0800, Pravin B Shelar wrote:
> similar to skb_linearize(), this API takes skb list as arg and
> linearize it into one big skb. STT driver patch will use this.
>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> include/linux/skbuff.h | 2 ++
> net/core/skbuff.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 85ab7d7..c9194c1 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2532,6 +2532,8 @@ static inline int skb_linearize(struct sk_buff *skb)
> return skb_is_nonlinear(skb) ? __skb_linearize(skb) : 0;
> }
>
> +int skb_list_linearize(struct sk_buff *head, gfp_t gfp_mask);
> +
> /**
> * skb_has_shared_frag - can any frag be overwritten
> * @skb: buffer to test
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 56db472..9ef2881 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2329,6 +2329,41 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
> }
> EXPORT_SYMBOL(skb_copy_and_csum_dev);
>
> +int skb_list_linearize(struct sk_buff *head, gfp_t gfp_mask)
> +{
> + struct sk_buff *skb;
> + int tlen = 0;
> + int err;
> +
> + err = skb_linearize(head);
> + if (err)
> + return err;
> +
> + skb = head->next;
> + while (skb) {
> + tlen += skb->len;
> + skb = skb->next;
> + }
> + err = pskb_expand_head(head, 0, tlen, gfp_mask);
> + if (err)
> + return err;
> +
> + skb = head->next;
> + while (skb) {
> + err = skb_copy_bits(skb, 0, skb_tail_pointer(head), skb->len);
> + if (err)
> + return err;
> + head->tail += skb->len;
> + skb = skb->next;
> + }
> + kfree_skb_list(head->next);
> + head->next = NULL;
> + head->len += tlen;
> + head->truesize = SKB_TRUESIZE(skb_end_offset(head));
This is buggy : skb can have a destructor, like sock_wfree()
^ permalink raw reply
* [PATCH v5 1/5] can: kvaser_usb: Update net interface state before exiting on OOM
From: Ahmed S. Darwish @ 2015-01-20 21:44 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Update all of the can interface's state and error counters before
trying any skb allocation that can actually fail with -ENOMEM.
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 182 +++++++++++++++++++++++----------------
1 file changed, 106 insertions(+), 76 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index c32cd61..971c5f9 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -273,6 +273,10 @@ struct kvaser_msg {
} u;
} __packed;
+struct kvaser_usb_error_summary {
+ u8 channel, status, txerr, rxerr, error_factor;
+};
+
struct kvaser_usb_tx_urb_context {
struct kvaser_usb_net_priv *priv;
u32 echo_index;
@@ -615,6 +619,55 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
priv->tx_contexts[i].echo_index = MAX_TX_URBS;
}
+static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
+ const struct kvaser_usb_error_summary *es)
+{
+ struct net_device_stats *stats;
+ enum can_state new_state;
+
+ stats = &priv->netdev->stats;
+ new_state = priv->can.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++;
+ 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++;
+ 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++;
+ new_state = CAN_STATE_ERROR_WARNING;
+ } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
+ new_state = CAN_STATE_ERROR_ACTIVE;
+ }
+ }
+
+ if (!es->status)
+ new_state = CAN_STATE_ERROR_ACTIVE;
+
+ if (priv->can.restart_ms &&
+ (priv->can.state >= CAN_STATE_BUS_OFF) &&
+ (new_state < CAN_STATE_BUS_OFF)) {
+ priv->can.can_stats.restarts++;
+ }
+
+ if (es->error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+ }
+
+ 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)
{
@@ -622,30 +675,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
struct sk_buff *skb;
struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
- unsigned int new_state;
- u8 channel, status, txerr, rxerr, error_factor;
+ struct kvaser_usb_error_summary es = { };
+ enum can_state old_state;
switch (msg->id) {
case CMD_CAN_ERROR_EVENT:
- channel = msg->u.error_event.channel;
- status = msg->u.error_event.status;
- txerr = msg->u.error_event.tx_errors_count;
- rxerr = msg->u.error_event.rx_errors_count;
- error_factor = msg->u.error_event.error_factor;
+ es.channel = msg->u.error_event.channel;
+ es.status = msg->u.error_event.status;
+ es.txerr = msg->u.error_event.tx_errors_count;
+ es.rxerr = msg->u.error_event.rx_errors_count;
+ es.error_factor = msg->u.error_event.error_factor;
break;
case CMD_LOG_MESSAGE:
- channel = msg->u.log_message.channel;
- status = msg->u.log_message.data[0];
- txerr = msg->u.log_message.data[2];
- rxerr = msg->u.log_message.data[3];
- error_factor = msg->u.log_message.data[1];
+ es.channel = msg->u.log_message.channel;
+ es.status = msg->u.log_message.data[0];
+ es.txerr = msg->u.log_message.data[2];
+ es.rxerr = msg->u.log_message.data[3];
+ es.error_factor = msg->u.log_message.data[1];
break;
case CMD_CHIP_STATE_EVENT:
- channel = msg->u.chip_state_event.channel;
- status = msg->u.chip_state_event.status;
- txerr = msg->u.chip_state_event.tx_errors_count;
- rxerr = msg->u.chip_state_event.rx_errors_count;
- error_factor = 0;
+ es.channel = msg->u.chip_state_event.channel;
+ es.status = msg->u.chip_state_event.status;
+ es.txerr = msg->u.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.chip_state_event.rx_errors_count;
+ es.error_factor = 0;
break;
default:
dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
@@ -653,122 +706,99 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
return;
}
- if (channel >= dev->nchannels) {
+ if (es.channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", channel);
+ "Invalid channel number (%d)\n", es.channel);
return;
}
- priv = dev->nets[channel];
+ priv = dev->nets[es.channel];
stats = &priv->netdev->stats;
- if (status & M16C_STATE_BUS_RESET) {
+ if (es.status & M16C_STATE_BUS_RESET) {
kvaser_usb_unlink_tx_urbs(priv);
return;
}
+ /* Update all of the can interface's state and error counters before
+ * trying any skb allocation that can actually fail with -ENOMEM.
+ */
+ old_state = priv->can.state;
+ kvaser_usb_rx_error_update_can_state(priv, &es);
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
return;
}
- new_state = priv->can.state;
-
- netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
-
- if (status & M16C_STATE_BUS_OFF) {
+ if (es.status & M16C_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_BUSOFF;
- priv->can.can_stats.bus_off++;
if (!priv->can.restart_ms)
kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
-
netif_carrier_off(priv->netdev);
-
- new_state = CAN_STATE_BUS_OFF;
- } else if (status & M16C_STATE_BUS_PASSIVE) {
- if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
+ } else if (es.status & M16C_STATE_BUS_PASSIVE) {
+ if (old_state != CAN_STATE_ERROR_PASSIVE) {
cf->can_id |= CAN_ERR_CRTL;
- if (txerr || rxerr)
- cf->data[1] = (txerr > rxerr)
+ 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;
-
- priv->can.can_stats.error_passive++;
}
-
- new_state = CAN_STATE_ERROR_PASSIVE;
}
- if (status == M16C_STATE_BUS_ERROR) {
- if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((txerr >= 96) || (rxerr >= 96))) {
+ 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] = (txerr > rxerr)
+ cf->data[1] = (es.txerr > es.rxerr)
? CAN_ERR_CRTL_TX_WARNING
: CAN_ERR_CRTL_RX_WARNING;
-
- priv->can.can_stats.error_warning++;
- new_state = CAN_STATE_ERROR_WARNING;
- } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
+ } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
- new_state = CAN_STATE_ERROR_ACTIVE;
}
}
- if (!status) {
+ if (!es.status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
- new_state = CAN_STATE_ERROR_ACTIVE;
}
if (priv->can.restart_ms &&
- (priv->can.state >= CAN_STATE_BUS_OFF) &&
- (new_state < CAN_STATE_BUS_OFF)) {
+ (old_state >= CAN_STATE_BUS_OFF) &&
+ (priv->can.state < CAN_STATE_BUS_OFF)) {
cf->can_id |= CAN_ERR_RESTARTED;
netif_carrier_on(priv->netdev);
-
- priv->can.can_stats.restarts++;
}
- if (error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
-
+ if (es.error_factor) {
cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
- if (error_factor & M16C_EF_ACKE)
+ if (es.error_factor & M16C_EF_ACKE)
cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- if (error_factor & M16C_EF_CRCE)
+ if (es.error_factor & M16C_EF_CRCE)
cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL);
- if (error_factor & M16C_EF_FORME)
+ if (es.error_factor & M16C_EF_FORME)
cf->data[2] |= CAN_ERR_PROT_FORM;
- if (error_factor & M16C_EF_STFE)
+ if (es.error_factor & M16C_EF_STFE)
cf->data[2] |= CAN_ERR_PROT_STUFF;
- if (error_factor & M16C_EF_BITE0)
+ if (es.error_factor & M16C_EF_BITE0)
cf->data[2] |= CAN_ERR_PROT_BIT0;
- if (error_factor & M16C_EF_BITE1)
+ if (es.error_factor & M16C_EF_BITE1)
cf->data[2] |= CAN_ERR_PROT_BIT1;
- if (error_factor & M16C_EF_TRE)
+ if (es.error_factor & M16C_EF_TRE)
cf->data[2] |= CAN_ERR_PROT_TX;
}
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
-
- priv->bec.txerr = txerr;
- priv->bec.rxerr = rxerr;
-
- priv->can.state = new_state;
+ cf->data[6] = es.txerr;
+ cf->data[7] = es.rxerr;
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
@@ -792,6 +822,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
}
if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ stats->rx_over_errors++;
+ stats->rx_errors++;
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -801,9 +834,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
- stats->rx_over_errors++;
- stats->rx_errors++;
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
netif_rx(skb);
--
1.9.1
^ permalink raw reply related
* [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-20 21:45 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214409.GA16828@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.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@marel.com>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.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
^ permalink raw reply related
* [PATCH v5 3/5] can: kvaser_usb: Fix state handling upon BUS_ERROR events
From: Ahmed S. Darwish @ 2015-01-20 21:47 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214537.GB16828@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
While being in an ERROR_WARNING state and receiving further
bus error events with error counts in the range of 97-127 inclusive,
the state handling code erroneously reverts back to ERROR_ACTIVE.
As per the CAN standard recommendations, only revert to ERROR_ACTIVE
when the error counters are less than 96.
Moreover, in certain Kvaser models, the BUS_ERROR flag is always set
along with undefined bits in the M16C status register. Thus use
bitwise ops instead of full equality for checking the register
against bus errors.
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 0386d3f..640b0eb 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -635,10 +635,12 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
new_state = CAN_STATE_BUS_OFF;
else if (es->status & M16C_STATE_BUS_PASSIVE)
new_state = CAN_STATE_ERROR_PASSIVE;
-
- if (es->status == M16C_STATE_BUS_ERROR) {
- if ((cur_state < CAN_STATE_ERROR_WARNING) &&
- ((es->txerr >= 96) || (es->rxerr >= 96)))
+ else if (es->status & M16C_STATE_BUS_ERROR) {
+ if ((es->txerr >= 256) || (es->rxerr >= 256))
+ new_state = CAN_STATE_BUS_OFF;
+ else if ((es->txerr >= 128) || (es->rxerr >= 128))
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ else if ((es->txerr >= 96) || (es->rxerr >= 96))
new_state = CAN_STATE_ERROR_WARNING;
else if (cur_state > CAN_STATE_ERROR_ACTIVE)
new_state = CAN_STATE_ERROR_ACTIVE;
@@ -748,15 +750,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
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;
- }
+ } else if (es.status & M16C_STATE_BUS_ERROR) {
+ if ((es.txerr < 96 && es.rxerr < 96) &&
+ (old_state > CAN_STATE_ERROR_ACTIVE)) {
+ cf->can_id |= CAN_ERR_PROT;
+ cf->data[2] = CAN_ERR_PROT_ACTIVE;
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] udp: Do not require sock in udp_tunnel_xmit_skb
From: Or Gerlitz @ 2015-01-20 21:47 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Thomas Graf, Linux Netdev List
In-Reply-To: <CA+mtBx8dihFYb1v_WeyukfV5hk==2=3_b_GNUJoxCuL8_GUf6A@mail.gmail.com>
On Tue, Jan 20, 2015 at 7:36 PM, Tom Herbert <therbert@google.com> wrote:
> On Sun, Jan 18, 2015 at 2:43 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Sat, Jan 17, 2015 at 8:18 PM, Tom Herbert <therbert@google.com> wrote:
>>> The UDP tunnel transmit functions udp_tunnel_xmit_skb and
>>> udp_tunnel6_xmit_skb include a socket argument. The socket being
>>> passed to the functions (from VXLAN) is a UDP created for receive
>>> side. The only thing that the socket is used for in the transmit
>>> functions is to get the setting for checksum (enabled or zero).
>>
>> Tom, just to clarify - re the sockets usage in the transmit side,
>> somewhere bind or alike is done on them such that we have multiple
>> source UDP ports for given host VXLAN traffic. Here for example the
>> sender host is 192.168.31.17 and two ports are seen here 54206 and
>> 50795.
>>
>> Just wanted to make sure this series doesn't change that, since if
>> this is the case, we introduce here a regression w.r.t RSS hash
>> spreading from the outer UDP header data at the receiver side (which
>> is the right thing to do, per your LKS session...)
>>
> Hi Or,
>
> Using or not using a socket on transmit should have no bearing to the
> receive side. RSS works based on the hash of the UDP 5-tuple which
> should include a source port set to a value for the inner flow. Since
> the UDP socket is unconnected it should have no bearing on RFS or XPS
> either.
Hi Tom,
You say "which should include a source port set to a value for the
inner flow", well this series doesn't add such logic, nor I am fully
clear what piece exactly is responsible for the fact that I see
multiple source udp ports used from vxlan traffic flowing out of a
certain host. I just wanted to make sure that these patches don't
introduce a regression w.r.t to the **current** (not future) state of
things, can you confirm this?
^ permalink raw reply
* [PATCH v5 4/5] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT
From: Ahmed S. Darwish @ 2015-01-20 21:48 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214709.GC16828@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
On some x86 laptops, plugging a Kvaser device again after an
unplug makes the firmware always ignore the very first command.
For such a case, provide some room for retries instead of
completly exiting the driver init code.
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 640b0eb..068e76c 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1614,7 +1614,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
{
struct kvaser_usb *dev;
int err = -ENOMEM;
- int i;
+ int i, retry = 3;
dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -1632,7 +1632,15 @@ static int kvaser_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, dev);
- err = kvaser_usb_get_software_info(dev);
+ /* On some x86 laptops, plugging a Kvaser device again after
+ * an unplug makes the firmware always ignore the very first
+ * command. For such a case, provide some room for retries
+ * instead of completly exiting the driver.
+ */
+ do {
+ err = kvaser_usb_get_software_info(dev);
+ } while (--retry && err == -ETIMEDOUT);
+
if (err) {
dev_err(&intf->dev,
"Cannot get software infos, error %d\n", err);
--
1.9.1
^ permalink raw reply related
* [PATCH v5 5/5] can: kvaser_usb: Add support for the USBcan-II family
From: Ahmed S. Darwish @ 2015-01-20 21:50 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214834.GD16828@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
divided into two major families: 'Leaf', and 'USBcanII'. From an
Operating System perspective, the firmware of both families behave
in a not too drastically different fashion.
This patch adds support for the USBcanII family of devices to the
current Kvaser Leaf-only driver.
CAN frames sending, receiving, and error handling paths has been
tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
should also work nicely with other products in the same category.
List of new devices supported by this driver update:
- Kvaser USBcan II HS/HS
- Kvaser USBcan II HS/LS
- Kvaser USBcan Rugged ("USBcan Rev B")
- Kvaser Memorator HS/HS
- Kvaser Memorator HS/LS
- Scania VCI2 (if you have the Kvaser logo on top)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/Kconfig | 8 +-
drivers/net/can/usb/kvaser_usb.c | 598 ++++++++++++++++++++++++++++++---------
2 files changed, 478 insertions(+), 128 deletions(-)
** V5 Changelog:
- Rebase on the new CAN error state changes added for the Leaf driver
- Add minor changes (remove unused commands, constify poniters, etc.)
** V4 Changelog:
- Use type-safe C methods instead of cpp macros
- Remove defensive checks against non-existing families
- Re-order methods to remove forward declarations
- Smaller stuff spotted by earlier review (function prefexes, etc.)
** V3 Changelog:
- Fix padding for the usbcan_msg_tx_acknowledge command
- Remove kvaser_usb->max_channels and the MAX_NET_DEVICES macro
- Rename commands to CMD_LEAF_xxx and CMD_USBCAN_xxx
- Apply checkpatch.pl suggestions ('net/' comments, multi-line strings, etc.)
** V2 Changelog:
- Update Kconfig entries
- Use actual number of CAN channels (instead of max) where appropriate
- Rebase over a new set of UsbcanII-independent driver fixes
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index a77db919..f6f5500 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -25,7 +25,7 @@ config CAN_KVASER_USB
tristate "Kvaser CAN/USB interface"
---help---
This driver adds support for Kvaser CAN/USB devices like Kvaser
- Leaf Light.
+ Leaf Light and Kvaser USBcan II.
The driver provides support for the following devices:
- Kvaser Leaf Light
@@ -46,6 +46,12 @@ config CAN_KVASER_USB
- Kvaser USBcan R
- Kvaser Leaf Light v2
- Kvaser Mini PCI Express HS
+ - Kvaser USBcan II HS/HS
+ - Kvaser USBcan II HS/LS
+ - Kvaser USBcan Rugged ("USBcan Rev B")
+ - Kvaser Memorator HS/HS
+ - Kvaser Memorator HS/LS
+ - Scania VCI2 (if you have the Kvaser logo on top)
If unsure, say N.
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 068e76c..3e1eb5d 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -6,10 +6,12 @@
* Parts of this driver are based on the following:
* - Kvaser linux leaf driver (version 4.78)
* - CAN driver for esd CAN-USB/2
+ * - Kvaser linux usbcanII driver (version 5.3)
*
* Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
* Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
* Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
+ * Copyright (C) 2015 Valeo A.S.
*/
#include <linux/completion.h>
@@ -30,8 +32,9 @@
#define RX_BUFFER_SIZE 3072
#define CAN_USB_CLOCK 8000000
#define MAX_NET_DEVICES 3
+#define MAX_USBCAN_NET_DEVICES 2
-/* Kvaser USB devices */
+/* Kvaser Leaf USB devices */
#define KVASER_VENDOR_ID 0x0bfd
#define USB_LEAF_DEVEL_PRODUCT_ID 10
#define USB_LEAF_LITE_PRODUCT_ID 11
@@ -56,6 +59,24 @@
#define USB_LEAF_LITE_V2_PRODUCT_ID 288
#define USB_MINI_PCIE_HS_PRODUCT_ID 289
+static inline bool kvaser_is_leaf(const struct usb_device_id *id)
+{
+ return id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
+ id->idProduct <= USB_MINI_PCIE_HS_PRODUCT_ID;
+}
+
+/* Kvaser USBCan-II devices */
+#define USB_USBCAN_REVB_PRODUCT_ID 2
+#define USB_VCI2_PRODUCT_ID 3
+#define USB_USBCAN2_PRODUCT_ID 4
+#define USB_MEMORATOR_PRODUCT_ID 5
+
+static inline bool kvaser_is_usbcan(const struct usb_device_id *id)
+{
+ return id->idProduct >= USB_USBCAN_REVB_PRODUCT_ID &&
+ id->idProduct <= USB_MEMORATOR_PRODUCT_ID;
+}
+
/* USB devices features */
#define KVASER_HAS_SILENT_MODE BIT(0)
#define KVASER_HAS_TXRX_ERRORS BIT(1)
@@ -73,7 +94,7 @@
#define MSG_FLAG_TX_ACK BIT(6)
#define MSG_FLAG_TX_REQUEST BIT(7)
-/* Can states */
+/* Can states (M16C CxSTRH register) */
#define M16C_STATE_BUS_RESET BIT(0)
#define M16C_STATE_BUS_ERROR BIT(4)
#define M16C_STATE_BUS_PASSIVE BIT(5)
@@ -98,7 +119,11 @@
#define CMD_START_CHIP_REPLY 27
#define CMD_STOP_CHIP 28
#define CMD_STOP_CHIP_REPLY 29
-#define CMD_GET_CARD_INFO2 32
+
+#define CMD_LEAF_GET_CARD_INFO2 32
+#define CMD_USBCAN_RESET_CLOCK 32
+#define CMD_USBCAN_CLOCK_OVERFLOW_EVENT 33
+
#define CMD_GET_CARD_INFO 34
#define CMD_GET_CARD_INFO_REPLY 35
#define CMD_GET_SOFTWARE_INFO 38
@@ -108,8 +133,9 @@
#define CMD_RESET_ERROR_COUNTER 49
#define CMD_TX_ACKNOWLEDGE 50
#define CMD_CAN_ERROR_EVENT 51
-#define CMD_USB_THROTTLE 77
-#define CMD_LOG_MESSAGE 106
+
+#define CMD_LEAF_USB_THROTTLE 77
+#define CMD_LEAF_LOG_MESSAGE 106
/* error factors */
#define M16C_EF_ACKE BIT(0)
@@ -121,6 +147,14 @@
#define M16C_EF_RCVE BIT(6)
#define M16C_EF_TRE BIT(7)
+/* Only Leaf-based devices can report M16C error factors,
+ * thus define our own error status flags for USBCANII
+ */
+#define USBCAN_ERROR_STATE_NONE 0
+#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
+#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
+#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
+
/* bittiming parameters */
#define KVASER_USB_TSEG1_MIN 1
#define KVASER_USB_TSEG1_MAX 16
@@ -137,9 +171,18 @@
#define KVASER_CTRL_MODE_SELFRECEPTION 3
#define KVASER_CTRL_MODE_OFF 4
-/* log message */
+/* Extended CAN identifier flag */
#define KVASER_EXTENDED_FRAME BIT(31)
+/* Kvaser USB CAN dongles are divided into two major families:
+ * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
+ * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
+ */
+enum kvaser_usb_family {
+ KVASER_LEAF,
+ KVASER_USBCAN,
+};
+
struct kvaser_msg_simple {
u8 tid;
u8 channel;
@@ -148,30 +191,55 @@ struct kvaser_msg_simple {
struct kvaser_msg_cardinfo {
u8 tid;
u8 nchannels;
- __le32 serial_number;
- __le32 padding;
+ union {
+ struct {
+ __le32 serial_number;
+ __le32 padding;
+ } __packed leaf0;
+ struct {
+ __le32 serial_number_low;
+ __le32 serial_number_high;
+ } __packed usbcan0;
+ } __packed;
__le32 clock_resolution;
__le32 mfgdate;
u8 ean[8];
u8 hw_revision;
- u8 usb_hs_mode;
- __le16 padding2;
+ union {
+ struct {
+ u8 usb_hs_mode;
+ } __packed leaf1;
+ struct {
+ u8 padding;
+ } __packed usbcan1;
+ } __packed;
+ __le16 padding;
} __packed;
struct kvaser_msg_cardinfo2 {
u8 tid;
- u8 channel;
+ u8 reserved;
u8 pcb_id[24];
__le32 oem_unlock_code;
} __packed;
-struct kvaser_msg_softinfo {
+struct leaf_msg_softinfo {
u8 tid;
- u8 channel;
+ u8 padding0;
__le32 sw_options;
__le32 fw_version;
__le16 max_outstanding_tx;
- __le16 padding[9];
+ __le16 padding1[9];
+} __packed;
+
+struct usbcan_msg_softinfo {
+ u8 tid;
+ u8 fw_name[5];
+ __le16 max_outstanding_tx;
+ u8 padding[6];
+ __le32 fw_version;
+ __le16 checksum;
+ __le16 sw_options;
} __packed;
struct kvaser_msg_busparams {
@@ -188,36 +256,86 @@ struct kvaser_msg_tx_can {
u8 channel;
u8 tid;
u8 msg[14];
- u8 padding;
- u8 flags;
+ union {
+ struct {
+ u8 padding;
+ u8 flags;
+ } __packed leaf;
+ struct {
+ u8 flags;
+ u8 padding;
+ } __packed usbcan;
+ } __packed;
+} __packed;
+
+struct kvaser_msg_rx_can_header {
+ u8 channel;
+ u8 flag;
} __packed;
-struct kvaser_msg_rx_can {
+struct leaf_msg_rx_can {
u8 channel;
u8 flag;
+
__le16 time[3];
u8 msg[14];
} __packed;
-struct kvaser_msg_chip_state_event {
+struct usbcan_msg_rx_can {
+ u8 channel;
+ u8 flag;
+
+ u8 msg[14];
+ __le16 time;
+} __packed;
+
+struct leaf_msg_chip_state_event {
u8 tid;
u8 channel;
+
__le16 time[3];
u8 tx_errors_count;
u8 rx_errors_count;
+
+ u8 status;
+ u8 padding[3];
+} __packed;
+
+struct usbcan_msg_chip_state_event {
+ u8 tid;
+ u8 channel;
+
+ u8 tx_errors_count;
+ u8 rx_errors_count;
+ __le16 time;
+
u8 status;
u8 padding[3];
} __packed;
-struct kvaser_msg_tx_acknowledge {
+struct kvaser_msg_tx_acknowledge_header {
u8 channel;
u8 tid;
+} __packed;
+
+struct leaf_msg_tx_acknowledge {
+ u8 channel;
+ u8 tid;
+
__le16 time[3];
u8 flags;
u8 time_offset;
} __packed;
-struct kvaser_msg_error_event {
+struct usbcan_msg_tx_acknowledge {
+ u8 channel;
+ u8 tid;
+
+ __le16 time;
+ __le16 padding;
+} __packed;
+
+struct leaf_msg_error_event {
u8 tid;
u8 flags;
__le16 time[3];
@@ -229,6 +347,18 @@ struct kvaser_msg_error_event {
u8 error_factor;
} __packed;
+struct usbcan_msg_error_event {
+ u8 tid;
+ u8 padding;
+ u8 tx_errors_count_ch0;
+ u8 rx_errors_count_ch0;
+ u8 tx_errors_count_ch1;
+ u8 rx_errors_count_ch1;
+ u8 status_ch0;
+ u8 status_ch1;
+ __le16 time;
+} __packed;
+
struct kvaser_msg_ctrl_mode {
u8 tid;
u8 channel;
@@ -243,7 +373,7 @@ struct kvaser_msg_flush_queue {
u8 padding[3];
} __packed;
-struct kvaser_msg_log_message {
+struct leaf_msg_log_message {
u8 channel;
u8 flags;
__le16 time[3];
@@ -260,21 +390,55 @@ struct kvaser_msg {
struct kvaser_msg_simple simple;
struct kvaser_msg_cardinfo cardinfo;
struct kvaser_msg_cardinfo2 cardinfo2;
- struct kvaser_msg_softinfo softinfo;
struct kvaser_msg_busparams busparams;
+
+ struct kvaser_msg_rx_can_header rx_can_header;
+ struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
+
+ union {
+ struct leaf_msg_softinfo softinfo;
+ struct leaf_msg_rx_can rx_can;
+ struct leaf_msg_chip_state_event chip_state_event;
+ struct leaf_msg_tx_acknowledge tx_acknowledge;
+ struct leaf_msg_error_event error_event;
+ struct leaf_msg_log_message log_message;
+ } __packed leaf;
+
+ union {
+ struct usbcan_msg_softinfo softinfo;
+ struct usbcan_msg_rx_can rx_can;
+ struct usbcan_msg_chip_state_event chip_state_event;
+ struct usbcan_msg_tx_acknowledge tx_acknowledge;
+ struct usbcan_msg_error_event error_event;
+ } __packed usbcan;
+
struct kvaser_msg_tx_can tx_can;
- struct kvaser_msg_rx_can rx_can;
- struct kvaser_msg_chip_state_event chip_state_event;
- struct kvaser_msg_tx_acknowledge tx_acknowledge;
- struct kvaser_msg_error_event error_event;
struct kvaser_msg_ctrl_mode ctrl_mode;
struct kvaser_msg_flush_queue flush_queue;
- struct kvaser_msg_log_message log_message;
} u;
} __packed;
+/* Summary of a kvaser error event, for a unified Leaf/Usbcan error
+ * handling. Some discrepancies between the two families exist:
+ *
+ * - USBCAN firmware does not report M16C "error factors"
+ * - USBCAN controllers has difficulties reporting if the raised error
+ * event is for ch0 or ch1. They leave such arbitration to the OS
+ * driver by letting it compare error counters with previous values
+ * and decide the error event's channel. Thus for USBCAN, the channel
+ * field is only advisory.
+ */
struct kvaser_usb_error_summary {
- u8 channel, status, txerr, rxerr, error_factor;
+ u8 channel, status, txerr, rxerr;
+ union {
+ struct {
+ u8 error_factor;
+ } leaf;
+ struct {
+ u8 other_ch_status;
+ u8 error_state;
+ } usbcan;
+ };
};
struct kvaser_usb_tx_urb_context {
@@ -292,6 +456,7 @@ struct kvaser_usb {
u32 fw_version;
unsigned int nchannels;
+ enum kvaser_usb_family family;
bool rxinitdone;
void *rxbuf[MAX_RX_URBS];
@@ -315,6 +480,7 @@ struct kvaser_usb_net_priv {
};
static const struct usb_device_id kvaser_usb_table[] = {
+ /* Leaf family IDs */
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
@@ -364,6 +530,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
.driver_info = KVASER_HAS_TXRX_ERRORS },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
+
+ /* USBCANII family IDs */
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+
{ }
};
MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
@@ -467,7 +644,14 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
if (err)
return err;
- dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
+ switch (dev->family) {
+ case KVASER_LEAF:
+ dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
+ break;
+ case KVASER_USBCAN:
+ dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
+ break;
+ }
return 0;
}
@@ -486,7 +670,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
return err;
dev->nchannels = msg.u.cardinfo.nchannels;
- if (dev->nchannels > MAX_NET_DEVICES)
+ if ((dev->nchannels > MAX_NET_DEVICES) ||
+ (dev->family == KVASER_USBCAN &&
+ dev->nchannels > MAX_USBCAN_NET_DEVICES))
return -EINVAL;
return 0;
@@ -500,8 +686,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
struct kvaser_usb_net_priv *priv;
struct sk_buff *skb;
struct can_frame *cf;
- u8 channel = msg->u.tx_acknowledge.channel;
- u8 tid = msg->u.tx_acknowledge.tid;
+ u8 channel, tid;
+
+ channel = msg->u.tx_acknowledge_header.channel;
+ tid = msg->u.tx_acknowledge_header.tid;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -623,12 +811,12 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
const struct kvaser_usb_error_summary *es,
struct can_frame *cf)
{
- struct net_device_stats *stats;
+ struct kvaser_usb *dev = priv->dev;
+ struct net_device_stats *stats = &priv->netdev->stats;
enum can_state cur_state, new_state, tx_state, rx_state;
netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
- stats = &priv->netdev->stats;
new_state = cur_state = priv->can.state;
if (es->status & M16C_STATE_BUS_OFF)
@@ -663,9 +851,22 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
priv->can.can_stats.restarts++;
}
- if (es->error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ if (es->leaf.error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+ }
+ break;
+ case KVASER_USBCAN:
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
+ stats->tx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
+ stats->rx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
+ priv->can.can_stats.bus_error++;
+ }
+ break;
}
priv->bec.txerr = es->txerr;
@@ -673,53 +874,24 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
}
static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
- const struct kvaser_msg *msg)
+ const struct kvaser_usb_error_summary *es)
{
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, new_state;
- switch (msg->id) {
- case CMD_CAN_ERROR_EVENT:
- es.channel = msg->u.error_event.channel;
- es.status = msg->u.error_event.status;
- es.txerr = msg->u.error_event.tx_errors_count;
- es.rxerr = msg->u.error_event.rx_errors_count;
- es.error_factor = msg->u.error_event.error_factor;
- break;
- case CMD_LOG_MESSAGE:
- es.channel = msg->u.log_message.channel;
- es.status = msg->u.log_message.data[0];
- es.txerr = msg->u.log_message.data[2];
- es.rxerr = msg->u.log_message.data[3];
- es.error_factor = msg->u.log_message.data[1];
- break;
- case CMD_CHIP_STATE_EVENT:
- es.channel = msg->u.chip_state_event.channel;
- es.status = msg->u.chip_state_event.status;
- es.txerr = msg->u.chip_state_event.tx_errors_count;
- es.rxerr = msg->u.chip_state_event.rx_errors_count;
- es.error_factor = 0;
- break;
- default:
- dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
- msg->id);
- return;
- }
-
- if (es.channel >= dev->nchannels) {
+ if (es->channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", es.channel);
+ "Invalid channel number (%d)\n", es->channel);
return;
}
- priv = dev->nets[es.channel];
+ priv = dev->nets[es->channel];
stats = &priv->netdev->stats;
- if (es.status & M16C_STATE_BUS_RESET) {
+ if (es->status & M16C_STATE_BUS_RESET) {
kvaser_usb_unlink_tx_urbs(priv);
return;
}
@@ -735,7 +907,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
* frame ID and data to userspace. Remove stack allocation afterwards.
*/
old_state = priv->can.state;
- kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
+ kvaser_usb_rx_error_update_can_state(priv, es, &tmp_cf);
new_state = priv->can.state;
skb = alloc_can_err_skb(priv->netdev, &cf);
@@ -746,19 +918,19 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
memcpy(cf, &tmp_cf, sizeof(*cf));
if (new_state != old_state) {
- if (es.status & M16C_STATE_BUS_OFF) {
+ 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);
- } else if (es.status & M16C_STATE_BUS_ERROR) {
- if ((es.txerr < 96 && es.rxerr < 96) &&
+ } else if (es->status & M16C_STATE_BUS_ERROR) {
+ if ((es->txerr < 96 && es->rxerr < 96) &&
(old_state > CAN_STATE_ERROR_ACTIVE)) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
}
}
- if (!es.status) {
+ if (!es->status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
}
@@ -771,34 +943,161 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
}
- if (es.error_factor) {
- cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
-
- if (es.error_factor & M16C_EF_ACKE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- if (es.error_factor & M16C_EF_CRCE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
- CAN_ERR_PROT_LOC_CRC_DEL);
- if (es.error_factor & M16C_EF_FORME)
- cf->data[2] |= CAN_ERR_PROT_FORM;
- if (es.error_factor & M16C_EF_STFE)
- cf->data[2] |= CAN_ERR_PROT_STUFF;
- if (es.error_factor & M16C_EF_BITE0)
- cf->data[2] |= CAN_ERR_PROT_BIT0;
- if (es.error_factor & M16C_EF_BITE1)
- cf->data[2] |= CAN_ERR_PROT_BIT1;
- if (es.error_factor & M16C_EF_TRE)
- cf->data[2] |= CAN_ERR_PROT_TX;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ if (es->leaf.error_factor) {
+ cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
+
+ if (es->leaf.error_factor & M16C_EF_ACKE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
+ if (es->leaf.error_factor & M16C_EF_CRCE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
+ CAN_ERR_PROT_LOC_CRC_DEL);
+ if (es->leaf.error_factor & M16C_EF_FORME)
+ cf->data[2] |= CAN_ERR_PROT_FORM;
+ if (es->leaf.error_factor & M16C_EF_STFE)
+ cf->data[2] |= CAN_ERR_PROT_STUFF;
+ if (es->leaf.error_factor & M16C_EF_BITE0)
+ cf->data[2] |= CAN_ERR_PROT_BIT0;
+ if (es->leaf.error_factor & M16C_EF_BITE1)
+ cf->data[2] |= CAN_ERR_PROT_BIT1;
+ if (es->leaf.error_factor & M16C_EF_TRE)
+ cf->data[2] |= CAN_ERR_PROT_TX;
+ }
+ break;
+ case KVASER_USBCAN:
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
+ cf->can_id |= CAN_ERR_BUSERROR;
+ }
+ break;
}
- cf->data[6] = es.txerr;
- cf->data[7] = es.rxerr;
+ cf->data[6] = es->txerr;
+ cf->data[7] = es->rxerr;
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
netif_rx(skb);
}
+/* For USBCAN, report error to userspace iff the channels's errors counter
+ * has increased, or we're the only channel seeing a bus error state.
+ */
+static void kvaser_usbcan_conditionally_rx_error(const struct kvaser_usb *dev,
+ struct kvaser_usb_error_summary *es)
+{
+ struct kvaser_usb_net_priv *priv;
+ int channel;
+ bool report_error;
+
+ channel = es->channel;
+ if (channel >= dev->nchannels) {
+ dev_err(dev->udev->dev.parent,
+ "Invalid channel number (%d)\n", channel);
+ return;
+ }
+
+ priv = dev->nets[channel];
+ report_error = false;
+
+ if (es->txerr > priv->bec.txerr) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+ report_error = true;
+ }
+ if (es->rxerr > priv->bec.rxerr) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+ report_error = true;
+ }
+ if ((es->status & M16C_STATE_BUS_ERROR) &&
+ !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
+ report_error = true;
+ }
+
+ if (report_error)
+ kvaser_usb_rx_error(dev, es);
+}
+
+static void kvaser_usbcan_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_usb_error_summary es = { };
+
+ switch (msg->id) {
+ /* Sometimes errors are sent as unsolicited chip state events */
+ case CMD_CHIP_STATE_EVENT:
+ es.channel = msg->u.usbcan.chip_state_event.channel;
+ es.status = msg->u.usbcan.chip_state_event.status;
+ es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
+ kvaser_usbcan_conditionally_rx_error(dev, &es);
+ break;
+
+ case CMD_CAN_ERROR_EVENT:
+ es.channel = 0;
+ es.status = msg->u.usbcan.error_event.status_ch0;
+ es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
+ es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch1;
+ kvaser_usbcan_conditionally_rx_error(dev, &es);
+
+ /* The USBCAN firmware supports up to 2 channels.
+ * Now that ch0 was checked, check if ch1 has any errors.
+ */
+ if (dev->nchannels == MAX_USBCAN_NET_DEVICES) {
+ es.channel = 1;
+ es.status = msg->u.usbcan.error_event.status_ch1;
+ es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch1;
+ es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch1;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch0;
+ kvaser_usbcan_conditionally_rx_error(dev, &es);
+ }
+ break;
+
+ default:
+ dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+ msg->id);
+ }
+}
+
+static void kvaser_leaf_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_usb_error_summary es = { };
+
+ switch (msg->id) {
+ case CMD_CAN_ERROR_EVENT:
+ es.channel = msg->u.leaf.error_event.channel;
+ es.status = msg->u.leaf.error_event.status;
+ es.txerr = msg->u.leaf.error_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.error_event.rx_errors_count;
+ es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
+ break;
+ case CMD_LEAF_LOG_MESSAGE:
+ es.channel = msg->u.leaf.log_message.channel;
+ es.status = msg->u.leaf.log_message.data[0];
+ es.txerr = msg->u.leaf.log_message.data[2];
+ es.rxerr = msg->u.leaf.log_message.data[3];
+ es.leaf.error_factor = msg->u.leaf.log_message.data[1];
+ break;
+ case CMD_CHIP_STATE_EVENT:
+ es.channel = msg->u.leaf.chip_state_event.channel;
+ es.status = msg->u.leaf.chip_state_event.status;
+ es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
+ es.leaf.error_factor = 0;
+ break;
+ default:
+ dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+ msg->id);
+ return;
+ }
+
+ kvaser_usb_rx_error(dev, &es);
+}
+
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
const struct kvaser_msg *msg)
{
@@ -806,16 +1105,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
struct sk_buff *skb;
struct net_device_stats *stats = &priv->netdev->stats;
- if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
+ if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
MSG_FLAG_NERR)) {
netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
stats->rx_errors++;
return;
}
- if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
stats->rx_over_errors++;
stats->rx_errors++;
@@ -841,7 +1140,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
struct can_frame *cf;
struct sk_buff *skb;
struct net_device_stats *stats;
- u8 channel = msg->u.rx_can.channel;
+ u8 channel = msg->u.rx_can_header.channel;
+ const u8 *rx_msg = NULL; /* GCC */
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -852,60 +1152,68 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
priv = dev->nets[channel];
stats = &priv->netdev->stats;
- if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
- (msg->id == CMD_LOG_MESSAGE)) {
- kvaser_usb_rx_error(dev, msg);
+ if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+ (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
+ kvaser_leaf_rx_error(dev, msg);
return;
- } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
- MSG_FLAG_NERR |
- MSG_FLAG_OVERRUN)) {
+ } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
+ MSG_FLAG_NERR |
+ MSG_FLAG_OVERRUN)) {
kvaser_usb_rx_can_err(priv, msg);
return;
- } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
+ } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
netdev_warn(priv->netdev,
"Unhandled frame (flags: 0x%02x)",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
return;
}
+ switch (dev->family) {
+ case KVASER_LEAF:
+ rx_msg = msg->u.leaf.rx_can.msg;
+ break;
+ case KVASER_USBCAN:
+ rx_msg = msg->u.usbcan.rx_can.msg;
+ break;
+ }
+
skb = alloc_can_skb(priv->netdev, &cf);
if (!skb) {
stats->tx_dropped++;
return;
}
- if (msg->id == CMD_LOG_MESSAGE) {
- cf->can_id = le32_to_cpu(msg->u.log_message.id);
+ if (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE) {
+ cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
if (cf->can_id & KVASER_EXTENDED_FRAME)
cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
else
cf->can_id &= CAN_SFF_MASK;
- cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
+ cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
- if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.log_message.data,
+ memcpy(cf->data, &msg->u.leaf.log_message.data,
cf->can_dlc);
} else {
- cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
- (msg->u.rx_can.msg[1] & 0x3f);
+ cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
if (msg->id == CMD_RX_EXT_MESSAGE) {
cf->can_id <<= 18;
- cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
- ((msg->u.rx_can.msg[3] & 0xff) << 6) |
- (msg->u.rx_can.msg[4] & 0x3f);
+ cf->can_id |= ((rx_msg[2] & 0x0f) << 14) |
+ ((rx_msg[3] & 0xff) << 6) |
+ (rx_msg[4] & 0x3f);
cf->can_id |= CAN_EFF_FLAG;
}
- cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+ cf->can_dlc = get_can_dlc(rx_msg[5]);
- if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.rx_can.msg[6],
+ memcpy(cf->data, &rx_msg[6],
cf->can_dlc);
}
@@ -968,21 +1276,35 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
case CMD_RX_STD_MESSAGE:
case CMD_RX_EXT_MESSAGE:
- case CMD_LOG_MESSAGE:
+ kvaser_usb_rx_can_msg(dev, msg);
+ break;
+
+ case CMD_LEAF_LOG_MESSAGE:
+ if (dev->family != KVASER_LEAF)
+ goto warn;
kvaser_usb_rx_can_msg(dev, msg);
break;
case CMD_CHIP_STATE_EVENT:
case CMD_CAN_ERROR_EVENT:
- kvaser_usb_rx_error(dev, msg);
+ if (dev->family == KVASER_LEAF)
+ kvaser_leaf_rx_error(dev, msg);
+ else
+ kvaser_usbcan_rx_error(dev, msg);
break;
case CMD_TX_ACKNOWLEDGE:
kvaser_usb_tx_acknowledge(dev, msg);
break;
+ /* Ignored messages */
+ case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
+ if (dev->family != KVASER_USBCAN)
+ goto warn;
+ break;
+
default:
- dev_warn(dev->udev->dev.parent,
+warn: dev_warn(dev->udev->dev.parent,
"Unhandled message (%d)\n", msg->id);
break;
}
@@ -1202,7 +1524,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
dev->rxbuf[i],
dev->rxbuf_dma[i]);
- for (i = 0; i < MAX_NET_DEVICES; i++) {
+ for (i = 0; i < dev->nchannels; i++) {
struct kvaser_usb_net_priv *priv = dev->nets[i];
if (priv)
@@ -1310,6 +1632,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
struct kvaser_msg *msg;
int i, err;
int ret = NETDEV_TX_OK;
+ u8 *msg_tx_can_flags = NULL; /* GCC */
if (can_dropped_invalid_skb(netdev, skb))
return NETDEV_TX_OK;
@@ -1331,9 +1654,19 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
msg = buf;
msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
- msg->u.tx_can.flags = 0;
msg->u.tx_can.channel = priv->channel;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
+ break;
+ case KVASER_USBCAN:
+ msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
+ break;
+ }
+
+ *msg_tx_can_flags = 0;
+
if (cf->can_id & CAN_EFF_FLAG) {
msg->id = CMD_TX_EXT_MESSAGE;
msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
@@ -1351,7 +1684,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
if (cf->can_id & CAN_RTR_FLAG)
- msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
+ *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME;
for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
@@ -1620,6 +1953,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!dev)
return -ENOMEM;
+ if (kvaser_is_leaf(id)) {
+ dev->family = KVASER_LEAF;
+ } else if (kvaser_is_usbcan(id)) {
+ dev->family = KVASER_USBCAN;
+ } else {
+ dev_err(&intf->dev,
+ "Product ID (%d) does not belong to any known Kvaser USB family",
+ id->idProduct);
+ return -ENODEV;
+ }
+
err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
if (err) {
dev_err(&intf->dev, "Cannot get usb endpoint(s)");
--
1.9.1
^ permalink raw reply related
* [PATCH net] ipv4: try to cache dst_entries which would cause a redirect
From: Hannes Frederic Sowa @ 2015-01-20 22:08 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, Marcelo Leitner, Florian Westphal
Not caching dst_entries which cause redirects could be exploited by hosts
on the same subnet, causing a severe DoS attack. This effect aggravated
since commit f88649721268999 ("ipv4: fix dst race in sk_dst_get()").
Lookups causing redirects will be allocated with DST_NOCACHE set which
will force dst_release to free them via RCU. Unfortunately waiting for
RCU grace period just takes too long, we can end up with >1M dst_entries
waiting to be released and the system will run OOM. rcuos threads cannot
catch up under high softirq load.
Attaching the flag to emit a redirect later on to the specific skb allows
us to cache those dst_entries thus reducing the pressure on allocation
and deallocation.
This issue was discovered by Marcelo Leitner.
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Marcelo Leitner <mleitner@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/ip.h | 11 ++++++-----
net/ipv4/ip_forward.c | 3 ++-
net/ipv4/route.c | 6 ++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 0e5a0ba..14211ea 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -39,11 +39,12 @@ struct inet_skb_parm {
struct ip_options opt; /* Compiled IP options */
unsigned char flags;
-#define IPSKB_FORWARDED 1
-#define IPSKB_XFRM_TUNNEL_SIZE 2
-#define IPSKB_XFRM_TRANSFORMED 4
-#define IPSKB_FRAG_COMPLETE 8
-#define IPSKB_REROUTED 16
+#define IPSKB_FORWARDED BIT(0)
+#define IPSKB_XFRM_TUNNEL_SIZE BIT(1)
+#define IPSKB_XFRM_TRANSFORMED BIT(2)
+#define IPSKB_FRAG_COMPLETE BIT(3)
+#define IPSKB_REROUTED BIT(4)
+#define IPSKB_DOREDIRECT BIT(5)
u16 frag_max_size;
};
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 3a83ce5..787b3c2 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -129,7 +129,8 @@ int ip_forward(struct sk_buff *skb)
* We now generate an ICMP HOST REDIRECT giving the route
* we calculated.
*/
- if (rt->rt_flags&RTCF_DOREDIRECT && !opt->srr && !skb_sec_path(skb))
+ if (IPCB(skb)->flags & IPSKB_DOREDIRECT && !opt->srr &&
+ !skb_sec_path(skb))
ip_rt_send_redirect(skb);
skb->priority = rt_tos2priority(iph->tos);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 2000110..f41cd21 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1568,10 +1568,8 @@ static int __mkroute_input(struct sk_buff *skb,
do_cache = res->fi && !itag;
if (out_dev == in_dev && err && IN_DEV_TX_REDIRECTS(out_dev) &&
(IN_DEV_SHARED_MEDIA(out_dev) ||
- inet_addr_onlink(out_dev, saddr, FIB_RES_GW(*res)))) {
- flags |= RTCF_DOREDIRECT;
- do_cache = false;
- }
+ inet_addr_onlink(out_dev, saddr, FIB_RES_GW(*res))))
+ IPCB(skb)->flags |= IPSKB_DOREDIRECT;
if (skb->protocol != htons(ETH_P_IP)) {
/* Not IP (i.e. ARP). Do not create route, if it is
--
2.1.0
^ permalink raw reply related
* Re: [net-next PATCH v3 04/12] net: flow_table: create a set of common headers and actions
From: John Fastabend @ 2015-01-20 22:10 UTC (permalink / raw)
To: John W. Linville
Cc: tgraf, simon.horman, sfeldma, netdev, jhs, davem, gerlitz.or,
andy, ast
In-Reply-To: <20150120205938.GD1854@tuxdriver.com>
On 01/20/2015 12:59 PM, John W. Linville wrote:
> On Tue, Jan 20, 2015 at 12:27:53PM -0800, John Fastabend wrote:
>> This adds common headers and actions that drivers can use.
>>
>> I have not yet moved the header graphs into the common header
>> because I'm not entirely convinced its re-usable. The devices
>> I have been looking at have different enough header graphs that
>> they wouldn't be re-usable. However possibly many 40Gbp NICs
>> for example could share a common header graph. When we get
>> multiple implementations we can move this into the common file
>> if it makes sense.
>>
>> And table structures seem to be unique enough that there is
>> little value in putting each devices table layout into the
>> common file so its left for device specific implementation.
>>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> ---
>> include/linux/if_flow_common.h | 257 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 257 insertions(+)
>> create mode 100644 include/linux/if_flow_common.h
>>
>> diff --git a/include/linux/if_flow_common.h b/include/linux/if_flow_common.h
>> new file mode 100644
>> index 0000000..ef2d66f
>> --- /dev/null
>> +++ b/include/linux/if_flow_common.h
>
> <snip>
>
>> +struct net_flow_action net_flow_pop_vlan = {
>> + .name = "pop_vlan",
>> + .uid = ACTION_POP_VLAN,
>> + .args = net_flow_null_args,
>> +};
>
> Random thought, should there be a "push_vlan" (for double VLAN tagging)?
>
>
Yep I should add that one there are also some other actions
on my todo list but some of them require updates to the driver.
Assuming its not a big issue for anyone I would like like to
get this series in with the match/actions it has and then extend
the action and match lists.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next 1/2] udp: Do not require sock in udp_tunnel_xmit_skb
From: Tom Herbert @ 2015-01-20 22:24 UTC (permalink / raw)
To: Or Gerlitz; +Cc: David Miller, Thomas Graf, Linux Netdev List
In-Reply-To: <CAJ3xEMit2By8S=SwRGJKcwii9J3LKj29sX=tAbCC3bcAMDkr5Q@mail.gmail.com>
On Tue, Jan 20, 2015 at 1:47 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Tue, Jan 20, 2015 at 7:36 PM, Tom Herbert <therbert@google.com> wrote:
>> On Sun, Jan 18, 2015 at 2:43 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Sat, Jan 17, 2015 at 8:18 PM, Tom Herbert <therbert@google.com> wrote:
>>>> The UDP tunnel transmit functions udp_tunnel_xmit_skb and
>>>> udp_tunnel6_xmit_skb include a socket argument. The socket being
>>>> passed to the functions (from VXLAN) is a UDP created for receive
>>>> side. The only thing that the socket is used for in the transmit
>>>> functions is to get the setting for checksum (enabled or zero).
>>>
>>> Tom, just to clarify - re the sockets usage in the transmit side,
>>> somewhere bind or alike is done on them such that we have multiple
>>> source UDP ports for given host VXLAN traffic. Here for example the
>>> sender host is 192.168.31.17 and two ports are seen here 54206 and
>>> 50795.
>>>
>>> Just wanted to make sure this series doesn't change that, since if
>>> this is the case, we introduce here a regression w.r.t RSS hash
>>> spreading from the outer UDP header data at the receiver side (which
>>> is the right thing to do, per your LKS session...)
>>>
>> Hi Or,
>>
>> Using or not using a socket on transmit should have no bearing to the
>> receive side. RSS works based on the hash of the UDP 5-tuple which
>> should include a source port set to a value for the inner flow. Since
>> the UDP socket is unconnected it should have no bearing on RFS or XPS
>> either.
>
> Hi Tom,
>
> You say "which should include a source port set to a value for the
> inner flow", well this series doesn't add such logic, nor I am fully
> clear what piece exactly is responsible for the fact that I see
> multiple source udp ports used from vxlan traffic flowing out of a
> certain host. I just wanted to make sure that these patches don't
> introduce a regression w.r.t to the **current** (not future) state of
> things, can you confirm this?
vxlan_xmit_one calls udp_flow_src_port to get a source port value
based on the encapsulated flow.
^ permalink raw reply
* Re: [PATCH net-next 1/3] skbuff: Add skb_list_linearize()
From: Pravin Shelar @ 2015-01-20 22:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1421790195.4832.12.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Jan 20, 2015 at 1:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2015-01-20 at 12:25 -0800, Pravin B Shelar wrote:
>> similar to skb_linearize(), this API takes skb list as arg and
>> linearize it into one big skb. STT driver patch will use this.
>>
>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>> ---
...
>> @@ -2329,6 +2329,41 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
>> }
>> EXPORT_SYMBOL(skb_copy_and_csum_dev);
>>
>> +int skb_list_linearize(struct sk_buff *head, gfp_t gfp_mask)
>> +{
>> + struct sk_buff *skb;
>> + int tlen = 0;
>> + int err;
>> +
>> + err = skb_linearize(head);
>> + if (err)
>> + return err;
>> +
>> + skb = head->next;
>> + while (skb) {
>> + tlen += skb->len;
>> + skb = skb->next;
>> + }
>> + err = pskb_expand_head(head, 0, tlen, gfp_mask);
>> + if (err)
>> + return err;
>> +
>> + skb = head->next;
>> + while (skb) {
>> + err = skb_copy_bits(skb, 0, skb_tail_pointer(head), skb->len);
>> + if (err)
>> + return err;
>> + head->tail += skb->len;
>> + skb = skb->next;
>> + }
>> + kfree_skb_list(head->next);
>> + head->next = NULL;
>> + head->len += tlen;
>> + head->truesize = SKB_TRUESIZE(skb_end_offset(head));
>
> This is buggy : skb can have a destructor, like sock_wfree()
>
In current use-case it is never called for skb which is tied to
socket. But I agree it need to be fixed. I can call skb_orphan() to
fix it.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 2/2] vxlan: Eliminate dependency on UDP socket in transmit path
From: Tom Herbert @ 2015-01-20 22:53 UTC (permalink / raw)
To: Thomas Graf; +Cc: David Miller, Linux Netdev List
In-Reply-To: <20150120181335.GL20315@casper.infradead.org>
On Tue, Jan 20, 2015 at 10:13 AM, Thomas Graf <tgraf@suug.ch> wrote:
> On 01/20/15 at 09:29am, Tom Herbert wrote:
>> I didn't see any reason to preclude that, if it needs to be symmetric
>> in that case it can be forced at the configuration. Being able to
>> receive RCO but not have to send it to certain peers is important use
>> case. You may want to consider this also for GBP if there are cases
>> where we accept GBP from different peers, but only send it to certain
>> ones.
>
> I think asymmetric configurations are fine, in particular
> receive-only. I was reluctant to the send-only scenario initially
> as I would expect a VTEP sending RCO frames on UDP dport 8472 to
> also always be able to accept RCO frames on that port. I can't
> come up with any specific cases where this would lead to problems
> though so I have no objections.
>
> As for GBP, as processing of the policy group requires additional
> iptables or OVS rules anyway, such behaviour would be implemented
> in those rules by either ignoring the mark or dropping such frames.
It's risky from a protocol perspective to assume that sending
something will be properly ignored. Just because we're willing to
receive something, doesn't mean we necessarily want to send it--
that's the robustness principle :-)
^ permalink raw reply
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Tom Herbert @ 2015-01-20 23:06 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: David Miller, Linux Netdev List
In-Reply-To: <1421785536-19793-1-git-send-email-pshelar@nicira.com>
On Tue, Jan 20, 2015 at 12:25 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
> Following patch series adds support for Stateless Transport
> Tunneling protocol.
> STT uses TCP segmentation offload available in most of NIC. On
> packet xmit STT driver appends STT header along with TCP header
> to the packet. For GSO packet GSO parameters are set according
> to tunnel configuration and packet is handed over to networking
> stack. This allows use of segmentation offload available in NICs
>
> Netperf unidirectional test gives ~9.4 Gbits/s performance on 10Gbit
> NIC with 1500 byte MTU with two TCP streams.
>
Having packets marked TCP which really aren't TCP is a rather scary
prospect to deploy in a real data center (TCP is kind of an important
protocol ;-) ). Can you give some more motivation on this, more data
that shows what the benefits are and how this compares to equivalent
encapsulation protocols that implement GRO and GSO.
Thanks,
Tom
> The protocol is documented at
> http://www.ietf.org/archive/id/draft-davie-stt-06.txt
>
> I will send out OVS userspace patch on ovs-dev mailing list.
>
> Pravin B Shelar (3):
> skbuff: Add skb_list_linearize()
> net: Add STT tunneling protocol.
> openvswitch: Add support for STT tunneling.
>
> include/linux/skbuff.h | 2 +
> include/net/stt.h | 55 ++
> include/uapi/linux/openvswitch.h | 1 +
> net/core/skbuff.c | 35 +
> net/ipv4/Kconfig | 11 +
> net/ipv4/Makefile | 1 +
> net/ipv4/stt.c | 1386 ++++++++++++++++++++++++++++++++++++++
> net/openvswitch/Kconfig | 10 +
> net/openvswitch/Makefile | 1 +
> net/openvswitch/vport-stt.c | 214 ++++++
> 10 files changed, 1716 insertions(+)
> create mode 100644 include/net/stt.h
> create mode 100644 net/ipv4/stt.c
> create mode 100644 net/openvswitch/vport-stt.c
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-20 23:40 UTC (permalink / raw)
To: Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <874mrlu18e.fsf@nemi.mork.no>
> Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
> Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
>
> > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >
> > Add netlink directives and ndo entry to control VF multicast promiscuous mode.
> >
> > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC addresses
> > per VF. It means that we cannot assign over 30 IPv6 addresses to a single
> > VF interface on VM. We want thousands IPv6 addresses in VM.
> >
> > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > It enables all multicast packets are delivered to the target VF.
> >
> > This patch prepares to control that VF multicast promiscuous functionality.
>
> Adding a new hook for this seems over-complicated to me. And it still
> doesn't solve the real problems that
> a) the user has to know about this limit, and
> b) manually configure the feature
>
> Most of us, lacking the ability to imagine such arbitrary hardware
> limitations, will go through a few hours of frustrating debugging before
> we figure this one out...
>
> Why can't the ixgbevf driver just automatically signal the ixgbe driver
> to enable multicast promiscuous mode whenever the list grows past the
> limit?
I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
https://lkml.org/lkml/2014/11/27/269
The previous patch introduces API between ixgbe and ixgbevf driver to
enable multicast promiscuous mode, and ixgbevf enables it automatically
if the number of addresses is over than 30.
I got some comment and I would like to clarify the point, but there was no
answer.
That's the reason I submitted this patch.
Do you think a patch for the ixgbe/ixgbevf driver is preferred?
thanks,
Hiroshi
>
> I'd also like to note that this comment in
> drivers/net/ethernet/intel/ixgbevf/vf.c
> indicates that the author had some ideas about how more than 30
> addresses could/should be handled:
>
> static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> struct net_device *netdev)
> {
> struct netdev_hw_addr *ha;
> u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> u16 *vector_list = (u16 *)&msgbuf[1];
> u32 cnt, i;
>
> /* Each entry in the list uses 1 16 bit word. We have 30
> * 16 bit words available in our HW msg buffer (minus 1 for the
> * msg type). That's 30 hash values if we pack 'em right. If
> * there are more than 30 MC addresses to add then punt the
> * extras for now and then add code to handle more than 30 later.
> * It would be unusual for a server to request that many multi-cast
> * addresses except for in large enterprise network environments.
> */
>
>
>
> The last 2 lines of that comment are of course totally bogus and
> pointless and should be deleted in any case... It's obvious that 30
> multicast addresses is ridiculously low for lots of normal use cases.
>
>
> Bjørn
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH net-next 1/3] skbuff: Add skb_list_linearize()
From: Eric Dumazet @ 2015-01-20 23:51 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, netdev
In-Reply-To: <CALnjE+pDdvcSdQV83J_DLJwmcM8OsC94xDnH2mSSsxDCNnsztg@mail.gmail.com>
On Tue, 2015-01-20 at 14:46 -0800, Pravin Shelar wrote:
> In current use-case it is never called for skb which is tied to
> socket. But I agree it need to be fixed. I can call skb_orphan() to
> fix it.
I would do nothing at all, like skb_linearize()
^ permalink raw reply
* [PATCH] net: dsa: set parent of hwmon device
From: Vivien Didelot @ 2015-01-21 0:13 UTC (permalink / raw)
To: netdev
Cc: Vivien Didelot, David S . Miller, Guenter Roeck, linux-kernel,
kernel
Set the dsa device as the parent of the hwmon device, in order to link
the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
sysfs directory.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3731714..363102a 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
hname[j] = '\0';
scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
hname, index);
- ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
+ ds->hwmon_dev = hwmon_device_register_with_groups(parent,
ds->hwmon_name, ds, dsa_hwmon_groups);
if (IS_ERR(ds->hwmon_dev))
ds->hwmon_dev = NULL;
--
2.2.2
^ permalink raw reply related
* [PATCH] net: dsa: set slave MII bus PHY mask
From: Vivien Didelot @ 2015-01-21 0:13 UTC (permalink / raw)
To: netdev
Cc: Vivien Didelot, David S . Miller, Florian Fainelli, linux-kernel,
kernel
In-Reply-To: <1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com>
When registering a mdio bus, Linux assumes than every port has a PHY and tries
to scan it. If a switch port has no PHY registered, DSA will fail to register
the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
PHYs mask.
As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
registered PHYs for port 5 and port 6), with the following declared names:
static struct dsa_chip_data switch_cdata = {
[...]
.port_names[0] = "sw0",
.port_names[1] = "sw1",
.port_names[2] = "sw2",
.port_names[3] = "sw3",
.port_names[4] = "sw4",
.port_names[5] = "cpu",
};
DSA will fail to create the switch instance. With the PHY mask set for the
slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
be successfully created.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 515569f..589aafd 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
ds->index, ds->pd->sw_addr);
ds->slave_mii_bus->parent = ds->master_dev;
+ ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
}
--
2.2.2
^ permalink raw reply related
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Skidmore, Donald C @ 2015-01-21 0:26 UTC (permalink / raw)
To: Hiroshi Shimamoto, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E07B7C@BPXM14GP.gisp.nec.co.jp>
> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Tuesday, January 20, 2015 3:40 PM
> To: Bjørn Mork
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> Jong; linux-kernel@vger.kernel.org; Hayato Momma
> Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> mode control
>
> > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode
> > control
> >
> > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> >
> > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > >
> > > Add netlink directives and ndo entry to control VF multicast promiscuous
> mode.
> > >
> > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > addresses to a single VF interface on VM. We want thousands IPv6
> addresses in VM.
> > >
> > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > It enables all multicast packets are delivered to the target VF.
> > >
> > > This patch prepares to control that VF multicast promiscuous
> functionality.
> >
> > Adding a new hook for this seems over-complicated to me. And it still
> > doesn't solve the real problems that
> > a) the user has to know about this limit, and
> > b) manually configure the feature
> >
> > Most of us, lacking the ability to imagine such arbitrary hardware
> > limitations, will go through a few hours of frustrating debugging
> > before we figure this one out...
> >
> > Why can't the ixgbevf driver just automatically signal the ixgbe
> > driver to enable multicast promiscuous mode whenever the list grows
> > past the limit?
>
> I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> https://lkml.org/lkml/2014/11/27/269
>
> The previous patch introduces API between ixgbe and ixgbevf driver to
> enable multicast promiscuous mode, and ixgbevf enables it automatically if
> the number of addresses is over than 30.
I believe the issue is with allowing a VF to automatically enter Promiscuous Multicast without the PF's ok is concern over VM isolation. Of course that isolation, when it comes to multicast, is rather limited anyway given that our multicast filter uses only 12-bit of the address for a match. Still this (or doing it by default) would only open that up considerably more (all multicasts). I assume for your application you're not concerned, but are there other use cases that would worry about such things?
Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>
>
> I got some comment and I would like to clarify the point, but there was no
> answer.
> That's the reason I submitted this patch.
>
> Do you think a patch for the ixgbe/ixgbevf driver is preferred?
>
>
> thanks,
> Hiroshi
>
> >
> > I'd also like to note that this comment in
> > drivers/net/ethernet/intel/ixgbevf/vf.c
> > indicates that the author had some ideas about how more than 30
> > addresses could/should be handled:
> >
> > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > struct net_device *netdev)
> > {
> > struct netdev_hw_addr *ha;
> > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > u16 *vector_list = (u16 *)&msgbuf[1];
> > u32 cnt, i;
> >
> > /* Each entry in the list uses 1 16 bit word. We have 30
> > * 16 bit words available in our HW msg buffer (minus 1 for the
> > * msg type). That's 30 hash values if we pack 'em right. If
> > * there are more than 30 MC addresses to add then punt the
> > * extras for now and then add code to handle more than 30 later.
> > * It would be unusual for a server to request that many multi-cast
> > * addresses except for in large enterprise network environments.
> > */
> >
> >
> >
> > The last 2 lines of that comment are of course totally bogus and
> > pointless and should be deleted in any case... It's obvious that 30
> > multicast addresses is ridiculously low for lots of normal use cases.
> >
> >
> > Bjørn
^ permalink raw reply
* Re: [PATCH] net: dsa: set slave MII bus PHY mask
From: Florian Fainelli @ 2015-01-21 0:37 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel
In-Reply-To: <1421799212-2028-2-git-send-email-vivien.didelot@savoirfairelinux.com>
On 20/01/15 16:13, Vivien Didelot wrote:
> When registering a mdio bus, Linux assumes than every port has a PHY and tries
> to scan it. If a switch port has no PHY registered, DSA will fail to register
> the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
> PHYs mask.
>
> As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
> registered PHYs for port 5 and port 6), with the following declared names:
>
> static struct dsa_chip_data switch_cdata = {
> [...]
> .port_names[0] = "sw0",
> .port_names[1] = "sw1",
> .port_names[2] = "sw2",
> .port_names[3] = "sw3",
> .port_names[4] = "sw4",
> .port_names[5] = "cpu",
> };
>
> DSA will fail to create the switch instance. With the PHY mask set for the
> slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
> be successfully created.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/dsa/slave.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 515569f..589aafd 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
> snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
> ds->index, ds->pd->sw_addr);
> ds->slave_mii_bus->parent = ds->master_dev;
> + ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
> }
>
>
>
--
Florian
^ permalink raw reply
* [PATCH net-next 0/2] net: phy and dsa random fixes/cleanups
From: Florian Fainelli @ 2015-01-21 0:41 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
These two patches were already present as part of my attempt to make
DSA modules work properly, these are the only two "valid" patches at
this point which should not need any further rework.
Thanks!
Florian Fainelli (2):
net: phy: fixed: allow setting no update_link callback
net: dsa: bcm_sf2: factor interrupt disabling in a function
drivers/net/dsa/bcm_sf2.c | 24 ++++++++++++------------
drivers/net/phy/fixed_phy.c | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net-next 1/2] net: phy: fixed: allow setting no update_link callback
From: Florian Fainelli @ 2015-01-21 0:41 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1421800920-6281-1-git-send-email-f.fainelli@gmail.com>
fixed_phy_set_link_update() contains an early check against a NULL
callback pointer, which basically prevents us from removing any
previous callback we may have set. The users of the fp->link_update
callback deal with a NULL callback just fine, so we really want to allow
"removing" a link_update callback to avoid dangling callback pointers
during e.g: module removal.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/fixed_phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 3ad0e6e16c39..a08a3c78ba97 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -168,7 +168,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
struct fixed_mdio_bus *fmb = &platform_fmb;
struct fixed_phy *fp;
- if (!link_update || !phydev || !phydev->bus)
+ if (!phydev || !phydev->bus)
return -EINVAL;
list_for_each_entry(fp, &fmb->phys, node) {
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 2/2] net: dsa: bcm_sf2: factor interrupt disabling in a function
From: Florian Fainelli @ 2015-01-21 0:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1421800920-6281-1-git-send-email-f.fainelli@gmail.com>
Factor the interrupt disabling in a function: bcm_sf2_intr_disable()
since we are doing the same thing in the setup and suspend paths.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index feb29c4526f7..09f6b3cc1f66 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -400,6 +400,16 @@ static int bcm_sf2_sw_rst(struct bcm_sf2_priv *priv)
return 0;
}
+static void bcm_sf2_intr_disable(struct bcm_sf2_priv *priv)
+{
+ intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
+ intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
+ intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
+ intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
+ intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+}
+
static int bcm_sf2_sw_setup(struct dsa_switch *ds)
{
const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
@@ -440,12 +450,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
}
/* Disable all interrupts and request them */
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ bcm_sf2_intr_disable(priv);
ret = request_irq(priv->irq0, bcm_sf2_switch_0_isr, 0,
"switch_0", priv);
@@ -747,12 +752,7 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
struct bcm_sf2_priv *priv = ds_to_priv(ds);
unsigned int port;
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ bcm_sf2_intr_disable(priv);
/* Disable all ports physically present including the IMP
* port, the other ones have already been disabled during
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-21 1:07 UTC (permalink / raw)
To: Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <F6FB0E698C9B3143BDF729DF222866469129B7F5@ORSMSX110.amr.corp.intel.com>
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
>
>
> > -----Original Message-----
> > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > Sent: Tuesday, January 20, 2015 3:40 PM
> > To: Bjørn Mork
> > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> > Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> > mode control
> >
> > > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode
> > > control
> > >
> > > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> > >
> > > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > > >
> > > > Add netlink directives and ndo entry to control VF multicast promiscuous
> > mode.
> > > >
> > > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > > addresses to a single VF interface on VM. We want thousands IPv6
> > addresses in VM.
> > > >
> > > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > > It enables all multicast packets are delivered to the target VF.
> > > >
> > > > This patch prepares to control that VF multicast promiscuous
> > functionality.
> > >
> > > Adding a new hook for this seems over-complicated to me. And it still
> > > doesn't solve the real problems that
> > > a) the user has to know about this limit, and
> > > b) manually configure the feature
> > >
> > > Most of us, lacking the ability to imagine such arbitrary hardware
> > > limitations, will go through a few hours of frustrating debugging
> > > before we figure this one out...
> > >
> > > Why can't the ixgbevf driver just automatically signal the ixgbe
> > > driver to enable multicast promiscuous mode whenever the list grows
> > > past the limit?
> >
> > I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> > https://lkml.org/lkml/2014/11/27/269
> >
> > The previous patch introduces API between ixgbe and ixgbevf driver to
> > enable multicast promiscuous mode, and ixgbevf enables it automatically if
> > the number of addresses is over than 30.
>
> I believe the issue is with allowing a VF to automatically enter Promiscuous Multicast without the PF's ok is concern
So you mean that we should take care about enabling VF multicast promiscuous mode
in host side, right? The host allows multicast promiscuous and VF requests it too,
then enables VF multicast promiscuous mode.
So, what is preferred way to do in host do you think?
> over VM isolation. Of course that isolation, when it comes to multicast, is rather limited anyway given that our multicast
> filter uses only 12-bit of the address for a match. Still this (or doing it by default) would only open that up considerably
> more (all multicasts). I assume for your application you're not concerned, but are there other use cases that would worry
> about such things?
Sorry I couldn't catch the point.
What is the issue? I think there is no difference for the users who don't want
many multicast addresses in guest. In the current implementation, overflowed
multicast addresses silently discarded in ixgbevf. I believe there is no user
who want to use over 30 multicast addresses now. If VF multicast promiscuous mode
is enabled in certain VF, the behavior of other VFs is not changed.
thanks,
Hiroshi
>
> Thanks,
> -Don Skidmore <donald.c.skidmore@intel.com>
>
> >
> > I got some comment and I would like to clarify the point, but there was no
> > answer.
> > That's the reason I submitted this patch.
> >
> > Do you think a patch for the ixgbe/ixgbevf driver is preferred?
> >
> >
> > thanks,
> > Hiroshi
> >
> > >
> > > I'd also like to note that this comment in
> > > drivers/net/ethernet/intel/ixgbevf/vf.c
> > > indicates that the author had some ideas about how more than 30
> > > addresses could/should be handled:
> > >
> > > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > > struct net_device *netdev)
> > > {
> > > struct netdev_hw_addr *ha;
> > > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > > u16 *vector_list = (u16 *)&msgbuf[1];
> > > u32 cnt, i;
> > >
> > > /* Each entry in the list uses 1 16 bit word. We have 30
> > > * 16 bit words available in our HW msg buffer (minus 1 for the
> > > * msg type). That's 30 hash values if we pack 'em right. If
> > > * there are more than 30 MC addresses to add then punt the
> > > * extras for now and then add code to handle more than 30 later.
> > > * It would be unusual for a server to request that many multi-cast
> > > * addresses except for in large enterprise network environments.
> > > */
> > >
> > >
> > >
> > > The last 2 lines of that comment are of course totally bogus and
> > > pointless and should be deleted in any case... It's obvious that 30
> > > multicast addresses is ridiculously low for lots of normal use cases.
> > >
> > >
> > > Bjørn
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Skidmore, Donald C @ 2015-01-21 1:30 UTC (permalink / raw)
To: Hiroshi Shimamoto, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E07F09@BPXM14GP.gisp.nec.co.jp>
> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Tuesday, January 20, 2015 5:07 PM
> To: Skidmore, Donald C; Bjørn Mork
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> Jong; linux-kernel@vger.kernel.org; Hayato Momma
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> mode control
>
> > Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > promiscuous mode control
> >
> >
> >
> > > -----Original Message-----
> > > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > > Sent: Tuesday, January 20, 2015 3:40 PM
> > > To: Bjørn Mork
> > > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi,
> > > Sy Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > > Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > > promiscuous mode control
> > >
> > > > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous
> > > > mode control
> > > >
> > > > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> > > >
> > > > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > > > >
> > > > > Add netlink directives and ndo entry to control VF multicast
> > > > > promiscuous
> > > mode.
> > > > >
> > > > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > > > addresses to a single VF interface on VM. We want thousands IPv6
> > > addresses in VM.
> > > > >
> > > > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > > > It enables all multicast packets are delivered to the target VF.
> > > > >
> > > > > This patch prepares to control that VF multicast promiscuous
> > > functionality.
> > > >
> > > > Adding a new hook for this seems over-complicated to me. And it
> > > > still doesn't solve the real problems that
> > > > a) the user has to know about this limit, and
> > > > b) manually configure the feature
> > > >
> > > > Most of us, lacking the ability to imagine such arbitrary hardware
> > > > limitations, will go through a few hours of frustrating debugging
> > > > before we figure this one out...
> > > >
> > > > Why can't the ixgbevf driver just automatically signal the ixgbe
> > > > driver to enable multicast promiscuous mode whenever the list
> > > > grows past the limit?
> > >
> > > I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> > > https://lkml.org/lkml/2014/11/27/269
> > >
> > > The previous patch introduces API between ixgbe and ixgbevf driver
> > > to enable multicast promiscuous mode, and ixgbevf enables it
> > > automatically if the number of addresses is over than 30.
> >
> > I believe the issue is with allowing a VF to automatically enter
> > Promiscuous Multicast without the PF's ok is concern
>
> So you mean that we should take care about enabling VF multicast
> promiscuous mode in host side, right? The host allows multicast promiscuous
> and VF requests it too, then enables VF multicast promiscuous mode.
> So, what is preferred way to do in host do you think?
I think we are saying the same thing. I believe it would be fine if the VF requests this to happen (threw a mailbox message like you set up) and the PF will do it if the systems policy has been set up that way (as you did with the control mode).
This way the behavior (related to multicast) is the same as it has been, unless the system has been setup specifically to allow VF multicast promiscuous mode.
I know it's been mentioned that this is onerous on those who want this behavior to be automatic, but I don't see how else it could be done and take account for people that are concerned about allowing a (possibly untrusted) VM promoting itself in to multicast promiscuous mode.
>
> > over VM isolation. Of course that isolation, when it comes to multicast, is
> rather limited anyway given that our multicast
> > filter uses only 12-bit of the address for a match. Still this (or
> > doing it by default) would only open that up considerably more (all
> > multicasts). I assume for your application you're not concerned, but are
> there other use cases that would worry about such things?
>
> Sorry I couldn't catch the point.
>
> What is the issue? I think there is no difference for the users who don't want
> many multicast addresses in guest. In the current implementation,
> overflowed multicast addresses silently discarded in ixgbevf. I believe there
> is no user who want to use over 30 multicast addresses now. If VF multicast
> promiscuous mode is enabled in certain VF, the behavior of other VFs is not
> changed.
>
> thanks,
> Hiroshi
>
> >
> > Thanks,
> > -Don Skidmore <donald.c.skidmore@intel.com>
> >
> > >
> > > I got some comment and I would like to clarify the point, but there
> > > was no answer.
> > > That's the reason I submitted this patch.
> > >
> > > Do you think a patch for the ixgbe/ixgbevf driver is preferred?
> > >
> > >
> > > thanks,
> > > Hiroshi
> > >
> > > >
> > > > I'd also like to note that this comment in
> > > > drivers/net/ethernet/intel/ixgbevf/vf.c
> > > > indicates that the author had some ideas about how more than 30
> > > > addresses could/should be handled:
> > > >
> > > > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > > > struct net_device *netdev)
> > > > {
> > > > struct netdev_hw_addr *ha;
> > > > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > > > u16 *vector_list = (u16 *)&msgbuf[1];
> > > > u32 cnt, i;
> > > >
> > > > /* Each entry in the list uses 1 16 bit word. We have 30
> > > > * 16 bit words available in our HW msg buffer (minus 1 for the
> > > > * msg type). That's 30 hash values if we pack 'em right. If
> > > > * there are more than 30 MC addresses to add then punt the
> > > > * extras for now and then add code to handle more than 30 later.
> > > > * It would be unusual for a server to request that many multi-cast
> > > > * addresses except for in large enterprise network environments.
> > > > */
> > > >
> > > >
> > > >
> > > > The last 2 lines of that comment are of course totally bogus and
> > > > pointless and should be deleted in any case... It's obvious that
> > > > 30 multicast addresses is ridiculously low for lots of normal use cases.
> > > >
> > > >
> > > > Bjørn
^ permalink raw reply
* Re: [PATCH] net: dsa: set parent of hwmon device
From: Guenter Roeck @ 2015-01-21 2:48 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel
In-Reply-To: <1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com>
On 01/20/2015 04:13 PM, Vivien Didelot wrote:
> Set the dsa device as the parent of the hwmon device, in order to link
> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
> sysfs directory.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> net/dsa/dsa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..363102a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
> hname[j] = '\0';
> scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
> hname, index);
> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
> + ds->hwmon_dev = hwmon_device_register_with_groups(parent,
> ds->hwmon_name, ds, dsa_hwmon_groups);
> if (IS_ERR(ds->hwmon_dev))
> ds->hwmon_dev = NULL;
>
I'll have to look into this again; not sure if this works because there is
no 1:1 relationship between dsa device and hwmon device (or switch chip).
Guenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox