* Re: [PATCH net-next RFC 5/5] net-timestamp: tx timestamping default mode flag
From: Richard Cochran @ 2015-01-11 20:32 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, eric.dumazet, luto
In-Reply-To: <1420824719-28848-6-git-send-email-willemb@google.com>
On Fri, Jan 09, 2015 at 12:31:59PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> The number of timestamping points along the transmit path has grown,
> as have the options. Preferred behavior is to request timestamps with
> ID, without data (which enables batching) and for all supported
> timestamp points. Define a short option that enables all these
> defaults.
This "preferred behavior" is subjective, and it depends on the
application. I am sure it reflects your own interest, but for people
doing PTP over UDP or raw, it is a bit misleading.
I would drop this default and just let applications define their own.
Thanks,
Richard
^ permalink raw reply
* [PATCH v4 3/4] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2015-01-11 20:36 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150111201519.GC8855@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 | 612 ++++++++++++++++++++++++++++++---------
2 files changed, 487 insertions(+), 133 deletions(-)
** V4 Changelog:
- Use type-safe C methods instead of cpp macros
- Further clarify the code and comments on error events channel arbitration
- 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 0eb870b..da47d17 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 Corporation
*/
#include <linux/completion.h>
@@ -21,6 +23,15 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
+/* 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,
+};
+
#define MAX_TX_URBS 16
#define MAX_RX_URBS 4
#define START_TIMEOUT 1000 /* msecs */
@@ -30,8 +41,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 */
+/* Leaf USB devices */
#define KVASER_VENDOR_ID 0x0bfd
#define USB_LEAF_DEVEL_PRODUCT_ID 10
#define USB_LEAF_LITE_PRODUCT_ID 11
@@ -56,6 +68,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;
+}
+
+/* USBCANII 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 +103,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 +128,13 @@
#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_READ_CLOCK 30
+#define CMD_READ_CLOCK_REPLY 31
+
+#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 +144,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 +158,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,7 +182,7 @@
#define KVASER_CTRL_MODE_SELFRECEPTION 3
#define KVASER_CTRL_MODE_OFF 4
-/* log message */
+/* Extended CAN identifier flag */
#define KVASER_EXTENDED_FRAME BIT(31)
struct kvaser_msg_simple {
@@ -148,30 +193,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 +258,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 kvaser_msg_tx_acknowledge {
+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_header {
+ u8 channel;
+ u8 tid;
+};
+
+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 +349,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 +375,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,19 +392,57 @@ 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_error_summary {
+ 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 {
struct kvaser_usb_net_priv *priv;
u32 echo_index;
@@ -288,6 +458,7 @@ struct kvaser_usb {
u32 fw_version;
unsigned int nchannels;
+ enum kvaser_usb_family family;
bool rxinitdone;
void *rxbuf[MAX_RX_URBS];
@@ -311,6 +482,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),
@@ -360,6 +532,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);
@@ -463,7 +646,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;
}
@@ -482,7 +672,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;
@@ -496,8 +688,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,
@@ -616,53 +810,24 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
}
static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
- const struct kvaser_msg *msg)
+ struct kvaser_error_summary *es)
{
struct can_frame *cf;
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;
-
- 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;
- 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];
- 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;
- break;
- default:
- dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
- msg->id);
- 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;
}
@@ -675,9 +840,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
new_state = priv->can.state;
- netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
+ netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->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++;
@@ -687,12 +852,12 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
netif_carrier_off(priv->netdev);
new_state = CAN_STATE_BUS_OFF;
- } else if (status & M16C_STATE_BUS_PASSIVE) {
+ } else if (es->status & M16C_STATE_BUS_PASSIVE) {
if (priv->can.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
@@ -703,13 +868,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
new_state = CAN_STATE_ERROR_PASSIVE;
- }
-
- if (status == M16C_STATE_BUS_ERROR) {
+ } else if (es->status & M16C_STATE_BUS_ERROR) {
if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((txerr >= 96) || (rxerr >= 96))) {
+ ((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;
@@ -723,7 +886,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
}
- if (!status) {
+ if (!es->status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
@@ -739,34 +902,48 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
priv->can.can_stats.restarts++;
}
- if (error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
-
- cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
-
- if (error_factor & M16C_EF_ACKE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- if (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)
- cf->data[2] |= CAN_ERR_PROT_FORM;
- if (error_factor & M16C_EF_STFE)
- cf->data[2] |= CAN_ERR_PROT_STUFF;
- if (error_factor & M16C_EF_BITE0)
- cf->data[2] |= CAN_ERR_PROT_BIT0;
- if (error_factor & M16C_EF_BITE1)
- cf->data[2] |= CAN_ERR_PROT_BIT1;
- if (error_factor & M16C_EF_TRE)
- cf->data[2] |= CAN_ERR_PROT_TX;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ if (es->leaf.error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+
+ 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_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++;
+ cf->can_id |= CAN_ERR_BUSERROR;
+ }
+ break;
}
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
+ cf->data[6] = es->txerr;
+ cf->data[7] = es->rxerr;
- priv->bec.txerr = txerr;
- priv->bec.rxerr = rxerr;
+ priv->bec.txerr = es->txerr;
+ priv->bec.rxerr = es->rxerr;
priv->can.state = new_state;
@@ -775,6 +952,124 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
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_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_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 does not support more than 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_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)
{
@@ -782,16 +1077,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++;
@@ -817,7 +1112,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;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -828,19 +1124,32 @@ 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;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
return;
}
@@ -850,38 +1159,37 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
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);
}
@@ -944,21 +1252,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;
}
@@ -1178,7 +1500,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)
@@ -1286,6 +1608,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;
+ uint8_t *msg_tx_can_flags;
if (can_dropped_invalid_skb(netdev, skb))
return NETDEV_TX_OK;
@@ -1307,9 +1630,23 @@ 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;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ goto releasebuf;
+ }
+
+ *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;
@@ -1327,7 +1664,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) {
@@ -1596,6 +1933,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 v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT
From: Ahmed S. Darwish @ 2015-01-11 20:45 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Greg Kroah-Hartman, Linux-USB, Linux-CAN, netdev, LKML
In-Reply-To: <20150111203612.GA8999@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
(This is a draft patch, I'm not sure if this fixes the USB
bug or only its psymptom. Feedback from the linux-usb folks
is really appreciated.)
When plugging the Kvaser USB/CAN dongle the first time, everything
works as expected and all of the transfers from and to the USB
device succeeds.
Meanwhile, after unplugging the device and plugging it again, the
first bulk transfer _always_ returns an -ETIMEDOUT. The following
behaviour was observied:
- Setting higher timeout values for the first bulk transfer never
solved the issue.
- Unloading, then loading, our kvaser_usb module in question
__always__ solved the issue.
- Checking first bulk transfer status, and retry the transfer
again in case of an -ETIMEDOUT also __always__ solved the issue.
This is what the patch below does.
- In the testing done so far, this issue appears only on laptops
but never on PCs (possibly power related?)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index da47d17..5925637 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1927,7 +1927,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)
@@ -1956,7 +1956,16 @@ 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 USBCAN 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.
+ */
+ while (retry--) {
+ err = kvaser_usb_get_software_info(dev);
+ if (err != -ETIMEDOUT)
+ break;
+ }
if (err) {
dev_err(&intf->dev,
"Cannot get software infos, error %d\n", err);
--
1.9.1
^ permalink raw reply related
* [PATCH v4 01/04] can: kvaser_usb: Don't dereference skb after a netif_rx()
From: Ahmed S. Darwish @ 2015-01-11 20:49 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150111201116.GB8855@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
We should not touch the packet after a netif_rx: it might
get freed behind our back.
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 | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
(Resend, fix the garbled subject line. Sorry)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index cc7bfc0..c32cd61 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -520,10 +520,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
skb = alloc_can_err_skb(priv->netdev, &cf);
if (skb) {
cf->can_id |= CAN_ERR_RESTARTED;
- netif_rx(skb);
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
} else {
netdev_err(priv->netdev,
"No memory left for err_skb\n");
@@ -770,10 +770,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
priv->can.state = new_state;
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -805,10 +804,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
stats->rx_over_errors++;
stats->rx_errors++;
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
}
@@ -887,10 +885,9 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
cf->can_dlc);
}
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT
From: Marc Kleine-Budde @ 2015-01-11 20:51 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger
Cc: Greg Kroah-Hartman, Linux-USB, Linux-CAN, netdev, LKML
In-Reply-To: <20150111204508.GB8999@linux>
[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]
On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> (This is a draft patch, I'm not sure if this fixes the USB
> bug or only its psymptom. Feedback from the linux-usb folks
> is really appreciated.)
>
> When plugging the Kvaser USB/CAN dongle the first time, everything
> works as expected and all of the transfers from and to the USB
> device succeeds.
>
> Meanwhile, after unplugging the device and plugging it again, the
> first bulk transfer _always_ returns an -ETIMEDOUT. The following
> behaviour was observied:
>
> - Setting higher timeout values for the first bulk transfer never
> solved the issue.
>
> - Unloading, then loading, our kvaser_usb module in question
> __always__ solved the issue.
>
> - Checking first bulk transfer status, and retry the transfer
> again in case of an -ETIMEDOUT also __always__ solved the issue.
> This is what the patch below does.
>
> - In the testing done so far, this issue appears only on laptops
> but never on PCs (possibly power related?)
>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Does this patch apply apply between 3 and 4? If not, please re-arrange
the series. As this is a bug fix, patches 1, 2 and 4 will go via
net/master, 3 will go via net-next/master.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [RFC] Make predictable/persistent network interface names more handy
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen
Since the Linux distribution of my choice makes use of
predictable network interface names[0] my USB gadgets
are no longer usb0 but enp0s29u1u2. Same for all other network devices.
While I can fully understand that this new naming scheme makes sense
for a lot of people and makes their work easier it does not really work for me.
My brain is not able to remember that my Beaglebone's USB-Ethernet
is now enp0s29u1u2. Even after looking at the output of ifconfig
I have to copy&paste the interface name.
Instead of just disabling the feature I thought about
a generic solution which satisfies both needs.
For block devices we also have predictable device names,
udev creates a symlink to the kernel device.
This works very good and reliable.
My idea is to use the network device alias as symlink.
Such that we can have both the easy to use kernel name and
the predictable/persistent name from udev.
systemd/udev could store the original kernel interface name
as alias after renaming the interface.
Existing users would not notice but one can still use the kernel name.
So I can use tcpdump -i usb0 _and_ tcpdump -i enp0s29u1u2.
This patch series implements my idea.
I'd love to get some feedback!
Patch 1/3 exposes the interfaces alias for general userspace usage, i.e. that ifconfig <alias> works.
Of course you can only use the first 15 chars of an alias.
In-kernel users of interface names need also an update, patch 2/3 updates x_tables.
I'm sure there is more todo, i.e. nftables.
We could also define that netfilter will never use the alias but this needs documented cleary.
Patch 3/3 is a cleanup in continuation of 2/3.
[0] http://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
Thanks,
//richard
git://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git netalias
[PATCH 1/3] net: Make interface aliases available for general usage
[PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
[PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
include/linux/netdevice.h | 1 +
include/linux/netfilter/x_tables.h | 41 +++++++++++++++++++++++++++---
include/net/net_namespace.h | 1 +
net/core/dev.c | 52 ++++++++++++++++++++++++++++++++++++++
net/ipv4/netfilter/arp_tables.c | 37 ++++-----------------------
net/ipv4/netfilter/ip_tables.c | 15 ++++-------
net/ipv6/netfilter/ip6_tables.c | 18 +++++--------
net/netfilter/xt_physdev.c | 9 ++-----
8 files changed, 110 insertions(+), 64 deletions(-)
^ permalink raw reply
* [PATCH 1/3] net: Make interface aliases available for general usage
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, Richard Weinberger
In-Reply-To: <1421009571-5279-1-git-send-email-richard@nod.at>
Allow interface aliases to be used as regular interfaces.
Such that a command sequence like this one works:
$ ip l set eth0 alias internet
$ ip a s internet
$ tcpdump -n -i internet
Signed-off-by: Richard Weinberger <richard@nod.at>
---
include/linux/netdevice.h | 1 +
include/net/net_namespace.h | 1 +
net/core/dev.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 679e6e9..e00b4e2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1493,6 +1493,7 @@ struct net_device {
char name[IFNAMSIZ];
struct hlist_node name_hlist;
char *ifalias;
+ struct hlist_node ifalias_hlist;
/*
* I/O specific fields
* FIXME: Merge these and struct ifmap into one
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 2e8756b8..9fa0939 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -76,6 +76,7 @@ struct net {
struct list_head dev_base_head;
struct hlist_head *dev_name_head;
struct hlist_head *dev_index_head;
+ struct hlist_head *dev_ifalias_head;
unsigned int dev_base_seq; /* protected by rtnl_mutex */
int ifindex;
unsigned int dev_unreg_count;
diff --git a/net/core/dev.c b/net/core/dev.c
index 683d493..2551b03 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -202,6 +202,14 @@ static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
return &net->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
}
+static inline struct hlist_head *dev_ifalias_hash(struct net *net,
+ const char *ifalias)
+{
+ unsigned int hash = full_name_hash(ifalias, strnlen(ifalias, IFALIASZ));
+
+ return &net->dev_ifalias_head[hash_32(hash, NETDEV_HASHBITS)];
+}
+
static inline void rps_lock(struct softnet_data *sd)
{
#ifdef CONFIG_RPS
@@ -228,6 +236,9 @@ static void list_netdevice(struct net_device *dev)
hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
hlist_add_head_rcu(&dev->index_hlist,
dev_index_hash(net, dev->ifindex));
+ if (dev->ifalias)
+ hlist_add_head_rcu(&dev->ifalias_hlist,
+ dev_ifalias_hash(net, dev->ifalias));
write_unlock_bh(&dev_base_lock);
dev_base_seq_inc(net);
@@ -245,6 +256,8 @@ static void unlist_netdevice(struct net_device *dev)
list_del_rcu(&dev->dev_list);
hlist_del_rcu(&dev->name_hlist);
hlist_del_rcu(&dev->index_hlist);
+ if (dev->ifalias)
+ hlist_del_rcu(&dev->ifalias_hlist);
write_unlock_bh(&dev_base_lock);
dev_base_seq_inc(dev_net(dev));
@@ -679,6 +692,11 @@ struct net_device *__dev_get_by_name(struct net *net, const char *name)
if (!strncmp(dev->name, name, IFNAMSIZ))
return dev;
+ head = dev_ifalias_hash(net, name);
+ hlist_for_each_entry(dev, head, ifalias_hlist)
+ if (dev->ifalias && !strncmp(dev->ifalias, name, IFALIASZ))
+ return dev;
+
return NULL;
}
EXPORT_SYMBOL(__dev_get_by_name);
@@ -704,6 +722,11 @@ struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
if (!strncmp(dev->name, name, IFNAMSIZ))
return dev;
+ head = dev_ifalias_hash(net, name);
+ hlist_for_each_entry_rcu(dev, head, ifalias_hlist)
+ if (dev->ifalias && !strncmp(dev->ifalias, name, IFALIASZ))
+ return dev;
+
return NULL;
}
EXPORT_SYMBOL(dev_get_by_name_rcu);
@@ -1169,6 +1192,20 @@ rollback:
return err;
}
+static void __hlist_del_alias(struct net_device *dev)
+{
+ write_lock_bh(&dev_base_lock);
+ hlist_del_rcu(&dev->ifalias_hlist);
+ write_unlock_bh(&dev_base_lock);
+}
+
+static void __hlist_add_alias(struct net_device *dev)
+{
+ write_lock_bh(&dev_base_lock);
+ hlist_add_head_rcu(&dev->ifalias_hlist, dev_ifalias_hash(dev_net(dev), dev->ifalias));
+ write_unlock_bh(&dev_base_lock);
+}
+
/**
* dev_set_alias - change ifalias of a device
* @dev: device
@@ -1189,15 +1226,24 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
if (!len) {
kfree(dev->ifalias);
dev->ifalias = NULL;
+ __hlist_del_alias(dev);
return 0;
}
new_ifalias = krealloc(dev->ifalias, len + 1, GFP_KERNEL);
if (!new_ifalias)
return -ENOMEM;
+
+ if (dev->ifalias) {
+ __hlist_del_alias(dev);
+ synchronize_rcu();
+ }
+
dev->ifalias = new_ifalias;
strlcpy(dev->ifalias, alias, len+1);
+ __hlist_add_alias(dev);
+
return len;
}
@@ -7150,8 +7196,14 @@ static int __net_init netdev_init(struct net *net)
if (net->dev_index_head == NULL)
goto err_idx;
+ net->dev_ifalias_head = netdev_create_hash();
+ if (net->dev_ifalias_head == NULL)
+ goto err_alias;
+
return 0;
+err_alias:
+ kfree(net->dev_index_head);
err_idx:
kfree(net->dev_name_head);
err_name:
--
1.8.4.5
^ permalink raw reply related
* [PATCH 2/3] x_tables: Use also dev->ifalias for interface matching
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, Richard Weinberger
In-Reply-To: <1421009571-5279-1-git-send-email-richard@nod.at>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
include/linux/netfilter/x_tables.h | 22 ++++++++++++++++++++++
net/ipv4/netfilter/arp_tables.c | 28 +++++++++++++++++-----------
net/ipv4/netfilter/ip_tables.c | 15 +++++----------
net/ipv6/netfilter/ip6_tables.c | 18 +++++++-----------
net/netfilter/xt_physdev.c | 9 ++-------
5 files changed, 53 insertions(+), 39 deletions(-)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index a3e215b..15bda23 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -351,6 +351,28 @@ static inline unsigned long ifname_compare_aligned(const char *_a,
return ret;
}
+/*
+ * A wrapper around ifname_compare_aligned() to match against dev->name and
+ * dev->ifalias.
+ */
+static inline unsigned long ifname_compare_all(const struct net_device *dev,
+ const char *name,
+ const char *mask)
+{
+ unsigned long res = 0;
+
+ if (!dev)
+ goto out;
+
+ res = ifname_compare_aligned(dev->name, name, mask);
+ if (unlikely(dev->ifalias && res))
+ res = ifname_compare_aligned(dev->ifalias, name, mask);
+
+out:
+ return res;
+}
+
+
struct nf_hook_ops *xt_hook_link(const struct xt_table *, nf_hookfn *);
void xt_hook_unlink(const struct xt_table *, struct nf_hook_ops *);
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..457d4ed 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -81,19 +81,30 @@ static inline int arp_devaddr_compare(const struct arpt_devaddr_info *ap,
* Some arches dont care, unrolling the loop is a win on them.
* For other arches, we only have a 16bit alignement.
*/
-static unsigned long ifname_compare(const char *_a, const char *_b, const char *_mask)
+static unsigned long ifname_compare(const struct net_device *dev,
+ const char *_b, const char *_mask)
{
#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
- unsigned long ret = ifname_compare_aligned(_a, _b, _mask);
+ unsigned long ret = ifname_compare_all(dev, _b, _mask);
#else
unsigned long ret = 0;
- const u16 *a = (const u16 *)_a;
+ const u16 *a = (const u16 *)dev->name;
const u16 *b = (const u16 *)_b;
const u16 *mask = (const u16 *)_mask;
int i;
for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
ret |= (a[i] ^ b[i]) & mask[i];
+
+ if (likely(!(dev->ifalias && ret)))
+ goto out;
+
+ ret = 0;
+ a = (const u16 *)dev->ifalias;
+ for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
+ ret |= (a[i] ^ b[i]) & mask[i];
+
+out:
#endif
return ret;
}
@@ -101,8 +112,8 @@ static unsigned long ifname_compare(const char *_a, const char *_b, const char *
/* Returns whether packet matches rule or not. */
static inline int arp_packet_match(const struct arphdr *arphdr,
struct net_device *dev,
- const char *indev,
- const char *outdev,
+ const struct net_device *indev,
+ const struct net_device *outdev,
const struct arpt_arp *arpinfo)
{
const char *arpptr = (char *)(arphdr + 1);
@@ -252,11 +263,9 @@ unsigned int arpt_do_table(struct sk_buff *skb,
const struct net_device *out,
struct xt_table *table)
{
- static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
unsigned int verdict = NF_DROP;
const struct arphdr *arp;
struct arpt_entry *e, *back;
- const char *indev, *outdev;
void *table_base;
const struct xt_table_info *private;
struct xt_action_param acpar;
@@ -265,9 +274,6 @@ unsigned int arpt_do_table(struct sk_buff *skb,
if (!pskb_may_pull(skb, arp_hdr_len(skb->dev)))
return NF_DROP;
- indev = in ? in->name : nulldevname;
- outdev = out ? out->name : nulldevname;
-
local_bh_disable();
addend = xt_write_recseq_begin();
private = table->private;
@@ -291,7 +297,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
do {
const struct xt_entry_target *t;
- if (!arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
+ if (!arp_packet_match(arp, skb->dev, in, out, &e->arp)) {
e = arpt_next_entry(e);
continue;
}
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..87df9ef 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -73,8 +73,8 @@ EXPORT_SYMBOL_GPL(ipt_alloc_initial_table);
/* Performance critical - called for every packet */
static inline bool
ip_packet_match(const struct iphdr *ip,
- const char *indev,
- const char *outdev,
+ const struct net_device *indev,
+ const struct net_device *outdev,
const struct ipt_ip *ipinfo,
int isfrag)
{
@@ -97,7 +97,7 @@ ip_packet_match(const struct iphdr *ip,
return false;
}
- ret = ifname_compare_aligned(indev, ipinfo->iniface, ipinfo->iniface_mask);
+ ret = ifname_compare_all(indev, ipinfo->iniface, ipinfo->iniface_mask);
if (FWINV(ret != 0, IPT_INV_VIA_IN)) {
dprintf("VIA in mismatch (%s vs %s).%s\n",
@@ -106,7 +106,7 @@ ip_packet_match(const struct iphdr *ip,
return false;
}
- ret = ifname_compare_aligned(outdev, ipinfo->outiface, ipinfo->outiface_mask);
+ ret = ifname_compare_all(outdev, ipinfo->outiface, ipinfo->outiface_mask);
if (FWINV(ret != 0, IPT_INV_VIA_OUT)) {
dprintf("VIA out mismatch (%s vs %s).%s\n",
@@ -292,11 +292,9 @@ ipt_do_table(struct sk_buff *skb,
const struct net_device *out,
struct xt_table *table)
{
- static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
const struct iphdr *ip;
/* Initializing verdict to NF_DROP keeps gcc happy. */
unsigned int verdict = NF_DROP;
- const char *indev, *outdev;
const void *table_base;
struct ipt_entry *e, **jumpstack;
unsigned int *stackptr, origptr, cpu;
@@ -306,8 +304,6 @@ ipt_do_table(struct sk_buff *skb,
/* Initialization */
ip = ip_hdr(skb);
- indev = in ? in->name : nulldevname;
- outdev = out ? out->name : nulldevname;
/* We handle fragments by dealing with the first fragment as
* if it was a normal packet. All other fragments are treated
* normally, except that they will NEVER match rules that ask
@@ -348,8 +344,7 @@ ipt_do_table(struct sk_buff *skb,
const struct xt_entry_match *ematch;
IP_NF_ASSERT(e);
- if (!ip_packet_match(ip, indev, outdev,
- &e->ip, acpar.fragoff)) {
+ if (!ip_packet_match(ip, in, out, &e->ip, acpar.fragoff)) {
no_match:
e = ipt_next_entry(e);
continue;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e080fbb..9ed5d70 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -83,8 +83,8 @@ EXPORT_SYMBOL_GPL(ip6t_alloc_initial_table);
/* Performance critical - called for every packet */
static inline bool
ip6_packet_match(const struct sk_buff *skb,
- const char *indev,
- const char *outdev,
+ const struct net_device *indev,
+ const struct net_device *outdev,
const struct ip6t_ip6 *ip6info,
unsigned int *protoff,
int *fragoff, bool *hotdrop)
@@ -109,7 +109,7 @@ ip6_packet_match(const struct sk_buff *skb,
return false;
}
- ret = ifname_compare_aligned(indev, ip6info->iniface, ip6info->iniface_mask);
+ ret = ifname_compare_all(indev, ip6info->iniface, ip6info->iniface_mask);
if (FWINV(ret != 0, IP6T_INV_VIA_IN)) {
dprintf("VIA in mismatch (%s vs %s).%s\n",
@@ -118,7 +118,7 @@ ip6_packet_match(const struct sk_buff *skb,
return false;
}
- ret = ifname_compare_aligned(outdev, ip6info->outiface, ip6info->outiface_mask);
+ ret = ifname_compare_all(outdev, ip6info->outiface, ip6info->outiface_mask);
if (FWINV(ret != 0, IP6T_INV_VIA_OUT)) {
dprintf("VIA out mismatch (%s vs %s).%s\n",
@@ -318,10 +318,8 @@ ip6t_do_table(struct sk_buff *skb,
const struct net_device *out,
struct xt_table *table)
{
- static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
/* Initializing verdict to NF_DROP keeps gcc happy. */
unsigned int verdict = NF_DROP;
- const char *indev, *outdev;
const void *table_base;
struct ip6t_entry *e, **jumpstack;
unsigned int *stackptr, origptr, cpu;
@@ -329,10 +327,8 @@ ip6t_do_table(struct sk_buff *skb,
struct xt_action_param acpar;
unsigned int addend;
- /* Initialization */
- indev = in ? in->name : nulldevname;
- outdev = out ? out->name : nulldevname;
- /* We handle fragments by dealing with the first fragment as
+ /* Initialization:
+ * We handle fragments by dealing with the first fragment as
* if it was a normal packet. All other fragments are treated
* normally, except that they will NEVER match rules that ask
* things we don't know, ie. tcp syn flag or ports). If the
@@ -368,7 +364,7 @@ ip6t_do_table(struct sk_buff *skb,
IP_NF_ASSERT(e);
acpar.thoff = 0;
- if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
+ if (!ip6_packet_match(skb, in, out, &e->ipv6,
&acpar.thoff, &acpar.fragoff, &acpar.hotdrop)) {
no_match:
e = ip6t_next_entry(e);
diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c
index f440f57..8d2ee7d 100644
--- a/net/netfilter/xt_physdev.c
+++ b/net/netfilter/xt_physdev.c
@@ -25,10 +25,8 @@ MODULE_ALIAS("ip6t_physdev");
static bool
physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
{
- static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
const struct xt_physdev_info *info = par->matchinfo;
unsigned long ret;
- const char *indev, *outdev;
const struct nf_bridge_info *nf_bridge;
/* Not a bridged IP packet or no info available yet:
@@ -68,8 +66,7 @@ physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
if (!(info->bitmask & XT_PHYSDEV_OP_IN))
goto match_outdev;
- indev = nf_bridge->physindev ? nf_bridge->physindev->name : nulldevname;
- ret = ifname_compare_aligned(indev, info->physindev, info->in_mask);
+ ret = ifname_compare_all(nf_bridge->physindev, info->physindev, info->in_mask);
if (!ret ^ !(info->invert & XT_PHYSDEV_OP_IN))
return false;
@@ -77,9 +74,7 @@ physdev_mt(const struct sk_buff *skb, struct xt_action_param *par)
match_outdev:
if (!(info->bitmask & XT_PHYSDEV_OP_OUT))
return true;
- outdev = nf_bridge->physoutdev ?
- nf_bridge->physoutdev->name : nulldevname;
- ret = ifname_compare_aligned(outdev, info->physoutdev, info->out_mask);
+ ret = ifname_compare_all(nf_bridge->physoutdev, info->physoutdev, info->out_mask);
return (!!ret ^ !(info->invert & XT_PHYSDEV_OP_OUT));
}
--
1.8.4.5
^ permalink raw reply related
* [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Richard Weinberger @ 2015-01-11 20:52 UTC (permalink / raw)
To: davem
Cc: coreteam, netfilter-devel, linux-kernel, netdev, bhutchings,
john.fastabend, herbert, vyasevic, jiri, vfalico, therbert,
edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber, pablo, kay,
stephen, Richard Weinberger
In-Reply-To: <1421009571-5279-1-git-send-email-richard@nod.at>
arp_tables.c has a 16bit aligment ifname_compare(), factor
it out to use it for all tables.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
include/linux/netfilter/x_tables.h | 25 ++++++++++++++++++-------
net/ipv4/netfilter/arp_tables.c | 37 ++-----------------------------------
2 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 15bda23..26dddc1 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
/*
* This helper is performance critical and must be inlined
*/
-static inline unsigned long ifname_compare_aligned(const char *_a,
- const char *_b,
- const char *_mask)
+static inline unsigned long ifname_compare(const char *_a,
+ const char *_b,
+ const char *_mask)
{
+ unsigned long ret;
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
const unsigned long *a = (const unsigned long *)_a;
const unsigned long *b = (const unsigned long *)_b;
const unsigned long *mask = (const unsigned long *)_mask;
- unsigned long ret;
ret = (a[0] ^ b[0]) & mask[0];
if (IFNAMSIZ > sizeof(unsigned long))
@@ -348,11 +349,21 @@ static inline unsigned long ifname_compare_aligned(const char *_a,
if (IFNAMSIZ > 3 * sizeof(unsigned long))
ret |= (a[3] ^ b[3]) & mask[3];
BUILD_BUG_ON(IFNAMSIZ > 4 * sizeof(unsigned long));
+#else
+ const u16 *a = (const u16 *)_a;
+ const u16 *b = (const u16 *)_b;
+ const u16 *mask = (const u16 *)_mask;
+ int i;
+
+ ret = 0;
+ for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
+ ret |= (a[i] ^ b[i]) & mask[i];
+#endif
return ret;
}
/*
- * A wrapper around ifname_compare_aligned() to match against dev->name and
+ * A wrapper around ifname_compare() to match against dev->name and
* dev->ifalias.
*/
static inline unsigned long ifname_compare_all(const struct net_device *dev,
@@ -364,9 +375,9 @@ static inline unsigned long ifname_compare_all(const struct net_device *dev,
if (!dev)
goto out;
- res = ifname_compare_aligned(dev->name, name, mask);
+ res = ifname_compare(dev->name, name, mask);
if (unlikely(dev->ifalias && res))
- res = ifname_compare_aligned(dev->ifalias, name, mask);
+ res = ifname_compare(dev->ifalias, name, mask);
out:
return res;
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 457d4ed..c978691 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -76,39 +76,6 @@ static inline int arp_devaddr_compare(const struct arpt_devaddr_info *ap,
return ret != 0;
}
-/*
- * Unfortunately, _b and _mask are not aligned to an int (or long int)
- * Some arches dont care, unrolling the loop is a win on them.
- * For other arches, we only have a 16bit alignement.
- */
-static unsigned long ifname_compare(const struct net_device *dev,
- const char *_b, const char *_mask)
-{
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
- unsigned long ret = ifname_compare_all(dev, _b, _mask);
-#else
- unsigned long ret = 0;
- const u16 *a = (const u16 *)dev->name;
- const u16 *b = (const u16 *)_b;
- const u16 *mask = (const u16 *)_mask;
- int i;
-
- for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
- ret |= (a[i] ^ b[i]) & mask[i];
-
- if (likely(!(dev->ifalias && ret)))
- goto out;
-
- ret = 0;
- a = (const u16 *)dev->ifalias;
- for (i = 0; i < IFNAMSIZ/sizeof(u16); i++)
- ret |= (a[i] ^ b[i]) & mask[i];
-
-out:
-#endif
- return ret;
-}
-
/* Returns whether packet matches rule or not. */
static inline int arp_packet_match(const struct arphdr *arphdr,
struct net_device *dev,
@@ -192,7 +159,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
}
/* Look for ifname matches. */
- ret = ifname_compare(indev, arpinfo->iniface, arpinfo->iniface_mask);
+ ret = ifname_compare_all(indev, arpinfo->iniface, arpinfo->iniface_mask);
if (FWINV(ret != 0, ARPT_INV_VIA_IN)) {
dprintf("VIA in mismatch (%s vs %s).%s\n",
@@ -201,7 +168,7 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
return 0;
}
- ret = ifname_compare(outdev, arpinfo->outiface, arpinfo->outiface_mask);
+ ret = ifname_compare_all(outdev, arpinfo->outiface, arpinfo->outiface_mask);
if (FWINV(ret != 0, ARPT_INV_VIA_OUT)) {
dprintf("VIA out mismatch (%s vs %s).%s\n",
--
1.8.4.5
^ permalink raw reply related
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Joe Perches @ 2015-01-11 20:59 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <1421009571-5279-4-git-send-email-richard@nod.at>
On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
> arp_tables.c has a 16bit aligment ifname_compare(), factor
> it out to use it for all tables.
[]
> diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
[]
> @@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
> /*
> * This helper is performance critical and must be inlined
> */
> -static inline unsigned long ifname_compare_aligned(const char *_a,
> - const char *_b,
> - const char *_mask)
> +static inline unsigned long ifname_compare(const char *_a,
> + const char *_b,
> + const char *_mask)
Perhaps this would be better as bool ifname_compare
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Richard Weinberger @ 2015-01-11 21:02 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <1421009969.9233.5.camel@perches.com>
Am 11.01.2015 um 21:59 schrieb Joe Perches:
> On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
>> arp_tables.c has a 16bit aligment ifname_compare(), factor
>> it out to use it for all tables.
> []
>> diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
> []
>> @@ -331,14 +331,15 @@ static inline void xt_write_recseq_end(unsigned int addend)
>> /*
>> * This helper is performance critical and must be inlined
>> */
>> -static inline unsigned long ifname_compare_aligned(const char *_a,
>> - const char *_b,
>> - const char *_mask)
>> +static inline unsigned long ifname_compare(const char *_a,
>> + const char *_b,
>> + const char *_mask)
>
> Perhaps this would be better as bool ifname_compare
Let's discuss the whole concept first, then we can go to bikeshedding mode.
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Joe Perches @ 2015-01-11 21:14 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <54B2E4DD.90200@nod.at>
On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
> Am 11.01.2015 um 21:59 schrieb Joe Perches:
> > On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
> >> arp_tables.c has a 16bit aligment ifname_compare(), factor
> >> it out to use it for all tables.
[]
> > Perhaps this would be better as bool ifname_compare
> Let's discuss the whole concept first
The concept seems obvious enough
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Richard Weinberger @ 2015-01-11 21:30 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <1421010853.9233.7.camel@perches.com>
Am 11.01.2015 um 22:14 schrieb Joe Perches:
> On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
>> Am 11.01.2015 um 21:59 schrieb Joe Perches:
>>> On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
>>>> arp_tables.c has a 16bit aligment ifname_compare(), factor
>>>> it out to use it for all tables.
> []
>>> Perhaps this would be better as bool ifname_compare
>> Let's discuss the whole concept first
>
> The concept seems obvious enough
Anyway, I agree with Linus wrt. bool.
https://lkml.org/lkml/2013/8/31/138
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Daniel Borkmann @ 2015-01-11 21:38 UTC (permalink / raw)
To: Christoph Jaeger; +Cc: davem, willemb, edumazet, netdev, linux-kernel
In-Reply-To: <1420999276-28225-1-git-send-email-cj@linux.com>
On 01/11/2015 07:01 PM, Christoph Jaeger wrote:
> Due to a misplaced parenthesis, the expression
>
> (unlikely(offset) < 0),
>
> which expands to
>
> (__builtin_expect(!!(offset), 0) < 0),
>
> never evaluates to true. Therefore, when sending packets with
> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
> if the creation of the layer 2 header fails.
>
> Spotted by Coverity - CID 1259975 ("Operands don't affect result").
>
> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
> Signed-off-by: Christoph Jaeger <cj@linux.com>
Thanks, Christoph!
Acked-by: Daniel Borkmann <dborkman@redhat.com>
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Joe Perches @ 2015-01-11 21:39 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <54B2EB81.5050604@nod.at>
On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:
> Am 11.01.2015 um 22:14 schrieb Joe Perches:
> > On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
> >> Am 11.01.2015 um 21:59 schrieb Joe Perches:
> >>> On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
> >>>> arp_tables.c has a 16bit aligment ifname_compare(), factor
> >>>> it out to use it for all tables.
> > []
> >>> Perhaps this would be better as bool ifname_compare
> >> Let's discuss the whole concept first
> >
> > The concept seems obvious enough
>
> Anyway, I agree with Linus wrt. bool.
> https://lkml.org/lkml/2013/8/31/138
I don't. He was right when he wrote:
https://lkml.org/lkml/2014/3/10/760
Linus Torvalds <>
I guess I haven't gotten over my hatred of people playing games with
them because support wasn't universal enough. But maybe it's
approaching being irrational these days.
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Richard Weinberger @ 2015-01-11 21:42 UTC (permalink / raw)
To: Joe Perches
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay, stephen
In-Reply-To: <1421012376.9233.9.camel@perches.com>
Am 11.01.2015 um 22:39 schrieb Joe Perches:
> On Sun, 2015-01-11 at 22:30 +0100, Richard Weinberger wrote:
>> Am 11.01.2015 um 22:14 schrieb Joe Perches:
>>> On Sun, 2015-01-11 at 22:02 +0100, Richard Weinberger wrote:
>>>> Am 11.01.2015 um 21:59 schrieb Joe Perches:
>>>>> On Sun, 2015-01-11 at 21:52 +0100, Richard Weinberger wrote:
>>>>>> arp_tables.c has a 16bit aligment ifname_compare(), factor
>>>>>> it out to use it for all tables.
>>> []
>>>>> Perhaps this would be better as bool ifname_compare
>>>> Let's discuss the whole concept first
>>>
>>> The concept seems obvious enough
>>
>> Anyway, I agree with Linus wrt. bool.
>> https://lkml.org/lkml/2013/8/31/138
>
> I don't. He was right when he wrote:
Joe, I really don't care. This is the least significant
patch of the series.
I'll no longer waste my time with that.
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 3/3] x_tables: Factor out 16bit aligment ifname_compare()
From: Jan Engelhardt @ 2015-01-11 22:23 UTC (permalink / raw)
To: Richard Weinberger
Cc: Joe Perches, Netfilter Developer Mailing List,
Linux Kernel Mailing List,
Linux Networking Developer Mailing List
In-Reply-To: <54B2EB81.5050604@nod.at>
On Sunday 2015-01-11 22:30, Richard Weinberger wrote:
>>>> Perhaps this would be better as bool ifname_compare
>
>Anyway, I agree with Linus wrt. bool.
>https://lkml.org/lkml/2013/8/31/138
Had the function return "bool", it would have been obvious enough
what to do with its return type. A return type of "int" might have
hinted towards negative-is-error (in general) or strcmpish values
(functions doing string compare work).
Now that it returns "unsigned long", one is pressed to look at the
function body (not bad per se, but it is a hump) for the return
value's semantics.
Linus says bool is dangerous to the unsuspecting user — but so is
"volatile", microwave ovens, etc. If the kernel really cared for
entry-level coders, it would be written in something like MISRA C.
^ permalink raw reply
* Re: [PATCH 1/3] net: Make interface aliases available for general usage
From: Stephen Hemminger @ 2015-01-11 22:40 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
In-Reply-To: <1421009571-5279-2-git-send-email-richard@nod.at>
On Sun, 11 Jan 2015 21:52:49 +0100
Richard Weinberger <richard@nod.at> wrote:
> Allow interface aliases to be used as regular interfaces.
> Such that a command sequence like this one works:
> $ ip l set eth0 alias internet
> $ ip a s internet
> $ tcpdump -n -i internet
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
There is already a ifalias and it is used by SNMP.
But the common practice is to put longer descriptive names which aren't going
to be usable and there is no requirement that they be unique.
I think you can't do this without breaking some of our users.
^ permalink raw reply
* Re: [RFC] Make predictable/persistent network interface names more handy
From: Stephen Hemminger @ 2015-01-11 22:42 UTC (permalink / raw)
To: Richard Weinberger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
In-Reply-To: <1421009571-5279-1-git-send-email-richard@nod.at>
On Sun, 11 Jan 2015 21:52:48 +0100
Richard Weinberger <richard@nod.at> wrote:
> Since the Linux distribution of my choice makes use of
> predictable network interface names[0] my USB gadgets
> are no longer usb0 but enp0s29u1u2. Same for all other network devices.
> While I can fully understand that this new naming scheme makes sense
> for a lot of people and makes their work easier it does not really work for me.
> My brain is not able to remember that my Beaglebone's USB-Ethernet
> is now enp0s29u1u2. Even after looking at the output of ifconfig
> I have to copy&paste the interface name.
> Instead of just disabling the feature I thought about
> a generic solution which satisfies both needs.
Fix your distro. Easier said than done.
Current systemd names are much shorter
P.s: ifconfig is deprecated.
^ permalink raw reply
* Re: [PATCH 1/3] net: Make interface aliases available for general usage
From: Richard Weinberger @ 2015-01-11 22:43 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
In-Reply-To: <20150111144039.426f0c48@urahara>
Stephen,
Am 11.01.2015 um 23:40 schrieb Stephen Hemminger:
> On Sun, 11 Jan 2015 21:52:49 +0100
> Richard Weinberger <richard@nod.at> wrote:
>
>> Allow interface aliases to be used as regular interfaces.
>> Such that a command sequence like this one works:
>> $ ip l set eth0 alias internet
>> $ ip a s internet
>> $ tcpdump -n -i internet
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>
> There is already a ifalias and it is used by SNMP.
> But the common practice is to put longer descriptive names which aren't going
> to be usable and there is no requirement that they be unique.
Actually I'm using ifalias. This patch just exposes it.
> I think you can't do this without breaking some of our users.
My idea was that udev will not set the alias if already one is used.
Thanks,
//richard
^ permalink raw reply
* Re: [RFC] Make predictable/persistent network interface names more handy
From: Richard Weinberger @ 2015-01-11 22:47 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
In-Reply-To: <20150111144236.1b693c93@urahara>
Am 11.01.2015 um 23:42 schrieb Stephen Hemminger:
> On Sun, 11 Jan 2015 21:52:48 +0100
> Richard Weinberger <richard@nod.at> wrote:
>
>> Since the Linux distribution of my choice makes use of
>> predictable network interface names[0] my USB gadgets
>> are no longer usb0 but enp0s29u1u2. Same for all other network devices.
>> While I can fully understand that this new naming scheme makes sense
>> for a lot of people and makes their work easier it does not really work for me.
>> My brain is not able to remember that my Beaglebone's USB-Ethernet
>> is now enp0s29u1u2. Even after looking at the output of ifconfig
>> I have to copy&paste the interface name.
>> Instead of just disabling the feature I thought about
>> a generic solution which satisfies both needs.
>
> Fix your distro. Easier said than done.
Sorry for using a mainstream distro. :-)
> Current systemd names are much shorter
Hmm, are you sure? When was this changed?
Maybe I misread http://cgit.freedesktop.org/systemd/systemd/tree/src/udev/udev-builtin-net_id.c
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-11 22:49 UTC (permalink / raw)
To: Amir Vadai
Cc: Florian Fainelli, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Saeed Mahameed, David S. Miller, Jason Wang, Michael S. Tsirkin,
Herbert Xu, Al Viro, Ben Hutchings, Masatake YAMATO, Xi Wang,
Neil Horman, WANG Cong, Flavio Leitner, Tom Gundersen, Jiri Pirko,
Vlad Yasevich, Eric W. Biederman, Venkata Duvvuru,
Govindarajulu Varadarajan
In-Reply-To: <54AE4282.20009@mellanox.com>
Thanks for the input. Please ignore this patch series: I'm preparing a
new version: new commands, bitmap-based that should allow us to live
happily ever after, should take your feedback into account. Will send
an RFC patch series in the next hours/days.
On Thu, Jan 8, 2015 at 12:40 AM, Amir Vadai <amirv@mellanox.com> wrote:
> On 1/6/2015 7:36 PM, David Decotigny wrote:
>> Interesting. It seems that the band-aid I was proposing is already
>> obsolete. We could still use the remaining reserved 16 bits to encode
>> 5 more bits per mask (that is: 53 bits / mask total). But if I
>> understand you, it would allow us to survive only a few months longer,
>> as opposed to a few weeks.
>>
>> One short-term alternative solution I can imagine is the following:
>> /* For example bitmap-based for variable length: */
>> struct ethtool_link_mode {
>> __u32 cmd;
>> __u8 autoneg :1;
>> __u8 duplex :2;
>> __u16 supported_nbits;
>> __u16 advertising_nbits;
>> __u16 lp_advertising_nbits;
>> __u32 reserved[4];
>> __u8 masks[0];
>> };
>> /* Or simpler, statically limited to 64b / mask, but easier to migrate
>> to for driver authors: */
> I think the first options is better. A driver will have to do changes in
> order to support >32 link modes, so better change it once now, without
> having to change it again for >64 link modes.
>
>> struct ethtool_link_mode {
>> __u32 cmd;
>> __u8 autoneg :1;
>> __u8 duplex :2;
>> __u64 supported;
>> __u64 advertising;
>> __u64 lp_advertising;
>> __u32 reserved[4];
>> };
>> #define ETHTOOL_GLINK_MODE 0x0000004a
>> #define ETHTOOL_SLINK_MODE 0x0000004b
>> struct ethtool_ops {
>> ...
>> int (*get_link_mode)(struct net_device *, struct ethtool_link_mode *);
>> int (*set_link_mode)(struct net_device *, struct ethtool_link_mode *);
>> };
>>
>> The same thing required for EEE.
> Yeh :(
>
>>
>> I am not sure about moving the autoneg and duplex fields into the new
>> struct. Especially the "duplex" field.
> I think so too. ethtool user space will call ETHTOOL_[GS]SET and after
> that ETHTOOL_[GS]LINK_MODE (if supported). No need to get the
> duplex/autoneg fields again.
>
>>
>> Then the idea would be to update the ethtool user-space tool to try
>> get/set_link mode when reporting/changing the autoneg/advertising
>> settings.
>>
>> Both will require significant effort from the driver authors.
>> Especially if the variable-length bitmap approach is preferred:
>> - most drivers currently use simple bitwise arithmetic in their code,
>> and that goes far beyond get/set_settings, it is sometimes part of the
>> core driver logic. They will have to migrate to the bitmap API if they
>> want to use the larger bitmaps (note: no change needed if they are
>> happy with <= 32b / mask)
> As I said above, it will save as doing this work again in the future,
> and more problematic, save another version to backport in the future. In
> addition, not all drivers will have to do it, only if >32 link speeds is
> needed - this work will be required.
>
>> - we would have to progressively deprecate the use of #define
>> ADVERTISED_1000baseT_Full in favor of an enum of the bit indices.
> Maybe we could use some macro juggling to define the legacy macro's
> using enum for the first 32 bits, and fail the compilation if used on
>>32. For example, calling this:
> DEFINE_LINK_MODE(ADVERTISED_1000baseT_Full, 5)
>
> Will add the following:
> ADVERTISED_1000baseT_Full_SHIFT = 5
> ADVERTISED_1000baseT_Full = (1<<5)
>
> DEFINE_LINK_MODE(ADVERTISED_100000baseKR5_Full, 50) will add:
> ADVERTISED_100000baseKR5_Full_SHIFT = 50
> ADVERTISED_100000baseKR5_Full = #error new link speeds must be defined
> using [gs]et_link_speed
>
> This will break compilation if ADVERTISED_100000baseKR5_Full is used in
> [gs]et_settings (I know the '#error' will not print something very
> pretty - I used it only to explain what I meant)
>
>>
>> Any feedback welcome. In the meantime, I am going to propose a v3 of
>> current option with 53 bits / mask. I can also propose a prototype of
>> the scheme described above, please let me know.
> I think that it is better to do it once, and skip the 53 bits / mask
> version.
> I'll be happy to assist.
>
> Amir
^ permalink raw reply
* Re: [RFC] Make predictable/persistent network interface names more handy
From: Richard Weinberger @ 2015-01-11 22:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, coreteam, netfilter-devel, linux-kernel, netdev,
bhutchings, john.fastabend, herbert, vyasevic, jiri, vfalico,
therbert, edumazet, yoshfuji, jmorris, kuznet, kadlec, kaber,
pablo, kay
In-Reply-To: <20150111144236.1b693c93@urahara>
Am 11.01.2015 um 23:42 schrieb Stephen Hemminger:
> P.s: ifconfig is deprecated.
ifconfig was just an example. It will work if iproute2 too.
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH v2 0/7] Fix sti drivers whcih mix reg address spaces
From: David Miller @ 2015-01-11 23:54 UTC (permalink / raw)
To: peter.griffin-QSEj5FYQhm4dnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
peppe.cavallaro-qxv4g6HH51o, kishon-l0cyMroinI0, arnd-r2nGTMty4D4,
lee.jones-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1420643052-4506-1-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Date: Wed, 7 Jan 2015 15:04:05 +0000
> A V2 of this old series incorporating Arnd and Lees Feedback form v1.
>
> Following on from Arnds comments about the picophy driver here
> https://lkml.org/lkml/2014/11/13/161, this series fixes the
> remaining upstreamed drivers for STI, which are mixing address spaces
> in the reg property. We do this in a way similar to the keystone
> and bcm7445 platforms, by having sysconfig phandle/ offset pair
> (where only one register is required). Or phandle / integer array
> where multiple offsets in the same bank are needed).
>
> This series breaks DT compatability! But the platform support
> is WIP and only being used by the few developers who are upstreaming
> support for it. I've made each change to the driver / dt doc / dt
> file as a single atomic commit so the kernel will remain bisectable.
>
> This series then also enables the picophy driver, and adds back in
> the ehci/ohci dt nodes for stih410 which make use of the picophy.
Series applied to net-next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] team: Remove dead code
From: Kenneth Williams @ 2015-01-12 0:43 UTC (permalink / raw)
To: netdev; +Cc: jiri
The deleted lines are called from a function which is called:
1) Only through __team_options_register via team_options_register and
2) Only during initialization / mode initialization when there are no
ports attached.
Therefore the ports list is guarenteed to be empty and this code will
never be executed.
Signed-off-by: Kenneth Williams <ken@williamsclan.us>
---
drivers/net/team/team.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 93e2242..43bcfff 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -176,7 +176,6 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
static int __team_option_inst_add_option(struct team *team,
struct team_option *option)
{
- struct team_port *port;
int err;
if (!option->per_port) {
@@ -184,12 +183,6 @@ static int __team_option_inst_add_option(struct team *team,
if (err)
goto inst_del_option;
}
-
- list_for_each_entry(port, &team->port_list, list) {
- err = __team_option_inst_add(team, option, port);
- if (err)
- goto inst_del_option;
- }
return 0;
inst_del_option:
--
1.7.9.5
^ permalink raw reply related
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