Commit e0a7683d authored by Leandro Dorileo's avatar Leandro Dorileo Committed by David S. Miller

net/sched: cbs: fix port_rate miscalculation

The Credit Based Shaper heavily depends on link speed to calculate
the scheduling credits, we can't properly calculate the credits if the
device has failed to report the link speed.

In that case we can't dequeue packets assuming a wrong port rate that will
result into an inconsistent credit distribution.

This patch makes sure we fail to dequeue case:

1) __ethtool_get_link_ksettings() reports error or 2) the ethernet driver
failed to set the ksettings' speed value (setting link speed to
SPEED_UNKNOWN).

Additionally we properly re calculate the port rate whenever the link speed
is changed.

Fixes: 3d0bd028 ("net/sched: Add support for HW offloading for CBS")
Signed-off-by: default avatarLeandro Dorileo <leandro.maciel.dorileo@intel.com>
Reviewed-by: default avatarVedang Patel <vedang.patel@intel.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7b9eba7b
...@@ -61,16 +61,20 @@ ...@@ -61,16 +61,20 @@
#include <linux/string.h> #include <linux/string.h>
#include <linux/errno.h> #include <linux/errno.h>
#include <linux/skbuff.h> #include <linux/skbuff.h>
#include <net/netevent.h>
#include <net/netlink.h> #include <net/netlink.h>
#include <net/sch_generic.h> #include <net/sch_generic.h>
#include <net/pkt_sched.h> #include <net/pkt_sched.h>
static LIST_HEAD(cbs_list);
static DEFINE_SPINLOCK(cbs_list_lock);
#define BYTES_PER_KBIT (1000LL / 8) #define BYTES_PER_KBIT (1000LL / 8)
struct cbs_sched_data { struct cbs_sched_data {
bool offload; bool offload;
int queue; int queue;
s64 port_rate; /* in bytes/s */ atomic64_t port_rate; /* in bytes/s */
s64 last; /* timestamp in ns */ s64 last; /* timestamp in ns */
s64 credits; /* in bytes */ s64 credits; /* in bytes */
s32 locredit; /* in bytes */ s32 locredit; /* in bytes */
...@@ -82,6 +86,7 @@ struct cbs_sched_data { ...@@ -82,6 +86,7 @@ struct cbs_sched_data {
struct sk_buff **to_free); struct sk_buff **to_free);
struct sk_buff *(*dequeue)(struct Qdisc *sch); struct sk_buff *(*dequeue)(struct Qdisc *sch);
struct Qdisc *qdisc; struct Qdisc *qdisc;
struct list_head cbs_list;
}; };
static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch, static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch,
...@@ -181,6 +186,11 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch) ...@@ -181,6 +186,11 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
s64 credits; s64 credits;
int len; 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) { if (q->credits < 0) {
credits = timediff_to_credits(now - q->last, q->idleslope); credits = timediff_to_credits(now - q->last, q->idleslope);
...@@ -207,7 +217,8 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch) ...@@ -207,7 +217,8 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
/* As sendslope is a negative number, this will decrease the /* As sendslope is a negative number, this will decrease the
* amount of q->credits. * 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; credits += q->credits;
q->credits = max_t(s64, credits, q->locredit); q->credits = max_t(s64, credits, q->locredit);
...@@ -294,6 +305,50 @@ static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q, ...@@ -294,6 +305,50 @@ static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q,
return 0; 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, static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
...@@ -315,16 +370,7 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt, ...@@ -315,16 +370,7 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
qopt = nla_data(tb[TCA_CBS_PARMS]); qopt = nla_data(tb[TCA_CBS_PARMS]);
if (!qopt->offload) { if (!qopt->offload) {
struct ethtool_link_ksettings ecmd; cbs_set_port_rate(dev, q);
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_disable_offload(dev, q); cbs_disable_offload(dev, q);
} else { } else {
err = cbs_enable_offload(dev, q, qopt, extack); err = cbs_enable_offload(dev, q, qopt, extack);
...@@ -347,6 +393,7 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt, ...@@ -347,6 +393,7 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt,
{ {
struct cbs_sched_data *q = qdisc_priv(sch); struct cbs_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch); struct net_device *dev = qdisc_dev(sch);
int err;
if (!opt) { if (!opt) {
NL_SET_ERR_MSG(extack, "Missing CBS qdisc options which are mandatory"); NL_SET_ERR_MSG(extack, "Missing CBS qdisc options which are mandatory");
...@@ -367,7 +414,17 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt, ...@@ -367,7 +414,17 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt,
qdisc_watchdog_init(&q->watchdog, sch); 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) static void cbs_destroy(struct Qdisc *sch)
...@@ -375,8 +432,11 @@ static void cbs_destroy(struct Qdisc *sch) ...@@ -375,8 +432,11 @@ static void cbs_destroy(struct Qdisc *sch)
struct cbs_sched_data *q = qdisc_priv(sch); struct cbs_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(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); cbs_disable_offload(dev, q);
if (q->qdisc) if (q->qdisc)
...@@ -487,14 +547,24 @@ static struct Qdisc_ops cbs_qdisc_ops __read_mostly = { ...@@ -487,14 +547,24 @@ static struct Qdisc_ops cbs_qdisc_ops __read_mostly = {
.owner = THIS_MODULE, .owner = THIS_MODULE,
}; };
static struct notifier_block cbs_device_notifier = {
.notifier_call = cbs_dev_notifier,
};
static int __init cbs_module_init(void) 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); return register_qdisc(&cbs_qdisc_ops);
} }
static void __exit cbs_module_exit(void) static void __exit cbs_module_exit(void)
{ {
unregister_qdisc(&cbs_qdisc_ops); unregister_qdisc(&cbs_qdisc_ops);
unregister_netdevice_notifier(&cbs_device_notifier);
} }
module_init(cbs_module_init) module_init(cbs_module_init)
module_exit(cbs_module_exit) module_exit(cbs_module_exit)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment