DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] ethdev: add firmware version get
From: Qiming Yang @ 2016-12-06  7:16 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang
In-Reply-To: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>

This patch adds a new API 'rte_eth_dev_fwver_get' for fetching firmware
version by a given device.

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
v2 changes:
* modified some comment statements.
---
---
 lib/librte_ether/rte_ethdev.c          | 12 ++++++++++++
 lib/librte_ether/rte_ethdev.h          | 18 ++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 +++++++
 3 files changed, 37 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..793e50f 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1538,6 +1538,18 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint8_t port_id, uint16_t rx_queue_id,
 }
 
 void
+rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_RET(port_id);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->fw_version_get);
+	(*dev->dev_ops->fw_version_get)(dev, fw_version, fw_length);
+}
+
+void
 rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..2c05df4 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1150,6 +1150,10 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
 /**< @internal Check DD bit of specific RX descriptor */
 
+typedef void (*eth_fw_version_get_t)(struct rte_eth_dev *dev,
+				    char *fw_version, int fw_length);
+/**< @internal Get firmware information of an Ethernet device. */
+
 typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
 	uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
 
@@ -1444,6 +1448,7 @@ struct eth_dev_ops {
 	/**< Get names of extended statistics. */
 	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
 	/**< Configure per queue stat counter mapping. */
+	eth_fw_version_get_t       fw_version_get; /**< Get firmware version. */
 	eth_dev_infos_get_t        dev_infos_get; /**< Get device info. */
 	eth_dev_supported_ptypes_get_t dev_supported_ptypes_get;
 	/**< Get packet types supported and identified by device*/
@@ -2385,6 +2390,19 @@ void rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr);
 void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info);
 
 /**
+ * Retrieve the firmware version of a device.
+ *
+ * @param port_id
+ *   The port identifier of the device.
+ * @param fw_version
+ *   A array pointer to store the firmware version of a device,
+ *   allocated by caller.
+ * @param fw_length
+ *   The size of the array pointed by fw_version.
+ */
+void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length);
+
+/**
  * Retrieve the supported packet types of an Ethernet device.
  *
  * When a packet type is announced as supported, it *must* be recognized by
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 72be66d..5e6387f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -147,3 +147,10 @@ DPDK_16.11 {
 	rte_eth_dev_pci_remove;
 
 } DPDK_16.07;
+
+DPDK_17.02 {
+	global:
+
+	rte_eth_dev_fwver_get;
+
+} DPDK_16.11;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 2/5] net/e1000: add firmware version get
From: Qiming Yang @ 2016-12-06  7:16 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang
In-Reply-To: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/e1000/igb_ethdev.c | 46 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..c47f1c8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -120,6 +120,8 @@ static int eth_igb_xstats_get_names(struct rte_eth_dev *dev,
 				    unsigned limit);
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_xstats_reset(struct rte_eth_dev *dev);
+static void eth_igb_fw_version_get(struct rte_eth_dev *dev,
+				   char *fw_version, int fw_length);
 static void eth_igb_infos_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static const uint32_t *eth_igb_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -388,6 +390,7 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.xstats_get_names     = eth_igb_xstats_get_names,
 	.stats_reset          = eth_igb_stats_reset,
 	.xstats_reset         = eth_igb_xstats_reset,
+	.fw_version_get       = eth_igb_fw_version_get,
 	.dev_infos_get        = eth_igb_infos_get,
 	.dev_supported_ptypes_get = eth_igb_supported_ptypes_get,
 	.mtu_set              = eth_igb_mtu_set,
@@ -1977,6 +1980,49 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+		       int fw_length)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_fw_version fw;
+
+	e1000_get_fw_version(hw, &fw);
+
+	switch (hw->mac.type) {
+	case e1000_i210:
+	case e1000_i211:
+		if (!(e1000_get_flash_presence_i210(hw))) {
+			snprintf(fw_version, fw_length,
+				 "%2d.%2d-%d",
+				 fw.invm_major, fw.invm_minor,
+				 fw.invm_img_type);
+			break;
+		}
+		/* fall through */
+	default:
+		/* if option rom is valid, display its version too*/
+		if (fw.or_valid) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x, %d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id,
+				 fw.or_major, fw.or_build, fw.or_patch);
+		/* no option rom */
+		} else {
+			if (fw.etrack_id != 0X0000) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id);
+			} else {
+			snprintf(fw_version, fw_length,
+				 "%d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.eep_build);
+			}
+		}
+		break;
+	}
+}
+
+static void
 eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 3/5] net/ixgbe: add firmware version get
From: Qiming Yang @ 2016-12-06  7:16 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang
In-Reply-To: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..da9aa31 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -193,6 +193,8 @@ static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
 					     uint8_t is_rx);
+static void ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+				 int fw_length);
 static void ixgbe_dev_info_get(struct rte_eth_dev *dev,
 			       struct rte_eth_dev_info *dev_info);
 static const uint32_t *ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -537,6 +539,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.xstats_reset         = ixgbe_dev_xstats_reset,
 	.xstats_get_names     = ixgbe_dev_xstats_get_names,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
+	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
 	.dev_supported_ptypes_get = ixgbe_dev_supported_ptypes_get,
 	.mtu_set              = ixgbe_dev_mtu_set,
@@ -3029,6 +3032,21 @@ ixgbevf_dev_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	u16 eeprom_verh, eeprom_verl;
+	u32 etrack_id;
+
+	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
+	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
+
+	etrack_id = (eeprom_verh << 16) | eeprom_verl;
+	snprintf(fw_version, fw_length,
+		 "0x%08x", etrack_id);
+}
+
+static void
 ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 4/5] net/i40e: add firmware version get
From: Qiming Yang @ 2016-12-06  7:16 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang
In-Reply-To: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..f79bc5e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -324,6 +324,8 @@ static int i40e_dev_queue_stats_mapping_set(struct rte_eth_dev *dev,
 					    uint16_t queue_id,
 					    uint8_t stat_idx,
 					    uint8_t is_rx);
+static void i40e_fw_version_get(struct rte_eth_dev *dev,
+				char *fw_version, int fw_length);
 static void i40e_dev_info_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static int i40e_vlan_filter_set(struct rte_eth_dev *dev,
@@ -503,6 +505,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.stats_reset                  = i40e_dev_stats_reset,
 	.xstats_reset                 = i40e_dev_stats_reset,
 	.queue_stats_mapping_set      = i40e_dev_queue_stats_mapping_set,
+	.fw_version_get               = i40e_fw_version_get,
 	.dev_infos_get                = i40e_dev_info_get,
 	.dev_supported_ptypes_get     = i40e_dev_supported_ptypes_get,
 	.vlan_filter_set              = i40e_vlan_filter_set,
@@ -2577,6 +2580,18 @@ i40e_dev_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 static void
+i40e_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	snprintf(fw_version, fw_length,
+		 "%d.%d%d 0x%04x",
+		 ((hw->nvm.version >> 12) & 0xf),
+		 ((hw->nvm.version >> 4) & 0xff),
+		 (hw->nvm.version & 0xf), hw->nvm.eetrack);
+}
+
+static void
 i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 5/5] ethtool: dispaly bus info and firmware version
