From 5f7517687c08925c8edfa8841ed3bf840dd62b88 Mon Sep 17 00:00:00 2001
From: Yaroslav Kolomiiets <yrk@meta.com>
Date: Tue, 15 Oct 2024 07:34:38 -0700
Subject: [PATCH] simplify `ptpheck map` (#406)

Summary:

Simplify implementation of the `ptpcheck map` command.

Deprecate `phc.IfacesInfo` which is a mere combination of `net.Interfaces` and `phc.IfaceInfo`, was only used in `ptpcheck map` which is simpler AND more optimal without it.

Reviewed By: t3lurid3

Differential Revision: D64395255
---
 cmd/ptpcheck/cmd/map.go | 57 ++++++++++++++---------------------------
 phc/device.go           | 30 +---------------------
 2 files changed, 20 insertions(+), 67 deletions(-)

diff --git a/cmd/ptpcheck/cmd/map.go b/cmd/ptpcheck/cmd/map.go
index 273ef12c..7ba2d5b3 100644
--- a/cmd/ptpcheck/cmd/map.go
+++ b/cmd/ptpcheck/cmd/map.go
@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"fmt"
+	"net"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -45,65 +46,46 @@ func ptpDeviceNum(ptpPath string) (int, error) {
 	}))
 }
 
-func printIfaceData(d phc.IfaceData, reverse bool) {
-	if d.TSInfo.PHCIndex < 0 {
-		fmt.Printf("No PHC support for %s\n", d.Iface.Name)
+func printIfaceData(ifname string, tsinfo *phc.EthtoolTSinfo, reverse bool) {
+	if tsinfo.PHCIndex < 0 {
+		fmt.Printf("No PHC support for %s\n", ifname)
 		return
 	}
 	if reverse {
-		fmt.Printf("/dev/ptp%d -> %s\n", d.TSInfo.PHCIndex, d.Iface.Name)
+		fmt.Printf("/dev/ptp%d -> %s\n", tsinfo.PHCIndex, ifname)
 		return
 	}
-	fmt.Printf("%s -> /dev/ptp%d\n", d.Iface.Name, d.TSInfo.PHCIndex)
+	fmt.Printf("%s -> /dev/ptp%d\n", ifname, tsinfo.PHCIndex)
 }
 
 func getDevice(iface string) error {
-	ifaces, err := phc.IfacesInfo()
+	tsinfo, err := phc.IfaceInfo(iface)
 	if err != nil {
 		return err
 	}
-	if len(ifaces) == 0 {
-		return fmt.Errorf("no network devices found")
-	}
-	found := []phc.IfaceData{}
-	for _, d := range ifaces {
-		if d.Iface.Name == iface {
-			found = append(found, d)
-		}
-	}
-	if len(found) == 0 {
-		return fmt.Errorf("no nic information for %s", iface)
-	}
-	for _, d := range found {
-		printIfaceData(d, false)
-	}
+	printIfaceData(iface, tsinfo, false)
 	return nil
 }
 
 func getIface(ptpDevice int) error {
-	ifaces, err := phc.IfacesInfo()
+	ifaces, err := net.Interfaces()
 	if err != nil {
 		return err
 	}
-	if len(ifaces) == 0 {
-		return fmt.Errorf("no network devices found")
-	}
-	found := []phc.IfaceData{}
-	if ptpDevice < 0 {
-		found = ifaces
-	} else {
-		for _, d := range ifaces {
-			if int(d.TSInfo.PHCIndex) == ptpDevice {
-				found = append(found, d)
-			}
+	n := 0
+	for _, iface := range ifaces {
+		tsinfo, err := phc.IfaceInfo(iface.Name)
+		if err != nil {
+			continue
+		}
+		if int(tsinfo.PHCIndex) == ptpDevice || ptpDevice < 0 {
+			printIfaceData(iface.Name, tsinfo, true)
+			n++
 		}
 	}
-	if len(found) == 0 {
+	if n == 0 {
 		return fmt.Errorf("no nic found for /dev/ptp%d", ptpDevice)
 	}
-	for _, d := range found {
-		printIfaceData(d, true)
-	}
 	return nil
 }
 
@@ -131,7 +113,6 @@ var mapCmd = &cobra.Command{
 				if err := getDevice(arg); err != nil {
 					log.Fatal(err)
 				}
-
 			}
 			return
 		}
diff --git a/phc/device.go b/phc/device.go
index 9f2dfc22..a8cade1e 100644
--- a/phc/device.go
+++ b/phc/device.go
@@ -18,7 +18,6 @@ package phc
 
 import (
 	"fmt"
-	"net"
 	"unsafe"
 
 	"github.com/vtolstov/go-ioctl"
@@ -215,34 +214,7 @@ func IfaceInfo(iface string) (*EthtoolTSinfo, error) {
 		uintptr(unsafe.Pointer(ifreq)),
 	)
 	if errno != 0 {
-		return nil, fmt.Errorf("failed get phc ID: %w", errno)
+		return nil, fmt.Errorf("failed get phc ID for %s: %w", iface, errno)
 	}
 	return data, nil
 }
-
-// IfaceData has both net.Interface and EthtoolTSinfo
-type IfaceData struct {
-	Iface  net.Interface
-	TSInfo EthtoolTSinfo
-}
-
-// IfacesInfo is like net.Interfaces() but with added EthtoolTSinfo
-func IfacesInfo() ([]IfaceData, error) {
-	ifaces, err := net.Interfaces()
-	if err != nil {
-		return nil, err
-	}
-	res := []IfaceData{}
-	for _, iface := range ifaces {
-		data, err := IfaceInfo(iface.Name)
-		if err != nil {
-			return nil, err
-		}
-		res = append(res,
-			IfaceData{
-				Iface:  iface,
-				TSInfo: *data,
-			})
-	}
-	return res, nil
-}