Update bpfman to use a priority-based API for TCX

Also add a tcx bpf test program and use it in the tcx integration tests Move our github actions to Ubuntu 24.04 to get a kernel that supports tcx Signed-off-by: Andre Fredette <[email protected]>
bpfman · Aug 26, 2024 · a058491 · a058491
1 parent ef0b262
commit a058491
Show file tree

Hide file tree

Showing 30 changed files with 1,070 additions and 860 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -15,7 +15,7 @@ env:
 
 jobs:
   check-license:
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     timeout-minutes: 3
 
     steps:
@@ -44,7 +44,7 @@ jobs:
             # rust-target: s390x-unknown-linux-gnu
             # filename: linux-s390x
             # command: cross
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
     steps:
       - name: Install dependencies
         # Dependencies only needed to build eBPF and used by integration-tests, only on amd64
@@ -274,7 +274,7 @@ jobs:
             filename: linux-ppc64le
           - arch: s390x
             filename: linux-s390x
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Checkout bpfman
         uses: actions/checkout@v4
@@ -407,7 +407,7 @@ jobs:
         run: cargo xtask integration-test
 
   build-docs:
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Checkout bpfman
         uses: actions/checkout@v4
@@ -425,7 +425,7 @@ jobs:
 
   coverage:
     needs: [build, build-go]
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Download rust coverage artifacts
         uses: actions/download-artifact@v4
@@ -445,7 +445,7 @@ jobs:
     if: startsWith(github.ref, 'refs/tags/v')
     needs: [build]
     environment: crates.io
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Checkout bpfman
         uses: actions/checkout@v4
@@ -495,7 +495,7 @@ jobs:
         coverage,
         basic-integration-tests,
       ]
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Build Complete
         run: echo "Build Complete"
diff --git a/.github/workflows/docs-build.yaml b/.github/workflows/docs-build.yaml
@@ -8,7 +8,7 @@ on: # yamllint disable-line rule:truthy
 
 jobs:
   build-docs:
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     timeout-minutes: 3
     steps:
       - uses: actions/checkout@v4

diff --git a/.github/workflows/image-build.yaml b/.github/workflows/image-build.yaml
@@ -11,7 +11,7 @@ on: # yamllint disable-line rule:truthy
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     steps:
       - name: Checkout bpfman
         uses: actions/checkout@v4
@@ -37,7 +37,7 @@ jobs:
       packages: write
       id-token: write # needed for signing the images with GitHub OIDC Token
 
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     strategy:
       fail-fast: false
       matrix:
@@ -233,7 +233,7 @@ jobs:
       packages: write
       id-token: write # needed for signing the images with GitHub OIDC Token
 
-    runs-on: ubuntu-latest
+    runs-on: runs-on: ubuntu-24.04
     strategy:
       fail-fast: false
       matrix:
@@ -398,6 +398,22 @@ jobs:
               # set latest tag for default branch
               type=raw,value=latest,enable={{is_default_branch}}
 
+          - registry: quay.io
+            build_language: rust
+            bpf_build_wrapper: rust
+            repository: bpfman-bytecode
+            image: tcx_test
+            context: .
+            dockerfile: ./Containerfile.bytecode.multi.arch
+            bytecode_dir: ./tests/integration-test/bpf/.output/tcx_test.bpf
+            tags: |
+              type=ref,event=branch
+              type=ref,event=tag
+              type=ref,event=pr
+              type=sha,format=long
+              # set latest tag for default branch
+              type=raw,value=latest,enable={{is_default_branch}}
+
           - registry: quay.io
             build_language: rust
             bpf_build_wrapper: rust

