Fix route delete to fail on error instead of just warn

crates/bgp_packet/src/nlri.rs

@@ -18,6 +18,7 @@ use crate::traits::ParserContext;
 use crate::traits::ReadablePacket;
 use crate::traits::WritablePacket;
 
+use eyre::{bail, eyre};
 use nom::bytes::complete::take;
 use nom::number::complete::be_u8;
 use nom::Err::Failure;
@@ -121,14 +122,17 @@ impl<'de> Deserialize<'de> for NLRI {
 }
 
 impl TryFrom<NLRI> for Ipv6Addr {
-    type Error = String;
+    type Error = eyre::ErrReport;
 
     fn try_from(value: NLRI) -> Result<Self, Self::Error> {
         match value.afi {
             AddressFamilyIdentifier::Ipv6 => {
                 let mut v: [u8; 16] = [0u8; 16];
                 if value.prefix.len() > v.len() {
-                    return Err("prefix length greater than IPv6 address length".to_string());
+                    bail!(
+                        "prefix length {} greater than IPv6 address length 16",
+                        value.prefix.len()
+                    );
                 }
                 for (pos, e) in value.prefix.iter().enumerate() {
                     v[pos] = *e;
@@ -136,20 +140,23 @@ impl TryFrom<NLRI> for Ipv6Addr {
                 let ip6: Ipv6Addr = v.into();
                 Ok(ip6)
             }
-            _ => Err("Unsupported AFI type".to_string()),
+            other => bail!("Unsupported AddressFamily type {}", other),
         }
     }
 }
 
 impl TryFrom<NLRI> for Ipv4Addr {
-    type Error = String;
+    type Error = eyre::Report;
 
     fn try_from(value: NLRI) -> Result<Self, Self::Error> {
         match value.afi {
             AddressFamilyIdentifier::Ipv4 => {
                 let mut v: [u8; 4] = [0u8; 4];
                 if value.prefix.len() > v.len() {
-                    return Err("prefix length greater than IPv4 address length".to_string());
+                    bail!(
+                        "prefix length {} greater than IPv4 address length 4",
+                        value.prefix.len()
+                    );
                 }
                 for (pos, e) in value.prefix.iter().enumerate() {
                     v[pos] = *e;
@@ -157,22 +164,22 @@ impl TryFrom<NLRI> for Ipv4Addr {
                 let ip4 = Ipv4Addr::new(v[0], v[1], v[2], v[3]);
                 Ok(ip4)
             }
-            _ => Err("Unsupported AFI type".to_string()),
+            other => bail!("Unsupported AddressFamily type: {}", other),
         }
     }
 }
 
 impl TryInto<IpAddr> for NLRI {
-    type Error = std::io::Error;
+    type Error = eyre::ErrReport;
     fn try_into(self) -> Result<IpAddr, Self::Error> {
         match self.afi {
             AddressFamilyIdentifier::Ipv4 => {
                 let mut v: [u8; 4] = [0u8; 4];
                 if self.prefix.len() > v.len() {
-                    return Err(std::io::Error::new(
+                    bail!(
-                        ErrorKind::InvalidData,
+                        "prefix length {} greater than IPv4 address length 4",
-                        "prefix length greater than IPv4 address length",
+                        self.prefix.len()
-                    ));
+                    );
                 }
                 for (pos, e) in self.prefix.iter().enumerate() {
                     v[pos] = *e;
@@ -183,10 +190,10 @@ impl TryInto<IpAddr> for NLRI {
             AddressFamilyIdentifier::Ipv6 => {
                 let mut v: [u8; 16] = [0u8; 16];
                 if self.prefix.len() > v.len() {
-                    return Err(std::io::Error::new(
+                    bail!(
-                        ErrorKind::InvalidData,
+                        "prefix length {} greater than IPv6 address length 16",
-                        "prefix length greater than IPv6 address length",
+                        self.prefix.len()
-                    ));
+                    );
                 }
                 for (pos, e) in self.prefix.iter().enumerate() {
                     v[pos] = *e;
@@ -199,27 +206,28 @@ impl TryInto<IpAddr> for NLRI {
 }
 
 impl TryFrom<&str> for NLRI {
-    type Error = String;
+    type Error = eyre::Report;
     fn try_from(value: &str) -> Result<Self, Self::Error> {
         let parts: Vec<&str> = value.split("/").collect();
         if parts.len() != 2 {
-            return Err(format!("Expected ip_addr/prefixlen but got: {}", value));
+            bail!("Expected ip_addr/prefixlen but got: {}", value);
         }
 
-        let prefixlen: u8 = u8::from_str(parts[1]).map_err(|_| "failed to parse prefixlen")?;
+        let prefixlen: u8 =
+            u8::from_str(parts[1]).map_err(|_| eyre!("failed to parse prefixlen"))?;
         let mut octets: Vec<u8>;
         let afi: AddressFamilyIdentifier;
 
         if parts[0].contains(":") {
             afi = AddressFamilyIdentifier::Ipv6;
-            let addr: Ipv6Addr = Ipv6Addr::from_str(parts[0]).map_err(|e| e.to_string())?;
+            let addr: Ipv6Addr = Ipv6Addr::from_str(parts[0]).map_err(|e| eyre!(e))?;
             octets = addr.octets().to_vec();
         } else if parts[0].contains(".") {
             afi = AddressFamilyIdentifier::Ipv4;
-            let addr: Ipv4Addr = Ipv4Addr::from_str(parts[0]).map_err(|e| e.to_string())?;
+            let addr: Ipv4Addr = Ipv4Addr::from_str(parts[0]).map_err(|e| eyre!(e))?;
             octets = addr.octets().to_vec();
         } else {
-            return Err(format!("Could not detect IP address type: {}", parts[0]));
+            bail!("Could not detect IP address type: {}", parts[0]);
         }
 
         // Truncate octets to prefixlen

crates/route_client/src/fib_state.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use eyre::Result;
+use eyre::{bail, Result};
 use ip_network_table_deps_treebitmap::address::Address;
 use ip_network_table_deps_treebitmap::IpLookupTable;
 use std::convert::{TryFrom, TryInto};
@@ -79,7 +79,7 @@ impl<
         S: SouthboundInterface,
     > FibState<A, S>
 where
-    String: From<<A as TryFrom<NLRI>>::Error>,
+    eyre::ErrReport: From<<A as TryFrom<NLRI>>::Error>,
 {
     pub async fn get_routing_table(&mut self) -> Result<()> {
         todo!();
@@ -87,7 +87,7 @@ where
 
     /// route_add requests updating the nexthop to a particular path if it is not already
     /// the best path.
-    pub async fn route_add(&mut self, nlri: &NLRI, nexthop: IpAddr) -> Result<(), String> {
+    pub async fn route_add(&mut self, nlri: &NLRI, nexthop: IpAddr) -> Result<()> {
         info!(af = ?self.af, %nlri, %nexthop);
         // Lookup the path in the Fib, there are three possible outcomes:
         // 1. The route is not yet known, we add it to the FibState and inject it into the kernel,
@@ -112,7 +112,7 @@ where
                                 "Southbound interface returned error when trying to remove route: {} via {}, error: {}",
                                 nlri, entry.nexthop, e
                             );
-                        return Err("Netlink remove error".to_string());
+                        bail!("Failed to delete route in update: {}", e);
                     }
 
                     // Add new route
@@ -130,7 +130,7 @@ where
                             "Netlink returned error when trying to add route: {} via {}, error: {}",
                             nlri, nexthop, e
                         );
-                        return Err("Netlink add error".to_string());
+                        bail!("Failed to add route to netlink in update: {}", e);
                     }
 
                     entry.nexthop = nexthop;
@@ -151,7 +151,7 @@ where
                         "Netlink returned error when trying to add route: {} via {}, error: {}",
                         nlri, nexthop, e
                     );
-                    return Err("Netlink add error".to_string());
+                    bail!("Failed to add new route: {}", e);
                 }
 
                 let addr: A = nlri.clone().try_into()?;
@@ -164,22 +164,24 @@ where
     }
 
     /// route_del removes a route from the FibState and kernel.
-    pub async fn route_del(&mut self, nlri: NLRI) -> Result<(), String> {
+    pub async fn route_del(&mut self, nlri: NLRI) -> Result<()> {
         let prefix_addr: A = nlri.clone().try_into()?;
         if let Some(entry_wrapped) = self.fib.exact_match(prefix_addr, nlri.prefixlen.into()) {
             {
                 let entry = entry_wrapped.lock().await;
                 info!(%nlri, %prefix_addr, %entry.nexthop, "route_del");
                 if let Err(e) = self.southbound.route_del(nlri.clone(), entry.nexthop).await {
-                    warn!(
+                    bail!(
                         "Failed to apply route mutation to remove NLRI: {}, error: {}",
-                        nlri, e
+                        nlri,
+                        e
                     );
                 }
             }
             self.fib.remove(prefix_addr, nlri.prefixlen.into());
+            info!(%nlri, "Successfully removed route from kernel");
         } else {
-            warn!("Failed to find prefix to remove from FIB: {}", nlri);
+            bail!("Failed to find prefix to remove from FIB: {}", nlri);
         }
 
         Ok(())

crates/route_client/src/netlink.rs

@@ -1,6 +1,6 @@
 use async_trait::async_trait;
 use bgp_packet::{constants::AddressFamilyIdentifier, nlri::NLRI};
-use eyre::{eyre, Result};
+use eyre::Result;
 use futures::TryStreamExt;
 use netlink_packet_route::route::RouteAddress;
 use netlink_packet_route::route::RouteAttribute;
@@ -111,12 +111,8 @@ impl SouthboundInterface for NetlinkConnector {
     async fn route_del(&mut self, prefix: NLRI, nexthop: IpAddr) -> Result<()> {
         let rt_handle = self.handle.route();
         let destination = match prefix.afi {
-            AddressFamilyIdentifier::Ipv4 => {
+            AddressFamilyIdentifier::Ipv4 => RouteAddress::Inet(prefix.clone().try_into()?),
-                RouteAddress::Inet(prefix.clone().try_into().map_err(|e: String| eyre!(e))?)
+            AddressFamilyIdentifier::Ipv6 => RouteAddress::Inet6(prefix.clone().try_into()?),
-            }
-            AddressFamilyIdentifier::Ipv6 => {
-                RouteAddress::Inet6(prefix.clone().try_into().map_err(|e: String| eyre!(e))?)
-            }
         };
         let nexthop = match nexthop {
             IpAddr::V4(ipv4) => RouteAddress::Inet(ipv4),