Rao Shoaib wrote:
Why would a lock be needed? The loop over ills need to ensure that an ill doesn't disappear while it is being accessed, but that doesn't require preventing other concerns. Thus I don't understand what you think is hard here.
>
To walk the ill's one needs to hold the global ill_g_lock.
That is merely how current code does it. If we want current mib code to be more efficient in this respect, it can restrict itself to hold ill_g_lock while skipping from one ill to the next, and then call ill_refhold and then drop the lock. Thus it would acquire ill_g_lock once around the loop.
But I'm far from convinced that is necessary if the goal for this project is merely to not make performance any worse.
I did these Q&A session with myself to try to understand the impact even if we do not do a refhold and drop the lock each time around the loop.
Q: When I do a netstat -s today, how many times does the kernel do an rw_enter(&ill_g_lock, RW_READER)?
A: at least 6 times. And it calls allocb (i.e. potentially all the way down into the slab allocator) while holding the lock.
Q: if we want ip_snmp_get_mib2_ip return both per interface statistics and global ones, how many more walks over the ill are needed? (We want this so that netstat -sa can display the per interface statistics for IPv4 and IPv6.)
A: One more. But this is the case whether ip_snmp_get_mib2_ip needs to add per interface ones together to get the global number or not; in order to produce the per-interface ones it needs to walk the ills and while doing that it can also produce the sum.
We do have to do some more work in ip_kstat_update to walk the ills. I haven't looked at the potential performance impact of that in detail. But ip_kstat seems woefully incomplete in that it doesn't export the IPv6 stats.
Yes, you should address this now. Postponing things to RFEs means they are likely to never get done.
>
But what you are suggesting is really an RFE as we are not introducing this behavior. This is what Solaris does today. If we were to make this change than we will have to go and test all cases to make sure there is no degradation.
My point is that you should not introduce the reduction of code quality by replacing every BUMP_MIB(&ip_mib,...) with two BUMP_MIBs. Today the code doesn't have two BUMP_MIBs next to each other for the same stat.
And the result should be uniform across the IPv4 and IPv6 code paths.
>
Can you elaborate on this please. If V4 is to return aggregate stats and V6 does not have to than how can the result be uniform or are you suggesting that we deprecate returning aggregates for V4.
What I was referring to was the above dual BUMP_MIBs. IPv6 also shows where it is necessary to use the global instance (in the output path when there is no ire hence no ill.)
Erik _______________________________________________ networking-discuss mailing list [email protected]