diff --git a/bpfman-api/src/bin/rpc/rpc.rs b/bpfman-api/src/bin/rpc/rpc.rs
@@ -3,9 +3,9 @@
 use bpfman::{
     add_program, get_program, list_programs, pull_bytecode, remove_program,
     types::{
-        FentryProgram, FexitProgram, KprobeProgram, LinkAction, ListFilter, Location, Program,
-        ProgramData, TcProceedOn, TcProgram, TcxProgram, TracepointProgram, UprobeProgram,
-        XdpProceedOn, XdpProgram,
+        FentryProgram, FexitProgram, KprobeProgram, ListFilter, Location, Program, ProgramData,
+        TcProceedOn, TcProgram, TcxProgram, TracepointProgram, UprobeProgram, XdpProceedOn,
+        XdpProgram,
     },
 };
 use bpfman_api::v1::{
@@ -93,35 +93,19 @@ impl Bpfman for BpfmanLoader {
                 )
             }
             Info::TcxAttachInfo(TcxAttachInfo {
+                priority,
                 iface,
+                position: _,
                 direction,
-                link_order,
             }) => {
                 let direction = direction
                     .try_into()
                     .map_err(|_| Status::aborted("direction is not a string"))?;
-
-                if let Some(lo) = link_order {
-                    let action = if let Some(a) = lo.action {
-                        Some(LinkAction::try_from(a.to_string()).map_err(|e| {
-                            Status::aborted(format!("failed to parse link order action: {e}"))
-                        })?)
-                    } else {
-                        None
-                    };
-                    Program::Tcx(
-                        TcxProgram::new(data, iface, direction, lo.id, lo.fd, action, lo.revision)
-                            .map_err(|e| {
-                                Status::aborted(format!("failed to create tcxprogram: {e}"))
-                            })?,
-                    )
-                } else {
-                    Program::Tcx(
-                        TcxProgram::new(data, iface, direction, None, None, None, None).map_err(
-                            |e| Status::aborted(format!("failed to create tcxprogram: {e}")),
-                        )?,
-                    )
-                }
+                Program::Tcx(
+                    TcxProgram::new(data, priority, iface, direction).map_err(|e| {
+                        Status::aborted(format!("failed to create tcxprogram: {e}"))
+                    })?,
+                )
             }
             Info::TracepointAttachInfo(TracepointAttachInfo { tracepoint }) => Program::Tracepoint(
                 TracepointProgram::new(data, tracepoint)

diff --git a/bpfman-api/src/bpfman.v1.rs b/bpfman-api/src/bpfman.v1.rs
@@ -118,25 +118,15 @@ pub struct TcAttachInfo {
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
-pub struct LinkOrder {
-    #[prost(uint32, optional, tag = "1")]
-    pub id: ::core::option::Option<u32>,
-    #[prost(int32, optional, tag = "2")]
-    pub fd: ::core::option::Option<i32>,
-    #[prost(string, optional, tag = "3")]
-    pub action: ::core::option::Option<::prost::alloc::string::String>,
-    #[prost(uint64, optional, tag = "4")]
-    pub revision: ::core::option::Option<u64>,
-}
-#[allow(clippy::derive_partial_eq_without_eq)]
-#[derive(Clone, PartialEq, ::prost::Message)]
 pub struct TcxAttachInfo {
-    #[prost(string, tag = "1")]
-    pub iface: ::prost::alloc::string::String,
+    #[prost(int32, tag = "1")]
+    pub priority: i32,
     #[prost(string, tag = "2")]
+    pub iface: ::prost::alloc::string::String,
+    #[prost(int32, tag = "3")]
+    pub position: i32,
+    #[prost(string, tag = "4")]
     pub direction: ::prost::alloc::string::String,
-    #[prost(message, optional, tag = "3")]
-    pub link_order: ::core::option::Option<LinkOrder>,
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]

diff --git a/bpfman-api/src/lib.rs b/bpfman-api/src/lib.rs
@@ -5,7 +5,7 @@ use bpfman::{
     errors::BpfmanError,
     types::{BytecodeImage, Location, Program},
 };
-use v1::{LinkOrder, TcxAttachInfo};
+use v1::TcxAttachInfo;
 
 use crate::v1::{
     attach_info::Info, bytecode_location::Location as V1Location, AttachInfo,
@@ -58,30 +58,12 @@ impl TryFrom<&Program> for ProgramInfo {
                     direction: p.get_direction()?.to_string(),
                     proceed_on: p.get_proceed_on()?.as_action_vec(),
                 })),
-                Program::Tcx(p) => {
-                    let id = p.get_id()?;
-                    let fd = p.get_fd()?;
-                    let action = p.get_action()?;
-                    let revision = p.get_revision()?;
-
-                    let link_order: Option<LinkOrder> =
-                        if id.is_none() && fd.is_none() && action.is_none() && revision.is_none() {
-                            None
-                        } else {
-                            Some(LinkOrder {
-                                id,
-                                fd,
-                                action: action.map(|a| a.to_string()),
-                                revision,
-                            })
-                        };
-
-                    Some(Info::TcxAttachInfo(TcxAttachInfo {
-                        iface: p.get_iface()?.to_string(),
-                        direction: p.get_direction()?.to_string(),
-                        link_order,
-                    }))
-                }
+                Program::Tcx(p) => Some(Info::TcxAttachInfo(TcxAttachInfo {
+                    priority: p.get_priority()?,
+                    iface: p.get_iface()?.to_string(),
+                    position: p.get_current_position()?.unwrap_or(0) as i32,
+                    direction: p.get_direction()?.to_string(),
+                })),
                 Program::Tracepoint(p) => Some(Info::TracepointAttachInfo(TracepointAttachInfo {
                     tracepoint: p.get_tracepoint()?.to_string(),
                 })),

diff --git a/bpfman/src/bin/cli/args.rs b/bpfman/src/bin/cli/args.rs
@@ -198,22 +198,11 @@ pub(crate) enum LoadCommands {
         #[clap(short, long)]
         iface: String,
 
-        /// Optional: ID of anchor program.
-        #[clap(long, verbatim_doc_comment)]
-        id: Option<u32>,
-
-        /// Optional: File descriptor of anchor program.
-        #[clap(short, long, verbatim_doc_comment)]
-        fd: Option<i32>,
-
-        /// Optional: Position relative to the anchor program.
-        #[clap(short, long, verbatim_doc_comment)]
-        action: Option<String>,
-
-        /// Optional: Expected revision.
-        /// [possible values: after, before, replace]
-        #[clap(short, long, verbatim_doc_comment)]
-        revision: Option<u64>,
+        /// Optional: Priority to run program in chain. Lower value runs first.
+        /// [possible values: 1-1000]
+        /// [default: 1000]
+        #[clap(short, long)]
+        priority: i32,
     },
     #[command(disable_version_flag = true)]
     /// Install an eBPF program on a Tracepoint.

diff --git a/bpfman/src/bin/cli/load.rs b/bpfman/src/bin/cli/load.rs
@@ -7,9 +7,8 @@ use anyhow::bail;
 use bpfman::{
     add_program,
     types::{
-        FentryProgram, FexitProgram, KprobeProgram, LinkAction, Location, Program, ProgramData,
-        TcProceedOn, TcProgram, TcxProgram, TracepointProgram, UprobeProgram, XdpProceedOn,
-        XdpProgram,
+        FentryProgram, FexitProgram, KprobeProgram, Location, Program, ProgramData, TcProceedOn,
+        TcProgram, TcxProgram, TracepointProgram, UprobeProgram, XdpProceedOn, XdpProgram,
     },
 };
 
@@ -117,28 +116,17 @@ impl LoadCommands {
             LoadCommands::Tcx {
                 direction,
                 iface,
-                id,
-                fd,
-                action,
-                revision,
+                priority,
             } => {
                 match direction.as_str() {
                     "ingress" | "egress" => (),
                     other => bail!("{} is not a valid direction", other),
                 };
-                let action = if let Some(a) = action {
-                    Some(LinkAction::try_from(a.to_string())?)
-                } else {
-                    None
-                };
                 Ok(Program::Tcx(TcxProgram::new(
                     data,
+                    *priority,
                     iface.to_string(),
                     direction.to_string().try_into()?,
-                    *id,
-                    *fd,
-                    action,
-                    *revision,
                 )?))
             }
             LoadCommands::Tracepoint { tracepoint } => Ok(Program::Tracepoint(

diff --git a/bpfman/src/bin/cli/table.rs b/bpfman/src/bin/cli/table.rs
@@ -130,28 +130,16 @@ impl ProgTable {
             }
             Program::Tcx(p) => {
                 table.add_row(vec!["Attach Type:", "tcx"]);
+                table.add_row(vec!["Priority:", &p.get_priority()?.to_string()]);
                 table.add_row(vec!["Iface:", &p.get_iface()?]);
+                table.add_row(vec![
+                    "Position:",
+                    &match p.get_current_position()? {
+                        Some(pos) => pos.to_string(),
+                        None => "NONE".to_string(),
+                    },
+                ]);
                 table.add_row(vec!["Direction:", &p.get_direction()?.to_string()]);
-                if let Some(id) = &p.get_id()? {
-                    table.add_row(vec!["ID:", &id.to_string()]);
-                } else {
-                    table.add_row(vec!["ID:", "None"]);
-                }
-                if let Some(fd) = &p.get_fd()? {
-                    table.add_row(vec!["FD:", &fd.to_string()]);
-                } else {
-                    table.add_row(vec!["FD:", "None"]);
-                }
-                if let Some(action) = &p.get_action()? {
-                    table.add_row(vec!["Action:", &action.to_string()]);
-                } else {
-                    table.add_row(vec!["Action:", "None"]);
-                }
-                if let Some(revision) = &p.get_revision()? {
-                    table.add_row(vec!["Revision:", &revision.to_string()]);
-                } else {
-                    table.add_row(vec!["Revision:", "None"]);
-                }
             }
             Program::Tracepoint(p) => {
                 table.add_row(vec!["Tracepoint:", &p.get_tracepoint()?]);

diff --git a/bpfman/src/errors.rs b/bpfman/src/errors.rs
@@ -71,6 +71,10 @@ pub enum BpfmanError {
     DatabaseLockError,
     #[error("dispatcher failed to load. {0}")]
     DispatcherLoadError(String),
+    #[error("Invalid priority {0}")]
+    InvalidPriority(i32),
+    #[error("direction not found")]
+    InvalidDirection,
 }
 
 #[derive(Error, Debug)]