From: "YU, Xiangning" <[email protected]> Date: Tue, 07 Jul 2020 02:08:13 +0800
> Lockless Token Bucket (LTB) is a qdisc implementation that controls the > use of outbound bandwidth on a shared link. With the help of lockless > qdisc, and by decoupling rate limiting and bandwidth sharing, LTB is > designed to scale in the cloud data centers. > > Signed-off-by: Xiangning Yu <[email protected]> I'm very skeptical about having a kthread for each qdisc, that doesn't sound like a good idea at all. Also: > +static inline struct ltb_skb_cb *ltb_skb_cb(const struct sk_buff *skb) No inline functions in foo.c files please. > +static inline s64 get_linkspeed(struct net_device *dev) Likewise. > +static inline int ltb_drain(struct ltb_class *cl) > +{ > + typeof(&cl->drain_queue) queue; Use the actual type not typeof(). > + struct sk_buff *skb; > + int npkts, bytes; > + unsigned long now = NOW(); > + int cpu; > + struct ltb_sched *ltb = qdisc_priv(cl->root_qdisc); > + struct ltb_pcpu_sched *pcpu_q; > + s64 timestamp; > + bool need_watchdog = false; > + struct cpumask cpumask; Please order local variables in reverse christmas tree order. > +static void ltb_aggregate(struct ltb_class *cl) > +{ > + s64 timestamp = ktime_get_ns(); > + struct ltb_sched *ltb = qdisc_priv(cl->root_qdisc); > + int num_cpus = ltb->num_cpus; > + int i; Likewise. > +static inline void ltb_fanout(struct ltb_sched *ltb) > +{ No inline please. > +/* How many classes within the same group want more bandwidth */ > +static inline int bw_class_want_more_count(struct list_head *head) > +{ > + int n = 0; > + struct ltb_class *cl; No inline, and reverse christmas tree ordering for local variables. > +/* Redistribute bandwidth among classes with the same priority */ > +static int bw_redistribute_prio(struct list_head *lhead, int bw_available, > + int n, bool *all_reached_ceil) > +{ > + struct ltb_class *cl; > + int avg = 0; > + int orig_bw_allocated; > + int safe_loop = 0; > + Likewise. > +static int bw_redistribute(struct ltb_sched *ltb, int bw_available) > +{ > + int prio = 0; > + int n; > + int highest_non_saturated_prio = TC_LTB_NUMPRIO; > + bool all_reached_ceil; Likewise. > +static void bw_balance(struct ltb_sched *ltb) > +{ > + struct ltb_class *cl; > + s64 link_speed = ltb->link_speed; > + int bw_available = link_speed; > + s64 total = 0; > + int high = TC_LTB_NUMPRIO; > + int is_light_traffic = 1; > + int i; Likewise. And so on and so forth. This code needs a lot of style fixes and removal of the per-qdisc kthread design. Thank you.
