Skip to content

decode/vlan: Extend VLAN encapsulation support to 3 levels; flowhash livedev#8908

Closed
jlucovsky wants to merge 7 commits into
OISF:masterfrom
jlucovsky:2816.v9
Closed

decode/vlan: Extend VLAN encapsulation support to 3 levels; flowhash livedev#8908
jlucovsky wants to merge 7 commits into
OISF:masterfrom
jlucovsky:2816.v9

Conversation

@jlucovsky
Copy link
Copy Markdown
Contributor

Continuation of #8890 and #8895.

This PR extends Suricata's support for VLANs from 2 to 3 levels. There is no standard for 3 levels of VLANs but 3 levels are not uncommon in some environments.

Link to redmine tickets:

Describe changes:

  • Increase VLAN level support to 3 levels
  • List all VLAN levels in output.
  • Add "Q-in-Q-in-Q" unittest.
  • flow: optionally use livedev for hash
  • Add defines for VLAN max layer and max index
  • Unit test updates per review

Updates

SV_BRANCH=pr/1204

OISF/suricata-verify#1204

jlucovsky and others added 7 commits May 23, 2023 07:59
Issue: 2816

This commit extends the JSON schema with the additional VLAN stat for
tracking VLAN encapsulated packets with 3 levels.
Issue: 2816

This commit increase the number of VLAN layers supported by Suricata
from 2 to 3. 3-layers are dubbed "Q-in-Q-in-Q".

Note that 3 layers are not compliant with any existing standard but are
often seen in larger deployments.
This commit removes unused functions and macros related to fetching VLAN
values.
Meaning that we support 65535 live devices at the most
For easier reasoning about the code
So that in a setup with different interfaces capturing different
networks, flows do not get mixed up

Ticket: OISF#5270
@jlucovsky jlucovsky requested review from a team and victorjulien as code owners May 23, 2023 12:03
@jlucovsky jlucovsky mentioned this pull request May 23, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2023

Codecov Report

Merging #8908 (4b0d33e) into master (ebe0a7b) will increase coverage by 0.00%.
The diff coverage is 59.63%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8908   +/-   ##
=======================================
  Coverage   82.30%   82.30%           
=======================================
  Files         969      969           
  Lines      273335   273391   +56     
=======================================
+ Hits       224961   225022   +61     
+ Misses      48374    48369    -5     
Flag Coverage Δ
fuzzcorpus 64.72% <49.65%> (+0.07%) ⬆️
suricata-verify 60.40% <49.65%> (-0.06%) ⬇️
unittests 62.95% <44.57%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link
Copy Markdown

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.http.memuse 2104 0 -

Pipeline 13893

@catenacyber
Copy link
Copy Markdown
Contributor

Do you need something from me here @jlucovsky ?
Did you find out the QA diff origin ?

@jlucovsky
Copy link
Copy Markdown
Contributor Author

Do you need something from me here @jlucovsky ? Did you find out the QA diff origin ?

I don't think I need anything right now.

We suspect the memory use difference is due to the stats being written before everything has been deallocated/released.

@victorjulien thoughts on the memuse difference?

@victorjulien
Copy link
Copy Markdown
Member

Do you need something from me here @jlucovsky ? Did you find out the QA diff origin ?

I don't think I need anything right now.

We suspect the memory use difference is due to the stats being written before everything has been deallocated/released.

@victorjulien thoughts on the memuse difference?

I don't know about the http.memuse one, but it's clear that the bigger one we discussed in Vienna is unrelated, see #8928 (comment)

@victorjulien victorjulien mentioned this pull request Jun 5, 2023
@victorjulien
Copy link
Copy Markdown
Member

Merged in #8965, thanks!

@jlucovsky jlucovsky deleted the 2816.v9 branch June 8, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants