Adding a Firewall NF #80

dennisafa · 2019-03-12T17:49:09Z

Continuing @AaronCoplan 's work of adding a firewall NF. This adds LPM rules json parsing functionality and debug mode.

Summary:

This NF drops packets based on rules that follow LPM (longest prefix matching) protocol. Default behavior is to drop unless the packet matches a rule.

Usage:

This PR includes
Resolves issues
Breaking API changes
Internal API changes
Usability improvements
Bug fixes
New functionality	👍
New NF/onvm_mgr args	👍
Changes to starting NFs
Dependency updates
Web stats updates

Merging notes:

Dependencies: Upgrading from Ubuntu 14.04.1 to 18.04.1 #72

TODO before merging :

PR is ready for review
merge ubuntu upgrade PR
fix bug where closing nf then starting again while manager is running causes a "Cannot get LPM request" error
change and merge IP parse function

Test Plan:

Run from examples directory with ./start_nf.sh firewall 1 -d 1 -f rules.json for normal mode, or ./start_nf.sh firewall 1 -d 1 -b -f rules.json for debug mode (this will print when packets are dropped/forwarded)
You could also run from within the firewall directory.
Used pcap files to verify that packets that do not follow LPM rules are dropped. Verify that packets that match rules are forwarded.

Review:

Sanity checks, assigned to @koolzz @kevindweb
- Run make in /onvm and /examples directories
- Test new functionality or bug fix from the pr (if needed)
Code style, assigned to @koolzz @kevindweb
- Run linter
- Check everything style related
Code design, assigned to @koolzz @kevindweb
- Check for memory leaks
- Check that code is reusable
- Code is clean, functions are concise
- Verify that edge cases properly handled
Performance, assigned to @koolzz @kevindweb
- Run Speed Tester NF, report performance.
- Run pktgen, report performance (if needed)
- Run mTCP epserver, verify wget works, report performance for epwget/ab (if needed)
Documentation, assigned to @koolzz @kevindweb
- Check if the new changes are well documented, in both code and READMEs

examples/firewall/firewall.c

koolzz · 2019-04-26T23:53:49Z

@dennisafa Fix Makefile conflict

@nks5295 Did you have any specific tests in mind? As this is just an example like most of our NFs the testing was pretty minimal. If not I'll just run a few quick tests and merge

dennisafa · 2019-04-29T15:22:17Z

There are a few issues I need to take care of since the latest merge. I will fix asap @koolzz
Edit: good to go

koolzz

I've tested with speed_tester and pcaps. Things work as expected so I'm approving!
I've added a few naming nits/code style things. Once you fix those we're good to merge

examples/firewall/firewall.c

onvm/onvm_mgr/onvm_nf.c

onvm/onvm_nflib/onvm_common.h

onvm/onvm_nflib/onvm_nflib.h

nks5295

Looks good overall, but a couple simple fixes to adhere to our naming conventions

onvm/onvm_mgr/onvm_nf.c

onvm/onvm_nflib/onvm_common.h

koolzz · 2019-05-20T21:08:34Z

@onvm do your thing

onvm · 2019-05-20T21:08:42Z

@onvm do your thing

CI Message

Your results will arrive shortly

onvm

@onvm do your thing

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
❌ Linter Failed (please fix style errors)

[Results from nimbnode30]

Median TX pps for Speed Tester: 35231909
Performance rating - 100.66% (compared to 35000000 average)

Linter Output

examples/firewall/firewall.c:47: "unistd.h" already included at examples/firewall/firewall.c:42 [build/include] [4]
examples/firewall/firewall.c:114: Lines should be <= 120 characters long [whitespace/line_length] [5]
examples/firewall/firewall.c:137: Almost always, snprintf is better than strcpy [runtime/printf] [4]
examples/firewall/firewall.c:142: Lines should very rarely be longer than 150 characters [whitespace/line_length] [4]
examples/firewall/firewall.c:265: Almost always, snprintf is better than strcpy [runtime/printf] [4]
Total errors found: 5
onvm/onvm_nflib/onvm_pkt_helper.c:378: Lines should be <= 120 characters long [whitespace/line_length] [5]
onvm/onvm_nflib/onvm_pkt_helper.c:378: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
Total errors found: 2

koolzz

@dennisafa alsmost good to merge, pls fix the linter suggestions from ci + the ones I've pointed out.

examples/firewall/firewall.c

dennisafa · 2019-05-21T14:33:20Z

@onvm dance your dance

onvm · 2019-05-21T14:33:28Z

@onvm dance your dance

CI Message

Your results will arrive shortly

onvm

@onvm dance your dance

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
✔️ Linter passed

[Results from nimbnode30]

Median TX pps for Speed Tester: 35246316
Performance rating - 100.70% (compared to 35000000 average)

dennisafa · 2019-05-21T14:43:36Z

@onvm one more time

