[dpdk-dev] [PATCH 0/2] Enabling default mbuf segments support

[dpdk-dev] [PATCH 0/2] Enabling default mbuf segments support

[Sunil Kumar Kori]

rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max to provide
maximum number of segments supported by the platform but API does not provide
any default value as well as no show case of above mentioned fields while
creating mbuf pool.

Also in absence of this, there may be a gap bewteen application configuration
and PMD capabilities.

Consider below mentioned points:
Point 1:
- PMD may support N or infinte segments. So no defined value is assigned to
  represent infinite value.

Point 2:
- PMD supports n segments at max and exposed the same to application.
- But application didn't created mbuf pool accoding to maximum supported segments
  so it may be that to support larger packet, application needs more segments than
  PMD's capability. So packet transmission operation may be discarded by the PMD.

Patch set implenments default value and their usage in testpmd to provide a show
case. It will help application to create mbuf pool with correct buffer size so
that application can cater all sized packets properly.

Sunil Kumar Kori (2):
  lib/librte_ethdev: add in default value of rte_eth_dev_info
  app/testpmd: creating mbuf pool based on maximum supported segments

 app/test-pmd/testpmd.c         | 21 +++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.c |  2 ++
 lib/librte_ethdev/rte_ethdev.h |  2 ++
 3 files changed, 25 insertions(+)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Sunil Kumar Kori]

rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max
to provide maximum number of supported segments for a given platform.

Defining UINT16_MAX as default value of above mentioned variables to
expose support of infinite/maximum segments.

Based on above values, application can decide best size for buffers
while creating mbuf pool.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index d7cfa3d..6933757 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2543,6 +2543,8 @@ struct rte_eth_dev *
 		.nb_max = UINT16_MAX,
 		.nb_min = 0,
 		.nb_align = 1,
+		.nb_seg_max = UINT16_MAX,
+		.nb_mtu_seg_max = UINT16_MAX,
 	};
 
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 1f35e1d..6bd30b1 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2333,6 +2333,8 @@ int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
  *      .nb_max = UINT16_MAX,
  *      .nb_min = 0,
  *      .nb_align = 1,
+ *	.nb_seg_max = UINT16_MAX,
+ *	.nb_mtu_seg_max = UINT16_MAX,
  *  };
  *
  * device = dev->device
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Andrew Rybchenko]

On 5/27/19 11:35 AM, Sunil Kumar Kori wrote:
> rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max
> to provide maximum number of supported segments for a given platform.
>
> Defining UINT16_MAX as default value of above mentioned variables to
> expose support of infinite/maximum segments.
>
> Based on above values, application can decide best size for buffers
> while creating mbuf pool.
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Ferruh Yigit]

On 6/9/2019 3:33 PM, Andrew Rybchenko wrote:
> On 5/27/19 11:35 AM, Sunil Kumar Kori wrote:
>> rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max
>> to provide maximum number of supported segments for a given platform.
>>
>> Defining UINT16_MAX as default value of above mentioned variables to
>> expose support of infinite/maximum segments.
>>
>> Based on above values, application can decide best size for buffers
>> while creating mbuf pool.
>>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> 
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Mo, YufengX]

Hi, Sunil Kumar Kori

This series' patches have been merged on dpdk/master. They cause testpmd core dumped on intel nics.

./usertools/dpdk-devbind.py -b igb_uio 0000:xx:00.0 0000:xx:00.1
./x86_64-native-linuxapp-gcc/app/testpmd -v -c 0x3f -n 4  -- -i

Running environment as the following:

* OS:
fedora 20/22/27/30
3.16.4/4.4.14/5.1.0

* Compiler:
gcc version 5.3.1
gcc version 7.3.1
gcc version 4.8.3

* Hardware platform:
Broadwell-EP Xeon E5-2600
Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
Intel(R) Xeon(R) Platinum 8160 CPU @ 2.10GHz

* NIC hardware:
fortville_spirit
Ethernet Controller XL710 for 40GbE QSFP+ 1583
version: 1.5.16
firmware-version: 6.01 0x800034a4 1.1747.0

fortville(25G 2 ports nic)
Ethernet Controller XXV710 for 25GbE SFP28 158b
driver: i40e
version: 2.1.14-k
firmware-version: 6.01 0x80003554 1.1747.0

fortville(10G 2 ports nic)
Ethernet Controller X710 for 10GbE SFP+ 1572
driver: i40e
version: 2.1.14-k
firmware-version: 6.01 0x800035b0 1.1747.0

niantic
Device_str: 82599ES 10-Gigabit SFI/SFP+ Network Connection
firmware: 0x61bf0001
ixgbe: 4.3.13
ixgbevf: 2.12.1-k


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sunil Kumar Kori
> Sent: Monday, May 27, 2019 4:35 PM
> To: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; arybchenko@solarflare.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Sunil Kumar Kori <skori@marvell.com>
> Subject: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info
> 
> rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max
> to provide maximum number of supported segments for a given platform.
> 
> Defining UINT16_MAX as default value of above mentioned variables to
> expose support of infinite/maximum segments.
> 
> Based on above values, application can decide best size for buffers
> while creating mbuf pool.
> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>  lib/librte_ethdev/rte_ethdev.c | 2 ++
>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index d7cfa3d..6933757 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -2543,6 +2543,8 @@ struct rte_eth_dev *
>  		.nb_max = UINT16_MAX,
>  		.nb_min = 0,
>  		.nb_align = 1,
> +		.nb_seg_max = UINT16_MAX,
> +		.nb_mtu_seg_max = UINT16_MAX,
>  	};
> 
>  	RTE_ETH_VALID_PORTID_OR_RET(port_id);
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 1f35e1d..6bd30b1 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2333,6 +2333,8 @@ int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
>   *      .nb_max = UINT16_MAX,
>   *      .nb_min = 0,
>   *      .nb_align = 1,
> + *	.nb_seg_max = UINT16_MAX,
> + *	.nb_mtu_seg_max = UINT16_MAX,
>   *  };
>   *
>   * device = dev->device
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Ferruh Yigit]

On 6/14/2019 6:31 AM, Mo, YufengX wrote:
> Hi, Sunil Kumar Kori
> 
> This series' patches have been merged on dpdk/master. They cause testpmd core dumped on intel nics.

Right, since they can provide values as "nb_seg_max = 0, nb_mtu_seg_max = 0",
I am sending a patch now.

> 
> ./usertools/dpdk-devbind.py -b igb_uio 0000:xx:00.0 0000:xx:00.1
> ./x86_64-native-linuxapp-gcc/app/testpmd -v -c 0x3f -n 4  -- -i
> 
> Running environment as the following:
> 
> * OS:
> fedora 20/22/27/30
> 3.16.4/4.4.14/5.1.0
> 
> * Compiler:
> gcc version 5.3.1
> gcc version 7.3.1
> gcc version 4.8.3
> 
> * Hardware platform:
> Broadwell-EP Xeon E5-2600
> Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
> Intel(R) Xeon(R) Platinum 8160 CPU @ 2.10GHz
> 
> * NIC hardware:
> fortville_spirit
> Ethernet Controller XL710 for 40GbE QSFP+ 1583
> version: 1.5.16
> firmware-version: 6.01 0x800034a4 1.1747.0
> 
> fortville(25G 2 ports nic)
> Ethernet Controller XXV710 for 25GbE SFP28 158b
> driver: i40e
> version: 2.1.14-k
> firmware-version: 6.01 0x80003554 1.1747.0
> 
> fortville(10G 2 ports nic)
> Ethernet Controller X710 for 10GbE SFP+ 1572
> driver: i40e
> version: 2.1.14-k
> firmware-version: 6.01 0x800035b0 1.1747.0
> 
> niantic
> Device_str: 82599ES 10-Gigabit SFI/SFP+ Network Connection
> firmware: 0x61bf0001
> ixgbe: 4.3.13
> ixgbevf: 2.12.1-k
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sunil Kumar Kori
>> Sent: Monday, May 27, 2019 4:35 PM
>> To: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; arybchenko@solarflare.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
>> Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
>> Cc: dev@dpdk.org; Sunil Kumar Kori <skori@marvell.com>
>> Subject: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info
>>
>> rte_eth_dev_info structure exposes, nb_seg_max & nb_mtu_seg_max
>> to provide maximum number of supported segments for a given platform.
>>
>> Defining UINT16_MAX as default value of above mentioned variables to
>> expose support of infinite/maximum segments.
>>
>> Based on above values, application can decide best size for buffers
>> while creating mbuf pool.
>>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.c | 2 ++
>>  lib/librte_ethdev/rte_ethdev.h | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index d7cfa3d..6933757 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -2543,6 +2543,8 @@ struct rte_eth_dev *
>>  		.nb_max = UINT16_MAX,
>>  		.nb_min = 0,
>>  		.nb_align = 1,
>> +		.nb_seg_max = UINT16_MAX,
>> +		.nb_mtu_seg_max = UINT16_MAX,
>>  	};
>>
>>  	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 1f35e1d..6bd30b1 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -2333,6 +2333,8 @@ int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
>>   *      .nb_max = UINT16_MAX,
>>   *      .nb_min = 0,
>>   *      .nb_align = 1,
>> + *	.nb_seg_max = UINT16_MAX,
>> + *	.nb_mtu_seg_max = UINT16_MAX,
>>   *  };
>>   *
>>   * device = dev->device
>> --
>> 1.8.3.1
> 

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Ferruh Yigit]

On 6/14/2019 5:51 PM, Ferruh Yigit wrote:
> On 6/14/2019 6:31 AM, Mo, YufengX wrote:
>> Hi, Sunil Kumar Kori
>>
>> This series' patches have been merged on dpdk/master. They cause testpmd core dumped on intel nics.
> 
> Right, since they can provide values as "nb_seg_max = 0, nb_mtu_seg_max = 0",
> I am sending a patch now.

@Yufeng, Can you please try with patch https://patches.dpdk.org/patch/54811/?

@Thomas, Can it be possible to merge fix to master? Or I can merge if you want?

Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info

[Mo, YufengX]

Hi, Ferruh  Yigit

Sorry, reply late, I have no vpn/laptop to connect to intel servers. 
Daily regression is running good today since your patch has been merged yesterday.
I have applied your patch manually, it is ok.


> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Saturday, June 15, 2019 1:22 AM
> To: Mo, YufengX <yufengx.mo@intel.com>; Sunil Kumar Kori <skori@marvell.com>; thomas@monjalon.net; arybchenko@solarflare.com;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] lib/librte_ethdev: add in default value of rte_eth_dev_info
> 
> On 6/14/2019 5:51 PM, Ferruh Yigit wrote:
> > On 6/14/2019 6:31 AM, Mo, YufengX wrote:
> >> Hi, Sunil Kumar Kori
> >>
> >> This series' patches have been merged on dpdk/master. They cause testpmd core dumped on intel nics.
> >
> > Right, since they can provide values as "nb_seg_max = 0, nb_mtu_seg_max = 0",
> > I am sending a patch now.
> 
> @Yufeng, Can you please try with patch https://patches.dpdk.org/patch/54811/?
> 
> @Thomas, Can it be possible to merge fix to master? Or I can merge if you want?

[dpdk-dev] [PATCH 2/2] app/testpmd: creating mbuf pool based on maximum supported segments

[Sunil Kumar Kori]

Configuring buffer size based following parameters:
- max-pkt-len
- max supported segments per MTU

Buffer size are configured as given below:
- If platform supports infinite segments per packet then default
  buffer size is used.
- If platform supports nb_mtu_seg_max segments then buffer size
  is configured as (max-pkt-len / nb_mtu_seg_max) + headroom

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 app/test-pmd/testpmd.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f0061d9..b8c006b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1100,6 +1100,8 @@ struct extmem_param {
 	uint8_t port_per_socket[RTE_MAX_NUMA_NODES];
 	struct rte_gro_param gro_param;
 	uint32_t gso_types;
+	uint16_t data_size;
+	bool warning = 0;
 	int k;
 
 	memset(port_per_socket,0,RTE_MAX_NUMA_NODES);
@@ -1167,8 +1169,27 @@ struct extmem_param {
 		port->need_reconfig = 1;
 		port->need_reconfig_queues = 1;
 		port->tx_metadata = 0;
+
+		/* Check for maximum number of segments per MTU. Accordingly
+		 * update the mbuf data size.
+		 */
+		if (port->dev_info.rx_desc_lim.nb_mtu_seg_max != UINT16_MAX) {
+			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) {
+				mbuf_data_size = data_size +
+						 RTE_PKTMBUF_HEADROOM;
+				warning = 1;
+			}
+		}
 	}
 
+	if (warning)
+		TESTPMD_LOG(WARNING, "Configured mbuf size %hu\n",
+			    mbuf_data_size);
+
 	/*
 	 * Create pools of mbuf.
 	 * If NUMA support is disabled, create a single pool of mbuf in
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] app/testpmd: creating mbuf pool based on maximum supported segments

[Ferruh Yigit]

On 5/27/2019 9:35 AM, Sunil Kumar Kori wrote:
> Configuring buffer size based following parameters:
> - max-pkt-len
> - max supported segments per MTU
> 
> Buffer size are configured as given below:
> - If platform supports infinite segments per packet then default
>   buffer size is used.
> - If platform supports nb_mtu_seg_max segments then buffer size
>   is configured as (max-pkt-len / nb_mtu_seg_max) + headroom
> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.