UDP multicast packet loss not reported if TX ring overrun?

If I use a large send queue

(echo 2000000 >/proc/sys/net/core/wmem_default)

then lots of packet loss results if I try to send more packets than the bandwidth of the wire (300 bytes the maximum rate is 341k pps). With a size of 2M for the output buffer the TX ring is overrun.

But this loss is nowhere seen in any counter increments:

Is there any chance this is getting dropped at qdisc? Can you check with tc?

Quoted 2 lines · Nivedita Singhvi

Is there any chance this is getting dropped at qdisc? Can you check with tc?

I checked and the counter there is zero.

:/home/clameter# tc -s qdisc show

qdisc pfifo_fast 0: dev eth0 root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Sent 3042712708 bytes 8896862 pkt (dropped 0, overlimits 0 requeues 56303) rate 0bit 0pps backlog 0b 0p requeues 56303

What about ethtool -S ? Does it report any errors?

Quoted 1 line · Sridhar Samudrala

What about ethtool -S ? Does it report any errors?

Neither. This is is a broadcom bnx2 NIC.

Quoted 5 lines · Christoph Lameter

Quoted 1 line · Sridhar Samudrala

What about ethtool -S ? Does it report any errors?

Neither. This is is a broadcom bnx2 NIC.

Are you sure the packets are dropped at the sender?

Another place where packet drops/errors are counted is
/proc/net/softnet_stat
It tracks some counters that could result in drops. I thought these are all receive statistics. But looks like cpu_collision is a tx stat. The name of the structure is netif_rx_stat and it includes cpu_collison counter.

Quoted 8 lines · Sridhar Samudrala

Quoted 5 lines · Christoph Lameter

Quoted 1 line · Sridhar Samudrala

What about ethtool -S ? Does it report any errors?

Neither. This is is a broadcom bnx2 NIC.

Are you sure the packets are dropped at the sender?

Yes I am sending 400k messages from the app and the receiver only gets 341k @300 byte (which is the line rate). There is no way that the 400k get over the line. Also if I reduce SO_SNDBUF then both receiver and sender get down to 341k.

I added the output of ethtool -S at the end.

The mcast tool can be had from http://gentwo.org/ll or from my directory on www.kernel.org.

Quoted 6 lines

Another place where packet drops/errors are counted is
/proc/net/softnet_stat
It tracks some counters that could result in drops. I thought these are all receive statistics. But looks like cpu_collision is a tx stat. The name of the structure is netif_rx_stat and it includes cpu_collison counter.

How do I decode that information?

The first 3 are rx stats and the last one is a tx stat.
Anyway, only the first field(total packets received) seems to be non-zero in your softnet_stat output on both sender and receiver.
So it is possible that there is some other place in the stack where the packets are gettting dropped but not counted.

So it is possible that there is some other place in the stack where the packets are gettting dropped but not counted.

Quoted 2 lines · Sridhar Samudrala

So it is possible that there is some other place in the stack where the packets are gettting dropped but not counted.

Such a deed occurs in ip_push_pending_frames():

        /* Netfilter gets whole the not fragmented skb. */
        err = ip_local_out(skb);
        if (err) {
                if (err > 0)
                        err = inet->recverr ? net_xmit_errno(err) : 0;
			^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                if (err)
                        goto error;
        }

out:

        ip_cork_release(inet);
        return err;

error:

        IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
        goto out;

So if ip_local_out returns NET_XMIT_DROP then its simply going to be replaced by 0. Then we check err again and there is no error!!!!

The statistics are only generated if IP_RECVERR is set.

Could we move the increment of IPSTATS_MIB_OUTDISCARDS up so that it is incremented regardless of the setting of IP_RECVERR?

F.e?

Subject: Report TX drops

Incrementing of TX drop counters currently does not work if errors from the network stack are suppressed (IP_RECVERR off). Increment the statistics independently of the setting of IP_RECVERR.

Signed-off-by Christoph Lameter <[email protected]>

---
 net/ipv4/ip_output.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Index: linux-2.6/net/ipv4/ip_output.c
===================================================================

net/ipv4/ip_output.c +10 -5

@@ -1300,20 +1300,21 @@ int ip_push_pending_frames(struct sock *
 
 	/* Netfilter gets whole the not fragmented skb. */
 	err = ip_local_out(skb);
-	if (err) {
-		if (err > 0)
-			err = inet->recverr ? net_xmit_errno(err) : 0;
-		if (err)
-			goto error;
+	if (err > 0) {
+		/* The packet was dropped by the network subsystem */
+		IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
+
+		/*
+		 * Errors are not passed on if the socket
+		 * does not process errors (see IP_RECVERR).
+		 * net_xmit_errno filters NET_XMIT_CN.
+		 */
+		err = inet->recverr ? net_xmit_errno(err) : 0;
 	}
 

Quoted 21 lines · Christoph Lameter

Quoted 2 lines · Sridhar Samudrala

So it is possible that there is some other place in the stack where the packets are gettting dropped but not counted.

Such a deed occurs in ip_push_pending_frames():

        /* Netfilter gets whole the not fragmented skb. */
        err = ip_local_out(skb);
        if (err) {
                if (err > 0)
                        err = inet->recverr ? net_xmit_errno(err) : 0;
			^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                if (err)
                        goto error;
        }

out:

        ip_cork_release(inet);
        return err;

Quoted 7 lines

error:

        IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
        goto out;

So if ip_local_out returns NET_XMIT_DROP then its simply going to be replaced by 0. Then we check err again and there is no error!!!!

Quoted 1 line

NET_XMIT_CN strikes again :)

Well, if ip_local_out() returns a negative error (say -EPERM for example),
your patch disables OUTDISCARDS increments.

Maybe a simpler patch like this one ?

[PATCH] net: correctly updates OUTDISCARDS in ip_push_pending_frames()

ip_push_pending_frames() can fail to send a frame because of a congestioned device. In this case, we increment SNMP OUTDISCARDS only if user set IP_RECVERR, which is not RFC conformant.

Only case where we should not update OUTDISCARDS is when ip_local_output() return value is NET_XMIT_CN (meaning skb was xmitted but future frames might be dropped)

Signed-off-by Christoph Lameter <[email protected]>

Signed-off-by Eric Dumazet <[email protected]>

---

net/ipv4/ip_output.c +2 -6

index 7d08210..27a5b79 100644
@@ -1301,19 +1301,15 @@ int ip_push_pending_frames(struct sock *sk)
 	/* Netfilter gets whole the not fragmented skb. */
 	err = ip_local_out(skb);
 	if (err) {
+		if (err != NET_XMIT_CN)
+			IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
-		if (err)
-			goto error;
 	}
 
 out:
 	ip_cork_release(inet);
 	return err;
-
-error:
-	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
-	goto out;
 }
 
 /*

So are you hitting this case with your workload and does this account for all the packet losses you are seeing?

If we are dropping the packet and returing NET_XMIT_DROP, should we also increment qdisc drop stats (sch->qstats.drops)?

If we count these drops as qdisc drops, should we also count them as IP OUTDISCARDS?

Quoted 2 lines

If we count these drops as qdisc drops, should we also count them as

Yes.

IP_OUTDISCARDS should count the packets IP dropped, not anything dropped at a lower layer (which, in general, it is not aware of). If you count these in multiple layers, then you don't really know who dropped it.

Christoph Lameter <[email protected]> wrote on 08/25/2009 06:48:24 AM:

Quoted 3 lines · Sridhar Samudrala

Quoted 2 lines

If we count these drops as qdisc drops, should we also count them as

IP OUTDISCARDS?

Quoted 1 line

Quoted 1 line

Yes.

Actually, no. (!)

IP_OUTDISCARDS should count the packets IP dropped, not anything dropped at a lower layer (which, in general, it is not aware of). If you count these in multiple layers, then you don't really know who dropped it.

Quoted 4 lines · David Stevens

Christoph Lameter <[email protected]> wrote on 08/25/2009 06:48:24 AM:

Quoted 2 lines · Sridhar Samudrala

Quoted 1 line

If we count these drops as qdisc drops, should we also count them as

Quoted 7 lines

IP OUTDISCARDS?

Quoted 1 line

Yes.

Actually, no. (!)
IP_OUTDISCARDS should count the packets IP dropped, not anything dropped at a lower layer (which, in general, it is not aware of). If you count these in multiple layers, then you don't really know who dropped it.

They need to be accounted at the qdisc level though.

Quoted 12 lines · David Stevens

Christoph Lameter <[email protected]> wrote on 08/25/2009 06:48:24 AM:

Quoted 3 lines · Sridhar Samudrala

Quoted 2 lines

If we count these drops as qdisc drops, should we also count them as

IP OUTDISCARDS?

Quoted 2 lines

Yes.

Actually, no. (!)

Quoted 4 lines

IP_OUTDISCARDS should count the packets IP dropped, not anything dropped at a lower layer (which, in general, it is not aware of). If you count these in multiple layers, then you don't really know who dropped it.

You are right. I skipped that IP OUTDICARDS reference. They need to be accounted at the qdisc level though.

I also see no UDP stats for packet drops. If Erics fix gets in then ip_push_pending_frames() will give us an error code on drop. Incrementing SNDBUFERRORS would require this simple patch.

UDP: Account for TX drops

UDP layer is currently not incrementing error counters when packets are dropped. Use the SNDBUFERRORS to indicate packet drops on send.

Signed-off-by Christoph Lameter <[email protected]>

---
 net/ipv4/udp.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux-2.6/net/ipv4/udp.c
===================================================================

net/ipv4/udp.c +7 -0

@@ -559,6 +559,13 @@ static int udp_push_pending_frames(struc
 
 send:
 	err = ip_push_pending_frames(sk);
+
+	if (err)
+		/*
+		 * Packet was dropped.
+		 */
+		UDP_INC_STATS_USER(sock_net(sk),
+			UDP_MIB_SNDBUFERRORS, is_udplite);
 out:
 	up->len = 0;
 	up->pending = 0;

I think it's already done in udp_sendmsg()>
Code starting at line 765 in net/ipv4/udp.c

/*
* ENOBUFS = no kernel mem, SOCK_NOSPACE = no sndbuf space. Reporting
* ENOBUFS might not be good (it's not tunable per se), but otherwise
* we don't have a good statistic (IpOutDiscards but it can be too many
* things). We could add another new stat but at least for now that
* seems like overkill.
*/
if (err == -ENOBUFS || test_bit(SOCK_NOSPACE, &sk->sk_socket->flags)) {
UDP_INC_STATS_USER(sock_net(sk),
UDP_MIB_SNDBUFERRORS, is_udplite);
}

Could you check if the UDP packet losses you are seeing are accounted for in qdisc drops with this patch. But i am not completely positive on this as this case happens only if qdisc is deactivated.

Quoted 3 lines · Sridhar Samudrala

Could you check if the UDP packet losses you are seeing are accounted for in qdisc drops with this patch. But i am not completely positive on this as this case happens only if qdisc is deactivated.

This does not work. qdisc drops are still not reported.

They are reported for IP and UDP.

Test tool crashes on first TX overrun:

clameter@rd-strategy3-deb64:~$ ./mcast -n1 -r400000
Receiver: Listening to control channel 239.0.192.1
Receiver: Subscribing to 0 MC addresses 239.0.192-254.2-254 offset 0
origin 10.2.36.123
Sender: Sending 400000 msgs/ch/sec on 1 channels. Probe interval=0.001-1
sec.
sendto: No buffer space available
Socket Send error

netstat reports exactly one packet loss:

clameter@rd-strategy3-deb64:~$ netstat -su
IcmpMsg:
    InType3: 1
    OutType3: 1
Udp:
    298 packets received
    0 packets to unknown port received.
    0 packet receive errors
    7232136 packets sent
    SndbufErrors: 1

root@rd-strategy3-deb64:/home/clameter#tc -s qdisc show
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Sent 6208 bytes 64 pkt (dropped 0, overlimits 0 requeues 0) rate 0bit 0pps backlog 0b 0p requeues 0

Quoted 1 line

They are reported for IP and UDP.

Not clear what you meant by this.

Quoted 5 lines

root@rd-strategy3-deb64:/home/clameter#tc -s qdisc show
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Sent 6208 bytes 64 pkt (dropped 0, overlimits 0 requeues 0) rate 0bit 0pps backlog 0b 0p requeues 0

Even the Sent count seems to be too low. Are you looking at the right device?

So based on the current analysis, the packets are getting dropped after the call to ip_local_out() in ip_push_pending_frames(). ip_local_out() is failing with NET_XMIT_DROP. But we are not sure where they are getting dropped. Is that right?

I think we need to figure out where they are getting dropped and then decide on the appropriate counter to be incremented.

Quoted 2 lines · Sridhar Samudrala

Quoted 1 line

They are reported for IP and UDP.

Not clear what you meant by this.

The SNMP and UDP statistics show the loss. qdisc level does not show the loss.

Quoted 8 lines

Quoted 5 lines

root@rd-strategy3-deb64:/home/clameter#tc -s qdisc show
qdisc pfifo_fast 0: dev eth0 root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Sent 6208 bytes 64 pkt (dropped 0, overlimits 0 requeues 0) rate 0bit 0pps backlog 0b 0p requeues 0

Even the Sent count seems to be too low. Are you looking at the right device?

I would think that tc displays all queues? It says eth0 and eth0 is the device that we sent the data out on.

Quoted 4 lines

So based on the current analysis, the packets are getting dropped after the call to ip_local_out() in ip_push_pending_frames(). ip_local_out() is failing with NET_XMIT_DROP. But we are not sure where they are getting dropped. Is that right?

ip_local_out is returning ENOBUFS. Something at the qdisc layer is dropping the packet and not incrementing counters.

Quoted 3 lines

I think we need to figure out where they are getting dropped and then decide on the appropriate counter to be incremented.

Right. Where in the qdisc layer do drops occur?

Quoted 2 lines

ip_local_out is returning ENOBUFS. Something at the qdisc layer is dropping the packet and not incrementing counters.

Is the ENOBUFS return with your/Eric's patch? I thought you were were seeing NET_XMIT_DROP without any patches.

Quoted 4 lines

Quoted 2 lines

I think we need to figure out where they are getting dropped and then decide on the appropriate counter to be incremented.

Right. Where in the qdisc layer do drops occur?

The normal path where the packets are dropped when the tx qlen is exceeded is
pfifo_fast_enqueue() -> qdisc_drop()
In this path, drops are counted.
The other place is in dev_queue_xmit(), but you are not hitting that case too.

So it looks like there is another place where they are getting dropped.

The qdisc drop counter is incremented in pfifo_fast. So Sridhar's patch is not necessary.

Seems though that the qdisc drop count does not flow into the tx_dropped counter for the interface. Incrementing the tx_dropped count in the netdev_queue associated with the outbound qdisc also had no effect (see the following patch).

Plus I only see one queue for eth0 with "tc -s qdisc show". I think that what I see there is the queue for receiving packets. tc uses this ugly netlink interface. Could be a bug in there or in the netlink interface? Or is there some other trick to display queue statistics for outgoing packets?

WTH is going on here? Noone was ever interested in making outbound packet loss account right?

Index: linux-2.6.31-rc7/include/net/sch_generic.h
===================================================================
--- linux-2.6.31-rc7.orig/include/net/sch_generic.h 2009-08-27
21:20:03.000000000 +0000
+++ linux-2.6.31-rc7/include/net/sch_generic.h 2009-08-27
21:26:33.000000000 +0000

"tc -s qdisc show" only displays queue info for tx packets.

Review

Quoted 7 lines

(quoted diff) +3 -0

 	sch->qstats.drops++;
 
+	/* device queue statistics */
+	sch->dev_queue->tx_dropped++;
+
 	return NET_XMIT_DROP;
 }

locking problem here, tx_dropped can be changed by another cpu.

As David Stevens pointed out, device was not ever called at all when your packet(s) was/were lost. Why should we account a non existent drop at device level ?

When a process wants a new memory page and hits its own limit, do you want to increment a system global counter saying 'memory allocation failed' ?

Only change you want is eventually to account for the UDP drop (SndbufErrors).

Quoted 1 line · Eric Dumazet

Only change you want is eventually to account for the UDP drop (SndbufErrors).

That is only a counter at the UDP layer. That one does not allow you to identify which NIC it was nor which application caused it.

But its already a big improvement to see TX drops at all. Could we get your latest patch merged soon?

[PATCH] ip: Report qdisc packet drops

Christoph Lameter pointed out that packet drops at qdisc level where not accounted in SNMP counters. Only if application sets IP_RECVERR, drops are reported to user and SNMP counters updated.

IP_RECVERR is used to enable extended reliable error message passing. In case of tx drops at qdisc level, no error packet will be generated. It seems un-necessary to hide the qdisc drops for non IP_RECVERR enabled sockets (as probably most sockets are)

By removing the check of IP_RECVERR enabled sockets in ip_push_pending_frames()/ raw_send_hdrinc() / ip6_push_pending_frames() / rawv6_send_hdrinc(), we can properly update IPSTATS_MIB_OUTDISCARDS, and in case of UDP, update UDP_MIB_SNDBUFERRORS SNMP counters.

Application send() syscalls, instead of returning an OK status (thus lying), will return -ENOBUFS error.

Note : send() manual page explicitly says for -ENOBUFS error :

"The output queue for a network interface was full.
This generally indicates that the interface has stopped sending, but may be caused by transient congestion.
(Normally, this does not occur in Linux. Packets are just silently dropped when a device queue overflows.) "

This was not true for IP_RECVERR enabled sockets for < 2.6.32 linuxes, and starting from linux 2.6.32, last part wont be true at all.

Signed-off-by Eric Dumazet <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

From: Eric Dumazet <[email protected]>
Date: Fri, 28 Aug 2009 19:26:04 +0200

Quoted 27 lines

[PATCH] ip: Report qdisc packet drops

Christoph Lameter pointed out that packet drops at qdisc level where not accounted in SNMP counters. Only if application sets IP_RECVERR, drops are reported to user and SNMP counters updated.

IP_RECVERR is used to enable extended reliable error message passing. In case of tx drops at qdisc level, no error packet will be generated. It seems un-necessary to hide the qdisc drops for non IP_RECVERR enabled sockets (as probably most sockets are)

By removing the check of IP_RECVERR enabled sockets in ip_push_pending_frames()/ raw_send_hdrinc() / ip6_push_pending_frames() / rawv6_send_hdrinc(), we can properly update IPSTATS_MIB_OUTDISCARDS, and in case of UDP, update UDP_MIB_SNDBUFERRORS SNMP counters.

Application send() syscalls, instead of returning an OK status (thus lying), will return -ENOBUFS error.

Note : send() manual page explicitly says for -ENOBUFS error :

"The output queue for a network interface was full.
This generally indicates that the interface has stopped sending, but may be caused by transient congestion.
(Normally, this does not occur in Linux. Packets are just silently dropped when a device queue overflows.) "

Quoted 5 lines

This was not true for IP_RECVERR enabled sockets for < 2.6.32 linuxes, and starting from linux 2.6.32, last part wont be true at all.

Signed-off-by Eric Dumazet <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

The core question in all of this is what IP_RECVERR means.

As far as I remember Alexey Kuznetsov's intentions, it means that the application is interested in learning about errors caused by the infrastructure of the network between local and remote stacks.

Reporting a qdisc level drop to the application by default has the potential to break applications, because BSD and other stacks do not do this.

I can see why we might be able to get away with making this change now. And I also can see the benefits of it, for sure.

Let me think about this some more.

Re-reading again this stuff, I realized ip6_push_pending_frames() was not updating IPSTATS_MIB_OUTDISCARDS, even if IP_RECVERR was set.

May I suggest following path :

1) Correct ip6_push_pending_frames() to properly account for dropped-by-qdisc frames when IP_RECVERR is set

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Quoted 2 lines

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Sounds good.

If we are to make such applications "more sophisticated" such converted apps can be indicated simply their use of IP_RECVERR.

As usual, Alexey Kuznetsov's analysis of this situation is timeless, accurate, and wise. And he understood all of this 10+ years ago.

There may be a minor issue here in that IP_RECVERR sometimes sends error packets that have to be intercepted using special code. Or can those be simply ignored? If so then I will ask UDP app vendors to use IP_RECVERR.

From: Eric Dumazet <[email protected]>
Date: Mon, 31 Aug 2009 14:09:50 +0200

Quoted 7 lines

Re-reading again this stuff, I realized ip6_push_pending_frames() was not updating IPSTATS_MIB_OUTDISCARDS, even if IP_RECVERR was set.

May I suggest following path :

1) Correct ip6_push_pending_frames() to properly account for dropped-by-qdisc frames when IP_RECVERR is set

Your patch is applied to net-next-2.6, thanks!

Quoted 2 lines

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Sounds good.

I think if you sample random UDP applications, you will find that such errors will upset them terribly, make them log tons of crap to /var/log/messages et al., and consume tons of CPU.

And in such cases silent ignoring of drops is entirely appropriate and optimal, which supports our current behavior.

If we are to make such applications "more sophisticated" such converted apps can be indicated simply their use of IP_RECVERR.

If you want to be notified of all asynchronous errors we can detect, you use this, end of story. It is the only way to handle this situation without breaking the world.

As usual, Alexey Kuznetsov's analysis of this situation is timeless, accurate, and wise. And he understood all of this 10+ years ago.

The patch is smaller if you remove the handling of recverr completely from ip_push_pending_frames() and return NET_RX_DROP etc. Two of the callers never even inspect the return code. For them this is useless processing.

Quoted 28 lines · David Miller

From: Eric Dumazet <[email protected]>
Date: Mon, 31 Aug 2009 14:09:50 +0200

Quoted 7 lines

Re-reading again this stuff, I realized ip6_push_pending_frames() was not updating IPSTATS_MIB_OUTDISCARDS, even if IP_RECVERR was set.

May I suggest following path :

1) Correct ip6_push_pending_frames() to properly account for dropped-by-qdisc frames when IP_RECVERR is set

Your patch is applied to net-next-2.6, thanks!

Quoted 2 lines

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Sounds good.

I think if you sample random UDP applications, you will find that such errors will upset them terribly, make them log tons of crap to /var/log/messages et al., and consume tons of CPU.

And in such cases silent ignoring of drops is entirely appropriate and optimal, which supports our current behavior.

If we are to make such applications "more sophisticated" such converted apps can be indicated simply their use of IP_RECVERR.

Quoted 6 lines

If you want to be notified of all asynchronous errors we can detect, you use this, end of story. It is the only way to handle this situation without breaking the world.

As usual, Alexey Kuznetsov's analysis of this situation is timeless, accurate, and wise. And he understood all of this 10+ years ago.

Thanks David, here is the 2nd patch then :

[PATCH net-next-2.6] ip: Report qdisc packet drops

Christoph Lameter pointed out that packet drops at qdisc level where not accounted in SNMP counters. Only if application sets IP_RECVERR, drops are reported to user (-ENOBUFS errors) and SNMP counters updated.

IP_RECVERR is used to enable extended reliable error message passing, but these are not needed to update system wide SNMP stats.

This patch changes things a bit to allow SNMP counters to be updated, regardless of IP_RECVERR being set or not on the socket.

Example after an UDP tx flood

# netstat -s 
...
IP:
    1487048 outgoing packets dropped
...
Udp:
...
    SndbufErrors: 1487048

send() syscalls, do however still return an OK status, to not break applications.

Note : send() manual page explicitly says for -ENOBUFS error :

"The output queue for a network interface was full.
This generally indicates that the interface has stopped sending, but may be caused by transient congestion.
(Normally, this does not occur in Linux. Packets are just silently dropped when a device queue overflows.) "

This is not true for IP_RECVERR enabled sockets : a send() syscall that hit a qdisc drop returns an ENOBUFS error.

Many thanks to Christoph, David, and last but not least, Alexey !

Signed-off-by Eric Dumazet <[email protected]>

---
 include/net/ip.h      |    2 +-
 include/net/ipv6.h    |    2 +-
 include/net/udp.h     |    2 +-
 net/ipv4/icmp.c       |    2 +-
 net/ipv4/ip_output.c  |   19 ++++++++++---------
 net/ipv4/raw.c        |   14 ++++++++++----
 net/ipv4/udp.c        |   20 +++++++++++++-------
 net/ipv6/icmp.c       |    2 +-
 net/ipv6/ip6_output.c |   18 +++++++++++-------
 net/ipv6/raw.c        |   15 ++++++++++-----
 net/ipv6/udp.c        |   14 ++++++++++----
 11 files changed, 69 insertions(+), 41 deletions(-)

include/net/ip.h +1 -1

index 72c3692..9dd19a8 100644
@@ -116,7 +116,7 @@ extern int		ip_append_data(struct sock *sk,
 extern int		ip_generic_getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb);
 extern ssize_t		ip_append_page(struct sock *sk, struct page *page,
 				int offset, size_t size, int flags);
-extern int		ip_push_pending_frames(struct sock *sk);
+extern int		ip_push_pending_frames(struct sock *sk, int recverr);
 extern void		ip_flush_pending_frames(struct sock *sk);
 
 /* datagram.c */

include/net/ipv6.h +1 -1

index ad9a511..f514257 100644
@@ -498,7 +498,7 @@ extern int			ip6_append_data(struct sock *sk,
 						struct rt6_info *rt,
 						unsigned int flags);
 
-extern int			ip6_push_pending_frames(struct sock *sk);
+extern int			ip6_push_pending_frames(struct sock *sk, int recverr);
 
 extern void			ip6_flush_pending_frames(struct sock *sk);
 

include/net/udp.h +1 -1

index 5fb029f..a60ef10 100644
@@ -145,7 +145,7 @@ extern int 	udp_lib_getsockopt(struct sock *sk, int level, int optname,
 			           char __user *optval, int __user *optlen);
 extern int 	udp_lib_setsockopt(struct sock *sk, int level, int optname,
 				   char __user *optval, int optlen,
-				   int (*push_pending_frames)(struct sock *));
+				   int (*push_pending_frames)(struct sock *, int));
 
 extern struct sock *udp4_lib_lookup(struct net *net, __be32 saddr, __be16 sport,
 				    __be32 daddr, __be16 dport,

net/ipv4/icmp.c +1 -1

index 97c410e..f46a53c 100644
@@ -345,7 +345,7 @@ static void icmp_push_reply(struct icmp_bxm *icmp_param,
 						 icmp_param->head_len, csum);
 		icmph->checksum = csum_fold(csum);
 		skb->ip_summed = CHECKSUM_NONE;
-		ip_push_pending_frames(sk);
+		ip_push_pending_frames(sk, 0);
 	}
 }
 

net/ipv4/ip_output.c +10 -9

index 7d08210..8f81dab 100644
@@ -1216,7 +1216,7 @@ static void ip_cork_release(struct inet_sock *inet)
  *	Combined all pending IP fragments on the socket as one IP datagram
  *	and push them out.
  */
