Messages in this thread | | | Date | Thu, 25 May 2017 10:02:08 +0300 | From | Mika Westerberg <> | Subject | Re: [PATCH 14/24] thunderbolt: Let the connection manager handle all notifications |
| |
On Wed, May 24, 2017 at 04:00:49PM +0200, Lukas Wunner wrote: > On Thu, May 18, 2017 at 05:39:04PM +0300, Mika Westerberg wrote: > > @@ -320,18 +330,42 @@ static void tb_ctl_rx_callback(struct tb_ring *ring, struct ring_frame *frame, > > } > > > > frame->size -= 4; /* remove checksum */ > > - if (*(__be32 *) (pkg->buffer + frame->size) > > - != tb_crc(pkg->buffer, frame->size)) { > > - tb_ctl_err(pkg->ctl, > > - "RX: checksum mismatch, dropping packet\n"); > > - goto rx; > > - } > > + crc32 = tb_crc(pkg->buffer, frame->size); > > be32_to_cpu_array(pkg->buffer, pkg->buffer, frame->size / 4); > > > > - if (frame->eof == TB_CFG_PKG_EVENT) { > > - tb_ctl_handle_plug_event(pkg->ctl, pkg); > > + switch (frame->eof) { > > + case TB_CFG_PKG_READ: > > + case TB_CFG_PKG_WRITE: > > + case TB_CFG_PKG_ERROR: > > + case TB_CFG_PKG_OVERRIDE: > > + case TB_CFG_PKG_RESET: > > + if (*(__be32 *)(pkg->buffer + frame->size) != crc32) { > > + tb_ctl_err(pkg->ctl, > > + "RX: checksum mismatch, dropping packet\n"); > > + goto rx; > > + } > > Any harm keeping the crc32 check above the switch/case statement? > (And thus also execute it for unknown packets?)
Not all packets carry crc32, only these configuration space packets.
> > + case TB_CFG_PKG_EVENT: > > + if (*(__be32 *)(pkg->buffer + frame->size) != crc32) { > > + tb_ctl_err(pkg->ctl, > > + "RX: checksum mismatch, dropping packet\n"); > > + goto rx; > > + } > > + tb_ctl_handle_event(pkg->ctl, frame->eof, pkg, frame->size); > > + goto rx; > > + > > + default: > > + tb_ctl_dbg(pkg->ctl, "RX: unknown package %#x, dropping\n", > > The packet / package terminology is a bit inconsistent here. > Andreas originally used package. What's the term used by the TB spec?
IIRC it uses packet.
| |