From 62310a4432d108ab159ce53765bcb488f8250c51 Mon Sep 17 00:00:00 2001 From: Ruslan Bayandinov Date: Thu, 26 Jan 2023 17:00:05 +0000 Subject: [PATCH 1/4] refactor errors with stack Signed-off-by: Ruslan Bayandinov --- .../ipcontext/ipaddress/common.go | 8 +++---- .../ipcontext/ipneighbors/common.go | 6 +++--- .../ipcontext/iprule/common.go | 18 +++++++++------- .../ipcontext/iprule/heal.go | 6 ++++-- .../iptables4nattemplate/common.go | 21 +++++++++---------- .../routelocalnet/common.go | 7 +++++-- pkg/kernel/networkservice/inject/common.go | 6 +++--- pkg/kernel/tools/nshandle/ns_handle.go | 11 ++++++++-- 8 files changed, 48 insertions(+), 35 deletions(-) diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go index 9b3a770b..d87d724f 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go @@ -1,7 +1,7 @@ -// Copyright (c) 2020-2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2020-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -73,14 +73,14 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) var forwarderNetNS netns.NsHandle forwarderNetNS, err = nshandle.Current() if err != nil { - return errors.WithStack(err) + return err } defer func() { _ = forwarderNetNS.Close() }() var targetNetNS netns.NsHandle targetNetNS, err = nshandle.FromURL(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer func() { _ = targetNetNS.Close() }() diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go index 35405e61..89ae6deb 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go @@ -1,9 +1,9 @@ -// Copyright (c) 2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // // Copyright (c) 2022 Doc.ai and/or its affiliates. // +// Copyright (c) 2022-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -58,7 +58,7 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) } if err := setIPContextNeighbors(ctx, netlinkHandle, conn.GetContext().GetIpContext().GetIpNeighbors(), l); err != nil { - return errors.WithStack(err) + return err } // If payload is IP - we need to add additional neighbor diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go index 2472da12..20a5c2e7 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go @@ -1,5 +1,7 @@ // Copyright (c) 2022 Doc.ai and/or its affiliates. // +// Copyright (c) 2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -162,7 +164,7 @@ func policyToRule(policy *networkservice.PolicyRoute) (*netlink.Rule, error) { func ruleAdd(ctx context.Context, handle *netlink.Handle, policy *networkservice.PolicyRoute, tableID int) error { rule, err := policyToRule(policy) if err != nil { - return errors.WithStack(err) + return err } rule.Table = tableID @@ -243,14 +245,14 @@ func del(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) er if mechanism := kernel.ToMechanism(conn.GetMechanism()); mechanism != nil && mechanism.GetVLAN() == 0 { netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ps, ok := tableIDs.LoadAndDelete(conn.GetId()) if ok { for tableID, policy := range ps { if err := delRule(ctx, netlinkHandle, policy, tableID); err != nil { - return errors.WithStack(err) + return err } } } @@ -261,7 +263,7 @@ func del(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) er func delRule(ctx context.Context, handle *netlink.Handle, policy *networkservice.PolicyRoute, tableID int) error { rule, err := policyToRule(policy) if err != nil { - return errors.WithStack(err) + return err } if err := flushTable(ctx, handle, tableID); err != nil { @@ -277,7 +279,7 @@ func delRule(ctx context.Context, handle *netlink.Handle, policy *networkservice WithField("SrcPort", policy.SrcPort). WithField("duration", time.Since(now)). WithField("netlink", "RuleDel").Errorf("error %+v", err) - return errors.Wrapf(errors.WithStack(err), "failed to delete rule") + return errors.Wrapf(err, "failed to delete rule") } log.FromContext(ctx). WithField("From", policy.From). @@ -296,12 +298,12 @@ func flushTable(ctx context.Context, handle *netlink.Handle, tableID int) error }, netlink.RT_FILTER_TABLE) if err != nil { - return errors.Wrapf(errors.WithStack(err), "failed to list routes") + return errors.Wrapf(err, "failed to list routes") } for i := 0; i < len(routes); i++ { err := handle.RouteDel(&routes[i]) if err != nil { - return errors.Wrapf(errors.WithStack(err), "failed to delete route") + return errors.Wrapf(err, "failed to delete route") } } log.FromContext(ctx). @@ -317,7 +319,7 @@ func getFreeTableID(ctx context.Context, handle *netlink.Handle) (int, error) { }, netlink.RT_FILTER_TABLE) if err != nil { - return 0, errors.Wrapf(errors.WithStack(err), "getFreeTableID: failed to list routes") + return 0, errors.Wrapf(err, "getFreeTableID: failed to list routes") } // tableID = 0 is reserved diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go index 42a50314..97615c49 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go @@ -2,6 +2,8 @@ // // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -59,7 +61,7 @@ func recoverTableIDs(ctx context.Context, conn *networkservice.Connection, table for _, policy := range conn.Context.IpContext.Policies { policyRule, err := policyToRule(policy) if err != nil { - return errors.WithStack(err) + return err } for i := range podRules { if ruleEquals(&podRules[i], policyRule) { @@ -71,7 +73,7 @@ func recoverTableIDs(ctx context.Context, conn *networkservice.Connection, table WithField("Table", podRules[i].Table).Debug("policy recovered") err := delRule(ctx, netlinkHandle, policy, podRules[i].Table) if err != nil { - return errors.WithStack(err) + return err } break } diff --git a/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go b/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go index be3ae9a9..55dfd38b 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go @@ -1,5 +1,7 @@ // Copyright (c) 2022 Xored Software Inc and others. // +// Copyright (c) 2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -57,8 +59,7 @@ func (m *iptableManagerImpl) Get() (string, error) { exechelper.WithStderr(buf), ) if err != nil { - err = errors.Wrapf(err, "%s", buf.String()) - return "", err + return "", errors.Wrapf(err, "%s", buf.String()) } return buf.String(), nil @@ -76,8 +77,7 @@ func (m *iptableManagerImpl) Apply(rules []string) error { exechelper.WithStderr(buf), ) if err != nil { - err = errors.Wrapf(err, "%s", buf.String()) - return err + return errors.Wrapf(err, "%s", buf.String()) } } @@ -87,13 +87,13 @@ func (m *iptableManagerImpl) Apply(rules []string) error { func (m *iptableManagerImpl) writeTmpRule(rules string) (string, error) { fo, err := os.CreateTemp("/tmp", "rules-*") if err != nil { - return "", err + return "", errors.WithStack(err) } defer func() { _ = fo.Close() }() _, err = fo.WriteString(rules) if err != nil { - return "", err + return "", errors.WithStack(err) } return fo.Name(), nil @@ -116,8 +116,7 @@ func (m *iptableManagerImpl) Restore(rules string) error { exechelper.WithStderr(buf), ) if err != nil { - err = errors.Wrapf(err, "%s", buf.String()) - return err + return errors.Wrapf(err, "%s", buf.String()) } return nil @@ -136,7 +135,7 @@ func applyIptablesRules(ctx context.Context, conn *networkservice.Connection, c if mechanism != nil && len(mechanism.GetIPTables4NatTemplate()) != 0 { rules, err := mechanism.EvaluateIPTables4NatTemplate(conn) if err != nil { - return err + return errors.WithStack(err) } currentNsHandler, err := nshandle.Current() @@ -154,14 +153,14 @@ func applyIptablesRules(ctx context.Context, conn *networkservice.Connection, c err = nshandle.RunIn(currentNsHandler, targetHsHandler, func() error { initialRules, iptableErr := c.manager.Get() if iptableErr != nil { - return iptableErr + return errors.WithStack(iptableErr) } ctxMap.Store(applyIPTablesKey{}, initialRules) iptableErr = c.manager.Apply(rules) if iptableErr != nil { - return iptableErr + return errors.WithStack(iptableErr) } return nil diff --git a/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go b/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go index acff31a8..093f9f17 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go @@ -1,5 +1,7 @@ // Copyright (c) 2022 Xored Software Inc and others. // +// Copyright (c) 2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -25,6 +27,7 @@ import ( "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel" + "github.com/pkg/errors" "github.com/networkservicemesh/sdk-kernel/pkg/kernel/tools/nshandle" ) @@ -48,14 +51,14 @@ func setRouteLocalNet(conn *networkservice.Connection) error { fo, fileErr := os.Create(fmt.Sprintf("/proc/sys/net/ipv4/conf/%s/route_localnet", mechanism.GetInterfaceName())) if fileErr != nil { - return fileErr + return errors.WithStack(fileErr) } defer func() { _ = fo.Close() }() _, fileErr = fo.WriteString("1") if fileErr != nil { - return fileErr + return errors.WithStack(fileErr) } return nil diff --git a/pkg/kernel/networkservice/inject/common.go b/pkg/kernel/networkservice/inject/common.go index 4207a29a..ab0129d6 100644 --- a/pkg/kernel/networkservice/inject/common.go +++ b/pkg/kernel/networkservice/inject/common.go @@ -1,7 +1,7 @@ -// Copyright (c) 2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2022-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -153,7 +153,7 @@ func moveToContNetNS(vfConfig *vfconfig.VFConfig, vfRefCountMap map[string]int, } else { err = moveInterfaceToAnotherNamespace(ifName, hostNetNS, hostNetNS, contNetNS) } - return + return err } func moveToHostNetNS(vfConfig *vfconfig.VFConfig, vfRefCountMap map[string]int, vfRefKey, ifName string, hostNetNS, contNetNS netns.NsHandle) error { diff --git a/pkg/kernel/tools/nshandle/ns_handle.go b/pkg/kernel/tools/nshandle/ns_handle.go index b6dfae01..3f5d84de 100644 --- a/pkg/kernel/tools/nshandle/ns_handle.go +++ b/pkg/kernel/tools/nshandle/ns_handle.go @@ -1,5 +1,7 @@ // Copyright (c) 2020-2022 Doc.ai and/or its affiliates. // +// Copyright (c) 2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -30,7 +32,12 @@ func Current() (handle netns.NsHandle, err error) { runtime.LockOSThread() defer runtime.UnlockOSThread() - return netns.Get() + nsHandle, err := netns.Get() + if err != nil { + return -1, errors.WithStack(err) + } + + return nsHandle, nil } // FromURL creates net NS handle by file://path URL @@ -56,7 +63,7 @@ func RunIn(current, target netns.NsHandle, runner func() error) error { curr, err := netns.Get() if err != nil { - return err + return errors.WithStack(err) } defer func() { _ = curr.Close() }() From 1f7c3f21bed88ded91602e5018431bc2cd4495ce Mon Sep 17 00:00:00 2001 From: Ruslan Bayandinov Date: Thu, 26 Jan 2023 17:00:19 +0000 Subject: [PATCH 2/4] update generated files Signed-off-by: Ruslan Bayandinov --- .../connectioncontextkernel/ipcontext/iprule/table_map.gen.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/table_map.gen.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/table_map.gen.go index 630daf4e..15d44cce 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/table_map.gen.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/table_map.gen.go @@ -1,4 +1,6 @@ // Code generated by "-output table_map.gen.go -type Map -output table_map.gen.go -type Map"; DO NOT EDIT. +// Install -output table_map.gen.go -type Map by "go get -u github.com/searKing/golang/tools/-output table_map.gen.go -type Map" + package iprule import ( From 2ae7da142cf560d2784f5b99a47b2e3b5fcac225 Mon Sep 17 00:00:00 2001 From: Ruslan Bayandinov Date: Wed, 1 Feb 2023 17:08:16 +0000 Subject: [PATCH 3/4] update errors handling Signed-off-by: Ruslan Bayandinov --- pkg/kernel/link.go | 10 ++++---- .../ipcontext/ipaddress/common.go | 7 +++--- .../ipcontext/ipneighbors/common.go | 7 +++--- .../ipcontext/iprule/common.go | 25 ++++++++++--------- .../ipcontext/iprule/heal.go | 4 +-- .../ipcontext/routes/common.go | 13 +++++----- .../iptables4nattemplate/common.go | 8 +++--- .../connectioncontextkernel/mtu/common.go | 11 ++++---- .../routelocalnet/common.go | 9 ++++--- .../ethernetcontext/vf_common.go | 13 +++++----- pkg/kernel/tools/nshandle/ns_handle.go | 4 +-- 11 files changed, 54 insertions(+), 57 deletions(-) diff --git a/pkg/kernel/link.go b/pkg/kernel/link.go index f1791c6a..ee681356 100644 --- a/pkg/kernel/link.go +++ b/pkg/kernel/link.go @@ -1,9 +1,9 @@ -// Copyright (c) 2022 Cisco and/or its affiliates. -// // Copyright (c) 2020-2022 Intel Corporation. All Rights Reserved. // // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2022-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -290,19 +290,19 @@ func searchByName(ns netns.NsHandle, name, pciAddress string) (netlink.Link, err func GetNetlinkHandle(urlString string) (*netlink.Handle, error) { curNSHandle, err := nshandle.Current() if err != nil { - return nil, errors.WithStack(err) + return nil, err } defer func() { _ = curNSHandle.Close() }() nsHandle, err := nshandle.FromURL(urlString) if err != nil { - return nil, errors.WithStack(err) + return nil, err } defer func() { _ = nsHandle.Close() }() handle, err := netlink.NewHandleAtFrom(nsHandle, curNSHandle) if err != nil { - return nil, errors.WithStack(err) + return nil, errors.Wrap(err, "failed to create netlink NS handle") } return handle, nil } diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go index d87d724f..a05bc2dc 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go @@ -55,19 +55,18 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ifName := mechanism.GetInterfaceName() - l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) } var forwarderNetNS netns.NsHandle diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go index 89ae6deb..d41199be 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipneighbors/common.go @@ -46,15 +46,14 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) if mechanism := kernel.ToMechanism(conn.GetMechanism()); mechanism != nil && mechanism.GetVLAN() == 0 { netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ifName := mechanism.GetInterfaceName() - l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } if err := setIPContextNeighbors(ctx, netlinkHandle, conn.GetContext().GetIpContext().GetIpNeighbors(), l); err != nil { @@ -140,7 +139,7 @@ func setPeerNeighbor(ctx context.Context, handle *netlink.Handle, l, peerLink ne WithField("hardwareAddr", neigh.HardwareAddr). WithField("duration", time.Since(now)). WithField("netlink", "NeighSet").Error("setPeerNeighbor failed") - return errors.WithStack(err) + return errors.Wrapf(err, "failed to update ARP table with %s %s", neigh.IP.String(), neigh.HardwareAddr.String()) } log.FromContext(ctx). WithField("linkIndex", neigh.LinkIndex). diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go index 20a5c2e7..13b97c5c 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go @@ -45,13 +45,14 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) // Construct the netlink handle for the target namespace for this kernel interface netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() - l, err := netlinkHandle.LinkByName(mechanism.GetInterfaceName()) + ifName := mechanism.GetInterfaceName() + l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } ps, ok := tableIDs.Load(conn.GetId()) @@ -77,7 +78,7 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) for _, policy := range toAdd { tableID, err := getFreeTableID(ctx, netlinkHandle) if err != nil { - return errors.Wrapf(err, "failed to get free tableId") + return err } // If policy doesn't contain any route - add default if len(policy.Routes) == 0 { @@ -86,11 +87,11 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) for _, route := range policy.Routes { if err := routeAdd(ctx, netlinkHandle, l, route, tableID); err != nil { - return errors.Wrapf(err, "failed to add route") + return err } } if err := ruleAdd(ctx, netlinkHandle, policy, tableID); err != nil { - return errors.Wrapf(err, "failed to add rule") + return err } ps[tableID] = policy tableIDs.Store(conn.GetId(), ps) @@ -133,27 +134,27 @@ func policyToRule(policy *networkservice.PolicyRoute) (*netlink.Rule, error) { if policy.From != "" { src, err := netlink.ParseIPNet(policy.From) if err != nil { - return nil, errors.WithStack(err) + return nil, errors.Wrapf(err, "failed to parse string %s in ip/net format", policy.From) } rule.Src = src } if policy.Proto != "" { protocol, err := strconv.Atoi(policy.Proto) if err != nil { - return nil, errors.WithStack(err) + return nil, errors.Wrapf(err, "failed to parse ip protocol number %s", policy.Proto) } rule.IPProto = protocol } dstPortRange, err := networkservice.ParsePortRange(policy.DstPort) if err != nil { - return nil, errors.WithStack(err) + return nil, errors.Wrapf(err, "failed to parse port range %s", policy.DstPort) } if dstPortRange != nil { rule.Dport = netlink.NewRulePortRange(dstPortRange.Start, dstPortRange.End) } srcPortRange, err := networkservice.ParsePortRange(policy.SrcPort) if err != nil { - return nil, errors.WithStack(err) + return nil, errors.Wrapf(err, "failed to parse port range %s", policy.DstPort) } if srcPortRange != nil { rule.Sport = netlink.NewRulePortRange(srcPortRange.Start, srcPortRange.End) @@ -178,7 +179,7 @@ func ruleAdd(ctx context.Context, handle *netlink.Handle, policy *networkservice WithField("Table", tableID). WithField("duration", time.Since(now)). WithField("netlink", "RuleAdd").Errorf("error %+v", err) - return errors.WithStack(err) + return errors.Wrap(err, "failed to add rule") } log.FromContext(ctx). WithField("From", policy.From). @@ -227,7 +228,7 @@ func routeAdd(ctx context.Context, handle *netlink.Handle, l netlink.Link, route WithField("Table", tableID). WithField("duration", time.Since(now)). WithField("netlink", "RouteReplace").Errorf("error %+v", err) - return errors.WithStack(err) + return errors.Wrap(err, "failed to add route") } log.FromContext(ctx). WithField("link.Name", l.Attrs().Name). diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go index 97615c49..6946393b 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go @@ -48,13 +48,13 @@ func recoverTableIDs(ctx context.Context, conn *networkservice.Connection, table netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() podRules, err := netlinkHandle.RuleList(netlink.FAMILY_ALL) if err != nil { - return errors.WithStack(err) + return errors.Wrap(err, "failed to get list of rules") } // try to find the corresponding missing policies in the network namespace of the pod diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go index d275cb28..103f48ab 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go @@ -1,7 +1,7 @@ -// Copyright (c) 2020-2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2020-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -39,19 +39,18 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) if mechanism := kernel.ToMechanism(conn.GetMechanism()); mechanism != nil && mechanism.GetVLAN() == 0 { netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ifName := mechanism.GetInterfaceName() - l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) } var linkRoutes []*networkservice.Route @@ -106,7 +105,7 @@ func routeAdd(ctx context.Context, handle *netlink.Handle, l netlink.Link, scope WithField("Flags", kernelRoute.Flags). WithField("duration", time.Since(now)). WithField("netlink", "RouteReplace").Errorf("error %+v", err) - return errors.WithStack(err) + return errors.Wrap(err, "failed to add route") } log.FromContext(ctx). WithField("link.Name", l.Attrs().Name). diff --git a/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go b/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go index 55dfd38b..56ff74ad 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go @@ -87,13 +87,13 @@ func (m *iptableManagerImpl) Apply(rules []string) error { func (m *iptableManagerImpl) writeTmpRule(rules string) (string, error) { fo, err := os.CreateTemp("/tmp", "rules-*") if err != nil { - return "", errors.WithStack(err) + return "", errors.Wrapf(err, "failed to create new temporary file") } defer func() { _ = fo.Close() }() _, err = fo.WriteString(rules) if err != nil { - return "", errors.WithStack(err) + return "", errors.Wrap(err, "failed to write to temporary file") } return fo.Name(), nil @@ -153,14 +153,14 @@ func applyIptablesRules(ctx context.Context, conn *networkservice.Connection, c err = nshandle.RunIn(currentNsHandler, targetHsHandler, func() error { initialRules, iptableErr := c.manager.Get() if iptableErr != nil { - return errors.WithStack(iptableErr) + return errors.Wrap(iptableErr, "failed to get iptables rules") } ctxMap.Store(applyIPTablesKey{}, initialRules) iptableErr = c.manager.Apply(rules) if iptableErr != nil { - return errors.WithStack(iptableErr) + return errors.Wrap(iptableErr, "failed to apply iptables rules") } return nil diff --git a/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go b/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go index 7bed406d..36e6b413 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go @@ -1,7 +1,7 @@ -// Copyright (c) 2021-2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2021-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -45,19 +45,18 @@ func setMTU(ctx context.Context, conn *networkservice.Connection) error { netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ifName := mechanism.GetInterfaceName() - l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) } now := time.Now() diff --git a/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go b/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go index 093f9f17..0a11b71a 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/routelocalnet/common.go @@ -24,6 +24,7 @@ package routelocalnet import ( "fmt" "os" + "path/filepath" "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel" @@ -48,17 +49,17 @@ func setRouteLocalNet(conn *networkservice.Connection) error { defer func() { _ = targetHsHandler.Close() }() err = nshandle.RunIn(currentNsHandler, targetHsHandler, func() error { - fo, fileErr := os.Create(fmt.Sprintf("/proc/sys/net/ipv4/conf/%s/route_localnet", mechanism.GetInterfaceName())) - + file := filepath.Clean(fmt.Sprintf("/proc/sys/net/ipv4/conf/%s/route_localnet", mechanism.GetInterfaceName())) + fo, fileErr := os.Create(file) if fileErr != nil { - return errors.WithStack(fileErr) + return errors.Wrapf(fileErr, "failed to create file %s", file) } defer func() { _ = fo.Close() }() _, fileErr = fo.WriteString("1") if fileErr != nil { - return errors.WithStack(fileErr) + return errors.Wrapf(fileErr, "failed to write to file %s", file) } return nil diff --git a/pkg/kernel/networkservice/ethernetcontext/vf_common.go b/pkg/kernel/networkservice/ethernetcontext/vf_common.go index 0d24121f..c4806c12 100644 --- a/pkg/kernel/networkservice/ethernetcontext/vf_common.go +++ b/pkg/kernel/networkservice/ethernetcontext/vf_common.go @@ -1,7 +1,7 @@ -// Copyright (c) 2022 Cisco and/or its affiliates. -// // Copyright (c) 2021-2022 Nordix Foundation. // +// Copyright (c) 2022-2023 Cisco and/or its affiliates. +// // SPDX-License-Identifier: Apache-2.0 // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -41,15 +41,14 @@ func setKernelHwAddress(ctx context.Context, conn *networkservice.Connection, is if mechanism := kernel.ToMechanism(conn.GetMechanism()); mechanism != nil { netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL()) if err != nil { - return errors.WithStack(err) + return err } defer netlinkHandle.Close() ifName := mechanism.GetInterfaceName() - l, err := netlinkHandle.LinkByName(ifName) if err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to find link %s", ifName) } if ethernetContext := conn.GetContext().GetEthernetContext(); ethernetContext != nil { @@ -71,13 +70,13 @@ func setKernelHwAddress(ctx context.Context, conn *networkservice.Connection, is return nil } if err = netlinkHandle.LinkSetDown(l); err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to disable link device %s", l.Attrs().Name) } if err = netlinkHandle.LinkSetHardwareAddr(l, macAddr); err != nil { return errors.Wrapf(err, "failed to set MAC address for the VF: %v", macAddr) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.WithStack(err) + return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) } log.FromContext(ctx). WithField("link.Name", l.Attrs().Name). diff --git a/pkg/kernel/tools/nshandle/ns_handle.go b/pkg/kernel/tools/nshandle/ns_handle.go index 3f5d84de..efd60991 100644 --- a/pkg/kernel/tools/nshandle/ns_handle.go +++ b/pkg/kernel/tools/nshandle/ns_handle.go @@ -34,7 +34,7 @@ func Current() (handle netns.NsHandle, err error) { nsHandle, err := netns.Get() if err != nil { - return -1, errors.WithStack(err) + return -1, errors.Wrap(err, "failed to create net NS handle") } return nsHandle, nil @@ -63,7 +63,7 @@ func RunIn(current, target netns.NsHandle, runner func() error) error { curr, err := netns.Get() if err != nil { - return errors.WithStack(err) + return errors.Wrap(err, "failed to create net NS handle") } defer func() { _ = curr.Close() }() From 5e663a2f58763a9f4140e29a0324b79e218cd7c0 Mon Sep 17 00:00:00 2001 From: Ruslan Bayandinov Date: Tue, 14 Feb 2023 12:04:46 +0000 Subject: [PATCH 4/4] address review comments Signed-off-by: Ruslan Bayandinov --- .../connectioncontextkernel/ipcontext/ipaddress/common.go | 2 +- .../connectioncontextkernel/ipcontext/routes/common.go | 2 +- .../networkservice/connectioncontextkernel/mtu/common.go | 2 +- pkg/kernel/networkservice/ethernetcontext/vf_common.go | 2 +- pkg/kernel/tools/nshandle/ns_handle.go | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go index a05bc2dc..9d9c5703 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/common.go @@ -66,7 +66,7 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) + return errors.Wrapf(err, "failed to setup link for the interface %v", l) } var forwarderNetNS netns.NsHandle diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go index 103f48ab..2c6a708a 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/routes/common.go @@ -50,7 +50,7 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) + return errors.Wrapf(err, "failed to setup link for the interface %v", l) } var linkRoutes []*networkservice.Route diff --git a/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go b/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go index 36e6b413..0f4e7176 100644 --- a/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go +++ b/pkg/kernel/networkservice/connectioncontextkernel/mtu/common.go @@ -56,7 +56,7 @@ func setMTU(ctx context.Context, conn *networkservice.Connection) error { } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) + return errors.Wrapf(err, "failed to setup link for the interface %v", l) } now := time.Now() diff --git a/pkg/kernel/networkservice/ethernetcontext/vf_common.go b/pkg/kernel/networkservice/ethernetcontext/vf_common.go index c4806c12..7776d485 100644 --- a/pkg/kernel/networkservice/ethernetcontext/vf_common.go +++ b/pkg/kernel/networkservice/ethernetcontext/vf_common.go @@ -76,7 +76,7 @@ func setKernelHwAddress(ctx context.Context, conn *networkservice.Connection, is return errors.Wrapf(err, "failed to set MAC address for the VF: %v", macAddr) } if err = netlinkHandle.LinkSetUp(l); err != nil { - return errors.Wrapf(err, "failed to enable link device %s", l.Attrs().Name) + return errors.Wrapf(err, "failed to setup link for the interface %v", l) } log.FromContext(ctx). WithField("link.Name", l.Attrs().Name). diff --git a/pkg/kernel/tools/nshandle/ns_handle.go b/pkg/kernel/tools/nshandle/ns_handle.go index efd60991..401fc07d 100644 --- a/pkg/kernel/tools/nshandle/ns_handle.go +++ b/pkg/kernel/tools/nshandle/ns_handle.go @@ -34,7 +34,7 @@ func Current() (handle netns.NsHandle, err error) { nsHandle, err := netns.Get() if err != nil { - return -1, errors.Wrap(err, "failed to create net NS handle") + return -1, errors.Wrap(err, "failed to get net NS handle") } return nsHandle, nil @@ -63,7 +63,7 @@ func RunIn(current, target netns.NsHandle, runner func() error) error { curr, err := netns.Get() if err != nil { - return errors.Wrap(err, "failed to create net NS handle") + return errors.Wrap(err, "failed to get net NS handle") } defer func() { _ = curr.Close() }()