Re: iwm(4): decoding of multiple MPDUs in one receive packet
OK
On 2020 Dec 07 (Mon) at 10:28:59 +0100 (+0100), Tobias Heider wrote:
:Hi,
:
:In iwm_rx_pkt() the calculation of "remain" seems to be wrong if
:there are three or more MPDUs in one packet.
:"remain" is initialized with the output buffer size.
:Each time an MPDU is found in the packet remain is reduced
:by the offset of the MPDU in the receive buffer, which is only
:correct for the first MPDU in the packet.
:This causes spurious input errors.
:
:We can fix this by removing the "remain" variable.
:The variable "offset" always points to the current position in the
:receive buffer and the maximum size of the receive buffer is fixed.
:Thus offset can be used for the caculation of maxlen.
:
:ok?
:
:Index: if_iwm.c
:===
:RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
:retrieving revision 1.315
:diff -u -p -r1.315 if_iwm.c
:--- if_iwm.c 11 Oct 2020 07:05:28 - 1.315
:+++ if_iwm.c 7 Dec 2020 09:02:44 -
:@@ -8494,7 +8494,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
: 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,
:@@ -8531,7 +8530,7 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
: break;
:
: case IWM_REPLY_RX_MPDU_CMD: {
:- size_t maxlen = remain - minsz;
:+ size_t maxlen = IWM_RBUF_SIZE - offset - minsz;
: nextoff = offset +
: roundup(len, IWM_FH_RSCSR_FRAME_ALIGN);
: nextpkt = (struct iwm_rx_packet *)
:@@ -8569,11 +8568,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
: iwm_rx_mpdu(sc, m, pkt->data,
: maxlen, ml);
: }
:-
:- if (offset + minsz < remain)
:- remain -= offset;
:- else
:- remain = minsz;
: break;
: }
:
:
--
What color is a chameleon on a mirror?
Re: iwm(4): decoding of multiple MPDUs in one receive packet
On Mon, Dec 07, 2020 at 10:28:59AM +0100, Tobias Heider wrote:
> Hi,
>
> In iwm_rx_pkt() the calculation of "remain" seems to be wrong if
> there are three or more MPDUs in one packet.
> "remain" is initialized with the output buffer size.
> Each time an MPDU is found in the packet remain is reduced
> by the offset of the MPDU in the receive buffer, which is only
> correct for the first MPDU in the packet.
> This causes spurious input errors.
>
> We can fix this by removing the "remain" variable.
> The variable "offset" always points to the current position in the
> receive buffer and the maximum size of the receive buffer is fixed.
> Thus offset can be used for the caculation of maxlen.
>
> ok?
Please credit Christian Ehrhardt who did all the hard work in
tracking this down :)
ok!
>
> Index: if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.315
> diff -u -p -r1.315 if_iwm.c
> --- if_iwm.c 11 Oct 2020 07:05:28 - 1.315
> +++ if_iwm.c 7 Dec 2020 09:02:44 -
> @@ -8494,7 +8494,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
> 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,
> @@ -8531,7 +8530,7 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
> break;
>
> case IWM_REPLY_RX_MPDU_CMD: {
> - size_t maxlen = remain - minsz;
> + size_t maxlen = IWM_RBUF_SIZE - offset - minsz;
> nextoff = offset +
> roundup(len, IWM_FH_RSCSR_FRAME_ALIGN);
> nextpkt = (struct iwm_rx_packet *)
> @@ -8569,11 +8568,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
> iwm_rx_mpdu(sc, m, pkt->data,
> maxlen, ml);
> }
> -
> - if (offset + minsz < remain)
> - remain -= offset;
> - else
> - remain = minsz;
> break;
> }
>
>
>
iwm(4): decoding of multiple MPDUs in one receive packet
Hi,
In iwm_rx_pkt() the calculation of "remain" seems to be wrong if
there are three or more MPDUs in one packet.
"remain" is initialized with the output buffer size.
Each time an MPDU is found in the packet remain is reduced
by the offset of the MPDU in the receive buffer, which is only
correct for the first MPDU in the packet.
This causes spurious input errors.
We can fix this by removing the "remain" variable.
The variable "offset" always points to the current position in the
receive buffer and the maximum size of the receive buffer is fixed.
Thus offset can be used for the caculation of maxlen.
ok?
Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.315
diff -u -p -r1.315 if_iwm.c
--- if_iwm.c11 Oct 2020 07:05:28 - 1.315
+++ if_iwm.c7 Dec 2020 09:02:44 -
@@ -8494,7 +8494,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
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,
@@ -8531,7 +8530,7 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
break;
case IWM_REPLY_RX_MPDU_CMD: {
- size_t maxlen = remain - minsz;
+ size_t maxlen = IWM_RBUF_SIZE - offset - minsz;
nextoff = offset +
roundup(len, IWM_FH_RSCSR_FRAME_ALIGN);
nextpkt = (struct iwm_rx_packet *)
@@ -8569,11 +8568,6 @@ iwm_rx_pkt(struct iwm_softc *sc, struct
iwm_rx_mpdu(sc, m, pkt->data,
maxlen, ml);
}
-
- if (offset + minsz < remain)
- remain -= offset;
- else
- remain = minsz;
break;
}
