From f4ca6b97bad53aea8a7e041b0a2586b322947a35 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Mon, 30 Sep 2024 12:30:12 +0200 Subject: [PATCH] Fix vxlan tools link deployment (#2213) * added debug with vscode notes * check for error during tools vxlan create on ep.deploy and do not create veth link when host is used * deploy stitched link instead of using ep.Deploy * remove unused params * remove unused tc code --- Makefile | 4 ++ clab/tc.go | 92 ------------------------------------ cmd/vxlan.go | 10 ++-- docs/manual/dev/debug.md | 88 ++++++++++++++++++++++++++++++++++ links/link_vxlan.go | 33 +++++++++---- links/link_vxlan_stitched.go | 8 ++-- mkdocs.yml | 1 + 7 files changed, 126 insertions(+), 110 deletions(-) delete mode 100644 clab/tc.go create mode 100644 docs/manual/dev/debug.md diff --git a/Makefile b/Makefile index 671af160f..cec1c7b39 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,10 @@ build-debug: mkdir -p $(BIN_DIR) go build -o $(BINARY) -gcflags=all="-N -l" -race -cover main.go +build-dlv-debug: + mkdir -p $(BIN_DIR) + go build -o $(BINARY) -gcflags=all="-N -l" main.go + build-with-podman: mkdir -p $(BIN_DIR) diff --git a/clab/tc.go b/clab/tc.go deleted file mode 100644 index d8f71ba83..000000000 --- a/clab/tc.go +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2020 Nokia -// Licensed under the BSD 3-Clause License. -// SPDX-License-Identifier: BSD-3-Clause - -package clab - -import ( - "fmt" - "os" - "syscall" - - log "github.com/sirupsen/logrus" - "github.com/vishvananda/netlink" -) - -// BindIfacesWithTC creates TC ingress mirred redirection between a pair of interfaces -// to make them virtually form a kind of a veth pair -// see https://netdevops.me/2021/transparently-redirecting-packets/frames-between-interfaces/#4-tc-to-the-rescue -// used https://github.com/redhat-nfvpe/koko/blob/bd156c82bf25837545fb109c69c7b91c3457b318/api/koko_api.go#L224 -// and https://gist.github.com/mcastelino/7d85f4164ffdaf48242f9281bb1d0f9b#gistcomment-2734521 -func BindIfacesWithTC(if1, if2 string) (err error) { - if err := SetIngressMirror(if1, if2); err != nil { - return err - } - if err := SetIngressMirror(if2, if1); err != nil { - return err - } - return err -} - -// SetIngressMirror sets TC to mirror ingress from given port -// as MirrorIngress. -func SetIngressMirror(src, dst string) (err error) { - var linkSrc, linkDest netlink.Link - log.Infof("configuring ingress mirroring with tc in the direction of %s -> %s", src, dst) - - if linkSrc, err = netlink.LinkByName(src); err != nil { - return fmt.Errorf("failed to lookup %q: %v", - src, err) - } - - if linkDest, err = netlink.LinkByName(dst); err != nil { - return fmt.Errorf("failed to lookup %q: %v", - dst, err) - } - - // tc qdisc add dev $SRC_IFACE ingress - qdisc := &netlink.Ingress{ - QdiscAttrs: netlink.QdiscAttrs{ - LinkIndex: linkSrc.Attrs().Index, - Handle: netlink.MakeHandle(0xffff, 0), - Parent: netlink.HANDLE_INGRESS, - }, - } - if err = netlink.QdiscAdd(qdisc); err != nil { - if !os.IsExist(err) { - return err - } - } - - // tc filter add dev $SRC_IFACE parent fffff: - // protocol all - // u32 match u32 0 0 - // action mirred egress mirror dev $DST_IFACE - filter := &netlink.U32{ - FilterAttrs: netlink.FilterAttrs{ - LinkIndex: linkSrc.Attrs().Index, - Parent: netlink.MakeHandle(0xffff, 0), - Protocol: syscall.ETH_P_ALL, - }, - Sel: &netlink.TcU32Sel{ - Keys: []netlink.TcU32Key{ - { - Mask: 0x0, - Val: 0, - }, - }, - Flags: netlink.TC_U32_TERMINAL, - }, - Actions: []netlink.Action{ - &netlink.MirredAction{ - ActionAttrs: netlink.ActionAttrs{ - Action: netlink.TC_ACT_PIPE, - }, - MirredAction: netlink.TCA_EGRESS_MIRROR, - Ifindex: linkDest.Attrs().Index, - }, - }, - } - - return netlink.FilterAdd(filter) -} diff --git a/cmd/vxlan.go b/cmd/vxlan.go index 95c7c790f..d32586d22 100644 --- a/cmd/vxlan.go +++ b/cmd/vxlan.go @@ -56,7 +56,7 @@ var vxlanCmd = &cobra.Command{ var vxlanCreateCmd = &cobra.Command{ Use: "create", Short: "create vxlan interface", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { ctx := context.Background() if _, err := netlink.LinkByName(cntLink); err != nil { @@ -108,8 +108,10 @@ var vxlanCreateCmd = &cobra.Command{ return fmt.Errorf("not a VxlanStitched link") } - for _, ep := range vxl.GetEndpoints() { - ep.Deploy(ctx) + // deploy the vxlan with existing link. The first endpoint is the host endpoint + err = vxl.DeployWithExistingVeth(ctx, vxl.GetEndpoints()[0]) + if err != nil { + return err } return nil @@ -119,7 +121,7 @@ var vxlanCreateCmd = &cobra.Command{ var vxlanDeleteCmd = &cobra.Command{ Use: "delete", Short: "delete vxlan interface", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { var ls []netlink.Link var err error diff --git a/docs/manual/dev/debug.md b/docs/manual/dev/debug.md new file mode 100644 index 000000000..450d57fac --- /dev/null +++ b/docs/manual/dev/debug.md @@ -0,0 +1,88 @@ +# Debugging + +There is a point where you realized that putting another `fmt.Println("here100500")` is not enough. You need to have a debugger to inspect the state of your program. + +## Debugging in VSCode + +Debugging containerlab in VSCode relies on the [Go Dlv integration with VSCode](https://github.com/golang/vscode-go/wiki/debugging) and can be split into two categories: + +1. Debugging using the root user +2. Debugging using the non-`root` user + +Since containerlab requires the superuser privileges to run, the workflow will be slightly different depending on if operate as a `root` user already or not. + +We will document the workflow for the latter (non-root user) case, as this is the most common scenario. In the non-root user case a developer should create a debug configuration file and optionally a task file to build the binary. The reason for the build step is rooted in a fact that we would need to build the binary first as our user, and then the debugger will be called as a `root` user and execute the binary with the debug mode. + +### Create a debug configuration + +The debug configuration defined in the `launch.json` file will contain the important fields such as `asRoot` and `console`, both needed for the debugging as a root user. + +Here is an example of a configuration file that launches `containerlab tools vxlan create` command in the debug mode: + +```json +{ + "version": "0.2.0", + "configurations": [ + { + "name": "tools vxlan create", + "type": "go", + "request": "launch", + "mode": "exec", + "console": "integratedTerminal", + "asRoot": true, + "program": "${workspaceFolder}/bin/containerlab", + "args": [ + "tools", + "vxlan", + "create", + "--remote", + "10.0.0.20", + "-l", + "ens3" + ], + "preLaunchTask": "delete vx-ens3 interface", + } + ] +} +``` + +The debug config is run in the `exec` mode, which means that the debugger expects a program to be built first. This is why we need to create a task file to build the binary first. + +The build happens via the `preLaunchTask` field, that references the task in a `tasks.json` file. + +### Create a task file to build the binary + +The [task file](https://code.visualstudio.com/docs/editor/tasks) provides the means to define arbitrary tasks that can be executed via VSCode UI and as well hooked up in the debug configuration. + +Here is a simple task file that contains two tasks - one is building the binary with the debug information, and the other is a simple command that removes a test interface that the `tools vxlan create` command creates. The only task you need is the build task, but we wanted to show you how to define additional tasks that might be required before your run containerlab in the debug mode to cleanup from the previous execution. + +The dependencies between the tasks are defined in the `dependsOn` field, and this is how you can first build the binary and then run the preparation step. + +```json +{ + "version": "2.0.0", + "tasks": [ + { + "label": "delete vx-ens3 interface", + "type": "shell", + "command": "sudo ip link delete vx-ens3", + "presentation": { + "reveal": "always", + "panel": "new" + }, + "dependsOn": "make build-dlv-debug", + "problemMatcher": [] + }, + { + "label": "make build-dlv-debug", + "type": "shell", + "command": "make", + "args": [ + "build-dlv-debug" + ], + } + ] +} +``` + +Reach out via [Discord](https://discord.gg/vAyddtaEV9) to get help if you get stuck. diff --git a/links/link_vxlan.go b/links/link_vxlan.go index 764f303ec..349556225 100644 --- a/links/link_vxlan.go +++ b/links/link_vxlan.go @@ -43,8 +43,8 @@ func (lr *LinkVxlanRaw) Resolve(params *ResolveParams) (Link, error) { } } -// resolveStitchedVEthComponent creates the veth link and return it, the endpoint that is -// supposed to be stitched is returned seperately for further processing. +// resolveStitchedVEthComponent creates the veth link and returns it, the endpoint that is +// supposed to be stitched is returned separately for further processing. func (lr *LinkVxlanRaw) resolveStitchedVEthComponent(params *ResolveParams) (*LinkVEth, Endpoint, error) { var err error @@ -85,16 +85,29 @@ func (lr *LinkVxlanRaw) resolveStitchedVxlan(params *ResolveParams) (Link, error return nil, err } - // prepare the veth struct - vethLink, stitchEp, err := lr.resolveStitchedVEthComponent(params) - if err != nil { - return nil, err + var vethLink *LinkVEth + var stitchEp Endpoint + + // if the endpoint is host, we don't need to create the veth link + // the stitch endpoint is just a host endpoint with the passed interface name + if lr.Endpoint.Node == "host" { + // a fake endpoint used only to print the host interface name in the outputs + // it is not deployed as it meant to exist + epHost := NewEndpointHost(NewEndpointGeneric(vxlanLink.localEndpoint.GetNode(), params.VxlanIfaceNameOverwrite, nil)) + vethLink, stitchEp, err = nil, epHost, nil + } else { + // otherwise we need to create the veth link + vethLink, stitchEp, err = lr.resolveStitchedVEthComponent(params) + if err != nil { + return nil, err + } + } // return the stitched vxlan link - stitchedLink := NewVxlanStitched(vxlanLink, vethLink, stitchEp) + vxlanStitchedLink := NewVxlanStitched(vxlanLink, vethLink, stitchEp) - return stitchedLink, nil + return vxlanStitchedLink, nil } func (lr *LinkVxlanRaw) resolveVxlan(params *ResolveParams, stitched bool) (*LinkVxlan, error) { @@ -109,7 +122,7 @@ func (lr *LinkVxlanRaw) resolveVxlan(params *ResolveParams, stitched bool) (*Lin } ip := net.ParseIP(lr.Remote) - // if the returned ip is nil, an error occured. + // if the returned ip is nil, an error occurred. // we consider, that we maybe have a textual hostname // e.g. dns name so we try to resolve the string next if ip == nil { @@ -182,7 +195,7 @@ func (lr *LinkVxlanRaw) resolveLocalEndpoint(stitched bool, params *ResolveParam vxlanRawEp.Iface = fmt.Sprintf("vx-%s", params.VxlanIfaceNameOverwrite) } - // in the stiched vxlan mode we create vxlan interface in the host node namespace + // in the stitched vxlan mode we create vxlan interface in the host node namespace vxlanRawEp.Node = "host" vxlanRawEp.MAC = "" diff --git a/links/link_vxlan_stitched.go b/links/link_vxlan_stitched.go index 6f2d20e42..9a8a54421 100644 --- a/links/link_vxlan_stitched.go +++ b/links/link_vxlan_stitched.go @@ -15,8 +15,8 @@ type VxlanStitched struct { vxlanLink *LinkVxlan vethLink *LinkVEth // the veth does not distinguish between endpoints. but we - // need to know which endpoint is the one used for - // stitching therefore we get that endpoint seperately + // need to know which endpoint is the one used for + // stitching therefore we get that endpoint separately vethStitchEp Endpoint } @@ -33,14 +33,14 @@ func NewVxlanStitched(vxlan *LinkVxlan, veth *LinkVEth, vethStitchEp Endpoint) * return vxlanStitched } -// DeployWithExistingVeth provisons the stitched vxlan link whilst the +// DeployWithExistingVeth provisions the stitched vxlan link whilst the // veth interface does already exist, hence it is not created as part of this // deployment. func (l *VxlanStitched) DeployWithExistingVeth(ctx context.Context, ep Endpoint) error { return l.internalDeploy(ctx, ep, true) } -// Deploy provisions the stitched vxlan link with all its underlaying sub-links. +// Deploy provisions the stitched vxlan link with all its underlying sub-links. func (l *VxlanStitched) Deploy(ctx context.Context, ep Endpoint) error { return l.internalDeploy(ctx, ep, false) } diff --git a/mkdocs.yml b/mkdocs.yml index a814734c3..a5ada39d7 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -79,6 +79,7 @@ nav: - manual/dev/index.md - Documentation: manual/dev/doc.md - Testing: manual/dev/test.md + - Debugging: manual/dev/debug.md - Command reference: - deploy: cmd/deploy.md - destroy: cmd/destroy.md