Skip to content

Conversation

@pomodorox
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Dec 30, 2020

Codecov Report

Merging #726 (c5f1965) into master (edc6e74) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #726      +/-   ##
==========================================
- Coverage   82.30%   82.25%   -0.05%     
==========================================
  Files          29       29              
  Lines        4181     4181              
==========================================
- Hits         3441     3439       -2     
- Misses        740      742       +2     
Flag Coverage Δ
aux 74.59% <ø> (-0.08%) ⬇️
base 76.91% <ø> (-0.08%) ⬇️
dhcp 73.30% <ø> (-0.05%) ⬇️
many 73.33% <ø> (-0.51%) ⬇️
topo 72.20% <ø> (ø)

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

Impacted Files Coverage Δ
daq/host.py 91.14% <0.00%> (-0.16%) ⬇️
daq/runner.py 89.32% <0.00%> (-0.15%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edc6e74...c5f1965. Read the comment docs.

Copy link
Collaborator

@grafnu grafnu left a comment

Choose a reason for hiding this comment

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

Not sure if it was "ready for review" or not, but I had some time so I took a look!

device_bcast="$device_traffic and ether broadcast"
device_ucast="$device_traffic and ether dst 9a:02:57:1e:8f:02"
device_xcast="$device_traffic and ether dst 9a:02:57:1e:8f:03"
device_ufr1="$device_traffic and ether src 9a:02:57:1e:8f:03"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why keep the cntelr_ lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

device_bcast="$device_traffic and ether broadcast"
device_ucast="$device_traffic and ether dst 9a:02:57:1e:8f:02"
device_xcast="$device_traffic and ether dst 9a:02:57:1e:8f:03"
device_ufr1="$device_traffic and ether src 9a:02:57:1e:8f:03"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need this line still?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

bfr3=$($device_bfr3 | wc -l)
ufr_peer=$($device_ufr_peer | wc -l)
ufr3=$($device_ufr3 | wc -l)
echo device $type $(($bfr_peer > 2)) $(($bfr_3 > 2)) $(($ufr_peer > 0)) $(($ufr3 > 0)) | tee -a $TEST_RESULTS
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need the $ in front of $bfr_peer -- it's already in "expression" mode so it's redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated


device_traffic="tcpdump -en -r inst/run-9a02571e8f0$device_num/scans/monitor.pcap port 47808"
device_bcast="$device_traffic and ether broadcast"
device_bfr_peer="$device_traffic and ether broadcast and ether src $peer_mac"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be consistent with ordering of src & dst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

peer_mac=9a:02:57:1e:8f:0$peer_num

device_traffic="tcpdump -en -r inst/run-9a02571e8f0$device_num/scans/monitor.pcap port 47808"
device_bcast="$device_traffic and ether broadcast"
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's device_bcast for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@pomodorox pomodorox requested a review from grafnu December 31, 2020 02:46

device_traffic="tcpdump -en -r inst/run-9a02571e8f0$device_num/scans/monitor.pcap port 47808"
device_bfr_peer="$device_traffic and ether src $peer_mac and ether broadcast"
device_bfr3="$device_traffic and ether src 9a:02:57:1e:8f:03 and ether broadcast"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I'd consider extracting the 3rd MAC address to a named variable (assigned right after peer_mac) just for consistency.

@pomodorox pomodorox merged commit a3f781d into faucetsdn:master Dec 31, 2020
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.

2 participants