misc/164130: broken netisr initialization

[Коньков Евгений]

Hi, rwatson.

This is unsafe to do same things in different places:

368:
sysctl_netisr_dispatch_policy(SYSCTL_HANDLER_ARGS)
{
        char tmp[NETISR_DISPATCH_POLICY_MAXSTR];
        u_int dispatch_policy;
        int error;

        netisr_dispatch_policy_to_str(netisr_dispatch_policy, tmp,
            sizeof(tmp));
        error = sysctl_handle_string(oidp, tmp, sizeof(tmp), req);
        if (error == 0 && req->newptr != NULL) {
!                error = netisr_dispatch_policy_from_str(tmp,
!                    &dispatch_policy);
!                if (error == 0 && dispatch_policy == NETISR_DISPATCH_DEFAULT)
!                        error = EINVAL;
!                if (error == 0) {
!                        netisr_dispatch_policy = dispatch_policy;
!                        netisr_dispatch_policy_compat();
                }
        }
        return (error);
}


1197:
        if (TUNABLE_STR_FETCH("net.isr.dispatch", tmp, sizeof(tmp))) {
!                error = netisr_dispatch_policy_from_str(tmp,
!                    &dispatch_policy);
!                if (error == 0 && dispatch_policy == NETISR_DISPATCH_DEFAULT)
!                        error = EINVAL;
!                if (error == 0) {
!                        netisr_dispatch_policy = dispatch_policy;
!                        netisr_dispatch_policy_compat();
                } else
                        printf(
                            "%s: invalid dispatch policy %s, using default\n",
                            __func__, tmp);
        }

Create procedure for such things!

Index: netisr.c
===================================================================
--- netisr.c    (revision 230107)
+++ netisr.c    (working copy)
@@ -158,11 +158,11 @@
  * dispatch policy state.  Now, we provide read-only export via them so that
  * older netstat binaries work.  At some point they can be garbage collected.
  */
-static int     netisr_direct_force;
+static int     netisr_direct_force = 1;
 SYSCTL_INT(_net_isr, OID_AUTO, direct_force, CTLFLAG_RD,
     &netisr_direct_force, 0, "compat: force direct dispatch");

-static int     netisr_direct;
+static int     netisr_direct = 1;
 SYSCTL_INT(_net_isr, OID_AUTO, direct, CTLFLAG_RD, &netisr_direct, 0,
     "compat: enable direct dispatch");

@@ -716,6 +716,8 @@
                 * current CPU ID, which will force an immediate direct
                 * dispatch.  In the queued case, fall back on the SOURCE
                 * policy.
+                * XXX: Make comment to hetisr.h to NETISR_POLICY_CPU
+                * Describing that it fallback to POLICY_SOURCE
                 */
                if (*cpuidp != NETISR_CPUID_NONE)
                        return (m);
@@ -727,6 +729,18 @@
        }

        if (policy == NETISR_POLICY_FLOW) {
+               /*
+                * Do notice about: mbuf.h(202)
+                * #define>M_FLOWID<------>0x00400000 /* deprecated: flowid is valid */
+                *
+                * and also notice some strange code:
+                * if( !a && b ) ...C1
+                * if( a ) ...C2
+                *   it is same as next:
+                * if( a ) ...C1
+                * else if( b ) ...C2
+                * Second one is more clearer, is not? Or here maybe mistake...
+                */
                if (!(m->m_flags & M_FLOWID) && npp->np_m2flow != NULL) {
                        m = npp->np_m2flow(m, source);
                        if (m == NULL)
@@ -995,11 +1009,10 @@
            proto));

        dispatch_policy = netisr_get_dispatch(npp);
-       if (dispatch_policy == NETISR_DISPATCH_DEFERRED)
-               return (netisr_queue_src(proto, source, m));

        /*
-        * If direct dispatch is forced, then unconditionally dispatch
+        * direct dispatch has more priority, so check on it first
+        * If there is direct dispatch, then unconditionally dispatch
         * without a formal CPU selection.  Borrow the current CPU's stats,
         * even if there's no worker on it.  In this case we don't update
         * nws_flags because all netisr processing will be source ordered due
@@ -1010,11 +1023,16 @@
                npwp = &nwsp->nws_work[proto];
                npwp->nw_dispatched++;
                npwp->nw_handled++;
-               netisr_proto[proto].np_handler(m);
+               npp->np_handler(m);
                error = 0;
                goto out_unlock;
        }

+       /* DEFFERED DISPATCH */
+       if (dispatch_policy == NETISR_DISPATCH_DEFERRED)
+               return (netisr_queue_src(proto, source, m));
+
+
        KASSERT(dispatch_policy == NETISR_DISPATCH_HYBRID,
            ("%s: unknown dispatch policy (%u)", __func__, dispatch_policy));

@@ -1023,12 +1041,12 @@
         * dispatch if we're on the right CPU and the netisr worker isn't
         * already running.
         */
-       sched_pin();
-       m = netisr_select_cpuid(&netisr_proto[proto], NETISR_DISPATCH_HYBRID,
+       sched_pin();  //Why you are trying to shced_pin() ....
+       m = netisr_select_cpuid(npp, NETISR_DISPATCH_HYBRID,
            source, m, &cpuid);
        if (m == NULL) {
                error = ENOBUFS;
-               goto out_unpin;
+               goto out_unpin; //...to immediately free it? (revert changes to r222249)
        }
        KASSERT(!CPU_ABSENT(cpuid), ("%s: CPU %u absent", __func__, cpuid));
        if (cpuid != curcpu)
@@ -1061,7 +1079,7 @@
         */
        nwsp->nws_flags |= NWS_DISPATCHING;
        NWS_UNLOCK(nwsp);
-       netisr_proto[proto].np_handler(m);
+       npp->np_handler(m);
        NWS_LOCK(nwsp);
        nwsp->nws_flags &= ~NWS_DISPATCHING;
        npwp->nw_handled++;
@@ -1201,13 +1219,14 @@
                        error = EINVAL;
                if (error == 0) {
                        netisr_dispatch_policy = dispatch_policy;
-                       netisr_dispatch_policy_compat();
                } else
                        printf(
                            "%s: invalid dispatch policy %s, using default\n",
                            __func__, tmp);
        }

+       netisr_dispatch_policy_compat();
+
        netisr_start_swi(curcpu, pcpu_find(curcpu));
 }
 SYSINIT(netisr_init, SI_SUB_SOFTINTR, SI_ORDER_FIRST, netisr_init, NULL);