On Sat, Nov 09, 2019 at 03:20:10PM +0100, Florian Obser wrote:
> My 7260 doesn't like it. Same problem as last time.
> These counters go through the roof and network stalls.
> Also when packets are flowing throughput varies a lot.
>         30131 input block ack window slides
>         30171 input frames above block ack window end
>         827766 expected input block ack frames never arrived

I have similar issues with a 8265:
        65731 input frames above block ack window end
        65711 input block ack window slides
        150884 expected input block ack frames never arrived

It works but feels slower than before.
 
> On Fri, Nov 08, 2019 at 07:23:36PM +0200, Stefan Sperling wrote:
> > On Fri, Nov 08, 2019 at 07:15:18PM +0200, Stefan Sperling wrote:
> > > On Fri, Nov 08, 2019 at 07:13:17PM +0200, Stefan Sperling wrote:
> > > > Previous versions of this diff included monitor mode support. I have 
> > > > stripped
> > > > this out for now and I am looking for stability tests in client mode 
> > > > only.
> > > > If this diff passes through testing this time around, adding monitor 
> > > > mode
> > > > on top will be easy.
> > > 
> > > Sorry, I made a mistake when splitting the diff. The diff I sent doesn't 
> > > build.
> > > I will post a new one shortly.
> > 
> > Fixed diff.
> > 
> > diff aa91687117b06dc9f357920a56ef1a22c0de6a7e 
> > 16cec9db4484c7e9e6fa4838707a4c0c7ba2f314
> > blob - f7449f560623517dfa1b3fbe6d55b10a5494e26c
> > blob + 0d5f766801f75e839476ebc2393568ed31494e62
> > --- sys/dev/pci/if_iwm.c
> > +++ sys/dev/pci/if_iwm.c
> > @@ -366,8 +366,10 @@ int    iwm_get_signal_strength(struct iwm_softc *, 
> > struct
> >  void       iwm_rx_rx_phy_cmd(struct iwm_softc *, struct iwm_rx_packet *,
> >         struct iwm_rx_data *);
> >  int        iwm_get_noise(const struct iwm_statistics_rx_non_phy *);
> > -void       iwm_rx_rx_mpdu(struct iwm_softc *, struct iwm_rx_packet *,
> > -       struct iwm_rx_data *, struct mbuf_list *);
> > +int        iwm_rx_frame(struct iwm_softc *, struct mbuf *, uint32_t,
> > +       struct mbuf_list *);
> > +int        iwm_ccmp_decap(struct iwm_softc *, struct mbuf *,
> > +       struct ieee80211_node *);
> >  void       iwm_enable_ht_cck_fallback(struct iwm_softc *, struct iwm_node 
> > *);
> >  void       iwm_rx_tx_cmd_single(struct iwm_softc *, struct iwm_rx_packet *,
> >         struct iwm_node *);
> > @@ -472,6 +474,11 @@ const char *iwm_desc_lookup(uint32_t);
> >  void       iwm_nic_error(struct iwm_softc *);
> >  void       iwm_nic_umac_error(struct iwm_softc *);
> >  #endif
> > +void       iwm_rx_mpdu(struct iwm_softc *, struct mbuf *, size_t,
> > +       struct mbuf_list *);
> > +int        iwm_rx_pkt_valid(struct iwm_rx_packet *);
> > +void       iwm_rx_pkt(struct iwm_softc *, struct iwm_rx_data *,
> > +       struct mbuf_list *);
> >  void       iwm_notif_intr(struct iwm_softc *);
> >  int        iwm_intr(void *);
> >  int        iwm_match(struct device *, void *, void *);
> > @@ -1807,7 +1814,6 @@ iwm_nic_rx_init(struct iwm_softc *sc)
> >         IWM_FH_RCSR_RX_CONFIG_CHNL_EN_ENABLE_VAL            |
> >         IWM_FH_RCSR_CHNL0_RX_IGNORE_RXF_EMPTY               |  /* HW bug */
> >         IWM_FH_RCSR_CHNL0_RX_CONFIG_IRQ_DEST_INT_HOST_VAL   |
> > -       IWM_FH_RCSR_CHNL0_RX_CONFIG_SINGLE_FRAME_MSK        |
> >         (IWM_RX_RB_TIMEOUT << IWM_FH_RCSR_RX_CONFIG_REG_IRQ_RBTH_POS) |
> >         IWM_FH_RCSR_RX_CONFIG_REG_VAL_RB_SIZE_4K            |
> >         IWM_RX_QUEUE_SIZE_LOG << IWM_FH_RCSR_RX_CONFIG_RBDCB_SIZE_POS);
> > @@ -3539,56 +3545,24 @@ iwm_get_noise(const struct 
> > iwm_statistics_rx_non_phy *
> >     return (nbant == 0) ? -127 : (total / nbant) - 107;
> >  }
> >  
> > -void
> > -iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_packet *pkt,
> > -    struct iwm_rx_data *data, struct mbuf_list *ml)
> > +int
> > +iwm_rx_frame(struct iwm_softc *sc, struct mbuf *m, uint32_t rx_pkt_status,
> > +    struct mbuf_list *ml)
> >  {
> >     struct ieee80211com *ic = &sc->sc_ic;
> >     struct ieee80211_frame *wh;
> >     struct ieee80211_node *ni;
> >     struct ieee80211_rxinfo rxi;
> >     struct ieee80211_channel *bss_chan;
> > -   struct mbuf *m;
> >     struct iwm_rx_phy_info *phy_info;
> > -   struct iwm_rx_mpdu_res_start *rx_res;
> >     int device_timestamp;
> > -   uint32_t len;
> > -   uint32_t rx_pkt_status;
> >     int rssi, chanidx;
> >     uint8_t saved_bssid[IEEE80211_ADDR_LEN] = { 0 };
> >  
> > -   bus_dmamap_sync(sc->sc_dmat, data->map, 0, IWM_RBUF_SIZE,
> > -       BUS_DMASYNC_POSTREAD);
> > -
> >     phy_info = &sc->sc_last_phy_info;
> > -   rx_res = (struct iwm_rx_mpdu_res_start *)pkt->data;
> > -   wh = (struct ieee80211_frame *)(pkt->data + sizeof(*rx_res));
> > -   len = le16toh(rx_res->byte_count);
> > -   if (len < IEEE80211_MIN_LEN) {
> > -           ic->ic_stats.is_rx_tooshort++;
> > -           IC2IFP(ic)->if_ierrors++;
> > -           return;
> > -   }
> > -   if (len > IWM_RBUF_SIZE - sizeof(*rx_res)) {
> > -           IC2IFP(ic)->if_ierrors++;
> > -           return;
> > -   }
> > -   rx_pkt_status = le32toh(*(uint32_t *)(pkt->data +
> > -       sizeof(*rx_res) + len));
> > -
> >     if (__predict_false(phy_info->cfg_phy_cnt > 20))
> > -           return;
> > +           return EINVAL;
> >  
> > -   if (!(rx_pkt_status & IWM_RX_MPDU_RES_STATUS_CRC_OK) ||
> > -       !(rx_pkt_status & IWM_RX_MPDU_RES_STATUS_OVERRUN_OK))
> > -           return; /* drop */
> > -
> > -   m = data->m;
> > -   if (iwm_rx_addbuf(sc, IWM_RBUF_SIZE, sc->rxq.cur) != 0)
> > -           return;
> > -   m->m_data = pkt->data + sizeof(*rx_res);
> > -   m->m_pkthdr.len = m->m_len = len;
> > -
> >     device_timestamp = le32toh(phy_info->system_timestamp);
> >  
> >     rssi = iwm_get_signal_strength(sc, phy_info);
> > @@ -3599,6 +3573,7 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac
> >     if (chanidx < 0 || chanidx >= nitems(ic->ic_channels))  
> >             chanidx = ieee80211_chan2ieee(ic, ic->ic_ibss_chan);
> >  
> > +   wh = mtod(m, struct ieee80211_frame *);
> >     ni = ieee80211_find_rxnode(ic, wh);
> >     if (ni == ic->ic_bss) {
> >             /* 
> > @@ -3672,6 +3647,8 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac
> >     if (ni == ic->ic_bss && IEEE80211_ADDR_EQ(saved_bssid, ni->ni_macaddr))
> >             ni->ni_chan = bss_chan;
> >     ieee80211_release_node(ic, ni);
> > +
> > +   return 0;
> >  }
> >  
> >  void
> > @@ -7525,38 +7502,103 @@ do {                                               
> >                         \
> >  #define ADVANCE_RXQ(sc) (sc->rxq.cur = (sc->rxq.cur + 1) % 
> > IWM_RX_RING_COUNT);
> >  
> >  void
> > -iwm_notif_intr(struct iwm_softc *sc)
> > +iwm_rx_mpdu(struct iwm_softc *sc, struct mbuf *m, size_t maxlen,
> > +    struct mbuf_list *ml)
> >  {
> > -   struct mbuf_list ml = MBUF_LIST_INITIALIZER();
> > -   uint16_t hw;
> > +   struct ieee80211com *ic = &sc->sc_ic;
> > +   struct ifnet *ifp = IC2IFP(ic);
> > +   struct iwm_rx_packet *pkt;
> > +   struct iwm_rx_mpdu_res_start *rx_res;
> > +   uint16_t len;
> > +   uint32_t rx_pkt_status;
> > +   int rxfail;
> >  
> > -   bus_dmamap_sync(sc->sc_dmat, sc->rxq.stat_dma.map,
> > -       0, sc->rxq.stat_dma.size, BUS_DMASYNC_POSTREAD);
> > +   pkt = mtod(m, struct iwm_rx_packet *);
> > +   rx_res = (struct iwm_rx_mpdu_res_start *)pkt->data;
> > +   len = le16toh(rx_res->byte_count);
> > +   if (len < IEEE80211_MIN_LEN) {
> > +           ic->ic_stats.is_rx_tooshort++;
> > +           IC2IFP(ic)->if_ierrors++;
> > +           m_freem(m);
> > +           return;
> > +   }
> > +   if (len + sizeof(*rx_res) + sizeof(rx_pkt_status) > maxlen ||
> > +       len > IEEE80211_MAX_LEN) {
> > +           IC2IFP(ic)->if_ierrors++;
> > +           m_freem(m);
> > +           return;
> > +   }
> >  
> > -   hw = le16toh(sc->rxq.stat->closed_rb_num) & 0xfff;
> > -   hw &= (IWM_RX_RING_COUNT - 1);
> > -   while (sc->rxq.cur != hw) {
> > -           struct iwm_rx_data *data = &sc->rxq.data[sc->rxq.cur];
> > -           struct iwm_rx_packet *pkt;
> > -           int qid, idx, code, handled = 1;
> > +   memcpy(&rx_pkt_status, pkt->data + sizeof(*rx_res) + len,
> > +       sizeof(rx_pkt_status));
> > +   rx_pkt_status = le32toh(rx_pkt_status);
> > +   rxfail = ((rx_pkt_status & IWM_RX_MPDU_RES_STATUS_CRC_OK) == 0 ||
> > +       (rx_pkt_status & IWM_RX_MPDU_RES_STATUS_OVERRUN_OK) == 0);
> > +   if (rxfail) {
> > +           ifp->if_ierrors++;
> > +           m_freem(m);
> > +           return;
> > +   }
> >  
> > -           bus_dmamap_sync(sc->sc_dmat, data->map, 0, sizeof(*pkt),
> > -               BUS_DMASYNC_POSTREAD);
> > -           pkt = mtod(data->m, struct iwm_rx_packet *);
> > +   /* Extract the 802.11 frame. */
> > +   m->m_data = (caddr_t)pkt->data + sizeof(*rx_res);
> > +   m->m_pkthdr.len = m->m_len = len;
> > +   if (iwm_rx_frame(sc, m, rx_pkt_status, ml) != 0) {
> > +           ifp->if_ierrors++;
> > +           m_freem(m);
> > +   }
> > +}
> >  
> > +int
> > +iwm_rx_pkt_valid(struct iwm_rx_packet *pkt)
> > +{
> > +   int qid, idx, code;
> > +
> > +   qid = pkt->hdr.qid & ~0x80;
> > +   idx = pkt->hdr.idx;
> > +   code = IWM_WIDE_ID(pkt->hdr.flags, pkt->hdr.code);
> > +
> > +   return (!(qid == 0 && idx == 0 && code == 0) &&
> > +       pkt->len_n_flags != htole32(IWM_FH_RSCSR_FRAME_INVALID));
> > +}
> > +
> > +void
> > +iwm_rx_pkt(struct iwm_softc *sc, struct iwm_rx_data *data, struct 
> > mbuf_list *ml)
> > +{
> > +   struct ifnet *ifp = IC2IFP(&sc->sc_ic);
> > +   struct iwm_rx_packet *pkt, *nextpkt;
> > +   uint32_t offset = 0, nextoff = 0, nmpdu = 0, len;
> > +   struct mbuf *m0, *m;
> > +   const size_t minsz = sizeof(pkt->len_n_flags) + sizeof(pkt->hdr);
> > +   size_t remain = IWM_RBUF_SIZE;
> > +   int qid, idx, code, handled = 1;
> > +
> > +   bus_dmamap_sync(sc->sc_dmat, data->map, 0, IWM_RBUF_SIZE,
> > +       BUS_DMASYNC_POSTREAD);
> > +
> > +   m0 = data->m;
> > +   while (m0 && offset + minsz < IWM_RBUF_SIZE) {
> > +           pkt = (struct iwm_rx_packet *)(m0->m_data + offset);
> >             qid = pkt->hdr.qid;
> >             idx = pkt->hdr.idx;
> >  
> >             code = IWM_WIDE_ID(pkt->hdr.flags, pkt->hdr.code);
> >  
> > -           /*
> > -            * randomly get these from the firmware, no idea why.
> > -            * they at least seem harmless, so just ignore them for now
> > -            */
> > -           if (__predict_false((pkt->hdr.code == 0 && (qid & ~0x80) == 0
> > -               && idx == 0) || pkt->len_n_flags == htole32(0x55550000))) {
> > -                   ADVANCE_RXQ(sc);
> > -                   continue;
> > +           if (!iwm_rx_pkt_valid(pkt))
> > +                   break;
> > +
> > +           len = sizeof(pkt->len_n_flags) + iwm_rx_packet_len(pkt);
> > +           if (len < sizeof(pkt->hdr) ||
> > +               len > (IWM_RBUF_SIZE - offset - minsz))
> > +                   break;
> > +
> > +           if (code == IWM_REPLY_RX_MPDU_CMD && ++nmpdu == 1) {
> > +                   /* Take mbuf m0 off the RX ring. */
> > +                   if (iwm_rx_addbuf(sc, IWM_RBUF_SIZE, sc->rxq.cur)) {
> > +                           ifp->if_ierrors++;
> > +                           break;
> > +                   }
> > +                   KASSERT(data->m != m0);
> >             }
> >  
> >             switch (code) {
> > @@ -7564,10 +7606,40 @@ iwm_notif_intr(struct iwm_softc *sc)
> >                     iwm_rx_rx_phy_cmd(sc, pkt, data);
> >                     break;
> >  
> > -           case IWM_REPLY_RX_MPDU_CMD:
> > -                   iwm_rx_rx_mpdu(sc, pkt, data, &ml);
> > -                   break;
> > +           case IWM_REPLY_RX_MPDU_CMD: {
> > +                   nextoff = offset +
> > +                       roundup(len, IWM_FH_RSCSR_FRAME_ALIGN);
> > +                   nextpkt = (struct iwm_rx_packet *)
> > +                       (m0->m_data + nextoff);
> > +                   if (nextoff + minsz >= IWM_RBUF_SIZE ||
> > +                       !iwm_rx_pkt_valid(nextpkt)) {
> > +                           /* No need to copy last frame in buffer. */
> > +                           if (offset > 0)
> > +                                   m_adj(m0, offset);
> > +                           iwm_rx_mpdu(sc, m0, remain - minsz, ml);
> > +                           m0 = NULL; /* stack owns m0 now; abort loop */
> > +                   } else {
> > +                           /*
> > +                            * Create an mbuf which points to the current
> > +                            * packet. Always copy from offset zero to
> > +                            * preserve m_pkthdr.
> > +                            */
> > +                           m = m_copym(m0, 0, M_COPYALL, M_DONTWAIT);
> > +                           if (m == NULL) {
> > +                                   ifp->if_ierrors++;
> > +                                   break;
> > +                           }
> > +                           m_adj(m, offset);
> > +                           iwm_rx_mpdu(sc, m, remain - minsz, ml);
> > +                   }
> >  
> > +                   if (offset + minsz < remain)
> > +                           remain -= offset;
> > +                   else
> > +                           remain = minsz;
> > +                   break;
> > +           }
> > +
> >             case IWM_TX_CMD:
> >                     iwm_rx_tx_cmd(sc, pkt, data);
> >                     break;
> > @@ -7818,6 +7890,27 @@ iwm_notif_intr(struct iwm_softc *sc)
> >                     iwm_cmd_done(sc, qid, idx, code);
> >             }
> >  
> > +           offset += roundup(len, IWM_FH_RSCSR_FRAME_ALIGN);
> > +   }
> > +
> > +   if (m0 && m0 != data->m)
> > +           m_freem(m0);
> > +}
> > +
> > +void
> > +iwm_notif_intr(struct iwm_softc *sc)
> > +{
> > +   struct mbuf_list ml = MBUF_LIST_INITIALIZER();
> > +   uint16_t hw;
> > +
> > +   bus_dmamap_sync(sc->sc_dmat, sc->rxq.stat_dma.map,
> > +       0, sc->rxq.stat_dma.size, BUS_DMASYNC_POSTREAD);
> > +
> > +   hw = le16toh(sc->rxq.stat->closed_rb_num) & 0xfff;
> > +   hw &= (IWM_RX_RING_COUNT - 1);
> > +   while (sc->rxq.cur != hw) {
> > +           struct iwm_rx_data *data = &sc->rxq.data[sc->rxq.cur];
> > +           iwm_rx_pkt(sc, data, &ml);
> >             ADVANCE_RXQ(sc);
> >     }
> >     if_input(&sc->sc_ic.ic_if, &ml);
> > 
> 
> -- 
> I'm not entirely sure you are real.
> 

-- 
:wq Claudio

Reply via email to