On 01/19/2018 03:09 PM, Cong Wang wrote:
> Introduce a new qdisc ops ->change_tx_queue_len() so that
> each qdisc could decide how to implement this if it wants.
> Previously we simply read dev->tx_queue_len, after pfifo_fast
> switches to skb array, we need this API to resize the skb array
> when we change dev->tx_queue_len.
>
> To avoid handling race conditions with TX BH, we need to
> deactivate all TX queues before change the value and bring them
> back after we are done, this also makes implementation easier.
>
> Cc: John Fastabend <john.fastab...@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
> ---
> include/net/sch_generic.h | 2 ++
> net/core/dev.c | 1 +
> net/sched/sch_generic.c | 22 ++++++++++++++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index cd1be1f25c36..aae1baa1c30f 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -200,6 +200,7 @@ struct Qdisc_ops {
> struct nlattr *arg,
> struct netlink_ext_ack *extack);
> void (*attach)(struct Qdisc *sch);
> + void (*change_tx_queue_len)(struct Qdisc *, unsigned
> int);
>
> int (*dump)(struct Qdisc *, struct sk_buff *);
> int (*dump_stats)(struct Qdisc *, struct gnet_dump
> *);
> @@ -488,6 +489,7 @@ void qdisc_class_hash_remove(struct Qdisc_class_hash *,
> void qdisc_class_hash_grow(struct Qdisc *, struct Qdisc_class_hash *);
> void qdisc_class_hash_destroy(struct Qdisc_class_hash *);
>
> +void dev_qdisc_change_tx_queue_len(struct net_device *dev);
> void dev_init_scheduler(struct net_device *dev);
> void dev_shutdown(struct net_device *dev);
> void dev_activate(struct net_device *dev);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 99d353e4cbb2..24809d858a64 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7058,6 +7058,7 @@ int dev_change_tx_queue_len(struct net_device *dev,
> unsigned long new_len)
> dev->tx_queue_len = orig_len;
> return res;
> }
> + dev_qdisc_change_tx_queue_len(dev);
> }
>
> return 0;
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index ef8b4ecde2ac..30aaeb3c1bf1 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -1178,6 +1178,28 @@ void dev_deactivate(struct net_device *dev)
> }
> EXPORT_SYMBOL(dev_deactivate);
>
> +static void qdisc_change_tx_queue_len(struct net_device *dev,
> + struct netdev_queue *dev_queue,
> + void *unused)
> +{
> + struct Qdisc *qdisc = dev_queue->qdisc_sleeping;
> + const struct Qdisc_ops *ops = qdisc->ops;
> +
> + if (ops->change_tx_queue_len)
> + ops->change_tx_queue_len(qdisc, dev->tx_queue_len);
hmm what happens if the resize fails in the next patch,
>
> +static void pfifo_fast_change_tx_queue_len(struct Qdisc *sch, unsigned int
> new_len)
> +{
> + struct pfifo_fast_priv *priv = qdisc_priv(sch);
> + int prio;
> +
> + for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
> + struct skb_array *q = band2list(priv, prio);
> +
> + skb_array_resize(q, new_len, GFP_KERNEL);
> + }
> +}
> +
Here skb_array_resize() can fail with ENOMEM, do we need to unwind the
change and push the error up the stack?
Thanks,
John
