[dpdk-dev] net/enic: fix MAC address add and remove

Message ID 20170126201212.31796-1-johndale@cisco.com
State Accepted, archived
Delegated to: Ferruh Yigit
Headers show

Checks

Context Check Description
ci/Intel compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

John Daley (johndale) Jan. 26, 2017, 8:12 p.m.
The mac_addr_add callback function was simply replacing the primary MAC
address instead of adding new ones and the mac_addr_remove callback would
only remove the primary MAC form the adapter. Fix the functions to add or
remove new address. Allow up to 64 MAC addresses per port.

Fixes: fefed3d1e62c ("enic: new driver")

Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
---
 drivers/net/enic/enic.h        |  5 +++--
 drivers/net/enic/enic_ethdev.c |  6 +++---
 drivers/net/enic/enic_main.c   | 21 ++++++++-------------
 3 files changed, 14 insertions(+), 18 deletions(-)

Comments

Ferruh Yigit Jan. 27, 2017, 11:06 a.m. | #1
On 1/26/2017 8:12 PM, John Daley wrote:
> The mac_addr_add callback function was simply replacing the primary MAC
> address instead of adding new ones and the mac_addr_remove callback would
> only remove the primary MAC form the adapter. Fix the functions to add or
> remove new address. Allow up to 64 MAC addresses per port.
> 
> Fixes: fefed3d1e62c ("enic: new driver")
> 
> Signed-off-by: John Daley <johndale@cisco.com>
> Reviewed-by: Nelson Escobar <neescoba@cisco.com>

Applied to dpdk-next-net/master, thanks.

(Since it is controversial that if this patch is a fix or implementing a
missing feature (allow up to 64 MAC), I am getting patch as it is, but
not CC'ed to stable tree, please shout if you disagree.)
John Daley (johndale) Jan. 27, 2017, 7:04 p.m. | #2
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Friday, January 27, 2017 3:07 AM
> To: John Daley (johndale) <johndale@cisco.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] net/enic: fix MAC address add and remove
> 
> On 1/26/2017 8:12 PM, John Daley wrote:
> > The mac_addr_add callback function was simply replacing the primary
> > MAC address instead of adding new ones and the mac_addr_remove
> > callback would only remove the primary MAC form the adapter. Fix the
> > functions to add or remove new address. Allow up to 64 MAC addresses
> per port.
> >
> > Fixes: fefed3d1e62c ("enic: new driver")
> >
> > Signed-off-by: John Daley <johndale@cisco.com>
> > Reviewed-by: Nelson Escobar <neescoba@cisco.com>
> 
> Applied to dpdk-next-net/master, thanks.
> 
> (Since it is controversial that if this patch is a fix or implementing a missing
> feature (allow up to 64 MAC), I am getting patch as it is, but not CC'ed to
> stable tree, please shout if you disagree.)

That's fine to not put it on stable. Thank you for applying.
-john

Patch

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 7ff994b..a454017 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -60,6 +60,7 @@ 
 #define ENIC_RQ_MAX		16
 #define ENIC_CQ_MAX		(ENIC_WQ_MAX + (ENIC_RQ_MAX / 2))
 #define ENIC_INTR_MAX		(ENIC_CQ_MAX + 2)
+#define ENIC_MAX_MAC_ADDR	64
 
 #define VLAN_ETH_HLEN           18
 
@@ -277,8 +278,8 @@  extern void enic_dev_stats_get(struct enic *enic,
 	struct rte_eth_stats *r_stats);
 extern void enic_dev_stats_clear(struct enic *enic);
 extern void enic_add_packet_filter(struct enic *enic);
-extern void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
-extern void enic_del_mac_address(struct enic *enic);
+void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
+void enic_del_mac_address(struct enic *enic, int mac_index);
 extern unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
 extern void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
 			  struct rte_mbuf *tx_pkt, unsigned short len,
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index c3ba2aa..bffa870 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -465,7 +465,7 @@  static void enicpmd_dev_info_get(struct rte_eth_dev *eth_dev,
 	device_info->max_tx_queues = enic->conf_wq_count;
 	device_info->min_rx_bufsize = ENIC_MIN_MTU;
 	device_info->max_rx_pktlen = enic->max_mtu + ETHER_HDR_LEN + 4;
-	device_info->max_mac_addrs = 1;
+	device_info->max_mac_addrs = ENIC_MAX_MAC_ADDR;
 	device_info->rx_offload_capa =
 		DEV_RX_OFFLOAD_VLAN_STRIP |
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
@@ -547,12 +547,12 @@  static void enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
 	enic_set_mac_address(enic, mac_addr->addr_bytes);
 }
 
-static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, __rte_unused uint32_t index)
+static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, uint32_t index)
 {
 	struct enic *enic = pmd_priv(eth_dev);
 
 	ENICPMD_FUNC_TRACE();
-	enic_del_mac_address(enic);
+	enic_del_mac_address(enic, index);
 }
 
 static int enicpmd_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index f0b15ac..21e8ede 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -190,9 +190,12 @@  void enic_dev_stats_get(struct enic *enic, struct rte_eth_stats *r_stats)
 	r_stats->rx_nombuf = rte_atomic64_read(&soft_stats->rx_nombuf);
 }
 
-void enic_del_mac_address(struct enic *enic)
+void enic_del_mac_address(struct enic *enic, int mac_index)
 {
-	if (vnic_dev_del_addr(enic->vdev, enic->mac_addr))
+	struct rte_eth_dev *eth_dev = enic->rte_dev;
+	uint8_t *mac_addr = eth_dev->data->mac_addrs[mac_index].addr_bytes;
+
+	if (vnic_dev_del_addr(enic->vdev, mac_addr))
 		dev_err(enic, "del mac addr failed\n");
 }
 
@@ -205,15 +208,6 @@  void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
 		return;
 	}
 
-	err = vnic_dev_del_addr(enic->vdev, enic->mac_addr);
-	if (err) {
-		dev_err(enic, "del mac addr failed\n");
-		return;
-	}
-
-	ether_addr_copy((struct ether_addr *)mac_addr,
-		(struct ether_addr *)enic->mac_addr);
-
 	err = vnic_dev_add_addr(enic->vdev, mac_addr);
 	if (err) {
 		dev_err(enic, "add mac addr failed\n");
@@ -1308,13 +1302,14 @@  static int enic_dev_init(struct enic *enic)
 	/* Get the supported filters */
 	enic_fdir_info(enic);
 
-	eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr", ETH_ALEN, 0);
+	eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr", ETH_ALEN
+						* ENIC_MAX_MAC_ADDR, 0);
 	if (!eth_dev->data->mac_addrs) {
 		dev_err(enic, "mac addr storage alloc failed, aborting.\n");
 		return -1;
 	}
 	ether_addr_copy((struct ether_addr *) enic->mac_addr,
-		&eth_dev->data->mac_addrs[0]);
+			eth_dev->data->mac_addrs);
 
 	vnic_dev_set_reset_flag(enic->vdev, 0);