Hello Konstantin,
Mmm, good points.
I am looking into them.
I will send new patches by tomorrow (hopefully).
On 06/10/2014 07:23 PM, Ananyev, Konstantin wrote:
> Hi David,
>
>> This patch adds two new functions in ethdev api to retrieve current MTU and
>> change MTU of a port.
>> These operations have been implemented for rte_em_pmd, rte_igb_pmd and
>> rte_ixgbe_pmd.
>
>> +
>> +void ixgbe_dev_set_rx_scatter_mode(struct rte_eth_dev *dev)
>> +{
>> + if (dev->rx_pkt_burst != ixgbe_recv_scattered_pkts) {
>> + dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
>> + /* we must ensure that this is done when we leave the
>> + function */
>> + rte_wmb();
>> + }
>> +}
>
> I don't think it is safe to change RX function on the fly (without calling
> dev_stop first).
> If before that ixgbe_recv_pkts_bulk_alloc() was used, then up to 32 mbufs
> could be stored internally in the stage[],
> and ixgbe_recv_scattered_pkts() doesn't have a clue about them.
> Also with ixgbe_recv_pkts_bulk_alloc(), it could be up to rx_free_thresh RX
> descriptors not armed yet with proper buffer addresses,
> while ixgbe_recv_scattered_pkts() doesn't expect that - it re-arms RXD
> straight after it receives a packet from it.
>
> I wonder is the ability to change mtu on the fly (without dev_stop) is really
> needed?
> If so, then we probably can allow ixgbe_dev_set_mtu() to increase MTU only if
> new frame size is less than RX buffer size OR that device already using
> ixgbe_recv_scattered_pkts().
> Something like:
> frame_size <= rx_buf_size || dev->rx_pkt_burst == ixgbe_recv_scattered_pkts
>
>
>> +static int
>> +ixgbe_dev_set_mtu(struct rte_eth_dev *dev, uint16_t *mtu)
>> +{
>> ....
>> +
>> + /* switch to jumbo mode if needed */
>> + if (frame_size > ETHER_MAX_LEN) {
>
> Why ETHER_MAX_LEN?
> If we have rx_buffer big enough, we don't need to split packets even for
> jumbo frames.
> Shouldn't it be something like inside ixgbe_dev_rx_init():
> If (mtu + 2 * IXGBE_VLAN_TAG_SIZE) > rx_buf_size) {...
>
> Same thing for igb.
>
--
David Marchand
