On 06/17/2014 10:57 AM, Ananyev, Konstantin wrote:
> Yes, I understand that it will be initialised to 0 together with whole
> dev->data.
> But then, the condition:
> if (dev->data->min_rx_buf_size > mbp_buf_size)
> would never be true, and min_rx_buf_size would always remain 0?
> I thought you need to initialise it with UINT32_MAX(or UINT16_MAX).
> BTW, not big deal, but I think uint16_t is enough for min_rx_buf_size.
- Oh, right...
We need a check on this :
if (!dev->data->min_rx_buf_size ||
dev->data->min_rx_buf_size > mbp_buf_size)
- Yep, uint16_t should be enough for min_rx_buf_size, but then, we might
want to update other places where bufsizes are compared to uin32_t as well.
- Actually, looking at dev->data structure, there is something
suspicious to me.
From what I understood, secondary processes are not supposed to touch
dev->data, at it is shared between processes.
So I don't understand why rte_eth_dev_allocate() writes
dev->data->port_id, without looking at process type.
Idem, later in rte_eth_dev_init(), where
eth_dev->data->rx_mbuf_alloc_failed is set to 0 (which should already be
set to 0 anyway).
I think a cleanup is required here but it can wait until 1.7 is out.
Plus, I am not sure we should let secondary processes use fdir calls,
change vlan offloads etc...
>
>>>
>>> 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 ?
>
> Sounds good to me.
>
>> I am not sure where this belongs, it could go in rte_ethdev.h.
>
> Probably rte_ether.h?
Ok, I spoke to Ivan and Thomas off-list.
I propose to add the following definition in rte_ether.h :
#define ETHER_MIN_MTU 68
/**< Minimum MTU for IPv4 packets, see RFC 791. */
What do you think of this ?
--
David Marchand
