Page MenuHomeFreeBSD

ifnet: Make if_inc_counter() a static inline function
Needs RevisionPublic

Authored by zlei on Oct 4 2022, 6:53 AM.
Referenced Files
Unknown Object (File)
Fri, Jan 6, 1:23 PM
Unknown Object (File)
Fri, Jan 6, 8:48 AM
Unknown Object (File)
Dec 15 2022, 8:04 PM
Unknown Object (File)
Dec 14 2022, 3:07 AM
Subscribers

Details

Reviewers
ae
melifaro
glebius
Group Reviewers
network
Summary

The function is widely used in hardwire interrupt routine and network stack, generally are in fast path. This function has few instructions and no branches on all Tier 1 platforms, also the ifp is within context should have good data locality, probably in the L1 cache, thus can fulfill the cpu pipeline.

Test Plan

No performance regression on Tier 1 platforms.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Oct 4 2022, 6:53 AM

Is there any performance increase?

For 12.x, i386 is Tier 1 supported platform. The counter_u64_add() still has runtime branches.

static inline void
counter_u64_add(counter_u64_t c, int64_t inc)
{

        if ((cpu_feature & CPUID_CX8) == 0) {
                critical_enter();
                *(uint64_t *)zpcpu_get(c) += inc;
                critical_exit();
        } else {
                counter_64_inc_8b(c, inc);
        }
}

I wonder if this could be improved by means like ifunc @glebius .

glebius requested changes to this revision.Oct 4 2022, 7:02 AM

There is a long trend in FreeBSD to make struct ifnet as less visible to drivers as possible. Ideally make it fully opaque. That will allow to change struct ifnet without breaking KBI of drivers. Some years ago I was really close, see https://svnweb.freebsd.org/base/projects/ifnet/. Actually today we have less drivers and this project is worth resurrecting, if I or somebody else have time for it.

The suggested change is step backwards of the desired direction. And I'd guess doesn't have a measurable improvement. High performance NICs do hardware counting anyway.

This revision now requires changes to proceed.Oct 4 2022, 7:02 AM
In D36872#837077, @zlei.huang_gmail.com wrote:

For 12.x, i386 is Tier 1 supported platform. The counter_u64_add() still has runtime branches.

Please, don't bring up i386 in a discussion that started as a performance improvement for high packet rate environments.

Is there any performance increase?

On my small box Intel N5105 it show no noticeable improvement, less than ~1% .

I would expect there are other avoidable slowdowns which prevent realizing the benefit.

the struct can still be opaque provided this is all conditional on KLD_TIED -- if the module is built with the kernel, it can dig into whatever is considered "hidden" from the rest.