Merge branch 'nexthop-nexthop-dump-fixes'

kuba-moo · kuba-moo · commit f8d3e0dc4b3a · 2023-08-09T13:45:19.000-07:00
Ido Schimmel says: ==================== nexthop: Nexthop dump fixes Patches #1 and STMicroelectronics#3 fix two problems related to nexthops and nexthop buckets dump, respectively. Patch STMicroelectronics#2 is a preparation for the third patch. The pattern described in these patches of splitting the NLMSG_DONE to a separate response is prevalent in other rtnetlink dump callbacks. I don't know if it's because I'm missing something or if this was done intentionally to ensure the message is delivered to user space. After commit 0642840 ("af_netlink: ensure that NLMSG_DONE never fails in dumps") this is no longer necessary and I can improve these dump callbacks assuming this analysis is correct. No regressions in existing tests: # ./fib_nexthops.sh [...] Tests passed: 230 Tests failed: 0 ==================== Link: https://lore.kernel.org/r/20230808075233.3337922-1-idosch@nvidia.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
@@ -3221,13 +3221,9 @@ static int rtm_dump_nexthop(struct sk_buff *skb, struct netlink_callback *cb)
 				     &rtm_dump_nexthop_cb, &filter);
 	if (err < 0) {
 		if (likely(skb->len))
-			goto out;
-		goto out_err;
+			err = skb->len;
 	}
 
-out:
-	err = skb->len;
-out_err:
 	cb->seq = net->nexthop.seq;
 	nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 	return err;
@@ -3367,25 +3363,19 @@ static int rtm_dump_nexthop_bucket_nh(struct sk_buff *skb,
 		    dd->filter.res_bucket_nh_id != nhge->nh->id)
 			continue;
 
+		dd->ctx->bucket_index = bucket_index;
 		err = nh_fill_res_bucket(skb, nh, bucket, bucket_index,
 					 RTM_NEWNEXTHOPBUCKET, portid,
 					 cb->nlh->nlmsg_seq, NLM_F_MULTI,
 					 cb->extack);
-		if (err < 0) {
-			if (likely(skb->len))
-				goto out;
-			goto out_err;
-		}
+		if (err)
+			return err;
 	}
 
 	dd->ctx->done_nh_idx = dd->ctx->nh.idx + 1;
-	bucket_index = 0;
+	dd->ctx->bucket_index = 0;
 
-out:
-	err = skb->len;
-out_err:
-	dd->ctx->bucket_index = bucket_index;
-	return err;
+	return 0;
 }
 
 static int rtm_dump_nexthop_bucket_cb(struct sk_buff *skb,
@@ -3434,13 +3424,9 @@ static int rtm_dump_nexthop_bucket(struct sk_buff *skb,
 
 	if (err < 0) {
 		if (likely(skb->len))
-			goto out;
-		goto out_err;
+			err = skb->len;
 	}
 
-out:
-	err = skb->len;
-out_err:
 	cb->seq = net->nexthop.seq;
 	nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 	return err;
diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
@@ -1981,6 +1981,11 @@ basic()
 
 	run_cmd "$IP link set dev lo up"
 
+	# Dump should not loop endlessly when maximum nexthop ID is configured.
+	run_cmd "$IP nexthop add id $((2**32-1)) blackhole"
+	run_cmd "timeout 5 $IP nexthop"
+	log_test $? 0 "Maximum nexthop ID dump"
+
 	#
 	# groups
 	#
@@ -2201,6 +2206,11 @@ basic_res()
 	run_cmd "$IP nexthop bucket list fdb"
 	log_test $? 255 "Dump all nexthop buckets with invalid 'fdb' keyword"
 
+	# Dump should not loop endlessly when maximum nexthop ID is configured.
+	run_cmd "$IP nexthop add id $((2**32-1)) group 1/2 type resilient buckets 4"
+	run_cmd "timeout 5 $IP nexthop bucket"
+	log_test $? 0 "Maximum nexthop ID dump"
+
 	#
 	# resilient nexthop buckets get requests
 	#

Original file line number	Diff line number	Diff line change
`@@ -1981,6 +1981,11 @@ basic()`
`1981`	`1981`
`1982`	`1982`	`run_cmd "$IP link set dev lo up"`
`1983`	`1983`
	`1984`	`+ # Dump should not loop endlessly when maximum nexthop ID is configured.`
	`1985`	`+ run_cmd "$IP nexthop add id $((2**32-1)) blackhole"`
	`1986`	`+ run_cmd "timeout 5 $IP nexthop"`
	`1987`	`+ log_test $? 0 "Maximum nexthop ID dump"`
	`1988`	`+`
`1984`	`1989`	`#`
`1985`	`1990`	`# groups`
`1986`	`1991`	`#`
`@@ -2201,6 +2206,11 @@ basic_res()`
`2201`	`2206`	`run_cmd "$IP nexthop bucket list fdb"`
`2202`	`2207`	`log_test $? 255 "Dump all nexthop buckets with invalid 'fdb' keyword"`
`2203`	`2208`
	`2209`	`+ # Dump should not loop endlessly when maximum nexthop ID is configured.`
	`2210`	`+ run_cmd "$IP nexthop add id $((2**32-1)) group 1/2 type resilient buckets 4"`
	`2211`	`+ run_cmd "timeout 5 $IP nexthop bucket"`
	`2212`	`+ log_test $? 0 "Maximum nexthop ID dump"`
	`2213`	`+`
`2204`	`2214`	`#`
`2205`	`2215`	`# resilient nexthop buckets get requests`
`2206`	`2216`	`#`