Skip to content

Commit 8c0cea5

Browse files
tavipdavem330
authored andcommitted
net_sched: sch_sfq: use a temporary work area for validating configuration
Many configuration parameters have influence on others (e.g. divisor -> flows -> limit, depth -> limit) and so it is difficult to correctly do all of the validation before applying the configuration. And if a validation error is detected late it is difficult to roll back a partially applied configuration. To avoid these issues use a temporary work area to update and validate the configuration and only then apply the configuration to the internal state. Signed-off-by: Octavian Purdila <tavip@google.com> Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 7f1ff1b commit 8c0cea5

File tree

1 file changed

+44
-12
lines changed

1 file changed

+44
-12
lines changed

net/sched/sch_sfq.c

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,15 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
631631
struct red_parms *p = NULL;
632632
struct sk_buff *to_free = NULL;
633633
struct sk_buff *tail = NULL;
634+
unsigned int maxflows;
635+
unsigned int quantum;
636+
unsigned int divisor;
637+
int perturb_period;
638+
u8 headdrop;
639+
u8 maxdepth;
640+
int limit;
641+
u8 flags;
642+
634643

635644
if (opt->nla_len < nla_attr_size(sizeof(*ctl)))
636645
return -EINVAL;
@@ -656,36 +665,59 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
656665
NL_SET_ERR_MSG_MOD(extack, "invalid limit");
657666
return -EINVAL;
658667
}
668+
659669
sch_tree_lock(sch);
670+
671+
limit = q->limit;
672+
divisor = q->divisor;
673+
headdrop = q->headdrop;
674+
maxdepth = q->maxdepth;
675+
maxflows = q->maxflows;
676+
perturb_period = q->perturb_period;
677+
quantum = q->quantum;
678+
flags = q->flags;
679+
680+
/* update and validate configuration */
660681
if (ctl->quantum)
661-
q->quantum = ctl->quantum;
662-
WRITE_ONCE(q->perturb_period, ctl->perturb_period * HZ);
682+
quantum = ctl->quantum;
683+
perturb_period = ctl->perturb_period * HZ;
663684
if (ctl->flows)
664-
q->maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
685+
maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
665686
if (ctl->divisor) {
666-
q->divisor = ctl->divisor;
667-
q->maxflows = min_t(u32, q->maxflows, q->divisor);
687+
divisor = ctl->divisor;
688+
maxflows = min_t(u32, maxflows, divisor);
668689
}
669690
if (ctl_v1) {
670691
if (ctl_v1->depth)
671-
q->maxdepth = min_t(u32, ctl_v1->depth, SFQ_MAX_DEPTH);
692+
maxdepth = min_t(u32, ctl_v1->depth, SFQ_MAX_DEPTH);
672693
if (p) {
673-
swap(q->red_parms, p);
674-
red_set_parms(q->red_parms,
694+
red_set_parms(p,
675695
ctl_v1->qth_min, ctl_v1->qth_max,
676696
ctl_v1->Wlog,
677697
ctl_v1->Plog, ctl_v1->Scell_log,
678698
NULL,
679699
ctl_v1->max_P);
680700
}
681-
q->flags = ctl_v1->flags;
682-
q->headdrop = ctl_v1->headdrop;
701+
flags = ctl_v1->flags;
702+
headdrop = ctl_v1->headdrop;
683703
}
684704
if (ctl->limit) {
685-
q->limit = min_t(u32, ctl->limit, q->maxdepth * q->maxflows);
686-
q->maxflows = min_t(u32, q->maxflows, q->limit);
705+
limit = min_t(u32, ctl->limit, maxdepth * maxflows);
706+
maxflows = min_t(u32, maxflows, limit);
687707
}
688708

709+
/* commit configuration */
710+
q->limit = limit;
711+
q->divisor = divisor;
712+
q->headdrop = headdrop;
713+
q->maxdepth = maxdepth;
714+
q->maxflows = maxflows;
715+
WRITE_ONCE(q->perturb_period, perturb_period);
716+
q->quantum = quantum;
717+
q->flags = flags;
718+
if (p)
719+
swap(q->red_parms, p);
720+
689721
qlen = sch->q.qlen;
690722
while (sch->q.qlen > q->limit) {
691723
dropped += sfq_drop(sch, &to_free);

0 commit comments

Comments
 (0)