Re: [RFC net-next 2/2] ipv6: ioam: Support for Buffer occupancy data field

[Justin Iurman <justin.iurman@uliege.be>]

On Dec 21, 2021, at 9:13 PM, Jakub Kicinski kuba@kernel.org wrote:
> On Tue, 21 Dec 2021 19:23:37 +0200 Vladimir Oltean wrote:
>> On Tue, Dec 21, 2021 at 06:06:39PM +0100, Justin Iurman wrote:
>> > On Dec 10, 2021, at 1:38 AM, Jakub Kicinski kuba@kernel.org wrote:
>> > > I think we're on the same page, the main problem is I've not seen
>> > > anyone use the skbuff_head_cache occupancy as a signal in practice.
>> > > 
>> > > I'm adding a bunch of people to the CC list, hopefully someone has
>> > > an opinion one way or the other.
>> > 
>> > It looks like we won't have more opinions on that, unfortunately.
>> > 
>> > @Jakub - Should I submit it as a PATCH and see if we receive more
>> > feedback there?
>> 
>> I know nothing about OAM and therefore did not want to comment, but I
>> think the point raised about the metric you propose being irrelevant in
>> the context of offloaded data paths is quite important. The "devlink-sb"
>> proposal was dismissed very quickly on grounds of requiring sleepable
>> context, is that a deal breaker, and if it is, why? Not only offloaded
>> interfaces like switches/routers can report buffer occupancy. Plain NICs
>> also have buffer pools, DMA RX/TX rings, MAC FIFOs, etc, that could
>> indicate congestion or otherwise high load. Maybe slab information could
>> be relevant, for lack of a better option, on virtual interfaces, but if
>> they're physical, why limit ourselves on reporting that? The IETF draft
>> you present says "This field indicates the current status of the
>> occupancy of the common buffer pool used by a set of queues." It appears
>> to me that we could try to get a reporting that has better granularity
>> (per interface, per queue) than just something based on
>> skbuff_head_cache. What if someone will need that finer granularity in
>> the future.
> 
> Indeed.
> 
> In my experience finding meaningful metrics is heard, the chances that
> something that seems useful on the surface actually provides meaningful
> signal in deployments is a lot lower than one may expect. And the

True.

> commit message reads as if the objective was checking a box in the
> implemented IOAM metrics, rather exporting relevant information.

Indeed, but not only. I sent this patchset as a Request for Comments to
see if it was correct and relevant. I mean, if there is no consensus on
this, I'll keep this data field as not supported, not a big deal. But
it would obviously be good to have it at some point (as long as what we
retrieve makes sense enough, and for all cases).

> We can do a roll call on people CCed but I read their silence as nobody

I thought that silence means consent. That's why more opinions would be
welcome, even if we seem to converge. Not only for opinions, but also
for any idea or guidance for a better solution, if any.

> thinks this metric is useful. Is there any experimental data you can
> point to which proves the signal strength?

Apart from the fact that I monitored the metric during normal and
high-load situations, honestly no. Values made sense during
those tests, though.