Macros: introduce a macro to simplify enums

13 files changed, 293 insertions, 281 deletions

diff --git a/macros/Cargo.toml b/macros/Cargo.toml
index 7d9167f..82c8ad6 100644
--- a/macros/Cargo.toml
+++ b/macros/Cargo.toml
@@ -7,7 +7,7 @@ edition = "2021"
 proc-macro = true
 
 [dependencies]
-syn = { version = "1.0", features = ["full", "extra-traits"] }
+syn = { version = "1.0", features = ["full"] }
 quote = "1.0"
 proc-macro2 = "1.0"
 proc-macro-error = "1"
diff --git a/macros/src/lib.rs b/macros/src/lib.rs
index 38cde50..11aedaf 100644
--- a/macros/src/lib.rs
+++ b/macros/src/lib.rs
@@ -1,28 +1,26 @@
 use proc_macro::TokenStream;
-use proc_macro2::Group;
+use proc_macro2::{Group, Span};
 use quote::quote;
 
 use proc_macro_error::{abort, proc_macro_error};
 use syn::parse::Parser;
 use syn::punctuated::Punctuated;
 use syn::spanned::Spanned;
-use syn::token::Struct;
 use syn::{
-    parse, parse2, parse_macro_input, Attribute, Expr, ExprLit, FnArg, Ident, ItemFn, ItemStruct,
-    Lit, Meta, NestedMeta, Pat, PatIdent, Path, Result, ReturnType, Token, Type, TypePath,
+    parse, parse2, Attribute, Expr, ExprCast, Ident, ItemEnum, ItemStruct, Lit, Meta, Path, Result,
+    Token, Type, TypePath, Visibility,
 };
-use syn::{parse::Parse, PatReference};
-use syn::{parse::ParseStream, TypeReference};
 
 struct Field<'a> {
     name: &'a Ident,
     ty: &'a Type,
     args: FieldArgs,
     netlink_type: Path,
+    vis: &'a Visibility,
     attrs: Vec<&'a Attribute>,
 }
 
-#[derive(Debug, Default)]
+#[derive(Default)]
 struct FieldArgs {
     netlink_type: Option<Path>,
     override_function_name: Option<String>,
@@ -68,7 +66,6 @@ fn parse_field_args(input: proc_macro2::TokenStream) -> Result<FieldArgs> {
     Ok(args)
 }
 
-#[derive(Debug)]
 struct StructArgs {
     nested: bool,
     derive_decoder: bool,
@@ -85,12 +82,10 @@ impl Default for StructArgs {
     }
 }
 
-fn parse_struct_args(args: &mut StructArgs, input: TokenStream) -> Result<()> {
-    if input.is_empty() {
-        return Ok(());
-    }
+fn parse_struct_args(input: TokenStream) -> Result<StructArgs> {
+    let mut args = StructArgs::default();
     let parser = Punctuated::<Meta, Token![,]>::parse_terminated;
-    let attribute_args = parser.parse(input)?;
+    let attribute_args = parser.parse(input.clone())?;
     for arg in attribute_args.iter() {
         if let Meta::NameValue(namevalue) = arg {
             let key = namevalue
@@ -126,7 +121,7 @@ fn parse_struct_args(args: &mut StructArgs, input: TokenStream) -> Result<()> {
             abort!(arg.span(), "Unrecognized argument");
         }
     }
-    Ok(())
+    Ok(args)
 }
 
 #[proc_macro_error]
@@ -135,8 +130,10 @@ pub fn nfnetlink_struct(attrs: TokenStream, item: TokenStream) -> TokenStream {
     let ast: ItemStruct = parse(item).unwrap();
     let name = ast.ident;
 
-    let mut args = StructArgs::default();
-    parse_struct_args(&mut args, attrs).expect("Could not parse the macro arguments");
+    let args = match parse_struct_args(attrs) {
+        Ok(x) => x,
+        Err(_) => abort!(Span::call_site(), "Could not parse the macro arguments"),
+    };
 
     let mut fields = Vec::with_capacity(ast.fields.len());
     let mut identical_fields = Vec::new();
@@ -145,15 +142,25 @@ pub fn nfnetlink_struct(attrs: TokenStream, item: TokenStream) -> TokenStream {
         for attr in field.attrs.iter() {
             if let Some(id) = attr.path.get_ident() {
                 if id == "field" {
-                    let field_args = parse_field_args(attr.tokens.clone())
-                        .expect("Could not parse the field attributes");
+                    let field_args = match parse_field_args(attr.tokens.clone()) {
+                        Ok(x) => x,
+                        Err(_) => {
+                            abort!(attr.tokens.span(), "Could not parse the field attributes")
+                        }
+                    };
                     if let Some(netlink_type) = field_args.netlink_type.clone() {
                         fields.push(Field {
                             name: field.ident.as_ref().expect("Should be a names struct"),
                             ty: &field.ty,
                             args: field_args,
                             netlink_type,
-                            attrs: field.attrs.iter().filter(|x| *x != attr).collect(),
+                            vis: &field.vis,
+                            // drop the "field" attribute
+                            attrs: field
+                                .attrs
+                                .iter()
+                                .filter(|x| x.path.get_ident() != attr.path.get_ident())
+                                .collect(),
                         });
                     } else {
                         abort!(attr.tokens.span(), "Missing Netlink Type in field");
@@ -297,7 +304,8 @@ pub fn nfnetlink_struct(attrs: TokenStream, item: TokenStream) -> TokenStream {
         let name = field.name;
         let ty = field.ty;
         let attrs = &field.attrs;
-        quote!( #(#attrs) * #name: Option<#ty>, )
+        let vis = &field.vis;
+        quote!( #(#attrs) * #vis #name: Option<#ty>, )
     });
     let nfnetlinkdeserialize_impl = if args.derive_deserialize {
         quote!(
@@ -327,3 +335,149 @@ pub fn nfnetlink_struct(attrs: TokenStream, item: TokenStream) -> TokenStream {
 
     res.into()
 }
+
+struct Variant<'a> {
+    inner: &'a syn::Variant,
+    name: &'a Ident,
+    value: &'a Path,
+}
+
+#[derive(Default)]
+struct EnumArgs {
+    nested: bool,
+    ty: Option<Path>,
+}
+
+fn parse_enum_args(input: TokenStream) -> Result<EnumArgs> {
+    let mut args = EnumArgs::default();
+    let parser = Punctuated::<Meta, Token![,]>::parse_terminated;
+    let attribute_args = parser.parse(input)?;
+    for arg in attribute_args.iter() {
+        match arg {
+            Meta::Path(path) => {
+                if args.ty.is_none() {
+                    args.ty = Some(path.clone());
+                } else {
+                    abort!(arg.span(), "A value can only have a single representation");
+                }
+            }
+            Meta::NameValue(namevalue) => {
+                let key = namevalue
+                    .path
+                    .get_ident()
+                    .expect("the macro parameter is not an ident?")
+                    .to_string();
+                match key.as_str() {
+                    "nested" => {
+                        if let Lit::Bool(boolean) = &namevalue.lit {
+                            args.nested = boolean.value;
+                        } else {
+                            abort!(&namevalue.lit.span(), "Expected a boolean");
+                        }
+                    }
+                    _ => abort!(key.span(), "Unsupported macro parameter"),
+                }
+            }
+            _ => abort!(arg.span(), "Unrecognized argument"),
+        }
+    }
+    Ok(args)
+}
+
+#[proc_macro_error]
+#[proc_macro_attribute]
+pub fn nfnetlink_enum(attrs: TokenStream, item: TokenStream) -> TokenStream {
+    let ast: ItemEnum = parse(item).unwrap();
+    let name = ast.ident;
+
+    let args = match parse_enum_args(attrs) {
+        Ok(x) => x,
+        Err(_) => abort!(Span::call_site(), "Could not parse the macro arguments"),
+    };
+
+    if args.ty.is_none() {
+        abort!(
+            Span::call_site(),
+            "The target type representation is unspecified"
+        );
+    }
+
+    let mut variants = Vec::with_capacity(ast.variants.len());
+
+    for variant in ast.variants.iter() {
+        if variant.discriminant.is_none() {
+            abort!(variant.ident.span(), "Missing value");
+        }
+        let discriminant = variant.discriminant.as_ref().unwrap();
+        if let syn::Expr::Path(path) = &discriminant.1 {
+            variants.push(Variant {
+                inner: variant,
+                name: &variant.ident,
+                value: &path.path,
+            });
+        } else {
+            abort!(discriminant.1.span(), "Expected a path");
+        }
+    }
+
+    let repr_type = args.ty.unwrap();
+    let match_entries = variants.iter().map(|variant| {
+        let variant_name = variant.name;
+        let variant_value = &variant.value;
+        quote!( x if x == (#variant_value as #repr_type) => Self::#variant_name, )
+    });
+    let unknown_type_ident = Ident::new(&format!("Unknown{}", name.to_string()), name.span());
+    let nfnetlinkdeserialize_impl = quote!(
+        impl crate::nlmsg::NfNetlinkDeserializable for #name {
+            fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), crate::parser::DecodeError> {
+                let (v, remaining_data) = #repr_type::deserialize(buf)?;
+                Ok((
+                    match v {
+                        #(#match_entries) *
+                        value => return Err(crate::parser::DecodeError::#unknown_type_ident(value))
+                    },
+                    remaining_data,
+                ))
+            }
+        }
+    );
+    let vis = &ast.vis;
+    let attrs = ast.attrs;
+    let original_variants = variants.into_iter().map(|x| {
+        let mut inner = x.inner.clone();
+        let mut discriminant = inner.discriminant.as_mut().unwrap();
+        let cur_value = discriminant.1.clone();
+        let cast_value = Expr::Cast(ExprCast {
+            attrs: vec![],
+            expr: Box::new(cur_value),
+            as_token: Token![as](name.span()),
+            ty: Box::new(Type::Path(TypePath {
+                qself: None,
+                path: repr_type.clone(),
+            })),
+        });
+        discriminant.1 = cast_value;
+        inner
+    });
+    let res = quote! {
+        #[repr(#repr_type)]
+        #(#attrs) * #vis enum #name {
+            #(#original_variants),*
+        }
+
+        impl crate::nlmsg::NfNetlinkAttribute for #name {
+            fn get_size(&self) -> usize {
+                (*self as #repr_type).get_size()
+            }
+
+            unsafe fn write_payload(&self, addr: *mut u8) {
+                (*self as #repr_type).write_payload(addr);
+            }
+        }
+
+        #nfnetlinkdeserialize_impl
+
+    };
+
+    res.into()
+}
diff --git a/src/chain_methods.rs b/src/chain_methods.rs
new file mode 100644
index 0000000..d384c35
--- /dev/null
+++ b/src/chain_methods.rs
@@ -0,0 +1,40 @@
+use crate::{Batch, Chain, Hook, MsgType, Policy, Table};
+use std::ffi::CString;
+use std::rc::Rc;
+
+
+/// A helper trait over [`crate::Chain`].
+pub trait ChainMethods {
+    /// Creates a new Chain instance from a [`crate::Hook`] over a [`crate::Table`].
+    fn from_hook(hook: Hook, table: Rc<Table>) -> Self
+        where Self: std::marker::Sized;
+    /// Adds a [`crate::Policy`] to the current Chain.
+    fn verdict(self, policy: Policy) -> Self;
+    fn add_to_batch(self, batch: &mut Batch) -> Self;
+}
+
+
+impl ChainMethods for Chain {
+    fn from_hook(hook: Hook, table: Rc<Table>) -> Self {
+        let chain_name = match hook {
+            Hook::PreRouting => "prerouting",
+            Hook::Out => "out",
+            Hook::PostRouting => "postrouting",
+            Hook::Forward => "forward",
+            Hook::In => "in",
+        };
+        let chain_name = CString::new(chain_name).unwrap();
+        let mut chain = Chain::new(&chain_name, table);
+        chain.set_hook(hook, 0);
+        chain
+    }
+    fn verdict(mut self, policy: Policy) -> Self {
+        self.set_policy(policy);
+        self
+    }
+    fn add_to_batch(self, batch: &mut Batch) -> Self {
+        batch.add(&self, MsgType::Add);
+        self
+    }
+}
+
diff --git a/src/expr/counter.rs b/src/expr/counter.rs
index 4732e85..d22fb8a 100644
--- a/src/expr/counter.rs
+++ b/src/expr/counter.rs
@@ -1,46 +1,21 @@
-use super::{DeserializationError, Expression, Rule};
+use rustables_macros::nfnetlink_struct;
+
+use super::Expression;
 use crate::sys;
-use std::os::raw::c_char;
 
 /// A counter expression adds a counter to the rule that is incremented to count number of packets
 /// and number of bytes for all packets that have matched the rule.
-#[derive(Debug, PartialEq)]
+#[derive(Default, Clone, Debug, PartialEq, Eq)]
+#[nfnetlink_struct]
 pub struct Counter {
+    #[field(sys::NFTA_COUNTER_BYTES)]
     pub nb_bytes: u64,
+    #[field(sys::NFTA_COUNTER_PACKETS)]
     pub nb_packets: u64,
 }
 
-impl Counter {
-    pub fn new() -> Self {
-        Self {
-            nb_bytes: 0,
-            nb_packets: 0,
-        }
-    }
-}
-
 impl Expression for Counter {
-    fn get_raw_name() -> *const c_char {
-        b"counter\0" as *const _ as *const c_char
-    }
-
-    fn from_expr(expr: *const sys::nftnl_expr) -> Result<Self, DeserializationError> {
-        unsafe {
-            let nb_bytes = sys::nftnl_expr_get_u64(expr, sys::NFTNL_EXPR_CTR_BYTES as u16);
-            let nb_packets = sys::nftnl_expr_get_u64(expr, sys::NFTNL_EXPR_CTR_PACKETS as u16);
-            Ok(Counter {
-                nb_bytes,
-                nb_packets,
-            })
-        }
-    }
-
-    fn to_expr(&self, _rule: &Rule) -> *mut sys::nftnl_expr {
-        unsafe {
-            let expr = try_alloc!(sys::nftnl_expr_alloc(Self::get_raw_name()));
-            sys::nftnl_expr_set_u64(expr, sys::NFTNL_EXPR_CTR_BYTES as u16, self.nb_bytes);
-            sys::nftnl_expr_set_u64(expr, sys::NFTNL_EXPR_CTR_PACKETS as u16, self.nb_packets);
-            expr
-        }
+    fn get_name() -> &'static str {
+        "counter"
     }
 }
diff --git a/src/expr/log.rs b/src/expr/log.rs
index 3c72257..80bb7a9 100644
--- a/src/expr/log.rs
+++ b/src/expr/log.rs
@@ -8,7 +8,7 @@ use crate::sys::{NFTA_LOG_GROUP, NFTA_LOG_PREFIX};
 /// A Log expression will log all packets that match the rule.
 pub struct Log {
     #[field(NFTA_LOG_GROUP)]
-    group: u32,
+    group: u16,
     #[field(NFTA_LOG_PREFIX)]
     prefix: String,
 }
diff --git a/src/expr/meta.rs b/src/expr/meta.rs
index c4c1adb..79016bd 100644
--- a/src/expr/meta.rs
+++ b/src/expr/meta.rs
@@ -1,15 +1,11 @@
-use rustables_macros::nfnetlink_struct;
+use rustables_macros::{nfnetlink_enum, nfnetlink_struct};
 
 use super::{Expression, Register};
-use crate::{
-    nlmsg::{NfNetlinkAttribute, NfNetlinkDeserializable},
-    parser::DecodeError,
-    sys,
-};
+use crate::sys;
 
 /// A meta expression refers to meta data associated with a packet.
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
-#[repr(u32)]
+#[nfnetlink_enum(u32)]
 #[non_exhaustive]
 pub enum MetaType {
     /// Packet ethertype protocol (skb->protocol), invalid in OUTPUT.
@@ -42,42 +38,6 @@ pub enum MetaType {
     PRandom = sys::NFT_META_PRANDOM,
 }
 
-impl NfNetlinkAttribute for MetaType {
-    fn get_size(&self) -> usize {
-        (*self as u32).get_size()
-    }
-
-    unsafe fn write_payload(&self, addr: *mut u8) {
-        (*self as u32).write_payload(addr);
-    }
-}
-
-impl NfNetlinkDeserializable for MetaType {
-    fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), DecodeError> {
-        let (v, remaining_data) = u32::deserialize(buf)?;
-        Ok((
-            match v {
-                sys::NFT_META_PROTOCOL => Self::Protocol,
-                sys::NFT_META_MARK => Self::Mark,
-                sys::NFT_META_IIF => Self::Iif,
-                sys::NFT_META_OIF => Self::Oif,
-                sys::NFT_META_IIFNAME => Self::IifName,
-                sys::NFT_META_OIFNAME => Self::OifName,
-                sys::NFT_META_IFTYPE => Self::IifType,
-                sys::NFT_META_OIFTYPE => Self::OifType,
-                sys::NFT_META_SKUID => Self::SkUid,
-                sys::NFT_META_SKGID => Self::SkGid,
-                sys::NFT_META_NFPROTO => Self::NfProto,
-                sys::NFT_META_L4PROTO => Self::L4Proto,
-                sys::NFT_META_CGROUP => Self::Cgroup,
-                sys::NFT_META_PRANDOM => Self::PRandom,
-                value => return Err(DecodeError::UnknownMetaType(value)),
-            },
-            remaining_data,
-        ))
-    }
-}
-
 #[derive(Clone, PartialEq, Eq, Default, Debug)]
 #[nfnetlink_struct]
 pub struct Meta {
diff --git a/src/expr/mod.rs b/src/expr/mod.rs
index 63385e0..d2cd917 100644
--- a/src/expr/mod.rs
+++ b/src/expr/mod.rs
@@ -25,9 +25,11 @@ pub use self::bitwise::*;
 /*
 mod cmp;
 pub use self::cmp::*;
+*/
 
 mod counter;
 pub use self::counter::*;
+/*
 
 pub mod ct;
 pub use self::ct::*;
@@ -222,7 +224,8 @@ create_expr_variant!(
     [Bitwise, Bitwise],
     [ExpressionRaw, ExpressionRaw],
     [Meta, Meta],
-    [Reject, Reject]
+    [Reject, Reject],
+    [Counter, Counter]
 );
 
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
diff --git a/src/expr/register.rs b/src/expr/register.rs
index def58a5..9cc1bee 100644
--- a/src/expr/register.rs
+++ b/src/expr/register.rs
@@ -1,15 +1,13 @@
 use std::fmt::Debug;
 
-use crate::{
-    nlmsg::{NfNetlinkAttribute, NfNetlinkDeserializable},
-    parser::DecodeError,
-    sys::{NFT_REG_1, NFT_REG_2, NFT_REG_3, NFT_REG_4, NFT_REG_VERDICT},
-};
+use rustables_macros::nfnetlink_enum;
+
+use crate::sys::{NFT_REG_1, NFT_REG_2, NFT_REG_3, NFT_REG_4, NFT_REG_VERDICT};
 
 /// A netfilter data register. The expressions store and read data to and from these when
 /// evaluating rule statements.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-#[repr(u32)]
+#[nfnetlink_enum(u32)]
 pub enum Register {
     Verdict = NFT_REG_VERDICT,
     Reg1 = NFT_REG_1,
@@ -17,26 +15,3 @@ pub enum Register {
     Reg3 = NFT_REG_3,
     Reg4 = NFT_REG_4,
 }
-
-impl NfNetlinkAttribute for Register {
-    unsafe fn write_payload(&self, addr: *mut u8) {
-        (*self as u32).write_payload(addr);
-    }
-}
-
-impl NfNetlinkDeserializable for Register {
-    fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), crate::parser::DecodeError> {
-        let (val, remaining) = u32::deserialize(buf)?;
-        Ok((
-            match val {
-                NFT_REG_VERDICT => Self::Verdict,
-                NFT_REG_1 => Self::Reg1,
-                NFT_REG_2 => Self::Reg2,
-                NFT_REG_3 => Self::Reg3,
-                NFT_REG_4 => Self::Reg4,
-                _ => return Err(DecodeError::UnknownRegisterValue),
-            },
-            remaining,
-        ))
-    }
-}
diff --git a/src/expr/reject.rs b/src/expr/reject.rs
index 10b95ea..83fd843 100644
--- a/src/expr/reject.rs
+++ b/src/expr/reject.rs
@@ -1,10 +1,6 @@
-use rustables_macros::nfnetlink_struct;
+use rustables_macros::{nfnetlink_enum, nfnetlink_struct};
 
-use crate::{
-    nlmsg::{NfNetlinkAttribute, NfNetlinkDeserializable},
-    parser::DecodeError,
-    sys,
-};
+use crate::sys;
 
 use super::Expression;
 
@@ -26,70 +22,19 @@ pub struct Reject {
 
 /// An ICMP reject code.
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
-#[repr(u32)]
+#[nfnetlink_enum(u32)]
 pub enum RejectType {
     IcmpUnreach = sys::NFT_REJECT_ICMP_UNREACH,
     TcpRst = sys::NFT_REJECT_TCP_RST,
     IcmpxUnreach = sys::NFT_REJECT_ICMPX_UNREACH,
 }
 
-impl NfNetlinkDeserializable for RejectType {
-    fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), DecodeError> {
-        let (v, remaining_code) = u32::deserialize(buf)?;
-        Ok((
-            match v {
-                sys::NFT_REJECT_ICMP_UNREACH => Self::IcmpUnreach,
-                sys::NFT_REJECT_TCP_RST => Self::TcpRst,
-                sys::NFT_REJECT_ICMPX_UNREACH => Self::IcmpxUnreach,
-                _ => return Err(DecodeError::UnknownRejectType(v)),
-            },
-            remaining_code,
-        ))
-    }
-}
-
-impl NfNetlinkAttribute for RejectType {
-    fn get_size(&self) -> usize {
-        (*self as u32).get_size()
-    }
-
-    unsafe fn write_payload(&self, addr: *mut u8) {
-        (*self as u32).write_payload(addr);
-    }
-}
-
 /// An ICMP reject code.
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
-#[repr(u8)]
+#[nfnetlink_enum(u8)]
 pub enum IcmpCode {
-    NoRoute = sys::NFT_REJECT_ICMPX_NO_ROUTE as u8,
-    PortUnreach = sys::NFT_REJECT_ICMPX_PORT_UNREACH as u8,
-    HostUnreach = sys::NFT_REJECT_ICMPX_HOST_UNREACH as u8,
-    AdminProhibited = sys::NFT_REJECT_ICMPX_ADMIN_PROHIBITED as u8,
-}
-
-impl NfNetlinkDeserializable for IcmpCode {
-    fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), DecodeError> {
-        let (value, remaining_code) = u8::deserialize(buf)?;
-        Ok((
-            match value as u32 {
-                sys::NFT_REJECT_ICMPX_NO_ROUTE => Self::NoRoute,
-                sys::NFT_REJECT_ICMPX_PORT_UNREACH => Self::PortUnreach,
-                sys::NFT_REJECT_ICMPX_HOST_UNREACH => Self::HostUnreach,
-                sys::NFT_REJECT_ICMPX_ADMIN_PROHIBITED => Self::AdminProhibited,
-                _ => return Err(DecodeError::UnknownIcmpCode(value)),
-            },
-            remaining_code,
-        ))
-    }
-}
-
-impl NfNetlinkAttribute for IcmpCode {
-    fn get_size(&self) -> usize {
-        (*self as u8).get_size()
-    }
-
-    unsafe fn write_payload(&self, addr: *mut u8) {
-        (*self as u8).write_payload(addr);
-    }
+    NoRoute = sys::NFT_REJECT_ICMPX_NO_ROUTE,
+    PortUnreach = sys::NFT_REJECT_ICMPX_PORT_UNREACH,
+    HostUnreach = sys::NFT_REJECT_ICMPX_HOST_UNREACH,
+    AdminProhibited = sys::NFT_REJECT_ICMPX_ADMIN_PROHIBITED,
 }
diff --git a/src/expr/verdict.rs b/src/expr/verdict.rs
index fc13f8a..c4facfb 100644
--- a/src/expr/verdict.rs
+++ b/src/expr/verdict.rs
@@ -1,20 +1,16 @@
 use std::fmt::Debug;
 
 use libc::{NF_ACCEPT, NF_DROP, NF_QUEUE};
-use rustables_macros::nfnetlink_struct;
+use rustables_macros::{nfnetlink_enum, nfnetlink_struct};
 
 use super::{ExpressionData, Immediate, Register};
-use crate::{
-    nlmsg::{NfNetlinkAttribute, NfNetlinkDeserializable},
-    parser::DecodeError,
-    sys::{
-        NFTA_VERDICT_CHAIN, NFTA_VERDICT_CHAIN_ID, NFTA_VERDICT_CODE, NFT_BREAK, NFT_CONTINUE,
-        NFT_GOTO, NFT_JUMP, NFT_RETURN,
-    },
+use crate::sys::{
+    NFTA_VERDICT_CHAIN, NFTA_VERDICT_CHAIN_ID, NFTA_VERDICT_CODE, NFT_BREAK, NFT_CONTINUE,
+    NFT_GOTO, NFT_JUMP, NFT_RETURN,
 };
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-#[repr(i32)]
+#[nfnetlink_enum(i32)]
 pub enum VerdictType {
     Drop = NF_DROP,
     Accept = NF_ACCEPT,
@@ -26,36 +22,6 @@ pub enum VerdictType {
     Return = NFT_RETURN,
 }
 
-impl NfNetlinkAttribute for VerdictType {
-    fn get_size(&self) -> usize {
-        (*self as i32).get_size()
-    }
-
-    unsafe fn write_payload(&self, addr: *mut u8) {
-        (*self as i32).write_payload(addr);
-    }
-}
-
-impl NfNetlinkDeserializable for VerdictType {
-    fn deserialize(buf: &[u8]) -> Result<(Self, &[u8]), DecodeError> {
-        let (v, remaining_data) = i32::deserialize(buf)?;
-        Ok((
-            match v {
-                NF_DROP => VerdictType::Drop,
-                NF_ACCEPT => VerdictType::Accept,
-                NF_QUEUE => VerdictType::Queue,
-                NFT_CONTINUE => VerdictType::Continue,
-                NFT_BREAK => VerdictType::Break,
-                NFT_JUMP => VerdictType::Jump,
-                NFT_GOTO => VerdictType::Goto,
-                NFT_RETURN => VerdictType::Goto,
-                _ => return Err(DecodeError::UnknownExpressionVerdictType),
-            },
-            remaining_data,
-        ))
-    }
-}
-
 #[derive(Clone, PartialEq, Eq, Default, Debug)]
 #[nfnetlink_struct(nested = true)]
 pub struct VerdictAttribute {
diff --git a/src/lib.rs b/src/lib.rs
index 044030f..fecbc83 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -71,7 +71,6 @@
 //! [`build.rs`]: https://gitlab.com/rustwall/rustables/-/blob/master/build.rs
 
 use parser::DecodeError;
-use thiserror::Error;
 
 #[macro_use]
 extern crate log;
diff --git a/src/parser.rs b/src/parser.rs
index 7d89a1e..55f1e1c 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -1,5 +1,4 @@
 use std::{
-    any::TypeId,
     convert::TryFrom,
     fmt::{Debug, DebugStruct},
     mem::{size_of, transmute},
@@ -9,10 +8,7 @@ use std::{
 use thiserror::Error;
 
 use crate::{
-    //expr::ExpressionHolder,
-    nlmsg::{
-        AttributeDecoder, NetlinkType, NfNetlinkAttribute, NfNetlinkDeserializable, NfNetlinkWriter,
-    },
+    nlmsg::{AttributeDecoder, NetlinkType, NfNetlinkAttribute, NfNetlinkDeserializable},
     sys::{
         nfgenmsg, nlattr, nlmsgerr, nlmsghdr, NFNETLINK_V0, NFNL_MSG_BATCH_BEGIN,
         NFNL_MSG_BATCH_END, NFNL_SUBSYS_NFTABLES, NLA_F_NESTED, NLA_TYPE_MASK, NLMSG_ALIGNTO,
@@ -75,10 +71,10 @@ pub enum DecodeError {
     UnknownIcmpCode(u8),
 
     #[error("Invalid value for a register")]
-    UnknownRegisterValue,
+    UnknownRegister(u32),
 
     #[error("Invalid type for a verdict expression")]
-    UnknownExpressionVerdictType,
+    UnknownVerdictType(i32),
 
     #[error("The object does not contain a name for the expression being parsed")]
     MissingExpressionName,
diff --git a/tests/expr.rs b/tests/expr.rs
index 4a90309..5baec2a 100644
--- a/tests/expr.rs
+++ b/tests/expr.rs
@@ -1,15 +1,15 @@
 use rustables::{
     expr::{
-        Bitwise, ExpressionList, IcmpCode, Immediate, Meta, MetaType, Register, Reject, RejectType,
-        VerdictKind,
+        Bitwise, ExpressionList, IcmpCode, Immediate, Log, Meta, MetaType, Register, Reject,
+        RejectType, VerdictKind,
     },
     sys::{
         NFTA_BITWISE_DREG, NFTA_BITWISE_LEN, NFTA_BITWISE_MASK, NFTA_BITWISE_SREG,
         NFTA_BITWISE_XOR, NFTA_DATA_VALUE, NFTA_DATA_VERDICT, NFTA_EXPR_DATA, NFTA_EXPR_NAME,
-        NFTA_IMMEDIATE_DATA, NFTA_IMMEDIATE_DREG, NFTA_LIST_ELEM, NFTA_META_DREG, NFTA_META_KEY,
-        NFTA_REJECT_ICMP_CODE, NFTA_REJECT_TYPE, NFTA_RULE_CHAIN, NFTA_RULE_EXPRESSIONS,
-        NFTA_RULE_TABLE, NFTA_VERDICT_CODE, NFT_META_PROTOCOL, NFT_REG_1, NFT_REG_VERDICT,
-        NFT_REJECT_ICMPX_UNREACH,
+        NFTA_IMMEDIATE_DATA, NFTA_IMMEDIATE_DREG, NFTA_LIST_ELEM, NFTA_LOG_GROUP, NFTA_LOG_PREFIX,
+        NFTA_META_DREG, NFTA_META_KEY, NFTA_REJECT_ICMP_CODE, NFTA_REJECT_TYPE, NFTA_RULE_CHAIN,
+        NFTA_RULE_EXPRESSIONS, NFTA_RULE_TABLE, NFTA_VERDICT_CODE, NFT_META_PROTOCOL, NFT_REG_1,
+        NFT_REG_VERDICT, NFT_REJECT_ICMPX_UNREACH,
     },
 };
 //use rustables::expr::{
@@ -246,42 +246,41 @@ fn immediate_expr_is_valid() {
     );
 }
 
-//#[test]
-//fn log_expr_is_valid() {
-//    let log = Log {
-//        group: Some(LogGroup(1)),
-//        prefix: Some(LogPrefix::new("mockprefix").unwrap()),
-//    };
-//    let mut rule = get_test_rule();
-//    let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg_from_expr(&mut rule, &log);
-//    assert_eq!(nlmsghdr.nlmsg_len, 96);
-//
-//    assert_eq!(
-//        raw_expr,
-//        NetlinkExpr::List(vec![
-//            NetlinkExpr::Final(NFTA_RULE_TABLE, TABLE_NAME.to_vec()),
-//            NetlinkExpr::Final(NFTA_RULE_CHAIN, CHAIN_NAME.to_vec()),
-//            NetlinkExpr::Nested(
-//                NFTA_RULE_EXPRESSIONS,
-//                vec![NetlinkExpr::Nested(
-//                    NFTA_LIST_ELEM,
-//                    vec![
-//                        NetlinkExpr::Final(NFTA_EXPR_NAME, b"log\0".to_vec()),
-//                        NetlinkExpr::Nested(
-//                            NFTA_EXPR_DATA,
-//                            vec![
-//                                NetlinkExpr::Final(NFTA_LOG_PREFIX, b"mockprefix\0".to_vec()),
-//                                NetlinkExpr::Final(NFTA_LOG_GROUP, 1u16.to_be_bytes().to_vec())
-//                            ]
-//                        )
-//                    ]
-//                )]
-//            )
-//        ])
-//        .to_raw()
-//    );
-//}
-//
+#[test]
+fn log_expr_is_valid() {
+    let log = Log::new(Some(1337), Some("mockprefix")).expect("Could not build a log expression");
+    let mut rule = get_test_rule().with_expressions(ExpressionList::builder().with_expression(log));
+
+    let mut buf = Vec::new();
+    let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);
+    assert_eq!(nlmsghdr.nlmsg_len, 96);
+
+    assert_eq!(
+        raw_expr,
+        NetlinkExpr::List(vec![
+            NetlinkExpr::Final(NFTA_RULE_TABLE, TABLE_NAME.as_bytes().to_vec()),
+            NetlinkExpr::Final(NFTA_RULE_CHAIN, CHAIN_NAME.as_bytes().to_vec()),
+            NetlinkExpr::Nested(
+                NFTA_RULE_EXPRESSIONS,
+                vec![NetlinkExpr::Nested(
+                    NFTA_LIST_ELEM,
+                    vec![
+                        NetlinkExpr::Final(NFTA_EXPR_NAME, b"log".to_vec()),
+                        NetlinkExpr::Nested(
+                            NFTA_EXPR_DATA,
+                            vec![
+                                NetlinkExpr::Final(NFTA_LOG_GROUP, 1337u16.to_be_bytes().to_vec()),
+                                NetlinkExpr::Final(NFTA_LOG_PREFIX, b"mockprefix".to_vec()),
+                            ]
+                        )
+                    ]
+                )]
+            )
+        ])
+        .to_raw()
+    );
+}
+
 //#[test]
 //fn lookup_expr_is_valid() {
 //    let set_name = &CStr::from_bytes_with_nul(b"mockset\0").unwrap();