-int ip_push_pending_frames(struct sock *sk)
+int ip_push_pending_frames(struct sock *sk, int recverr)
 {
 	struct sk_buff *skb, *tmp_skb;
 	struct sk_buff **tail_skb;

@@ -1301,19 +1301,20 @@ int ip_push_pending_frames(struct sock *sk)
 	/* Netfilter gets whole the not fragmented skb. */
 	err = ip_local_out(skb);
 	if (err) {
-		if (err > 0)
-			err = inet->recverr ? net_xmit_errno(err) : 0;
+		if (err > 0) {
+			err = net_xmit_errno(err);
+			if (err && !recverr) {
+				IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
+				err = 0;
+			}
+		}
 		if (err)
-			goto error;
+			IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
 	}
 
 out:
 	ip_cork_release(inet);
 	return err;
-
-error:
-	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
-	goto out;
 }
 
 /*

@@ -1412,7 +1413,7 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
 			  arg->csumoffset) = csum_fold(csum_add(skb->csum,
 								arg->csum));
 		skb->ip_summed = CHECKSUM_NONE;
-		ip_push_pending_frames(sk);
+		ip_push_pending_frames(sk, 0);
 	}
 
 	bh_unlock_sock(sk);

net/ipv4/raw.c +10 -4

index 2979f14..444c465 100644
@@ -374,8 +374,13 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
 
 	err = NF_HOOK(PF_INET, NF_INET_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
 		      dst_output);
-	if (err > 0)
-		err = inet->recverr ? net_xmit_errno(err) : 0;
+	if (err > 0) {
+		err = net_xmit_errno(err);
+		if (!inet->recverr && err) {
+			IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
+			err = 0;
+		}
+	}
 	if (err)
 		goto error;
 out:

@@ -576,8 +581,9 @@ back_from_confirm:
 					&ipc, &rt, msg->msg_flags);
 		if (err)
 			ip_flush_pending_frames(sk);
-		else if (!(msg->msg_flags & MSG_MORE))
-			err = ip_push_pending_frames(sk);
+		else if (!(msg->msg_flags & MSG_MORE)) {
+			err = ip_push_pending_frames(sk, inet->recverr);
+		}
 		release_sock(sk);
 	}
 done:

net/ipv4/udp.c +13 -7

index 29ebb0d..6a6bf1d 100644
@@ -513,7 +513,7 @@ static void udp4_hwcsum_outgoing(struct sock *sk, struct sk_buff *skb,
 /*
  * Push out all pending data as one UDP datagram. Socket is locked.
  */
-static int udp_push_pending_frames(struct sock *sk)
+static int udp_push_pending_frames(struct sock *sk, int recverr)
 {
 	struct udp_sock  *up = udp_sk(sk);
 	struct inet_sock *inet = inet_sk(sk);

@@ -560,7 +560,7 @@ static int udp_push_pending_frames(struct sock *sk)
 		uh->check = CSUM_MANGLED_0;
 
 send:
-	err = ip_push_pending_frames(sk);
+	err = ip_push_pending_frames(sk, recverr);
 out:
 	up->len = 0;
 	up->pending = 0;

@@ -752,8 +752,14 @@ do_append_data:
 			corkreq ? msg->msg_flags|MSG_MORE : msg->msg_flags);
 	if (err)
 		udp_flush_pending_frames(sk);
-	else if (!corkreq)
-		err = udp_push_pending_frames(sk);
+	else if (!corkreq) {
+		err = udp_push_pending_frames(sk, 1);
+		if (err == -ENOBUFS && !inet->recverr) {
+			UDP_INC_STATS_USER(sock_net(sk),
+					   UDP_MIB_SNDBUFERRORS, is_udplite);
+			err = 0;
+		}
+	}
 	else if (unlikely(skb_queue_empty(&sk->sk_write_queue)))
 		up->pending = 0;
 	release_sock(sk);

@@ -826,7 +832,7 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
 
 	up->len += size;
 	if (!(up->corkflag || (flags&MSG_MORE)))
-		ret = udp_push_pending_frames(sk);
+		ret = udp_push_pending_frames(sk, inet_sk(sk)->recverr);
 	if (!ret)
 		ret = size;
 out:

@@ -1354,7 +1360,7 @@ void udp_destroy_sock(struct sock *sk)
  */
 int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		       char __user *optval, int optlen,
-		       int (*push_pending_frames)(struct sock *))
+		       int (*push_pending_frames)(struct sock *, int))
 {
 	struct udp_sock *up = udp_sk(sk);
 	int val;

@@ -1374,7 +1380,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		} else {
 			up->corkflag = 0;
 			lock_sock(sk);
-			(*push_pending_frames)(sk);
+			(*push_pending_frames)(sk, 0);
 			release_sock(sk);
 		}
 		break;

net/ipv6/icmp.c +1 -1

index e2325f6..a9c54c2 100644
@@ -253,7 +253,7 @@ static int icmpv6_push_pending_frames(struct sock *sk, struct flowi *fl, struct
 						      len, fl->proto,
 						      tmp_csum);
 	}
-	ip6_push_pending_frames(sk);
+	ip6_push_pending_frames(sk, 0);
 out:
 	return err;
 }

net/ipv6/ip6_output.c +11 -7

index a931229..ade5707 100644
@@ -1440,7 +1440,7 @@ static void ip6_cork_release(struct inet_sock *inet, struct ipv6_pinfo *np)
 	memset(&inet->cork.fl, 0, sizeof(inet->cork.fl));
 }
 
-int ip6_push_pending_frames(struct sock *sk)
+int ip6_push_pending_frames(struct sock *sk, int recverr)
 {
 	struct sk_buff *skb, *tmp_skb;
 	struct sk_buff **tail_skb;

@@ -1510,18 +1510,22 @@ int ip6_push_pending_frames(struct sock *sk)
 
 	err = ip6_local_out(skb);
 	if (err) {
-		if (err > 0)
-			err = np->recverr ? net_xmit_errno(err) : 0;
+		if (err > 0) {
+			err = net_xmit_errno(err);
+			if (err && !recverr) {
+				IP6_INC_STATS(net, rt->rt6i_idev,
+					      IPSTATS_MIB_OUTDISCARDS);
+				err = 0;
+			}
+		}
 		if (err)
-			goto error;
+			IP6_INC_STATS(net, rt->rt6i_idev,
+				      IPSTATS_MIB_OUTDISCARDS);
 	}
 
 out:
 	ip6_cork_release(inet, np);
 	return err;
-error:
-	IP6_INC_STATS(net, rt->rt6i_idev, IPSTATS_MIB_OUTDISCARDS);
-	goto out;
 }
 
 void ip6_flush_pending_frames(struct sock *sk)

net/ipv6/raw.c +10 -5

index 5068410..d054fa2 100644
@@ -523,7 +523,7 @@ csum_copy_err:
 }
 
 static int rawv6_push_pending_frames(struct sock *sk, struct flowi *fl,
-				     struct raw6_sock *rp)
+				     struct raw6_sock *rp, int recverr)
 {
 	struct sk_buff *skb;
 	int err = 0;

@@ -595,7 +595,7 @@ static int rawv6_push_pending_frames(struct sock *sk, struct flowi *fl,
 		BUG();
 
 send:
-	err = ip6_push_pending_frames(sk);
+	err = ip6_push_pending_frames(sk, recverr);
 out:
 	return err;
 }

@@ -641,8 +641,13 @@ static int rawv6_send_hdrinc(struct sock *sk, void *from, int length,
 	IP6_UPD_PO_STATS(sock_net(sk), rt->rt6i_idev, IPSTATS_MIB_OUT, skb->len);
 	err = NF_HOOK(PF_INET6, NF_INET_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
 		      dst_output);
-	if (err > 0)
-		err = np->recverr ? net_xmit_errno(err) : 0;
+	if (err > 0) {
+		err = net_xmit_errno(err);
+		if (!np->recverr && err) {
+			IP6_INC_STATS(sock_net(sk), rt->rt6i_idev, IPSTATS_MIB_OUTDISCARDS);
+			err = 0;
+		}
+	}
 	if (err)
 		goto error;
 out:

@@ -895,7 +900,7 @@ back_from_confirm:
 		if (err)
 			ip6_flush_pending_frames(sk);
 		else if (!(msg->msg_flags & MSG_MORE))
-			err = rawv6_push_pending_frames(sk, &fl, rp);
+			err = rawv6_push_pending_frames(sk, &fl, rp, np->recverr);
 		release_sock(sk);
 	}
 done:

net/ipv6/udp.c +10 -5

index 20d2ffc..963dd0a 100644
@@ -683,7 +683,7 @@ static void udp6_hwcsum_outgoing(struct sock *sk, struct sk_buff *skb,
  *	Sending
  */
 
-static int udp_v6_push_pending_frames(struct sock *sk)
+static int udp_v6_push_pending_frames(struct sock *sk, int recverr)
 {
 	struct sk_buff *skb;
 	struct udphdr *uh;

@@ -723,7 +723,7 @@ static int udp_v6_push_pending_frames(struct sock *sk)
 		uh->check = CSUM_MANGLED_0;
 
 send:
-	err = ip6_push_pending_frames(sk);
+	err = ip6_push_pending_frames(sk, recverr);
 out:
 	up->len = 0;
 	up->pending = 0;

@@ -975,8 +975,14 @@ do_append_data:
 		corkreq ? msg->msg_flags|MSG_MORE : msg->msg_flags);
 	if (err)
 		udp_v6_flush_pending_frames(sk);
-	else if (!corkreq)
-		err = udp_v6_push_pending_frames(sk);
+	else if (!corkreq) {
+		err = udp_v6_push_pending_frames(sk, 1);
+		if (err == -ENOBUFS && !np->recverr) {
+			UDP6_INC_STATS_USER(sock_net(sk),
+					   UDP_MIB_SNDBUFERRORS, is_udplite);
+			err = 0;
+		}
+	}
 	else if (unlikely(skb_queue_empty(&sk->sk_write_queue)))
 		up->pending = 0;
 
--

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

Quoted 49 lines · Eric Dumazet

Quoted 26 lines · David Miller

From: Eric Dumazet <[email protected]>
Date: Mon, 31 Aug 2009 14:09:50 +0200

Quoted 7 lines

Re-reading again this stuff, I realized ip6_push_pending_frames() was not updating IPSTATS_MIB_OUTDISCARDS, even if IP_RECVERR was set.

May I suggest following path :

1) Correct ip6_push_pending_frames() to properly account for dropped-by-qdisc frames when IP_RECVERR is set

Your patch is applied to net-next-2.6, thanks!

Quoted 2 lines

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Sounds good.

I think if you sample random UDP applications, you will find that such errors will upset them terribly, make them log tons of crap to /var/log/messages et al., and consume tons of CPU.

And in such cases silent ignoring of drops is entirely appropriate and optimal, which supports our current behavior.

If we are to make such applications "more sophisticated" such converted apps can be indicated simply their use of IP_RECVERR.

Quoted 6 lines

If you want to be notified of all asynchronous errors we can detect, you use this, end of story. It is the only way to handle this situation without breaking the world.

As usual, Alexey Kuznetsov's analysis of this situation is timeless, accurate, and wise. And he understood all of this 10+ years ago.

Thanks David, here is the 2nd patch then :

[PATCH net-next-2.6] ip: Report qdisc packet drops

Christoph Lameter pointed out that packet drops at qdisc level where not accounted in SNMP counters. Only if application sets IP_RECVERR, drops are reported to user (-ENOBUFS errors) and SNMP counters updated.

IP_RECVERR is used to enable extended reliable error message passing, but these are not needed to update system wide SNMP stats.

This patch changes things a bit to allow SNMP counters to be updated, regardless of IP_RECVERR being set or not on the socket.

Example after an UDP tx flood

Quoted 9 lines

# netstat -s 
...
IP:
    1487048 outgoing packets dropped
...
Udp:
...
    SndbufErrors: 1487048

Didn't we agree that qdisc drops should not be counted as IP or UDP drops as David Stevens pointed out?
I would say that even when IP_RECVERR is set, SNMP counters at IP and UDP should not be counted when a packet is dropped at qdisc level, but the error can be reported to user.

Now that qdisc stats issue is figured out and they can be accounted and seen at qdisc level, doesn't it confuse if we count the same drop at IP, UDP and qdisc level?

Thanks
Sridhar

Quoted 26 lines · David Miller

From: Eric Dumazet <[email protected]>
Date: Mon, 31 Aug 2009 14:09:50 +0200

Quoted 7 lines

Re-reading again this stuff, I realized ip6_push_pending_frames() was not updating IPSTATS_MIB_OUTDISCARDS, even if IP_RECVERR was set.

May I suggest following path :

1) Correct ip6_push_pending_frames() to properly account for dropped-by-qdisc frames when IP_RECVERR is set

Your patch is applied to net-next-2.6, thanks!

Quoted 2 lines

2) Submit a patch to account for qdisc-dropped frames in SNMP counters but still return a OK to user application, to not break them ?

Sounds good.

I think if you sample random UDP applications, you will find that such errors will upset them terribly, make them log tons of crap to /var/log/messages et al., and consume tons of CPU.

And in such cases silent ignoring of drops is entirely appropriate and optimal, which supports our current behavior.

If we are to make such applications "more sophisticated" such converted apps can be indicated simply their use of IP_RECVERR.

Quoted 6 lines

If you want to be notified of all asynchronous errors we can detect, you use this, end of story. It is the only way to handle this situation without breaking the world.

As usual, Alexey Kuznetsov's analysis of this situation is timeless, accurate, and wise. And he understood all of this 10+ years ago.

Thanks David, here is the 2nd patch then :

Here is an updated version of the patch, after Christoph comments.

[PATCH net-next-2.6] ip: Report qdisc packet drops

net/ipv4/udp.c static int udp_push_pending_frames(struct sock *sk) · line 567

Quoted 11 lines · Eric Dumazet

index 29ebb0d..ebaaa7f 100644

net/ipv4/udp.c +4 -0

@@ -561,12 +561,18 @@ static int udp_push_pending_frames(struct sock *sk)
 
 send:
 	err = ip_push_pending_frames(sk);
