API for GRE #1445
Merged
API for GRE #1445
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -22,6 +22,7 @@ message Interface { | |||
| IPSEC_TUNNEL = 8; | |||
| VMXNET3_INTERFACE = 9; | |||
| BOND_INTERFACE = 10; | |||
| GRE = 11; | |||
There was a problem hiding this comment.
This should be GRE_TUNNEL to match with other tunnel types.
| Type tunnel_type = 1; | ||
| string src_addr = 2; | ||
| string dst_addr = 3; | ||
| uint32 outerFibID = 4; |
There was a problem hiding this comment.
| Instance uint32 | ||
| IsIPv6 uint8 | ||
| TunnelType uint8 | ||
| SrcAddress string |
There was a problem hiding this comment.
Using type net.IP here, will allow you to get rid of IsIPv6.
There was a problem hiding this comment.
not sure if this will help. As I understand, you are saying about checking IsIPv6 in dump function:
if greDetails.IsIPv6 == 1 { ...But in greDetails addresses comes as []byte and, for example, ip 10.10.10.10 will be presented as [10 10 10 10 0 0 0 0 0 0 0 0 0 0 0 0], on the other hand ip 2019::8:23 will be presented as [32 25 0 0 0 0 0 0 0 0 0 0 0 8 0 35].
If I do not check IsIPv6 and not slice this slice if it's IPv4, then String() method on such net.IP in a case with 10.10.10.10 will return a0a:a0a:: and this is definitely not what I've expected to see.
One thing is actually redundant is calling To4() and To16(), because slice was already trimmed to proper size.
There was a problem hiding this comment.
What I meant was to use the net.IP values parsed in dump:
vpp-agent/plugins/vpp/ifplugin/vppcalls/vpp1904/gre_vppcalls.go
Lines 108 to 109 in cd0dc98
And store as net.IP here:
vpp-agent/plugins/vpp/ifplugin/vppcalls/vpp1904/gre_vppcalls.go
Lines 120 to 121 in cd0dc98
And the caller who calls dump can simply use the addresses directly without parsing them from string again. You can easily check if net.IP is IPv4 or IPv6:
ip4 := net.ParseIP("10.0.1.1")
fmt.Println(ip4)
fmt.Printf("is IPv6: %v\n\n", ip4.To4() == nil) // prints false
ip6 := net.ParseIP("fe80::200:5aee:feaa:20a2")
fmt.Println(ip6)
fmt.Printf("is IPv6: %v\n", ip6.To4() == nil) // prints trueThere was a problem hiding this comment.
oh I see now. Yes, it would be better to keep IP addresses as net.IP
| t.Run(test.name, func(t *testing.T) { | ||
| ifIdx, err := h.AddGreTunnel(fmt.Sprintf("test%d", i), test.greLink) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
I'm not sure this will handle test case where we expect some non-nil error, but it gets nil.
|
Manual tests with localclient: VPP was not restarted during this tests Definition of ERSPAN GRE tunnel in code
var (
greInterface = &vpp_interfaces.Interface{
Name: "myGreInterface",
Type: vpp_interfaces.Interface_GRE_TUNNEL,
Enabled: true,
Link: &vpp_interfaces.Interface_Gre{
Gre: &vpp_interfaces.GreLink{
TunnelType: vpp_interfaces.GreLink_ERSPAN,
SrcAddr: "10.10.10.10",
DstAddr: "20.20.20.20",
},
},
}
)Create ERSPAN
Restart Agent (without restarting VPP)
Disable interface
Change "Source" for GRE tunnel
Enable interface
Stop Agent, change source address and start Agent again
Stop Agent, add one more GRE tunnel in code, start Agent
Output from vppctl after all of this
|
milanlenco
approved these changes
Aug 30, 2019
ondrej-fabry
approved these changes
Sep 2, 2019
VladoLavor
pushed a commit
to VladoLavor/vpp-agent
that referenced
this pull request
Oct 4, 2019
* Add GRE VPP calls. Add GRE type and link to inteface message * add basic validation and support of create and delete operations for GRE * Rename things. Store addresses as net.IP * add GRE to interfaces dump * Add UNKNOWN to types of GRE tunnel * Support vpp1901 and vpp1908 * Test also removing of GRE tunnel
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements GRE tunnel interface from #1387