sync remaining changes from my-huatuo-review

- ICMPInfo.Detail: output seq and checksum fields alongside id
- pcapfilter: rename InjectFilter→Apply in doc comment and test
- symbol.FormatStackLines: inline loop, remove separate FormatStack helper
- server_test: delegate connect to Client.handshake instead of sendConnect;
  keep deferred rawClient.Close in TestUnexpectedClose so handleConn exits
- types/dropwatch: add DropSourceTypesEvent and DropSourceTypesTool constants
- SKB_DROP_REASON: support kernel skb drop reason

Change-Id: If1ffcb02728f5c6c1355c8eff3bfe5603ecd192f

Author: huhong-web

bpf/dropwatch.c

@@ -20,9 +20,11 @@
 #define SK_FL_TYPE_SHIFT 16
 #define SK_FL_TYPE_MASK 0xffff0000
 
-/* Drop reason (kernel skb_drop_reason enum; UNKNOWN until plumbed). */
-#define DROP_REASON_UNKNOWN   0
-#define DROP_REASON_UNSUPPORT 1
+/* Reserved for future kernel skb_drop_reason (SKB_DROP_REASON_NOT_SPECIFIED,
+ * ...) passthrough; (u32)-1 is out of band: kernel reason values grow upward
+ * from 0 (low 16 bits code, high 16 bits subsystem since v6.4) and can never
+ * reach it. */
+#define SKB_DROP_REASON_UNSUPPORT ((u32)-1)
 #define PKT_RAW_LEN 120
 
 struct packet_meta {
@@ -192,7 +194,7 @@ int bpf_kfree_skb_prog(struct trace_event_raw_kfree_skb *ctx)
 	bpf_get_current_comm(&data->meta.comm, sizeof(data->meta.comm));
 	data->meta.kfree_skb_addr = (u64)(unsigned long)ctx->location;
 	data->meta.queue_mapping = BPF_CORE_READ(skb, queue_mapping);
-	data->meta.drop_reason = DROP_REASON_UNKNOWN;
+	data->meta.drop_reason = SKB_DROP_REASON_UNSUPPORT;
 	data->meta.type = 0;
 
 	data->pkt_hdr.pkt_len = BPF_CORE_READ(skb, len);

cmd/dropwatch/main.go

@@ -103,7 +103,7 @@ func loadBPFWithFilter(bpfPath, filterExpr string, consts map[string]any) (bpf.B
 // deviceFilterConsts resolves devName to an ifindex and returns the consts map
 // expected by bpf_skb_filter.h. Returns nil consts when devName is empty
 // (filter disabled at the BPF level by leaving filter_ifindex_included == 0).
-func deviceFilterConsts(devName string, exclude bool) (map[string]any, error) {
+func deviceFilterConsts(devName string, excluded bool) (map[string]any, error) {
 	if devName == "" {
 		return nil, nil
 	}
@@ -112,7 +112,7 @@ func deviceFilterConsts(devName string, exclude bool) (map[string]any, error) {
 		return nil, fmt.Errorf("--device %q: %w", devName, err)
 	}
 	var excl uint32
-	if exclude {
+	if excluded {
 		excl = 1
 	}
 	return map[string]any{
@@ -182,7 +182,7 @@ func mainAction(c *cli.Context) error {
 		defer sockClient.End()
 	}
 
-	consts, err := deviceFilterConsts(c.String("device"), c.Bool("device-exclude"))
+	consts, err := deviceFilterConsts(c.String("device"), c.Bool("device-excluded"))
 	if err != nil {
 		return fmt.Errorf("dropwatch: %w", err)
 	}
@@ -268,7 +268,7 @@ func main() {
 				Usage: "interface to filter on (e.g. eth0); empty = all devices",
 			},
 			&cli.BoolFlag{
-				Name:  "device-exclude",
+				Name:  "device-excluded",
 				Usage: "treat --device as a blacklist (drop matching) instead of whitelist (pass matching)",
 			},
 			&cli.IntFlag{

internal/packet/types.go

@@ -144,8 +144,8 @@ func (t *UDPInfo) Detail() string {
 }
 
 func (t *ICMPInfo) Detail() string {
-	return fmt.Sprintf("[ICMP %s] %s > %s id=%d smac=%s dmac=%s",
-		t.ICMPType, t.Saddr, t.Daddr, t.ID, t.SrcMAC, t.DstMAC)
+	return fmt.Sprintf("[ICMP %s] %s > %s id=%d seq=%d chk=0x%04x smac=%s dmac=%s",
+		t.ICMPType, t.Saddr, t.Daddr, t.ID, t.Seq, t.Checksum, t.SrcMAC, t.DstMAC)
 }
 
 func (t *ARPInfo) Detail() string {

internal/pcapfilter/bpf_filter.go

@@ -18,7 +18,7 @@
 // Usage:
 //
 //	spec, _ := ebpf.LoadCollectionSpec("prog.o")
-//	if err := pcapfilter.InjectFilter(spec, "host 10.0.0.1"); err != nil { ... }
+//	if err := pcapfilter.Apply(spec, "host 10.0.0.1"); err != nil { ... }
 //	coll, _ := ebpf.NewCollection(spec)
 //
 // Limitations inherited from go-pcap's pure-Go compiler:

internal/pcapfilter/load_default_test.go

@@ -62,7 +62,7 @@ func dumpPrograms(t *testing.T, spec *ebpf.CollectionSpec, prefix string) {
 	}
 }
 
-func TestInjectFilter(t *testing.T) {
+func TestApply(t *testing.T) {
 	if os.Getuid() != 0 {
 		t.Skip("Skipping: requires root")
 	}
@@ -79,7 +79,7 @@ func TestInjectFilter(t *testing.T) {
 
 	filterExpr := "ip and src net 192.168.1.0/24 and tcp dst port 3306"
 	if err := Apply(specs, filterExpr); err != nil {
-		t.Fatalf("InjectFilter: %v", err)
+		t.Fatalf("Apply: %v", err)
 	}
 
 	dumpPrograms(t, specs, "Program")

internal/symbol/symbols.go

@@ -370,30 +370,18 @@ func lowerDirFromMountInfo(pid uint32) (string, error) {
 	return "", fmt.Errorf("lowerdir not found for pid %d", pid)
 }
 
-// FormatStack formats a stack trace (newline-separated addresses)
-// into a readable form with frame indices.
-// Returns nil if stack is empty.
-func FormatStack(stack string) []string {
-	if stack == "" {
-		return nil
-	}
-
-	var lines []string
+// FormatStackLines formats a stack trace (newline-separated addresses)
+// and writes it to w with frame indices.
+func FormatStackLines(w io.Writer, stack string) error {
+	i := 0
 	for _, frame := range strings.Split(strings.TrimRight(stack, "\n"), "\n") {
-		if frame != "" {
-			lines = append(lines, frame)
+		if frame == "" {
+			continue
 		}
-	}
-	return lines
-}
-
-// FormatStackLines formats stack lines for output with indices.
-func FormatStackLines(w io.Writer, stack string) error {
-	lines := FormatStack(stack)
-	for i, frame := range lines {
 		if _, err := fmt.Fprintf(w, "\t#%-2d  %s\n", i, frame); err != nil {
 			return err
 		}
+		i++
 	}
 	return nil
 }

internal/toolstream/transport/server_test.go

@@ -91,35 +91,10 @@ func newPipedServer(t *testing.T) (rawClient net.Conn, clientEnc *capnp.Encoder,
 	}
 }
 
-// sendConnect sends a Connect frame via enc.
-func sendConnect(t *testing.T, enc *capnp.Encoder, toolName, version, taskID string) error {
+// handshake sends a Connect frame via enc using Client.handshake.
+func handshake(t *testing.T, enc *capnp.Encoder, rawClient net.Conn, toolName, version, taskID string) error {
 	t.Helper()
-	m, seg, err := capnp.NewMessage(capnp.SingleSegment(nil))
-	if err != nil {
-		return fmt.Errorf("send connect new message: %w", err)
-	}
-	root, err := NewRootMessage(seg)
-	if err != nil {
-		return fmt.Errorf("send connect new root: %w", err)
-	}
-	c, err := root.NewConnect()
-	if err != nil {
-		return fmt.Errorf("send connect new connect: %w", err)
-	}
-	if err := c.SetToolName(toolName); err != nil {
-		return fmt.Errorf("send connect set tool name: %w", err)
-	}
-	if err := c.SetVersion(version); err != nil {
-		return fmt.Errorf("send connect set version: %w", err)
-	}
-	if err := c.SetTaskID(taskID); err != nil {
-		return fmt.Errorf("send connect set task id: %w", err)
-	}
-	c.SetProtoVersion(1)
-	if err := enc.Encode(m); err != nil {
-		return fmt.Errorf("send connect encode: %w", err)
-	}
-	return nil
+	return (&Client{encoder: enc, conn: rawClient}).handshake(toolName, version, taskID)
 }
 
 // sendChunk sends a Chunk frame via enc.
@@ -159,8 +134,7 @@ func TestNormalPath(t *testing.T) {
 	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
 	go func() {
-		defer rawClient.Close()
-		if err := sendConnect(t, clientEnc, "dropwatch", "1.0", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "dropwatch", "1.0", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}
@@ -202,8 +176,7 @@ func TestErrorEnd(t *testing.T) {
 	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
 	go func() {
-		defer rawClient.Close()
-		if err := sendConnect(t, clientEnc, "tool", "v", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "tool", "v", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}
@@ -229,8 +202,7 @@ func TestDataAndEndCombined(t *testing.T) {
 	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
 	go func() {
-		defer rawClient.Close()
-		if err := sendConnect(t, clientEnc, "tool", "v", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "tool", "v", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}
@@ -257,7 +229,7 @@ func TestUnexpectedClose(t *testing.T) {
 
 	go func() {
 		defer rawClient.Close()
-		if err := sendConnect(t, clientEnc, "tool", "v", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "tool", "v", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}
@@ -294,7 +266,7 @@ func TestEmptyToolName(t *testing.T) {
 	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
 	go func() {
-		if err := sendConnect(t, clientEnc, "", "v", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "", "v", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}
@@ -313,9 +285,9 @@ func TestEmptyToolName(t *testing.T) {
 // omits the client-side close so that "accept-then-loop" behavior would hang
 // here until wait() trips its 2s timeout.
 func TestEmptyToolNameClosesEarly(t *testing.T) {
-	_, clientEnc, rec, wait := newPipedServer(t)
+	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
-	if err := sendConnect(t, clientEnc, "", "v", ""); err != nil {
+	if err := handshake(t, clientEnc, rawClient, "", "v", ""); err != nil {
 		t.Logf("send connect: %v", err)
 		return
 	}
@@ -333,7 +305,7 @@ func TestDecodeFailureAfterConnect(t *testing.T) {
 	rawClient, clientEnc, rec, wait := newPipedServer(t)
 
 	go func() {
-		if err := sendConnect(t, clientEnc, "tool", "v", ""); err != nil {
+		if err := handshake(t, clientEnc, rawClient, "tool", "v", ""); err != nil {
 			t.Logf("send connect: %v", err)
 			return
 		}

pkg/types/dropwatch.go

@@ -40,3 +40,9 @@ type DropWatchTracing struct {
 	Source             string            `json:"source,omitempty"`
 	Stack              string            `json:"stack"`
 }
+
+// Values for DropWatchTracing.Source.
+const (
+	DropSourceTypesEvent = "events"
+	DropSourceTypesTool  = "tools"
+)