[ovs-dev] [PATCH] bridge: Log a warning when QoS misconfiguration is likely.

[Ben Pfaff]

I read hfsc_classify() in net/sched/sch_hfsc.c as first trying to find
the correct class for a packet, then looking for a default class if
there is none, then returning NULL if there is no default class.  That
function's caller in hfsc_enqueue() does a kfree_skb() and returns an
error if hfsc_classify() returns NULL.  So it looks to me like having
no default queue (class) causes packet drops, am I missing anything?

On Wed, Jul 20, 2011 at 11:36:57AM -0700, Ethan Jackson wrote:
> I think the idea of warning is a good one.  Last time I tried (months
> ago), if you don't configure a default queue on the QoS types
> supported by OVS currently (hfsc/htb), the behavior is not to drop
> packets.  Instead it simply egresses the link shrinking the available
> bandwidth for the properly queued traffic.  This can cause extremely
> strange QoS behavior, but not packet drops.
> 
> Ethan
> 
> On Wed, Jul 20, 2011 at 10:40, Ben Pfaff <blp at nicira.com> wrote:
> > Queue 0 is documented as the "default queue" used when a packet is not
> > directed to any specific queue. ?Many qdiscs drop packets not directed to a
> > queue if the default queue is not configured. ?This is therefore likely to
> > be a misconfiguration, so warn about it.
> > ---
> > ?vswitchd/bridge.c | ? 13 +++++++++++++
> > ?1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index 449957a..fcd7a78 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -2474,6 +2474,7 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
> > ? ? } else {
> > ? ? ? ? struct iface_delete_queues_cbdata cbdata;
> > ? ? ? ? struct shash details;
> > + ? ? ? ?bool queue_zero;
> > ? ? ? ? size_t i;
> >
> > ? ? ? ? /* Configure top-level Qos for 'iface'. */
> > @@ -2489,16 +2490,28 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
> > ? ? ? ? netdev_dump_queues(iface->netdev, iface_delete_queues, &cbdata);
> >
> > ? ? ? ? /* Configure queues for 'iface'. */
> > + ? ? ? ?queue_zero = false;
> > ? ? ? ? for (i = 0; i < qos->n_queues; i++) {
> > ? ? ? ? ? ? const struct ovsrec_queue *queue = qos->value_queues[i];
> > ? ? ? ? ? ? unsigned int queue_id = qos->key_queues[i];
> >
> > + ? ? ? ? ? ?if (queue_id == 0) {
> > + ? ? ? ? ? ? ? ?queue_zero = true;
> > + ? ? ? ? ? ?}
> > +
> > ? ? ? ? ? ? shash_from_ovs_idl_map(queue->key_other_config,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?queue->value_other_config,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?queue->n_other_config, &details);
> > ? ? ? ? ? ? netdev_set_queue(iface->netdev, queue_id, &details);
> > ? ? ? ? ? ? shash_destroy(&details);
> > ? ? ? ? }
> > + ? ? ? ?if (!queue_zero) {
> > + ? ? ? ? ? ?static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > + ? ? ? ? ? ?VLOG_WARN_RL(&rl, "interface %s: QoS configured without a default "
> > + ? ? ? ? ? ? ? ? ? ? ? ? "queue (queue 0). ?Packets not directed to a "
> > + ? ? ? ? ? ? ? ? ? ? ? ? "correctly configured queue may be dropped.",
> > + ? ? ? ? ? ? ? ? ? ? ? ? iface->name);
> > + ? ? ? ?}
> > ? ? }
> >
> > ? ? netdev_set_policing(iface->netdev,
> > --
> > 1.7.4.4
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >