Netdev List
 help / color / mirror / Atom feed
* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Stephen Hemminger @ 2007-10-08 21:03 UTC (permalink / raw)
  To: Michael Chan
  Cc: Eliezer, David Miller, jeff@garzik.org, netdev, eilong, vladz,
	gertner
In-Reply-To: <1191879622.5277.11.camel@dell>

On Mon, 08 Oct 2007 14:40:22 -0700
"Michael Chan" <mchan@broadcom.com> wrote:

> On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote:
> > On Mon, 08 Oct 2007 20:34:41 +0200
> > "Eliezer" <eliezert@broadcom.com> 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 <shemminger@linux-foundation.org>

^ permalink raw reply

* RE: [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-08 20:48 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter P
  Cc: David Miller, krkumar2, johnpol, herbert, kaber, shemminger,
	jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	mcarlson, jeff, mchan, general, tgraf, randy.dunlap, sri
In-Reply-To: <D5C1322C3E673F459512FB59E0DDC32903BC1234@orsmsx414.amr.corp.intel.com>

On Mon, 2007-08-10 at 12:46 -0700, Waskiewicz Jr, Peter P wrote:

> 	I still have concerns how this will work with Tx multiqueue.
> The way the batching code looks right now, you will probably send a
> batch of skb's from multiple bands from PRIO or RR to the driver.  For
> non-Tx multiqueue drivers, this is fine.  For Tx multiqueue drivers,
> this isn't fine, since the Tx ring is selected by the value of
> skb->queue_mapping (set by the qdisc on {prio|rr}_classify()).  If the
> whole batch comes in with different queue_mappings, this could prove to
> be an interesting issue.

true, that needs some resolution. Heres a hand-waving thought:
Assuming all packets of a specific map end up in the same qdiscn queue,
it seems feasible to ask the qdisc scheduler to give us enough packages
(ive seen people use that terms to refer to packets) for each hardware
ring's available space. With the patches i posted, i do that via
dev->xmit_win that assumes only one view of the driver; essentially a
single ring.  
If that is doable, then it is up to the driver to say
"i have space for 5 in ring[0], 10 in ring[1] 0 in ring[2]" based on
what scheduling scheme the driver implements - the dev->blist can stay
the same. Its a handwave, so there may be issues there and there could
be better ways to handle this.

Note: The other issue that needs resolving that i raised earlier was in
regards to multiqueue running on multiple cpus servicing different rings
concurently. 

> 	Now I see in the driver HOWTO you recently sent that the driver
> will be expected to loop over the list and call it's ->hard_start_xmit()
> for each skb.  

It's the core that does that, not the driver; the driver continues to
use ->hard_start_xmit() (albeit modified one). The idea is not to have
many new interfaces.

> I think that should be fine for multiqueue, I just wanted
> to see if you had any thoughts on how it should work, any performance
> issues you can see (I can't think of any).  Since the batching feature
> and Tx multiqueue are very new features, I'd like to make sure we can
> think of any possible issues with them coexisting before they are both
> mainline.

Isnt multiqueue mainline already?

> 	Looking ahead for multiqueue, I'm still working on the per-queue
> lock implementation for multiqueue, which I know will not work with
> batching as it's designed today.  

The point behind batching is to reduce the cost of the locks by
amortizing across the locks. Even better if one can, they should get rid
of locks. Remind me, why do you need the per-queuemap lock? And is it
needed from the enqueuing side too? Maybe lets start there to help me
understand things?

> I'm still not sure how to handle this,
> because it really would require the batch you send to have the same
> queue_mapping in each skb, so you're grabbing the correct queue_lock.

Sure, that is doable if the driver can set a per queue_mapping xmit_win
and the qdisc can be taught to say "give me packets for queue_mapping X"

> Or, we could have the core grab all the queue locks for each
> skb->queue_mapping represented in the batch.  That would block another
> batch though if it had any of those queues in it's next batch before the
> first one completed.  Thoughts?

I am not understanding the desire to have locks on a per-queuemap. I
think the single queuelock we have today should suffice. If the intent
is to have concurent cpus running to each hardware ring, then this is
what i questioned earlier whether it was the right thing to do(very top
of email where i mention it as "other issue"). 

cheers,
jamal



^ permalink raw reply

* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Michael Chan @ 2007-10-08 21:40 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Eliezer, David Miller, jeff@garzik.org, netdev, eilong, vladz,
	gertner
In-Reply-To: <20071008120855.758d10e7@freepuppy.rosehill>

On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote:
> On Mon, 08 Oct 2007 20:34:41 +0200
> "Eliezer" <eliezert@broadcom.com> 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.


^ permalink raw reply

* [ofa-general] [NET_BATCH] Some perf results
From: jamal @ 2007-10-08 19:53 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]


Ive attached a small pdf with results. This adds on top of results I
posted yesterday (although i didnt see them reflected on netdev).

1) "batch-ntlst" is the patches posted today that remove the temporary
list in qdisc restart and is derived from this AM net-2.6.24
2) "batch-kern" is result of batching patches posted yesterday that had
the temporary list and is based on net-2.6.24 from yesterday AM
3) "net-2.6.2" is yesterday's AM net-2.6.24 with no changes,

So #1 is not a completely fair comparison with #2 and #3. However,
looking at the logs, the changes that have gone in are unrelated to the
areas i have touched, so i dont expect any effect.

Overall, removing the temporay list from qdisc_restart provides a small
improvement noticeable only at the smaller packet sizes. 

In any case, enjoy.

cheers,
jamal

[-- Attachment #2: batch-res2.pdf --]
[-- Type: application/pdf, Size: 12277 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* RE: [PATCH 2/3][NET_BATCH] net core use batching
From: Waskiewicz Jr, Peter P @ 2007-10-08 19:46 UTC (permalink / raw)
  To: hadi, David Miller
  Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
	Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	mcarlson, jeff, mchan, general, tgraf, randy.dunlap, sri
In-Reply-To: <1191868010.4335.33.camel@localhost>

> -----Original Message-----
> From: J Hadi Salim [mailto:j.hadi123@gmail.com] On Behalf Of jamal
> Sent: Monday, October 08, 2007 11:27 AM
> To: David Miller
> Cc: krkumar2@in.ibm.com; johnpol@2ka.mipt.ru; 
> herbert@gondor.apana.org.au; kaber@trash.net; 
> shemminger@linux-foundation.org; jagana@us.ibm.com; 
> Robert.Olsson@data.slu.se; rick.jones2@hp.com; 
> xma@us.ibm.com; gaagaan@gmail.com; netdev@vger.kernel.org; 
> rdreier@cisco.com; Waskiewicz Jr, Peter P; 
> mcarlson@broadcom.com; jeff@garzik.org; mchan@broadcom.com; 
> general@lists.openfabrics.org; kumarkr@linux.ibm.com; 
> tgraf@suug.ch; randy.dunlap@oracle.com; sri@us.ibm.com
> Subject: [PATCH 2/3][NET_BATCH] net core use batching
> 
> This patch adds the usage of batching within the core.
> 
> cheers,
> jamal

Hey Jamal,
	I still have concerns how this will work with Tx multiqueue.
The way the batching code looks right now, you will probably send a
batch of skb's from multiple bands from PRIO or RR to the driver.  For
non-Tx multiqueue drivers, this is fine.  For Tx multiqueue drivers,
this isn't fine, since the Tx ring is selected by the value of
skb->queue_mapping (set by the qdisc on {prio|rr}_classify()).  If the
whole batch comes in with different queue_mappings, this could prove to
be an interesting issue.
	Now I see in the driver HOWTO you recently sent that the driver
will be expected to loop over the list and call it's ->hard_start_xmit()
for each skb.  I think that should be fine for multiqueue, I just wanted
to see if you had any thoughts on how it should work, any performance
issues you can see (I can't think of any).  Since the batching feature
and Tx multiqueue are very new features, I'd like to make sure we can
think of any possible issues with them coexisting before they are both
mainline.

	Looking ahead for multiqueue, I'm still working on the per-queue
lock implementation for multiqueue, which I know will not work with
batching as it's designed today.  I'm still not sure how to handle this,
because it really would require the batch you send to have the same
queue_mapping in each skb, so you're grabbing the correct queue_lock.
Or, we could have the core grab all the queue locks for each
skb->queue_mapping represented in the batch.  That would block another
batch though if it had any of those queues in it's next batch before the
first one completed.  Thoughts?

Thanks Jamal,

-PJ Waskiewicz
<peter.p.waskiewicz.jr@intel.com>

^ permalink raw reply

* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Eliezer Tamir @ 2007-10-08 19:18 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem@davemloft.net, jeff@garzik.org, netdev@vger.kernel.org,
	Michael Chan
In-Reply-To: <20071008120855.758d10e7@freepuppy.rosehill>

On Mon, 2007-10-08 at 12:08 -0700, Stephen Hemminger wrote:
> On Mon, 08 Oct 2007 20:34:41 +0200
> "Eliezer" <eliezert@broadcom.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.
> 



^ permalink raw reply

* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Stephen Hemminger @ 2007-10-08 19:08 UTC (permalink / raw)
  To: Eliezer
  Cc: davem@davemloft.net, jeff@garzik.org, netdev@vger.kernel.org,
	Michael Chan, eilong, vladz, gertner
In-Reply-To: <1191868481.29746.27.camel@eliezer>

On Mon, 08 Oct 2007 20:34:41 +0200
"Eliezer" <eliezert@broadcom.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


> > * 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 <shemminger@linux-foundation.org>

^ permalink raw reply

* Re: [PATCH 0/8][BNX2X] resubmit as attachments: New driver for Broadcom 10Gb Ethernet, take two.
From: Eliezer Tamir @ 2007-10-08 18:54 UTC (permalink / raw)
  To: davem
  Cc: jeff, netdev, Michael Chan, Eilon Greenstein, Vladislav Zolotarov,
	Yitchak Gertner
In-Reply-To: <470A5785.5010104@broadcom.com>

Here are FTP links.

All the parts in a zipped tar.
ftp://Net_sys_anon@ftp1.broadcom.com/bnx2x-0.40.10-net-2.6.24.tgz

One big patch.
ftp://Net_sys_anon@ftp1.broadcom.com/bnx2x-0.40.10-net-2.6.24-one.patch.txt

I noticed that the big microcode files did not seem to get through to netdev, so here they are:
ftp://Net_sys_anon@ftp1.broadcom.com/0006-add-bnx2x_init_values.h.txt
ftp://Net_sys_anon@ftp1.broadcom.com/0008-add-bnx2x_asm.h.txt

I want to thank Michael Chan and Matti Aarnio for helping me with resolving the email problem.


Thanks
Eliezer



^ permalink raw reply

* raw PF_PACKET protocol selection
From: Joakim Tjernlund @ 2007-10-08 18:36 UTC (permalink / raw)
  To: Netdev

Hi List

I trying to open my own raw PF_PACKET socket to receive 
pkgs sent to this socket. I can only make ETH_P_ALL protocol
work, but then I receive all pkgs and I want pkgs with a specific
protocol type. I have tried lots of ETH_P types and none of them work.
Naturally I make sure the sender is using the same protocol as my test
program below. I guess I must be doing something wrong???


Here is the test program:

#include <stdio.h>
#include <errno.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <linux/in.h>
#include <linux/if_ether.h>

int main(int argc, char **argv) {
  int sock, n;
  char buffer[2048];
  unsigned char *iphead, *ethhead;

  if ( (sock=socket(PF_PACKET, SOCK_RAW,
                    htons(ETH_P_IP)))<0) { // ETH_P_IP is just an example
    perror("socket");
    exit(1);
  }

  while (1) {
    printf("----------\n");
    n = recvfrom(sock,buffer,2048,0,NULL,NULL);
    printf("%d bytes read\n",n);

    /* Check to see if the packet contains at least
     * complete Ethernet (14), IP (20) and TCP/UDP
     * (8) headers.
     */
    if (n<42) {
      perror("recvfrom():");
      printf("Incomplete packet (errno is %d)\n",
             errno);
      close(sock);
      exit(0);
    }

    ethhead = buffer;
    printf("Source MAC address: "
           "%02x:%02x:%02x:%02x:%02x:%02x\n",
           ethhead[0],ethhead[1],ethhead[2],
           ethhead[3],ethhead[4],ethhead[5]);
    printf("Destination MAC address: "
           "%02x:%02x:%02x:%02x:%02x:%02x\n",
           ethhead[6],ethhead[7],ethhead[8],
           ethhead[9],ethhead[10],ethhead[11]);

    iphead = buffer+14; /* Skip Ethernet header */
    if (*iphead==0x45) { /* Double check for IPv4
                          * and no options present */
      printf("Source host %d.%d.%d.%d\n",
             iphead[12],iphead[13],
             iphead[14],iphead[15]);
      printf("Dest host %d.%d.%d.%d\n",
             iphead[16],iphead[17],
             iphead[18],iphead[19]);
      printf("Source,Dest ports %d,%d\n",
             (iphead[20]<<8)+iphead[21],
             (iphead[22]<<8)+iphead[23]);
      printf("Layer-4 protocol %d\n",iphead[9]);
    } else {
       int i;
       iphead = buffer+12;
       for(i=0; i<n-12; i++)
	  printf(" pkt:%x, %c\n",
	      iphead[i],iphead[i]);
    }
  }

}



^ permalink raw reply

* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Eliezer @ 2007-10-08 18:34 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem@davemloft.net, jeff@garzik.org, netdev@vger.kernel.org,
	Michael Chan, eilong, vladz, gertner
In-Reply-To: <20071008102919.1cbc7983@freepuppy.rosehill>

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



^ permalink raw reply

* RE: [Bugme-new] [Bug 9132] New: fcntl GET_OWN reports 0 for sockets instead of PID
From: François-Frédéric Ozog @ 2007-10-08 18:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: bugme-daemon, netdev

You are correct, I have tested back to 2.6.18 and the bug is still here. So I don't know when the problem did NOT occur.

François-Frédéric

-----Message d'origine-----
De : Andrew Morton [mailto:akpm@linux-foundation.org] 
Envoyé : lundi 8 octobre 2007 19:11
À : François-Frédéric Ozog
Cc : bugme-daemon@bugzilla.kernel.org; netdev@vger.kernel.org
Objet : Re: [Bugme-new] [Bug 9132] New: fcntl GET_OWN reports 0 for sockets instead of PID


(please respond via emailed reply-to-all, not vie the bugzilla web interface)

On Mon,  8 Oct 2007 09:18:02 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9132
> 
>            Summary: fcntl GET_OWN reports 0 for sockets instead of PID
>            Product: Networking
>            Version: 2.5
>      KernelVersion: 2.6.22
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>         AssignedTo: acme@ghostprotocols.net
>         ReportedBy: ff@ozog.com
> 
> 
> Most recent kernel where this bug did not occur: 2.6.22
> Distribution: Kubuntu
> Hardware Environment: VMWare workstation 6
> Software Environment: 
> Problem Description: calling fcntl with F_GETOWN on a socket gives allways zero
> instead of the PID of the socket. The fcntl code is right but the data is zero
> in the struct file element. It sounds like pid is not set when the socket is
> attached to the fd in sock_attach_fd() of net/socket.c.
> 
> I would add something like this:
> 
> file->f_owner.pid=find_get_pid(task->pid);
> file->f_owner.pid_type=PIDTYPE_PID;
> 
> in sock_attach_fd() of net/socket.c
> 
> Steps to reproduce:
> 
> s=socket(...);
> pid=fcntl(s, F_GETOWN);
> 

You state that the problem is present in 2.6.22 and also did not occur in
2.6.22.  I assume it has always been like this.


^ permalink raw reply

* [ofa-general] [PATCH 3/3][NET_BATCH] kill dev->gso_skb
From: jamal @ 2007-10-08 18:27 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber

[-- Attachment #1: Type: text/plain, Size: 97 bytes --]

This patch removes dev->gso_skb as it is no longer necessary with
batching code.

cheers,
jamal


[-- Attachment #2: 03-kill-dev-gso-skb.patch --]
[-- Type: text/x-patch, Size: 2277 bytes --]

[NET_BATCH] kill dev->gso_skb
The batching code does what gso used to batch at the drivers.
There is no more need for gso_skb. If for whatever reason the
requeueing is a bad idea we are going to leave packets in dev->blist
(and still not need dev->gso_skb)

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit 3a6202d62adff75b85d6ca0f8fd491abf9d63f4b
tree 80497c538bdb3eab6ab81ff1dec1c3263da79826
parent 63381156b35719a364d0f81fec487e6263fb5467
author Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:11:02 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:11:02 -0400

 include/linux/netdevice.h |    3 ---
 net/sched/sch_generic.c   |   12 ------------
 2 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b31df5c..4ddc6eb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -577,9 +577,6 @@ struct net_device
 	struct list_head	qdisc_list;
 	unsigned long		tx_queue_len;	/* Max frames per queue allowed */
 
-	/* Partially transmitted GSO packet. */
-	struct sk_buff		*gso_skb;
-
 	/* ingress path synchronizer */
 	spinlock_t		ingress_lock;
 	struct Qdisc		*qdisc_ingress;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 96bfdcb..9112ea0 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -172,13 +172,6 @@ static int xmit_get_pkts(struct net_device *dev,
 	struct sk_buff *skb;
 	int count = dev->xmit_win;
 
-	if (count  && dev->gso_skb) {
-		skb = dev->gso_skb;
-		dev->gso_skb = NULL;
-		count -= xmit_count_skbs(skb);
-		__skb_queue_tail(pktlist, skb);
-	}
-
 	while (count > 0) {
 		skb = q->dequeue(q);
 		if (!skb)
@@ -647,7 +640,6 @@ void dev_activate(struct net_device *dev)
 void dev_deactivate(struct net_device *dev)
 {
 	struct Qdisc *qdisc;
-	struct sk_buff *skb;
 
 	spin_lock_bh(&dev->queue_lock);
 	qdisc = dev->qdisc;
@@ -655,15 +647,11 @@ void dev_deactivate(struct net_device *dev)
 
 	qdisc_reset(qdisc);
 
-	skb = dev->gso_skb;
-	dev->gso_skb = NULL;
 	if (!skb_queue_empty(&dev->blist))
 		skb_queue_purge(&dev->blist);
 	dev->xmit_win = 1;
 	spin_unlock_bh(&dev->queue_lock);
 
-	kfree_skb(skb);
-
 	dev_watchdog_down(dev);
 
 	/* Wait for outstanding dev_queue_xmit calls. */

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-08 18:26 UTC (permalink / raw)
  To: David Miller
  Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
	Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
	tgraf, randy.dunlap, sri

[-- Attachment #1: Type: text/plain, Size: 71 bytes --]

This patch adds the usage of batching within the core.

cheers,
jamal


[-- Attachment #2: 02-net-core-use-batching.patch --]
[-- Type: text/x-patch, Size: 4790 bytes --]

[NET_BATCH] net core use batching

This patch adds the usage of batching within the core.
Performance results demonstrating improvement are provided separately.

I have #if-0ed some of the old functions so the patch is more readable.
A future patch will remove all if-0ed content.
Patrick McHardy eyeballed a bug that will cause re-ordering in case
of a requeue.

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit 63381156b35719a364d0f81fec487e6263fb5467
tree 615dfb5f8617a8a4edffbb965e81a75c60597319
parent 8b9960cfbb98d48c0ac30b2c00d27c25217adb26
author Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:10:14 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:10:14 -0400

 net/sched/sch_generic.c |  116 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 95ae119..96bfdcb 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,7 @@ static inline int qdisc_qlen(struct Qdisc *q)
 	return q->q.qlen;
 }
 
+#if 0
 static inline int dev_requeue_skb(struct sk_buff *skb, struct net_device *dev,
 				  struct Qdisc *q)
 {
@@ -110,6 +111,97 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
 
 	return ret;
 }
+#endif
+
+static inline int handle_dev_cpu_collision(struct net_device *dev)
+{
+	if (unlikely(dev->xmit_lock_owner == smp_processor_id())) {
+		if (net_ratelimit())
+			printk(KERN_WARNING
+				"Dead loop on netdevice %s, fix it urgently!\n",
+				dev->name);
+		return 1;
+	}
+	__get_cpu_var(netdev_rx_stat).cpu_collision++;
+	return 0;
+}
+
+static inline int
+dev_requeue_skbs(struct sk_buff_head *skbs, struct net_device *dev,
+	       struct Qdisc *q)
+{
+
+	struct sk_buff *skb;
+
+	while ((skb = __skb_dequeue_tail(skbs)) != NULL)
+		q->ops->requeue(skb, q);
+
+	netif_schedule(dev);
+	return 0;
+}
+
+static inline int
+xmit_islocked(struct sk_buff_head *skbs, struct net_device *dev,
+	    struct Qdisc *q)
+{
+	int ret = handle_dev_cpu_collision(dev);
+
+	if (ret) {
+		if (!skb_queue_empty(skbs))
+			skb_queue_purge(skbs);
+		return qdisc_qlen(q);
+	}
+
+	return dev_requeue_skbs(skbs, dev, q);
+}
+
+static int xmit_count_skbs(struct sk_buff *skb)
+{
+	int count = 0;
+	for (; skb; skb = skb->next) {
+		count += skb_shinfo(skb)->nr_frags;
+		count += 1;
+	}
+	return count;
+}
+
+static int xmit_get_pkts(struct net_device *dev,
+			   struct Qdisc *q,
+			   struct sk_buff_head *pktlist)
+{
+	struct sk_buff *skb;
+	int count = dev->xmit_win;
+
+	if (count  && dev->gso_skb) {
+		skb = dev->gso_skb;
+		dev->gso_skb = NULL;
+		count -= xmit_count_skbs(skb);
+		__skb_queue_tail(pktlist, skb);
+	}
+
+	while (count > 0) {
+		skb = q->dequeue(q);
+		if (!skb)
+			break;
+
+		count -= xmit_count_skbs(skb);
+		__skb_queue_tail(pktlist, skb);
+	}
+
+	return skb_queue_len(pktlist);
+}
+
+static int xmit_prepare_pkts(struct net_device *dev,
+			     struct sk_buff_head *tlist)
+{
+	struct sk_buff *skb;
+	struct sk_buff_head *flist = &dev->blist;
+
+	while ((skb = __skb_dequeue(tlist)) != NULL)
+		xmit_prepare_skb(skb, dev);
+
+	return skb_queue_len(flist);
+}
 
 /*
  * NOTE: Called under dev->queue_lock with locally disabled BH.
@@ -130,22 +222,23 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
  *				>0 - queue is not empty.
  *
  */
+
 static inline int qdisc_restart(struct net_device *dev)
 {
 	struct Qdisc *q = dev->qdisc;
-	struct sk_buff *skb;
-	int ret;
+	int ret = 0;
 
-	/* Dequeue packet */
-	if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
-		return 0;
+	ret = xmit_get_pkts(dev, q, &dev->blist);
 
+	if (!ret)
+		return 0;
 
-	/* And release queue */
+	/* We got em packets */
 	spin_unlock(&dev->queue_lock);
 
+	/* bye packets ....*/
 	HARD_TX_LOCK(dev, smp_processor_id());
-	ret = dev_hard_start_xmit(skb, dev);
+	ret = dev_batch_xmit(dev);
 	HARD_TX_UNLOCK(dev);
 
 	spin_lock(&dev->queue_lock);
@@ -158,8 +251,8 @@ static inline int qdisc_restart(struct net_device *dev)
 		break;
 
 	case NETDEV_TX_LOCKED:
-		/* Driver try lock failed */
-		ret = handle_dev_cpu_collision(skb, dev, q);
+		/* Driver lock failed */
+		ret = xmit_islocked(&dev->blist, dev, q);
 		break;
 
 	default:
@@ -168,7 +261,7 @@ static inline int qdisc_restart(struct net_device *dev)
 			printk(KERN_WARNING "BUG %s code %d qlen %d\n",
 			       dev->name, ret, q->q.qlen);
 
-		ret = dev_requeue_skb(skb, dev, q);
+		ret = dev_requeue_skbs(&dev->blist, dev, q);
 		break;
 	}
 
@@ -564,6 +657,9 @@ void dev_deactivate(struct net_device *dev)
 
 	skb = dev->gso_skb;
 	dev->gso_skb = NULL;
+	if (!skb_queue_empty(&dev->blist))
+		skb_queue_purge(&dev->blist);
+	dev->xmit_win = 1;
 	spin_unlock_bh(&dev->queue_lock);
 
 	kfree_skb(skb);

^ permalink raw reply related

* [PATCH 1/3] [NET_BATCH] Introduce batching interface
From: jamal @ 2007-10-08 18:24 UTC (permalink / raw)
  To: David Miller
  Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
	Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
	tgraf, randy.dunlap, sri

[-- Attachment #1: Type: text/plain, Size: 76 bytes --]

This patch introduces the netdevice interface for batching.

cheers,
jamal


[-- Attachment #2: 01-introduce-batching-interface.patch --]
[-- Type: text/x-patch, Size: 6320 bytes --]

[NET_BATCH] Introduce batching interface

This patch introduces the netdevice interface for batching.

BACKGROUND
---------

A driver dev->hard_start_xmit() has 4 typical parts:
a) packet formating (example vlan, mss, descriptor counting etc)
b) chip specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew on,
tx completion interupts, set last tx time, etc

[For code cleanliness/readability sake, regardless of this work,
one should break the dev->hard_start_xmit() into those 4 functions
anyways].

INTRODUCING API
---------------

With the api introduced in this patch, a driver which has all
4 parts and needing to support batching is advised to split its
dev->hard_start_xmit() in the following manner:
1)Remove #d from dev->hard_start_xmit() and put it in
dev->hard_end_xmit() method.
2)#b and #c can stay in ->hard_start_xmit() (or whichever way you want
to do this)
3) #a is deffered to future work to reduce confusion (since it holds
on its own).

Note: There are drivers which may need not support any of the two
approaches (example the tun driver i patched) so the methods are
optional.

xmit_win variable is set by the driver to tell the core how much space
it has to take on new skbs. It is introduced to ensure that when we pass
the driver a list of packets it will swallow all of them - which is
useful because we dont requeue to the qdisc (and avoids burning
unnecessary cpu cycles or introducing any strange re-ordering). The driver
tells us when it invokes netif_wake_queue how much space it has for
descriptors by setting this variable.

Refer to the driver howto for more details.

THEORY OF OPERATION
-------------------

1. Core dequeues from qdiscs upto dev->xmit_win packets. Fragmented
and GSO packets are accounted for as well.
2. Core grabs TX_LOCK
3. Core loop for all skbs:
                    invokes driver dev->hard_start_xmit()
4. Core invokes driver dev->hard_end_xmit()

ACKNOWLEDGEMENT AND SOME HISTORY
--------------------------------

There's a lot of history and reasoning of "why batching" in a document
i am writting which i may submit as a patch.
Thomas Graf (who doesnt know this probably) gave me the impetus to
start looking at this back in 2004 when he invited me to the linux
conference he was organizing. Parts of what i presented in SUCON in
2004 talk about batching. Herbert Xu forced me to take a second look around
2.6.18 - refer to my netconf 2006 presentation. Krishna Kumar provided
me with more motivation in May 2007 when he posted on netdev and engaged
me.
Sridhar Samudrala, Krishna Kumar, Matt Carlson, Michael Chan,
Jeremy Ethridge, Evgeniy Polyakov, Sivakumar Subramani, David Miller,
and Patrick McHardy, Jeff Garzik and Bill Fink have contributed in one or
more of {bug fixes, enhancements, testing, lively discussion}. The
Broadcom and neterion folks have been outstanding in their help.

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit 8b9960cfbb98d48c0ac30b2c00d27c25217adb26
tree d8ab164757cee431f71a6011df4b09e0c382e2da
parent cef811600354e9f41f059570fc877d19a1daed99
author Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:03:42 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:03:42 -0400

 include/linux/netdevice.h |   11 +++++++++++
 net/core/dev.c            |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 91cd3f3..b31df5c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -467,6 +467,7 @@ struct net_device
 #define NETIF_F_NETNS_LOCAL	8192	/* Does not change network namespaces */
 #define NETIF_F_MULTI_QUEUE	16384	/* Has multiple TX/RX queues */
 #define NETIF_F_LRO		32768	/* large receive offload */
+#define NETIF_F_BTX		65536	/* Capable of batch tx */
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16
@@ -595,6 +596,9 @@ struct net_device
 	void			*priv;	/* pointer to private data	*/
 	int			(*hard_start_xmit) (struct sk_buff *skb,
 						    struct net_device *dev);
+	void			(*hard_end_xmit) (struct net_device *dev);
+	int			xmit_win;
+
 	/* These may be needed for future network-power-down code. */
 	unsigned long		trans_start;	/* Time (in jiffies) of last Tx	*/
 
@@ -609,6 +613,7 @@ struct net_device
 
 	/* delayed register/unregister */
 	struct list_head	todo_list;
+	struct sk_buff_head     blist;
 	/* device index hash chain */
 	struct hlist_node	index_hlist;
 
@@ -1044,6 +1049,12 @@ extern int		dev_set_mac_address(struct net_device *,
 					    struct sockaddr *);
 extern int		dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev);
+extern int		dev_batch_xmit(struct net_device *dev);
+extern int		prepare_gso_skb(struct sk_buff *skb,
+					struct net_device *dev,
+					struct sk_buff_head *skbs);
+extern int		xmit_prepare_skb(struct sk_buff *skb,
+					 struct net_device *dev);
 
 extern int		netdev_budget;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 1aa0704..164977f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1559,6 +1559,44 @@ out_kfree_skb:
 	return 0;
 }
 
+int dev_batch_xmit(struct net_device *dev)
+{
+	struct sk_buff_head *skbs = &dev->blist;
+	int rc = NETDEV_TX_OK;
+	int tqd = 0;
+	struct sk_buff *skb;
+	int orig_w = dev->xmit_win;
+	int orig_pkts = skb_queue_len(skbs);
+
+	while ((skb = __skb_dequeue(skbs)) != NULL) {
+		rc = dev_hard_start_xmit(skb, dev);
+		if (unlikely(rc))
+			break;
+		tqd++;
+	}
+
+	/* driver is likely buggy and lied to us on how much
+	 * space it had. Damn you driver ..
+	*/
+	if (unlikely(skb_queue_len(skbs))) {
+		printk(KERN_WARNING "Likely bug %s %s (%d) "
+				"left %d/%d window now %d, orig %d\n",
+			dev->name, rc?"busy":"locked",
+			netif_queue_stopped(dev),
+			skb_queue_len(skbs),
+			orig_pkts,
+			dev->xmit_win,
+			orig_w);
+			rc = NETDEV_TX_BUSY;
+	}
+
+	if (tqd)
+		if (dev->hard_end_xmit)
+			dev->hard_end_xmit(dev);
+
+	return rc;
+}
+
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit
@@ -3581,6 +3619,8 @@ int register_netdevice(struct net_device *dev)
 		}
 	}
 
+	dev->xmit_win = 1;
+	skb_queue_head_init(&dev->blist);
 	ret = netdev_register_kobject(dev);
 	if (ret)
 		goto err_uninit;

^ permalink raw reply related

* Fixed PHY regression
From: Joakim Tjernlund @ 2007-10-08 18:17 UTC (permalink / raw)
  To: davem, netdev


David, any chance the second item, Oops while modprobing phy fixed module, in
http://marc.info/?l=linux-kernel&m=119178673421891&w=2
will make it into 2.6.23? 

The fix is now in your tree,
http://git.kernel.org/?p=linux/kernel/git/davem/bak-net-2.6.24.git;a=commit;h=bbb4c0c35a4c2aed5e025b668c8dfc99c5b74cff

It has been in Andrews tree for a while, then in Garzik and now it is in your tree.

 Regards Jocke


^ permalink raw reply

* [ofa-general] [PATCHES] TX batching rev2
From: jamal @ 2007-10-08 18:21 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber


Please provide feedback on the code and/or architecture.
Last time i posted them i received little. They are now updated to 
work with the latest net-2.6.24 from a few hours ago.

Patch 1: Introduces batching interface
Patch 2: Core uses batching interface
Patch 3: get rid of dev->gso_skb

What has changed since i posted last:
Killed the temporary packet list that is passed to qdisc restart.

Dave please let me know if this meets your desires to allow devices
which are SG and able to compute CSUM benefit just in case i
misunderstood. 
Herbert, if you can look at at least patch 3 i will appreaciate it
(since it kills dev->gso_skb that you introduced).

UPCOMING PATCHES
---------------
As before:
More patches to follow later if i get some feedback - i didnt want to 
overload people by dumping too many patches. Most of these patches 
mentioned below are ready to go; some need some re-testing and others 
need a little porting from an earlier kernel: 
- tg3 driver 
- tun driver
- pktgen
- netiron driver
- e1000 driver (LLTX)
- e1000e driver (non-LLTX)
- ethtool interface
- There is at least one other driver promised to me

Theres also a driver-howto i wrote that was posted on netdev last week
as well as one that describes the architectural decisions made.

PERFORMANCE TESTING
--------------------

System under test hardware is still a 2xdual core opteron with a 
couple of tg3s. 
A test tool generates udp traffic of different sizes for upto 60 
seconds per run or a total of 30M packets. I have 4 threads each 
running on a specific CPU which keep all the CPUs as busy as they can 
sending packets targetted at a directly connected box's udp discard
port.
All 4 CPUs target a single tg3 to send. The receiving box has a tc rule 
which counts and drops all incoming udp packets to discard port - this
allows me to make sure that the receiver is not the bottleneck in the
testing. Packet sizes sent are {8B, 32B, 64B, 128B, 256B, 512B, 1024B}. 
Each packet size run is repeated 10 times to ensure that there are no
transients. The average of all 10 runs is then computed and collected.

I do plan also to run forwarding and TCP tests in the future when the
dust settles.

cheers,
jamal

^ permalink raw reply

* [patch 04/12] sky2: fix transmit state on resume
From: Greg KH @ 2007-10-08 18:06 UTC (permalink / raw)
  To: linux-kernel, stable, Krzysztof Oledzki, Greg KH
  Cc: Justin Forbes, Zwane Mwaikambo, Theodore Ts'o, Randy Dunlap,
	Dave Jones, Chuck Wolber, Chris Wedgwood, Michael Krufky,
	Chuck Ebbert, Domenico Andreoli, torvalds, akpm, alan, netdev,
	Stephen Hemminger
In-Reply-To: <20071008180551.GA7627@kroah.com>

[-- Attachment #1: sky2-tx-sum-resume.patch --]
[-- Type: text/plain, Size: 1293 bytes --]

From: Stephen Hemminger <shemminger@linux-foundation.org>

Already upstream.

After resume, driver has reset the chip so the current state
of transmit checksum offload state machine and DMA state machine
will be undefined.

The fix is to set the state so that first Tx will set MSS and offset
values.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/sky2.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -831,6 +831,20 @@ static inline struct sky2_tx_le *get_tx_
 	return le;
 }
 
+static void tx_init(struct sky2_port *sky2)
+{
+	struct sky2_tx_le *le;
+
+	sky2->tx_prod = sky2->tx_cons = 0;
+	sky2->tx_tcpsum = 0;
+	sky2->tx_last_mss = 0;
+
+	le = get_tx_le(sky2);
+	le->addr = 0;
+	le->opcode = OP_ADDR64 | HW_OWNER;
+	sky2->tx_addr64 = 0;
+}
+
 static inline struct tx_ring_info *tx_le_re(struct sky2_port *sky2,
 					    struct sky2_tx_le *le)
 {
@@ -1244,7 +1258,8 @@ static int sky2_up(struct net_device *de
 				GFP_KERNEL);
 	if (!sky2->tx_ring)
 		goto err_out;
-	sky2->tx_prod = sky2->tx_cons = 0;
+
+	tx_init(sky2);
 
 	sky2->rx_le = pci_alloc_consistent(hw->pdev, RX_LE_BYTES,
 					   &sky2->rx_le_map);

-- 

^ permalink raw reply

* [patch 03/12] sky2: fix VLAN receive processing
From: Greg KH @ 2007-10-08 18:06 UTC (permalink / raw)
  To: linux-kernel, stable, Krzysztof Oledzki, Greg KH
  Cc: Justin Forbes, Zwane Mwaikambo, Theodore Ts'o, Randy Dunlap,
	Dave Jones, Chuck Wolber, Chris Wedgwood, Michael Krufky,
	Chuck Ebbert, Domenico Andreoli, torvalds, akpm, alan, netdev,
	Pierre-Yves Ritschard, Stephen Hemminger
In-Reply-To: <20071008180551.GA7627@kroah.com>

[-- Attachment #1: sky2-vlan-len.patch --]
[-- Type: text/plain, Size: 1315 bytes --]

From: Stephen Hemminger <shemminger@linux-foundation.org>

Already upstream.

The length check for truncated frames was not correctly handling
the case where VLAN acceleration had already read the tag.
Also, the Yukon EX has some features that use high bit of status
as security tag.

Signed-off-by: Pierre-Yves Ritschard <pyr@spootnik.org>
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/sky2.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -2049,6 +2049,7 @@ static struct sk_buff *sky2_receive(stru
  	struct sky2_port *sky2 = netdev_priv(dev);
 	struct rx_ring_info *re = sky2->rx_ring + sky2->rx_next;
 	struct sk_buff *skb = NULL;
+	u16 count;
 
 	if (unlikely(netif_msg_rx_status(sky2)))
 		printk(KERN_DEBUG PFX "%s: rx slot %u status 0x%x len %d\n",
@@ -2063,7 +2064,13 @@ static struct sk_buff *sky2_receive(stru
 	if (!(status & GMR_FS_RX_OK))
 		goto resubmit;
 
-	if (status >> 16 != length)
+	count = (status & GMR_FS_LEN) >> 16;
+#ifdef SKY2_VLAN_TAG_USED
+	/* Account for vlan tag */
+	if (sky2->vlgrp && (status & GMR_FS_VLAN))
+		count -= VLAN_HLEN;
+#endif
+	if (count != length)
 		goto len_mismatch;
 
 	if (length < copybreak)

-- 

^ permalink raw reply

* [ofa-general] [DOC][NET_BATCH] Driver howto
From: jamal @ 2007-10-08 18:10 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber

[-- Attachment #1: Type: text/plain, Size: 140 bytes --]

This is an updated driver howto for batching that works with patches
from yesterday and the revised ones i am going to post.

cheers,
jamal

[-- Attachment #2: batch-driver-howto.txt --]
[-- Type: text/plain, Size: 8256 bytes --]

Here's the beginning of a howto for driver authors.

The intended audience for this howto is people already
familiar with netdevices.

1.0  Netdevice Prerequisites
------------------------------

For hardware-based netdevices, you must have at least hardware that 
is capable of doing DMA with many descriptors; i.e., having hardware 
with a queue length of 3 (as in some fscked ethernet hardware) is 
not very useful in this case.

2.0  What is new in the driver API
-----------------------------------

There is 1 new method and one new variable introduced that the
driver author needs to be aware of. These are:
1) dev->hard_end_xmit()
2) dev->xmit_win

2.1 Using Core driver changes
-----------------------------

To provide context, let's look at a typical driver abstraction
for dev->hard_start_xmit(). It has 4 parts:
a) packet formatting (example: vlan, mss, descriptor counting, etc.)
b) chip-specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew 
on, tx completion interrupts, etc.

[For code cleanliness/readability sake, regardless of this work,
one should break the dev->hard_start_xmit() into those 4 functional
blocks anyways].

A driver which has all 4 parts and needing to support batching is 
advised to split its dev->hard_start_xmit() in the following manner:

1) use its dev->hard_end_xmit() method to achieve #d
2) use dev->xmit_win to tell the core how much space you have.

#b and #c can stay in ->hard_start_xmit() (or whichever way you 
want to do this)
Section 3. shows more details on the suggested usage.

2.1.1 Theory of operation
--------------------------

1. Core dequeues from qdiscs upto dev->xmit_win packets. Fragmented
and GSO packets are accounted for as well.
2. Core grabs device's TX_LOCK
3. Core loop for all skbs:
             ->invokes driver dev->hard_start_xmit()
4. Core invokes driver dev->hard_end_xmit() if packets xmitted

2.1.1.1 The slippery LLTX
-------------------------

Since these type of drivers are being phased out and they require extra
code they will not be supported anymore. So as oct07 the code that 
supports them has been removed.

2.1.1.2 xmit_win
----------------

dev->xmit_win variable is set by the driver to tell us how
much space it has in its rings/queues. This detail is then 
used to figure out how many packets are retrieved from the qdisc 
queues (in order to send to the driver). 
dev->xmit_win is introduced to ensure that when we pass the driver 
a list of packets it will swallow all of them -- which is useful 
because we don't requeue to the qdisc (and avoids burning unnecessary 
CPU cycles or introducing any strange re-ordering). 
Essentially the driver signals us how much space it has for descriptors 
by setting this variable. 

2.1.1.2.1 Setting xmit_win
--------------------------

This variable should be set during xmit path shutdown(netif_stop), 
wakeup(netif_wake) and ->hard_end_xmit(). In the case of the first
one the value is set to 1 and in the other two it is set to whatever
the driver deems to be available space on the ring.

3.0 Driver Essentials
---------------------

The typical driver tx state machine is:

----
-1-> +Core sends packets
     +--> Driver puts packet onto hardware queue
     +    if hardware queue is full, netif_stop_queue(dev)
     +
-2-> +core stops sending because of netif_stop_queue(dev)
..
.. time passes ...
..
-3-> +---> driver has transmitted packets, opens up tx path by
          invoking netif_wake_queue(dev)
-1-> +Cycle repeats and core sends more packets (step 1).
----

3.1  Driver prerequisite
--------------------------

This is _a very important_ requirement in making batching useful.
The prerequisite for batching changes is that the driver should 
provide a low threshold to open up the tx path.
Drivers such as tg3 and e1000 already do this.
Before you invoke netif_wake_queue(dev) you check if there is a
threshold of space reached to insert new packets.

Here's an example of how I added it to tun driver. Observe the
setting of dev->xmit_win.

---
+#define NETDEV_LTT 4 /* the low threshold to open up the tx path */
..
..
	u32 t = skb_queue_len(&tun->readq);
	if (netif_queue_stopped(tun->dev) && t < NETDEV_LTT) {
		tun->dev->xmit_win = tun->dev->tx_queue_len;
		netif_wake_queue(tun->dev);
	}
---

Heres how the batching e1000 driver does it:

--
if (unlikely(cleaned && netif_carrier_ok(netdev) &&
     E1000_DESC_UNUSED(tx_ring) >= TX_WAKE_THRESHOLD)) {

	if (netif_queue_stopped(netdev)) {
	       int rspace =  E1000_DESC_UNUSED(tx_ring) - (MAX_SKB_FRAGS +  2);
	       netdev->xmit_win = rspace;
	       netif_wake_queue(netdev);
       }
---

in tg3 code (with no batching changes) looks like:

-----
	if (netif_queue_stopped(tp->dev) &&
		(tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))
			netif_wake_queue(tp->dev);
---

3.2 Driver Setup
-----------------

*) On initialization (before netdev registration)
 1) set NETIF_F_BTX in dev->features 
  i.e., dev->features |= NETIF_F_BTX
  This makes the core do proper initialization.

 2) set dev->xmit_win to something reasonable like
  maybe half the tx DMA ring size etc.

 3) create proper pointer to the ->hard_end_xmit() method.

3.3 Annotation on the different methods 
----------------------------------------
This section shows examples and offers suggestions on how the different 
methods and variable could be used.

3.3.1 dev->hard_start_xmit()
----------------------------
  
Here's an example of tx routine that is similar to the one I added 
to the current tun driver. bxmit suffix is kept so that you can turn
off batching if needed via an ethtool interface 
and call already existing interface.

----
  static int xxx_net_bxmit(struct net_device *dev)
  {
  ....
  ....
	enqueue onto hardware ring
				           
	if (hardware ring full) {
		  netif_stop_queue(dev);
		  dev->xmit_win = 1;
	}

   .......
   ..
   .
  }
------

All return codes like NETDEV_TX_OK etc. still apply.

3.3.2 The tx complete, dev->hard_end_xmit()
-------------------------------------------------
  
In this method, if there are any IO operations that apply to a 
set of packets such as kicking DMA, setting of interrupt thresholds etc.,
leave them to the end and apply them once if you have successfully enqueued. 
This provides a mechanism for saving a lot of CPU cycles since IO
is cycle expensive.
Here is a simplified tg3 dev->hard_end_xmit():

----
void tg3_complete_xmit(struct net_device *dev)
{
        /* Packets are ready, update Tx producer idx local and on card. */
        tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), entry);

        if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
                netif_stop_queue(dev);
                dev->xmit_win = 1;
                if (tg3_tx_avail(tp) >= TG3_TX_WAKEUP_THRESH(tp)) {
                        tg3_set_win(tp);
                        netif_wake_queue(dev);
                }
        } else {
                tg3_set_win(tp);
        }

        mmiowb();
        dev->trans_start = jiffies;
}
-------

3.3.3 setting the dev->xmit_win 
---------------------------------

As mentioned earlier this variable provides hints on how much
data to send from the core to the driver. Here are the obvious ways:

a) on doing a netif_stop, set it to 1. By default all drivers have 
this value set to 1 to emulate old behavior where a driver only
receives one packet at a time.
b) on netif_wake_queue set it to the max available space. You have
to be careful if your hardware does scatter-gather since the core
will pass you scatter-gatherable skbs and so you want to at least
leave enough space for the maximum allowed. Look at the tg3 and
e1000 to see how this is implemented.

The variable is important because it avoids the core sending
any more than what the driver can handle, therefore avoiding 
any need to muck with packet scheduling mechanisms.

Appendix 1: History
-------------------
June 11/2007: Initial revision
June 11/2007: Fixed typo on e1000 netif_wake description ..
Aug  08/2007: Added info on VLAN and the skb->cb[] danger ..
Sep  24/2007: Revised and cleaned up
Sep  25/2007: Cleanups from Randy Dunlap
Oct  08/2007: Removed references to LLTX and packet formatting

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* [patch 02/12] sky2: reduce impact of watchdog timer
From: Greg KH @ 2007-10-08 18:06 UTC (permalink / raw)
  To: linux-kernel, stable, Krzysztof Oledzki, Greg KH
  Cc: Justin Forbes, Zwane Mwaikambo, Theodore Ts'o, Randy Dunlap,
	Dave Jones, Chuck Wolber, Chris Wedgwood, Michael Krufky,
	Chuck Ebbert, Domenico Andreoli, torvalds, akpm, alan, netdev,
	Stephen Hemminger
In-Reply-To: <20071008180551.GA7627@kroah.com>

[-- Attachment #1: sky2-hw-watchdog.patch --]
[-- Type: text/plain, Size: 4220 bytes --]

From: Stephen Hemminger <shemminger@linux-foundation.org>

This is the 2.6.22 version of a regression fix that is already
in 2.6.23.  Change the watchdog timer form 10 per second all the time,
to 1 per second and only if interface is up.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>


---
 drivers/net/sky2.c |   45 ++++++++++++++++++---------------------------
 drivers/net/sky2.h |    2 +-
 2 files changed, 19 insertions(+), 28 deletions(-)

--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -96,10 +96,6 @@ static int disable_msi = 0;
 module_param(disable_msi, int, 0);
 MODULE_PARM_DESC(disable_msi, "Disable Message Signaled Interrupt (MSI)");
 
-static int idle_timeout = 100;
-module_param(idle_timeout, int, 0);
-MODULE_PARM_DESC(idle_timeout, "Watchdog timer for lost interrupts (ms)");
-
 static const struct pci_device_id sky2_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9000) }, /* SK-9Sxx */
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9E00) }, /* SK-9Exx */
@@ -1693,6 +1689,8 @@ static void sky2_link_up(struct sky2_por
 
 	netif_carrier_on(sky2->netdev);
 
+	mod_timer(&hw->watchdog_timer, jiffies + 1);
+
 	/* Turn on link LED */
 	sky2_write8(hw, SK_REG(port, LNK_LED_REG),
 		    LINKLED_ON | LINKLED_BLINK_OFF | LINKLED_LINKSYNC_OFF);
@@ -2384,25 +2382,25 @@ static void sky2_le_error(struct sky2_hw
 	sky2_write32(hw, Q_ADDR(q, Q_CSR), BMU_CLR_IRQ_CHK);
 }
 
-/* If idle then force a fake soft NAPI poll once a second
- * to work around cases where sharing an edge triggered interrupt.
- */
-static inline void sky2_idle_start(struct sky2_hw *hw)
-{
-	if (idle_timeout > 0)
-		mod_timer(&hw->idle_timer,
-			  jiffies + msecs_to_jiffies(idle_timeout));
-}
-
-static void sky2_idle(unsigned long arg)
+/* Force a fake soft NAPI poll to handle lost IRQ's */
+static void sky2_watchdog(unsigned long arg)
 {
 	struct sky2_hw *hw = (struct sky2_hw *) arg;
 	struct net_device *dev = hw->dev[0];
+	int i, active = 0;
 
 	if (__netif_rx_schedule_prep(dev))
 		__netif_rx_schedule(dev);
 
-	mod_timer(&hw->idle_timer, jiffies + msecs_to_jiffies(idle_timeout));
+	for (i = 0; i < hw->ports; i++) {
+		dev = hw->dev[i];
+		if (!netif_running(dev))
+			continue;
+		++active;
+	}
+
+	if (active)
+		mod_timer(&hw->watchdog_timer, round_jiffies(jiffies + HZ));
 }
 
 /* Hardware/software error handling */
@@ -2692,8 +2690,6 @@ static void sky2_restart(struct work_str
 
 	dev_dbg(&hw->pdev->dev, "restarting\n");
 
-	del_timer_sync(&hw->idle_timer);
-
 	rtnl_lock();
 	sky2_write32(hw, B0_IMSK, 0);
 	sky2_read32(hw, B0_IMSK);
@@ -2722,8 +2718,6 @@ static void sky2_restart(struct work_str
 		}
 	}
 
-	sky2_idle_start(hw);
-
 	rtnl_unlock();
 }
 
@@ -3713,11 +3707,9 @@ static int __devinit sky2_probe(struct p
 			sky2_show_addr(dev1);
 	}
 
-	setup_timer(&hw->idle_timer, sky2_idle, (unsigned long) hw);
+	setup_timer(&hw->watchdog_timer, sky2_watchdog, (unsigned long) hw);
 	INIT_WORK(&hw->restart_work, sky2_restart);
 
-	sky2_idle_start(hw);
-
 	pci_set_drvdata(pdev, hw);
 
 	return 0;
@@ -3752,7 +3744,7 @@ static void __devexit sky2_remove(struct
 	if (!hw)
 		return;
 
-	del_timer_sync(&hw->idle_timer);
+	del_timer_sync(&hw->watchdog_timer);
 
 	flush_scheduled_work();
 
@@ -3796,7 +3788,7 @@ static int sky2_suspend(struct pci_dev *
 	if (!hw)
 		return 0;
 
-	del_timer_sync(&hw->idle_timer);
+	del_timer_sync(&hw->watchdog_timer);
 	netif_poll_disable(hw->dev[0]);
 
 	for (i = 0; i < hw->ports; i++) {
@@ -3862,7 +3854,7 @@ static int sky2_resume(struct pci_dev *p
 	}
 
 	netif_poll_enable(hw->dev[0]);
-	sky2_idle_start(hw);
+
 	return 0;
 out:
 	dev_err(&pdev->dev, "resume failed (%d)\n", err);
@@ -3879,7 +3871,6 @@ static void sky2_shutdown(struct pci_dev
 	if (!hw)
 		return;
 
-	del_timer_sync(&hw->idle_timer);
 	netif_poll_disable(hw->dev[0]);
 
 	for (i = 0; i < hw->ports; i++) {
--- a/drivers/net/sky2.h
+++ b/drivers/net/sky2.h
@@ -1921,7 +1921,7 @@ struct sky2_hw {
 	u32		     st_idx;
 	dma_addr_t   	     st_dma;
 
-	struct timer_list    idle_timer;
+	struct timer_list    watchdog_timer;
 	struct work_struct   restart_work;
 	int		     msi;
 	wait_queue_head_t    msi_wait;

-- 

^ permalink raw reply

* Re: [ofa-general] [PATCH v3] iw_cxgb3: Support"iwarp-only"interfacesto avoid 4-tuple conflicts.
From: Steve Wise @ 2007-10-08 18:03 UTC (permalink / raw)
  To: Kanevsky, Arkady; +Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <C98692FD98048C41885E0B0FACD9DFB80518755A@exnane01.hq.netapp.com>



Kanevsky, Arkady wrote:
> Sean,
> IB aside,
> it looks like an ULP which is capable of being both RDMA aware and RDMA
> not-aware,
> like iSER and iSCSI, NFS-RDMA and NFS, SDP and sockets, 
> will be treated as two separete ULPs.
> Each has its own IP address, since there is a different IP address for
> iWARP
> port and "regular" Ethernet port. So it falls on the users of ULPs to
> "handle" it
> via DNS or some other services.
> Is this "acceptable" to users? I doubt it.
> 
> Recall that ULPs are going in opposite directions by having a different
> port number for RDMA aware and RDMA unaware versions of the ULP.
> This way, ULP "connection manager" handles RDMA-ness under the covers,
> while users plug an IP address for a server to connect to.
> Thanks,

NOTE: iSCSI/iSER over iWARP won't work with the current Linux RDMA/Verbs 
anyway due to the requirement that the login connection be migrated into 
RDMA mode.  That's a separate issue.  Currently there is not even a way 
to setup an RDMA connection in streaming mode, then allow streaming mode 
I/O, then transitioning the connection in to RDMA mode.  None of that is 
implemented.  Also, iSCSI/ISER does _not_ use different ports for 
streaming mode vs data-mover/rdma modes.  It is negotiated and assumes 
the same 4tuple.

But, if we assume that reasonable services should use different ports 
for tcp vs rdma connections for the same service, then maybe all thats 
needed is a way to choose ephemeral ports without colliding with the TCP 
stack.  Like maybe segmenting the ephemeral port space for TCP and RDMA 
ranges?  This could be done without impacting the core networking code I 
think.   This would still require a mvapich2 change to have the stack 
choose a port instead of randomly trying ports until one is available.

This angle doesn't solve everything either, but it avoids 2 separate 
subnets...


Steve.


> 
> Arkady Kanevsky                       email: arkady@netapp.com
> Network Appliance Inc.               phone: 781-768-5395
> 1601 Trapelo Rd. - Suite 16.        Fax: 781-895-1195
> Waltham, MA 02451                   central phone: 781-768-5300
>  
> 
>> -----Original Message-----
>> From: Sean Hefty [mailto:sean.hefty@intel.com] 
>> Sent: Thursday, September 27, 2007 3:12 PM
>> To: Kanevsky, Arkady; Sean Hefty; Steve Wise
>> Cc: netdev@vger.kernel.org; rdreier@cisco.com; 
>> linux-kernel@vger.kernel.org; general@lists.openfabrics.org
>> Subject: RE: [ofa-general] [PATCH v3] iw_cxgb3: 
>> Support"iwarp-only"interfacesto avoid 4-tuple conflicts.
>>
>>> What is the model on how client connects, say for iSCSI, when client 
>>> and server both support, iWARP and 10GbE or 1GbE, and would like to 
>>> setup "most" performant "connection" for ULP?
>> For the "most" performance connection, the ULP would use IB, 
>> and all these problems go away.  :)
>>
>> This proposal is for each iwarp interface to have its own IP 
>> address.  Clients would need an iwarp usable address of the 
>> server and would connect using rdma_connect().  If that call 
>> (or rdma_resolve_addr/route) fails, the client could try 
>> connecting using sockets, aoi, or some other interface.  I 
>> don't see that Steve's proposal changes anything from the 
>> client's perspective.
>>
>> - Sean
>> _______________________________________________
>> general mailing list
>> general@lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>> To unsubscribe, please visit 
>> http://openib.org/mailman/listinfo/openib-general
>>

^ permalink raw reply

* Re: 2.6.23-rc8-mm2 BUG: register_netdevice() issue as (ab)used by ISDN
From: Stephen Hemminger @ 2007-10-08 17:39 UTC (permalink / raw)
  To: Karsten Keil; +Cc: isdn4linux, netdev
In-Reply-To: <20071008114844.GB11453@pingi.kke.suse.de>

On Mon, 8 Oct 2007 13:48:44 +0200
Karsten Keil <kkeil@suse.de> wrote:

> On Sun, Oct 07, 2007 at 02:06:53PM +0200, Andreas Mohr wrote:
> > [not necessarily a very recent regression, used 2.6.19 kernels before...]
> > 
> > Hi all,
> > 
> > wondered why my main internet server (headless!) didn't come up properly
> > on a new 2.6.23-rc8-mm2
> ...
> > as if it's the
> >        BUG_ON(!dev->nd_net);
> > check which caused the BUG message.
> 
> The regression comes from the net namespace patches.
> Up to now in ISDN the struct netdevice is contained in struct isdn_net_dev and so
> it's not allocated using alloc_netdev(), which would set the missing dev->nd_net
> pointer.
> 
> 

Then ISDN is BROKEN in existing code.  ALL network devices must be allocated
by alloc_netdev() not the device driver otherwise there is no way to prevent
oopsing through sysfs. RTFM Documentation/networking/netdevices.txt

Please fix for 2.6.23 and 2.6.22 stable.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* Re: [BNX2X RESUBMIT][PATCH 0/8] New driver for Broadcom 10Gb Ethernet, take two.
From: Stephen Hemminger @ 2007-10-08 17:29 UTC (permalink / raw)
  To: Eliezer Tamir
  Cc: davem@davemloft.net, jeff@garzik.org, netdev@vger.kernel.org,
	Michael Chan, eilong, vladz, gertner
In-Reply-To: <470A23E5.6030102@broadcom.com>

On Mon, 08 Oct 2007 14:34:45 +0200
"Eliezer Tamir" <eliezert@broadcom.com> 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 <shemminger@linux-foundation.org>

^ permalink raw reply

* Re: [PATCH 2/4][TG3]: ASIC decoding and basic CPMU support.
From: Michael Chan @ 2007-10-08 18:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: David Miller, netdev, andy, mcarlson
In-Reply-To: <20071008071813.GA29669@infradead.org>

On Mon, 2007-10-08 at 08:18 +0100, Christoph Hellwig wrote:

> Just curious:  is there a diagram somewhere that shows the relation of
> the various tg3 chips to each other and what families exist?
> 

Some high level descriptions of some of the chips can be found here:

http://www.broadcom.com/products/Enterprise-Networking/Gigabit-Ethernet-Controllers


^ permalink raw reply

* Re: [Bugme-new] [Bug 9132] New: fcntl GET_OWN reports 0 for sockets instead of PID
From: Andrew Morton @ 2007-10-08 17:11 UTC (permalink / raw)
  To: ff; +Cc: bugme-daemon, netdev
In-Reply-To: <bug-9132-10286@http.bugzilla.kernel.org/>


(please respond via emailed reply-to-all, not vie the bugzilla web interface)

On Mon,  8 Oct 2007 09:18:02 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9132
> 
>            Summary: fcntl GET_OWN reports 0 for sockets instead of PID
>            Product: Networking
>            Version: 2.5
>      KernelVersion: 2.6.22
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>         AssignedTo: acme@ghostprotocols.net
>         ReportedBy: ff@ozog.com
> 
> 
> Most recent kernel where this bug did not occur: 2.6.22
> Distribution: Kubuntu
> Hardware Environment: VMWare workstation 6
> Software Environment: 
> Problem Description: calling fcntl with F_GETOWN on a socket gives allways zero
> instead of the PID of the socket. The fcntl code is right but the data is zero
> in the struct file element. It sounds like pid is not set when the socket is
> attached to the fd in sock_attach_fd() of net/socket.c.
> 
> I would add something like this:
> 
> file->f_owner.pid=find_get_pid(task->pid);
> file->f_owner.pid_type=PIDTYPE_PID;
> 
> in sock_attach_fd() of net/socket.c
> 
> Steps to reproduce:
> 
> s=socket(...);
> pid=fcntl(s, F_GETOWN);
> 

You state that the problem is present in 2.6.22 and also did not occur in
2.6.22.  I assume it has always been like this.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox