-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cnf network: add qinq second PR multi-cvlans #10
Conversation
annotation = defineNetworkAnnotation(srIovNetworkDot1Q, nadCVLAN101, true) | ||
serverDotQPod := createServerTestPod(serverNameDot1q, workerNodeList[0].Definition.Name, testCmdNet2, | ||
annotation) | ||
By("Define and create a 802.1AD client container") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be .1Q, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct fixed
|
||
By("Validate that the TCP traffic is double tagged") | ||
readAndValidateTCPDump(tcpDumpContainer, tcpDumpReadFileCMD, tcpDumpDot1QOutput) | ||
}) | ||
|
||
It("Verify network traffic over a double tagging QinQ tunnel with multiple C-VLANs using the same S-VLAN", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double tagging QinQ is too general - it's better 802.1q/802.1q or two times 802.1q
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about double tagged 802.1Q ? If so I will update the other times it appears in all the test cases
/lgtm |
serverIPAddressesNet2 []string | ||
serverIPAddressesNet3 []string | ||
clientIPAddressesNet2 []string | ||
clientIPAddressesNet3 []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace by
serverIPAddressesNet2 = []string{serverIPV4IP.String(), serverIPV6IP.String()}
serverIPAddressesNet3 = []string{serverIPV4IP2.String(), serverIPV6IP2.String()}
clientIPAddressesNet2 = []string{tsparams.ClientIPv4IPAddress, tsparams.ClientIPv6IPAddress}
clientIPAddressesNet3 = []string{tsparams.ClientIPv6IPAddress2, tsparams.ClientIPv6IPAddress2}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
/lgtm |
lgtm |
Adding 4 test cases.
2 multi-cvlans
1 negative test
1 simultaneous 802.1ad and 802.1q