pr feedback

.github/workflows/ci.yml

@@ -844,7 +844,7 @@ jobs:
 
       - name: Generate bindings
         working-directory: dc/wireshark
-        run: ./generate-bindings.sh
+        run: cargo xtask bindings
 
       - name: Run cargo fmt
         working-directory: dc/wireshark
@@ -856,8 +856,8 @@ jobs:
 
       - name: Run tests
         working-directory: dc/wireshark
-        run: cargo test
+        run: cargo xtask test
 
       - name: Run build
         working-directory: dc/wireshark
-        run: cargo build --release
+        run: cargo xtask build

dc/wireshark/.cargo/config.toml

@@ -0,0 +1,2 @@
+[alias]
+xtask = "run --package xtask --"

dc/wireshark/.gitignore

@@ -1,3 +1,2 @@
 *.pcap
 *.pcapng
-src/wireshark_sys.rs

dc/wireshark/Cargo.toml

@@ -19,10 +19,14 @@ s2n-quic-core = { path = "../../quic/s2n-quic-core", features = ["testing", "gen
 s2n-quic-dc = { path = "../s2n-quic-dc", features = ["testing"] }
 
 [workspace]
-members = ["."]
+members = [".", "xtask"]
 
 [profile.fuzz]
 inherits = "dev"
 opt-level = 3
 incremental = false
 codegen-units = 1
+
+# this is to avoid conflicts with already installed plugins
+[profile.release-test]
+inherits = "release"

dc/wireshark/README.md

@@ -47,9 +47,7 @@ tshark -r stream-request-response.pcap -O dcquic 'dcquic && not tcp'
 
 If you need access to more Wireshark APIs that currently don't have bindings in
 `src/wireshark_sys.rs`, you can re-generate that file with
-`./generate-bindings.sh`. Note that it currently has only been run and written
-against one particular environment and isn't run in CI, so it's likely you'll
-need to tweak it to get it working. But it's a good starting point.
+`./generate-bindings.sh`.
 
 https://www.wireshark.org/docs/wsdg_html/#ChapterDissection is a good starting
 point for understanding the basics of the Wireshark interface.

dc/wireshark/build.rs

@@ -1,21 +1,21 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::process::Command;
-
 fn main() {
-    let flags = Command::new("pkg-config")
-        .arg("--libs")
-        .arg("glib-2.0")
-        .arg("wireshark")
-        .output()
-        .unwrap();
+    let plugin_name = option_env("PLUGIN_NAME").unwrap_or_else(|| "dcQUIC".to_string());
+    println!("cargo:rustc-env=PLUGIN_NAME={plugin_name}");
+    println!(
+        "cargo:rustc-env=PLUGIN_NAME_LOWER={}",
+        plugin_name.to_lowercase()
+    );
+
+    // don't link any libraries and prefer pulling symbols from the wireshark/tshark binary
+    println!("cargo:rustc-link-arg=-U");
+    println!("cargo:rustc-link-arg=-shared");
+}
 
-    for flag in core::str::from_utf8(&flags.stdout)
-        .unwrap()
-        .split(' ')
-        .filter(|f| !f.is_empty())
-    {
-        println!("cargo:rustc-link-arg={flag}");
-    }
+fn option_env<N: AsRef<str>>(name: N) -> Option<String> {
+    let name = name.as_ref();
+    println!("cargo:rerun-if-env-changed={}", name);
+    std::env::var(name).ok()
 }

dc/wireshark/generate-bindings.sh

@@ -25,7 +25,7 @@ elif command -v brew &> /dev/null; then
 elif command -v apt-get &> /dev/null; then
   sudo add-apt-repository ppa:wireshark-dev/stable
   sudo apt-get update
-  sudo apt-get install pkg-config wireshark-dev -y
+  sudo apt-get install pkg-config wireshark-dev tshark -y
 fi
 
 INCLUDES=(
@@ -36,6 +36,8 @@ INCLUDES=(
 # It's possible there's a better way to do this -- some of the Wireshark
 # headers end up pulling in C++ so we do need some filtering.
 bindgen \
+  --raw-line '// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.' \
+  --raw-line '// SPDX-License-Identifier: Apache-2.0' \
   --allowlist-type 'gint' \
   --allowlist-type 'guint' \
   --allowlist-type 'guint16' \

dc/wireshark/src/bin/generate-pcap.rs

@@ -2,14 +2,17 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use s2n_codec::{EncoderBuffer, EncoderValue};
-use std::{io::Write, net::Ipv4Addr};
+use std::{io::Write, net::Ipv4Addr, path::Path};
 
 const MAGIC_NUMBER: u32 = 0xa1b2c3d4;
 
 fn main() {
     // https://wiki.wireshark.org/Development/LibpcapFileFormat#overview
-    let mut output =
-        std::io::BufWriter::new(std::fs::File::create("generated-datagrams.pcap").unwrap());
+    let out_dir = std::env::args().nth(1).unwrap_or_default();
+
+    let output = Path::new(&out_dir).join("datagram.pcap");
+
+    let mut output = std::io::BufWriter::new(std::fs::File::create(output).unwrap());
 
     output.write_all(&MAGIC_NUMBER.to_ne_bytes()).unwrap();
 

dc/wireshark/src/field.rs

@@ -161,11 +161,13 @@ fn register_protocol() -> Protocol {
 
 #[cfg(not(test))]
 fn register_protocol() -> Protocol {
+    let name = concat!(env!("PLUGIN_NAME"), "\0");
+    let lower = concat!(env!("PLUGIN_NAME_LOWER"), "\0");
     Protocol(unsafe {
         wireshark_sys::proto_register_protocol(
-            c"dcQUIC".as_ptr(),
-            c"dcQUIC".as_ptr(),
-            c"dcquic".as_ptr(),
+            name.as_ptr() as *const _,
+            name.as_ptr() as *const _,
+            lower.as_ptr() as *const _,
         )
     })
 }

dc/wireshark/src/lib.rs

@@ -63,7 +63,7 @@ unsafe extern "C" fn proto_reg_handoff() {
         c"udp".as_ptr(),
         Some(dissect_heur_udp),
         c"dcQUIC over UDP".as_ptr(),
-        c"dcquic".as_ptr(),
+        concat!(env!("PLUGIN_NAME_LOWER"), "\0").as_ptr() as *const _,
         field::get().protocol,
         wireshark_sys::heuristic_enable_e_HEURISTIC_ENABLE,
     );
@@ -72,7 +72,7 @@ unsafe extern "C" fn proto_reg_handoff() {
         c"tcp".as_ptr(),
         Some(dissect_heur_tcp),
         c"dcQUIC over TCP".as_ptr(),
-        c"dcquic_tcp".as_ptr(),
+        concat!(env!("PLUGIN_NAME_LOWER"), "_tcp\0").as_ptr() as *const _,
         field::get().protocol,
         wireshark_sys::heuristic_enable_e_HEURISTIC_ENABLE,
     );
@@ -82,7 +82,7 @@ unsafe extern "C" fn proto_reg_handoff() {
             wireshark_sys::create_dissector_handle_with_name_and_description(
                 Some(dissect_heur_tcp),
                 field::get().protocol,
-                c"dcquic_tcp_stream".as_ptr(),
+                concat!(env!("PLUGIN_NAME_LOWER"), "_tcp_stream\0").as_ptr() as *const _,
                 c"dcQUIC stream".as_ptr(),
             ),
         )