[2.6 patch] unexport icmpmsg_statistics
This patch removes the unused EXPORT_SYMBOL(icmpmsg_statistics). Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> --- 4ce74657ac0b1bdcb4c7bc359d05643f8cc4a08b diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 272c69e..233de06 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -1104,5 +1104,4 @@ void __init icmp_init(struct net_proto_family *ops) EXPORT_SYMBOL(icmp_err_convert); EXPORT_SYMBOL(icmp_send); EXPORT_SYMBOL(icmp_statistics); -EXPORT_SYMBOL(icmpmsg_statistics); EXPORT_SYMBOL(xrlim_allow); - 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
Re: [2.6 patch] unexport icmpmsg_statistics
[EMAIL PROTECTED] wrote on 10/24/2007 09:24:10 AM: > This patch removes the unused EXPORT_SYMBOL(icmpmsg_statistics). > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> > > --- > 4ce74657ac0b1bdcb4c7bc359d05643f8cc4a08b > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > index 272c69e..233de06 100644 > --- a/net/ipv4/icmp.c > +++ b/net/ipv4/icmp.c > @@ -1104,5 +1104,4 @@ void __init icmp_init(struct net_proto_family *ops) > EXPORT_SYMBOL(icmp_err_convert); > EXPORT_SYMBOL(icmp_send); > EXPORT_SYMBOL(icmp_statistics); > -EXPORT_SYMBOL(icmpmsg_statistics); > EXPORT_SYMBOL(xrlim_allow); "icmpmsg_statistics" belongs with (and replaces some of the old...) "icmp_statistics". I'm not sure that any modules use it, but I think you should remove both or neither. +-DLS - 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
Re: [2.6 patch] unexport icmpmsg_statistics
On Wed, Oct 24, 2007 at 12:07:45PM -0700, David Stevens wrote: > [EMAIL PROTECTED] wrote on 10/24/2007 09:24:10 AM: > > > This patch removes the unused EXPORT_SYMBOL(icmpmsg_statistics). > > > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> > > > > --- > > 4ce74657ac0b1bdcb4c7bc359d05643f8cc4a08b > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > > index 272c69e..233de06 100644 > > --- a/net/ipv4/icmp.c > > +++ b/net/ipv4/icmp.c > > @@ -1104,5 +1104,4 @@ void __init icmp_init(struct net_proto_family > *ops) > > EXPORT_SYMBOL(icmp_err_convert); > > EXPORT_SYMBOL(icmp_send); > > EXPORT_SYMBOL(icmp_statistics); > > -EXPORT_SYMBOL(icmpmsg_statistics); > > EXPORT_SYMBOL(xrlim_allow); > > "icmpmsg_statistics" belongs with (and replaces some of the > old...) > "icmp_statistics". I'm not sure that any modules use it, but I think you > should remove both or neither. icmp_statistics is used by the dccp_ipv4 and sctp modules. icmpmsg_statistics is not used by any modules. > +-DLS cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - 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
Re: [2.6 patch] unexport icmpmsg_statistics
[EMAIL PROTECTED] wrote on 10/24/2007 12:14:37 PM: > On Wed, Oct 24, 2007 at 12:07:45PM -0700, David Stevens wrote: > > [EMAIL PROTECTED] wrote on 10/24/2007 09:24:10 AM: > > > > > This patch removes the unused EXPORT_SYMBOL(icmpmsg_statistics). > > > > > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> > > > > > > --- > > > 4ce74657ac0b1bdcb4c7bc359d05643f8cc4a08b > > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > > > index 272c69e..233de06 100644 > > > --- a/net/ipv4/icmp.c > > > +++ b/net/ipv4/icmp.c > > > @@ -1104,5 +1104,4 @@ void __init icmp_init(struct net_proto_family > > *ops) > > > EXPORT_SYMBOL(icmp_err_convert); > > > EXPORT_SYMBOL(icmp_send); > > > EXPORT_SYMBOL(icmp_statistics); > > > -EXPORT_SYMBOL(icmpmsg_statistics); > > > EXPORT_SYMBOL(xrlim_allow); > > > > "icmpmsg_statistics" belongs with (and replaces some of the > > old...) > > "icmp_statistics". I'm not sure that any modules use it, but I think you > > should remove both or neither. > > icmp_statistics is used by the dccp_ipv4 and sctp modules. The only items left in icmp_statistics are "InMsgs, InErrs, OutMsgs, OutErrs", so if dccp and sctp are sending or receiving any in or out ICMP messages, they should be using the new macros (which reference icmpmsg_statistics, not icmp_statistics) to count them. I took a quick look at SCTP. I don't know if that's going through icmp_rcv() or not; if so, I think it's double-counting; if not, then it isn't counting the individual types (as it should), and it should have ICMPMSG macros doing that. So, again, icmpmsg_statistics either should stay exported, or neither icmpmsg_statistics nor icmp_statistics should be exported (depending on how SCTP and DCCP code is resolved). It's incorrect in the current code to incrememnt ICMP_MIB_INMSGS without incrementing one of the types too. +-DLS - 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
Re: [2.6 patch] unexport icmpmsg_statistics
I took a look at the DCCP references, and I think they're just incrementing the wrong MIB variable -- e.g., it's incrementing ICMP_MIB_INERRORS when the skb length is less than the header indicates. That's not an ICMP_MIB_INERRORS error, that's an IPSTATS_MIB_INHDRERRORS error. ICMP_MIB_INERRORS is when you receive an ICMP error packet; an IP header error is something else entirely. That's followed by a failed lookup incrementing ICMP_MIB_INERRORS which should be an unknown port error in the transport MIB (assuming it has one-- it's not an ICMP error; could be an IP error, if the address isn't local, rather than unknown port). In SCTP, it appears to have similar problems. SCTP errors are not ICMP errors, though it perhaps should be calling icmp_send() to send one to the offending host for some of the cases. I haven't seen any ICMP-relevant stats correctly referenced in these yet. I don't want to patch them directly, since I can't easily test them; if someone who works with DCCP and SCTP would like to, I'd be happy to review. Any volunteers? +-DLS - 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
Re: [2.6 patch] unexport icmpmsg_statistics
David Stevens wrote: > I took a look at the DCCP references, and I think they're just > incrementing the wrong MIB variable -- e.g., it's incrementing > ICMP_MIB_INERRORS when the skb length is less than the > header indicates. That's not an ICMP_MIB_INERRORS error, > that's an IPSTATS_MIB_INHDRERRORS error. ICMP_MIB_INERRORS > is when you receive an ICMP error packet; an IP header error > is something else entirely. > > That's followed by a failed lookup incrementing ICMP_MIB_INERRORS > which should be an unknown port error in the transport MIB (assuming > it has one-- it's not an ICMP error; could be an IP error, if the address > isn't local, rather than unknown port). > > In SCTP, it appears to have similar problems. SCTP errors are not > ICMP errors, though it perhaps should be calling icmp_send() to > send one to the offending host for some of the cases. > > I haven't seen any ICMP-relevant stats correctly referenced in > these yet. > > I don't want to patch them directly, since I can't easily test them; > if someone who works with DCCP and SCTP would like to, I'd > be happy to review. Any volunteers? I'll take a look at the SCTP ones. Thanks for review. -vlad > > +-DLS > > - > 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 > - 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
Re: [2.6 patch] unexport icmpmsg_statistics
My bad -- I see what it's doing, and it looks ok after all. I thought I saw an INMSGS (but didn't). These are ICMP errors that went through icmp_rcv() and were counted correctly before getting to the protocol error handlers. These are failures due mostly to not having enough, or the right protocol info in the error packet being handled. I'm not sure I'd count those as ICMP errors, since the ICMP header itself is correct, but ok... SCTP doesn't look so bad, though I think the references are still questionable (but debatable) as ICMP errors. sctp_v4_err is incrementing ICMP_MIB_INERRORS if there isn't enough IP header to find the ports, I see. I'm not sure that counts as an ICMP error, but it's not so terrible. It's doing the same thing if a lookup fails to match "vtag" from the encapsulated error packet. Again, I don't know that those are ICMP errors (which normally are something wrong with the ICMP header). So, I stand corrected, and sorry about the histrionics. Since these are arguably ICMP errors, and since errors is the only thing being MIB-counted in DCCP and SCTP, then it now looks ok to me as-is, and also ok to remove icmpmsg_statistics from exporting. +-DLS - 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
Re: [2.6 patch] unexport icmpmsg_statistics
David Stevens wrote: > I took a look at the DCCP references, and I think they're just > incrementing the wrong MIB variable -- e.g., it's incrementing > ICMP_MIB_INERRORS when the skb length is less than the > header indicates. That's not an ICMP_MIB_INERRORS error, > that's an IPSTATS_MIB_INHDRERRORS error. ICMP_MIB_INERRORS > is when you receive an ICMP error packet; an IP header error > is something else entirely. Looking at icmp_rcv(), ICMP_MIB_INERRORS is incremented if: a) checksum fails b) no enough room in skb for icmp header c) type out of bound and other error conditions while processing ICMP packet. Are all of these wrong as well? There are other places that increment this statistic for errors during processing. > > That's followed by a failed lookup incrementing ICMP_MIB_INERRORS > which should be an unknown port error in the transport MIB (assuming > it has one-- it's not an ICMP error; could be an IP error, if the address > isn't local, rather than unknown port). > > In SCTP, it appears to have similar problems. SCTP errors are not > ICMP errors, though it perhaps should be calling icmp_send() to > send one to the offending host for some of the cases. I'll skip the insufficient buffer space statistic yet, since, per above, it's not clear which one should be used. Others, I agree are move ULP errors, but the mibs don't account for those yet. -vlad - 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
Re: [2.6 patch] unexport icmpmsg_statistics
David Stevens wrote: > My bad -- I see what it's doing, and it looks ok after all. > jinx, crossed in flight. ;) - 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
Re: [2.6 patch] unexport icmpmsg_statistics
David Stevens wrote: > My bad -- I see what it's doing, and it looks ok after all. > > I thought I saw an INMSGS (but didn't). These are ICMP errors that > went through icmp_rcv() and were counted correctly before getting > to the protocol error handlers. These are failures due mostly to not > having enough, or the right protocol info in the error packet being > handled. I'm not sure I'd count those as ICMP errors, since the > ICMP header itself is correct, but ok... > > SCTP doesn't look so bad, though I think the references are > still questionable (but debatable) as ICMP errors. > > sctp_v4_err is incrementing ICMP_MIB_INERRORS if there > isn't enough IP header to find the ports, I see. I'm not sure > that counts as an ICMP error, but it's not so terrible. > > It's doing the same thing if a lookup fails to match "vtag" from > the encapsulated error packet. Again, I don't know that those > are ICMP errors (which normally are something wrong with > the ICMP header). This particular case is the one that bugs the most, but that error matches best. Seems like all ULPs treat socket lookup error as ICMP_MIB_INERRORS (tcp, udp, sctp, dccp). SCTP is a little special in that in needs to check one one piece of data (the 'vtag') to correctly identify the connection. If that piece doesn't match, we treat that as the same error. -vlad - 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
Re: [2.6 patch] unexport icmpmsg_statistics
From: David Stevens <[EMAIL PROTECTED]> Date: Wed, 24 Oct 2007 13:43:14 -0700 > So, I stand corrected, and sorry about the histrionics. Since > these are arguably ICMP errors, and since errors is the only > thing being MIB-counted in DCCP and SCTP, then it now looks > ok to me as-is, and also ok to remove icmpmsg_statistics from > exporting. I've applied Adrian's export removal patch, thanks! - 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