Author: marius
Date: Sat Jan 26 21:35:51 2019
New Revision: 343481
URL: https://svnweb.freebsd.org/changeset/base/343481

Log:
  - In _iflib_fl_refill(), don't mark an RX buffer as available in the
    corresponding bitmap before adding an mbuf has actually succeeded.
    Previously, m_gethdr(M_NOWAIT, ...) failing caused a "hole" in the
    RX ring but not in its bitmap. One implication of such a hole was
    that in a subsequent call to _iflib_fl_refill() with the RX buffer
    accounting still indicating another reclaimable buffer, bit_ffc(3)
    nevertheless returned -1 in frag_idx which in turn caused havoc
    when used as an index. Thus, additionally assert that frag_idx is
    0 or greater.
    Another possible consequence of a hole in the RX ring was a NULL-
    dereference when trying to use the unallocated mbuf, for example
    in iflib_rxd_pkt_get().
  
    While at it, make the variable declarations in _iflib_fl_refill()
    conform to style(9) and remove redundant checks already performed
    by bit_ffc{,_at}(3).
  
  - In iflib_queues_alloc(), don't pass redundant M_ZERO to bit_alloc(3).
  
  Reported and tested by: pho

Modified:
  head/sys/net/iflib.c

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c        Sat Jan 26 21:30:26 2019        (r343480)
+++ head/sys/net/iflib.c        Sat Jan 26 21:35:51 2019        (r343481)
@@ -1921,27 +1921,27 @@ _rxq_refill_cb(void *arg, bus_dma_segment_t *segs, int
 static void
 _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int count)
 {
+       struct if_rxd_update iru;
+       struct rxq_refill_cb_arg cb_arg;
        struct mbuf *m;
-       int idx, frag_idx = fl->ifl_fragidx;
-        int pidx = fl->ifl_pidx;
        caddr_t cl, *sd_cl;
        struct mbuf **sd_m;
-       struct if_rxd_update iru;
-       struct rxq_refill_cb_arg cb_arg;
        bus_dmamap_t *sd_map;
-       int n, i = 0;
        bus_addr_t bus_addr, *sd_ba;
-       int err;
+       int err, frag_idx, i, idx, n, pidx;
        qidx_t credits;
 
        sd_m = fl->ifl_sds.ifsd_m;
        sd_map = fl->ifl_sds.ifsd_map;
        sd_cl = fl->ifl_sds.ifsd_cl;
        sd_ba = fl->ifl_sds.ifsd_ba;
+       pidx = fl->ifl_pidx;
        idx = pidx;
+       frag_idx = fl->ifl_fragidx;
        credits = fl->ifl_credits;
 
-       n  = count;
+       i = 0;
+       n = count;
        MPASS(n > 0);
        MPASS(credits + n <= fl->ifl_size);
 
@@ -1963,9 +1963,11 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun
                 *
                 * If the cluster is still set then we know a minimum sized 
packet was received
                 */
-               bit_ffc_at(fl->ifl_rx_bitmap, frag_idx, fl->ifl_size,  
&frag_idx);
-               if ((frag_idx < 0) || (frag_idx >= fl->ifl_size))
-                       bit_ffc(fl->ifl_rx_bitmap, fl->ifl_size, &frag_idx);
+               bit_ffc_at(fl->ifl_rx_bitmap, frag_idx, fl->ifl_size,
+                   &frag_idx);
+               if (frag_idx < 0)
+                       bit_ffc(fl->ifl_rx_bitmap, fl->ifl_size, &frag_idx);
+               MPASS(frag_idx >= 0);
                if ((cl = sd_cl[frag_idx]) == NULL) {
                        if ((cl = m_cljget(NULL, M_NOWAIT, fl->ifl_buf_size)) 
== NULL)
                                break;
@@ -1995,12 +1997,12 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun
                        bus_addr = sd_ba[frag_idx];
                }
 
-               bit_set(fl->ifl_rx_bitmap, frag_idx);
                MPASS(sd_m[frag_idx] == NULL);
                if ((m = m_gethdr(M_NOWAIT, MT_NOINIT)) == NULL) {
                        break;
                }
                sd_m[frag_idx] = m;
+               bit_set(fl->ifl_rx_bitmap, frag_idx);
 #if MEMORY_LOGGING
                fl->ifl_m_enqueued++;
 #endif
@@ -2025,7 +2027,6 @@ _iflib_fl_refill(if_ctx_t ctx, iflib_fl_t fl, int coun
                        fl->ifl_pidx = idx;
                        fl->ifl_credits = credits;
                }
-
        }
 
        if (i) {
@@ -4896,7 +4897,6 @@ iflib_device_deregister(if_ctx_t ctx)
 
                for (j = 0, fl = rxq->ifr_fl; j < rxq->ifr_nfl; j++, fl++)
                        free(fl->ifl_rx_bitmap, M_IFLIB);
-                       
        }
        tqg = qgroup_if_config_tqg;
        if (ctx->ifc_admin_task.gt_uniq != NULL)
@@ -5304,7 +5304,8 @@ iflib_queues_alloc(if_ctx_t ctx)
                }
 
                for (j = 0, fl = rxq->ifr_fl; j < rxq->ifr_nfl; j++, fl++) 
-                       fl->ifl_rx_bitmap = bit_alloc(fl->ifl_size, M_IFLIB, 
M_WAITOK|M_ZERO);
+                       fl->ifl_rx_bitmap = bit_alloc(fl->ifl_size, M_IFLIB,
+                           M_WAITOK);
        }
 
        /* TXQs */
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to