Skip to content

Commit

Permalink
Merge branch 'net-sched-taprio-fix-picos_per_byte-miscalculation'
Browse files Browse the repository at this point in the history 
Leandro Dorileo says:

====================
net/sched: taprio: fix picos_per_byte miscalculation

This set fixes miscalculations based on invalid link speed values.

Changes in v6:
 + Avoid locking a spinlock while calling __ethtool_get_link_ksettings()
   (suggested by: Cong Wang);

Changes in v5:
 + Don't iterate over all the net_device maintained list (suggested by: Florian Fainelli);

Changes in v4:
 + converted pr_info calls to netdev_dbg (suggested by: Florian Fainelli);

Changes in v3:
 + yet pr_info() format warnings;

Changes in v2:
 + fixed pr_info() format both on cbs and taprio patches;
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Apr 11, 2019
2 parents 93e2125 + e0a7683 commit b8c7e2c
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 30 deletions.
98 changes: 84 additions & 14 deletions net/sched/sch_cbs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,20 @@
#include <linux/string.h>
#include <linux/errno.h>
#include <linux/skbuff.h>
#include <net/netevent.h>
#include <net/netlink.h>
#include <net/sch_generic.h>
#include <net/pkt_sched.h>

static LIST_HEAD(cbs_list);
static DEFINE_SPINLOCK(cbs_list_lock);

#define BYTES_PER_KBIT (1000LL / 8)

struct cbs_sched_data {
bool offload;
int queue;
s64 port_rate; /* in bytes/s */
atomic64_t port_rate; /* in bytes/s */
s64 last; /* timestamp in ns */
s64 credits; /* in bytes */
s32 locredit; /* in bytes */
Expand All @@ -82,6 +86,7 @@ struct cbs_sched_data {
struct sk_buff **to_free);
struct sk_buff *(*dequeue)(struct Qdisc *sch);
struct Qdisc *qdisc;
struct list_head cbs_list;
};

static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch,
Expand Down Expand Up @@ -181,6 +186,11 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
s64 credits;
int len;

if (atomic64_read(&q->port_rate) == -1) {
WARN_ONCE(1, "cbs: dequeue() called with unknown port rate.");
return NULL;
}

if (q->credits < 0) {
credits = timediff_to_credits(now - q->last, q->idleslope);

Expand All @@ -207,7 +217,8 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
/* As sendslope is a negative number, this will decrease the
* amount of q->credits.
*/
credits = credits_from_len(len, q->sendslope, q->port_rate);
credits = credits_from_len(len, q->sendslope,
atomic64_read(&q->port_rate));
credits += q->credits;

q->credits = max_t(s64, credits, q->locredit);
Expand Down Expand Up @@ -294,6 +305,50 @@ static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q,
return 0;
}

static void cbs_set_port_rate(struct net_device *dev, struct cbs_sched_data *q)
{
struct ethtool_link_ksettings ecmd;
int port_rate = -1;

if (!__ethtool_get_link_ksettings(dev, &ecmd) &&
ecmd.base.speed != SPEED_UNKNOWN)
port_rate = ecmd.base.speed * 1000 * BYTES_PER_KBIT;

atomic64_set(&q->port_rate, port_rate);
netdev_dbg(dev, "cbs: set %s's port_rate to: %lld, linkspeed: %d\n",
dev->name, (long long)atomic64_read(&q->port_rate),
ecmd.base.speed);
}

static int cbs_dev_notifier(struct notifier_block *nb, unsigned long event,
void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct cbs_sched_data *q;
struct net_device *qdev;
bool found = false;

ASSERT_RTNL();

if (event != NETDEV_UP && event != NETDEV_CHANGE)
return NOTIFY_DONE;

spin_lock(&cbs_list_lock);
list_for_each_entry(q, &cbs_list, cbs_list) {
qdev = qdisc_dev(q->qdisc);
if (qdev == dev) {
found = true;
break;
}
}
spin_unlock(&cbs_list_lock);

if (found)
cbs_set_port_rate(dev, q);

return NOTIFY_DONE;
}

