> -----Original Message----- > From: Zhang, AlvinX <alvinx.zh...@intel.com> > Sent: Friday, April 16, 2021 09:14 > To: Wang, Haiyue <haiyue.w...@intel.com>; Guo, Jia <jia....@intel.com> > Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zh...@intel.com>; sta...@dpdk.org > Subject: [PATCH] net/igc: fix Rx packet size error > > When DEV_RX_OFFLOAD_KEEP_CRC is enabled, the PMD will minus 4 bytes > of CRC from the size of a packet, but the NIC will strip the CRC > because the CRC strip bit in DVMOLR register is not cleared. This > will cause the size of a packet to be 4 bytes less. > > This patch updates the CRC strip bit according to whether > DEV_RX_OFFLOAD_KEEP_CRC is enabled. > > Fixes: a5aeb2b9e225 ("net/igc: support Rx and Tx") > Cc: sta...@dpdk.org > > Signed-off-by: Alvin Zhang <alvinx.zh...@intel.com> > --- > drivers/net/igc/igc_txrx.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c > index c0a5d5e..68b102d 100644 > --- a/drivers/net/igc/igc_txrx.c > +++ b/drivers/net/igc/igc_txrx.c > @@ -1290,20 +1290,24 @@ int eth_igc_rx_descriptor_status(void *rx_queue, > uint16_t offset) > * This needs to be done after enable. > */ > for (i = 0; i < dev->data->nb_rx_queues; i++) { > + uint32_t dvmolr; > + > rxq = dev->data->rx_queues[i]; > IGC_WRITE_REG(hw, IGC_RDH(rxq->reg_idx), 0); > - IGC_WRITE_REG(hw, IGC_RDT(rxq->reg_idx), > - rxq->nb_rx_desc - 1); > + IGC_WRITE_REG(hw, IGC_RDT(rxq->reg_idx), rxq->nb_rx_desc - 1); > + > + dvmolr = IGC_READ_REG(hw, IGC_DVMOLR(rxq->queue_id)); > > /* strip queue vlan offload */ > - if (rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) { > - uint32_t dvmolr; > - dvmolr = IGC_READ_REG(hw, IGC_DVMOLR(rxq->queue_id)); > + dvmolr = (rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) ? > + (dvmolr | IGC_DVMOLR_STRVLAN) : > + (dvmolr & ~IGC_DVMOLR_STRVLAN);
Just use "if ... else .."to make code readable: if (rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) dvmolr |= IGC_DVMOLR_STRVLAN; else dvmolr &= ~IGC_DVMOLR_STRVLAN; > > - /* If vlan been stripped off, the CRC is meaningless. */ Looks like we need to handle CRC & VLAN_STRIP co-exist issue: If user enables VLAN strip, then keep CRC should be rejected, and vice versa. > - dvmolr |= IGC_DVMOLR_STRVLAN | IGC_DVMOLR_STRCRC; > - IGC_WRITE_REG(hw, IGC_DVMOLR(rxq->reg_idx), dvmolr); > - } > + dvmolr = (offloads & DEV_RX_OFFLOAD_KEEP_CRC) ? > + (dvmolr & ~IGC_DVMOLR_STRCRC) : > + (dvmolr | IGC_DVMOLR_STRCRC); Just use "if ... else .."to make code readable: if (offloads & DEV_RX_OFFLOAD_KEEP_CRC) dvmolr &= ~IGC_DVMOLR_STRCRC; else dvmolr |= IGC_DVMOLR_STRCRC; > + > + IGC_WRITE_REG(hw, IGC_DVMOLR(rxq->reg_idx), dvmolr); > } > > return 0; > -- > 1.8.3.1