svn commit: r259562 - head/usr.bin/netstat

[John Baldwin]

On Thursday, December 19, 2013 6:57:35 am Gleb Smirnoff wrote:
> On Wed, Dec 18, 2013 at 04:40:52PM -0500, John Baldwin wrote:
> J> On Wednesday, December 18, 2013 3:07:58 pm Alexander V. Chernikov wrote:
> J> > On 18.12.2013 22:45, John-Mark Gurney wrote:
> J> > > Alexander V. Chernikov wrote this message on Wed, Dec 18, 2013 at 18:25 +0000:
> J> > >> Author: melifaro
> J> > >> Date: Wed Dec 18 18:25:27 2013
> J> > >> New Revision: 259562
> J> > >> URL: http://svnweb.freebsd.org/changeset/base/259562
> J> > >>
> J> > >> Log:
> J> > >>   Switch netstat -rn to use standard API for retrieving list of routes
> J> > >>   instead of peeking inside in-kernel radix via kget.
> J> > >>   This permits us to change kernel structures without breaking userland.
> J> > >>   Additionally, this change provide more reliable and faster output.
> J> > >>   
> J> > >>   `Refs` and `Use` fields available in IPv4 by default (and via -W
> J> > >>   for other families) were removed. `Refs` is radix-specific thing
> J> > >>   which is not informative for users. `Use` field value is handy sometimes,
> J> > >>   but a) current API does not support it and b) I'm not sure we will
> J> > >>   support per-rte pcpu counters in near future.
> J> > >>   
> J> > >>   Old method of retrieving data is still supported (either by defining
> J> > >>   NewTree=0 or running netstat with -A). However, Refs/Use fields are
> J> > >>   hidden.
> J> > >>   
> J> > >>   Sponsored by:	Yandex LLC
> J> > >>   MFC after:	4 weeks
> J> > >>   PR:		kern/167204
> J> > > 
> J> > > How will this impact the use of netstat -rn -M vmcore -N kernel ?  Will
> J> > > this change make it not usable, or will you still automatically use
> J> > Well. It will probably break in (maybe, near) future.
> J> 
> J> Please don't gratuitiously break things that /usr/sbin/crashinfo runs.  It's
> J> fine if kvm mode is fragile and requires the binary to be in sync with the
> J> kernel and is only used for crash dumps, but it is very useful to extract
> J> all sorts of info out of a crash dump.
> 
> The problem is that these tools (netstat, and some others) prevent us from
> improving the kernel network stack. We can't make improvements that are
> mergeable to stable/x branch, since the tools would be broken.

No, I explicitly said "it's fine if the kvm mode is fragile and requires the
binary to be in sync and is only used for crash dumps".  That means it's
fine to merge changes as long as you merge the whole thing together.  It also
means you don't have to support all possible internal kernel ABIs in a branch.

> Moreover
> any improvement in head/, requires from developer additional work in netstat
> code, which I must admit isn't a pleasure to work with.

Well, yes, there all sorts of things like, say, documentation that one has to
do beyond the original hacking in order to make a logical change more complete.
Plus, if you think writing netstat stuff in C is bad, try writing the equivalent
in kgdb scripts instead (which is what you'd need to do to provide equivalent
functionality)

> I agree that usage of tools on vmcores is useful. But we all should agree that
> it has very limited use. Only kernel hackers and developers are expected to
> do this.

If users enable crashinfo then they all run this for each crash, so not just
developers.  Having worked in an environment with hundreds of crashes per day,
having some post-processed output to dig through rather than firing up kgdb
every time is quite valuable.

> My suggestion is that all this code is deleted from src/usr.bin/netstat, and
> moved to src/tools, and we relax assertion that src/tools must be compatible
> with any kernel within the branch. So, any person who wants this functionality,
> needs to keep his src/tools in sync with kernel and compile a tool when he
> desires to dump routing table from a vmcore.

That is mostly what I suggested in terms of not having the same ABI guarantees,
but I still want crashinfo to provide a comprehensive set of details to developers
so that users can just mail that file to a developer without having to be walked
through intricate gdb gymnastics.

> Finally, when we remove all the kvm(3) usage from a tool, then we can remove
> the sugid bit from it, which would be a another fine point.

That is 100% irrelevant to crash dumps.  I only ask that the KVM mode needs
to work for crash dumps.  I am more than happy for all "live" access to use
sysctls instead of kvm and to not need the set-gid bit as a result.

-- 
John Baldwin