Skip to content

Added support for choosing encapsulation type#27

Merged
shaharyarn merged 15 commits intomasterfrom
feature/choosing-encapsulation
May 31, 2020
Merged

Added support for choosing encapsulation type#27
shaharyarn merged 15 commits intomasterfrom
feature/choosing-encapsulation

Conversation

@shaharyarn
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator

@yehonatanz yehonatanz left a comment

Choose a reason for hiding this comment

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

Looks good, not approving yet due to potential marine-core changes

Copy link
Copy Markdown
Collaborator

@yehonatanz yehonatanz left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link
Copy Markdown
Owner

@tomlegkov tomlegkov left a comment

Choose a reason for hiding this comment

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

Except for the tests, I think the benchmark should be changed like how I suggested in the core PR. But that's definitely not required and up to @yehonatanz :)

@@ -5,44 +5,53 @@
from typing import List, Union, Optional, Dict
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think some more tests are required:

  1. Test that different encapsulations affect BPF validation
  2. Test that different encapsulations affect parsing
  3. Show that display filter validation works for types of different encapsulation (for example, add a test that shows both radiotap and ethernet are valid display filters without needing to specify encapsulation)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I added a test that checks for effect on BPF validation and tests that affect parsing.
I could not add a test that checks the 3rd requirement as the display filter validation is not affected by encapsulation type, as the display filter is not dependent on it during compilation.


ENCAP_ETHERNET = 1

"""Your friendly neighbourhood wifi"""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:)

@shaharyarn shaharyarn merged commit 0fe6d51 into master May 31, 2020
@shaharyarn shaharyarn deleted the feature/choosing-encapsulation branch May 31, 2020 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants