[LEDE-DEV] [PATCH 2/3] ubusd: make `tx_queue` backlog dynamic

[Felix Fietkau]

On 2017-06-07 15:44, Felix Fietkau wrote:
> On 2017-06-07 13:09, Alexandru Ardelean wrote:
>> It's not very often that the tx_queue is used,
>> to store backlog messages to send to a client.
>> 
>> And for most cases, 32 backlog messages seems to be enough.
>> In fact, for most cases, I've seen ~1 entry in the queue
>> being used every now-n-then.
>> 
>> The issue is more visible/present with the `ubus list` command.
>> I've traced the code to ubusd_handle_lookup() in:
>> 
>> ```
>>     if (!attr[UBUS_ATTR_OBJPATH]) {
>>         avl_for_each_element(&path, obj, path)
>>             ubusd_send_obj(cl, ub, obj);
>>         return 0;
>>     }
>> ```
>> The code-path eventually leads to `ubus_msg_send()`.
>> It seems that once the first element is queued, then
>> the condition check for `(!cl->tx_queue[cl->txq_cur])`
>> will queue all messages iterated in the above snippet,
>> without trying any writes.
>> 
>> This can be solved, either by making the queue dynamic
>> and allow it to expand above the current fixed limit (1).
>> 
>> Or, by forcing/allowing writes during the tx_queue-ing (2).
>> 
>> This patch implements (1).
>> 
>> Signed-off-by: Alexandru Ardelean <ardeleanalex at gmail.com>
> If I remember correctly, I chose the backlog array because a message can
> be referenced multiple times (e.g. in the notify/subscribe case).
> This would break in your linked-list implementation if a message needs
> to be queued for multiple clients.
I just re-checked and it seems that multiple references are no longer
used, so this implementation would probably work. I will take a closer look.

- Felix