Skip to content

Commit

Permalink
Merge branch 'qdisc-visibility'
Browse files Browse the repository at this point in the history
Jakub Kicinski says:

====================
net: sched: update default qdisc visibility after Tx queue cnt changes

Matthew noticed that number of children reported by mq does not match
number of queues on reconfigured interfaces. For example if mq is
instantiated when there is 8 queues it will always show 8 children,
regardless of config being changed:

 # ethtool -L eth0 combined 8
 # tc qdisc replace dev eth0 root handle 100: mq
 # tc qdisc show dev eth0
 qdisc mq 100: root
 qdisc pfifo_fast 0: parent 100:8 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:7 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:6 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:5 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:4 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:3 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:2 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:1 bands 3 priomap 1 2 ...
 # ethtool -L eth0 combined 1
 # tc qdisc show dev eth0
 qdisc mq 100: root
 qdisc pfifo_fast 0: parent 100:8 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:7 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:6 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:5 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:4 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:3 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:2 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:1 bands 3 priomap 1 2 ...
 # ethtool -L eth0 combined 32
 # tc qdisc show dev eth0
 qdisc mq 100: root
 qdisc pfifo_fast 0: parent 100:8 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:7 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:6 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:5 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:4 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:3 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:2 bands 3 priomap 1 2 ...
 qdisc pfifo_fast 0: parent 100:1 bands 3 priomap 1 2 ...

This patchset fixes this by hashing and unhasing the default
child qdiscs as number of queues gets adjusted.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Sep 15, 2021
2 parents c506cc5 + 2d6a589 commit dc50b93
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 1 deletion.
28 changes: 28 additions & 0 deletions drivers/net/netdevsim/ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,30 @@ static int nsim_set_ringparam(struct net_device *dev,
return 0;
}

static void
nsim_get_channels(struct net_device *dev, struct ethtool_channels *ch)
{
struct netdevsim *ns = netdev_priv(dev);

ch->max_combined = ns->nsim_bus_dev->num_queues;
ch->combined_count = ns->ethtool.channels;
}

static int
nsim_set_channels(struct net_device *dev, struct ethtool_channels *ch)
{
struct netdevsim *ns = netdev_priv(dev);
int err;

err = netif_set_real_num_queues(dev, ch->combined_count,
ch->combined_count);
if (err)
return err;

ns->ethtool.channels = ch->combined_count;
return 0;
}

static int
nsim_get_fecparam(struct net_device *dev, struct ethtool_fecparam *fecparam)
{
Expand Down Expand Up @@ -118,6 +142,8 @@ static const struct ethtool_ops nsim_ethtool_ops = {
.get_coalesce = nsim_get_coalesce,
.get_ringparam = nsim_get_ringparam,
.set_ringparam = nsim_set_ringparam,
.get_channels = nsim_get_channels,
.set_channels = nsim_set_channels,
.get_fecparam = nsim_get_fecparam,
.set_fecparam = nsim_set_fecparam,
};
Expand All @@ -141,6 +167,8 @@ void nsim_ethtool_init(struct netdevsim *ns)
ns->ethtool.fec.fec = ETHTOOL_FEC_NONE;
ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;

ns->ethtool.channels = ns->nsim_bus_dev->num_queues;

ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);

debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
Expand Down
1 change: 1 addition & 0 deletions drivers/net/netdevsim/netdevsim.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ struct nsim_ethtool_pauseparam {
struct nsim_ethtool {
u32 get_err;
u32 set_err;
u32 channels;
struct nsim_ethtool_pauseparam pauseparam;
struct ethtool_coalesce coalesce;
struct ethtool_ringparam ring;
Expand Down
4 changes: 4 additions & 0 deletions include/net/sch_generic.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,8 @@ struct Qdisc_ops {
struct netlink_ext_ack *extack);
void (*attach)(struct Qdisc *sch);
int (*change_tx_queue_len)(struct Qdisc *, unsigned int);
void (*change_real_num_tx)(struct Qdisc *sch,
unsigned int new_real_tx);

int (*dump)(struct Qdisc *, struct sk_buff *);
int (*dump_stats)(struct Qdisc *, struct gnet_dump *);
Expand Down Expand Up @@ -684,6 +686,8 @@ void qdisc_class_hash_grow(struct Qdisc *, struct Qdisc_class_hash *);
void qdisc_class_hash_destroy(struct Qdisc_class_hash *);

int dev_qdisc_change_tx_queue_len(struct net_device *dev);
void dev_qdisc_change_real_num_tx(struct net_device *dev,
unsigned int new_real_tx);
void dev_init_scheduler(struct net_device *dev);
void dev_shutdown(struct net_device *dev);
void dev_activate(struct net_device *dev);
Expand Down
2 changes: 2 additions & 0 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2921,6 +2921,8 @@ int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
if (dev->num_tc)
netif_setup_tc(dev, txq);

dev_qdisc_change_real_num_tx(dev, txq);

dev->real_num_tx_queues = txq;

if (disabling) {
Expand Down
9 changes: 9 additions & 0 deletions net/sched/sch_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,15 @@ static int qdisc_change_tx_queue_len(struct net_device *dev,
return 0;
}

void dev_qdisc_change_real_num_tx(struct net_device *dev,
unsigned int new_real_tx)
{
struct Qdisc *qdisc = dev->qdisc;

if (qdisc->ops->change_real_num_tx)
qdisc->ops->change_real_num_tx(qdisc, new_real_tx);
}

int dev_qdisc_change_tx_queue_len(struct net_device *dev)
{
bool up = dev->flags & IFF_UP;
Expand Down
24 changes: 24 additions & 0 deletions net/sched/sch_mq.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,29 @@ static void mq_attach(struct Qdisc *sch)
priv->qdiscs = NULL;
}

static void mq_change_real_num_tx(struct Qdisc *sch, unsigned int new_real_tx)
{
#ifdef CONFIG_NET_SCHED
struct net_device *dev = qdisc_dev(sch);
struct Qdisc *qdisc;
unsigned int i;

for (i = new_real_tx; i < dev->real_num_tx_queues; i++) {
qdisc = netdev_get_tx_queue(dev, i)->qdisc_sleeping;
/* Only update the default qdiscs we created,
* qdiscs with handles are always hashed.
*/
if (qdisc != &noop_qdisc && !qdisc->handle)
qdisc_hash_del(qdisc);
}
for (i = dev->real_num_tx_queues; i < new_real_tx; i++) {
qdisc = netdev_get_tx_queue(dev, i)->qdisc_sleeping;
if (qdisc != &noop_qdisc && !qdisc->handle)
qdisc_hash_add(qdisc, false);
}
#endif
}

static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
{
struct net_device *dev = qdisc_dev(sch);
Expand Down Expand Up @@ -288,6 +311,7 @@ struct Qdisc_ops mq_qdisc_ops __read_mostly = {
.init = mq_init,
.destroy = mq_destroy,
.attach = mq_attach,
.change_real_num_tx = mq_change_real_num_tx,
.dump = mq_dump,
.owner = THIS_MODULE,
};
23 changes: 23 additions & 0 deletions net/sched/sch_mqprio.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,28 @@ static void mqprio_attach(struct Qdisc *sch)
priv->qdiscs = NULL;
}

static void mqprio_change_real_num_tx(struct Qdisc *sch,
unsigned int new_real_tx)
{
struct net_device *dev = qdisc_dev(sch);
struct Qdisc *qdisc;
unsigned int i;

for (i = new_real_tx; i < dev->real_num_tx_queues; i++) {
qdisc = netdev_get_tx_queue(dev, i)->qdisc_sleeping;
/* Only update the default qdiscs we created,
* qdiscs with handles are always hashed.
*/
if (qdisc != &noop_qdisc && !qdisc->handle)
qdisc_hash_del(qdisc);
}
for (i = dev->real_num_tx_queues; i < new_real_tx; i++) {
qdisc = netdev_get_tx_queue(dev, i)->qdisc_sleeping;
if (qdisc != &noop_qdisc && !qdisc->handle)
qdisc_hash_add(qdisc, false);
}
}

static struct netdev_queue *mqprio_queue_get(struct Qdisc *sch,
unsigned long cl)
{
Expand Down Expand Up @@ -623,6 +645,7 @@ static struct Qdisc_ops mqprio_qdisc_ops __read_mostly = {
.init = mqprio_init,
.destroy = mqprio_destroy,
.attach = mqprio_attach,
.change_real_num_tx = mqprio_change_real_num_tx,
.dump = mqprio_dump,
.owner = THIS_MODULE,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function make_netdev {
modprobe netdevsim
fi

echo $NSIM_ID > /sys/bus/netdevsim/new_device
echo $NSIM_ID $@ > /sys/bus/netdevsim/new_device
# get new device name
ls /sys/bus/netdevsim/devices/netdevsim${NSIM_ID}/net/
}
77 changes: 77 additions & 0 deletions tools/testing/selftests/drivers/net/netdevsim/tc-mq-visibility.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0-only

source ethtool-common.sh

set -o pipefail

n_children() {
n=$(tc qdisc show dev $NDEV | grep '^qdisc' | wc -l)
echo $((n - 1))
}

tcq() {
tc qdisc $1 dev $NDEV ${@:2}
}

n_child_assert() {
n=$(n_children)
if [ $n -ne $1 ]; then
echo "ERROR ($root): ${@:2}, expected $1 have $n"
((num_errors++))
else
((num_passes++))
fi
}


for root in mq mqprio; do
NDEV=$(make_netdev 1 4)

opts=
[ $root == "mqprio" ] && opts='hw 0 num_tc 1 map 0 0 0 0 queues 1@0'

tcq add root handle 100: $root $opts
n_child_assert 4 'Init'

# All defaults

for n in 3 2 1 2 3 4 1 4; do
ethtool -L $NDEV combined $n
n_child_assert $n "Change queues to $n while down"
done

ip link set dev $NDEV up

for n in 3 2 1 2 3 4 1 4; do
ethtool -L $NDEV combined $n
n_child_assert $n "Change queues to $n while up"
done

# One real one
tcq replace parent 100:4 handle 204: pfifo_fast
n_child_assert 4 "One real queue"

ethtool -L $NDEV combined 1
n_child_assert 2 "One real queue, one default"

ethtool -L $NDEV combined 4
n_child_assert 4 "One real queue, rest default"

# Graft some
tcq replace parent 100:1 handle 204:
n_child_assert 3 "Grafted"

ethtool -L $NDEV combined 1
n_child_assert 1 "Grafted, one"

cleanup_nsim
done

if [ $num_errors -eq 0 ]; then
echo "PASSED all $((num_passes)) checks"
exit 0
else
echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
exit 1
fi

0 comments on commit dc50b93

Please sign in to comment.