+	if (err) {
+		if (err == -ENOBUFS && !inet->recverr) {
+			UDP_INC_STATS_USER(sock_net(sk),
+					   UDP_MIB_SNDBUFERRORS, is_udplite);

This means that we now do not increment SNDBUFERRORS if IP_RECVERR is set.

I think it would be better to move UDP_INC_STATS_USER before the if statement. That will also simplify the code further.

Quoted 13 lines

net/ipv4/udp.c static int udp_push_pending_frames(struct sock *sk) · line 567

Quoted 10 lines

net/ipv4/udp.c +4 -0

@@ -561,12 +561,18 @@ static int udp_push_pending_frames(struct sock *sk)
 
 send:
 	err = ip_push_pending_frames(sk);
+	if (err) {
+		if (err == -ENOBUFS && !inet->recverr) {
+			UDP_INC_STATS_USER(sock_net(sk),
+					   UDP_MIB_SNDBUFERRORS, is_udplite);

This means that we now do not increment SNDBUFERRORS if IP_RECVERR is set.

Quoted 4 lines

I think it would be better to move UDP_INC_STATS_USER before the if statement. That will also simplify the code further.

I believe you already said this once Christoph on a previous patch, and I replied that in case of IP_RECVERR set, udp_push_pending_frames() returns -ENOBUFS to its caller : udp_sendmsg()

Because you need drop statistics on a device to figure out when you may want to increase the TX buffers for a device. If a packet was dropped because of a lack of TX buffers then we need to know.

I'm not accepting changes like the patch you propose here, it's not the correct thing to do.

The device never saw the packet, it never dropped it.

The qdisc saw it, the qdisc dropped it, and therefore that's where we account for it.

Are you saying that this behavior is okay? It is desired behavior that a socket option changes the way how global network counters are handled?

Seems though that the qdisc drop count does not flow into the tx_dropped counter for the interface.

From: Christoph Lameter <[email protected]>
Date: Fri, 28 Aug 2009 09:53:40 -0400 (EDT)

Quoted 2 lines

Seems though that the qdisc drop count does not flow into the tx_dropped counter for the interface.

And it should not.

The qdisc drops the packet due to flow control, not the hardware device.

Device drops are for things like transmission errors on the wire.

If you start incrementing tx_dropped here, people won't be able to tell they have a deteriorating cable or bad switch or similar.

Quoted 3 lines

If you start incrementing tx_dropped here, people won't be able to tell they have a deteriorating cable or bad switch or similar.

?? The patch does not do that. That was extra stuff not covered by the patch.

From: Christoph Lameter <[email protected]>
Date: Fri, 28 Aug 2009 15:53:42 -0400 (EDT)

Quoted 2 lines · David Miller

Quoted 5 lines

Quoted 2 lines

If you start incrementing tx_dropped here, people won't be able to tell they have a deteriorating cable or bad switch or similar.

?? The patch does not do that. That was extra stuff not covered by the patch.

Yes you did, you put a device tx_dropped counter bump into qdisc_drop().

From: Christoph Lameter <[email protected]>
Date: Fri, 28 Aug 2009 09:53:40 -0400 (EDT)

Quoted 2 lines

Seems though that the qdisc drop count does not flow into the tx_dropped counter for the interface.

And it should not.

The qdisc drops the packet due to flow control, not the hardware device.

Device drops are for things like transmission errors on the wire.

If you start incrementing tx_dropped here, people won't be able to tell they have a deteriorating cable or bad switch or similar.

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

NET_XMIT_CN strikes again :)

Well, if ip_local_out() returns a negative error (say -EPERM for example),
your patch disables OUTDISCARDS increments.

Maybe a simpler patch like this one ?

Quoted 6 lines · Eric Dumazet

NET_XMIT_CN strikes again :)

Well, if ip_local_out() returns a negative error (say -EPERM for example),
your patch disables OUTDISCARDS increments.

Maybe a simpler patch like this one ?

Yes looks good. Can we get this in soon?

Please hold on, I would like to fully understand what's happening, and test the patch :)

Quoted 2 lines · Eric Dumazet

Please hold on, I would like to fully understand what's happening, and test the patch :)

Ok. It would be good if the drops would also be somehow noted by the UDP subsystem (one should see something with netstat -su) and may be even the socket. I see a drops column in /proc/net/udp. rx_drops, tx_drops?

Christoph Lameter a ?crit :

Quoted 8 lines · Eric Dumazet

Quoted 3 lines

Please hold on, I would like to fully understand what's happening, and test the patch :)

Ok. It would be good if the drops would also be somehow noted by the UDP subsystem (one should see something with netstat -su) and may be even the socket. I see a drops column in /proc/net/udp. rx_drops, tx_drops?

This /proc/net/udp column is for rx_drops currently and was recently added...

Quoted 3 lines · Eric Dumazet

Christoph Lameter a ?crit :

Quoted 2 lines · Eric Dumazet

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

Please hold on, I would like to fully understand what's happening, and test the patch :)

Ok. It would be good if the drops would also be somehow noted by the UDP subsystem (one should see something with netstat -su) and may be even the socket. I see a drops column in /proc/net/udp. rx_drops, tx_drops?

This /proc/net/udp column is for rx_drops currently and was recently added...

So lets rename it to rx_drops and then add tx_drops?

It wont be very nice, because it'll add yet another 32bits counter in each socket structure, for a unlikely use. While rx_drops can happen if application is slow.

Also, tx_drops might be done later and not noticed.

Please read this old (and usefull) thread, with Alexey words...

http://oss.sgi.com/archives/netdev/2002-10/msg00612.html

http://oss.sgi.com/archives/netdev/2002-10/msg00617.html

So I bet your best choice is to set IP_RECVERR, as mentioned in 2002 by Jamal and Alexey :)

Quoted 2 lines · Eric Dumazet

It wont be very nice, because it'll add yet another 32bits counter in each socket structure, for a unlikely use. While rx_drops can happen if application is slow.

tx_drops happen if the application sends too fast.

TX drop tracking is important due to the braindamaged throttling logic during send. If SO_SNDBUF is less than what happens to fit in the TX ring then the application will be throttled and no packet loss happens. If SO_SNDBUF is set high then the TX ring will overflow and packets are dropped.

We need some way to diagnose TX drops per socket as long as we have that mind boggling issue. TX drops means that one should reduce the size of the sendbuffer in order to get better throttling which reduces packet loss.

Quoted 9 lines

Also, tx_drops might be done later and not noticed.

Please read this old (and usefull) thread, with Alexey words...

http://oss.sgi.com/archives/netdev/2002-10/msg00612.html

http://oss.sgi.com/archives/netdev/2002-10/msg00617.html

Quoted 1 line

So I bet your best choice is to set IP_RECVERR, as mentioned in 2002 by Jamal and Alexey :)

I read this just yesterday. IP_RECVERR means that the application wants to see details on each loss. We just want some counters that give us accurate statistics to gauge where packet loss is occurring. Applications are usually not interested in tracking the fate of each packet.

Quoted 4 lines

I read this just yesterday. IP_RECVERR means that the application wants to see details on each loss. We just want some counters that give us accurate statistics to gauge where packet loss is occurring. Applications are usually not interested in tracking the fate of each packet.

Yep, but IP_RECVERR also has the side effect of letting kernel returns -ENOBUFS error in sending and congestion, which was your initial point :)

Quoted 6 lines · Eric Dumazet

Quoted 4 lines

I read this just yesterday. IP_RECVERR means that the application wants to see details on each loss. We just want some counters that give us accurate statistics to gauge where packet loss is occurring. Applications are usually not interested in tracking the fate of each packet.

Yep, but IP_RECVERR also has the side effect of letting kernel returns -ENOBUFS error in sending and congestion, which was your initial point :)

The initial point was that the SNMP counters are not updated if IP_RECVERR is not set which is clearly a bug and your and my patch addresses that.

Then Sridhar noted that there are other tx drop counters. qdisc counters are also not updated. Wish we would maintain tx drops counters there as well so that we can track down which NIC drops it.

Then came the wishlist of UDP counters for tx drops and socket based tx_drop accounting for tuning and tracking down which app is sending too fast .... ;-)

The apps could be third party apps. Just need to be able to troubleshoot packet loss.

Quoted 2 lines

The initial point was that the SNMP counters are not updated if IP_RECVERR is not set which is clearly a bug and your and my patch addresses that.

