events: adding detail to DatagramDropped event (#2438)

* events: adding detail to DatagramDropped event

* formatting

* add test

* remove unread fields

quic/s2n-quic-core/events/connection.rs

@@ -272,7 +272,11 @@ struct DatagramReceived {
 #[event("transport:datagram_dropped")]
 //= https://tools.ietf.org/id/draft-marx-qlog-event-definitions-quic-h3-02#5.3.12
 /// Datagram dropped by a connection
-struct DatagramDropped {
+struct DatagramDropped<'a> {
+    local_addr: SocketAddress<'a>,
+    remote_addr: SocketAddress<'a>,
+    destination_cid: ConnectionId<'a>,
+    source_cid: Option<ConnectionId<'a>>,
     #[measure("bytes", Bytes)]
     #[counter("bytes.total", Bytes)]
     len: u16,

quic/s2n-quic-core/src/event/generated.rs

@@ -2289,20 +2289,28 @@ pub mod api {
     #[derive(Clone, Debug)]
     #[non_exhaustive]
     #[doc = " Datagram dropped by a connection"]
-    pub struct DatagramDropped {
+    pub struct DatagramDropped<'a> {
+        pub local_addr: SocketAddress<'a>,
+        pub remote_addr: SocketAddress<'a>,
+        pub destination_cid: ConnectionId<'a>,
+        pub source_cid: Option<ConnectionId<'a>>,
         pub len: u16,
         pub reason: DatagramDropReason,
     }
     #[cfg(any(test, feature = "testing"))]
-    impl crate::event::snapshot::Fmt for DatagramDropped {
+    impl<'a> crate::event::snapshot::Fmt for DatagramDropped<'a> {
         fn fmt(&self, fmt: &mut core::fmt::Formatter) -> core::fmt::Result {
             let mut fmt = fmt.debug_struct("DatagramDropped");
+            fmt.field("local_addr", &self.local_addr);
+            fmt.field("remote_addr", &self.remote_addr);
+            fmt.field("destination_cid", &self.destination_cid);
+            fmt.field("source_cid", &self.source_cid);
             fmt.field("len", &self.len);
             fmt.field("reason", &self.reason);
             fmt.finish()
         }
     }
-    impl Event for DatagramDropped {
+    impl<'a> Event for DatagramDropped<'a> {
         const NAME: &'static str = "transport:datagram_dropped";
     }
     #[derive(Clone, Debug)]
@@ -3902,8 +3910,15 @@ pub mod tracing {
             event: &api::DatagramDropped,
         ) {
             let id = context.id();
-            let api::DatagramDropped { len, reason } = event;
-            tracing :: event ! (target : "datagram_dropped" , parent : id , tracing :: Level :: DEBUG , { len = tracing :: field :: debug (len) , reason = tracing :: field :: debug (reason) });
+            let api::DatagramDropped {
+                local_addr,
+                remote_addr,
+                destination_cid,
+                source_cid,
+                len,
+                reason,
+            } = event;
+            tracing :: event ! (target : "datagram_dropped" , parent : id , tracing :: Level :: DEBUG , { local_addr = tracing :: field :: debug (local_addr) , remote_addr = tracing :: field :: debug (remote_addr) , destination_cid = tracing :: field :: debug (destination_cid) , source_cid = tracing :: field :: debug (source_cid) , len = tracing :: field :: debug (len) , reason = tracing :: field :: debug (reason) });
         }
         #[inline]
         fn on_connection_id_updated(
@@ -5931,15 +5946,30 @@ pub mod builder {
     }
     #[derive(Clone, Debug)]
     #[doc = " Datagram dropped by a connection"]
-    pub struct DatagramDropped {
+    pub struct DatagramDropped<'a> {
+        pub local_addr: SocketAddress<'a>,
+        pub remote_addr: SocketAddress<'a>,
+        pub destination_cid: ConnectionId<'a>,
+        pub source_cid: Option<ConnectionId<'a>>,
         pub len: u16,
         pub reason: DatagramDropReason,
     }
-    impl IntoEvent<api::DatagramDropped> for DatagramDropped {
+    impl<'a> IntoEvent<api::DatagramDropped<'a>> for DatagramDropped<'a> {
         #[inline]
-        fn into_event(self) -> api::DatagramDropped {
-            let DatagramDropped { len, reason } = self;
+        fn into_event(self) -> api::DatagramDropped<'a> {
+            let DatagramDropped {
+                local_addr,
+                remote_addr,
+                destination_cid,
+                source_cid,
+                len,
+                reason,
+            } = self;
             api::DatagramDropped {
+                local_addr: local_addr.into_event(),
+                remote_addr: remote_addr.into_event(),
+                destination_cid: destination_cid.into_event(),
+                source_cid: source_cid.into_event(),
                 len: len.into_event(),
                 reason: reason.into_event(),
             }

quic/s2n-quic-transport/src/endpoint/initial.rs

@@ -338,6 +338,13 @@ impl<Config: endpoint::Config> endpoint::Endpoint<Config> {
                         endpoint_context.event_subscriber,
                         |publisher, _path| {
                             publisher.on_datagram_dropped(event::builder::DatagramDropped {
+                                local_addr: header.path.local_address().into_event(),
+                                remote_addr: header.path.remote_address().into_event(),
+                                destination_cid: datagram.destination_connection_id.into_event(),
+                                source_cid: datagram
+                                    .source_connection_id
+                                    .as_ref()
+                                    .map(|cid| cid.into_event()),
                                 len: datagram.payload_len as u16,
                                 reason: err,
                             });

quic/s2n-quic-transport/src/endpoint/mod.rs

@@ -575,6 +575,15 @@ impl<Cfg: Config> Endpoint<Cfg> {
                             endpoint_context.event_subscriber,
                             |publisher, _path| {
                                 publisher.on_datagram_dropped(event::builder::DatagramDropped {
+                                    local_addr: header.path.local_address().into_event(),
+                                    remote_addr: header.path.remote_address().into_event(),
+                                    destination_cid: datagram
+                                        .destination_connection_id
+                                        .into_event(),
+                                    source_cid: datagram
+                                        .source_connection_id
+                                        .as_ref()
+                                        .map(|cid| cid.into_event()),
                                     len: datagram.payload_len as u16,
                                     reason: datagram_drop_reason,
                                 });

quic/s2n-quic/src/tests/connection_migration.rs

@@ -4,7 +4,7 @@
 use super::*;
 use s2n_codec::DecoderBufferMut;
 use s2n_quic_core::{
-    event::api::Subject,
+    event::api::{DatagramDropReason, Subject},
     packet::interceptor::{Datagram, Interceptor},
 };
 
@@ -218,3 +218,71 @@ fn port_rebind_test() {
 fn ip_and_port_rebind_test() {
     run_test(|addr| rebind_ip(rebind_port(addr)));
 }
+
+// Changes the port of the second datagram received
+#[derive(Default)]
+struct RebindPortBeforeHandshakeConfirmed {
+    datagram_count: usize,
+    changed_port: bool,
+}
+
+const REBIND_PORT: u16 = 55555;
+impl Interceptor for RebindPortBeforeHandshakeConfirmed {
+    fn intercept_rx_remote_port(&mut self, _subject: &Subject, port: &mut u16) {
+        if self.datagram_count == 1 && !self.changed_port {
+            *port = REBIND_PORT;
+            self.changed_port = true;
+        }
+    }
+
+    fn intercept_rx_datagram<'a>(
+        &mut self,
+        _subject: &Subject,
+        _datagram: &Datagram,
+        payload: DecoderBufferMut<'a>,
+    ) -> DecoderBufferMut<'a> {
+        self.datagram_count += 1;
+        payload
+    }
+}
+
+/// Ensures that a datagram received from a client that changes
+/// its port before the handshake is confirmed is dropped.
+#[test]
+fn rebind_before_handshake_confirmed() {
+    let model = Model::default();
+    let subscriber = recorder::DatagramDropped::new();
+    let datagram_dropped_events = subscriber.events();
+
+    test(model, move |handle| {
+        let server = Server::builder()
+            .with_io(handle.builder().build()?)?
+            .with_tls(SERVER_CERTS)?
+            .with_event((tracing_events(), subscriber))?
+            .with_random(Random::with_seed(456))?
+            .with_packet_interceptor(RebindPortBeforeHandshakeConfirmed::default())?
+            .start()?;
+
+        let client = Client::builder()
+            .with_io(handle.builder().build()?)?
+            .with_tls(certificates::CERT_PEM)?
+            .with_event(tracing_events())?
+            .with_random(Random::with_seed(456))?
+            .start()?;
+
+        let addr = start_server(server)?;
+        start_client(client, addr, Data::new(1000))?;
+        Ok(addr)
+    })
+    .unwrap();
+
+    let datagram_dropped_events = datagram_dropped_events.lock().unwrap();
+
+    assert_eq!(1, datagram_dropped_events.len());
+    let event = datagram_dropped_events.first().unwrap();
+    assert!(matches!(
+        event.reason,
+        DatagramDropReason::ConnectionMigrationDuringHandshake { .. },
+    ));
+    assert_eq!(REBIND_PORT, event.remote_addr.port());
+}

quic/s2n-quic/src/tests/recorder.rs

@@ -165,3 +165,28 @@ event_recorder!(
         storage.push(addr);
     }
 );
+
+use s2n_quic_core::event::api::DatagramDropReason;
+pub struct DatagramDroppedEvent {
+    pub remote_addr: SocketAddr,
+    pub reason: DatagramDropReason,
+}
+
+impl<'a> From<&events::DatagramDropped<'a>> for DatagramDroppedEvent {
+    fn from(value: &events::DatagramDropped<'a>) -> Self {
+        DatagramDroppedEvent {
+            remote_addr: value.remote_addr.to_string().parse().unwrap(),
+            reason: value.reason.clone(),
+        }
+    }
+}
+
+event_recorder!(
+    DatagramDropped,
+    DatagramDropped,
+    on_datagram_dropped,
+    DatagramDroppedEvent,
+    |event: &events::DatagramDropped, storage: &mut Vec<DatagramDroppedEvent>| {
+        storage.push(event.into());
+    }
+);