| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 03 Oct 2012 17:29:13 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: David Stevens <dlstevens@...ibm.com>
Cc: chris2553@...glemail.com, Dave Jones <davej@...hat.com>,
David Miller <davem@...emloft.net>, gpiez@....de,
Julian Anastasov <ja@....bg>, netdev@...r.kernel.org,
netdev-owner@...r.kernel.org
Subject: Re: [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive
On Wed, 2012-10-03 at 10:09 -0400, David Stevens wrote:
> Eric Dumazet <eric.dumazet@...il.com> wrote on 10/03/2012 09:15:51 AM:
>
> > So when a host receives an UDP datagram but there was no application
> > at the destination port we should increment udpNoPorts, and its not
> > an error but just a fact.
>
> Of course. I think our difference is on the definition of
> "receives".
A receive is a packet delivered to this host.
Interface being promiscuous or not doesnt really matter.
> I don't think a packet delivered locally due to promiscuous mode,
> broadcast
> or an imperfect multicast address filter match is a host UDP datagram
> receive.
> These packets really shouldn't be delivered to UDP at all; they are not
> addressed to this host (at least the non-broadcast, no-membership ones).
Thats the bug we currently are tracking. If some error is happening and
packet is delivered instead of being forwarded or dropped, we need a
counter being incremented to catch the bug.
> A unicast UDP packet that doesn't match a local IP address does
> not
> increment this counter.
It _does_ increment this counter right now, not sure what you mean.
We currently correctly increment udpNoPorts if we receive an unicast UDP
packet that doesnt find a matching socket (because socket(s) are bound
to specific addresses instead of ANY_ADDR)
This is an extension of the "there was no application at the destination
port" to "there was no application at the destination port and
destination address"
> A promiscuous mode multicast delivery is no
> different,
> except that the destination alone doesn't tell us if it is for us.
>
> I think counting these will primarily lead to administrators
> seeing
> non-zero drops and wasting their time trying to track them down.
Well, as I said, seeing increments of this counter is perfectly fine and
matches RFC. It permits better diagnostics. Hiding bugs is not very
helpful.
Most of the time I am trying to track a bug in linux network stack, the
very first thing I ask to reporters is to post "netstat -s" before/after
their tests exactly because I want to see _some_ counters be incremented
and catch obvious problems.
And alas, many drops in our stack are not correctly reported because we
forgot to increment a counter at the right place.
I am fine adding a new SNMP McastDrops counter if you feel its better.
# grep Udp: /proc/net/snmp
Udp: InDatagrams NoPorts InErrors OutDatagrams RcvbufErrors SndbufErrors McastDrops
Udp: 11449164 15473 514616 290821178 0 184352 134
"netstat -s -u" would display :
Udp:
11449164 packets received
15473 packets to unknown port received.
514616 packet receive errors
290821178 packets sent
SndbufErrors: 184352
McastDrops: 134
Non official patch since net-next is not open :
include/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/udp.c | 2 ++
net/ipv6/proc.c | 2 ++
net/ipv6/udp.c | 2 ++
5 files changed, 8 insertions(+)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index 00bc189..321d643 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -145,6 +145,7 @@ enum
UDP_MIB_OUTDATAGRAMS, /* OutDatagrams */
UDP_MIB_RCVBUFERRORS, /* RcvbufErrors */
UDP_MIB_SNDBUFERRORS, /* SndbufErrors */
+ UDP_MIB_MCASTDROPS, /* McastDrops (linux extension) */
__UDP_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 957acd1..1e932ee 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -172,6 +172,7 @@ static const struct snmp_mib snmp4_udp_list[] = {
SNMP_MIB_ITEM("OutDatagrams", UDP_MIB_OUTDATAGRAMS),
SNMP_MIB_ITEM("RcvbufErrors", UDP_MIB_RCVBUFERRORS),
SNMP_MIB_ITEM("SndbufErrors", UDP_MIB_SNDBUFERRORS),
+ SNMP_MIB_ITEM("McastDrops", UDP_MIB_MCASTDROPS),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 2814f66..4e2a4f7 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1591,6 +1591,8 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
sock_put(stack[i]);
} else {
kfree_skb(skb);
+ UDP_INC_STATS_BH(net, UDP_MIB_MCASTDROPS,
+ udptable != &udp_table);
}
return 0;
}
diff --git a/net/ipv6/proc.c b/net/ipv6/proc.c
index 745a320..f2c12ea 100644
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -129,6 +129,7 @@ static const struct snmp_mib snmp6_udp6_list[] = {
SNMP_MIB_ITEM("Udp6OutDatagrams", UDP_MIB_OUTDATAGRAMS),
SNMP_MIB_ITEM("Udp6RcvbufErrors", UDP_MIB_RCVBUFERRORS),
SNMP_MIB_ITEM("Udp6SndbufErrors", UDP_MIB_SNDBUFERRORS),
+ SNMP_MIB_ITEM("Udp6McastDrops", UDP_MIB_MCASTDROPS),
SNMP_MIB_SENTINEL
};
@@ -139,6 +140,7 @@ static const struct snmp_mib snmp6_udplite6_list[] = {
SNMP_MIB_ITEM("UdpLite6OutDatagrams", UDP_MIB_OUTDATAGRAMS),
SNMP_MIB_ITEM("UdpLite6RcvbufErrors", UDP_MIB_RCVBUFERRORS),
SNMP_MIB_ITEM("UdpLite6SndbufErrors", UDP_MIB_SNDBUFERRORS),
+ SNMP_MIB_ITEM("UdpLite6McastDrops", UDP_MIB_MCASTDROPS);
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 07e2bfe..c8caf1b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -748,6 +748,8 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
sock_put(stack[i]);
} else {
kfree_skb(skb);
+ UDP6_INC_STATS_BH(net, UDP_MIB_MCASTDROPS,
+ udptable != &udp_table);
}
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists