Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3] 3c59x: Fix memory leaks in vortex_open
From: Neil Horman @ 2014-12-23 15:43 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: davem, ebiederm, dingtianhong, paul.gortmaker,
	justinvanwijngaarden, netdev
In-Reply-To: <54998371.7060109@163.com>

On Tue, Dec 23, 2014 at 11:00:01PM +0800, Jia-Ju Bai wrote:
> Thanks for the reply!
> 
> >This doesn't make sense.  We free all the skbs in vortex_open if we don't
> >allocate all of them (the if (i != RX_RING_SIZE) check), the only place we miss
> >is if vortex_up fails, and you didn't remove the if (!retval) goto out check, so
> >this code won't get run appropriately.
> 
> In the code, when vortex_up is failed and does not returns 0,
> "if (!retval)" is failed and "goto out" is not executed, so error handling
> code
> below is executed, including my added code.
> Is it right?
> 
Yup, you're right, I missed the sense of the check.

> >
> >That said, it does seem we need to clean up if vortex_up fails, but it would
> >seem to me to be easier to just call vortex_close if it does, since that will do
> >all of the approriate cleanup.
> >
> >Neil
> >
> 
> In the code, vortex_close does too many releasing operations, such as free
> vp->tx_skbuff, but vortex_open only allocates memory for vp->rx_skbuff
> before
> vortex_up is called.
> So I think it is enough to just free the memory of vp->rx_skbuff when
> vortex_up
> is failed.
> 
No, I don't think so.  vortex_close predicates each free with a NULL check, so
if its not been allocated, it shouldn't be freed.  vortex_close also puts the
adapter back into a known state (undoing all the setup that vortex_open does).
I really think its better to go with the proper close path than just unwinding
the allocation

Neil

> 

^ permalink raw reply

* [PATCH] can: kvaser_usb: Don't free packets when tight on URBs
From: Ahmed S. Darwish @ 2014-12-23 15:46 UTC (permalink / raw)
  To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde
  Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in high frequency caused seemingly-random panics in the
kernel.

On further inspection, it seems the driver erroneously freed the
to-be-transmitted packet upon getting tight on URBs and returning
NETDEV_TX_BUSY, leading to invalid memory writes and double frees
at a later point in time.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

 (Generated over 3.19.0-rc1)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 541fb7a..34c35d8 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1286,6 +1286,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;
+	bool kfree_skb_on_error = true;
 
 	if (can_dropped_invalid_skb(netdev, skb))
 		return NETDEV_TX_OK;
@@ -1336,6 +1337,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 
 	if (!context) {
 		netdev_warn(netdev, "cannot find free context\n");
+		kfree_skb_on_error = false;
 		ret =  NETDEV_TX_BUSY;
 		goto releasebuf;
 	}
@@ -1364,8 +1366,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	if (unlikely(err)) {
 		can_free_echo_skb(netdev, context->echo_index);
 
-		skb = NULL; /* set to NULL to avoid double free in
-			     * dev_kfree_skb(skb) */
+		kfree_skb_on_error = false;
 
 		atomic_dec(&priv->active_tx_urbs);
 		usb_unanchor_urb(urb);
@@ -1389,7 +1390,8 @@ releasebuf:
 nobufmem:
 	usb_free_urb(urb);
 nourbmem:
-	dev_kfree_skb(skb);
+	if (kfree_skb_on_error)
+		dev_kfree_skb(skb);
 	return ret;
 }

^ permalink raw reply related

* [PATCH] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2014-12-23 15:53 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: <20141223154654.GB6460@vivalin-002>

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.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c | 630 +++++++++++++++++++++++++++++++--------
 1 file changed, 505 insertions(+), 125 deletions(-)

 (Generated over 3.19.0-rc1 + generic bugfix at
  can-kvaser_usb-Don-t-free-packets-when-tight-on-URBs.patch)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 34c35d8..e7076da 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -6,12 +6,15 @@
  * 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) 2014 Valeo Corporation
  */
 
+#include <linux/kernel.h>
 #include <linux/completion.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
@@ -21,6 +24,18 @@
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
 
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+
+/*
+ * 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 */
@@ -29,9 +44,12 @@
 #define USB_RECV_TIMEOUT		1000 /* msecs */
 #define RX_BUFFER_SIZE			3072
 #define CAN_USB_CLOCK			8000000
-#define MAX_NET_DEVICES			3
+#define LEAF_MAX_NET_DEVICES		3
+#define USBCAN_MAX_NET_DEVICES		2
+#define MAX_NET_DEVICES			MAX(LEAF_MAX_NET_DEVICES, \
+					    USBCAN_MAX_NET_DEVICES)
 
-/* 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
@@ -55,6 +73,16 @@
 #define USB_CAN_R_PRODUCT_ID		39
 #define USB_LEAF_LITE_V2_PRODUCT_ID	288
 #define USB_MINI_PCIE_HS_PRODUCT_ID	289
+#define LEAF_PRODUCT_ID(id)		(id >= USB_LEAF_DEVEL_PRODUCT_ID && \
+					 id <= 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
+#define USBCAN_PRODUCT_ID(id)		(id >= USB_USBCAN_REVB_PRODUCT_ID && \
+					 id <= USB_MEMORATOR_PRODUCT_ID)
 
 /* USB devices features */
 #define KVASER_HAS_SILENT_MODE		BIT(0)
@@ -73,7 +101,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 +126,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 LEAF_CMD_GET_CARD_INFO2		32
+#define USBCAN_CMD_RESET_CLOCK		32
+#define USBCAN_CMD_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 +142,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 LEAF_CMD_USB_THROTTLE		77
+#define LEAF_CMD_LOG_MESSAGE		106
 
 /* error factors */
 #define M16C_EF_ACKE			BIT(0)
@@ -121,6 +156,13 @@
 #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 USBCAN */
+#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 +179,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 +190,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 +255,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 +346,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 +372,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 +389,49 @@ 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;
 
+/* Leaf/USBCAN-agnostic summary of an error event.
+ * No M16C error factors for USBCAN-based devices. */
+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 +447,8 @@ struct kvaser_usb {
 
 	u32 fw_version;
 	unsigned int nchannels;
+	enum kvaser_usb_family family;
+	unsigned int max_channels;
 
 	bool rxinitdone;
 	void *rxbuf[MAX_RX_URBS];
@@ -311,6 +472,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 +522,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 +636,18 @@ 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;
+	default:
+		dev_err(dev->udev->dev.parent,
+			"Invalid device family (%d)\n", dev->family);
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -482,7 +666,7 @@ 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 > dev->max_channels)
 		return -EINVAL;
 
 	return 0;
@@ -496,8 +680,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,
@@ -615,37 +801,83 @@ 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(const struct kvaser_usb *dev,
-				const struct kvaser_msg *msg)
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+				      struct kvaser_error_summary *es);
+
+/*
+ * Report error to userspace iff the controller's errors counter has
+ * increased, or we're the only channel seeing the bus error state.
+ *
+ * As reported by USBCAN sheets, "the CAN controller has difficulties
+ * to tell whether an error frame arrived on channel 1 or on channel 2."
+ * Thus, error counters are compared with their earlier values to
+ * determine which channel was responsible for the error event.
+ */
+static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
+					      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;
+	int old_tx_err_count, old_rx_err_count, channel, 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];
+	old_tx_err_count = priv->bec.txerr;
+	old_rx_err_count = priv->bec.rxerr;
+
+	report_error = 0;
+	if (es->txerr > old_tx_err_count) {
+		es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+		report_error = 1;
+	}
+	if (es->rxerr > old_rx_err_count) {
+		es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+		report_error = 1;
+	}
+	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 = 1;
+	}
+
+	if (report_error)
+		kvaser_report_error_event(dev, es);
+}
+
+/*
+ * Extract error summary from a Leaf-based device error message
+ */
+static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
+					const struct kvaser_msg *msg)
+{
+	struct kvaser_error_summary es = { 0, };
 
 	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.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_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];
+	case LEAF_CMD_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:
-		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.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",
@@ -653,16 +885,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 		return;
 	}
 
-	if (channel >= dev->nchannels) {
+	kvaser_report_error_event(dev, &es);
+}
+
+/*
+ * Extract summary from a USBCANII-based device error message.
+ */
+static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
+					const struct kvaser_msg *msg)
+{
+	struct kvaser_error_summary es = { 0, };
+
+	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;
+		usbcan_report_error_if_applicable(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;
+		usbcan_report_error_if_applicable(dev, &es);
+
+		/* For error events, the USBCAN firmware does not support
+		 * more than 2 channels: ch0, and ch1. */
+		if (dev->nchannels > 1) {
+			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;
+			usbcan_report_error_if_applicable(dev, &es);
+		}
+		break;
+
+	default:
+		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+			msg->id);
+	}
+}
+
+static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
+				const struct kvaser_msg *msg)
+{
+	switch (dev->family) {
+	case KVASER_LEAF:
+		leaf_extract_error_from_msg(dev, msg);
+		break;
+	case KVASER_USBCAN:
+		usbcan_extract_error_from_msg(dev, msg);
+		break;
+	default:
 		dev_err(dev->udev->dev.parent,
-			"Invalid channel number (%d)\n", channel);
+			"Invalid device family (%d)\n", dev->family);
 		return;
 	}
+}
 
-	priv = dev->nets[channel];
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+				      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;
+
+	if (es->channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", es->channel);
+		return;
+	}
+
+	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 +983,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 +995,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 +1011,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 +1029,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 +1045,52 @@ 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;
+	default:
+		dev_err(dev->udev->dev.parent,
+			"Invalid device family (%d)\n", dev->family);
+		goto err;
 	}
 
-	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;
 
@@ -774,6 +1098,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
+
+	return;
+
+err:
+	dev_kfree_skb(skb);
 }
 
 static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -783,16 +1112,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) {
 		skb = alloc_can_err_skb(priv->netdev, &cf);
 		if (!skb) {
 			stats->rx_dropped++;
@@ -819,7 +1148,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,
@@ -830,19 +1160,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)) {
+	if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+	    (dev->family == KVASER_LEAF && msg->id == LEAF_CMD_LOG_MESSAGE)) {
 		kvaser_usb_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;
 	}
 
@@ -852,38 +1195,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 == LEAF_CMD_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);
 	}
 
@@ -947,7 +1289,12 @@ 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 LEAF_CMD_LOG_MESSAGE:
+		if (dev->family != KVASER_LEAF)
+			goto warn;
 		kvaser_usb_rx_can_msg(dev, msg);
 		break;
 
@@ -960,8 +1307,14 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
 		kvaser_usb_tx_acknowledge(dev, msg);
 		break;
 
+	/* Ignored messages */
+	case USBCAN_CMD_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;
 	}
@@ -1181,7 +1534,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->max_channels; i++) {
 		struct kvaser_usb_net_priv *priv = dev->nets[i];
 
 		if (priv)
@@ -1286,6 +1639,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;
 	bool kfree_skb_on_error = true;
 
 	if (can_dropped_invalid_skb(netdev, skb))
@@ -1306,9 +1660,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;
@@ -1326,7 +1694,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 +1964,18 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 	if (!dev)
 		return -ENOMEM;
 
+	if (LEAF_PRODUCT_ID(id->idProduct)) {
+		dev->family = KVASER_LEAF;
+		dev->max_channels = LEAF_MAX_NET_DEVICES;
+	} else if (USBCAN_PRODUCT_ID(id->idProduct)) {
+		dev->family = KVASER_USBCAN;
+		dev->max_channels = USBCAN_MAX_NET_DEVICES;
+	} 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)");
@@ -1608,7 +1988,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
 
 	usb_set_intfdata(intf, dev);
 
-	for (i = 0; i < MAX_NET_DEVICES; i++)
+	for (i = 0; i < dev->max_channels; i++)
 		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
 
 	err = kvaser_usb_get_software_info(dev);

^ permalink raw reply related

* Re: [PATCH] bonding: change error message to debug message in __bond_release_one()
From: Andy Gospodarek @ 2014-12-23 16:02 UTC (permalink / raw)
  To: Wengang Wang; +Cc: netdev, dingtianhong
In-Reply-To: <1419297876-1412-1-git-send-email-wen.gang.wang@oracle.com>

On Tue, Dec 23, 2014 at 09:24:36AM +0800, Wengang Wang wrote:
> In __bond_release_one(), when the interface is not a slave or not a slave of
> "this" master, it log error message.
> 
> The message actually should be a debug message matching what bond_enslave()
> does.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>

Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>

> ---
>  drivers/net/bonding/bond_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 184c434..0dceba1 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1648,7 +1648,7 @@ static int __bond_release_one(struct net_device *bond_dev,
>  	/* slave is not a slave or master is not master of this slave */
>  	if (!(slave_dev->flags & IFF_SLAVE) ||
>  	    !netdev_has_upper_dev(slave_dev, bond_dev)) {
> -		netdev_err(bond_dev, "cannot release %s\n",
> +		netdev_dbg(bond_dev, "cannot release %s\n",
>  			   slave_dev->name);
>  		return -EINVAL;
>  	}
> -- 
> 1.8.3.1
> 

^ permalink raw reply

* [PATCH net] neigh: remove next ptr from struct neigh_table
From: Nicolas Dichtel @ 2014-12-23 16:50 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nicolas Dichtel, Cong Wang

After commit
d7480fd3b173 ("neigh: remove dynamic neigh table registration support"),
this field is not used anymore.

CC: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/neighbour.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index eb070b3674a1..76f708486aae 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -190,7 +190,6 @@ struct neigh_hash_table {
 
 
 struct neigh_table {
-	struct neigh_table	*next;
 	int			family;
 	int			entry_size;
 	int			key_len;
-- 
2.1.0

^ permalink raw reply related

* [PATCH] net: incorrect use of init_completion fixup
From: Nicholas Mc Guire @ 2014-12-23 17:47 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: netdev, linux-kernel, Nicholas Mc Guire

The second init_completion call should be a reinit_completion here.

patch is against 3.18.0 linux-next

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 drivers/net/ethernet/brocade/bna/bnad_debugfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
index 7d6aa8c..619083a 100644
--- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
+++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
@@ -172,7 +172,7 @@ bnad_get_debug_drvinfo(struct bnad *bnad, void *buffer, u32 len)

 	/* Retrieve flash partition info */
 	fcomp.comp_status = 0;
-	init_completion(&fcomp.comp);
+	reinit_completion(&fcomp.comp);
 	spin_lock_irqsave(&bnad->bna_lock, flags);
 	ret = bfa_nw_flash_get_attr(&bnad->bna.flash, &drvinfo->flash_attr,
 				bnad_cb_completion, &fcomp);
--
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net] net: Reset secmark when scrubbing packet
From: Flavio Leitner @ 2014-12-23 19:28 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, netdev
In-Reply-To: <efb09aca5173a9a18f15066c33a4998ed70bd34a.1419293504.git.tgraf@suug.ch>

On Tuesday, December 23, 2014 01:13:18 AM Thomas Graf wrote:
> 
> skb_scrub_packet() is called when a packet switches between a context
> such as between underlay and overlay, between namespaces, or between
> L3 subnets.
> 
> While we already scrub the packet mark, connection tracking entry,
> and cached destination, the security mark/context is left intact.
> 
> It seems wrong to inherit the security context of a packet when going
> from overlay to underlay or across forwarding paths.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
> net/core/skbuff.c | 1 +
> 1 file changed, 1 insertion(+)

Acked-by: Flavio Leitner <fbl@sysclose.org>

^ permalink raw reply

* [PATCH v2] genetlink: pass multicast bind/unbind to families
From: Johannes Berg @ 2014-12-23 19:54 UTC (permalink / raw)
  To: netdev; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

In order to make the newly fixed multicast bind/unbind
functionality in generic netlink, pass them down to the
appropriate family.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/genetlink.h |  5 +++++
 net/netlink/genetlink.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 38620da4aa7a..3ed31e5a445b 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -31,6 +31,9 @@ struct genl_info;
  *	do additional, common, filtering and return an error
  * @post_doit: called after an operation's doit callback, it may
  *	undo operations done by pre_doit, for example release locks
+ * @mcast_bind: a socket bound to the given multicast group (which
+ *	is given as the offset into the groups array)
+ * @mcast_unbind: a socket was unbound from the given multicast group
  * @attrbuf: buffer to store parsed attributes
  * @family_list: family list
  * @mcgrps: multicast groups used by this family (private)
@@ -53,6 +56,8 @@ struct genl_family {
 	void			(*post_doit)(const struct genl_ops *ops,
 					     struct sk_buff *skb,
 					     struct genl_info *info);
+	int			(*mcast_bind)(int group);
+	void			(*mcast_unbind)(int group);
 	struct nlattr **	attrbuf;	/* private */
 	const struct genl_ops *	ops;		/* private */
 	const struct genl_multicast_group *mcgrps; /* private */
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 76393f2f4b22..05bf40bbd189 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -983,11 +983,70 @@ static struct genl_multicast_group genl_ctrl_groups[] = {
 	{ .name = "notify", },
 };
 
+static int genl_bind(int group)
+{
+	int i, err;
+	bool found = false;
+
+	down_read(&cb_lock);
+	for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
+		struct genl_family *f;
+
+		list_for_each_entry(f, genl_family_chain(i), family_list) {
+			if (group >= f->mcgrp_offset &&
+			    group < f->mcgrp_offset + f->n_mcgrps) {
+				int fam_grp = group - f->mcgrp_offset;
+
+				if (f->mcast_bind)
+					err = f->mcast_bind(fam_grp);
+				else
+					err = 0;
+				found = true;
+				break;
+			}
+		}
+	}
+	up_read(&cb_lock);
+
+	if (WARN_ON(!found))
+		err = 0;
+
+	return err;
+}
+
+static void genl_unbind(int group)
+{
+	int i;
+	bool found = false;
+
+	down_read(&cb_lock);
+	for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
+		struct genl_family *f;
+
+		list_for_each_entry(f, genl_family_chain(i), family_list) {
+			if (group >= f->mcgrp_offset &&
+			    group < f->mcgrp_offset + f->n_mcgrps) {
+				int fam_grp = group - f->mcgrp_offset;
+
+				if (f->mcast_unbind)
+					f->mcast_unbind(fam_grp);
+				found = true;
+				break;
+			}
+		}
+	}
+	up_read(&cb_lock);
+
+	WARN_ON(!found);
+}
+
 static int __net_init genl_pernet_init(struct net *net)
 {
 	struct netlink_kernel_cfg cfg = {
 		.input		= genl_rcv,
 		.flags		= NL_CFG_F_NONROOT_RECV,
+		.bind		= genl_bind,
+		.unbind		= genl_unbind,
 	};
 
 	/* we'll bump the group number right afterwards */
-- 
2.1.1

^ permalink raw reply related

* Re: [PATCH net] neigh: remove next ptr from struct neigh_table
From: Cong Wang @ 2014-12-23 19:57 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David Miller, Linux Kernel Network Developers
In-Reply-To: <1419353437-4213-1-git-send-email-nicolas.dichtel@6wind.com>

On Tue, Dec 23, 2014 at 8:50 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> After commit
> d7480fd3b173 ("neigh: remove dynamic neigh table registration support"),
> this field is not used anymore.
>
> CC: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Thanks for fixing this leftover!

^ permalink raw reply

* [PATCH] netlink/genetlink: pass network namespace to bind/unbind
From: Johannes Berg @ 2014-12-23 20:00 UTC (permalink / raw)
  To: netdev; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Netlink families can exist in multiple namespaces, and for the most
part multicast subscriptions are per network namespace. Thus it only
makes sense to have bind/unbind notifications per network namespace.

To achieve this, pass the network namespace of a given client socket
to the bind/unbind functions.

Also do this in generic netlink, and there also make sure that any
bind for multicast groups that only exist in init_net is rejected.
This isn't really a problem if it is accepted since a client in a
different namespace will never receive any notifications from such
a group, but it can confuse the family if not rejected (it's also
possible to silently (without telling the family) accept it, but it
would also have to be ignored on unbind so families that take any
kind of action on bind/unbind won't do unnecessary work for invalid
clients like that.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/netlink.h   |  4 ++--
 include/net/genetlink.h   |  4 ++--
 kernel/audit.c            |  2 +-
 net/netfilter/nfnetlink.c |  2 +-
 net/netlink/af_netlink.c  | 21 +++++++++++----------
 net/netlink/af_netlink.h  |  8 ++++----
 net/netlink/genetlink.c   | 12 +++++++-----
 7 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 9e572daa15d5..02fc86d2348e 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -46,8 +46,8 @@ struct netlink_kernel_cfg {
 	unsigned int	flags;
 	void		(*input)(struct sk_buff *skb);
 	struct mutex	*cb_mutex;
-	int		(*bind)(int group);
-	void		(*unbind)(int group);
+	int		(*bind)(struct net *net, int group);
+	void		(*unbind)(struct net *net, int group);
 	bool		(*compare)(struct net *net, struct sock *sk);
 };
 
diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 3ed31e5a445b..84125088c309 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -56,8 +56,8 @@ struct genl_family {
 	void			(*post_doit)(const struct genl_ops *ops,
 					     struct sk_buff *skb,
 					     struct genl_info *info);
-	int			(*mcast_bind)(int group);
-	void			(*mcast_unbind)(int group);
+	int			(*mcast_bind)(struct net *net, int group);
+	void			(*mcast_unbind)(struct net *net, int group);
 	struct nlattr **	attrbuf;	/* private */
 	const struct genl_ops *	ops;		/* private */
 	const struct genl_multicast_group *mcgrps; /* private */
diff --git a/kernel/audit.c b/kernel/audit.c
index 1f37f15117e5..0264ab84c446 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1100,7 +1100,7 @@ static void audit_receive(struct sk_buff  *skb)
 }
 
 /* Run custom bind function on netlink socket group connect or bind requests. */
-static int audit_bind(int group)
+static int audit_bind(struct net *net, int group)
 {
 	if (!capable(CAP_AUDIT_READ))
 		return -EPERM;
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 13c2e17bbe27..cde4a6702fa3 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -463,7 +463,7 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 }
 
 #ifdef CONFIG_MODULES
-static int nfnetlink_bind(int group)
+static int nfnetlink_bind(struct net *net, int group)
 {
 	const struct nfnetlink_subsystem *ss;
 	int type;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 07a903de4439..5fe199abfa41 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1161,8 +1161,8 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
 	struct module *module = NULL;
 	struct mutex *cb_mutex;
 	struct netlink_sock *nlk;
-	int (*bind)(int group);
-	void (*unbind)(int group);
+	int (*bind)(struct net *net, int group);
+	void (*unbind)(struct net *net, int group);
 	int err = 0;
 
 	sock->state = SS_UNCONNECTED;
@@ -1271,7 +1271,7 @@ static int netlink_release(struct socket *sock)
 
 		for (i = 0; i < nlk->ngroups; i++)
 			if (test_bit(i, nlk->groups))
-				nlk->netlink_unbind(i + 1);
+				nlk->netlink_unbind(sock_net(sk), i + 1);
 	}
 	kfree(nlk->groups);
 	nlk->groups = NULL;
@@ -1438,8 +1438,9 @@ static int netlink_realloc_groups(struct sock *sk)
 }
 
 static void netlink_undo_bind(int group, long unsigned int groups,
-			      struct netlink_sock *nlk)
+			      struct sock *sk)
 {
+	struct netlink_sock *nlk = nlk_sk(sk);
 	int undo;
 
 	if (!nlk->netlink_unbind)
@@ -1447,7 +1448,7 @@ static void netlink_undo_bind(int group, long unsigned int groups,
 
 	for (undo = 0; undo < group; undo++)
 		if (test_bit(undo, &groups))
-			nlk->netlink_unbind(undo);
+			nlk->netlink_unbind(sock_net(sk), undo);
 }
 
 static int netlink_bind(struct socket *sock, struct sockaddr *addr,
@@ -1485,10 +1486,10 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 		for (group = 0; group < nlk->ngroups; group++) {
 			if (!test_bit(group, &groups))
 				continue;
-			err = nlk->netlink_bind(group);
+			err = nlk->netlink_bind(net, group);
 			if (!err)
 				continue;
-			netlink_undo_bind(group, groups, nlk);
+			netlink_undo_bind(group, groups, sk);
 			return err;
 		}
 	}
@@ -1498,7 +1499,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 			netlink_insert(sk, net, nladdr->nl_pid) :
 			netlink_autobind(sock);
 		if (err) {
-			netlink_undo_bind(nlk->ngroups, groups, nlk);
+			netlink_undo_bind(nlk->ngroups, groups, sk);
 			return err;
 		}
 	}
@@ -2149,7 +2150,7 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
 		if (!val || val - 1 >= nlk->ngroups)
 			return -EINVAL;
 		if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) {
-			err = nlk->netlink_bind(val);
+			err = nlk->netlink_bind(sock_net(sk), val);
 			if (err)
 				return err;
 		}
@@ -2158,7 +2159,7 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
 					 optname == NETLINK_ADD_MEMBERSHIP);
 		netlink_table_ungrab();
 		if (optname == NETLINK_DROP_MEMBERSHIP && nlk->netlink_unbind)
-			nlk->netlink_unbind(val);
+			nlk->netlink_unbind(sock_net(sk), val);
 
 		err = 0;
 		break;
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index b20a1731759b..f123a88496f8 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -39,8 +39,8 @@ struct netlink_sock {
 	struct mutex		*cb_mutex;
 	struct mutex		cb_def_mutex;
 	void			(*netlink_rcv)(struct sk_buff *skb);
-	int			(*netlink_bind)(int group);
-	void			(*netlink_unbind)(int group);
+	int			(*netlink_bind)(struct net *net, int group);
+	void			(*netlink_unbind)(struct net *net, int group);
 	struct module		*module;
 #ifdef CONFIG_NETLINK_MMAP
 	struct mutex		pg_vec_lock;
@@ -65,8 +65,8 @@ struct netlink_table {
 	unsigned int		groups;
 	struct mutex		*cb_mutex;
 	struct module		*module;
-	int			(*bind)(int group);
-	void			(*unbind)(int group);
+	int			(*bind)(struct net *net, int group);
+	void			(*unbind)(struct net *net, int group);
 	bool			(*compare)(struct net *net, struct sock *sock);
 	int			registered;
 };
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 05bf40bbd189..91566ed36c43 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -983,7 +983,7 @@ static struct genl_multicast_group genl_ctrl_groups[] = {
 	{ .name = "notify", },
 };
 
-static int genl_bind(int group)
+static int genl_bind(struct net *net, int group)
 {
 	int i, err;
 	bool found = false;
@@ -997,8 +997,10 @@ static int genl_bind(int group)
 			    group < f->mcgrp_offset + f->n_mcgrps) {
 				int fam_grp = group - f->mcgrp_offset;
 
-				if (f->mcast_bind)
-					err = f->mcast_bind(fam_grp);
+				if (!f->netnsok && net != &init_net)
+					err = -ENOENT;
+				else if (f->mcast_bind)
+					err = f->mcast_bind(net, fam_grp);
 				else
 					err = 0;
 				found = true;
@@ -1014,7 +1016,7 @@ static int genl_bind(int group)
 	return err;
 }
 
-static void genl_unbind(int group)
+static void genl_unbind(struct net *net, int group)
 {
 	int i;
 	bool found = false;
@@ -1029,7 +1031,7 @@ static void genl_unbind(int group)
 				int fam_grp = group - f->mcgrp_offset;
 
 				if (f->mcast_unbind)
-					f->mcast_unbind(fam_grp);
+					f->mcast_unbind(net, fam_grp);
 				found = true;
 				break;
 			}
-- 
2.1.1

^ permalink raw reply related

* Re: [PATCH net-next 01/11] time: move the timecounter/cyclecounter code into its own file.
From: Jeff Kirsher @ 2014-12-23 21:03 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky
In-Reply-To: <3698bac182b6c5aba4dd541afabc1a5d61db1d00.1418504887.git.richardcochran@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

On Sun, 2014-12-21 at 19:46 +0100, Richard Cochran wrote:
> The timecounter code has almost nothing to do with the clocksource
> code. Let it live in its own file. This will help isolate the
> timecounter users from the clocksource users in the source tree.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
For the Intel driver changes...

> ---
>  drivers/net/ethernet/amd/xgbe/xgbe.h        |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h |    2 +-
>  drivers/net/ethernet/freescale/fec.h        |    1 +
>  drivers/net/ethernet/intel/e1000e/e1000.h   |    2 +-
>  drivers/net/ethernet/intel/igb/igb.h        |    2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h    |    2 +-
>  drivers/net/ethernet/ti/cpts.h              |    1 +
>  include/clocksource/arm_arch_timer.h        |    2 +-
>  include/linux/clocksource.h                 |  102
> ----------------------
>  include/linux/mlx4/device.h                 |    2 +-
>  include/linux/timecounter.h                 |  122
> +++++++++++++++++++++++++++
>  include/linux/types.h                       |    3 +
>  kernel/time/Makefile                        |    2 +-
>  kernel/time/clocksource.c                   |   76 -----------------
>  kernel/time/timecounter.c                   |   95
> +++++++++++++++++++++
>  sound/pci/hda/hda_priv.h                    |    2 +-
>  16 files changed, 231 insertions(+), 187 deletions(-)
>  create mode 100644 include/linux/timecounter.h
>  create mode 100644 kernel/time/timecounter.c



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 06/11] net: e1000e: convert to timecounter adjtime.
From: Jeff Kirsher @ 2014-12-23 21:04 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky
In-Reply-To: <7455b879a8aab91f11c3082c2c8c1dc3ef71b96e.1418504889.git.richardcochran@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/e1000e/ptp.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 07/11] net: igb: convert to timecounter adjtime.
From: Jeff Kirsher @ 2014-12-23 21:04 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky
In-Reply-To: <bb15b44a539ec6da39812229b5fb0718bd6859f0.1418504889.git.richardcochran@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 08/11] net: ixgbe: convert to timecounter adjtime.
From: Jeff Kirsher @ 2014-12-23 21:07 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky
In-Reply-To: <5d83679d541974c164cfc415c0bb171e2b363f16.1418504889.git.richardcochran@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On Sun, 2014-12-21 at 19:47 +0100, Richard Cochran wrote:
> This patch changes the driver to use the new and improved method
> for adjusting the offset of a timecounter.
> 
> Compile tested only.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Just for sanity sake, I will have Phillip test the changes, but there is
no need to hold this up from being committed in the mean time.

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 08/11] net: ixgbe: convert to timecounter adjtime.
From: Richard Cochran @ 2014-12-23 21:42 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: netdev, linux-kernel, Amir Vadai, Ariel Elior, Carolyn Wyborny,
	David Miller, Frank Li, John Stultz, Matthew Vick,
	Miroslav Lichvar, Mugunthan V N, Or Gerlitz, Thomas Gleixner,
	Tom Lendacky
In-Reply-To: <1419368839.2470.32.camel@jtkirshe-mobl>

On Tue, Dec 23, 2014 at 01:07:19PM -0800, Jeff Kirsher wrote:
> Just for sanity sake, I will have Phillip test the changes, but there is
> no need to hold this up from being committed in the mean time.

FYI, here is a test that may show the improved behavior when comparing
before/after.

1. Start ptp4l, let it run long enough to find the frequency offset.

2. Stop ptp4l with ^C.

3. Wait a bit, to let the clock drift away.

4. Restart ptp4l, it will now start with the frequency offset learned
   in step 1.

5. Notice the the offsets in states s0 -> s1 -> s2.
   There will likely be less offset error with the patches applied.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net] net: Generalize ndo_gso_check to ndo_features_check
From: Tom Herbert @ 2014-12-23 21:54 UTC (permalink / raw)
  To: Sathya Perla
  Cc: Jesse Gross, David Miller, Linux Netdev List, Joe Stringer,
	Eric Dumazet
In-Reply-To: <CF9D1877D81D214CB0CA0669EFAE020C68D41FA3@CMEXMB1.ad.emulex.com>

On Mon, Dec 22, 2014 at 10:24 PM, Sathya Perla <Sathya.Perla@emulex.com> wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>> owner@vger.kernel.org] On Behalf Of Tom Herbert
>>
>> On Mon, Dec 22, 2014 at 8:03 AM, Jesse Gross <jesse@nicira.com> wrote:
>> > GSO isn't the only offload feature with restrictions that
>> > potentially can't be expressed with the current features mechanism.
>> > Checksum is another although it's a general issue that could in
>> > theory apply to anything. Even if it may be possible to
>> > implement these restrictions in other ways, it can result in
>> > duplicate code or inefficient per-packet behavior.
>> >
>> > This generalizes ndo_gso_check so that drivers can remove any
>> > features that don't make sense for a given packet, similar to
>> > netif_skb_features(). It also converts existing driver
>> > restrictions to the new format, completing the work that was
>> > done to support tunnel protocols since the issues apply to
>> > checksums as well.
>> >
>> It's a nice feature, but I really hope that this is not used for
>> checksums. We already have a sufficiently general interface for that
>> and checksum is already computed in drivers to work around HW bugs.
>>
> The ndo_featureas_check() interface that includes a means to report
> inability to compute inner checksums on some tunnel types, seems like
> a useful feature. The Skyhawk-R NIC can support inner csum offload for
> either vxlan or nv-gre, but not both simultaneously. So, this ndo_
> is useful in reporting this situation.
> This would also obviate the need to have extra code in the drivers to
> compute csums in the above scenario.
>
You could use it for that, but many drivers already call
skb_checksum_help and I really doubt it makes sense to change all of
those. Besides that, I don't think we should be "encouraging" vendors
to continue developing NICs that do protocol specific checksums. The
general interface (NETIF_HW_CSUM) is incredibly simple and obviates a
whole bunch of complexity in drivers to figure out whether it can
checksum a packet.

Tom

> thanks,
> -Sathya

^ permalink raw reply

* [PATCH] arm: sa1100: move irda header to linux/platform_data
From: Dmitry Eremin-Solenikov @ 2014-12-23 22:14 UTC (permalink / raw)
  To: Russell King, Samuel Ortiz; +Cc: linux-arm-kernel, netdev

In the end asm/mach/irda.h header is not used by anybody except sa1100.
Move the header to the platform data includes dir and rename it to
irda-sa11x0.h.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/assabet.c                                          | 2 +-
 arch/arm/mach-sa1100/collie.c                                           | 2 +-
 arch/arm/mach-sa1100/h3100.c                                            | 2 +-
 arch/arm/mach-sa1100/h3600.c                                            | 2 +-
 drivers/net/irda/sa1100_ir.c                                            | 2 +-
 .../asm/mach/irda.h => include/linux/platform_data/irda-sa11x0.h        | 0
 6 files changed, 5 insertions(+), 5 deletions(-)
 rename arch/arm/include/asm/mach/irda.h => include/linux/platform_data/irda-sa11x0.h (100%)

diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index 7dd894e..d28ecb9 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -37,7 +37,7 @@
 
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
-#include <asm/mach/irda.h>
+#include <linux/platform_data/irda-sa11x0.h>
 #include <asm/mach/map.h>
 #include <mach/assabet.h>
 #include <linux/platform_data/mfd-mcp-sa11x0.h>
diff --git a/arch/arm/mach-sa1100/collie.c b/arch/arm/mach-sa1100/collie.c
index 108939f..0a816e2 100644
--- a/arch/arm/mach-sa1100/collie.c
+++ b/arch/arm/mach-sa1100/collie.c
@@ -43,7 +43,7 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/irda.h>
+#include <linux/platform_data/irda-sa11x0.h>
 
 #include <asm/hardware/scoop.h>
 #include <asm/mach/sharpsl_param.h>
diff --git a/arch/arm/mach-sa1100/h3100.c b/arch/arm/mach-sa1100/h3100.c
index 3c43219..c6b4120 100644
--- a/arch/arm/mach-sa1100/h3100.c
+++ b/arch/arm/mach-sa1100/h3100.c
@@ -18,7 +18,7 @@
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
-#include <asm/mach/irda.h>
+#include <linux/platform_data/irda-sa11x0.h>
 
 #include <mach/h3xxx.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-sa1100/h3600.c b/arch/arm/mach-sa1100/h3600.c
index 5be54c2..118338e 100644
--- a/arch/arm/mach-sa1100/h3600.c
+++ b/arch/arm/mach-sa1100/h3600.c
@@ -18,7 +18,7 @@
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
-#include <asm/mach/irda.h>
+#include <linux/platform_data/irda-sa11x0.h>
 
 #include <mach/h3xxx.h>
 #include <mach/irqs.h>
diff --git a/drivers/net/irda/sa1100_ir.c b/drivers/net/irda/sa1100_ir.c
index 42fde9e..dd14722 100644
--- a/drivers/net/irda/sa1100_ir.c
+++ b/drivers/net/irda/sa1100_ir.c
@@ -38,7 +38,7 @@
 #include <net/irda/irda_device.h>
 
 #include <mach/hardware.h>
-#include <asm/mach/irda.h>
+#include <linux/platform_data/irda-sa11x0.h>
 
 static int power_level = 3;
 static int tx_lpm;
diff --git a/arch/arm/include/asm/mach/irda.h b/include/linux/platform_data/irda-sa11x0.h
similarity index 100%
rename from arch/arm/include/asm/mach/irda.h
rename to include/linux/platform_data/irda-sa11x0.h
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH v4] can: Convert to runtime_pm
From: Sören Brinkmann @ 2014-12-23 22:43 UTC (permalink / raw)
  To: Kedareswara rao Appana
  Cc: wg, mkl, michal.simek, grant.likely, linux-can, netdev,
	linux-kernel, Kedareswara rao Appana
In-Reply-To: <58f37b6fd9104ce185c413c473fe047b@BY2FFO11FD050.protection.gbl>

On Tue, 2014-12-23 at 05:55PM +0530, Kedareswara rao Appana wrote:
> Instead of enabling/disabling clocks at several locations in the driver,
> use the runtime_pm framework. This consolidates the actions for
> runtime PM in the appropriate callbacks and makes the driver more
> readable and mantainable.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> Chnages for v4:
>  - Updated with the review comments.
> Changes for v3:
>   - Converted the driver to use runtime_pm.
> Changes for v2:
>   - Removed the struct platform_device* from suspend/resume
>     as suggest by Lothar.
> 
>  drivers/net/can/xilinx_can.c |  123 +++++++++++++++++++++++++-----------------
>  1 files changed, 74 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 6c67643..c71f683 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -32,6 +32,7 @@
>  #include <linux/can/dev.h>
>  #include <linux/can/error.h>
>  #include <linux/can/led.h>
> +#include <linux/pm_runtime.h>
>  
>  #define DRIVER_NAME	"xilinx_can"
>  
> @@ -138,7 +139,7 @@ struct xcan_priv {
>  	u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
>  	void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
>  			u32 val);
> -	struct net_device *dev;
> +	struct device *dev;
>  	void __iomem *reg_base;
>  	unsigned long irq_flags;
>  	struct clk *bus_clk;
> @@ -842,6 +843,13 @@ static int xcan_open(struct net_device *ndev)
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  	int ret;
>  
> +	ret = pm_runtime_get_sync(priv->dev);
> +	if (ret < 0) {
> +		netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",

Does this create the intended output? I haven't seen '\r' anywhere else.
Shouldn't this simply be:
	netdev_err(ndev, "%s: pm_runtime_get failed (%d)\n",

[...]
> @@ -934,27 +927,20 @@ static int xcan_get_berr_counter(const struct net_device *ndev,
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  	int ret;
>  
> -	ret = clk_prepare_enable(priv->can_clk);
> -	if (ret)
> -		goto err;
> -
> -	ret = clk_prepare_enable(priv->bus_clk);
> -	if (ret)
> -		goto err_clk;
> +	ret = pm_runtime_get_sync(priv->dev);
> +	if (ret < 0) {
> +		netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",

ditto

	Sören

^ permalink raw reply

* Marvell Kirkwood - MV643XX: near 100% UDP RX packet loss
From: Bruno Prémont @ 2014-12-23 23:18 UTC (permalink / raw)
  To: Sebastian Hesselbarth; +Cc: netdev

Hi,

On a SheevaPlug, Marvell Kirkwood based, I get nearly 100% packet loss
when running iperf in UDP (rx) mode while I get ~400Gb/s in tx.
For TCP both rx and tx result in about 650Mb/s.

Running iperf server on the sheevaplug:
# iperf3 -s -p 3740

Running iperf client on a AMD APU:
# iperf3 -p 3740 -t 5 -b 0 -4 -c sheevaplug $extra

iperf output at the end.

Both systems are interconnected with a Gb/s switch [Netgear GS108E]
which successfully handles 950Mb/s between my client and a AMD turion
system (both directions, UDP as well as TCP).


Am I mis-configuring the SheevaPlug or is there some bug causing all
the UDP packets to get lost? (slow rate UDP is working fine however)


Thanks,
Bruno


On the SheevaPlug:

# uname -a
Linux sheevaplug 3.15.0-sheeva+ #1 Mon Jun 9 21:58:19 CEST 2014 armv5tel Marvell Kirkwood (Flattened Device Tree) GNU/Linux

# ethtool -g eth0
Ring parameters for eth0:
Pre-set maximums:
RX:             4096
RX Mini:        0
RX Jumbo:       0
TX:             4096
Current hardware settings:
RX:             2048             (this defaults to 128 on boot)
RX Mini:        0
RX Jumbo:       0
TX:             256

# ethtool -k eth0
Features for eth0:
rx-checksumming: on
tx-checksumming: on
        tx-checksum-ipv4: on
        tx-checksum-ip-generic: off [fixed]
        tx-checksum-ipv6: off [fixed]
        tx-checksum-fcoe-crc: off [fixed]
        tx-checksum-sctp: off [fixed]
scatter-gather: on
        tx-scatter-gather: on
        tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
        tx-tcp-segmentation: off [fixed]
        tx-tcp-ecn-segmentation: off [fixed]
        tx-tcp6-segmentation: off [fixed]
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: off [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-ipip-segmentation: off [fixed]
tx-sit-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-mpls-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
l2-fwd-offload: off [fixed]
busy-poll: off [fixed]


# ethtool -S eth0
NIC statistics:
     rx_packets: 6188650
     tx_packets: 5407424
     rx_bytes: 2949700388
     tx_bytes: 1527361112
     rx_errors: 0
     tx_errors: 0
     rx_dropped: 0
     tx_dropped: 0
     good_octets_received: 9696302398
     bad_octets_received: 0
     internal_mac_transmit_err: 0
     good_frames_received: 7961482
     bad_frames_received: 0
     broadcast_frames_received: 45063
     multicast_frames_received: 35667
     frames_64_octets: 390741
     frames_65_to_127_octets: 1213008
     frames_128_to_255_octets: 1159241
     frames_256_to_511_octets: 10833
     frames_512_to_1023_octets: 1123895
     frames_1024_to_max_octets: 9471171
     good_octets_sent: 5851110022
     good_frames_sent: 5407407
     excessive_collision: 0
     multicast_frames_sent: 35326
     broadcast_frames_sent: 109099
     unrec_mac_control_received: 0
     fc_sent: 0
     good_fc_received: 0
     bad_fc_received: 0
     undersize_received: 0
     fragments_received: 0
     oversize_received: 0
     jabber_received: 0
     mac_receive_error: 0
     bad_crc_event: 0
     collision: 0
     late_collision: 0
     rx_discard: 1772832
     rx_overrun: 0


Why are so many packets being discarded?


========= UDP ($extra = -u) ====================
Connecting to host sheevaplug, port 3740
[  4] local 192.168.0.139 port 46162 connected to 192.168.0.70 port 3740
[ ID] Interval           Transfer     Bandwidth       Total Datagrams
[  4]   0.00-1.00   sec   114 MBytes   959 Mbits/sec  14640  
[  4]   1.00-2.00   sec   114 MBytes   958 Mbits/sec  14620  
[  4]   2.00-3.00   sec   114 MBytes   958 Mbits/sec  14620  
[  4]   3.00-4.00   sec   114 MBytes   958 Mbits/sec  14620  
[  4]   4.00-5.00   sec   114 MBytes   958 Mbits/sec  14630  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  4]   0.00-5.00   sec   571 MBytes   958 Mbits/sec  10.405 ms  72415/72673 (1e+02%)  
[  4] Sent 72673 datagrams

iperf Done.

-----------------------------------------------------------
Server listening on 3740
-----------------------------------------------------------
Accepted connection from 192.168.0.139, port 45461
[  5] local 192.168.0.70 port 3740 connected to 192.168.0.139 port 46162
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  5]   0.00-1.07   sec   368 KBytes  2.83 Mbits/sec  80.399 ms  5086/5132 (99%)  
[  5]   1.07-2.01   sec   280 KBytes  2.44 Mbits/sec  42.903 ms  16726/16761 (1e+02%)  
[  5]   2.01-3.02   sec   408 KBytes  3.29 Mbits/sec  20.992 ms  15643/15694 (1e+02%)  
[  5]   3.02-4.03   sec   392 KBytes  3.17 Mbits/sec  25.503 ms  15140/15189 (1e+02%)  
[  5]   4.03-5.02   sec   488 KBytes  4.07 Mbits/sec  11.793 ms  17642/17703 (1e+02%)  
[  5]   5.02-5.24   sec   128 KBytes  4.69 Mbits/sec  10.405 ms  2178/2194 (99%)  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  5]   0.00-5.24   sec   571 MBytes   915 Mbits/sec  10.405 ms  72415/72673 (1e+02%)  


========= UDP reverse ($extra = -u -R) ============
Connecting to host sheevaplug, port 3740
Reverse mode, remote host sheevaplug is sending
[  4] local 192.168.0.139 port 48931 connected to 192.168.0.70 port 3740
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  4]   0.00-1.00   sec  55.0 MBytes   462 Mbits/sec  0.083 ms  320/7366 (4.3%)  
[  4]   1.00-2.00   sec  57.5 MBytes   482 Mbits/sec  0.132 ms  0/7362 (0%)  
[  4]   2.00-3.00   sec  57.0 MBytes   478 Mbits/sec  0.079 ms  0/7295 (0%)  
[  4]   3.00-4.00   sec  58.1 MBytes   488 Mbits/sec  0.088 ms  0/7439 (0%)  
[  4]   4.00-5.00   sec  58.2 MBytes   488 Mbits/sec  0.073 ms  0/7446 (0%)  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  4]   0.00-5.00   sec   288 MBytes   484 Mbits/sec  0.079 ms  320/36920 (0.87%)  
[  4] Sent 36920 datagrams

iperf Done.

-----------------------------------------------------------
Server listening on 3740
-----------------------------------------------------------
Accepted connection from 192.168.0.139, port 45462
[  5] local 192.168.0.70 port 3740 connected to 192.168.0.139 port 48931
[ ID] Interval           Transfer     Bandwidth       Total Datagrams
[  5]   0.00-1.00   sec  55.7 MBytes   467 Mbits/sec  7130  
[  5]   1.00-2.00   sec  56.9 MBytes   477 Mbits/sec  7280  
[  5]   2.00-3.00   sec  57.0 MBytes   478 Mbits/sec  7290  
[  5]   3.00-4.00   sec  58.2 MBytes   488 Mbits/sec  7450  
[  5]   4.00-5.00   sec  58.1 MBytes   488 Mbits/sec  7440  
[  5]   5.00-5.05   sec  2.58 MBytes   483 Mbits/sec  330  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Jitter    Lost/Total Datagrams
[  5]   0.00-5.05   sec   288 MBytes   479 Mbits/sec  0.079 ms  320/36920 (0.87%)  


========= TCP ($extra = ) ====================
Connecting to host sheevaplug, port 3740
[  4] local 192.168.0.139 port 45464 connected to 192.168.0.70 port 3740
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  68.6 MBytes   575 Mbits/sec    0    361 KBytes       
[  4]   1.00-2.00   sec  79.6 MBytes   669 Mbits/sec    0    451 KBytes       
[  4]   2.00-3.00   sec  80.1 MBytes   672 Mbits/sec    0    451 KBytes       
[  4]   3.00-4.00   sec  80.8 MBytes   678 Mbits/sec    0    451 KBytes       
[  4]   4.00-5.00   sec  81.0 MBytes   678 Mbits/sec    0    451 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-5.00   sec   390 MBytes   654 Mbits/sec    0             sender
[  4]   0.00-5.00   sec   387 MBytes   649 Mbits/sec                  receiver

iperf Done.

-----------------------------------------------------------
Server listening on 3740
-----------------------------------------------------------
Accepted connection from 192.168.0.139, port 45463
[  5] local 192.168.0.70 port 3740 connected to 192.168.0.139 port 45464
[ ID] Interval           Transfer     Bandwidth
[  5]   0.00-1.00   sec  67.0 MBytes   561 Mbits/sec                  
[  5]   1.00-2.00   sec  77.0 MBytes   646 Mbits/sec                  
[  5]   2.00-3.00   sec  80.1 MBytes   671 Mbits/sec                  
[  5]   3.00-4.00   sec  80.8 MBytes   678 Mbits/sec                  
[  5]   4.00-5.00   sec  80.9 MBytes   679 Mbits/sec                  
[  5]   5.00-5.02   sec  1.00 MBytes   595 Mbits/sec                  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  5]   0.00-5.02   sec   390 MBytes   652 Mbits/sec    0             sender
[  5]   0.00-5.02   sec   387 MBytes   647 Mbits/sec                  receiver


========= TCP reverse ($extra = -R) ============
Connecting to host sheevaplug, port 3740
Reverse mode, remote host sheevaplug is sending
[  4] local 192.168.0.139 port 45466 connected to 192.168.0.70 port 3740
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec  72.2 MBytes   605 Mbits/sec                  
[  4]   1.00-2.00   sec  72.0 MBytes   604 Mbits/sec                  
[  4]   2.00-3.00   sec  72.2 MBytes   606 Mbits/sec                  
[  4]   3.00-4.00   sec  73.8 MBytes   619 Mbits/sec                  
[  4]   4.00-5.00   sec  73.5 MBytes   616 Mbits/sec                  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-5.00   sec   364 MBytes   611 Mbits/sec    0             sender
[  4]   0.00-5.00   sec   364 MBytes   611 Mbits/sec                  receiver

iperf Done.

-----------------------------------------------------------
Server listening on 3740
-----------------------------------------------------------
Accepted connection from 192.168.0.139, port 45465
[  5] local 192.168.0.70 port 3740 connected to 192.168.0.139 port 45466
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  5]   0.00-1.02   sec  73.0 MBytes   599 Mbits/sec    0    257 KBytes       
[  5]   1.02-2.03   sec  72.5 MBytes   604 Mbits/sec    0    257 KBytes       
[  5]   2.03-3.03   sec  72.5 MBytes   606 Mbits/sec    0    257 KBytes       
[  5]   3.03-4.03   sec  73.8 MBytes   619 Mbits/sec    0    257 KBytes       
[  5]   4.03-5.02   sec  72.5 MBytes   616 Mbits/sec    0    257 KBytes       
[  5]   5.02-5.02   sec  0.00 Bytes  0.00 bits/sec    0    257 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  5]   0.00-5.02   sec   364 MBytes   609 Mbits/sec    0             sender
[  5]   0.00-5.02   sec   364 MBytes   609 Mbits/sec                  receiver

^ permalink raw reply

* [PATCH net 0/6] openvswitch: datapath fixes
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

Following patch series is mostly targeted to MPLS fixes. other
patches are related datapth transmit path error handling. 

Pravin B Shelar (6):
  mpls: Fix config check for mpls.
  mpls: Fix allowed protocols for mpls gso
  openvswitch: Fix MPLS action validation.
  openvswitch: Fix GSO with multiple MPLS label.
  openvswitch: Fix vport_send double free
  vxlan: Fix double free of skb.

 drivers/net/vxlan.c            |   34 ++++++++++++++++++++++++----------
 net/core/dev.c                 |    2 +-
 net/ipv4/geneve.c              |    6 +++++-
 net/mpls/mpls_gso.c            |    5 +----
 net/openvswitch/actions.c      |    3 ++-
 net/openvswitch/flow_netlink.c |   13 +------------
 net/openvswitch/vport-geneve.c |    3 +++
 net/openvswitch/vport-gre.c    |   18 +++++++++++-------
 net/openvswitch/vport-vxlan.c  |    2 ++
 net/openvswitch/vport.c        |    5 ++---
 10 files changed, 52 insertions(+), 39 deletions(-)

^ permalink raw reply

* [PATCH net 1/6] mpls: Fix config check for mpls.
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

Fixes MPLS GSO for case when mpls is compiled as kernel module.

Fixes: 0d89d2035f ("MPLS: Add limited GSO support").
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/core/dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index f411c28..fa621fd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2522,7 +2522,7 @@ static int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
 /* If MPLS offload request, verify we are testing hardware MPLS features
  * instead of standard features for the netdev.
  */
-#ifdef CONFIG_NET_MPLS_GSO
+#if IS_ENABLED(CONFIG_NET_MPLS_GSO)
 static netdev_features_t net_mpls_features(struct sk_buff *skb,
 					   netdev_features_t features,
 					   __be16 type)
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 3/6] openvswitch: Fix MPLS action validation.
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

