From ad5980caa303137fff9567b28badd9691835d5ec Mon Sep 17 00:00:00 2001
From: Mazdak Nasab <mazdak.nasab@gmail.com>
Date: Wed, 10 Jan 2024 16:43:38 -0800
Subject: [PATCH 1/5] Skip non-calico ipsets

---
 felix/ipsets/ipsets.go         | 471 ++++++++++++++++++++-------------
 felix/ipsets/ipsets_test.go    |   4 +-
 felix/ipsets/utils_for_test.go |  70 ++---
 3 files changed, 326 insertions(+), 219 deletions(-)

diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go
index e5100e9c22d..4b60f3666ce 100644
--- a/felix/ipsets/ipsets.go
+++ b/felix/ipsets/ipsets.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2017-2023 Tigera, Inc. All rights reserved.
+// Copyright (c) 2017-2024 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -388,7 +388,93 @@ func (s *IPSets) tryResync() (err error) {
 		}).Debug("Finished IPSets resync")
 	}()
 
-	// Start an 'ipset list' child process, which will emit output of the following form:
+	// Figure out if debug logging is enabled so we can disable some expensive-to-calculate logs
+	// in the tight loop below if they're not going to be emitted.  This speeds up the loop
+	// by a factor of 3-4x!
+	debug := log.GetLevel() >= log.DebugLevel
+
+	// Clear the dataplane metadata view, we'll build it back up again as we
+	// scan.
+	s.setNameToProgrammedMetadata.Dataplane().DeleteAll()
+
+	ipsets, err := s.list(debug)
+	if err != nil {
+		s.logCxt.WithError(err).Error("Failed to get the list of ipsets")
+		return
+	}
+
+	for _, name := range ipsets {
+		// Look up to see if this is one of our IP sets.
+		if !s.IPVersionConfig.OwnsIPSet(name) {
+			if debug {
+				s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.")
+			}
+			continue
+		}
+		err = s.parse(name, debug)
+		if err != nil {
+			s.logCxt.WithError(err).Errorf("Failed to parse ipset %v", name)
+			return
+		}
+	}
+
+	// Mark any IP sets that we didn't see as empty.
+	for name, members := range s.mainSetNameToMembers {
+		if _, ok := s.setNameToProgrammedMetadata.Dataplane().Get(name); ok {
+			// In the dataplane, we should have updated its members above.
+			continue
+		}
+		if _, ok := s.setNameToAllMetadata[name]; !ok {
+			// Defensive: this IP set is not in the dataplane, and it's not
+			// one we are tracking, clean up its member tracker.
+			log.WithField("name", name).Warn(
+				"Cleaning up leaked(?) IP set member tracker.")
+			delete(s.mainSetNameToMembers, name)
+			continue
+		}
+		// We're tracking this IP set, but we didn't find it in the dataplane;
+		// reset the members set to empty.
+		members.Dataplane().DeleteAll()
+	}
+
+	return
+}
+
+func (s *IPSets) list(debug bool) ([]string, error) {
+	// Start an 'ipset list -name' child process, which will emit ipset's name, one at each line:
+	//
+	// 	test-100
+	//	test-1
+	//  ...
+	getNames := func(scanner *bufio.Scanner) ([]string, error) {
+		var ipSets []string
+		for scanner.Scan() {
+			ipSets = append(ipSets, scanner.Text())
+		}
+		return ipSets, nil
+	}
+
+	// Run ipset with -name to get the name of all ipsets
+	ipSets, err := s.runIpSetList("-name", debug, getNames)
+	if err != nil {
+		return nil, err
+	}
+	if debug {
+		s.logCxt.Debugf("List of ipsets: %v", ipSets)
+	}
+	return ipSets, nil
+}
+
+func (s *IPSets) get(ipSetName string, debug bool) ([]string, error) {
+	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
+	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
+	if ipSetName == "" {
+		return nil, fmt.Errorf("no ipset name specified")
+	}
+	if debug {
+		s.logCxt.WithField("setName", ipSetName).Debug("Getting IP set.")
+	}
+	// Start an 'ipset list [name]' child process, which will emit output of the following form:
 	//
 	// 	Name: test-100
 	//	Type: hash:ip
@@ -399,8 +485,36 @@ func (s *IPSets) tryResync() (err error) {
 	//	Members:
 	//	10.0.0.2
 	//	10.0.0.1
+	getIpSet := func(scanner *bufio.Scanner) ([]string, error) {
+		var lines []string
+		for scanner.Scan() {
+			lines = append(lines, scanner.Text())
+		}
+		return lines, nil
+	}
+
+	lines, err := s.runIpSetList(ipSetName, debug, getIpSet)
+	if err != nil {
+		return nil, err
+	}
+	if debug {
+		s.logCxt.Debugf("IP set %v:\n%v", ipSetName, lines)
+	}
+	return lines, nil
+}
+
+func (s *IPSets) parse(ipSetName string, debug bool) error {
+	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
+	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
+	if ipSetName == "" {
+		return fmt.Errorf("no ipset name specified")
+	}
+	if debug {
+		s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.")
+	}
+	// Start an 'ipset list [name]' child process, which will emit output of the following form:
 	//
-	//	Name: test-1
+	// 	Name: test-1
 	//	Type: hash:ip
 	//	Revision: 4
 	//	Header: family inet hashsize 1024 maxelem 65536
@@ -412,219 +526,184 @@ func (s *IPSets) tryResync() (err error) {
 	//
 	// As we stream through the data, we extract the name of the IP set and its members. We
 	// use the IP set's metadata to convert each member to its canonical form for comparison.
-	cmd := s.newCmd("ipset", "list")
-	// Grab stdout as a pipe so we can stream through the (potentially very large) output.
-	out, err := cmd.StdoutPipe()
-	if err != nil {
-		s.logCxt.WithError(err).Error("Failed to get pipe for 'ipset list'")
-		return
-	}
-	// Capture error output into a buffer.
-	var stderr bytes.Buffer
-	cmd.SetStderr(&stderr)
-	execStartTime := time.Now()
-	err = cmd.Start()
-	if err != nil {
-		s.logCxt.WithError(err).Error("Failed to start 'ipset list'")
-		return
-	}
-	summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0)
-
-	// Use a scanner to chunk the input into lines.
-	scanner := bufio.NewScanner(out)
-
-	// Values of the last-seen header fields.
-	ipSetName := ""
-	var ipSetType IPSetType
-
-	// Figure out if debug logging is enabled so we can disable some expensive-to-calculate logs
-	// in the tight loop below if they're not going to be emitted.  This speeds up the loop
-	// by a factor of 3-4x!
-	debug := log.GetLevel() >= log.DebugLevel
-
-	// Clear the dataplane metadata view, we'll build it back up again as we
-	// scan.
-	s.setNameToProgrammedMetadata.Dataplane().DeleteAll()
-	for scanner.Scan() {
-		line := scanner.Text()
-		if debug {
-			s.logCxt.Debugf("Parsing line: %q", line)
-		}
-		if strings.HasPrefix(line, "Name:") {
-			ipSetName = strings.Split(line, " ")[1]
-			if debug {
-				s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.")
-			}
-		}
-		if strings.HasPrefix(line, "Type:") {
-			ipSetType = IPSetType(strings.Split(line, " ")[1])
+	parseIpSet := func(scanner *bufio.Scanner) ([]string, error) {
+		ipSetName := ""
+		var ipSetType IPSetType
+		for scanner.Scan() {
+			line := scanner.Text()
 			if debug {
-				s.logCxt.WithField("type", ipSetType).Debug("Parsed type of IP set.")
+				s.logCxt.Debugf("Parsing line: %q", line)
 			}
-		}
-		if strings.HasPrefix(line, "Header:") {
-			// When we hit the Header line we should know the name, and type of the IP set, which lets
-			// us update the tracker.
-			if !s.IPVersionConfig.OwnsIPSet(ipSetName) {
-				s.logCxt.WithField("name", ipSetName).Debug("Skip non-Calico/wrong version IP set.")
-				continue
+			if strings.HasPrefix(line, "Name:") {
+				ipSetName = strings.Split(line, " ")[1]
+				if debug {
+					s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.")
+				}
 			}
-			parts := strings.Split(line, " ")
-			meta := dataplaneMetadata{
-				Type: ipSetType,
+			if strings.HasPrefix(line, "Type:") {
+				ipSetType = IPSetType(strings.Split(line, " ")[1])
+				if debug {
+					s.logCxt.WithField("type", ipSetType).Debug("Parsed type of IP set.")
+				}
 			}
-			for idx, p := range parts {
-				if p == "maxelem" {
-					if idx+1 >= len(parts) {
-						log.WithField("line", line).Error(
-							"Failed to parse ipset list Header line, nothing after 'maxelem'.")
-						break
-					}
-					maxElem, err := strconv.Atoi(parts[idx+1])
-					if err != nil {
-						log.WithError(err).WithField("line", line).Error(
-							"Failed to parse ipset list Header line.")
-						break
-					}
-					meta.MaxSize = maxElem
-					break
+			if strings.HasPrefix(line, "Header:") {
+				// When we hit the Header line we should know the name, and type of the IP set, which lets
+				// us update the tracker.
+				parts := strings.Split(line, " ")
+				meta := dataplaneMetadata{
+					Type: ipSetType,
 				}
-				if p == "range" {
-					if idx+1 >= len(parts) {
-						log.WithField("line", line).Error(
-							"Failed to parse ipset list Header line, nothing after 'range'.")
+				for idx, p := range parts {
+					if p == "maxelem" {
+						if idx+1 >= len(parts) {
+							return nil, fmt.Errorf(
+								"Failed to parse ipset list Header line, nothing after 'maxelem'. line: '%v'", line)
+						}
+						maxElem, err := strconv.Atoi(parts[idx+1])
+						if err != nil {
+							return nil, fmt.Errorf(
+								"Failed to parse ipset list Header line. line: '%v', err: %w", line, err)
+						}
+						meta.MaxSize = maxElem
 						break
 					}
-					// For bitmaps, we see "range 123-456"
-					rMin, rMAx, err := ParseRange(parts[idx+1])
-					if err != nil {
-						log.WithError(err).WithField("line", line).Error(
-							"Failed to parse ipset list Header line.")
+					if p == "range" {
+						if idx+1 >= len(parts) {
+							return nil, fmt.Errorf(
+								"Failed to parse ipset list Header line, nothing after 'range'. line: '%v'", line)
+						}
+						// For bitmaps, we see "range 123-456"
+						rMin, rMAx, err := ParseRange(parts[idx+1])
+						if err != nil {
+							return nil, fmt.Errorf(
+								"Failed to parse ipset list Header line. line: '%v', err: %w", line, err)
+						}
+						meta.RangeMin = rMin
+						meta.RangeMax = rMAx
 						break
 					}
-					meta.RangeMin = rMin
-					meta.RangeMax = rMAx
-					break
 				}
+				s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta)
 			}
-			s.setNameToProgrammedMetadata.Dataplane().Set(ipSetName, meta)
-		}
-		if strings.HasPrefix(line, "Members:") {
-			// Start of a Members entry, following this, there'll be one member per
-			// line then EOF or a blank line.
+			if strings.HasPrefix(line, "Members:") {
+				// Start of a Members entry, following this, there'll be one member per
+				// line then EOF or a blank line.
 
-			// Look up to see if this is one of our IP sets.
-			if !s.IPVersionConfig.OwnsIPSet(ipSetName) || s.IPVersionConfig.IsTempIPSetName(ipSetName) {
-				if debug {
-					s.logCxt.WithField("name", ipSetName).Debug("Skip parsing members of IP set.")
-				}
-				for scanner.Scan() {
-					line := scanner.Bytes()
-					if len(line) == 0 {
-						// End of members
-						break
+				// Look up to see if this is one of our IP sets.
+				if !s.IPVersionConfig.OwnsIPSet(ipSetName) || s.IPVersionConfig.IsTempIPSetName(ipSetName) {
+					if debug {
+						s.logCxt.WithField("name", ipSetName).Debug("Skip parsing members of IP set.")
 					}
+					return nil, nil
 				}
-				ipSetName = ""
-				ipSetType = ""
-				continue
-			}
 
-			if !ipSetType.IsValid() {
-				s.logCxt.WithFields(log.Fields{
-					"setName": ipSetName,
-					"type":    string(ipSetType),
-				}).Warning("Dataplane IP set has unknown type.")
-			}
+				if !ipSetType.IsValid() {
+					s.logCxt.WithFields(log.Fields{
+						"setName": ipSetName,
+						"type":    string(ipSetType),
+					}).Warning("Dataplane IP set has unknown type.")
+				}
 
-			// One of our IP sets; we need to parse its members.
-			logCxt := s.logCxt.WithField("setName", ipSetName)
-			memberTracker := s.getOrCreateMemberTracker(ipSetName)
-			numExtrasExpected := memberTracker.PendingDeletions().Len()
-			err = memberTracker.Dataplane().ReplaceFromIter(func(f func(k IPSetMember)) error {
-				for scanner.Scan() {
-					line := scanner.Text()
-					if line == "" {
-						// End of members
-						break
-					}
-					var canonMember IPSetMember
-					if ipSetType.IsValid() {
-						canonMember = ipSetType.CanonicaliseMember(line)
-					} else {
-						// Unknown type found in dataplane, record it as
-						// a raw string.  Then we'll clean up the IP set
-						// when we go to sync.
-						canonMember = rawIPSetMember(line)
-					}
-					if debug {
-						logCxt.WithFields(log.Fields{
-							"member": line,
-							"canon":  canonMember,
-						}).Debug("Found member in dataplane")
+				// One of our IP sets; we need to parse its members.
+				logCxt := s.logCxt.WithField("setName", ipSetName)
+				memberTracker := s.getOrCreateMemberTracker(ipSetName)
+				numExtrasExpected := memberTracker.PendingDeletions().Len()
+				err := memberTracker.Dataplane().ReplaceFromIter(func(f func(k IPSetMember)) error {
+					for scanner.Scan() {
+						line := scanner.Text()
+						if line == "" {
+							// End of members
+							break
+						}
+						var canonMember IPSetMember
+						if ipSetType.IsValid() {
+							canonMember = ipSetType.CanonicaliseMember(line)
+						} else {
+							// Unknown type found in dataplane, record it as
+							// a raw string.  Then we'll clean up the IP set
+							// when we go to sync.
+							canonMember = rawIPSetMember(line)
+						}
+						if debug {
+							logCxt.WithFields(log.Fields{
+								"member": line,
+								"canon":  canonMember,
+							}).Debug("Found member in dataplane")
+						}
+						f(canonMember)
 					}
-					f(canonMember)
+					return scanner.Err()
+				})
+				if err != nil {
+					return nil, fmt.Errorf("Failed to read members from 'ipset list'. err: %w", err)
 				}
-				return scanner.Err()
-			})
-			if err != nil {
-				logCxt.WithError(err).Error("Failed to read members from 'ipset list'.")
-				break
-			}
 
-			if numMissing := memberTracker.PendingUpdates().Len(); numMissing > 0 {
-				logCxt.WithField("numMissing", numMissing).Info(
-					"Resync found members missing from dataplane.")
-			}
-			if numExtras := memberTracker.PendingDeletions().Len() - numExtrasExpected; numExtras > 0 {
-				logCxt.WithField("numExtras", numExtras).Info(
-					"Resync found extra members in dataplane.")
+				if numMissing := memberTracker.PendingUpdates().Len(); numMissing > 0 {
+					logCxt.WithField("numMissing", numMissing).Info(
+						"Resync found members missing from dataplane.")
+				}
+				if numExtras := memberTracker.PendingDeletions().Len() - numExtrasExpected; numExtras > 0 {
+					logCxt.WithField("numExtras", numExtras).Info(
+						"Resync found extra members in dataplane.")
+				}
+
+				s.updateDirtiness(ipSetName)
 			}
+		}
+		return nil, nil
+	}
 
-			s.updateDirtiness(ipSetName)
+	_, err := s.runIpSetList(ipSetName, debug, parseIpSet)
+	if err != nil {
+		return err
+	}
+	return nil
+}
 
-			ipSetName = ""
-			ipSetType = ""
-		}
+func (s *IPSets) runIpSetList(arg string, debug bool, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
+	cmd := s.newCmd("ipset", "list", arg)
+	cmdStr := fmt.Sprintf("ipset list %v", arg)
+	// Grab stdout as a pipe so we can stream through the (potentially very large) output.
+	out, err := cmd.StdoutPipe()
+	if err != nil {
+		s.logCxt.WithError(err).Errorf("Failed to get pipe for '%v'.", cmdStr)
+		return nil, err
 	}
+	// Capture error output into a buffer.
+	var stderr bytes.Buffer
+	cmd.SetStderr(&stderr)
+	execStartTime := time.Now()
+	err = cmd.Start()
+	if err != nil {
+		s.logCxt.WithError(err).Errorf("Failed to start '%v'.", cmdStr)
+		return nil, err
+	}
+	summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0)
+
+	// Use a scanner to chunk the input into lines.
+	scanner := bufio.NewScanner(out)
+	res, parsingErr := parsingFunc(scanner)
 	closeErr := out.Close()
 	err = cmd.Wait()
 	logCxt := s.logCxt.WithField("stderr", stderr.String())
 	if scanner.Err() != nil {
-		logCxt.WithError(scanner.Err()).Error("Failed to read 'ipset list' output.")
 		err = scanner.Err()
-		return
+		logCxt.WithError(err).Errorf("Failed to read '%v' output.", cmdStr)
+		return nil, err
 	}
 	if err != nil {
-		logCxt.WithError(err).Error("Bad return code from 'ipset list'.")
-		return
+		logCxt.WithError(err).Errorf("Bad return code from '%v'.", cmdStr)
+		return nil, err
 	}
 	if closeErr != nil {
-		err = closeErr
-		logCxt.WithError(err).Error("Failed to close stdout from 'ipset list'.")
-		return
+		logCxt.WithError(closeErr).Errorf("Failed to close stdout from '%v'.", cmdStr)
+		return nil, closeErr
 	}
-
-	// Mark any IP sets that we didn't see as empty.
-	for name, members := range s.mainSetNameToMembers {
-		if _, ok := s.setNameToProgrammedMetadata.Dataplane().Get(name); ok {
-			// In the dataplane, we should have updated its members above.
-			continue
-		}
-		if _, ok := s.setNameToAllMetadata[name]; !ok {
-			// Defensive: this IP set is not in the dataplane, and it's not
-			// one we are tracking, clean up its member tracker.
-			log.WithField("name", name).Warn(
-				"Cleaning up leaked(?) IP set member tracker.")
-			delete(s.mainSetNameToMembers, name)
-			continue
-		}
-		// We're tracking this IP set, but we didn't find it in the dataplane;
-		// reset the members set to empty.
-		members.Dataplane().DeleteAll()
+	if parsingErr != nil {
+		logCxt.WithError(parsingErr).Errorf("Failed to process '%v' output.", cmdStr)
+		return nil, parsingErr
 	}
-
-	return
+	return res, nil
 }
 
 func ParseRange(s string) (min int, max int, err error) {
@@ -966,13 +1045,33 @@ func (s *IPSets) deleteIPSet(setName string) error {
 }
 
 func (s *IPSets) dumpIPSetsToLog() {
-	cmd := s.newCmd("ipset", "list")
-	output, err := cmd.Output()
+	debug := log.GetLevel() >= log.DebugLevel
+	ipSets, err := s.list(debug)
 	if err != nil {
-		s.logCxt.WithError(err).Error("Failed to read IP sets")
+		s.logCxt.WithError(err).Error("Failed to get the list of IP sets.")
 		return
 	}
-	s.logCxt.WithField("output", string(output)).Info("Current state of IP sets")
+	output := []string{
+		fmt.Sprintf("Lists of IP sets: %v", strings.Join(ipSets, " ")),
+		"Dump of Calico IP sets:",
+	}
+
+	for _, name := range ipSets {
+		// Look up to see if this is one of our IP sets.
+		if !s.IPVersionConfig.OwnsIPSet(name) {
+			if debug {
+				s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.")
+			}
+			continue
+		}
+		lines, err := s.get(name, debug)
+		if err != nil {
+			s.logCxt.WithError(err).Errorf("Failed to read ipset %v", name)
+			return
+		}
+		output = append(output, lines...)
+	}
+	s.logCxt.Infof("Current state of IP sets:\n%v", strings.Join(output, "\n"))
 }
 
 func firstNonNilErr(errs ...error) error {
diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go
index 10360b296bc..ecb59e2a89a 100644
--- a/felix/ipsets/ipsets_test.go
+++ b/felix/ipsets/ipsets_test.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2023 Tigera, Inc. All rights reserved.
+// Copyright (c) 2016-2024 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -952,7 +952,7 @@ var _ = Describe("IP sets dataplane", func() {
 		It("shouldn't do any work on resync", func() {
 			dataplane.CmdNames = nil
 			resyncAndApply()
-			Expect(dataplane.CmdNames).To(ConsistOf("list"))
+			Expect(dataplane.CmdNames).To(ConsistOf("list", "list"))
 		})
 	})
 
diff --git a/felix/ipsets/utils_for_test.go b/felix/ipsets/utils_for_test.go
index 6f54229bdd5..e81a084ac09 100644
--- a/felix/ipsets/utils_for_test.go
+++ b/felix/ipsets/utils_for_test.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2018 Tigera, Inc. All rights reserved.
+// Copyright (c) 2016-2024 Tigera, Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -108,10 +108,12 @@ func (d *mockDataplane) newCmd(name string, arg ...string) CmdIface {
 			SetName:   name,
 		}
 	case "list":
-		Expect(len(arg)).To(Equal(1))
+		Expect(len(arg)).To(Equal(2))
 		cmd = &listCmd{
 			Dataplane: d,
 			resultC:   make(chan error),
+			allIpSets: arg[1] == "-name",
+			SetName:   arg[1], // either ipset name or '-name' to return the list of all ipsets
 		}
 	default:
 		Fail(fmt.Sprintf("Unexpected command %v", arg))
@@ -521,6 +523,7 @@ func (d *destroyCmd) CombinedOutput() ([]byte, error) {
 type listCmd struct {
 	Dataplane *mockDataplane
 	SetName   string
+	allIpSets bool
 	Stdout    *io.PipeWriter
 	resultC   chan error
 }
@@ -697,36 +700,41 @@ func (c *listCmd) main() {
 		return
 	}
 
-	first := true
-	for setName, members := range c.Dataplane.IPSetMembers {
-		if !first {
-			fmt.Fprint(c.Stdout, "\n")
-		}
-		fmt.Fprintf(c.Stdout, "Name: %s\n", setName)
-		meta, ok := c.Dataplane.IPSetMetadata[setName]
-		if !ok {
-			// Default metadata for IP sets created by tests.
-			meta = setMetadata{
-				Name:    v4MainIPSetName,
-				Family:  IPFamilyV4,
-				Type:    IPSetTypeHashIP,
-				MaxSize: 1234,
-			}
+	if c.allIpSets {
+		for setName := range c.Dataplane.IPSetMembers {
+			fmt.Fprintf(c.Stdout, "%s\n", setName)
 		}
-		fmt.Fprintf(c.Stdout, "Type: %s\n", meta.Type)
-		if meta.Type == IPSetTypeBitmapPort {
-			fmt.Fprintf(c.Stdout, "Header: family %s range %d-%d\n", meta.Family, meta.RangeMin, meta.RangeMax)
-		} else if meta.Type == "unknown:type" {
-			fmt.Fprintf(c.Stdout, "Header: floop\n")
-		} else {
-			fmt.Fprintf(c.Stdout, "Header: family %s hashsize 1024 maxelem %d\n", meta.Family, meta.MaxSize)
+		return
+	}
+
+	members, exists := c.Dataplane.IPSetMembers[c.SetName]
+	if !exists {
+		result = fmt.Errorf("ipset %v does not exists", c.SetName)
+		return
+	}
+	fmt.Fprintf(c.Stdout, "Name: %s\n", c.SetName)
+	meta, ok := c.Dataplane.IPSetMetadata[c.SetName]
+	if !ok {
+		// Default metadata for IP sets created by tests.
+		meta = setMetadata{
+			Name:    v4MainIPSetName,
+			Family:  IPFamilyV4,
+			Type:    IPSetTypeHashIP,
+			MaxSize: 1234,
 		}
-		fmt.Fprint(c.Stdout, "Field: foobar\n") // Dummy field, should get ignored.
-		fmt.Fprint(c.Stdout, "Members:\n")
-		members.Iter(func(member string) error {
-			fmt.Fprintf(c.Stdout, "%s\n", member)
-			return nil
-		})
-		first = false
 	}
+	fmt.Fprintf(c.Stdout, "Type: %s\n", meta.Type)
+	if meta.Type == IPSetTypeBitmapPort {
+		fmt.Fprintf(c.Stdout, "Header: family %s range %d-%d\n", meta.Family, meta.RangeMin, meta.RangeMax)
+	} else if meta.Type == "unknown:type" {
+		fmt.Fprintf(c.Stdout, "Header: floop\n")
+	} else {
+		fmt.Fprintf(c.Stdout, "Header: family %s hashsize 1024 maxelem %d\n", meta.Family, meta.MaxSize)
+	}
+	fmt.Fprint(c.Stdout, "Field: foobar\n") // Dummy field, should get ignored.
+	fmt.Fprint(c.Stdout, "Members:\n")
+	members.Iter(func(member string) error {
+		fmt.Fprintf(c.Stdout, "%s\n", member)
+		return nil
+	})
 }

From 248e560ba41078aaaa18a97ebe245e2a7df43ae7 Mon Sep 17 00:00:00 2001
From: Mazdak Nasab <mazdak.nasab@gmail.com>
Date: Thu, 11 Jan 2024 11:48:29 -0800
Subject: [PATCH 2/5] Apply suggestions from code review

Co-authored-by: Shaun Crampton <shaun@tigera.io>
---
 felix/ipsets/ipsets.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go
index 4b60f3666ce..4cebab5e811 100644
--- a/felix/ipsets/ipsets.go
+++ b/felix/ipsets/ipsets.go
@@ -440,7 +440,7 @@ func (s *IPSets) tryResync() (err error) {
 	return
 }
 
-func (s *IPSets) list(debug bool) ([]string, error) {
+func (s *IPSets) listAllIPSetNames() ([]string, error) {
 	// Start an 'ipset list -name' child process, which will emit ipset's name, one at each line:
 	//
 	// 	test-100
@@ -485,7 +485,7 @@ func (s *IPSets) get(ipSetName string, debug bool) ([]string, error) {
 	//	Members:
 	//	10.0.0.2
 	//	10.0.0.1
-	getIpSet := func(scanner *bufio.Scanner) ([]string, error) {
+	getIPSet := func(scanner *bufio.Scanner) ([]string, error) {
 		var lines []string
 		for scanner.Scan() {
 			lines = append(lines, scanner.Text())
@@ -557,7 +557,7 @@ func (s *IPSets) parse(ipSetName string, debug bool) error {
 					if p == "maxelem" {
 						if idx+1 >= len(parts) {
 							return nil, fmt.Errorf(
-								"Failed to parse ipset list Header line, nothing after 'maxelem'. line: '%v'", line)
+								"failed to parse ipset list Header line, nothing after 'maxelem'. line: '%v'", line)
 						}
 						maxElem, err := strconv.Atoi(parts[idx+1])
 						if err != nil {
@@ -660,7 +660,7 @@ func (s *IPSets) parse(ipSetName string, debug bool) error {
 	return nil
 }
 
-func (s *IPSets) runIpSetList(arg string, debug bool, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
+func (s *IPSets) runIPSetList(arg string, debug bool, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
 	cmd := s.newCmd("ipset", "list", arg)
 	cmdStr := fmt.Sprintf("ipset list %v", arg)
 	// Grab stdout as a pipe so we can stream through the (potentially very large) output.

From dec85c253a1555eabe3e6a35abba342d6139a034 Mon Sep 17 00:00:00 2001
From: Mazdak Nasab <mazdak.nasab@gmail.com>
Date: Thu, 11 Jan 2024 12:22:05 -0800
Subject: [PATCH 3/5] fix comments#1

---
 felix/ipsets/ipsets.go | 108 ++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 55 deletions(-)

diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go
index 4cebab5e811..00ad6c450dc 100644
--- a/felix/ipsets/ipsets.go
+++ b/felix/ipsets/ipsets.go
@@ -397,13 +397,19 @@ func (s *IPSets) tryResync() (err error) {
 	// scan.
 	s.setNameToProgrammedMetadata.Dataplane().DeleteAll()
 
-	ipsets, err := s.list(debug)
+	ipSets, err := s.ListAllIPSetNames()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of ipsets")
 		return
 	}
+	if debug {
+		s.logCxt.Debugf("List of ipsets: %v", ipSets)
+	}
 
-	for _, name := range ipsets {
+	for _, name := range ipSets {
+		if debug {
+			s.logCxt.Debugf("Parsing IP set %v.", name)
+		}
 		// Look up to see if this is one of our IP sets.
 		if !s.IPVersionConfig.OwnsIPSet(name) {
 			if debug {
@@ -411,7 +417,7 @@ func (s *IPSets) tryResync() (err error) {
 			}
 			continue
 		}
-		err = s.parse(name, debug)
+		err = s.parse(name)
 		if err != nil {
 			s.logCxt.WithError(err).Errorf("Failed to parse ipset %v", name)
 			return
@@ -440,7 +446,7 @@ func (s *IPSets) tryResync() (err error) {
 	return
 }
 
-func (s *IPSets) listAllIPSetNames() ([]string, error) {
+func (s *IPSets) ListAllIPSetNames() ([]string, error) {
 	// Start an 'ipset list -name' child process, which will emit ipset's name, one at each line:
 	//
 	// 	test-100
@@ -455,63 +461,19 @@ func (s *IPSets) listAllIPSetNames() ([]string, error) {
 	}
 
 	// Run ipset with -name to get the name of all ipsets
-	ipSets, err := s.runIpSetList("-name", debug, getNames)
+	ipSets, err := s.runIPSetList("-name", getNames)
 	if err != nil {
 		return nil, err
 	}
-	if debug {
-		s.logCxt.Debugf("List of ipsets: %v", ipSets)
-	}
 	return ipSets, nil
 }
 
-func (s *IPSets) get(ipSetName string, debug bool) ([]string, error) {
-	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
-	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
-	if ipSetName == "" {
-		return nil, fmt.Errorf("no ipset name specified")
-	}
-	if debug {
-		s.logCxt.WithField("setName", ipSetName).Debug("Getting IP set.")
-	}
-	// Start an 'ipset list [name]' child process, which will emit output of the following form:
-	//
-	// 	Name: test-100
-	//	Type: hash:ip
-	//	Revision: 4
-	//	Header: family inet hashsize 1024 maxelem 65536
-	//	Size in memory: 224
-	//	References: 0
-	//	Members:
-	//	10.0.0.2
-	//	10.0.0.1
-	getIPSet := func(scanner *bufio.Scanner) ([]string, error) {
-		var lines []string
-		for scanner.Scan() {
-			lines = append(lines, scanner.Text())
-		}
-		return lines, nil
-	}
-
-	lines, err := s.runIpSetList(ipSetName, debug, getIpSet)
-	if err != nil {
-		return nil, err
-	}
-	if debug {
-		s.logCxt.Debugf("IP set %v:\n%v", ipSetName, lines)
-	}
-	return lines, nil
-}
-
-func (s *IPSets) parse(ipSetName string, debug bool) error {
+func (s *IPSets) parse(ipSetName string) error {
 	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
 	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
 	if ipSetName == "" {
 		return fmt.Errorf("no ipset name specified")
 	}
-	if debug {
-		s.logCxt.WithField("setName", ipSetName).Debug("Parsing IP set.")
-	}
 	// Start an 'ipset list [name]' child process, which will emit output of the following form:
 	//
 	// 	Name: test-1
@@ -527,6 +489,7 @@ func (s *IPSets) parse(ipSetName string, debug bool) error {
 	// As we stream through the data, we extract the name of the IP set and its members. We
 	// use the IP set's metadata to convert each member to its canonical form for comparison.
 	parseIpSet := func(scanner *bufio.Scanner) ([]string, error) {
+		debug := log.GetLevel() >= log.DebugLevel
 		ipSetName := ""
 		var ipSetType IPSetType
 		for scanner.Scan() {
@@ -653,14 +616,14 @@ func (s *IPSets) parse(ipSetName string, debug bool) error {
 		return nil, nil
 	}
 
-	_, err := s.runIpSetList(ipSetName, debug, parseIpSet)
+	_, err := s.runIPSetList(ipSetName, parseIpSet)
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
-func (s *IPSets) runIPSetList(arg string, debug bool, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
+func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
 	cmd := s.newCmd("ipset", "list", arg)
 	cmdStr := fmt.Sprintf("ipset list %v", arg)
 	// Grab stdout as a pipe so we can stream through the (potentially very large) output.
@@ -1045,8 +1008,7 @@ func (s *IPSets) deleteIPSet(setName string) error {
 }
 
 func (s *IPSets) dumpIPSetsToLog() {
-	debug := log.GetLevel() >= log.DebugLevel
-	ipSets, err := s.list(debug)
+	ipSets, err := s.ListAllIPSetNames()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of IP sets.")
 		return
@@ -1056,7 +1018,11 @@ func (s *IPSets) dumpIPSetsToLog() {
 		"Dump of Calico IP sets:",
 	}
 
+	debug := log.GetLevel() >= log.DebugLevel
 	for _, name := range ipSets {
+		if debug {
+			s.logCxt.Debugf("Dumping IP set %v.", name)
+		}
 		// Look up to see if this is one of our IP sets.
 		if !s.IPVersionConfig.OwnsIPSet(name) {
 			if debug {
@@ -1064,7 +1030,7 @@ func (s *IPSets) dumpIPSetsToLog() {
 			}
 			continue
 		}
-		lines, err := s.get(name, debug)
+		lines, err := s.dumpIPSetFoDiags(name)
 		if err != nil {
 			s.logCxt.WithError(err).Errorf("Failed to read ipset %v", name)
 			return
@@ -1074,6 +1040,38 @@ func (s *IPSets) dumpIPSetsToLog() {
 	s.logCxt.Infof("Current state of IP sets:\n%v", strings.Join(output, "\n"))
 }
 
+func (s *IPSets) dumpIPSetFoDiags(ipSetName string) ([]string, error) {
+	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
+	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
+	if ipSetName == "" {
+		return nil, fmt.Errorf("no ipset name specified")
+	}
+	// Start an 'ipset list [name]' child process, which will emit output of the following form:
+	//
+	// 	Name: test-100
+	//	Type: hash:ip
+	//	Revision: 4
+	//	Header: family inet hashsize 1024 maxelem 65536
+	//	Size in memory: 224
+	//	References: 0
+	//	Members:
+	//	10.0.0.2
+	//	10.0.0.1
+	getIPSet := func(scanner *bufio.Scanner) ([]string, error) {
+		var lines []string
+		for scanner.Scan() {
+			lines = append(lines, scanner.Text())
+		}
+		return lines, nil
+	}
+
+	lines, err := s.runIPSetList(ipSetName, getIPSet)
+	if err != nil {
+		return nil, err
+	}
+	return lines, nil
+}
+
 func firstNonNilErr(errs ...error) error {
 	for _, err := range errs {
 		if err != nil {

From ae8224a6c052411f108c41c5c2ca57c46ec07ef7 Mon Sep 17 00:00:00 2001
From: Mazdak Nasab <mazdak.nasab@gmail.com>
Date: Thu, 11 Jan 2024 15:17:36 -0800
Subject: [PATCH 4/5] fix comments#2

---
 felix/ipsets/ipsets.go      | 148 ++++++++++++++----------------------
 felix/ipsets/ipsets_test.go |  10 +++
 2 files changed, 68 insertions(+), 90 deletions(-)

diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go
index 00ad6c450dc..444acc20d11 100644
--- a/felix/ipsets/ipsets.go
+++ b/felix/ipsets/ipsets.go
@@ -397,7 +397,7 @@ func (s *IPSets) tryResync() (err error) {
 	// scan.
 	s.setNameToProgrammedMetadata.Dataplane().DeleteAll()
 
-	ipSets, err := s.ListAllIPSetNames()
+	ipSets, err := s.ListCalicoIPSets()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of ipsets")
 		return
@@ -410,14 +410,7 @@ func (s *IPSets) tryResync() (err error) {
 		if debug {
 			s.logCxt.Debugf("Parsing IP set %v.", name)
 		}
-		// Look up to see if this is one of our IP sets.
-		if !s.IPVersionConfig.OwnsIPSet(name) {
-			if debug {
-				s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.")
-			}
-			continue
-		}
-		err = s.parse(name)
+		err = s.resyncIPSet(name)
 		if err != nil {
 			s.logCxt.WithError(err).Errorf("Failed to parse ipset %v", name)
 			return
@@ -446,29 +439,36 @@ func (s *IPSets) tryResync() (err error) {
 	return
 }
 
-func (s *IPSets) ListAllIPSetNames() ([]string, error) {
+func (s *IPSets) ListCalicoIPSets() ([]string, error) {
 	// Start an 'ipset list -name' child process, which will emit ipset's name, one at each line:
 	//
 	// 	test-100
 	//	test-1
 	//  ...
-	getNames := func(scanner *bufio.Scanner) ([]string, error) {
-		var ipSets []string
+	var ipSets []string
+	// Run ipset with -name to get the name of all ipsets
+	err := s.runIPSetList("-name", func(scanner *bufio.Scanner) error {
+		debug := log.GetLevel() >= log.DebugLevel
 		for scanner.Scan() {
-			ipSets = append(ipSets, scanner.Text())
+			name := scanner.Text()
+			// Look up to see if this is one of our IP sets.
+			if !s.IPVersionConfig.OwnsIPSet(name) {
+				if debug {
+					s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.")
+				}
+				continue
+			}
+			ipSets = append(ipSets, name)
 		}
-		return ipSets, nil
-	}
-
-	// Run ipset with -name to get the name of all ipsets
-	ipSets, err := s.runIPSetList("-name", getNames)
+		return scanner.Err()
+	})
 	if err != nil {
 		return nil, err
 	}
 	return ipSets, nil
 }
 
-func (s *IPSets) parse(ipSetName string) error {
+func (s *IPSets) resyncIPSet(ipSetName string) error {
 	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
 	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
 	if ipSetName == "" {
@@ -488,7 +488,7 @@ func (s *IPSets) parse(ipSetName string) error {
 	//
 	// As we stream through the data, we extract the name of the IP set and its members. We
 	// use the IP set's metadata to convert each member to its canonical form for comparison.
-	parseIpSet := func(scanner *bufio.Scanner) ([]string, error) {
+	err := s.runIPSetList(ipSetName, func(scanner *bufio.Scanner) error {
 		debug := log.GetLevel() >= log.DebugLevel
 		ipSetName := ""
 		var ipSetType IPSetType
@@ -519,12 +519,12 @@ func (s *IPSets) parse(ipSetName string) error {
 				for idx, p := range parts {
 					if p == "maxelem" {
 						if idx+1 >= len(parts) {
-							return nil, fmt.Errorf(
+							return fmt.Errorf(
 								"failed to parse ipset list Header line, nothing after 'maxelem'. line: '%v'", line)
 						}
 						maxElem, err := strconv.Atoi(parts[idx+1])
 						if err != nil {
-							return nil, fmt.Errorf(
+							return fmt.Errorf(
 								"Failed to parse ipset list Header line. line: '%v', err: %w", line, err)
 						}
 						meta.MaxSize = maxElem
@@ -532,13 +532,13 @@ func (s *IPSets) parse(ipSetName string) error {
 					}
 					if p == "range" {
 						if idx+1 >= len(parts) {
-							return nil, fmt.Errorf(
+							return fmt.Errorf(
 								"Failed to parse ipset list Header line, nothing after 'range'. line: '%v'", line)
 						}
 						// For bitmaps, we see "range 123-456"
 						rMin, rMAx, err := ParseRange(parts[idx+1])
 						if err != nil {
-							return nil, fmt.Errorf(
+							return fmt.Errorf(
 								"Failed to parse ipset list Header line. line: '%v', err: %w", line, err)
 						}
 						meta.RangeMin = rMin
@@ -557,7 +557,7 @@ func (s *IPSets) parse(ipSetName string) error {
 					if debug {
 						s.logCxt.WithField("name", ipSetName).Debug("Skip parsing members of IP set.")
 					}
-					return nil, nil
+					return nil
 				}
 
 				if !ipSetType.IsValid() {
@@ -598,7 +598,7 @@ func (s *IPSets) parse(ipSetName string) error {
 					return scanner.Err()
 				})
 				if err != nil {
-					return nil, fmt.Errorf("Failed to read members from 'ipset list'. err: %w", err)
+					return fmt.Errorf("Failed to read members from 'ipset list'. err: %w", err)
 				}
 
 				if numMissing := memberTracker.PendingUpdates().Len(); numMissing > 0 {
@@ -613,24 +613,22 @@ func (s *IPSets) parse(ipSetName string) error {
 				s.updateDirtiness(ipSetName)
 			}
 		}
-		return nil, nil
-	}
-
-	_, err := s.runIPSetList(ipSetName, parseIpSet)
+		return scanner.Err()
+	})
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
-func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) ([]string, error)) ([]string, error) {
+func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) error) error {
 	cmd := s.newCmd("ipset", "list", arg)
 	cmdStr := fmt.Sprintf("ipset list %v", arg)
 	// Grab stdout as a pipe so we can stream through the (potentially very large) output.
 	out, err := cmd.StdoutPipe()
 	if err != nil {
 		s.logCxt.WithError(err).Errorf("Failed to get pipe for '%v'.", cmdStr)
-		return nil, err
+		return err
 	}
 	// Capture error output into a buffer.
 	var stderr bytes.Buffer
@@ -639,34 +637,34 @@ func (s *IPSets) runIPSetList(arg string, parsingFunc func(*bufio.Scanner) ([]st
 	err = cmd.Start()
 	if err != nil {
 		s.logCxt.WithError(err).Errorf("Failed to start '%v'.", cmdStr)
-		return nil, err
+		return err
 	}
 	summaryExecStart.Observe(float64(time.Since(execStartTime).Nanoseconds()) / 1000.0)
 
 	// Use a scanner to chunk the input into lines.
 	scanner := bufio.NewScanner(out)
-	res, parsingErr := parsingFunc(scanner)
+	parsingErr := parsingFunc(scanner)
 	closeErr := out.Close()
 	err = cmd.Wait()
 	logCxt := s.logCxt.WithField("stderr", stderr.String())
 	if scanner.Err() != nil {
 		err = scanner.Err()
 		logCxt.WithError(err).Errorf("Failed to read '%v' output.", cmdStr)
-		return nil, err
+		return err
 	}
 	if err != nil {
 		logCxt.WithError(err).Errorf("Bad return code from '%v'.", cmdStr)
-		return nil, err
+		return err
 	}
 	if closeErr != nil {
 		logCxt.WithError(closeErr).Errorf("Failed to close stdout from '%v'.", cmdStr)
-		return nil, closeErr
+		return closeErr
 	}
 	if parsingErr != nil {
 		logCxt.WithError(parsingErr).Errorf("Failed to process '%v' output.", cmdStr)
-		return nil, parsingErr
+		return parsingErr
 	}
-	return res, nil
+	return nil
 }
 
 func ParseRange(s string) (min int, max int, err error) {
@@ -1008,68 +1006,38 @@ func (s *IPSets) deleteIPSet(setName string) error {
 }
 
 func (s *IPSets) dumpIPSetsToLog() {
-	ipSets, err := s.ListAllIPSetNames()
+	ipSets, err := s.ListCalicoIPSets()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of IP sets.")
 		return
 	}
-	output := []string{
-		fmt.Sprintf("Lists of IP sets: %v", strings.Join(ipSets, " ")),
-		"Dump of Calico IP sets:",
-	}
+	s.logCxt.Infof("Current state of IP sets: %v", strings.Join(ipSets, " "))
 
-	debug := log.GetLevel() >= log.DebugLevel
 	for _, name := range ipSets {
-		if debug {
-			s.logCxt.Debugf("Dumping IP set %v.", name)
-		}
-		// Look up to see if this is one of our IP sets.
-		if !s.IPVersionConfig.OwnsIPSet(name) {
-			if debug {
-				s.logCxt.WithField("name", name).Debug("Skip non-Calico/wrong version IP set.")
+		s.logCxt.Infof("Dumping IP set %v.", name)
+
+		// Start an 'ipset list [name]' child process, which will emit output of the following form:
+		//
+		// 	Name: test-100
+		//	Type: hash:ip
+		//	Revision: 4
+		//	Header: family inet hashsize 1024 maxelem 65536
+		//	Size in memory: 224
+		//	References: 0
+		//	Members:
+		//	10.0.0.2
+		//	10.0.0.1
+		err := s.runIPSetList(name, func(scanner *bufio.Scanner) error {
+			for scanner.Scan() {
+				s.logCxt.Infof("%v", scanner.Text())
 			}
-			continue
-		}
-		lines, err := s.dumpIPSetFoDiags(name)
+			return scanner.Err()
+		})
 		if err != nil {
 			s.logCxt.WithError(err).Errorf("Failed to read ipset %v", name)
-			return
-		}
-		output = append(output, lines...)
-	}
-	s.logCxt.Infof("Current state of IP sets:\n%v", strings.Join(output, "\n"))
-}
-
-func (s *IPSets) dumpIPSetFoDiags(ipSetName string) ([]string, error) {
-	// If ipSetName == "", it will run 'ipset list' which will return the list and details of all ipsets.
-	// We should prevent this to not hit ipset protocol mismatch from non-calico ipsets.
-	if ipSetName == "" {
-		return nil, fmt.Errorf("no ipset name specified")
-	}
-	// Start an 'ipset list [name]' child process, which will emit output of the following form:
-	//
-	// 	Name: test-100
-	//	Type: hash:ip
-	//	Revision: 4
-	//	Header: family inet hashsize 1024 maxelem 65536
-	//	Size in memory: 224
-	//	References: 0
-	//	Members:
-	//	10.0.0.2
-	//	10.0.0.1
-	getIPSet := func(scanner *bufio.Scanner) ([]string, error) {
-		var lines []string
-		for scanner.Scan() {
-			lines = append(lines, scanner.Text())
+			continue
 		}
-		return lines, nil
-	}
-
-	lines, err := s.runIPSetList(ipSetName, getIPSet)
-	if err != nil {
-		return nil, err
 	}
-	return lines, nil
 }
 
 func firstNonNilErr(errs ...error) error {
diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go
index ecb59e2a89a..a1004a66015 100644
--- a/felix/ipsets/ipsets_test.go
+++ b/felix/ipsets/ipsets_test.go
@@ -1018,6 +1018,16 @@ var _ = Describe("IP sets dataplane", func() {
 		resyncAndApply()
 		dataplane.ExpectMembers(map[string][]string{"noncali": v4Members1And2})
 	})
+	It("ListCalicoIPSets() should ignore non-calico IP sets", func() {
+		dataplane.IPSetMembers["noncali"] =
+			set.From("10.0.0.1", "10.0.0.2")
+		dataplane.IPSetMembers[v4MainIPSetName] =
+			set.From("10.0.0.1", "10.0.0.3", "10.0.0.4")
+
+		ipsets, err := ipsets.ListCalicoIPSets()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ipsets).Should(Equal([]string{v4MainIPSetName}))
+	})
 })
 
 var _ = Describe("Standard IPv4 IPVersionConfig", func() {

From 8fbdbc28cd57417e528c19b8b9bbff3705972f88 Mon Sep 17 00:00:00 2001
From: Mazdak Nasab <mazdak.nasab@gmail.com>
Date: Mon, 15 Jan 2024 09:21:30 -0800
Subject: [PATCH 5/5] Final changes

---
 felix/ipsets/ipsets.go         | 6 +++---
 felix/ipsets/ipsets_test.go    | 4 ++--
 felix/ipsets/utils_for_test.go | 7 +++++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go
index 444acc20d11..d634eb88c9c 100644
--- a/felix/ipsets/ipsets.go
+++ b/felix/ipsets/ipsets.go
@@ -397,7 +397,7 @@ func (s *IPSets) tryResync() (err error) {
 	// scan.
 	s.setNameToProgrammedMetadata.Dataplane().DeleteAll()
 
-	ipSets, err := s.ListCalicoIPSets()
+	ipSets, err := s.CalicoIPSets()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of ipsets")
 		return
@@ -439,7 +439,7 @@ func (s *IPSets) tryResync() (err error) {
 	return
 }
 
-func (s *IPSets) ListCalicoIPSets() ([]string, error) {
+func (s *IPSets) CalicoIPSets() ([]string, error) {
 	// Start an 'ipset list -name' child process, which will emit ipset's name, one at each line:
 	//
 	// 	test-100
@@ -1006,7 +1006,7 @@ func (s *IPSets) deleteIPSet(setName string) error {
 }
 
 func (s *IPSets) dumpIPSetsToLog() {
-	ipSets, err := s.ListCalicoIPSets()
+	ipSets, err := s.CalicoIPSets()
 	if err != nil {
 		s.logCxt.WithError(err).Error("Failed to get the list of IP sets.")
 		return
diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go
index a1004a66015..bda7be4aef5 100644
--- a/felix/ipsets/ipsets_test.go
+++ b/felix/ipsets/ipsets_test.go
@@ -1018,13 +1018,13 @@ var _ = Describe("IP sets dataplane", func() {
 		resyncAndApply()
 		dataplane.ExpectMembers(map[string][]string{"noncali": v4Members1And2})
 	})
-	It("ListCalicoIPSets() should ignore non-calico IP sets", func() {
+	It("CalicoIPSets() should ignore non-calico IP sets", func() {
 		dataplane.IPSetMembers["noncali"] =
 			set.From("10.0.0.1", "10.0.0.2")
 		dataplane.IPSetMembers[v4MainIPSetName] =
 			set.From("10.0.0.1", "10.0.0.3", "10.0.0.4")
 
-		ipsets, err := ipsets.ListCalicoIPSets()
+		ipsets, err := ipsets.CalicoIPSets()
 		Expect(err).NotTo(HaveOccurred())
 		Expect(ipsets).Should(Equal([]string{v4MainIPSetName}))
 	})
diff --git a/felix/ipsets/utils_for_test.go b/felix/ipsets/utils_for_test.go
index e81a084ac09..f7babe06af5 100644
--- a/felix/ipsets/utils_for_test.go
+++ b/felix/ipsets/utils_for_test.go
@@ -109,12 +109,15 @@ func (d *mockDataplane) newCmd(name string, arg ...string) CmdIface {
 		}
 	case "list":
 		Expect(len(arg)).To(Equal(2))
-		cmd = &listCmd{
+		command := &listCmd{
 			Dataplane: d,
 			resultC:   make(chan error),
 			allIpSets: arg[1] == "-name",
-			SetName:   arg[1], // either ipset name or '-name' to return the list of all ipsets
 		}
+		if !command.allIpSets {
+			command.SetName = arg[1]
+		}
+		cmd = command
 	default:
 		Fail(fmt.Sprintf("Unexpected command %v", arg))
 	}