[dpdk-dev] [PATCH 0/4] net/enic: minor updates

[dpdk-dev] [PATCH 0/4] net/enic: minor updates

[Hyong Youb Kim]

These are patches for 19.02.

Hyong Youb Kim (4):
  net/enic: release port upon close
  net/enic: add handler to return firmware version string
  net/enic: support multicast filtering
  doc: update release notes for enic

 doc/guides/nics/features/enic.ini      |   3 +-
 doc/guides/rel_notes/release_19_02.rst |   5 ++
 drivers/net/enic/base/vnic_dev.c       |  26 +++++++
 drivers/net/enic/enic.h                |   6 +-
 drivers/net/enic/enic_ethdev.c         | 119 ++++++++++++++++++++++++++++++++-
 drivers/net/enic/enic_main.c           |   9 ++-
 6 files changed, 159 insertions(+), 9 deletions(-)

-- 
2.16.2

[dpdk-dev] [PATCH 1/4] net/enic: release port upon close

[Hyong Youb Kim]

Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so rte_eth_dev_close() can
later free port resources including mac_addrs.

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_ethdev.c | 2 ++
 drivers/net/enic/enic_main.c   | 4 +---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 996bb5542..ed8dda568 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -1048,6 +1048,8 @@ static int eth_enicpmd_dev_init(struct rte_eth_dev *eth_dev)
 	eth_dev->rx_pkt_burst = &enic_recv_pkts;
 	eth_dev->tx_pkt_burst = &enic_xmit_pkts;
 	eth_dev->tx_pkt_prepare = &enic_prep_pkts;
+	/* Let rte_eth_dev_close() release the port resources */
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
 
 	pdev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	rte_eth_copy_pci_info(eth_dev, pdev);
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index c3869de36..f45717374 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1379,12 +1379,10 @@ int enic_get_link_status(struct enic *enic)
 
 static void enic_dev_deinit(struct enic *enic)
 {
-	struct rte_eth_dev *eth_dev = enic->rte_dev;
-
 	/* stop link status checking */
 	vnic_dev_notify_unset(enic->vdev);
 
-	rte_free(eth_dev->data->mac_addrs);
+	/* mac_addrs is freed by rte_eth_dev_release_port() */
 	rte_free(enic->cq);
 	rte_free(enic->intr);
 	rte_free(enic->rq);
-- 
2.16.2

[dpdk-dev] [PATCH 2/4] net/enic: add handler to return firmware version string

[Hyong Youb Kim]

Cisco VIC adapters run firmware. Add the fw_version_get handler to
help diagnostics.

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/features/enic.ini |  1 +
 drivers/net/enic/base/vnic_dev.c  | 26 ++++++++++++++++++++++++++
 drivers/net/enic/enic_ethdev.c    | 21 +++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/doc/guides/nics/features/enic.ini b/doc/guides/nics/features/enic.ini
index 8a4bad29f..020b75cf0 100644
--- a/doc/guides/nics/features/enic.ini
+++ b/doc/guides/nics/features/enic.ini
@@ -31,6 +31,7 @@ Inner L3 checksum    = Y
 Inner L4 checksum    = Y
 Packet type parsing  = Y
 Basic stats          = Y
+FW version           = Y
 Multiprocess aware   = Y
 BSD nic_uio          = Y
 Linux UIO            = Y
diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c
index fd303fece..1e5b12a80 100644
--- a/drivers/net/enic/base/vnic_dev.c
+++ b/drivers/net/enic/base/vnic_dev.c
@@ -458,6 +458,32 @@ int vnic_dev_cmd_args(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 	}
 }
 
+int vnic_dev_fw_info(struct vnic_dev *vdev,
+		     struct vnic_devcmd_fw_info **fw_info)
+{
+	char name[NAME_MAX];
+	u64 a0, a1 = 0;
+	int wait = 1000;
+	int err = 0;
+	static u32 instance;
+
+	if (!vdev->fw_info) {
+		snprintf((char *)name, sizeof(name), "vnic_fw_info-%u",
+			 instance++);
+		vdev->fw_info = vdev->alloc_consistent(vdev->priv,
+			sizeof(struct vnic_devcmd_fw_info),
+			&vdev->fw_info_pa, (u8 *)name);
+		if (!vdev->fw_info)
+			return -ENOMEM;
+		a0 = vdev->fw_info_pa;
+		a1 = sizeof(struct vnic_devcmd_fw_info);
+		err = vnic_dev_cmd(vdev, CMD_MCPU_FW_INFO,
+				   &a0, &a1, wait);
+	}
+	*fw_info = vdev->fw_info;
+	return err;
+}
+
 static int vnic_dev_advanced_filters_cap(struct vnic_dev *vdev, u64 *args,
 		int nargs)
 {
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index ed8dda568..9b206e8f0 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -887,6 +887,26 @@ static int enicpmd_dev_udp_tunnel_port_del(struct rte_eth_dev *eth_dev,
 	return update_vxlan_port(enic, ENIC_DEFAULT_VXLAN_PORT);
 }
 
+static int enicpmd_dev_fw_version_get(struct rte_eth_dev *eth_dev,
+				      char *fw_version, size_t fw_size)
+{
+	struct vnic_devcmd_fw_info *info;
+	struct enic *enic;
+	int ret;
+
+	ENICPMD_FUNC_TRACE();
+	if (fw_version == NULL || fw_size <= 0)
+		return -EINVAL;
+	enic = pmd_priv(eth_dev);
+	ret = vnic_dev_fw_info(enic->vdev, &info);
+	if (ret)
+		return ret;
+	snprintf(fw_version, fw_size, "%s %s",
+		 info->fw_version, info->fw_build);
+	fw_version[fw_size - 1] = '\0';
+	return 0;
+}
+
 static const struct eth_dev_ops enicpmd_eth_dev_ops = {
 	.dev_configure        = enicpmd_dev_configure,
 	.dev_start            = enicpmd_dev_start,
@@ -938,6 +958,7 @@ static const struct eth_dev_ops enicpmd_eth_dev_ops = {
 	.rss_hash_update      = enicpmd_dev_rss_hash_update,
 	.udp_tunnel_port_add  = enicpmd_dev_udp_tunnel_port_add,
 	.udp_tunnel_port_del  = enicpmd_dev_udp_tunnel_port_del,
+	.fw_version_get       = enicpmd_dev_fw_version_get,
 };
 
 static int enic_parse_zero_one(const char *key,
-- 
2.16.2

[dpdk-dev] [PATCH 3/4] net/enic: support multicast filtering

[Hyong Youb Kim]

The VIC hardware has 64 MAC filters per vNIC, which can be either
unicast or multicast. Use one half for unicast and the other half for
multicast, as the VIC kernel drivers for Linux and Windows do.

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/features/enic.ini |  2 +-
 drivers/net/enic/enic.h           |  6 ++-
 drivers/net/enic/enic_ethdev.c    | 96 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/enic/enic_main.c      |  5 +-
 4 files changed, 103 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/features/enic.ini b/doc/guides/nics/features/enic.ini
index 020b75cf0..c6d374984 100644
--- a/doc/guides/nics/features/enic.ini
+++ b/doc/guides/nics/features/enic.ini
@@ -15,7 +15,7 @@ TSO                  = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
-Multicast MAC filter =
+Multicast MAC filter = Y
 RSS hash             = Y
 RSS key update       = Y
 RSS reta update      = Y
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 7bca3cad2..6c497e9a2 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -25,8 +25,6 @@
 #define DRV_DESCRIPTION		"Cisco VIC Ethernet NIC Poll-mode Driver"
 #define DRV_COPYRIGHT		"Copyright 2008-2015 Cisco Systems, Inc"
 
-#define ENIC_MAX_MAC_ADDR	64
-
 #define VLAN_ETH_HLEN           18
 
 #define ENICPMD_SETTING(enic, f) ((enic->config.flags & VENETF_##f) ? 1 : 0)
@@ -196,6 +194,10 @@ struct enic {
 	uint64_t tx_offload_capa; /* DEV_TX_OFFLOAD flags */
 	uint64_t tx_queue_offload_capa; /* DEV_TX_OFFLOAD flags */
 	uint64_t tx_offload_mask; /* PKT_TX flags accepted */
+
+	/* Multicast MAC addresses added to the NIC */
+	uint32_t mc_count;
+	struct ether_addr mc_addrs[ENIC_MULTICAST_PERFECT_FILTERS];
 };
 
 /* Compute ethdev's max packet size from MTU */
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 9b206e8f0..fbbec2266 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -367,6 +367,7 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev)
 		return ret;
 	}
 
+	enic->mc_count = 0;
 	enic->hw_ip_checksum = !!(eth_dev->data->dev_conf.rxmode.offloads &
 				  DEV_RX_OFFLOAD_CHECKSUM);
 	/* All vlan offload masks to apply the current settings */
@@ -473,7 +474,7 @@ static void enicpmd_dev_info_get(struct rte_eth_dev *eth_dev,
 	 * ignoring vNIC mtu.
 	 */
 	device_info->max_rx_pktlen = enic_mtu_to_max_rx_pktlen(enic->max_mtu);
-	device_info->max_mac_addrs = ENIC_MAX_MAC_ADDR;
+	device_info->max_mac_addrs = ENIC_UNICAST_PERFECT_FILTERS;
 	device_info->rx_offload_capa = enic->rx_offload_capa;
 	device_info->tx_offload_capa = enic->tx_offload_capa;
 	device_info->tx_queue_offload_capa = enic->tx_queue_offload_capa;
@@ -643,6 +644,98 @@ static int enicpmd_set_mac_addr(struct rte_eth_dev *eth_dev,
 	return enic_set_mac_address(enic, addr->addr_bytes);
 }
 
+static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
+{
+	char mac_str[ETHER_ADDR_FMT_SIZE];
+	if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
+		ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
+		PMD_INIT_LOG(ERR, " %s address %s\n",
+			     add ? "add" : "remove", mac_str);
+	}
+}
+
+static int enicpmd_set_mc_addr_list(struct rte_eth_dev *eth_dev,
+				    struct ether_addr *mc_addr_set,
+				    uint32_t nb_mc_addr)
+{
+	struct enic *enic = pmd_priv(eth_dev);
+	char mac_str[ETHER_ADDR_FMT_SIZE];
+	struct ether_addr *addr;
+	uint32_t i, j;
+	int ret;
+
+	ENICPMD_FUNC_TRACE();
+
+	/* Validate the given addresses first */
+	for (i = 0; i < nb_mc_addr && mc_addr_set != NULL; i++) {
+		addr = &mc_addr_set[i];
+		if (!is_multicast_ether_addr(addr) ||
+		    is_broadcast_ether_addr(addr)) {
+			ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
+			PMD_INIT_LOG(ERR, " invalid multicast address %s\n",
+				     mac_str);
+			return -EINVAL;
+		}
+	}
+
+	/* Flush all if requested */
+	if (nb_mc_addr == 0 || mc_addr_set == NULL) {
+		PMD_INIT_LOG(DEBUG, " flush multicast addresses\n");
+		for (i = 0; i < enic->mc_count; i++) {
+			addr = &enic->mc_addrs[i];
+			debug_log_add_del_addr(addr, false);
+			ret = vnic_dev_del_addr(enic->vdev, addr->addr_bytes);
+			if (ret)
+				return ret;
+		}
+		enic->mc_count = 0;
+		return 0;
+	}
+
+	if (nb_mc_addr > ENIC_MULTICAST_PERFECT_FILTERS) {
+		PMD_INIT_LOG(ERR, " too many multicast addresses: max=%d\n",
+			     ENIC_MULTICAST_PERFECT_FILTERS);
+		return -ENOSPC;
+	}
+	/*
+	 * devcmd is slow, so apply the difference instead of flushing and
+	 * adding everything.
+	 * 1. Delete addresses on the NIC but not on the host
+	 */
+	for (i = 0; i < enic->mc_count; i++) {
+		addr = &enic->mc_addrs[i];
+		for (j = 0; j < nb_mc_addr; j++) {
+			if (is_same_ether_addr(addr, &mc_addr_set[j]))
+				break;
+		}
+		if (j < nb_mc_addr)
+			continue;
+		debug_log_add_del_addr(addr, false);
+		ret = vnic_dev_del_addr(enic->vdev, addr->addr_bytes);
+		if (ret)
+			return ret;
+	}
+	/* 2. Add addresses on the host but not on the NIC */
+	for (i = 0; i < nb_mc_addr; i++) {
+		addr = &mc_addr_set[i];
+		for (j = 0; j < enic->mc_count; j++) {
+			if (is_same_ether_addr(addr, &enic->mc_addrs[j]))
+				break;
+		}
+		if (j < enic->mc_count)
+			continue;
+		debug_log_add_del_addr(addr, true);
+		ret = vnic_dev_add_addr(enic->vdev, addr->addr_bytes);
+		if (ret)
+			return ret;
+	}
+	/* Keep a copy so we can flush/apply later on.. */
+	memcpy(enic->mc_addrs, mc_addr_set,
+	       nb_mc_addr * sizeof(struct ether_addr));
+	enic->mc_count = nb_mc_addr;
+	return 0;
+}
+
 static int enicpmd_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
 {
 	struct enic *enic = pmd_priv(eth_dev);
@@ -951,6 +1044,7 @@ static const struct eth_dev_ops enicpmd_eth_dev_ops = {
 	.mac_addr_add         = enicpmd_add_mac_addr,
 	.mac_addr_remove      = enicpmd_remove_mac_addr,
 	.mac_addr_set         = enicpmd_set_mac_addr,
+	.set_mc_addr_list     = enicpmd_set_mc_addr_list,
 	.filter_ctrl          = enicpmd_dev_filter_ctrl,
 	.reta_query           = enicpmd_dev_rss_reta_query,
 	.reta_update          = enicpmd_dev_rss_reta_update,
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index f45717374..621a984a0 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1667,8 +1667,9 @@ 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
-						* ENIC_MAX_MAC_ADDR, 0);
+	eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr",
+					sizeof(struct ether_addr) *
+					ENIC_UNICAST_PERFECT_FILTERS, 0);
 	if (!eth_dev->data->mac_addrs) {
 		dev_err(enic, "mac addr storage alloc failed, aborting.\n");
 		return -1;
-- 
2.16.2

[dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Hyong Youb Kim]

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 doc/guides/rel_notes/release_19_02.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst
index a94fa86a7..4c789d2a9 100644
--- a/doc/guides/rel_notes/release_19_02.rst
+++ b/doc/guides/rel_notes/release_19_02.rst
@@ -54,6 +54,11 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Updated the enic driver.**
+
+  * Added support for ``RTE_ETH_DEV_CLOSE_REMOVE`` flag.
+  * Added the handler to get firmware version string.
+  * Added support for multicast filtering.
 
 Removed Items
 -------------
-- 
2.16.2

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Varghese, Vipin]

Hi Hyong,

Thanks for sharing the information a query 'is ENIC Poll Mode Driver is been updated too?'(Section 16 under Network Interface Controller). 
a. If yes is it ok to link the patchwork in comment section? 
b. If no, will you be updating the documentation?

Thanks
Vipin Varghese

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Hyong Youb Kim
> Sent: Monday, December 10, 2018 11:59 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; John Daley <johndale@cisco.com>; Hyong Youb Kim
> <hyonkim@cisco.com>
> Subject: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic
> 
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
> ---
>  doc/guides/rel_notes/release_19_02.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_19_02.rst
> b/doc/guides/rel_notes/release_19_02.rst
> index a94fa86a7..4c789d2a9 100644
> --- a/doc/guides/rel_notes/release_19_02.rst
> +++ b/doc/guides/rel_notes/release_19_02.rst
> @@ -54,6 +54,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
> 
> +* **Updated the enic driver.**
> +
> +  * Added support for ``RTE_ETH_DEV_CLOSE_REMOVE`` flag.
> +  * Added the handler to get firmware version string.
> +  * Added support for multicast filtering.
> 
>  Removed Items
>  -------------
> --
> 2.16.2

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Hyong Youb Kim]

On Tue, Dec 11, 2018 at 02:44:43AM +0000, Varghese, Vipin wrote:
> Hi Hyong,
> 
> Thanks for sharing the information a query 'is ENIC Poll Mode Driver is been updated too?'(Section 16 under Network Interface Controller). 
> a. If yes is it ok to link the patchwork in comment section? 
> b. If no, will you be updating the documentation?
> 

Vipin,

I am not planning to update the enic guide, as these changes (fw
version string, multicast filtering) do not need additional
explanations. Did you have something else in mind?

Thanks..
-Hyong

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Varghese, Vipin]

Hi Hyong,

snipped
> > Hi Hyong,
> >
> > Thanks for sharing the information a query 'is ENIC Poll Mode Driver is been
> updated too?'(Section 16 under Network Interface Controller).
> > a. If yes is it ok to link the patchwork in comment section?
> > b. If no, will you be updating the documentation?
> >
> 
> Vipin,
> 
> I am not planning to update the enic guide, as these changes (fw version string,
> multicast filtering) do not need additional explanations. Did you have
> something else in mind?

Thanks for sharing the information.

In my humble opinion

In documentation section '16.13. Supported features' states 'Unicast, multicast and broadcast transmission and reception'. So multicast 'filtering' to be added as supported feature or not is upto the individual for the PMD. But as you shared the information ' I am not planning to update the enic guide' I will leave this open to others for comment.

Question: Will there update for test cases for mac filtering been added? Reason the patch set is not covering test case addition. 

> 
> Thanks..
> -Hyong

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Hyong Youb Kim]

On Tue, Dec 11, 2018 at 04:31:50PM +0000, Varghese, Vipin wrote:
> Hi Hyong,
> 
> snipped
> > > Hi Hyong,
> > >
> > > Thanks for sharing the information a query 'is ENIC Poll Mode Driver is been
> > updated too?'(Section 16 under Network Interface Controller).
> > > a. If yes is it ok to link the patchwork in comment section?
> > > b. If no, will you be updating the documentation?
> > >
> > 
> > Vipin,
> > 
> > I am not planning to update the enic guide, as these changes (fw version string,
> > multicast filtering) do not need additional explanations. Did you have
> > something else in mind?
> 
> Thanks for sharing the information.
> 
> In my humble opinion
> 
> In documentation section '16.13. Supported features' states 'Unicast, multicast and broadcast transmission and reception'. So multicast 'filtering' to be added as supported feature or not is upto the individual for the PMD. But as you shared the information ' I am not planning to update the enic guide' I will leave this open to others for comment.

I see what you mean now. Sure, I will send another patch to update
that section later. As is, it is really a duplicate of
doc/guides/nics/features. Not sure if we want to keep maintaining two
copies of the same information.

> 
> Question: Will there update for test cases for mac filtering been added? Reason the patch set is not covering test case addition. 
> 

We have an internal DTS based test case for multicast filtering. Do we
need to mention test cases in commit logs now? I am a bit confused
where this question is coming from..

-Hyong

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Varghese, Vipin]

snipped
> > Hi Hyong,
> >
> > snipped
> > > > Hi Hyong,
> > > >
> > > > Thanks for sharing the information a query 'is ENIC Poll Mode
> > > > Driver is been
> > > updated too?'(Section 16 under Network Interface Controller).
> > > > a. If yes is it ok to link the patchwork in comment section?
> > > > b. If no, will you be updating the documentation?
> > > >
> > >
> > > Vipin,
> > >
> > > I am not planning to update the enic guide, as these changes (fw
> > > version string, multicast filtering) do not need additional
> > > explanations. Did you have something else in mind?
> >
> > Thanks for sharing the information.
> >
> > In my humble opinion
> >
> > In documentation section '16.13. Supported features' states 'Unicast,
> multicast and broadcast transmission and reception'. So multicast 'filtering' to
> be added as supported feature or not is upto the individual for the PMD. But
> as you shared the information ' I am not planning to update the enic guide' I
> will leave this open to others for comment.
> 
> I see what you mean now. Sure, I will send another patch to update that
> section later. As is, it is really a duplicate of doc/guides/nics/features. Not sure
> if we want to keep maintaining two copies of the same information.
> 
> >
> > Question: Will there update for test cases for mac filtering been added?
> Reason the patch set is not covering test case addition.
> >
> 
> We have an internal DTS based test case for multicast filtering. Do we need to
> mention test cases in commit logs now? I am a bit confused where this
> question is coming from..

Apologies if the question was not clear, Let me try again 'Is there plan add test cases under dpdk/test/test folder?'

> 
> -Hyong

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Hyong Youb Kim]

On Tue, Dec 11, 2018 at 04:49:26PM +0000, Varghese, Vipin wrote:
> snipped
[...]
> 
> Apologies if the question was not clear, Let me try again 'Is there plan add test cases under dpdk/test/test folder?'

Not in this cycle.

Thanks.
-Hyong

Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic

[Varghese, Vipin]

> -----Original Message-----
> From: Hyong Youb Kim <hyonkim@cisco.com>
> Sent: Tuesday, December 11, 2018 10:56 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; John Daley
> <johndale@cisco.com>
> Subject: Re: [dpdk-dev] [PATCH 4/4] doc: update release notes for enic
> 
> On Tue, Dec 11, 2018 at 04:49:26PM +0000, Varghese, Vipin wrote:
> > snipped
> [...]
> >
> > Apologies if the question was not clear, Let me try again 'Is there plan add
> test cases under dpdk/test/test folder?'
> 
> Not in this cycle.

Thanks for update, hope we can see some unit test for enabling multicast filtering in upcoming cycles.

> 
> Thanks.
> -Hyong

Re: [dpdk-dev] [PATCH 0/4] net/enic: minor updates

[Ferruh Yigit]

