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 <hyon...@cisco.com>
> > Reviewed-by: John Daley <johnd...@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
