Netdev List
 help / color / mirror / Atom feed
* [PATCHv3 net-next 2/4] caif: Add support for CAIF over CDC NCM USB interface
From: Sjur Brændeland @ 2011-12-04 21:22 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: Alexey Orishko, Eric Dumazet, Sjur Brændeland
In-Reply-To: <1323033775-3496-1-git-send-email-sjur.brandeland@stericsson.com>

NCM 1.0 does not support anything but Ethernet framing, hence
CAIF payload will be put into Ethernet frames.

Discovery is based on fixed USB vendor 0x04cc (ST-Ericsson),
product-id 0x230f (NCM). In this variant only CAIF payload is sent over
the NCM interface.

The CAIF stack (cfusbl.c) will when USB interface register first check if
we got a CDC NCM USB interface with the right VID, PID.
It will then read the device's Ethernet address and create a 'template'
Ethernet TX header, using a broadcast address as the destination address,
and EthType 0x88b5 (802.1 Local Experimental - vendor specific).

A protocol handler for 0x88b5 is setup for reception of CAIF frames from
the CDC NCM USB interface.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
Hi Dave,

>Please make this feature a tristate and make the init and exit
>routines executed via the normal modula_init()/module_exit()
>mechanisms.
>That way you don't need that dopey header file to export the
>init and exit routines, and you don't need to ifdef crap up the
>code CAIF code to invoke them.
Done.

Changes from V2:
o As suggested: Killed of header file and ifdefs, and made it tristate
o Renamed module name to caif_usb
o Exported a few functions
o Had to handle refcnt for caif_usb module when USB NCM device registers 
  and unregisters.

Regards,
Sjur

 net/caif/Kconfig        |   11 +++
 net/caif/Makefile       |    1 +
 net/caif/caif_dev.c     |    1 +
 net/caif/caif_usb.c     |  208 +++++++++++++++++++++++++++++++++++++++++++++++
 net/caif/cfpkt_skbuff.c |   12 +--
 5 files changed, 224 insertions(+), 9 deletions(-)
 create mode 100644 net/caif/caif_usb.c

diff --git a/net/caif/Kconfig b/net/caif/Kconfig
index 529750d..936361e 100644
--- a/net/caif/Kconfig
+++ b/net/caif/Kconfig
@@ -40,3 +40,14 @@ config CAIF_NETDEV
 	If you select to build it as a built-in then the main CAIF device must
 	also be a built-in.
 	If unsure say Y.
+
+config CAIF_USB
+	tristate "CAIF USB support"
+	depends on CAIF
+	default n
+	---help---
+	Say Y if you are using CAIF over USB CDC NCM.
+	This can be either built-in or a loadable module,
+	If you select to build it as a built-in then the main CAIF device must
+	also be a built-in.
+	If unsure say N.
diff --git a/net/caif/Makefile b/net/caif/Makefile
index ebcd4e7..cc2b511 100644
--- a/net/caif/Makefile
+++ b/net/caif/Makefile
@@ -10,5 +10,6 @@ caif-y := caif_dev.o \
 obj-$(CONFIG_CAIF) += caif.o
 obj-$(CONFIG_CAIF_NETDEV) += chnl_net.o
 obj-$(CONFIG_CAIF) += caif_socket.o
+obj-$(CONFIG_CAIF_USB) += caif_usb.o
 
 export-y := caif.o
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index f7e8c70..6acec19 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -262,6 +262,7 @@ void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
 	if (rcv_func)
 		*rcv_func = receive;
 }
+EXPORT_SYMBOL(caif_enroll_dev);
 
 /* notify Caif of device events */
 static int caif_device_notify(struct notifier_block *me, unsigned long what,
diff --git a/net/caif/caif_usb.c b/net/caif/caif_usb.c
new file mode 100644
index 0000000..f5db57c
--- /dev/null
+++ b/net/caif/caif_usb.c
@@ -0,0 +1,208 @@
+/*
+ * CAIF USB handler
+ * Copyright (C) ST-Ericsson AB 2011
+ * Author:	Sjur Brendeland/sjur.brandeland@stericsson.com
+ * License terms: GNU General Public License (GPL) version 2
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s(): " fmt, __func__
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <linux/netdevice.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/usbnet.h>
+#include <net/netns/generic.h>
+#include <net/caif/caif_dev.h>
+#include <net/caif/caif_layer.h>
+#include <net/caif/cfpkt.h>
+#include <net/caif/cfcnfg.h>
+
+MODULE_LICENSE("GPL");
+
+#define CFUSB_PAD_DESCR_SZ 1	/* Alignment descriptor length */
+#define CFUSB_ALIGNMENT 4	/* Number of bytes to align. */
+#define CFUSB_MAX_HEADLEN (CFUSB_PAD_DESCR_SZ + CFUSB_ALIGNMENT-1)
+#define STE_USB_VID 0x04cc	/* USB Product ID for ST-Ericsson */
+#define STE_USB_PID_CAIF 0x2306	/* Product id for CAIF Modems */
+
+struct cfusbl {
+	struct cflayer layer;
+	u8 tx_eth_hdr[ETH_HLEN];
+};
+
+static bool pack_added;
+
+static int cfusbl_receive(struct cflayer *layr, struct cfpkt *pkt)
+{
+	u8 hpad;
+
+	/* Remove padding. */
+	cfpkt_extr_head(pkt, &hpad, 1);
+	cfpkt_extr_head(pkt, NULL, hpad);
+	return layr->up->receive(layr->up, pkt);
+}
+
+static int cfusbl_transmit(struct cflayer *layr, struct cfpkt *pkt)
+{
+	struct caif_payload_info *info;
+	u8 hpad;
+	u8 zeros[CFUSB_ALIGNMENT];
+	struct sk_buff *skb;
+	struct cfusbl *usbl = container_of(layr, struct cfusbl, layer);
+
+	skb = cfpkt_tonative(pkt);
+
+	skb_reset_network_header(skb);
+	skb->protocol = htons(ETH_P_IP);
+
+	info = cfpkt_info(pkt);
+	hpad = (info->hdr_len + CFUSB_PAD_DESCR_SZ) & (CFUSB_ALIGNMENT - 1);
+
+	if (skb_headroom(skb) < ETH_HLEN + CFUSB_PAD_DESCR_SZ + hpad) {
+		pr_warn("Headroom to small\n");
+		kfree_skb(skb);
+		return -EIO;
+	}
+	memset(zeros, 0, hpad);
+
+	cfpkt_add_head(pkt, zeros, hpad);
+	cfpkt_add_head(pkt, &hpad, 1);
+	cfpkt_add_head(pkt, usbl->tx_eth_hdr, sizeof(usbl->tx_eth_hdr));
+	return layr->dn->transmit(layr->dn, pkt);
+}
+
+static void cfusbl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
+					int phyid)
+{
+	if (layr->up && layr->up->ctrlcmd)
+		layr->up->ctrlcmd(layr->up, ctrl, layr->id);
+}
+
+struct cflayer *cfusbl_create(int phyid, u8 ethaddr[ETH_ALEN],
+					u8 braddr[ETH_ALEN])
+{
+	struct cfusbl *this = kmalloc(sizeof(struct cfusbl), GFP_ATOMIC);
+
+	if (!this) {
+		pr_warn("Out of memory\n");
+		return NULL;
+	}
+	caif_assert(offsetof(struct cfusbl, layer) == 0);
+
+	memset(this, 0, sizeof(struct cflayer));
+	this->layer.receive = cfusbl_receive;
+	this->layer.transmit = cfusbl_transmit;
+	this->layer.ctrlcmd = cfusbl_ctrlcmd;
+	snprintf(this->layer.name, CAIF_LAYER_NAME_SZ, "usb%d", phyid);
+	this->layer.id = phyid;
+
+	/*
+	 * Construct TX ethernet header:
+	 *	0-5	destination address
+	 *	5-11	source address
+	 *	12-13	protocol type
+	 */
+	memcpy(&this->tx_eth_hdr[ETH_ALEN], braddr, ETH_ALEN);
+	memcpy(&this->tx_eth_hdr[ETH_ALEN], ethaddr, ETH_ALEN);
+	this->tx_eth_hdr[12] = cpu_to_be16(ETH_P_802_EX1) & 0xff;
+	this->tx_eth_hdr[13] = (cpu_to_be16(ETH_P_802_EX1) >> 8) & 0xff;
+	pr_debug("caif ethernet TX-header dst:%pM src:%pM type:%02x%02x\n",
+			this->tx_eth_hdr, this->tx_eth_hdr + ETH_ALEN,
+			this->tx_eth_hdr[12], this->tx_eth_hdr[13]);
+
+	return (struct cflayer *) this;
+}
+
+static struct packet_type caif_usb_type __read_mostly = {
+	.type = cpu_to_be16(ETH_P_802_EX1),
+};
+
+static int cfusbl_device_notify(struct notifier_block *me, unsigned long what,
+			      void *arg)
+{
+	struct net_device *dev = arg;
+	struct caif_dev_common common;
+	struct cflayer *layer, *link_support;
+	struct usbnet	*usbnet = netdev_priv(dev);
+	struct usb_device	*usbdev = usbnet->udev;
+	struct ethtool_drvinfo drvinfo;
+
+	/*
+	 * Quirks: High-jack ethtool to find if we have a NCM device,
+	 * and find it's VID/PID.
+	 */
+	if (dev->ethtool_ops == NULL || dev->ethtool_ops->get_drvinfo == NULL)
+		return 0;
+
+	dev->ethtool_ops->get_drvinfo(dev, &drvinfo);
+	if (strncmp(drvinfo.driver, "cdc_ncm", 7) != 0)
+		return 0;
+
+	pr_debug("USB CDC NCM device VID:0x%4x PID:0x%4x\n",
+		le16_to_cpu(usbdev->descriptor.idVendor),
+		le16_to_cpu(usbdev->descriptor.idProduct));
+
+	/* Check for VID/PID that supports CAIF */
+	if (!(le16_to_cpu(usbdev->descriptor.idVendor) == STE_USB_VID &&
+		le16_to_cpu(usbdev->descriptor.idProduct) == STE_USB_PID_CAIF))
+		return 0;
+
+	if (what == NETDEV_UNREGISTER)
+		module_put(THIS_MODULE);
+
+	if (what != NETDEV_REGISTER)
+		return 0;
+
+	__module_get(THIS_MODULE);
+
+	memset(&common, 0, sizeof(common));
+	common.use_frag = false;
+	common.use_fcs = false;
+	common.use_stx = false;
+	common.link_select = CAIF_LINK_HIGH_BANDW;
+	common.flowctrl = NULL;
+
+	link_support = cfusbl_create(dev->ifindex, dev->dev_addr,
+					dev->broadcast);
+
+	if (!link_support)
+		return -ENOMEM;
+
+	if (dev->num_tx_queues > 1)
+		pr_warn("USB device uses more than one tx queue\n");
+
+	caif_enroll_dev(dev, &common, link_support, CFUSB_MAX_HEADLEN,
+			&layer, &caif_usb_type.func);
+	if (!pack_added)
+		dev_add_pack(&caif_usb_type);
+	pack_added = 1;
+
+	strncpy(layer->name, dev->name,
+			sizeof(layer->name) - 1);
+	layer->name[sizeof(layer->name) - 1] = 0;
+
+	return 0;
+}
+
+static struct notifier_block caif_device_notifier = {
+	.notifier_call = cfusbl_device_notify,
+	.priority = 0,
+};
+
+static int __init cfusbl_init(void)
+{
+	return register_netdevice_notifier(&caif_device_notifier);
+}
+
+static void __exit cfusbl_exit(void)
+{
+	unregister_netdevice_notifier(&caif_device_notifier);
+	dev_remove_pack(&caif_usb_type);
+}
+
+module_init(cfusbl_init);
+module_exit(cfusbl_exit);
diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index de53907..e335ba8 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -63,7 +63,6 @@ static inline struct cfpkt *skb_to_pkt(struct sk_buff *skb)
 	return (struct cfpkt *) skb;
 }
 
-
 struct cfpkt *cfpkt_fromnative(enum caif_direction dir, void *nativepkt)
 {
 	struct cfpkt *pkt = skb_to_pkt(nativepkt);
@@ -105,14 +104,12 @@ void cfpkt_destroy(struct cfpkt *pkt)
 	kfree_skb(skb);
 }
 
-
 inline bool cfpkt_more(struct cfpkt *pkt)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
 	return skb->len > 0;
 }
 
-
 int cfpkt_peek_head(struct cfpkt *pkt, void *data, u16 len)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
@@ -148,6 +145,7 @@ int cfpkt_extr_head(struct cfpkt *pkt, void *data, u16 len)
 		memcpy(data, from, len);
 	return 0;
 }
+EXPORT_SYMBOL(cfpkt_extr_head);
 
 int cfpkt_extr_trail(struct cfpkt *pkt, void *dta, u16 len)
 {
@@ -171,13 +169,11 @@ int cfpkt_extr_trail(struct cfpkt *pkt, void *dta, u16 len)
 	return 0;
 }
 
-
 int cfpkt_pad_trail(struct cfpkt *pkt, u16 len)
 {
 	return cfpkt_add_body(pkt, NULL, len);
 }
 
-
 int cfpkt_add_body(struct cfpkt *pkt, const void *data, u16 len)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
@@ -256,21 +252,19 @@ int cfpkt_add_head(struct cfpkt *pkt, const void *data2, u16 len)
 	memcpy(to, data, len);
 	return 0;
 }
-
+EXPORT_SYMBOL(cfpkt_add_head);
 
 inline int cfpkt_add_trail(struct cfpkt *pkt, const void *data, u16 len)
 {
 	return cfpkt_add_body(pkt, data, len);
 }
 
-
 inline u16 cfpkt_getlen(struct cfpkt *pkt)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
 	return skb->len;
 }
 
-
 inline u16 cfpkt_iterate(struct cfpkt *pkt,
 			    u16 (*iter_func)(u16, void *, u16),
 			    u16 data)
@@ -288,7 +282,6 @@ inline u16 cfpkt_iterate(struct cfpkt *pkt,
 	return iter_func(data, pkt->skb.data, cfpkt_getlen(pkt));
 }
 
-
 int cfpkt_setlen(struct cfpkt *pkt, u16 len)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
@@ -400,3 +393,4 @@ struct caif_payload_info *cfpkt_info(struct cfpkt *pkt)
 {
 	return (struct caif_payload_info *)&pkt_to_skb(pkt)->cb;
 }
+EXPORT_SYMBOL(cfpkt_info);
-- 
1.7.0.4

^ permalink raw reply related

* [PATCHv3 net-next 3/4] caif: Add support for flow-control on device's tx-queue
From: Sjur Brændeland @ 2011-12-04 21:22 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: Alexey Orishko, Eric Dumazet, Sjur Brændeland
In-Reply-To: <1323033775-3496-1-git-send-email-sjur.brandeland@stericsson.com>

Flow control is implemented by inspecting the qdisc queue length
in order to detect potential overflow on the TX queue. When a threshold
is reached flow-off is sent upwards in the CAIF stack. At the same time
the skb->destructor is hi-jacked by orphaning the SKB and the original
destructor is replaced with a "flow-on" callback. When the "hi-jacked"
SKB is consumed the queue should be empty, and the "flow-on" callback
is called and xon is sent upwards in the CAIF stack.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_dev.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 6acec19..74c1273 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -17,6 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/mutex.h>
 #include <linux/module.h>
+#include <linux/spinlock.h>
 #include <net/netns/generic.h>
 #include <net/net_namespace.h>
 #include <net/pkt_sched.h>
@@ -34,6 +35,8 @@ struct caif_device_entry {
 	struct list_head list;
 	struct net_device *netdev;
 	int __percpu *pcpu_refcnt;
+	spinlock_t flow_lock;
+	bool xoff;
 };
 
 struct caif_device_entry_list {
@@ -48,6 +51,7 @@ struct caif_net {
 };
 
 static int caif_net_id;
+static int q_high = 50; /* Percent */
 
 struct cfcnfg *get_cfcnfg(struct net *net)
 {
@@ -126,17 +130,94 @@ static struct caif_device_entry *caif_get(struct net_device *dev)
 	return NULL;
 }
 
+void caif_flow_cb(struct sk_buff *skb)
+{
+	struct caif_device_entry *caifd;
+	bool send_xoff;
+
+	WARN_ON(skb->dev == NULL);
+
+	rcu_read_lock();
+	caifd = caif_get(skb->dev);
+	caifd_hold(caifd);
+	rcu_read_unlock();
+
+	spin_lock_bh(&caifd->flow_lock);
+	send_xoff = caifd->xoff;
+	caifd->xoff = 0;
+	spin_unlock_bh(&caifd->flow_lock);
+
+	if (send_xoff)
+		caifd->layer.up->
+			ctrlcmd(caifd->layer.up,
+				_CAIF_CTRLCMD_PHYIF_FLOW_ON_IND,
+				caifd->layer.id);
+	caifd_put(caifd);
+}
+
 static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 {
-	int err;
+	int err, high = 0, qlen = 0;
+	struct caif_dev_common *caifdev;
 	struct caif_device_entry *caifd =
 	    container_of(layer, struct caif_device_entry, layer);
 	struct sk_buff *skb;
+	struct netdev_queue *txq;
+
+	rcu_read_lock_bh();
 
 	skb = cfpkt_tonative(pkt);
 	skb->dev = caifd->netdev;
 	skb_reset_network_header(skb);
 	skb->protocol = htons(ETH_P_CAIF);
+	caifdev = netdev_priv(caifd->netdev);
+
+	/* Check if we need to handle xoff */
+	if (likely(caifd->netdev->tx_queue_len == 0))
+		goto noxoff;
+
+	if (unlikely(caifd->xoff))
+		goto noxoff;
+
+	if (likely(!netif_queue_stopped(caifd->netdev))) {
+		/* If we run with a TX queue, check if the queue is too long*/
+		txq = netdev_get_tx_queue(skb->dev, 0);
+		qlen = qdisc_qlen(rcu_dereference_bh(txq->qdisc));
+
+		if (likely(qlen == 0))
+			goto noxoff;
+
+		high = (caifd->netdev->tx_queue_len * q_high) / 100;
+		if (likely(qlen < high))
+			goto noxoff;
+	}
+
+	/* Hold lock while accessing xoff */
+	spin_lock_bh(&caifd->flow_lock);
+	if (caifd->xoff) {
+		spin_unlock_bh(&caifd->flow_lock);
+		goto noxoff;
+	}
+
+	/*
+	 * Handle flow off, we do this by temporary hi-jacking this
+	 * skb's destructor function, and replace it with our own
+	 * flow-on callback. The callback will set flow-on and call
+	 * the original destructor.
+	 */
+
+	pr_debug("queue has stopped(%d) or is full (%d > %d)\n",
+			netif_queue_stopped(caifd->netdev),
+			qlen, high);
+	caifd->xoff = 1;
+	spin_unlock_bh(&caifd->flow_lock);
+	skb_orphan(skb);
+
+	caifd->layer.up->ctrlcmd(caifd->layer.up,
+					_CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND,
+					caifd->layer.id);
+noxoff:
+	rcu_read_unlock_bh();
 
 	err = dev_queue_xmit(skb);
 	if (err > 0)
@@ -232,6 +313,7 @@ void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
 	if (!caifd)
 		return;
 	*layer = &caifd->layer;
+	spin_lock_init(&caifd->flow_lock);
 
 	switch (caifdev->link_select) {
 	case CAIF_LINK_HIGH_BANDW:
@@ -316,6 +398,7 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
 			break;
 		}
 
+		caifd->xoff = 0;
 		cfcnfg_set_phy_state(cfg, &caifd->layer, true);
 		rcu_read_unlock();
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCHv3 net-next 4/4] caif: Stash away hijacked skb destructor and call it later
From: Sjur Brændeland @ 2011-12-04 21:22 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: Alexey Orishko, Eric Dumazet, Sjur Brændeland
In-Reply-To: <1323033775-3496-1-git-send-email-sjur.brandeland@stericsson.com>

This patch adds functionality for avoiding orphaning SKB too early.
The original skb is stashed away and the original destructor is called
from the hi-jacked flow-on callback. If CAIF interface goes down and a
hi-jacked SKB exists, the original skb->destructor is restored.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_dev.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 74c1273..9b298c1 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -36,6 +36,8 @@ struct caif_device_entry {
 	struct net_device *netdev;
 	int __percpu *pcpu_refcnt;
 	spinlock_t flow_lock;
+	struct sk_buff *xoff_skb;
+	void (*xoff_skb_dtor)(struct sk_buff *skb);
 	bool xoff;
 };
 
@@ -133,6 +135,7 @@ static struct caif_device_entry *caif_get(struct net_device *dev)
 void caif_flow_cb(struct sk_buff *skb)
 {
 	struct caif_device_entry *caifd;
+	void (*dtor)(struct sk_buff *skb) = NULL;
 	bool send_xoff;
 
 	WARN_ON(skb->dev == NULL);
@@ -145,8 +148,17 @@ void caif_flow_cb(struct sk_buff *skb)
 	spin_lock_bh(&caifd->flow_lock);
 	send_xoff = caifd->xoff;
 	caifd->xoff = 0;
+	if (!WARN_ON(caifd->xoff_skb_dtor == NULL)) {
+		WARN_ON(caifd->xoff_skb != skb);
+		dtor = caifd->xoff_skb_dtor;
+		caifd->xoff_skb = NULL;
+		caifd->xoff_skb_dtor = NULL;
+	}
 	spin_unlock_bh(&caifd->flow_lock);
 
+	if (dtor)
+		dtor(skb);
+
 	if (send_xoff)
 		caifd->layer.up->
 			ctrlcmd(caifd->layer.up,
@@ -210,8 +222,10 @@ static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 			netif_queue_stopped(caifd->netdev),
 			qlen, high);
 	caifd->xoff = 1;
+	caifd->xoff_skb = skb;
+	caifd->xoff_skb_dtor = skb->destructor;
+	skb->destructor = caif_flow_cb;
 	spin_unlock_bh(&caifd->flow_lock);
-	skb_orphan(skb);
 
 	caifd->layer.up->ctrlcmd(caifd->layer.up,
 					_CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND,
@@ -420,6 +434,24 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
 		caifd->layer.up->ctrlcmd(caifd->layer.up,
 					 _CAIF_CTRLCMD_PHYIF_DOWN_IND,
 					 caifd->layer.id);
+
+		spin_lock_bh(&caifd->flow_lock);
+
+		/*
+		 * Replace our xoff-destructor with original destructor.
+		 * We trust that skb->destructor *always* is called before
+		 * the skb reference is invalid. The hijacked SKB destructor
+		 * takes the flow_lock so manipulating the skb->destructor here
+		 * should be safe.
+		*/
+		if (caifd->xoff_skb_dtor != NULL && caifd->xoff_skb != NULL)
+			caifd->xoff_skb->destructor = caifd->xoff_skb_dtor;
+
+		caifd->xoff = 0;
+		caifd->xoff_skb_dtor = NULL;
+		caifd->xoff_skb = NULL;
+
+		spin_unlock_bh(&caifd->flow_lock);
 		caifd_put(caifd);
 		break;
 
-- 
1.7.0.4

^ permalink raw reply related

* r8169 performance?
From: Chris Adams @ 2011-12-04 21:10 UTC (permalink / raw)
  To: netdev

I have a system with an on-board RealTek gigabit network interface that
is giving me poor performance.  I'm using a simple test, running "nc -l"
on one system and "dd if=/dev/zero | nc" on another.  I see about 280
Mbps transmit and 511 Mbps receive.  I checked the obvious things
(cable, switch port) but they didn't seem to be the source of the
problem (and nothing is showing any errors).

I have another system with a different motherboard (different
manufacturer) but with an onboard NIC that appears the same chip from
lspci and dmesg (and they both appear to be wired up to PCI-E).  It gets
over 900 Mbps in the same type test.

I'm running Fedora (64 bit) on my systems; I upgraded the "problem"
system to F16 with kernel 3.1.2.

Are there any suggestions for what I might be able to do to improve
throughput?  Is there a driver issue, or is it in how the motherboard
implemented the NIC?

Here's the lspci -vv output for the "problem" card.

02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit Ethernet controller (rev 03)
	Subsystem: ASRock Incorporation Motherboard (one of many)
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 42
	Region 0: I/O ports at e800 [size=256]
	Region 2: Memory at fdfff000 (64-bit, prefetchable) [size=4K]
	Region 4: Memory at fdff8000 (64-bit, prefetchable) [size=16K]
	Expansion ROM at febe0000 [disabled] [size=128K]
	Capabilities: [40] Power Management version 3
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
		Address: 00000000fee0100c  Data: 4171
	Capabilities: [70] Express (v2) Endpoint, MSI 01
		DevCap:	MaxPayload 256 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 4096 bytes
		DevSta:	CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <512ns, L1 <64us
			ClockPM+ Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Not Supported, TimeoutDis+
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
		LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -6dB
	Capabilities: [ac] MSI-X: Enable- Count=4 Masked-
		Vector table: BAR=4 offset=00000000
		PBA: BAR=4 offset=00000800
	Capabilities: [cc] Vital Product Data
		Unknown small resource type 00, will not decode more.
	Capabilities: [100 v1] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr+ BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		AERCap:	First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
	Capabilities: [140 v1] Virtual Channel
		Caps:	LPEVC=0 RefClk=100ns PATEntryBits=1
		Arb:	Fixed- WRR32- WRR64- WRR128-
		Ctrl:	ArbSelect=Fixed
		Status:	InProgress-
		VC0:	Caps:	PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
			Arb:	Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
			Ctrl:	Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
			Status:	NegoPending- InProgress-
	Capabilities: [160 v1] Device Serial Number 03-00-00-00-68-4c-e0-00
	Kernel driver in use: r8169
	Kernel modules: r8169


-- 
Chris Adams <cmadams@hiwaay.net>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.

^ permalink raw reply

* Re: r8169 performance?
From: Eric Dumazet @ 2011-12-04 21:50 UTC (permalink / raw)
  To: Chris Adams; +Cc: netdev
In-Reply-To: <20111204211007.GA32098@hiwaay.net>

Le dimanche 04 décembre 2011 à 15:10 -0600, Chris Adams a écrit :
> I have a system with an on-board RealTek gigabit network interface that
> is giving me poor performance.  I'm using a simple test, running "nc -l"
> on one system and "dd if=/dev/zero | nc" on another.  I see about 280
> Mbps transmit and 511 Mbps receive.


Do you mean that if this problematic NIC is the receiver, it receives
280Mbps, and if it is the sender, speed is 511 Mbps ?

Could you check counters ?  

netstat -s
ethtool -S ethX
cat /proc/net/softnet_stat

^ permalink raw reply

* Re: [PATCH -next 0/4] netfilter reverse path filter matches
From: Pablo Neira Ayuso @ 2011-12-04 21:51 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, netdev, laforge
In-Reply-To: <1320877188-1972-1-git-send-email-fw@strlen.de>

Hi Florian,

On Wed, Nov 09, 2011 at 11:19:44PM +0100, Florian Westphal wrote:
> Userspace part is stored in my iptables repository on
> http://git.breakpoint.cc/cgi-bin/gitweb.cgi?p=fw/iptables.git (branch 'xt_rpfilter_9').

I have taken this into iptables, now it is available in the rpfilter
branch.

One inquiry, I need to know copyright / license details for this
extensions/libxt_rpfilter.c

I remember that Harald insisted that having one copyright notice per
.c file is a good practise (even if this doesn't seem to happen in
other iptables extensions files).

No need to resend the patch, I can ammend the patch with the copyright
/ licensing header.

See extensions/libxt_cluster.c for reference.

> Kernel patches are located in the 'xt_rpfilter_9' branch on
> http://git.breakpoint.cc/cgi-bin/gitweb.cgi?p=fw/nf-next.git.

Applied to my nf-next tree.

http://1984.lsi.us.es/git/net-next/

thanks Florian.

^ permalink raw reply

* Re: r8169 performance?
From: Chris Adams @ 2011-12-04 22:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1323035402.2762.189.camel@edumazet-laptop>

Once upon a time, Eric Dumazet <eric.dumazet@gmail.com> said:
> Le dimanche 04 décembre 2011 à 15:10 -0600, Chris Adams a écrit :
> > I have a system with an on-board RealTek gigabit network interface that
> > is giving me poor performance.  I'm using a simple test, running "nc -l"
> > on one system and "dd if=/dev/zero | nc" on another.  I see about 280
> > Mbps transmit and 511 Mbps receive.
> 
> Do you mean that if this problematic NIC is the receiver, it receives
> 280Mbps, and if it is the sender, speed is 511 Mbps ?

No, the other way around (the problem NIC transmits at 280Mbps and
receives at 511Mbps).

> Could you check counters ?  

Is there anything specific to look at?  I'm not seeing anything that
looks like a problem to me (for example, the "144 dropped because of
missing route" all appear to be from during boot; that number isn't
changing).

> netstat -s

Ip:
    818382 total packets received
    0 forwarded
    0 incoming packets discarded
    818382 incoming packets delivered
    1190521 requests sent out
    144 dropped because of missing route
Icmp:
    2 ICMP messages received
    0 input ICMP message failed.
    ICMP input histogram:
        destination unreachable: 1
        echo requests: 1
    1 ICMP messages sent
    0 ICMP messages failed
    ICMP output histogram:
        echo replies: 1
IcmpMsg:
        InType3: 1
        InType8: 1
        OutType0: 1
Tcp:
    11 active connections openings
    15 passive connection openings
    5 failed connection attempts
    1 connection resets received
    5 connections established
    821140 segments received
    1193282 segments send out
    7 segments retransmited
    0 bad segments received.
    11 resets sent
Udp:
    318 packets received
    0 packets to unknown port received.
    0 packet receive errors
    318 packets sent
    0 receive buffer errors
    0 send buffer errors
UdpLite:
TcpExt:
    4 TCP sockets finished time wait in fast timer
    23 delayed acks sent
    7 packets directly queued to recvmsg prequeue.
    410916 packets header predicted
    105061 acknowledgments not containing data received
    304005 predicted acknowledgments
    0 TCP data loss events
    2 retransmits in slow start
    6 other TCP timeouts
    TCPSackShiftFallback: 1
    TCPBacklogDrop: 75
IpExt:
    InMcastPkts: 21
    InBcastPkts: 43
    InOctets: 1653475944
    OutOctets: 1201322384
    InMcastOctets: 672
    InBcastOctets: 7813

> ethtool -S ethX

NIC statistics:
     tx_packets: 1192416
     rx_packets: 1522908
     tx_errors: 0
     rx_errors: 0
     rx_missed: 0
     align_errors: 0
     tx_single_collisions: 0
     tx_multi_collisions: 0
     unicast: 1522391
     broadcast: 474
     multicast: 43
     tx_aborted: 0
     tx_underrun: 0

> cat /proc/net/softnet_stat

000c8dc9 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000

-- 
Chris Adams <cmadams@hiwaay.net>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.

^ permalink raw reply

* [PATCH net-next] bql: fix CONFIG_XPS=n build
From: Eric Dumazet @ 2011-12-04 22:38 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Tom Herbert

netdev_queue_release() should be called even if CONFIG_XPS=n
to properly release device reference.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/core/net-sysfs.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 3bf72b6..9d13463 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1221,9 +1221,7 @@ static void netdev_queue_release(struct kobject *kobj)
 
 static struct kobj_type netdev_queue_ktype = {
 	.sysfs_ops = &netdev_queue_sysfs_ops,
-#ifdef CONFIG_XPS
 	.release = netdev_queue_release,
-#endif
 	.default_attrs = netdev_queue_default_attrs,
 };
 

^ permalink raw reply related

* Re: r8169 performance?
From: Francois Romieu @ 2011-12-04 23:40 UTC (permalink / raw)
  To: Chris Adams, netdev; +Cc: hayeswang
In-Reply-To: <20111204211007.GA32098@hiwaay.net>

Chris Adams <cmadams@hiwaay.net> :
[...]
> Are there any suggestions for what I might be able to do to improve
> throughput?  Is there a driver issue, or is it in how the motherboard
> implemented the NIC ?

Can you grep for the r8169 lines in the dmesg of both computers and
send the XID lines ?

It should show if the nics are the same or not.

-- 
Ueimor

^ permalink raw reply

* Re: r8169 performance?
From: Chris Adams @ 2011-12-05  0:46 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, hayeswang
In-Reply-To: <20111204234004.GA32515@electric-eye.fr.zoreil.com>

Once upon a time, Francois Romieu <romieu@fr.zoreil.com> said:
> Chris Adams <cmadams@hiwaay.net> :
> [...]
> > Are there any suggestions for what I might be able to do to improve
> > throughput?  Is there a driver issue, or is it in how the motherboard
> > implemented the NIC ?
> 
> Can you grep for the r8169 lines in the dmesg of both computers and
> send the XID lines ?
> 
> It should show if the nics are the same or not.

On the "problem" computer (on Fedora 16, kernel 3.1.2):

[    7.101106] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[    7.102665] r8169 0000:02:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
[    7.107308] r8169 0000:02:00.0: setting latency timer to 64
[    7.107370] r8169 0000:02:00.0: irq 42 for MSI/MSI-X
[    7.107703] r8169 0000:02:00.0: eth0: RTL8168d/8111d at 0xffffc900112fc000, 00:19:66:f2:dc:0b, XID 081000c0 IRQ 42

On the "okay" computer (on Fedora 14, kernel 2.6.35.14):

[    9.053574] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[    9.053594] r8169 0000:06:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
[    9.053689] r8169 0000:06:00.0: setting latency timer to 64
[    9.053740] r8169 0000:06:00.0: irq 47 for MSI/MSI-X
[    9.053830] r8169 0000:06:00.0: eth0: RTL8168d/8111d at 0xffffc90012998000, 6c:f0:49:b7:67:91, XID 083000c0 IRQ 47

Both are configured with static IPv4/v6 addresses (no NetworkManager).
The only other config difference I can think of is that the "okay"
computer is using bridging (eth0 in br0, IP config on br0) so I can
bridge virtual machines to the LAN.

-- 
Chris Adams <cmadams@hiwaay.net>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.

^ permalink raw reply

* Re: [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Michael Wang @ 2011-12-05  1:05 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: Flavio Leitner, David Miller, netdev@vger.kernel.org,
	gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1322983717.24828.8.camel@jtkirshe-mobl>

On 12/04/2011 03:28 PM, Jeff Kirsher wrote:

> On Sat, 2011-12-03 at 19:26 -0800, David Miller wrote:
>> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> Date: Sat,  3 Dec 2011 03:44:26 -0800
>>
>>> +	if ((!adapter->tx_hang_recheck) &&
>>
>> Excessive parenthesis, please remove.
>>
>>> +		adapter->tx_hang_recheck = 1;
>>
>> This variable is a bool, set it to true or false.
>>
>>> +			adapter->tx_hang_recheck = 0;
>>
>> Likewise.
>>
>>> +	adapter->tx_hang_recheck = 0;
>>
>> Likewise.
> 
> Michael/Flavio -
> 
> To expedite this patch, I can make the changes that Dave is requesting
> and re-submit v2 of the patch, if that is ok with you.
> 

Hi, Jeff

That's ok for me, I think it's good if you can work with Dave and make
out a final version for us, if you want my help, please mail me at any
time, I'm glad to work with you.

Flavio:
What's your opinion?

Thanks,
Michael Wang

> -Jeff

^ permalink raw reply

* Re: [Linux-decnet-user] Proposed removal of DECnet support (was:Re: [BUG] 3.2-rc2:BUG kmalloc-8: Redzone overwritten)
From: Ben Hutchings @ 2011-12-05  1:23 UTC (permalink / raw)
  To: Philipp Schafft
  Cc: Steven Whitehouse, mike.gair, Chrissie Caulfield,
	Christoph Lameter, David Miller, Eric Dumazet, Sasha Levin,
	Linux-DECnet user, linux-kernel, linux-mm, Matt Mackall, netdev,
	Pekka Enberg, RoarAudio
In-Reply-To: <20111204195055.A36077AD9C@priderock.keep-cool.org>

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

On Sun, 2011-12-04 at 20:50 +0100, Philipp Schafft wrote:
> reflum,
> 
> On Wed, 2011-11-30 at 14:52 +0000, Steven Whitehouse wrote:
[...]
> > It is good to know that people are still using the Linux DECnet code
> > too. It has lived far beyond the time when I'd envisioned it still being
> > useful :-)
> 
> There are still some people interested in it. Btw. on Debian popcon
> counts 5356 users.

This is grossly misleading.  Here's the historical graph showing <100
installations of libdnet until early 2011:
http://qa.debian.org/popcon-graph.php?packages=libdnet

The increase in 2011 is not a sudden resurgence of interest; it comes
from roaraudio[1] users.  For some reason (a joke?) roaraudio has DECnet
support and its packages depend on libdnet.  You can see that the above
graph is precisely correlated with this:
http://qa.debian.org/popcon-graph.php?packages=libroar1

(And so far as I can work out, libroar1 is mostly being installed as a
dependency of an unofficial package of Xine.)

The only reason I know this is because there was a sudden spate of bug
reports on the kernel due to people getting dnet-common installed as a
recommendation of libdnet and then having their Ethernet MAC addresses
reconfigured for DECnet.

[1] Yet another audio mixing daemon

Ben.

-- 
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer

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

^ permalink raw reply

* linux-next: manual merge of the net-next tree with the  tree
From: Stephen Rothwell @ 2011-12-05  1:34 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Wey-Yi Guy, John W. Linville,
	Johannes Berg

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

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
drivers/net/wireless/iwlwifi/iwl-agn.c between commits from the
tree and commit 7335613ae27a ("iwlwifi: move all mac80211 related
functions to one place") from the net-next tree.

I just used the net-next version (which may require some fixes).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v7 02/10] foundations of per-cgroup memory pressure controlling.
From: KAMEZAWA Hiroyuki @ 2011-12-05  1:59 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups
In-Reply-To: <4ED90F06.102@parallels.com>

On Fri, 2 Dec 2011 15:46:46 -0200
Glauber Costa <glommer@parallels.com> wrote:

> 
> >>   static void proto_seq_printf(struct seq_file *seq, struct proto *proto)
> >>   {
> >> +	struct mem_cgroup *memcg = mem_cgroup_from_task(current);
> >> +
> >>   	seq_printf(seq, "%-9s %4u %6d  %6ld   %-3s %6u   %-3s  %-10s "
> >>   			"%2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c\n",
> >>   		   proto->name,
> >>   		   proto->obj_size,
> >>   		   sock_prot_inuse_get(seq_file_net(seq), proto),
> >> -		   proto->memory_allocated != NULL ? atomic_long_read(proto->memory_allocated) : -1L,
> >> -		   proto->memory_pressure != NULL ? *proto->memory_pressure ? "yes" : "no" : "NI",
> >> +		   sock_prot_memory_allocated(proto, memcg),
> >> +		   sock_prot_memory_pressure(proto, memcg),
> >
> > I wonder I should say NO, here. (Networking guys are ok ??)
> >
> > IIUC, this means there is no way to see aggregated sockstat of all system.
> > And the result depends on the cgroup which the caller is under control.
> >
> > I think you should show aggregated sockstat(global + per-memcg) here and
> > show per-memcg ones via /cgroup interface or add private_sockstat to show
> > per cgroup summary.
> >
> 
> Hi Kame,
> 
> Yes, the statistics displayed depends on which cgroup you live.
> Also, note that the parent cgroup here is always updated (even when 
> use_hierarchy is set to 0). So it is always possible to grab global 
> statistics, by being in the root cgroup.
> 
> For the others, I believe it to be a question of naturalization. Any 
> tool that is fetching these values is likely interested in the amount of 
> resources available/used. When you are on a cgroup, the amount of 
> resources available/used changes, so that's what you should see.
> 
> Also brings the point of resource isolation: if you shouldn't interfere 
> with other set of process' resources, there is no reason for you to see 
> them in the first place.
> 
> So given all that, I believe that whenever we talk about resources in a 
> cgroup, we should talk about cgroup-local ones.

But you changes /proc/ information without any arguments with other guys.
If you go this way, you should move this patch as independent add-on patch
and discuss what this should be. For example, /proc/meminfo doesn't reflect
memcg's information (for now). And scheduler statiscits in /proc/stat doesn't
reflect cgroup's information.

So, please discuss the problem in open way. This issue is not only related to
this patch but also to other cgroups. Sneaking this kind of _big_ change in
a middle of complicated patch series isn't good.

In short, could you divide this patch into a independent patch and discuss
again ? If we agree the general diection should go this way, other guys will
post patches for cpu, memory, blkio, etc.


Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v7 04/10] tcp memory pressure controls
From: KAMEZAWA Hiroyuki @ 2011-12-05  2:01 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA, KAMEZAWA Hiroyuki
In-Reply-To: <4ED91188.6030503-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Fri, 2 Dec 2011 15:57:28 -0200
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> On 11/29/2011 11:49 PM, KAMEZAWA Hiroyuki wrote:
> >
> >> -static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >> +struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
> >>   {
> >>   	return container_of(cgroup_subsys_state(cont,
> >>   				mem_cgroup_subsys_id), struct mem_cgroup,
> >> @@ -4717,14 +4732,27 @@ static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
> >>
> >>   	ret = cgroup_add_files(cont, ss, kmem_cgroup_files,
> >>   			       ARRAY_SIZE(kmem_cgroup_files));
> >> +
> >> +	if (!ret)
> >> +		ret = mem_cgroup_sockets_init(cont, ss);
> >>   	return ret;
> >>   };
> >
> > You does initizalication here. The reason what I think is
> > 1. 'proto_list' is not available at createion of root cgroup and
> >      you need to delay set up until mounting.
> >
> > If so, please add comment or find another way.
> > This seems not very clean to me.
> 
> Yes, we do can run into some ordering issues. A part of the 
> initialization can be done earlier. But I preferred to move it all later
> instead of creating two functions for it. But I can change that if you 
> want, no big deal.
> 

Hmm. please add comments about the 'issue'. It will help readers.


> >> +	tcp->tcp_prot_mem[0] = sysctl_tcp_mem[0];
> >> +	tcp->tcp_prot_mem[1] = sysctl_tcp_mem[1];
> >> +	tcp->tcp_prot_mem[2] = sysctl_tcp_mem[2];
> >> +	tcp->tcp_memory_pressure = 0;
> >
> > Question:
> >
> > Is this value will be updated when an admin chages sysctl ?
> 
> yes.
> 
> > I guess, this value is set at system init script or some which may
> > happen later than mounting cgroup.
> > I don't like to write a guideline 'please set sysctl val before
> > mounting cgroup'
> 
> Agreed.
> 
> This code is in patch 6 (together with the limiting):
> 
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
> +       rcu_read_lock();
> +       memcg = mem_cgroup_from_task(current);
> +
> +       tcp_prot_mem(memcg, vec[0], 0);
> +       tcp_prot_mem(memcg, vec[1], 1);
> +       tcp_prot_mem(memcg, vec[2], 2);
> +       rcu_read_unlock();
> +#endif
> 
> tcp_prot_mem is just a wrapper around the assignment so we can access 
> memcg's inner fields.
> 


Ok. sysctl and cgroup are updated at the same time.
thank you.
-Kame

--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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

* Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure
From: KAMEZAWA Hiroyuki @ 2011-12-05  2:06 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4ED91318.1030803-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Fri, 2 Dec 2011 16:04:08 -0200
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
> > On Tue, 29 Nov 2011 21:56:51 -0200
> > Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> >
> >> Hi,
> >>
> >> This patchset implements per-cgroup tcp memory pressure controls. It did not change
> >> significantly since last submission: rather, it just merges the comments Kame had.
> >> Most of them are style-related and/or Documentation, but there are two real bugs he
> >> managed to spot (thanks)
> >>
> >> Please let me know if there is anything else I should address.
> >>
> >
> > After reading all codes again, I feel some strange. Could you clarify ?
> >
> > Here.
> > ==
> > +void sock_update_memcg(struct sock *sk)
> > +{
> > +	/* right now a socket spends its whole life in the same cgroup */
> > +	if (sk->sk_cgrp) {
> > +		WARN_ON(1);
> > +		return;
> > +	}
> > +	if (static_branch(&memcg_socket_limit_enabled)) {
> > +		struct mem_cgroup *memcg;
> > +
> > +		BUG_ON(!sk->sk_prot->proto_cgroup);
> > +
> > +		rcu_read_lock();
> > +		memcg = mem_cgroup_from_task(current);
> > +		if (!mem_cgroup_is_root(memcg))
> > +			sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg);
> > +		rcu_read_unlock();
> > ==
> >
> > sk->sk_cgrp is set to a memcg without any reference count.
> >
> > Then, no check for preventing rmdir() and freeing memcgroup.
> >
> > Is there some css_get() or mem_cgroup_get() somewhere ?
> >
> 
> There were a css_get in the first version of this patchset. It was 
> removed, however, because it was deemed anti-intuitive to prevent rmdir, 
> since we can't know which sockets are blocking it, or do anything about 
> it. Or did I misunderstand something ?
> 

Maybe I misuderstood. Thank you. Ok, there is no css_get/put and
rmdir() is allowed. But, hmm....what's guarding threads from stale
pointer access ? 

Does a memory cgroup which is pointed by sk->sk_cgrp always exist ?

-Kame

--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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

* Re: [PATCH v7 10/10] Disable task moving when using kernel memory accounting
From: KAMEZAWA Hiroyuki @ 2011-12-05  2:18 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4ED914EC.6020500-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Fri, 2 Dec 2011 16:11:56 -0200
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> On 11/30/2011 12:22 AM, KAMEZAWA Hiroyuki wrote:
> > On Tue, 29 Nov 2011 21:57:01 -0200
> > Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> >
> >> Since this code is still experimental, we are leaving the exact
> >> details of how to move tasks between cgroups when kernel memory
> >> accounting is used as future work.
> >>
> >> For now, we simply disallow movement if there are any pending
> >> accounted memory.
> >>
> >> Signed-off-by: Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> >> CC: Hiroyouki Kamezawa<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
> >> ---
> >>   mm/memcontrol.c |   23 ++++++++++++++++++++++-
> >>   1 files changed, 22 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index a31a278..dd9a6d9 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -5453,10 +5453,19 @@ static int mem_cgroup_can_attach(struct cgroup_subsys *ss,
> >>   {
> >>   	int ret = 0;
> >>   	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
> >> +	struct mem_cgroup *from = mem_cgroup_from_task(p);
> >> +
> >> +#if defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM)&&  defined(CONFIG_INET)
> >> +	if (from != memcg&&  !mem_cgroup_is_root(from)&&
> >> +	    res_counter_read_u64(&from->tcp_mem.tcp_memory_allocated, RES_USAGE)) {
> >> +		printk(KERN_WARNING "Can't move tasks between cgroups: "
> >> +			"Kernel memory held.\n");
> >> +		return 1;
> >> +	}
> >> +#endif
> >
> > I wonder....reading all codes again, this is incorrect check.
> >
> > Hm, let me cralify. IIUC, in old code, "prevent moving" is because you hold
> > reference count of cgroup, which can cause trouble at rmdir() as leaking refcnt.
> right.
> 
> > BTW, because socket is a shared resource between cgroup, changes in mm->owner
> > may cause task cgroup moving implicitly. So, if you allow leak of resource
> > here, I guess... you can take mem_cgroup_get() refcnt which is memcg-local and
> > allow rmdir(). Then, this limitation may disappear.
> 
> Sorry, I didn't fully understand. Can you clarify further?
> If the task is implicitly moved, it will end up calling can_attach as 
> well, right?
> 
I'm sorry that my explanation is bad.

You can take memory cgroup itself's reference count by mem_cgroup_put/get.
By getting this, memory cgroup object will continue to exist even after
its struct cgroup* is freed by rmdir().

So, assume you do mem_cgroup_get()/put at socket attaching/detatching.

0) A task has a tcp socekts in memcg0.

task(memcg0)
 +- socket0 --> memcg0,usage=4096

1) move this task to memcg1

task(memcg1)
 +- socket0 --> memcg0,usage=4096

2) The task create a new socket.

task(memcg1)
 +- socekt0 --> memcg0,usage=4096 
 +- socket1 --> memcg1,usage=xxxx

Here, the task will hold 4096bytes of usage in memcg0 implicitly.

3) an admin removes memcg0
task(memcg1)
 +- socket0 -->memcg0, usage=4096 <-----(*)
 +- socket1 -->memcg1, usage=xxxx

(*) is invisible to users....but this will not be very big problem.

Thanks,
-Kame











Thanks,
-Kame






--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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] datapath: Fix build breakage on kernel 2.6.40
From: Zhi Yong Wu @ 2011-12-05  2:33 UTC (permalink / raw)
  To: dev-yBygre7rU0TnMu66kgdUjQ
  Cc: aliguori-r/Jw6+rmf7HQT0dZR+AlfA, stefanha-Re5JQEeQqe8AvxtiuMwx3w,
	ryanh-r/Jw6+rmf7HQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA

Today i played with openvswitch on my workstation with kernel 2.6.40 and found that it break when i built. The
+issue is introduced by commit ceb176fdb72bb7ce90debc66e1eeb1d25823d30a

Below is the error log:

from /home/zwu/work/virt/openvswitch/datapath/linux/genetlink-brcompat.c:10:
/home/zwu/work/virt/openvswitch/datapath/linux/compat/include/linux/skbuff.h:243:20: error: redefinition of
+‘skb_reset_mac_len’
include/linux/skbuff.h:1259:20: note: previous definition of ‘skb_reset_mac_len’ was here
make[5]: *** [/home/zwu/work/virt/openvswitch/datapath/linux/genetlink-brcompat.o] Error 1
make[4]: *** [_module_/home/zwu/work/virt/openvswitch/datapath/linux] Error 2
make[4]: Leaving directory `/usr/src/kernels/2.6.40.6-0.fc15.x86_64'
make[3]: *** [default] Error 2
make[3]: Leaving directory `/home/zwu/work/virt/openvswitch/datapath/linux'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/zwu/work/virt/openvswitch/datapath'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/zwu/work/virt/openvswitch'
make: *** [all] Error 2


Signed-off-by: Zhi Yong Wu <zwu.kernel@gmail.com>
---
 datapath/linux/compat/include/linux/skbuff.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h
index 311bfdb..96d8012 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -239,7 +239,7 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,0,0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,40)
 static inline void skb_reset_mac_len(struct sk_buff *skb)
 {
 	skb->mac_len = skb->network_header - skb->mac_header;
-- 
1.7.6

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply related

* ipv6: addrconf_add_mroute() problem with loopback interface.
From: Li Wei @ 2011-12-05  2:57 UTC (permalink / raw)
  To: netdev

Hi All,

addrconf_add_mroute() create "default" multicast route for the interface, 
but with loopback interface, there would be some problem:

If we set ipv6 address for the "lo" interface *before* other interfaces, the
function addrconf_add_mroute() will create a "default" multicast route with
error set to ENETUNREACH for "lo" interface.

Later, when we send multicast packet, route subsystem will return this *error*
route, so no multicast packets can go out.

Does somebody can give me some advice that if it's necessary to add default
multicast route for loopback interface?

^ permalink raw reply

* Re: [GIT PULL v3] Open vSwitch
From: Zhi Yong Wu @ 2011-12-05  3:32 UTC (permalink / raw)
  To: Jesse Gross
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller
In-Reply-To: <1322937531-8071-1-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>

Although this series of patchset is a bit large, therefore their codes
aren't directly pasted here, but if anyone find some issues in the
codes, how will we point out them?

On Sun, Dec 4, 2011 at 2:38 AM, Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org> wrote:
> This series of patches proposes the Open vSwitch kernel components for
> upstream.  Open vSwitch has existed as a separate project for several
> years and we now believe it to be mature enough for inclusion.  The
> actual functionality is described more fully in the commit that adds
> the kernel code.
>
> The following changes since commit 340e8dc1fb4032b6c8334c9bff20b2aec42ecfd8:
>
>  atm: clip: Remove code commented out since eternity. (2011-12-02 14:27:11 -0500)
>
> are available in the git repository at:
>  git://git.kernel.org/pub/scm/linux/kernel/git/jesse/openvswitch.git for-upstream
>
> Jesse Gross (3):
>      genetlink: Add rcu_dereference_genl and genl_dereference.
>      ipv6: Add fragment reporting to ipv6_skip_exthdr().
>      net: Add Open vSwitch kernel components.
>
> Pravin B Shelar (3):
>      genetlink: Add genl_notify()
>      genetlink: Add lockdep_genl_is_held().
>      vlan: Move vlan_set_encap_proto() to vlan header file
>
>  Documentation/networking/00-INDEX        |    2 +
>  Documentation/networking/openvswitch.txt |  195 +++
>  MAINTAINERS                              |    8 +
>  include/linux/genetlink.h                |   24 +
>  include/linux/if_vlan.h                  |   34 +
>  include/linux/openvswitch.h              |  452 +++++++
>  include/net/genetlink.h                  |    2 +
>  include/net/ipv6.h                       |    2 +-
>  net/8021q/vlan_core.c                    |   33 -
>  net/Kconfig                              |    1 +
>  net/Makefile                             |    1 +
>  net/bridge/br_multicast.c                |    3 +-
>  net/bridge/netfilter/ebt_ip6.c           |    3 +-
>  net/bridge/netfilter/ebt_log.c           |    3 +-
>  net/ipv6/exthdrs_core.c                  |   11 +-
>  net/ipv6/icmp.c                          |    7 +-
>  net/ipv6/ip6_input.c                     |    3 +-
>  net/ipv6/ip6_output.c                    |    3 +-
>  net/ipv6/netfilter/ip6t_REJECT.c         |    3 +-
>  net/netfilter/ipset/ip_set_getport.c     |    4 +-
>  net/netfilter/xt_AUDIT.c                 |    3 +-
>  net/netfilter/xt_TCPMSS.c                |    3 +-
>  net/netfilter/xt_TCPOPTSTRIP.c           |    3 +-
>  net/netfilter/xt_hashlimit.c             |    3 +-
>  net/netfilter/xt_socket.c                |    4 +-
>  net/netlink/genetlink.c                  |   21 +
>  net/openvswitch/Kconfig                  |   28 +
>  net/openvswitch/Makefile                 |   14 +
>  net/openvswitch/actions.c                |  415 +++++++
>  net/openvswitch/datapath.c               | 1912 ++++++++++++++++++++++++++++++
>  net/openvswitch/datapath.h               |  125 ++
>  net/openvswitch/dp_notify.c              |   66 +
>  net/openvswitch/flow.c                   | 1346 +++++++++++++++++++++
>  net/openvswitch/flow.h                   |  199 ++++
>  net/openvswitch/vport-internal_dev.c     |  241 ++++
>  net/openvswitch/vport-internal_dev.h     |   28 +
>  net/openvswitch/vport-netdev.c           |  198 +++
>  net/openvswitch/vport-netdev.h           |   42 +
>  net/openvswitch/vport.c                  |  396 ++++++
>  net/openvswitch/vport.h                  |  205 ++++
>  security/lsm_audit.c                     |    3 +-
>  security/selinux/hooks.c                 |    3 +-
>  42 files changed, 6000 insertions(+), 52 deletions(-)
>  create mode 100644 Documentation/networking/openvswitch.txt
>  create mode 100644 include/linux/openvswitch.h
>  create mode 100644 net/openvswitch/Kconfig
>  create mode 100644 net/openvswitch/Makefile
>  create mode 100644 net/openvswitch/actions.c
>  create mode 100644 net/openvswitch/datapath.c
>  create mode 100644 net/openvswitch/datapath.h
>  create mode 100644 net/openvswitch/dp_notify.c
>  create mode 100644 net/openvswitch/flow.c
>  create mode 100644 net/openvswitch/flow.h
>  create mode 100644 net/openvswitch/vport-internal_dev.c
>  create mode 100644 net/openvswitch/vport-internal_dev.h
>  create mode 100644 net/openvswitch/vport-netdev.c
>  create mode 100644 net/openvswitch/vport-netdev.h
>  create mode 100644 net/openvswitch/vport.c
>  create mode 100644 net/openvswitch/vport.h
> _______________________________________________
> dev mailing list
> dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org
> http://openvswitch.org/mailman/listinfo/dev



-- 
Regards,

Zhi Yong Wu

^ permalink raw reply

* Re: linux-next: manual merge of the net-next tree with the  tree
From: Stephen Rothwell @ 2011-12-05  4:34 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Wey-Yi Guy, John W. Linville,
	Johannes Berg
In-Reply-To: <20111205123428.47213da24624d85aa84711f3@canb.auug.org.au>

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

On Mon, 5 Dec 2011 12:34:28 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the net-next tree got a conflict in
> drivers/net/wireless/iwlwifi/iwl-agn.c between commits from the

wireless (sorry)

> tree and commit 7335613ae27a ("iwlwifi: move all mac80211 related
> functions to one place") from the net-next tree.
> 
> I just used the net-next version (which may require some fixes).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* 3.0.8 not syncing: Fatal exception in interrupt
From: Stefan Priebe - Profihost AG @ 2011-12-05  6:21 UTC (permalink / raw)
  To: netdev; +Cc: stable

Dear list,

i encountered the following bug today while running v3.0.8

Better readable here:
http://pastebin.com/raw.php?i=2s75P0wF

2011-12-05 02:57:41     CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
2011-12-05 02:57:41     CR2: 000000000000002c CR3: 0000000119bb2000 CR4:
00000000000006e0
2011-12-05 02:57:41     DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
2011-12-05 02:57:41     DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
2011-12-05 02:57:41     Process kworker/0:1 (pid: 0, threadinfo
ffff88011af2c000, task ffff88011af09820)Stack:
2011-12-05 02:57:41     ffff88002589f6e0 ffff880052664d00
ffff880061911c00 ffff88010572c000
2011-12-05 02:57:41     ffff88011fd03d30 ffffffff814f56e2
ffff880061911c00 ffff880061911c00
2011-12-05 02:57:41     ffff88011fd03d80 ffffffff8155e4c6
ffff88011fd03e20 ffffffff81497230
2011-12-05 02:57:41     Call Trace:<IRQ>
2011-12-05 02:57:41     [<ffffffff814f56e2>] __sk_dst_check+0x42/0x70
2011-12-05 02:57:41     [<ffffffff8155e4c6>]
inet_sk_rebuild_header+0x26/0x330
2011-12-05 02:57:41     [<ffffffff81497230>] ? tg3_poll_work+0x1e0/0xe80
2011-12-05 02:57:41     [<ffffffff81548afb>] tcp_retransmit_skb+0xab/0x640
2011-12-05 02:57:41     [<ffffffff81012ca4>] ? x86_pmu_enable+0x1f4/0x270
2011-12-05 02:57:41     [<ffffffff8154b91e>]
tcp_retransmit_timer+0x26e/0x6e0
2011-12-05 02:57:41     [<ffffffff8154bf28>] tcp_write_timer+0x198/0x210
2011-12-05 02:57:41     [<ffffffff81055e3c>] run_timer_softirq+0x1cc/0x320
2011-12-05 02:57:41     [<ffffffff8154bd90>] ?
tcp_retransmit_timer+0x6e0/0x6e0
2011-12-05 02:57:41     [<ffffffff8101dfb8>] ? lapic_next_event+0x18/0x20
2011-12-05 02:57:41     [<ffffffff8104d1d0>] __do_softirq+0xd0/0x1c0
2011-12-05 02:57:41     [<ffffffff815d1d0c>] call_softirq+0x1c/0x30
2011-12-05 02:57:41     [<ffffffff81004a05>] do_softirq+0x55/0x90
2011-12-05 02:57:41     [<ffffffff8104cfcd>] irq_exit+0x7d/0xa0
2011-12-05 02:57:41     [<ffffffff8101eca9>]
smp_apic_timer_interrupt+0x69/0xa0
2011-12-05 02:57:41     [<ffffffff815d14d3>]
apic_timer_interrupt+0x13/0x20<EOI>
2011-12-05 02:57:41     [<ffffffff815d14ce>] ? apic_timer_interrupt+0xe/0x20
2011-12-05 02:57:41     [<ffffffff8100b7b7>] ? mwait_idle+0x117/0x120
2011-12-05 02:57:41     [<ffffffff8106cfd5>] ?
atomic_notifier_call_chain+0x15/0x20
2011-12-05 02:57:41     [<ffffffff8100208d>] cpu_idle+0xad/0xf0
2011-12-05 02:57:41     [<ffffffff815c79aa>] start_secondary+0x201/0x297
2011-12-05 02:57:41     Code: 51 a2 8e 00 89 83 d4 00 00 00 eb 9b 48 85
db 74 16 48 8b 53 40 31 ff 48 85 d2 74 0f 48 8b 05 f2 04 5a 00 48 89 42
18 48 8b 7b 40 <f0> ff 4f 2c 0f 94 c0 84 c0 0f 85 ac 00 00 00 48 c7 43
40 00 00
2011-12-05 02:57:41     RIP [<ffffffff81528796>] ipv4_dst_check+0xb6/0x190
2011-12-05 02:57:41     RSP <ffff88011fd03cf0>
2011-12-05 02:57:41     CR2: 000000000000002c
2011-12-05 02:57:41     ---[ end trace bd99ce386f3021bd ]---
2011-12-05 02:57:41     Kernel panic - not syncing: Fatal exception in
interrupt
2011-12-05 02:57:41     Pid: 0, comm: kworker/0:1 Tainted: G D 2.6.40.8p4 #1
2011-12-05 02:57:41     Call Trace:
2011-12-05 02:57:41     <IRQ> [<ffffffff815cd21c>] panic+0xba/0x1e5
2011-12-05 02:57:41     [<ffffffff815d14d3>] ?
apic_timer_interrupt+0x13/0x20
2011-12-05 02:57:41     [<ffffffff8104717a>] ? kmsg_dump+0x4a/0x100
2011-12-05 02:57:41     [<ffffffff81005ec4>] oops_end+0xd4/0xe0
2011-12-05 02:57:41     [<ffffffff81029501>] no_context+0xf1/0x270
2011-12-05 02:57:41     [<ffffffff810297d5>]
__bad_area_nosemaphore+0x155/0x200
2011-12-05 02:57:41     [<ffffffff810fa37b>] ? kmem_cache_free+0x1b/0xf0
2011-12-05 02:57:41     [<ffffffff814f8f72>] ? __kfree_skb+0x42/0xa0
2011-12-05 02:57:41     [<ffffffff81545b84>] ?
tcp_rcv_established+0x364/0x630
2011-12-05 02:57:41     [<ffffffff8154f364>] ? tcp_v4_do_rcv+0x154/0x2d0
2011-12-05 02:57:41     [<ffffffff8102988e>] bad_area_nosemaphore+0xe/0x10
2011-12-05 02:57:41     [<ffffffff81029e1e>] do_page_fault+0x2ee/0x420
2011-12-05 02:57:41     [<ffffffff8154fbdb>] ? tcp_v4_rcv+0x6fb/0x880
2011-12-05 02:57:41     [<ffffffff8152d087>] ?
ip_local_deliver_finish+0x127/0x250
2011-12-05 02:57:41     [<ffffffff8152d23d>] ? ip_local_deliver+0x8d/0xa0
2011-12-05 02:57:41     [<ffffffff8152c942>] ? ip_rcv_finish+0x172/0x340
2011-12-05 02:57:41     [<ffffffff815d079f>] page_fault+0x1f/0x30
2011-12-05 02:57:41     [<ffffffff81528796>] ? ipv4_dst_check+0xb6/0x190
2011-12-05 02:57:41     [<ffffffff814f56e2>] __sk_dst_check+0x42/0x70
2011-12-05 02:57:41     [<ffffffff8155e4c6>]
inet_sk_rebuild_header+0x26/0x330
2011-12-05 02:57:41     [<ffffffff81497230>] ? tg3_poll_work+0x1e0/0xe80
2011-12-05 02:57:41     [<ffffffff81548afb>] tcp_retransmit_skb+0xab/0x640
2011-12-05 02:57:41     [<ffffffff81012ca4>] ? x86_pmu_enable+0x1f4/0x270
2011-12-05 02:57:41     [<ffffffff8154b91e>]
tcp_retransmit_timer+0x26e/0x6e0
2011-12-05 02:57:41     [<ffffffff8154bf28>] tcp_write_timer+0x198/0x210
2011-12-05 02:57:41     [<ffffffff81055e3c>] run_timer_softirq+0x1cc/0x320
2011-12-05 02:57:41     [<ffffffff8154bd90>] ?
tcp_retransmit_timer+0x6e0/0x6e0
2011-12-05 02:57:41     [<ffffffff8101dfb8>] ? lapic_next_event+0x18/0x20
2011-12-05 02:57:41     [<ffffffff8104d1d0>] __do_softirq+0xd0/0x1c0
2011-12-05 02:57:41     [<ffffffff815d1d0c>] call_softirq+0x1c/0x30
2011-12-05 02:57:41     [<ffffffff81004a05>] do_softirq+0x55/0x90
2011-12-05 02:57:41     [<ffffffff8104cfcd>] irq_exit+0x7d/0xa0
2011-12-05 02:57:41     [<ffffffff8101eca9>]
smp_apic_timer_interrupt+0x69/0xa0
2011-12-05 02:57:41     [<ffffffff815d14d3>] apic_timer_interrupt+0x13/0x20
2011-12-05 02:57:41     <EOI> [<ffffffff815d14ce>] ?
apic_timer_interrupt+0xe/0x20
2011-12-05 02:57:41     [<ffffffff8100b7b7>] ? mwait_idle+0x117/0x120
2011-12-05 02:57:41     [<ffffffff8106cfd5>] ?
atomic_notifier_call_chain+0x15/0x20
2011-12-05 02:57:41     [<ffffffff8100208d>] cpu_idle+0xad/0xf0
2011-12-05 02:57:41     [<ffffffff815c79aa>] start_secondary+0x201/0x297
2011-12-05 02:57:41     BUG: unable to handle kernel NULL pointer
dereference at 000000000000002c
2011-12-05 02:57:41     IP: [<ffffffff81528796>] ipv4_dst_check+0xb6/0x190
2011-12-05 02:57:41     PGD 119889067 PUD 1167d2067 PMD 0
2011-12-05 02:57:41     Oops: 0002 [#2] SMP
2011-12-05 02:57:41     CPU 2
2011-12-05 02:57:41     Modules linked in: ipt_REJECT xt_tcpudp
iptable_filter ip_tables x_tables coretemp k8temp ipv6 dm_snapshot dm_mod
2011-12-05 02:57:41     Pid: 0, comm: kworker/0:1 Tainted: G D
2.6.40.8p4 #1 FUJITSU SIEMENS D2581-A5 /D2581-A5
2011-12-05 02:57:41     RIP: 0010:[<ffffffff81528796>]
[<ffffffff81528796>] ipv4_dst_check+0xb6/0x190
2011-12-05 02:57:41     RSP: 0018:ffff88011fd03cf0 EFLAGS: 00010282
2011-12-05 02:57:41     RAX: 00000001a916c098 RBX: ffff880052664d00 RCX:
0000000000000000
2011-12-05 02:57:41     RDX: ffff88002a179a40 RSI: ffff880027c3ff98 RDI:
0000000000000000
2011-12-05 02:57:41     RBP: ffff88011fd03d10 R08: 0000000000000000 R09:
0000000000000000
2011-12-05 02:57:41     R10: 0000000000000000 R11: ffffffff81604c10 R12:
ffff880027c3ff00
2011-12-05 02:57:41     R13: 0000000001b79e55 R14: ffff88010572c028 R15:
0000000000000100
2011-12-05 02:57:41     FS: 0000000000000000(0000)
GS:ffff88011fd00000(0000) knlGS:0000000000000000


Stefan

^ permalink raw reply

* Re: [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Jeff Kirsher @ 2011-12-05  6:25 UTC (permalink / raw)
  To: Michael Wang
  Cc: Flavio Leitner, David Miller, netdev@vger.kernel.org,
	gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <4EDC18D3.4060900@linux.vnet.ibm.com>

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

On Sun, 2011-12-04 at 17:05 -0800, Michael Wang wrote:
> On 12/04/2011 03:28 PM, Jeff Kirsher wrote:
> 
> > On Sat, 2011-12-03 at 19:26 -0800, David Miller wrote:
> >> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >> Date: Sat,  3 Dec 2011 03:44:26 -0800
> >>
> >>> +	if ((!adapter->tx_hang_recheck) &&
> >>
> >> Excessive parenthesis, please remove.
> >>
> >>> +		adapter->tx_hang_recheck = 1;
> >>
> >> This variable is a bool, set it to true or false.
> >>
> >>> +			adapter->tx_hang_recheck = 0;
> >>
> >> Likewise.
> >>
> >>> +	adapter->tx_hang_recheck = 0;
> >>
> >> Likewise.
> > 
> > Michael/Flavio -
> > 
> > To expedite this patch, I can make the changes that Dave is requesting
> > and re-submit v2 of the patch, if that is ok with you.
> > 
> 
> Hi, Jeff
> 
> That's ok for me, I think it's good if you can work with Dave and make
> out a final version for us, if you want my help, please mail me at any
> time, I'm glad to work with you.
> 
> Flavio:
> What's your opinion?
> 
> Thanks,
> Michael Wang

I have the patch read to push, so I will go ahead an push v2 out
tonight.  Since I am making changes to your patch, I will be removing
your signed-off-by (and Flavio's) and keep you as a CC: so that you can
verify the changes I have made to resolve the issues that Dave saw.

Cheers,
Jeff

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

^ permalink raw reply

* [PATCH 1/2] r8169: Rx FIFO overflow fixes.
From: Francois Romieu @ 2011-12-05  6:30 UTC (permalink / raw)
  To: booster
  Cc: hayeswang, 'Jonathan Nieder', 'Eric Dumazet',
	netdev, 'nic_swsd', linux-kernel, 'Armin Kazmi'
In-Reply-To: <20111201222612.GA27998@electric-eye.fr.zoreil.com>

Realtek has specified that the post 8168c gigabit chips and the post
8105e fast ethernet chips recover automatically from a Rx FIFO overflow.
The driver does not need to clear the RxFIFOOver bit of IntrStatus and
it should rather avoid messing it.

The implementation deserves some explanation:
1. events outside of the intr_event bit mask are now ignored. It enforces
   a no-processing policy for the events that either should not be there
   or should be ignored.

2. RxFIFOOver was already ignored in rtl_cfg_infos[RTL_CFG_1] for the
   whole 8168 line of chips with two exceptions:
   - RTL_GIGA_MAC_VER_22 since b5ba6d12bdac21bc0620a5089e0f24e362645efd
     ("use RxFIFO overflow workaround for 8168c chipset.").
     This one should now be correctly handled.
   - RTL_GIGA_MAC_VER_11 (8168b) which requires a different Rx FIFO
     overflow processing.

   Though it does not conform to Realtek suggestion above, the updated
   driver includes no change for RTL_GIGA_MAC_VER_12 and RTL_GIGA_MAC_VER_17.
   Both are 8168b. RTL_GIGA_MAC_VER_12 is common and a bit old so I'd rather
   wait for experimental evidence that the change suggested by Realtek really
   helps or does not hurt in unexpected ways.

   Removed case statements in rtl8169_interrupt are only 8168 relevant.

3. RxFIFOOver is masked for post 8105e 810x chips, namely the sole 8105e
   (RTL_GIGA_MAC_VER_30) itself.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: hayeswang <hayeswang@realtek.com>
---

 Gerd, can you add your Tested-by: to this patch ? Thanks.

 drivers/net/ethernet/realtek/r8169.c |   42 +++++++++++++--------------------
 1 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 6f06aa1..7a1e3a6 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1183,11 +1183,13 @@ static u8 rtl8168d_efuse_read(void __iomem *ioaddr, int reg_addr)
 	return value;
 }
 
-static void rtl8169_irq_mask_and_ack(void __iomem *ioaddr)
+static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
 {
-	RTL_W16(IntrMask, 0x0000);
+	void __iomem *ioaddr = tp->mmio_addr;
 
-	RTL_W16(IntrStatus, 0xffff);
+	RTL_W16(IntrMask, 0x0000);
+	RTL_W16(IntrStatus, tp->intr_event);
+	RTL_R8(ChipCmd);
 }
 
 static unsigned int rtl8169_tbi_reset_pending(struct rtl8169_private *tp)
@@ -4339,7 +4341,7 @@ static void rtl8169_hw_reset(struct rtl8169_private *tp)
 	void __iomem *ioaddr = tp->mmio_addr;
 
 	/* Disable interrupts */
-	rtl8169_irq_mask_and_ack(ioaddr);
+	rtl8169_irq_mask_and_ack(tp);
 
 	rtl_rx_close(tp);
 
@@ -4885,8 +4887,7 @@ static void rtl_hw_start_8168(struct net_device *dev)
 	RTL_W16(IntrMitigate, 0x5151);
 
 	/* Work around for RxFIFO overflow. */
-	if (tp->mac_version == RTL_GIGA_MAC_VER_11 ||
-	    tp->mac_version == RTL_GIGA_MAC_VER_22) {
+	if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
 		tp->intr_event |= RxFIFOOver | PCSTimeout;
 		tp->intr_event &= ~RxOverflow;
 	}
@@ -5076,6 +5077,11 @@ static void rtl_hw_start_8101(struct net_device *dev)
 	void __iomem *ioaddr = tp->mmio_addr;
 	struct pci_dev *pdev = tp->pci_dev;
 
+	if (tp->mac_version >= RTL_GIGA_MAC_VER_30) {
+		tp->intr_event &= ~RxFIFOOver;
+		tp->napi_event &= ~RxFIFOOver;
+	}
+
 	if (tp->mac_version == RTL_GIGA_MAC_VER_13 ||
 	    tp->mac_version == RTL_GIGA_MAC_VER_16) {
 		int cap = pci_pcie_cap(pdev);
@@ -5342,7 +5348,7 @@ static void rtl8169_wait_for_quiescence(struct net_device *dev)
 	/* Wait for any pending NAPI task to complete */
 	napi_disable(&tp->napi);
 
-	rtl8169_irq_mask_and_ack(ioaddr);
+	rtl8169_irq_mask_and_ack(tp);
 
 	tp->intr_mask = 0xffff;
 	RTL_W16(IntrMask, tp->intr_event);
@@ -5804,6 +5810,10 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	 */
 	status = RTL_R16(IntrStatus);
 	while (status && status != 0xffff) {
+		status &= tp->intr_event;
+		if (!status)
+			break;
+
 		handled = 1;
 
 		/* Handle all of the error cases first. These will reset
@@ -5818,27 +5828,9 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 			switch (tp->mac_version) {
 			/* Work around for rx fifo overflow */
 			case RTL_GIGA_MAC_VER_11:
-			case RTL_GIGA_MAC_VER_22:
-			case RTL_GIGA_MAC_VER_26:
 				netif_stop_queue(dev);
 				rtl8169_tx_timeout(dev);
 				goto done;
-			/* Testers needed. */
-			case RTL_GIGA_MAC_VER_17:
-			case RTL_GIGA_MAC_VER_19:
-			case RTL_GIGA_MAC_VER_20:
-			case RTL_GIGA_MAC_VER_21:
-			case RTL_GIGA_MAC_VER_23:
-			case RTL_GIGA_MAC_VER_24:
-			case RTL_GIGA_MAC_VER_27:
-			case RTL_GIGA_MAC_VER_28:
-			case RTL_GIGA_MAC_VER_31:
-			/* Experimental science. Pktgen proof. */
-			case RTL_GIGA_MAC_VER_12:
-			case RTL_GIGA_MAC_VER_25:
-				if (status == RxFIFOOver)
-					goto done;
-				break;
 			default:
 				break;
 			}
-- 
1.7.6.4

^ permalink raw reply related

* [PATCH 2/2] r8169: fix Rx index race between FIFO overflow recovery and NAPI handler.
From: Francois Romieu @ 2011-12-05  6:30 UTC (permalink / raw)
  To: booster
  Cc: hayeswang, 'Jonathan Nieder', 'Eric Dumazet',
	netdev, 'nic_swsd', linux-kernel, 'Armin Kazmi'
In-Reply-To: <20111201222612.GA27998@electric-eye.fr.zoreil.com>

Since 92fc43b4159b518f5baae57301f26d770b0834c9, rtl8169_tx_timeout ends up
resetting Rx and Tx indexes and thus racing with the NAPI handler via
-> rtl8169_hw_reset
   -> rtl_hw_reset
      -> rtl8169_init_ring_indexes

What about returning to the original state ?

rtl_hw_reset is only used by rtl8169_hw_reset and rtl8169_init_one.

The latter does not need rtl8169_init_ring_indexes because the indexes
still contain their original values from the newly allocated network
device private data area (i.e. 0).

rtl8169_hw_reset is used by:
1. rtl8169_down
   Helper for rtl8169_close. rtl8169_open explicitely inits the indexes
   anyway.
2. rtl8169_pcierr_interrupt
   Indexes are set by rtl8169_reinit_task.
3. rtl8169_interrupt
   rtl8169_hw_reset is needed when the device goes down. See 1.
4. rtl_shutdown
   System shutdown handler. Indexes are irrelevant.
5. rtl8169_reset_task
   Indexes must be set before rtl_hw_start is called.
6. rtl8169_tx_timeout
   Indexes should not be set. This is the job of rtl8169_reset_task anyway.

The removal of rtl8169_hw_reset in rtl8169_tx_timeout and its move in
rtl8169_reset_task do not change the analysis.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: hayeswang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/r8169.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 7a1e3a6..67bf078 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -3935,8 +3935,6 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
 			break;
 		udelay(100);
 	}
-
-	rtl8169_init_ring_indexes(tp);
 }
 
 static int __devinit
@@ -5395,14 +5393,16 @@ static void rtl8169_reset_task(struct work_struct *work)
 	if (!netif_running(dev))
 		goto out_unlock;
 
+	rtl8169_hw_reset(tp);
+
 	rtl8169_wait_for_quiescence(dev);
 
 	for (i = 0; i < NUM_RX_DESC; i++)
 		rtl8169_mark_to_asic(tp->RxDescArray + i, rx_buf_sz);
 
 	rtl8169_tx_clear(tp);
+	rtl8169_init_ring_indexes(tp);
 
-	rtl8169_hw_reset(tp);
 	rtl_hw_start(dev);
 	netif_wake_queue(dev);
 	rtl8169_check_link_status(dev, tp, tp->mmio_addr);
@@ -5413,11 +5413,6 @@ out_unlock:
 
 static void rtl8169_tx_timeout(struct net_device *dev)
 {
-	struct rtl8169_private *tp = netdev_priv(dev);
-
-	rtl8169_hw_reset(tp);
-
-	/* Let's wait a bit while any (async) irq lands on */
 	rtl8169_schedule_work(dev, rtl8169_reset_task);
 }
 
-- 
1.7.6.4

^ 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