Date: Fri, 02 Jul 2021 11:18:41 +0200 From: "Kristof Provost" <kp@FreeBSD.org> To: "Gleb Smirnoff" <glebius@freebsd.org> Cc: "Warner Losh" <imp@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: f13da24715a7 - main - net/bpf: Fix writing of buffer bigger than PAGESIZE Message-ID: <4EAFAECE-9E03-48A7-840B-26E7D871FE29@FreeBSD.org> In-Reply-To: <YNObBaVrqLmoS2Ci@FreeBSD.org> References: <202106231641.15NGf1xH083695@gitrepo.freebsd.org> <YNObBaVrqLmoS2Ci@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23 Jun 2021, at 22:35, Gleb Smirnoff wrote: > Warner, Kristof, > > On Wed, Jun 23, 2021 at 04:41:01PM +0000, Warner Losh wrote: > W> net/bpf: Fix writing of buffer bigger than PAGESIZE > W> > W> When allocating the mbuf we used m_get2 which fails > W> if len is superior to MJUMPAGESIZE, if its the case, > W> use m_getjcl instead. > W> > W> Reviewed by: kp@ > W> PR: 205164 > W> Pull Request: https://github.com/freebsd/freebsd-src/pull/131 > > m_get2() used to provide jumbo mbufs in the past, see > 3112ae76449ae0931d207603f14b083627bd731d. > > IMHO, makes sense to create m_get3() and use it in bpf. What do you > think? > Do you mean, create an m_get3() like this (untested): diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c index a46c576bad90..202a1c1a5b41 100644 --- a/sys/kern/kern_mbuf.c +++ b/sys/kern/kern_mbuf.c @@ -1370,6 +1370,51 @@ m_get2(int size, int how, short type, int flags) return (m); } +/* + * m_get3() allocates minimum mbuf that would fit "size" argument. + * Unlike m_get2() is can allocate clusters up to MJUM16BYTES. + */ +struct mbuf * +m_get3(int size, int how, short type, int flags) +{ + struct mb_args args; + struct mbuf *m, *n; + uma_zone_t zone; + + args.flags = flags; + args.type = type; + + if (size <= MHLEN || (size <= MLEN && (flags & M_PKTHDR) == 0)) + return (uma_zalloc_arg(zone_mbuf, &args, how)); + if (size <= MCLBYTES) + return (uma_zalloc_arg(zone_pack, &args, how)); + + if (size > MJUM16BYTES) + return (NULL); + + m = uma_zalloc_arg(zone_mbuf, &args, how); + if (m == NULL) + return (NULL); + +#if MJUMPAGESIZE != MCLBYTES + if (size <= MJUMPAGESIZE) + zone = zone_jumbop; + else +#endif + if (size <= MJUM9BYTES) + zone = zone_jumbo9; + else + zone = zone_jumbo16; + + n = uma_zalloc_arg(zone_jumbop, m, how); + if (n == NULL) { + uma_zfree(zone_mbuf, m); + return (NULL); + } + + return (m); +} + /* * m_getjcl() returns an mbuf with a cluster of the specified size attached. * For size it takes MCLBYTES, MJUMPAGESIZE, MJUM9BYTES, MJUM16BYTES. diff --git a/sys/net/bpf.c b/sys/net/bpf.c index ec05dd6d337b..ef7285241fdf 100644 --- a/sys/net/bpf.c +++ b/sys/net/bpf.c @@ -641,15 +641,7 @@ bpf_movein(struct uio *uio, int linktype, struct ifnet *ifp, struct mbuf **mp, if (len < hlen || len - hlen > ifp->if_mtu) return (EMSGSIZE); - /* Allocate a mbuf for our write, since m_get2 fails if len >= to MJUMPAGESIZE, use m_getjcl for bigger buffers */ - if (len < MJUMPAGESIZE) - m = m_get2(len, M_WAITOK, MT_DATA, M_PKTHDR); - else if (len <= MJUM9BYTES) - m = m_getjcl(M_WAITOK, MT_DATA, M_PKTHDR, MJUM9BYTES); - else if (len <= MJUM16BYTES) - m = m_getjcl(M_WAITOK, MT_DATA, M_PKTHDR, MJUM16BYTES); - else - m = NULL; + m = m_get3(len, M_WAITOK, MT_DATA, M_PKTHDR); if (m == NULL) return (EIO); m->m_pkthdr.len = m->m_len = len; diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h index e37b872c74fe..b9211aaedc3c 100644 --- a/sys/sys/mbuf.h +++ b/sys/sys/mbuf.h @@ -828,6 +828,7 @@ u_int m_fixhdr(struct mbuf *); struct mbuf *m_fragment(struct mbuf *, int, int); void m_freem(struct mbuf *); struct mbuf *m_get2(int, int, short, int); +struct mbuf *m_get3(int, int, short, int); struct mbuf *m_getjcl(int, short, int, int); struct mbuf *m_getm2(struct mbuf *, int, int, short, int); struct mbuf *m_getptr(struct mbuf *, int, int *); — Kristof From owner-dev-commits-src-main@freebsd.org Fri Jul 2 13:49:15 2021 Return-Path: <owner-dev-commits-src-main@freebsd.org> Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D479B66CEAD; Fri, 2 Jul 2021 13:49:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GGbzH5jPSz4c9V; Fri, 2 Jul 2021 13:49:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A817B1535C; Fri, 2 Jul 2021 13:49:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 162DnFeR045398; Fri, 2 Jul 2021 13:49:15 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 162DnF4N045397; Fri, 2 Jul 2021 13:49:15 GMT (envelope-from git) Date: Fri, 2 Jul 2021 13:49:15 GMT Message-Id: <202107021349.162DnF4N045397@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost <kp@FreeBSD.org> Subject: git: d8d43b2de1fa - main - pf tests: Stress state retrieval MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: d8d43b2de1fa679179f7089cb3c31e6780ec82af Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository <dev-commits-src-main.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/dev-commits-src-main>, <mailto:dev-commits-src-main-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/dev-commits-src-main/>; List-Post: <mailto:dev-commits-src-main@freebsd.org> List-Help: <mailto:dev-commits-src-main-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main>, <mailto:dev-commits-src-main-request@freebsd.org?subject=subscribe> X-List-Received-Date: Fri, 02 Jul 2021 13:49:15 -0000 The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=d8d43b2de1fa679179f7089cb3c31e6780ec82af commit d8d43b2de1fa679179f7089cb3c31e6780ec82af Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2021-06-28 10:48:20 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2021-07-02 12:46:32 +0000 pf tests: Stress state retrieval Create and retrieve 20.000 states. There have been issues with nvlists causing very slow state retrieval. We don't impose a specific limit on the time required to retrieve the states, but do log it. In excessive cases the Kyua timeout will fail this test. Reviewed by: donner MFC after: 1 week Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D30943 --- tests/sys/netpfil/common/Makefile | 2 + tests/sys/netpfil/common/pft_synflood.py | 62 ++++++++++++++++++++++++ tests/sys/netpfil/pf/Makefile | 1 + tests/sys/netpfil/pf/get_state.sh | 81 ++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+) diff --git a/tests/sys/netpfil/common/Makefile b/tests/sys/netpfil/common/Makefile index f4d5fb2be3fc..749e0dd99469 100644 --- a/tests/sys/netpfil/common/Makefile +++ b/tests/sys/netpfil/common/Makefile @@ -17,8 +17,10 @@ ${PACKAGE}FILES+= \ utils.subr \ runner.subr \ pft_ping.py \ + pft_synflood.py \ sniffer.py ${PACKAGE}FILESMODE_pft_ping.py= 0555 +${PACKAGE}FILESMODE_pft_synflood.py= 0555 .include <bsd.test.mk> diff --git a/tests/sys/netpfil/common/pft_synflood.py b/tests/sys/netpfil/common/pft_synflood.py new file mode 100644 index 000000000000..3d37724593e2 --- /dev/null +++ b/tests/sys/netpfil/common/pft_synflood.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +# +# SPDX-License-Identifier: BSD-2-Clause-FreeBSD +# +# Copyright (c) 2021 Rubicon Communications, LLC (Netgate) +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +import argparse +import logging +logging.getLogger("scapy").setLevel(logging.CRITICAL) +import scapy.all as sp + +def syn_flood(args): + s = sp.conf.L2socket(iface=args.sendif[0]) + + # Set a src mac, to avoid doing lookups which really slow us down. + ether = sp.Ether(src='01:02:03:04:05') + ip = sp.IP(dst=args.to[0]) + for i in range(int(args.count[0])): + tcp = sp.TCP(flags='S', sport=1+i, dport=22, seq=500+i) + pkt = ether / ip / tcp + s.send(pkt) + +def main(): + parser = argparse.ArgumentParser("pft_synflood.py", + description="SYN flooding tool") + parser.add_argument('--sendif', nargs=1, + required=True, + help='The interface through which the packet(s) will be sent') + parser.add_argument('--to', nargs=1, + required=True, + help='The destination IP address for the SYN packets') + parser.add_argument('--count', nargs=1, + required=True, + help='The number of packets to send') + + args = parser.parse_args() + + syn_flood(args) + +if __name__ == '__main__': + main() diff --git a/tests/sys/netpfil/pf/Makefile b/tests/sys/netpfil/pf/Makefile index 8bbc498f38c5..2f92d835772b 100644 --- a/tests/sys/netpfil/pf/Makefile +++ b/tests/sys/netpfil/pf/Makefile @@ -11,6 +11,7 @@ ATF_TESTS_SH+= altq \ dup \ forward \ fragmentation \ + get_state \ icmp \ killstate \ map_e \ diff --git a/tests/sys/netpfil/pf/get_state.sh b/tests/sys/netpfil/pf/get_state.sh new file mode 100644 index 000000000000..5e6a9040e016 --- /dev/null +++ b/tests/sys/netpfil/pf/get_state.sh @@ -0,0 +1,81 @@ +# $FreeBSD$ +# +# SPDX-License-Identifier: BSD-2-Clause-FreeBSD +# +# Copyright (c) 2021 Rubicon Communications, LLC (Netgate) +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +. $(atf_get_srcdir)/utils.subr + +common_dir=$(atf_get_srcdir)/../common + +atf_test_case "many" "cleanup" +many_head() +{ + atf_set descr 'Test retrieving many states' + atf_set require.user root + atf_set require.progs scapy +} + +many_body() +{ + pft_init + + epair=$(vnet_mkepair) + ifconfig ${epair}a 192.0.2.1/24 up + + vnet_mkjail alcatraz ${epair}b + jexec alcatraz ifconfig ${epair}b 192.0.2.2/24 up + jexec alcatraz pfctl -e + + pft_set_rules alcatraz "set timeout tcp.closed 60000" \ + "pass in proto icmp" \ + "pass in proto tcp" + + # Sanity check + atf_check -s exit:0 -o ignore ping -c 1 -W 1 192.0.2.2 + + # Now syn flood to create many states + ${common_dir}/pft_synflood.py \ + --sendif ${epair}a \ + --to 192.0.2.2 \ + --count 20000 + + count=$(time jexec alcatraz pfctl -ss | wc -l 2>time.txt) + echo "Found $count states in `cat time.txt`" + if [ $count -lt 20000 ]; + then + atf_fail "Fail to retrieve states" + fi +} + +many_cleanup() +{ + rm -f time.txt + pft_cleanup +} + +atf_init_test_cases() +{ + atf_add_test_case "many" +}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4EAFAECE-9E03-48A7-840B-26E7D871FE29>