From: Qiming Yang @ 2016-12-06  7:16 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang
In-Reply-To: <1481008582-69416-1-git-send-email-qiming.yang@intel.com>

This patch enhances the ethtool example to support to show
bus information and firmware version, in the same way that
the Linux kernel ethtool does.

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
v2 changes:
*  modified the commit log
---
---
 examples/ethtool/ethtool-app/ethapp.c | 2 ++
 examples/ethtool/lib/rte_ethtool.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/examples/ethtool/ethtool-app/ethapp.c b/examples/ethtool/ethtool-app/ethapp.c
index 38e466c..9b77385 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -184,6 +184,8 @@ pcmd_drvinfo_callback(__rte_unused void *ptr_params,
 		printf("Port %i driver: %s (ver: %s)\n",
 			id_port, info.driver, info.version
 		      );
+		printf("bus-info: %s\n", info.bus_info);
+		printf("firmware-version: %s\n", info.fw_version);
 	}
 }
 
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index a1f91d4..542179c 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -54,6 +54,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
+	rte_eth_dev_fwver_get(port_id, drvinfo->fw_version,
+			      sizeof(drvinfo->fw_version));
+
 	memset(&dev_info, 0, sizeof(dev_info));
 	rte_eth_dev_info_get(port_id, &dev_info);
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 4/4] lib/librte_vhost: improve vhost perf using rte_memset
From: Yang, Zhiyong @ 2016-12-06  8:04 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev@dpdk.org, yuanhan.liu@linux.intel.com, Richardson, Bruce,
	Ananyev, Konstantin
In-Reply-To: <1608392.pphkK3YPda@xps13>

Hi, Thomas:

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, December 2, 2016 5:46 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 4/4] lib/librte_vhost: improve vhost perf
> using rte_memset
> 
> 2016-12-05 16:26, Zhiyong Yang:
> > +* **Introduced rte_memset and related test on IA platform.**
> > +
> > +  Performance drop had been caused in some cases on Ivybridge when
> > + DPDK code calls glibc  function memset. It was necessary to introduce
> more high efficient function to fix it.
> > +  The function rte_memset supported three types of instruction sets
> > + including sse & avx(128 bits),
> > +  avx2(256 bits) and avx512(512bits).
> > +
> > +  * Added rte_memset support on IA platform.
> > +  * Added functional autotest support for rte_memset.
> > +  * Added performance autotest support for rte_memset.
> 
> No need to reference autotests in the release notes.

Ok.
I will remove the two lines.
> 
> > +  * Improved performance to use rte_memset instead of
> copy_virtio_net_hdr in lib/librte_vhost.
> 
> Please describe this change at a higher level. Which case it is improving?

Ok, good comments.

* Improved performance to get 3% or so perf improvement
on IA platform by using rte_memset when running virtio/vhost non-mergeable
loopback test without NIC.

Thanks
Zhiyong

^ permalink raw reply

* [PATCH] doc: correct description of attach to indirect mbuf
From: Baruch Siach @ 2016-12-06  8:15 UTC (permalink / raw)
  To: dev; +Cc: John McNamara, Olivier Matz, Baruch Siach

Since commit 4a96e8b652 ("mbuf: allow to clone an indirect mbuf") attaching to
an indirect mbuf creates a clone of that mbuf. Update the mbuf documentation
to match.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 doc/guides/prog_guide/mbuf_lib.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/mbuf_lib.rst b/doc/guides/prog_guide/mbuf_lib.rst
index f0aa21a6d4aa..6e73fc5a834b 100644
--- a/doc/guides/prog_guide/mbuf_lib.rst
+++ b/doc/guides/prog_guide/mbuf_lib.rst
@@ -253,7 +253,8 @@ Similarly, whenever the indirect buffer is detached, the reference counter on th
 If the resulting reference counter is equal to 0, the direct buffer is freed since it is no longer in use.
 
 There are a few things to remember when dealing with indirect buffers.
-First of all, it is not possible to attach an indirect buffer to another indirect buffer.
+First of all, an indirect buffer is never attached to another indirect buffer.
+Attempting to attach buffer A to indirect buffer B that is attached to C, makes rte_pktmbuf_attach() automatically attach A to C, effectively cloning B.
 Secondly, for a buffer to become indirect, its reference counter must be equal to 1,
 that is, it must not be already referenced by another indirect buffer.
 Finally, it is not possible to reattach an indirect buffer to the direct buffer (unless it is detached first).
-- 
2.10.2

^ permalink raw reply related

* [PATCH v2] doc: correct description of attach to indirect mbuf
From: Baruch Siach @ 2016-12-06  8:25 UTC (permalink / raw)
  To: dev; +Cc: John McNamara, Olivier Matz, Baruch Siach

Since commit 4a96e8b652181 ("mbuf: allow to clone an indirect mbuf")
attaching to an indirect mbuf creates a clone of that mbuf. Update the mbuf
documentation to match.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: fix checkpatch issues: long commit log lines, short commit id
---
 doc/guides/prog_guide/mbuf_lib.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/mbuf_lib.rst b/doc/guides/prog_guide/mbuf_lib.rst
index f0aa21a6d4aa..6e73fc5a834b 100644
--- a/doc/guides/prog_guide/mbuf_lib.rst
+++ b/doc/guides/prog_guide/mbuf_lib.rst
@@ -253,7 +253,8 @@ Similarly, whenever the indirect buffer is detached, the reference counter on th
 If the resulting reference counter is equal to 0, the direct buffer is freed since it is no longer in use.
 
 There are a few things to remember when dealing with indirect buffers.
-First of all, it is not possible to attach an indirect buffer to another indirect buffer.
+First of all, an indirect buffer is never attached to another indirect buffer.
+Attempting to attach buffer A to indirect buffer B that is attached to C, makes rte_pktmbuf_attach() automatically attach A to C, effectively cloning B.
 Secondly, for a buffer to become indirect, its reference counter must be equal to 1,
 that is, it must not be already referenced by another indirect buffer.
 Finally, it is not possible to reattach an indirect buffer to the direct buffer (unless it is detached first).
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH 0/4] eal/common: introduce rte_memset and related test
From: Maxime Coquelin @ 2016-12-06  8:29 UTC (permalink / raw)
  To: Yang, Zhiyong, dev@dpdk.org
  Cc: yuanhan.liu@linux.intel.com, Richardson, Bruce,
	Ananyev, Konstantin, Pierre Pfister (ppfister)
