Re: [PATCH 1/3] sfq: timer is deferrable

Re: [PATCH 1/3] sfq: timer is deferrable

[Paul E. McKenney]

On Fri, Jan 18, 2008 at 02:49:00PM -0800, Stephen Hemminger wrote:
> The perturbation timer used for re-keying can be deferred, it doesn't
> need to be deterministic.

The only concern that I can come up with is that the sfq_perturbation
timer might be on one CPU, and all the operations using the corresponding
SFQ on another.  This could in theory allow a nearly omniscient attacker
to exploit an SFQ imbalance while preventing perturbation of the hash
function.

This does not seem to be a valid concern at this point, since there are
very few uses of init_timer_deferrable().  And if it should become a
problem, one approach would be to have some sort of per-timer limit to
the deferral.  Of course, at that point one would need to figure out
what this limit should be!

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>
> 
> 
> --- a/net/sched/sch_sfq.c	2008-01-17 08:29:24.000000000 -0800
> +++ b/net/sched/sch_sfq.c	2008-01-17 09:00:58.000000000 -0800
> @@ -426,7 +426,9 @@ static int sfq_init(struct Qdisc *sch, s
>  	struct sfq_sched_data *q = qdisc_priv(sch);
>  	int i;
> 
> -	setup_timer(&q->perturb_timer, sfq_perturbation, (unsigned long)sch);
> +	q->perturb_timer.function = sfq_perturbation;
> +	q->perturb_timer.data = (unsigned long)sch;;
> +	init_timer_deferrable(&q->perturb_timer);
> 
>  	for (i=0; i<SFQ_HASH_DIVISOR; i++)
>  		q->ht[i] = SFQ_DEPTH;

Re: [PATCH 1/3] sfq: timer is deferrable

[Stephen Hemminger]

On Fri, 18 Jan 2008 20:34:46 -0800
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Fri, Jan 18, 2008 at 02:49:00PM -0800, Stephen Hemminger wrote:
> > The perturbation timer used for re-keying can be deferred, it doesn't
> > need to be deterministic.
> 
> The only concern that I can come up with is that the sfq_perturbation
> timer might be on one CPU, and all the operations using the corresponding
> SFQ on another.  This could in theory allow a nearly omniscient attacker
> to exploit an SFQ imbalance while preventing perturbation of the hash
> function.
> 
> This does not seem to be a valid concern at this point, since there are
> very few uses of init_timer_deferrable().  And if it should become a
> problem, one approach would be to have some sort of per-timer limit to
> the deferral.  Of course, at that point one would need to figure out
> what this limit should be!
> 
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

But the only threat is getting more bandwidth for a longer interval.
It is all kind of moot anyway because the bandwidth hogs all open
multiple connections anyway, so SFQ is of no use.

-- 
Stephen Hemminger <stephen.hemminger@vyatta.com>

Re: [PATCH 1/3] sfq: timer is deferrable

[Paul E. McKenney]

On Fri, Jan 18, 2008 at 08:36:55PM -0800, Stephen Hemminger wrote:
> On Fri, 18 Jan 2008 20:34:46 -0800
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Fri, Jan 18, 2008 at 02:49:00PM -0800, Stephen Hemminger wrote:
> > > The perturbation timer used for re-keying can be deferred, it doesn't
> > > need to be deterministic.
> > 
> > The only concern that I can come up with is that the sfq_perturbation
> > timer might be on one CPU, and all the operations using the corresponding
> > SFQ on another.  This could in theory allow a nearly omniscient attacker
> > to exploit an SFQ imbalance while preventing perturbation of the hash
> > function.
> > 
> > This does not seem to be a valid concern at this point, since there are
> > very few uses of init_timer_deferrable().  And if it should become a
> > problem, one approach would be to have some sort of per-timer limit to
> > the deferral.  Of course, at that point one would need to figure out
> > what this limit should be!
> > 
> > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> But the only threat is getting more bandwidth for a longer interval.
> It is all kind of moot anyway because the bandwidth hogs all open
> multiple connections anyway, so SFQ is of no use.

Good point, and an additional reason for my Acked-by above.  ;-)

						Thanx, Paul

Re: [PATCH 1/3] sfq: timer is deferrable

[David Miller]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Fri, 18 Jan 2008 21:57:00 -0800

> On Fri, Jan 18, 2008 at 08:36:55PM -0800, Stephen Hemminger wrote:
> > On Fri, 18 Jan 2008 20:34:46 -0800
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > On Fri, Jan 18, 2008 at 02:49:00PM -0800, Stephen Hemminger wrote:
> > > > The perturbation timer used for re-keying can be deferred, it doesn't
> > > > need to be deterministic.
> > > 
> > > The only concern that I can come up with is that the sfq_perturbation
> > > timer might be on one CPU, and all the operations using the corresponding
> > > SFQ on another.  This could in theory allow a nearly omniscient attacker
> > > to exploit an SFQ imbalance while preventing perturbation of the hash
> > > function.
> > > 
> > > This does not seem to be a valid concern at this point, since there are
> > > very few uses of init_timer_deferrable().  And if it should become a
> > > problem, one approach would be to have some sort of per-timer limit to
> > > the deferral.  Of course, at that point one would need to figure out
> > > what this limit should be!
> > > 
> > > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > But the only threat is getting more bandwidth for a longer interval.
> > It is all kind of moot anyway because the bandwidth hogs all open
> > multiple connections anyway, so SFQ is of no use.
> 
> Good point, and an additional reason for my Acked-by above.  ;-)

I've applied this patch, thanks :-)

[PATCH 1/3] sfq: timer is deferrable

[Stephen Hemminger]

The perturbation timer used for re-keying can be deferred, it doesn't
need to be deterministic.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/sched/sch_sfq.c	2008-01-17 08:29:24.000000000 -0800
+++ b/net/sched/sch_sfq.c	2008-01-17 09:00:58.000000000 -0800
@@ -426,7 +426,9 @@ static int sfq_init(struct Qdisc *sch, s
 	struct sfq_sched_data *q = qdisc_priv(sch);
 	int i;
 
-	setup_timer(&q->perturb_timer, sfq_perturbation, (unsigned long)sch);
+	q->perturb_timer.function = sfq_perturbation;
+	q->perturb_timer.data = (unsigned long)sch;;
+	init_timer_deferrable(&q->perturb_timer);
 
 	for (i=0; i<SFQ_HASH_DIVISOR; i++)
 		q->ht[i] = SFQ_DEPTH;