I have already taken care of this in v5 of the patch , If possible please review the same .
-----Original Message----- From: Yuanhan Liu [mailto:yuanhan....@linux.intel.com] Sent: Tuesday, September 20, 2016 3:12 AM To: Kavanagh, Mark B <mark.b.kavanagh at intel.com> Cc: Dey, Souvik <sodey at sonusnet.com>; dev at dpdk.org; stephen at networkplumber.org Subject: Re: [PATCH v4]net/virtio: add mtu set in virtio On Wed, Sep 14, 2016 at 12:15:37PM +0000, Kavanagh, Mark B wrote: > > > >>+{ > >>+?????? struct rte_eth_dev_info dev_info; > >>+?????? uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + > >>+VLAN_TAG_LEN; > >>+?????? uint32_t frame_size = mtu + ether_hdr_len; > >>+ > >>+?????? virtio_dev_info_get(dev, &dev_info); > >>+ > >>+?????? if (mtu < dev_info.min_rx_bufsize || frame_size > > >>+dev_info.max_rx_pktlen) { > > > >It's not clear to me whether 'mtu' in this case should be compared > >with ETHER_MIN_MTU, as per other DPDK drivers, or alternatively > >whether 'frame_size' should be compared with dev_info.min_rx_bufsize. > >Any thoughts? > >[Dey, Souvik] I am not sure why virtio min_rx_bufsize is less than > >ETHER_MIN_MTU, i think it will be good to have the compare statement > >as If(frame_size < ETHER_MIN_MTU || frame_size > > >dev_info.max_rx_pktlen) , then error. What do you suggest ? > > Again, this all depends on what 'mtu' means in this context. > > Since you mentioned previously that it relates to the packet (i.e. L3) > length, and not the frame (i.e. L2) length, I would suggest that the > comparison should be: > > if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) > > Yuanhan, any thoughts on this? I think you are right. At least, that's how the ixgbe PMD driver code looks like. --yliu
