Skip to content

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7717

Describe changes:

  • Treat vxlan as its own tunnel in order to be able to log ARP over VXLAN
  • ebpf: check maps compatibility (and realize that our current ebpf does not handle 3 layers of vlan)

SV_BRANCH=OISF/suricata-verify#2521

Let me know if you want to handle the ebpf maps commit separately

These are the first commits of #13839 with a dedicated ticket

#14014 with cleaner history and SV rebased on latest

Note: there are other structures that may benefit from such an optimization : git grep "enum " src/*.h | grep ';' | grep -v ');'

For example in struct SSLState_ :

    enum TlsStateClient client_state;
    enum TlsStateServer server_state;

Instead of directly accessing the field

Will allow PacketTunnelType to hold the precise tunnel type like
DECODE_TUNNEL_ERSPANII with a modification of PacketIsTunnelChild
So that we know for a packet which precise type of tunnel it
is (like erspan2).
ebpf program does not handle 3 layers of vlan
Ticket: 7717

Allows for instance to process/log ARP packets over VXLAN.

That means we need to decode the ethernet layer above vxlan
instead of skipping it as part of the vxlan, even if the vxlan
decoder still checks the ethernet layer to avoid FPs.
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.84%. Comparing base (16d124c) to head (c0f8420).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14018      +/-   ##
==========================================
- Coverage   83.87%   83.84%   -0.03%     
==========================================
  Files        1011     1011              
  Lines      275671   275675       +4     
==========================================
- Hits       231207   231132      -75     
- Misses      44464    44543      +79     
Flag Coverage Δ
fuzzcorpus 63.41% <61.11%> (-0.10%) ⬇️
livemode 19.33% <0.00%> (-0.12%) ⬇️
pcap 44.78% <61.11%> (+0.02%) ⬆️
suricata-verify 65.16% <88.88%> (+0.01%) ⬆️
unittests 59.14% <44.44%> (+<0.01%) ⬆️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline = 27969


struct PacketL2 {
enum PacketL2Types type;
uint8_t type; // enum PacketL2Types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will safe anything due to alignment requirements of the pointer that follows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.


struct PacketL3 {
enum PacketL3Types type;
uint8_t type; // enum PacketL3Types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment of pointers in hdrs will lead to this not saving any space I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 32-bit architecture, it would, right ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't optimize for 32 bit anymore, in fact we're considering dropping official support for it

};

struct PacketL4 {
enum PacketL4Types type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again I expect no practical effect here due to alignment

@catenacyber
Copy link
Contributor Author

Next in #14020

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.

3 participants