Ospf dynamic hostname #41
Conversation
reoriginate on ibus hostname change rename to DynamicHostnameTlv dont originate tlv unless host name present Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
Signed-off-by: fliqqs <[email protected]>
holo-ospf/src/ospfv3/lsdb.rs
Outdated
| @@ -281,6 +282,12 @@ impl LsdbVersion<Self> for Ospfv3 { | |||
| } | |||
| } | |||
| } | |||
| LsaOriginateEvent::HostnameChange => { | |||
| // (Re)originate Router-LSA(s) in all areas. | |||
There was a problem hiding this comment.
we should reoriginate Router Information LSAs here instead
holo-ospf/src/packet/tlv.rs
Outdated
| @@ -81,6 +82,8 @@ bitflags! { | |||
| #[derive(Deserialize, Serialize)] | |||
| pub struct RouterInfoCapsTlv(RouterInfoCaps); | |||
|
|
|||
| const MAX_HOSTNAME_LEN: usize = 255; | |||
There was a problem hiding this comment.
nit: please make this an associated const within the TLV struct
holo-ospf/src/packet/tlv.rs
Outdated
| pub(crate) fn encode(&self, buf: &mut BytesMut) { | ||
| let start_pos = | ||
| tlv_encode_start(buf, RouterInfoTlvType::DynamicHostname); | ||
| let mut hostname = self.hostname.clone(); |
There was a problem hiding this comment.
you could try something like this to avoid cloning the string:
let hostname = self.hostname.as_bytes();
let hostname_len = usize::min(hostname.len(), MAX_HOSTNAME_LEN);
buf.put_slice(&hostname[..hostname_len]);
holo-ospf/src/packet/tlv.rs
Outdated
| impl DynamicHostnameTlv { | ||
| pub(crate) fn decode(tlv_len: u16, buf: &mut Bytes) -> DecodeResult<Self> { | ||
| let mut hostname = String::new(); | ||
| for _ in 0..tlv_len { |
There was a problem hiding this comment.
we should validate the TLV length is within the 1..255 range
holo-ospf/src/ospfv2/lsdb.rs
Outdated
| &lsa.body | ||
| { | ||
| if let Some(hostname_tlv) = router_info.info_hostname.as_ref() { | ||
| debug!( |
There was a problem hiding this comment.
please remove this and the other debug! call
holo-ospf/src/ospfv2/lsdb.rs
Outdated
| @@ -346,6 +353,44 @@ impl LsdbVersion<Self> for Ospfv2 { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // examine for DynamicHostnameTlv | |||
There was a problem hiding this comment.
you could try this to avoid the code duplication (area/AS scope):
// Update hostname database.
if let LsaBody::OpaqueArea(LsaOpaque::RouterInfo(router_info))
| LsaBody::OpaqueAs(LsaOpaque::RouterInfo(router_info)) = &lsa.body
{
if let Some(hostname_tlv) = router_info.info_hostname.as_ref() {
// Install or update hostname.
instance
.state
.hostnames
.insert(lsa.hdr.adv_rtr, hostname_tlv.hostname.clone());
} else {
// Remove hostname if it exists.
instance.state.hostnames.remove(&lsa.hdr.adv_rtr);
}
}
holo-ospf/src/instance.rs
Outdated
| @@ -103,6 +103,8 @@ pub struct InstanceState<V: Version> { | |||
| pub gr_helper_count: usize, | |||
| // Authentication non-decreasing sequence number. | |||
| pub auth_seqno: Arc<AtomicU64>, | |||
| // Hostname cache | |||
There was a problem hiding this comment.
nit: missing trailing dot for consistency with the other comments
| @@ -597,6 +597,7 @@ static LSA2: Lazy<(Vec<u8>, Lsa<Ospfv2>)> = Lazy::new(|| { | |||
| )], | |||
| msds: None, | |||
| srms_pref: None, | |||
| info_hostname: Some(DynamicHostnameTlv::new("holo".to_owned())), | |||
There was a problem hiding this comment.
need to update reference data (vector of bytes) for this test to pass
| @@ -122,6 +122,8 @@ pub struct LsaRouterInfo { | |||
| pub srlb: Vec<SrLocalBlockTlv>, | |||
| pub msds: Option<MsdTlv>, | |||
| pub srms_pref: Option<SrmsPrefTlv>, | |||
| // #[serde(skip)] | |||
There was a problem hiding this comment.
we need #[serde(skip)] here otherwise some conformance tests will fail. This is a workaround for something that should be fixed in the test framework soonish.
Signed-off-by: fliqqs <[email protected]>
No description provided.