svn commit: r304313 - head/sys/net

[Adrian Chadd]

On 17 August 2016 at 13:21, Andrey V. Elsukov <ae at freebsd.org> wrote:
> Author: ae
> Date: Wed Aug 17 20:21:33 2016
> New Revision: 304313
> URL: https://svnweb.freebsd.org/changeset/base/304313
>
> Log:
>   Teach netisr_get_cpuid() to limit a given value to supported by netisr.
>   Use netisr_get_cpuid() in netisr_select_cpuid() to limit cpuid value
>   returned by protocol to be sure that it is not greather than nws_count.
>
>   PR:           211836
>   Reviewed by:  adrian
>   MFC after:    3 days
>
> Modified:
>   head/sys/net/if_epair.c
>   head/sys/net/netisr.c
>
> Modified: head/sys/net/if_epair.c
> ==============================================================================
> --- head/sys/net/if_epair.c     Wed Aug 17 19:43:45 2016        (r304312)
> +++ head/sys/net/if_epair.c     Wed Aug 17 20:21:33 2016        (r304313)
> @@ -807,9 +807,9 @@ epair_clone_create(struct if_clone *ifc,
>          * cache locality but we can at least allow parallelism.
>          */
>         sca->cpuid =
> -           netisr_get_cpuid(sca->ifp->if_index % netisr_get_cpucount());
> +           netisr_get_cpuid(sca->ifp->if_index);
>         scb->cpuid =
> -           netisr_get_cpuid(scb->ifp->if_index % netisr_get_cpucount());
> +           netisr_get_cpuid(scb->ifp->if_index);
>
>         /* Initialise pseudo media types. */
>         ifmedia_init(&sca->media, 0, epair_media_change, epair_media_status);
>
> Modified: head/sys/net/netisr.c
> ==============================================================================
> --- head/sys/net/netisr.c       Wed Aug 17 19:43:45 2016        (r304312)
> +++ head/sys/net/netisr.c       Wed Aug 17 20:21:33 2016        (r304313)
> @@ -272,10 +272,7 @@ u_int
>  netisr_get_cpuid(u_int cpunumber)
>  {
>
> -       KASSERT(cpunumber < nws_count, ("%s: %u > %u", __func__, cpunumber,
> -           nws_count));
> -
> -       return (nws_array[cpunumber]);
> +       return (nws_array[cpunumber % nws_count]);
>  }
>
>  /*
> @@ -810,10 +807,12 @@ netisr_select_cpuid(struct netisr_proto
>                  * dispatch.  In the queued case, fall back on the SOURCE
>                  * policy.
>                  */
> -               if (*cpuidp != NETISR_CPUID_NONE)
> +               if (*cpuidp != NETISR_CPUID_NONE) {
> +                       *cpuidp = netisr_get_cpuid(*cpuidp);
>                         return (m);
> +               }
>                 if (dispatch_policy == NETISR_DISPATCH_HYBRID) {
> -                       *cpuidp = curcpu;
> +                       *cpuidp = netisr_get_cpuid(curcpu);
>                         return (m);
>                 }
>                 policy = NETISR_POLICY_SOURCE;
>

I wonder if the right-er thing to do here is to allow the cpuid to be
whatever it needs to be, but limit the cpuid lookups when it resolves
to a netisr array.

that'd mean the hybrid model would still return the current CPU up to
the max CPU id, but anything trying to queue into a netisr would not
overflow the netisr queue count.

Thoughts?


-a