lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 8 Jul 2010 01:40:43 -0700
From:	"Vladislav Zolotarov" <vladz@...adcom.com>
To:	"Tom Herbert" <therbert@...gle.com>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] bnx2x: add support for receive hashing



> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
> Behalf Of Tom Herbert
> Sent: Wednesday, July 07, 2010 10:18 PM
> To: Vladislav Zolotarov
> Cc: netdev@...r.kernel.org
> Subject: Re: [PATCH] bnx2x: add support for receive hashing
> 
> On Tue, Jul 6, 2010 at 12:16 AM, Vladislav Zolotarov <vladz@...adcom.com>
> wrote:
> > Let me rephrase: the part of your patch below enables RSS flow in our FW
> even if there is only one HW queue and I wonder why?
> > To refresh:
> >        1) FW won't provide Toeplitz hash on CQE if RSS is not enabled.
> >        2) There can be one HW queue in only 2 cases:
> >                - There is only one CPU in the system. In that case I wonder
> if u have anything to do with Toeplitz hash on the skb at all.
> >                - User has explicitly requested 1 HW queue with module
> parameter (num_queues=1). In that case if u r going to use the RSS hash on
> the skb means that u r actually going to do SW RSS instead of HW RSS, which
> sounds strange to me.
> 
> It might not be so strange.  Previously, we has hit a firmware bug in
> bnx2x that was make multi-queue not perform well under some loads, so
> we had disabled it for a while... it is a valid configuration we have
> used.

Tom, according to our FW guys there are no known RSS issues since FW 5.0.x versions. Currently upstream has 5.2.13 FW so, if u know about any issue with the current upstream bnx2x FW, pls., let us know ASAP. Pls., include the relevant performance data. We would also like to know if u see that the FW bug u've mentioned has been resolved in the current upstream bnx2x FW so that we could remove these lines unless u have any other reason to leave them.

Thanks,
vlad

