Skip to content

Conversation

@glebkin
Copy link
Contributor

@glebkin glebkin commented Oct 10, 2025

We're using GoVPP and have a lot of cases dumping from VPP using streams. Sometimes we need to find only one single item and close stream after that without reading it till the end. It leads to tonns of errors in logs like: Channel ID not known, ignoring the message: *memclnt.ControlPingReply

So I would like to change "Channel ID not known" log message level, as it looks like expected behaviour and not an error.

…iour and not an error.

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>
@VladoLavor
Copy link
Collaborator

Hello @glebkin,
The behavior and the error you see are expected because you are creating a race condition by misusing the api.

GoVPP dump (i.e., multirequest) tells the VPP to stream the given items and send the control ping at the end. On the GoVPP side, messages are received by stream.RecvMsg() in a loop until a control_ping message is received, marking the end of the stream.

Now, GoVPP watches all received messages and does "message callback", where the appropriate channel for the reply is looked for. If the stream is closed before the dump loop is finished, it won't prevent receiving the rest of the messages, but it removes the assigned channel. The callback then cannot find it, and you see the error you described.

In most cases, there is no performance gain from breaking the dump loop. You can do is to check the api you are using if any filters are available (for example, SwInterfaceDump provides NameFilter that limits the number of received messages, but you still should wait for the control ping tho).
If this is not the case, you may use a separate channel to send the reply as soon as it is received for processing, but let the dump loop finish properly.

To your PR, hiding the error message like that is not desired, and it won't solve the underlying problem. If you have any questions, feel free to ask.

l.Errorf("Channel ID not known, ignoring the message: %T %+v", msg, msg)
// This can happen when a stream was closed explicitly before the last message received.
// It's expected and not an error.
if log.Level >= logrus.DebugLevel {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have included the part above that does decoding as that is actually what can have performance hit.

@ondrej-fabry
Copy link
Member

@VladoLavor is correct on some the reasoning. However, I've seen this too many times as well and I believe there are many uses that only need single value (no way to filter) and then skip reading rest. In these cases if we look at it, it extra boilerplate for the users to write reading the rest of data, which can easily be handled discreetly by GoVPP. This definitely should not be an error level thing in such cases.

Shooting a bit here, but perhaps we could differentiate between cases where these "pre-maturely closed streams" and those cases where such stream (channel id) is totally unknown. We would have to keep those channels in memory and not reuse their ID, but for how long is another problem.

Perhaps a middle ground could be changing the log level to warning and also adding some opt-in variable that could control the log level - allowing users to set debug level at start of their application (in init or main, etc).

Let me know what you think @glebkin and thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants