| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADvbK_coeb1v7BCwfA8fMGwJz7o6QqLsV8Ky+YOAX6LZpH1LBA@mail.gmail.com>
Date: Tue, 15 Nov 2022 13:07:35 -0500
From: Xin Long <lucien.xin@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: network dev <netdev@...r.kernel.org>, linux-sctp@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org,
Eric Dumazet <edumazet@...gle.com>,
Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
Neil Horman <nhorman@...driver.com>,
David Ahern <dsahern@...il.com>,
Carlo Carraro <colrack@...il.com>
Subject: Re: [PATCH net-next 7/7] selftests: add a selftest for sctp vrf
On Tue, Nov 15, 2022 at 10:33 AM Xin Long <lucien.xin@...il.com> wrote:
>
> On Tue, Nov 15, 2022 at 5:29 AM Paolo Abeni <pabeni@...hat.com> wrote:
> >
> > Hello,
> >
> > On Sun, 2022-11-13 at 11:44 -0500, Xin Long wrote:
> > > This patch adds 12 small test cases: 01-04 test for the sysctl
> > > net.sctp.l3mdev_accept. 05-10 test for only binding to a right
> > > l3mdev device, the connection can be created. 11-12 test for
> > > two socks binding to different l3mdev devices at the same time,
> > > each of them can process the packets from the corresponding
> > > peer. The tests run for both IPv4 and IPv6 SCTP.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@...il.com>
> > > ---
> > > tools/testing/selftests/net/Makefile | 2 +
> > > tools/testing/selftests/net/sctp_hello.c | 139 ++++++++++++++++++++
> > > tools/testing/selftests/net/sctp_vrf.sh | 160 +++++++++++++++++++++++
> > > 3 files changed, 301 insertions(+)
> > > create mode 100644 tools/testing/selftests/net/sctp_hello.c
> > > create mode 100755 tools/testing/selftests/net/sctp_vrf.sh
> > >
> > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > > index cec4800cb017..880e6ded6ed5 100644
> > > --- a/tools/testing/selftests/net/Makefile
> > > +++ b/tools/testing/selftests/net/Makefile
> > > @@ -72,6 +72,8 @@ TEST_GEN_PROGS += sk_bind_sendto_listen
> > > TEST_GEN_PROGS += sk_connect_zero_addr
> > > TEST_PROGS += test_ingress_egress_chaining.sh
> > > TEST_GEN_PROGS += so_incoming_cpu
> > > +TEST_PROGS += sctp_vrf.sh
> > > +TEST_GEN_FILES += sctp_hello
> > >
> > > TEST_FILES := settings
> > >
> > > diff --git a/tools/testing/selftests/net/sctp_hello.c b/tools/testing/selftests/net/sctp_hello.c
> > > new file mode 100644
> > > index 000000000000..58f763ca8b47
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/net/sctp_hello.c
> > > @@ -0,0 +1,139 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <unistd.h>
> > > +#include <sys/types.h>
> > > +#include <netinet/in.h>
> > > +#include <arpa/inet.h>
> > > +
> > > +static void set_addr(struct sockaddr_storage *ss, char *ip, char *port, int *len)
> > > +{
> > > + if (ss->ss_family == AF_INET) {
> > > + struct sockaddr_in *a = (struct sockaddr_in *)ss;
> > > +
> > > + a->sin_addr.s_addr = inet_addr(ip);
> > > + a->sin_port = htons(atoi(port));
> > > + *len = sizeof(*a);
> > > + } else {
> > > + struct sockaddr_in6 *a = (struct sockaddr_in6 *)ss;
> > > +
> > > + a->sin6_family = AF_INET6;
> > > + inet_pton(AF_INET6, ip, &a->sin6_addr);
> > > + a->sin6_port = htons(atoi(port));
> > > + *len = sizeof(*a);
> > > + }
> > > +}
> > > +
> > > +static int do_client(int argc, char *argv[])
> > > +{
> > > + struct sockaddr_storage ss;
> > > + char buf[] = "hello";
> > > + int csk, ret, len;
> > > +
> > > + if (argc < 5) {
> > > + printf("%s client -4|6 IP PORT [IP PORT]\n", argv[0]);
> > > + return -1;
> > > + }
> > > +
> > > + bzero((void *)&ss, sizeof(ss));
> > > + ss.ss_family = !strcmp(argv[2], "-4") ? AF_INET : AF_INET6;
> > > + csk = socket(ss.ss_family, SOCK_STREAM, IPPROTO_SCTP);
> > > + if (csk < 0) {
> > > + printf("failed to create socket\n");
> > > + return -1;
> > > + }
> > > +
> > > + if (argc >= 7) {
> > > + set_addr(&ss, argv[5], argv[6], &len);
> > > + ret = bind(csk, (struct sockaddr *)&ss, len);
> > > + if (ret < 0) {
> > > + printf("failed to bind to address\n");
> > > + return -1;
> > > + }
> > > + }
> > > +
> > > + set_addr(&ss, argv[3], argv[4], &len);
> > > + ret = connect(csk, (struct sockaddr *)&ss, len);
> > > + if (ret < 0) {
> > > + printf("failed to connect to peer\n");
> > > + return -1;
> > > + }
> > > +
> > > + ret = send(csk, buf, strlen(buf) + 1, 0);
> > > + if (ret < 0) {
> > > + printf("failed to send msg %d\n", ret);
> > > + return -1;
> > > + }
> > > + sleep(1);
> > > + close(csk);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int main(int argc, char *argv[])
> > > +{
> > > + struct sockaddr_storage ss;
> > > + int lsk, csk, ret, len;
> > > + char buf[20];
> > > +
> > > + if (argc < 2 || (strcmp(argv[1], "server") && strcmp(argv[1], "client"))) {
> > > + printf("%s server|client ...\n", argv[0]);
> > > + return -1;
> > > + }
> > > +
> > > + if (!strcmp(argv[1], "client"))
> > > + return do_client(argc, argv);
> > > +
> > > + if (argc < 5) {
> > > + printf("%s server -4|6 IP PORT [IFACE]\n", argv[0]);
> > > + return -1;
> > > + }
> > > +
> > > + ss.ss_family = !strcmp(argv[2], "-4") ? AF_INET : AF_INET6;
> > > + lsk = socket(ss.ss_family, SOCK_STREAM, IPPROTO_SCTP);
> > > + if (lsk < 0) {
> > > + printf("failed to create lsk\n");
> > > + return -1;
> > > + }
> > > +
> > > + if (argc >= 6) {
> > > + ret = setsockopt(lsk, SOL_SOCKET, SO_BINDTODEVICE,
> > > + argv[5], strlen(argv[5]) + 1);
> > > + if (ret < 0) {
> > > + printf("failed to bind to device\n");
> > > + return -1;
> > > + }
> > > + }
> > > +
> > > + set_addr(&ss, argv[3], argv[4], &len);
> > > + ret = bind(lsk, (struct sockaddr *)&ss, len);
> > > + if (ret < 0) {
> > > + printf("failed to bind to address\n");
> > > + return -1;
> > > + }
> > > +
> > > + ret = listen(lsk, 5);
> > > + if (ret < 0) {
> > > + printf("failed to listen on port\n");
> > > + return -1;
> > > + }
> > > +
> > > + csk = accept(lsk, (struct sockaddr *)NULL, (socklen_t *)NULL);
> > > + if (csk < 0) {
> > > + printf("failed to accept new client\n");
> > > + return -1;
> > > + }
> > > +
> > > + ret = recv(csk, buf, sizeof(buf), 0);
> > > + if (ret <= 0) {
> > > + printf("failed to recv msg %d\n", ret);
> > > + return -1;
> > > + }
> > > + sleep(2);
> >
> > Why do you need such sleep here? aren't blocking operations on socket
> > enough?
Sorry, forgot to reply to this one.
I was expecting to close it from the client side.
But it seems fine to close it by the server side, I will delete this
sleep and leave sleep(1) on the client only.
Thanks.
> >
> > > + close(csk);
> > > + close(lsk);
> > > +
> > > + return 0;
> > > +}
> > > diff --git a/tools/testing/selftests/net/sctp_vrf.sh b/tools/testing/selftests/net/sctp_vrf.sh
> > > new file mode 100755
> > > index 000000000000..310749f58386
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/net/sctp_vrf.sh
> > > @@ -0,0 +1,160 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Testing For SCTP VRF.
> > > +# TOPO: CLIENT_NS1 (veth1) <---> (veth1) -> vrf_s1
> > > +# SERVER_NS
> > > +# CLIENT_NS2 (veth1) <---> (veth2) -> vrf_s2
> > > +
> > > +CLIENT_NS1="client-ns1"
> > > +CLIENT_NS2="client-ns2"
> > > +CLIENT_IP4="10.0.0.1"
> > > +CLIENT_IP6="2000::1"
> > > +CLIENT_PORT=1234
> > > +
> > > +SERVER_NS="server-ns"
> > > +SERVER_IP4="10.0.0.2"
> > > +SERVER_IP6="2000::2"
> > > +SERVER_PORT=1234
> > > +
> > > +setup() {
> > > + modprobe sctp
> > > + ip netns add $CLIENT_NS1
> > > + ip netns add $CLIENT_NS2
> > > + ip netns add $SERVER_NS
> > > + ip -n $SERVER_NS link add veth1 type veth peer name veth1 netns $CLIENT_NS1
> > > + ip -n $SERVER_NS link add veth2 type veth peer name veth1 netns $CLIENT_NS2
> > > +
> > > + ip -n $CLIENT_NS1 link set veth1 up
> > > + ip -n $CLIENT_NS1 addr add $CLIENT_IP4/24 dev veth1
> > > + ip -n $CLIENT_NS1 addr add $CLIENT_IP6/24 dev veth1
> > > +
> > > + ip -n $CLIENT_NS2 link set veth1 up
> > > + ip -n $CLIENT_NS2 addr add $CLIENT_IP4/24 dev veth1
> > > + ip -n $CLIENT_NS2 addr add $CLIENT_IP6/24 dev veth1
> > > +
> > > + ip -n $SERVER_NS link add dummy1 type dummy
> > > + ip -n $SERVER_NS link set dummy1 up
> > > + ip -n $SERVER_NS link add vrf-1 type vrf table 10
> > > + ip -n $SERVER_NS link add vrf-2 type vrf table 20
> > > + ip -n $SERVER_NS link set vrf-1 up
> > > + ip -n $SERVER_NS link set vrf-2 up
> > > + ip -n $SERVER_NS link set veth1 master vrf-1
> > > + ip -n $SERVER_NS link set veth2 master vrf-2
> > > +
> > > + ip -n $SERVER_NS addr add $SERVER_IP4/24 dev dummy1
> > > + ip -n $SERVER_NS addr add $SERVER_IP4/24 dev veth1
> > > + ip -n $SERVER_NS addr add $SERVER_IP4/24 dev veth2
> > > + ip -n $SERVER_NS addr add $SERVER_IP6/24 dev dummy1
> > > + ip -n $SERVER_NS addr add $SERVER_IP6/24 dev veth1
> > > + ip -n $SERVER_NS addr add $SERVER_IP6/24 dev veth2
> > > +
> > > + ip -n $SERVER_NS link set veth1 up
> > > + ip -n $SERVER_NS link set veth2 up
> > > + sleep 3
> >
> > I guess you need this 'sleep' to wait for dad completion on the above
> > ipv6 addresses. If so, you can avoid the sleep adding 'nodad' to the ip
> > route command line.
> sounds good, I will give it a try.
>
> >
> > > + ip -n $SERVER_NS route add table 10 $CLIENT_IP4 dev veth1 src $SERVER_IP4
> > > + ip -n $SERVER_NS route add table 20 $CLIENT_IP4 dev veth2 src $SERVER_IP4
> > > + ip -n $SERVER_NS route add $CLIENT_IP4 dev veth1 src $SERVER_IP4
> > > + ip -n $SERVER_NS route add table 10 $CLIENT_IP6 dev veth1 src $SERVER_IP6
> > > + ip -n $SERVER_NS route add table 20 $CLIENT_IP6 dev veth2 src $SERVER_IP6
> > > + ip -n $SERVER_NS route add $CLIENT_IP6 dev veth1 src $SERVER_IP6
> > > +
> > > + L3MDEV=`ip netns exec $SERVER_NS cat /proc/sys/net/sctp/l3mdev_accept`
> > > +}
> > > +
> > > +cleanup() {
> > > + ip netns exec $SERVER_NS echo $L3MDEV > /proc/sys/net/sctp/l3mdev_accept
> > > + ip netns exec $SERVER_NS pkill sctp_hello 2>&1 >/dev/null
> > > + ip netns del "$CLIENT_NS1"
> > > + ip netns del "$CLIENT_NS2"
> > > + ip netns del "$SERVER_NS"
> > > +}
> > > +
> > > +do_testx() {
> > > + IFACE1=$1
> > > + IFACE2=$2
> > > +
> > > + ip netns exec $SERVER_NS pkill sctp_hello 2>&1 >/dev/null
> > > + ip netns exec $SERVER_NS ./sctp_hello server $AF $SERVER_IP \
> > > + $SERVER_PORT $IFACE1 2>&1 >/dev/null &
> > > + disown
> > > + ip netns exec $SERVER_NS ./sctp_hello server $AF $SERVER_IP \
> > > + $SERVER_PORT $IFACE2 2>&1 >/dev/null &
> > > + disown
> > > + sleep 1
> >
> > Altertantivelly you can wait untill the relevant sctp socket is liste
> > by 'ss' output, it would be more roboust.
> right, this one looks better :-).
> >
> > > + timeout 3 ip netns exec $CLIENT_NS1 ./sctp_hello client $AF \
> > > + $SERVER_IP $SERVER_PORT $CLIENT_IP $CLIENT_PORT 2>&1 >/dev/null && \
> > > + timeout 3 ip netns exec $CLIENT_NS2 ./sctp_hello client $AF \
> > > + $SERVER_IP $SERVER_PORT $CLIENT_IP $CLIENT_PORT 2>&1 >/dev/null
> > > +}
> > > +
> > > +do_test() {
> > > + CLIENT_NS=$1
> > > + IFACE=$2
> > > +
> > > + ip netns exec $SERVER_NS pkill sctp_hello 2>&1 >/dev/null
> > > + ip netns exec $SERVER_NS ./sctp_hello server $AF $SERVER_IP \
> > > + $SERVER_PORT $IFACE 2>&1 >/dev/null &
> > > + disown
> > > + sleep 1
> > > + timeout 3 ip netns exec $CLIENT_NS ./sctp_hello client $AF \
> > > + $SERVER_IP $SERVER_PORT $CLIENT_IP $CLIENT_PORT 2>&1 >/dev/null
> > > +}
> > > +
> > > +testup() {
> > > + ip netns exec $SERVER_NS sysctl -w net.sctp.l3mdev_accept=1 2>&1 >/dev/null
> > > + echo -n "TEST 01: nobind, connect from client 1, l3mdev_accept=1, Y "
> > > + do_test $CLIENT_NS1 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 02: nobind, connect from client 2, l3mdev_accept=1, N "
> > > + do_test $CLIENT_NS2 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + ip netns exec $SERVER_NS sysctl -w net.sctp.l3mdev_accept=0 2>&1 >/dev/null
> > > + echo -n "TEST 03: nobind, connect from client 1, l3mdev_accept=0, N "
> > > + do_test $CLIENT_NS1 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 04: nobind, connect from client 2, l3mdev_accept=0, N "
> > > + do_test $CLIENT_NS2 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 05: bind veth2 in server, connect from client 1, N "
> > > + do_test $CLIENT_NS1 veth2 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 06: bind veth1 in server, connect from client 1, Y "
> > > + do_test $CLIENT_NS1 veth1 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 07: bind vrf-1 in server, connect from client 1, Y "
> > > + do_test $CLIENT_NS1 vrf-1 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 08: bind vrf-2 in server, connect from client 1, N "
> > > + do_test $CLIENT_NS1 vrf-2 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 09: bind vrf-2 in server, connect from client 2, Y "
> > > + do_test $CLIENT_NS2 vrf-2 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 10: bind vrf-1 in server, connect from client 2, N "
> > > + do_test $CLIENT_NS2 vrf-1 && { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 11: bind vrf-1 & 2 in server, connect from client 1 & 2, Y "
> > > + do_testx vrf-1 vrf-2 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +
> > > + echo -n "TEST 12: bind vrf-2 & 1 in server, connect from client 1 & 2, N "
> > > + do_testx vrf-2 vrf-1 || { echo "[FAIL]"; return 1; }
> > > + echo "[PASS]"
> > > +}
> > > +
> > > +trap cleanup EXIT
> > > +setup || exit $?
> > > +echo "Testing For SCTP VRF:"
> > > +CLIENT_IP=$CLIENT_IP4 SERVER_IP=$SERVER_IP4 AF="-4" testup && echo "***v4 Tests Done***" &&
> > > +CLIENT_IP=$CLIENT_IP6 SERVER_IP=$SERVER_IP6 AF="-6" testup && echo "***v6 Tests Done***"
> >
> > To properly integrate with the self-test suite, you need to ensure that
> > the script exits with an error code in case of failure, e.g. storing
> > the error in a global variable 'ret' and adding a final:
> >
> > exit $ret
> the above lines are equal to "exit $ret".
> the exit code in testup() will return if it's not 0.
> Do you mean I should make it more clear?
>
> Thanks for the review.
> >
> > Cheers,
> >
> > Paolo
> >
Powered by blists - more mailing lists