static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack)
{
Expand All @@ -315,16 +370,7 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
qopt = nla_data(tb[TCA_CBS_PARMS]);

if (!qopt->offload) {
struct ethtool_link_ksettings ecmd;
s64 link_speed;

if (!__ethtool_get_link_ksettings(dev, &ecmd))
link_speed = ecmd.base.speed;
else
link_speed = SPEED_1000;

q->port_rate = link_speed * 1000 * BYTES_PER_KBIT;

cbs_set_port_rate(dev, q);
cbs_disable_offload(dev, q);
} else {
err = cbs_enable_offload(dev, q, qopt, extack);
Expand All @@ -347,6 +393,7 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt,
{
struct cbs_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);
int err;

if (!opt) {
NL_SET_ERR_MSG(extack, "Missing CBS qdisc options which are mandatory");
Expand All @@ -367,16 +414,29 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt,

qdisc_watchdog_init(&q->watchdog, sch);

return cbs_change(sch, opt, extack);
err = cbs_change(sch, opt, extack);
if (err)
return err;

if (!q->offload) {
spin_lock(&cbs_list_lock);
list_add(&q->cbs_list, &cbs_list);
spin_unlock(&cbs_list_lock);
}

return 0;
}

static void cbs_destroy(struct Qdisc *sch)
{
struct cbs_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);

qdisc_watchdog_cancel(&q->watchdog);
spin_lock(&cbs_list_lock);
list_del(&q->cbs_list);
spin_unlock(&cbs_list_lock);

qdisc_watchdog_cancel(&q->watchdog);
cbs_disable_offload(dev, q);

if (q->qdisc)
Expand Down Expand Up @@ -487,14 +547,24 @@ static struct Qdisc_ops cbs_qdisc_ops __read_mostly = {
.owner = THIS_MODULE,
};

static struct notifier_block cbs_device_notifier = {
.notifier_call = cbs_dev_notifier,
};

static int __init cbs_module_init(void)
{
int err = register_netdevice_notifier(&cbs_device_notifier);

if (err)
return err;

return register_qdisc(&cbs_qdisc_ops);
}

static void __exit cbs_module_exit(void)
{
unregister_qdisc(&cbs_qdisc_ops);
unregister_netdevice_notifier(&cbs_device_notifier);
}
module_init(cbs_module_init)
module_exit(cbs_module_exit)
Expand Down
97 changes: 81 additions & 16 deletions net/sched/sch_taprio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
#include <net/pkt_cls.h>
#include <net/sch_generic.h>

static LIST_HEAD(taprio_list);
static DEFINE_SPINLOCK(taprio_list_lock);

#define TAPRIO_ALL_GATES_OPEN -1

struct sched_entry {
Expand All @@ -42,9 +45,9 @@ struct taprio_sched {
struct Qdisc *root;
s64 base_time;
int clockid;
int picos_per_byte; /* Using picoseconds because for 10Gbps+
* speeds it's sub-nanoseconds per byte
*/
atomic64_t picos_per_byte; /* Using picoseconds because for 10Gbps+
* speeds it's sub-nanoseconds per byte
*/
size_t num_entries;

/* Protects the update side of the RCU protected current_entry */
Expand All @@ -53,6 +56,7 @@ struct taprio_sched {
struct list_head entries;
ktime_t (*get_time)(void);
struct hrtimer advance_timer;
struct list_head taprio_list;
};

static int taprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
Expand Down Expand Up @@ -117,7 +121,7 @@ static struct sk_buff *taprio_peek(struct Qdisc *sch)

static inline int length_to_duration(struct taprio_sched *q, int len)
{
return (len * q->picos_per_byte) / 1000;
return (len * atomic64_read(&q->picos_per_byte)) / 1000;
}

static struct sk_buff *taprio_dequeue(struct Qdisc *sch)
Expand All @@ -129,6 +133,11 @@ static struct sk_buff *taprio_dequeue(struct Qdisc *sch)
u32 gate_mask;
int i;

if (atomic64_read(&q->picos_per_byte) == -1) {
WARN_ONCE(1, "taprio: dequeue() called with unknown picos per byte.");
return NULL;
}

rcu_read_lock();
entry = rcu_dereference(q->current_entry);
/* if there's no entry, it means that the schedule didn't
Expand Down Expand Up @@ -233,7 +242,7 @@ static enum hrtimer_restart advance_sched(struct hrtimer *timer)

next->close_time = close_time;
atomic_set(&next->budget,
(next->interval * 1000) / q->picos_per_byte);
(next->interval * 1000) / atomic64_read(&q->picos_per_byte));

first_run:
rcu_assign_pointer(q->current_entry, next);
Expand Down Expand Up @@ -567,24 +576,69 @@ static void taprio_start_sched(struct Qdisc *sch, ktime_t start)

first->close_time = ktime_add_ns(start, first->interval);
atomic_set(&first->budget,
(first->interval * 1000) / q->picos_per_byte);
(first->interval * 1000) /
atomic64_read(&q->picos_per_byte));
rcu_assign_pointer(q->current_entry, NULL);

spin_unlock_irqrestore(&q->current_entry_lock, flags);

hrtimer_start(&q->advance_timer, start, HRTIMER_MODE_ABS);
}

static void taprio_set_picos_per_byte(struct net_device *dev,
struct taprio_sched *q)
{
struct ethtool_link_ksettings ecmd;
int picos_per_byte = -1;

if (!__ethtool_get_link_ksettings(dev, &ecmd) &&
ecmd.base.speed != SPEED_UNKNOWN)
picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
ecmd.base.speed * 1000 * 1000);

atomic64_set(&q->picos_per_byte, picos_per_byte);
netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
dev->name, (long long)atomic64_read(&q->picos_per_byte),
ecmd.base.speed);
}

static int taprio_dev_notifier(struct notifier_block *nb, unsigned long event,
void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct net_device *qdev;
struct taprio_sched *q;
bool found = false;

ASSERT_RTNL();

if (event != NETDEV_UP && event != NETDEV_CHANGE)
return NOTIFY_DONE;

spin_lock(&taprio_list_lock);
list_for_each_entry(q, &taprio_list, taprio_list) {
qdev = qdisc_dev(q->root);
if (qdev == dev) {
found = true;
break;
}
}
spin_unlock(&taprio_list_lock);

if (found)
taprio_set_picos_per_byte(dev, q);

return NOTIFY_DONE;
}

static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack)
{
struct nlattr *tb[TCA_TAPRIO_ATTR_MAX + 1] = { };
struct taprio_sched *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);
struct tc_mqprio_qopt *mqprio = NULL;
struct ethtool_link_ksettings ecmd;
int i, err, size;
s64 link_speed;
ktime_t start;

err = nla_parse_nested(tb, TCA_TAPRIO_ATTR_MAX, opt,
Expand Down Expand Up @@ -657,14 +711,7 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
mqprio->prio_tc_map[i]);
}

if (!__ethtool_get_link_ksettings(dev, &ecmd))
link_speed = ecmd.base.speed;
else
link_speed = SPEED_1000;

q->picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
link_speed * 1000 * 1000);

taprio_set_picos_per_byte(dev, q);
start = taprio_get_start_time(sch);
if (!start)
return 0;
Expand All @@ -681,6 +728,10 @@ static void taprio_destroy(struct Qdisc *sch)
struct sched_entry *entry, *n;
unsigned int i;

spin_lock(&taprio_list_lock);
list_del(&q->taprio_list);
spin_unlock(&taprio_list_lock);

hrtimer_cancel(&q->advance_timer);

if (q->qdiscs) {
Expand Down Expand Up @@ -735,6 +786,10 @@ static int taprio_init(struct Qdisc *sch, struct nlattr *opt,
if (!opt)
return -EINVAL;

spin_lock(&taprio_list_lock);
list_add(&q->taprio_list, &taprio_list);
spin_unlock(&taprio_list_lock);

return taprio_change(sch, opt, extack);
}

Expand Down Expand Up @@ -947,14 +1002,24 @@ static struct Qdisc_ops taprio_qdisc_ops __read_mostly = {
.owner = THIS_MODULE,
};

static struct notifier_block taprio_device_notifier = {
.notifier_call = taprio_dev_notifier,
};

static int __init taprio_module_init(void)
{
int err = register_netdevice_notifier(&taprio_device_notifier);

if (err)
return err;

return register_qdisc(&taprio_qdisc_ops);
}

static void __exit taprio_module_exit(void)
{
unregister_qdisc(&taprio_qdisc_ops);
unregister_netdevice_notifier(&taprio_device_notifier);
}

module_init(taprio_module_init);
Expand Down

0 comments on commit b8c7e2c

Please sign in to comment.