Netdev List
 help / color / mirror / Atom feed
* Time in Queue, bufferbloat, and... our accidentally interplanetary network
From: Dave Taht @ 2011-12-05  9:05 UTC (permalink / raw)
  To: bloat, bloat-devel, netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless

I was tickled to see that expiring packets based on 'time in queue'
was already a
key design feature of the IPN,

http://www.science20.com/interwebometry/lessons_nasa_can_learn_internet-84861

and the idea was suggested also independently in saturday's CACM
article's comments...

http://queue.acm.org/detail.cfm?id=2071893


Making decisions based on time in queue (TiQ), rather than length of
queue, would
seem to be a win, especially for wireless, but also for  that does
'soft' traffic
shaping with HTB-like qdiscs and sub qdiscs. Anything that is not running
at GigE plus speeds would benefit.

... since basically what's been happening with bufferbloat is a too early
implementation of the IPN, with detours between here and the moon!

... and so far I haven't seen any major theoretical holes in with TiQ, except
for deciding as to how long is too long as to consider a packet as 'expired',
(which can be measured in ms or 10s of ms), and having reasonably
monotonic time.

I just wish I (or someone) could come up with a way to implement it in Linux
without multiple layer violations. skb->queuestamp has a nice ring to it, but
when I look at this portion of the stack I freely admit quivering in ignorance
and fear.

I did a bit of work on a set of timestamping fifos, but realized that
it was the
entire queue's duration from entrance to exit (through multiple scheduling
qdiscs) was what needed to be  measured, and the drop/no drop decision
needs to be made as late as possible - and preferably, the queue being
pulled from needs the next packet pulled forward so as to inform the
receiver that congestion is happening...

And Eric dumazet also produced a preliminary patch a few weeks back
that tied timestamping to before the head of a queue, but that tried to use a
reserved field in the skb that appears from points A to Z is not guaranteed
to be preserved.

Thoughts?

-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* [net-next RFC PATCH 5/5] virtio-net: flow director support
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

In order to let the packets of a flow to be passed to the desired
guest cpu, we can co-operate with devices through programming the flow
director which was just a hash to queue table.

This kinds of co-operation is done through the accelerate RFS support,
a device specific flow sterring method virtnet_fd() is used to modify
the flow director based on rfs mapping. The desired queue were
calculated through reverse mapping of the irq affinity table. In order
to parallelize the ingress path, irq affinity of rx queue were also
provides by the driver.

In addition to accelerate RFS, we can also use the guest scheduler to
balance the load of TX and reduce the lock contention on egress path,
so the processor_id() were used to tx queue selection.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c   |  165 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/virtio_net.h |    6 ++
 2 files changed, 169 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0d871f8..89bb5e7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,10 @@
 #include <linux/scatterlist.h>
 #include <linux/if_vlan.h>
 #include <linux/slab.h>
+#include <linux/highmem.h>
+#include <linux/cpu_rmap.h>
+#include <linux/interrupt.h>
+#include <linux/cpumask.h>
 
 static int napi_weight = 128;
 module_param(napi_weight, int, 0444);
@@ -40,6 +44,7 @@ module_param(gso, bool, 0444);
 
 #define VIRTNET_SEND_COMMAND_SG_MAX    2
 #define VIRTNET_DRIVER_VERSION "1.0.0"
+#define TAP_HASH_MASK 0xFF
 
 struct virtnet_send_stats {
 	struct u64_stats_sync syncp;
@@ -89,6 +94,9 @@ struct receive_queue {
 
 	/* Active rx statistics */
 	struct virtnet_recv_stats __percpu *stats;
+
+	/* FIXME: per vector instead of per queue ?? */
+	cpumask_var_t affinity_mask;
 };
 
 struct virtnet_info {
@@ -110,6 +118,11 @@ struct virtnet_info {
 
 	/* Host will pass rxhash to us. */
 	bool has_rxhash;
+
+	/* A page of flow director */
+	struct page *fd_page;
+
+	cpumask_var_t affinity_mask;
 };
 
 struct skb_vnet_hdr {
@@ -386,6 +399,7 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
 	if (vi->has_rxhash)
 		skb->rxhash = hdr->rhdr.rxhash;
 
+	skb_record_rx_queue(skb, rq->vq->queue_index / 2);
 	netif_receive_skb(skb);
 	return;
 
@@ -722,6 +736,19 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static int virtnet_set_fd(struct net_device *dev, u32 pfn)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	struct virtio_device *vdev = vi->vdev;
+
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		vdev->config->set(vdev,
+				  offsetof(struct virtio_net_config_fd, addr),
+				  &pfn, sizeof(u32));
+	}
+	return 0;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -1017,6 +1044,39 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
+#ifdef CONFIG_RFS_ACCEL
+
+int virtnet_fd(struct net_device *net_dev, const struct sk_buff *skb,
+	       u16 rxq_index, u32 flow_id)
+{
+	struct virtnet_info *vi = netdev_priv(net_dev);
+	u16 *table = NULL;
+
+	if (skb->protocol != htons(ETH_P_IP) || !skb->rxhash)
+		return -EPROTONOSUPPORT;
+
+	table = kmap_atomic(vi->fd_page);
+	table[skb->rxhash & TAP_HASH_MASK] = rxq_index;
+	kunmap_atomic(table);
+
+	return 0;
+}
+#endif
+
+static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+	int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
+					       smp_processor_id();
+
+	/* As we make use of the accelerate rfs which let the scheduler to
+	 * balance the load, it make sense to choose the tx queue also based on
+	 * theprocessor id?
+	 */
+	while (unlikely(txq >= dev->real_num_tx_queues))
+		txq -= dev->real_num_tx_queues;
+	return txq;
+}
+
 static const struct net_device_ops virtnet_netdev = {
 	.ndo_open            = virtnet_open,
 	.ndo_stop   	     = virtnet_close,
@@ -1028,9 +1088,13 @@ static const struct net_device_ops virtnet_netdev = {
 	.ndo_get_stats64     = virtnet_stats,
 	.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
+	.ndo_select_queue    = virtnet_select_queue,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = virtnet_netpoll,
 #endif
+#ifdef CONFIG_RFS_ACCEL
+	.ndo_rx_flow_steer   = virtnet_fd,
+#endif
 };
 
 static void virtnet_update_status(struct virtnet_info *vi)
@@ -1272,12 +1336,76 @@ static int virtnet_setup_vqs(struct virtnet_info *vi)
 	return ret;
 }
 
+static int virtnet_init_rx_cpu_rmap(struct virtnet_info *vi)
+{
+#ifdef CONFIG_RFS_ACCEL
+	struct virtio_device *vdev = vi->vdev;
+	int i, rc;
+
+	vi->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(vi->num_queue_pairs);
+	if (!vi->dev->rx_cpu_rmap)
+		return -ENOMEM;
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		rc = irq_cpu_rmap_add(vi->dev->rx_cpu_rmap,
+				vdev->config->get_vq_irq(vdev, vi->rq[i]->vq));
+		if (rc) {
+			free_irq_cpu_rmap(vi->dev->rx_cpu_rmap);
+			vi->dev->rx_cpu_rmap = NULL;
+			return rc;
+		}
+	}
+#endif
+	return 0;
+}
+
+static int virtnet_init_rq_affinity(struct virtnet_info *vi)
+{
+	struct virtio_device *vdev = vi->vdev;
+	int i;
+
+	/* FIXME: TX/RX share a vector */
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		if (!alloc_cpumask_var(&vi->rq[i]->affinity_mask, GFP_KERNEL))
+			goto err_out;
+		cpumask_set_cpu(i, vi->rq[i]->affinity_mask);
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      vi->rq[i]->affinity_mask);
+	}
+
+	return 0;
+err_out:
+	while (i) {
+		i--;
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      NULL);
+		free_cpumask_var(vi->rq[i]->affinity_mask);
+	}
+	return -ENOMEM;
+}
+
+static void virtnet_free_rq_affinity(struct virtnet_info *vi)
+{
+	struct virtio_device *vdev = vi->vdev;
+	int i;
+
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      NULL);
+		free_cpumask_var(vi->rq[i]->affinity_mask);
+	}
+}
+
 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;
 	struct net_device *dev;
 	struct virtnet_info *vi;
 	u16 num_queues, num_queue_pairs;
+	struct page *page = NULL;
+	u16 *table = NULL;
 
 	/* Find if host supports multiqueue virtio_net device */
 	err = virtio_config_val(vdev, VIRTIO_NET_F_MULTIQUEUE,
@@ -1298,7 +1426,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	/* Set up network device as normal. */
 	dev->priv_flags |= IFF_UNICAST_FLT;
 	dev->netdev_ops = &virtnet_netdev;
-	dev->features = NETIF_F_HIGHDMA;
+	dev->features = NETIF_F_HIGHDMA | NETIF_F_NTUPLE;
 
 	SET_ETHTOOL_OPS(dev, &virtnet_ethtool_ops);
 	SET_NETDEV_DEV(dev, &vdev->dev);
@@ -1342,6 +1470,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	vdev->priv = vi;
 	vi->num_queue_pairs = num_queue_pairs;
 
+
 	/* If we can receive ANY GSO packets, we must allocate large ones. */
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
@@ -1382,6 +1511,31 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
+	/* Config flow director */
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		page = alloc_page(GFP_KERNEL);
+		if (!page)
+			return -ENOMEM;
+		table = (u16 *)kmap_atomic(page);
+		for (i = 0; i < (PAGE_SIZE / 16); i++) {
+			/* invalid all entries */
+			table[i] = num_queue_pairs;
+		}
+
+		vi->fd_page = page;
+		kunmap_atomic(table);
+		virtnet_set_fd(dev, page_to_pfn(page));
+
+		err = virtnet_init_rx_cpu_rmap(vi);
+		if (err)
+			goto free_recv_bufs;
+
+		err = virtnet_init_rq_affinity(vi);
+		if (err)
+			goto free_recv_bufs;
+
+	}
+
 	/* Assume link up if device can't report link status,
 	   otherwise get link status from config. */
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
@@ -1437,6 +1591,13 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
 	/* Free memory for send and receive queues */
 	free_rq_sq(vi);
 
+	/* Free the page of flow director */
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		if (vi->fd_page)
+			put_page(vi->fd_page);
+
+		virtnet_free_rq_affinity(vi);
+	}
 	free_netdev(vi->dev);
 }
 
@@ -1453,7 +1614,7 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, VIRTIO_NET_F_MULTIQUEUE,
-	VIRTIO_NET_F_GUEST_RXHASH,
+	VIRTIO_NET_F_GUEST_RXHASH, VIRTIO_NET_F_HOST_FD,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 2291317..abcea52 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -51,6 +51,7 @@
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20	/* Extra RX mode control support */
 #define VIRTIO_NET_F_MULTIQUEUE	21	/* Device supports multiple TXQ/RXQ */
 #define VIRTIO_NET_F_GUEST_RXHASH 22    /* Guest can receive rxhash */
+#define VIRTIO_NET_F_HOST_FD    23      /* Host has a flow director */
 
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 
@@ -63,6 +64,11 @@ struct virtio_net_config {
 	__u16 num_queues;
 } __attribute__((packed));
 
+struct virtio_net_config_fd {
+	struct virtio_net_config cfg;
+	u32 addr;
+} __packed;
+
 /* This is the first element of the scatter-gather list.  If you don't
  * specify GSO, CSUM or HASH features, you can simply ignore the header. */
 struct virtio_net_hdr {


^ permalink raw reply related

* [net-next RFC PATCH 4/5] virtio: introduce a method to get the irq of a specific virtqueue
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

Device specific irq configuration may be need in order to do some
optimization. So a new configuration is needed to get the irq of a
virtqueue.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/lguest/lguest_device.c |    8 ++++++++
 drivers/s390/kvm/kvm_virtio.c  |    6 ++++++
 drivers/virtio/virtio_mmio.c   |    8 ++++++++
 drivers/virtio/virtio_pci.c    |   12 ++++++++++++
 include/linux/virtio_config.h  |    4 ++++
 5 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 595d731..6483bff 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -386,6 +386,13 @@ static const char *lg_bus_name(struct virtio_device *vdev)
 	return "";
 }
 
+static int lg_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct lguest_vq_info *lvq = vq->priv;
+
+	return lvq->config.irq;
+}
+
 /* The ops structure which hooks everything together. */
 static struct virtio_config_ops lguest_config_ops = {
 	.get_features = lg_get_features,
@@ -398,6 +405,7 @@ static struct virtio_config_ops lguest_config_ops = {
 	.find_vqs = lg_find_vqs,
 	.del_vqs = lg_del_vqs,
 	.bus_name = lg_bus_name,
+	.get_vq_irq = lg_get_vq_irq,
 };
 
 /*
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 8af868b..a8d5ca1 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -268,6 +268,11 @@ static const char *kvm_bus_name(struct virtio_device *vdev)
 	return "";
 }
 
+static int kvm_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	return 0x2603;
+}
+
 /*
  * The config ops structure as defined by virtio config
  */
@@ -282,6 +287,7 @@ static struct virtio_config_ops kvm_vq_configspace_ops = {
 	.find_vqs = kvm_find_vqs,
 	.del_vqs = kvm_del_vqs,
 	.bus_name = kvm_bus_name,
+	.get_vq_irq = kvm_get_vq_irq,
 };
 
 /*
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 2f57380..309d471 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -368,6 +368,13 @@ static const char *vm_bus_name(struct virtio_device *vdev)
 	return vm_dev->pdev->name;
 }
 
+static int vm_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+
+	return platform_get_irq(vm_dev->pdev, 0);
+}
+
 static struct virtio_config_ops virtio_mmio_config_ops = {
 	.get		= vm_get,
 	.set		= vm_set,
@@ -379,6 +386,7 @@ static struct virtio_config_ops virtio_mmio_config_ops = {
 	.get_features	= vm_get_features,
 	.finalize_features = vm_finalize_features,
 	.bus_name	= vm_bus_name,
+	.get_vq_irq     = vm_get_vq_irq,
 };
 
 
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 229ea56..4f99164 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -583,6 +583,17 @@ static const char *vp_bus_name(struct virtio_device *vdev)
 	return pci_name(vp_dev->pci_dev);
 }
 
+static int vp_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_vq_info *info = vq->priv;
+
+	if (vp_dev->intx_enabled)
+		return vp_dev->pci_dev->irq;
+	else
+		return vp_dev->msix_entries[info->msix_vector].vector;
+}
+
 static struct virtio_config_ops virtio_pci_config_ops = {
 	.get		= vp_get,
 	.set		= vp_set,
@@ -594,6 +605,7 @@ static struct virtio_config_ops virtio_pci_config_ops = {
 	.get_features	= vp_get_features,
 	.finalize_features = vp_finalize_features,
 	.bus_name	= vp_bus_name,
+	.get_vq_irq     = vp_get_vq_irq,
 };
 
 static void virtio_pci_release_dev(struct device *_d)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 63f98d0..7b783a6 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -104,6 +104,9 @@
  *	vdev: the virtio_device
  *      This returns a pointer to the bus name a la pci_name from which
  *      the caller can then copy.
+ * @get_vq_irq: get the irq numer of the specific virt queue.
+ *      vdev: the virtio_device
+ *      vq: the virtqueue
  */
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
@@ -122,6 +125,7 @@ struct virtio_config_ops {
 	u32 (*get_features)(struct virtio_device *vdev);
 	void (*finalize_features)(struct virtio_device *vdev);
 	const char *(*bus_name)(struct virtio_device *vdev);
+	int (*get_vq_irq)(struct virtio_device *vdev, struct virtqueue *vq);
 };
 
 /* If driver didn't advertise the feature, it will never appear. */

^ permalink raw reply related

* [net-next RFC PATCH 3/5] macvtap: flow director support
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/macvlan.c      |    4 ++++
 drivers/net/macvtap.c      |   36 ++++++++++++++++++++++++++++++++++--
 include/linux/if_macvlan.h |    1 +
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 7413497..b0cb7ce 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -706,6 +706,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	vlan->port     = port;
 	vlan->receive  = receive;
 	vlan->forward  = forward;
+	vlan->fd_page[0] = NULL;
 
 	vlan->mode     = MACVLAN_MODE_VEPA;
 	if (data && data[IFLA_MACVLAN_MODE])
@@ -749,6 +750,9 @@ void macvlan_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 
+	if (vlan->fd_page[0])
+		put_page(vlan->fd_page[0]);
+
 	list_del(&vlan->list);
 	unregister_netdevice_queue(dev, head);
 }
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 504c745..a34eb84 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -14,6 +14,7 @@
 #include <linux/wait.h>
 #include <linux/cdev.h>
 #include <linux/fs.h>
+#include <linux/highmem.h>
 
 #include <net/net_namespace.h>
 #include <net/rtnetlink.h>
@@ -62,6 +63,8 @@ static DEFINE_IDR(minor_idr);
 static struct class *macvtap_class;
 static struct cdev macvtap_cdev;
 
+#define TAP_HASH_MASK 0xFF
+
 static const struct proto_ops macvtap_socket_ops;
 
 /*
@@ -189,6 +192,11 @@ static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
 	/* Check if we can use flow to select a queue */
 	rxq = skb_get_rxhash(skb);
 	if (rxq) {
+		if (vlan->fd_page[0]) {
+			u16 *table = kmap_atomic(vlan->fd_page[0]);
+			rxq = table[rxq & TAP_HASH_MASK];
+			kunmap_atomic(table);
+		}
 		tap = rcu_dereference(vlan->taps[rxq % numvtaps]);
 		if (tap)
 			goto out;
@@ -851,6 +859,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 {
 	struct macvtap_queue *q = file->private_data;
 	struct macvlan_dev *vlan;
+	struct tun_fd tfd;
 	void __user *argp = (void __user *)arg;
 	struct ifreq __user *ifr = argp;
 	unsigned int __user *up = argp;
@@ -891,8 +900,8 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		return ret;
 
 	case TUNGETFEATURES:
-		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_RXHASH,
-			     up))
+		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_RXHASH |
+			     IFF_FD, up))
 			return -EFAULT;
 		return 0;
 
@@ -918,6 +927,29 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		q->vnet_hdr_sz = s;
 		return 0;
 
+	case TUNSETFD:
+		rcu_read_lock_bh();
+		vlan = rcu_dereference(q->vlan);
+		if (!vlan)
+			ret = -ENOLINK;
+		else {
+			if (copy_from_user(&tfd, argp, sizeof(tfd)))
+				ret = -EFAULT;
+			if (vlan->fd_page[0]) {
+				put_page(vlan->fd_page[0]);
+				vlan->fd_page[0] = NULL;
+			}
+
+			/* put_page() in macvlan_dellink() */
+			if (get_user_pages_fast(tfd.addr, 1, 0,
+						&vlan->fd_page[0]) != 1)
+				ret = -EFAULT;
+			else
+				ret = 0;
+		}
+		rcu_read_unlock_bh();
+		return ret;
+
 	case TUNSETOFFLOAD:
 		/* let the user check for future flags */
 		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index d103dca..69a87a1 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -65,6 +65,7 @@ struct macvlan_dev {
 	struct macvtap_queue	*taps[MAX_MACVTAP_QUEUES];
 	int			numvtaps;
 	int			minor;
+	struct page             *fd_page[1];
 };
 
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,

^ permalink raw reply related

* [net-next RFC PATCH 2/5] tuntap: simple flow director support
From: Jason Wang @ 2011-12-05  8:58 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

This patch adds a simple flow director to tun/tap device. It is just a
page that contains the hash to queue mapping which could be changed by
user-space. The backend (tap/macvtap) would query this table to get
the desired queue of a packets when it send packets to userspace.

The page address were set through a new kind of ioctl - TUNSETFD and
were pinned until device exit or another new page were specified.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c      |   63 ++++++++++++++++++++++++++++++++++++++++--------
 include/linux/if_tun.h |   10 ++++++++
 2 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7d22b4b..2efaf81 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -64,6 +64,7 @@
 #include <linux/nsproxy.h>
 #include <linux/virtio_net.h>
 #include <linux/rcupdate.h>
+#include <linux/highmem.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 #include <net/rtnetlink.h>
@@ -109,6 +110,7 @@ struct tap_filter {
 };
 
 #define MAX_TAP_QUEUES (NR_CPUS < 16 ? NR_CPUS : 16)
+#define TAP_HASH_MASK  0xFF
 
 struct tun_file {
 	struct sock sk;
@@ -128,6 +130,7 @@ struct tun_sock;
 
 struct tun_struct {
 	struct tun_file		*tfiles[MAX_TAP_QUEUES];
+	struct page             *fd_page[1];
 	unsigned int            numqueues;
 	unsigned int 		flags;
 	uid_t			owner;
@@ -156,7 +159,7 @@ static struct tun_file *tun_get_queue(struct net_device *dev,
 	struct tun_struct *tun = netdev_priv(dev);
 	struct tun_file *tfile = NULL;
 	int numqueues = tun->numqueues;
-	__u32 rxq;
+	__u32 rxq, rxhash;
 
 	BUG_ON(!rcu_read_lock_held());
 
@@ -168,6 +171,22 @@ static struct tun_file *tun_get_queue(struct net_device *dev,
 		goto out;
 	}
 
+	rxhash = skb_get_rxhash(skb);
+	if (rxhash) {
+		if (tun->fd_page[0]) {
+			u16 *table = kmap_atomic(tun->fd_page[0]);
+			rxq = table[rxhash & TAP_HASH_MASK];
+			kunmap_atomic(table);
+			if (rxq < numqueues) {
+				tfile = rcu_dereference(tun->tfiles[rxq]);
+				goto out;
+			}
+		}
+		rxq = ((u64)rxhash * numqueues) >> 32;
+		tfile = rcu_dereference(tun->tfiles[rxq]);
+		goto out;
+	}
+
 	if (likely(skb_rx_queue_recorded(skb))) {
 		rxq = skb_get_rx_queue(skb);
 
@@ -178,14 +197,6 @@ static struct tun_file *tun_get_queue(struct net_device *dev,
 		goto out;
 	}
 
-	/* Check if we can use flow to select a queue */
-	rxq = skb_get_rxhash(skb);
-	if (rxq) {
-		u32 idx = ((u64)rxq * numqueues) >> 32;
-		tfile = rcu_dereference(tun->tfiles[idx]);
-		goto out;
-	}
-
 	tfile = rcu_dereference(tun->tfiles[0]);
 out:
 	return tfile;
@@ -1020,6 +1031,14 @@ out:
 	return ret;
 }
 
+static void tun_destructor(struct net_device *dev)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+	if (tun->fd_page[0])
+		put_page(tun->fd_page[0]);
+	free_netdev(dev);
+}
+
 static void tun_setup(struct net_device *dev)
 {
 	struct tun_struct *tun = netdev_priv(dev);
@@ -1028,7 +1047,7 @@ static void tun_setup(struct net_device *dev)
 	tun->group = -1;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
-	dev->destructor = free_netdev;
+	dev->destructor = tun_destructor;
 }
 
 /* Trivial set of netlink ops to allow deleting tun or tap
@@ -1230,6 +1249,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		tun = netdev_priv(dev);
 		tun->dev = dev;
 		tun->flags = flags;
+		tun->fd_page[0] = NULL;
 
 		security_tun_dev_post_create(&tfile->sk);
 
@@ -1353,6 +1373,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	struct net_device *dev = NULL;
 	void __user* argp = (void __user*)arg;
 	struct ifreq ifr;
+	struct tun_fd tfd;
 	int ret;
 
 	if (cmd == TUNSETIFF || cmd == TUNATTACHQUEUE || _IOC_TYPE(cmd) == 0x89)
@@ -1364,7 +1385,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		 * This is needed because we never checked for invalid flags on
 		 * TUNSETIFF. */
 		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
-				IFF_VNET_HDR | IFF_MULTI_QUEUE | IFF_RXHASH,
+				IFF_VNET_HDR | IFF_MULTI_QUEUE | IFF_RXHASH |
+				IFF_FD,
 				(unsigned int __user*)argp);
 	}
 
@@ -1476,6 +1498,25 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		ret = set_offload(tun, arg);
 		break;
 
+	case TUNSETFD:
+		if (copy_from_user(&tfd, argp, sizeof(tfd)))
+			ret = -EFAULT;
+		else {
+			if (tun->fd_page[0]) {
+				put_page(tun->fd_page[0]);
+				tun->fd_page[0] = NULL;
+			}
+
+			/* put_page() in tun_destructor() */
+			if (get_user_pages_fast(tfd.addr, 1, 0,
+						&tun->fd_page[0]) != 1)
+				ret = -EFAULT;
+			else
+				ret = 0;
+		}
+
+		break;
+
 	case SIOCGIFHWADDR:
 		/* Get hw address */
 		memcpy(ifr.ifr_hwaddr.sa_data, tun->dev->dev_addr, ETH_ALEN);
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index a1f6f3f..726731d 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -36,6 +36,8 @@
 #define TUN_VNET_HDR 	0x0200
 #define TUN_TAP_MQ      0x0400
 
+struct tun_fd;
+
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 
 #define TUNSETDEBUG   _IOW('T', 201, int) 
@@ -56,6 +58,7 @@
 #define TUNSETVNETHDRSZ _IOW('T', 216, int)
 #define TUNATTACHQUEUE  _IOW('T', 217, int)
 #define TUNDETACHQUEUE  _IOW('T', 218, int)
+#define TUNSETFD        _IOW('T', 219, struct tun_fd)
 
 
 /* TUNSETIFF ifr flags */
@@ -67,6 +70,7 @@
 #define IFF_TUN_EXCL	0x8000
 #define IFF_MULTI_QUEUE 0x0100
 #define IFF_RXHASH      0x0200
+#define IFF_FD          0x0400
 
 /* Features for GSO (TUNSETOFFLOAD). */
 #define TUN_F_CSUM	0x01	/* You can hand me unchecksummed packets. */
@@ -97,6 +101,12 @@ struct tun_filter {
 	__u8   addr[0][ETH_ALEN];
 };
 
+/* Programmable flow director */
+struct tun_fd {
+	unsigned long addr;
+	size_t size;
+};
+
 #ifdef __KERNEL__
 #if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE)
 struct socket *tun_get_socket(struct file *);

^ permalink raw reply related

* [net-next RFC PATCH 1/5] virtio_net: passing rxhash through vnet_hdr
From: Jason Wang @ 2011-12-05  8:58 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

This patch enables the ability to pass the rxhash value to guest
through vnet_hdr. This is useful for guest when it wants to cooperate
with virtual device to steer a flow to dedicated guest cpu.

This feature is negotiated through VIRTIO_NET_F_GUEST_RXHASH.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/macvtap.c      |   10 ++++++----
 drivers/net/tun.c          |   44 +++++++++++++++++++++++++-------------------
 drivers/net/virtio_net.c   |   26 ++++++++++++++++++++++----
 drivers/vhost/net.c        |   10 +++++++---
 drivers/vhost/vhost.h      |    5 +++--
 include/linux/if_tun.h     |    1 +
 include/linux/virtio_net.h |   10 +++++++++-
 7 files changed, 73 insertions(+), 33 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7c88d13..504c745 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -760,16 +760,17 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 	int vnet_hdr_len = 0;
 
 	if (q->flags & IFF_VNET_HDR) {
-		struct virtio_net_hdr vnet_hdr;
+		struct virtio_net_hdr_rxhash vnet_hdr;
 		vnet_hdr_len = q->vnet_hdr_sz;
 		if ((len -= vnet_hdr_len) < 0)
 			return -EINVAL;
 
-		ret = macvtap_skb_to_vnet_hdr(skb, &vnet_hdr);
+		ret = macvtap_skb_to_vnet_hdr(skb, &vnet_hdr.hdr.hdr);
 		if (ret)
 			return ret;
 
-		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
+		vnet_hdr.rxhash = skb->rxhash;
+		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, q->vnet_hdr_sz))
 			return -EFAULT;
 	}
 
@@ -890,7 +891,8 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		return ret;
 
 	case TUNGETFEATURES:
-		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR, up))
+		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_RXHASH,
+			     up))
 			return -EFAULT;
 		return 0;
 
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index afb11d1..7d22b4b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -869,49 +869,55 @@ static ssize_t tun_put_user(struct tun_file *tfile,
 	}
 
 	if (tfile->flags & TUN_VNET_HDR) {
-		struct virtio_net_hdr gso = { 0 }; /* no info leak */
-		if ((len -= tfile->vnet_hdr_sz) < 0)
+		struct virtio_net_hdr_rxhash hdr;
+		struct virtio_net_hdr *gso = (struct virtio_net_hdr *)&hdr;
+
+		if ((len -= tfile->vnet_hdr_sz) < 0 ||
+		    tfile->vnet_hdr_sz > sizeof(struct virtio_net_hdr_rxhash))
 			return -EINVAL;
 
+		memset(&hdr, 0, sizeof(hdr));
 		if (skb_is_gso(skb)) {
 			struct skb_shared_info *sinfo = skb_shinfo(skb);
 
 			/* This is a hint as to how much should be linear. */
-			gso.hdr_len = skb_headlen(skb);
-			gso.gso_size = sinfo->gso_size;
+			gso->hdr_len = skb_headlen(skb);
+			gso->gso_size = sinfo->gso_size;
 			if (sinfo->gso_type & SKB_GSO_TCPV4)
-				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
+				gso->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
 			else if (sinfo->gso_type & SKB_GSO_TCPV6)
-				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+				gso->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
 			else if (sinfo->gso_type & SKB_GSO_UDP)
-				gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
+				gso->gso_type = VIRTIO_NET_HDR_GSO_UDP;
 			else {
 				pr_err("unexpected GSO type: "
 				       "0x%x, gso_size %d, hdr_len %d\n",
-				       sinfo->gso_type, gso.gso_size,
-				       gso.hdr_len);
+				       sinfo->gso_type, gso->gso_size,
+				       gso->hdr_len);
 				print_hex_dump(KERN_ERR, "tun: ",
 					       DUMP_PREFIX_NONE,
 					       16, 1, skb->head,
-					       min((int)gso.hdr_len, 64), true);
+					       min((int)gso->hdr_len, 64),
+					       true);
 				WARN_ON_ONCE(1);
 				return -EINVAL;
 			}
 			if (sinfo->gso_type & SKB_GSO_TCP_ECN)
-				gso.gso_type |= VIRTIO_NET_HDR_GSO_ECN;
+				gso->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
 		} else
-			gso.gso_type = VIRTIO_NET_HDR_GSO_NONE;
+			gso->gso_type = VIRTIO_NET_HDR_GSO_NONE;
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
-			gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-			gso.csum_start = skb_checksum_start_offset(skb);
-			gso.csum_offset = skb->csum_offset;
+			gso->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
+			gso->csum_start = skb_checksum_start_offset(skb);
+			gso->csum_offset = skb->csum_offset;
 		} else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
-			gso.flags = VIRTIO_NET_HDR_F_DATA_VALID;
+			gso->flags = VIRTIO_NET_HDR_F_DATA_VALID;
 		} /* else everything is zero */
 
-		if (unlikely(memcpy_toiovecend(iv, (void *)&gso, total,
-					       sizeof(gso))))
+		hdr.rxhash = skb_get_rxhash(skb);
+		if (unlikely(memcpy_toiovecend(iv, (void *)&hdr, total,
+					       tfile->vnet_hdr_sz)))
 			return -EFAULT;
 		total += tfile->vnet_hdr_sz;
 	}
@@ -1358,7 +1364,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		 * This is needed because we never checked for invalid flags on
 		 * TUNSETIFF. */
 		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
+				IFF_VNET_HDR | IFF_MULTI_QUEUE | IFF_RXHASH,
 				(unsigned int __user*)argp);
 	}
 
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 157ee63..0d871f8 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -107,12 +107,16 @@ struct virtnet_info {
 
 	/* Host will merge rx buffers for big packets (shake it! shake it!) */
 	bool mergeable_rx_bufs;
+
+	/* Host will pass rxhash to us. */
+	bool has_rxhash;
 };
 
 struct skb_vnet_hdr {
 	union {
 		struct virtio_net_hdr hdr;
 		struct virtio_net_hdr_mrg_rxbuf mhdr;
+		struct virtio_net_hdr_rxhash rhdr;
 	};
 	unsigned int num_sg;
 };
@@ -205,7 +209,10 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
 	hdr = skb_vnet_hdr(skb);
 
 	if (vi->mergeable_rx_bufs) {
-		hdr_len = sizeof hdr->mhdr;
+		if (vi->has_rxhash)
+			hdr_len = sizeof hdr->rhdr;
+		else
+			hdr_len = sizeof hdr->mhdr;
 		offset = hdr_len;
 	} else {
 		hdr_len = sizeof hdr->hdr;
@@ -376,6 +383,9 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
 		skb_shinfo(skb)->gso_segs = 0;
 	}
 
+	if (vi->has_rxhash)
+		skb->rxhash = hdr->rhdr.rxhash;
+
 	netif_receive_skb(skb);
 	return;
 
@@ -645,9 +655,12 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb,
 	hdr->mhdr.num_buffers = 0;
 
 	/* Encode metadata header at front. */
-	if (vi->mergeable_rx_bufs)
-		sg_set_buf(sg, &hdr->mhdr, sizeof hdr->mhdr);
-	else
+	if (vi->mergeable_rx_bufs) {
+		if (vi->has_rxhash)
+			sg_set_buf(sg, &hdr->rhdr, sizeof hdr->rhdr);
+		else
+			sg_set_buf(sg, &hdr->mhdr, sizeof hdr->mhdr);
+	} else
 		sg_set_buf(sg, &hdr->hdr, sizeof hdr->hdr);
 
 	hdr->num_sg = skb_to_sgvec(skb, sg + 1, 0, skb->len) + 1;
@@ -1338,8 +1351,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_RXHASH))
+		vi->has_rxhash = true;
+
 	/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
 	err = virtnet_setup_vqs(vi);
+
 	if (err)
 		goto free_netdev;
 
@@ -1436,6 +1453,7 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, VIRTIO_NET_F_MULTIQUEUE,
+	VIRTIO_NET_F_GUEST_RXHASH,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 882a51f..b2d6548 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -768,9 +768,13 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
 	size_t vhost_hlen, sock_hlen, hdr_len;
 	int i;
 
-	hdr_len = (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ?
-			sizeof(struct virtio_net_hdr_mrg_rxbuf) :
-			sizeof(struct virtio_net_hdr);
+	if (features & (1 << VIRTIO_NET_F_MRG_RXBUF))
+		hdr_len = (features & (1 << VIRTIO_NET_F_GUEST_RXHASH)) ?
+			sizeof(struct virtio_net_hdr_rxhash) :
+			sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	else
+		hdr_len = sizeof(struct virtio_net_hdr);
+
 	if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
 		/* vhost provides vnet_hdr */
 		vhost_hlen = hdr_len;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a801e28..4ad2d5f 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -115,7 +115,7 @@ struct vhost_virtqueue {
 	/* hdr is used to store the virtio header.
 	 * Since each iovec has >= 1 byte length, we never need more than
 	 * header length entries to store the header. */
-	struct iovec hdr[sizeof(struct virtio_net_hdr_mrg_rxbuf)];
+	struct iovec hdr[sizeof(struct virtio_net_hdr_rxhash)];
 	struct iovec *indirect;
 	size_t vhost_hlen;
 	size_t sock_hlen;
@@ -203,7 +203,8 @@ enum {
 			 (1ULL << VIRTIO_RING_F_EVENT_IDX) |
 			 (1ULL << VHOST_F_LOG_ALL) |
 			 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
-			 (1ULL << VIRTIO_NET_F_MRG_RXBUF),
+			 (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
+			 (1ULL << VIRTIO_NET_F_GUEST_RXHASH) ,
 };
 
 static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index d3f24d8..a1f6f3f 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -66,6 +66,7 @@
 #define IFF_VNET_HDR	0x4000
 #define IFF_TUN_EXCL	0x8000
 #define IFF_MULTI_QUEUE 0x0100
+#define IFF_RXHASH      0x0200
 
 /* Features for GSO (TUNSETOFFLOAD). */
 #define TUN_F_CSUM	0x01	/* You can hand me unchecksummed packets. */
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index c92b83f..2291317 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -50,6 +50,7 @@
 #define VIRTIO_NET_F_CTRL_VLAN	19	/* Control channel VLAN filtering */
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20	/* Extra RX mode control support */
 #define VIRTIO_NET_F_MULTIQUEUE	21	/* Device supports multiple TXQ/RXQ */
+#define VIRTIO_NET_F_GUEST_RXHASH 22    /* Guest can receive rxhash */
 
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 
@@ -63,7 +64,7 @@ struct virtio_net_config {
 } __attribute__((packed));
 
 /* This is the first element of the scatter-gather list.  If you don't
- * specify GSO or CSUM features, you can simply ignore the header. */
+ * specify GSO, CSUM or HASH features, you can simply ignore the header. */
 struct virtio_net_hdr {
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM	1	// Use csum_start, csum_offset
 #define VIRTIO_NET_HDR_F_DATA_VALID	2	// Csum is valid
@@ -87,6 +88,13 @@ struct virtio_net_hdr_mrg_rxbuf {
 	__u16 num_buffers;	/* Number of merged rx buffers */
 };
 
+/* This is the version of the header to use when GUEST_RXHASH
+ * feature has been negotiated. */
+struct virtio_net_hdr_rxhash {
+	struct virtio_net_hdr_mrg_rxbuf hdr;
+	__u32 rxhash;
+};
+
 /*
  * Control virtqueue data structures
  *

^ permalink raw reply related

* [net-next RFC PATCH 0/5] Series short description
From: Jason Wang @ 2011-12-05  8:58 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings

multiple queue virtio-net: flow steering through host/guest cooperation

Hello all:

This is a rough series adds the guest/host cooperation of flow
steering support based on Krish Kumar's multiple queue virtio-net
driver patch 3/3 (http://lwn.net/Articles/467283/).

This idea is simple, the backend pass the rxhash to the guest and
guest would tell the backend the hash to queue mapping when necessary
then backend can choose the queue based on the hash value of the
packet.  The table is just a page shared bettwen userspace and the
backend.

Patch 1 enable the ability to pass the rxhash through vnet_hdr to
guest.
Patch 2,3 implement a very simple flow director for tap and
mavtap. tap part is based on the multiqueue tap patches posted by me
(http://lwn.net/Articles/459270/).
Patch 4 implement a method for virtio device to find the irq of a
specific virtqueue, in order to do device specific interrupt
optimization
Patch 5 is the part of the guest driver that using accelerate rfs to
program the flow director and with some optimizations on irq affinity
and tx queue selection.

This is just a prototype that demonstrates the idea, there are still
things need to be discussed:

- An alternative idea instead of shared page is ctrl vq, the reason
  that a shared table is preferable is the delay of ctrl vq itself.
- Optimization on irq affinity and tx queue selection

Comments are welcomed, thanks!

---

Jason Wang (5):
      virtio_net: passing rxhash through vnet_hdr
      tuntap: simple flow director support
      macvtap: flow director support
      virtio: introduce a method to get the irq of a specific virtqueue
      virtio-net: flow director support


 drivers/lguest/lguest_device.c |    8 ++
 drivers/net/macvlan.c          |    4 +
 drivers/net/macvtap.c          |   42 ++++++++-
 drivers/net/tun.c              |  105 ++++++++++++++++------
 drivers/net/virtio_net.c       |  189 +++++++++++++++++++++++++++++++++++++++-
 drivers/s390/kvm/kvm_virtio.c  |    6 +
 drivers/vhost/net.c            |   10 +-
 drivers/vhost/vhost.h          |    5 +
 drivers/virtio/virtio_mmio.c   |    8 ++
 drivers/virtio/virtio_pci.c    |   12 +++
 include/linux/if_macvlan.h     |    1 
 include/linux/if_tun.h         |   11 ++
 include/linux/virtio_config.h  |    4 +
 include/linux/virtio_net.h     |   16 +++
 14 files changed, 377 insertions(+), 44 deletions(-)

-- 
Signature

^ permalink raw reply

* Re: [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Michael Wang @ 2011-12-05  8:37 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, sassmann, Flavio Leitner
In-Reply-To: <1323073241-21144-2-git-send-email-jeffrey.t.kirsher@intel.com>

Hi, Jeff

Please reserve "From: Michael Wang <wangyun@linux.vnet.ibm.com>" in content.
And because Flavio not respond these days, I add his name for him.

Thanks,
Michael Wang

On 12/05/2011 04:20 PM, Jeff Kirsher wrote:

> Based on the original patch submitted my Michael Wang
> <wangyun@linux.vnet.ibm.com>.

From: Michael Wang <wangyun@linux.vnet.ibm.com>

> Descriptors may not be write-back while checking TX hang with flag
> FLAG2_DMA_BURST on.
> So when we detect hang, we just flush the descriptor and detect
> again for once.
> 
> -v2 change 1 to true and 0 to false and remove extra ()
> 
> CC: Michael Wang <wangyun@linux.vnet.ibm.com>
> CC: Flavio Leitner <fbl@redhat.com>

Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
Signed-off-by: Flavio Leitner <fbl@redhat.com>

> Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>

> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |    1 +
>  drivers/net/ethernet/intel/e1000e/netdev.c |   23 ++++++++++++++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
> index 9fe18d1..f478a22 100644
> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
> @@ -309,6 +309,7 @@ struct e1000_adapter {
>  	u32 txd_cmd;
> 
>  	bool detect_tx_hung;
> +	bool tx_hang_recheck;
>  	u8 tx_timeout_factor;
> 
>  	u32 tx_int_delay;
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index c6e9763..c12df69 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -1014,6 +1014,7 @@ static void e1000_print_hw_hang(struct work_struct *work)
>  	struct e1000_adapter *adapter = container_of(work,
>  	                                             struct e1000_adapter,
>  	                                             print_hang_task);
> +	struct net_device *netdev = adapter->netdev;
>  	struct e1000_ring *tx_ring = adapter->tx_ring;
>  	unsigned int i = tx_ring->next_to_clean;
>  	unsigned int eop = tx_ring->buffer_info[i].next_to_watch;
> @@ -1025,6 +1026,21 @@ static void e1000_print_hw_hang(struct work_struct *work)
>  	if (test_bit(__E1000_DOWN, &adapter->state))
>  		return;
> 
> +	if (!adapter->tx_hang_recheck &&
> +	    (adapter->flags2 & FLAG2_DMA_BURST)) {
> +		/* May be block on write-back, flush and detect again
> +		 * flush pending descriptor writebacks to memory
> +		 */
> +		ew32(TIDV, adapter->tx_int_delay | E1000_TIDV_FPD);
> +		/* execute the writes immediately */
> +		e1e_flush();
> +		adapter->tx_hang_recheck = true;
> +		return;
> +	}
> +	/* Real hang detected */
> +	adapter->tx_hang_recheck = false;
> +	netif_stop_queue(netdev);
> +
>  	e1e_rphy(hw, PHY_STATUS, &phy_status);
>  	e1e_rphy(hw, PHY_1000T_STATUS, &phy_1000t_status);
>  	e1e_rphy(hw, PHY_EXT_STATUS, &phy_ext_status);
> @@ -1145,10 +1161,10 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter)
>  		if (tx_ring->buffer_info[i].time_stamp &&
>  		    time_after(jiffies, tx_ring->buffer_info[i].time_stamp
>  			       + (adapter->tx_timeout_factor * HZ)) &&
> -		    !(er32(STATUS) & E1000_STATUS_TXOFF)) {
> +		    !(er32(STATUS) & E1000_STATUS_TXOFF))
>  			schedule_work(&adapter->print_hang_task);
> -			netif_stop_queue(netdev);
> -		}
> +		else
> +			adapter->tx_hang_recheck = false;
>  	}
>  	adapter->total_tx_bytes += total_tx_bytes;
>  	adapter->total_tx_packets += total_tx_packets;
> @@ -3838,6 +3854,7 @@ static int e1000_open(struct net_device *netdev)
> 
>  	e1000_irq_enable(adapter);
> 
> +	adapter->tx_hang_recheck = false;
>  	netif_start_queue(netdev);
> 
>  	adapter->idle_check = true;

