On 10/13/15 at 06:58P, Bryan Drewery wrote: > On 10/13/2015 5:35 PM, Hiren Panchasara wrote: > > Author: hiren > > Date: Wed Oct 14 00:35:37 2015 > > New Revision: 289276 > > URL: https://svnweb.freebsd.org/changeset/base/289276 > > > > Log: > > There are times when it would be really nice to have a record of the last > > few > > packets and/or state transitions from each TCP socket. That would help > > with > > narrowing down certain problems we see in the field that are hard to > > reproduce > > without understanding the history of how we got into a certain state. This > > change provides just that. > > > > It saves copies of the last N packets in a list in the tcpcb. When the > > tcpcb is > > destroyed, the list is freed. I thought this was likely to be more > > performance-friendly than saving copies of the tcpcb. Plus, with the > > packets, > > you should be able to reverse-engineer what happened to the tcpcb. > > > > To enable the feature, you will need to compile a kernel with the TCPPCAP > > option. Even then, the feature defaults to being deactivated. You can > > activate > > it by setting a positive value for the number of captured packets. You > > can do > > that on either a global basis or on a per-socket basis (via a setsockopt > > call). > > > > There is no way to get the packets out of the kernel other than using > > kmem or > > getting a coredump. I thought that would help some of the legal/privacy > > concerns > > regarding such a feature. However, it should be possible to add a future > > effort > > to export them in PCAP format. > > > > I tested this at low scale, and found that there were no mbuf leaks and > > the peak > > mbuf usage appeared to be unchanged with and without the feature. > > > > The main performance concern I can envision is the number of mbufs that > > would be > > used on systems with a large number of sockets. If you save five packets > > per > > direction per socket and have 3,000 sockets, that will consume at least > > 30,000 > > mbufs just to keep these packets. I tried to reduce the concerns > > associated with > > this by limiting the number of clusters (not mbufs) that could be used > > for this > > feature. Again, in my testing, that appears to work correctly. > > > > Differential Revision: D3100 > > You're supposed to use the full URL here which will auto close the review.
Okay. It did pick up the commit though. What more does it need to know? > > I also replied to the review with style findings just now. > I'll ask Jonathan to look at it. Cheers, Hiren
pgpbPlapt3Owh.pgp
Description: PGP signature