On Wed, May 08, 2019 at 03:01:05PM -0400, Stefan Sperling wrote: > Don't write to mbuf memory before we've actually completed all > sanity checks and iwm_rx_addbuf() has succcessfully put a new > buffer on the ring. > > Same as dragonfly commit 96eaecf93d9f731459a0df8efc72cfad034320bd > "Avoids leaving the mbuf in a weird state when dropping a packet."
This doesn't look right to me. Doesn't iwm_rx_addbuf() replace data->m with a new mbuf? In dragonfly, iwm_mvm_rx_rx_mpdu() already has the mbuf in a parameter called 'm', so it doesn't do quite the same thing. > > ok? > > diff 74c6ffce73f932dd8464cc1def1ee4fcaa3e671f /usr/src > blob - 335033d21091a23511403804f09d1548b109b104 > file + sys/dev/pci/if_iwm.c > --- sys/dev/pci/if_iwm.c > +++ sys/dev/pci/if_iwm.c > @@ -3465,10 +3465,6 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac > rx_pkt_status = le32toh(*(uint32_t *)(pkt->data + > sizeof(*rx_res) + len)); > > - m = data->m; > - m->m_data = pkt->data + sizeof(*rx_res); > - m->m_pkthdr.len = m->m_len = len; > - > if (__predict_false(phy_info->cfg_phy_cnt > 20)) > return; > > @@ -3488,6 +3484,10 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac > > if (iwm_rx_addbuf(sc, IWM_RBUF_SIZE, sc->rxq.cur) != 0) > return; > + > + m = data->m; > + m->m_data = pkt->data + sizeof(*rx_res); > + m->m_pkthdr.len = m->m_len = len; > > chanidx = letoh32(phy_info->channel); > if (chanidx < 0 || chanidx >= nitems(ic->ic_channels)) >
