From owner-freebsd-current@FreeBSD.ORG Wed Aug 20 18:44:31 2014 Return-Path: Delivered-To: freebsd-current@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 A224B90D; Wed, 20 Aug 2014 18:44:31 +0000 (UTC) Received: from mail-pa0-x22a.google.com (mail-pa0-x22a.google.com [IPv6:2607:f8b0:400e:c03::22a]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6E6A938B3; Wed, 20 Aug 2014 18:44:31 +0000 (UTC) Received: by mail-pa0-f42.google.com with SMTP id lf10so12789518pab.15 for ; Wed, 20 Aug 2014 11:44:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=7GC/NyvsdtfbrqnjCHst0OjcrxRKe5VSynfwWwyIYtw=; b=w5A5IBHMUlaA4Rp2nOON2UvyzvPFLy0CWG7H0xhhbkq/gQqVQu996OVrkyH9UvlYcC Xz2NZaXDv2Rpkt6jdsoRlFbe2PRC/WPAl+juf5he0wSIXauvZALj/z90zqzIu2LHjOHi 4WDLtcDYatOnTbe84VtKplpok2DlmB4Z5AJNzKIMcvmccbYa5c6Vh03kk2T6gif9TvYA ziEuJdKhYbWLINQZOrKPkK5sgVnHFn3d3IondpHIAs7WA2rIhsis/DJZdukdJfgRXrIv HOiALxG1lEeVVIb9KblvoOYVkfYo1DmxC8BYLw/GUzGBAzisMxrC9Yq1d/s5dEBRB+Jx LMEw== X-Received: by 10.68.69.46 with SMTP id b14mr54338257pbu.70.1408560270443; Wed, 20 Aug 2014 11:44:30 -0700 (PDT) Received: from [10.192.166.0] (stargate.chelsio.com. [67.207.112.58]) by mx.google.com with ESMTPSA id ou8sm11171547pbc.84.2014.08.20.11.44.29 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Aug 2014 11:44:29 -0700 (PDT) Sender: Navdeep Parhar Message-ID: <53F4EC8A.9090804@FreeBSD.org> Date: Wed, 20 Aug 2014 11:44:26 -0700 From: Navdeep Parhar User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Hans Petter Selasky , freebsd-net@freebsd.org, FreeBSD Current Subject: Re: [RFC] Add support for hardware transmit rate limiting queues [WAS: Add support for changing the flow ID of TCP connections] References: <53BC2E73.6090700@selasky.org> <53BC43AE.3040409@FreeBSD.org> <53BD5385.4090208@selasky.org> <20140709163146.GA21731@ox> <53F44F91.2060006@selasky.org> In-Reply-To: <53F44F91.2060006@selasky.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Aug 2014 18:44:31 -0000 On 08/20/14 00:34, Hans Petter Selasky wrote: > Hi, > > A month has passed since the last e-mail on this topic, and in the > meanwhile some new patches have been created and tested: > > Basically the approach has been changed a little bit: > > - The creation of hardware transmit rings has been made independent of > the TCP stack. This allows firewall applications to forward traffic into > hardware transmit rings aswell, and not only native TCP applications. > This should be one more reason to get the feature into the kernel. > > - A hardware transmit ring basically can have two modes: FIXED-RATE or > AUTOMATIC-RATE. In the fixed rate mode all traffic is sent at a fixed > bytes per second rate. In the automatic mode you can configure a time > after which the TX queue must be empty. The hardware driver uses this to > configure the actual rate. In automatic mode you can also set an upper > and lower transmit rate limit. > > - The MBUF has got a new field in the packet header: "txringid" > > - IOCTLs for TCP v4 and v6 sockets has been updated to allow setting of > the "txringid" field in the mbuf. > > The current patch [see attachment] should be much simpler and less > intrusive than the previous one. > > Any comments ? Here are some thoughts. The first two bullets cover relatively minor issues, the rest are more important. - All of the mbuf pkthdr fields today have the same meaning no matter what the context. It is not clear what txringid's global meaning is. Is it even possible for driver foo to interpret it the same way as driver bar? What if the number of rings are different, or if the ring at the particular index for foo is setup differently than the ring at that same index for bar? You are attempting to influence the driver's txq selection and traditionally the mbuf's flowid has been used for this purpose. Have you considered allowing the user to set the flowid directly? And mark it as such via a new rsstype so the kernel will leave it alone. - uint32_t -> m_flowid_t is plain gratuitous. Now we need to include mbuf.h in more places just to get this definition. What's the advantage of this? style(9) isn't too fond of typedefs either. Also, drivers *do* need to know the width of the flowid. At least lagg(4) looks at the high bits of the flowid (see flowid_shift in lagg). How high it can go depends on the width of the flowid. - Interfaces can come and go, routes can change, and so the relationship between an inpcb and txringid is not stable at all. What happens when the outbound route for an inpcb changes? - The in_ratectlreq structure that you propose is inadequate in its current form. For example, cxgbe's hardware can do rate limiting on a per-ring as well as per-connection basis, and it allows for pps, bandwidth, or min-max limits. I think this is the critical piece that we NIC maintainers must agree on before any code hits the core kernel: how to express a rate-limit policy in a standard way and allow for hardware assistance opportunistically. ipfw(4)'s dummynet is probably interested in this part too, so it's great that Luigi is paying attention to this thread. - The RATECTL ioctls deal with in_ratectlreq so we need to standardize the ratectlreq structure before these ioctls can be considered generic ifnet ioctls. This is the reason cxgbetool (and not ifconfig) has a private ioctl to frob cxgbe's per-queue rate-limiters. I did not want to add ifnet ioctls that in reality were cxgbe only. Ditto for i2c ioctls. Now we have multiple drivers with i2c and melifaro@ is doing the right thing by promoting these private ioctls to a standard ifnet ioctl. Have you considered a private mlxtool as a stop gap measure? To summarize my take on all of this: we need a standard ratectlreq structure, a standard way to associate an inpcb with one, and a standard way to pass on this info to if_transmit. After all this is in place we could even have a dummynet-ish software layer that implements rate limiters when the underlying hardware offers no assistance. Regards, Navdeep