zebra: backpressure - Zebra push back on Buffer/Stream creation

[raja-rajasekar]

Currently, the way zebra works is it creates pthread per client (BGP is of interest in this case) and this thread loops itself in zserv_read() to check for any incoming data. If there is one, then it reads, validates and adds it in the ibuf_fifo signalling the main thread to process the message. The main thread when it gets a change, processes the message, and invokes the function pointer registered in the header command. (Ex: zserv_handlers).

Finally, if all of this was successful, this task reschedules itself and loops in zserv_read() again

However, if there are already items on the ibuf FIFO, that means zebra is slow in processing. And with the current mechanism if Zebra main is busy, the ibuf FIFO keeps growing holding up the memory.

Show memory zebra:(Example: 15k streams hoarding ~160 MB of data)
--- qmem libfrr ---
Stream : 44 variable 3432352 15042 161243800
Stream FIFO : 45 72 3240 48 3456

Fix:

• Stop doing the read events when we know there are X number of items on the FIFO already.(X - zebra zapi-packets <1-10000> (Default-1000)

• In zserv_read(), determine the number of items on the zserv->ibuf_fifo. Subtract this from the work items and only pull the number of items off that would take us to X items on the ibuf_fifo again.

• If the number of items in the ibuf_fifo has reached to the maximum * Either initially when zserv_read() is called (or) * when processing the remainders of the incoming buffer the zserv_read which runs in client’s pthread expects the zebra main to schedule a wake up in zserv_process_message after the processing of all the items on the buffer.

NOTE

These code changes are on the server(Zebra) side.
The code changes from the client (BGP) will be raised as a separate review.

Ticket: #3390099

[ton31337]

Is this related to #15390?

[donaldsharp]

It is insofar as that 15390 has changes for both zebra and bgp. I believe we are running into some internal discussions in regards to the bgp changes and these zebra changes are independent of the bgp changes. So the goal is to get these in while we continue to work on the bgp changes. These changes are independent enough that all it would do is add more data to the buffer side of bgp at this point in time. This is in opposition to the backup being spread between bgp and zebra currently.

[raja-rajasekar]

Testing/observation: Testing.pdf

[ton31337]

Looking at testing.pdf, the question comes to my head is: why the heap is increasing with the code change while the streams size remains OK?

[raja-rajasekar]

Looking at testing.pdf, the question comes to my head is: why the heap is increasing with the code change while the streams size remains OK?

The Idea in the testing.pdf is to compare Streams of "initial state" and "after trigger" for baseline and with code changes seperately. The heap is more in one case than the other because the topology used is diferent for with/without code changes. Heap increases because the dplane_ctx and other modules taking extra memory. But the increase before and after trigger is ~200-250MB.

Apologies for the confusion. Here is the new test results where the setup is same for baseline and with code changes and complete show memory output is provided.
New_Testing.pdf

[ton31337]

LGTM, but I'll let others look at this also. /cc @mjstapp

[riw777]

looks good

[raja-rajasekar]

Final Test Results @
Latest_Testing.pdf

[mjstapp]

hmm, gosh, do you really think the cli changes are necessary? I don't really think we need to expose the inner workings of the module.

[raja-rajasekar]

Final test results @
Final_Testing.pdf

[mjstapp]

Thanks - looks good to me now