linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [patch 4/10] s390: network driver.
       [not found] <OF66747F56.DA01CD7C-ON42256F4A.00376891-42256F4A.0037821E@LocalDomain>
@ 2004-11-12 10:28 ` Thomas Spatzier
  2004-11-14  1:29   ` Jeff Garzik
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Spatzier @ 2004-11-12 10:28 UTC (permalink / raw)
  To: linux-kernel; +Cc: jgarzik





> You should be using netif_carrier_{on,off} properly, and not drop the
> packets.  When (if) link comes back, you requeue the packets to hardware
> (or hypervisor or whatever).  Your dev->stop() should stop operation and
> clean up anything left in your send/receive {rings | buffers}.
>

When we do not drop packets, but call netif_stop_queue the write queues
of all sockets associated to the net device are blocked as soon as they
get full. This causes problems with programs such as the zebra routing
daemon. So we have to keep the netif queue running in order to not block
any programs.
We also had a look at some other drivers and the common behaviour seems to
be that packets are lost if the network cable is pulled out.

Regards,
Thomas.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-12 10:28 ` [patch 4/10] s390: network driver Thomas Spatzier
@ 2004-11-14  1:29   ` Jeff Garzik
  2004-11-15  7:52     ` Paul Jakma
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2004-11-14  1:29 UTC (permalink / raw)
  To: Thomas Spatzier; +Cc: linux-kernel, Netdev

Thomas Spatzier wrote:
> 
> 
> 
>>You should be using netif_carrier_{on,off} properly, and not drop the
>>packets.  When (if) link comes back, you requeue the packets to hardware
>>(or hypervisor or whatever).  Your dev->stop() should stop operation and
>>clean up anything left in your send/receive {rings | buffers}.
>>
> 
> 
> When we do not drop packets, but call netif_stop_queue the write queues
> of all sockets associated to the net device are blocked as soon as they
> get full. This causes problems with programs such as the zebra routing
> daemon. So we have to keep the netif queue running in order to not block
> any programs.

This is very, very wrong.  You are essentially creating in in-driver 
/dev/null simulator.  This does nothing but chew CPU cycles by having 
the driver drop packets, rather than allowing the system to work as it 
should.

Queues are DESIGNED to fill up under various conditions.

Would not the zebra routing software have the same problems with cable 
pull under an e1000 or tg3 gigabit NIC?


> We also had a look at some other drivers and the common behaviour seems to
> be that packets are lost if the network cable is pulled out.

Which drivers, specifically, are these?

The most popular drivers -- e1000, tg3, etc. -- do not do this, for very 
good reasons.

	Jeff



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-14  1:29   ` Jeff Garzik
@ 2004-11-15  7:52     ` Paul Jakma
  2004-11-21  8:16       ` Paul Jakma
  2004-11-29 15:57       ` Thomas Spatzier
  0 siblings, 2 replies; 15+ messages in thread
From: Paul Jakma @ 2004-11-15  7:52 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Thomas Spatzier, linux-kernel, Netdev

On Sat, 13 Nov 2004, Jeff Garzik wrote:

> Queues are DESIGNED to fill up under various conditions.

What happens when they are full? Blocking isnt good, at least not 
from GNU Zebra / Quagga's POV.

> Would not the zebra routing software have the same problems with 
> cable pull under an e1000 or tg3 gigabit NIC?

If they block further writes, yes.

> The most popular drivers -- e1000, tg3, etc. -- do not do this, for 
> very good reasons.

The problem is that GNU Zebra / Quagga uses a single raw socket for 
certain protocols, eg OSPF. Blocking the socket because one NIC has a 
cable pulled is undesireable, and there's no point queueing the 
packets, by the time the link comes back, if ever, its highly 
unlikely there is any use in sending the packets (sending OSPF 
hello's from X minutes ago on a link that just came back is useless).

The kernel really shouldnt get too much in the way of an application 
that already is fully aware of reliability issues[1] - at least that 
is the application's expectation in this case.

We could change it to use a socket/interface on Linux, but it seems a 
bit unnecessary to me, at least for raw socket/included-header 
sockets.

> 	Jeff

1. an application must be if its uses raw sockets surely? Even for 
non-raw/header-included sockets, eg BGP tcp sockets, a user like GNU 
Zebra / Quagga would much prefer packets to be dropped.

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
My interest is in the future because I am going to spend the rest of my
life there.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-15  7:52     ` Paul Jakma
@ 2004-11-21  8:16       ` Paul Jakma
  2004-11-29 15:57       ` Thomas Spatzier
  1 sibling, 0 replies; 15+ messages in thread
From: Paul Jakma @ 2004-11-21  8:16 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Thomas Spatzier, linux-kernel, Netdev

On Mon, 15 Nov 2004, Paul Jakma wrote:

> non-raw/header-included sockets, eg BGP tcp sockets, a user like 
> GNU Zebra / Quagga would much prefer packets to be dropped.

Ur... not for TCP.. obviously.

Anyway, is there any advice on how applications that use a single 
socket for raw/udp should deal with this new behaviour? All of the 
link-orientated routing protocol daemons in Quagga/GNU Zebra are 
going to break on Linux with this new behaviour.

Should such applications be changed to open a seperate socket per 
interface? Or could we have a SO_DROP_DONT_QUEUE sockopt to allow a 
privileged application to retain the previous behaviour, or some way 
to flush the queue for a socket?

Using a socket per interface wont address problem of sending quite 
stale packets when a link comes back after a long time down, AUI. 
(not a huge problem - but not nice).

Jeff???

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
Be incomprehensible.  If they can't understand, they can't disagree.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-15  7:52     ` Paul Jakma
  2004-11-21  8:16       ` Paul Jakma
@ 2004-11-29 15:57       ` Thomas Spatzier
  2004-11-29 16:30         ` Paul Jakma
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Spatzier @ 2004-11-29 15:57 UTC (permalink / raw)
  To: paul; +Cc: jgarzik, linux-kernel, netdev





> Using a socket per interface wont address problem of sending
> quite stale packets when a link > comes back after a long time
> down, AUI. (not a huge problem - but not nice).
>
> Jeff???

Has there been any outcome on the discussion about whether or not
a device driver should drop packets when the cable is disconnected?
It seems that from the zebra point of view, as Paul wrote,
it would be better to not block sockets by queueing up packets
when there is no cable connection.

I do also think that it does not make sense to keep packets in the
queue and then send those packets when the cable is plugged in
again after a possibly long time.
There are protocols like TCP that handle packet loss anyway.

Regards,
Thomas.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-29 15:57       ` Thomas Spatzier
@ 2004-11-29 16:30         ` Paul Jakma
  2004-11-29 16:41           ` Thomas Spatzier
  0 siblings, 1 reply; 15+ messages in thread
From: Paul Jakma @ 2004-11-29 16:30 UTC (permalink / raw)
  To: Thomas Spatzier; +Cc: jgarzik, linux-kernel, netdev

On Mon, 29 Nov 2004, Thomas Spatzier wrote:

> Has there been any outcome on the discussion about whether or not a 
> device driver should drop packets when the cable is disconnected?

There hasnt.

> It seems that from the zebra point of view, as Paul wrote, it would 
> be better to not block sockets by queueing up packets when there is 
> no cable connection.

I'd prefer either to get ENOBUFS or to have kernel drop the packet - 
we're privileged apps writing to raw sockets and/or with IP_HDRINCL, 
the kernel should assume we know what we're doing (cause if we dont, 
there's far /worse/ we could do than send packets to an 
effective /dev/null).

> I do also think that it does not make sense to keep packets in the 
> queue and then send those packets when the cable is plugged in 
> again after a possibly long time.

Well, if the kernel is going to queue these packets without notifying 
us, we absolutely *must* have some way to flush those queues. Sending 
stale packets many minutes after the application generated them could 
have serious consequences for routing (eg, think sending RIP, IPv4 
IRDP or v6 RAs which are no longer valid - client receives them and 
installs routes which are long invalid and loses connectivity to some 
part of the network).

So even if we moved to socket/interface, we still need some way to 
tell kernel to flush a socket when we receive link-down over netlink 
later.

Not trying to queue on sockets where app has no expectation of 
reliability in first place would be even better ;)

> There are protocols like TCP that handle packet loss anyway.

Yes.

I'd be very interested to hear advice from the kernel gurus (eg "god, 
dont be so stupid, do xyz in your application instead"). We can 
accomodate whatever kernel wants as long as its workable.

> Regards,
> Thomas.

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
You will pay for your sins.  If you have already paid, please disregard
this message.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-29 16:30         ` Paul Jakma
@ 2004-11-29 16:41           ` Thomas Spatzier
  2004-11-29 20:27             ` Paul Jakma
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Spatzier @ 2004-11-29 16:41 UTC (permalink / raw)
  To: Paul Jakma; +Cc: jgarzik, linux-kernel, netdev





Paul Jakma <paul@clubi.ie> wrote on 29.11.2004 17:30:23:
> Well, if the kernel is going to queue these packets without notifying
> us, we absolutely *must* have some way to flush those queues. Sending
> stale packets many minutes after the application generated them could
> have serious consequences for routing (eg, think sending RIP, IPv4
> IRDP or v6 RAs which are no longer valid - client receives them and
> installs routes which are long invalid and loses connectivity to some
> part of the network).
>

Yes, for the examples you mentioned the app should better be notified.
However, AFAICS, there are no such notification mechanisms on a
per-packet basis implemented in the kernel.
And I doubt that they are going to be implemented.

> I'd be very interested to hear advice from the kernel gurus (eg "god,
> dont be so stupid, do xyz in your application instead"). We can
> accomodate whatever kernel wants as long as its workable.

Good suggestion, if anyone has an interesting and feasible solution
I will be happy to integrate it. So far, however, it don't see one and I
would point people being worried about lost packets to TCP.


Regards,
Thomas.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-29 16:41           ` Thomas Spatzier
@ 2004-11-29 20:27             ` Paul Jakma
  2004-11-30  7:22               ` Thomas Spatzier
  0 siblings, 1 reply; 15+ messages in thread
From: Paul Jakma @ 2004-11-29 20:27 UTC (permalink / raw)
  To: Thomas Spatzier; +Cc: jgarzik, linux-kernel, netdev

On Mon, 29 Nov 2004, Thomas Spatzier wrote:

> Yes, for the examples you mentioned the app should better be 
> notified. However, AFAICS, there are no such notification 
> mechanisms on a per-packet basis implemented in the kernel. And I 
> doubt that they are going to be implemented.

We do get notified. We get a netlink interface update with 
unset IFF_RUNNING from the interface flags.

However, it can take time before zebra gets to read it, and further 
time before zebra sends its own interface update to protocol daemons, 
and further time before they might get to read and update their own 
interface state. By which time those daemons may have sent packets 
down those interfaces (which packets may become invalid before link 
becomes back, but which still will be sent and hence which 
potentially could disrupt routing).

Ideally, we should be notified synchronously (EBUFS?) or if thats not 
possible, packet should be dropped (see my below presumption though).

The other solution is some means for a daemon to be able flush queues 
for a specific interface. (but thats a bit ugly).

> Good suggestion, if anyone has an interesting and feasible solution 
> I will be happy to integrate it. So far, however, it don't see one 
> and I would point people being worried about lost packets to TCP.

Surely TCP already was able to take care of retransmits? I'm not 
familiar with Linux internals, but how did TCP cope with the previous 
driver behaviour? (I get the vague impression that we're talking 
about a driver specific buffer here, rather than the applications 
socket buffer - is that correct?).

Jeff, David, enlighten us please! :)

> Regards,
> Thomas.

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
I can't understand why people are frightened of new ideas.  I'm frightened
of the old ones.
 		-- John Cage

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-29 20:27             ` Paul Jakma
@ 2004-11-30  7:22               ` Thomas Spatzier
  2004-12-05  6:25                 ` Paul Jakma
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Spatzier @ 2004-11-30  7:22 UTC (permalink / raw)
  To: Paul Jakma; +Cc: jgarzik, linux-kernel, netdev





Paul Jakma <paul@clubi.ie> wrote on 29.11.2004 21:27:57:
> We do get notified. We get a netlink interface update with
> unset IFF_RUNNING from the interface flags.
>
> However, it can take time before zebra gets to read it, and further
> time before zebra sends its own interface update to protocol daemons,
> and further time before they might get to read and update their own
> interface state. By which time those daemons may have sent packets
> down those interfaces (which packets may become invalid before link
> becomes back, but which still will be sent and hence which
> potentially could disrupt routing).

Ok, then some logic could be implemented in userland to take
appropriate actions. It must be ensured that zebra handles the
netlink notification fast enough.

>
> Ideally, we should be notified synchronously (EBUFS?) or if thats not
> possible, packet should be dropped (see my below presumption though).

In the manpages for send/sendto/sendmsg it says that there is a -ENOBUFS
return value, if a sockets write queue is full. It also says:
"Normally, this does not occur in Linux. Packets are just silently dropped
when a device queue overflows."
So, if packets are 'silently dropped' anyway, the fact that we drop them in
our driver (and increment the error count in the net_device_stats
accordingly)
should not be a problem.

> Surely TCP already was able to take care of retransmits? I'm not
> familiar with Linux internals, but how did TCP cope with the previous
> driver behaviour?

I think that both behaviours are similar for TCP. TCP waits for ACKs for
each packet. If they do not arrive, a retransmit is done. Sooner or later
the connection will be reset, if no responses from the other side arrive.
So the result for both driver behaviours should be the same.

Regards,
Thomas


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-30  7:22               ` Thomas Spatzier
@ 2004-12-05  6:25                 ` Paul Jakma
  2004-12-06 11:01                   ` Post Network dev questions to netdev Please WAS(Re: " jamal
  2004-12-06 11:27                   ` jamal
  0 siblings, 2 replies; 15+ messages in thread
From: Paul Jakma @ 2004-12-05  6:25 UTC (permalink / raw)
  To: Thomas Spatzier; +Cc: jgarzik, linux-kernel, netdev

On Tue, 30 Nov 2004, Thomas Spatzier wrote:

