LLDP Layers hold redundant data
- Dominant language
- Go
- Stars
- 6.8k
- Forks
- 1.2k
- PR merge metrics
- No merged PRs in 30d
Description
As best I understand it, layers are supposed to be non-overlapping. By this I mean that fields of a given layer struct (in this case `LinkLayerDiscovery`) shouldn't contain any data that is also contained in other layer structs. As-is, the information in the `LinkLayerDiscoveryInfo` layer is entirely redundant with the `Values` field of `LinkLayerDiscovery`:
```
type LinkLayerDiscovery struct {
BaseLayer
ChassisID LLDPChassisID
PortID LLDPPortID
TTL uint16
Values []LinkLayerDiscoveryValue
}
```
All information that the `LinkLayerDiscoveryInfo` struct contains is parsed purely from the Values list in `LinkLayerDiscovery` in lines [846](https://github.com/google/gopacket/blob/master/layers/lldp.go#L846) to [883](https://github.com/google/gopacket/blob/master/layers/lldp.go#L883) of [lldp.go](https://github.com/google/gopacket/blob/master/layers/lldp.go). Even if I'm mistaken and layers _can_ overlap in such a fashion, the way that these two layers are currently written makes it impossible to implement the `DecodingLayer` interface. Since information from the earlier layer is required to decode the `LinkLayerDiscoveryInfo` layer, I can't write a `DecodeFromBytes` method. (As-is, the `DecodeLinkLayerDiscovery()` function decodes into two different layers).
There are two different fixes that are possible. One would be to entirely merge the `LinkLayerDiscoveryInfo` layer into the `LinkLayerDiscovery` layer, since they're already redundant. The other would be to remove the `Values` field from the `LinkLayerDiscovery` struct (said field is never used except internally within `DecodeLinkLayerDiscovery()`)and separate out the latter half of parsing done in `DecodeLinkLayerDiscovery()`into its own method to decode `LinkLayerDiscoveryInfo`. I plan to submit a pull request changing this as well as implementing the DecodingLayer interface for both of these layers, but I wanted to first check that I haven't misunderstood the requirements of what a "layer" is.
**TD;DR:** The `LinkLayerDiscovery` and `LinkLayerDiscoveryInfo` layers overlap and can't be turned into DecodingLayers; I plan to fix this as well as implementing said interface.
Contributor guide
Assessment
This issue has not been assessed yet.