From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 1802F3195 for ; Tue, 8 Dec 2015 22:38:35 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 08 Dec 2015 13:38:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,400,1444719600"; d="scan'208";a="9815609" Received: from amedina1-mobl2.amr.corp.intel.com ([10.254.185.174]) by fmsmga004.fm.intel.com with SMTP; 08 Dec 2015 13:38:32 -0800 Received: by (sSMTP sendmail emulation); Tue, 08 Dec 2015 21:38:31 +0025 Date: Tue, 8 Dec 2015 21:38:31 +0000 From: Bruce Richardson To: "Charles (Chas) Williams" <3chas3@gmail.com> Message-ID: <20151208213830.GA24204@bricha3-MOBL3> References: <1447880321-16187-1-git-send-email-3chas3@gmail.com> <1449486117.2645.73.camel@gmail.com> <1449584825.2645.77.camel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449584825.2645.77.camel@gmail.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] bnx2x: set Ethernet address type during transmit for VF's X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Dec 2015 21:38:36 -0000 On Tue, Dec 08, 2015 at 09:27:05AM -0500, Charles (Chas) Williams wrote: > On Mon, 2015-12-07 at 17:29 +0000, Harish Patil wrote: > > > > > >On Sun, 2015-12-06 at 23:34 +0000, Harish Patil wrote: > > >> > > > >> >The original was always setting unicast. While here, clean up some > > >> >other references that also point into the Ethernet header. > > >> > > > >> >Signed-off-by: Chas Williams <3chas3@gmail.com> > > >> >--- > > >> > drivers/net/bnx2x/bnx2x.c | 23 +++++++++++++++-------- > > >> > drivers/net/bnx2x/ecore_hsi.h | 5 +++-- > > >> > 2 files changed, 18 insertions(+), 10 deletions(-) > > >> > > > >> >diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c > > >> >index 76444eb..294711f 100644 > > >> >--- a/drivers/net/bnx2x/bnx2x.c > > >> >+++ b/drivers/net/bnx2x/bnx2x.c > > >> >@@ -2177,25 +2177,32 @@ int bnx2x_tx_encap(struct bnx2x_tx_queue *txq, > > >> >struct rte_mbuf **m_head, int m_p > > >> > bd_prod = NEXT_TX_BD(bd_prod); > > >> > if (IS_VF(sc)) { > > >> > struct eth_tx_parse_bd_e2 *tx_parse_bd; > > >> >- uint8_t *data = rte_pktmbuf_mtod(m0, uint8_t *); > > >> >+ const struct ether_hdr *eh = > > >>rte_pktmbuf_mtod(m0, struct ether_hdr *); > > >> >+ uint8_t mac_type = UNICAST_ADDRESS; > > >> > > > >> > tx_parse_bd = > > >> > &txq->tx_ring[TX_BD(bd_prod, > > >>txq)].parse_bd_e2; > > >> >+ if (is_multicast_ether_addr(&eh->d_addr)) { > > >> > > >> Minor comment. unlikely() may be used here to keep it consistent with > > >>base > > >> driver. > > > > > >It wasn't clear to me that this code path is all that unlikely(). > > > > Its an optional comment, unlikely() is because fast path traffic is mostly > > unicast. > > Multicast traffic isn't all that uncommon. I also don't see that we gain > much from branch prediction here regardless. My understanding is that > unlikely() should be used for really unlikely situations since the branch > will not be optimized. +1 to this. Let the cpu branch predictor do the work at run-time picking the correct leg of the branch. likely()/unlikely() should only ever be used for things like fatal error conditions where it doesn't matter how optimized the unlikely branch part is. For normal code where there is no error, don't try and optimize it using likely()/unlikely() unless you can prove there is a definite performance improvement by doing so. /Bruce