On 12/10/2018 6:28 PM, Hyong Youb Kim wrote:
> These are patches for 19.02.
> 
> Hyong Youb Kim (4):
>   net/enic: release port upon close
>   net/enic: add handler to return firmware version string
>   net/enic: support multicast filtering
>   doc: update release notes for enic

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

Re: [dpdk-dev] [PATCH 3/4] net/enic: support multicast filtering

[Ferruh Yigit]

On 12/10/2018 6:28 PM, Hyong Youb Kim wrote:
> The VIC hardware has 64 MAC filters per vNIC, which can be either
> unicast or multicast. Use one half for unicast and the other half for
> multicast, as the VIC kernel drivers for Linux and Windows do.
> 
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>

<...>

> +static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
> +{
> +	char mac_str[ETHER_ADDR_FMT_SIZE];
> +	if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
> +		ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
> +		PMD_INIT_LOG(ERR, " %s address %s\n",
> +			     add ? "add" : "remove", mac_str);
> +	}
> +}

Why logging with 'ERR' level after checking if 'DEBUG' level is set?

And why need that check at all?

Re: [dpdk-dev] [PATCH 3/4] net/enic: support multicast filtering

[Hyong Youb Kim]

On Fri, Jan 11, 2019 at 03:46:47PM +0000, Ferruh Yigit wrote:
> On 12/10/2018 6:28 PM, Hyong Youb Kim wrote:
> > The VIC hardware has 64 MAC filters per vNIC, which can be either
> > unicast or multicast. Use one half for unicast and the other half for
> > multicast, as the VIC kernel drivers for Linux and Windows do.
> > 
> > Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> > Reviewed-by: John Daley <johndale@cisco.com>
> 
> <...>
> 
> > +static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
> > +{
> > +	char mac_str[ETHER_ADDR_FMT_SIZE];
> > +	if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
> > +		ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
> > +		PMD_INIT_LOG(ERR, " %s address %s\n",
> > +			     add ? "add" : "remove", mac_str);
> > +	}
> > +}
> 
> Why logging with 'ERR' level after checking if 'DEBUG' level is set?

Thanks for pointing this out. Our mistake. Should be DEBUG.

> And why need that check at all?

The outer check ("is log level debug?") is there to make this function
to do nothing (including ether_format_addr) if log level > debug. I
could not find a good way to combine the level test,
ether_format_addr, and rte_log into one clean macro..

Since this function is on a slow path, I think I can just remove the
level check and do something like the following. Would this satisfy
your concern?

char mac_str[ETHER_ADDR_FMT_SIZE];
ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
PMD_INIT_LOG(DEBUG, " %s address %s\n", add ? "add" : "remove", mac_str);

Thanks.
-Hyong

Re: [dpdk-dev] [PATCH 3/4] net/enic: support multicast filtering

[Ferruh Yigit]

On 1/12/2019 4:49 AM, Hyong Youb Kim wrote:
> On Fri, Jan 11, 2019 at 03:46:47PM +0000, Ferruh Yigit wrote:
>> On 12/10/2018 6:28 PM, Hyong Youb Kim wrote:
>>> The VIC hardware has 64 MAC filters per vNIC, which can be either
>>> unicast or multicast. Use one half for unicast and the other half for
>>> multicast, as the VIC kernel drivers for Linux and Windows do.
>>>
>>> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
>>> Reviewed-by: John Daley <johndale@cisco.com>
>>
>> <...>
>>
>>> +static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
>>> +{
>>> +	char mac_str[ETHER_ADDR_FMT_SIZE];
>>> +	if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
>>> +		ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
>>> +		PMD_INIT_LOG(ERR, " %s address %s\n",
>>> +			     add ? "add" : "remove", mac_str);
>>> +	}
>>> +}
>>
>> Why logging with 'ERR' level after checking if 'DEBUG' level is set?
> 
> Thanks for pointing this out. Our mistake. Should be DEBUG.
> 
>> And why need that check at all?
> 
> The outer check ("is log level debug?") is there to make this function
> to do nothing (including ether_format_addr) if log level > debug. I
> could not find a good way to combine the level test,
> ether_format_addr, and rte_log into one clean macro..
> 
> Since this function is on a slow path, I think I can just remove the
> level check and do something like the following. Would this satisfy
> your concern?
> 
> char mac_str[ETHER_ADDR_FMT_SIZE];
> ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
> PMD_INIT_LOG(DEBUG, " %s address %s\n", add ? "add" : "remove", mac_str);

Yes, I think this is better, thanks.