From 0f3864968eb3ca9f2b1018dff044ae8259499f2d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 13 May 2020 14:44:24 +0000 Subject: [PATCH 1/5] refactor: supply /proc path to CreateAndAttachVeth() Move the assumptions about where we're running up towards the caller. --- net/bridge.go | 22 +++++++++++----------- net/veth.go | 14 ++++++++------ plugin/net/driver.go | 2 +- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/net/bridge.go b/net/bridge.go index f35da524..d36bad11 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -63,10 +63,10 @@ const ( ) type Bridge interface { - init(config *BridgeConfig) error // create and initialise bridge device(s) - attach(veth *netlink.Veth) error // attach veth to bridge - IsFastdp() bool // does this bridge use fastdp? - String() string // human-readable type string + init(procPath string, config *BridgeConfig) error // create and initialise bridge device(s) + attach(veth *netlink.Veth) error // attach veth to bridge + IsFastdp() bool // does this bridge use fastdp? + String() string // human-readable type string } // Used to indicate a fallback to the Bridge type @@ -250,7 +250,7 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger, ips } for { - if err := bridgeType.init(config); err != nil { + if err := bridgeType.init(procPath, config); err != nil { if errors.Cause(err) == errBridgeNotSupported { log.Warnf("Skipping bridge creation of %q due to: %s", bridgeType, err) bridgeType = bridgeImpl{} @@ -345,11 +345,11 @@ func (b bridgeImpl) initPrep(config *BridgeConfig) error { return nil } -func (b bridgeImpl) init(config *BridgeConfig) error { +func (b bridgeImpl) init(procPath string, config *BridgeConfig) error { if err := b.initPrep(config); err != nil { return err } - if _, err := CreateAndAttachVeth(BridgeIfName, PcapIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { + if _, err := CreateAndAttachVeth(procPath, BridgeIfName, PcapIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { return netlink.LinkSetUp(veth) }); err != nil { return errors.Wrap(err, "creating pcap veth pair") @@ -361,7 +361,7 @@ func (b bridgeImpl) init(config *BridgeConfig) error { return nil } -func (f fastdpImpl) init(config *BridgeConfig) error { +func (f fastdpImpl) init(procPath string, config *BridgeConfig) error { odpSupported, err := odp.CreateDatapath(f.datapathName) if !odpSupported { msg := "" @@ -392,14 +392,14 @@ func (f fastdpImpl) init(config *BridgeConfig) error { return nil } -func (bf bridgedFastdpImpl) init(config *BridgeConfig) error { - if err := bf.fastdpImpl.init(config); err != nil { +func (bf bridgedFastdpImpl) init(procPath string, config *BridgeConfig) error { + if err := bf.fastdpImpl.init(procPath, config); err != nil { return err } if err := bf.bridgeImpl.initPrep(config); err != nil { return err } - if _, err := CreateAndAttachVeth(BridgeIfName, DatapathIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { + if _, err := CreateAndAttachVeth(procPath, BridgeIfName, DatapathIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { if err := netlink.LinkSetUp(veth); err != nil { return errors.Wrapf(err, "setting link up on %q", veth.Attrs().Name) } diff --git a/net/veth.go b/net/veth.go index dd82abef..a022292f 100644 --- a/net/veth.go +++ b/net/veth.go @@ -13,7 +13,7 @@ import ( ) // create and attach a veth to the Weave bridge -func CreateAndAttachVeth(name, peerName, bridgeName string, mtu int, keepTXOn bool, errIfLinkExist bool, init func(peer netlink.Link) error) (*netlink.Veth, error) { +func CreateAndAttachVeth(procPath, name, peerName, bridgeName string, mtu int, keepTXOn bool, errIfLinkExist bool, init func(peer netlink.Link) error) (*netlink.Veth, error) { bridge, err := netlink.LinkByName(bridgeName) if err != nil { return nil, fmt.Errorf(`bridge "%s" not present; did you launch weave?`, bridgeName) @@ -112,6 +112,9 @@ func interfaceExistsInNamespace(netNSPath string, ifName string) bool { } func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMulticastRoute bool, cidrs []*net.IPNet, keepTXOn bool, hairpinMode bool) error { + // AttachContainer expects to be called in host pid namespace + const procPath = "/proc" + ns, err := netns.GetFromPath(netNSPath) if err != nil { return err @@ -124,12 +127,12 @@ func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMult id = id[:maxIDLen] // trim passed ID if too long } name, peerName := vethPrefix+"pl"+id, vethPrefix+"pg"+id - veth, err := CreateAndAttachVeth(name, peerName, bridgeName, mtu, keepTXOn, true, func(veth netlink.Link) error { + veth, err := CreateAndAttachVeth(procPath, name, peerName, bridgeName, mtu, keepTXOn, true, func(veth netlink.Link) error { if err := netlink.LinkSetNsFd(veth, int(ns)); err != nil { return fmt.Errorf("failed to move veth to container netns: %s", err) } if err := WithNetNS(ns, func() error { - return setupIface(peerName, ifName) + return setupIface(procPath, peerName, ifName) }); err != nil { return fmt.Errorf("error setting up interface: %s", err) } @@ -206,7 +209,7 @@ func setupIfaceAddrs(veth netlink.Link, withMulticastRoute bool, cidrs []*net.IP } // setupIface expects to be called in the container's netns -func setupIface(ifaceName, newIfName string) error { +func setupIface(procPath, ifaceName, newIfName string) error { ipt, err := iptables.New() if err != nil { return err @@ -219,8 +222,7 @@ func setupIface(ifaceName, newIfName string) error { if err := netlink.LinkSetName(link, newIfName); err != nil { return err } - // This is only called by AttachContainer which is only called in host pid namespace - if err := configureARPCache("/proc", newIfName); err != nil { + if err := configureARPCache(procPath, newIfName); err != nil { return err } return ipt.Append("filter", "INPUT", "-i", newIfName, "-d", "224.0.0.0/4", "-j", "DROP") diff --git a/plugin/net/driver.go b/plugin/net/driver.go index 70de3efa..cea245b7 100644 --- a/plugin/net/driver.go +++ b/plugin/net/driver.go @@ -134,7 +134,7 @@ func (driver *driver) CreateEndpoint(create *api.CreateEndpointRequest) (*api.Cr // create veths. note we assume endpoint IDs are unique in the first 9 chars name, peerName := vethPair(create.EndpointID) - if _, err := weavenet.CreateAndAttachVeth(name, peerName, weavenet.WeaveBridgeName, 0, false, true, nil); err != nil { + if _, err := weavenet.CreateAndAttachVeth("/proc", name, peerName, weavenet.WeaveBridgeName, 0, false, true, nil); err != nil { return nil, driver.error("JoinEndpoint", "%s", err) } From e2088ef401a794e686c4bc1882ec63a37e8a4f3c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 13 May 2020 15:01:37 +0000 Subject: [PATCH 2/5] Disable accept_ra setting on interfaces we create --- net/bridge.go | 4 ++++ net/veth.go | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/net/bridge.go b/net/bridge.go index d36bad11..0cf0e17b 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -279,6 +279,10 @@ func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger, ips return bridgeType, errors.Wrap(err, "setting proxy_arp") } } + // No ipv6 router advertisments please + if err := sysctl(procPath, "net/ipv6/conf/"+config.WeaveBridgeName+"/accept_ra", "0"); err != nil { + return bridgeType, errors.Wrap(err, "setting accept_ra to 0") + } if err := linkSetUpByName(config.WeaveBridgeName); err != nil { return bridgeType, err diff --git a/net/veth.go b/net/veth.go index a022292f..aa21ff9a 100644 --- a/net/veth.go +++ b/net/veth.go @@ -49,6 +49,13 @@ func CreateAndAttachVeth(procPath, name, peerName, bridgeName string, mtu int, k if err := bridgeType.attach(veth); err != nil { return cleanup("attaching veth %q to %q: %s", name, bridgeName, err) } + // No ipv6 router advertisments please + if err := sysctl(procPath, "net/ipv6/conf/"+name+"/accept_ra", "0"); err != nil { + return cleanup("setting accept_ra to 0: %s", err) + } + if err := sysctl(procPath, "net/ipv6/conf/"+peerName+"/accept_ra", "0"); err != nil { + return cleanup("setting accept_ra to 0: %s", err) + } if !bridgeType.IsFastdp() && !keepTXOn { if err := EthtoolTXOff(veth.PeerName); err != nil { return cleanup(`unable to set tx off on %q: %s`, peerName, err) From 6ad3e69faf9c88425db559cc8bf51c0a9f83c5e1 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 18 May 2020 14:47:26 +0000 Subject: [PATCH 3/5] Plumb through /proc path config to Docker plugin --- net/netns.go | 6 +++--- net/veth.go | 4 ++-- plugin/net/driver.go | 6 ++++-- plugin/net/watcher.go | 9 ++------- plugin/plugin.go | 9 +++++---- prog/weaver/main.go | 1 + 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/net/netns.go b/net/netns.go index 0a5c2249..56081914 100644 --- a/net/netns.go +++ b/net/netns.go @@ -55,9 +55,9 @@ func WithNetNSByPath(path string, work func() error) error { } func NSPathByPid(pid int) string { - return NSPathByPidWithRoot("/", pid) + return NSPathByPidWithProc("/proc", pid) } -func NSPathByPidWithRoot(root string, pid int) string { - return filepath.Join(root, fmt.Sprintf("/proc/%d/ns/net", pid)) +func NSPathByPidWithProc(procPath string, pid int) string { + return filepath.Join(procPath, fmt.Sprint(pid), "/ns/net") } diff --git a/net/veth.go b/net/veth.go index aa21ff9a..dc2f776a 100644 --- a/net/veth.go +++ b/net/veth.go @@ -236,7 +236,7 @@ func setupIface(procPath, ifaceName, newIfName string) error { } // configureARP is a helper for the Docker plugin which doesn't set the addresses itself -func ConfigureARP(prefix, rootPath string) error { +func ConfigureARP(prefix, procPath string) error { links, err := netlink.LinkList() if err != nil { return err @@ -244,7 +244,7 @@ func ConfigureARP(prefix, rootPath string) error { for _, link := range links { ifName := link.Attrs().Name if strings.HasPrefix(ifName, prefix) { - configureARPCache(rootPath+"/proc", ifName) + configureARPCache(procPath, ifName) if addrs, err := netlink.AddrList(link, netlink.FAMILY_V4); err == nil { for _, addr := range addrs { arping.GratuitousArpOverIfaceByName(addr.IPNet.IP, ifName) diff --git a/plugin/net/driver.go b/plugin/net/driver.go index cea245b7..5af2aa9b 100644 --- a/plugin/net/driver.go +++ b/plugin/net/driver.go @@ -39,9 +39,10 @@ type driver struct { // used only by plugin-v2 forceMulticast bool networks map[string]network + procPath string } -func New(client *docker.Client, weave *weaveapi.Client, name, scope string, dns, isPluginV2, forceMulticast bool) (skel.Driver, error) { +func New(client *docker.Client, weave *weaveapi.Client, name, scope string, dns, isPluginV2, forceMulticast bool, procPath string) (skel.Driver, error) { driver := &driver{ name: name, scope: scope, @@ -51,6 +52,7 @@ func New(client *docker.Client, weave *weaveapi.Client, name, scope string, dns, // make sure that it's used only by plugin-v2 forceMulticast: isPluginV2 && forceMulticast, networks: make(map[string]network), + procPath: procPath, } // Do not start watcher in the case of plugin v2, which prevents us from @@ -134,7 +136,7 @@ func (driver *driver) CreateEndpoint(create *api.CreateEndpointRequest) (*api.Cr // create veths. note we assume endpoint IDs are unique in the first 9 chars name, peerName := vethPair(create.EndpointID) - if _, err := weavenet.CreateAndAttachVeth("/proc", name, peerName, weavenet.WeaveBridgeName, 0, false, true, nil); err != nil { + if _, err := weavenet.CreateAndAttachVeth(driver.procPath, name, peerName, weavenet.WeaveBridgeName, 0, false, true, nil); err != nil { return nil, driver.error("JoinEndpoint", "%s", err) } diff --git a/plugin/net/watcher.go b/plugin/net/watcher.go index 2c386a8c..b763d1a6 100644 --- a/plugin/net/watcher.go +++ b/plugin/net/watcher.go @@ -43,14 +43,9 @@ func (w *watcher) ContainerStarted(id string) { w.driver.warn("ContainerStarted", "unable to register %s with weaveDNS: %s", id, err) } } - rootDir := "/" - if w.driver.isPluginV2 { - // We bind mount host's /proc to /host/proc for plugin-v2 - rootDir = "/host" - } - netNSPath := weavenet.NSPathByPidWithRoot(rootDir, info.State.Pid) + netNSPath := weavenet.NSPathByPidWithProc(w.driver.procPath, info.State.Pid) if err := weavenet.WithNetNSByPath(netNSPath, func() error { - return weavenet.ConfigureARP(weavenet.VethName, rootDir) + return weavenet.ConfigureARP(weavenet.VethName, w.driver.procPath) }); err != nil { w.driver.warn("ContainerStarted", "unable to configure interfaces: %s", err) } diff --git a/plugin/plugin.go b/plugin/plugin.go index c1890193..5b644bf6 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -32,6 +32,7 @@ type Config struct { EnableV2Multicast bool DNS bool DefaultSubnet string + ProcPath string // path to reach host /proc filesystem } type Plugin struct { @@ -62,7 +63,7 @@ func (plugin *Plugin) run(dockerClient *docker.Client, weave *weaveapi.Client, r endChan := make(chan error, 1) if plugin.Socket != "" { - globalListener, err := listenAndServe(dockerClient, weave, plugin.Socket, endChan, "global", false, plugin.DNS, plugin.EnableV2, plugin.EnableV2Multicast) + globalListener, err := listenAndServe(dockerClient, weave, plugin.Socket, endChan, "global", false, plugin.DNS, plugin.EnableV2, plugin.EnableV2Multicast, plugin.ProcPath) if err != nil { return err } @@ -70,7 +71,7 @@ func (plugin *Plugin) run(dockerClient *docker.Client, weave *weaveapi.Client, r defer globalListener.Close() } if plugin.MeshSocket != "" { - meshListener, err := listenAndServe(dockerClient, weave, plugin.MeshSocket, endChan, "local", true, plugin.DNS, plugin.EnableV2, plugin.EnableV2Multicast) + meshListener, err := listenAndServe(dockerClient, weave, plugin.MeshSocket, endChan, "local", true, plugin.DNS, plugin.EnableV2, plugin.EnableV2Multicast, plugin.ProcPath) if err != nil { return err } @@ -87,7 +88,7 @@ func (plugin *Plugin) run(dockerClient *docker.Client, weave *weaveapi.Client, r return <-endChan } -func listenAndServe(dockerClient *docker.Client, weave *weaveapi.Client, address string, endChan chan<- error, scope string, withIpam, dns bool, isPluginV2, forceMulticast bool) (net.Listener, error) { +func listenAndServe(dockerClient *docker.Client, weave *weaveapi.Client, address string, endChan chan<- error, scope string, withIpam, dns bool, isPluginV2, forceMulticast bool, procPath string) (net.Listener, error) { var name string if isPluginV2 { name = pluginV2Name @@ -95,7 +96,7 @@ func listenAndServe(dockerClient *docker.Client, weave *weaveapi.Client, address name = pluginNameFromAddress(address) } - d, err := netplugin.New(dockerClient, weave, name, scope, dns, isPluginV2, forceMulticast) + d, err := netplugin.New(dockerClient, weave, name, scope, dns, isPluginV2, forceMulticast, procPath) if err != nil { return nil, err } diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 7194b205..ca89c642 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -453,6 +453,7 @@ func main() { pluginConfig.DNS = !noDNS pluginConfig.DefaultSubnet = defaultSubnet.String() + pluginConfig.ProcPath = procPath plugin := plugin.NewPlugin(pluginConfig) // The weave script always waits for a status call to succeed, From 77fbb2c12b2c6a91992d259714e5d7c0c9d4356a Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Thu, 16 Apr 2020 13:13:52 +0530 Subject: [PATCH 4/5] introduce weavenpc_policy_enforcement_errors prometheus metric to track number of errors occured during enforcing network policies Fixes #3791 --- npc/metrics/metrics.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/npc/metrics/metrics.go b/npc/metrics/metrics.go index 5c6149b8..cad65531 100644 --- a/npc/metrics/metrics.go +++ b/npc/metrics/metrics.go @@ -22,6 +22,12 @@ var ( }, []string{"protocol", "dport"}, ) + PolicyEnforcementErrors = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "weavenpc_policy_enforcement_errors", + Help: "Errors occured in enforcing the network policies.", + }, + ) ) func gatherMetrics() { @@ -95,6 +101,9 @@ func Start(addr string) error { if err := prometheus.Register(blockedConnections); err != nil { return err } + if err := prometheus.Register(PolicyEnforcementErrors); err != nil { + return err + } http.Handle("/metrics", promhttp.Handler()) From 09a204b9786ca49342db0542a194972b055aa3a3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 22 May 2020 09:55:07 +0000 Subject: [PATCH 5/5] NPC: increment metric each time an error is logged --- prog/weave-npc/main.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 9ac6f778..4da4a1de 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -34,7 +34,12 @@ var ( bridgePortName string ) -func handleError(err error) { common.CheckError(err) } +func handleError(err error) { + if err != nil { + metrics.PolicyEnforcementErrors.Inc() + common.Log.Error(err) + } +} func handleFatal(err error) { common.CheckFatal(err) }