Hi Vlad,
On Thu, Nov 29, 2012 at 12:31:17PM -0500, Vlad Yasevich wrote:
> On 11/28/2012 02:39 PM, Michele Baldessari wrote:
>
> >diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c
> >index 397296f..7edc89d 100644
> >--- a/net/sctp/inqueue.c
> >+++ b/net/sctp/inqueue.c
> >@@ -105,6 +105,9 @@ void sctp_inq_push(struct sctp_inq *q, struct sctp_chunk *chunk)
> > */
> > list_add_tail(&chunk->list, &q->in_chunk_list);
> > q->immediate.func(&q->immediate);
> >+
> >+ if (chunk->asoc)
> >+ chunk->asoc->stats.ipackets++;
>
> you may want to consider putting this before the immediate.func()
> call, so that you record an incoming packet before it's fully
> processed. This
> would mimic the behavior of the rest of the stats you are collecting, as
> you typically increment the stat and then process the data.
ok, makes sense.
> > }
> >
> > /* Peek at the next chunk on the inqeue. */
>
> >
> >+/*
> >+ * SCTP_GET_ASSOC_STATS
> >+ *
> >+ * This option retrieves local per endpoint statistics. It is modeled
> >+ * after OpenSolaris' implementation
> >+ */
> >+static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
> >+ char __user *optval,
> >+ int __user *optlen)
> >+{
> >+ struct sctp_assoc_stats sas;
> >+ struct sctp_association *asoc = NULL;
> >+
> >+ /* User must provide at least the assoc id */
> >+ if (len < sizeof(sctp_assoc_t))
> >+ return -EINVAL;
> >+
> >+ if (copy_from_user(&sas, optval, len))
> >+ return -EFAULT;
> >+
> >+ asoc = sctp_id2assoc(sk, sas.sas_assoc_id);
> >+ if (!asoc)
> >+ return -EINVAL;
> >+
> >+ sas.sas_rtxchunks = asoc->stats.rtxchunks;
> >+ sas.sas_gapcnt = asoc->stats.gapcnt;
> >+ sas.sas_outofseqtsns = asoc->stats.outofseqtsns;
> >+ sas.sas_osacks = asoc->stats.osacks;
> >+ sas.sas_isacks = asoc->stats.isacks;
> >+ sas.sas_octrlchunks = asoc->stats.octrlchunks;
> >+ sas.sas_ictrlchunks = asoc->stats.ictrlchunks;
> >+ sas.sas_oodchunks = asoc->stats.oodchunks;
> >+ sas.sas_iodchunks = asoc->stats.iodchunks;
> >+ sas.sas_ouodchunks = asoc->stats.ouodchunks;
> >+ sas.sas_iuodchunks = asoc->stats.iuodchunks;
> >+ sas.sas_idupchunks = asoc->stats.idupchunks;
> >+ sas.sas_opackets = asoc->stats.opackets;
> >+ sas.sas_ipackets = asoc->stats.ipackets;
> >+
> >+ /* New high max rto observed, will return 0 if not a single
> >+ * RTO update took place. obs_rto_ipaddr will be bogus
> >+ * in such a case
> >+ */
> >+ sas.sas_maxrto = asoc->stats.max_obs_rto;
> >+ memcpy(&sas.sas_obs_rto_ipaddr, &asoc->stats.obs_rto_ipaddr,
> >+ sizeof(struct sockaddr_storage));
> >+
> >+ /* Mark beginning of a new observation period */
> >+ asoc->stats.max_obs_rto = 0;
>
> Ok. That's better. If there are 2 fast queries you get 0 on the
> second, but that still feels a bit strange to me. I think resetting
> max_obs_rto to rto_min might make more sense. rto_min is the low bound
> and rto can't go below that. So, on the next rto measurement, that'll
> be the smallest value of max_obs_rto. IMO, it might be better to reset
> the max_obs_rto to rto_min, so that
> 1) We always see a valid rto value in the field.
> 2) We can more easily see the variances in rto over time.
>
> What do you think?
I agree, it makes more sense (will send an updated version shortly)
thanks,
Michele
--
Michele Baldessari <michele@xxxxxxxxxx>
C2A5 9DA3 9961 4FFB E01B D0BC DDD4 DCCB 7515 5C6D
--
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
