Relieve length check for NeighAttributes#208
Conversation
jsimonetti
force-pushed
the
neigh_no_length_check
branch
from
19f322c to
7dbdd69
Compare
The attribute length has more context then previously presumed. Especially when using tunnels and other link types, the attributes can use different lengths then expected. Signed-off-by: Jeroen Simonetti <[email protected]>
jsimonetti
force-pushed
the
neigh_no_length_check
branch
from
7dbdd69 to
7acbaad
Compare
jsimonetti
changed the title
|
@jsimonetti Can we isolate whether the problem is occurring in the decoding of My gut feeling about #200 was that it treated the symptoms, not the cause. As I already mentioned in prometheus/node_exporter#2849, node_exporter should already correctly handle NDA_LLADDR responses which have State In other words, I don't think that there need to be explicit workarounds for zero-length responses, so long as there also aren't enforced ideas such as: if l != 6 && l != 8 && l != 20 {
return errInvalidNeighMessageAttr
}I'm pretty sure that if rtnetlink simply decoded any length response and handed it to the consumer, with the understanding that the consumer will check for things such as address family and It would be nice if we could determine a simple test case to reproduce this. So far it seems to involve Docker, bridges and Wireguard. But I have a hunch that it should be reproducible with GRE or SIT tunnels, since point-to-point tunnels typically do not use ARP (since there is no need). |
Signed-off-by: Jeroen Simonetti <[email protected]>
|
This latest version completely removes any length check for neighbor attributes. This places the burden of checking for correctness to the consumer of this library. |
2 participants
The attribute length has more context then previously presumed. Especially when using tunnels and other link types, the attributes can use different lengths then expected.
@SuperQ and @dswarbrick you have been previously involved in this part and are (afaik) using these parts in node_exporter.
Would this by ok by you or would this introduce more issues on your end?
This PR is made in lieu of prometheus/node_exporter#2849