Re: [dpdk-dev] [PATCH v2] ether: use a default for max Rx frame size in configure()

["Yang, Qiming" <qiming.yang@intel.com>]

Hi, Andriy

I don't think this is a bug. Return errors when configure an invalid max_rx_pkt_len is suitable for this generic API.
rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_queue, uint16_t nb_tx_queue, const struct rte_eth_conf *eth_conf) 
is used to configure an Ethernet device as eth_conf demand. 
It's not suitable to give a default value in this function.

BRs,
Qiming

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andriy
> Berestovskyy
> Sent: Friday, March 24, 2017 7:52 PM
> To: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] ether: use a default for max Rx frame size in
> configure()
> 
> At the moment behaves inconsistent:
>  - for normal frames: out of range max_rx_pkt_len uses a default
>  - for jumbo frames: out of range max_rx_pkt_len gives an error
> 
> This patch fixes this inconsistency by using a default value for
> max_rx_pkt_len both for normal and jumbo frames.
> 
> Signed-off-by: Andriy Berestovskyy
> <Andriy.Berestovskyy@caviumnetworks.com>
> ---
> 
> Notes:
>     v2 changes:
>      - reword the commit title according to the check-git-log.sh
> 
>  lib/librte_ether/rte_ethdev.c | 20 +++++---------------
> lib/librte_ether/rte_ethdev.h |  6 +++++-
>  2 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index eb0a94a..f560051 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -856,21 +856,11 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t
> nb_rx_q, uint16_t nb_tx_q,
>  	 * length is supported by the configured device.
>  	 */
>  	if (dev_conf->rxmode.jumbo_frame == 1) {
> -		if (dev_conf->rxmode.max_rx_pkt_len >
> -		    dev_info.max_rx_pktlen) {
> -			RTE_PMD_DEBUG_TRACE("ethdev port_id=%d
> max_rx_pkt_len %u"
> -				" > max valid value %u\n",
> -				port_id,
> -				(unsigned)dev_conf-
> >rxmode.max_rx_pkt_len,
> -				(unsigned)dev_info.max_rx_pktlen);
> -			return -EINVAL;
> -		} else if (dev_conf->rxmode.max_rx_pkt_len <
> ETHER_MIN_LEN) {
> -			RTE_PMD_DEBUG_TRACE("ethdev port_id=%d
> max_rx_pkt_len %u"
> -				" < min valid value %u\n",
> -				port_id,
> -				(unsigned)dev_conf-
> >rxmode.max_rx_pkt_len,
> -				(unsigned)ETHER_MIN_LEN);
> -			return -EINVAL;
> +		if (dev_conf->rxmode.max_rx_pkt_len >
> dev_info.max_rx_pktlen ||
> +			dev_conf->rxmode.max_rx_pkt_len <
> ETHER_MIN_LEN) {
> +			/* Use maximum frame size the NIC supports */
> +			dev->data->dev_conf.rxmode.max_rx_pkt_len =
> +
> 	dev_info.max_rx_pktlen;
>  		}
>  	} else {
>  		if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN
> || diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 4be217c..2adfd77 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -349,7 +349,11 @@ enum rte_eth_tx_mq_mode {  struct
> rte_eth_rxmode {
>  	/** The multi-queue packet distribution mode to be used, e.g. RSS.
> */
>  	enum rte_eth_rx_mq_mode mq_mode;
> -	uint32_t max_rx_pkt_len;  /**< Only used if jumbo_frame enabled.
> */
> +	/**
> +	 * Desired maximum RX frame size. Too short or too long size will be
> +	 * substituted by a default value.
> +	 */
> +	uint32_t max_rx_pkt_len;
>  	uint16_t split_hdr_size;  /**< hdr buf size (header_split enabled).*/
>  	__extension__
>  	uint16_t header_split : 1, /**< Header Split enable. */
> --
> 2.7.4