Re: [RFC net] Fixes: b63c5478e9cb ("ipv6: ioam: Support for Queue depth data field")

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/8/22 01:04, Jakub Kicinski wrote:
On Wed, 7 Dec 2022 13:07:18 +0100 Justin Iurman wrote:
Can you say more about the use? What signal do you derive from it?
I do track qlen on Meta's servers but haven't found a strong use
for it yet (I did for backlog drops but not the qlen itself).

The specification goal of the queue depth was initially to be able to
track the entire path with a detailed view for packets or flows (kind of
a zoom on the interface to have details about its queues). With the
current definition/implementation of the queue depth, if only one queue
is congested, you're able to know it. Which doesn't necessarily mean
that all queues are full, but this one is and there might be something
going on. And this is something operators might want to be able to
detect precisely, for a lot of use cases depending on the situation. On
the contrary, if all queues are full, then you could deduce that as well
for each queue separately, as soon as a packet is assigned to it. So I
think that with "queue depth = sum(queues)", you don't have details and
you're not able to detect a single queue congestion, while with "queue
depth = queue" you could detect both. One might argue that it's fine to
only have the aggregation in some situation. I'd say that we might need
both, actually. Which is technically possible (even though expensive, as
Eric mentioned) thanks to the way it is specified by the RFC, where some
freedom was intentionally given. I could come up with a solution for that.

Understood. My hope was that by now there was some in-field experience
which could help us judge how much signal can one derive from a single
queue. Or a user that could attest.

I asked the people concerned. I'll get back to you.

Because it measures the length of a single queue not the device.

Yep, I figured that out after the off-list discussion we've had with Eric.

So my plan would be, if you all agree with, to correct and repost this
patch to fix the NULL qdisc issue. Then, I'd come with a solution to
allow both (with and without aggregation of queues) and post it on
net-next. But again, if the consensus is to revert this patch (which I
think would bring no benefit IMHO), then so be it. Thoughts?

To summarize - we have reservations about correctness and about

Ack. And this is exactly why I proposed the alternative of having both solutions implemented (i.e., a specific queue depth or an aggregation of queues per interface).

breaking layering (ip6 calling down to net/sched).

Indeed. Funny enough, this one's going to be hard to beat. How frustrating when the value to be retrieved is there, but cannot be used. Especially when it's probably one of the most important metric from IOAM. I guess it's the price to pay when dealing with telemetry over multiple layers. Anyway...

You can stick to your approach, respost and see if any of the other
maintainer is willing to pick this up (i.e. missed this nack).
If you ask for my option I'll side with Eric
Got it, thanks.



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux