Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Stephen Hemminger <[EMAIL PROTECTED]> Date: Mon, 8 Oct 2007 14:03:01 -0700 > On Mon, 08 Oct 2007 14:40:22 -0700 > "Michael Chan" <[EMAIL PROTECTED]> wrote: > > > On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote: > > > On Mon, 08 Oct 2007 20:34:41 +0200 > > > "Eliezer" <[EMAIL PROTECTED]> wrote: > > > > > > > > * The MACRO's for 64 bit stats look like they could be done with > > > > > u64 and/or turned into inline's. > > > > > > > > The MACRO's modify some of their arguments, plus they need to work on 32 > > > > bit machines (are 64 bit counters always available on 32 bit machines?). > > > > so using an inline would allow the inline to be more readable but > > > > calling it would get ugly. > > > > I'm open to suggestions. > > > > > > > > > > u64 exists on all platforms (including 32 bit). > > > > > > > I think the biggest problem with these 64-bits counters (and 64-bit > > addresses) is that the hardware treats them as big endian and they get > > DMA'ed in big endian format. We control the byte swap so that 32-bit > > quantities will have the correct endianness, but the high and low 32-bit > > words will be in the wrong spots on little endian machines. That's why > > we need to separate the high and the low words and convert them back and > > forth. > > > There are types and tools for checking endianness see be64, etc. It's not a be64 Stephen, Michael is trying to explain this. They configure the hardware to swap the bytes within a 32-bit word into cpu endianness, but the chip doesn't swap the 32-bit words within a 64-bit word properly, which is why the macros are necessary. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 08 Oct 2007 14:40:22 -0700 "Michael Chan" <[EMAIL PROTECTED]> wrote: > On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote: > > On Mon, 08 Oct 2007 20:34:41 +0200 > > "Eliezer" <[EMAIL PROTECTED]> wrote: > > > > > > * The MACRO's for 64 bit stats look like they could be done with > > > > u64 and/or turned into inline's. > > > > > > The MACRO's modify some of their arguments, plus they need to work on 32 > > > bit machines (are 64 bit counters always available on 32 bit machines?). > > > so using an inline would allow the inline to be more readable but > > > calling it would get ugly. > > > I'm open to suggestions. > > > > > > > u64 exists on all platforms (including 32 bit). > > > > I think the biggest problem with these 64-bits counters (and 64-bit > addresses) is that the hardware treats them as big endian and they get > DMA'ed in big endian format. We control the byte swap so that 32-bit > quantities will have the correct endianness, but the high and low 32-bit > words will be in the wrong spots on little endian machines. That's why > we need to separate the high and the low words and convert them back and > forth. > There are types and tools for checking endianness see be64, etc. -- Stephen Hemminger <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote: > On Mon, 08 Oct 2007 20:34:41 +0200 > "Eliezer" <[EMAIL PROTECTED]> wrote: > > > > * The MACRO's for 64 bit stats look like they could be done with > > > u64 and/or turned into inline's. > > > > The MACRO's modify some of their arguments, plus they need to work on 32 > > bit machines (are 64 bit counters always available on 32 bit machines?). > > so using an inline would allow the inline to be more readable but > > calling it would get ugly. > > I'm open to suggestions. > > > > u64 exists on all platforms (including 32 bit). > I think the biggest problem with these 64-bits counters (and 64-bit addresses) is that the hardware treats them as big endian and they get DMA'ed in big endian format. We control the byte swap so that 32-bit quantities will have the correct endianness, but the high and low 32-bit words will be in the wrong spots on little endian machines. That's why we need to separate the high and the low words and convert them back and forth. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote:
> On Mon, 08 Oct 2007 20:34:41 +0200
> "Eliezer" <[EMAIL PROTECTED]> wrote:
>
> > On Mon, 2007-10-08 at 10:29 -0700, Stephen Hemminger wrote:
> >
> > >
> > > Looks good. Some minor stuff:
> > > * You can use network device stats in network device structure and
> > > no longer need the copy in bp
> >
> > We can't use net device stats in the bp because the elements are also
> > used by HW and we can't rearrange them.
> > struct bmac_stats and struct emac_stats are written to by the HW
> > struct nig_stats and struct bnx2x_eth_stats are read by HW
> > (I would have loved to get rid of at least one of these long structs.)
>
> Here:
> static void bnx2x_update_net_stats(struct bnx2x *bp)
> +{
> + struct bnx2x_eth_stats *estats = bnx2x_sp(bp, eth_stats);
> +
> + bp->net_stats.rx_packets =
> + bnx2x_hilo(&estats->total_unicast_packets_received_hi) +
> + bnx2x_hilo(&estats->total_multicast_packets_received_hi) +
> + bnx2x_hilo(&estats->total_broadcast_packets_received_hi);
> +
> + bp->net_stats.tx_packets =
> + bnx2x_hilo(&estats->total_unicast_packets_transmitted_hi) +
> + bnx2x_hilo(&estats->total_multicast_packets_transmitted_hi) +
> + bnx2x_hilo(&estats->total_broadcast_packets_transmitted_hi);
> +
> + bp->net_stats.rx_bytes = bnx2x_hilo(&estats->total_bytes_received_hi);
> +
> + bp->net_stats.tx_bytes =
> + bnx2x_hilo(&estats->total_bytes_transmitted_hi);
> +
> + bp->net_stats.rx_dropped = estats->no_buff_discard;
> + bp->net_stats.tx_dropped = 0;
>
> bp->net_stats could be replaced by dev->net_stats
>
OK, I will do that on the next version.
>
> > > * The MACRO's for 64 bit stats look like they could be done with
> > > u64 and/or turned into inline's.
> >
> > The MACRO's modify some of their arguments, plus they need to work on 32
> > bit machines (are 64 bit counters always available on 32 bit machines?).
> > so using an inline would allow the inline to be more readable but
> > calling it would get ugly.
> > I'm open to suggestions.
> >
>
> u64 exists on all platforms (including 32 bit).
I will see if I can use that to simplify the MACROs.
>
> > > * RCS/CVS tags for date and version # are kind of ugly. They git system
> > > doesn't use them, perhaps you just want to track your internal version
> > > control information, but what happens when changes come from outside?
> >
> > We need some way to correlate cvs to opensource to bug tracking tool.
> > Note that driver and Microcode code are very tightly coupled on this
> > device, so if I want to debug a microcode related problem I would need
> > both CVS version and microcode version.
> >
> > I agree that tags are ugly but can't think of a better way to do it.
> > Changes coming from the outside are a problem, but using the cvs version
> > as an anchor I can track them using git.
> > Again, I'm open to suggestions and if someone can think of an elegant
> > way of doing it, I will gladly use it.
> >
>
> Leave them for now, but be careful about relying too strongly on
> the version having any real meaning.
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 08 Oct 2007 20:34:41 +0200
"Eliezer" <[EMAIL PROTECTED]> wrote:
> On Mon, 2007-10-08 at 10:29 -0700, Stephen Hemminger wrote:
>
> >
> > Looks good. Some minor stuff:
> > * You can use network device stats in network device structure and
> > no longer need the copy in bp
>
> We can't use net device stats in the bp because the elements are also
> used by HW and we can't rearrange them.
> struct bmac_stats and struct emac_stats are written to by the HW
> struct nig_stats and struct bnx2x_eth_stats are read by HW
> (I would have loved to get rid of at least one of these long structs.)
Here:
static void bnx2x_update_net_stats(struct bnx2x *bp)
+{
+ struct bnx2x_eth_stats *estats = bnx2x_sp(bp, eth_stats);
+
+ bp->net_stats.rx_packets =
+ bnx2x_hilo(&estats->total_unicast_packets_received_hi) +
+ bnx2x_hilo(&estats->total_multicast_packets_received_hi) +
+ bnx2x_hilo(&estats->total_broadcast_packets_received_hi);
+
+ bp->net_stats.tx_packets =
+ bnx2x_hilo(&estats->total_unicast_packets_transmitted_hi) +
+ bnx2x_hilo(&estats->total_multicast_packets_transmitted_hi) +
+ bnx2x_hilo(&estats->total_broadcast_packets_transmitted_hi);
+
+ bp->net_stats.rx_bytes = bnx2x_hilo(&estats->total_bytes_received_hi);
+
+ bp->net_stats.tx_bytes =
+ bnx2x_hilo(&estats->total_bytes_transmitted_hi);
+
+ bp->net_stats.rx_dropped = estats->no_buff_discard;
+ bp->net_stats.tx_dropped = 0;
bp->net_stats could be replaced by dev->net_stats
> > * The MACRO's for 64 bit stats look like they could be done with
> > u64 and/or turned into inline's.
>
> The MACRO's modify some of their arguments, plus they need to work on 32
> bit machines (are 64 bit counters always available on 32 bit machines?).
> so using an inline would allow the inline to be more readable but
> calling it would get ugly.
> I'm open to suggestions.
>
u64 exists on all platforms (including 32 bit).
> > * RCS/CVS tags for date and version # are kind of ugly. They git system
> > doesn't use them, perhaps you just want to track your internal version
> > control information, but what happens when changes come from outside?
>
> We need some way to correlate cvs to opensource to bug tracking tool.
> Note that driver and Microcode code are very tightly coupled on this
> device, so if I want to debug a microcode related problem I would need
> both CVS version and microcode version.
>
> I agree that tags are ugly but can't think of a better way to do it.
> Changes coming from the outside are a problem, but using the cvs version
> as an anchor I can track them using git.
> Again, I'm open to suggestions and if someone can think of an elegant
> way of doing it, I will gladly use it.
>
Leave them for now, but be careful about relying too strongly on
the version having any real meaning.
--
Stephen Hemminger <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 2007-10-08 at 10:29 -0700, Stephen Hemminger wrote: > > Looks good. Some minor stuff: > * You can use network device stats in network device structure and > no longer need the copy in bp We can't use net device stats in the bp because the elements are also used by HW and we can't rearrange them. struct bmac_stats and struct emac_stats are written to by the HW struct nig_stats and struct bnx2x_eth_stats are read by HW (I would have loved to get rid of at least one of these long structs.) > * The MACRO's for 64 bit stats look like they could be done with > u64 and/or turned into inline's. The MACRO's modify some of their arguments, plus they need to work on 32 bit machines (are 64 bit counters always available on 32 bit machines?). so using an inline would allow the inline to be more readable but calling it would get ugly. I'm open to suggestions. > * RCS/CVS tags for date and version # are kind of ugly. They git system > doesn't use them, perhaps you just want to track your internal version > control information, but what happens when changes come from outside? We need some way to correlate cvs to opensource to bug tracking tool. Note that driver and Microcode code are very tightly coupled on this device, so if I want to debug a microcode related problem I would need both CVS version and microcode version. I agree that tags are ugly but can't think of a better way to do it. Changes coming from the outside are a problem, but using the cvs version as an anchor I can track them using git. Again, I'm open to suggestions and if someone can think of an elegant way of doing it, I will gladly use it. thanks Eliezer - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 08 Oct 2007 14:34:45 +0200 "Eliezer Tamir" <[EMAIL PROTECTED]> wrote: > [resubmitting, this time without line breaks, sorry] > > This is an initial version of the BNX2X, the Linux driver for the > BCM5771X 10Gb Ethernet controller family. > > Although the chip is very different from the 5706-8 family we based the > driver code on the BNX2 driver. > > Since the hardware is supposed to be generally available soon I have > posted an initial version and after hearing all the comments I am > reposting with changes to address them. > > Some planned feature are still under development, but we want to get > whatever we have out now so people can start using the HW. > > Main changes from first version. > > * Fixed most issues raised by Michael Buesch. (Thanks Michael!) > * Some slow path bug fixes. > * Yitchak Gertner re-grouped the code in a more logical manner. > * A lot of work was done to get the generated code to comply with coding > style requirements. > > Known issues/TODO. > * Move slowpath event handling from tasklet to workqueue context. This > will allow replacing the busy waits in the link management code with sleeps. > > Please consider applying to 2.6.24 > > Thanks > Eliezer Looks good. Some minor stuff: * You can use network device stats in network device structure and no longer need the copy in bp * The MACRO's for 64 bit stats look like they could be done with u64 and/or turned into inline's. * RCS/CVS tags for date and version # are kind of ugly. They git system doesn't use them, perhaps you just want to track your internal version control information, but what happens when changes come from outside? -- Stephen Hemminger <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, 8 Oct 2007 16:02:48 +0300 Matti Aarnio wrote: > On Mon, Oct 08, 2007 at 02:34:45PM +0200, Eliezer Tamir wrote: > > Message-ID: <[EMAIL PROTECTED]> > > Date: Mon, 08 Oct 2007 14:34:45 +0200 > > From: Eliezer Tamir <[EMAIL PROTECTED]> > > User-Agent: Thunderbird 2.0.0.6 (X11/20070728) > > MIME-Version: 1.0 > > To: "netdev@vger.kernel.org" (...) > > Subject: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb > > Ethernet, take two. > > X-WSS-ID: 6B14FC184VK13380170-01-01 > > Content-Type: text/plain; > > charset=utf-8; > > format=flowed > > Content-Transfer-Encoding: 7bit > > > > [resubmitting, this time without line breaks, sorry] > > I am sorry, but you had no success in this round either. > > Thunderbird is "somewhat"(*) difficult when you try to include the source > (diff-) file into message body text. > > (*: like "serious pain in the rear section..) > > > The classical instruction is that "thy shall post source as diffs against > baseline in email message textbody" --- but now your only real choice is > in between using genuine attachments, and placing it into some public > downloadable file, which the rest of the world can download - or to use > the ATTACH -button. > > Two failures with preferred method is (IMO) a good excuse to resort on > MIME attachments. In tbird v1.5x at least, the External Editor option works fine for me to send patches inline. --- ~Randy - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
On Mon, Oct 08, 2007 at 02:34:45PM +0200, Eliezer Tamir wrote: > Message-ID: <[EMAIL PROTECTED]> > Date: Mon, 08 Oct 2007 14:34:45 +0200 > From: Eliezer Tamir <[EMAIL PROTECTED]> > User-Agent: Thunderbird 2.0.0.6 (X11/20070728) > MIME-Version: 1.0 > To: "netdev@vger.kernel.org" (...) > Subject: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb > Ethernet, take two. > X-WSS-ID: 6B14FC184VK13380170-01-01 > Content-Type: text/plain; > charset=utf-8; > format=flowed > Content-Transfer-Encoding: 7bit > > [resubmitting, this time without line breaks, sorry] I am sorry, but you had no success in this round either. Thunderbird is "somewhat"(*) difficult when you try to include the source (diff-) file into message body text. (*: like "serious pain in the rear section..) The classical instruction is that "thy shall post source as diffs against baseline in email message textbody" --- but now your only real choice is in between using genuine attachments, and placing it into some public downloadable file, which the rest of the world can download - or to use the ATTACH -button. Two failures with preferred method is (IMO) a good excuse to resort on MIME attachments. > Please consider applying to 2.6.24 > > Thanks > Eliezer Matti Aarnio - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
[resubmitting, this time without line breaks, sorry] This is an initial version of the BNX2X, the Linux driver for the BCM5771X 10Gb Ethernet controller family. Although the chip is very different from the 5706-8 family we based the driver code on the BNX2 driver. Since the hardware is supposed to be generally available soon I have posted an initial version and after hearing all the comments I am reposting with changes to address them. Some planned feature are still under development, but we want to get whatever we have out now so people can start using the HW. Main changes from first version. * Fixed most issues raised by Michael Buesch. (Thanks Michael!) * Some slow path bug fixes. * Yitchak Gertner re-grouped the code in a more logical manner. * A lot of work was done to get the generated code to comply with coding style requirements. Known issues/TODO. * Move slowpath event handling from tasklet to workqueue context. This will allow replacing the busy waits in the link management code with sleeps. Please consider applying to 2.6.24 Thanks Eliezer - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