In-Reply-To: <E182254E98A5DA4EB1E657AC7CB9BD2A3EB4C842@BGSMSX101.gar.corp.intel.com>



On 12/06/2016 07:33 AM, Yang, Zhiyong wrote:
> Hi, Maxime:
>
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Friday, December 2, 2016 6:01 PM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
>> Cc: yuanhan.liu@linux.intel.com; Richardson, Bruce
>> <bruce.richardson@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 0/4] eal/common: introduce rte_memset
>> and related test
>>
>> Hi Zhiyong,
>>
>> On 12/05/2016 09:26 AM, Zhiyong Yang wrote:
>>> DPDK code has met performance drop badly in some case when calling
>>> glibc function memset. Reference to discussions about memset in
>>> http://dpdk.org/ml/archives/dev/2016-October/048628.html
>>> It is necessary to introduce more high efficient function to fix it.
>>> One important thing about rte_memset is that we can get clear control
>>> on what instruction flow is used.
>>>
>>> This patchset introduces rte_memset to bring more high efficient
>>> implementation, and will bring obvious perf improvement, especially
>>> for small N bytes in the most application scenarios.
>>>
>>> Patch 1 implements rte_memset in the file rte_memset.h on IA platform
>>> The file supports three types of instruction sets including sse & avx
>>> (128bits), avx2(256bits) and avx512(512bits). rte_memset makes use of
>>> vectorization and inline function to improve the perf on IA. In
>>> addition, cache line and memory alignment are fully taken into
>> consideration.
>>>
>>> Patch 2 implements functional autotest to validates the function
>>> whether to work in a right way.
>>>
>>> Patch 3 implements performance autotest separately in cache and memory.
>>>
>>> Patch 4 Using rte_memset instead of copy_virtio_net_hdr can bring
>>> 3%~4% performance improvements on IA platform from virtio/vhost
>>> non-mergeable loopback testing.
>>>
>>> Zhiyong Yang (4):
>>>   eal/common: introduce rte_memset on IA platform
>>>   app/test: add functional autotest for rte_memset
>>>   app/test: add performance autotest for rte_memset
>>>   lib/librte_vhost: improve vhost perf using rte_memset
>>>
>>>  app/test/Makefile                                  |   3 +
>>>  app/test/test_memset.c                             | 158 +++++++++
>>>  app/test/test_memset_perf.c                        | 348 +++++++++++++++++++
>>>  doc/guides/rel_notes/release_17_02.rst             |  11 +
>>>  .../common/include/arch/x86/rte_memset.h           | 376
>> +++++++++++++++++++++
>>>  lib/librte_eal/common/include/generic/rte_memset.h |  51 +++
>>>  lib/librte_vhost/virtio_net.c                      |  18 +-
>>>  7 files changed, 958 insertions(+), 7 deletions(-)  create mode
>>> 100644 app/test/test_memset.c  create mode 100644
>>> app/test/test_memset_perf.c  create mode 100644
>>> lib/librte_eal/common/include/arch/x86/rte_memset.h
>>>  create mode 100644
>> lib/librte_eal/common/include/generic/rte_memset.h
>>>
>>
>> Thanks for the series, idea looks good to me.
>>
>> Wouldn't be worth to also use rte_memset in Virtio PMD (not
>> compiled/tested)? :
>>
>
> I think  rte_memset  maybe can bring some benefit here,  but , I'm not clear how to
> enter the branch and test it. :)

Indeed, you will need Pierre's patch:
[dpdk-dev] [PATCH] virtio: tx with can_push when VERSION_1 is set

Thanks,
Maxime
>
> thanks
> Zhiyong
>
>> diff --git a/drivers/net/virtio/virtio_rxtx.c
>> b/drivers/net/virtio/virtio_rxtx.c
>> index 22d97a4..a5f70c4 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -287,7 +287,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq,
>> struct rte_mbuf *cookie,
>>                          rte_pktmbuf_prepend(cookie, head_size);
>>                  /* if offload disabled, it is not zeroed below, do it now */
>>                  if (offload == 0)
>> -                       memset(hdr, 0, head_size);
>> +                       rte_memset(hdr, 0, head_size);
>>          } else if (use_indirect) {
>>                  /* setup tx ring slot to point to indirect
>>                   * descriptor list stored in reserved region.
>>
>> Cheers,
>> Maxime

^ permalink raw reply

* Re: [PATCH 08/13] eal: enable probe and remove from bus infrastructure
From: Shreyansh Jain @ 2016-12-06 10:45 UTC (permalink / raw)
  To: dev; +Cc: thomas.monjalon, david.marchand
In-Reply-To: <1480846288-2517-9-git-send-email-shreyansh.jain@nxp.com>

On Sunday 04 December 2016 03:41 PM, Shreyansh Jain wrote:
> The model is:
>  rte_eal_init
>  `--> calls rte_eal_bus_probe()
>       This iterates over all the drivers and devices and matches them. For
>       matched bus specific device-driver:
>       `--> bus->probe()
>            This would be responsible for generic work, equivalent to
>            rte_eal_pci_probe - specific to the bus. Handles over control
>            to,
>            `--> rte_driver->probe()
>                 which works equivalent to rte_eth_dev_pci_probe for alloc-
>                 -ating a ethernet device. This would hand over the control
>                 to,
>                 `--> rte_xxx_driver->probe()
>                      Calls driver specific initialization of the eth_dev
>                      Similar to what eth_dev_init of eth_driver does.
>                      In further changes, eth_driver would be removed.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_eal/common/eal_common_bus.c | 50 +++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 67b808b..469abac 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -195,11 +195,59 @@ rte_eal_bus_scan(void)
>  	return 0;
>  }
>
> +static int
> +perform_probe(struct rte_bus *bus __rte_unused, struct rte_driver *driver,
> +	      struct rte_device *device)
> +{
> +	int ret;
> +
> +	if (!driver->probe) {
> +		RTE_LOG(ERR, EAL, "Driver (%s) doesn't support probe.\n",
> +			driver->name);
> +		/* This is not an error - just a badly implemented PMD */
> +		return 0;
> +	}
> +
> +	ret = driver->probe(driver, device);

Just noticed an issue here: It is unlike what I have stated in the patch 
headline. bus->probe() is not getting called.


Just to open a debate:
eal=> rte_bus->probe() => rte_driver->probe() => rte_pci_driver->probe()
is debatable. 1) bus never probes and 2) it just looks very odd.

But, if bus->probe() doesn't happen, which layer is responsible for 
things like devargs which currently rte_eal_pci_probe() does in EAL.
If rte_driver->probe() does the devargs layer, eth_dev allocation might 
be shifted to rte_pci_driver->probe() - and subsequently calling 
rte_eth_dev->eth_dev_init().

I do need to get my head around this. Suggestions would be really 
appreciated.

> +	if (ret < 0)
> +		/* One of the probes failed */
> +		RTE_LOG(ERR, EAL, "Probe failed for (%s).\n", driver->name);
> +
> +	/* In either case, ret <0 (error), ret > 0 (not supported) and ret = 0
> +	 * success, return ret
> +	 */
> +	return ret;
> +}
> +
>  /* Match driver<->device and call driver->probe() */
>  int
>  rte_eal_bus_probe(void)
>  {
> -	/* Until driver->probe is available, this is dummy implementation */
> +	int ret;
> +	struct rte_bus *bus;
> +	struct rte_device *device;
> +	struct rte_driver *driver;
> +
> +	/* For each bus registered with EAL */
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		TAILQ_FOREACH(device, &bus->device_list, next) {
> +			TAILQ_FOREACH(driver, &bus->driver_list, next) {
> +				ret = bus->match(driver, device);
> +				if (!ret) {
> +					ret = perform_probe(bus, driver,
> +							    device);
> +					if (ret < 0)
> +						return ret;
> +
> +					/* ret == 0 is success; ret >0 implies
> +					 * driver doesn't support the device.
> +					 * in either case, continue
> +					 */
> +				}
> +			}
> +		}
> +	}
> +
>  	return 0;
>  }
>
>

^ permalink raw reply

* Re: [PATCH v3] eal/linuxapp: fix return value check of mknod()
From: Olivier Matz @ 2016-12-06 10:44 UTC (permalink / raw)
  To: Wei Dai; +Cc: david.marchand, dev
In-Reply-To: <1479365235-134903-1-git-send-email-wei.dai@intel.com>

Hi Wei,

On Thu, 17 Nov 2016 14:47:15 +0800, Wei Dai <wei.dai@intel.com> wrote:
> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> The return value of mknod() is ret, not f got by fopen().
> So the value of ret should be checked for mknod().
> 
> Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without
> hotplug")
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks

^ permalink raw reply

* Re: [PATCH] maintainers: update for qede PMD and bnx2x PMD
From: Thomas Monjalon @ 2016-12-06 10:47 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: dev, Dept-EngDPDKDev
In-Reply-To: <1480746289-24602-1-git-send-email-Rasesh.Mody@cavium.com>

2016-12-02 22:24, Rasesh Mody:
>  QLogic bnx2x
> -M: Sony Chacko <sony.chacko@qlogic.com>
> -M: Harish Patil <harish.patil@qlogic.com>
> -M: Rasesh Mody <rasesh.mody@qlogic.com>
> +M: Harish Patil <Harish.Patil@cavium.com>
> +M: Rasesh Mody <Rasesh.Mody@cavium.com>
>  F: drivers/net/bnx2x/
>  F: doc/guides/nics/bnx2x.rst
>  
>  QLogic qede PMD
> -M: Harish Patil <harish.patil@qlogic.com>
> -M: Rasesh Mody <rasesh.mody@qlogic.com>
> -M: Sony Chacko <sony.chacko@qlogic.com>
> +M: Rasesh Mody <Rasesh.Mody@cavium.com>
> +M: Harish Patil <Harish.Patil@cavium.com>
>  F: drivers/net/qede/
>  F: doc/guides/nics/qede.rst

I suggest to keep consistency and use only lowercase in email address.

^ permalink raw reply

* Re: [PATCH 0/3] net: optimize Rx/Tx log message level
From: Ferruh Yigit @ 2016-12-06 10:51 UTC (permalink / raw)
  To: Qiming Yang, dev; +Cc: Wenzhuo Lu, Helin Zhang, Konstantin Ananyev, Jingjing Wu
In-Reply-To: <1480761783-36467-1-git-send-email-qiming.yang@intel.com>

Hi Qiming,

On 12/3/2016 10:43 AM, Qiming Yang wrote:
> These three patches optimized the level of Rx and Tx log
> messages. Add a new log control function PMD_RX_FREE_LOG
> to control the Rx message which is not printed in packet
> receive processing. This function switched by macro 
> RTE_LIBRTE_<PMD>_DEBUG_RX_FREE.

Please CC the maintainers,

CC: Wenzhuo Lu <wenzhuo.lu@intel.com>
CC: Konstantin Ananyev <konstantin.ananyev@intel.com>, Helin Zhang
<helin.zhang@intel.com>
CC: Jingjing Wu <jingjing.wu@intel.com>

1- Do we really need a new config option just for this log function, we
already may have more config options?

2- Do we really need this new logging function at all? This is to report
when receive descriptor tail updated, as far as I can see. What is the
use case that makes this case so special?

3- Log level of some messages are increased, like mbuf allocation
failure message level increased to ERR, what is the expectation here? In
high throughput traffic this may cause lots of noise, and what user can
do with this message? If user can't do anything, why we bloating user
with error messages? There is already a dynamic counter, I believe which
is good thing to the for this error, and debug level logging.


Overall I am for rejecting this patchset, unless there is a good
justification for new debug log macro.

Thanks,
ferruh

^ permalink raw reply

* Re: [PATCH v12 0/6] add Tx preparation
From: Ananyev, Konstantin @ 2016-12-06 10:56 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: Thomas Monjalon, dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd,
	Jan Medala, Jakub Palider, John Daley, Alejandro Lucero,
	Harish Patil, Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang,
	Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <20161205181021.GT10340@6wind.com>


Hi Adrien,

> 
> On Mon, Dec 05, 2016 at 04:43:52PM +0000, Ananyev, Konstantin wrote:
> [...]
> > > On Fri, Dec 02, 2016 at 01:00:55AM +0000, Ananyev, Konstantin wrote:
> > > [...]
> > > > > On Wed, Nov 30, 2016 at 10:54:50AM +0000, Ananyev, Konstantin wrote:
> > > > > [...]
> > > > > > Do you have anything particular in mind here?
> > > > >
> > > > > Nothing in particular, so for the sake of the argument, let's suppose that I
> > > > > would like to add a field to expose some limitation that only applies to my
> > > > > PMD during TX but looks generic enough to make sense, e.g. maximum packet
> > > > > size when VLAN tagging is requested.
> > > >
> > > > Hmm, I didn't hear about such limitations so far, but if it is real case -
> > > > sure, feel free to submit the patch.
> > >
> > > I won't, that was hypothetical.
> >
> > Then why we discussing it? :)
> 
> Just to make a point, which is that new limitations may appear anytime and
> tx_prepare() can now be used to check for them. First patch of the series
> does it:
> 
>  +   uint16_t nb_seg_max;     /**< Max number of segments per whole packet. */
>  +   uint16_t nb_mtu_seg_max; /**< Max number of segments per one MTU */
> 
> And states that:
> 
>  + * For each packet to send, the rte_eth_tx_prepare() function performs
>  + * the following operations:
>  + *
>  + * - Check if packet meets devices requirements for tx offloads.
>  + *
>  + * - Check limitations about number of segments.
>  + *
>  + * - Check additional requirements when debug is enabled.
>  + *
>  + * - Update and/or reset required checksums when tx offload is set for packet.

I think I already explained in my previous email why I think that
nb_seg_max and nb_mtu_seg_max are not redundant because of tx_prepare().
>From my point they are complement to tx_prepare(): 
Even if people do use tx_prepare() they still should take this information into account.
As an example ixgbe can't TX packets with then 40 segments.
tx_prepare() for ixgbe will flag that issue, but it can't make a decision on user behalf 
what to do in that case: drop the packet, try to coalesce it into the packet with less
number of segments, split the packet into several smaller, etc.
That's up to user to make such decision, and to make it, user might need this information.

> 
> It's like making this function mandatory IMO.

That's probably where confusion starts: I don't think that
tx_prepare() should be mandatory for the user to call.
Yes, it should be a recommended way.
But the user still should have the ability to by-pass it,
if he believes there is no need for it, or he prefers to implement
the same functionality on his own.
As an example,  if the user knows that he is going to send  a group
of one-segment packets that don't require any tx offloads, he can safely skip
tx_prepare() for them.

> 
> > > > > PMDs are free to set that field to some
> > > > > special value (say, 0) if they do not care.
> > > > >
> > > > > Since that field exists however, conscious applications should check its
> > > > > value for each packet that needs to be transmitted. This extra code causes a
> > > > > slowdown just by sitting in the data path. Since it is not the only field in
> > > > > that structure, the performance impact can be significant.

Conscious user will  probably use this information at the stage of packet formation.
He probably has to do this sort of things for large packets anyway:
Check what is the underlying mtu, to decide does he need to split the packet,
or enable tso for it, etc.  

> > > > >
> > > > > Even though this code is inside applications, it remains unfair to PMDs for
> > > > > which these tests are irrelevant. This problem is identified and addressed
> > > > > by tx_prepare().
> > > >
> > > > I suppose the question is why do we need:
> > > > uint16_t nb_seg_max;
> > > > uint16_t nb_mtu_seg_max;
> > > > as we now have tx_prepare(), right?
> > > >
> > > > For two reasons:
> > > > 1. Some people might feel that tx_prepare() is not good (smart/fast) enough
> > > > for them and would prefer to do necessary preparations for TX offloads themselves.
> > > >
> > > > 2. Even if people do use tx_prepare() they still should take this information into accout.
> > > > As an example ixgbe can't TX packets with then 40 segments.
> > > > Obviously ixbge_tx_prep() performs that check and returns an error.
> > >
> > > Problem is that tx_prepare() also provides safeties which are not part of
> > > tx_burst(), such as not going over nb_mtu_seg_max. Because of this and the
> > > fact struct rte_eth_desc_lim can grow new fields anytime, application
> > > developers will be tempted to just call tx_prepare() and focus on more
> > > useful things.
> >
> > NP with that, that was an intention beyond introducing it.
> >
> > > Put another way, from a user's point of view, tx_prepare() is an opaque
> > > function that greatly increases tx_burst()'s ability to send mbufs as
> > > requested, with extra error checking on top; applications not written to run
> > > on a specific PMD/device (all of them ideally) will thus call tx_prepare()
> > > at some point.
> > >
> > > > But it wouldn't try to merge/reallocate mbufs for you.
> > > > User still has to do it himself, or just prevent creating such long chains somehow.
> > >
> > > Yes, that's another debate. PMDs could still implement a software fallback
> > > for unlikely slow events like these. The number of PMDs is not going to
> > > decrease, each device having its own set of weird limitations in specific
> > > cases, PMDs should do their best to process mbufs even if that means slowly
> > > due to the lack of preparation.
> > >
> > > tx_prepare() has its uses but should really be optional, in the sense that
> > > if that function is not called, tx_burst() should deal with it somehow.
> >
> > As I said before, I don't think it is a good idea to put everything in tx_burst().
> > If PMD driver prefer things that way, yes tx_burst() can deal with each and
> > possible offload requirement itself, but it shouldn't be mandatory.
> 
> In effect, having to call tx_prepare() otherwise makes this step mandatory
> anyway. Looks like we are not going to agree here.
> 
> > > > > Thanks to tx_prepare(), these checks are moved back into PMDs where they
> > > > > belong. PMDs that do not need them do not have to provide support for
> > > > > tx_prepare() and do not suffer any performance impact as result;
> > > > > applications only have to make sure tx_prepare() is always called at some
> > > > > point before tx_burst().
> > > > >
> > > > > Once you reach this stage, you've effectively made tx_prepare() mandatory
> > > > > before tx_burst(). If some bug occurs, then perhaps you forgot to call
> > > > > tx_prepare(), you just need to add it. The total cost for doing TX is
> > > > > therefore tx_prepare() + tx_burst().
> > > > >
> > > > > I'm perhaps a bit pessimistic mind you, but I do not think tx_prepare() will
> > > > > remain optional for long. Sure, PMDs that do not implement it do not care,
> > > > > I'm focusing on applications, for which the performance impact of calling
> > > > > tx_prepare() followed by tx_burst() is higher than a single tx_burst()
> > > > > performing all the necessary preparation at once.
> > > > >
> > > > > [...]
> > > > > > > Following the same logic, why can't such a thing be made part of the TX
> > > > > > > burst function as well (through a direct call to rte_phdr_cksum_fix()
> > > > > > > whenever necessary). From an application standpoint, what are the advantages
> > > > > > > of having to:
> > > > > > >
> > > > > > >  if (tx_prep()) // iterate and update mbufs as needed
> > > > > > >      tx_burst(); // iterate and send
> > > > > > >
> > > > > > > Compared to:
> > > > > > >
> > > > > > >  tx_burst(); // iterate, update as needed and send
> > > > > >
> > > > > > I think that was discussed extensively quite a lot previously here:
> > > > > > As Thomas already replied - main motivation is to allow user
> > > > > > to execute them on different stages of packet TX pipeline,
> > > > > > and probably on different cores.
> > > > > > I think that provides better flexibility to the user to when/where
> > > > > > do these preparations and hopefully would lead to better performance.
> > > > >
> > > > > And I agree, I think this use case is valid but does not warrant such a high
> > > > > penalty when your application does not need that much flexibility. Simple
> > > > > (yet conscious) applications need the highest performance. Complex ones as
> > > > > you described already suffer quite a bit from IPCs and won't mind a couple
> > > > > of extra CPU cycles right?
> > > >
> > > > It would mean an extra cache-miss for every packet, so I think performance hit
> > > > would be quite significant.
> > >
> > > A performance hit has to occur somewhere regardless, because something has
> > > to be done in order to send packets that need it. Whether this cost is in
> > > application code or in a PMD function, it remains part of TX.
> >
> > Depending on the place the final cost would differ quite a lot.
> > If you call tx_prepare() somewhere close to the place where you fill the packet header
> > contents, then most likely the data that tx_prepare() has to access will be already in the cache.
> > So the performance penalty will be minimal.
> > If you'll try to access the same data later (at tx_burst), then the possibility that it would still
> > be in cache is much less.
> > If you calling tx_burst() from other core then data would for sure be out of cache,
> > and  even worse can still be in another core cache.
> 
> Well sure, that's why I also think tx_prepare() has its uses, only that
> since tx_prepare() is optional, tx_burst() should provide the same
> functionality when tx_prepare() is not called.

As I understand, to implement what you are proposing (TX_PREPARED mbuf->ol_flag)
it will be required: 

a) Modify all existing applications that do similar to tx_prepare() stuff on their own,
otherwise they'll would hit performance penalty.
b) Modify at least all Intel PMDs and might be some others too (vmxnet3?).

Step b) probably wouldn't cause any significant performance impact straightway,
but it's for sure wouldn't make things faster, and would increase tx_burst() code
complexity quite a lot. 
>From other side, I can't see any real benefit that we will have in return.
So I still opposed to that idea.

> 
> > > > About the 'simple' case when tx_prep() and tx_burst() are called on the same core,
> > > > Why do you believe that:
> > > > tx_prep(); tx_burst(); would be much slower than tx_burst() {tx_prep(), ...}?
> > >
> > > I mean instead of two function calls with their own loops:
> > >
> > >  tx_prepare() { foreach (pkt) { check(); extra_check(); ... } }
> > >
> > >  tx_burst() { foreach (pkt) { check(); stuff(); ... } }
> > >
> > > You end up with one:
> > >
> > >  tx_burst() { foreach (pkt) { check(); extra_check(); stuff(); ... } }
> > >
> > > Which usually is more efficient.
> >
> > I really doubt that.
> > If it would be that, what is the point to process packet in bulks?
> > Usually dividing processing into different stages and at each stage processing
> > multiple packet at once helps to improve performance.
> > At  least for IA.
> > Look for example how we had to change l3fwd to improve its performance.
> 
> Depends quite a bit on usage pattern. It is less efficient for applications
> that do not modify mbuf contents because of the additional function call and
> inner loop.

If the application doesn't modify mbuf contents that it can simply skip calling tx_prepare().

> 
> Note that I'm only pushing for the ability to conveniently address both
> cases with maximum performance.
> 
> > > > tx_prep() itself is quite expensive, let say for Intel HW it includes:
> > > > - read mbuf fileds (2 cache-lines),
> > > > - read packet header (1/2 cache-lines)
> > > > - calculate pseudo-header csum
> > > >  - update packet header
> > > > Comparing to that price of extra function call seems neglectable
> > > > (if we TX packets in bursts of course).
> > >
> > > We agree its performance is a critical issue then, sharing half the read
> > > steps with tx_burst() would make sense to me.
> >
> > I didn't understand that sentence.
> 
> I meant this step can be shared (in addition to loop etc):
> 
>  - read mbuf fileds (2 cache-lines),

Ah ok, you still believe that mixing tx_burst and tx_prepare code together
would give us noticeable performance benefit.
As I said above, I don't think it would, but you are welcome to try and
prove me wrong.

> 
> > > > > Yes they will, therefore we need a method that satisfies both cases.
> > > > >
> > > > > As a possible solution, a special mbuf flag could be added to each mbuf
> > > > > having gone through tx_prepare(). That way, tx_burst() could skip some
> > > > > checks and things it would otherwise have done.
> > > >
> > > > That's an interesting idea, but it has one drawback:
> > > > As I understand, it means that from now on if user doing preparations on his own,
> > > > he had to setup this flag, otherwise tx_burst() would do extra unnecessary work.
> > > > So any existing applications that using TX offloads and do preparation by themselves
> > > > would have to be modified to avoid performance loss.
> > >
> > > In my opinion, users should not do preparation on their own.
> >
> > People already do it now.
> 
> But we do not want them to anymore thanks to this new API, for reasons
> described in the motivation section of the cover letter, right?

We probably wouldn't recommend that, but if people would like to use their own stuff,
or shortcuts - I don't want to stop them here.

> 
> > > If we provide a
> > > generic method, it has to be fast enough to replace theirs. Perhaps not as
> > > fast since it would work with all PMDs (usual trade-off), but acceptably so.
> > >
> > > > > Another possibility, telling the PMD first that you always intend to use
> > > > > tx_prepare() and getting a simpler/faster tx_burst() callback as a result.
> > > >
> > > > That what we have right now (at least for Intel HW):
> > > > it is a user responsibility to do the necessary preparations/checks before calling tx_burst().
> > > > With tx_prepare() we just remove from user the headache to implement tx_prepare() on his own.
> > > > Now he can use a 'proper' PMD provided function.
> > > >
> > > > My vote still would be for that model.
> > >
> > > OK, then in a nutshell:
> > >
> > > 1. Users are not expected to perform preparation/checks themselves anymore,
> > >    if they do, it's their problem.
> >
> > I think we need to be backward compatible here.
> > If the existing app doing what tx_prepare() supposed to do, it should keep working.
> 
> It should work, only if they keep doing it as well as call tx_burst()
> directly, they will likely get lower performance.
> 
> > > 2. If configured through an API to be defined, tx_burst() can be split in
> > >    two and applications must call tx_prepare() at some point before
> > >    tx_burst().
> > >
> > > 3. Otherwise tx_burst() should perform the necessary preparation and checks
> > >    on its own by default (when tx_prepare() is not expected).
> >
> > As I said before, I don't think it should be mandatory for tx_burst() to do what tx_prepare() does.
> > If some particular implementation of tx_burst() prefers to do things that way - that's fine.
> > But it shouldn't be required to.
> 
> You're right, however applications might find it convenient. I think most
> will end up with something like the following:
> 
>  if (tx_prepare(pkts))
>      tx_burst(pkts));

Looking at existing DPDK apps - most of them do use some sort of TX bufferization.
So, even in a simplistic app it would probably be:
  
tx_prepare(pkts);
tx_buffer(pkts);

> 
> > > 4. We probably still need some mbuf flag to mark mbufs that cannot be
> > >    modified, the refcount could also serve as a hint.
> >
> > If mbuf can't be modified, you probably just wouldn't call the function that supposed to do that,
> > tx_prepare() in that case.
> 
> I think it would be easier to document what offload flags may cause the
> tx_burst() function to modify mbuf contents, so applications have the
> ability to set or strip these flags on a mbuf basis. That way there is no
> need to call tx_prepare() without knowing exactly what it's going to do.

Not sure I understand what exactly do you propose in the last paragraph?
Konstantin

^ permalink raw reply

* Re: [PATCH v3] eal/linuxapp: fix return value check of mknod()
From: Thomas Monjalon @ 2016-12-06 11:01 UTC (permalink / raw)
  To: Wei Dai; +Cc: dev, Olivier Matz, david.marchand, stable
In-Reply-To: <20161206114459.0a05cbe0@platinum>

2016-12-06 11:44, Olivier Matz:
> Hi Wei,
> 
> On Thu, 17 Nov 2016 14:47:15 +0800, Wei Dai <wei.dai@intel.com> wrote:
> > In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> > The return value of mknod() is ret, not f got by fopen().
> > So the value of ret should be checked for mknod().
> > 
> > Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without
> > hotplug")

CC: stable@dpdk.org

> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thanks

Applied, thanks

^ permalink raw reply

* [PATCH 1/2] doc: remove chkincs reference
From: Baruch Siach @ 2016-12-06 11:02 UTC (permalink / raw)
  To: dev; +Cc: John McNamara, Baruch Siach

The chkincs app was removed in commit 1b938bb9dfb6 ("app: remove chkincs").

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 doc/guides/prog_guide/source_org.rst | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/guides/prog_guide/source_org.rst b/doc/guides/prog_guide/source_org.rst
index d9c140f704e3..4a4a0a325dd7 100644
--- a/doc/guides/prog_guide/source_org.rst
+++ b/doc/guides/prog_guide/source_org.rst
@@ -129,7 +129,6 @@ The app directory contains sample applications that are used to test DPDK (such
 or the Poll Mode Drivers (test-pmd)::
 
     app
-    +-- chkincs            # Test program to check include dependencies
     +-- cmdline_test       # Test the commandline library
     +-- test               # Autotests to validate DPDK features
     +-- test-acl           # Test the ACL library
-- 
2.10.2

^ permalink raw reply related

* [PATCH 2/2] doc: note that the app list is incomplete
From: Baruch Siach @ 2016-12-06 11:02 UTC (permalink / raw)
  To: dev; +Cc: John McNamara, Baruch Siach
In-Reply-To: <33b0b0e68842ec18280b049c28f785b26871affc.1481022133.git.baruch@tkos.co.il>

Extend the note at the end of Source Organization chapter to also mention
the app directory.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 doc/guides/prog_guide/source_org.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/source_org.rst b/doc/guides/prog_guide/source_org.rst
index 4a4a0a325dd7..8fd98d507008 100644
--- a/doc/guides/prog_guide/source_org.rst
+++ b/doc/guides/prog_guide/source_org.rst
@@ -162,5 +162,5 @@ The examples directory contains sample applications that show how libraries can
 
 .. note::
 
-    The actual examples directory may contain additional sample applications to those shown above.
+    The actual app and examples directories may contain additional sample applications to those shown above.
     Check the latest DPDK source files for details.
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH v2] doc: correct description of attach to indirect mbuf
From: Mcnamara, John @ 2016-12-06 11:03 UTC (permalink / raw)
  To: Baruch Siach, dev@dpdk.org; +Cc: Olivier Matz
In-Reply-To: <824b47fc043902610646e26a8b053199620744da.1481012725.git.baruch@tkos.co.il>



> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Tuesday, December 6, 2016 8:25 AM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; Baruch Siach <baruch@tkos.co.il>
> Subject: [PATCH v2] doc: correct description of attach to indirect mbuf
> 
> Since commit 4a96e8b652181 ("mbuf: allow to clone an indirect mbuf")
> attaching to an indirect mbuf creates a clone of that mbuf. Update the
> mbuf documentation to match.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: fix checkpatch issues: long commit log lines, short commit id
> ---
>  doc/guides/prog_guide/mbuf_lib.rst | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/prog_guide/mbuf_lib.rst
> b/doc/guides/prog_guide/mbuf_lib.rst
> index f0aa21a6d4aa..6e73fc5a834b 100644
> --- a/doc/guides/prog_guide/mbuf_lib.rst
> +++ b/doc/guides/prog_guide/mbuf_lib.rst
> @@ -253,7 +253,8 @@ Similarly, whenever the indirect buffer is detached,
> the reference counter on th  If the resulting reference counter is equal
> to 0, the direct buffer is freed since it is no longer in use.
> 
>  There are a few things to remember when dealing with indirect buffers.
> -First of all, it is not possible to attach an indirect buffer to another
> indirect buffer.
> +First of all, an indirect buffer is never attached to another indirect
> buffer.
> +Attempting to attach buffer A to indirect buffer B that is attached to C,
> makes rte_pktmbuf_attach() automatically attach A to C, effectively
> cloning B.

Thanks, please keep the doc fixes coming.

It would be better to quote the function in fixed width text like: ``rte_pktmbuf_attach()``.

^ permalink raw reply

* Re: [PATCH v2] doc: correct description of attach to indirect mbuf
From: Baruch Siach @ 2016-12-06 11:11 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev@dpdk.org, Olivier Matz
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20266F431@IRSMSX103.ger.corp.intel.com>

Hi John,

On Tue, Dec 06, 2016 at 11:03:18AM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: Baruch Siach [mailto:baruch@tkos.co.il]
> > Sent: Tuesday, December 6, 2016 8:25 AM
> > To: dev@dpdk.org
> > Cc: Mcnamara, John <john.mcnamara@intel.com>; Olivier Matz
> > <olivier.matz@6wind.com>; Baruch Siach <baruch@tkos.co.il>
> > Subject: [PATCH v2] doc: correct description of attach to indirect mbuf
> > 
> > Since commit 4a96e8b652181 ("mbuf: allow to clone an indirect mbuf")
> > attaching to an indirect mbuf creates a clone of that mbuf. Update the
> > mbuf documentation to match.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > v2: fix checkpatch issues: long commit log lines, short commit id
> > ---
> >  doc/guides/prog_guide/mbuf_lib.rst | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/guides/prog_guide/mbuf_lib.rst
> > b/doc/guides/prog_guide/mbuf_lib.rst
> > index f0aa21a6d4aa..6e73fc5a834b 100644
> > --- a/doc/guides/prog_guide/mbuf_lib.rst
> > +++ b/doc/guides/prog_guide/mbuf_lib.rst
> > @@ -253,7 +253,8 @@ Similarly, whenever the indirect buffer is detached,
> > the reference counter on th  If the resulting reference counter is equal
> > to 0, the direct buffer is freed since it is no longer in use.
> > 
> >  There are a few things to remember when dealing with indirect buffers.
> > -First of all, it is not possible to attach an indirect buffer to another
> > indirect buffer.
> > +First of all, an indirect buffer is never attached to another indirect
> > buffer.
> > +Attempting to attach buffer A to indirect buffer B that is attached to C,
> > makes rte_pktmbuf_attach() automatically attach A to C, effectively
> > cloning B.
> 
> Thanks, please keep the doc fixes coming.
> 
> It would be better to quote the function in fixed width text like: 
> ``rte_pktmbuf_attach()``.

