Skip to content

Commit

Permalink
Fix tunnelDstIP field in traceflow on dual-stack
Browse files Browse the repository at this point in the history 
On dual-stack testbed, Inter-Node traceflow observation for
IPv4 packet in encap mode has empty tunnelDstIP field because
isIPv6 variable is set.

Change the logic to set isIPv6 variable based on the packet
IPv4 or IPv6 on dual-stack testbed.

Add verification of tunnelDstIP in observation traceflow e2e tests.

Fixes #4502

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>
  • Loading branch information
@Atish-iaf
Atish-iaf committed Jan 6, 2023
1 parent 7e055c6 commit d361404
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
8 changes: 7 additions & 1 deletion pkg/agent/controller/traceflow/packetin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,13 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
if tableID == openflow.L2ForwardingOutTable.GetID() {
ob := new(crdv1alpha1.Observation)
tunnelDstIP := ""
isIPv6 := c.nodeConfig.NodeIPv6Addr != nil
var isIPv6 bool
// On dual stack, decide according to packet.
if c.nodeConfig.NodeIPv4Addr != nil && c.nodeConfig.NodeIPv6Addr != nil {
isIPv6 = etherData.Ethertype == protocol.IPv6_MSG
} else {
isIPv6 = c.nodeConfig.NodeIPv6Addr != nil
}
if match := getMatchTunnelDstField(matchers, isIPv6); match != nil {
tunnelDstIP, err = getTunnelDstValue(match)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/agent/controller/traceflow/packetin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ func TestParsePacketIn(t *testing.T) {
egressIP: egressIP,
}

_, nodeIPv6Addr, err := net.ParseCIDR("fd74:ca9b:172:18::/64")
require.NoError(t, err)
_, nodeIPv4Addr, err := net.ParseCIDR("192.168.0.1/24")
require.NoError(t, err)

tests := []struct {
name string
networkConfig *config.NetworkConfig
Expand All @@ -261,6 +266,8 @@ func TestParsePacketIn(t *testing.T) {
TrafficEncapMode: 0,
},
nodeConfig: &config.NodeConfig{
NodeIPv4Addr: nodeIPv4Addr,
NodeIPv6Addr: nodeIPv6Addr,
TunnelOFPort: 1,
GatewayConfig: &config.GatewayConfig{
OFPort: 2,
Expand Down Expand Up @@ -322,6 +329,7 @@ func TestParsePacketIn(t *testing.T) {
TrafficEncapMode: 0,
},
nodeConfig: &config.NodeConfig{
NodeIPv4Addr: nodeIPv4Addr,
TunnelOFPort: 2,
GatewayConfig: &config.GatewayConfig{
OFPort: 1,
Expand Down Expand Up @@ -384,6 +392,8 @@ func TestParsePacketIn(t *testing.T) {
TrafficEncapMode: 0,
},
nodeConfig: &config.NodeConfig{
NodeIPv4Addr: nodeIPv4Addr,
NodeIPv6Addr: nodeIPv6Addr,
TunnelOFPort: 1,
GatewayConfig: &config.GatewayConfig{
OFPort: 2,
Expand Down
36 changes: 35 additions & 1 deletion test/e2e/traceflow_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,28 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
agnhostIP, err := data.podWaitForIPs(defaultTimeout, agnhostPodName, data.testNamespace)
require.NoError(t, err)

node2TunnelDstIPv4, node2TunnelDstIPv6 := "", ""
encapMode, err := data.GetEncapMode()
require.NoError(t, err)

switch encapMode {
case config.TrafficEncapModeEncap:
node2TunnelDstIPv4 = clusterInfo.nodes[nodeIdx1].ipv4Addr
node2TunnelDstIPv6 = clusterInfo.nodes[nodeIdx1].ipv6Addr
case config.TrafficEncapModeHybrid:
// check if node1 and node2 are in same subnet.
node2IP := net.ParseIP(nodeIP(nodeIdx1))
cmd := getPingCommand(pingCount, 0, "", &node2IP)
stdout, _, err := data.RunCommandFromPod(data.testNamespace, node1Pods[0], agnhostContainerName, cmd)
require.NoError(t, err)
fmt.Println(stdout)
if !(strings.Contains(stdout, "ttl=63") || strings.Contains(stdout, "ttl=127")) {
// node1 and node2 are in different subnet.
node2TunnelDstIPv4 = clusterInfo.nodes[nodeIdx1].ipv4Addr
node2TunnelDstIPv6 = clusterInfo.nodes[nodeIdx1].ipv6Addr
}
}

var agnhostIPv4Str, agnhostIPv6Str, svcIPv4Name, svcIPv6Name string
if agnhostIP.ipv4 != nil {
agnhostIPv4Str = agnhostIP.ipv4.String()
Expand Down Expand Up @@ -1272,6 +1294,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv4,
},
},
},
Expand Down Expand Up @@ -1337,6 +1360,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv4,
},
},
},
Expand Down Expand Up @@ -1397,6 +1421,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv4,
},
},
},
Expand Down Expand Up @@ -1473,6 +1498,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv4,
},
},
},
Expand Down Expand Up @@ -1593,6 +1619,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv4,
},
},
},
Expand Down Expand Up @@ -1660,6 +1687,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv6,
},
},
},
Expand Down Expand Up @@ -1728,6 +1756,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv6,
},
},
},
Expand Down Expand Up @@ -1787,6 +1816,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv6,
},
},
},
Expand Down Expand Up @@ -1860,6 +1890,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv6,
},
},
},
Expand Down Expand Up @@ -1982,6 +2013,7 @@ func testTraceflowInterNode(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: node2TunnelDstIPv6,
},
},
},
Expand Down Expand Up @@ -2207,6 +2239,7 @@ func testTraceflowEgress(t *testing.T, data *TestData) {
Component: v1alpha1.ComponentForwarding,
ComponentInfo: "Output",
Action: v1alpha1.ActionForwarded,
TunnelDstIP: egressIP,
},
},
},
Expand Down Expand Up @@ -2273,7 +2306,8 @@ func compareObservations(expected v1alpha1.NodeResult, actual v1alpha1.NodeResul
exObs[i].TranslatedDstIP != acObs[i].TranslatedDstIP ||
exObs[i].EgressIP != acObs[i].EgressIP ||
exObs[i].Egress != acObs[i].Egress ||
exObs[i].Action != acObs[i].Action {
exObs[i].Action != acObs[i].Action ||
exObs[i].TunnelDstIP != acObs[i].TunnelDstIP {
return fmt.Errorf("Observations should be %v, but got %v", exObs, acObs)
}
}
Expand Down

0 comments on commit d361404

Please sign in to comment.