From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Marchand Subject: Re: [PATCH 3/5] ethdev: add mtu accessors Date: Thu, 12 Jun 2014 16:20:08 +0200 Message-ID: <5399B718.4050102@6wind.com> References: <1401103892-17225-1-git-send-email-david.marchand@6wind.com> <1401103892-17225-4-git-send-email-david.marchand@6wind.com> <2601191342CEEE43887BDE71AB9772580EFB5EEE@IRSMSX105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit To: "Ananyev, Konstantin" , "dev-VfR2kkLFssw@public.gmane.org" Return-path: In-Reply-To: <2601191342CEEE43887BDE71AB9772580EFB5EEE-kPTMFJFq+rEu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" 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