Date: Sat, 15 Mar 2014 12:03:47 +0000 (GMT) From: Robert Watson <rwatson@FreeBSD.org> To: Adrian Chadd <adrian@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r263198 - in head/sys: amd64/conf conf net netinet netinet6 sys Message-ID: <alpine.BSF.2.00.1403151202440.36417@fledge.watson.org> In-Reply-To: <CAJ-Vmo=5YdLTkD0Dv5qHQokTQxtjFB=t2NqXfFzuZRLqjOOu%2Bg@mail.gmail.com> References: <201403150057.s2F0vofg081606@svn.freebsd.org> <CAJ-Vmo=5YdLTkD0Dv5qHQokTQxtjFB=t2NqXfFzuZRLqjOOu%2Bg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 14 Mar 2014, Adrian Chadd wrote: > Woo! Thanks! I'd characterise this work as "early" in that it would benefit from performance optimisation, device-driver work, and feature enhancement (e.g., not just stuff like load rebalancing, but also statistics to detect RSS configuration problems leading to work being directed to the wrong CPU, etc). In any case, hopefully this is a useful starting point for people doing work in this space. Robert > > Adrian > > On Mar 14, 2014 5:57 PM, "Robert Watson" <rwatson@freebsd.org> wrote: > Author: rwatson > Date: Sat Mar 15 00:57:50 2014 > New Revision: 263198 > URL: http://svnweb.freebsd.org/changeset/base/263198 > > Log: > Several years after initial development, merge prototype > support for > linking NIC Receive Side Scaling (RSS) to the network stack's > connection-group implementation. This prototype (and derived > patches) > are in use at Juniper and several other FreeBSD-using > companies, so > despite some reservations about its maturity, merge the patch > to the > base tree so that it can be iteratively refined in > collaboration rather > than maintained as a set of gradually diverging patch sets. > > (1) Merge a software implementation of the Toeplitz hash > specified in > RSS implemented by David Malone. This is used to allow > suitable > pcbgroup placement of connections before the first packet > is > received from the NIC. Software hashing is generally > avoided, > however, due to high cost of the hash on general-purpose > CPUs. > > (2) In in_rss.c, maintain authoritative versions of RSS state > intended > to be pushed to each NIC, including keying material, hash > algorithm/ configuration, and buckets. Provide > software-facing > interfaces to hash 2- and 4-tuples for IPv4 and IPv6 using > both > the RSS standardised Toeplitz and a 'naive' variation with > a hash > efficient in software but with poor distribution > properties. > Implement rss_m2cpuid()to be used by netisr and other load > balancing code to look up the CPU on which an mbuf should > be > processed. > > (3) In the Ethernet link layer, allow netisr distribution > using RSS as > a source of policy as an alternative to source ordering; > continue > to default to direct dispatch (i.e., don't try and requeue > packets > for processing on the 'right' CPU if they arrive in a > directly > dispatchable context). > > (4) Allow RSS to control tuning of connection groups in order > to align > groups with RSS buckets. If a packet arrives on a > protocol using > connection groups, and contains a suitable > hardware-generated > hash, use that hash value to select the connection group > for pcb > lookup for both IPv4 and IPv6. If no hardware-generated > Toeplitz > hash is available, we fall back on regular PCB lookup > risking > contention rather than pay the cost of Toeplitz in > software -- > this is a less scalable but, at my last measurement, > faster > approach. As core counts go up, we may want to revise > this > strategy despite CPU overhead. > > Where device drivers suitably configure NICs, and connection > groups / > RSS are enabled, this should avoid both lock and line > contention during > connection lookup for TCP. This commit does not modify any > device > drivers to tune device RSS configuration to the global RSS > configuration; patches are in circulation to do this for at > least > Chelsio T3 and Intel 1G/10G drivers. Currently, the KPI for > device > drivers is not particularly robust, nor aware of more advanced > features > such as runtime reconfiguration/rebalancing. This will > hopefully prove > a useful starting point for refinement. > > No MFC is scheduled as we will first want to nail down a more > mature > and maintainable KPI/KBI for device drivers. > > Sponsored by: Juniper Networks (original work) > Sponsored by: EMC/Isilon (patch update and merge) > > Added: > head/sys/netinet/in_rss.c (contents, props changed) > head/sys/netinet/in_rss.h (contents, props changed) > head/sys/netinet/toeplitz.c (contents, props changed) > head/sys/netinet/toeplitz.h (contents, props changed) > Modified: > head/sys/amd64/conf/GENERIC > head/sys/conf/files > head/sys/conf/options > head/sys/net/if_ethersubr.c > head/sys/netinet/in_pcb.c > head/sys/netinet/in_pcbgroup.c > head/sys/netinet6/in6_pcb.c > head/sys/netinet6/in6_pcbgroup.c > head/sys/sys/priv.h > > Modified: head/sys/amd64/conf/GENERIC > =========================================================================== > === > --- head/sys/amd64/conf/GENERIC Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/amd64/conf/GENERIC Sat Mar 15 00:57:50 2014 > (r263198) > @@ -28,6 +28,8 @@ options SCHED_ULE # ULE > scheduler > options PREEMPTION # Enable kernel thread > preemption > options INET # InterNETworking > options INET6 # IPv6 communications > protocols > +options PCBGROUP # Protocol control-block > groups > +options RSS # Receive-side scaling > support > options TCP_OFFLOAD # TCP offload > options SCTP # Stream Control > Transmission Protocol > options FFS # Berkeley Fast > Filesystem > > Modified: head/sys/conf/files > =========================================================================== > === > --- head/sys/conf/files Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/conf/files Sat Mar 15 00:57:50 2014 > (r263198) > @@ -3267,6 +3267,7 @@ netinet/in_pcb.c optional inet | > inet6 > netinet/in_pcbgroup.c optional inet pcbgroup | inet6 > pcbgroup > netinet/in_proto.c optional inet | inet6 > netinet/in_rmx.c optional inet > +netinet/in_rss.c optional inet rss | inet6 rss > netinet/ip_divert.c optional inet ipdivert > ipfirewall > netinet/ip_ecn.c optional inet | inet6 > netinet/ip_encap.c optional inet | inet6 > @@ -3308,6 +3309,7 @@ netinet/tcp_syncache.c optional > inet | > netinet/tcp_timer.c optional inet | inet6 > netinet/tcp_timewait.c optional inet | inet6 > netinet/tcp_usrreq.c optional inet | inet6 > +netinet/toeplitz.c optional inet rss | inet6 rss > netinet/udp_usrreq.c optional inet | inet6 > netinet/libalias/alias.c optional libalias inet | > netgraph_nat inet > netinet/libalias/alias_db.c optional libalias inet | > netgraph_nat inet > > Modified: head/sys/conf/options > =========================================================================== > === > --- head/sys/conf/options Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/conf/options Sat Mar 15 00:57:50 2014 > (r263198) > @@ -427,6 +427,7 @@ PCBGROUP opt_pcbgroup.h > PF_DEFAULT_TO_DROP opt_pf.h > RADIX_MPATH opt_mpath.h > ROUTETABLES opt_route.h > +RSS opt_rss.h > SLIP_IFF_OPTS opt_slip.h > TCPDEBUG > TCP_OFFLOAD opt_inet.h # Enable code to dispatch TCP > offloading > > Modified: head/sys/net/if_ethersubr.c > =========================================================================== > === > --- head/sys/net/if_ethersubr.c Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/net/if_ethersubr.c Sat Mar 15 00:57:50 2014 > (r263198) > @@ -34,6 +34,7 @@ > #include "opt_inet6.h" > #include "opt_netgraph.h" > #include "opt_mbuf_profiling.h" > +#include "opt_rss.h" > > #include <sys/param.h> > #include <sys/systm.h> > @@ -70,6 +71,7 @@ > #include <netinet/in.h> > #include <netinet/in_var.h> > #include <netinet/if_ether.h> > +#include <netinet/in_rss.h> > #include <netinet/ip_carp.h> > #include <netinet/ip_var.h> > #endif > @@ -583,7 +585,22 @@ ether_input_internal(struct ifnet *ifp, > > /* > * Ethernet input dispatch; by default, direct dispatch here > regardless of > - * global configuration. > + * global configuration. However, if RSS is enabled, hook up > RSS affinity > + * so that when deferred or hybrid dispatch is enabled, we can > redistribute > + * load based on RSS. > + * > + * XXXRW: Would be nice if the ifnet passed up a flag > indicating whether or > + * not it had already done work distribution via multi-queue. > Then we could > + * direct dispatch in the event load balancing was already > complete and > + * handle the case of interfaces with different capabilities > better. > + * > + * XXXRW: Sort of want an M_DISTRIBUTED flag to avoid multiple > distributions > + * at multiple layers? > + * > + * XXXRW: For now, enable all this only if RSS is compiled in, > although it > + * works fine without RSS. Need to characterise the > performance overhead > + * of the detour through the netisr code in the event the > result is always > + * direct dispatch. > */ > static void > ether_nh_input(struct mbuf *m) > @@ -596,8 +613,14 @@ static struct netisr_handler ether_nh > = > .nh_name = "ether", > .nh_handler = ether_nh_input, > .nh_proto = NETISR_ETHER, > +#ifdef RSS > + .nh_policy = NETISR_POLICY_CPU, > + .nh_dispatch = NETISR_DISPATCH_DIRECT, > + .nh_m2cpuid = rss_m2cpuid, > +#else > .nh_policy = NETISR_POLICY_SOURCE, > .nh_dispatch = NETISR_DISPATCH_DIRECT, > +#endif > }; > > static void > > Modified: head/sys/netinet/in_pcb.c > =========================================================================== > === > --- head/sys/netinet/in_pcb.c Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/netinet/in_pcb.c Sat Mar 15 00:57:50 2014 > (r263198) > @@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$"); > #include "opt_inet.h" > #include "opt_inet6.h" > #include "opt_pcbgroup.h" > +#include "opt_rss.h" > > #include <sys/param.h> > #include <sys/systm.h> > @@ -75,6 +76,7 @@ __FBSDID("$FreeBSD$"); > #if defined(INET) || defined(INET6) > #include <netinet/in.h> > #include <netinet/in_pcb.h> > +#include <netinet/in_rss.h> > #include <netinet/ip_var.h> > #include <netinet/tcp_var.h> > #include <netinet/udp.h> > @@ -1820,7 +1822,7 @@ struct inpcb * > in_pcblookup(struct inpcbinfo *pcbinfo, struct in_addr faddr, > u_int fport, > struct in_addr laddr, u_int lport, int lookupflags, struct > ifnet *ifp) > { > -#if defined(PCBGROUP) > +#if defined(PCBGROUP) && !defined(RSS) > struct inpcbgroup *pcbgroup; > #endif > > @@ -1829,7 +1831,17 @@ in_pcblookup(struct inpcbinfo *pcbinfo, > KASSERT((lookupflags & (INPLOOKUP_RLOCKPCB | > INPLOOKUP_WLOCKPCB)) != 0, > ("%s: LOCKPCB not set", __func__)); > > -#if defined(PCBGROUP) > + /* > + * When not using RSS, use connection groups in > preference to the > + * reservation table when looking up 4-tuples. When > using RSS, just > + * use the reservation table, due to the cost of the > Toeplitz hash > + * in software. > + * > + * XXXRW: This policy belongs in the pcbgroup code, as > in principle > + * we could be doing RSS with a non-Toeplitz hash that > is affordable > + * in software. > + */ > +#if defined(PCBGROUP) && !defined(RSS) > if (in_pcbgroup_enabled(pcbinfo)) { > pcbgroup = in_pcbgroup_bytuple(pcbinfo, laddr, > lport, faddr, > fport); > @@ -1856,16 +1868,27 @@ in_pcblookup_mbuf(struct inpcbinfo *pcbi > ("%s: LOCKPCB not set", __func__)); > > #ifdef PCBGROUP > - if (in_pcbgroup_enabled(pcbinfo)) { > + /* > + * If we can use a hardware-generated hash to look up > the connection > + * group, use that connection group to find the inpcb. > Otherwise > + * fall back on a software hash -- or the reservation > table if we're > + * using RSS. > + * > + * XXXRW: As above, that policy belongs in the pcbgroup > code. > + */ > + if (in_pcbgroup_enabled(pcbinfo) && > + !(M_HASHTYPE_TEST(m, M_HASHTYPE_NONE))) { > pcbgroup = in_pcbgroup_byhash(pcbinfo, > M_HASHTYPE_GET(m), > m->m_pkthdr.flowid); > if (pcbgroup != NULL) > return (in_pcblookup_group(pcbinfo, > pcbgroup, faddr, > fport, laddr, lport, lookupflags, > ifp)); > +#ifndef RSS > pcbgroup = in_pcbgroup_bytuple(pcbinfo, laddr, > lport, faddr, > fport); > return (in_pcblookup_group(pcbinfo, pcbgroup, > faddr, fport, > laddr, lport, lookupflags, ifp)); > +#endif > } > #endif > return (in_pcblookup_hash(pcbinfo, faddr, fport, laddr, > lport, > > Modified: head/sys/netinet/in_pcbgroup.c > =========================================================================== > === > --- head/sys/netinet/in_pcbgroup.c Sat Mar 15 00:23:35 2014 > (r263197) > +++ head/sys/netinet/in_pcbgroup.c Sat Mar 15 00:57:50 2014 > (r263198) > @@ -32,6 +32,7 @@ > __FBSDID("$FreeBSD$"); > > #include "opt_inet6.h" > +#include "opt_rss.h" > > #include <sys/param.h> > #include <sys/lock.h> > @@ -43,6 +44,7 @@ __FBSDID("$FreeBSD$"); > > #include <netinet/in.h> > #include <netinet/in_pcb.h> > +#include <netinet/in_rss.h> > #ifdef INET6 > #include <netinet6/in6_pcb.h> > #endif /* INET6 */ > @@ -60,6 +62,13 @@ __FBSDID("$FreeBSD$"); > * minimal cache line migration and lock contention during > steady state > * operation. > * > + * Hardware-offloaded checksums are often inefficient in > software -- for > + * example, Toeplitz, specified by RSS, introduced a > significant overhead if > + * performed during per-packge processing. It is therefore > desirable to fall > + * back on traditional reservation table lookups without > affinity where > + * hardware-offloaded checksums aren't available, such as for > traffic over > + * non-RSS interfaces. > + * > * Internet protocols, such as UDP and TCP, register to use > connection groups > * by providing an ipi_hashfields value other than > IPI_HASHFIELDS_NONE; this > * indicates to the connection group code whether a 2-tuple or > 4-tuple is > @@ -72,6 +81,11 @@ __FBSDID("$FreeBSD$"); > * signficantly more expensive than without connection groups, > but that > * steady-state processing can be significantly faster. > * > + * When RSS is used, certain connection group parameters, such > as the number > + * of groups, are provided by the RSS implementation, found in > in_rss.c. > + * Otherwise, in_pcbgroup.c selects possible sensible > parameters > + * corresponding to the degree of parallelism exposed by > netisr. > + * > * Most of the implementation of connection groups is in this > file; however, > * connection group lookup is implemented in in_pcb.c alongside > reservation > * table lookups -- see in_pcblookup_group(). > @@ -126,11 +140,25 @@ in_pcbgroup_init(struct inpcbinfo *pcbin > if (mp_ncpus == 1) > return; > > +#ifdef RSS > /* > - * Use one group per CPU for now. If we decide to do > dynamic > - * rebalancing a la RSS, we'll need to shift left by at > least 1. > + * If we're using RSS, then RSS determines the number of > connection > + * groups to use: one connection group per RSS bucket. > If for some > + * reason RSS isn't able to provide a number of buckets, > disable > + * connection groups entirely. > + * > + * XXXRW: Can this ever happen? > + */ > + numpcbgroups = rss_getnumbuckets(); > + if (numpcbgroups == 0) > + return; > +#else > + /* > + * Otherwise, we'll just use one per CPU for now. If we > decide to > + * do dynamic rebalancing a la RSS, we'll need similar > logic here. > */ > numpcbgroups = mp_ncpus; > +#endif > > pcbinfo->ipi_hashfields = hashfields; > pcbinfo->ipi_pcbgroups = malloc(numpcbgroups * > @@ -146,10 +174,19 @@ in_pcbgroup_init(struct inpcbinfo *pcbin > > /* > * Initialise notional affinity of the pcbgroup > -- for RSS, > - * we want the same notion of affinity as NICs > to be used. > - * Just round robin for the time being. > + * we want the same notion of affinity as NICs > to be used. In > + * the non-RSS case, just round robin for the > time being. > + * > + * XXXRW: The notion of a bucket to CPU mapping > is common at > + * both pcbgroup and RSS layers -- does that > mean that we > + * should migrate it all from RSS to here, and > just leave RSS > + * responsible only for providing hashing and > mapping funtions? > */ > +#ifdef RSS > + pcbgroup->ipg_cpu = rss_getcpu(pgn); > +#else > pcbgroup->ipg_cpu = (pgn % mp_ncpus); > +#endif > } > } > > @@ -179,23 +216,38 @@ in_pcbgroup_destroy(struct inpcbinfo *pc > > /* > * Given a hash of whatever the covered tuple might be, return > a pcbgroup > - * index. > + * index. Where RSS is supported, try to align bucket > selection with RSS CPU > + * affinity strategy. > */ > static __inline u_int > in_pcbgroup_getbucket(struct inpcbinfo *pcbinfo, uint32_t hash) > { > > +#ifdef RSS > + return (rss_getbucket(hash)); > +#else > return (hash % pcbinfo->ipi_npcbgroups); > +#endif > } > > /* > * Map a (hashtype, hash) tuple into a connection group, or > NULL if the hash > - * information is insufficient to identify the pcbgroup. > + * information is insufficient to identify the pcbgroup. This > might occur if > + * a TCP packet turns up with a 2-tuple hash, or if an RSS hash > is present but > + * RSS is not compiled into the kernel. > */ > struct inpcbgroup * > in_pcbgroup_byhash(struct inpcbinfo *pcbinfo, u_int hashtype, > uint32_t hash) > { > > +#ifdef RSS > + if ((pcbinfo->ipi_hashfields == IPI_HASHFIELDS_4TUPLE && > + hashtype == M_HASHTYPE_RSS_TCP_IPV4) || > + (pcbinfo->ipi_hashfields == IPI_HASHFIELDS_2TUPLE && > + hashtype == M_HASHTYPE_RSS_IPV4)) > + return (&pcbinfo->ipi_pcbgroups[ > + in_pcbgroup_getbucket(pcbinfo, hash)]); > +#endif > return (NULL); > } > > @@ -213,13 +265,26 @@ in_pcbgroup_bytuple(struct inpcbinfo *pc > { > uint32_t hash; > > + /* > + * RSS note: we pass foreign addr/port as source, and > local addr/port > + * as destination, as we want to align with what the > hardware is > + * doing. > + */ > switch (pcbinfo->ipi_hashfields) { > case IPI_HASHFIELDS_4TUPLE: > +#ifdef RSS > + hash = rss_hash_ip4_4tuple(faddr, fport, laddr, > lport); > +#else > hash = faddr.s_addr ^ fport; > +#endif > break; > > case IPI_HASHFIELDS_2TUPLE: > +#ifdef RSS > + hash = rss_hash_ip4_2tuple(faddr, laddr); > +#else > hash = faddr.s_addr ^ laddr.s_addr; > +#endif > break; > > default: > > Added: head/sys/netinet/in_rss.c > =========================================================================== > === > --- /dev/null 00:00:00 1970 (empty, because file is newly > added) > +++ head/sys/netinet/in_rss.c Sat Mar 15 00:57:50 2014 > (r263198) > @@ -0,0 +1,505 @@ > +/*- > + * Copyright (c) 2010-2011 Juniper Networks, Inc. > + * All rights reserved. > + * > + * This software was developed by Robert N. M. Watson under > contract > + * to Juniper Networks, Inc. > + * > + * 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. > + */ > + > +#include <sys/cdefs.h> > + > +__FBSDID("$FreeBSD$"); > + > +#include "opt_inet6.h" > +#include "opt_pcbgroup.h" > + > +#ifndef PCBGROUP > +#error "options RSS depends on options PCBGROUP" > +#endif > + > +#include <sys/param.h> > +#include <sys/mbuf.h> > +#include <sys/socket.h> > +#include <sys/priv.h> > +#include <sys/kernel.h> > +#include <sys/smp.h> > +#include <sys/sysctl.h> > + > +#include <net/if.h> > +#include <net/if_var.h> > +#include <net/netisr.h> > + > +#include <netinet/in.h> > +#include <netinet/in_pcb.h> > +#include <netinet/in_rss.h> > +#include <netinet/in_var.h> > +#include <netinet/toeplitz.h> > + > +/*- > + * Operating system parts of receiver-side scaling (RSS), which > allows > + * network cards to direct flows to particular receive queues > based on hashes > + * of header tuples. This implementation aligns RSS buckets > with connection > + * groups at the TCP/IP layer, so each bucket is associated > with exactly one > + * group. As a result, the group lookup structures (and lock) > should have an > + * effective affinity with exactly one CPU. > + * > + * Network device drivers needing to configure RSS will query > this framework > + * for parameters, such as the current RSS key, hashing > policies, number of > + * bits, and indirection table mapping hashes to buckets and > CPUs. They may > + * provide their own supplementary information, such as > queue<->CPU bindings. > + * It is the responsibility of the network device driver to > inject packets > + * into the stack on as close to the right CPU as possible, if > playing by RSS > + * rules. > + * > + * TODO: > + * > + * - Synchronization for rss_key and other future-configurable > parameters. > + * - Event handler drivers can register to pick up RSS > configuration changes. > + * - Should we allow rss_basecpu to be configured? > + * - Randomize key on boot. > + * - IPv6 support. > + * - Statistics on how often there's a misalignment between > hardware > + * placement and pcbgroup expectations. > + */ > + > +SYSCTL_NODE(_net_inet, OID_AUTO, rss, CTLFLAG_RW, 0, > "Receive-side steering"); > + > +/* > + * Toeplitz is the only required hash function in the RSS spec, > so use it by > + * default. > + */ > +static u_int rss_hashalgo = RSS_HASH_TOEPLITZ; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, hashalgo, CTLFLAG_RD, > &rss_hashalgo, 0, > + "RSS hash algorithm"); > +TUNABLE_INT("net.inet.rss.hashalgo", &rss_hashalgo); > + > +/* > + * Size of the indirection table; at most 128 entries per the > RSS spec. We > + * size it to at least 2 times the number of CPUs by default to > allow useful > + * rebalancing. If not set explicitly with a loader tunable, > we tune based > + * on the number of CPUs present. > + * > + * XXXRW: buckets might be better to use for the tunable than > bits. > + */ > +static u_int rss_bits; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, bits, CTLFLAG_RD, > &rss_bits, 0, > + "RSS bits"); > +TUNABLE_INT("net.inet.rss.bits", &rss_bits); > + > +static u_int rss_mask; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, mask, CTLFLAG_RD, > &rss_mask, 0, > + "RSS mask"); > + > +static const u_int rss_maxbits = RSS_MAXBITS; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, maxbits, CTLFLAG_RD, > + __DECONST(int *, &rss_maxbits), 0, "RSS maximum bits"); > + > +/* > + * RSS's own count of the number of CPUs it could be using for > processing. > + * Bounded to 64 by RSS constants. > + */ > +static u_int rss_ncpus; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, ncpus, CTLFLAG_RD, > &rss_ncpus, 0, > + "Number of CPUs available to RSS"); > + > +#define RSS_MAXCPUS (1 << (RSS_MAXBITS - 1)) > +static const u_int rss_maxcpus = RSS_MAXCPUS; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, maxcpus, CTLFLAG_RD, > + __DECONST(int *, &rss_maxcpus), 0, "RSS maximum CPUs that > can be used"); > + > +/* > + * Variable exists just for reporting rss_bits in a > user-friendly way. > + */ > +static u_int rss_buckets; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, buckets, CTLFLAG_RD, > &rss_buckets, 0, > + "RSS buckets"); > + > +/* > + * Base CPU number; devices will add this to all CPU numbers > returned by the > + * RSS indirection table. Currently unmodifable in FreeBSD. > + */ > +static const u_int rss_basecpu; > +SYSCTL_INT(_net_inet_rss, OID_AUTO, basecpu, CTLFLAG_RD, > + __DECONST(int *, &rss_basecpu), 0, "RSS base CPU"); > + > +/* > + * RSS secret key, intended to prevent attacks on > load-balancing. Its > + * effectiveness may be limited by algorithm choice and > available entropy > + * during the boot. > + * > + * XXXRW: And that we don't randomize it yet! > + * > + * XXXRW: This default is actually the default key from Chelsio > T3 cards, as > + * it offers reasonable distribution, unlike all-0 keys which > always > + * generate a hash of 0 (upsettingly). > + */ > +static uint8_t rss_key[RSS_KEYSIZE] = { > + 0x43, 0xa3, 0x8f, 0xb0, 0x41, 0x67, 0x25, 0x3d, > + 0x25, 0x5b, 0x0e, 0xc2, 0x6d, 0x5a, 0x56, 0xda, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > +}; > + > +/* > + * RSS hash->CPU table, which maps hashed packet headers to > particular CPUs. > + * Drivers may supplement this table with a seperate > CPU<->queue table when > + * programming devices. > + */ > +struct rss_table_entry { > + uint8_t rte_cpu; /* CPU affinity of > bucket. */ > +}; > +static struct rss_table_entry rss_table[RSS_TABLE_MAXLEN]; > + > +static void > +rss_init(__unused void *arg) > +{ > + u_int i; > + > + /* > + * Validate tunables, coerce to sensible values. > + */ > + switch (rss_hashalgo) { > + case RSS_HASH_TOEPLITZ: > + case RSS_HASH_NAIVE: > + break; > + > + default: > + printf("%s: invalid RSS hashalgo %u, coercing to > %u", > + __func__, rss_hashalgo, RSS_HASH_TOEPLITZ); > + rss_hashalgo = RSS_HASH_TOEPLITZ; > + } > + > + /* > + * Count available CPUs. > + * > + * XXXRW: Note incorrect assumptions regarding > contiguity of this set > + * elsewhere. > + */ > + rss_ncpus = 0; > + for (i = 0; i <= mp_maxid; i++) { > + if (CPU_ABSENT(i)) > + continue; > + rss_ncpus++; > + } > + if (rss_ncpus > RSS_MAXCPUS) > + rss_ncpus = RSS_MAXCPUS; > + > + /* > + * Tune RSS table entries to be no less than 2x the > number of CPUs > + * -- unless we're running uniprocessor, in which case > there's not > + * much point in having buckets to rearrange for > load-balancing! > + */ > + if (rss_ncpus > 1) { > + if (rss_bits == 0) > + rss_bits = fls(rss_ncpus - 1) + 1; > + > + /* > + * Microsoft limits RSS table entries to 128, so > apply that > + * limit to both auto-detected CPU counts and > user-configured > + * ones. > + */ > + if (rss_bits == 0 || rss_bits > RSS_MAXBITS) { > + printf("%s: RSS bits %u not valid, > coercing to %u", > + __func__, rss_bits, RSS_MAXBITS); > + rss_bits = RSS_MAXBITS; > + } > + > + /* > + * Figure out how many buckets to use; warn if > less than the > + * number of configured CPUs, although this is > not a fatal > + * problem. > + */ > + rss_buckets = (1 << rss_bits); > + if (rss_buckets < rss_ncpus) > + printf("%s: WARNING: rss_buckets (%u) > less than " > + "rss_ncpus (%u)\n", __func__, > rss_buckets, > + rss_ncpus); > + rss_mask = rss_buckets - 1; > + } else { > + rss_bits = 0; > + rss_buckets = 1; > + rss_mask = 0; > + } > + > + /* > + * Set up initial CPU assignments: round-robin by > default. > + * > + * XXXRW: Need a mapping to non-contiguous IDs here. > + */ > + for (i = 0; i < rss_buckets; i++) > + rss_table[i].rte_cpu = i % rss_ncpus; > + > + /* > + * Randomize rrs_key. > + * > + * XXXRW: Not yet. If nothing else, will require an > rss_isbadkey() > + * loop to check for "bad" RSS keys. > + */ > +} > +SYSINIT(rss_init, SI_SUB_SOFTINTR, SI_ORDER_SECOND, rss_init, > NULL); > + > +static uint32_t > +rss_naive_hash(u_int keylen, const uint8_t *key, u_int datalen, > + const uint8_t *data) > +{ > + uint32_t v; > + u_int i; > + > + v = 0; > + for (i = 0; i < keylen; i++) > + v += key[i]; > + for (i = 0; i < datalen; i++) > + v += data[i]; > + return (v); > +} > + > +static uint32_t > +rss_hash(u_int datalen, const uint8_t *data) > +{ > + > + switch (rss_hashalgo) { > + case RSS_HASH_TOEPLITZ: > + return (toeplitz_hash(sizeof(rss_key), rss_key, > datalen, > + data)); > + > + case RSS_HASH_NAIVE: > + return (rss_naive_hash(sizeof(rss_key), rss_key, > datalen, > + data)); > + > + default: > + panic("%s: unsupported/unknown hashalgo %d", > __func__, > + rss_hashalgo); > + } > +} > + > +/* > + * Hash an IPv4 2-tuple. > + */ > +uint32_t > +rss_hash_ip4_2tuple(struct in_addr src, struct in_addr dst) > +{ > + uint8_t data[sizeof(src) + sizeof(dst)]; > + u_int datalen; > + > + datalen = 0; > + bcopy(&src, &data[datalen], sizeof(src)); > + datalen += sizeof(src); > + bcopy(&dst, &data[datalen], sizeof(dst)); > + datalen += sizeof(dst); > + return (rss_hash(datalen, data)); > +} > + > +/* > + * Hash an IPv4 4-tuple. > + */ > +uint32_t > +rss_hash_ip4_4tuple(struct in_addr src, u_short srcport, struct > in_addr dst, > + u_short dstport) > +{ > + uint8_t data[sizeof(src) + sizeof(dst) + sizeof(srcport) > + > + sizeof(dstport)]; > + u_int datalen; > + > + datalen = 0; > + bcopy(&src, &data[datalen], sizeof(src)); > + datalen += sizeof(src); > + bcopy(&dst, &data[datalen], sizeof(dst)); > + datalen += sizeof(dst); > + bcopy(&srcport, &data[datalen], sizeof(srcport)); > + datalen += sizeof(srcport); > + bcopy(&dstport, &data[datalen], sizeof(dstport)); > + datalen += sizeof(dstport); > + return (rss_hash(datalen, data)); > +} > + > +#ifdef INET6 > +/* > + * Hash an IPv6 2-tuple. > + */ > +uint32_t > +rss_hash_ip6_2tuple(struct in6_addr src, struct in6_addr dst) > +{ > + uint8_t data[sizeof(src) + sizeof(dst)]; > + u_int datalen; > + > + datalen = 0; > + bcopy(&src, &data[datalen], sizeof(src)); > + datalen += sizeof(src); > + bcopy(&dst, &data[datalen], sizeof(dst)); > + datalen += sizeof(dst); > + return (rss_hash(datalen, data)); > +} > + > +/* > + * Hash an IPv6 4-tuple. > + */ > +uint32_t > +rss_hash_ip6_4tuple(struct in6_addr src, u_short srcport, > + struct in6_addr dst, u_short dstport) > +{ > + uint8_t data[sizeof(src) + sizeof(dst) + sizeof(srcport) > + > + sizeof(dstport)]; > + u_int datalen; > + > + datalen = 0; > + bcopy(&src, &data[datalen], sizeof(src)); > + datalen += sizeof(src); > + bcopy(&dst, &data[datalen], sizeof(dst)); > + datalen += sizeof(dst); > + bcopy(&srcport, &data[datalen], sizeof(srcport)); > + datalen += sizeof(srcport); > + bcopy(&dstport, &data[datalen], sizeof(dstport)); > + datalen += sizeof(dstport); > + return (rss_hash(datalen, data)); > +} > +#endif /* INET6 */ > + > +/* > + * Query the number of RSS bits in use. > + */ > +u_int > +rss_getbits(void) > +{ > + > + return (rss_bits); > +} > + > +/* > + * Query the RSS bucket associated with an RSS hash. > + */ > +u_int > +rss_getbucket(u_int hash) > +{ > + > + return (hash & rss_mask); > +} > + > +/* > + * Query the RSS CPU associated with an RSS bucket. > + */ > +u_int > +rss_getcpu(u_int bucket) > +{ > + > + return (rss_table[bucket].rte_cpu); > +} > + > +/* > + * netisr CPU affinity lookup routine for use by protocols. > + */ > +struct mbuf * > +rss_m2cpuid(struct mbuf *m, uintptr_t source, u_int *cpuid) > +{ > + > + M_ASSERTPKTHDR(m); > + > + switch (M_HASHTYPE_GET(m)) { > + case M_HASHTYPE_RSS_IPV4: > + case M_HASHTYPE_RSS_TCP_IPV4: > + *cpuid = > rss_getcpu(rss_getbucket(m->m_pkthdr.flowid)); > + return (m); > + > + default: > + *cpuid = NETISR_CPUID_NONE; > + return (m); > + } > +} > + > +/* > + * Query the RSS hash algorithm. > + */ > +u_int > +rss_gethashalgo(void) > +{ > + > + return (rss_hashalgo); > +} > + > +/* > + * Query the current RSS key; likely to be used by device > drivers when > + * configuring hardware RSS. Caller must pass an array of size > RSS_KEYSIZE. > + * > + * XXXRW: Perhaps we should do the accept-a-length-and-truncate > thing? > + */ > +void > +rss_getkey(uint8_t *key) > +{ > + > + bcopy(rss_key, key, sizeof(rss_key)); > +} > + > +/* > + * Query the number of buckets; this may be used by both > network device > + * drivers, which will need to populate hardware shadows of the > software > + * indirection table, and the network stack itself (such as > when deciding how > + * many connection groups to allocate). > + */ > +u_int > +rss_getnumbuckets(void) > +{ > + > + return (rss_buckets); > +} > + > +/* > + * Query the number of CPUs in use by RSS; may be useful to > device drivers > + * trying to figure out how to map a larger number of CPUs into > a smaller > + * number of receive queues. > + */ > +u_int > +rss_getnumcpus(void) > +{ > + > + return (rss_ncpus); > +} > + > +/* > + * XXXRW: Confirm that sysctl -a won't dump this keying > material, don't want > + * it appearing in debugging output unnecessarily. > + */ > +static int > +sysctl_rss_key(SYSCTL_HANDLER_ARGS) > +{ > + uint8_t temp_rss_key[RSS_KEYSIZE]; > + int error; > + > + error = priv_check(req->td, PRIV_NETINET_HASHKEY); > + if (error) > + return (error); > + > + bcopy(rss_key, temp_rss_key, sizeof(temp_rss_key)); > + error = sysctl_handle_opaque(oidp, temp_rss_key, > + sizeof(temp_rss_key), req); > + if (error) > + return (error); > + if (req->newptr != NULL) { > + /* XXXRW: Not yet. */ > + return (EINVAL); > + } > + return (0); > +} > +SYSCTL_PROC(_net_inet_rss, OID_AUTO, key, > + CTLTYPE_OPAQUE | CTLFLAG_RD | CTLFLAG_MPSAFE, NULL, 0, > sysctl_rss_key, > + "", "RSS keying material"); > > Added: head/sys/netinet/in_rss.h > =========================================================================== > === > --- /dev/null 00:00:00 1970 (empty, because file is newly > added) > +++ head/sys/netinet/in_rss.h Sat Mar 15 00:57:50 2014 > (r263198) > @@ -0,0 +1,94 @@ > +/*- > + * Copyright (c) 2010-2011 Juniper Networks, Inc. > + * All rights reserved. > + * > + * This software was developed by Robert N. M. Watson under > contract > + * to Juniper Networks, Inc. > + * > + * 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. > + * > + * $FreeBSD$ > + */ > + > +#ifndef _NETINET_IN_RSS_H_ > +#define _NETINET_IN_RSS_H_ > + > +#include <netinet/in.h> /* in_addr_t */ > + > +/* > + * Supported RSS hash functions. > + */ > +#define RSS_HASH_NAIVE 0x00000001 /* Poor > but fast hash. */ > +#define RSS_HASH_TOEPLITZ 0x00000002 /* > Required by RSS. */ > +#define RSS_HASH_CRC32 0x00000004 /* > Future; some NICs do it. */ > + > +#define RSS_HASH_MASK (RSS_HASH_NAIVE | > RSS_HASH_TOEPLITZ) > + > +/* > + * Instances of struct inpcbinfo declare an RSS hash type > indicating what > + * header fields are covered. > + */ > +#define RSS_HASHFIELDS_NONE 0 > +#define RSS_HASHFIELDS_4TUPLE 1 > +#define RSS_HASHFIELDS_2TUPLE 2 > + > +/* > + * Compile-time limits on the size of the indirection table. > + */ > +#define RSS_MAXBITS 7 > +#define RSS_TABLE_MAXLEN (1 << RSS_MAXBITS) > + > +/* > + * Maximum key size used throughout. It's OK for hardware to > use only the > + * first 16 bytes, which is all that's required for IPv4. > + */ > +#define RSS_KEYSIZE 40 > + > +/* > + * Device driver interfaces to query RSS properties that must > be programmed > + * into hardware. > + */ > +u_int rss_getbits(void); > +u_int rss_getbucket(u_int hash); > +u_int rss_getcpu(u_int bucket); > +void rss_getkey(uint8_t *key); > +u_int rss_gethashalgo(void); > +u_int rss_getnumbuckets(void); > +u_int rss_getnumcpus(void); > + > +/* > + * Network stack interface to generate a hash for a protocol > tuple. > + */ > +uint32_t rss_hash_ip4_4tuple(struct in_addr src, u_short > srcport, > + struct in_addr dst, u_short dstport); > +uint32_t rss_hash_ip4_2tuple(struct in_addr src, struct > in_addr dst); > +uint32_t rss_hash_ip6_4tuple(struct in6_addr src, u_short > srcport, > + struct in6_addr dst, u_short dstport); > +uint32_t rss_hash_ip6_2tuple(struct in6_addr src, > + struct in6_addr dst); > + > +/* > + * Network stack interface to query desired CPU affinity of a > packet. > + */ > +struct mbuf *rss_m2cpuid(struct mbuf *m, uintptr_t source, > u_int *cpuid); > + > +#endif /* !_NETINET_IN_RSS_H_ */ > > Added: head/sys/netinet/toeplitz.c > =========================================================================== > === > --- /dev/null 00:00:00 1970 (empty, because file is newly > added) > +++ head/sys/netinet/toeplitz.c Sat Mar 15 00:57:50 2014 > (r263198) > @@ -0,0 +1,58 @@ > +/*- > + * Copyright (c) 2010 David Malone <dwmalone@FreeBSD.org> > + * All rights reserved. > + * > + * 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. > + */ > + > +#include <sys/cdefs.h> > +__FBSDID("$FreeBSD$"); > + > +#include <sys/types.h> > + > +#include <netinet/in_rss.h> > +#include <netinet/toeplitz.h> > + > +#include <sys/systm.h> > + > +uint32_t > +toeplitz_hash(u_int keylen, const uint8_t *key, u_int datalen, > + const uint8_t *data) > +{ > + uint32_t hash = 0, v; > + u_int i, b; > + > + /* XXXRW: Perhaps an assertion about key length vs. data > length? */ > + > + v = (key[0]<<24) + (key[1]<<16) + (key[2] <<8) + key[3]; > + for (i = 0; i < datalen; i++) { > + for (b = 0; b < 8; b++) { > + if (data[i] & (1<<(7-b))) > + hash ^= v; > + v <<= 1; > + if ((i + 4) < RSS_KEYSIZE && > + (key[i+4] & (1<<(7-b)))) > + v |= 1; > + } > + } > + return (hash); > +} > > Added: head/sys/netinet/toeplitz.h > =========================================================================== > === > --- /dev/null 00:00:00 1970 (empty, because file is newly > added) > > *** DIFF OUTPUT TRUNCATED AT 1000 LINES *** > > > From owner-svn-src-head@FreeBSD.ORG Sat Mar 15 14:58:50 2014 Return-Path: <owner-svn-src-head@FreeBSD.ORG> Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2B10CCC4; Sat, 15 Mar 2014 14:58:50 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 16BAB112; Sat, 15 Mar 2014 14:58:50 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.8/8.14.8) with ESMTP id s2FEwnN7028621; Sat, 15 Mar 2014 14:58:49 GMT (envelope-from jilles@svn.freebsd.org) Received: (from jilles@localhost) by svn.freebsd.org (8.14.8/8.14.8/Submit) id s2FEwnxg028616; Sat, 15 Mar 2014 14:58:49 GMT (envelope-from jilles@svn.freebsd.org) Message-Id: <201403151458.s2FEwnxg028616@svn.freebsd.org> From: Jilles Tjoelker <jilles@FreeBSD.org> Date: Sat, 15 Mar 2014 14:58:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r263206 - in head/bin: kill sh sh/bltin sh/tests/builtins X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current <svn-src-head.freebsd.org> List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-src-head>, <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>; List-Post: <mailto:svn-src-head@freebsd.org> List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help> List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>, <mailto:svn-src-head-request@freebsd.org?subject=subscribe> X-List-Received-Date: Sat, 15 Mar 2014 14:58:50 -0000 Author: jilles Date: Sat Mar 15 14:58:48 2014 New Revision: 263206 URL: http://svnweb.freebsd.org/changeset/base/263206 Log: sh: Allow kill %job on jobs started without job control. When killing a %job started without job control, kill all processes in it. As with process groups and zombies, if any process in the job can be killed or has already terminated, the command is successful. This also fixes occasional failures of the builtins/kill1.0 test. Added: head/bin/sh/tests/builtins/kill2.0 (contents, props changed) Modified: head/bin/kill/kill.c head/bin/sh/bltin/bltin.h head/bin/sh/jobs.c head/bin/sh/tests/builtins/Makefile Modified: head/bin/kill/kill.c ============================================================================== --- head/bin/kill/kill.c Sat Mar 15 10:26:13 2014 (r263205) +++ head/bin/kill/kill.c Sat Mar 15 14:58:48 2014 (r263206) @@ -67,7 +67,7 @@ static void usage(void); int main(int argc, char *argv[]) { - int errors, numsig, pid; + int errors, numsig, pid, ret; char *ep; if (argc < 2) @@ -133,22 +133,17 @@ main(int argc, char *argv[]) for (errors = 0; argc; argc--, argv++) { #ifdef SHELL - if (**argv == '%') { - pid = getjobpgrp(*argv); - /* - * Silently ignore terminated jobs, like the kernel - * silently ignores zombies. - */ - if (pid == 0) - continue; - } else + if (**argv == '%') + ret = killjob(*argv, numsig); + else #endif { pid = strtol(*argv, &ep, 10); if (!**argv || *ep) errx(2, "illegal process id: %s", *argv); + ret = kill(pid, numsig); } - if (kill(pid, numsig) == -1) { + if (ret == -1) { warn("%s", *argv); errors = 1; } Modified: head/bin/sh/bltin/bltin.h ============================================================================== --- head/bin/sh/bltin/bltin.h Sat Mar 15 10:26:13 2014 (r263205) +++ head/bin/sh/bltin/bltin.h Sat Mar 15 14:58:48 2014 (r263206) @@ -74,6 +74,6 @@ pointer stalloc(int); void error(const char *, ...) __printf0like(1, 2); -pid_t getjobpgrp(char *); +int killjob(const char *, int); extern char *commandname; Modified: head/bin/sh/jobs.c ============================================================================== --- head/bin/sh/jobs.c Sat Mar 15 10:26:13 2014 (r263205) +++ head/bin/sh/jobs.c Sat Mar 15 14:58:48 2014 (r263206) @@ -95,9 +95,9 @@ static void restartjob(struct job *); #endif static void freejob(struct job *); static int waitcmdloop(struct job *); -pid_t getjobpgrp(char *); static struct job *getjob_nonotfound(const char *); static struct job *getjob(const char *); +pid_t killjob(const char *, int); static pid_t dowait(int, struct job *); static void checkzombies(void); static void cmdtxt(union node *); @@ -639,15 +639,26 @@ getjob(const char *name) } -pid_t -getjobpgrp(char *name) +int +killjob(const char *name, int sig) { struct job *jp; + int i, ret; jp = getjob(name); if (jp->state == JOBDONE) return 0; - return -jp->ps[0].pid; + if (jp->jobctl) + return kill(-jp->ps[0].pid, sig); + ret = -1; + errno = ESRCH; + for (i = 0; i < jp->nprocs; i++) + if (jp->ps[i].status == -1 || WIFSTOPPED(jp->ps[i].status)) { + if (kill(jp->ps[i].pid, sig) == 0) + ret = 0; + } else + ret = 0; + return ret; } /* Modified: head/bin/sh/tests/builtins/Makefile ============================================================================== --- head/bin/sh/tests/builtins/Makefile Sat Mar 15 10:26:13 2014 (r263205) +++ head/bin/sh/tests/builtins/Makefile Sat Mar 15 14:58:48 2014 (r263206) @@ -86,7 +86,7 @@ FILES+= hash3.0 hash3.0.stdout FILES+= hash4.0 FILES+= jobid1.0 FILES+= jobid2.0 -FILES+= kill1.0 +FILES+= kill1.0 kill2.0 FILES+= lineno.0 lineno.0.stdout FILES+= lineno2.0 FILES+= local1.0 Added: head/bin/sh/tests/builtins/kill2.0 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/bin/sh/tests/builtins/kill2.0 Sat Mar 15 14:58:48 2014 (r263206) @@ -0,0 +1,7 @@ +# $FreeBSD$ + +sleep 1 | sleep 1 & +kill %+ +wait "$!" +r=$? +[ "$r" -gt 128 ] && [ "$(kill -l "$r")" = TERM ]
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1403151202440.36417>