From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hyong Youb Kim <hyonkim@cisco.com>
Subject: Re: [PATCH 3/4] net/enic: support multicast filtering
Date: Sat, 12 Jan 2019 13:49:57 +0900
Message-ID: <20190112044956.GA23480@HYONKIM-7R0DR.cisco.com>
References: <20181210182857.13043-1-hyonkim@cisco.com>
 <20181210182857.13043-4-hyonkim@cisco.com>
 <25e615b7-35ba-8019-62fa-64590ab69d19@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, John Daley <johndale@cisco.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from alln-iport-5.cisco.com (alln-iport-5.cisco.com [173.37.142.92])
 by dpdk.org (Postfix) with ESMTP id 217D11B9E7
 for <dev@dpdk.org>; Sat, 12 Jan 2019 05:50:03 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <25e615b7-35ba-8019-62fa-64590ab69d19@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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