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

[Ethan Jackson]

I haven't looked at the code, maybe the behavior has changed.  I
definitely remember running into this with HTB at least.  It certainly
didn't drop packets in that case.  I'll try it out experimentally and
get back to you.

Ethan

On Wed, Jul 20, 2011 at 11:42, Ben Pfaff <blp at nicira.com> wrote:
> 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
>> >
>