Re: [PATCH v2 0/7] add mtu and flow control handlers

[David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

Hello Konstantin,


On 06/16/2014 07:07 PM, Ananyev, Konstantin wrote:
>
> 1)  [PATCH v2 3/7] ethdev: store min rx buffer size
> @@ -879,6 +879,8 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>   		       const struct rte_eth_rxconf *rx_conf,
>   		       struct rte_mempool *mp)
>   {
> ...
> +	if (!ret) {
> +		if (dev->data->min_rx_buf_size > mbp_buf_size)
> +			dev->data->min_rx_buf_size = mbp_buf_size;
> +	}
> +
> +	return ret;
>
> Where do you set the initial value of min_rx_buf_size?
> Can't find it by some reason.

Hum, actually, dev->data structure is supposed to be set to 0 at init 
time or I missed something.

I would say this happens once for the whole rte_eth_dev_data array in 
rte_eth_dev_data_alloc() in primary process (first call to 
rte_eth_dev_allocate()).


>
> 2)  [PATCH v2 5/7] ethdev: add mtu accessors
> +static int
> +ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> ...
> +	if (!dev->data->scattered_rx &&
> +	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
> +		return -EINVAL;
>
> Reading 82599 spec, 8.2.3.22.13 Max Frame Size - MAXFRS (0x04268; RW):
> " The MFS does not include the 4 bytes of the VLAN header. Packets with VLAN header
> can be as large as MFS + 4. When double VLAN is enabled, the device adds 8 to the
> MFS for any packets."
>
> So, I suppose it should be:
> if (!dev->data->scattered_rx &&
> frame_size + 2 * IXGBE_VLAN_TAG_SIZE > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
>
> Like in ixgbe_dev_rx_init().

Ok, I forgot to take this part you mentioned earlier.
I will send an update later (depending on the points 1) and 3)).


>
> 3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
> Can we add a new define for min allowable MTU (68) as it used in few places.

RTE_IPV4_MIN_MTU then ?
I am not sure where this belongs, it could go in rte_ethdev.h.



-- 
David Marchand