From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-qg0-f48.google.com (mail-qg0-f48.google.com [209.85.192.48]) by dpdk.org (Postfix) with ESMTP id 975D937A6 for ; Tue, 8 Dec 2015 15:27:07 +0100 (CET) Received: by qgeb1 with SMTP id b1so17961970qge.1 for ; Tue, 08 Dec 2015 06:27:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; bh=sdPRKemjJpWCvhPiJ85EQwYaBM3dCBBksVucO8HkXkI=; b=myHUCLzzf3bk1b9s83FDFlqtjGC6QOOgntj/jpFwZh/ORtWo0YcEZvRhhTUwT78vgx YfKDeuEXEmSIYThOhEf/vZVyqRIY1uBXMtwUl/p/2oYleqBfX/c5Qd2PyM3bMyrC9YXr tcZAV7bnHdrUIiLSJHoRzeiHGurX6U3Y1BbetCfevadTMr5+13Ap6LYXCmXlOOW/APBE oTKWGMNDmb4viiDrEYK1w5amjYJFrVRdE0UiaLtmwTHI9TRXvdnIPgH9pmkqtIQgCi17 A+TpfPCRQnGb+qjT3ID1z6jD0JMgwOJSXpPGySZ0zweBIlaLF7IMaKaTZpdOJ4//FVxE IjNA== X-Received: by 10.140.172.3 with SMTP id s3mr5201045qhs.6.1449584827064; Tue, 08 Dec 2015 06:27:07 -0800 (PST) Received: from foobar.home (pool-71-163-182-126.washdc.fios.verizon.net. [71.163.182.126]) by smtp.googlemail.com with ESMTPSA id d69sm1589250qkb.45.2015.12.08.06.27.06 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 08 Dec 2015 06:27:06 -0800 (PST) Message-ID: <1449584825.2645.77.camel@gmail.com> From: "Charles (Chas) Williams" <3chas3@gmail.com> To: Harish Patil Date: Tue, 08 Dec 2015 09:27:05 -0500 In-Reply-To: References: <1447880321-16187-1-git-send-email-3chas3@gmail.com> <1449486117.2645.73.camel@gmail.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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 14:27:07 -0000 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. > BTW, have you tested the patches? Another question, not related to your > change though. Yes. This patch is necessary (for VF anyway). Without it, you need to manually assign arp addresses. > I guess this block of code does not have to been under the IS_VF() check. I don't know. It is under IS_VF() in the linux driver and it looks like an attempt to "program" some internal switch on the card. I don't have a programmer's guide for this card so it's a complete guess. > >> >+ if > >>(is_broadcast_ether_addr(&eh->d_addr)) > >> >+ mac_type = BROADCAST_ADDRESS; > >> >+ else > >> >+ mac_type = MULTICAST_ADDRESS; > >> >+ }