net: sched: taprio: Refactor taprio_get_start_time()
authorAndre Guedes <andre.guedes@intel.com>
Tue, 23 Apr 2019 19:44:21 +0000 (12:44 -0700)
committerDavid S. Miller <davem@davemloft.net>
Wed, 24 Apr 2019 02:52:32 +0000 (19:52 -0700)
This patch does a code refactoring to taprio_get_start_time() function
to improve readability and report error properly.

If 'base' time is later than 'now', the start time is equal to 'base'
and taprio_get_start_time() is done. That's the natural case so we move
that code to the beginning of the function. Also, if 'cycle' calculation
is zero, something went really wrong with taprio and we should log that
internal error properly.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/sched/sch_taprio.c

index d91a7ec673486a4807d9f71a48b429288894b2d1..d0aae7b5e6081e2467b5af47edcd4511b02fba6e 100644 (file)
@@ -539,7 +539,7 @@ static int taprio_parse_mqprio_opt(struct net_device *dev,
        return 0;
 }
 
-static ktime_t taprio_get_start_time(struct Qdisc *sch)
+static int taprio_get_start_time(struct Qdisc *sch, ktime_t *start)
 {
        struct taprio_sched *q = qdisc_priv(sch);
        struct sched_entry *entry;
@@ -547,27 +547,33 @@ static ktime_t taprio_get_start_time(struct Qdisc *sch)
        s64 n;
 
        base = ns_to_ktime(q->base_time);
-       cycle = 0;
+       now = q->get_time();
+
+       if (ktime_after(base, now)) {
+               *start = base;
+               return 0;
+       }
 
        /* Calculate the cycle_time, by summing all the intervals.
         */
+       cycle = 0;
        list_for_each_entry(entry, &q->entries, list)
                cycle = ktime_add_ns(cycle, entry->interval);
 
-       if (!cycle)
-               return base;
-
-       now = q->get_time();
-
-       if (ktime_after(base, now))
-               return base;
+       /* The qdisc is expected to have at least one sched_entry.  Moreover,
+        * any entry must have 'interval' > 0. Thus if the cycle time is zero,
+        * something went really wrong. In that case, we should warn about this
+        * inconsistent state and return error.
+        */
+       if (WARN_ON(!cycle))
+               return -EFAULT;
 
        /* Schedule the start time for the beginning of the next
         * cycle.
         */
        n = div64_s64(ktime_sub_ns(now, base), cycle);
-
-       return ktime_add_ns(base, (n + 1) * cycle);
+       *start = ktime_add_ns(base, (n + 1) * cycle);
+       return 0;
 }
 
 static void taprio_start_sched(struct Qdisc *sch, ktime_t start)
@@ -716,9 +722,12 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
        }
 
        taprio_set_picos_per_byte(dev, q);
-       start = taprio_get_start_time(sch);
-       if (!start)
-               return 0;
+
+       err = taprio_get_start_time(sch, &start);
+       if (err < 0) {
+               NL_SET_ERR_MSG(extack, "Internal error: failed get start time");
+               return err;
+       }
 
        taprio_start_sched(sch, start);