netipmid consumes much CPU when obmc-console socket is shutdown

[Ed Tanous]

On Tue, Jan 4, 2022 at 6:31 PM Heyi Guo <guoheyi at linux.alibaba.com> wrote:
>
> Hi all,
>
> We found netipmid will consumes much CPU when SOL is activated but
> obmc-console socket is shutdown by some reason (can simply shutdown
> obmc-console by systemctl stop ....).
>
> After obmc-console socket is closed, the async_wait() in
> startHostConsole() is always triggered, and consoleInputHandler() will
> read empty data (readSize == 0 and readDataLen == 0), but all the ec
> condition check will NOT hit!
>
>  From boost reference, it is said the function read_some() will:
>
> The function call will block until one or more bytes of data has been
> read successfully, or until an error occurs.
>
> Is it a bug of boost? Or is there anything wrong in ipmi-net? And how
> can we make netipmid more robust on obmc-console socket shutdown?
>

With not much knowledge of IPMI, but coming from a lot of knowledge of
boost and asio, that usage looks odd.  Instead of the
consoleSocket.async_wait done here:
https://github.com/openbmc/phosphor-net-ipmid/blob/12d199b27764496bfff8a45661239b1e509c336f/sol/sol_manager.cpp#L92
Which then calls into a blocking async_read on the socket, I would've
expected a consoleSocket.async_read_some with a given buffer to reduce
the number of system calls, and to read out partial data as it's
available.  Whether or not it would have different behavior in this
case, I can't say, but doing things the more expected way, and letting
asio handle it in the expected way in the past has netted us good
results in other applications.

Another interesting thing is the use of std::deque for the console
buffer type here.
https://github.com/openbmc/phosphor-net-ipmid/blob/d4a4bed525f79c39705fa526b20ab663bb2c2069/sol/console_buffer.hpp#L12

I would've expected to see one of the streaming buffer types like
flat_buffer (https://www.boost.org/doc/libs/develop/libs/beast/doc/html/beast/ref/boost__beast__flat_buffer.html)
or multi-buffer
(https://www.boost.org/doc/libs/1_78_0/libs/beast/doc/html/beast/ref/boost__beast__multi_buffer.html),
which are designed for exactly what's being done here, streaming data
in and out of a pipe of variable lengths, and can be streamed into and
out of directly without having the extra copy.  Additionally,
deque<uint8_t> is going to have a lot of memory overhead compared to a
flat buffer type.

Not sure if any of the above is helpful to you or not, but it might
give you some things to try.

> Thanks,
>
> Heyi
>