onvm · 2019-05-21T14:43:44Z

@onvm one more time

CI Message

Your results will arrive shortly

onvm

@onvm one more time

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
✔️ Linter passed

[Results from nimbnode30]

Median TX pps for Speed Tester: 35241691
Performance rating - 100.69% (compared to 35000000 average)

dennisafa added 30 commits March 6, 2019 18:12

Syncing with current

14f3f2b

Updates for ubuntu 18

044d647

Update firewall.c

ad6af5e

Update firewall.c

75b5b1e

Update firewall.c

0afb6d9

Update firewall.c

3b370c3

Update firewall.c

e9579c2

Update firewall.c

66754e7

Update firewall.c

91634ed

Update firewall.c

5321d8e

Update firewall.c

2638ab3

Updates

78d463e

Update firewall.c

c44a852

Update firewall.c

1e2b94b

Update firewall.c

3ee1f89

Update firewall.c

f0cedb1

Update firewall.c

6db0bf5

Update firewall.c

b17baa5

Update firewall.c

7d3a4fb

Update firewall.c

92fc623

Update firewall.c

a14ea3d

Update firewall.c

ebf7116

Update firewall.c

a7ddaee

Update firewall.c

78101f2

Update firewall.c

f9fcef8

Update firewall.c

0bf61e2

Update firewall.c

e0b7555

Update firewall.c

bf31b65

Update firewall.c

c3c8a41

Update firewall.c

2f876af

dennisafa added 2 commits April 22, 2019 11:33

Style nits and TODO

1349219

TODO

d3e4182

koolzz reviewed Apr 24, 2019

View reviewed changes

examples/firewall/firewall.c Outdated Show resolved Hide resolved

koolzz reviewed Apr 24, 2019

View reviewed changes

examples/firewall/firewall.c Outdated Show resolved Hide resolved

dennisafa and others added 2 commits April 24, 2019 16:16

ip_disp to ip_string

cfe95a0

Restoring lpm struct (accidentally deleted)

7b25050

dennisafa added 2 commits April 26, 2019 20:19

Merge branch 'develop' into firewall1902

87a5750

Merge branch 'develop' into firewall1902

1f8cd50

dennisafa added 2 commits April 29, 2019 10:29

Merge remote-tracking branch 'origin/develop' into firewall1902

b1f0667

Restoring lpm status macro

aedc66c

koolzz approved these changes May 16, 2019

View reviewed changes

nks5295 requested changes May 16, 2019

View reviewed changes

onvm/onvm_mgr/onvm_nf.c Outdated Show resolved Hide resolved

onvm/onvm_nflib/onvm_common.h Outdated Show resolved Hide resolved

dennisafa added 2 commits May 19, 2019 16:47

Style fixes, naming, etc

6eb0e97

Style fixes, naming

3a6e347

onvm suggested changes May 20, 2019

View reviewed changes

koolzz reviewed May 20, 2019

View reviewed changes

examples/firewall/firewall.c Outdated Show resolved Hide resolved

examples/firewall/firewall.c Outdated Show resolved Hide resolved

examples/firewall/firewall.c Outdated Show resolved Hide resolved

examples/firewall/firewall.c Show resolved Hide resolved

Style fixes

b738660

onvm approved these changes May 21, 2019

View reviewed changes

Style nit

d33a105

onvm approved these changes May 21, 2019

View reviewed changes

koolzz merged commit a003a40 into sdnfv:develop May 23, 2019

koolzz added this to the ONVM 19.05 Release milestone May 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a Firewall NF #80

Adding a Firewall NF #80

dennisafa commented Mar 12, 2019 •

edited

Loading

koolzz commented Apr 26, 2019

dennisafa commented Apr 29, 2019 •

edited

Loading

koolzz left a comment

nks5295 left a comment

koolzz commented May 20, 2019

onvm commented May 20, 2019

onvm left a comment

koolzz left a comment

dennisafa commented May 21, 2019

onvm commented May 21, 2019

onvm left a comment

dennisafa commented May 21, 2019

onvm commented May 21, 2019

onvm left a comment

Adding a Firewall NF #80

Adding a Firewall NF #80

Conversation

dennisafa commented Mar 12, 2019 • edited Loading

Summary:

Merging notes:

Test Plan:

Review:

koolzz commented Apr 26, 2019

dennisafa commented Apr 29, 2019 • edited Loading

koolzz left a comment

Choose a reason for hiding this comment

nks5295 left a comment

Choose a reason for hiding this comment

koolzz commented May 20, 2019

onvm commented May 20, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

Linter Output

koolzz left a comment

Choose a reason for hiding this comment

dennisafa commented May 21, 2019

onvm commented May 21, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

dennisafa commented May 21, 2019

onvm commented May 21, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

dennisafa commented Mar 12, 2019 •

edited

Loading

dennisafa commented Apr 29, 2019 •

edited

Loading