None of the function names mentioned in this chapter are quoted. I can post a 
fix for that, but I think it should be a separate patch.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* Re: [PATCH] net: introduce big and little endian types
From: Ananyev, Konstantin @ 2016-12-06 11:23 UTC (permalink / raw)
  To: Nélio Laranjeiro
  Cc: dev@dpdk.org, Olivier Matz, Lu, Wenzhuo, Adrien Mazarguil
In-Reply-To: <20161205120603.GL21794@autoinstall.dev.6wind.com>

Hi Neilo,


Hi Neilo,
> > >
> > > This commit introduces new rte_{le,be}{16,32,64}_t types and updates
> > > rte_{le,be,cpu}_to_{le,be,cpu}_*() and network header structures
> > > accordingly.
> > >
> > > Specific big/little endian types avoid uncertainty and conversion mistakes.
> > >
> > > No ABI change since these are simply typedefs to the original types.
> >
> > It seems like quite a lot of changes...
> > Could you probably explain what will be the benefit in return?
> > Konstantin
> 
> Hi Konstantin,
> 
> The benefit is to provide documented byte ordering for data types
> software is manipulating to determine when network to CPU (or CPU to
> network) conversion must be performed.

Ok, but is it really worth it?
User can still make a mistake and forget to call ntoh()/hton() at some particular place.
>From other side most people do know that network protocols headers are usually in BE format. 
I would understand the effort, if we'll have some sort of tool that would do some sort of static code analysis
based on these special types or so.
Again, does it mean that we should go and change uint32_t to rte_le_32 inside all Intel PMDs
(and might be  in some others too) to be consistent?
Konstantin

> 
> Regards,
> 
> --
> Nélio Laranjeiro
> 6WIND

^ permalink raw reply

* [PATCH] doc: remove the repeat words in documentation.rst
From: Yong Wang @ 2016-12-06 12:30 UTC (permalink / raw)
  To: john.mcnamara; +Cc: dev, Yong Wang

Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
---
 doc/guides/contributing/documentation.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 2cfb1a2..5ec0675 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -332,7 +332,7 @@ Whitespace
 Section Headers
 ~~~~~~~~~~~~~~~
 
-* Section headers should use the use the following underline formats::
+* Section headers should use the following underline formats::
 
    Level 1 Heading
    ===============
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] doc: remove the repeat words in documentation.rst
From: Mcnamara, John @ 2016-12-06 11:33 UTC (permalink / raw)
  To: Yong Wang; +Cc: dev@dpdk.org
In-Reply-To: <1481027449-22987-1-git-send-email-wang.yong19@zte.com.cn>



> -----Original Message-----
> From: Yong Wang [mailto:wang.yong19@zte.com.cn]
> Sent: Tuesday, December 6, 2016 12:31 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Yong Wang <wang.yong19@zte.com.cn>
> Subject: [PATCH] doc: remove the repeat words in documentation.rst
> 
> Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>

Good catch.


Acked-by: John McNamara <john.mcnamara@intel.com> 

^ permalink raw reply

* Re: [PATCH v2] doc: correct description of attach to indirect mbuf
From: Mcnamara, John @ 2016-12-06 11:35 UTC (permalink / raw)
  To: Baruch Siach; +Cc: dev@dpdk.org, Olivier Matz
In-Reply-To: <20161206111105.g6u2bjbhcgnl5kfs@tarshish>

> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Tuesday, December 6, 2016 11:11 AM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>
> Subject: Re: [PATCH v2] doc: correct description of attach to indirect
> mbuf
> 
> ...
> > Thanks, please keep the doc fixes coming.
> >
> > It would be better to quote the function in fixed width text like:
> > ``rte_pktmbuf_attach()``.
> 
> None of the function names mentioned in this chapter are quoted. I can
> post a fix for that, but I think it should be a separate patch.
> 

Unfortunately, the documentation isn't very consistent in this. You can
ignore it for this patch.

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply

* Re: [PATCH 2/2] doc: note that the app list is incomplete
From: Mcnamara, John @ 2016-12-06 11:41 UTC (permalink / raw)
  To: Baruch Siach, dev@dpdk.org
In-Reply-To: <bca19b108b04899e64d20c9668982edfcc6a1b80.1481022133.git.baruch@tkos.co.il>



> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Tuesday, December 6, 2016 11:02 AM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Baruch Siach
> <baruch@tkos.co.il>
> Subject: [PATCH 2/2] doc: note that the app list is incomplete
> 
> Extend the note at the end of Source Organization chapter to also mention
> the app directory.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  doc/guides/prog_guide/source_org.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/guides/prog_guide/source_org.rst
> b/doc/guides/prog_guide/source_org.rst
> index 4a4a0a325dd7..8fd98d507008 100644
> --- a/doc/guides/prog_guide/source_org.rst
> +++ b/doc/guides/prog_guide/source_org.rst
> @@ -162,5 +162,5 @@ The examples directory contains sample applications
> that show how libraries can
> 
>  .. note::
> 
> -    The actual examples directory may contain additional sample
> applications to those shown above.
> +    The actual app and examples directories may contain additional sample
> applications to those shown above.
>      Check the latest DPDK source files for details.

Strictly speaking the applications in the "app" directory aren't "sample" apps
and also they don't change very often. So I would leave this note as it is or
change it to:

The actual app and examples directories may contain additional applications to those shown above.

^ permalink raw reply

* Re: [PATCH 0/2] Event queue exception handling in Solarflare PMD
From: Ferruh Yigit @ 2016-12-06 11:42 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
In-Reply-To: <1480664481-26427-1-git-send-email-arybchenko@solarflare.com>

On 12/2/2016 7:41 AM, Andrew Rybchenko wrote:
> The patch series adds event queue exception handling.
> 
> It is not required when everything goes fine, but has chances to
> recover if something goes wrong in either driver or HW.
> 
> The patch series should be applied after
> [PATCH v2 00/55] Solarflare libefx-based PMD
> (Message-ID: 1480436367-20749-1-git-send-email-arybchenko@solarflare.com)
> 
> 
> Andrew Rybchenko (1):
>   net/sfc: restart RxQ in case of exception on its event queue
> 
> Ivan Malov (1):
>   net/sfc: restart TxQ in case of exception on its event queue
> 
>  drivers/net/sfc/sfc.h    |  6 ++++++
>  drivers/net/sfc/sfc_ev.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox