ethdev: fix max Rx packet length

There is a confusion on setting max Rx packet length, this patch aims to
clarify it.

'rte_eth_dev_configure()' API accepts max Rx packet size via
'uint32_t max_rx_pkt_len' field of the config struct 'struct
rte_eth_conf'.

Also 'rte_eth_dev_set_mtu()' API can be used to set the MTU, and result
stored into '(struct rte_eth_dev)->data->mtu'.

These two APIs are related but they work in a disconnected way, they
store the set values in different variables which makes hard to figure
out which one to use, also having two different method for a related
functionality is confusing for the users.

Other issues causing confusion is:
* maximum transmission unit (MTU) is payload of the Ethernet frame. And
  'max_rx_pkt_len' is the size of the Ethernet frame. Difference is
  Ethernet frame overhead, and this overhead may be different from
  device to device based on what device supports, like VLAN and QinQ.
* 'max_rx_pkt_len' is only valid when application requested jumbo frame,
  which adds additional confusion and some APIs and PMDs already
  discards this documented behavior.
* For the jumbo frame enabled case, 'max_rx_pkt_len' is an mandatory
  field, this adds configuration complexity for application.

As solution, both APIs gets MTU as parameter, and both saves the result
in same variable '(struct rte_eth_dev)->data->mtu'. For this
'max_rx_pkt_len' updated as 'mtu', and it is always valid independent
from jumbo frame.

For 'rte_eth_dev_configure()', 'dev->data->dev_conf.rxmode.mtu' is user
request and it should be used only within configure function and result
should be stored to '(struct rte_eth_dev)->data->mtu'. After that point
both application and PMD uses MTU from this variable.

When application doesn't provide an MTU during 'rte_eth_dev_configure()'
default 'RTE_ETHER_MTU' value is used.

Additional clarification done on scattered Rx configuration, in
relation to MTU and Rx buffer size.
MTU is used to configure the device for physical Rx/Tx size limitation,
Rx buffer is where to store Rx packets, many PMDs use mbuf data buffer
size as Rx buffer size.
PMDs compare MTU against Rx buffer size to decide enabling scattered Rx
or not. If scattered Rx is not supported by device, MTU bigger than Rx
buffer size should fail.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Somnath Kotur <somnath.kotur@broadcom.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Rosen Xu <rosen.xu@intel.com>
Acked-by: Hyong Youb Kim <hyonkim@cisco.com>

app/test-eventdev/test_perf_common.c

@@ -669,7 +669,6 @@ perf_ethdev_setup(struct evt_test *test, struct evt_options *opt)
 	struct rte_eth_conf port_conf = {
 		.rxmode = {
 			.mq_mode = ETH_MQ_RX_RSS,
-			.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 			.split_hdr_size = 0,
 		},
 		.rx_adv_conf = {

app/test-eventdev/test_pipeline_common.c

@@ -197,8 +197,9 @@ pipeline_ethdev_setup(struct evt_test *test, struct evt_options *opt)
 		return -EINVAL;
 	}
 
-	port_conf.rxmode.max_rx_pkt_len = opt->max_pkt_sz;
-	if (opt->max_pkt_sz > RTE_ETHER_MAX_LEN)
+	port_conf.rxmode.mtu = opt->max_pkt_sz - RTE_ETHER_HDR_LEN -
+		RTE_ETHER_CRC_LEN;
+	if (port_conf.rxmode.mtu > RTE_ETHER_MTU)
 		port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
 
 	t->internal_port = 1;

app/test-pmd/cmdline.c

@@ -1880,45 +1880,38 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
 				__rte_unused void *data)
 {
 	struct cmd_config_max_pkt_len_result *res = parsed_result;
-	uint32_t max_rx_pkt_len_backup = 0;
-	portid_t pid;
+	portid_t port_id;
 	int ret;
 
+	if (strcmp(res->name, "max-pkt-len") != 0) {
+		printf("Unknown parameter\n");
+		return;
+	}
+
 	if (!all_ports_stopped()) {
 		fprintf(stderr, "Please stop all ports first\n");
 		return;
 	}
 
-	RTE_ETH_FOREACH_DEV(pid) {
-		struct rte_port *port = &ports[pid];
+	RTE_ETH_FOREACH_DEV(port_id) {
+		struct rte_port *port = &ports[port_id];
 
-		if (!strcmp(res->name, "max-pkt-len")) {
-			if (res->value < RTE_ETHER_MIN_LEN) {
-				fprintf(stderr,
-					"max-pkt-len can not be less than %d\n",
-					RTE_ETHER_MIN_LEN);
-				return;
-			}
-			if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)
-				return;
-
-			ret = eth_dev_info_get_print_err(pid, &port->dev_info);
-			if (ret != 0) {
-				fprintf(stderr,
-					"rte_eth_dev_info_get() failed for port %u\n",
-					pid);
-				return;
-			}
-
-			max_rx_pkt_len_backup = port->dev_conf.rxmode.max_rx_pkt_len;
+		if (res->value < RTE_ETHER_MIN_LEN) {
+			fprintf(stderr,
+				"max-pkt-len can not be less than %d\n",
+				RTE_ETHER_MIN_LEN);
+			return;
+		}
 
-			port->dev_conf.rxmode.max_rx_pkt_len = res->value;
-			if (update_jumbo_frame_offload(pid) != 0)
-				port->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len_backup;
-		} else {
-			fprintf(stderr, "Unknown parameter\n");
+		ret = eth_dev_info_get_print_err(port_id, &port->dev_info);
+		if (ret != 0) {
+			fprintf(stderr,
+				"rte_eth_dev_info_get() failed for port %u\n",
+				port_id);
 			return;
 		}
+
+		update_jumbo_frame_offload(port_id, res->value);
 	}
 
 	init_port_config();

app/test-pmd/config.c

@@ -1209,7 +1209,6 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
 	int diag;
 	struct rte_port *rte_port = &ports[port_id];
 	struct rte_eth_dev_info dev_info;
-	uint16_t eth_overhead;
 	int ret;
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
@@ -1226,21 +1225,18 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
 		return;
 	}
 	diag = rte_eth_dev_set_mtu(port_id, mtu);
-	if (diag)
+	if (diag != 0) {
 		fprintf(stderr, "Set MTU failed. diag=%d\n", diag);
-	else if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) {
-		/*
-		 * Ether overhead in driver is equal to the difference of
-		 * max_rx_pktlen and max_mtu in rte_eth_dev_info when the
-		 * device supports jumbo frame.
-		 */
-		eth_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;
-		if (mtu > RTE_ETHER_MTU) {
+		return;
+	}
+
+	rte_port->dev_conf.rxmode.mtu = mtu;
+
+	if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) {
+		if (mtu > RTE_ETHER_MTU)
 			rte_port->dev_conf.rxmode.offloads |=
 						DEV_RX_OFFLOAD_JUMBO_FRAME;
-			rte_port->dev_conf.rxmode.max_rx_pkt_len =
-						mtu + eth_overhead;
-		} else
+		else
 			rte_port->dev_conf.rxmode.offloads &=
 						~DEV_RX_OFFLOAD_JUMBO_FRAME;
 	}

app/test-pmd/parameters.c

@@ -951,7 +951,7 @@ launch_args_parse(int argc, char** argv)
 			if (!strcmp(lgopts[opt_idx].name, "max-pkt-len")) {
 				n = atoi(optarg);
 				if (n >= RTE_ETHER_MIN_LEN)
-					rx_mode.max_rx_pkt_len = (uint32_t) n;
+					max_rx_pkt_len = n;
 				else
 					rte_exit(EXIT_FAILURE,
 						 "Invalid max-pkt-len=%d - should be > %d\n",

app/test-pmd/testpmd.c

@@ -219,6 +219,11 @@ unsigned int xstats_display_num; /**< Size of extended statistics to show */
  */
 uint8_t f_quit;
 
+/*
+ * Max Rx frame size, set by '--max-pkt-len' parameter.
+ */
+uint32_t max_rx_pkt_len;
+
 /*
  * Configuration of packet segments used to scatter received packets
  * if some of split features is configured.
@@ -451,13 +456,7 @@ lcoreid_t latencystats_lcore_id = -1;
 /*
  * Ethernet device configuration.
  */
-struct rte_eth_rxmode rx_mode = {
-	/* Default maximum frame length.
-	 * Zero is converted to "RTE_ETHER_MTU + PMD Ethernet overhead"
-	 * in init_config().
-	 */
-	.max_rx_pkt_len = 0,
-};
+struct rte_eth_rxmode rx_mode;
 
 struct rte_eth_txmode tx_mode = {
 	.offloads = DEV_TX_OFFLOAD_MBUF_FAST_FREE,
@@ -1542,11 +1541,24 @@ check_nb_hairpinq(queueid_t hairpinq)
 	return 0;
 }
 
+static int
+get_eth_overhead(struct rte_eth_dev_info *dev_info)
+{
+	uint32_t eth_overhead;
+
+	if (dev_info->max_mtu != UINT16_MAX &&
+	    dev_info->max_rx_pktlen > dev_info->max_mtu)
+		eth_overhead = dev_info->max_rx_pktlen - dev_info->max_mtu;
+	else
+		eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	return eth_overhead;
+}
+
 static void
 init_config_port_offloads(portid_t pid, uint32_t socket_id)
 {
 	struct rte_port *port = &ports[pid];
-	uint16_t data_size;
 	int ret;
 	int i;
 
@@ -1560,7 +1572,7 @@ init_config_port_offloads(portid_t pid, uint32_t socket_id)
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "rte_eth_dev_info_get() failed\n");
 
-	ret = update_jumbo_frame_offload(pid);
+	ret = update_jumbo_frame_offload(pid, 0);
 	if (ret != 0)
 		fprintf(stderr,
 			"Updating jumbo frame offload failed for port %u\n",
@@ -1580,6 +1592,10 @@ init_config_port_offloads(portid_t pid, uint32_t socket_id)
 	if (eth_link_speed)
 		port->dev_conf.link_speeds = eth_link_speed;
 
+	if (max_rx_pkt_len)
+		port->dev_conf.rxmode.mtu = max_rx_pkt_len -
+			get_eth_overhead(&port->dev_info);
+
 	/* set flag to initialize port/queue */
 	port->need_reconfig = 1;
 	port->need_reconfig_queues = 1;
@@ -1592,14 +1608,20 @@ init_config_port_offloads(portid_t pid, uint32_t socket_id)
 	 */
 	if (port->dev_info.rx_desc_lim.nb_mtu_seg_max != UINT16_MAX &&
 	    port->dev_info.rx_desc_lim.nb_mtu_seg_max != 0) {
-		data_size = rx_mode.max_rx_pkt_len /
-			port->dev_info.rx_desc_lim.nb_mtu_seg_max;
-
-		if ((data_size + RTE_PKTMBUF_HEADROOM) > mbuf_data_size[0]) {
-			mbuf_data_size[0] = data_size + RTE_PKTMBUF_HEADROOM;
-			TESTPMD_LOG(WARNING,
-				    "Configured mbuf size of the first segment %hu\n",
-				    mbuf_data_size[0]);
+		uint32_t eth_overhead = get_eth_overhead(&port->dev_info);
+		uint16_t mtu;
+
+		if (rte_eth_dev_get_mtu(pid, &mtu) == 0) {
+			uint16_t data_size = (mtu + eth_overhead) /
+				port->dev_info.rx_desc_lim.nb_mtu_seg_max;
+			uint16_t buffer_size = data_size + RTE_PKTMBUF_HEADROOM;
+
+			if (buffer_size > mbuf_data_size[0]) {
+				mbuf_data_size[0] = buffer_size;
+				TESTPMD_LOG(WARNING,
+					"Configured mbuf size of the first segment %hu\n",
+					mbuf_data_size[0]);
+			}
 		}
 	}
 }
@@ -2735,6 +2757,7 @@ start_port(portid_t pid)
 					pi);
 				return -1;
 			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
@@ -3669,44 +3692,45 @@ rxtx_port_config(struct rte_port *port)
 
 /*
  * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
- * MTU is also aligned if JUMBO_FRAME offload is not set.
+ * MTU is also aligned.
  *
  * port->dev_info should be set before calling this function.
  *
+ * if 'max_rx_pktlen' is zero, it is set to current device value, "MTU +
+ * ETH_OVERHEAD". This is useful to update flags but not MTU value.
+ *
  * return 0 on success, negative on error
  */
 int
-update_jumbo_frame_offload(portid_t portid)
+update_jumbo_frame_offload(portid_t portid, uint32_t max_rx_pktlen)
 {
 	struct rte_port *port = &ports[portid];
 	uint32_t eth_overhead;
 	uint64_t rx_offloads;
-	int ret;
+	uint16_t mtu, new_mtu;
 	bool on;
 
-	/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
-	if (port->dev_info.max_mtu != UINT16_MAX &&
-	    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
-		eth_overhead = port->dev_info.max_rx_pktlen -
-				port->dev_info.max_mtu;
-	else
-		eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+	eth_overhead = get_eth_overhead(&port->dev_info);
 
-	rx_offloads = port->dev_conf.rxmode.offloads;
+	if (rte_eth_dev_get_mtu(portid, &mtu) != 0) {
+		printf("Failed to get MTU for port %u\n", portid);
+		return -1;
+	}
 
-	/* Default config value is 0 to use PMD specific overhead */
-	if (port->dev_conf.rxmode.max_rx_pkt_len == 0)
-		port->dev_conf.rxmode.max_rx_pkt_len = RTE_ETHER_MTU + eth_overhead;
+	if (max_rx_pktlen == 0)
+		max_rx_pktlen = mtu + eth_overhead;
+
+	rx_offloads = port->dev_conf.rxmode.offloads;
+	new_mtu = max_rx_pktlen - eth_overhead;
 
-	if (port->dev_conf.rxmode.max_rx_pkt_len <= RTE_ETHER_MTU + eth_overhead) {
+	if (new_mtu <= RTE_ETHER_MTU) {
 		rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
 		on = false;
 	} else {
 		if ((port->dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
 			fprintf(stderr,
 				"Frame size (%u) is not supported by port %u\n",
-				port->dev_conf.rxmode.max_rx_pkt_len,
-				portid);
+				max_rx_pktlen, portid);
 			return -1;
 		}
 		rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
@@ -3727,19 +3751,18 @@ update_jumbo_frame_offload(portid_t portid)
 		}
 	}
 
-	/* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
-	 * if unset do it here
-	 */
-	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
-		ret = eth_dev_set_mtu_mp(portid,
-				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
-		if (ret)
-			fprintf(stderr,
-				"Failed to set MTU to %u for port %u\n",
-				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
-				portid);
+	if (mtu == new_mtu)
+		return 0;
+
+	if (eth_dev_set_mtu_mp(portid, new_mtu) != 0) {
+		fprintf(stderr,
+			"Failed to set MTU to %u for port %u\n",
+			new_mtu, portid);
+		return -1;
 	}
 
+	port->dev_conf.rxmode.mtu = new_mtu;
+
 	return 0;
 }
 

app/test-pmd/testpmd.h

@@ -467,6 +467,8 @@ extern uint8_t bitrate_enabled;
 
 extern struct rte_fdir_conf fdir_conf;
 
+extern uint32_t max_rx_pkt_len;
+
 /*
  * Configuration of packet segments used to scatter received packets
  * if some of split features is configured.
@@ -1043,7 +1045,7 @@ uint16_t tx_pkt_set_dynf(uint16_t port_id, __rte_unused uint16_t queue,
 			 __rte_unused void *user_param);
 void add_tx_dynf_callback(portid_t portid);
 void remove_tx_dynf_callback(portid_t portid);
-int update_jumbo_frame_offload(portid_t portid);
+int update_jumbo_frame_offload(portid_t portid, uint32_t max_rx_pktlen);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the

app/test/test_link_bonding.c

@@ -136,7 +136,6 @@ static struct rte_eth_conf default_pmd_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_NONE,
 		.split_hdr_size = 0,
-		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 	},
 	.txmode = {
 		.mq_mode = ETH_MQ_TX_NONE,

app/test/test_link_bonding_mode4.c

@@ -108,7 +108,6 @@ static struct link_bonding_unittest_params test_params  = {
 static struct rte_eth_conf default_pmd_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_NONE,
-		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
 	},
 	.txmode = {

app/test/test_link_bonding_rssconf.c

@@ -81,7 +81,6 @@ static struct link_bonding_rssconf_unittest_params test_params  = {
 static struct rte_eth_conf default_pmd_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_NONE,
-		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
 	},
 	.txmode = {
@@ -93,7 +92,6 @@ static struct rte_eth_conf default_pmd_conf = {
 static struct rte_eth_conf rss_pmd_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_RSS,
-		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
 	},
 	.txmode = {

app/test/test_pmd_perf.c

@@ -63,7 +63,6 @@ static struct rte_ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
 static struct rte_eth_conf port_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_NONE,
-		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
 	},
 	.txmode = {