Technically speaking, the send() syscall is in error. Frame is not sent, so there is no drop at all. Like trying to send() from a bad user buffer, or write() to a too big file...

Question is : should we just allow send() to return an error (-ENOBUF) regardless of IP_RECVERR being set or not ? I dont think it would be so bad after all. Most apps probably dont care, or already handle the error.

Quoted 5 lines · Eric Dumazet

Quoted 2 lines

The initial point was that the SNMP counters are not updated if IP_RECVERR is not set which is clearly a bug and your and my patch addresses that.

Technically speaking, the send() syscall is in error. Frame is not sent, so there is no drop at all. Like trying to send() from a bad user buffer, or write() to a too big file...

Frame is submitted to the IP layer which discards it. That is the definition of an output discard.

Quoted 3 lines

Question is : should we just allow send() to return an error (-ENOBUF) regardless of IP_RECVERR being set or not ? I dont think it would be so bad after all. Most apps probably dont care, or already handle the error.

Some applications will then start to fail because so far you can send with impunity without getting errors. AFAICT IP_RECVERR was added to preserve that behavior. Your patch is changing basic send() semantics.

Christoph Lameter a ?crit :

Quoted 8 lines · Eric Dumazet

Quoted 6 lines

Quoted 3 lines

The initial point was that the SNMP counters are not updated if IP_RECVERR is not set which is clearly a bug and your and my patch addresses that.

Technically speaking, the send() syscall is in error. Frame is not sent, so there is no drop at all. Like trying to send() from a bad user buffer, or write() to a too big file...

Quoted 3 lines

Frame is submitted to the IP layer which discards it. That is the definition of an output discard.

Last patch accounts for this *error* AFAIK, or did I missed something ?

Quoted 7 lines

Quoted 3 lines

Question is : should we just allow send() to return an error (-ENOBUF) regardless of IP_RECVERR being set or not ? I dont think it would be so bad after all. Most apps probably dont care, or already handle the error.

Some applications will then start to fail because so far you can send with impunity without getting errors. AFAICT IP_RECVERR was added to preserve that behavior. Your patch is changing basic send() semantics.

Sorry ???, I guess your machines have plenty available LOWMEM then, and kmalloc() never fail then...

basic send() semantics are respected.

Quoted 11 lines · Christoph Lameter

Quoted 6 lines · Eric Dumazet

Quoted 4 lines

I read this just yesterday. IP_RECVERR means that the application wants to see details on each loss. We just want some counters that give us accurate statistics to gauge where packet loss is occurring. Applications are usually not interested in tracking the fate of each packet.

Yep, but IP_RECVERR also has the side effect of letting kernel returns -ENOBUFS error in sending and congestion, which was your initial point :)

The initial point was that the SNMP counters are not updated if IP_RECVERR is not set which is clearly a bug and your and my patch addresses that.

Technically speaking, the send() syscall is in error. Frame is not sent, so there is no drop at all. Like trying to send() from a bad user buffer, or write() to a too big file...

Quoted 9 lines

Then Sridhar noted that there are other tx drop counters. qdisc counters are also not updated. Wish we would maintain tx drops counters there as well so that we can track down which NIC drops it.

Then came the wishlist of UDP counters for tx drops and socket based tx_drop accounting for tuning and tracking down which app is sending too fast .... ;-)

Quoted 3 lines

The apps could be third party apps. Just need to be able to troubleshoot packet loss.

Question is : should we just allow send() to return an error (-ENOBUF) regardless of IP_RECVERR being set or not ? I dont think it would be so bad after all. Most apps probably dont care, or already handle the error.

net/ipv4/ip_output.c +1 -1

index 7d08210..afae0cb 100644
@@ -1302,7 +1302,7 @@ int ip_push_pending_frames(struct sock *sk)
 	err = ip_local_out(skb);
 	if (err) {
 		if (err > 0)
-			err = inet->recverr ? net_xmit_errno(err) : 0;
+			err = net_xmit_errno(err);
 		if (err)
 			goto error;
 	}

net/ipv6/ip6_output.c +1 -1

index 87f8419..a7e5f93 100644
@@ -1526,7 +1526,7 @@ int ip6_push_pending_frames(struct sock *sk)
 	err = ip6_local_out(skb);
 	if (err) {
 		if (err > 0)
-			err = np->recverr ? net_xmit_errno(err) : 0;
+			err = net_xmit_errno(err);
 		if (err)
 			goto error;
 	}

Quoted 6 lines · Sridhar Samudrala

Quoted 4 lines · Christoph Lameter

Quoted 1 line · Sridhar Samudrala

What about ethtool -S ? Does it report any errors?

Neither. This is is a broadcom bnx2 NIC.

Are you sure the packets are dropped at the sender?

Yes I am sending 400k messages from the app and the receiver only gets 341k @300 byte (which is the line rate). There is no way that the 400k get over the line. Also if I reduce SO_SNDBUF then both receiver and sender get down to 341k.

I added the output of ethtool -S at the end.

The mcast tool can be had from http://gentwo.org/ll or from my directory on www.kernel.org.

Quoted 13 lines · Christoph Lameter

Quoted 6 lines · Sridhar Samudrala

Quoted 4 lines · Christoph Lameter

Quoted 1 line · Sridhar Samudrala

What about ethtool -S ? Does it report any errors?

Neither. This is is a broadcom bnx2 NIC.

Are you sure the packets are dropped at the sender?

Yes I am sending 400k messages from the app and the receiver only gets 341k @300 byte (which is the line rate). There is no way that the 400k get over the line. Also if I reduce SO_SNDBUF then both receiver and sender get down to 341k.

Quoted 5 lines

I added the output of ethtool -S at the end.

The mcast tool can be had from http://gentwo.org/ll or from my directory on www.kernel.org.

# gcc -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.4.1/configure --enable-languages=c,c++ --prefix=/usr
Thread model: posix
gcc version 4.4.1 (GCC)
# pwd
/opt/src/lldiag-0.14
# make
gcc -Wall -omcast mcast.c -lrt -lm
mcast.c: In function ‘set_ip’:
mcast.c:121: warning: implicit declaration of function ‘htons’
mcast.c: In function ‘build_pattern_array’:
mcast.c:168: warning: implicit declaration of function ‘htonl’
/tmp/cc4sYCDr.o: In function `lock':
mcast.c:(.text+0xcad): undefined reference to `__sync_fetch_and_add_4'
collect2: ld returned 1 exit status
make: *** [mcast] Error 1

I have no idea where is defined sync_fetch_and_add

Thread model: posix
gcc version 4.4.1 (GCC)

# pwd
/opt/src/lldiag-0.14
# make
gcc -Wall -omcast mcast.c -lrt -lm
mcast.c: In function ?set_ip?:
mcast.c:121: warning: implicit declaration of function ?htons?
mcast.c: In function ?build_pattern_array?:
mcast.c:168: warning: implicit declaration of function ?htonl?
/tmp/cc4sYCDr.o: In function `lock':
mcast.c:(.text+0xcad): undefined reference to `__sync_fetch_and_add_4'
collect2: ld returned 1 exit status
make: *** [mcast] Error 1

I have no idea where is defined sync_fetch_and_add