From 4f9f0dfc3e1b29a6e12a1f487d4040daa827adc2 Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Tue, 2 Oct 2018 11:02:36 +0530 Subject: [PATCH 1/5] Replace weave-local-pods ipset with physdev iptables rule by-pass ingress network policy chains and accept the packet if destination is not one of the local pods return the packets so the egress network policy chains are skipped in case packet's source is not one of the local pods Fixes #3344 --- prog/weave-npc/main.go | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 9d054206..ab895977 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -125,6 +125,12 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { } } + // If the destination address is not any of the local pods, let it through + if err := ipt.Append(npc.TableFilter, npc.MainChain, + "-m", "physdev", "--physdev-out=vethwe-bridge", "-j", "ACCEPT"); err != nil { + return err + } + if err := ipt.Append(npc.TableFilter, npc.MainChain, "-m", "state", "--state", "NEW", "-j", string(npc.DefaultChain)); err != nil { return err @@ -135,21 +141,6 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { return err } - // If the destination address is not any of the local pods, let it through - found, err := ipsetExist(ips, npc.LocalIpset) - if err != nil { - return err - } - if !found { - if err := ips.Create(npc.LocalIpset, ipset.HashIP); err != nil { - return err - } - } - if err := ipt.Append(npc.TableFilter, npc.MainChain, - "-m", "set", "!", "--match-set", npc.LocalIpset, "dst", "-j", "ACCEPT"); err != nil { - return err - } - if err := ipt.Append(npc.TableFilter, npc.EgressMarkChain, "-j", "MARK", "--set-xmark", npc.EgressMark); err != nil { return err @@ -158,7 +149,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { // Egress rules: // // -A WEAVE-NPC-EGRESS -m state --state RELATED,ESTABLISHED -j ACCEPT - // -A WEAVE-NPC-EGRESS -m set ! --match-set weave-local-pods src -j RETURN + // -A WEAVE-NPC-EGRESS -m physdev --physdev-in=vethwe-bridge -j RETURN // -A WEAVE-NPC-EGRESS -d 224.0.0.0/4 -j RETURN // -A WEAVE-NPC-EGRESS -m state --state NEW -j WEAVE-NPC-EGRESS-DEFAULT // -A WEAVE-NPC-EGRESS -m state --state NEW -m mark ! --mark 0x40000/0x40000 -j WEAVE-NPC-EGRESS-CUSTOM @@ -179,7 +170,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { ruleSpecs := [][]string{ {"-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT"}, - {"-m", "state", "--state", "NEW", "-m", "set", "!", "--match-set", npc.LocalIpset, "src", "-j", "RETURN"}, + {"-m", "physdev", "--physdev-in=vethwe-bridge", "-j", "RETURN"}, } if allowMcast { ruleSpecs = append(ruleSpecs, []string{"-d", "224.0.0.0/4", "-j", "RETURN"}) From e3eb16a1f92b272104a832ef19f05ff92543cb2f Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Tue, 9 Oct 2018 11:21:47 +0530 Subject: [PATCH 2/5] addressing review comments --- prog/weave-npc/main.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index ab895977..61cae54c 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -31,6 +31,7 @@ var ( allowMcast bool nodeName string maxList int + bridgeName string ) func handleError(err error) { common.CheckFatal(err) } @@ -127,7 +128,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { // If the destination address is not any of the local pods, let it through if err := ipt.Append(npc.TableFilter, npc.MainChain, - "-m", "physdev", "--physdev-out=vethwe-bridge", "-j", "ACCEPT"); err != nil { + "-m", "physdev", "--physdev-out="+bridgeName, "-j", "ACCEPT"); err != nil { return err } @@ -170,7 +171,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { ruleSpecs := [][]string{ {"-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT"}, - {"-m", "physdev", "--physdev-in=vethwe-bridge", "-j", "RETURN"}, + {"-m", "physdev", "--physdev-in=" + bridgeName, "-j", "RETURN"}, } if allowMcast { ruleSpecs = append(ruleSpecs, []string{"-d", "224.0.0.0/4", "-j", "RETURN"}) @@ -323,6 +324,7 @@ func main() { rootCmd.PersistentFlags().BoolVar(&allowMcast, "allow-mcast", true, "allow all multicast traffic") rootCmd.PersistentFlags().StringVar(&nodeName, "node-name", "", "only generate rules that apply to this node") rootCmd.PersistentFlags().IntVar(&maxList, "max-list-size", 1024, "maximum size of ipset list (for namespaces)") + rootCmd.PersistentFlags().StringVar(&bridgeName, "bridge-name", "vethwe-bridge", "name of the brige on which packets are received and sent") handleError(rootCmd.Execute()) } From d00e7335bb196d6d8ff25e1b5990ce0c90965777 Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Tue, 16 Oct 2018 14:02:23 +0530 Subject: [PATCH 3/5] dont maintain weave-local-pods ipset on pod add/delete/update --- npc/namespace.go | 15 --------------- prog/weave-npc/main.go | 20 ++++++++++---------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/npc/namespace.go b/npc/namespace.go index 9d43f1f8..76a44993 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -169,10 +169,6 @@ func (ns *ns) addPod(obj *coreapi.Pod) error { return nil } - if ns.checkLocalPod(obj) { - ns.ips.AddEntry(obj.ObjectMeta.UID, LocalIpset, obj.Status.PodIP, podComment(obj)) - } - foundIngress, foundEgress, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) if err != nil { return err @@ -201,10 +197,6 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { } if hasIP(oldObj) && !hasIP(newObj) { - if ns.checkLocalPod(oldObj) { - ns.ips.DelEntry(oldObj.ObjectMeta.UID, LocalIpset, oldObj.Status.PodIP) - } - if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, oldObj.Status.PodIP); err != nil { return err } @@ -216,9 +208,6 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { } if !hasIP(oldObj) && hasIP(newObj) { - if ns.checkLocalPod(newObj) { - ns.ips.AddEntry(newObj.ObjectMeta.UID, LocalIpset, newObj.Status.PodIP, podComment(newObj)) - } foundIngress, foundEgress, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) if err != nil { return err @@ -303,10 +292,6 @@ func (ns *ns) deletePod(obj *coreapi.Pod) error { return nil } - if ns.checkLocalPod(obj) { - ns.ips.DelEntry(obj.ObjectMeta.UID, LocalIpset, obj.Status.PodIP) - } - if err := ns.ips.DelEntry(obj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, obj.Status.PodIP); err != nil { return err } diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 61cae54c..ea164d4a 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -25,13 +25,13 @@ import ( ) var ( - version = "unreleased" - metricsAddr string - logLevel string - allowMcast bool - nodeName string - maxList int - bridgeName string + version = "unreleased" + metricsAddr string + logLevel string + allowMcast bool + nodeName string + maxList int + bridgePortName string ) func handleError(err error) { common.CheckFatal(err) } @@ -128,7 +128,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { // If the destination address is not any of the local pods, let it through if err := ipt.Append(npc.TableFilter, npc.MainChain, - "-m", "physdev", "--physdev-out="+bridgeName, "-j", "ACCEPT"); err != nil { + "-m", "physdev", "--physdev-out="+bridgePortName, "-j", "ACCEPT"); err != nil { return err } @@ -171,7 +171,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { ruleSpecs := [][]string{ {"-m", "state", "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT"}, - {"-m", "physdev", "--physdev-in=" + bridgeName, "-j", "RETURN"}, + {"-m", "physdev", "--physdev-in=" + bridgePortName, "-j", "RETURN"}, } if allowMcast { ruleSpecs = append(ruleSpecs, []string{"-d", "224.0.0.0/4", "-j", "RETURN"}) @@ -324,7 +324,7 @@ func main() { rootCmd.PersistentFlags().BoolVar(&allowMcast, "allow-mcast", true, "allow all multicast traffic") rootCmd.PersistentFlags().StringVar(&nodeName, "node-name", "", "only generate rules that apply to this node") rootCmd.PersistentFlags().IntVar(&maxList, "max-list-size", 1024, "maximum size of ipset list (for namespaces)") - rootCmd.PersistentFlags().StringVar(&bridgeName, "bridge-name", "vethwe-bridge", "name of the brige on which packets are received and sent") + rootCmd.PersistentFlags().StringVar(&bridgePortName, "bridge-port-name", "vethwe-bridge", "name of the brige port on which packets are received and sent") handleError(rootCmd.Execute()) } From d351b9f0203f4d13e6f1739914932cb7146641f0 Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Wed, 17 Oct 2018 15:34:46 +0530 Subject: [PATCH 4/5] remove weave-local-pods ipset which no longer used by weave-npc --- prog/weave-npc/main.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index ea164d4a..44c65a37 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -186,6 +186,17 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { return err } + // delete `weave-local-pods` ipset which is no longer used by weave-npc + weaveLocalPodExist, err := ipsetExist(ips, npc.LocalIpset) + if err != nil { + common.Log.Errorf("Failed to destroy ipset '%s'", npc.LocalIpset) + } else if weaveLocalPodExist { + common.Log.Debugf("Destroying ipset '%s'", npc.LocalIpset) + if err := ips.Destroy(npc.LocalIpset); err != nil { + common.Log.Errorf("Failed to destroy ipset '%s'", npc.LocalIpset) + } + } + return nil } From 408e5f7bddd2f0e6b50cfb6e84818cacb395a39f Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Wed, 17 Oct 2018 21:16:52 +0530 Subject: [PATCH 5/5] make error message more appropriate --- prog/weave-npc/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 44c65a37..12b5078f 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -189,7 +189,7 @@ func createBaseRules(ipt *iptables.IPTables, ips ipset.Interface) error { // delete `weave-local-pods` ipset which is no longer used by weave-npc weaveLocalPodExist, err := ipsetExist(ips, npc.LocalIpset) if err != nil { - common.Log.Errorf("Failed to destroy ipset '%s'", npc.LocalIpset) + common.Log.Errorf("Failed to look if ipset '%s' exists", npc.LocalIpset) } else if weaveLocalPodExist { common.Log.Debugf("Destroying ipset '%s'", npc.LocalIpset) if err := ips.Destroy(npc.LocalIpset); err != nil {