From 77f80565b6be1f7815b88f79fa204d9b34065f8b Mon Sep 17 00:00:00 2001
From: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Date: Wed, 9 Feb 2022 12:20:05 +0900
Subject: [PATCH] Do not allow missing `attach` attribute for cgroup SKB

Even though cgroup SKB (`BPF_PROG_TYPE_CGROUP_SKB`) has to set Ingress
or Egress, aya allows missing `attach` attribute.
It causes an error and difficult to debug.

This patch adds the validation.
---
 aya/src/obj/mod.rs               |  9 +++++++++
 bpf/aya-bpf-macros/src/expand.rs | 20 +++++++-------------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/aya/src/obj/mod.rs b/aya/src/obj/mod.rs
index a8a07e81..dd1a601d 100644
--- a/aya/src/obj/mod.rs
+++ b/aya/src/obj/mod.rs
@@ -201,6 +201,15 @@ impl FromStr for ProgramSection {
             "classifier" => SchedClassifier { name },
             "cgroup_skb/ingress" => CgroupSkbIngress { name },
             "cgroup_skb/egress" => CgroupSkbEgress { name },
+            "cgroup_skb" => match &*name {
+                "ingress" => CgroupSkbIngress { name },
+                "egress" => CgroupSkbEgress { name },
+                _ => {
+                    return Err(ParseError::InvalidProgramSection {
+                        section: section.to_owned(),
+                    })
+                }
+            },
             "lirc_mode2" => LircMode2 { name },
             "perf_event" => PerfEvent { name },
             "raw_tp" | "raw_tracepoint" => RawTracePoint { name },
diff --git a/bpf/aya-bpf-macros/src/expand.rs b/bpf/aya-bpf-macros/src/expand.rs
index ed10ca3f..44b24ce5 100644
--- a/bpf/aya-bpf-macros/src/expand.rs
+++ b/bpf/aya-bpf-macros/src/expand.rs
@@ -238,10 +238,8 @@ impl CgroupSkb {
             } else {
                 format!("cgroup_skb/{}", attach)
             }
-        } else if let Some(name) = &self.name {
-            format!("cgroup/skb/{}", name)
         } else {
-            ("cgroup/skb").to_owned()
+            return Err(Error::new_spanned(&self.expected_attach_type, "missing attach type"));
         };
         let fn_name = &self.item.sig.ident;
         let item = &self.item;
@@ -611,7 +609,7 @@ mod tests {
     }
 
     #[test]
-    fn cgroup_skb_with_name() {
+    fn cgroup_skb_no_attach() {
         let prog = CgroupSkb::from_syn(
             parse_quote!(name = "foo"),
             parse_quote!(
@@ -621,14 +619,12 @@ mod tests {
             ),
         )
         .unwrap();
-        let stream = prog.expand().unwrap();
-        assert!(stream
-            .to_string()
-            .contains("[link_section = \"cgroup/skb/foo\"]"));
+        let err = prog.expand().unwrap_err();
+        assert_eq!(err.to_string(), "missing attach type")
     }
 
     #[test]
-    fn cgroup_skb_no_name() {
+    fn cgroup_skb_no_name_no_attach() {
         let prog = CgroupSkb::from_syn(
             parse_quote!(),
             parse_quote!(
@@ -638,10 +634,8 @@ mod tests {
             ),
         )
         .unwrap();
-        let stream = prog.expand().unwrap();
-        assert!(stream
-            .to_string()
-            .contains("[link_section = \"cgroup/skb\"]"));
+        let err = prog.expand().unwrap_err();
+        assert_eq!(err.to_string(), "missing attach type")
     }
 
     #[test]