From: "Charles (Chas) Williams" <3chas3@gmail.com>
To: Harish Patil <harish.patil@qlogic.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] bnx2x: set Ethernet address type during transmit for VF's
Date: Tue, 08 Dec 2015 09:27:05 -0500 [thread overview]
Message-ID: <1449584825.2645.77.camel@gmail.com> (raw)
In-Reply-To: <D28B00ED.109257%harish.patil@qlogic.com>
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;
> >> >+ }
next prev parent reply other threads:[~2015-12-08 14:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-18 20:58 Chas Williams
2015-12-06 22:20 ` Thomas Monjalon
2015-12-06 22:22 ` Thomas Monjalon
2015-12-06 23:34 ` Harish Patil
2015-12-07 11:01 ` Charles (Chas) Williams
2015-12-07 17:29 ` Harish Patil
2015-12-08 14:27 ` Charles (Chas) Williams [this message]
2015-12-08 21:38 ` Bruce Richardson
2015-12-08 21:48 ` Harish Patil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1449584825.2645.77.camel@gmail.com \
--to=3chas3@gmail.com \
--cc=dev@dpdk.org \
--cc=harish.patil@qlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).