linux-kernel - Re: [PATCH] Fixed NetEm's reorder option

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <6de6b1650806111305m51bf2eccocbe0d0c7afee266d@mail.gmail.com>
Date:	Wed, 11 Jun 2008 17:05:10 -0300
From:	"Rafael Almeida" <almeidaraf@...il.com>
To:	shemminger@...ux-foundation.org
Cc:	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] Fixed NetEm's reorder option

I noticed no one said anything about my patch below, did I do
something terrible wrong? Should I just be more patient?

On Thu, Jun 5, 2008 at 1:34 AM, Rafael C. de Almeida
<almeidaraf@...il.com> wrote:
> The reorder option wasn't working correctly. It would only kick in if
> the gap option was set. In that case it would only lower the
> probability of reordering packets because of how the if conditional
> was stated.
>
> This patch separetes the algorithm for reordering with gap from the
> rest, rendering the code more readable and correct.
>
> Signed-off-by: Rafael C. de Almeida <rafaelc@....ufmg.br>
> ---
>  net/sched/sch_netem.c |   45 ++++++++++++++++++++++++++++++++-------------
>  1 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index c9c649b..477ee09 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -206,27 +206,46 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
>        }
>
>        cb = (struct netem_skb_cb *)skb->cb;
> -       if (q->gap == 0                 /* not doing reordering */
> -           || q->counter < q->gap      /* inside last reordering gap */
> -           || q->reorder < get_crandom(&q->reorder_cor)) {
> +       if (q->gap) {
> +               /* The comparison of reorder is here to keep compatibility with
> +                * the old behaviour when one had reorder and gap set, maybe it
> +                * could be removed.
> +                */
> +               if (q->counter < q->gap
> +                   || q->reorder < get_crandom(&q->reorder_cor)) {
> +                       psched_time_t now;
> +                       psched_tdiff_t delay;
> +
> +                       delay = tabledist(q->latency, q->jitter,
> +                                         &q->delay_cor, q->delay_dist);
> +
> +                       now = psched_get_time();
> +                       cb->time_to_send = now + delay;
> +                       ++q->counter;
> +                       ret = q->qdisc->enqueue(skb, q->qdisc);
> +               } else {
> +                       /*
> +                        * Do re-ordering by putting one out of N packets at the front
> +                        * of the queue.
> +                        */
> +                       cb->time_to_send = psched_get_time();
> +                       q->counter = 0;
> +                       ret = q->qdisc->ops->requeue(skb, q->qdisc);
> +               }
> +       } else {
>                psched_time_t now;
>                psched_tdiff_t delay;
>
> -               delay = tabledist(q->latency, q->jitter,
> -                                 &q->delay_cor, q->delay_dist);
> +               if (!q->reorder || q->reorder < get_crandom(&q->reorder_cor))
> +                       delay = tabledist(q->latency, q->jitter,
> +                                         &q->delay_cor, q->delay_dist);
> +               else
> +                       delay = 0;
>
>                now = psched_get_time();
>                cb->time_to_send = now + delay;
>                ++q->counter;
>                ret = q->qdisc->enqueue(skb, q->qdisc);
> -       } else {
> -               /*
> -                * Do re-ordering by putting one out of N packets at the front
> -                * of the queue.
> -                */
> -               cb->time_to_send = psched_get_time();
> -               q->counter = 0;
> -               ret = q->qdisc->ops->requeue(skb, q->qdisc);
>        }
>
>        if (likely(ret == NET_XMIT_SUCCESS)) {
> --
> 1.5.5.GIT
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/