Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

No error on failure to parse TcOpts #244

Open
DieracDelta opened this issue Mar 6, 2022 · 4 comments
Open

No error on failure to parse TcOpts #244

DieracDelta opened this issue Mar 6, 2022 · 4 comments

Comments

@DieracDelta
Copy link

DieracDelta commented Mar 6, 2022

If the TcOpts fail to parse in one of the qdiscs here, the entire qdisc is skipped. I see the error being generated with the .context() here, but for some reason it's not making its way back up to the stream. Any advice on how to access the errors without dropping in a bunch of print statements?

@little-dude
Copy link
Owner

Hello @DieracDelta

This is a problem I'm aware of but don't have time to dedicate to properly fix. There are actually two problems:

  • first the fact that we fail to parse messages. This may be due to a bug, but this may also be due to the fact that on some platforms, messages have slightly different format. Here is the issue about it: Avoid defining kernel dependent data structures. #44. This is a difficult problem to solve imo.
  • second, in netlink-proto, we discard messages that we can't parse. I think we could do better here and distinguish between different kinds of errors:
    • failing to parse the message header: this is problematic because we can't find the sequence ID so we have no other choice than dropping the message (we can't even return an error because we use the sequence ID to send it back to the correct handle)
    • failing to parse the payload: here we could actually return an error back to the correct handle, which could contain the buffer for the message we failed to parse. Actually we could even push that further, but maybe as an opt-in feature: if we only fail to parse an NLA payload, log a warning or an error and use the DefaultNla type instead.

The first issue really is a terrible limitation which is going to bite many people but fixing it is not trivial :( The second issue is more easily fixable. I'm not sure whether I'm going to have time though. Maybe next Sunday, but no promise. Otherwise, it's going to be in April at best since I'll be AFK for three weeks from March 18th.

@wllenyj
Copy link
Contributor

wllenyj commented Mar 15, 2022

Hi, @little-dude

Is it possible to release a new version before you AFK?

@little-dude
Copy link
Owner

little-dude commented Mar 18, 2022

Sorry, @wllenyj I didn't manage to find time to do it. The last two weeks have been crazy at work, and I spent the little free time I had to help out some of the many Ukrainians refugees who are arriving in Berlin.

@wllenyj
Copy link
Contributor

wllenyj commented Mar 20, 2022

@little-dude Wow, It's so cool.
Thank you for what you are doing with Uncharted.
Can @ me know if you can find the time to do it.
Thanks again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants