netdev - Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.

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]

Message-ID: <1191871085.29746.43.camel@eliezer>
Date:	Mon, 08 Oct 2007 21:18:05 +0200
From:	"Eliezer Tamir" <eliezert@...adcom.com>
To:	"Stephen Hemminger" <shemminger@...ux-foundation.org>
cc:	"davem@...emloft.net" <davem@...emloft.net>,
	"jeff@...zik.org" <jeff@...zik.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Michael Chan" <mchan@...adcom.com>
Subject: 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" <eliezert@...adcom.com> 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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html