Skip to content
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

When endpoint definition in brief format missing a : it causes panic #1690

Closed
hellt opened this issue Nov 1, 2023 · 6 comments · Fixed by #1691
Closed

When endpoint definition in brief format missing a : it causes panic #1690

hellt opened this issue Nov 1, 2023 · 6 comments · Fixed by #1691
Assignees

Comments

@hellt
Copy link
Member

hellt commented Nov 1, 2023

User report:

if we define two bridges and put the same interface name into each clab panics.

Smth like

- endpoints: ["node1:e1-48", "br-1:eth1"]
- endpoints: ["node1:e1-49", "br-2:eth1"]
INFO[0000] Containerlab v0.47.2 started
panic: runtime error: index out of range [1] with length 1 [recovered]
        panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
gopkg.in/yaml%2ev2.handleErr(0xc000fbf560)
        gopkg.in/yaml.v2@v2.4.0/yaml.go:249 +0x6d
panic({0x2d20500, 0xc00062b398})
        runtime/panic.go:884 +0x213
github.com/srl-labs/containerlab/links.extractHostNodeInterfaceData(0xc000628510, 0x10?)
        github.com/srl-labs/containerlab/links/link.go:320 +0x12b
github.com/srl-labs/containerlab/links.linkVEthRawFromLinkBriefRaw(0xc000628510)
        github.com/srl-labs/containerlab/links/link_veth.go:74 +0x25
github.com/srl-labs/containerlab/links.(*LinkBriefRaw).ToTypeSpecificRawLink(0xc000628510)
        github.com/srl-labs/containerlab/links/link_brief.go:41 +0x1d6
github.com/srl-labs/containerlab/links.(*LinkDefinition).UnmarshalYAML(0xc000f3d140, 0xc000f3d160)
        github.com/srl-labs/containerlab/links/link.go:214 +0x2f6
gopkg.in/yaml%2ev2.(*decoder).callUnmarshaler(0xc000c88180, 0xc000829500, {0x7fc3329fd598, 0xc000f3d140})
        gopkg.in/yaml.v2@v2.4.0/decode.go:270 +0xa4
@hellt
Copy link
Member Author

hellt commented Nov 1, 2023

might be of interest to you @steiler

@steiler
Copy link
Collaborator

steiler commented Nov 1, 2023

Try with a different interface on the node side in this example they are both the same... But yes also thus error should be caught

@hellt
Copy link
Member Author

hellt commented Nov 1, 2023

@steiler the node interfaces were different (copy paste issue)
it is the same interface on the bridges side are not handled proprely.

@steiler
Copy link
Collaborator

steiler commented Nov 1, 2023

Here is my config:

# topology documentation: http://containerlab.dev/lab-examples/single-srl/
name: test91

topology:
  nodes:
    l1:
      kind: linux
      image: alpine:latest
      cmd: "sleep 5d"
    br-1:
      kind: bridge
    br-2:
      kind: bridge
  links:
    - endpoints: ["l1:eth1", "br-1:eth1"]
    - endpoints: ["l1:eth2", "br-2:eth1"]

And this is what I get:

EBU[0000] Checking if docker network "clab" exists     
DEBU[0000] network "clab" was found. Reusing it...      
DEBU[0000] Docker network "clab", bridge name "br-110b4d465e7a" 
DEBU[0000] Disable RPF check on the docker host         
DEBU[0000] Enable LLDP on the linux bridge br-110b4d465e7a 
DEBU[0000] Disabling TX checksum offloading for the br-110b4d465e7a bridge interface... 
DEBU[0000] found iptables forwarding rule targeting the bridge "br-110b4d465e7a". Skipping creation of the forwarding rule. 
Error: root network namespace endpoint "eth1" defined by multiple nodes [br-1, br-2]

So to me, this looks fine. Please provide more input.

@steiler
Copy link
Collaborator

steiler commented Nov 1, 2023

Also where it crashed according to your output is the following function

func extractHostNodeInterfaceData(lb *LinkBriefRaw, specialEPIndex int) (host, hostIf, node, nodeIf string) {
	// the index of the node is the specialEndpointIndex +1  modulo 2
	nodeindex := (specialEPIndex + 1) % 2

	hostData := strings.SplitN(lb.Endpoints[specialEPIndex], ":", 2)
	nodeData := strings.SplitN(lb.Endpoints[nodeindex], ":", 2)

	host = hostData[0]
	hostIf = hostData[1]
	node = nodeData[0]
	nodeIf = nodeData[1]

	return host, hostIf, node, nodeIf
}

right at nodeIf = nodeData[1] ...
so that is still parsing the data you've provided... to me it looks like there was no colon between nodename and interface or so... such that the split did only yield a slice of length 1...

@hellt
Copy link
Member Author

hellt commented Nov 1, 2023

yep, you're spot on
I trimmed down the topo, since it was huge, and indeed one of the endpoints has been set as
br-ens-clab-eth3. I will change the description accordingly

@hellt hellt changed the title Clashed host namespaced interfaces cause panic When endpoint definition in brief format missing a : it causes panic Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants