On 01.02.2019 18:13, Jens Freimann wrote: > On Fri, Feb 01, 2019 at 05:27:37PM +0300, Ilya Maximets wrote: >> On 01.02.2019 13:03, Jens Freimann wrote: >>> + if (host_features & (1ULL << VIRTIO_NET_F_MTU)) { >>> + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN + >>> + hw->vtnet_hdr_size; >>> + if (dev->data->dev_conf.rxmode.max_rx_pkt_len <= >>> + hw->max_mtu + ether_hdr_len) >>> + dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME; >>> + } else { >>> + dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME; >>> + } >>> + >> >> As I wrote for v3, hw->max_mtu already calculated taking VIRTIO_NET_F_MTU >> into account. If VIRTIO_NET_F_MTU is not set, hw->max_mtu is equal to >> "VIRTIO_MAX_RX_PKTLEN - ETHER_HDR_LEN - VLAN_TAG_LEN - hw->vtnet_hdr_size". >> i.e. "hw->max_mtu + ether_hdr_len" equal to VIRTIO_MAX_RX_PKTLEN which is >> larger or equal to rxmode.max_rx_pkt_len. >> So, there is no need to check for VIRTIO_NET_F_MTU here. You may just perform >> same check for both cases. > > I should read more carefully :). I think I get what you mean now. > hw->max_mtu already includes ether_hdr_len, so it doesn't need > re-calculation here. And in case of VIRTIO_NET_F_MTU the mtu is > checked during feature negotiation. > So it basically boils down to if > (dev->data->dev_conf.rxmode.max_rx_pkt_len <= w->max_mtu) > dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME; >
'hw->max_mtu' does not include 'ether_hdr_len'. But yes, VIRTIO_NET_F_MTU already checked during negotiation. So, it will be: if (dev->data->dev_conf.rxmode.max_rx_pkt_len <= hw->max_mtu + ether_hdr_len) dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME; >> This doesn't give any performance or so, but will simplify the code. > > Thanks for the review! > > regards, > Jens >