Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 May 2019 20:38:57 -0700
From:      Conrad Meyer <cem@freebsd.org>
To:        Marius Strobl <marius@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>,  svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r347221 - head/sys/net
Message-ID:  <CAG6CVpU4esMi5Pq2LOHF14ntfyKkus_mRdbp25UJLHHp21yc_g@mail.gmail.com>
In-Reply-To: <201905070828.x478SZ3t019398@repo.freebsd.org>
References:  <201905070828.x478SZ3t019398@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Marius,

This change seems to break LINT-NOIP tinderbox builds; one reference
to tcp_lro_free() is covered by #if defined(INET) || defined(INET6),
but the one added in iflib_rx_structures_free() is not.

On Tue, May 7, 2019 at 1:28 AM Marius Strobl <marius@freebsd.org> wrote:
>
> Author: marius
> Date: Tue May  7 08:28:35 2019
> New Revision: 347221
> URL: https://svnweb.freebsd.org/changeset/base/347221
> ...
> Modified: head/sys/net/iflib.c
> ==============================================================================
> --- head/sys/net/iflib.c        Tue May  7 08:14:30 2019        (r347220)
> +++ head/sys/net/iflib.c        Tue May  7 08:28:35 2019        (r347221)
> ...

The one below is protected by INET or INET6 define:

> @@ -5627,14 +5668,14 @@ iflib_rx_structures_setup(if_ctx_t ctx)
>  #if defined(INET6) || defined(INET) <<<<
>  fail:
>         /*
> -        * Free RX software descriptors allocated so far, we will only handle
> +        * Free LRO resources allocated so far, we will only handle
>          * the rings that completed, the failing case will have
> -        * cleaned up for itself. 'q' failed, so its the terminus.
> +        * cleaned up for itself.  'q' failed, so its the terminus.
>          */
>         rxq = ctx->ifc_rxqs;
>         for (i = 0; i < q; ++i, rxq++) {
> -               iflib_rx_sds_free(rxq);
> -               rxq->ifr_cq_cidx = 0;
> +               if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO)
> +                       tcp_lro_free(&rxq->ifr_lc);   <<<
>         }
>         return (err);
>  #endif <<<

But the following one is not:

> @@ -5649,9 +5690,12 @@ static void
>  iflib_rx_structures_free(if_ctx_t ctx)
>  {
>         iflib_rxq_t rxq = ctx->ifc_rxqs;
> +       int i;
>
> -       for (int i = 0; i < ctx->ifc_softc_ctx.isc_nrxqsets; i++, rxq++) {
> +       for (i = 0; i < ctx->ifc_softc_ctx.isc_nrxqsets; i++, rxq++) {
>                 iflib_rx_sds_free(rxq);
> +               if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO)
> +                       tcp_lro_free(&rxq->ifr_lc);

^^^^^^^

>         }
>         free(ctx->ifc_rxqs, M_IFLIB);
>         ctx->ifc_rxqs = NULL;

This fails to compile on kernels without INET and INET6 (which is
something we still support, for reasons I cannot fathom) because
netinet/tcp_lro.c is conditional on option inet | inet6 in
sys/conf/files.

Best,
Conrad



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpU4esMi5Pq2LOHF14ntfyKkus_mRdbp25UJLHHp21yc_g>