Misc quality of life improvements

[jmeggitt]

This pull request aims to address points of friction using the library. From my experience, those points of friction were:

• Not many types implement Hash.
  • Makes it difficult to construct maps using these types as keys. Workarounds generally involve either making wrappers which can be hashed or converting types to primitives.
• Long field chains and lots of nesting required to access values
  • Can be addressed by adding iterators and implementing Deref where appropriate.
• A lot of boiler plate is required to access specific attributes
  • There are a few common attributes such as AsPath and MpReachNlri which are frequently used. Ideally, these attributes could be retrieved with helper methods instead of needing to iterate and match every attribute.
• There is no easy way to map *Value enums to their respective *Type enums.
• Error type is confusing to work with
  • Not covered by this pull request, but something worth looking into.
• Types do not implement Deserialize (covered by Add full serde support #122).

Notes on changes

• Attributes is quite inefficient at the moment and has lots of areas for improvement. Firstly, it probably needs more methods for additional common attributes. I also chose to implement IntoIterator with AttributeValue since that is generally what the user is most interested in, but an argument can be made for using Attribute instead.
• I think AttrType could have turned out better. Unfortunately, there is no default/other option for #[derive(Primitive)] so the inclusion of an unknown attribute type requires that it be implemented manually.
• The Attribute struct adds additional boiler plate to accessing attributes. It would be easier to use if AttributeValue took the place of Attribute so it could be matched on directly. We can already determine the AttrType based on just AttributeValue so the only thing missing is the Attribute flags. Perhaps, the flags could be added to AttributeValue instead. Well-known attributes already have their flags pre-defined so they would only need to be added to some of the attributes. That being said, if new flags were to be added or the received flags are inconsistent with what is expected, they must be retained so they can be handled by the user.
• I implemented Debug for a couple of types in an attempt to reduce the clutter when debug printing some values. I am not sure if this is worth keeping and may be better to simply derive.

[digizeph]

Great work! I left some comments for your questions.

[jmeggitt]

Can the ORIGINATOR_ID and other BGP identifiers be IPv6 addresses? So far, I have only seen this attribute referenced as a 4 octet value corresponding to a BGP identifier.

bgpkit-parser/src/parser/bgp/attributes/attr_09_originator.rs

Lines 10 to 15 in e2d1ed4

	let afi = match afi {
	None => &Afi::Ipv4,
	Some(a) => a,
	};
	let addr = input.read_address(afi)?;
	Ok(AttributeValue::OriginatorId(addr))

[digizeph]

Can the ORIGINATOR_ID and other BGP identifiers be IPv6 addresses? So far, I have only seen this attribute referenced as a 4 octet value corresponding to a BGP identifier.

bgpkit-parser/src/parser/bgp/attributes/attr_09_originator.rs

Lines 10 to 15 in e2d1ed4

	let afi = match afi {
	None => &Afi::Ipv4,
	Some(a) => a,
	};
	let addr = input.read_address(afi)?;
	Ok(AttributeValue::OriginatorId(addr))

They have to be 4 octets unless specified otherwise. They do not necessarily need to be IPv4 addresses.

Corresponding RFC4271 section for BGP Identifier:

   BGP Identifier
      A 4-octet unsigned integer that indicates the BGP Identifier of
      the sender of BGP messages.  A given BGP speaker sets the value of
      its BGP Identifier to an IP address assigned to that BGP speaker.
      The value of the BGP Identifier is determined upon startup and is
      the same for every local interface and BGP peer.

Corresponding RFC4456 section for ORIGINATOR_ID:

   ORIGINATOR_ID is a new optional, non-transitive BGP attribute of Type
   code 9.  This attribute is 4 bytes long and it will be created by an
   RR in reflecting a route.  This attribute will carry the BGP
   Identifier of the originator of the route in the local AS.  A BGP
   speaker SHOULD NOT create an ORIGINATOR_ID attribute if one already
   exists.  A router that recognizes the ORIGINATOR_ID attribute SHOULD
   ignore a route received with its BGP Identifier as the ORIGINATOR_ID.

(when in doubt, check this attribute index: https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml#bgp-parameters-2)

[jmeggitt]

I think the changes are now about ready for review. I want to apologize again about the size of the PR. I did not realize it would get this large when I first started going off on tangents.

Please let me know which areas you feel require changes, additional unit test coverage, or should be deferred/removed. I imagine there are plenty of things I either forgot or overlooked.

Going forward, my current thinking is that the conversion to thiserror and any performance tweaks should be separate PRs.