Commit f8d3e0dc authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'nexthop-nexthop-dump-fixes' 

Ido Schimmel says:

====================
nexthop: Nexthop dump fixes

Patches #1 and #3 fix two problems related to nexthops and nexthop
buckets dump, respectively. Patch #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 0642840b ("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: default avatarJakub Kicinski <kuba@kernel.org>
parents 718cb09a 8743aeff

net/ipv4/nexthop.c

+7 −21
Original line number Diff line number Diff line
@@ -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;

tools/testing/selftests/net/fib_nexthops.sh

+10 −0
Original line number Diff line number Diff line
@@ -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

	#