> Ok, then some logic could be implemented in userland to take 
> appropriate actions. It must be ensured that zebra handles the 
> netlink notification fast enough.

AIUI, netlink is not synchronous, it most definitely makes no 
reliability guarantees (and at the moment, zebra isnt terribly 
efficient at reading netlink, large numbers of interfaces will cause 
overruns in zebra - fixing this is on the TODO list). So we can never 
get rid of the window where a daemon could send a packet out a 
link-down interface - we can make that window smaller but not 
eliminate it.

Hence we need either a way to flush packets associated with an 
(interface,socket) (or just the socket) or we need the kernel to not 
accept such packets (and drop packets it has accepted).

> In the manpages for send/sendto/sendmsg it says that there is a -ENOBUFS
> return value, if a sockets write queue is full.

Yes, ENOBUFS, sorry.

> It also says:

> "Normally, this does not occur in Linux. Packets are just silently dropped
> when a device queue overflows."

This has always been (AFAIK) the behaviour yes. We started getting 
reports of the new queuing behaviour with, iirc, a version of Intel's 
e100 driver for 2.4.2x, which was later changed back to the old 
behaviour. However now that the queue behaviour is apparently the 
mandated behaviour we really need to work out what to do about the 
sending-long-stale packets problem.

> So, if packets are 'silently dropped' anyway, the fact that we drop 
> them in our driver (and increment the error count in the 
> net_device_stats accordingly) should not be a problem.

It shouldnt no.

The likes of OSPF already specify their own reliability mechanisms.

> I think that both behaviours are similar for TCP. TCP waits for 
> ACKs for each packet. If they do not arrive, a retransmit is done. 
> Sooner or later the connection will be reset, if no responses from 
> the other side arrive. So the result for both driver behaviours 
> should be the same.

But if TCP worked even when drivers dropped packets, then that 
implies TCP has its own queue? That we're talking about a seperate 
driver packet queue rather than the socket buffer (which is, 
presumably, where TCP retains packets until ACKed - i have no idea).

Anyway, we do, I think, need some way to deal with the 
sending-stale-packet-on-link-back problem. Either a way to flush this 
driver queue or else a guarantee that writes to sockets whose 
protocol makes no reliability guarantee will either return ENOBUFS or 
drop the packet.

Otherwise we will start getting reports of "Quagga on Linux sent an 
ancient {RIP,IRDP,RA} packet when we fixed a switch problem, and it 
caused an outage for a section of our network due to bad routes", I 
think.

Some comment or advice would be useful. (Am I kill-filed by all of 
netdev? feels like it).

> Regards,
> Thomas

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
No directory.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Post Network dev questions to netdev Please WAS(Re: [patch 4/10] s390: network driver.
  2004-12-05  6:25                 ` Paul Jakma
@ 2004-12-06 11:01                   ` jamal
  2004-12-06 11:27                   ` jamal
  1 sibling, 0 replies; 15+ messages in thread
From: jamal @ 2004-12-06 11:01 UTC (permalink / raw)
  To: Paul Jakma; +Cc: Thomas Spatzier, jgarzik, linux-kernel, netdev

On Sun, 2004-12-05 at 01:25, Paul Jakma wrote:

> 
> This has always been (AFAIK) the behaviour yes. We started getting 
> reports of the new queuing behaviour with, iirc, a version of Intel's 
> e100 driver for 2.4.2x, which was later changed back to the old 
> behaviour. However now that the queue behaviour is apparently the 
> mandated behaviour we really need to work out what to do about the 
> sending-long-stale packets problem.
> 

I missed the beginings of this thread. Seems some patch was posted
on lkml which started this discussion. I am pretty sure what the lkml
FAQ says is to post on netdev. Ok, If you insist posting on lkml
(because that the way to glory, good fortune and fame), then please have
the courtesy to post to netdev.  

Now lets see if we can help. Followups only on netdev.

cheers,
jamal




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-12-05  6:25                 ` Paul Jakma
  2004-12-06 11:01                   ` Post Network dev questions to netdev Please WAS(Re: " jamal
@ 2004-12-06 11:27                   ` jamal
  2004-12-06 18:44                     ` Paul Jakma
  1 sibling, 1 reply; 15+ messages in thread
From: jamal @ 2004-12-06 11:27 UTC (permalink / raw)
  To: Paul Jakma; +Cc: Thomas Spatzier, jgarzik, linux-kernel, netdev

On Sun, 2004-12-05 at 01:25, Paul Jakma wrote:
[..]
> Anyway, we do, I think, need some way to deal with the 
> sending-stale-packet-on-link-back problem. Either a way to flush this 
> driver queue or else a guarantee that writes to sockets whose 
> protocol makes no reliability guarantee will either return ENOBUFS or 
> drop the packet.
> 
> Otherwise we will start getting reports of "Quagga on Linux sent an 
> ancient {RIP,IRDP,RA} packet when we fixed a switch problem, and it 
> caused an outage for a section of our network due to bad routes", I 
> think.
> 
> Some comment or advice would be useful. (Am I kill-filed by all of 
> netdev? feels like it).

Dont post networking related patches on other lists. I havent seen said
patch, but it seems someone is complaining about some behavior changing?

In regards to link down and packets being queued.
Agreed this is a little problematic for some apps/transports. TCP is
definetely not one of them. TCP in Linux actually is told if the drop is
local. This way it can make better judgement (and not unnecesarily
adjust windows for example).
SCTP AFAIK is the only transport that provides its apps opportunity to
obsolete messages already sent. I am not sure how well implemented or
whtether it is implemented at all. Someone working on SCTP could
comment.

In the case the netdevice is administratively downed both the qdisc and
DMA ring packets are flushed. Newer packets will never be queued and
you should quickly be able to find from your app that the device is
down.
In the case of netdevice being operationally down - I am hoping this is
what the discussion is, having jumped on it - both queues stay intact.
What you can do is certainly from user space admin down/up the device
when you receive a netlink carrier off notification.
I am struggling to see whether dropping the packet inside the tx path
once it is operationaly down is so blasphemous ... need to get caffeine
first.

cheers,
jamal




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-12-06 11:27                   ` jamal
@ 2004-12-06 18:44                     ` Paul Jakma
  0 siblings, 0 replies; 15+ messages in thread
From: Paul Jakma @ 2004-12-06 18:44 UTC (permalink / raw)
  To: jamal; +Cc: Thomas Spatzier, jgarzik, linux-kernel, netdev

On Mon, 6 Dec 2004, jamal wrote:

> Dont post networking related patches on other lists. I havent seen said
> patch, but it seems someone is complaining about some behavior changing?

I missed the beginning of the thread too, but saw Jeff's reply to 
Thomas on netdev. It appears the original patch was to make the s390 
network driver discard packets on link-down.

Jeff had replied to say this was bad, that queues are meant to fill 
and that this was what other drivers (e1000, tg3) did.

> In regards to link down and packets being queued. Agreed this is a 
> little problematic for some apps/transports.

Tis yes. Particularly for apps using raw and UDP+IP_HDRINCL sockets.

This problem came to light when we got reports of ospfd blocking 
because link was down, late in 2.4 with a certain version of the 
(iirc) e100 driver. ospfd uses one single socket for all interfaces, 
and relies on IP_HDRINCL to have the packet routed out right 
interface. However this approach doesnt play well if the socket can 
be blocked completely because of /one/ interface having its link 
down. The behaviour we expected (and got up until now) is to receive 
either ENOBUFS or else, if the kernel accepts the packet write, for 
it to drop it if it can not be sent.

We can work around that by moving to a socket/interface. However it 
still leaves the problem of packets being queued indefinitely while 
the link is down and being sent when link comes back. This is *not* 
good for RIP, IPv4 IRDP and IPv6 RA.

> In the case the netdevice is administratively downed both the qdisc 
> and DMA ring packets are flushed.

What about any packets remaining in the socket buffer? (if that makes 
sense - i dont know enough about internals sadly). Are those queued?

> Newer packets will never be queued

That no longer appears to be the case though. The socket blocks, and 
/some/ packets are queued (presumably those which still were in the 
socket buffer? i dont know exactly..).

> and you should quickly be able to find from your app that 
> the device is down.

We can yes, via rtnetlink - but impossible to guarantee we'll know 
the link is down before we try write a packet.

> In the case of netdevice being operationally down

?

As in 'ip link set dev ... down'?

> - I am hoping this is what the discussion is, having jumped on it -

No, its for link-down, AIUI.

> both queues stay intact. What you can do is certainly from user 
> space admin down/up the device when you receive a netlink carrier 
> off notification.

That seems possible, but quite a hack. Something to work at a socket 
level would possibly be nicer. (Socket being the primary handle our 
application has).

> I am struggling to see whether dropping the packet inside the tx 
> path once it is operationaly down is so blasphemous ... need to get 
> caffeine first.

As long as reliable transports have some other transport specific 
queue, shouldnt be a problem. For UDP and raw no reliability or 
guarantees are expected by applications (least shouldnt be), and 
queueing some packets on link-down interferes with application-layer 
expectations.

> cheers,
> jamal

regards,
-- 
Paul Jakma	paul@clubi.ie	paul@jakma.org	Key ID: 64A2FF6A
Fortune:
The UPS doesn't have a battery backup.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/10] s390: network driver.
  2004-11-11 17:16 Martin Schwidefsky
@ 2004-11-11 22:51 ` Jeff Garzik
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2004-11-11 22:51 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: akpm, linux-kernel

Martin Schwidefsky wrote:
> [patch 4/10] s390: network driver.
> 
> From: Thomas Spatzier <tspat@de.ibm.com>
> From: Peter Tiedemann <ptiedem@de.ibm.com>
> 
> network driver changes:
>  - qeth: return -EINVAL if an skb is too large.
>  - qeth: don't call netif_stop_queue after cable pull. Drop the
>    packets instead.


You should be using netif_carrier_{on,off} properly, and not drop the 
packets.  When (if) link comes back, you requeue the packets to hardware 
(or hypervisor or whatever).  Your dev->stop() should stop operation and 
clean up anything left in your send/receive {rings | buffers}.

	Jeff



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 4/10] s390: network driver.
@ 2004-11-11 17:16 Martin Schwidefsky
  2004-11-11 22:51 ` Jeff Garzik
  0 siblings, 1 reply; 15+ messages in thread
From: Martin Schwidefsky @ 2004-11-11 17:16 UTC (permalink / raw)
  To: akpm, linux-kernel

[patch 4/10] s390: network driver.

From: Thomas Spatzier <tspat@de.ibm.com>
From: Peter Tiedemann <ptiedem@de.ibm.com>

network driver changes:
 - qeth: return -EINVAL if an skb is too large.
 - qeth: don't call netif_stop_queue after cable pull. Drop the
   packets instead.
 - qeth: fix race between SET_IP and SET_MC kernel thread by removing
   SET_MC thread and let the SET_IP thread do multicast requests as well.
 - qeth: make it compile without CONFIG_VLAN.
 - ctc: avoid compiler warnings.
 - lcs: write package sequence number to skb->cb.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diffstat:
 drivers/s390/net/ctcmain.c   |   22 ++-
 drivers/s390/net/lcs.c       |    6 -
 drivers/s390/net/lcs.h       |    3 
 drivers/s390/net/qeth.h      |    8 -
 drivers/s390/net/qeth_main.c |  253 +++++++++++++++++++++----------------------
 5 files changed, 154 insertions(+), 138 deletions(-)

diff -urN linux-2.6/drivers/s390/net/ctcmain.c linux-2.6-patched/drivers/s390/net/ctcmain.c
--- linux-2.6/drivers/s390/net/ctcmain.c	2004-11-11 15:06:39.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/ctcmain.c	2004-11-11 15:06:56.000000000 +0100
@@ -1,5 +1,5 @@
 /*
- * $Id: ctcmain.c,v 1.63 2004/07/28 12:27:54 ptiedem Exp $
+ * $Id: ctcmain.c,v 1.65 2004/10/27 09:12:48 mschwide Exp $
  *
  * CTC / ESCON network driver
  *
@@ -36,7 +36,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  *
- * RELEASE-TAG: CTC/ESCON network driver $Revision: 1.63 $
+ * RELEASE-TAG: CTC/ESCON network driver $Revision: 1.65 $
  *
  */
 \f
@@ -320,7 +320,7 @@
 print_banner(void)
 {
 	static int printed = 0;
-	char vbuf[] = "$Revision: 1.63 $";
+	char vbuf[] = "$Revision: 1.65 $";
 	char *version = vbuf;
 
 	if (printed)
@@ -1224,7 +1224,9 @@
 	fsm_deltimer(&ch->timer);
 	fsm_addtimer(&ch->timer, CTC_TIMEOUT_5SEC, CH_EVENT_TIMER, ch);
 	fsm_newstate(fi, CH_STATE_SETUPWAIT);
-	if (event == CH_EVENT_TIMER)
+	saveflags = 0;	/* avoids compiler warning with
+			   spin_unlock_irqrestore */
+	if (event == CH_EVENT_TIMER)	// only for timer not yet locked
 		spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
 	rc = ccw_device_start(ch->cdev, &ch->ccw[6], (unsigned long) ch, 0xff, 0);
 	if (event == CH_EVENT_TIMER)
@@ -1335,7 +1337,9 @@
 	DBF_TEXT(trace, 3, __FUNCTION__);
 	fsm_deltimer(&ch->timer);
 	fsm_addtimer(&ch->timer, CTC_TIMEOUT_5SEC, CH_EVENT_TIMER, ch);
-	if (event == CH_EVENT_STOP)
+	saveflags = 0;	/* avoids comp warning with
+			   spin_unlock_irqrestore */
+	if (event == CH_EVENT_STOP)	// only for STOP not yet locked
 		spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
 	oldstate = fsm_getstate(fi);
 	fsm_newstate(fi, CH_STATE_TERM);
@@ -1508,7 +1512,9 @@
 	fsm_addtimer(&ch->timer, CTC_TIMEOUT_5SEC, CH_EVENT_TIMER, ch);
 	oldstate = fsm_getstate(fi);
 	fsm_newstate(fi, CH_STATE_STARTWAIT);
-	if (event == CH_EVENT_TIMER)
+	saveflags = 0;	/* avoids compiler warning with
+			   spin_unlock_irqrestore */
+	if (event == CH_EVENT_TIMER)	// only for timer not yet locked
 		spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
 	rc = ccw_device_halt(ch->cdev, (unsigned long) ch);
 	if (event == CH_EVENT_TIMER)
@@ -1674,7 +1680,9 @@
 				return;
 			}
 			fsm_addtimer(&ch->timer, 1000, CH_EVENT_TIMER, ch);
-			if (event == CH_EVENT_TIMER)
+			saveflags = 0;	/* avoids compiler warning with
+					   spin_unlock_irqrestore */
+			if (event == CH_EVENT_TIMER) // only for TIMER not yet locked
 				spin_lock_irqsave(get_ccwdev_lock(ch->cdev),
 						  saveflags);
 			rc = ccw_device_start(ch->cdev, &ch->ccw[3],
diff -urN linux-2.6/drivers/s390/net/lcs.c linux-2.6-patched/drivers/s390/net/lcs.c
--- linux-2.6/drivers/s390/net/lcs.c	2004-11-11 15:06:39.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/lcs.c	2004-11-11 15:06:56.000000000 +0100
@@ -11,7 +11,7 @@
  *			  Frank Pavlic (pavlic@de.ibm.com) and
  *		 	  Martin Schwidefsky <schwidefsky@de.ibm.com>
  *
- *    $Revision: 1.94 $	 $Date: 2004/10/19 09:30:54 $
+ *    $Revision: 1.96 $	 $Date: 2004/11/11 13:42:33 $
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -59,7 +59,7 @@
 /**
  * initialization string for output
  */
-#define VERSION_LCS_C  "$Revision: 1.94 $"
+#define VERSION_LCS_C  "$Revision: 1.96 $"
 
 static char version[] __initdata = "LCS driver ("VERSION_LCS_C "/" VERSION_LCS_H ")";
 static char debug_buffer[255];
@@ -191,6 +191,7 @@
 		return NULL;
 	memset(card, 0, sizeof(struct lcs_card));
 	card->lan_type = LCS_FRAME_TYPE_AUTO;
+	card->pkt_seq = 0;
 	card->lancmd_timeout = LCS_LANCMD_TIMEOUT_DEFAULT;
 	/* Allocate io buffers for the read channel. */
 	rc = lcs_alloc_channel(&card->read);
@@ -1874,6 +1875,7 @@
 	skb->protocol =	card->lan_type_trans(skb, card->dev);
 	card->stats.rx_bytes += skb_len;
 	card->stats.rx_packets++;
+	*((__u32 *)skb->cb) = ++card->pkt_seq;
 	netif_rx(skb);
 }
 
diff -urN linux-2.6/drivers/s390/net/lcs.h linux-2.6-patched/drivers/s390/net/lcs.h
--- linux-2.6/drivers/s390/net/lcs.h	2004-10-18 23:54:40.000000000 +0200
+++ linux-2.6-patched/drivers/s390/net/lcs.h	2004-11-11 15:06:56.000000000 +0100
@@ -6,7 +6,7 @@
 #include <linux/workqueue.h>
 #include <asm/ccwdev.h>
 
-#define VERSION_LCS_H "$Revision: 1.18 $"
+#define VERSION_LCS_H "$Revision: 1.19 $"
 
 #define LCS_DBF_TEXT(level, name, text) \
 	do { \
@@ -309,6 +309,7 @@
 	__u16 ip_assists_supported;
 	__u16 ip_assists_enabled;
 	__s8 lan_type;
+	__u32 pkt_seq;
 	__u16 sequence_no;
 	__s16 portno;
 	/* Some info copied from probeinfo */
diff -urN linux-2.6/drivers/s390/net/qeth.h linux-2.6-patched/drivers/s390/net/qeth.h
--- linux-2.6/drivers/s390/net/qeth.h	2004-11-11 15:06:39.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/qeth.h	2004-11-11 15:06:56.000000000 +0100
@@ -24,7 +24,7 @@
 
 #include "qeth_mpc.h"
 
-#define VERSION_QETH_H 		"$Revision: 1.116 $"
+#define VERSION_QETH_H 		"$Revision: 1.123 $"
 
 #ifdef CONFIG_QETH_IPV6
 #define QETH_VERSION_IPV6 	":IPv6"
@@ -557,6 +557,7 @@
 	QETH_IP_TYPE_NORMAL,
 	QETH_IP_TYPE_VIPA,
 	QETH_IP_TYPE_RXIP,
+	QETH_IP_TYPE_DEL_ALL_MC,
 };
 
 enum qeth_cmd_buffer_state {
@@ -713,8 +714,7 @@
  */
 enum qeth_threads {
 	QETH_SET_IP_THREAD  = 1,
-	QETH_SET_MC_THREAD  = 2,
-	QETH_RECOVER_THREAD = 4,
+	QETH_RECOVER_THREAD = 2,
 };
 
 struct qeth_card {
@@ -748,7 +748,7 @@
 	volatile unsigned long thread_running_mask;
 	spinlock_t ip_lock;
 	struct list_head ip_list;
-	struct list_head ip_tbd_list;
+	struct list_head *ip_tbd_list;
 	struct qeth_ipato ipato;
 	struct list_head cmd_waiter_list;
 	/* QDIO buffer handling */
diff -urN linux-2.6/drivers/s390/net/qeth_main.c linux-2.6-patched/drivers/s390/net/qeth_main.c
--- linux-2.6/drivers/s390/net/qeth_main.c	2004-11-11 15:06:39.000000000 +0100
+++ linux-2.6-patched/drivers/s390/net/qeth_main.c	2004-11-11 15:06:56.000000000 +0100
@@ -1,6 +1,6 @@
 /*
  *
- * linux/drivers/s390/net/qeth_main.c ($Revision: 1.155 $)
+ * linux/drivers/s390/net/qeth_main.c ($Revision: 1.168 $)
  *
  * Linux on zSeries OSA Express and HiperSockets support
  *
@@ -12,7 +12,7 @@
  *			  Frank Pavlic (pavlic@de.ibm.com) and
  *		 	  Thomas Spatzier <tspat@de.ibm.com>
  *
- *    $Revision: 1.155 $	 $Date: 2004/10/21 13:27:46 $
+ *    $Revision: 1.168 $	 $Date: 2004/11/08 15:55:12 $
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -78,7 +78,7 @@
 #include "qeth_mpc.h"
 #include "qeth_fs.h"
 
-#define VERSION_QETH_C "$Revision: 1.155 $"
+#define VERSION_QETH_C "$Revision: 1.168 $"
 static const char *version = "qeth S/390 OSA-Express driver";
 
 /**
@@ -160,6 +160,9 @@
 qeth_get_addr_buffer(enum qeth_prot_versions);
 
 static void
+qeth_set_multicast_list(struct net_device *);
+
+static void
 qeth_notify_processes(void)
 {
 	/*notify all  registered processes */
@@ -249,6 +252,7 @@
 		free_netdev(card->dev);
 	qeth_clear_ip_list(card, 0, 0);
 	qeth_clear_ipato_list(card);
+	kfree(card->ip_tbd_list);
 	qeth_free_qdio_buffers(card);
 	kfree(card);
 }
@@ -660,7 +664,10 @@
 	struct qeth_ipaddr *tmp, *t;
 	int found = 0;
 
-	list_for_each_entry_safe(tmp, t, &card->ip_tbd_list, entry) {
+	list_for_each_entry_safe(tmp, t, card->ip_tbd_list, entry) {
+		if ((addr->type == QETH_IP_TYPE_DEL_ALL_MC) &&
+		    (tmp->type == QETH_IP_TYPE_DEL_ALL_MC))
+			return 0;
 		if ((tmp->proto        == QETH_PROT_IPV4)     &&
 		    (addr->proto       == QETH_PROT_IPV4)     &&
 		    (tmp->type         == addr->type)         &&
@@ -692,14 +699,18 @@
 		}
 		return 0;
 	} else {
-		if (addr->users == 0)
-			addr->users += add? 1:-1;
-		if (add && (addr->type == QETH_IP_TYPE_NORMAL) &&
-		    qeth_is_addr_covered_by_ipato(card, addr)){
-			QETH_DBF_TEXT(trace, 2, "tkovaddr");
-			addr->set_flags |= QETH_IPA_SETIP_TAKEOVER_FLAG;
+		if (addr->type == QETH_IP_TYPE_DEL_ALL_MC)
+			list_add(&addr->entry, card->ip_tbd_list);
+		else {
+			if (addr->users == 0)
+				addr->users += add? 1:-1;
+			if (add && (addr->type == QETH_IP_TYPE_NORMAL) &&
+			    qeth_is_addr_covered_by_ipato(card, addr)){
+				QETH_DBF_TEXT(trace, 2, "tkovaddr");
+				addr->set_flags |= QETH_IPA_SETIP_TAKEOVER_FLAG;
+			}
+			list_add_tail(&addr->entry, card->ip_tbd_list);
 		}
-		list_add_tail(&addr->entry, &card->ip_tbd_list);
 		return 1;
 	}
 }
@@ -717,8 +728,8 @@
 	if (addr->proto == QETH_PROT_IPV4)
 		QETH_DBF_HEX(trace,4,&addr->u.a4.addr,4);
 	else {
-		QETH_DBF_HEX(trace,4,&addr->u.a6.addr,4);
-		QETH_DBF_HEX(trace,4,((char *)&addr->u.a6.addr)+4,4);
+		QETH_DBF_HEX(trace,4,&addr->u.a6.addr,8);
+		QETH_DBF_HEX(trace,4,((char *)&addr->u.a6.addr)+8,8);
 	}
 	spin_lock_irqsave(&card->ip_lock, flags);
 	rc = __qeth_insert_ip_todo(card, addr, 0);
@@ -736,8 +747,8 @@
 	if (addr->proto == QETH_PROT_IPV4)
 		QETH_DBF_HEX(trace,4,&addr->u.a4.addr,4);
 	else {
-		QETH_DBF_HEX(trace,4,&addr->u.a6.addr,4);
-		QETH_DBF_HEX(trace,4,((char *)&addr->u.a6.addr)+4,4);
+		QETH_DBF_HEX(trace,4,&addr->u.a6.addr,8);
+		QETH_DBF_HEX(trace,4,((char *)&addr->u.a6.addr)+8,8);
 	}
 	spin_lock_irqsave(&card->ip_lock, flags);
 	rc = __qeth_insert_ip_todo(card, addr, 1);
@@ -745,19 +756,21 @@
 	return rc;
 }
 
-static void
-qeth_reinsert_todos(struct qeth_card *card, struct list_head *todos)
+static inline void
+__qeth_delete_all_mc(struct qeth_card *card, unsigned long *flags)
 {
-	struct qeth_ipaddr *todo, *tmp;
+	struct qeth_ipaddr *addr, *tmp;
+	int rc;
 
-	list_for_each_entry_safe(todo, tmp, todos, entry){
-		list_del_init(&todo->entry);
-		if (todo->users < 0) {
-			if (!qeth_delete_ip(card, todo))
-				kfree(todo);
-		} else {
-			if (!qeth_add_ip(card, todo))
-				kfree(todo);
+	list_for_each_entry_safe(addr, tmp, &card->ip_list, entry) {
+		if (addr->is_multicast) {
+			spin_unlock_irqrestore(&card->ip_lock, *flags);
+			rc = qeth_deregister_addr_entry(card, addr);
+			spin_lock_irqsave(&card->ip_lock, *flags);
+			if (!rc) {
+				list_del(&addr->entry);
+				kfree(addr);
+			}
 		}
 	}
 }
@@ -765,7 +778,7 @@
 static void
 qeth_set_ip_addr_list(struct qeth_card *card)
 {
-	struct list_head failed_todos;
+	struct list_head *tbd_list;
 	struct qeth_ipaddr *todo, *addr;
 	unsigned long flags;
 	int rc;
@@ -773,13 +786,25 @@
 	QETH_DBF_TEXT(trace, 2, "sdiplist");
 	QETH_DBF_HEX(trace, 2, &card, sizeof(void *));
 
-	INIT_LIST_HEAD(&failed_todos);
-
 	spin_lock_irqsave(&card->ip_lock, flags);
-	while (!list_empty(&card->ip_tbd_list)) {
-		todo = list_entry(card->ip_tbd_list.next,
-				  struct qeth_ipaddr, entry);
-		list_del_init(&todo->entry);
+	tbd_list = card->ip_tbd_list;
+	card->ip_tbd_list = kmalloc(sizeof(struct list_head), GFP_ATOMIC);
+	if (!card->ip_tbd_list) {
+		QETH_DBF_TEXT(trace, 0, "silnomem");
+		card->ip_tbd_list = tbd_list;
+		spin_unlock_irqrestore(&card->ip_lock, flags);
+		return;
+	} else
+		INIT_LIST_HEAD(card->ip_tbd_list);
+
+	while (!list_empty(tbd_list)){
+		todo = list_entry(tbd_list->next, struct qeth_ipaddr, entry);
+		list_del(&todo->entry);
+		if (todo->type == QETH_IP_TYPE_DEL_ALL_MC){
+			__qeth_delete_all_mc(card, &flags);
+			kfree(todo);
+			continue;
+		}
 		rc = __qeth_ref_ip_on_card(card, todo, &addr);
 		if (rc == 0) {
 			/* nothing to be done; only adjusted refcount */
@@ -792,24 +817,22 @@
 			if (!rc)
 				list_add_tail(&todo->entry, &card->ip_list);
 			else
-				list_add_tail(&todo->entry, &failed_todos);
+				kfree(todo);
 		} else if (rc == -1) {
 			/* on-card entry to be removed */
 			list_del_init(&addr->entry);
 			spin_unlock_irqrestore(&card->ip_lock, flags);
 			rc = qeth_deregister_addr_entry(card, addr);
 			spin_lock_irqsave(&card->ip_lock, flags);
-			if (!rc) {
+			if (!rc)
 				kfree(addr);
-				kfree(todo);
-			} else {
+			else
 				list_add_tail(&addr->entry, &card->ip_list);
-				list_add_tail(&todo->entry, &failed_todos);
-			}
+			kfree(todo);
 		}
 	}
 	spin_unlock_irqrestore(&card->ip_lock, flags);
-	qeth_reinsert_todos(card, &failed_todos);
+	kfree(tbd_list);
 }
 
 static void qeth_delete_mc_addresses(struct qeth_card *);
@@ -887,28 +910,7 @@
 }
 
 static int
-qeth_register_mc_addresses(void *ptr)
-{
-	struct qeth_card *card;
-
-	card = (struct qeth_card *) ptr;
-	daemonize("qeth_reg_mcaddrs");
-	QETH_DBF_TEXT(trace,4,"regmcth1");
-	if (!qeth_do_run_thread(card, QETH_SET_MC_THREAD))
-		return 0;
-	QETH_DBF_TEXT(trace,4,"regmcth2");
-	qeth_delete_mc_addresses(card);
-	qeth_add_multicast_ipv4(card);
-#ifdef CONFIG_QETH_IPV6
-	qeth_add_multicast_ipv6(card);
-#endif
-	qeth_set_ip_addr_list(card);
-	qeth_clear_thread_running_bit(card, QETH_SET_MC_THREAD);
-	return 0;
-}
-
-static int
-qeth_register_ip_address(void *ptr)
+qeth_register_ip_addresses(void *ptr)
 {
 	struct qeth_card *card;
 
@@ -988,9 +990,7 @@
 		return;
 
 	if (qeth_do_start_thread(card, QETH_SET_IP_THREAD))
-		kernel_thread(qeth_register_ip_address, (void *) card, SIGCHLD);
-	if (qeth_do_start_thread(card, QETH_SET_MC_THREAD))
-		kernel_thread(qeth_register_mc_addresses, (void *)card,SIGCHLD);
+		kernel_thread(qeth_register_ip_addresses, (void *)card,SIGCHLD);
 	if (qeth_do_start_thread(card, QETH_RECOVER_THREAD))
 		kernel_thread(qeth_recover, (void *) card, SIGCHLD);
 }
@@ -1041,7 +1041,12 @@
 	INIT_WORK(&card->kernel_thread_starter,
 		  (void *)qeth_start_kernel_thread,card);
 	INIT_LIST_HEAD(&card->ip_list);
-	INIT_LIST_HEAD(&card->ip_tbd_list);
+	card->ip_tbd_list = kmalloc(sizeof(struct list_head), GFP_KERNEL);
+	if (!card->ip_tbd_list) {
+		QETH_DBF_TEXT(setup, 0, "iptbdnom");
+		return -ENOMEM;
+	}
+	INIT_LIST_HEAD(card->ip_tbd_list);
 	INIT_LIST_HEAD(&card->cmd_waiter_list);
 	init_waitqueue_head(&card->wait_q);
 	/* intial options */
@@ -1575,9 +1580,8 @@
 	QETH_DBF_TEXT(trace, 2, "rstipadd");
 
 	qeth_clear_ip_list(card, 0, 1);
-	if ( (qeth_set_thread_start_bit(card, QETH_SET_IP_THREAD) == 0) ||
-	     (qeth_set_thread_start_bit(card, QETH_SET_MC_THREAD) == 0) )
-		schedule_work(&card->kernel_thread_starter);
+	/* this function will also schedule the SET_IP_THREAD */
+	qeth_set_multicast_list(card->dev);
 }
 
 static struct qeth_ipa_cmd *
@@ -1600,10 +1604,7 @@
 					   card->info.if_name,
 					   card->info.chpid);
 				card->lan_online = 0;
-				if (netif_carrier_ok(card->dev)) {
-					netif_carrier_off(card->dev);
-					netif_stop_queue(card->dev);
-				}
+				netif_carrier_off(card->dev);
 				return NULL;
 			case IPA_CMD_STARTLAN:
 				PRINT_INFO("Link reestablished on %s "
@@ -1612,10 +1613,7 @@
 					   card->info.if_name,
 					   card->info.chpid);
 				card->lan_online = 1;
-				if (!netif_carrier_ok(card->dev)) {
-					netif_carrier_on(card->dev);
-					netif_wake_queue(card->dev);
-				}
+				netif_carrier_on(card->dev);
 				qeth_reset_ip_addresses(card);
 				return NULL;
 			case IPA_CMD_REGISTER_LOCAL_ADDR:
@@ -2807,7 +2805,8 @@
 	}
 	atomic_sub(count, &queue->used_buffers);
 	/* check if we need to do something on this outbound queue */
-	qeth_check_outbound_queue(queue);
+	if (card->info.type != QETH_CARD_TYPE_IQD)
+		qeth_check_outbound_queue(queue);
 
 	netif_wake_queue(card->dev);
 #ifdef CONFIG_QETH_PERF_STATS
@@ -3381,13 +3380,16 @@
 	if (skb==NULL) {
 		card->stats.tx_dropped++;
 		card->stats.tx_errors++;
-		return -EIO;
+		/* return OK; otherwise ksoftirqd goes to 100% */
+		return NETDEV_TX_OK;
 	}
-	if ((card->state != CARD_STATE_UP) || !netif_carrier_ok(dev)) {
+	if ((card->state != CARD_STATE_UP) || !card->lan_online) {
 		card->stats.tx_dropped++;
 		card->stats.tx_errors++;
 		card->stats.tx_carrier_errors++;
-		return -EIO;
+		dev_kfree_skb_any(skb);
+		/* return OK; otherwise ksoftirqd goes to 100% */
+		return NETDEV_TX_OK;
 	}
 #ifdef CONFIG_QETH_PERF_STATS
 	card->perf_stats.outbound_cnt++;
@@ -3398,8 +3400,18 @@
 	 * got our own synchronization on queues we can keep the stack's
 	 * queue running.
 	 */
-	if ((rc = qeth_send_packet(card, skb)))
-		netif_stop_queue(dev);
+	if ((rc = qeth_send_packet(card, skb))){
+		if (rc == -EBUSY) {
+			netif_stop_queue(dev);
+			rc = NETDEV_TX_BUSY;
+		} else {
+			card->stats.tx_errors++;
+			card->stats.tx_dropped++;
+			dev_kfree_skb_any(skb);
+			/* set to OK; otherwise ksoftirqd goes to 100% */
+			rc = NETDEV_TX_OK;
+		}
+	}
 
 #ifdef CONFIG_QETH_PERF_STATS
 	card->perf_stats.outbound_time += qeth_get_micros() -
@@ -3503,7 +3515,6 @@
 	if (!card->lan_online){
 		if (netif_carrier_ok(dev))
 			netif_carrier_off(dev);
-		netif_stop_queue(dev);
 	}
 	return 0;
 }
@@ -4988,12 +4999,28 @@
 	spin_unlock_irqrestore(&card->vlanlock, flags);
 	if (card->options.layer2)
 		qeth_layer2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN);
- 	if ( (qeth_set_thread_start_bit(card, QETH_SET_IP_THREAD) == 0) ||
-	     (qeth_set_thread_start_bit(card, QETH_SET_MC_THREAD) == 0) )
-		schedule_work(&card->kernel_thread_starter);
+	qeth_set_multicast_list(card->dev);
 }
 #endif
 
+/**
+ * set multicast address on card
+ */
+static void
+qeth_set_multicast_list(struct net_device *dev)
+{
+	struct qeth_card *card = (struct qeth_card *) dev->priv;
+
+	QETH_DBF_TEXT(trace,3,"setmulti");
+	qeth_delete_mc_addresses(card);
+	qeth_add_multicast_ipv4(card);
+#ifdef CONFIG_QETH_IPV6
+	qeth_add_multicast_ipv6(card);
+#endif
+ 	if (qeth_set_thread_start_bit(card, QETH_SET_IP_THREAD) == 0)
+		schedule_work(&card->kernel_thread_starter);
+}
+
 static int
 qeth_neigh_setup(struct net_device *dev, struct neigh_parms *np)
 {
@@ -5049,24 +5076,19 @@
 static void
 qeth_delete_mc_addresses(struct qeth_card *card)
 {
-	struct qeth_ipaddr *ipm, *iptodo;
+	struct qeth_ipaddr *iptodo;
 	unsigned long flags;
 
 	QETH_DBF_TEXT(trace,4,"delmc");
-	spin_lock_irqsave(&card->ip_lock, flags);
-	list_for_each_entry(ipm, &card->ip_list, entry){
-		if (!ipm->is_multicast)
-			continue;
-		iptodo = qeth_get_addr_buffer(ipm->proto);
-		if (!iptodo) {
-			QETH_DBF_TEXT(trace, 2, "dmcnomem");
-			continue;
-		}
-		memcpy(iptodo, ipm, sizeof(struct qeth_ipaddr));
-		iptodo->users = iptodo->users * -1;
-		if (!__qeth_insert_ip_todo(card, iptodo, 0))
-			kfree(iptodo);
+	iptodo = qeth_get_addr_buffer(QETH_PROT_IPV4);
+	if (!iptodo) {
+		QETH_DBF_TEXT(trace, 2, "dmcnomem");
+		return;
 	}
+	iptodo->type = QETH_IP_TYPE_DEL_ALL_MC;
+	spin_lock_irqsave(&card->ip_lock, flags);
+	if (!__qeth_insert_ip_todo(card, iptodo, 0))
+		kfree(iptodo);
 	spin_unlock_irqrestore(&card->ip_lock, flags);
 }
 
@@ -5277,20 +5299,6 @@
 
 	return rc;
 }
-/**
- * set multicast address on card
- */
-static void
-qeth_set_multicast_list(struct net_device *dev)
-{
-	struct qeth_card *card;
-
-	QETH_DBF_TEXT(trace,3,"setmulti");
-	card = (struct qeth_card *) dev->priv;
-
-	if (qeth_set_thread_start_bit(card, QETH_SET_MC_THREAD) == 0)
-		schedule_work(&card->kernel_thread_starter);
-}
 
 static void
 qeth_fill_ipacmd_header(struct qeth_card *card, struct qeth_ipa_cmd *cmd,
@@ -6635,7 +6643,7 @@
 	QETH_DBF_TEXT(trace,4,"clearip");
 	spin_lock_irqsave(&card->ip_lock, flags);
 	/* clear todo list */
-	list_for_each_entry_safe(addr, tmp, &card->ip_tbd_list, entry){
+	list_for_each_entry_safe(addr, tmp, card->ip_tbd_list, entry){
 		list_del(&addr->entry);
 		kfree(addr);
 	}
@@ -6653,7 +6661,7 @@
 			kfree(addr);
 			continue;
 		}
-		list_add_tail(&addr->entry, &card->ip_tbd_list);
+		list_add_tail(&addr->entry, card->ip_tbd_list);
 	}
 	spin_unlock_irqrestore(&card->ip_lock, flags);
 }
@@ -6893,8 +6901,7 @@
 	rtnl_lock();
 	dev_open(card->dev);
 	rtnl_unlock();
- 	if (qeth_set_thread_start_bit(card, QETH_SET_MC_THREAD) == 0)
-		schedule_work(&card->kernel_thread_starter);
+	qeth_set_multicast_list(card->dev);
 }
 
 
@@ -7008,9 +7015,7 @@
 /*maybe it was set offline without ifconfig down
  * we can also use this state for recovery purposes*/
 	if (card->options.layer2)
-		qeth_set_allowed_threads(card,
-					 QETH_RECOVER_THREAD |
-					 QETH_SET_MC_THREAD,0);
+		qeth_set_allowed_threads(card, QETH_RECOVER_THREAD, 0);
 	else
 		qeth_set_allowed_threads(card, 0xffffffff, 0);
 	if (recover_flag == CARD_STATE_RECOVER)
@@ -7339,7 +7344,7 @@
 		return -ENOMEM;
 	spin_lock_irqsave(&card->ip_lock, flags);
 	if (__qeth_address_exists_in_list(&card->ip_list, ipaddr, 0) ||
-	    __qeth_address_exists_in_list(&card->ip_tbd_list, ipaddr, 0))
+	    __qeth_address_exists_in_list(card->ip_tbd_list, ipaddr, 0))
 		rc = -EEXIST;
 	spin_unlock_irqrestore(&card->ip_lock, flags);
 	if (rc){
@@ -7412,7 +7417,7 @@
 		return -ENOMEM;
 	spin_lock_irqsave(&card->ip_lock, flags);
 	if (__qeth_address_exists_in_list(&card->ip_list, ipaddr, 0) ||
-	    __qeth_address_exists_in_list(&card->ip_tbd_list, ipaddr, 0))
+	    __qeth_address_exists_in_list(card->ip_tbd_list, ipaddr, 0))
 		rc = -EEXIST;
 	spin_unlock_irqrestore(&card->ip_lock, flags);
 	if (rc){

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2004-12-06 18:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <OF66747F56.DA01CD7C-ON42256F4A.00376891-42256F4A.0037821E@LocalDomain>
2004-11-12 10:28 ` [patch 4/10] s390: network driver Thomas Spatzier
2004-11-14  1:29   ` Jeff Garzik
2004-11-15  7:52     ` Paul Jakma
2004-11-21  8:16       ` Paul Jakma
2004-11-29 15:57       ` Thomas Spatzier
2004-11-29 16:30         ` Paul Jakma
2004-11-29 16:41           ` Thomas Spatzier
2004-11-29 20:27             ` Paul Jakma
2004-11-30  7:22               ` Thomas Spatzier
2004-12-05  6:25                 ` Paul Jakma
2004-12-06 11:01                   ` Post Network dev questions to netdev Please WAS(Re: " jamal
2004-12-06 11:27                   ` jamal
2004-12-06 18:44                     ` Paul Jakma
2004-11-11 17:16 Martin Schwidefsky
2004-11-11 22:51 ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).