^ permalink raw reply

* Re: [PATCH -next 0/4] netfilter reverse path filter matches
From: Florian Westphal @ 2011-12-05  8:33 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel, netdev, laforge
In-Reply-To: <20111204215150.GA28125@1984>

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 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.

Thanks Pablo.

> One inquiry, I need to know copyright / license details for this
> extensions/libxt_rpfilter.c
> 
[..]
> No need to resend the patch, I can ammend the patch with the copyright
> / licensing header.
> 
> See extensions/libxt_cluster.c for reference.

In this case, please amend it to include the same GPL-2 boilerplate
as libxt_cluster.c has.

Thanks,
Florian

^ permalink raw reply

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

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

On Mon, 2011-12-05 at 00:18 -0800, Michael Wang wrote:
> On 12/05/2011 04:02 PM, Jeff Kirsher wrote:
> 
> > On Sun, 2011-12-04 at 23:15 -0800, Michael Wang wrote:
> >> On 12/05/2011 02:25 PM, Jeff Kirsher wrote:
> >>
> >>> 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.
> >>>
> >>
> >> Hi, Jeff
> >>
> >> Is that means you have a better patch which different from ours, and you
> >> will use your patch to instead of ours?
> >>
> >> Because David is just ask for some small change, I think your time zone
> >> may be better to work with him, so I ask for your help.
> >>
> >> I was just confused that why our signed-off-by should be removed?
> >>
> >> Thanks,
> >> Michael Wang
> >>
> >>> Cheers,
> >>> Jeff
> > 
> > It is your patch (your original work) but since I have made changes to
> > your patch, I (or anyone for that matter) should not assume that you as
> > the owner would signed off on the changes that I have made based on
> > feedback.  It would not be right for me to send out a patch with your
> > signed-off-by which is different from what you originally submitted,
> > without your ok.  Once I send out the v2 of the patch, please feel free
> > to add your signed-off-by OR acked-by to the patch.
> > 
> 
> Hi, Jeff
> 
> That make sense, I'm sorry but because I'm new to the community, and I
> just want to make every thing clear so I can do better in the future.

No problem, just trying to help get your work/fix upstream.
> 
> > While I personally do not have a problem keeping you as the owner and
> > your signed-off-by, I believe that takes in several assumptions which
> > only you as the owner should speak for.  I am not trying to take
> > ownership for stats purposes, I care less about the number of patches I
> > create and own and would rather make sure that the original owners get
> > the credit due for the work they did.
> > 
> 
> 
> I'm so sorry and I regret if I make you unhappy by some wrong word,
> please forgive me.

Not at all, you have not made me unhappy, so no need to apologize.  I am
just sorry if my email came across frustrated or unhappy.

> 
> > So with that, when I send out my next series of patches please feel free
> > to ACK or Sign-off on the changes made.  I just wanted to make sure that
> > we get these changes in soon (with out delay).
> > 
> > I can wait if you want to keep ownership of the patch, I just wanted to
> > ensure that we get your patch included as soon as possible based on the
> > problem it fixes.
> 
> Please help us to make the patch perfect, and I'm very glad if I can
> have the opportunity to work with you.

I am always here to help and welcome any submissions you want to provide
to make our Intel drivers better.  Thank you!

> 
> Thanks & Best regards
> Michael Wang
> 



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

^ permalink raw reply

* [net-next 5/6] ixgbe: DCB: IEEE transitions may fail to reprogram hardware.
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: John Fastabend <john.r.fastabend@intel.com>

Transitioning through an IEEE DCBX version from a CEE DCBX
and back (CEE->IEEE->CEE) may leave IEEE attributes programmed
in the hardware. DCB uses a bit field in the set routines to
determine which attributes PG, PFC, APP need to be reprogrammed.
This is needed because user flow allows queueing a series
of changes and then reprogramming the hardware with the
entire set in one operation.

When transitioning from IEEE DCBX mode back into CEE DCBX
mode the PG and PFC bits need to be set so the possibly

different CEE attributes get programmed into the device.

This patch fixes broken logic that was evaluating to 0
and never setting any bits. Further this removes some
checks for num_tc in set routines. This logic only worked
when the number of traffic classes and user priorities
were equal. This is no longer the case for X540 devices.
Besides we can trust user input in this case if the
device is incorrectly configured the DCB bandwidths will
be incorrectly mapped but no OOPs, BUG, or hardware
failure will occur.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
index 8c056c0..da31735 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
@@ -158,10 +158,6 @@ static void ixgbe_dcbnl_set_pg_tc_cfg_tx(struct net_device *netdev, int tc,
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
-	/* Abort a bad configuration */
-	if (ffs(up_map) > adapter->dcb_cfg.num_tcs.pg_tcs)
-		return;
-
 	if (prio != DCB_ATTR_VALUE_UNDEFINED)
 		adapter->temp_dcb_cfg.tc_config[tc].path[0].prio_type = prio;
 	if (bwg_id != DCB_ATTR_VALUE_UNDEFINED)
@@ -185,7 +181,7 @@ static void ixgbe_dcbnl_set_pg_tc_cfg_tx(struct net_device *netdev, int tc,
 
 	if (adapter->temp_dcb_cfg.tc_config[tc].path[0].up_to_tc_bitmap !=
 	     adapter->dcb_cfg.tc_config[tc].path[0].up_to_tc_bitmap)
-		adapter->dcb_set_bitmap |= BIT_PFC;
+		adapter->dcb_set_bitmap |= BIT_PFC | BIT_APP_UPCHG;
 }
 
 static void ixgbe_dcbnl_set_pg_bwg_cfg_tx(struct net_device *netdev, int bwg_id,
@@ -206,10 +202,6 @@ static void ixgbe_dcbnl_set_pg_tc_cfg_rx(struct net_device *netdev, int tc,
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
-	/* Abort bad configurations */
-	if (ffs(up_map) > adapter->dcb_cfg.num_tcs.pg_tcs)
-		return;
-
 	if (prio != DCB_ATTR_VALUE_UNDEFINED)
 		adapter->temp_dcb_cfg.tc_config[tc].path[1].prio_type = prio;
 	if (bwg_id != DCB_ATTR_VALUE_UNDEFINED)
@@ -434,7 +426,12 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 		adapter->hw.fc.current_mode = ixgbe_fc_pfc;
 
 #ifdef IXGBE_FCOE
-	if (up && !(up & (1 << adapter->fcoe.up))) {
+	/* Reprogam FCoE hardware offloads when the traffic class
+	 * FCoE is using changes. This happens if the APP info
+	 * changes or the up2tc mapping is updated.
+	 */
+	if ((up && !(up & (1 << adapter->fcoe.up))) ||
+	    (adapter->dcb_set_bitmap & BIT_APP_UPCHG)) {
 		adapter->fcoe.up = ffs(up) - 1;
 		ixgbe_dcbnl_devreset(netdev);
 		ret = DCB_HW_CHG_RST;
@@ -742,7 +739,9 @@ static u8 ixgbe_dcbnl_setdcbx(struct net_device *dev, u8 mode)
 		ixgbe_dcbnl_ieee_setets(dev, &ets);
 		ixgbe_dcbnl_ieee_setpfc(dev, &pfc);
 	} else if (mode & DCB_CAP_DCBX_VER_CEE) {
-		adapter->dcb_set_bitmap |= (BIT_PFC & BIT_PG_TX & BIT_PG_RX);
+		u8 mask = BIT_PFC | BIT_PG_TX | BIT_PG_RX | BIT_APP_UPCHG;
+
+		adapter->dcb_set_bitmap |= mask;
 		ixgbe_dcbnl_set_all(dev);
 	} else {
 		/* Drop into single TC mode strict priority as this
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 6/6] ixgbe: Remove function prototype for non-existent function
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Greg Rose <gregory.v.rose@intel.com>

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index df04f1a..e8badab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -33,7 +33,6 @@ void ixgbe_msg_task(struct ixgbe_adapter *adapter);
 int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
 void ixgbe_disable_tx_rx(struct ixgbe_adapter *adapter);
 void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter);
-void ixgbe_dump_registers(struct ixgbe_adapter *adapter);
 int ixgbe_ndo_set_vf_mac(struct net_device *netdev, int queue, u8 *mac);
 int ixgbe_ndo_set_vf_vlan(struct net_device *netdev, int queue, u16 vlan,
 			   u8 qos);
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 4/6] ixgbe: DCBnl set_all, order of operations fix
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: John Fastabend <john.r.fastabend@intel.com>

The order of operations is important in DCBnl set_all(). When FCoE
is configured it uses the up2tc map to learn which queues to configure
the hardware offloads on. Therefore we need to setup the map before
configuring FCoE.

This is only seen when the both up2tc mappings and APP info are
configured simultaneously.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c |   77 +++++++++--------------
 1 files changed, 29 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
index 33b93ff..8c056c0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
@@ -309,6 +309,27 @@ static void ixgbe_dcbnl_get_pfc_cfg(struct net_device *netdev, int priority,
 	*setting = adapter->dcb_cfg.tc_config[priority].dcb_pfc;
 }
 
+#ifdef IXGBE_FCOE
+static void ixgbe_dcbnl_devreset(struct net_device *dev)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+
+	while (test_and_set_bit(__IXGBE_RESETTING, &adapter->state))
+		usleep_range(1000, 2000);
+
+	if (netif_running(dev))
+		dev->netdev_ops->ndo_stop(dev);
+
+	ixgbe_clear_interrupt_scheme(adapter);
+	ixgbe_init_interrupt_scheme(adapter);
+
+	if (netif_running(dev))
+		dev->netdev_ops->ndo_open(dev);
+
+	clear_bit(__IXGBE_RESETTING, &adapter->state);
+}
+#endif
+
 static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
@@ -338,27 +359,6 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 	if (ret)
 		return DCB_NO_HW_CHG;
 
-#ifdef IXGBE_FCOE
-	if (up && !(up & (1 << adapter->fcoe.up)))
-		adapter->dcb_set_bitmap |= BIT_APP_UPCHG;
-
-	/*
-	 * Only take down the adapter if an app change occurred. FCoE
-	 * may shuffle tx rings in this case and this can not be done
-	 * without a reset currently.
-	 */
-	if (adapter->dcb_set_bitmap & BIT_APP_UPCHG) {
-		while (test_and_set_bit(__IXGBE_RESETTING, &adapter->state))
-			usleep_range(1000, 2000);
-
-		adapter->fcoe.up = ffs(up) - 1;
-
-		if (netif_running(netdev))
-			netdev->netdev_ops->ndo_stop(netdev);
-		ixgbe_clear_interrupt_scheme(adapter);
-	}
-#endif
-
 	if (adapter->dcb_cfg.pfc_mode_enable) {
 		switch (adapter->hw.mac.type) {
 		case ixgbe_mac_82599EB:
@@ -385,15 +385,6 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 		}
 	}
 
-#ifdef IXGBE_FCOE
-	if (adapter->dcb_set_bitmap & BIT_APP_UPCHG) {
-		ixgbe_init_interrupt_scheme(adapter);
-		if (netif_running(netdev))
-			netdev->netdev_ops->ndo_open(netdev);
-		ret = DCB_HW_CHG_RST;
-	}
-#endif
-
 	if (adapter->dcb_set_bitmap & (BIT_PG_TX|BIT_PG_RX)) {
 		u16 refill[MAX_TRAFFIC_CLASS], max[MAX_TRAFFIC_CLASS];
 		u8 bwg_id[MAX_TRAFFIC_CLASS], prio_type[MAX_TRAFFIC_CLASS];
@@ -442,8 +433,14 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 	if (adapter->dcb_cfg.pfc_mode_enable)
 		adapter->hw.fc.current_mode = ixgbe_fc_pfc;
 
-	if (adapter->dcb_set_bitmap & BIT_APP_UPCHG)
-		clear_bit(__IXGBE_RESETTING, &adapter->state);
+#ifdef IXGBE_FCOE
+	if (up && !(up & (1 << adapter->fcoe.up))) {
+		adapter->fcoe.up = ffs(up) - 1;
+		ixgbe_dcbnl_devreset(netdev);
+		ret = DCB_HW_CHG_RST;
+	}
+#endif
+
 	adapter->dcb_set_bitmap = 0x00;
 	return ret;
 }
@@ -661,22 +658,6 @@ static int ixgbe_dcbnl_ieee_setpfc(struct net_device *dev,
 	return ixgbe_dcb_hw_pfc_config(&adapter->hw, pfc->pfc_en, prio_tc);
 }
 
-#ifdef IXGBE_FCOE
-static void ixgbe_dcbnl_devreset(struct net_device *dev)
-{
-	struct ixgbe_adapter *adapter = netdev_priv(dev);
-
-	if (netif_running(dev))
-		dev->netdev_ops->ndo_stop(dev);
-
-	ixgbe_clear_interrupt_scheme(adapter);
-	ixgbe_init_interrupt_scheme(adapter);
-
-	if (netif_running(dev))
-		dev->netdev_ops->ndo_open(dev);
-}
-#endif
-
 static int ixgbe_dcbnl_ieee_setapp(struct net_device *dev,
 				   struct dcb_app *app)
 {
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 3/6] igb: Update DMA Coalescing threshold calculation.
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: Matthew Vick, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Matthew Vick <matthew.vick@intel.com>

This patch updates the DMA Coalescing feature parameters to account for
larger MTUs. Previously, sufficient space may not have been allocated in
the receive buffer, causing packet drop.

Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |   26 +++++++++++++++++++-------
 1 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b66b8aa..143cfeb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7061,15 +7061,28 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba)
 			wr32(E1000_DMCTXTH, 0);
 
 			/*
-			 * DMA Coalescing high water mark needs to be higher
-			 * than the RX threshold. set hwm to PBA -  2 * max
-			 * frame size
+			 * DMA Coalescing high water mark needs to be greater
+			 * than the Rx threshold. Set hwm to PBA - max frame
+			 * size in 16B units, capping it at PBA - 6KB.
 			 */
-			hwm = pba - (2 * adapter->max_frame_size);
+			hwm = 64 * pba - adapter->max_frame_size / 16;
+			if (hwm < 64 * (pba - 6))
+				hwm = 64 * (pba - 6);
+			reg = rd32(E1000_FCRTC);
+			reg &= ~E1000_FCRTC_RTH_COAL_MASK;
+			reg |= ((hwm << E1000_FCRTC_RTH_COAL_SHIFT)
+				& E1000_FCRTC_RTH_COAL_MASK);
+			wr32(E1000_FCRTC, reg);
+
+			/*
+			 * Set the DMA Coalescing Rx threshold to PBA - 2 * max
+			 * frame size, capping it at PBA - 10KB.
+			 */
+			dmac_thr = pba - adapter->max_frame_size / 512;
+			if (dmac_thr < pba - 10)
+				dmac_thr = pba - 10;
 			reg = rd32(E1000_DMACR);
 			reg &= ~E1000_DMACR_DMACTHR_MASK;
-			dmac_thr = pba - 4;
-
 			reg |= ((dmac_thr << E1000_DMACR_DMACTHR_SHIFT)
 				& E1000_DMACR_DMACTHR_MASK);
 
@@ -7085,7 +7098,6 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba)
 			 * coalescing(smart fifb)-UTRESH=0
 			 */
 			wr32(E1000_DMCRTRH, 0);
-			wr32(E1000_FCRTC, hwm);
 
 			reg = (IGB_DMCTLX_DCFLUSH_DIS | 0x4);
 
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 2/6] e1000e: hitting BUG_ON() from napi_enable
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Mike McElroy, Jeff Kirsher
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Based on a patch from Mike McElroy created against the out-of-tree e1000e
driver:

Hitting the BUG_ON in napi_enable(). Code inspection shows that this can
only be triggered by calling napi_enable() twice without an intervening
napi_disable().

I saw the following sequence of events in the stack trace:

1) We simulated a cable pull using an Extreme switch.
2) e1000_tx_timeout() was entered.
3) e1000_reset_task() was called. Saw the message from e_err() in the
console log.
4) e1000_reinit_locked was called. This function calls e1000_down() and
e1000_up(). These functions call napi_disable() and napi_enable()
respectively.
5) Then on another thread, a monitor task saw carrier was down and executed
'ip set link down' and 'ip set link up' commands.
6) Saw the '_E1000_RESETTING'warning fron the e1000_close function.
7) Either the e1000_open() executed between the e1000_down() and e1000_up()
calls in step 4 or the e1000_open() call executed after the e1000_up()
call.  In either case, napi_enable() is called twice which triggers the
BUG_ON.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Cc: Mike McElroy <mike.mcelroy@stratus.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c12df69..93ae0c2 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3516,7 +3516,6 @@ int e1000e_up(struct e1000_adapter *adapter)
 
 	clear_bit(__E1000_DOWN, &adapter->state);
 
-	napi_enable(&adapter->napi);
 	if (adapter->msix_entries)
 		e1000_configure_msix(adapter);
 	e1000_irq_enable(adapter);
@@ -3578,7 +3577,6 @@ void e1000e_down(struct e1000_adapter *adapter)
 	e1e_flush();
 	usleep_range(10000, 20000);
 
-	napi_disable(&adapter->napi);
 	e1000_irq_disable(adapter);
 
 	del_timer_sync(&adapter->watchdog_timer);
@@ -3901,6 +3899,8 @@ static int e1000_close(struct net_device *netdev)
 
 	pm_runtime_get_sync(&pdev->dev);
 
+	napi_disable(&adapter->napi);
+
 	if (!test_bit(__E1000_DOWN, &adapter->state)) {
 		e1000e_down(adapter);
 		e1000_free_irq(adapter);
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann, Michael Wang,
	Flavio Leitner
In-Reply-To: <1323073241-21144-1-git-send-email-jeffrey.t.kirsher@intel.com>

Based on the original patch submitted my Michael Wang
<wangyun@linux.vnet.ibm.com>.
Descriptors may not be write-back while checking TX hang with flag
FLAG2_DMA_BURST on.
So when we detect hang, we just flush the descriptor and detect
again for once.

-v2 change 1 to true and 0 to false and remove extra ()

CC: Michael Wang <wangyun@linux.vnet.ibm.com>
CC: Flavio Leitner <fbl@redhat.com>
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h  |    1 +
 drivers/net/ethernet/intel/e1000e/netdev.c |   23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 9fe18d1..f478a22 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -309,6 +309,7 @@ struct e1000_adapter {
 	u32 txd_cmd;
 
 	bool detect_tx_hung;
+	bool tx_hang_recheck;
 	u8 tx_timeout_factor;
 
 	u32 tx_int_delay;
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c6e9763..c12df69 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1014,6 +1014,7 @@ static void e1000_print_hw_hang(struct work_struct *work)
 	struct e1000_adapter *adapter = container_of(work,
 	                                             struct e1000_adapter,
 	                                             print_hang_task);
+	struct net_device *netdev = adapter->netdev;
 	struct e1000_ring *tx_ring = adapter->tx_ring;
 	unsigned int i = tx_ring->next_to_clean;
 	unsigned int eop = tx_ring->buffer_info[i].next_to_watch;
@@ -1025,6 +1026,21 @@ static void e1000_print_hw_hang(struct work_struct *work)
 	if (test_bit(__E1000_DOWN, &adapter->state))
 		return;
 
+	if (!adapter->tx_hang_recheck &&
+	    (adapter->flags2 & FLAG2_DMA_BURST)) {
+		/* May be block on write-back, flush and detect again
+		 * flush pending descriptor writebacks to memory
+		 */
+		ew32(TIDV, adapter->tx_int_delay | E1000_TIDV_FPD);
+		/* execute the writes immediately */
+		e1e_flush();
+		adapter->tx_hang_recheck = true;
+		return;
+	}
+	/* Real hang detected */
+	adapter->tx_hang_recheck = false;
+	netif_stop_queue(netdev);
+
 	e1e_rphy(hw, PHY_STATUS, &phy_status);
 	e1e_rphy(hw, PHY_1000T_STATUS, &phy_1000t_status);
 	e1e_rphy(hw, PHY_EXT_STATUS, &phy_ext_status);
@@ -1145,10 +1161,10 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter)
 		if (tx_ring->buffer_info[i].time_stamp &&
 		    time_after(jiffies, tx_ring->buffer_info[i].time_stamp
 			       + (adapter->tx_timeout_factor * HZ)) &&
-		    !(er32(STATUS) & E1000_STATUS_TXOFF)) {
+		    !(er32(STATUS) & E1000_STATUS_TXOFF))
 			schedule_work(&adapter->print_hang_task);
-			netif_stop_queue(netdev);
-		}
+		else
+			adapter->tx_hang_recheck = false;
 	}
 	adapter->total_tx_bytes += total_tx_bytes;
 	adapter->total_tx_packets += total_tx_packets;
@@ -3838,6 +3854,7 @@ static int e1000_open(struct net_device *netdev)
 
 	e1000_irq_enable(adapter);
 
