detect: add vlan.id keyword - v9 #12360
Conversation
vlan.id matches on Virtual Local Area Network IDs It is an unsigned 16-bit integer Valid range = [1-4094] Supports prefiltering Ticket: OISF#1065
AkakiAlice
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
vlan.layers matches on the number of VLAN layers per packet It is an unsigned 8-bit integer Valid range = [0-3] Supports prefiltering Ticket: OISF#1065
AkakiAlice
force-pushed
the
detect-vlan-1065-v9
branch
from
20c5fcf to
fa23564
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12360 +/- ##
==========================================
+ Coverage 83.23% 83.26% +0.02%
==========================================
Files 912 914 +2
Lines 257647 257897 +250
==========================================
+ Hits 214450 214726 +276
+ Misses 43197 43171 -26
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Thanks Alice, nice work, but some typos to fix in doc
CI : 🟢
Code : cool
Commits segmentation : ok
Commit messages : nice, Valid range = [1-4094] is it not 4096 ?
Git ID set : looks fine for me
CLA : you already contributed
Doc update : typos
Redmine ticket : ok
Rustfmt : ok
Tests : ok cool
Dependencies added: none
| #[derive(Debug, PartialEq)] | ||
| pub struct DetectVlanIdData { | ||
| pub du16: DetectUintData<u16>, // vlan id | ||
| pub layer: i8, // vlan layer |
There was a problem hiding this comment.
I think Victor wants more a comment explaining, that this can be DETECT_VLAN_ID_ANY or DETECT_VLAN_ID_ALL or negative index to match from the end...
| @@ -37,7 +38,9 @@ use std::os::raw::{c_int, c_void}; | |||
| /// derive StringEnum. | |||
| pub trait EnumString<T> { | |||
| /// Return the enum variant of the given numeric value. | |||
| fn from_u(v: T) -> Option<Self> where Self: Sized; | |||
| fn from_u(v: T) -> Option<Self> | |||
There was a problem hiding this comment.
Maybe we should rustfmt this file into its own commit
| DETECT_VLAN_ID_ANY | ||
| } else { | ||
| let u8_layer = i8::from_str(parts[1]).ok()?; | ||
| if !(-3..=VLAN_MAX_LAYER_IDX).contains(&u8_layer) { |
There was a problem hiding this comment.
Can we say !(-VLAN_MAX_LAYERS..= VLAN_MAX_LAYERS-1).contains(&u8_layer)
Ticket: #1065
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
(including schema descriptions)
https://redmine.openinfosecfoundation.org/projects/suricata/issues
Link to ticket: https://redmine.openinfosecfoundation.org/issues/1065
Description:
Changes:
detect-vlan-id.c/.htodetect-vlan.c/.hvlan_id.rstovlan.rsdetect-vlan-id-1065-v9todetect-vlan-1065-v9vlan-keywords.rst:
defaultandanyin the table from "Match all layers" to "Match with any layer": line 39, line43vlan.layers: line 87detect-vlan.c:
SIG_MASK_REQUIRE_FLOWwithSIG_MASK_REQUIRE_REAL_PKT: line 123vlan.layers: line 208vlan.rs:
SV_BRANCH=OISF/suricata-verify#2222
Previous PR: #12333