On Fri, Oct 26, 2012 at 03:42:53PM +0200, Michele Baldessari wrote: > The current SCTP stack is lacking an API to have per association > statistics. This is a kernel implementation modeled after OpenSolaris' > SCTP_GET_ASSOC_STATS. > > Userspace part will follow on lksctp if/when there is a general ACK on this. > > Signed-off-by: Michele Baldessari <michele@xxxxxxxxxx> > Acked-by: Thomas Graf <tgraf@xxxxxxx> > --- > include/net/sctp/sctp.h | 3 +++ > include/net/sctp/structs.h | 39 +++++++++++++++++++++++++++ > include/net/sctp/user.h | 26 ++++++++++++++++++ > net/sctp/associola.c | 20 ++++++++++++++ > net/sctp/endpointola.c | 5 +++- > net/sctp/input.c | 5 ++-- > net/sctp/output.c | 5 ++++ > net/sctp/outqueue.c | 12 +++++++++ > net/sctp/sm_sideeffect.c | 2 ++ > net/sctp/sm_statefuns.c | 12 +++++++-- > net/sctp/socket.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++ > net/sctp/transport.c | 2 ++ > 12 files changed, 193 insertions(+), 5 deletions(-) > We already have files in /proc/net/sctp to count snmp system-wide totals, per-endpoint totals, and per association totals. Why do these stats differently instead of just adding them the per-association file? I get that solaris does this, but its not codified in any of the RFC's or other standards. I would really rather see something like this go into the interfaces we have, rather than creating a new one. I also am a bit confused regarding the stats themselves. Most are fairly clear, but some seem lacking (you count most things sent and received, but only count received gap acks). Others seems vague and or confusing (when counting retransmitted chunks and packets, how do you count a packet that has both new and retransmitted chunks)? And the max observed rto stat is just odd. Each transport has an rto value, not each association, and you cal already see the individual transport rto values in /proc/net/sctp/remaddr. > + > + /* Gap Ack Blocks received */ > + __u64 gapcnt; > + No gapcnt for sent gap ack blocks? > unsigned long timeout; > diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c > index 1b4a7f8..569ee3a 100644 > --- a/net/sctp/outqueue.c > +++ b/net/sctp/outqueue.c > @@ -667,6 +667,8 @@ redo: > chunk->fast_retransmit = SCTP_DONT_FRTX; > > q->empty = 0; > + if (q->asoc) > + q->asoc->rtxchunks++; > break; > } > > @@ -678,6 +680,10 @@ redo: > break; > } > > + if (q->asoc) > + q->asoc->rtxpackets++; > + > + This seems incorrect to me. The packet being assembled here may have new chunks in it (either control or data). Counting a packet as being retransmitted just because it has a retransmitted chunk in it seems wrong. At the very least its a misleading/vague statistic. > break; > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c > index b6adef8..4f94432 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -6127,9 +6127,13 @@ static int sctp_eat_data(const struct sctp_association *asoc, > /* The TSN is too high--silently discard the chunk and > * count on it getting retransmitted later. > */ > + if (chunk->asoc) > + chunk->asoc->outseqtsns++; This just seems wrong. The definition states that this is counting the last TSN recevied (despite being name outseqtsns), yet this looks like you're: 1) just incrementing a counter, rather than recording the TSN value itself (which may or may not be what you meant, but seems to contradict what the comments at the definition) 2) Only incremanting it if the TSN is out of range, which makes very little sense to me. > -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