> 
> >
> > So, Herbert, could u, pls., explain, what was your original idea about
> these code lines?
> >
> 
> It is to enable the device to provide the RSS hash in RX descriptor.
> The hash severs two purposes now, it's used internally in the device
> to perform RSS table lookup and also value in RX descriptor.  The
> latter does not require multi-queue.  Strictly speaking, on a single
> processor system without multqueue, it would be true that enabling the
> RX hash on bnx2x is currently superfluous (notwithstanding some other
> use of the hash might be implemented).
> 
> > Thanks,
> > vlad
> >
> >> -----Original Message-----
> >> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On
> >> Behalf Of Vladislav Zolotarov
> >> Sent: Sunday, July 04, 2010 7:36 PM
> >> To: Tom Herbert
> >> Cc: netdev@...r.kernel.org
> >> Subject: RE: [PATCH] bnx2x: add support for receive hashing
> >>
> >> Tom, could u, pls., explain what did u mean by taking the (RSS) flags
> >> configuration out of RSS "if"? To recall "if(is_multi(bp))" is true iff
> RSS
> >> is enabled.
> >>
> >> Thanks,
> >> vlad
> >>
> >> > @@ -5750,10 +5757,10 @@ static void bnx2x_init_internal_func(struct
> bnx2x
> >> > *bp)
> >> >     u32 offset;
> >> >     u16 max_agg_size;
> >> >
> >> > -   if (is_multi(bp)) {
> >> > -           tstorm_config.config_flags = MULTI_FLAGS(bp);
> >> > +   tstorm_config.config_flags = RSS_FLAGS(bp);
> >> > +
> >> > +   if (is_multi(bp))
> >> >             tstorm_config.rss_result_mask = MULTI_MASK;
> >> > -   }
> >> >
> >> >     /* Enable TPA if needed */
> >> >     if (bp->flags & TPA_ENABLE_FLAG)
> >>
> >>
> >> > -----Original Message-----
> >> > From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On
> >> > Behalf Of Tom Herbert
> >> > Sent: Friday, April 23, 2010 8:54 AM
> >> > To: davem@...emloft.net; netdev@...r.kernel.org
> >> > Subject: [PATCH] bnx2x: add support for receive hashing
> >> >
> >> > Add support to bnx2x to extract Toeplitz hash out of the receive
> descriptor
> >> > for use in skb->rxhash.
> >> >
> >> > Signed-off-by: Tom Herbert <therbert@...gle.com>
> >> > ---
> >> > diff --git a/drivers/net/bnx2x.h b/drivers/net/bnx2x.h
> >> > index 0819530..8bd2368 100644
> >> > --- a/drivers/net/bnx2x.h
> >> > +++ b/drivers/net/bnx2x.h
> >> > @@ -1330,7 +1330,7 @@ static inline u32 reg_poll(struct bnx2x *bp, u32
> reg,
> >> > u32 expected, int ms,
> >> >             AEU_INPUTS_ATTN_BITS_MCP_LATCHED_UMP_TX_PARITY | \
> >> >             AEU_INPUTS_ATTN_BITS_MCP_LATCHED_SCPAD_PARITY)
> >> >
> >> > -#define MULTI_FLAGS(bp) \
> >> > +#define RSS_FLAGS(bp) \
> >> >             (TSTORM_ETH_FUNCTION_COMMON_CONFIG_RSS_IPV4_CAPABILITY | \
> >> >              TSTORM_ETH_FUNCTION_COMMON_CONFIG_RSS_IPV4_TCP_CAPABILITY |
> \
> >> >              TSTORM_ETH_FUNCTION_COMMON_CONFIG_RSS_IPV6_CAPABILITY | \
> >> > diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
> >> > index 0c6dba2..613f727 100644
> >> > --- a/drivers/net/bnx2x_main.c
> >> > +++ b/drivers/net/bnx2x_main.c
> >> > @@ -1582,7 +1582,7 @@ static int bnx2x_rx_int(struct bnx2x_fastpath *fp,
> >> int
> >> > budget)
> >> >             struct sw_rx_bd *rx_buf = NULL;
> >> >             struct sk_buff *skb;
> >> >             union eth_rx_cqe *cqe;
> >> > -           u8 cqe_fp_flags;
> >> > +           u8 cqe_fp_flags, cqe_fp_status_flags;
> >> >             u16 len, pad;
> >> >
> >> >             comp_ring_cons = RCQ_BD(sw_comp_cons);
> >> > @@ -1598,6 +1598,7 @@ static int bnx2x_rx_int(struct bnx2x_fastpath *fp,
> >> int
> >> > budget)
> >> >
> >> >             cqe = &fp->rx_comp_ring[comp_ring_cons];
> >> >             cqe_fp_flags = cqe->fast_path_cqe.type_error_flags;
> >> > +           cqe_fp_status_flags = cqe->fast_path_cqe.status_flags;
> >> >
> >> >             DP(NETIF_MSG_RX_STATUS, "CQE type %x  err %x  status %x"
> >> >                "  queue %x  vlan %x  len %u\n", CQE_TYPE(cqe_fp_flags),
> >> > @@ -1727,6 +1728,12 @@ reuse_rx:
> >> >
> >> >                     skb->protocol = eth_type_trans(skb, bp->dev);
> >> >
> >> > +                   if ((bp->dev->features & ETH_FLAG_RXHASH) &&
> >> > +                       (cqe_fp_status_flags &
> >> > +                        ETH_FAST_PATH_RX_CQE_RSS_HASH_FLG))
> >> > +                           skb->rxhash = le32_to_cpu(
> >> > +                               cqe->fast_path_cqe.rss_hash_result);
> >> > +
> >> >                     skb->ip_summed = CHECKSUM_NONE;
> >> >                     if (bp->rx_csum) {
> >> >                             if (likely(BNX2X_RX_CSUM_OK(cqe)))
> >> > @@ -5750,10 +5757,10 @@ static void bnx2x_init_internal_func(struct
> bnx2x
> >> > *bp)
> >> >     u32 offset;
> >> >     u16 max_agg_size;
> >> >
> >> > -   if (is_multi(bp)) {
> >> > -           tstorm_config.config_flags = MULTI_FLAGS(bp);
> >> > +   tstorm_config.config_flags = RSS_FLAGS(bp);
> >> > +
> >> > +   if (is_multi(bp))
> >> >             tstorm_config.rss_result_mask = MULTI_MASK;
> >> > -   }
> >> >
> >> >     /* Enable TPA if needed */
> >> >     if (bp->flags & TPA_ENABLE_FLAG)
> >> > @@ -6629,10 +6636,8 @@ static int bnx2x_init_common(struct bnx2x *bp)
> >> >     bnx2x_init_block(bp, PBF_BLOCK, COMMON_STAGE);
> >> >
> >> >     REG_WR(bp, SRC_REG_SOFT_RST, 1);
> >> > -   for (i = SRC_REG_KEYRSS0_0; i <= SRC_REG_KEYRSS1_9; i += 4) {
> >> > -           REG_WR(bp, i, 0xc0cac01a);
> >> > -           /* TODO: replace with something meaningful */
> >> > -   }
> >> > +   for (i = SRC_REG_KEYRSS0_0; i <= SRC_REG_KEYRSS1_9; i += 4)
> >> > +           REG_WR(bp, i, random32());
> >> >     bnx2x_init_block(bp, SRCH_BLOCK, COMMON_STAGE);
> >> >  #ifdef BCM_CNIC
> >> >     REG_WR(bp, SRC_REG_KEYSEARCH_0, 0x63285672);
> >> > @@ -11001,6 +11006,11 @@ static int bnx2x_set_flags(struct net_device
> *dev,
> >> > u32 data)
> >> >             changed = 1;
> >> >     }
> >> >
> >> > +   if (data & ETH_FLAG_RXHASH)
> >> > +           dev->features |= NETIF_F_RXHASH;
> >> > +   else
> >> > +           dev->features &= ~NETIF_F_RXHASH;
> >> > +
> >> >     if (changed && netif_running(dev)) {
> >> >             bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> >> >             rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> >> > the body of a message to majordomo@...r.kernel.org
> >> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >> the body of a message to majordomo@...r.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