Linux stack does not implement GSO for packet with multiple
encapsulations.  Therefore there was check in MPLS action
validation to detect such case, But this check introduced
bug which deleted one or more actions from actions list.
Following patch removes this check to fix the validation.

Fixes: 25cd9ba0abc ("openvswitch: Add basic MPLS support to
kernel").

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Reported-by: Srinivas Neginhal <sneginha@vmware.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
---
 net/openvswitch/flow_netlink.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 9645a21..d1eecf7 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -1753,7 +1753,6 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 				  __be16 eth_type, __be16 vlan_tci, bool log)
 {
 	const struct nlattr *a;
-	bool out_tnl_port = false;
 	int rem, err;
 
 	if (depth >= SAMPLE_ACTION_DEPTH)
@@ -1796,8 +1795,6 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 		case OVS_ACTION_ATTR_OUTPUT:
 			if (nla_get_u32(a) >= DP_MAX_PORTS)
 				return -EINVAL;
-			out_tnl_port = false;
-
 			break;
 
 		case OVS_ACTION_ATTR_HASH: {
@@ -1832,12 +1829,6 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 		case OVS_ACTION_ATTR_PUSH_MPLS: {
 			const struct ovs_action_push_mpls *mpls = nla_data(a);
 
-			/* Networking stack do not allow simultaneous Tunnel
-			 * and MPLS GSO.
-			 */
-			if (out_tnl_port)
-				return -EINVAL;
-
 			if (!eth_p_mpls(mpls->mpls_ethertype))
 				return -EINVAL;
 			/* Prohibit push MPLS other than to a white list
@@ -1873,11 +1864,9 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 
 		case OVS_ACTION_ATTR_SET:
 			err = validate_set(a, key, sfa,
-					   &out_tnl_port, eth_type, log);
+					   &skip_copy, eth_type, log);
 			if (err)
 				return err;
-
-			skip_copy = out_tnl_port;
 			break;
 
 		case OVS_ACTION_ATTR_SAMPLE:
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 2/6] mpls: Fix allowed protocols for mpls gso
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

MPLS and Tunnel GSO does not work together.  Reject packet which
request such GSO.

Fixes: 0d89d2035f ("MPLS: Add limited GSO support").
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/mpls/mpls_gso.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
index ca27837..349295d 100644
--- a/net/mpls/mpls_gso.c
+++ b/net/mpls/mpls_gso.c
@@ -31,10 +31,7 @@ static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
 				  SKB_GSO_TCPV6 |
 				  SKB_GSO_UDP |
 				  SKB_GSO_DODGY |
-				  SKB_GSO_TCP_ECN |
-				  SKB_GSO_GRE |
-				  SKB_GSO_GRE_CSUM |
-				  SKB_GSO_IPIP)))
+				  SKB_GSO_TCP_ECN)))
 		goto out;
 
 	/* Setup inner SKB. */
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 4/6] openvswitch: Fix GSO with multiple MPLS label.
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

MPLS GSO needs to know inner most protocol to process GSO packets.

Fixes: 25cd9ba0abc ("openvswitch: Add basic MPLS support to
kernel").

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/openvswitch/actions.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 764fdc3..770064c 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -147,7 +147,8 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	hdr = eth_hdr(skb);
 	hdr->h_proto = mpls->mpls_ethertype;
 
-	skb_set_inner_protocol(skb, skb->protocol);
+	if (!skb->inner_protocol)
+		skb_set_inner_protocol(skb, skb->protocol);
 	skb->protocol = mpls->mpls_ethertype;
 
 	invalidate_flow_key(key);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 5/6] openvswitch: Fix vport_send double free
From: Pravin B Shelar @ 2014-12-24  0:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

Today vport-send has complex error handling because it involves
freeing skb and updating stats depending on return value from
vport send implementation.
This can be simplified by delegating responsibility of freeing
skb to the vport implementation for all cases. So that
vport-send needs just update stats.

Fixes: 91b7514cdf ("openvswitch: Unify vport error stats
handling")
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/ipv4/geneve.c              |    6 +++++-
 net/openvswitch/vport-geneve.c |    3 +++
 net/openvswitch/vport-gre.c    |   18 +++++++++++-------
 net/openvswitch/vport-vxlan.c  |    2 ++
 net/openvswitch/vport.c        |    5 ++---
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 95e47c9..394a200 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -122,14 +122,18 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 	int err;
 
 	skb = udp_tunnel_handle_offloads(skb, !gs->sock->sk->sk_no_check_tx);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
 
 	min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
 			+ GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr)
 			+ (vlan_tx_tag_present(skb) ? VLAN_HLEN : 0);
 
 	err = skb_cow_head(skb, min_headroom);
-	if (unlikely(err))
+	if (unlikely(err)) {
+		kfree_skb(skb);
 		return err;
+	}
 
 	skb = vlan_hwaccel_push_inside(skb);
 	if (unlikely(!skb))
diff --git a/net/openvswitch/vport-geneve.c b/net/openvswitch/vport-geneve.c
index 347fa23..484864d 100644
--- a/net/openvswitch/vport-geneve.c
+++ b/net/openvswitch/vport-geneve.c
@@ -219,7 +219,10 @@ static int geneve_tnl_send(struct vport *vport, struct sk_buff *skb)
 			      false);
 	if (err < 0)
 		ip_rt_put(rt);
+	return err;
+
 error:
+	kfree_skb(skb);
 	return err;
 }
 
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 6b69df5..28f54e9 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -73,7 +73,7 @@ static struct sk_buff *__build_header(struct sk_buff *skb,
 
 	skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM));
 	if (IS_ERR(skb))
-		return NULL;
+		return skb;
 
 	tpi.flags = filter_tnl_flags(tun_key->tun_flags);
 	tpi.proto = htons(ETH_P_TEB);
@@ -144,7 +144,7 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 
 	if (unlikely(!OVS_CB(skb)->egress_tun_info)) {
 		err = -EINVAL;
-		goto error;
+		goto err_free_skb;
 	}
 
 	tun_key = &OVS_CB(skb)->egress_tun_info->tunnel;
@@ -157,8 +157,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 	fl.flowi4_proto = IPPROTO_GRE;
 
 	rt = ip_route_output_key(net, &fl);
-	if (IS_ERR(rt))
-		return PTR_ERR(rt);
+	if (IS_ERR(rt)) {
+		err = PTR_ERR(rt);
+		goto err_free_skb;
+	}
 
 	tunnel_hlen = ip_gre_calc_hlen(tun_key->tun_flags);
 
@@ -183,8 +185,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 
 	/* Push Tunnel header. */
 	skb = __build_header(skb, tunnel_hlen);
-	if (unlikely(!skb)) {
-		err = 0;
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(rt);
+		skb = NULL;
 		goto err_free_rt;
 	}
 
@@ -198,7 +201,8 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 			     tun_key->ipv4_tos, tun_key->ipv4_ttl, df, false);
 err_free_rt:
 	ip_rt_put(rt);
-error:
+err_free_skb:
+	kfree_skb(skb);
 	return err;
 }
 
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index 38f95a5..d7c46b3 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -187,7 +187,9 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
 			     false);
 	if (err < 0)
 		ip_rt_put(rt);
+	return err;
 error:
+	kfree_skb(skb);
 	return err;
 }
 
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 9584526..53f3ebb 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -519,10 +519,9 @@ int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
 		u64_stats_update_end(&stats->syncp);
 	} else if (sent < 0) {
 		ovs_vport_record_error(vport, VPORT_E_TX_ERROR);
-		kfree_skb(skb);
-	} else
+	} else {
 		ovs_vport_record_error(vport, VPORT_E_TX_DROPPED);
-
+	}
 	return sent;
 }
 
-- 
1.7.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox