[dpdk-dev] [RFC 0/4] misc enic fixes

[dpdk-dev] [RFC 0/4] misc enic fixes

[Aaron Conole]

These still need some testing, and I will cc to stable
with the official patch.

I'm especially interested in the comments on base.

Aaron Conole (4):
  enic: update format string to match arg types
  enic: fix assignment
  enic: remove unused code
  enic: remove anscillary assignment

 drivers/net/enic/base/vnic_cq.c  |  14 +----
 drivers/net/enic/base/vnic_dev.c | 110 ++-------------------------------------
 drivers/net/enic/base/vnic_rq.c  |   9 +---
 drivers/net/enic/base/vnic_rss.c |  32 ------------
 drivers/net/enic/base/vnic_wq.c  |   7 +--
 drivers/net/enic/enic.h          |   5 --
 drivers/net/enic/enic_compat.h   |  20 -------
 drivers/net/enic/enic_main.c     |   2 +-
 8 files changed, 10 insertions(+), 189 deletions(-)

-- 
2.9.5

[dpdk-dev] [RFC 1/4] enic: update format string to match arg types

[Aaron Conole]

The argument `index` (and unique_id) is unsigned, but the format
string type used was for signed types.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/enic/base/vnic_cq.c  | 4 ++--
 drivers/net/enic/base/vnic_dev.c | 6 +++---
 drivers/net/enic/base/vnic_rq.c  | 4 ++--
 drivers/net/enic/base/vnic_wq.c  | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/enic/base/vnic_cq.c b/drivers/net/enic/base/vnic_cq.c
index 2f65f35..33db10f 100644
--- a/drivers/net/enic/base/vnic_cq.c
+++ b/drivers/net/enic/base/vnic_cq.c
@@ -65,11 +65,11 @@ int vnic_cq_alloc(struct vnic_dev *vdev, struct vnic_cq *cq, unsigned int index,
 
 	cq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_CQ, index);
 	if (!cq->ctrl) {
-		pr_err("Failed to hook CQ[%d] resource\n", index);
+		pr_err("Failed to hook CQ[%u] resource\n", index);
 		return -EINVAL;
 	}
 
-	snprintf(res_name, sizeof(res_name), "%d-cq-%d", instance++, index);
+	snprintf(res_name, sizeof(res_name), "%d-cq-%u", instance++, index);
 	err = vnic_dev_alloc_desc_ring(vdev, &cq->ring, desc_count, desc_size,
 		socket_id, res_name);
 	if (err)
diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index 49b3655..808f95b 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -650,7 +650,7 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
 
 	if (!vdev->stats) {
 		snprintf((char *)name, sizeof(name),
-			"vnic_stats-%d", instance++);
+			"vnic_stats-%u", instance++);
 		vdev->stats = vdev->alloc_consistent(vdev->priv,
 			sizeof(struct vnic_stats), &vdev->stats_pa, (u8 *)name);
 		if (!vdev->stats)
@@ -900,7 +900,7 @@ int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr)
 	}
 	if (!vnic_dev_in_reset(vdev)) {
 		snprintf((char *)name, sizeof(name),
-			"vnic_notify-%d", instance++);
+			"vnic_notify-%u", instance++);
 		notify_addr = vdev->alloc_consistent(vdev->priv,
 			sizeof(struct vnic_devcmd_notify),
 			&notify_pa, (u8 *)name);
@@ -1150,7 +1150,7 @@ int vnic_dev_classifier(struct vnic_dev *vdev, u8 cmd, u16 *entry,
 		tlv_size = filter_size + action_size +
 		    2*sizeof(struct filter_tlv);
 		snprintf((char *)z_name, sizeof(z_name),
-			"vnic_clsf_%d", unique_id++);
+			"vnic_clsf_%u", unique_id++);
 		tlv_va = vdev->alloc_consistent(vdev->priv,
 			tlv_size, &tlv_pa, (u8 *)z_name);
 		if (!tlv_va)
diff --git a/drivers/net/enic/base/vnic_rq.c b/drivers/net/enic/base/vnic_rq.c
index 10a40c1..776a6f7 100644
--- a/drivers/net/enic/base/vnic_rq.c
+++ b/drivers/net/enic/base/vnic_rq.c
@@ -58,13 +58,13 @@ int vnic_rq_alloc(struct vnic_dev *vdev, struct vnic_rq *rq, unsigned int index,
 
 	rq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_RQ, index);
 	if (!rq->ctrl) {
-		pr_err("Failed to hook RQ[%d] resource\n", index);
+		pr_err("Failed to hook RQ[%u] resource\n", index);
 		return -EINVAL;
 	}
 
 	vnic_rq_disable(rq);
 
-	snprintf(res_name, sizeof(res_name), "%d-rq-%d", instance++, index);
+	snprintf(res_name, sizeof(res_name), "%d-rq-%u", instance++, index);
 	rc = vnic_dev_alloc_desc_ring(vdev, &rq->ring, desc_count, desc_size,
 		rq->socket_id, res_name);
 	return rc;
diff --git a/drivers/net/enic/base/vnic_wq.c b/drivers/net/enic/base/vnic_wq.c
index 7c4119c..273e3db 100644
--- a/drivers/net/enic/base/vnic_wq.c
+++ b/drivers/net/enic/base/vnic_wq.c
@@ -52,7 +52,7 @@ int vnic_wq_alloc_ring(struct vnic_dev *vdev, struct vnic_wq *wq,
 	char res_name[NAME_MAX];
 	static int instance;
 
-	snprintf(res_name, sizeof(res_name), "%d-wq-%d", instance++, wq->index);
+	snprintf(res_name, sizeof(res_name), "%d-wq-%u", instance++, wq->index);
 	return vnic_dev_alloc_desc_ring(vdev, &wq->ring, desc_count, desc_size,
 		wq->socket_id, res_name);
 }
-- 
2.9.5

[dpdk-dev] [RFC 2/4] enic: fix assignment

[Aaron Conole]

As it stands, the existing assingment to mbuf has no effect outside of
the function.  Prior to this change, the mbuf argument would contain
an invalid address, but it would not be null.  After this change, the
caller gets a null mbuf back.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/enic/enic_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 40dbec7..ff8e4c5 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -224,7 +224,7 @@ enic_free_rq_buf(struct rte_mbuf **mbuf)
 		return;
 
 	rte_pktmbuf_free(*mbuf);
-	mbuf = NULL;
+	*mbuf = NULL;
 }
 
 void enic_init_vnic_resources(struct enic *enic)
-- 
2.9.5

[dpdk-dev] [RFC 3/4] enic: remove unused code

[Aaron Conole]

The functions here aren't called anywhere in code, at least according to
both the compiler, and some greps.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/enic/base/vnic_cq.c  |  10 ----
 drivers/net/enic/base/vnic_dev.c | 102 ---------------------------------------
 drivers/net/enic/base/vnic_rq.c  |   5 --
 drivers/net/enic/base/vnic_rss.c |  32 ------------
 drivers/net/enic/base/vnic_wq.c  |   5 --
 drivers/net/enic/enic.h          |   5 --
 drivers/net/enic/enic_compat.h   |  20 --------
 7 files changed, 179 deletions(-)

diff --git a/drivers/net/enic/base/vnic_cq.c b/drivers/net/enic/base/vnic_cq.c
index 33db10f..3549fec 100644
--- a/drivers/net/enic/base/vnic_cq.c
+++ b/drivers/net/enic/base/vnic_cq.c
@@ -35,16 +35,6 @@
 #include "vnic_dev.h"
 #include "vnic_cq.h"
 
-int vnic_cq_mem_size(struct vnic_cq *cq, unsigned int desc_count,
-	unsigned int desc_size)
-{
-	int mem_size;
-
-	mem_size = vnic_dev_desc_ring_size(&cq->ring, desc_count, desc_size);
-
-	return mem_size;
-}
-
 void vnic_cq_free(struct vnic_cq *cq)
 {
 	vnic_dev_free_desc_ring(cq->vdev, &cq->ring);
diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index 808f95b..a4b7f4b 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -443,24 +443,6 @@ static int vnic_dev_cmd_no_proxy(struct vnic_dev *vdev,
 	return err;
 }
 
-void vnic_dev_cmd_proxy_by_index_start(struct vnic_dev *vdev, u16 index)
-{
-	vdev->proxy = PROXY_BY_INDEX;
-	vdev->proxy_index = index;
-}
-
-void vnic_dev_cmd_proxy_by_bdf_start(struct vnic_dev *vdev, u16 bdf)
-{
-	vdev->proxy = PROXY_BY_BDF;
-	vdev->proxy_index = bdf;
-}
-
-void vnic_dev_cmd_proxy_end(struct vnic_dev *vdev)
-{
-	vdev->proxy = PROXY_NONE;
-	vdev->proxy_index = 0;
-}
-
 int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 	u64 *a0, u64 *a1, int wait)
 {
@@ -672,15 +654,6 @@ int vnic_dev_close(struct vnic_dev *vdev)
 	return vnic_dev_cmd(vdev, CMD_CLOSE, &a0, &a1, wait);
 }
 
-/** Deprecated.  @see vnic_dev_enable_wait */
-int vnic_dev_enable(struct vnic_dev *vdev)
-{
-	u64 a0 = 0, a1 = 0;
-	int wait = 1000;
-
-	return vnic_dev_cmd(vdev, CMD_ENABLE, &a0, &a1, wait);
-}
-
 int vnic_dev_enable_wait(struct vnic_dev *vdev)
 {
 	u64 a0 = 0, a1 = 0;
@@ -725,31 +698,6 @@ int vnic_dev_open_done(struct vnic_dev *vdev, int *done)
 	return 0;
 }
 
-int vnic_dev_soft_reset(struct vnic_dev *vdev, int arg)
-{
-	u64 a0 = (u32)arg, a1 = 0;
-	int wait = 1000;
-
-	return vnic_dev_cmd(vdev, CMD_SOFT_RESET, &a0, &a1, wait);
-}
-
-int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done)
-{
-	u64 a0 = 0, a1 = 0;
-	int wait = 1000;
-	int err;
-
-	*done = 0;
-
-	err = vnic_dev_cmd(vdev, CMD_SOFT_RESET_STATUS, &a0, &a1, wait);
-	if (err)
-		return err;
-
-	*done = (a0 == 0);
-
-	return 0;
-}
-
 int vnic_dev_get_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
 {
 	u64 a0 = 0, a1 = 0;
@@ -840,19 +788,6 @@ int vnic_dev_set_ig_vlan_rewrite_mode(struct vnic_dev *vdev,
 		return 0;
 }
 
-int vnic_dev_raise_intr(struct vnic_dev *vdev, u16 intr)
-{
-	u64 a0 = intr, a1 = 0;
-	int wait = 1000;
-	int err;
-
-	err = vnic_dev_cmd(vdev, CMD_IAR, &a0, &a1, wait);
-	if (err)
-		pr_err("Failed to raise INTR[%d], err %d\n", intr, err);
-
-	return err;
-}
-
 void vnic_dev_set_reset_flag(struct vnic_dev *vdev, int state)
 {
 	vdev->in_reset = state;
@@ -985,14 +920,6 @@ int vnic_dev_init(struct vnic_dev *vdev, int arg)
 	return r;
 }
 
-int vnic_dev_deinit(struct vnic_dev *vdev)
-{
-	u64 a0 = 0, a1 = 0;
-	int wait = 1000;
-
-	return vnic_dev_cmd(vdev, CMD_DEINIT, &a0, &a1, wait);
-}
-
 void vnic_dev_intr_coal_timer_info_default(struct vnic_dev *vdev)
 {
 	/* Default: hardware intr coal timer is in units of 1.5 usecs */
@@ -1018,18 +945,6 @@ u32 vnic_dev_port_speed(struct vnic_dev *vdev)
 	return vdev->notify_copy.port_speed;
 }
 
-void vnic_dev_set_intr_mode(struct vnic_dev *vdev,
-	enum vnic_dev_intr_mode intr_mode)
-{
-	vdev->intr_mode = intr_mode;
-}
-
-enum vnic_dev_intr_mode vnic_dev_get_intr_mode(
-	struct vnic_dev *vdev)
-{
-	return vdev->intr_mode;
-}
-
 u32 vnic_dev_intr_coal_timer_usec_to_hw(struct vnic_dev *vdev, u32 usec)
 {
 	return (usec * vdev->intr_coal_timer_info.mul) /
@@ -1094,23 +1009,6 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
 	return NULL;
 }
 
-struct rte_pci_device *vnic_dev_get_pdev(struct vnic_dev *vdev)
-{
-	return vdev->pdev;
-}
-
-int vnic_dev_set_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
-{
-	u64 a0, a1 = 0;
-	int wait = 1000;
-	int i;
-
-	for (i = 0; i < ETH_ALEN; i++)
-		((u8 *)&a0)[i] = mac_addr[i];
-
-	return vnic_dev_cmd(vdev, CMD_SET_MAC_ADDR, &a0, &a1, wait);
-}
-
 /*
  *  vnic_dev_classifier: Add/Delete classifier entries
  *  @vdev: vdev of the device
diff --git a/drivers/net/enic/base/vnic_rq.c b/drivers/net/enic/base/vnic_rq.c
index 776a6f7..ea297ee 100644
--- a/drivers/net/enic/base/vnic_rq.c
+++ b/drivers/net/enic/base/vnic_rq.c
@@ -118,11 +118,6 @@ void vnic_rq_init(struct vnic_rq *rq, unsigned int cq_index,
 	rq->pkt_last_seg = NULL;
 }
 
-void vnic_rq_error_out(struct vnic_rq *rq, unsigned int error)
-{
-	iowrite32(error, &rq->ctrl->error_status);
-}
-
 unsigned int vnic_rq_error_status(struct vnic_rq *rq)
 {
 	return ioread32(&rq->ctrl->error_status);
diff --git a/drivers/net/enic/base/vnic_rss.c b/drivers/net/enic/base/vnic_rss.c
index 1cf055b..87d40c0 100644
--- a/drivers/net/enic/base/vnic_rss.c
+++ b/drivers/net/enic/base/vnic_rss.c
@@ -50,35 +50,3 @@ void vnic_set_rss_key(union vnic_rss_key *rss_key, u8 *key)
 	}
 }
 
-void vnic_set_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu)
-{
-	u32 i;
-	u32 *p = (u32 *)cpu;
-
-	for (i = 0; i < 32; ++i)
-		iowrite32(*p++, &rss_cpu->cpu[i].b[0]);
-}
-
-void vnic_get_rss_key(union vnic_rss_key *rss_key, u8 *key)
-{
-	u32 i;
-	u32 *p;
-	u16 *q;
-
-	for (i = 0; i < 4; ++i) {
-		p = (u32 *)(key + (10 * i));
-		*p++ = ioread32(&rss_key->key[i].b[0]);
-		*p++ = ioread32(&rss_key->key[i].b[4]);
-		q = (u16 *)p;
-		*q = (u16)ioread32(&rss_key->key[i].b[8]);
-	}
-}
-
-void vnic_get_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu)
-{
-	u32 i;
-	u32 *p = (u32 *)cpu;
-
-	for (i = 0; i < 32; ++i)
-		*p++ = ioread32(&rss_cpu->cpu[i].b[0]);
-}
diff --git a/drivers/net/enic/base/vnic_wq.c b/drivers/net/enic/base/vnic_wq.c
index 273e3db..0a1247f 100644
--- a/drivers/net/enic/base/vnic_wq.c
+++ b/drivers/net/enic/base/vnic_wq.c
@@ -145,11 +145,6 @@ void vnic_wq_init(struct vnic_wq *wq, unsigned int cq_index,
 	wq->last_completed_index = 0;
 }
 
-void vnic_wq_error_out(struct vnic_wq *wq, unsigned int error)
-{
-	iowrite32(error, &wq->ctrl->error_status);
-}
-
 unsigned int vnic_wq_error_status(struct vnic_wq *wq)
 {
 	return ioread32(&wq->ctrl->error_status);
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index e28f223..9dc86df 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -227,11 +227,6 @@ static inline unsigned int enic_cq_wq(struct enic *enic, unsigned int wq)
 	return enic->rq_count + wq;
 }
 
-static inline unsigned int enic_msix_err_intr(__rte_unused struct enic *enic)
-{
-	return 0;
-}
-
 static inline struct enic *pmd_priv(struct rte_eth_dev *eth_dev)
 {
 	return (struct enic *)eth_dev->data->dev_private;
diff --git a/drivers/net/enic/enic_compat.h b/drivers/net/enic/enic_compat.h
index fc58bb4..1cb5686 100644
--- a/drivers/net/enic/enic_compat.h
+++ b/drivers/net/enic/enic_compat.h
@@ -99,11 +99,6 @@ static inline uint32_t ioread32(volatile void *addr)
 	return rte_read32(addr);
 }
 
-static inline uint16_t ioread16(volatile void *addr)
-{
-	return rte_read16(addr);
-}
-
 static inline uint8_t ioread8(volatile void *addr)
 {
 	return rte_read8(addr);
@@ -119,26 +114,11 @@ static inline void iowrite32_relaxed(uint32_t val, volatile void *addr)
 	rte_write32_relaxed(val, addr);
 }
 
-static inline void iowrite16(uint16_t val, volatile void *addr)
-{
-	rte_write16(val, addr);
-}
-
-static inline void iowrite8(uint8_t val, volatile void *addr)
-{
-	rte_write8(val, addr);
-}
-
 static inline unsigned int readl(volatile void __iomem *addr)
 {
 	return rte_read32(addr);
 }
 
-static inline unsigned int readl_relaxed(volatile void __iomem *addr)
-{
-	return rte_read32_relaxed(addr);
-}
-
 static inline void writel(unsigned int val, volatile void __iomem *addr)
 {
 	rte_write32(val, addr);
-- 
2.9.5

[dpdk-dev] [RFC 4/4] enic: remove anscillary assignment

[Aaron Conole]

The assignment at initialization is overwritten immediately.  Drop the
assignment.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/enic/base/vnic_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index a4b7f4b..932667f 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -272,7 +272,7 @@ int vnic_dev_alloc_desc_ring(struct vnic_dev *vdev,
 	__attribute__((unused)) unsigned int socket_id,
 	char *z_name)
 {
-	void *alloc_addr = NULL;
+	void *alloc_addr;
 	dma_addr_t alloc_pa = 0;
 
 	vnic_dev_desc_ring_size(ring, desc_count, desc_size);
-- 
2.9.5

Re: [dpdk-dev] [RFC 0/4] misc enic fixes

[Ferruh Yigit]

On 9/26/2017 7:53 PM, Aaron Conole wrote:
> These still need some testing, and I will cc to stable
> with the official patch.
> 
> I'm especially interested in the comments on base.
> 
> Aaron Conole (4):
>   enic: update format string to match arg types
>   enic: fix assignment
>   enic: remove unused code
>   enic: remove anscillary assignment

Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

And please add the maintainers to cc next time.

Since this is RFC, I assume you will send v1 of the patchset, when
sending v1 can you please add "Fixes:" and "Cc: stable@dpdk.org" tags in
commit log for fixes, these assure patches will be picked by stable tree.


Thanks,
ferruh

Re: [dpdk-dev] [RFC 0/4] misc enic fixes

[Aaron Conole]

Ferruh Yigit <ferruh.yigit@intel.com> writes:

> On 9/26/2017 7:53 PM, Aaron Conole wrote:
>> These still need some testing, and I will cc to stable
>> with the official patch.
>> 
>> I'm especially interested in the comments on base.
>> 
>> Aaron Conole (4):
>>   enic: update format string to match arg types
>>   enic: fix assignment
>>   enic: remove unused code
>>   enic: remove anscillary assignment
>
> Series Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Thanks for the review, Ferruh!

> And please add the maintainers to cc next time.

Sorry about that.  It was sent in haste with John standing next to me -
I'll make sure not to repeat the mistake.

> Since this is RFC, I assume you will send v1 of the patchset, when
> sending v1 can you please add "Fixes:" and "Cc: stable@dpdk.org" tags in
> commit log for fixes, these assure patches will be picked by stable tree.

Will do.  John wanted some early feedback opportunity, so happy to
oblige :)

>
> Thanks,
> ferruh

Re: [dpdk-dev] [RFC 1/4] enic: update format string to match arg types

[John Daley (johndale)]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> Sent: Tuesday, September 26, 2017 11:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 1/4] enic: update format string to match arg types
> 
> The argument `index` (and unique_id) is unsigned, but the format string type
> used was for signed types.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/enic/base/vnic_cq.c  | 4 ++--  drivers/net/enic/base/vnic_dev.c
> | 6 +++---  drivers/net/enic/base/vnic_rq.c  | 4 ++--
> drivers/net/enic/base/vnic_wq.c  | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/enic/base/vnic_cq.c
> b/drivers/net/enic/base/vnic_cq.c index 2f65f35..33db10f 100644
> --- a/drivers/net/enic/base/vnic_cq.c
> +++ b/drivers/net/enic/base/vnic_cq.c
> @@ -65,11 +65,11 @@ int vnic_cq_alloc(struct vnic_dev *vdev, struct
> vnic_cq *cq, unsigned int index,
> 
>  	cq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_CQ, index);
>  	if (!cq->ctrl) {
> -		pr_err("Failed to hook CQ[%d] resource\n", index);
> +		pr_err("Failed to hook CQ[%u] resource\n", index);
>  		return -EINVAL;
>  	}
> 
> -	snprintf(res_name, sizeof(res_name), "%d-cq-%d", instance++,
> index);
> +	snprintf(res_name, sizeof(res_name), "%d-cq-%u", instance++,
> index);
>  	err = vnic_dev_alloc_desc_ring(vdev, &cq->ring, desc_count,
> desc_size,
>  		socket_id, res_name);
>  	if (err)
> diff --git a/drivers/net/enic/base/vnic_dev.c
> b/drivers/net/enic/base/vnic_dev.c
> index 49b3655..808f95b 100644
> --- a/drivers/net/enic/base/vnic_dev.c
> +++ b/drivers/net/enic/base/vnic_dev.c
> @@ -650,7 +650,7 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev,
> struct vnic_stats **stats)
> 
>  	if (!vdev->stats) {
>  		snprintf((char *)name, sizeof(name),
> -			"vnic_stats-%d", instance++);
> +			"vnic_stats-%u", instance++);
>  		vdev->stats = vdev->alloc_consistent(vdev->priv,
>  			sizeof(struct vnic_stats), &vdev->stats_pa, (u8
> *)name);
>  		if (!vdev->stats)
> @@ -900,7 +900,7 @@ int vnic_dev_notify_set(struct vnic_dev *vdev, u16
> intr)
>  	}
>  	if (!vnic_dev_in_reset(vdev)) {
>  		snprintf((char *)name, sizeof(name),
> -			"vnic_notify-%d", instance++);
> +			"vnic_notify-%u", instance++);
>  		notify_addr = vdev->alloc_consistent(vdev->priv,
>  			sizeof(struct vnic_devcmd_notify),
>  			&notify_pa, (u8 *)name);
> @@ -1150,7 +1150,7 @@ int vnic_dev_classifier(struct vnic_dev *vdev, u8
> cmd, u16 *entry,
>  		tlv_size = filter_size + action_size +
>  		    2*sizeof(struct filter_tlv);
>  		snprintf((char *)z_name, sizeof(z_name),
> -			"vnic_clsf_%d", unique_id++);
> +			"vnic_clsf_%u", unique_id++);
>  		tlv_va = vdev->alloc_consistent(vdev->priv,
>  			tlv_size, &tlv_pa, (u8 *)z_name);
>  		if (!tlv_va)
> diff --git a/drivers/net/enic/base/vnic_rq.c
> b/drivers/net/enic/base/vnic_rq.c index 10a40c1..776a6f7 100644
> --- a/drivers/net/enic/base/vnic_rq.c
> +++ b/drivers/net/enic/base/vnic_rq.c
> @@ -58,13 +58,13 @@ int vnic_rq_alloc(struct vnic_dev *vdev, struct vnic_rq
> *rq, unsigned int index,
> 
>  	rq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_RQ, index);
>  	if (!rq->ctrl) {
> -		pr_err("Failed to hook RQ[%d] resource\n", index);
> +		pr_err("Failed to hook RQ[%u] resource\n", index);
>  		return -EINVAL;
>  	}
> 
>  	vnic_rq_disable(rq);
> 
> -	snprintf(res_name, sizeof(res_name), "%d-rq-%d", instance++,
> index);
> +	snprintf(res_name, sizeof(res_name), "%d-rq-%u", instance++,
> index);
>  	rc = vnic_dev_alloc_desc_ring(vdev, &rq->ring, desc_count,
> desc_size,
>  		rq->socket_id, res_name);
>  	return rc;
> diff --git a/drivers/net/enic/base/vnic_wq.c
> b/drivers/net/enic/base/vnic_wq.c index 7c4119c..273e3db 100644
> --- a/drivers/net/enic/base/vnic_wq.c
> +++ b/drivers/net/enic/base/vnic_wq.c
> @@ -52,7 +52,7 @@ int vnic_wq_alloc_ring(struct vnic_dev *vdev, struct
> vnic_wq *wq,
>  	char res_name[NAME_MAX];
>  	static int instance;
> 
> -	snprintf(res_name, sizeof(res_name), "%d-wq-%d", instance++, wq-
> >index);
> +	snprintf(res_name, sizeof(res_name), "%d-wq-%u", instance++,
> +wq->index);
>  	return vnic_dev_alloc_desc_ring(vdev, &wq->ring, desc_count,
> desc_size,
>  		wq->socket_id, res_name);
>  }
> --
> 2.9.5

Reviewed-by: John Daley <johndale@cisco.com>

Thanks,
Johnd

Re: [dpdk-dev] [RFC 2/4] enic: fix assignment

[John Daley (johndale)]

Aaron,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> Sent: Tuesday, September 26, 2017 11:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 2/4] enic: fix assignment
> 
> As it stands, the existing assingment to mbuf has no effect outside of the
> function.  Prior to this change, the mbuf argument would contain an invalid
> address, but it would not be null.  After this change, the caller gets a null
> mbuf back.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/enic/enic_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 40dbec7..ff8e4c5 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -224,7 +224,7 @@ enic_free_rq_buf(struct rte_mbuf **mbuf)
>  		return;
> 
>  	rte_pktmbuf_free(*mbuf);
> -	mbuf = NULL;
> +	*mbuf = NULL;
>  }
> 
>  void enic_init_vnic_resources(struct enic *enic)

As it turns out, this function is only called when the device is stopped and restarting the device overwrites the mbuf pointers with newly allocated ones, so there is currently no bad behavior. The intent was to NULL out the pointer though and it's certainly better form so I agree with the change.

Reviewed-by: John Daley <johndale@cisco.com>

Thanks,
Johnd
> --
> 2.9.5

Re: [dpdk-dev] [RFC 3/4] enic: remove unused code

[John Daley (johndale)]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> Sent: Tuesday, September 26, 2017 11:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 3/4] enic: remove unused code
> 
> The functions here aren't called anywhere in code, at least according to both
> the compiler, and some greps.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/enic/base/vnic_cq.c  |  10 ----  drivers/net/enic/base/vnic_dev.c
> | 102 ---------------------------------------
>  drivers/net/enic/base/vnic_rq.c  |   5 --
>  drivers/net/enic/base/vnic_rss.c |  32 ------------
>  drivers/net/enic/base/vnic_wq.c  |   5 --
>  drivers/net/enic/enic.h          |   5 --
>  drivers/net/enic/enic_compat.h   |  20 --------
>  7 files changed, 179 deletions(-)
> 
> diff --git a/drivers/net/enic/base/vnic_cq.c
> b/drivers/net/enic/base/vnic_cq.c index 33db10f..3549fec 100644
> --- a/drivers/net/enic/base/vnic_cq.c
> +++ b/drivers/net/enic/base/vnic_cq.c
> @@ -35,16 +35,6 @@
>  #include "vnic_dev.h"
>  #include "vnic_cq.h"
> 
> -int vnic_cq_mem_size(struct vnic_cq *cq, unsigned int desc_count,
> -	unsigned int desc_size)
> -{
> -	int mem_size;
> -
> -	mem_size = vnic_dev_desc_ring_size(&cq->ring, desc_count,
> desc_size);
> -
> -	return mem_size;
> -}
> -
>  void vnic_cq_free(struct vnic_cq *cq)
>  {
>  	vnic_dev_free_desc_ring(cq->vdev, &cq->ring); diff --git
> a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
> index 808f95b..a4b7f4b 100644
> --- a/drivers/net/enic/base/vnic_dev.c
> +++ b/drivers/net/enic/base/vnic_dev.c
> @@ -443,24 +443,6 @@ static int vnic_dev_cmd_no_proxy(struct vnic_dev
> *vdev,
>  	return err;
>  }
> 
> -void vnic_dev_cmd_proxy_by_index_start(struct vnic_dev *vdev, u16
> index) -{
> -	vdev->proxy = PROXY_BY_INDEX;
> -	vdev->proxy_index = index;
> -}
> -
> -void vnic_dev_cmd_proxy_by_bdf_start(struct vnic_dev *vdev, u16 bdf) -{
> -	vdev->proxy = PROXY_BY_BDF;
> -	vdev->proxy_index = bdf;
> -}
> -
> -void vnic_dev_cmd_proxy_end(struct vnic_dev *vdev) -{
> -	vdev->proxy = PROXY_NONE;
> -	vdev->proxy_index = 0;
> -}
> -
>  int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
>  	u64 *a0, u64 *a1, int wait)
>  {
> @@ -672,15 +654,6 @@ int vnic_dev_close(struct vnic_dev *vdev)
>  	return vnic_dev_cmd(vdev, CMD_CLOSE, &a0, &a1, wait);  }
> 
> -/** Deprecated.  @see vnic_dev_enable_wait */ -int
> vnic_dev_enable(struct vnic_dev *vdev) -{
> -	u64 a0 = 0, a1 = 0;
> -	int wait = 1000;
> -
> -	return vnic_dev_cmd(vdev, CMD_ENABLE, &a0, &a1, wait);
> -}
> -
>  int vnic_dev_enable_wait(struct vnic_dev *vdev)  {
>  	u64 a0 = 0, a1 = 0;
> @@ -725,31 +698,6 @@ int vnic_dev_open_done(struct vnic_dev *vdev, int
> *done)
>  	return 0;
>  }
> 
> -int vnic_dev_soft_reset(struct vnic_dev *vdev, int arg) -{
> -	u64 a0 = (u32)arg, a1 = 0;
> -	int wait = 1000;
> -
> -	return vnic_dev_cmd(vdev, CMD_SOFT_RESET, &a0, &a1, wait);
> -}
> -
> -int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done) -{
> -	u64 a0 = 0, a1 = 0;
> -	int wait = 1000;
> -	int err;
> -
> -	*done = 0;
> -
> -	err = vnic_dev_cmd(vdev, CMD_SOFT_RESET_STATUS, &a0, &a1,
> wait);
> -	if (err)
> -		return err;
> -
> -	*done = (a0 == 0);
> -
> -	return 0;
> -}
> -
>  int vnic_dev_get_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)  {
>  	u64 a0 = 0, a1 = 0;
> @@ -840,19 +788,6 @@ int vnic_dev_set_ig_vlan_rewrite_mode(struct
> vnic_dev *vdev,
>  		return 0;
>  }
> 
> -int vnic_dev_raise_intr(struct vnic_dev *vdev, u16 intr) -{
> -	u64 a0 = intr, a1 = 0;
> -	int wait = 1000;
> -	int err;
> -
> -	err = vnic_dev_cmd(vdev, CMD_IAR, &a0, &a1, wait);
> -	if (err)
> -		pr_err("Failed to raise INTR[%d], err %d\n", intr, err);
> -
> -	return err;
> -}
> -
>  void vnic_dev_set_reset_flag(struct vnic_dev *vdev, int state)  {
>  	vdev->in_reset = state;
> @@ -985,14 +920,6 @@ int vnic_dev_init(struct vnic_dev *vdev, int arg)
>  	return r;
>  }
> 
> -int vnic_dev_deinit(struct vnic_dev *vdev) -{
> -	u64 a0 = 0, a1 = 0;
> -	int wait = 1000;
> -
> -	return vnic_dev_cmd(vdev, CMD_DEINIT, &a0, &a1, wait);
> -}
> -
>  void vnic_dev_intr_coal_timer_info_default(struct vnic_dev *vdev)  {
>  	/* Default: hardware intr coal timer is in units of 1.5 usecs */ @@ -
> 1018,18 +945,6 @@ u32 vnic_dev_port_speed(struct vnic_dev *vdev)
>  	return vdev->notify_copy.port_speed;
>  }
> 
> -void vnic_dev_set_intr_mode(struct vnic_dev *vdev,
> -	enum vnic_dev_intr_mode intr_mode)
> -{
> -	vdev->intr_mode = intr_mode;
> -}
> -
> -enum vnic_dev_intr_mode vnic_dev_get_intr_mode(
> -	struct vnic_dev *vdev)
> -{
> -	return vdev->intr_mode;
> -}
> -
>  u32 vnic_dev_intr_coal_timer_usec_to_hw(struct vnic_dev *vdev, u32
> usec)  {
>  	return (usec * vdev->intr_coal_timer_info.mul) / @@ -1094,23
> +1009,6 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
>  	return NULL;
>  }
> 
> -struct rte_pci_device *vnic_dev_get_pdev(struct vnic_dev *vdev) -{
> -	return vdev->pdev;
> -}
> -
> -int vnic_dev_set_mac_addr(struct vnic_dev *vdev, u8 *mac_addr) -{
> -	u64 a0, a1 = 0;
> -	int wait = 1000;
> -	int i;
> -
> -	for (i = 0; i < ETH_ALEN; i++)
> -		((u8 *)&a0)[i] = mac_addr[i];
> -
> -	return vnic_dev_cmd(vdev, CMD_SET_MAC_ADDR, &a0, &a1, wait);
> -}
> -
>  /*
>   *  vnic_dev_classifier: Add/Delete classifier entries
>   *  @vdev: vdev of the device
> diff --git a/drivers/net/enic/base/vnic_rq.c
> b/drivers/net/enic/base/vnic_rq.c index 776a6f7..ea297ee 100644
> --- a/drivers/net/enic/base/vnic_rq.c
> +++ b/drivers/net/enic/base/vnic_rq.c
> @@ -118,11 +118,6 @@ void vnic_rq_init(struct vnic_rq *rq, unsigned int
> cq_index,
>  	rq->pkt_last_seg = NULL;
>  }
> 
> -void vnic_rq_error_out(struct vnic_rq *rq, unsigned int error) -{
> -	iowrite32(error, &rq->ctrl->error_status);
> -}
> -
>  unsigned int vnic_rq_error_status(struct vnic_rq *rq)  {
>  	return ioread32(&rq->ctrl->error_status);
> diff --git a/drivers/net/enic/base/vnic_rss.c
> b/drivers/net/enic/base/vnic_rss.c
> index 1cf055b..87d40c0 100644
> --- a/drivers/net/enic/base/vnic_rss.c
> +++ b/drivers/net/enic/base/vnic_rss.c
> @@ -50,35 +50,3 @@ void vnic_set_rss_key(union vnic_rss_key *rss_key,
> u8 *key)
>  	}
>  }
> 
> -void vnic_set_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu) -{
> -	u32 i;
> -	u32 *p = (u32 *)cpu;
> -
> -	for (i = 0; i < 32; ++i)
> -		iowrite32(*p++, &rss_cpu->cpu[i].b[0]);
> -}
> -
> -void vnic_get_rss_key(union vnic_rss_key *rss_key, u8 *key) -{
> -	u32 i;
> -	u32 *p;
> -	u16 *q;
> -
> -	for (i = 0; i < 4; ++i) {
> -		p = (u32 *)(key + (10 * i));
> -		*p++ = ioread32(&rss_key->key[i].b[0]);
> -		*p++ = ioread32(&rss_key->key[i].b[4]);
> -		q = (u16 *)p;
> -		*q = (u16)ioread32(&rss_key->key[i].b[8]);
> -	}
> -}
> -
> -void vnic_get_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu) -{
> -	u32 i;
> -	u32 *p = (u32 *)cpu;
> -
> -	for (i = 0; i < 32; ++i)
> -		*p++ = ioread32(&rss_cpu->cpu[i].b[0]);
> -}
> diff --git a/drivers/net/enic/base/vnic_wq.c
> b/drivers/net/enic/base/vnic_wq.c index 273e3db..0a1247f 100644
> --- a/drivers/net/enic/base/vnic_wq.c
> +++ b/drivers/net/enic/base/vnic_wq.c
> @@ -145,11 +145,6 @@ void vnic_wq_init(struct vnic_wq *wq, unsigned int
> cq_index,
>  	wq->last_completed_index = 0;
>  }
> 
> -void vnic_wq_error_out(struct vnic_wq *wq, unsigned int error) -{
> -	iowrite32(error, &wq->ctrl->error_status);
> -}
> -
>  unsigned int vnic_wq_error_status(struct vnic_wq *wq)  {
>  	return ioread32(&wq->ctrl->error_status);
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h index
> e28f223..9dc86df 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -227,11 +227,6 @@ static inline unsigned int enic_cq_wq(struct enic
> *enic, unsigned int wq)
>  	return enic->rq_count + wq;
>  }
> 
> -static inline unsigned int enic_msix_err_intr(__rte_unused struct enic *enic)
> -{
> -	return 0;
> -}
> -
>  static inline struct enic *pmd_priv(struct rte_eth_dev *eth_dev)  {
>  	return (struct enic *)eth_dev->data->dev_private; diff --git
> a/drivers/net/enic/enic_compat.h b/drivers/net/enic/enic_compat.h index
> fc58bb4..1cb5686 100644
> --- a/drivers/net/enic/enic_compat.h
> +++ b/drivers/net/enic/enic_compat.h
> @@ -99,11 +99,6 @@ static inline uint32_t ioread32(volatile void *addr)
>  	return rte_read32(addr);
>  }
> 
> -static inline uint16_t ioread16(volatile void *addr) -{
> -	return rte_read16(addr);
> -}
> -
>  static inline uint8_t ioread8(volatile void *addr)  {
>  	return rte_read8(addr);
> @@ -119,26 +114,11 @@ static inline void iowrite32_relaxed(uint32_t val,
> volatile void *addr)
>  	rte_write32_relaxed(val, addr);
>  }
> 
> -static inline void iowrite16(uint16_t val, volatile void *addr) -{
> -	rte_write16(val, addr);
> -}
> -
> -static inline void iowrite8(uint8_t val, volatile void *addr) -{
> -	rte_write8(val, addr);
> -}
> -
>  static inline unsigned int readl(volatile void __iomem *addr)  {
>  	return rte_read32(addr);
>  }
> 
> -static inline unsigned int readl_relaxed(volatile void __iomem *addr) -{
> -	return rte_read32_relaxed(addr);
> -}
> -
>  static inline void writel(unsigned int val, volatile void __iomem *addr)  {
>  	rte_write32(val, addr);
> --
> 2.9.5

The files in enic/base were initially common with other Cisco VIC drivers, but have since diverged and are maintained separately for DPDK, so I don't have any problem with removing unused functions at this point. 

Reviewed-by: John Daley <johndale@cisco.com>

Thanks!
Johnd

Re: [dpdk-dev] [RFC 4/4] enic: remove anscillary assignment

[John Daley (johndale)]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> Sent: Tuesday, September 26, 2017 11:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 4/4] enic: remove anscillary assignment
> 
> The assignment at initialization is overwritten immediately.  Drop the
> assignment.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/enic/base/vnic_dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/enic/base/vnic_dev.c
> b/drivers/net/enic/base/vnic_dev.c
> index a4b7f4b..932667f 100644
> --- a/drivers/net/enic/base/vnic_dev.c
> +++ b/drivers/net/enic/base/vnic_dev.c
> @@ -272,7 +272,7 @@ int vnic_dev_alloc_desc_ring(struct vnic_dev *vdev,
>  	__attribute__((unused)) unsigned int socket_id,
>  	char *z_name)
>  {
> -	void *alloc_addr = NULL;
> +	void *alloc_addr;
>  	dma_addr_t alloc_pa = 0;
> 
>  	vnic_dev_desc_ring_size(ring, desc_count, desc_size);
> --
> 2.9.5

Reviewed-by: John Daley <johndale@cisco.com>

Thanks,
Johnd

Re: [dpdk-dev] [RFC 0/4] misc enic fixes

[Ferruh Yigit]

On 9/26/2017 7:53 PM, Aaron Conole wrote:
> These still need some testing, and I will cc to stable
> with the official patch.
> 
> I'm especially interested in the comments on base.
> 
> Aaron Conole (4):
>   enic: update format string to match arg types
>   enic: fix assignment
>   enic: remove unused code
>   enic: remove anscillary assignment

Series applied to dpdk-next-net/master, thanks.