Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next V2 1/2] cls_flower: Fix missing free of rhashtable
From: Paul Blakey @ 2018-06-05  7:44 UTC (permalink / raw)
  To: David Miller
  Cc: paulb, jiri, xiyou.wangcong, jhs, netdev, kliteyn, roid, shahark,
	markb, ogerlitz
In-Reply-To: <20180604.170409.1557542563226321470.davem@davemloft.net>



On 05/06/2018 00:04, David Miller wrote:
> From: Paul Blakey <paulb@mellanox.com>
> Date: Sun,  3 Jun 2018 10:06:13 +0300
> 
>> When destroying the instance, destroy the head rhashtable.
>>
>> Fixes: 05cd271fd61a ("cls_flower: Support multiple masks per priority")
>> Reported-by: Vlad Buslov <vladbu@mellanox.com>
>> Reviewed-by: Roi Dayan <roid@mellanox.com>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> 
> Applied.
> 

thanks.

^ permalink raw reply

* [RFC v6 5/5] virtio_ring: enable packed ring
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev
  Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180605074046.20709-1-tiwei.bie@intel.com>

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/s390/virtio/virtio_ccw.c | 8 ++++++++
 drivers/virtio/virtio_ring.c     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 8f5c1d7f751a..ff5b85736d8d 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -765,6 +765,11 @@ static u64 virtio_ccw_get_features(struct virtio_device *vdev)
 	return rc;
 }
 
+static void ccw_transport_features(struct virtio_device *vdev)
+{
+	__virtio_clear_bit(vdev, VIRTIO_F_RING_PACKED);
+}
+
 static int virtio_ccw_finalize_features(struct virtio_device *vdev)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
@@ -791,6 +796,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev)
 	/* Give virtio_ring a chance to accept features. */
 	vring_transport_features(vdev);
 
+	/* Give virtio_ccw a chance to accept features. */
+	ccw_transport_features(vdev);
+
 	features->index = 0;
 	features->features = cpu_to_le32((u32)vdev->features);
 	/* Write the first half of the feature bits to the host. */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index aefd7ac40928..fe849fd8733b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1965,6 +1965,8 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_F_IOMMU_PLATFORM:
 			break;
+		case VIRTIO_F_RING_PACKED:
+			break;
 		default:
 			/* We don't understand this bit. */
 			__virtio_clear_bit(vdev, i);
-- 
2.17.0

^ permalink raw reply related

* [RFC v6 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev
  Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180605074046.20709-1-tiwei.bie@intel.com>

This commit introduces the EVENT_IDX support in packed ring.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/virtio/virtio_ring.c | 74 ++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 983ce5ffda1b..aefd7ac40928 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1048,7 +1048,7 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
 static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	u16 flags;
+	u16 new, old, off_wrap, flags, wrap_counter, event_idx;
 	bool needs_kick;
 	u32 snapshot;
 
@@ -1057,9 +1057,19 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
 	 * suppressions. */
 	virtio_mb(vq->weak_barriers);
 
+	old = vq->next_avail_idx - vq->num_added;
+	new = vq->next_avail_idx;
+	vq->num_added = 0;
+
 	snapshot = *(u32 *)vq->vring_packed.device;
+	off_wrap = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot & 0xffff));
 	flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3;
 
+	wrap_counter = off_wrap >> 15;
+	event_idx = off_wrap & ~(1<<15);
+	if (wrap_counter != vq->avail_wrap_counter)
+		event_idx -= vq->vring_packed.num;
+
 #ifdef DEBUG
 	if (vq->last_add_time_valid) {
 		WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
@@ -1068,7 +1078,10 @@ static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
 	vq->last_add_time_valid = false;
 #endif
 
-	needs_kick = (flags != VRING_EVENT_F_DISABLE);
+	if (flags == VRING_EVENT_F_DESC)
+		needs_kick = vring_need_event(event_idx, new, old);
+	else
+		needs_kick = (flags != VRING_EVENT_F_DISABLE);
 	END_USE(vq);
 	return needs_kick;
 }
@@ -1177,6 +1190,15 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
 	ret = vq->desc_state_packed[id].data;
 	detach_buf_packed(vq, id, ctx);
 
+	/* If we expect an interrupt for the next entry, tell host
+	 * by writing event index and flush out the write before
+	 * the read in the next get_buf call. */
+	if (vq->event_flags_shadow == VRING_EVENT_F_DESC)
+		virtio_store_mb(vq->weak_barriers,
+				&vq->vring_packed.driver->off_wrap,
+				cpu_to_virtio16(_vq->vdev, vq->last_used_idx |
+					((u16)vq->used_wrap_counter << 15)));
+
 #ifdef DEBUG
 	vq->last_add_time_valid = false;
 #endif
@@ -1204,9 +1226,20 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
 
 	/* We optimistically turn back on interrupts, then check if there was
 	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always update the event index to keep code simple. */
+
+	vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
+			vq->last_used_idx |
+			((u16)vq->used_wrap_counter << 15));
 
 	if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
-		vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+		/* We need to update event offset and event wrap
+		 * counter first before updating event flags. */
+		virtio_wmb(vq->weak_barriers);
+		vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
+						     VRING_EVENT_F_ENABLE;
 		vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
 							vq->event_flags_shadow);
 	}
@@ -1232,21 +1265,48 @@ static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
 static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 bufs, used_idx, wrap_counter;
 
 	START_USE(vq);
 
 	/* We optimistically turn back on interrupts, then check if there was
 	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always update the event index to keep code simple. */
+
+	/* TODO: tune this threshold */
+	if (vq->next_avail_idx < vq->last_used_idx)
+		bufs = (vq->vring_packed.num + vq->next_avail_idx -
+				vq->last_used_idx) * 3 / 4;
+	else
+		bufs = (vq->next_avail_idx - vq->last_used_idx) * 3 / 4;
+
+	wrap_counter = vq->used_wrap_counter;
+
+	used_idx = vq->last_used_idx + bufs;
+	if (used_idx >= vq->vring_packed.num) {
+		used_idx -= vq->vring_packed.num;
+		wrap_counter ^= 1;
+	}
+
+	vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev,
+			used_idx | (wrap_counter << 15));
 
 	if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
-		vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+		/* We need to update event offset and event wrap
+		 * counter first before updating event flags. */
+		virtio_wmb(vq->weak_barriers);
+		vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC :
+						     VRING_EVENT_F_ENABLE;
 		vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
 							vq->event_flags_shadow);
-		/* We need to enable interrupts first before re-checking
-		 * for more used buffers. */
-		virtio_mb(vq->weak_barriers);
 	}
 
+	/* We need to update event suppression structure first
+	 * before re-checking for more used buffers. */
+	virtio_mb(vq->weak_barriers);
+
 	if (more_used_packed(vq)) {
 		END_USE(vq);
 		return false;
-- 
2.17.0

^ permalink raw reply related

* [RFC v6 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev
  Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180605074046.20709-1-tiwei.bie@intel.com>

This commit introduces the support (without EVENT_IDX) for
packed ring.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/virtio/virtio_ring.c | 486 ++++++++++++++++++++++++++++++++++-
 1 file changed, 479 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 416c33143125..983ce5ffda1b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -62,6 +62,12 @@ struct vring_desc_state {
 };
 
 struct vring_desc_state_packed {
+	void *data;			/* Data for callback. */
+	struct vring_packed_desc *indir_desc; /* Indirect descriptor, if any. */
+	int num;			/* Descriptor list length. */
+	dma_addr_t addr;		/* Buffer DMA addr. */
+	u32 len;			/* Buffer length. */
+	u16 flags;			/* Descriptor flags. */
 	int next;			/* The next desc state. */
 };
 
@@ -661,7 +667,6 @@ static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned last_used_idx)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	virtio_mb(vq->weak_barriers);
 	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
 }
 
@@ -758,6 +763,72 @@ static inline unsigned vring_size_packed(unsigned int num, unsigned long align)
 		& ~(align - 1)) + sizeof(struct vring_packed_desc_event) * 2;
 }
 
+static void vring_unmap_state_packed(const struct vring_virtqueue *vq,
+				     struct vring_desc_state_packed *state)
+{
+	u16 flags;
+
+	if (!vring_use_dma_api(vq->vq.vdev))
+		return;
+
+	flags = state->flags;
+
+	if (flags & VRING_DESC_F_INDIRECT) {
+		dma_unmap_single(vring_dma_dev(vq),
+				 state->addr, state->len,
+				 (flags & VRING_DESC_F_WRITE) ?
+				 DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	} else {
+		dma_unmap_page(vring_dma_dev(vq),
+			       state->addr, state->len,
+			       (flags & VRING_DESC_F_WRITE) ?
+			       DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	}
+}
+
+static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
+				   struct vring_packed_desc *desc)
+{
+	u16 flags;
+
+	if (!vring_use_dma_api(vq->vq.vdev))
+		return;
+
+	flags = virtio16_to_cpu(vq->vq.vdev, desc->flags);
+
+	if (flags & VRING_DESC_F_INDIRECT) {
+		dma_unmap_single(vring_dma_dev(vq),
+				 virtio64_to_cpu(vq->vq.vdev, desc->addr),
+				 virtio32_to_cpu(vq->vq.vdev, desc->len),
+				 (flags & VRING_DESC_F_WRITE) ?
+				 DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	} else {
+		dma_unmap_page(vring_dma_dev(vq),
+			       virtio64_to_cpu(vq->vq.vdev, desc->addr),
+			       virtio32_to_cpu(vq->vq.vdev, desc->len),
+			       (flags & VRING_DESC_F_WRITE) ?
+			       DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	}
+}
+
+static struct vring_packed_desc *alloc_indirect_packed(struct virtqueue *_vq,
+						       unsigned int total_sg,
+						       gfp_t gfp)
+{
+	struct vring_packed_desc *desc;
+
+	/*
+	 * We require lowmem mappings for the descriptors because
+	 * otherwise virt_to_phys will give us bogus addresses in the
+	 * virtqueue.
+	 */
+	gfp &= ~__GFP_HIGHMEM;
+
+	desc = kmalloc(total_sg * sizeof(struct vring_packed_desc), gfp);
+
+	return desc;
+}
+
 static inline int virtqueue_add_packed(struct virtqueue *_vq,
 				       struct scatterlist *sgs[],
 				       unsigned int total_sg,
@@ -767,47 +838,445 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
 				       void *ctx,
 				       gfp_t gfp)
 {
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	struct vring_packed_desc *desc;
+	struct scatterlist *sg;
+	unsigned int i, n, descs_used, uninitialized_var(prev), err_idx;
+	__virtio16 uninitialized_var(head_flags), flags;
+	u16 head, avail_wrap_counter, id, curr;
+	bool indirect;
+
+	START_USE(vq);
+
+	BUG_ON(data == NULL);
+	BUG_ON(ctx && vq->indirect);
+
+	if (unlikely(vq->broken)) {
+		END_USE(vq);
+		return -EIO;
+	}
+
+#ifdef DEBUG
+	{
+		ktime_t now = ktime_get();
+
+		/* No kick or get, with .1 second between?  Warn. */
+		if (vq->last_add_time_valid)
+			WARN_ON(ktime_to_ms(ktime_sub(now, vq->last_add_time))
+					    > 100);
+		vq->last_add_time = now;
+		vq->last_add_time_valid = true;
+	}
+#endif
+
+	BUG_ON(total_sg == 0);
+
+	head = vq->next_avail_idx;
+	avail_wrap_counter = vq->avail_wrap_counter;
+
+	if (virtqueue_use_indirect(_vq, total_sg))
+		desc = alloc_indirect_packed(_vq, total_sg, gfp);
+	else {
+		desc = NULL;
+		WARN_ON_ONCE(total_sg > vq->vring_packed.num && !vq->indirect);
+	}
+
+	if (desc) {
+		/* Use a single buffer which doesn't continue */
+		indirect = true;
+		/* Set up rest to use this indirect table. */
+		i = 0;
+		descs_used = 1;
+	} else {
+		indirect = false;
+		desc = vq->vring_packed.desc;
+		i = head;
+		descs_used = total_sg;
+	}
+
+	if (vq->vq.num_free < descs_used) {
+		pr_debug("Can't add buf len %i - avail = %i\n",
+			 descs_used, vq->vq.num_free);
+		/* FIXME: for historical reasons, we force a notify here if
+		 * there are outgoing parts to the buffer.  Presumably the
+		 * host should service the ring ASAP. */
+		if (out_sgs)
+			vq->notify(&vq->vq);
+		if (indirect)
+			kfree(desc);
+		END_USE(vq);
+		return -ENOSPC;
+	}
+
+	id = vq->free_head;
+	BUG_ON(id == vq->vring_packed.num);
+
+	curr = id;
+	for (n = 0; n < out_sgs + in_sgs; n++) {
+		for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+			dma_addr_t addr = vring_map_one_sg(vq, sg, n < out_sgs ?
+					       DMA_TO_DEVICE : DMA_FROM_DEVICE);
+			if (vring_mapping_error(vq, addr))
+				goto unmap_release;
+
+			flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_NEXT |
+				  (n < out_sgs ? 0 : VRING_DESC_F_WRITE) |
+				  VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
+				  VRING_DESC_F_USED(!vq->avail_wrap_counter));
+			if (!indirect && i == head)
+				head_flags = flags;
+			else
+				desc[i].flags = flags;
+
+			desc[i].addr = cpu_to_virtio64(_vq->vdev, addr);
+			desc[i].len = cpu_to_virtio32(_vq->vdev, sg->length);
+			i++;
+			if (!indirect) {
+				if (vring_use_dma_api(_vq->vdev)) {
+					vq->desc_state_packed[curr].addr = addr;
+					vq->desc_state_packed[curr].len =
+						sg->length;
+					vq->desc_state_packed[curr].flags =
+						virtio16_to_cpu(_vq->vdev,
+								flags);
+				}
+				curr = vq->desc_state_packed[curr].next;
+
+				if (i >= vq->vring_packed.num) {
+					i = 0;
+					vq->avail_wrap_counter ^= 1;
+				}
+			}
+		}
+	}
+
+	prev = (i > 0 ? i : vq->vring_packed.num) - 1;
+	desc[prev].id = cpu_to_virtio16(_vq->vdev, id);
+
+	/* Last one doesn't continue. */
+	if (total_sg == 1)
+		head_flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
+	else
+		desc[prev].flags &= cpu_to_virtio16(_vq->vdev,
+						~VRING_DESC_F_NEXT);
+
+	if (indirect) {
+		/* Now that the indirect table is filled in, map it. */
+		dma_addr_t addr = vring_map_single(
+			vq, desc, total_sg * sizeof(struct vring_packed_desc),
+			DMA_TO_DEVICE);
+		if (vring_mapping_error(vq, addr))
+			goto unmap_release;
+
+		head_flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_INDIRECT |
+				       VRING_DESC_F_AVAIL(avail_wrap_counter) |
+				       VRING_DESC_F_USED(!avail_wrap_counter));
+		vq->vring_packed.desc[head].addr = cpu_to_virtio64(_vq->vdev,
+								   addr);
+		vq->vring_packed.desc[head].len = cpu_to_virtio32(_vq->vdev,
+				total_sg * sizeof(struct vring_packed_desc));
+		vq->vring_packed.desc[head].id = cpu_to_virtio16(_vq->vdev, id);
+
+		if (vring_use_dma_api(_vq->vdev)) {
+			vq->desc_state_packed[id].addr = addr;
+			vq->desc_state_packed[id].len = total_sg *
+					sizeof(struct vring_packed_desc);
+			vq->desc_state_packed[id].flags =
+					virtio16_to_cpu(_vq->vdev, head_flags);
+		}
+	}
+
+	/* We're using some buffers from the free list. */
+	vq->vq.num_free -= descs_used;
+
+	/* Update free pointer */
+	if (indirect) {
+		n = head + 1;
+		if (n >= vq->vring_packed.num) {
+			n = 0;
+			vq->avail_wrap_counter ^= 1;
+		}
+		vq->next_avail_idx = n;
+		vq->free_head = vq->desc_state_packed[id].next;
+	} else {
+		vq->next_avail_idx = i;
+		vq->free_head = curr;
+	}
+
+	/* Store token and indirect buffer state. */
+	vq->desc_state_packed[id].num = descs_used;
+	vq->desc_state_packed[id].data = data;
+	if (indirect)
+		vq->desc_state_packed[id].indir_desc = desc;
+	else
+		vq->desc_state_packed[id].indir_desc = ctx;
+
+	/* A driver MUST NOT make the first descriptor in the list
+	 * available before all subsequent descriptors comprising
+	 * the list are made available. */
+	virtio_wmb(vq->weak_barriers);
+	vq->vring_packed.desc[head].flags = head_flags;
+	vq->num_added += descs_used;
+
+	pr_debug("Added buffer head %i to %p\n", head, vq);
+	END_USE(vq);
+
+	return 0;
+
+unmap_release:
+	err_idx = i;
+	i = head;
+
+	for (n = 0; n < total_sg; n++) {
+		if (i == err_idx)
+			break;
+		vring_unmap_desc_packed(vq, &desc[i]);
+		i++;
+		if (!indirect && i >= vq->vring_packed.num)
+			i = 0;
+	}
+
+	vq->avail_wrap_counter = avail_wrap_counter;
+
+	if (indirect)
+		kfree(desc);
+
+	END_USE(vq);
 	return -EIO;
 }
 
 static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
 {
-	return false;
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 flags;
+	bool needs_kick;
+	u32 snapshot;
+
+	START_USE(vq);
+	/* We need to expose the new flags value before checking notification
+	 * suppressions. */
+	virtio_mb(vq->weak_barriers);
+
+	snapshot = *(u32 *)vq->vring_packed.device;
+	flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3;
+
+#ifdef DEBUG
+	if (vq->last_add_time_valid) {
+		WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
+					      vq->last_add_time)) > 100);
+	}
+	vq->last_add_time_valid = false;
+#endif
+
+	needs_kick = (flags != VRING_EVENT_F_DISABLE);
+	END_USE(vq);
+	return needs_kick;
+}
+
+static void detach_buf_packed(struct vring_virtqueue *vq,
+			      unsigned int id, void **ctx)
+{
+	struct vring_desc_state_packed *state = NULL;
+	struct vring_packed_desc *desc;
+	unsigned int curr, i;
+
+	/* Clear data ptr. */
+	vq->desc_state_packed[id].data = NULL;
+
+	curr = id;
+	for (i = 0; i < vq->desc_state_packed[id].num; i++) {
+		state = &vq->desc_state_packed[curr];
+		vring_unmap_state_packed(vq, state);
+		curr = state->next;
+	}
+
+	BUG_ON(state == NULL);
+	vq->vq.num_free += vq->desc_state_packed[id].num;
+	state->next = vq->free_head;
+	vq->free_head = id;
+
+	if (vq->indirect) {
+		u32 len;
+
+		/* Free the indirect table, if any, now that it's unmapped. */
+		desc = vq->desc_state_packed[id].indir_desc;
+		if (!desc)
+			return;
+
+		if (vring_use_dma_api(vq->vq.vdev)) {
+			len = vq->desc_state_packed[id].len;
+			for (i = 0; i < len / sizeof(struct vring_packed_desc);
+					i++)
+				vring_unmap_desc_packed(vq, &desc[i]);
+		}
+		kfree(desc);
+		vq->desc_state_packed[id].indir_desc = NULL;
+	} else if (ctx) {
+		*ctx = vq->desc_state_packed[id].indir_desc;
+	}
 }
 
 static inline bool more_used_packed(const struct vring_virtqueue *vq)
 {
-	return false;
+	u16 last_used, flags;
+	u8 avail, used;
+
+	last_used = vq->last_used_idx;
+	flags = virtio16_to_cpu(vq->vq.vdev,
+				vq->vring_packed.desc[last_used].flags);
+	avail = !!(flags & VRING_DESC_F_AVAIL(1));
+	used = !!(flags & VRING_DESC_F_USED(1));
+
+	return avail == used && used == vq->used_wrap_counter;
 }
 
 static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
 					  unsigned int *len,
 					  void **ctx)
 {
-	return NULL;
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 last_used, id;
+	void *ret;
+
+	START_USE(vq);
+
+	if (unlikely(vq->broken)) {
+		END_USE(vq);
+		return NULL;
+	}
+
+	if (!more_used_packed(vq)) {
+		pr_debug("No more buffers in queue\n");
+		END_USE(vq);
+		return NULL;
+	}
+
+	/* Only get used elements after they have been exposed by host. */
+	virtio_rmb(vq->weak_barriers);
+
+	last_used = vq->last_used_idx;
+	id = virtio16_to_cpu(_vq->vdev, vq->vring_packed.desc[last_used].id);
+	*len = virtio32_to_cpu(_vq->vdev, vq->vring_packed.desc[last_used].len);
+
+	if (unlikely(id >= vq->vring_packed.num)) {
+		BAD_RING(vq, "id %u out of range\n", id);
+		return NULL;
+	}
+	if (unlikely(!vq->desc_state_packed[id].data)) {
+		BAD_RING(vq, "id %u is not a head!\n", id);
+		return NULL;
+	}
+
+	vq->last_used_idx += vq->desc_state_packed[id].num;
+	if (vq->last_used_idx >= vq->vring_packed.num) {
+		vq->last_used_idx -= vq->vring_packed.num;
+		vq->used_wrap_counter ^= 1;
+	}
+
+	/* detach_buf_packed clears data, so grab it now. */
+	ret = vq->desc_state_packed[id].data;
+	detach_buf_packed(vq, id, ctx);
+
+#ifdef DEBUG
+	vq->last_add_time_valid = false;
+#endif
+
+	END_USE(vq);
+	return ret;
 }
 
 static void virtqueue_disable_cb_packed(struct virtqueue *_vq)
 {
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	if (vq->event_flags_shadow != VRING_EVENT_F_DISABLE) {
+		vq->event_flags_shadow = VRING_EVENT_F_DISABLE;
+		vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+							vq->event_flags_shadow);
+	}
 }
 
 static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
 {
-	return 0;
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	START_USE(vq);
+
+	/* We optimistically turn back on interrupts, then check if there was
+	 * more to do. */
+
+	if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
+		vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+		vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+							vq->event_flags_shadow);
+	}
+
+	END_USE(vq);
+	return vq->last_used_idx;
 }
 
 static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
 {
-	return false;
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u8 avail, used;
+	u16 flags;
+
+	flags = virtio16_to_cpu(vq->vq.vdev,
+			vq->vring_packed.desc[last_used_idx].flags);
+	avail = !!(flags & VRING_DESC_F_AVAIL(1));
+	used = !!(flags & VRING_DESC_F_USED(1));
+
+	return avail == used && used == vq->used_wrap_counter;
 }
 
 static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
 {
-	return false;
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	START_USE(vq);
+
+	/* We optimistically turn back on interrupts, then check if there was
+	 * more to do. */
+
+	if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) {
+		vq->event_flags_shadow = VRING_EVENT_F_ENABLE;
+		vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev,
+							vq->event_flags_shadow);
+		/* We need to enable interrupts first before re-checking
+		 * for more used buffers. */
+		virtio_mb(vq->weak_barriers);
+	}
+
+	if (more_used_packed(vq)) {
+		END_USE(vq);
+		return false;
+	}
+
+	END_USE(vq);
+	return true;
 }
 
 static void *virtqueue_detach_unused_buf_packed(struct virtqueue *_vq)
 {
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	unsigned int i;
+	void *buf;
+
+	START_USE(vq);
+
+	for (i = 0; i < vq->vring_packed.num; i++) {
+		if (!vq->desc_state_packed[i].data)
+			continue;
+		/* detach_buf clears data, so grab it now. */
+		buf = vq->desc_state_packed[i].data;
+		detach_buf_packed(vq, i, NULL);
+		END_USE(vq);
+		return buf;
+	}
+	/* That should have freed everything. */
+	BUG_ON(vq->vq.num_free != vq->vring_packed.num);
+
+	END_USE(vq);
 	return NULL;
 }
 
@@ -1084,6 +1553,9 @@ bool virtqueue_poll(struct virtqueue *_vq, unsigned last_used_idx)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
+	/* We need to enable interrupts first before re-checking
+	 * for more used buffers. */
+	virtio_mb(vq->weak_barriers);
 	return vq->packed ? virtqueue_poll_packed(_vq, last_used_idx) :
 			    virtqueue_poll_split(_vq, last_used_idx);
 }
-- 
2.17.0

^ permalink raw reply related

* [RFC v6 2/5] virtio_ring: support creating packed ring
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev
  Cc: wexu, jfreimann, tiwei.bie
In-Reply-To: <20180605074046.20709-1-tiwei.bie@intel.com>

This commit introduces the support for creating packed ring.
All split ring specific functions are added _split suffix.
Some necessary stubs for packed ring are also added.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/virtio/virtio_ring.c | 801 +++++++++++++++++++++++------------
 include/linux/virtio_ring.h  |   8 +-
 2 files changed, 546 insertions(+), 263 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 71458f493cf8..416c33143125 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -61,11 +61,15 @@ struct vring_desc_state {
 	struct vring_desc *indir_desc;	/* Indirect descriptor, if any. */
 };
 
+struct vring_desc_state_packed {
+	int next;			/* The next desc state. */
+};
+
 struct vring_virtqueue {
 	struct virtqueue vq;
 
-	/* Actual memory layout for this queue */
-	struct vring vring;
+	/* Is this a packed ring? */
+	bool packed;
 
 	/* Can we use weak barriers? */
 	bool weak_barriers;
@@ -87,11 +91,39 @@ struct vring_virtqueue {
 	/* Last used index we've seen. */
 	u16 last_used_idx;
 
-	/* Last written value to avail->flags */
-	u16 avail_flags_shadow;
+	union {
+		/* Available for split ring */
+		struct {
+			/* Actual memory layout for this queue. */
+			struct vring vring;
 
-	/* Last written value to avail->idx in guest byte order */
-	u16 avail_idx_shadow;
+			/* Last written value to avail->flags */
+			u16 avail_flags_shadow;
+
+			/* Last written value to avail->idx in
+			 * guest byte order. */
+			u16 avail_idx_shadow;
+		};
+
+		/* Available for packed ring */
+		struct {
+			/* Actual memory layout for this queue. */
+			struct vring_packed vring_packed;
+
+			/* Driver ring wrap counter. */
+			bool avail_wrap_counter;
+
+			/* Device ring wrap counter. */
+			bool used_wrap_counter;
+
+			/* Index of the next avail descriptor. */
+			u16 next_avail_idx;
+
+			/* Last written value to driver->flags in
+			 * guest byte order. */
+			u16 event_flags_shadow;
+		};
+	};
 
 	/* How to notify other side. FIXME: commonalize hcalls! */
 	bool (*notify)(struct virtqueue *vq);
@@ -111,11 +143,24 @@ struct vring_virtqueue {
 #endif
 
 	/* Per-descriptor state. */
-	struct vring_desc_state desc_state[];
+	union {
+		struct vring_desc_state desc_state[1];
+		struct vring_desc_state_packed desc_state_packed[1];
+	};
 };
 
 #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
 
+static inline bool virtqueue_use_indirect(struct virtqueue *_vq,
+					  unsigned int total_sg)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	/* If the host supports indirect descriptor tables, and we have multiple
+	 * buffers, then go indirect. FIXME: tune this threshold */
+	return (vq->indirect && total_sg > 1 && vq->vq.num_free);
+}
+
 /*
  * Modern virtio devices have feature bits to specify whether they need a
  * quirk and bypass the IOMMU. If not there, just use the DMA API.
@@ -201,8 +246,17 @@ static dma_addr_t vring_map_single(const struct vring_virtqueue *vq,
 			      cpu_addr, size, direction);
 }
 
-static void vring_unmap_one(const struct vring_virtqueue *vq,
-			    struct vring_desc *desc)
+static int vring_mapping_error(const struct vring_virtqueue *vq,
+			       dma_addr_t addr)
+{
+	if (!vring_use_dma_api(vq->vq.vdev))
+		return 0;
+
+	return dma_mapping_error(vring_dma_dev(vq), addr);
+}
+
+static void vring_unmap_one_split(const struct vring_virtqueue *vq,
+				  struct vring_desc *desc)
 {
 	u16 flags;
 
@@ -226,17 +280,9 @@ static void vring_unmap_one(const struct vring_virtqueue *vq,
 	}
 }
 
-static int vring_mapping_error(const struct vring_virtqueue *vq,
-			       dma_addr_t addr)
-{
-	if (!vring_use_dma_api(vq->vq.vdev))
-		return 0;
-
-	return dma_mapping_error(vring_dma_dev(vq), addr);
-}
-
-static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
-					 unsigned int total_sg, gfp_t gfp)
+static struct vring_desc *alloc_indirect_split(struct virtqueue *_vq,
+					       unsigned int total_sg,
+					       gfp_t gfp)
 {
 	struct vring_desc *desc;
 	unsigned int i;
@@ -257,14 +303,14 @@ static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
 	return desc;
 }
 
-static inline int virtqueue_add(struct virtqueue *_vq,
-				struct scatterlist *sgs[],
-				unsigned int total_sg,
-				unsigned int out_sgs,
-				unsigned int in_sgs,
-				void *data,
-				void *ctx,
-				gfp_t gfp)
+static inline int virtqueue_add_split(struct virtqueue *_vq,
+				      struct scatterlist *sgs[],
+				      unsigned int total_sg,
+				      unsigned int out_sgs,
+				      unsigned int in_sgs,
+				      void *data,
+				      void *ctx,
+				      gfp_t gfp)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	struct scatterlist *sg;
@@ -300,10 +346,8 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
 	head = vq->free_head;
 
-	/* If the host supports indirect descriptor tables, and we have multiple
-	 * buffers, then go indirect. FIXME: tune this threshold */
-	if (vq->indirect && total_sg > 1 && vq->vq.num_free)
-		desc = alloc_indirect(_vq, total_sg, gfp);
+	if (virtqueue_use_indirect(_vq, total_sg))
+		desc = alloc_indirect_split(_vq, total_sg, gfp);
 	else {
 		desc = NULL;
 		WARN_ON_ONCE(total_sg > vq->vring.num && !vq->indirect);
@@ -424,7 +468,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 	for (n = 0; n < total_sg; n++) {
 		if (i == err_idx)
 			break;
-		vring_unmap_one(vq, &desc[i]);
+		vring_unmap_one_split(vq, &desc[i]);
 		i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
 	}
 
@@ -435,6 +479,355 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 	return -EIO;
 }
 
+static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 new, old;
+	bool needs_kick;
+
+	START_USE(vq);
+	/* We need to expose available array entries before checking avail
+	 * event. */
+	virtio_mb(vq->weak_barriers);
+
+	old = vq->avail_idx_shadow - vq->num_added;
+	new = vq->avail_idx_shadow;
+	vq->num_added = 0;
+
+#ifdef DEBUG
+	if (vq->last_add_time_valid) {
+		WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
+					      vq->last_add_time)) > 100);
+	}
+	vq->last_add_time_valid = false;
+#endif
+
+	if (vq->event) {
+		needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev, vring_avail_event(&vq->vring)),
+					      new, old);
+	} else {
+		needs_kick = !(vq->vring.used->flags & cpu_to_virtio16(_vq->vdev, VRING_USED_F_NO_NOTIFY));
+	}
+	END_USE(vq);
+	return needs_kick;
+}
+
+static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
+			     void **ctx)
+{
+	unsigned int i, j;
+	__virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
+
+	/* Clear data ptr. */
+	vq->desc_state[head].data = NULL;
+
+	/* Put back on free list: unmap first-level descriptors and find end */
+	i = head;
+
+	while (vq->vring.desc[i].flags & nextflag) {
+		vring_unmap_one_split(vq, &vq->vring.desc[i]);
+		i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next);
+		vq->vq.num_free++;
+	}
+
+	vring_unmap_one_split(vq, &vq->vring.desc[i]);
+	vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head);
+	vq->free_head = head;
+
+	/* Plus final descriptor */
+	vq->vq.num_free++;
+
+	if (vq->indirect) {
+		struct vring_desc *indir_desc = vq->desc_state[head].indir_desc;
+		u32 len;
+
+		/* Free the indirect table, if any, now that it's unmapped. */
+		if (!indir_desc)
+			return;
+
+		len = virtio32_to_cpu(vq->vq.vdev, vq->vring.desc[head].len);
+
+		BUG_ON(!(vq->vring.desc[head].flags &
+			 cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT)));
+		BUG_ON(len == 0 || len % sizeof(struct vring_desc));
+
+		for (j = 0; j < len / sizeof(struct vring_desc); j++)
+			vring_unmap_one_split(vq, &indir_desc[j]);
+
+		kfree(indir_desc);
+		vq->desc_state[head].indir_desc = NULL;
+	} else if (ctx) {
+		*ctx = vq->desc_state[head].indir_desc;
+	}
+}
+
+static inline bool more_used_split(const struct vring_virtqueue *vq)
+{
+	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx);
+}
+
+static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
+					 unsigned int *len,
+					 void **ctx)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	void *ret;
+	unsigned int i;
+	u16 last_used;
+
+	START_USE(vq);
+
+	if (unlikely(vq->broken)) {
+		END_USE(vq);
+		return NULL;
+	}
+
+	if (!more_used_split(vq)) {
+		pr_debug("No more buffers in queue\n");
+		END_USE(vq);
+		return NULL;
+	}
+
+	/* Only get used array entries after they have been exposed by host. */
+	virtio_rmb(vq->weak_barriers);
+
+	last_used = (vq->last_used_idx & (vq->vring.num - 1));
+	i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
+	*len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
+
+	if (unlikely(i >= vq->vring.num)) {
+		BAD_RING(vq, "id %u out of range\n", i);
+		return NULL;
+	}
+	if (unlikely(!vq->desc_state[i].data)) {
+		BAD_RING(vq, "id %u is not a head!\n", i);
+		return NULL;
+	}
+
+	/* detach_buf_split clears data, so grab it now. */
+	ret = vq->desc_state[i].data;
+	detach_buf_split(vq, i, ctx);
+	vq->last_used_idx++;
+	/* If we expect an interrupt for the next entry, tell host
+	 * by writing event index and flush out the write before
+	 * the read in the next get_buf call. */
+	if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
+		virtio_store_mb(vq->weak_barriers,
+				&vring_used_event(&vq->vring),
+				cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
+
+#ifdef DEBUG
+	vq->last_add_time_valid = false;
+#endif
+
+	END_USE(vq);
+	return ret;
+}
+
+static void virtqueue_disable_cb_split(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
+		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
+		if (!vq->event)
+			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+	}
+}
+
+static unsigned virtqueue_enable_cb_prepare_split(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 last_used_idx;
+
+	START_USE(vq);
+
+	/* We optimistically turn back on interrupts, then check if there was
+	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always do both to keep code simple. */
+	if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
+		vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
+		if (!vq->event)
+			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+	}
+	vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, last_used_idx = vq->last_used_idx);
+	END_USE(vq);
+	return last_used_idx;
+}
+
+static bool virtqueue_poll_split(struct virtqueue *_vq, unsigned last_used_idx)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	virtio_mb(vq->weak_barriers);
+	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
+}
+
+static bool virtqueue_enable_cb_delayed_split(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 bufs;
+
+	START_USE(vq);
+
+	/* We optimistically turn back on interrupts, then check if there was
+	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always update the event index to keep code simple. */
+	if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
+		vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
+		if (!vq->event)
+			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
+	}
+	/* TODO: tune this threshold */
+	bufs = (u16)(vq->avail_idx_shadow - vq->last_used_idx) * 3 / 4;
+
+	virtio_store_mb(vq->weak_barriers,
+			&vring_used_event(&vq->vring),
+			cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs));
+
+	if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
+		END_USE(vq);
+		return false;
+	}
+
+	END_USE(vq);
+	return true;
+}
+
+static void *virtqueue_detach_unused_buf_split(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	unsigned int i;
+	void *buf;
+
+	START_USE(vq);
+
+	for (i = 0; i < vq->vring.num; i++) {
+		if (!vq->desc_state[i].data)
+			continue;
+		/* detach_buf clears data, so grab it now. */
+		buf = vq->desc_state[i].data;
+		detach_buf_split(vq, i, NULL);
+		vq->avail_idx_shadow--;
+		vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow);
+		END_USE(vq);
+		return buf;
+	}
+	/* That should have freed everything. */
+	BUG_ON(vq->vq.num_free != vq->vring.num);
+
+	END_USE(vq);
+	return NULL;
+}
+
+/*
+ * The layout for the packed ring is a continuous chunk of memory
+ * which looks like this.
+ *
+ * struct vring_packed {
+ *	// The actual descriptors (16 bytes each)
+ *	struct vring_packed_desc desc[num];
+ *
+ *	// Padding to the next align boundary.
+ *	char pad[];
+ *
+ *	// Driver Event Suppression
+ *	struct vring_packed_desc_event driver;
+ *
+ *	// Device Event Suppression
+ *	struct vring_packed_desc_event device;
+ * };
+ */
+static inline void vring_init_packed(struct vring_packed *vr, unsigned int num,
+				     void *p, unsigned long align)
+{
+	vr->num = num;
+	vr->desc = p;
+	vr->driver = (void *)ALIGN(((uintptr_t)p +
+		sizeof(struct vring_packed_desc) * num), align);
+	vr->device = vr->driver + 1;
+}
+
+static inline unsigned vring_size_packed(unsigned int num, unsigned long align)
+{
+	return ((sizeof(struct vring_packed_desc) * num + align - 1)
+		& ~(align - 1)) + sizeof(struct vring_packed_desc_event) * 2;
+}
+
+static inline int virtqueue_add_packed(struct virtqueue *_vq,
+				       struct scatterlist *sgs[],
+				       unsigned int total_sg,
+				       unsigned int out_sgs,
+				       unsigned int in_sgs,
+				       void *data,
+				       void *ctx,
+				       gfp_t gfp)
+{
+	return -EIO;
+}
+
+static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
+{
+	return false;
+}
+
+static inline bool more_used_packed(const struct vring_virtqueue *vq)
+{
+	return false;
+}
+
+static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq,
+					  unsigned int *len,
+					  void **ctx)
+{
+	return NULL;
+}
+
+static void virtqueue_disable_cb_packed(struct virtqueue *_vq)
+{
+}
+
+static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq)
+{
+	return 0;
+}
+
+static bool virtqueue_poll_packed(struct virtqueue *_vq, unsigned last_used_idx)
+{
+	return false;
+}
+
+static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq)
+{
+	return false;
+}
+
+static void *virtqueue_detach_unused_buf_packed(struct virtqueue *_vq)
+{
+	return NULL;
+}
+
+static inline int virtqueue_add(struct virtqueue *_vq,
+				struct scatterlist *sgs[],
+				unsigned int total_sg,
+				unsigned int out_sgs,
+				unsigned int in_sgs,
+				void *data,
+				void *ctx,
+				gfp_t gfp)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	return vq->packed ? virtqueue_add_packed(_vq, sgs, total_sg, out_sgs,
+						 in_sgs, data, ctx, gfp) :
+			    virtqueue_add_split(_vq, sgs, total_sg, out_sgs,
+						in_sgs, data, ctx, gfp);
+}
+
 /**
  * virtqueue_add_sgs - expose buffers to other end
  * @vq: the struct virtqueue we're talking about.
@@ -551,34 +944,9 @@ EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
 bool virtqueue_kick_prepare(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	u16 new, old;
-	bool needs_kick;
 
-	START_USE(vq);
-	/* We need to expose available array entries before checking avail
-	 * event. */
-	virtio_mb(vq->weak_barriers);
-
-	old = vq->avail_idx_shadow - vq->num_added;
-	new = vq->avail_idx_shadow;
-	vq->num_added = 0;
-
-#ifdef DEBUG
-	if (vq->last_add_time_valid) {
-		WARN_ON(ktime_to_ms(ktime_sub(ktime_get(),
-					      vq->last_add_time)) > 100);
-	}
-	vq->last_add_time_valid = false;
-#endif
-
-	if (vq->event) {
-		needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev, vring_avail_event(&vq->vring)),
-					      new, old);
-	} else {
-		needs_kick = !(vq->vring.used->flags & cpu_to_virtio16(_vq->vdev, VRING_USED_F_NO_NOTIFY));
-	}
-	END_USE(vq);
-	return needs_kick;
+	return vq->packed ? virtqueue_kick_prepare_packed(_vq) :
+			    virtqueue_kick_prepare_split(_vq);
 }
 EXPORT_SYMBOL_GPL(virtqueue_kick_prepare);
 
@@ -626,58 +994,9 @@ bool virtqueue_kick(struct virtqueue *vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_kick);
 
-static void detach_buf(struct vring_virtqueue *vq, unsigned int head,
-		       void **ctx)
-{
-	unsigned int i, j;
-	__virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
-
-	/* Clear data ptr. */
-	vq->desc_state[head].data = NULL;
-
-	/* Put back on free list: unmap first-level descriptors and find end */
-	i = head;
-
-	while (vq->vring.desc[i].flags & nextflag) {
-		vring_unmap_one(vq, &vq->vring.desc[i]);
-		i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next);
-		vq->vq.num_free++;
-	}
-
-	vring_unmap_one(vq, &vq->vring.desc[i]);
-	vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head);
-	vq->free_head = head;
-
-	/* Plus final descriptor */
-	vq->vq.num_free++;
-
-	if (vq->indirect) {
-		struct vring_desc *indir_desc = vq->desc_state[head].indir_desc;
-		u32 len;
-
-		/* Free the indirect table, if any, now that it's unmapped. */
-		if (!indir_desc)
-			return;
-
-		len = virtio32_to_cpu(vq->vq.vdev, vq->vring.desc[head].len);
-
-		BUG_ON(!(vq->vring.desc[head].flags &
-			 cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT)));
-		BUG_ON(len == 0 || len % sizeof(struct vring_desc));
-
-		for (j = 0; j < len / sizeof(struct vring_desc); j++)
-			vring_unmap_one(vq, &indir_desc[j]);
-
-		kfree(indir_desc);
-		vq->desc_state[head].indir_desc = NULL;
-	} else if (ctx) {
-		*ctx = vq->desc_state[head].indir_desc;
-	}
-}
-
 static inline bool more_used(const struct vring_virtqueue *vq)
 {
-	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx);
+	return vq->packed ? more_used_packed(vq) : more_used_split(vq);
 }
 
 /**
@@ -700,57 +1019,9 @@ void *virtqueue_get_buf_ctx(struct virtqueue *_vq, unsigned int *len,
 			    void **ctx)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	void *ret;
-	unsigned int i;
-	u16 last_used;
 
-	START_USE(vq);
-
-	if (unlikely(vq->broken)) {
-		END_USE(vq);
-		return NULL;
-	}
-
-	if (!more_used(vq)) {
-		pr_debug("No more buffers in queue\n");
-		END_USE(vq);
-		return NULL;
-	}
-
-	/* Only get used array entries after they have been exposed by host. */
-	virtio_rmb(vq->weak_barriers);
-
-	last_used = (vq->last_used_idx & (vq->vring.num - 1));
-	i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id);
-	*len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len);
-
-	if (unlikely(i >= vq->vring.num)) {
-		BAD_RING(vq, "id %u out of range\n", i);
-		return NULL;
-	}
-	if (unlikely(!vq->desc_state[i].data)) {
-		BAD_RING(vq, "id %u is not a head!\n", i);
-		return NULL;
-	}
-
-	/* detach_buf clears data, so grab it now. */
-	ret = vq->desc_state[i].data;
-	detach_buf(vq, i, ctx);
-	vq->last_used_idx++;
-	/* If we expect an interrupt for the next entry, tell host
-	 * by writing event index and flush out the write before
-	 * the read in the next get_buf call. */
-	if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
-		virtio_store_mb(vq->weak_barriers,
-				&vring_used_event(&vq->vring),
-				cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
-
-#ifdef DEBUG
-	vq->last_add_time_valid = false;
-#endif
-
-	END_USE(vq);
-	return ret;
+	return vq->packed ? virtqueue_get_buf_ctx_packed(_vq, len, ctx) :
+			    virtqueue_get_buf_ctx_split(_vq, len, ctx);
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_buf_ctx);
 
@@ -772,12 +1043,10 @@ void virtqueue_disable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	if (!(vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
-		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
-		if (!vq->event)
-			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
-	}
-
+	if (vq->packed)
+		virtqueue_disable_cb_packed(_vq);
+	else
+		virtqueue_disable_cb_split(_vq);
 }
 EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
 
@@ -796,23 +1065,9 @@ EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
 unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	u16 last_used_idx;
 
-	START_USE(vq);
-
-	/* We optimistically turn back on interrupts, then check if there was
-	 * more to do. */
-	/* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to
-	 * either clear the flags bit or point the event index at the next
-	 * entry. Always do both to keep code simple. */
-	if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
-		vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
-		if (!vq->event)
-			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
-	}
-	vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, last_used_idx = vq->last_used_idx);
-	END_USE(vq);
-	return last_used_idx;
+	return vq->packed ? virtqueue_enable_cb_prepare_packed(_vq) :
+			    virtqueue_enable_cb_prepare_split(_vq);
 }
 EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare);
 
@@ -829,8 +1084,8 @@ bool virtqueue_poll(struct virtqueue *_vq, unsigned last_used_idx)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	virtio_mb(vq->weak_barriers);
-	return (u16)last_used_idx != virtio16_to_cpu(_vq->vdev, vq->vring.used->idx);
+	return vq->packed ? virtqueue_poll_packed(_vq, last_used_idx) :
+			    virtqueue_poll_split(_vq, last_used_idx);
 }
 EXPORT_SYMBOL_GPL(virtqueue_poll);
 
@@ -868,34 +1123,9 @@ EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
 bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	u16 bufs;
 
-	START_USE(vq);
-
-	/* We optimistically turn back on interrupts, then check if there was
-	 * more to do. */
-	/* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
-	 * either clear the flags bit or point the event index at the next
-	 * entry. Always update the event index to keep code simple. */
-	if (vq->avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT) {
-		vq->avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
-		if (!vq->event)
-			vq->vring.avail->flags = cpu_to_virtio16(_vq->vdev, vq->avail_flags_shadow);
-	}
-	/* TODO: tune this threshold */
-	bufs = (u16)(vq->avail_idx_shadow - vq->last_used_idx) * 3 / 4;
-
-	virtio_store_mb(vq->weak_barriers,
-			&vring_used_event(&vq->vring),
-			cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs));
-
-	if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
-		END_USE(vq);
-		return false;
-	}
-
-	END_USE(vq);
-	return true;
+	return vq->packed ? virtqueue_enable_cb_delayed_packed(_vq) :
+			    virtqueue_enable_cb_delayed_split(_vq);
 }
 EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
 
@@ -910,27 +1140,9 @@ EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
 void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
-	unsigned int i;
-	void *buf;
 
-	START_USE(vq);
-
-	for (i = 0; i < vq->vring.num; i++) {
-		if (!vq->desc_state[i].data)
-			continue;
-		/* detach_buf clears data, so grab it now. */
-		buf = vq->desc_state[i].data;
-		detach_buf(vq, i, NULL);
-		vq->avail_idx_shadow--;
-		vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow);
-		END_USE(vq);
-		return buf;
-	}
-	/* That should have freed everything. */
-	BUG_ON(vq->vq.num_free != vq->vring.num);
-
-	END_USE(vq);
-	return NULL;
+	return vq->packed ? virtqueue_detach_unused_buf_packed(_vq) :
+			    virtqueue_detach_unused_buf_split(_vq);
 }
 EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf);
 
@@ -955,7 +1167,8 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
 EXPORT_SYMBOL_GPL(vring_interrupt);
 
 struct virtqueue *__vring_new_virtqueue(unsigned int index,
-					struct vring vring,
+					union vring_union vring,
+					bool packed,
 					struct virtio_device *vdev,
 					bool weak_barriers,
 					bool context,
@@ -963,19 +1176,22 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
 					void (*callback)(struct virtqueue *),
 					const char *name)
 {
-	unsigned int i;
 	struct vring_virtqueue *vq;
+	unsigned int num, i;
+	size_t size;
 
-	vq = kmalloc(sizeof(*vq) + vring.num * sizeof(struct vring_desc_state),
-		     GFP_KERNEL);
+	num = packed ? vring.vring_packed.num : vring.vring_split.num;
+	size = packed ? num * sizeof(struct vring_desc_state_packed) :
+			num * sizeof(struct vring_desc_state);
+
+	vq = kmalloc(sizeof(*vq) + size, GFP_KERNEL);
 	if (!vq)
 		return NULL;
 
-	vq->vring = vring;
 	vq->vq.callback = callback;
 	vq->vq.vdev = vdev;
 	vq->vq.name = name;
-	vq->vq.num_free = vring.num;
+	vq->vq.num_free = num;
 	vq->vq.index = index;
 	vq->we_own_ring = false;
 	vq->queue_dma_addr = 0;
@@ -984,9 +1200,8 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
 	vq->weak_barriers = weak_barriers;
 	vq->broken = false;
 	vq->last_used_idx = 0;
-	vq->avail_flags_shadow = 0;
-	vq->avail_idx_shadow = 0;
 	vq->num_added = 0;
+	vq->packed = packed;
 	list_add_tail(&vq->vq.list, &vdev->vqs);
 #ifdef DEBUG
 	vq->in_use = false;
@@ -997,19 +1212,48 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
 		!context;
 	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
 
+	if (vq->packed) {
+		vq->vring_packed = vring.vring_packed;
+		vq->next_avail_idx = 0;
+		vq->avail_wrap_counter = 1;
+		vq->used_wrap_counter = 1;
+		vq->event_flags_shadow = 0;
+
+		memset(vq->desc_state_packed, 0,
+			num * sizeof(struct vring_desc_state_packed));
+
+		/* Put everything in free lists. */
+		vq->free_head = 0;
+		for (i = 0; i < num-1; i++)
+			vq->desc_state_packed[i].next = i + 1;
+	} else {
+		vq->vring = vring.vring_split;
+		vq->avail_flags_shadow = 0;
+		vq->avail_idx_shadow = 0;
+
+		/* Put everything in free lists. */
+		vq->free_head = 0;
+		for (i = 0; i < num-1; i++)
+			vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
+
+		memset(vq->desc_state, 0,
+			num * sizeof(struct vring_desc_state));
+	}
+
 	/* No callback?  Tell other side not to bother us. */
 	if (!callback) {
-		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
-		if (!vq->event)
-			vq->vring.avail->flags = cpu_to_virtio16(vdev, vq->avail_flags_shadow);
+		if (packed) {
+			vq->event_flags_shadow = VRING_EVENT_F_DISABLE;
+			vq->vring_packed.driver->flags = cpu_to_virtio16(vdev,
+						vq->event_flags_shadow);
+		} else {
+			vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
+			if (!vq->event)
+				vq->vring.avail->flags = cpu_to_virtio16(vdev,
+						vq->avail_flags_shadow);
+		}
 	}
 
-	/* Put everything in free lists. */
-	vq->free_head = 0;
-	for (i = 0; i < vring.num-1; i++)
-		vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
-	memset(vq->desc_state, 0, vring.num * sizeof(struct vring_desc_state));
-
 	return &vq->vq;
 }
 EXPORT_SYMBOL_GPL(__vring_new_virtqueue);
@@ -1056,6 +1300,12 @@ static void vring_free_queue(struct virtio_device *vdev, size_t size,
 	}
 }
 
+static inline int
+__vring_size(unsigned int num, unsigned long align, bool packed)
+{
+	return packed ? vring_size_packed(num, align) : vring_size(num, align);
+}
+
 struct virtqueue *vring_create_virtqueue(
 	unsigned int index,
 	unsigned int num,
@@ -1072,7 +1322,8 @@ struct virtqueue *vring_create_virtqueue(
 	void *queue = NULL;
 	dma_addr_t dma_addr;
 	size_t queue_size_in_bytes;
-	struct vring vring;
+	union vring_union vring;
+	bool packed;
 
 	/* We assume num is a power of 2. */
 	if (num & (num - 1)) {
@@ -1080,9 +1331,13 @@ struct virtqueue *vring_create_virtqueue(
 		return NULL;
 	}
 
+	packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED);
+
 	/* TODO: allocate each queue chunk individually */
-	for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
-		queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+	for (; num && __vring_size(num, vring_align, packed) > PAGE_SIZE;
+			num /= 2) {
+		queue = vring_alloc_queue(vdev, __vring_size(num, vring_align,
+							     packed),
 					  &dma_addr,
 					  GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
 		if (queue)
@@ -1094,17 +1349,21 @@ struct virtqueue *vring_create_virtqueue(
 
 	if (!queue) {
 		/* Try to get a single page. You are my only hope! */
-		queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+		queue = vring_alloc_queue(vdev, __vring_size(num, vring_align,
+							     packed),
 					  &dma_addr, GFP_KERNEL|__GFP_ZERO);
 	}
 	if (!queue)
 		return NULL;
 
-	queue_size_in_bytes = vring_size(num, vring_align);
-	vring_init(&vring, num, queue, vring_align);
+	queue_size_in_bytes = __vring_size(num, vring_align, packed);
+	if (packed)
+		vring_init_packed(&vring.vring_packed, num, queue, vring_align);
+	else
+		vring_init(&vring.vring_split, num, queue, vring_align);
 
-	vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
-				   notify, callback, name);
+	vq = __vring_new_virtqueue(index, vring, packed, vdev, weak_barriers,
+				   context, notify, callback, name);
 	if (!vq) {
 		vring_free_queue(vdev, queue_size_in_bytes, queue,
 				 dma_addr);
@@ -1130,10 +1389,17 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
 				      void (*callback)(struct virtqueue *vq),
 				      const char *name)
 {
-	struct vring vring;
-	vring_init(&vring, num, pages, vring_align);
-	return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
-				     notify, callback, name);
+	union vring_union vring;
+	bool packed;
+
+	packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED);
+	if (packed)
+		vring_init_packed(&vring.vring_packed, num, pages, vring_align);
+	else
+		vring_init(&vring.vring_split, num, pages, vring_align);
+
+	return __vring_new_virtqueue(index, vring, packed, vdev, weak_barriers,
+				     context, notify, callback, name);
 }
 EXPORT_SYMBOL_GPL(vring_new_virtqueue);
 
@@ -1143,7 +1409,9 @@ void vring_del_virtqueue(struct virtqueue *_vq)
 
 	if (vq->we_own_ring) {
 		vring_free_queue(vq->vq.vdev, vq->queue_size_in_bytes,
-				 vq->vring.desc, vq->queue_dma_addr);
+				 vq->packed ? (void *)vq->vring_packed.desc :
+					      (void *)vq->vring.desc,
+				 vq->queue_dma_addr);
 	}
 	list_del(&_vq->list);
 	kfree(vq);
@@ -1185,7 +1453,7 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
 
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	return vq->vring.num;
+	return vq->packed ? vq->vring_packed.num : vq->vring.num;
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_vring_size);
 
@@ -1228,6 +1496,10 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
 
 	BUG_ON(!vq->we_own_ring);
 
+	if (vq->packed)
+		return vq->queue_dma_addr + ((char *)vq->vring_packed.driver -
+				(char *)vq->vring_packed.desc);
+
 	return vq->queue_dma_addr +
 		((char *)vq->vring.avail - (char *)vq->vring.desc);
 }
@@ -1239,11 +1511,16 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
 
 	BUG_ON(!vq->we_own_ring);
 
+	if (vq->packed)
+		return vq->queue_dma_addr + ((char *)vq->vring_packed.device -
+				(char *)vq->vring_packed.desc);
+
 	return vq->queue_dma_addr +
 		((char *)vq->vring.used - (char *)vq->vring.desc);
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
 
+/* Only available for split ring */
 const struct vring *virtqueue_get_vring(struct virtqueue *vq)
 {
 	return &to_vvq(vq)->vring;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index bbf32524ab27..a0075894ad16 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,11 @@ static inline void virtio_store_mb(bool weak_barriers,
 struct virtio_device;
 struct virtqueue;
 
+union vring_union {
+	struct vring vring_split;
+	struct vring_packed vring_packed;
+};
+
 /*
  * Creates a virtqueue and allocates the descriptor ring.  If
  * may_reduce_num is set, then this may allocate a smaller ring than
@@ -79,7 +84,8 @@ struct virtqueue *vring_create_virtqueue(unsigned int index,
 
 /* Creates a virtqueue with a custom layout. */
 struct virtqueue *__vring_new_virtqueue(unsigned int index,
-					struct vring vring,
+					union vring_union vring,
+					bool packed,
 					struct virtio_device *vdev,
 					bool weak_barriers,
 					bool ctx,
-- 
2.17.0

^ permalink raw reply related

* [RFC v6 1/5] virtio: add packed ring definitions
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev; +Cc: wexu
In-Reply-To: <20180605074046.20709-1-tiwei.bie@intel.com>

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 include/uapi/linux/virtio_config.h |  5 ++++-
 include/uapi/linux/virtio_ring.h   | 36 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..932a6ecc8e46 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START	28
-#define VIRTIO_TRANSPORT_F_END		34
+#define VIRTIO_TRANSPORT_F_END		35
 
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 /* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,7 @@
  * this is for compatibility with legacy systems.
  */
 #define VIRTIO_F_IOMMU_PLATFORM		33
+
+/* This feature indicates support for the packed virtqueue layout. */
+#define VIRTIO_F_RING_PACKED		34
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 6d5d5faa989b..7b378da788a7 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -44,6 +44,9 @@
 /* This means the buffer contains a list of buffer descriptors. */
 #define VRING_DESC_F_INDIRECT	4
 
+#define VRING_DESC_F_AVAIL(b)	((__u16)(b) << 7)
+#define VRING_DESC_F_USED(b)	((__u16)(b) << 15)
+
 /* The Host uses this in used->flags to advise the Guest: don't kick me when
  * you add a buffer.  It's unreliable, so it's simply an optimization.  Guest
  * will still kick if it's out of buffers. */
@@ -53,6 +56,10 @@
  * optimization.  */
 #define VRING_AVAIL_F_NO_INTERRUPT	1
 
+#define VRING_EVENT_F_ENABLE	0x0
+#define VRING_EVENT_F_DISABLE	0x1
+#define VRING_EVENT_F_DESC	0x2
+
 /* We support indirect buffer descriptors */
 #define VIRTIO_RING_F_INDIRECT_DESC	28
 
@@ -171,4 +178,33 @@ static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old)
 	return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
 }
 
+struct vring_packed_desc_event {
+	/* __virtio16 off  : 15; // Descriptor Event Offset
+	 * __virtio16 wrap : 1;  // Descriptor Event Wrap Counter */
+	__virtio16 off_wrap;
+	/* __virtio16 flags : 2; // Descriptor Event Flags */
+	__virtio16 flags;
+};
+
+struct vring_packed_desc {
+	/* Buffer Address. */
+	__virtio64 addr;
+	/* Buffer Length. */
+	__virtio32 len;
+	/* Buffer ID. */
+	__virtio16 id;
+	/* The flags depending on descriptor type. */
+	__virtio16 flags;
+};
+
+struct vring_packed {
+	unsigned int num;
+
+	struct vring_packed_desc *desc;
+
+	struct vring_packed_desc_event *driver;
+
+	struct vring_packed_desc_event *device;
+};
+
 #endif /* _UAPI_LINUX_VIRTIO_RING_H */
-- 
2.17.0

^ permalink raw reply related

* [RFC v6 0/5] virtio: support packed ring
From: Tiwei Bie @ 2018-06-05  7:40 UTC (permalink / raw)
  To: mst, jasowang, virtualization, linux-kernel, netdev
  Cc: wexu, jfreimann, tiwei.bie

Hello everyone,

This RFC implements packed ring support in virtio driver.

Some functional tests have been done with Jason's
packed ring implementation in vhost (RFC v5):

https://lwn.net/Articles/755862/

Both of ping and netperf worked as expected.

TODO:
- Refinements (for code and commit log);
- More tests and bug fixes if any;
- Send the formal patch set;

RFC v5 -> RFC v6:
- Avoid tracking addr/len/flags when DMA API isn't used (MST/Jason);
- Define wrap counter as bool (Jason);
- Use ALIGN() in vring_init_packed() (Jason);
- Avoid using pointer to track `next` in detach_buf_packed() (Jason);
- Add comments for barriers (Jason);
- Don't enable RING_PACKED on ccw for now (noticed by Jason);
- Refine the memory barrier in virtqueue_poll();
- Add a missing memory barrier in virtqueue_enable_cb_delayed_packed();
- Remove the hacks in virtqueue_enable_cb_prepare_packed();

RFC v4 -> RFC v5:
- Save DMA addr, etc in desc state (Jason);
- Track used wrap counter;

RFC v3 -> RFC v4:
- Make ID allocation support out-of-order (Jason);
- Various fixes for EVENT_IDX support;

RFC v2 -> RFC v3:
- Split into small patches (Jason);
- Add helper virtqueue_use_indirect() (Jason);
- Just set id for the last descriptor of a list (Jason);
- Calculate the prev in virtqueue_add_packed() (Jason);
- Fix/improve desc suppression code (Jason/MST);
- Refine the code layout for XXX_split/packed and wrappers (MST);
- Fix the comments and API in uapi (MST);
- Remove the BUG_ON() for indirect (Jason);
- Some other refinements and bug fixes;

RFC v1 -> RFC v2:
- Add indirect descriptor support - compile test only;
- Add event suppression supprt - compile test only;
- Move vring_packed_init() out of uapi (Jason, MST);
- Merge two loops into one in virtqueue_add_packed() (Jason);
- Split vring_unmap_one() for packed ring and split ring (Jason);
- Avoid using '%' operator (Jason);
- Rename free_head -> next_avail_idx (Jason);
- Add comments for virtio_wmb() in virtqueue_add_packed() (Jason);
- Some other refinements and bug fixes;

Thanks!

Tiwei Bie (5):
  virtio: add packed ring definitions
  virtio_ring: support creating packed ring
  virtio_ring: add packed ring support
  virtio_ring: add event idx support in packed ring
  virtio_ring: enable packed ring

 drivers/s390/virtio/virtio_ccw.c   |    8 +
 drivers/virtio/virtio_ring.c       | 1361 ++++++++++++++++++++++------
 include/linux/virtio_ring.h        |    8 +-
 include/uapi/linux/virtio_config.h |    5 +-
 include/uapi/linux/virtio_ring.h   |   36 +
 5 files changed, 1141 insertions(+), 277 deletions(-)

-- 
2.17.0

^ permalink raw reply

* Re: [PATCH net-next] net: sched: return error code when tcf proto is not found
From: Vlad Buslov @ 2018-06-05  7:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, jhs, xiyou.wangcong, jiri, dan.carpenter
In-Reply-To: <20180604.173219.1997841279674931039.davem@davemloft.net>


On Mon 04 Jun 2018 at 21:32, David Miller <davem@davemloft.net> wrote:
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Mon,  4 Jun 2018 18:32:23 +0300
>
>> If requested tcf proto is not found, get and del filter netlink protocol
>> handlers output error message to extack, but do not return actual error
>> code. Add check to return ENOENT when result of tp find function is NULL
>> pointer.
>> 
>> Fixes: c431f89b18a2 ("net: sched: split tc_ctl_tfilter into three
>> handlers")
>
> Please do not split up a Fixes: tag into multiple lines.  I fixed it
> up for you this time.

Got it, thanks.

>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>
> Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] net/mlx5e: fix error return code in mlx5e_alloc_rq()
From: Tariq Toukan @ 2018-06-05  6:48 UTC (permalink / raw)
  To: Wei Yongjun, Saeed Mahameed, Leon Romanovsky, Tariq Toukan
  Cc: netdev, linux-rdma, kernel-janitors
In-Reply-To: <1528166576-55047-1-git-send-email-weiyongjun1@huawei.com>



On 05/06/2018 5:42 AM, Wei Yongjun wrote:
> Fix to return error code -ENOMEM from the kvzalloc_node() error handling
> case instead of 0, as done elsewhere in this function.
> 
> Fixes: 069d11465a80 ("net/mlx5e: RX, Enhance legacy Receive Queue memory scheme")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 333d4ed..89c96a0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -566,8 +566,10 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
>   			kvzalloc_node((wq_sz << rq->wqe.info.log_num_frags) *
>   				      sizeof(*rq->wqe.frags),
>   				      GFP_KERNEL, cpu_to_node(c->cpu));
> -		if (!rq->wqe.frags)
> +		if (!rq->wqe.frags) {
> +			err = -ENOMEM;
>   			goto err_free;
> +		}
>   
>   		err = mlx5e_init_di_list(rq, params, wq_sz, c->cpu);
>   		if (err)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Thanks Wei!

Reviewed-by: Tariq Toukan <tariqt@mellanox.com>

^ permalink raw reply

* Re: WARNING: CPU: 3 PID: 0 at net/sched/sch_hfsc.c:1388 hfsc_dequeue+0x319/0x350 [sch_hfsc]
From: Marco Berizzi @ 2018-06-05  6:39 UTC (permalink / raw)
  To: netdev
In-Reply-To: <746588000.720568.1520524944312@mail.libero.it>

> Il 8 marzo 2018 alle 17.02 Marco Berizzi <pupilla@libero.it> ha scritto:
> 
> > Marco Berizzi wrote:
> > 
> > Hello everyone,
> > 
> > Yesterday I got this error on a slackware linux 4.16-rc4 system
> > running as a traffic shaping gateway and netfilter nat.
> > The error has been arisen after a partial ISP network outage,
> > so unfortunately it will not trivial for me to reproduce it again.
> 
> Hello everyone,
> 
> I'm getting this error twice/day, so fortunately I'm able to
> reproduce it.

I'm getting the same error also with 4.17 (4.15.18 was
the latest working version):

[   11.445829] WARNING: CPU: 0 PID: 0 at net/sched/sch_hfsc.c:1388 hfsc_dequeue+0x319/0x350 [sch_hfsc]
[   11.445831] Modules linked in: cls_u32 sch_sfq sch_hfsc nf_nat_ftp nf_conntrack_ftp nf_nat nf_conntrack nft_set_hash nft_set_rbtree nf_tables nfnetlink ext2 hid_generic usbhid hid uhci_hcd ehci_pci ehci_hcd usbcore intel_cstate usb_common bnx2 rtc_cmos loop
[   11.445854] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.17.0 #1
[   11.445856] Hardware name: HP ProLiant DL360 G7, BIOS P68 08/16/2010
[   11.445859] RIP: 0010:hfsc_dequeue+0x319/0x350 [sch_hfsc]
[   11.445861] RSP: 0018:ffff88081fa03b20 EFLAGS: 00010246
[   11.445863] RAX: 0000000000000000 RBX: ffff8807f5055988 RCX: 0000000000000018
[   11.445864] RDX: 0000000000000002 RSI: 0000000000000000 RDI: ffff8807f5055c80
[   11.445866] RBP: 000000000aa8e4a6 R08: 0000000000000001 R09: 0000000000000000
[   11.445867] R10: ffff8807fbe41868 R11: 0000000000000001 R12: ffff8807f5055800
[   11.445869] R13: ffff8807f5055c80 R14: ffff8807f7f12800 R15: ffff8807f5055800
[   11.445871] FS:  0000000000000000(0000) GS:ffff88081fa00000(0000) knlGS:0000000000000000
[   11.445872] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   11.445874] CR2: 00007fdd5de915f8 CR3: 0000000001a0a000 CR4: 00000000000006b0
[   11.445875] Call Trace:
[   11.445878]  <IRQ>
[   11.445884]  __qdisc_run+0xbe/0x520
[   11.445890]  __dev_queue_xmit+0x387/0x740
[   11.445895]  ? get_nohz_timer_target+0x8c/0x150
[   11.445897]  ? eth_header+0x20/0xb0
[   11.445900]  ? neigh_update+0x34b/0x6b0
[   11.445902]  neigh_update+0x34b/0x6b0
[   11.445907]  arp_process+0x23a/0x780
[   11.445911]  __netif_receive_skb_core+0x338/0xaf0
[   11.445915]  ? __alloc_pages_nodemask+0x99/0x110
[   11.445918]  ? netif_receive_skb_internal+0x2e/0xc0
[   11.445920]  netif_receive_skb_internal+0x2e/0xc0
[   11.445923]  napi_gro_receive+0x70/0x90
[   11.445928]  bnx2_poll_work+0x777/0x1300 [bnx2]
[   11.445932]  ? check_preempt_curr+0x4b/0x80
[   11.445935]  bnx2_poll+0x44/0x210 [bnx2]
[   11.445938]  net_rx_action+0x1ca/0x2b0
[   11.445942]  __do_softirq+0xd4/0x1c9
[   11.445947]  irq_exit+0xa3/0xb0
[   11.445949]  do_IRQ+0x45/0xc0
[   11.445952]  common_interrupt+0xf/0xf
[   11.445953]  </IRQ>
[   11.445958] RIP: 0010:mwait_idle+0x45/0x70
[   11.445959] RSP: 0018:ffffffff81a03ef0 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffdb
[   11.445961] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[   11.445963] RDX: 0000000000000000 RSI: ffff88081fa19060 RDI: ffff88081fa1b5c0
[   11.445964] RBP: ffffffff81a10480 R08: 0000000000000000 R09: 00000002bbeff5c0
[   11.445965] R10: 00000000000002a6 R11: 0000000000000c00 R12: ffffffff81a10480
[   11.445967] R13: ffffffff81c770a0 R14: 0000000000000000 R15: 0000000000000000
[   11.445971]  do_idle+0x86/0xf0
[   11.445974]  cpu_startup_entry+0x14/0x20
[   11.445977]  start_kernel+0x31d/0x325
[   11.445981]  secondary_startup_64+0xa5/0xb0
[   11.445982] Code: 48 89 c5 e9 06 fe ff ff 48 8b b3 88 00 00 00 48 c7 c7 60 70 08 a0 e8 f7 7c 2c e1 31 c0 e9 c0 fe ff ff 48 85 f6 0f 85 75 ff ff ff <0f> 0b 31 f6 e9 70 ff ff ff 48 89 c6 e9 64 ff ff ff 48 85 db 0f 
[   11.446021] ---[ end trace 977c540fdfcce8ae ]---

^ permalink raw reply

* RE: [PATCH] r8169: Reinstate ALDPS and ASPM support
From: Ryankao @ 2018-06-05  6:34 UTC (permalink / raw)
  To: Kai Heng Feng, jrg.otte@gmail.com
  Cc: David Miller, Hayes Wang, hkallweit1@gmail.com,
	romieu@fr.zoreil.com, Linux Netdev List,
	Linux Kernel Mailing List, Hau
In-Reply-To: <9D9E2AF1-CD59-425B-85E9-DB89D620CA67@canonical.com>

Add realtek folk Hau

-----Original Message-----
From: Kai Heng Feng [mailto:kai.heng.feng@canonical.com] 
Sent: Tuesday, June 05, 2018 1:02 PM
To: jrg.otte@gmail.com
Cc: David Miller <davem@davemloft.net>; Hayes Wang <hayeswang@realtek.com>; hkallweit1@gmail.com; romieu@fr.zoreil.com; Linux Netdev List <netdev@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Ryankao <ryankao@realtek.com>
Subject: Re: [PATCH] r8169: Reinstate ALDPS and ASPM support

Hi Jörg Otte,

Can you give this patch a try?

Since you are the only one that reported ALDPS/ASPM regression,

And I think this patch should solve the issue you had [1].

Hopefully we don't need to go down the rabbit hole of blacklist/whitelist...

Kai-Heng

[1] https://lkml.org/lkml/2013/1/5/36

> On Jun 5, 2018, at 12:58 PM, Kai-Heng Feng 
> <kai.heng.feng@canonical.com>
> wrote:
>
> This patch reinstate ALDPS and ASPM support on r8169.
>
> On some Intel platforms, ASPM support on r8169 is the key factor to 
> let Package C-State achieve PC8. Without ASPM support, the deepest 
> Package C-State can hit is PC3. PC8 can save additional ~3W in 
> comparison with PC3.
>
> This patch is from Realtek.
>
> Fixes: e0c075577965 ("r8169: enable ALDPS for power saving")
> Fixes: d64ec841517a ("r8169: enable internal ASPM and clock request
> settings")
>
> Cc: Ryankao <ryankao@realtek.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 190 
> +++++++++++++++++++++------
>  1 file changed, 151 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c
> b/drivers/net/ethernet/realtek/r8169.c
> index 75dfac0248f4..a28ef20be221 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -319,6 +319,8 @@ static const struct pci_device_id 
> rtl8169_pci_tbl[] = {
>
>  MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
>
> +static int enable_aspm = 1;
> +static int enable_aldps = 1;
>  static int use_dac = -1;
>  static struct {
>  	u32 msg_enable;
> @@ -817,6 +819,10 @@ struct rtl8169_private {
>
>  MODULE_AUTHOR("Realtek and the Linux r8169 crew 
> <netdev@vger.kernel.org>");  MODULE_DESCRIPTION("RealTek RTL-8169 
> Gigabit Ethernet driver");
> +module_param(enable_aspm, int, 0);
> +MODULE_PARM_DESC(enable_aspm, "Enable ASPM"); 
> +module_param(enable_aldps, int, 0); MODULE_PARM_DESC(enable_aldps, 
> +"Enable ALDPS");
>  module_param(use_dac, int, 0);
>  MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI 
> slot.");  module_param_named(debug, debug.msg_enable, int, 0); @@ 
> -1567,25 +1573,6 @@ static void rtl_link_chg_patch(struct 
> rtl8169_private *tp)
>  	}
>  }
>
> -static void rtl8169_check_link_status(struct net_device *dev,
> -				      struct rtl8169_private *tp)
> -{
> -	struct device *d = tp_to_dev(tp);
> -
> -	if (tp->link_ok(tp)) {
> -		rtl_link_chg_patch(tp);
> -		/* This is to cancel a scheduled suspend if there's one. */
> -		pm_request_resume(d);
> -		netif_carrier_on(dev);
> -		if (net_ratelimit())
> -			netif_info(tp, ifup, dev, "link up\n");
> -	} else {
> -		netif_carrier_off(dev);
> -		netif_info(tp, ifdown, dev, "link down\n");
> -		pm_runtime_idle(d);
> -	}
> -}
> -
>  #define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | 
> WAKE_MCAST)
>
>  static u32 __rtl8169_get_wol(struct rtl8169_private *tp) @@ -3520,6 
> +3507,15 @@ static void rtl8168e_1_hw_phy_config(struct 
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x0d, 0x4007);
>  	rtl_writephy(tp, 0x0e, 0x0000);
>  	rtl_writephy(tp, 0x0d, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl_rar_exgmac_set(struct rtl8169_private *tp, u8 *addr) 
> @@ -3627,6 +3623,15 @@ static void rtl8168e_2_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Broken BIOS workaround: feed GigaMAC registers with MAC address. */
>  	rtl_rar_exgmac_set(tp, tp->dev->dev_addr);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168f_hw_phy_config(struct rtl8169_private *tp) @@ 
> -3649,6 +3654,15 @@ static void rtl8168f_hw_phy_config(struct 
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x05, 0x8b86);
>  	rtl_w0w1_phy(tp, 0x06, 0x0001, 0x0000);
>  	rtl_writephy(tp, 0x1f, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168f_1_hw_phy_config(struct rtl8169_private *tp) @@ 
> -3865,7 +3879,9 @@ static void rtl8168g_1_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -3874,6 +3890,14 @@ static void rtl8168g_1_hw_phy_config(struct 
> rtl8169_private *tp)  static void rtl8168g_2_hw_phy_config(struct 
> rtl8169_private *tp)  {
>  	rtl_apply_firmware(tp);
> +
> +	rtl_writephy(tp, 0x1f, 0x0a43);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
> +		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168h_1_hw_phy_config(struct rtl8169_private *tp) @@ 
> -3980,7 +4004,9 @@ static void rtl8168h_1_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4053,7 +4079,9 @@ static void rtl8168h_2_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4095,7 +4123,9 @@ static void rtl8168ep_1_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4186,7 +4216,9 @@ static void rtl8168ep_2_hw_phy_config(struct 
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4233,6 +4265,15 @@ static void rtl8105e_hw_phy_config(struct 
> rtl8169_private *tp)
>  	rtl_apply_firmware(tp);
>
>  	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8402_hw_phy_config(struct rtl8169_private *tp) @@ 
> -4250,6 +4291,15 @@ static void rtl8402_hw_phy_config(struct 
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x10, 0x401f);
>  	rtl_writephy(tp, 0x19, 0x7030);
>  	rtl_writephy(tp, 0x1f, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8106e_hw_phy_config(struct rtl8169_private *tp) @@ 
> -4272,6 +4322,15 @@ static void rtl8106e_hw_phy_config(struct 
> rtl8169_private *tp)
>  	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
>
>  	rtl_eri_write(tp, 0x1d0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl_hw_phy_config(struct net_device *dev) @@ -5290,6 
> +5349,18 @@ static void rtl_pcie_state_l2l3_enable(struct 
> rtl8169_private *tp, bool enable)
>  	RTL_W8(tp, Config3, data);
>  }
>
> +static void rtl_hw_internal_aspm_clkreq_enable(struct rtl8169_private *tp,
> +					       bool enable)
> +{
> +	if (enable) {
> +		RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn);
> +		RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en);
> +	} else {
> +		RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> +		RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	}
> +}
> +
>  static void rtl_hw_start_8168bb(struct rtl8169_private *tp)  {
>  	RTL_W8(tp, Config3, RTL_R8(tp, Config3) & ~Beacon_en); @@ -5646,9 
> +5717,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private 
> *tp)
>  	rtl_hw_start_8168g(tp);
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168g_1, ARRAY_SIZE(e_info_8168g_1));
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168g_2(struct rtl8169_private *tp) @@ 
> -5681,9 +5753,10 @@ static void rtl_hw_start_8411_2(struct 
> rtl8169_private *tp)
>  	rtl_hw_start_8168g(tp);
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8411_2, ARRAY_SIZE(e_info_8411_2));
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168h_1(struct rtl8169_private *tp) @@ 
> -5700,8 +5773,7 @@ static void rtl_hw_start_8168h_1(struct 
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
>
>  	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO); @@ 
> -5780,6 +5852,9 @@ static void rtl_hw_start_8168h_1(struct 
> rtl8169_private *tp)
>  	r8168_mac_ocp_write(tp, 0xe63e, 0x0000);
>  	r8168_mac_ocp_write(tp, 0xc094, 0x0000);
>  	r8168_mac_ocp_write(tp, 0xc09e, 0x0000);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep(struct rtl8169_private *tp) @@ 
> -5831,11 +5906,13 @@ static void rtl_hw_start_8168ep_1(struct 
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_1, ARRAY_SIZE(e_info_8168ep_1));
>
>  	rtl_hw_start_8168ep(tp);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp) @@ 
> -5847,14 +5924,16 @@ static void rtl_hw_start_8168ep_2(struct 
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_2, ARRAY_SIZE(e_info_8168ep_2));
>
>  	rtl_hw_start_8168ep(tp);
>
>  	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
>  	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp) @@ 
> -5868,8 +5947,7 @@ static void rtl_hw_start_8168ep_3(struct 
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_3, ARRAY_SIZE(e_info_8168ep_3));
>
>  	rtl_hw_start_8168ep(tp);
> @@ -5889,6 +5967,9 @@ static void rtl_hw_start_8168ep_3(struct 
> rtl8169_private *tp)
>  	data = r8168_mac_ocp_read(tp, 0xe860);
>  	data |= 0x0080;
>  	r8168_mac_ocp_write(tp, 0xe860, data);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168(struct rtl8169_private *tp) @@ -6364,7 
> +6445,7 @@ static void rtl8169_tx_clear(struct rtl8169_private
> *tp)
>  	tp->cur_tx = tp->dirty_tx = 0;
>  }
>
> -static void rtl_reset_work(struct rtl8169_private *tp)
> +static void _rtl_reset_work(struct rtl8169_private *tp)
>  {
>  	struct net_device *dev = tp->dev;
>  	int i;
> @@ -6384,6 +6465,33 @@ static void rtl_reset_work(struct 
> rtl8169_private
> *tp)
>  	napi_enable(&tp->napi);
>  	rtl_hw_start(tp);
>  	netif_wake_queue(dev);
> +}
> +
> +static void rtl8169_check_link_status(struct net_device *dev,
> +				      struct rtl8169_private *tp) {
> +	struct device *d = tp_to_dev(tp);
> +
> +	if (tp->link_ok(tp)) {
> +		rtl_link_chg_patch(tp);
> +		/* This is to cancel a scheduled suspend if there's one. */
> +		if (pm_request_resume(d))
> +			_rtl_reset_work(tp);
> +		netif_carrier_on(dev);
> +		if (net_ratelimit())
> +			netif_info(tp, ifup, dev, "link up\n");
> +	} else {
> +		netif_carrier_off(dev);
> +		netif_info(tp, ifdown, dev, "link down\n");
> +		pm_runtime_idle(d);
> +	}
> +}
> +
> +static void rtl_reset_work(struct rtl8169_private *tp) {
> +	struct net_device *dev = tp->dev;
> +
> +	_rtl_reset_work(tp);
>  	rtl8169_check_link_status(dev, tp);
>  }
>
> @@ -7649,8 +7757,12 @@ static int rtl_init_one(struct pci_dev *pdev, 
> const struct pci_device_id *ent)
>
>  	/* disable ASPM completely as that cause random device stop working
>  	 * problems as well as full system hangs for some PCIe devices users */
> -	pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |
> -				     PCIE_LINK_STATE_CLKPM);
> +	if (!enable_aspm) {
> +		pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S |
> +					     PCIE_LINK_STATE_L1 |
> +					     PCIE_LINK_STATE_CLKPM);
> +		netif_info(tp, probe, dev, "ASPM disabled\n");
> +	}
>
>  	/* enable device (incl. PCI PM wakeup and hotplug setup) */
>  	rc = pcim_enable_device(pdev);
> --
> 2.17.0

------Please consider the environment before printing this e-mail.

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO
From: Heiner Kallweit @ 2018-06-05  6:08 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20180604214829.GA14873@lunn.ch>

On 04.06.2018 23:48, Andrew Lunn wrote:
> On Sat, Jun 02, 2018 at 10:33:36PM +0200, Heiner Kallweit wrote:
>> Current implementation of MDIO bus PM ops doesn't actually implement
>> bus-specific PM ops but just calls PM ops defined on a device level
>> what doesn't seem to be fully in line with the core PM model.
>>
>> When looking e.g. at __device_suspend() the PM core looks for PM ops
>> of a device in a specific order:
>> 1. device PM domain
>> 2. device type
>> 3. device class
>> 4. device bus
>>
>> I think it has good reason that there's no PM ops on device level.
>> The situation can be improved by modeling PHY's as device type of
>> a MDIO device. If for some other type of MDIO device PM ops are
>> needed, it could be modeled as struct device_type as well.
> 
> Hi Heiner
> 
> I tested that the files in /sys/class/bus/mdio/devices/* are still
> there. And also not there for MDIO devices which are not PHYs,
> e.g. Ethernet switches.
> 
> I don't have any boards which do PM. So i cannot test suspend/resume.
> 
Thanks for reviewing! I tested suspend / resume by manually suspending
via "systemctl suspend" and resuming via power button or WoL.

The only behavior change I expect is an additional parameter in the PHY
udev calls, see following comment in device.h: If "name" is specified,
the uevent will contain it in the DEVTYPE variable.
This however shouldn't have any impact.

Heiner

> I also took a look at drivers/net/dsa/qca8k.c. This is an MDIO switch
> which has PM operations. I don't think this change will break it.
> 
> I would prefer a bit more testing, but i guess that is what -rc
> kernels are for.
> 
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew
> .
> 

^ permalink raw reply

* Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures
From: Ido Schimmel @ 2018-06-05  5:56 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, idosch, jiri, jakub.kicinski, David Ahern
In-Reply-To: <20180604221503.20329-4-dsahern@kernel.org>

On Mon, Jun 04, 2018 at 03:15:03PM -0700, dsahern@kernel.org wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Return messages in extack for port split/unsplit errors. e.g.,
>     $ devlink port split swp1s1 count 4
>     Error: mlxsw_spectrum: Port cannot be split further.
>     devlink answers: Invalid argument
> 
>     $ devlink port unsplit swp4
>     Error: mlxsw_spectrum: Port was not split.
>     devlink answers: Invalid argument
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

Thanks!

^ permalink raw reply

* Re: [PATCH net-next] net: metrics: add proper netlink validation
From: Eric Dumazet @ 2018-06-05  5:48 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller; +Cc: netdev, Eric Dumazet, David Ahern
In-Reply-To: <20180604234601.261823-1-edumazet@google.com>



On 06/04/2018 04:46 PM, Eric Dumazet wrote:
> Before using nla_get_u32(), better make sure the attribute
> is of the proper size.
> 
> 

> Fixes: a919525ad832 ("net: Move fib_convert_metrics to metrics file")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv4/fib_semantics.c | 2 ++
>  net/ipv4/metrics.c       | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 6608db23f54b6afdac0455650b47d64b1b22b255..9a890be8a0265edb78da225a82e2cac120f2150f 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -717,6 +717,8 @@ bool fib_metrics_match(struct fib_config *cfg, struct fib_info *fi)
>  			nla_strlcpy(tmp, nla, sizeof(tmp));
>  			val = tcp_ca_get_key_by_name(fi->fib_net, tmp, &ecn_ca);
>  		} else {
> +			if (nla_len(nla) != sizeof(u32)

Oh well, stupid typo.

> +				return false;
>  			val = nla_get_u32(nla);

I will send a V2.

^ permalink raw reply

* Re: [PATCH] r8169: Reinstate ALDPS and ASPM support
From: Kai Heng Feng @ 2018-06-05  5:02 UTC (permalink / raw)
  To: jrg.otte
  Cc: David Miller, Hayes Wang, hkallweit1, romieu, Linux Netdev List,
	Linux Kernel Mailing List, Ryankao
In-Reply-To: <20180605045812.17977-1-kai.heng.feng@canonical.com>

Hi Jörg Otte,

Can you give this patch a try?

Since you are the only one that reported ALDPS/ASPM regression,

And I think this patch should solve the issue you had [1].

Hopefully we don't need to go down the rabbit hole of blacklist/whitelist...

Kai-Heng

[1] https://lkml.org/lkml/2013/1/5/36

> On Jun 5, 2018, at 12:58 PM, Kai-Heng Feng <kai.heng.feng@canonical.com>  
> wrote:
>
> This patch reinstate ALDPS and ASPM support on r8169.
>
> On some Intel platforms, ASPM support on r8169 is the key factor to let
> Package C-State achieve PC8. Without ASPM support, the deepest Package
> C-State can hit is PC3. PC8 can save additional ~3W in comparison with
> PC3.
>
> This patch is from Realtek.
>
> Fixes: e0c075577965 ("r8169: enable ALDPS for power saving")
> Fixes: d64ec841517a ("r8169: enable internal ASPM and clock request  
> settings")
>
> Cc: Ryankao <ryankao@realtek.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 190 +++++++++++++++++++++------
>  1 file changed, 151 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c  
> b/drivers/net/ethernet/realtek/r8169.c
> index 75dfac0248f4..a28ef20be221 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -319,6 +319,8 @@ static const struct pci_device_id rtl8169_pci_tbl[] = {
>
>  MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
>
> +static int enable_aspm = 1;
> +static int enable_aldps = 1;
>  static int use_dac = -1;
>  static struct {
>  	u32 msg_enable;
> @@ -817,6 +819,10 @@ struct rtl8169_private {
>
>  MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
>  MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
> +module_param(enable_aspm, int, 0);
> +MODULE_PARM_DESC(enable_aspm, "Enable ASPM");
> +module_param(enable_aldps, int, 0);
> +MODULE_PARM_DESC(enable_aldps, "Enable ALDPS");
>  module_param(use_dac, int, 0);
>  MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
>  module_param_named(debug, debug.msg_enable, int, 0);
> @@ -1567,25 +1573,6 @@ static void rtl_link_chg_patch(struct  
> rtl8169_private *tp)
>  	}
>  }
>
> -static void rtl8169_check_link_status(struct net_device *dev,
> -				      struct rtl8169_private *tp)
> -{
> -	struct device *d = tp_to_dev(tp);
> -
> -	if (tp->link_ok(tp)) {
> -		rtl_link_chg_patch(tp);
> -		/* This is to cancel a scheduled suspend if there's one. */
> -		pm_request_resume(d);
> -		netif_carrier_on(dev);
> -		if (net_ratelimit())
> -			netif_info(tp, ifup, dev, "link up\n");
> -	} else {
> -		netif_carrier_off(dev);
> -		netif_info(tp, ifdown, dev, "link down\n");
> -		pm_runtime_idle(d);
> -	}
> -}
> -
>  #define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
>
>  static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
> @@ -3520,6 +3507,15 @@ static void rtl8168e_1_hw_phy_config(struct  
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x0d, 0x4007);
>  	rtl_writephy(tp, 0x0e, 0x0000);
>  	rtl_writephy(tp, 0x0d, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl_rar_exgmac_set(struct rtl8169_private *tp, u8 *addr)
> @@ -3627,6 +3623,15 @@ static void rtl8168e_2_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Broken BIOS workaround: feed GigaMAC registers with MAC address. */
>  	rtl_rar_exgmac_set(tp, tp->dev->dev_addr);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168f_hw_phy_config(struct rtl8169_private *tp)
> @@ -3649,6 +3654,15 @@ static void rtl8168f_hw_phy_config(struct  
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x05, 0x8b86);
>  	rtl_w0w1_phy(tp, 0x06, 0x0001, 0x0000);
>  	rtl_writephy(tp, 0x1f, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x15) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168f_1_hw_phy_config(struct rtl8169_private *tp)
> @@ -3865,7 +3879,9 @@ static void rtl8168g_1_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -3874,6 +3890,14 @@ static void rtl8168g_1_hw_phy_config(struct  
> rtl8169_private *tp)
>  static void rtl8168g_2_hw_phy_config(struct rtl8169_private *tp)
>  {
>  	rtl_apply_firmware(tp);
> +
> +	rtl_writephy(tp, 0x1f, 0x0a43);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
> +		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8168h_1_hw_phy_config(struct rtl8169_private *tp)
> @@ -3980,7 +4004,9 @@ static void rtl8168h_1_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4053,7 +4079,9 @@ static void rtl8168h_2_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4095,7 +4123,9 @@ static void rtl8168ep_1_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4186,7 +4216,9 @@ static void rtl8168ep_2_hw_phy_config(struct  
> rtl8169_private *tp)
>
>  	/* Check ALDPS bit, disable it if enabled */
>  	rtl_writephy(tp, 0x1f, 0x0a43);
> -	if (rtl_readphy(tp, 0x10) & 0x0004)
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
> +	else if (rtl_readphy(tp, 0x10) & 0x0004)
>  		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
>
>  	rtl_writephy(tp, 0x1f, 0x0000);
> @@ -4233,6 +4265,15 @@ static void rtl8105e_hw_phy_config(struct  
> rtl8169_private *tp)
>  	rtl_apply_firmware(tp);
>
>  	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8402_hw_phy_config(struct rtl8169_private *tp)
> @@ -4250,6 +4291,15 @@ static void rtl8402_hw_phy_config(struct  
> rtl8169_private *tp)
>  	rtl_writephy(tp, 0x10, 0x401f);
>  	rtl_writephy(tp, 0x19, 0x7030);
>  	rtl_writephy(tp, 0x1f, 0x0000);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl8106e_hw_phy_config(struct rtl8169_private *tp)
> @@ -4272,6 +4322,15 @@ static void rtl8106e_hw_phy_config(struct  
> rtl8169_private *tp)
>  	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
>
>  	rtl_eri_write(tp, 0x1d0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
> +
> +	/* Check ALDPS bit, disable it if enabled */
> +	rtl_writephy(tp, 0x1f, 0x0000);
> +	if (enable_aldps)
> +		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
> +	else if (rtl_readphy(tp, 0x18) & 0x1000)
> +		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
> +
> +	rtl_writephy(tp, 0x1f, 0x0000);
>  }
>
>  static void rtl_hw_phy_config(struct net_device *dev)
> @@ -5290,6 +5349,18 @@ static void rtl_pcie_state_l2l3_enable(struct  
> rtl8169_private *tp, bool enable)
>  	RTL_W8(tp, Config3, data);
>  }
>
> +static void rtl_hw_internal_aspm_clkreq_enable(struct rtl8169_private *tp,
> +					       bool enable)
> +{
> +	if (enable) {
> +		RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn);
> +		RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en);
> +	} else {
> +		RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> +		RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	}
> +}
> +
>  static void rtl_hw_start_8168bb(struct rtl8169_private *tp)
>  {
>  	RTL_W8(tp, Config3, RTL_R8(tp, Config3) & ~Beacon_en);
> @@ -5646,9 +5717,10 @@ static void rtl_hw_start_8168g_1(struct  
> rtl8169_private *tp)
>  	rtl_hw_start_8168g(tp);
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168g_1, ARRAY_SIZE(e_info_8168g_1));
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
> @@ -5681,9 +5753,10 @@ static void rtl_hw_start_8411_2(struct  
> rtl8169_private *tp)
>  	rtl_hw_start_8168g(tp);
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8411_2, ARRAY_SIZE(e_info_8411_2));
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
> @@ -5700,8 +5773,7 @@ static void rtl_hw_start_8168h_1(struct  
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
>
>  	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
> @@ -5780,6 +5852,9 @@ static void rtl_hw_start_8168h_1(struct  
> rtl8169_private *tp)
>  	r8168_mac_ocp_write(tp, 0xe63e, 0x0000);
>  	r8168_mac_ocp_write(tp, 0xc094, 0x0000);
>  	r8168_mac_ocp_write(tp, 0xc09e, 0x0000);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
> @@ -5831,11 +5906,13 @@ static void rtl_hw_start_8168ep_1(struct  
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_1, ARRAY_SIZE(e_info_8168ep_1));
>
>  	rtl_hw_start_8168ep(tp);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp)
> @@ -5847,14 +5924,16 @@ static void rtl_hw_start_8168ep_2(struct  
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_2, ARRAY_SIZE(e_info_8168ep_2));
>
>  	rtl_hw_start_8168ep(tp);
>
>  	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
>  	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
> @@ -5868,8 +5947,7 @@ static void rtl_hw_start_8168ep_3(struct  
> rtl8169_private *tp)
>  	};
>
>  	/* disable aspm and clock request before access ephy */
> -	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
> -	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
> +	rtl_hw_internal_aspm_clkreq_enable(tp, false);
>  	rtl_ephy_init(tp, e_info_8168ep_3, ARRAY_SIZE(e_info_8168ep_3));
>
>  	rtl_hw_start_8168ep(tp);
> @@ -5889,6 +5967,9 @@ static void rtl_hw_start_8168ep_3(struct  
> rtl8169_private *tp)
>  	data = r8168_mac_ocp_read(tp, 0xe860);
>  	data |= 0x0080;
>  	r8168_mac_ocp_write(tp, 0xe860, data);
> +
> +	if (enable_aspm)
> +		rtl_hw_internal_aspm_clkreq_enable(tp, true);
>  }
>
>  static void rtl_hw_start_8168(struct rtl8169_private *tp)
> @@ -6364,7 +6445,7 @@ static void rtl8169_tx_clear(struct rtl8169_private  
> *tp)
>  	tp->cur_tx = tp->dirty_tx = 0;
>  }
>
> -static void rtl_reset_work(struct rtl8169_private *tp)
> +static void _rtl_reset_work(struct rtl8169_private *tp)
>  {
>  	struct net_device *dev = tp->dev;
>  	int i;
> @@ -6384,6 +6465,33 @@ static void rtl_reset_work(struct rtl8169_private  
> *tp)
>  	napi_enable(&tp->napi);
>  	rtl_hw_start(tp);
>  	netif_wake_queue(dev);
> +}
> +
> +static void rtl8169_check_link_status(struct net_device *dev,
> +				      struct rtl8169_private *tp)
> +{
> +	struct device *d = tp_to_dev(tp);
> +
> +	if (tp->link_ok(tp)) {
> +		rtl_link_chg_patch(tp);
> +		/* This is to cancel a scheduled suspend if there's one. */
> +		if (pm_request_resume(d))
> +			_rtl_reset_work(tp);
> +		netif_carrier_on(dev);
> +		if (net_ratelimit())
> +			netif_info(tp, ifup, dev, "link up\n");
> +	} else {
> +		netif_carrier_off(dev);
> +		netif_info(tp, ifdown, dev, "link down\n");
> +		pm_runtime_idle(d);
> +	}
> +}
> +
> +static void rtl_reset_work(struct rtl8169_private *tp)
> +{
> +	struct net_device *dev = tp->dev;
> +
> +	_rtl_reset_work(tp);
>  	rtl8169_check_link_status(dev, tp);
>  }
>
> @@ -7649,8 +7757,12 @@ static int rtl_init_one(struct pci_dev *pdev,  
> const struct pci_device_id *ent)
>
>  	/* disable ASPM completely as that cause random device stop working
>  	 * problems as well as full system hangs for some PCIe devices users */
> -	pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |
> -				     PCIE_LINK_STATE_CLKPM);
> +	if (!enable_aspm) {
> +		pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S |
> +					     PCIE_LINK_STATE_L1 |
> +					     PCIE_LINK_STATE_CLKPM);
> +		netif_info(tp, probe, dev, "ASPM disabled\n");
> +	}
>
>  	/* enable device (incl. PCI PM wakeup and hotplug setup) */
>  	rc = pcim_enable_device(pdev);
> -- 
> 2.17.0

^ permalink raw reply

* [PATCH] r8169: Reinstate ALDPS and ASPM support
From: Kai-Heng Feng @ 2018-06-05  4:58 UTC (permalink / raw)
  To: davem
  Cc: hayeswang, hkallweit1, romieu, netdev, linux-kernel,
	Kai-Heng Feng, Ryankao

This patch reinstate ALDPS and ASPM support on r8169.

On some Intel platforms, ASPM support on r8169 is the key factor to let
Package C-State achieve PC8. Without ASPM support, the deepest Package
C-State can hit is PC3. PC8 can save additional ~3W in comparison with
PC3.

This patch is from Realtek.

Fixes: e0c075577965 ("r8169: enable ALDPS for power saving")
Fixes: d64ec841517a ("r8169: enable internal ASPM and clock request settings")

Cc: Ryankao <ryankao@realtek.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/realtek/r8169.c | 190 +++++++++++++++++++++------
 1 file changed, 151 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 75dfac0248f4..a28ef20be221 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -319,6 +319,8 @@ static const struct pci_device_id rtl8169_pci_tbl[] = {
 
 MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
 
+static int enable_aspm = 1;
+static int enable_aldps = 1;
 static int use_dac = -1;
 static struct {
 	u32 msg_enable;
@@ -817,6 +819,10 @@ struct rtl8169_private {
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
 MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
+module_param(enable_aspm, int, 0);
+MODULE_PARM_DESC(enable_aspm, "Enable ASPM");
+module_param(enable_aldps, int, 0);
+MODULE_PARM_DESC(enable_aldps, "Enable ALDPS");
 module_param(use_dac, int, 0);
 MODULE_PARM_DESC(use_dac, "Enable PCI DAC. Unsafe on 32 bit PCI slot.");
 module_param_named(debug, debug.msg_enable, int, 0);
@@ -1567,25 +1573,6 @@ static void rtl_link_chg_patch(struct rtl8169_private *tp)
 	}
 }
 
-static void rtl8169_check_link_status(struct net_device *dev,
-				      struct rtl8169_private *tp)
-{
-	struct device *d = tp_to_dev(tp);
-
-	if (tp->link_ok(tp)) {
-		rtl_link_chg_patch(tp);
-		/* This is to cancel a scheduled suspend if there's one. */
-		pm_request_resume(d);
-		netif_carrier_on(dev);
-		if (net_ratelimit())
-			netif_info(tp, ifup, dev, "link up\n");
-	} else {
-		netif_carrier_off(dev);
-		netif_info(tp, ifdown, dev, "link down\n");
-		pm_runtime_idle(d);
-	}
-}
-
 #define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
 
 static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
@@ -3520,6 +3507,15 @@ static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x0d, 0x4007);
 	rtl_writephy(tp, 0x0e, 0x0000);
 	rtl_writephy(tp, 0x0d, 0x0000);
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x15) & 0x1000)
+		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl_rar_exgmac_set(struct rtl8169_private *tp, u8 *addr)
@@ -3627,6 +3623,15 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Broken BIOS workaround: feed GigaMAC registers with MAC address. */
 	rtl_rar_exgmac_set(tp, tp->dev->dev_addr);
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x15) & 0x1000)
+		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl8168f_hw_phy_config(struct rtl8169_private *tp)
@@ -3649,6 +3654,15 @@ static void rtl8168f_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x05, 0x8b86);
 	rtl_w0w1_phy(tp, 0x06, 0x0001, 0x0000);
 	rtl_writephy(tp, 0x1f, 0x0000);
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x15, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x15) & 0x1000)
+		rtl_w0w1_phy(tp, 0x15, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl8168f_1_hw_phy_config(struct rtl8169_private *tp)
@@ -3865,7 +3879,9 @@ static void rtl8168g_1_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Check ALDPS bit, disable it if enabled */
 	rtl_writephy(tp, 0x1f, 0x0a43);
-	if (rtl_readphy(tp, 0x10) & 0x0004)
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
 		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -3874,6 +3890,14 @@ static void rtl8168g_1_hw_phy_config(struct rtl8169_private *tp)
 static void rtl8168g_2_hw_phy_config(struct rtl8169_private *tp)
 {
 	rtl_apply_firmware(tp);
+
+	rtl_writephy(tp, 0x1f, 0x0a43);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
+		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl8168h_1_hw_phy_config(struct rtl8169_private *tp)
@@ -3980,7 +4004,9 @@ static void rtl8168h_1_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Check ALDPS bit, disable it if enabled */
 	rtl_writephy(tp, 0x1f, 0x0a43);
-	if (rtl_readphy(tp, 0x10) & 0x0004)
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
 		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -4053,7 +4079,9 @@ static void rtl8168h_2_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Check ALDPS bit, disable it if enabled */
 	rtl_writephy(tp, 0x1f, 0x0a43);
-	if (rtl_readphy(tp, 0x10) & 0x0004)
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
 		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -4095,7 +4123,9 @@ static void rtl8168ep_1_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Check ALDPS bit, disable it if enabled */
 	rtl_writephy(tp, 0x1f, 0x0a43);
-	if (rtl_readphy(tp, 0x10) & 0x0004)
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
 		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -4186,7 +4216,9 @@ static void rtl8168ep_2_hw_phy_config(struct rtl8169_private *tp)
 
 	/* Check ALDPS bit, disable it if enabled */
 	rtl_writephy(tp, 0x1f, 0x0a43);
-	if (rtl_readphy(tp, 0x10) & 0x0004)
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x10, 0x0004, 0x0000);
+	else if (rtl_readphy(tp, 0x10) & 0x0004)
 		rtl_w0w1_phy(tp, 0x10, 0x0000, 0x0004);
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -4233,6 +4265,15 @@ static void rtl8105e_hw_phy_config(struct rtl8169_private *tp)
 	rtl_apply_firmware(tp);
 
 	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x18) & 0x1000)
+		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl8402_hw_phy_config(struct rtl8169_private *tp)
@@ -4250,6 +4291,15 @@ static void rtl8402_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x10, 0x401f);
 	rtl_writephy(tp, 0x19, 0x7030);
 	rtl_writephy(tp, 0x1f, 0x0000);
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x18) & 0x1000)
+		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl8106e_hw_phy_config(struct rtl8169_private *tp)
@@ -4272,6 +4322,15 @@ static void rtl8106e_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
 
 	rtl_eri_write(tp, 0x1d0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
+
+	/* Check ALDPS bit, disable it if enabled */
+	rtl_writephy(tp, 0x1f, 0x0000);
+	if (enable_aldps)
+		rtl_w0w1_phy(tp, 0x18, 0x1000, 0x0000);
+	else if (rtl_readphy(tp, 0x18) & 0x1000)
+		rtl_w0w1_phy(tp, 0x18, 0x0000, 0x1000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
 }
 
 static void rtl_hw_phy_config(struct net_device *dev)
@@ -5290,6 +5349,18 @@ static void rtl_pcie_state_l2l3_enable(struct rtl8169_private *tp, bool enable)
 	RTL_W8(tp, Config3, data);
 }
 
+static void rtl_hw_internal_aspm_clkreq_enable(struct rtl8169_private *tp,
+					       bool enable)
+{
+	if (enable) {
+		RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn);
+		RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en);
+	} else {
+		RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
+		RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	}
+}
+
 static void rtl_hw_start_8168bb(struct rtl8169_private *tp)
 {
 	RTL_W8(tp, Config3, RTL_R8(tp, Config3) & ~Beacon_en);
@@ -5646,9 +5717,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
 	rtl_hw_start_8168g(tp);
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168g_1, ARRAY_SIZE(e_info_8168g_1));
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
@@ -5681,9 +5753,10 @@ static void rtl_hw_start_8411_2(struct rtl8169_private *tp)
 	rtl_hw_start_8168g(tp);
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8411_2, ARRAY_SIZE(e_info_8411_2));
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
@@ -5700,8 +5773,7 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 	};
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168h_1, ARRAY_SIZE(e_info_8168h_1));
 
 	RTL_W32(tp, TxConfig, RTL_R32(tp, TxConfig) | TXCFG_AUTO_FIFO);
@@ -5780,6 +5852,9 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 	r8168_mac_ocp_write(tp, 0xe63e, 0x0000);
 	r8168_mac_ocp_write(tp, 0xc094, 0x0000);
 	r8168_mac_ocp_write(tp, 0xc09e, 0x0000);
+
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
@@ -5831,11 +5906,13 @@ static void rtl_hw_start_8168ep_1(struct rtl8169_private *tp)
 	};
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168ep_1, ARRAY_SIZE(e_info_8168ep_1));
 
 	rtl_hw_start_8168ep(tp);
+
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp)
@@ -5847,14 +5924,16 @@ static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp)
 	};
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168ep_2, ARRAY_SIZE(e_info_8168ep_2));
 
 	rtl_hw_start_8168ep(tp);
 
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
 	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
+
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
@@ -5868,8 +5947,7 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 	};
 
 	/* disable aspm and clock request before access ephy */
-	RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+	rtl_hw_internal_aspm_clkreq_enable(tp, false);
 	rtl_ephy_init(tp, e_info_8168ep_3, ARRAY_SIZE(e_info_8168ep_3));
 
 	rtl_hw_start_8168ep(tp);
@@ -5889,6 +5967,9 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 	data = r8168_mac_ocp_read(tp, 0xe860);
 	data |= 0x0080;
 	r8168_mac_ocp_write(tp, 0xe860, data);
+
+	if (enable_aspm)
+		rtl_hw_internal_aspm_clkreq_enable(tp, true);
 }
 
 static void rtl_hw_start_8168(struct rtl8169_private *tp)
@@ -6364,7 +6445,7 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 	tp->cur_tx = tp->dirty_tx = 0;
 }
 
-static void rtl_reset_work(struct rtl8169_private *tp)
+static void _rtl_reset_work(struct rtl8169_private *tp)
 {
 	struct net_device *dev = tp->dev;
 	int i;
@@ -6384,6 +6465,33 @@ static void rtl_reset_work(struct rtl8169_private *tp)
 	napi_enable(&tp->napi);
 	rtl_hw_start(tp);
 	netif_wake_queue(dev);
+}
+
+static void rtl8169_check_link_status(struct net_device *dev,
+				      struct rtl8169_private *tp)
+{
+	struct device *d = tp_to_dev(tp);
+
+	if (tp->link_ok(tp)) {
+		rtl_link_chg_patch(tp);
+		/* This is to cancel a scheduled suspend if there's one. */
+		if (pm_request_resume(d))
+			_rtl_reset_work(tp);
+		netif_carrier_on(dev);
+		if (net_ratelimit())
+			netif_info(tp, ifup, dev, "link up\n");
+	} else {
+		netif_carrier_off(dev);
+		netif_info(tp, ifdown, dev, "link down\n");
+		pm_runtime_idle(d);
+	}
+}
+
+static void rtl_reset_work(struct rtl8169_private *tp)
+{
+	struct net_device *dev = tp->dev;
+
+	_rtl_reset_work(tp);
 	rtl8169_check_link_status(dev, tp);
 }
 
@@ -7649,8 +7757,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	/* disable ASPM completely as that cause random device stop working
 	 * problems as well as full system hangs for some PCIe devices users */
-	pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |
-				     PCIE_LINK_STATE_CLKPM);
+	if (!enable_aspm) {
+		pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S |
+					     PCIE_LINK_STATE_L1 |
+					     PCIE_LINK_STATE_CLKPM);
+		netif_info(tp, probe, dev, "ASPM disabled\n");
+	}
 
 	/* enable device (incl. PCI PM wakeup and hotplug setup) */
 	rc = pcim_enable_device(pdev);
-- 
2.17.0

^ permalink raw reply related

* general protection fault in sockfs_setattr
From: shankarapailoor @ 2018-06-05  4:53 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, syzkaller, netdev

Hi,

I have been fuzzing Linux 4.17-rc7 with Syzkaller and found the
following crash: https://pastebin.com/ixX3RB9j

Syzkaller isolated the cause of the bug to the following program:

socketpair$unix(0x1, 0x1, 0x0,
&(0x7f0000000000)={<r0=>0xffffffffffffffff, <r1=>0xffffffffffffffff})
getresuid(&(0x7f0000000080)=<r2=>0x0, &(0x7f00000000c0),
&(0x7f0000000700))r3 = getegid()
fchownat(r0, &(0x7f0000000040)='\x00', r2, r3, 0x1000)
dup3(r1, r0, 0x80000)


The problematic area appears to be here:

static int sockfs_setattr(struct dentry *dentry, struct iattr *iattr)
{
    int err = simple_setattr(dentry, iattr);

    if (!err && (iattr->ia_valid & ATTR_UID)) {
         struct socket *sock = SOCKET_I(d_inode(dentry));

         sock->sk->sk_uid = iattr->ia_uid; //KASAN GPF
    }
    return err;
}

If dup3 is called concurrently with fchownat then can sock->sk be NULL?

-- 
Regards,
Shankara Pailoor

^ permalink raw reply

* Re: [RFC PATCH 0/2] net: macb: Disable TX checksum offloading on all Zynq
From: Harini Katakam @ 2018-06-05  4:51 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jennifer Dahm, netdev, David S . Miller, Nathan Sullivan,
	Rafal Ozieblo, Claudiu Beznea, Harini Katakam
In-Reply-To: <d3fe52fa-ae42-f818-4494-8352323cc489@microchip.com>

Hi Jeniffer,

On Mon, Jun 4, 2018 at 8:35 PM, Nicolas Ferre
<nicolas.ferre@microchip.com> wrote:
> Jennifer,
>
> On 25/05/2018 at 23:44, Jennifer Dahm wrote:
>>
>> During testing, I discovered that the Zynq GEM hardware overwrites all
>> outgoing UDP packet checksums, which is illegal in packet forwarding
>> cases. This happens both with and without the checksum-zeroing
>> behavior  introduced  in  007e4ba3ee137f4700f39aa6dbaf01a71047c5f6
>> ("net: macb: initialize checksum when using checksum offloading"). The
>> only solution to both the small packet bug and the packet forwarding
>> bug that I can find is to disable TX checksum offloading entirely.
>
>

Thanks for the extensive testing.
I'll try to reproduce and see if it is something to be fixed in the driver.

> Are the bugs listed above present in all revisions of the GEM IP, only for
> some revisions?
> Is there an errata that describe this issue for the Zynq GEM?

@Nicolas, AFAIK, there is no errata for this in either Cadence or
Zynq documentation.

Regards,
Harini

^ permalink raw reply

* Re: [PATCH net-next] net/mlx5e: Make function mlx5e_change_rep_mtu() static
From: Leon Romanovsky @ 2018-06-05  4:20 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Saeed Mahameed, Adi Nissim, netdev, linux-rdma, kernel-janitors
In-Reply-To: <1528166565-54828-1-git-send-email-weiyongjun1@huawei.com>

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

On Tue, Jun 05, 2018 at 02:42:45AM +0000, Wei Yongjun wrote:
> Fixes the following sparse warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:903:5: warning:
>  symbol 'mlx5e_change_rep_mtu' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH net] sctp: not allow transport timeout value less than HZ/5 for hb_timer
From: Xin Long @ 2018-06-05  4:16 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Eric Dumazet, Marcelo Ricardo Leitner, Neil Horman,
	Dmitry Vyukov, syzkaller

syzbot reported a rcu_sched self-detected stall on CPU which is caused
by too small value set on rto_min with SCTP_RTOINFO sockopt. With this
value, hb_timer will get stuck there, as in its timer handler it starts
this timer again with this value, then goes to the timer handler again.

This problem is there since very beginning, and thanks to Eric for the
reproducer shared from a syzbot mail.

This patch fixes it by not allowing sctp_transport_timeout to return a
smaller value than HZ/5 for hb_timer, which is based on TCP's min rto.

Note that it doesn't fix this issue by limiting rto_min, as some users
are still using small rto and no proper value was found for it yet.

Reported-by: syzbot+3dcd59a1f907245f891f@syzkaller.appspotmail.com
Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 47f82bd..03fc2c4 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -634,7 +634,7 @@ unsigned long sctp_transport_timeout(struct sctp_transport *trans)
 	    trans->state != SCTP_PF)
 		timeout += trans->hbinterval;
 
-	return timeout;
+	return max_t(unsigned long, timeout, HZ / 5);
 }
 
 /* Reset transport variables to their initial values */
-- 
2.1.0

^ permalink raw reply related

* [PATCH net] failover: eliminate callback hell
From: Stephen Hemminger @ 2018-06-05  3:42 UTC (permalink / raw)
  To: kys, haiyangz, davem, mst, sridhar.samudrala; +Cc: netdev, Stephen Hemminger

The net failover should be a simple library, not a virtual
object with function callbacks (see callback hell).
The code is simpler is smaller both for the netvsc and virtio use case.

The code is restructured in many ways. I should have given these
as review comments to net_failover during review
but did not want to overwhelm the original submitter.
Therefore it was merged prematurely.

Some of the many items changed are:

  * The support routines should just be selected as needed in
    kernel config, no need for them to be visible config items.

  * Both netvsc and net_failover should keep their list of their
    own devices. Not a common list.

  * The matching of secondary device to primary device policy
    is up to the network device. Both net_failover and netvsc
    will use MAC for now but can change separately.

  * The match policy is only used during initial discovery; after
    that the secondary device knows what the upper device is because
    of the parent/child relationship; no searching is required.

  * Now, netvsc and net_failover use the same delayed work type
    mechanism for setup. Previously, net_failover code was triggering off
    name change but a similar policy was rejected for netvsc.
    "what is good for the goose is good for the gander"

  * The net_failover private device info 'struct net_failover_info'
    should have been private to the driver file, not a visible
    API.

  * The net_failover device should use SET_NETDEV_DEV
    that is intended only for physical devices not virtual devices.

  * No point in having DocBook style comments on a driver file.
    They only make sense on an external exposed API.

  * net_failover only supports Ethernet, so use ether_addr_copy.

  * Set permanent and current address of net_failover device
    to match the primary.

  * Carrier should be marked off before registering device
    the net_failover device.

  * Use netdev_XXX for log messages, in net_failover (not dev_xxx)

  * Since failover infrastructure is about linking devices just
    use RTNL no need for other locking in init and teardown.

  * Don't bother with ERR_PTR() style return if only possible
    return is success or no memory.

  * As much as possible, the terms master and slave should be avoided
    because of their cultural connotations.

Note; this code has been tested on Hyper-V
but is compile tested only on virtio.

Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---

Although this patch needs to go into 4.18 (linux-net),
this version is based against net-next because net-next
hasn't been merged into linux-net yet.


 drivers/net/hyperv/hyperv_net.h |   3 +-
 drivers/net/hyperv/netvsc_drv.c | 173 +++++++++++------
 drivers/net/net_failover.c      | 312 ++++++++++++++++++++-----------
 drivers/net/virtio_net.c        |   9 +-
 include/net/failover.h          |  31 +---
 include/net/net_failover.h      |  32 +---
 net/Kconfig                     |  13 +-
 net/core/failover.c             | 316 ++++----------------------------
 8 files changed, 373 insertions(+), 516 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 99d8e7398a5b..c7d25d10765e 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -902,6 +902,8 @@ struct net_device_context {
 	struct hv_device *device_ctx;
 	/* netvsc_device */
 	struct netvsc_device __rcu *nvdev;
+	/* list of netvsc net_devices */
+	struct list_head list;
 	/* reconfigure work */
 	struct delayed_work dwork;
 	/* last reconfig time */
@@ -933,7 +935,6 @@ struct net_device_context {
 	/* Serial number of the VF to team with */
 	u32 vf_serial;
 
-	struct failover *failover;
 };
 
 /* Per channel data */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index bef4d55a108c..074e6b8578df 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -70,6 +70,8 @@ static int debug = -1;
 module_param(debug, int, 0444);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+static LIST_HEAD(netvsc_dev_list);
+
 static void netvsc_change_rx_flags(struct net_device *net, int change)
 {
 	struct net_device_context *ndev_ctx = netdev_priv(net);
@@ -1846,101 +1848,120 @@ static void netvsc_vf_setup(struct work_struct *w)
 	}
 
 	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
-	if (vf_netdev)
+	if (vf_netdev) {
 		__netvsc_vf_setup(ndev, vf_netdev);
-
+		dev_put(vf_netdev);
+	}
 	rtnl_unlock();
 }
 
-static int netvsc_pre_register_vf(struct net_device *vf_netdev,
-				  struct net_device *ndev)
+static struct net_device *get_netvsc_bymac(const u8 *mac)
 {
-	struct net_device_context *net_device_ctx;
-	struct netvsc_device *netvsc_dev;
+	struct net_device_context *ndev_ctx;
 
-	net_device_ctx = netdev_priv(ndev);
-	netvsc_dev = rtnl_dereference(net_device_ctx->nvdev);
-	if (!netvsc_dev || rtnl_dereference(net_device_ctx->vf_netdev))
-		return -ENODEV;
+	ASSERT_RTNL();
 
-	return 0;
+	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
+		struct net_device *dev = hv_get_drvdata(ndev_ctx->device_ctx);
+
+		if (ether_addr_equal(mac, dev->perm_addr))
+			return dev;
+	}
+
+	return NULL;
 }
 
-static int netvsc_register_vf(struct net_device *vf_netdev,
-			      struct net_device *ndev)
+static int netvsc_register_vf(struct net_device *vf_netdev)
 {
-	struct net_device_context *ndev_ctx = netdev_priv(ndev);
+	struct net_device *ndev;
+	struct net_device_context *ndev_ctx;
+
+	/* Must use Ethernet addresses */
+	if (vf_netdev->addr_len != ETH_ALEN)
+		return NOTIFY_DONE;
+
+	/* VF must be a physical device not VLAN, etc */
+	if (!vf_netdev->dev.parent)
+		return NOTIFY_DONE;
+
+	/* Use the MAC address to locate the synthetic interface to
+	 * associate with the VF interface.
+	 */
+	ndev = get_netvsc_bymac(vf_netdev->perm_addr);
+	if (!ndev)
+		return NOTIFY_DONE;
+
+	/* If network device is being removed, don't do anything */
+	ndev_ctx = netdev_priv(ndev);
+	if (!rtnl_dereference(ndev_ctx->nvdev))
+		return NOTIFY_DONE;
+
+	if (netdev_failover_join(vf_netdev, ndev, netvsc_vf_handle_frame)) {
+		netdev_err(vf_netdev, "could not join: %s", ndev->name);
+		return NOTIFY_DONE;
+	}
 
 	/* set slave flag before open to prevent IPv6 addrconf */
 	vf_netdev->flags |= IFF_SLAVE;
 
+	dev_hold(vf_netdev);
+
 	schedule_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT);
 
 	call_netdevice_notifiers(NETDEV_JOIN, vf_netdev);
 
 	netdev_info(vf_netdev, "joined to %s\n", ndev->name);
 
-	dev_hold(vf_netdev);
 	rcu_assign_pointer(ndev_ctx->vf_netdev, vf_netdev);
 
-	return 0;
+	return NOTIFY_OK;
 }
 
 /* VF up/down change detected, schedule to change data path */
-static int netvsc_vf_changed(struct net_device *vf_netdev,
-			     struct net_device *ndev)
+static int netvsc_vf_changed(struct net_device *vf_netdev)
 {
 	struct net_device_context *net_device_ctx;
 	struct netvsc_device *netvsc_dev;
+	struct net_device *ndev;
 	bool vf_is_up = netif_running(vf_netdev);
 
+	ndev = netdev_failover_upper_get(vf_netdev);
+	if (!ndev)
+		return NOTIFY_DONE;
+
 	net_device_ctx = netdev_priv(ndev);
 	netvsc_dev = rtnl_dereference(net_device_ctx->nvdev);
 	if (!netvsc_dev)
-		return -ENODEV;
+		return NOTIFY_DONE;
 
 	netvsc_switch_datapath(ndev, vf_is_up);
 	netdev_info(ndev, "Data path switched %s VF: %s\n",
 		    vf_is_up ? "to" : "from", vf_netdev->name);
 
-	return 0;
+	return NOTIFY_OK;
 }
 
-static int netvsc_pre_unregister_vf(struct net_device *vf_netdev,
-				    struct net_device *ndev)
+static int netvsc_unregister_vf(struct net_device *vf_netdev)
 {
 	struct net_device_context *net_device_ctx;
+	struct net_device *ndev;
 
-	net_device_ctx = netdev_priv(ndev);
-	cancel_delayed_work_sync(&net_device_ctx->vf_takeover);
-
-	return 0;
-}
-
-static int netvsc_unregister_vf(struct net_device *vf_netdev,
-				struct net_device *ndev)
-{
-	struct net_device_context *net_device_ctx;
+	ndev = netdev_failover_upper_get(vf_netdev);
+	if (!ndev)
+		return NOTIFY_DONE;
 
 	net_device_ctx = netdev_priv(ndev);
+	if (cancel_delayed_work_sync(&net_device_ctx->vf_takeover))
+		dev_put(vf_netdev);
 
 	netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
 
+	netdev_failover_unjoin(vf_netdev, ndev);
 	RCU_INIT_POINTER(net_device_ctx->vf_netdev, NULL);
-	dev_put(vf_netdev);
 
-	return 0;
+	return NOTIFY_OK;
 }
 
-static struct failover_ops netvsc_failover_ops = {
-	.slave_pre_register	= netvsc_pre_register_vf,
-	.slave_register		= netvsc_register_vf,
-	.slave_pre_unregister	= netvsc_pre_unregister_vf,
-	.slave_unregister	= netvsc_unregister_vf,
-	.slave_link_change	= netvsc_vf_changed,
-	.slave_handle_frame	= netvsc_vf_handle_frame,
-};
-
 static int netvsc_probe(struct hv_device *dev,
 			const struct hv_vmbus_device_id *dev_id)
 {
@@ -2009,6 +2030,8 @@ static int netvsc_probe(struct hv_device *dev,
 
 	memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
 
+	net->priv_flags |= IFF_FAILOVER;
+
 	/* hw_features computed in rndis_netdev_set_hwcaps() */
 	net->features = net->hw_features |
 		NETIF_F_HIGHDMA | NETIF_F_SG |
@@ -2024,23 +2047,19 @@ static int netvsc_probe(struct hv_device *dev,
 	else
 		net->max_mtu = ETH_DATA_LEN;
 
-	ret = register_netdev(net);
+	rtnl_lock();
+	ret = register_netdevice(net);
 	if (ret != 0) {
 		pr_err("Unable to register netdev.\n");
 		goto register_failed;
 	}
 
-	net_device_ctx->failover = failover_register(net, &netvsc_failover_ops);
-	if (IS_ERR(net_device_ctx->failover)) {
-		ret = PTR_ERR(net_device_ctx->failover);
-		goto err_failover;
-	}
-
-	return ret;
+	list_add(&net_device_ctx->list, &netvsc_dev_list);
+	rtnl_unlock();
+	return 0;
 
-err_failover:
-	unregister_netdev(net);
 register_failed:
+	rtnl_unlock();
 	rndis_filter_device_remove(dev, nvdev);
 rndis_failed:
 	free_percpu(net_device_ctx->vf_stats);
@@ -2079,15 +2098,17 @@ static int netvsc_remove(struct hv_device *dev)
 	 */
 	rtnl_lock();
 	vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
-	if (vf_netdev)
-		failover_slave_unregister(vf_netdev);
+	if (vf_netdev) {
+		netdev_failover_unjoin(vf_netdev, net);
+		dev_put(vf_netdev);
+	}
 
 	if (nvdev)
 		rndis_filter_device_remove(dev, nvdev);
 
 	unregister_netdevice(net);
 
-	failover_unregister(ndev_ctx->failover);
+	list_del(&ndev_ctx->list);
 
 	rtnl_unlock();
 	rcu_read_unlock();
@@ -2115,8 +2136,47 @@ static struct  hv_driver netvsc_drv = {
 	.remove = netvsc_remove,
 };
 
+/* On Hyper-V, every VF interface is matched with a corresponding
+ * synthetic interface. The synthetic interface is presented first
+ * to the guest. When the corresponding VF instance is registered,
+ * we will take care of switching the data path.
+ */
+static int netvsc_netdev_event(struct notifier_block *this,
+			       unsigned long event, void *ptr)
+{
+	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
+
+	/* Skip parent events */
+	if (netif_is_failover(event_dev))
+		return NOTIFY_DONE;
+
+	/* Avoid non-Ethernet type devices */
+	if (event_dev->type != ARPHRD_ETHER)
+		return NOTIFY_DONE;
+
+	switch (event) {
+	case NETDEV_REGISTER:
+		return netvsc_register_vf(event_dev);
+
+	case NETDEV_UNREGISTER:
+		return netvsc_unregister_vf(event_dev);
+
+	case NETDEV_UP:
+	case NETDEV_DOWN:
+		return netvsc_vf_changed(event_dev);
+
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static struct notifier_block netvsc_netdev_notifier = {
+	.notifier_call = netvsc_netdev_event,
+};
+
 static void __exit netvsc_drv_exit(void)
 {
+	unregister_netdevice_notifier(&netvsc_netdev_notifier);
 	vmbus_driver_unregister(&netvsc_drv);
 }
 
@@ -2136,6 +2196,7 @@ static int __init netvsc_drv_init(void)
 	if (ret)
 		return ret;
 
+	register_netdevice_notifier(&netvsc_netdev_notifier);
 	return 0;
 }
 
diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 83f7420ddea5..e0d30527f748 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -28,6 +28,46 @@
 #include <uapi/linux/if_arp.h>
 #include <net/net_failover.h>
 
+static LIST_HEAD(net_failover_list);
+
+/* failover state */
+struct net_failover_info {
+	struct net_device *failover_dev;
+
+	/* list of failover virtual devices */
+	struct list_head list;
+
+	/* primary netdev with same MAC */
+	struct net_device __rcu *primary_dev;
+
+	/* standby netdev */
+	struct net_device __rcu *standby_dev;
+
+	/* primary netdev stats */
+	struct rtnl_link_stats64 primary_stats;
+
+	/* standby netdev stats */
+	struct rtnl_link_stats64 standby_stats;
+
+	/* aggregated stats */
+	struct rtnl_link_stats64 failover_stats;
+
+	/* spinlock while updating stats */
+	spinlock_t stats_lock;
+
+	/* delayed setup of slave */
+	struct delayed_work standby_init;
+};
+
+#define FAILOVER_VLAN_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
+				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
+				 NETIF_F_HIGHDMA | NETIF_F_LRO)
+
+#define FAILOVER_ENC_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
+				 NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
+
+#define FAILOVER_SETUP_INTERVAL	(HZ / 10)
+
 static bool net_failover_xmit_ready(struct net_device *dev)
 {
 	return netif_running(dev) && netif_carrier_ok(dev);
@@ -460,22 +500,42 @@ static void net_failover_lower_state_changed(struct net_device *slave_dev,
 	netdev_lower_state_changed(slave_dev, &info);
 }
 
-static int net_failover_slave_pre_register(struct net_device *slave_dev,
-					   struct net_device *failover_dev)
+static struct net_device *get_net_failover_bymac(const u8 *mac)
 {
-	struct net_device *standby_dev, *primary_dev;
+	struct net_failover_info *nfo_info;
+
+	ASSERT_RTNL();
+
+	list_for_each_entry(nfo_info, &net_failover_list, list) {
+		struct net_device *failover_dev = nfo_info->failover_dev;
+
+		if (ether_addr_equal(mac, failover_dev->perm_addr))
+			return failover_dev;
+	}
+
+	return NULL;
+}
+
+static int net_failover_register_event(struct net_device *slave_dev)
+{
+	struct net_device *failover_dev, *standby_dev, *primary_dev;
 	struct net_failover_info *nfo_info;
 	bool slave_is_standby;
 
+	failover_dev = get_net_failover_bymac(slave_dev->perm_addr);
+	if (!failover_dev)
+		return NOTIFY_DONE;
+
 	nfo_info = netdev_priv(failover_dev);
 	standby_dev = rtnl_dereference(nfo_info->standby_dev);
 	primary_dev = rtnl_dereference(nfo_info->primary_dev);
 	slave_is_standby = slave_dev->dev.parent == failover_dev->dev.parent;
 	if (slave_is_standby ? standby_dev : primary_dev) {
-		netdev_err(failover_dev, "%s attempting to register as slave dev when %s already present\n",
+		netdev_err(failover_dev,
+			   "%s attempting to register as slave dev when %s already present\n",
 			   slave_dev->name,
 			   slave_is_standby ? "standby" : "primary");
-		return -EINVAL;
+		return NOTIFY_DONE;
 	}
 
 	/* We want to allow only a direct attached VF device as a primary
@@ -484,23 +544,33 @@ static int net_failover_slave_pre_register(struct net_device *slave_dev,
 	 */
 	if (!slave_is_standby && (!slave_dev->dev.parent ||
 				  !dev_is_pci(slave_dev->dev.parent)))
-		return -EINVAL;
+		return NOTIFY_DONE;
 
 	if (failover_dev->features & NETIF_F_VLAN_CHALLENGED &&
 	    vlan_uses_dev(failover_dev)) {
-		netdev_err(failover_dev, "Device %s is VLAN challenged and failover device has VLAN set up\n",
+		netdev_err(failover_dev,
+			   "Device %s is VLAN challenged and failover device has VLAN set up\n",
 			   failover_dev->name);
-		return -EINVAL;
+		return NOTIFY_DONE;
 	}
 
-	return 0;
+	if (netdev_failover_join(slave_dev, failover_dev,
+				 net_failover_handle_frame)) {
+		netdev_err(failover_dev, "could not join: %s", slave_dev->name);
+		return NOTIFY_DONE;
+	}
+
+	/* Trigger rest of setup in process context */
+	schedule_delayed_work(&nfo_info->standby_init, FAILOVER_SETUP_INTERVAL);
+
+	return NOTIFY_OK;
 }
 
-static int net_failover_slave_register(struct net_device *slave_dev,
-				       struct net_device *failover_dev)
+static void __net_failover_setup(struct net_device *failover_dev)
 {
+	struct net_failover_info *nfo_info = netdev_priv(failover_dev);
+	struct net_device *slave_dev = rtnl_dereference(nfo_info->standby_dev);
 	struct net_device *standby_dev, *primary_dev;
-	struct net_failover_info *nfo_info;
 	bool slave_is_standby;
 	u32 orig_mtu;
 	int err;
@@ -509,13 +579,12 @@ static int net_failover_slave_register(struct net_device *slave_dev,
 	orig_mtu = slave_dev->mtu;
 	err = dev_set_mtu(slave_dev, failover_dev->mtu);
 	if (err) {
-		netdev_err(failover_dev, "unable to change mtu of %s to %u register failed\n",
+		netdev_err(failover_dev,
+			   "unable to change mtu of %s to %u register failed\n",
 			   slave_dev->name, failover_dev->mtu);
 		goto done;
 	}
 
-	dev_hold(slave_dev);
-
 	if (netif_running(failover_dev)) {
 		err = dev_open(slave_dev);
 		if (err && (err != -EBUSY)) {
@@ -537,7 +606,6 @@ static int net_failover_slave_register(struct net_device *slave_dev,
 		goto err_vlan_add;
 	}
 
-	nfo_info = netdev_priv(failover_dev);
 	standby_dev = rtnl_dereference(nfo_info->standby_dev);
 	primary_dev = rtnl_dereference(nfo_info->primary_dev);
 	slave_is_standby = slave_dev->dev.parent == failover_dev->dev.parent;
@@ -562,52 +630,56 @@ static int net_failover_slave_register(struct net_device *slave_dev,
 	netdev_info(failover_dev, "failover %s slave:%s registered\n",
 		    slave_is_standby ? "standby" : "primary", slave_dev->name);
 
-	return 0;
+	return;
 
 err_vlan_add:
 	dev_uc_unsync(slave_dev, failover_dev);
 	dev_mc_unsync(slave_dev, failover_dev);
 	dev_close(slave_dev);
 err_dev_open:
-	dev_put(slave_dev);
 	dev_set_mtu(slave_dev, orig_mtu);
 done:
-	return err;
+	return;
 }
 
-static int net_failover_slave_pre_unregister(struct net_device *slave_dev,
-					     struct net_device *failover_dev)
+static void net_failover_setup(struct work_struct *w)
 {
-	struct net_device *standby_dev, *primary_dev;
-	struct net_failover_info *nfo_info;
+	struct net_failover_info *nfo_info
+		= container_of(w, struct net_failover_info, standby_init.work);
+	struct net_device *failover_dev = nfo_info->failover_dev;
 
-	nfo_info = netdev_priv(failover_dev);
-	primary_dev = rtnl_dereference(nfo_info->primary_dev);
-	standby_dev = rtnl_dereference(nfo_info->standby_dev);
-
-	if (slave_dev != primary_dev && slave_dev != standby_dev)
-		return -ENODEV;
+	/* handle race with cancel delayed work on removal */
+	if (!rtnl_trylock()) {
+		schedule_delayed_work(&nfo_info->standby_init, 0);
+		return;
+	}
 
-	return 0;
+	__net_failover_setup(failover_dev);
+	rtnl_unlock();
 }
 
-static int net_failover_slave_unregister(struct net_device *slave_dev,
-					 struct net_device *failover_dev)
+static int net_failover_unregister_event(struct net_device *slave_dev)
 {
-	struct net_device *standby_dev, *primary_dev;
+	struct net_device *failover_dev, *primary_dev, *standby_dev;
 	struct net_failover_info *nfo_info;
 	bool slave_is_standby;
 
+	failover_dev = netdev_failover_upper_get(slave_dev);
+	if (!failover_dev)
+		return NOTIFY_DONE;
+
 	nfo_info = netdev_priv(failover_dev);
 	primary_dev = rtnl_dereference(nfo_info->primary_dev);
 	standby_dev = rtnl_dereference(nfo_info->standby_dev);
 
+	if (slave_dev != primary_dev && slave_dev != standby_dev)
+		return NOTIFY_DONE;
+
 	vlan_vids_del_by_dev(slave_dev, failover_dev);
 	dev_uc_unsync(slave_dev, failover_dev);
 	dev_mc_unsync(slave_dev, failover_dev);
 	dev_close(slave_dev);
 
-	nfo_info = netdev_priv(failover_dev);
 	dev_get_stats(failover_dev, &nfo_info->failover_stats);
 
 	slave_is_standby = slave_dev->dev.parent == failover_dev->dev.parent;
@@ -628,22 +700,25 @@ static int net_failover_slave_unregister(struct net_device *slave_dev,
 	netdev_info(failover_dev, "failover %s slave:%s unregistered\n",
 		    slave_is_standby ? "standby" : "primary", slave_dev->name);
 
-	return 0;
+	return NOTIFY_OK;
 }
 
-static int net_failover_slave_link_change(struct net_device *slave_dev,
-					  struct net_device *failover_dev)
+static int net_failover_link_event(struct net_device *slave_dev)
+
 {
-	struct net_device *primary_dev, *standby_dev;
+	struct net_device *failover_dev, *primary_dev, *standby_dev;
 	struct net_failover_info *nfo_info;
 
-	nfo_info = netdev_priv(failover_dev);
+	failover_dev = netdev_failover_upper_get(slave_dev);
+	if (!failover_dev)
+		return NOTIFY_DONE;
 
+	nfo_info = netdev_priv(failover_dev);
 	primary_dev = rtnl_dereference(nfo_info->primary_dev);
 	standby_dev = rtnl_dereference(nfo_info->standby_dev);
 
 	if (slave_dev != primary_dev && slave_dev != standby_dev)
-		return -ENODEV;
+		return NOTIFY_DONE;
 
 	if ((primary_dev && net_failover_xmit_ready(primary_dev)) ||
 	    (standby_dev && net_failover_xmit_ready(standby_dev))) {
@@ -657,43 +732,11 @@ static int net_failover_slave_link_change(struct net_device *slave_dev,
 
 	net_failover_lower_state_changed(slave_dev, primary_dev, standby_dev);
 
-	return 0;
+	return NOTIFY_DONE;
 }
 
-static int net_failover_slave_name_change(struct net_device *slave_dev,
-					  struct net_device *failover_dev)
-{
-	struct net_device *primary_dev, *standby_dev;
-	struct net_failover_info *nfo_info;
-
-	nfo_info = netdev_priv(failover_dev);
-
-	primary_dev = rtnl_dereference(nfo_info->primary_dev);
-	standby_dev = rtnl_dereference(nfo_info->standby_dev);
-
-	if (slave_dev != primary_dev && slave_dev != standby_dev)
-		return -ENODEV;
-
-	/* We need to bring up the slave after the rename by udev in case
-	 * open failed with EBUSY when it was registered.
-	 */
-	dev_open(slave_dev);
-
-	return 0;
-}
-
-static struct failover_ops net_failover_ops = {
-	.slave_pre_register	= net_failover_slave_pre_register,
-	.slave_register		= net_failover_slave_register,
-	.slave_pre_unregister	= net_failover_slave_pre_unregister,
-	.slave_unregister	= net_failover_slave_unregister,
-	.slave_link_change	= net_failover_slave_link_change,
-	.slave_name_change	= net_failover_slave_name_change,
-	.slave_handle_frame	= net_failover_handle_frame,
-};
-
 /**
- * net_failover_create - Create and register a failover instance
+ * net_failover_create - Create and register a failover device
  *
  * @dev: standby netdev
  *
@@ -703,13 +746,12 @@ static struct failover_ops net_failover_ops = {
  * the original standby netdev and a VF netdev with the same MAC gets
  * registered as primary netdev.
  *
- * Return: pointer to failover instance
+ * Return: pointer to failover network device
  */
-struct failover *net_failover_create(struct net_device *standby_dev)
+struct net_device *net_failover_create(struct net_device *standby_dev)
 {
-	struct device *dev = standby_dev->dev.parent;
+	struct net_failover_info *nfo_info;
 	struct net_device *failover_dev;
-	struct failover *failover;
 	int err;
 
 	/* Alloc at least 2 queues, for now we are going with 16 assuming
@@ -717,18 +759,22 @@ struct failover *net_failover_create(struct net_device *standby_dev)
 	 */
 	failover_dev = alloc_etherdev_mq(sizeof(struct net_failover_info), 16);
 	if (!failover_dev) {
-		dev_err(dev, "Unable to allocate failover_netdev!\n");
-		return ERR_PTR(-ENOMEM);
+		netdev_err(standby_dev, "Unable to allocate failover_netdev!\n");
+		return NULL;
 	}
 
+	nfo_info = netdev_priv(failover_dev);
 	dev_net_set(failover_dev, dev_net(standby_dev));
-	SET_NETDEV_DEV(failover_dev, dev);
+	nfo_info->failover_dev = failover_dev;
+	INIT_DELAYED_WORK(&nfo_info->standby_init, net_failover_setup);
 
 	failover_dev->netdev_ops = &failover_dev_ops;
 	failover_dev->ethtool_ops = &failover_ethtool_ops;
 
 	/* Initialize the device options */
-	failover_dev->priv_flags |= IFF_UNICAST_FLT | IFF_NO_QUEUE;
+	failover_dev->priv_flags |= IFF_UNICAST_FLT |
+				    IFF_NO_QUEUE |
+				    IFF_FAILOVER;
 	failover_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE |
 				       IFF_TX_SKB_SHARING);
 
@@ -746,29 +792,38 @@ struct failover *net_failover_create(struct net_device *standby_dev)
 	failover_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
 	failover_dev->features |= failover_dev->hw_features;
 
-	memcpy(failover_dev->dev_addr, standby_dev->dev_addr,
-	       failover_dev->addr_len);
+	ether_addr_copy(failover_dev->dev_addr, standby_dev->dev_addr);
+	ether_addr_copy(failover_dev->perm_addr, standby_dev->perm_addr);
 
 	failover_dev->min_mtu = standby_dev->min_mtu;
 	failover_dev->max_mtu = standby_dev->max_mtu;
 
-	err = register_netdev(failover_dev);
+	netif_carrier_off(failover_dev);
+
+	rtnl_lock();
+	err = register_netdevice(failover_dev);
 	if (err) {
-		dev_err(dev, "Unable to register failover_dev!\n");
+		netdev_err(standby_dev, "Unable to register failover_dev!\n");
 		goto err_register_netdev;
 	}
 
-	netif_carrier_off(failover_dev);
+	err = netdev_failover_join(standby_dev, failover_dev,
+				   net_failover_handle_frame);
+	if (err) {
+		netdev_err(failover_dev, "Unable to join with %s\n",
+			   standby_dev->name);
+		goto err_failover_join;
+	}
 
-	failover = failover_register(failover_dev, &net_failover_ops);
-	if (IS_ERR(failover))
-		goto err_failover_register;
+	list_add(&nfo_info->list, &net_failover_list);
+	rtnl_unlock();
 
-	return failover;
+	return failover_dev;
 
-err_failover_register:
-	unregister_netdev(failover_dev);
+err_failover_join:
+	unregister_netdevice(failover_dev);
 err_register_netdev:
+	rtnl_unlock();
 	free_netdev(failover_dev);
 
 	return ERR_PTR(err);
@@ -786,31 +841,27 @@ EXPORT_SYMBOL_GPL(net_failover_create);
  * netdev. Used by paravirtual drivers that use 3-netdev model.
  *
  */
-void net_failover_destroy(struct failover *failover)
+void net_failover_destroy(struct net_device *failover_dev)
 {
-	struct net_failover_info *nfo_info;
-	struct net_device *failover_dev;
+	struct net_failover_info *nfo_info = netdev_priv(failover_dev);
 	struct net_device *slave_dev;
 
-	if (!failover)
-		return;
-
-	failover_dev = rcu_dereference(failover->failover_dev);
-	nfo_info = netdev_priv(failover_dev);
-
 	netif_device_detach(failover_dev);
 
 	rtnl_lock();
-
 	slave_dev = rtnl_dereference(nfo_info->primary_dev);
-	if (slave_dev)
-		failover_slave_unregister(slave_dev);
+	if (slave_dev) {
+		netdev_failover_unjoin(slave_dev, failover_dev);
+		dev_put(slave_dev);
+	}
 
 	slave_dev = rtnl_dereference(nfo_info->standby_dev);
-	if (slave_dev)
-		failover_slave_unregister(slave_dev);
+	if (slave_dev) {
+		netdev_failover_unjoin(slave_dev, failover_dev);
+		dev_put(slave_dev);
+	}
 
-	failover_unregister(failover);
+	list_del(&nfo_info->list);
 
 	unregister_netdevice(failover_dev);
 
@@ -820,9 +871,53 @@ void net_failover_destroy(struct failover *failover)
 }
 EXPORT_SYMBOL_GPL(net_failover_destroy);
 
+static int net_failover_event(struct notifier_block *this,
+			      unsigned long event, void *ptr)
+{
+	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
+
+	/* Skip parent events */
+	if (netif_is_failover(event_dev))
+		return NOTIFY_DONE;
+
+	/* Avoid non-Ethernet type devices */
+	if (event_dev->type != ARPHRD_ETHER)
+		return NOTIFY_DONE;
+
+	/* Avoid Vlan dev with same MAC registering as VF */
+	if (is_vlan_dev(event_dev))
+		return NOTIFY_DONE;
+
+	/* Avoid Bonding master dev with same MAC registering as VF */
+	if ((event_dev->priv_flags & IFF_BONDING) &&
+	    (event_dev->flags & IFF_MASTER))
+		return NOTIFY_DONE;
+
+	switch (event) {
+	case NETDEV_REGISTER:
+		return net_failover_register_event(event_dev);
+
+	case NETDEV_UNREGISTER:
+		return net_failover_unregister_event(event_dev);
+
+	case NETDEV_UP:
+	case NETDEV_DOWN:
+	case NETDEV_CHANGE:
+		return net_failover_link_event(event_dev);
+
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static struct notifier_block net_failover_notifier = {
+	.notifier_call = net_failover_event,
+};
+
 static __init int
 net_failover_init(void)
 {
+	register_netdevice_notifier(&net_failover_notifier);
 	return 0;
 }
 module_init(net_failover_init);
@@ -830,6 +925,7 @@ module_init(net_failover_init);
 static __exit
 void net_failover_exit(void)
 {
+	unregister_netdevice_notifier(&net_failover_notifier);
 }
 module_exit(net_failover_exit);
 
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6d710b8b41c5..b40ae28dac93 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -215,7 +215,7 @@ struct virtnet_info {
 	unsigned long guest_offloads;
 
 	/* failover when STANDBY feature enabled */
-	struct failover *failover;
+	struct net_device *failover;
 };
 
 struct padded_vnet_hdr {
@@ -2930,11 +2930,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 	virtnet_init_settings(dev);
 
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
-		vi->failover = net_failover_create(vi->dev);
-		if (IS_ERR(vi->failover)) {
-			err = PTR_ERR(vi->failover);
+		err = -ENOMEM;
+		vi->failover = net_failover_create(dev);
+		if (!vi->failover)
 			goto free_vqs;
-		}
 	}
 
 	err = register_netdev(dev);
diff --git a/include/net/failover.h b/include/net/failover.h
index bb15438f39c7..22d6c1369101 100644
--- a/include/net/failover.h
+++ b/include/net/failover.h
@@ -6,31 +6,10 @@
 
 #include <linux/netdevice.h>
 
-struct failover_ops {
-	int (*slave_pre_register)(struct net_device *slave_dev,
-				  struct net_device *failover_dev);
-	int (*slave_register)(struct net_device *slave_dev,
-			      struct net_device *failover_dev);
-	int (*slave_pre_unregister)(struct net_device *slave_dev,
-				    struct net_device *failover_dev);
-	int (*slave_unregister)(struct net_device *slave_dev,
-				struct net_device *failover_dev);
-	int (*slave_link_change)(struct net_device *slave_dev,
-				 struct net_device *failover_dev);
-	int (*slave_name_change)(struct net_device *slave_dev,
-				 struct net_device *failover_dev);
-	rx_handler_result_t (*slave_handle_frame)(struct sk_buff **pskb);
-};
-
-struct failover {
-	struct list_head list;
-	struct net_device __rcu *failover_dev;
-	struct failover_ops __rcu *ops;
-};
-
-struct failover *failover_register(struct net_device *dev,
-				   struct failover_ops *ops);
-void failover_unregister(struct failover *failover);
-int failover_slave_unregister(struct net_device *slave_dev);
+int netdev_failover_join(struct net_device *lower, struct net_device *upper,
+			 rx_handler_func_t *rx_handler);
+struct net_device *netdev_failover_upper_get(struct net_device *lower);
+void netdev_failover_unjoin(struct net_device *lower,
+			    struct net_device *upper);
 
 #endif /* _FAILOVER_H */
diff --git a/include/net/net_failover.h b/include/net/net_failover.h
index b12a1c469d1c..a99b3b00b4e3 100644
--- a/include/net/net_failover.h
+++ b/include/net/net_failover.h
@@ -6,35 +6,7 @@
 
 #include <net/failover.h>
 
-/* failover state */
-struct net_failover_info {
-	/* primary netdev with same MAC */
-	struct net_device __rcu *primary_dev;
-
-	/* standby netdev */
-	struct net_device __rcu *standby_dev;
-
-	/* primary netdev stats */
-	struct rtnl_link_stats64 primary_stats;
-
-	/* standby netdev stats */
-	struct rtnl_link_stats64 standby_stats;
-
-	/* aggregated stats */
-	struct rtnl_link_stats64 failover_stats;
-
-	/* spinlock while updating stats */
-	spinlock_t stats_lock;
-};
-
-struct failover *net_failover_create(struct net_device *standby_dev);
-void net_failover_destroy(struct failover *failover);
-
-#define FAILOVER_VLAN_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
-				 NETIF_F_HIGHDMA | NETIF_F_LRO)
-
-#define FAILOVER_ENC_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
+struct net_device *net_failover_create(struct net_device *standby_dev);
+void net_failover_destroy(struct net_device *failover_dev);
 
 #endif /* _NET_FAILOVER_H */
diff --git a/net/Kconfig b/net/Kconfig
index f738a6f27665..697d84202695 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -433,17 +433,8 @@ config PAGE_POOL
        bool
 
 config FAILOVER
-	tristate "Generic failover module"
-	help
-	  The failover module provides a generic interface for paravirtual
-	  drivers to register a netdev and a set of ops with a failover
-	  instance. The ops are used as event handlers that get called to
-	  handle netdev register/unregister/link change/name change events
-	  on slave pci ethernet devices with the same mac address as the
-	  failover netdev. This enables paravirtual drivers to use a
-	  VF as an accelerated low latency datapath. It also allows live
-	  migration of VMs with direct attached VFs by failing over to the
-	  paravirtual datapath when the VF is unplugged.
+	bool
+	default n
 
 endif   # if NET
 
diff --git a/net/core/failover.c b/net/core/failover.c
index 4a92a98ccce9..499f0fd7e4d3 100644
--- a/net/core/failover.c
+++ b/net/core/failover.c
@@ -1,10 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2018, Intel Corporation. */
 
-/* A common module to handle registrations and notifications for paravirtual
+/* A library for managing chained upper/oower devices such as
  * drivers to enable accelerated datapath and support VF live migration.
- *
- * The notifier and event handling code is based on netvsc driver.
  */
 
 #include <linux/module.h>
@@ -14,302 +12,62 @@
 #include <linux/if_vlan.h>
 #include <net/failover.h>
 
-static LIST_HEAD(failover_list);
-static DEFINE_SPINLOCK(failover_lock);
-
-static struct net_device *failover_get_bymac(u8 *mac, struct failover_ops **ops)
-{
-	struct net_device *failover_dev;
-	struct failover *failover;
-
-	spin_lock(&failover_lock);
-	list_for_each_entry(failover, &failover_list, list) {
-		failover_dev = rtnl_dereference(failover->failover_dev);
-		if (ether_addr_equal(failover_dev->perm_addr, mac)) {
-			*ops = rtnl_dereference(failover->ops);
-			spin_unlock(&failover_lock);
-			return failover_dev;
-		}
-	}
-	spin_unlock(&failover_lock);
-	return NULL;
-}
-
-/**
- * failover_slave_register - Register a slave netdev
- *
- * @slave_dev: slave netdev that is being registered
- *
- * Registers a slave device to a failover instance. Only ethernet devices
- * are supported.
- */
-static int failover_slave_register(struct net_device *slave_dev)
+/* failover_join - Join an lower netdev with an upper device. */
+int netdev_failover_join(struct net_device *lower_dev,
+			 struct net_device *upper_dev,
+			 rx_handler_func_t *rx_handler)
 {
-	struct netdev_lag_upper_info lag_upper_info;
-	struct net_device *failover_dev;
-	struct failover_ops *fops;
 	int err;
 
-	if (slave_dev->type != ARPHRD_ETHER)
-		goto done;
-
 	ASSERT_RTNL();
 
-	failover_dev = failover_get_bymac(slave_dev->perm_addr, &fops);
-	if (!failover_dev)
-		goto done;
+	/* Don't allow joining devices of different protocols */
+	if (upper_dev->type != lower_dev->type)
+		return -EINVAL;
 
-	if (fops && fops->slave_pre_register &&
-	    fops->slave_pre_register(slave_dev, failover_dev))
-		goto done;
-
-	err = netdev_rx_handler_register(slave_dev, fops->slave_handle_frame,
-					 failover_dev);
+	err = netdev_rx_handler_register(lower_dev, rx_handler, upper_dev);
 	if (err) {
-		netdev_err(slave_dev, "can not register failover rx handler (err = %d)\n",
+		netdev_err(lower_dev,
+			   "can not register failover rx handler (err = %d)\n",
 			   err);
-		goto done;
+		return err;
 	}
 
-	lag_upper_info.tx_type = NETDEV_LAG_TX_TYPE_ACTIVEBACKUP;
-	err = netdev_master_upper_dev_link(slave_dev, failover_dev, NULL,
-					   &lag_upper_info, NULL);
+	err = netdev_master_upper_dev_link(lower_dev, upper_dev, NULL,
+					   NULL, NULL);
 	if (err) {
-		netdev_err(slave_dev, "can not set failover device %s (err = %d)\n",
-			   failover_dev->name, err);
-		goto err_upper_link;
+		netdev_err(lower_dev,
+			   "can not set failover device %s (err = %d)\n",
+			   upper_dev->name, err);
+		netdev_rx_handler_unregister(lower_dev);
+		return err;
 	}
 
-	slave_dev->priv_flags |= IFF_FAILOVER_SLAVE;
-
-	if (fops && fops->slave_register &&
-	    !fops->slave_register(slave_dev, failover_dev))
-		return NOTIFY_OK;
-
-	netdev_upper_dev_unlink(slave_dev, failover_dev);
-	slave_dev->priv_flags &= ~IFF_FAILOVER_SLAVE;
-err_upper_link:
-	netdev_rx_handler_unregister(slave_dev);
-done:
-	return NOTIFY_DONE;
-}
-
-/**
- * failover_slave_unregister - Unregister a slave netdev
- *
- * @slave_dev: slave netdev that is being unregistered
- *
- * Unregisters a slave device from a failover instance.
- */
-int failover_slave_unregister(struct net_device *slave_dev)
-{
-	struct net_device *failover_dev;
-	struct failover_ops *fops;
-
-	if (!netif_is_failover_slave(slave_dev))
-		goto done;
-
-	ASSERT_RTNL();
-
-	failover_dev = failover_get_bymac(slave_dev->perm_addr, &fops);
-	if (!failover_dev)
-		goto done;
-
-	if (fops && fops->slave_pre_unregister &&
-	    fops->slave_pre_unregister(slave_dev, failover_dev))
-		goto done;
-
-	netdev_rx_handler_unregister(slave_dev);
-	netdev_upper_dev_unlink(slave_dev, failover_dev);
-	slave_dev->priv_flags &= ~IFF_FAILOVER_SLAVE;
-
-	if (fops && fops->slave_unregister &&
-	    !fops->slave_unregister(slave_dev, failover_dev))
-		return NOTIFY_OK;
-
-done:
-	return NOTIFY_DONE;
+	dev_hold(lower_dev);
+	lower_dev->priv_flags |= IFF_FAILOVER_SLAVE;
+	return 0;
 }
-EXPORT_SYMBOL_GPL(failover_slave_unregister);
+EXPORT_SYMBOL_GPL(netdev_failover_join);
 
-static int failover_slave_link_change(struct net_device *slave_dev)
+/* Find upper network device for failover slave device */
+struct net_device *netdev_failover_upper_get(struct net_device *lower_dev)
 {
-	struct net_device *failover_dev;
-	struct failover_ops *fops;
-
-	if (!netif_is_failover_slave(slave_dev))
-		goto done;
-
-	ASSERT_RTNL();
-
-	failover_dev = failover_get_bymac(slave_dev->perm_addr, &fops);
-	if (!failover_dev)
-		goto done;
-
-	if (!netif_running(failover_dev))
-		goto done;
+	if (!netif_is_failover_slave(lower_dev))
+		return NULL;
 
-	if (fops && fops->slave_link_change &&
-	    !fops->slave_link_change(slave_dev, failover_dev))
-		return NOTIFY_OK;
-
-done:
-	return NOTIFY_DONE;
+	return netdev_master_upper_dev_get(lower_dev);
 }
+EXPORT_SYMBOL_GPL(netdev_failover_upper_get);
 
-static int failover_slave_name_change(struct net_device *slave_dev)
+/* failover_unjoin - Break connection between lower and upper device. */
+void netdev_failover_unjoin(struct net_device *lower_dev,
+			    struct net_device *upper_dev)
 {
-	struct net_device *failover_dev;
-	struct failover_ops *fops;
-
-	if (!netif_is_failover_slave(slave_dev))
-		goto done;
-
 	ASSERT_RTNL();
 
-	failover_dev = failover_get_bymac(slave_dev->perm_addr, &fops);
-	if (!failover_dev)
-		goto done;
-
-	if (!netif_running(failover_dev))
-		goto done;
-
-	if (fops && fops->slave_name_change &&
-	    !fops->slave_name_change(slave_dev, failover_dev))
-		return NOTIFY_OK;
-
-done:
-	return NOTIFY_DONE;
-}
-
-static int
-failover_event(struct notifier_block *this, unsigned long event, void *ptr)
-{
-	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
-
-	/* Skip parent events */
-	if (netif_is_failover(event_dev))
-		return NOTIFY_DONE;
-
-	switch (event) {
-	case NETDEV_REGISTER:
-		return failover_slave_register(event_dev);
-	case NETDEV_UNREGISTER:
-		return failover_slave_unregister(event_dev);
-	case NETDEV_UP:
-	case NETDEV_DOWN:
-	case NETDEV_CHANGE:
-		return failover_slave_link_change(event_dev);
-	case NETDEV_CHANGENAME:
-		return failover_slave_name_change(event_dev);
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
-static struct notifier_block failover_notifier = {
-	.notifier_call = failover_event,
-};
-
-static void
-failover_existing_slave_register(struct net_device *failover_dev)
-{
-	struct net *net = dev_net(failover_dev);
-	struct net_device *dev;
-
-	rtnl_lock();
-	for_each_netdev(net, dev) {
-		if (netif_is_failover(dev))
-			continue;
-		if (ether_addr_equal(failover_dev->perm_addr, dev->perm_addr))
-			failover_slave_register(dev);
-	}
-	rtnl_unlock();
-}
-
-/**
- * failover_register - Register a failover instance
- *
- * @dev: failover netdev
- * @ops: failover ops
- *
- * Allocate and register a failover instance for a failover netdev. ops
- * provides handlers for slave device register/unregister/link change/
- * name change events.
- *
- * Return: pointer to failover instance
- */
-struct failover *failover_register(struct net_device *dev,
-				   struct failover_ops *ops)
-{
-	struct failover *failover;
-
-	if (dev->type != ARPHRD_ETHER)
-		return ERR_PTR(-EINVAL);
-
-	failover = kzalloc(sizeof(*failover), GFP_KERNEL);
-	if (!failover)
-		return ERR_PTR(-ENOMEM);
-
-	rcu_assign_pointer(failover->ops, ops);
-	dev_hold(dev);
-	dev->priv_flags |= IFF_FAILOVER;
-	rcu_assign_pointer(failover->failover_dev, dev);
-
-	spin_lock(&failover_lock);
-	list_add_tail(&failover->list, &failover_list);
-	spin_unlock(&failover_lock);
-
-	netdev_info(dev, "failover master:%s registered\n", dev->name);
-
-	failover_existing_slave_register(dev);
-
-	return failover;
-}
-EXPORT_SYMBOL_GPL(failover_register);
-
-/**
- * failover_unregister - Unregister a failover instance
- *
- * @failover: pointer to failover instance
- *
- * Unregisters and frees a failover instance.
- */
-void failover_unregister(struct failover *failover)
-{
-	struct net_device *failover_dev;
-
-	failover_dev = rcu_dereference(failover->failover_dev);
-
-	netdev_info(failover_dev, "failover master:%s unregistered\n",
-		    failover_dev->name);
-
-	failover_dev->priv_flags &= ~IFF_FAILOVER;
-	dev_put(failover_dev);
-
-	spin_lock(&failover_lock);
-	list_del(&failover->list);
-	spin_unlock(&failover_lock);
-
-	kfree(failover);
+	netdev_rx_handler_unregister(lower_dev);
+	netdev_upper_dev_unlink(lower_dev, upper_dev);
+	dev_put(lower_dev);
+	lower_dev->priv_flags &= ~IFF_FAILOVER_SLAVE;
 }
-EXPORT_SYMBOL_GPL(failover_unregister);
-
-static __init int
-failover_init(void)
-{
-	register_netdevice_notifier(&failover_notifier);
-
-	return 0;
-}
-module_init(failover_init);
-
-static __exit
-void failover_exit(void)
-{
-	unregister_netdevice_notifier(&failover_notifier);
-}
-module_exit(failover_exit);
-
-MODULE_DESCRIPTION("Generic failover infrastructure/interface");
-MODULE_LICENSE("GPL v2");
+EXPORT_SYMBOL_GPL(netdev_failover_unjoin);
-- 
2.17.1

^ permalink raw reply related

* RE: [PATCH 09/10] dpaa_eth: add support for hardware timestamping
From: Y.b. Lu @ 2018-06-05  3:35 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev@vger.kernel.org, Madalin-cristian Bucur, Rob Herring,
	Shawn Guo, David S . Miller, devicetree@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20180604134920.ezhe6jz5ntpnqyzj@localhost>

Hi Richard,

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Monday, June 4, 2018 9:49 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; Madalin-cristian Bucur
> <madalin.bucur@nxp.com>; Rob Herring <robh+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; David S . Miller <davem@davemloft.net>;
> devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 09/10] dpaa_eth: add support for hardware timestamping
> 
> On Mon, Jun 04, 2018 at 03:08:36PM +0800, Yangbo Lu wrote:
> 
> > +if FSL_DPAA_ETH
> > +config FSL_DPAA_ETH_TS
> > +	bool "DPAA hardware timestamping support"
> > +	select PTP_1588_CLOCK_QORIQ
> > +	default n
> > +	help
> > +	  Enable DPAA hardware timestamping support.
> > +	  This option is useful for applications to get
> > +	  hardware time stamps on the Ethernet packets
> > +	  using the SO_TIMESTAMPING API.
> > +endif
> 
> You should drop this #ifdef.  In general, if a MAC supports time stamping and
> PHC, then the driver support should simply be compiled in.
> 
> [ When time stamping incurs a large run time performance penalty to
>   non-PTP users, then it might make sense to have a Kconfig option to
>   disable it, but that doesn't appear to be the case here. ]

[Y.b. Lu] Actually these timestamping codes affected DPAA networking performance in our previous performance test.
That's why we used ifdef for it.

> 
> > @@ -1615,6 +1635,24 @@ static int dpaa_eth_refill_bpools(struct
> dpaa_priv *priv)
> >  	skbh = (struct sk_buff **)phys_to_virt(addr);
> >  	skb = *skbh;
> >
> > +#ifdef CONFIG_FSL_DPAA_ETH_TS
> > +	if (priv->tx_tstamp &&
> > +	    skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> 
> This condition fits on one line easily.

[Y.b. Lu] Right. I will use one line in next version.

> 
> > +		struct skb_shared_hwtstamps shhwtstamps;
> > +		u64 ns;
> 
> Local variables belong at the top of the function.

[Y.b. Lu] Ok, will move them to the top in next verison.

> 
> > +		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> > +
> > +		if (!dpaa_get_tstamp_ns(priv->net_dev, &ns,
> > +					priv->mac_dev->port[TX],
> > +					(void *)skbh)) {
> > +			shhwtstamps.hwtstamp = ns_to_ktime(ns);
> > +			skb_tstamp_tx(skb, &shhwtstamps);
> > +		} else {
> > +			dev_warn(dev, "dpaa_get_tstamp_ns failed!\n");
> > +		}
> > +	}
> > +#endif
> >  	if (unlikely(qm_fd_get_format(fd) == qm_fd_sg)) {
> >  		nr_frags = skb_shinfo(skb)->nr_frags;
> >  		dma_unmap_single(dev, addr, qm_fd_get_offset(fd) + @@ -2086,6
> > +2124,14 @@ static int dpaa_start_xmit(struct sk_buff *skb, struct
> net_device *net_dev)
> >  	if (unlikely(err < 0))
> >  		goto skb_to_fd_failed;
> >
> > +#ifdef CONFIG_FSL_DPAA_ETH_TS
> > +	if (priv->tx_tstamp &&
> > +	    skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> 
> One line please.

[Y.b. Lu] No problem.

> 
> > +		fd.cmd |= FM_FD_CMD_UPD;
> > +		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> > +	}
> > +#endif
> > +
> >  	if (likely(dpaa_xmit(priv, percpu_stats, queue_mapping, &fd) == 0))
> >  		return NETDEV_TX_OK;
> >
> 
> Thanks,
> Richard

^ permalink raw reply

* [PATCH net-next] bpfilter: switch to CC from HOSTCC
From: Alexei Starovoitov @ 2018-06-05  2:53 UTC (permalink / raw)
  To: David S . Miller
  Cc: daniel, netdev, linux-kernel, kernel-team, arnd, yamada.masahiro

check that CC can build executables and use that compiler instead of HOSTCC

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 Makefile               |  5 +++++
 net/Makefile           |  4 ++++
 net/bpfilter/Makefile  |  2 ++
 scripts/cc-can-link.sh | 11 +++++++++++
 4 files changed, 22 insertions(+)
 create mode 100755 scripts/cc-can-link.sh

diff --git a/Makefile b/Makefile
index 56ba070dfa09..62c110084fae 100644
--- a/Makefile
+++ b/Makefile
@@ -510,6 +510,11 @@ ifeq ($(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $
   KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
 endif
 
+ifeq ($(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC)), y)
+  CC_CAN_LINK := y
+  export CC_CAN_LINK
+endif
+
 ifeq ($(config-targets),1)
 # ===========================================================================
 # *config targets only - make sure prerequisites are updated, and descend
diff --git a/net/Makefile b/net/Makefile
index bdaf53925acd..13ec0d5415c7 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -20,7 +20,11 @@ obj-$(CONFIG_TLS)		+= tls/
 obj-$(CONFIG_XFRM)		+= xfrm/
 obj-$(CONFIG_UNIX)		+= unix/
 obj-$(CONFIG_NET)		+= ipv6/
+ifneq ($(CC_CAN_LINK),y)
+$(warning CC cannot link executables. Skipping bpfilter.)
+else
 obj-$(CONFIG_BPFILTER)		+= bpfilter/
+endif
 obj-$(CONFIG_PACKET)		+= packet/
 obj-$(CONFIG_NET_KEY)		+= key/
 obj-$(CONFIG_BRIDGE)		+= bridge/
diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index 3f3cb87c668f..aafa72001fcd 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile
@@ -6,6 +6,8 @@
 hostprogs-y := bpfilter_umh
 bpfilter_umh-objs := main.o
 HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi
+HOSTCC := $(CC)
+
 ifeq ($(CONFIG_BPFILTER_UMH), y)
 # builtin bpfilter_umh should be compiled with -static
 # since rootfs isn't mounted at the time of __init
diff --git a/scripts/cc-can-link.sh b/scripts/cc-can-link.sh
new file mode 100755
index 000000000000..208eb2825dab
--- /dev/null
+++ b/scripts/cc-can-link.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+cat << "END" | $@ -x c - -o /dev/null >/dev/null 2>&1 && echo "y"
+#include <stdio.h>
+int main(void)
+{
+	printf("");
+	return 0;
+}
+END
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next] net/mlx5e: fix error return code in mlx5e_alloc_rq()
From: Wei Yongjun @ 2018-06-05  2:42 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky, Tariq Toukan
  Cc: Wei Yongjun, netdev, linux-rdma, kernel-janitors

Fix to return error code -ENOMEM from the kvzalloc_node() error handling
case instead of 0, as done elsewhere in this function.

Fixes: 069d11465a80 ("net/mlx5e: RX, Enhance legacy Receive Queue memory scheme")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 333d4ed..89c96a0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -566,8 +566,10 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
 			kvzalloc_node((wq_sz << rq->wqe.info.log_num_frags) *
 				      sizeof(*rq->wqe.frags),
 				      GFP_KERNEL, cpu_to_node(c->cpu));
-		if (!rq->wqe.frags)
+		if (!rq->wqe.frags) {
+			err = -ENOMEM;
 			goto err_free;
+		}
 
 		err = mlx5e_init_di_list(rq, params, wq_sz, c->cpu);
 		if (err)

^ permalink raw reply related

* [PATCH net-next] net/mlx5e: Make function mlx5e_change_rep_mtu() static
From: Wei Yongjun @ 2018-06-05  2:42 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky, Adi Nissim
  Cc: Wei Yongjun, netdev, linux-rdma, kernel-janitors

Fixes the following sparse warning:

drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:903:5: warning:
 symbol 'mlx5e_change_rep_mtu' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 3857f22..57987f6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -900,7 +900,7 @@ int mlx5e_get_offload_stats(int attr_id, const struct net_device *dev,
 	.switchdev_port_attr_get	= mlx5e_attr_get,
 };
 
-int mlx5e_change_rep_mtu(struct net_device *netdev, int new_mtu)
+static int mlx5e_change_rep_mtu(struct net_device *netdev, int new_mtu)
 {
 	return mlx5e_change_mtu(netdev, new_mtu, NULL);
 }

^ 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