+	adapter->tx_hang_recheck = false;
 	netif_start_queue(netdev);
 
 	adapter->idle_check = true;
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 0/6 v2][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2011-12-05  8:20 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

The following series contains updates to e1000e, igb and ixgbe.  5 of
the patches are bug fixes and one patch is to cleanup a function
prototype of a non-existent function.

-v2 fix up patch 1 based on David Miller's suggestion
    fix up patch 5 so that the tested-by is in the correct place

The following are changes since commit 340e8dc1fb4032b6c8334c9bff20b2aec42ecfd8:
  atm: clip: Remove code commented out since eternity.
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Bruce Allan (1):
  e1000e: hitting BUG_ON() from napi_enable

Greg Rose (1):
  ixgbe: Remove function prototype for non-existent function

Jeff Kirsher (1):
  e1000e: Avoid wrong check on TX hang

John Fastabend (2):
  ixgbe: DCBnl set_all, order of operations fix
  ixgbe: DCB: IEEE transitions may fail to reprogram hardware.

Matthew Vick (1):
  igb: Update DMA Coalescing threshold calculation.

 drivers/net/ethernet/intel/e1000e/e1000.h       |    1 +
 drivers/net/ethernet/intel/e1000e/netdev.c      |   27 +++++-
 drivers/net/ethernet/intel/igb/igb_main.c       |   26 +++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c |   96 +++++++++--------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h  |    1 -
 5 files changed, 80 insertions(+), 71 deletions(-)

-- 
1.7.6.4

^ permalink raw reply

* Re: [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Michael Wang @ 2011-12-05  8:18 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: <1323072138.24828.32.camel@jtkirshe-mobl>

On 12/05/2011 04:02 PM, Jeff Kirsher wrote:

> On Sun, 2011-12-04 at 23:15 -0800, Michael Wang wrote:
>> On 12/05/2011 02:25 PM, Jeff Kirsher wrote:
>>
>>> 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.
>>>
>>
>> Hi, Jeff
>>
>> Is that means you have a better patch which different from ours, and you
>> will use your patch to instead of ours?
>>
>> Because David is just ask for some small change, I think your time zone
>> may be better to work with him, so I ask for your help.
>>
>> I was just confused that why our signed-off-by should be removed?
>>
>> Thanks,
>> Michael Wang
>>
>>> Cheers,
>>> Jeff
> 
> It is your patch (your original work) but since I have made changes to
> your patch, I (or anyone for that matter) should not assume that you as
> the owner would signed off on the changes that I have made based on
> feedback.  It would not be right for me to send out a patch with your
> signed-off-by which is different from what you originally submitted,
> without your ok.  Once I send out the v2 of the patch, please feel free
> to add your signed-off-by OR acked-by to the patch.
> 

Hi, Jeff

That make sense, I'm sorry but because I'm new to the community, and I
just want to make every thing clear so I can do better in the future.

> While I personally do not have a problem keeping you as the owner and
> your signed-off-by, I believe that takes in several assumptions which
> only you as the owner should speak for.  I am not trying to take
> ownership for stats purposes, I care less about the number of patches I
> create and own and would rather make sure that the original owners get
> the credit due for the work they did.
> 


I'm so sorry and I regret if I make you unhappy by some wrong word,
please forgive me.

> So with that, when I send out my next series of patches please feel free
> to ACK or Sign-off on the changes made.  I just wanted to make sure that
> we get these changes in soon (with out delay).
> 
> I can wait if you want to keep ownership of the patch, I just wanted to
> ensure that we get your patch included as soon as possible based on the
> problem it fixes.

Please help us to make the patch perfect, and I'm very glad if I can
have the opportunity to work with you.

Thanks & Best regards
Michael Wang

^ permalink raw reply

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

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

On Sun, 2011-12-04 at 23:15 -0800, Michael Wang wrote:
> On 12/05/2011 02:25 PM, Jeff Kirsher wrote:
> 
> > 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.
> > 
> 
> Hi, Jeff
> 
> Is that means you have a better patch which different from ours, and you
> will use your patch to instead of ours?
> 
> Because David is just ask for some small change, I think your time zone
> may be better to work with him, so I ask for your help.
> 
> I was just confused that why our signed-off-by should be removed?
> 
> Thanks,
> Michael Wang
> 
> > Cheers,
> > Jeff

It is your patch (your original work) but since I have made changes to
your patch, I (or anyone for that matter) should not assume that you as
the owner would signed off on the changes that I have made based on
feedback.  It would not be right for me to send out a patch with your
signed-off-by which is different from what you originally submitted,
without your ok.  Once I send out the v2 of the patch, please feel free
to add your signed-off-by OR acked-by to the patch.

While I personally do not have a problem keeping you as the owner and
your signed-off-by, I believe that takes in several assumptions which
only you as the owner should speak for.  I am not trying to take
ownership for stats purposes, I care less about the number of patches I
create and own and would rather make sure that the original owners get
the credit due for the work they did.

So with that, when I send out my next series of patches please feel free
to ACK or Sign-off on the changes made.  I just wanted to make sure that
we get these changes in soon (with out delay).

I can wait if you want to keep ownership of the patch, I just wanted to
ensure that we get your patch included as soon as possible based on the
problem it fixes.

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

^ permalink raw reply

* v6: faster tree-based sysctl implementation
From: Lucian Adrian Grijincu @ 2011-12-05  7:55 UTC (permalink / raw)
  To: Eric W . Biederman, linux-kernel
  Cc: netdev, Octavian Purdila, David S . Miller, Alexey Dobriyan,
	Damien Millescamps

This is version 6 of a patch series that introduces a faster/leaner
sysctl internal implementation.
git://github.com/luciang/linux-2.6-new-sysctl.git  v6-new-sysctl-alg
A subset of patches that add the minimal set of patches with the new
algorithm is here:
 git://github.com/luciang/linux-2.6-new-sysctl.git  v6-new-sysctl-alg-slim


= Stats =

$ time modprobe dummy numdummies=N

Without this patch series :(
- ipv4 only
  -  N=1000  time= 0m 07s
  -  N=2000  time= 0m 38s
  -  N=4000  time= 4m 01s
- ipv4 and ipv6
  -  N=1000  time= 0m 35s
  -  N=2000  time= 3m 09s
  -  N=4000  time=14m 27s

With this patch series :)
- ipv4 only
  -  N=1000  time= 0m  0.31s
  -  N=2000  time= 0m  1.23s
  -  N=4000  time= 0m  5.14s
  -  N=8000  time= 0m 20.20s
- ipv4 and ipv6
  -  N=1000  time= 0m  0.70s
  -  N=2000  time= 0m  1.93s
  -  N=4000  time= 0m  7.03s
  -  N=8000  time= 0m 40.89s



= Changes =
since v5:
 - https://lkml.org/lkml/2011/6/2/332 - nothing much because of lack
of review. Echo, echo, echo ... :)
 - minor tweaks
 - rebased on 3.2-rc4


since v4: - http://thread.gmane.org/gmane.linux.network/196495/focus=1144143
- rebased on 3.0-rc1 - added a new patch manually register
kernel/usermodehelper which   was added in 3.0-rc1 - minor changes to
the "sysctl: simplify find_in_table" patch

since v3: - http://thread.gmane.org/gmane.linux.network/196495/ -
removed a bad patch that shrinked a counter from int to u8

since v2: - http://thread.gmane.org/gmane.linux.kernel/1137032/focus=3D194748
- added a compatibility layer to support old registering complex
sysctl trees. This layer will be deleted once all users of the   old
are changed. - subdirectories and netns correspondent dirs are now
held in rbtrees - split of from the patches that make changes in the
rest of the tree - rebased on top of 2.6.39

since v1: - http://thread.gmane.org/gmane.linux.kernel/1133667 -
rebased on top of 2.6.39-rc6 - split the patch that adds the new
algorithm and data structures. - fixed a few bugs lingering in the old
code - shrinked a reference counter - added a new reference counter to
maintain ownership information - added method to register an empty
sysctl dir and converted some users - added checks enforcing the rule
that a non-netns specific directory may   not be registered after a
netns specific one has already been registered. - added cookie
support: register a piece of data with the header to be   used to make
simple conversions on the ctl_table.


= Summary =

Part 1: introduce compatibility layer:
  sysctl: introduce temporary sysctl wrappers
  sysctl: register only tables of sysctl files

Part 2: minimal changes to sysctl users:
  sysctl: call sysctl_init before the first sysctl registration
  sysctl: no-child: manually register kernel/random
  sysctl: no-child: manually register kernel/usermodehelper
  sysctl: no-child: manually register kernel/keys
  sysctl: no-child: manually register fs/inotify
  sysctl: no-child: manually register fs/epoll
  sysctl: no-child: manually register root tables

Part 3: cleanups simplifying the new algorithm:
  sysctl: faster reimplementation of sysctl_check_table
  sysctl: remove useless ctl_table->parent field
  sysctl: simplify find_in_table
  sysctl: sysctl_head_grab defaults to root header on NULL
  sysctl: delete useless grab_header function
  sysctl: rename ->used to ->ctl_use_refs
  sysctl: rename sysctl_head_grab/finish to sysctl_use_header/unuse
  sysctl: rename sysctl_head_next to sysctl_use_next_header
  sysctl: split ->count into ctl_procfs_refs and ctl_header_refs
  sysctl: rename sysctl_head_get/put to sysctl_proc_inode_get/put
  sysctl: rename (un)use_table to __sysctl_(un)use_header
  sysctl: simplify ->permissions hook
  sysctl: move removal from list out of start_unregistering
  sysctl: introduce ctl_table_group and ctl_table_group_ops



Part 4: new algorithm/data structures:
  sysctl: faster tree-based sysctl implementation



Part 5: checks/warns requested during review:
  sysctl: add duplicate entry and sanity ctl_table checks
  sysctl: alloc ctl_table_header with kmem_cache
  sysctl: check netns-specific registration order respected
  sysctl: warn if registration/unregistration order is not respected
  sysctl: always perform sysctl checks
  sysctl: reorder members of ctl_table_header (cleanup)
  sysctl: add ctl_type member

Part 6: further performance optimisations:
  sysctl: replace subdirectory list with rbtree
  sysctl: replace netns corresp list with rbtree
  sysctl: union-ize some ctl_table_header fields

Part 7: Eric requested ability to register an empty dir:
  sysctl: add register_sysctl_dir: register an empty sysctl directory

Part 8: unrequested feature I'd like to piggy back :)
  sysctl: add ctl_cookie and ctl_cookie_handler
  sysctl: add cookie to __register_sysctl_paths
  sysctl: add register_net_sysctl_table_net_cookie

 drivers/char/random.c            |   27 +-
 fs/eventpoll.c                   |   22 +-
 fs/notify/inotify/inotify_user.c |   22 +-
 fs/proc/inode.c                  |    2 +-
 fs/proc/proc_sysctl.c            |  236 +++++---
 include/linux/inotify.h          |    2 -
 include/linux/key.h              |    3 -
 include/linux/kmod.h             |    3 -
 include/linux/poll.h             |    2 -
 include/linux/sysctl.h           |  221 +++++---
 include/net/net_namespace.h      |    4 +-
 init/main.c                      |    1 +
 kernel/Makefile                  |    5 +-
 kernel/kmod.c                    |   14 +-
 kernel/sysctl.c                  | 1170 ++++++++++++++++++++++++++++----------
 kernel/sysctl_check.c            |  316 +++++++----
 lib/Kconfig.debug                |    8 -
 net/sysctl_net.c                 |   86 ++--
 security/keys/key.c              |    7 +
 security/keys/sysctl.c           |   18 +-
 20 files changed, 1500 insertions(+), 669 deletions(-)

-- 
1.7.5.4
-- 
 .
..: Lucian

^ permalink raw reply

* Re: [PATCH net-next V0 19/21] mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
From: Or Gerlitz @ 2011-12-05  7:40 UTC (permalink / raw)
  To: Jack Morgenstein
  Cc: Roland Dreier, Yevgeny Petrilin, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	liranl-VPRAkNaXOzVS1MOuV/RT9w
In-Reply-To: <201112050919.39280.jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On 12/5/2011 9:19 AM, Jack Morgenstein wrote:
> If CONFIG_PCI_IOV isn't set, pci_enable_sriov isn't there...

> Not so.  I checked. If CONFIG_PCI_IOV isn't set, pci_enable_sriov returns -ENODEV via a static inline function. Look in kernel file include/linux/pci.h (for the most recent kernel)

Sure, sorry, my bad, thanks for clarifying this out.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 net-next V0 19/21] mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
From: Jack Morgenstein @ 2011-12-05  7:19 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Roland Dreier, Yevgeny Petrilin, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	liranl-VPRAkNaXOzVS1MOuV/RT9w
In-Reply-To: <4EDB942F.1020101-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

On Sunday 04 December 2011 17:39, Or Gerlitz wrote:
> On 12/4/2011 4:29 PM, Jack Morgenstein wrote:
> > If the kernel is not configured to support IOV, pci_enable_sriov will 
> > fail.
> Jack,
> 
> If CONFIG_PCI_IOV isn't set, pci_enable_sriov isn't there...
> 
> Or.
> 
Not so.  I checked. If CONFIG_PCI_IOV isn't set, pci_enable_sriov returns -ENODEV
via a static inline function.
Look in kernel file include/linux/pci.h (for the most recent kernel):
 
#ifdef CONFIG_PCI_IOV
extern int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
extern void pci_disable_sriov(struct pci_dev *dev);
extern irqreturn_t pci_sriov_migration(struct pci_dev *dev);
extern int pci_num_vf(struct pci_dev *dev);
#else
static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
{
        return -ENODEV;
}
static inline void pci_disable_sriov(struct pci_dev *dev)
{
}
static inline irqreturn_t pci_sriov_migration(struct pci_dev *dev)
{
        return IRQ_NONE;
}
static inline int pci_num_vf(struct pci_dev *dev)
{
        return 0;
}
#endif

- Jack
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: [net-next 1/6] e1000e: Avoid wrong check on TX hang
From: Michael Wang @ 2011-12-05  7:15 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: <1323066318.24828.14.camel@jtkirshe-mobl>

On 12/05/2011 02:25 PM, Jeff Kirsher wrote:

> 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.
> 

Hi, Jeff

Is that means you have a better patch which different from ours, and you
will use your patch to instead of ours?

Because David is just ask for some small change, I think your time zone
may be better to work with him, so I ask for your help.

I was just confused that why our signed-off-by should be removed?

Thanks,
Michael Wang

> Cheers,
> Jeff

^ permalink raw reply

* Re: r8169 performance?
From: Francois Romieu @ 2011-12-05  6:44 UTC (permalink / raw)
  To: Chris Adams, netdev, hayeswang
In-Reply-To: <20111205004650.GA5948@hiwaay.net>

Chris Adams <cmadams@hiwaay.net> :
[...]
> 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

Almost the same: RTL_GIGA_MAC_VER_25 versus RTL_GIGA_MAC_VER_26 (faster).

The only difference in the driver is the hw_phy_config part.

-- 
Ueimor

^ permalink raw reply

* [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