Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

lwip weave tunnel test 02 #362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jenniexie
Copy link
Contributor

@jenniexie jenniexie commented Sep 10, 2019

enabled lwip tunnel test 02
#364

@gerickson
Copy link
Contributor

enabled lwip tunnel test 02
manual test steps are tracked in this doc:
https://docs.google.com/document/d/1XRXu2wNzAed3tEAYU6eloFx_gHiYj6Ysy62ObrZWgIo/edit?usp=sharing

@jenniexie, thanks for sharing broader context; however, non-Google Github users cannot access such documents. I would recommend creating a Github issue, and managing the content there.

@@ -168,6 +168,30 @@ def getNodeWeaveIPAddress(self, node_id=None, state=None):
return addr
return None

def getNodeInterfaceWeaveIPAddress(self, interface_id, node_id, state=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this control / influence whether that address is IPv4 or IPv6. Ostensibly that address should only be IPv6; however, the documentation should be clear about that fact and the code should, potentially, have an assertion or some such to trap / catch an IPv4 address that might "bubble up" through to this interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Grant:
Do you mean add a check to make sure there is ipv6 address in the node's address list before we do IP.prefixMatchAddress(weave_global_prefix, addr) in line 191?

This function will return weave ip address, which should be ipv6 address by default from my understanding.

@jenniexie
Copy link
Contributor Author

enabled lwip tunnel test 02
manual test steps are tracked in this doc:
https://docs.google.com/document/d/1XRXu2wNzAed3tEAYU6eloFx_gHiYj6Ysy62ObrZWgIo/edit?usp=sharing

@jenniexie, thanks for sharing broader context; however, non-Google Github users cannot access such documents. I would recommend creating a Github issue, and managing the content there.

Sure, I will remove the google doc and add an issue with information there.

@jenniexie jenniexie force-pushed the jexie/lwip_tunnel_test02_pr branch from 7d7db91 to 0008762 Compare September 12, 2019 18:05
@codecov-io
Copy link

codecov-io commented Sep 13, 2019

Codecov Report

Merging #362 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #362      +/-   ##
==========================================
+ Coverage   54.35%   54.36%   +<.01%     
==========================================
  Files         341      341              
  Lines       58207    58207              
==========================================
+ Hits        31641    31643       +2     
+ Misses      26566    26564       -2
Impacted Files Coverage Δ
src/lib/profiles/security/WeaveCert.cpp 84.41% <0%> (+0.64%) ⬆️

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 220b97b...0008762. Read the comment docs.

@jenniexie jenniexie requested a review from gerickson September 17, 2019 17:47
@jenniexie
Copy link
Contributor Author

Merging #362 into master will increase coverage by 1.61%.
This test case will improve code coverage a little bit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants