On Tue, Oct 15, 2024 at 10:51:52PM -0700, Leo Stone wrote:
> Add tests that check if getsockopt(TCP_AO_GET_KEYS) returns the right
> keys when using different filters.
>
> Sample output:
>
> > # ok 114 filter keys: by sndid, rcvid, address
> > # ok 115 filter keys: by is_current
> > # ok 116 filter keys: by is_rnext
> > # ok 117 filter keys: by sndid, rcvid
> > # ok 118 filter keys: correct nkeys when in.nkeys < matched_keys
>
> Signed-off-by: Leo Stone <leocstone@xxxxxxxxx>
> ---
> Changes in v2:
> - Changed 2 unnecessary test_error calls to test_fail
> - Added another test to make sure getsockopt returns the right nkeys
> value when the input nkeys is smaller than the number of matching keys
> - Removed the TODO that this patch addresses
>
> Thank you for your feedback.
> ---
> .../selftests/net/tcp_ao/setsockopt-closed.c | 180 +++++++++++++++++-
> 1 file changed, 175 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> index 084db4ecdff6..4bfa76c28e4e 100644
> --- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> +++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> @@ -6,6 +6,8 @@
>
> static union tcp_addr tcp_md5_client;
>
> +#define FILTER_TEST_NKEYS 16
> +
> static int test_port = 7788;
> static void make_listen(int sk)
> {
> @@ -813,23 +815,191 @@ static void duplicate_tests(void)
> setsockopt_checked(sk, TCP_AO_ADD_KEY, &ao, EEXIST, "duplicate: SendID differs");
> }
>
> +
> +static void fetch_all_keys(int sk, struct tcp_ao_getsockopt *keys)
> +{
> + socklen_t optlen = sizeof(struct tcp_ao_getsockopt);
> +
> + memset(keys, 0, sizeof(struct tcp_ao_getsockopt) * FILTER_TEST_NKEYS);
> + keys[0].get_all = 1;
> + keys[0].nkeys = FILTER_TEST_NKEYS;
> + if (getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS, &keys[0], &optlen))
> + test_error("getsockopt");
> +}
> +
> +static int prepare_test_keys(struct tcp_ao_getsockopt *keys)
> +{
> + struct tcp_ao_add test_ao[FILTER_TEST_NKEYS];
> + u8 rcvid = 100, sndid = 100;
> + const char *test_password = "Test password number ";
> + char test_password_scratch[64] = {};
> + int sk = socket(test_family, SOCK_STREAM, IPPROTO_TCP);
Hi Leo,
In Networking code it is preferred to arrange local variables in
reverse xmas tree order. In this case I think that could be as
follows (completely untested!).
Also, as the sk needs to be checked for errors, I would
separate it's assignment form it's declaration
const char *test_password = "Test password number ";
struct tcp_ao_add test_ao[FILTER_TEST_NKEYS];
char test_password_scratch[64] = {};
u8 rcvid = 100, sndid = 100;
int sk;
sk = socket(test_family, SOCK_STREAM, IPPROTO_TCP);
if (sk < 0)
test_error("socket()");
This tool can be of assistance here:
https://github.com/ecree-solarflare/xmastree
> +
> + if (sk < 0)
> + test_error("socket()");
> +
> + for (int i = 0; i < FILTER_TEST_NKEYS; i++) {
> + snprintf(test_password_scratch, 64, "%s %d", test_password, i);
> + test_prepare_key(&test_ao[i], DEFAULT_TEST_ALGO, this_ip_dest, false, false,
> + DEFAULT_TEST_PREFIX, 0, sndid++, rcvid++, 0, 0,
> + strlen(test_password_scratch), test_password_scratch);
Likewise, in Networking code it is still preferred to keep lines at or
below 80 columns wide, where it can trivially be achieved: don't split
strings across or otherwise make the code less readable because of this
guideline.
test_prepare_key(&test_ao[i], DEFAULT_TEST_ALGO, this_ip_dest,
false, false, DEFAULT_TEST_PREFIX, 0, sndid++,
rcvid++, 0, 0, strlen(test_password_scratch),
test_password_scratch);
You can check for this using:
./scripts/checkpatch.pl --strict --max-line-length=80
I think it would be good if you could do a pass over this patch with the
above in mind.
Lastly, please include the target tree, net or net-next, in the subject
when posting patches for Networking.
Subject: [PATCH net-next] ...
More information on processes for netdev can be found here:
https://docs.kernel.org/process/maintainer-netdev.html
--
pw-bot: changes-requested
