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.

Konstantin



Reply via email to