* [PATCH v3 21/29] app/testpmd: add command to test VF broadcast mode on i40e
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Bernard Iremonger
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: Bernard Iremonger <bernard.iremonger@intel.com>
Add command to call rte_pmd_i40e_set_vf_broadcast.
Add set vf broadcast in testpmd_funcs.rst file.
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
app/test-pmd/cmdline.c | 85 +++++++++++++++++++++++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 92 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 27df75c..48a6f59 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -277,6 +277,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"set vf mac antispoof (port_id) (vf_id) (on|off).\n"
" Set MAC antispoof for a VF from the PF.\n\n"
+ "set vf broadcast (port_id) (vf_id) (on|off)\n"
+ " Set VF broadcast for a VF from the PF.\n\n"
+
"vlan set strip (on|off) (port_id)\n"
" Set the VLAN strip on a port.\n\n"
@@ -11734,6 +11737,87 @@ cmdline_parse_inst_t cmd_set_vf_allmulti = {
},
};
+/* vf broadcast mode configuration */
+
+/* Common result structure for vf broadcast */
+struct cmd_set_vf_broadcast_result {
+ cmdline_fixed_string_t set;
+ cmdline_fixed_string_t vf;
+ cmdline_fixed_string_t broadcast;
+ uint8_t port_id;
+ uint16_t vf_id;
+ cmdline_fixed_string_t on_off;
+};
+
+/* Common CLI fields for vf broadcast enable disable */
+cmdline_parse_token_string_t cmd_set_vf_broadcast_set =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ set, "set");
+cmdline_parse_token_string_t cmd_set_vf_broadcast_vf =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ vf, "vf");
+cmdline_parse_token_string_t cmd_set_vf_broadcast_broadcast =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ broadcast, "broadcast");
+cmdline_parse_token_num_t cmd_set_vf_broadcast_port_id =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ port_id, UINT8);
+cmdline_parse_token_num_t cmd_set_vf_broadcast_vf_id =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ vf_id, UINT16);
+cmdline_parse_token_string_t cmd_set_vf_broadcast_on_off =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_broadcast_result,
+ on_off, "on#off");
+
+static void
+cmd_set_vf_broadcast_parsed(
+ void *parsed_result,
+ __attribute__((unused)) struct cmdline *cl,
+ __attribute__((unused)) void *data)
+{
+ struct cmd_set_vf_broadcast_result *res = parsed_result;
+ int ret;
+ int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+ return;
+
+ ret = rte_pmd_i40e_set_vf_broadcast(res->port_id, res->vf_id, is_on);
+ switch (ret) {
+ case 0:
+ break;
+ case -EINVAL:
+ printf("invalid vf_id %d or is_on %d\n", res->vf_id, is_on);
+ break;
+ case -ENODEV:
+ printf("invalid port_id %d\n", res->port_id);
+ break;
+ default:
+ printf("programming error: (%s)\n", strerror(-ret));
+ }
+}
+
+cmdline_parse_inst_t cmd_set_vf_broadcast = {
+ .f = cmd_set_vf_broadcast_parsed,
+ .data = NULL,
+ .help_str = "set vf broadcast port_id vf_id on|off",
+ .tokens = {
+ (void *)&cmd_set_vf_broadcast_set,
+ (void *)&cmd_set_vf_broadcast_vf,
+ (void *)&cmd_set_vf_broadcast_broadcast,
+ (void *)&cmd_set_vf_broadcast_port_id,
+ (void *)&cmd_set_vf_broadcast_vf_id,
+ (void *)&cmd_set_vf_broadcast_on_off,
+ NULL,
+ },
+};
+
/* ******************************************************************************** */
/* list of instructions */
@@ -11900,6 +11984,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
(cmdline_parse_inst_t *)&cmd_set_vf_promisc,
(cmdline_parse_inst_t *)&cmd_set_vf_allmulti,
+ (cmdline_parse_inst_t *)&cmd_set_vf_broadcast,
NULL,
};
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 45c5902..823db7e 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -507,6 +507,13 @@ Set mac antispoof for a VF from the PF::
testpmd> set vf mac antispoof (port_id) (vf_id) (on|off)
+set broadcast mode (for VF)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Set broadcast mode for a VF from the PF::
+
+ testpmd> set vf broadcast (port_id) (vf_id) (on|off)
+
vlan set strip
~~~~~~~~~~~~~~
--
2.7.4
^ permalink raw reply related
* [PATCH v3 22/29] app/testpmd: add command to test VF VLAN tag on i40e
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Bernard Iremonger
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: Bernard Iremonger <bernard.iremonger@intel.com>
command is: set vf vlan tag port_id vf_id on|off
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
app/test-pmd/cmdline.c | 91 +++++++++++++++++++++++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 98 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 48a6f59..a38ba92 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -295,6 +295,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"set vf vlan antispoof (port_id) (vf_id) (on|off)\n"
" Set VLAN antispoof for a VF from the PF.\n\n"
+ "set vf vlan tag (port_id) (vf_id) (on|off)\n"
+ " Set VLAN tag for a VF from the PF.\n\n"
+
"vlan set filter (on|off) (port_id)\n"
" Set the VLAN filter on a port.\n\n"
@@ -11818,6 +11821,93 @@ cmdline_parse_inst_t cmd_set_vf_broadcast = {
},
};
+/* vf vlan tag configuration */
+
+/* Common result structure for vf vlan tag */
+struct cmd_set_vf_vlan_tag_result {
+ cmdline_fixed_string_t set;
+ cmdline_fixed_string_t vf;
+ cmdline_fixed_string_t vlan;
+ cmdline_fixed_string_t tag;
+ uint8_t port_id;
+ uint16_t vf_id;
+ cmdline_fixed_string_t on_off;
+};
+
+/* Common CLI fields for vf vlan tag enable disable */
+cmdline_parse_token_string_t cmd_set_vf_vlan_tag_set =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ set, "set");
+cmdline_parse_token_string_t cmd_set_vf_vlan_tag_vf =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ vf, "vf");
+cmdline_parse_token_string_t cmd_set_vf_vlan_tag_vlan =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ vlan, "vlan");
+cmdline_parse_token_string_t cmd_set_vf_vlan_tag_tag =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ tag, "tag");
+cmdline_parse_token_num_t cmd_set_vf_vlan_tag_port_id =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ port_id, UINT8);
+cmdline_parse_token_num_t cmd_set_vf_vlan_tag_vf_id =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ vf_id, UINT16);
+cmdline_parse_token_string_t cmd_set_vf_vlan_tag_on_off =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_set_vf_vlan_tag_result,
+ on_off, "on#off");
+
+static void
+cmd_set_vf_vlan_tag_parsed(
+ void *parsed_result,
+ __attribute__((unused)) struct cmdline *cl,
+ __attribute__((unused)) void *data)
+{
+ struct cmd_set_vf_vlan_tag_result *res = parsed_result;
+ int ret;
+ int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+
+ if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+ return;
+
+ ret = rte_pmd_i40e_set_vf_vlan_tag(res->port_id, res->vf_id, is_on);
+ switch (ret) {
+ case 0:
+ break;
+ case -EINVAL:
+ printf("invalid vf_id %d or is_on %d\n", res->vf_id, is_on);
+ break;
+ case -ENODEV:
+ printf("invalid port_id %d\n", res->port_id);
+ break;
+ default:
+ printf("programming error: (%s)\n", strerror(-ret));
+ }
+}
+
+cmdline_parse_inst_t cmd_set_vf_vlan_tag = {
+ .f = cmd_set_vf_vlan_tag_parsed,
+ .data = NULL,
+ .help_str = "set vf vlan tag port_id vf_id on|off",
+ .tokens = {
+ (void *)&cmd_set_vf_vlan_tag_set,
+ (void *)&cmd_set_vf_vlan_tag_vf,
+ (void *)&cmd_set_vf_vlan_tag_vlan,
+ (void *)&cmd_set_vf_vlan_tag_tag,
+ (void *)&cmd_set_vf_vlan_tag_port_id,
+ (void *)&cmd_set_vf_vlan_tag_vf_id,
+ (void *)&cmd_set_vf_vlan_tag_on_off,
+ NULL,
+ },
+};
+
/* ******************************************************************************** */
/* list of instructions */
@@ -11985,6 +12075,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_set_vf_promisc,
(cmdline_parse_inst_t *)&cmd_set_vf_allmulti,
(cmdline_parse_inst_t *)&cmd_set_vf_broadcast,
+ (cmdline_parse_inst_t *)&cmd_set_vf_vlan_tag,
NULL,
};
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 823db7e..84805af 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -542,6 +542,13 @@ Set VLAN insert for a VF from the PF::
testpmd> set vf vlan insert (port_id) (vf_id) (vlan_id)
+vlan set tag (for VF)
+~~~~~~~~~~~~~~~~~~~~~
+
+Set VLAN tag for a VF from the PF::
+
+ testpmd> set vf vlan tag (port_id) (vf_id) (on|off)
+
vlan set antispoof (for VF)
~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.7.4
^ permalink raw reply related
* [PATCH v3 23/29] app/testpmd: handle i40e in VF VLAN filter command
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Bernard Iremonger
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: Bernard Iremonger <bernard.iremonger@intel.com>
modify set_vf_rx_vlan function to handle the i40e PMD.
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
app/test-pmd/config.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index fc0424a..c5da9b1 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -92,6 +92,9 @@
#include <rte_ethdev.h>
#include <rte_string_fns.h>
#include <rte_cycles.h>
+#ifdef RTE_LIBRTE_I40E_PMD
+#include <rte_pmd_i40e.h>
+#endif
#ifdef RTE_LIBRTE_IXGBE_PMD
#include <rte_pmd_ixgbe.h>
#endif
@@ -2353,12 +2356,22 @@ void
set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
{
int diag;
+ struct rte_eth_dev_info dev_info;
+
+ rte_eth_dev_info_get(port_id, &dev_info);
- diag = rte_pmd_ixgbe_set_vf_vlan_filter(port_id, vlan_id, vf_mask, on);
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ diag = rte_pmd_i40e_set_vf_vlan_filter(port_id, vlan_id,
+ vf_mask, on);
+ else
+#endif
+ diag = rte_pmd_ixgbe_set_vf_vlan_filter(port_id, vlan_id,
+ vf_mask, on);
if (diag == 0)
return;
- printf("rte_pmd_ixgbe_set_vf_vlan_filter for port_id=%d failed "
+ printf("rte_pmd_xxx_set_vf_vlan_filter for port_id=%d failed "
"diag=%d\n", port_id, diag);
}
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH v3 24/29] net/i40e: change version number to support Linux VF
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Chen Jing D(Mark)
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
i40e PF host only support to work with DPDK VF driver, Linux
VF driver is not supported. This change will enhance in version
number returned.
Current version info returned won't be able to be recognized
by Linux VF driver, change to values that both DPDK VF and Linux
driver can recognize.
The expense is original DPDK host specific feature like
CFG_VLAN_PVID and CONFIG_VSI_QUEUES_EXT will not available.
DPDK VF also can't identify host driver by version number returned.
It always assume talking with Linux PF.
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
drivers/net/i40e/i40e_pf.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 2bc3355..0f582ed 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -279,8 +279,19 @@ i40e_pf_host_process_cmd_version(struct i40e_pf_vf *vf, bool b_op)
{
struct i40e_virtchnl_version_info info;
- info.major = I40E_DPDK_VERSION_MAJOR;
- info.minor = I40E_DPDK_VERSION_MINOR;
+ /* Respond like a Linux PF host in order to support both DPDK VF and
+ * Linux VF driver. The expense is original DPDK host specific feature
+ * like CFG_VLAN_PVID and CONFIG_VSI_QUEUES_EXT will not available.
+ *
+ * DPDK VF also can't identify host driver by version number returned.
+ * It always assume talking with Linux PF.
+ *
+ * TODO:
+ * Discuss with Linux driver maintainer if possible to carry more info
+ * in this function to identify it's Linux or DPDK host.
+ */
+ info.major = I40E_VIRTCHNL_VERSION_MAJOR;
+ info.minor = I40E_VIRTCHNL_VERSION_MINOR_NO_VF_CAPS;
if (b_op)
i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_VERSION,
--
2.7.4
^ permalink raw reply related
* [PATCH v3 25/29] net/i40e: return correct VSI id
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Chen Jing D(Mark)
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
PF host didn't return correct VSI id to VF.
This change fix it.
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
drivers/net/i40e/i40e_pf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 0f582ed..8319c2c 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -351,8 +351,7 @@ i40e_pf_host_process_cmd_get_vf_resource(struct i40e_pf_vf *vf, bool b_op)
/* Change below setting if PF host can support more VSIs for VF */
vf_res->vsi_res[0].vsi_type = I40E_VSI_SRIOV;
- /* As assume Vf only has single VSI now, always return 0 */
- vf_res->vsi_res[0].vsi_id = 0;
+ vf_res->vsi_res[0].vsi_id = vf->vsi->vsi_id;
vf_res->vsi_res[0].num_queue_pairs = vf->vsi->nb_qps;
ether_addr_copy(&vf->mac_addr,
(struct ether_addr *)vf_res->vsi_res[0].default_mac_addr);
--
2.7.4
^ permalink raw reply related
* [PATCH v3 26/29] net/i40e: parse more VF parameter and configure
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Chen Jing D(Mark)
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
When VF requested to configure TX queue, a few parameters are
missed to be configured in PF host. This change have more
fields parsed and configured for TX context.
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
drivers/net/i40e/i40e_pf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 8319c2c..1ad5ed1 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -422,10 +422,12 @@ i40e_pf_host_hmc_config_txq(struct i40e_hw *hw,
/* clear the context structure first */
memset(&tx_ctx, 0, sizeof(tx_ctx));
- tx_ctx.new_context = 1;
tx_ctx.base = txq->dma_ring_addr / I40E_QUEUE_BASE_ADDR_UNIT;
tx_ctx.qlen = txq->ring_len;
tx_ctx.rdylist = rte_le_to_cpu_16(vf->vsi->info.qs_handle[0]);
+ tx_ctx.head_wb_ena = txq->headwb_enabled;
+ tx_ctx.head_wb_addr = txq->dma_headwb_addr;
+
err = i40e_clear_lan_tx_queue_context(hw, abs_queue_id);
if (err != I40E_SUCCESS)
return err;
--
2.7.4
^ permalink raw reply related
* [PATCH v3 27/29] net/i40e: support Linux VF to configure IRQ link list
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Chen Jing D(Mark)
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
i40e PF host only support to work with DPDK VF driver, Linux
VF driver is not supported. This change will enhance in
configuring IRQ link list.
This Change will identify VF client by number of vector
requested. DPDK VF will ask only single one while Linux VF
will request at least 2. It will have different configuration
for different clients. DPDK VF will be configured to link all
queue together, while Linux VF will be configured per request.
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
drivers/net/i40e/i40e_pf.c | 151 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 138 insertions(+), 13 deletions(-)
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 1ad5ed1..71e6a3f 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -585,14 +585,116 @@ i40e_pf_host_process_cmd_config_vsi_queues_ext(struct i40e_pf_vf *vf,
return ret;
}
+static void
+i40e_pf_config_irq_link_list(struct i40e_pf_vf *vf,
+ struct i40e_virtchnl_vector_map *vvm)
+{
+ uint64_t linklistmap = 0, tempmap;
+ struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
+ uint16_t qid;
+ bool b_first_q = true;
+ enum i40e_queue_type qtype;
+ uint16_t vector_id;
+ uint32_t reg, reg_idx;
+ uint16_t itr_idx = 0, i;
+
+ vector_id = vvm->vector_id;
+ /* setup the head */
+ if (!vector_id)
+ reg_idx = I40E_VPINT_LNKLST0(vf->vf_idx);
+ else
+ reg_idx = I40E_VPINT_LNKLSTN(
+ ((hw->func_caps.num_msix_vectors_vf - 1) * vf->vf_idx)
+ + (vector_id - 1));
+
+ if (vvm->rxq_map == 0 && vvm->txq_map == 0) {
+ I40E_WRITE_REG(hw, reg_idx,
+ I40E_VPINT_LNKLST0_FIRSTQ_INDX_MASK);
+ goto cfg_irq_done;
+ }
+
+ /* sort all rx and tx queues */
+ tempmap = vvm->rxq_map;
+ for (i = 0; i < sizeof(vvm->rxq_map) * 8; i++) {
+ if (tempmap & 0x1)
+ linklistmap |= (1 << (2 * i));
+ tempmap >>= 1;
+ }
+
+ tempmap = vvm->txq_map;
+ for (i = 0; i < sizeof(vvm->txq_map) * 8; i++) {
+ if (tempmap & 0x1)
+ linklistmap |= (1 << (2 * i + 1));
+ tempmap >>= 1;
+ }
+
+ /* Link all rx and tx queues into a chained list */
+ tempmap = linklistmap;
+ i = 0;
+ b_first_q = true;
+ do {
+ if (tempmap & 0x1) {
+ qtype = (enum i40e_queue_type)(i % 2);
+ qid = vf->vsi->base_queue + i / 2;
+ if (b_first_q) {
+ /* This is header */
+ b_first_q = false;
+ reg = ((qtype <<
+ I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)
+ | qid);
+ } else {
+ /* element in the link list */
+ reg = (vector_id) |
+ (qtype << I40E_QINT_RQCTL_NEXTQ_TYPE_SHIFT) |
+ (qid << I40E_QINT_RQCTL_NEXTQ_INDX_SHIFT) |
+ BIT(I40E_QINT_RQCTL_CAUSE_ENA_SHIFT) |
+ (itr_idx << I40E_QINT_RQCTL_ITR_INDX_SHIFT);
+ }
+ I40E_WRITE_REG(hw, reg_idx, reg);
+ /* find next register to program */
+ switch (qtype) {
+ case I40E_QUEUE_TYPE_RX:
+ reg_idx = I40E_QINT_RQCTL(qid);
+ itr_idx = vvm->rxitr_idx;
+ break;
+ case I40E_QUEUE_TYPE_TX:
+ reg_idx = I40E_QINT_TQCTL(qid);
+ itr_idx = vvm->txitr_idx;
+ break;
+ default:
+ break;
+ }
+ }
+ i++;
+ tempmap >>= 1;
+ } while (tempmap);
+
+ /* Terminate the link list */
+ reg = (vector_id) |
+ (0 << I40E_QINT_RQCTL_NEXTQ_TYPE_SHIFT) |
+ (0x7FF << I40E_QINT_RQCTL_NEXTQ_INDX_SHIFT) |
+ BIT(I40E_QINT_RQCTL_CAUSE_ENA_SHIFT) |
+ (itr_idx << I40E_QINT_RQCTL_ITR_INDX_SHIFT);
+ I40E_WRITE_REG(hw, reg_idx, reg);
+
+cfg_irq_done:
+ I40E_WRITE_FLUSH(hw);
+}
+
static int
i40e_pf_host_process_cmd_config_irq_map(struct i40e_pf_vf *vf,
uint8_t *msg, uint16_t msglen,
bool b_op)
{
int ret = I40E_SUCCESS;
+ struct i40e_pf *pf = vf->pf;
+ struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
struct i40e_virtchnl_irq_map_info *irqmap =
(struct i40e_virtchnl_irq_map_info *)msg;
+ struct i40e_virtchnl_vector_map *map;
+ int i;
+ uint16_t vector_id;
+ unsigned long qbit_max;
if (!b_op) {
i40e_pf_host_send_msg_to_vf(
@@ -608,23 +710,46 @@ i40e_pf_host_process_cmd_config_irq_map(struct i40e_pf_vf *vf,
goto send_msg;
}
- /* Assume VF only have 1 vector to bind all queues */
- if (irqmap->num_vectors != 1) {
- PMD_DRV_LOG(ERR, "DKDK host only support 1 vector");
- ret = I40E_ERR_PARAM;
+ /* PF host will support both DPDK VF or Linux VF driver, identify by
+ * number of vectors requested.
+ */
+
+ /* DPDK VF only requires single vector */
+ if (irqmap->num_vectors == 1) {
+ /* This MSIX intr store the intr in VF range */
+ vf->vsi->msix_intr = irqmap->vecmap[0].vector_id;
+ vf->vsi->nb_msix = irqmap->num_vectors;
+ vf->vsi->nb_used_qps = vf->vsi->nb_qps;
+
+ /* Don't care how the TX/RX queue mapping with this vector.
+ * Link all VF RX queues together. Only did mapping work.
+ * VF can disable/enable the intr by itself.
+ */
+ i40e_vsi_queues_bind_intr(vf->vsi);
goto send_msg;
}
- /* This MSIX intr store the intr in VF range */
- vf->vsi->msix_intr = irqmap->vecmap[0].vector_id;
- vf->vsi->nb_msix = irqmap->num_vectors;
- vf->vsi->nb_used_qps = vf->vsi->nb_qps;
+ /* Then, it's Linux VF driver */
+ qbit_max = 1 << pf->vf_nb_qp_max;
+ for (i = 0; i < irqmap->num_vectors; i++) {
+ map = &irqmap->vecmap[i];
+
+ vector_id = map->vector_id;
+ /* validate msg params */
+ if (vector_id >= hw->func_caps.num_msix_vectors_vf) {
+ ret = I40E_ERR_PARAM;
+ goto send_msg;
+ }
+
+ if ((map->rxq_map < qbit_max) && (map->txq_map < qbit_max)) {
+ i40e_pf_config_irq_link_list(vf, map);
+ } else {
+ /* configured queue size excceed limit */
+ ret = I40E_ERR_PARAM;
+ goto send_msg;
+ }
+ }
- /* Don't care how the TX/RX queue mapping with this vector.
- * Link all VF RX queues together. Only did mapping work.
- * VF can disable/enable the intr by itself.
- */
- i40e_vsi_queues_bind_intr(vf->vsi);
send_msg:
i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_CONFIG_IRQ_MAP,
ret, NULL, 0);
--
2.7.4
^ permalink raw reply related
* [PATCH v3 28/29] net/i40e: enhance in sanity check of MAC
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Chen Jing D(Mark)
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
When VF sends request to add a new MAC address, PF host
will check if it's a non-zero or unicast address, or it
will return with error. In fact, VF still can set multicast
address. This change remove to check if it's a unicast
address.
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
drivers/net/i40e/i40e_pf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 71e6a3f..79f4295 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -890,8 +890,8 @@ i40e_pf_host_process_cmd_add_ether_address(struct i40e_pf_vf *vf,
mac = (struct ether_addr *)(addr_list->list[i].addr);
(void)rte_memcpy(&filter.mac_addr, mac, ETHER_ADDR_LEN);
filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
- if(!is_valid_assigned_ether_addr(mac) ||
- i40e_vsi_add_mac(vf->vsi, &filter)) {
+ if (is_zero_ether_addr(mac) ||
+ i40e_vsi_add_mac(vf->vsi, &filter)) {
ret = I40E_ERR_INVALID_MAC_ADDR;
goto send_msg;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 29/29] net/i40e: set/clear VF stats from PF
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>
This patch add support to get/clear VF statistics
from PF side.
Two APIs are added:
rte_pmd_i40e_get_vf_stats.
rte_pmd_i40e_reset_vf_stats.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 76 +++++++++++++++++++++++++++++++
drivers/net/i40e/rte_pmd_i40e.h | 41 +++++++++++++++++
drivers/net/i40e/rte_pmd_i40e_version.map | 2 +
3 files changed, 119 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5ee010e..080eff2 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10462,3 +10462,79 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
return ret;
}
+
+int
+rte_pmd_i40e_get_vf_stats(uint8_t port,
+ uint16_t vf_id,
+ struct rte_eth_stats *stats)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct i40e_pf *pf;
+ struct i40e_vsi *vsi;
+ int ret = 0;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ rte_eth_dev_info_get(port, &dev_info);
+
+ if (vf_id >= dev_info.max_vfs)
+ return -EINVAL;
+
+ pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+ if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+ PMD_DRV_LOG(ERR, "Invalid argument.");
+ return -EINVAL;
+ }
+
+ vsi = pf->vfs[vf_id].vsi;
+ i40e_update_vsi_stats(vsi);
+
+ stats->ipackets = vsi->eth_stats.rx_unicast +
+ vsi->eth_stats.rx_multicast +
+ vsi->eth_stats.rx_broadcast;
+ stats->opackets = vsi->eth_stats.tx_unicast +
+ vsi->eth_stats.tx_multicast +
+ vsi->eth_stats.tx_broadcast;
+ stats->ibytes = vsi->eth_stats.rx_bytes;
+ stats->obytes = vsi->eth_stats.tx_bytes;
+ stats->ierrors = vsi->eth_stats.rx_discards;
+ stats->oerrors = vsi->eth_stats.tx_errors + vsi->eth_stats.tx_discards;
+
+ return ret;
+}
+
+int
+rte_pmd_i40e_reset_vf_stats(uint8_t port,
+ uint16_t vf_id)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct i40e_pf *pf;
+ struct i40e_vsi *vsi;
+ int ret = 0;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ rte_eth_dev_info_get(port, &dev_info);
+
+ if (vf_id >= dev_info.max_vfs)
+ return -EINVAL;
+
+ pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+ if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+ PMD_DRV_LOG(ERR, "Invalid argument.");
+ return -EINVAL;
+ }
+
+ vsi = pf->vfs[vf_id].vsi;
+
+ vsi->offset_loaded = false;
+ i40e_update_vsi_stats(vsi);
+
+ return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 96cda89..2bda36d 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -284,4 +284,45 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t port, uint16_t vf_id, uint8_t on);
int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
uint64_t vf_mask, uint8_t on);
+/**
+ * Get VF's statistics
+ *
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param vf_id
+ * VF on which to get.
+ * @param stats
+ * A pointer to a structure of type *rte_eth_stats* to be filled with
+ * the values of device counters for the following set of statistics:
+ * - *ipackets* with the total of successfully received packets.
+ * - *opackets* with the total of successfully transmitted packets.
+ * - *ibytes* with the total of successfully received bytes.
+ * - *obytes* with the total of successfully transmitted bytes.
+ * - *ierrors* with the total of erroneous received packets.
+ * - *oerrors* with the total of failed transmitted packets.
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if *port* invalid.
+ * - (-EINVAL) if bad parameter.
+ */
+
+int rte_pmd_i40e_get_vf_stats(uint8_t port,
+ uint16_t vf_id,
+ struct rte_eth_stats *stats);
+
+/**
+ * Clear VF's statistics
+ *
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param vf_id
+ * VF on which to get.
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if *port* invalid.
+ * - (-EINVAL) if bad parameter.
+ */
+int rte_pmd_i40e_reset_vf_stats(uint8_t port,
+ uint16_t vf_id);
+
#endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 818ff9c..e4dff0b 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -18,5 +18,7 @@ DPDK_17.02 {
rte_pmd_i40e_set_vf_broadcast;
rte_pmd_i40e_set_vf_vlan_tag;
rte_pmd_i40e_set_vf_vlan_filter;
+ rte_pmd_i40e_get_vf_stats;
+ rte_pmd_i40e_reset_vf_stats;
} DPDK_2.0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 23/28] net/ixgbe: use eal I/O device memory read/write API
From: Santosh Shukla @ 2016-12-16 4:40 UTC (permalink / raw)
To: Jianbo Liu
Cc: Jerin Jacob, dev, Ananyev, Konstantin, Thomas Monjalon,
Bruce Richardson, Jan Viktorin, Helin Zhang
In-Reply-To: <CAP4Qi38Bp6oL7uQcOoUqFHk53ARS3Yd+RKw4w1XeHUC+aveEMw@mail.gmail.com>
On Thu, Dec 15, 2016 at 04:37:12PM +0800, Jianbo Liu wrote:
> On 14 December 2016 at 09:55, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > From: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >
> > Replace the raw I/O device memory read/write access with eal
> > abstraction for I/O device memory read/write access to fix
> > portability issues across different architectures.
> >
> > Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: Helin Zhang <helin.zhang@intel.com>
> > CC: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > drivers/net/ixgbe/base/ixgbe_osdep.h | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_osdep.h b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > index 77f0af5..9d16c21 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_osdep.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > @@ -44,6 +44,7 @@
> > #include <rte_cycles.h>
> > #include <rte_log.h>
> > #include <rte_byteorder.h>
> > +#include <rte_io.h>
> >
> > #include "../ixgbe_logs.h"
> > #include "../ixgbe_bypass_defines.h"
> > @@ -121,16 +122,20 @@ typedef int bool;
> >
> > #define prefetch(x) rte_prefetch0(x)
> >
> > -#define IXGBE_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
> > +#define IXGBE_PCI_REG(reg) ({ \
> > + uint32_t __val; \
> > + __val = rte_readl(reg); \
> > + __val; \
> > +})
> >
> > static inline uint32_t ixgbe_read_addr(volatile void* addr)
> > {
> > return rte_le_to_cpu_32(IXGBE_PCI_REG(addr));
> > }
> >
> > -#define IXGBE_PCI_REG_WRITE(reg, value) do { \
> > - IXGBE_PCI_REG((reg)) = (rte_cpu_to_le_32(value)); \
> > -} while(0)
> > +#define IXGBE_PCI_REG_WRITE(reg, value) ({ \
> > + rte_writel(rte_cpu_to_le_32(value), reg); \
> > +})
> >
>
> memory barrier operation is put inside IXGBE_PCI_REG_READ/WRITE in
> your change, but I found rte_*mb is called before these macros in some
> places.
> Can you remove all these redundant calls? And please do the same
> checking for other drivers.
>
Ok.
Thinking of adding _relaxed_rd/wr style macro agnostic to arch for ixgbe case
in particular. Such that for those code incident:
x86 case> first default barrier + relaxed call.
arm case> first default barrier + relaxed call.
Does that make sense to you? If so then will take care in v2.
Santosh.
> > #define IXGBE_PCI_REG_ADDR(hw, reg) \
> > ((volatile uint32_t *)((char *)(hw)->hw_addr + (reg)))
> > --
> > 2.5.5
> >
^ permalink raw reply
* Re: [PATCH v2 01/12] eal: define container_of macro
From: Jan Blunck @ 2016-12-16 8:14 UTC (permalink / raw)
To: Shreyansh Jain
Cc: dev, David Marchand, Thomas Monjalon, Ferruh Yigit, jianbo.liu,
Jan Viktorin
In-Reply-To: <3310c320-fa39-cd8c-ab77-ced20daa5073@nxp.com>
On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>>
>> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> wrote:
>>>
>>> From: Jan Blunck <jblunck@infradead.org>
>>>
>>> This macro is based on Jan Viktorin's original patch but also checks the
>>> type of the passed pointer against the type of the member.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> [jblunck@infradead.org: add type checking and __extension__]
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>>
>>> --
>>> v2:
>>> - fix checkpatch error
>>> ---
>>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/common/include/rte_common.h
>>> b/lib/librte_eal/common/include/rte_common.h
>>> index db5ac91..3eb8d11 100644
>>> --- a/lib/librte_eal/common/include/rte_common.h
>>> +++ b/lib/librte_eal/common/include/rte_common.h
>>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
>>> #endif
>>>
>>> +/**
>>> + * Return pointer to the wrapping struct instance.
>>> + *
>>> + * Example:
>>> + *
>>> + * struct wrapper {
>>> + * ...
>>> + * struct child c;
>>> + * ...
>>> + * };
>>> + *
>>> + * struct child *x = obtain(...);
>>> + * struct wrapper *w = container_of(x, struct wrapper, c);
>>> + */
>>> +#ifndef container_of
>>> +#define container_of(ptr, type, member) (__extension__ ({
>>> \
>>> + typeof(((type *)0)->member) * _ptr = (ptr); \
>>> + (type *)(((char *)_ptr) - offsetof(type,
>>> member));\
>>> + }))
>>
>>
>> This is a checkpatch false positive. It should be fine to ignore this.
>> IIRC we already discussed this before.
>
>
> I too thought something similar was discussed. I tried searching the
> archives but couldn't find anything - thus, I thought probably I was
> hallucinating :P
>
> So, you want me to revert back the '()' change? Does it impact the expansion
> of this macro?
We haven't added this on any other usage of the __extension__ keyword
in the existing code. From my perspective it is more consistent to
revert it.
Anyone else with an opinion here? David? Thomas?
>
>>
>>
>>> +#endif
>>> +
>>> #define _RTE_STR(x) #x
>>> /** Take a macro value and get a string version of it */
>>> #define RTE_STR(x) _RTE_STR(x)
>>> --
>>> 2.7.4
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 00/22] Generic flow API (rte_flow)
From: Adrien Mazarguil @ 2016-12-16 8:22 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz
In-Reply-To: <57fdc1d0-1d2b-da61-481b-358aed8d91a2@intel.com>
On Thu, Dec 15, 2016 at 12:20:36PM +0000, Ferruh Yigit wrote:
> On 12/8/2016 3:19 PM, Adrien Mazarguil wrote:
> > Hi Ferruh,
> >
> > On Fri, Dec 02, 2016 at 04:58:53PM +0000, Ferruh Yigit wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 4:23 PM, Adrien Mazarguil wrote:
> >>> As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> >>> described in [3] (also pasted below), here is the first non-draft series
> >>> for this new API.
> >>>
> >>> Its capabilities are so generic that its name had to be vague, it may be
> >>> called "Generic flow API", "Generic flow interface" (possibly shortened
> >>> as "GFI") to refer to the name of the new filter type, or "rte_flow" from
> >>> the prefix used for its public symbols. I personally favor the latter.
> >>>
> >>> While it is currently meant to supersede existing filter types in order for
> >>> all PMDs to expose a common filtering/classification interface, it may
> >>> eventually evolve to cover the following ideas as well:
> >>>
> >>> - Rx/Tx offloads configuration through automatic offloads for specific
> >>> packets, e.g. performing checksum on TCP packets could be expressed with
> >>> an egress rule with a TCP pattern and a kind of checksum action.
> >>>
> >>> - RSS configuration (already defined actually). Could be global or per rule
> >>> depending on hardware capabilities.
> >>>
> >>> - Switching configuration for devices with many physical ports; rules doing
> >>> both ingress and egress could even be used to completely bypass software
> >>> if supported by hardware.
> >>>
> >>> [1] http://dpdk.org/ml/archives/dev/2016-July/043365.html
> >>> [2] http://dpdk.org/ml/archives/dev/2016-August/045383.html
> >>> [3] http://dpdk.org/ml/archives/dev/2016-November/050044.html
> >>>
> >>> Changes since RFC v2:
> >>>
> >>> - New separate VLAN pattern item (previously part of the ETH definition),
> >>> found to be much more convenient.
> >>>
> >>> - Removed useless "any" field from VF pattern item, the same effect can be
> >>> achieved by not providing a specification structure.
> >>>
> >>> - Replaced bit-fields from the VXLAN pattern item to avoid endianness
> >>> conversion issues on 24-bit fields.
> >>>
> >>> - Updated struct rte_flow_item with a new "last" field to create inclusive
> >>> ranges. They are defined as the interval between (spec & mask) and
> >>> (last & mask). All three parameters are optional.
> >>>
> >>> - Renamed ID action MARK.
> >>>
> >>> - Renamed "queue" fields in actions QUEUE and DUP to "index".
> >>>
> >>> - "rss_conf" field in RSS action is now const.
> >>>
> >>> - VF action now uses a 32 bit ID like its pattern item counterpart.
> >>>
> >>> - Removed redundant struct rte_flow_pattern, API functions now expect
> >>> struct
> >>> rte_flow_item lists terminated by END items.
> >>>
> >>> - Replaced struct rte_flow_actions for the same reason, with struct
> >>> rte_flow_action lists terminated by END actions.
> >>>
> >>> - Error types (enum rte_flow_error_type) have been updated and the cause
> >>> pointer in struct rte_flow_error is now const.
> >>>
> >>> - Function prototypes (rte_flow_create, rte_flow_validate) have also been
> >>> updated for clarity.
> >>>
> >>> Additions:
> >>>
> >>> - Public wrapper functions rte_flow_{validate|create|destroy|flush|query}
> >>> are now implemented in rte_flow.c, with their symbols exported and
> >>> versioned. Related filter type RTE_ETH_FILTER_GENERIC has been added.
> >>>
> >>> - A separate header (rte_flow_driver.h) has been added for driver-side
> >>> functionality, in particular struct rte_flow_ops which contains PMD
> >>> callbacks returned by RTE_ETH_FILTER_GENERIC query.
> >>>
> >>> - testpmd now exposes most of this API through the new "flow" command.
> >>>
> >>> What remains to be done:
> >>>
> >>> - Using endian-aware integer types (rte_beX_t) where necessary for clarity.
> >>>
> >>> - API documentation (based on RFC).
> >>>
> >>> - testpmd flow command documentation (although context-aware command
> >>> completion should already help quite a bit in this regard).
> >>>
> >>> - A few pattern item / action properties cannot be configured yet
> >>> (e.g. rss_conf parameter for RSS action) and a few completions
> >>> (e.g. possible queue IDs) should be added.
> >>>
> >>
> >> <...>
> >>
> >> I was trying to check driver filter API patches, but hit a few compiler
> >> errors with this patchset.
> >>
> >> [1] clang complains about variable bitfield value changed from -1 to 1.
> >> Which is correct, but I guess that is intentional, but I don't know how
> >> to tell this to clang?
> >>
> >> [2] shred library compilation error, because of missing rte_flow_flush
> >> in rte_ether_version.map file
> >>
> >> [3] bunch of icc compilation errors, almost all are same type:
> >> error #188: enumerated type mixed with another type
> >
> > Thanks for the report, I'll attempt to address them all in v2.
>
> Hi Adrien,
>
> I would like to remind that there are driver patch sets depends to this
> patch.
>
> New version of this patch should give some time to drivers to re-do (if
> required) the patchsets before integration deadline.
Hi Ferruh,
I intend to send v2 (including all the requested changes and fixes) shortly,
most likely today.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Adrien Mazarguil @ 2016-12-16 8:23 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen
In-Reply-To: <1481809599-27896-1-git-send-email-olivier.matz@6wind.com>
On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:
> Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> declare the list of kernel modules required to run properly.
>
> Today, most PCI drivers require uio/vfio.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>
> v2 -> v3:
> - fix kmods deps advertised by mellanox drivers as pointed out
> by Adrien
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH 12/22] app/testpmd: add rte_flow item spec handler
From: Adrien Mazarguil @ 2016-12-16 9:17 UTC (permalink / raw)
To: Pei, Yulong
Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
Olivier Matz, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03B6625B9@shsmsx102.ccr.corp.intel.com>
Hi Yulong,
On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> Hi Adrien,
>
> I try to setup the following rule, but it seems that after set 'spec' param, can not set 'mask' param, is it an issue here or am I wrong to use it ?
>
> testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
> dst [TOKEN]: destination MAC
> src [TOKEN]: source MAC
> type [TOKEN]: EtherType
> / [TOKEN]: specify next pattern item
You need to re-specify dst with "mask" instead of "spec". You can specify it
as many times you like to update each structure in turn, e.g.:
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask 00:00:00:00:ff:ff
If you want to specify both spec and mask at once assuming you want it full,
these commands yield the same result:
testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask ff:ff:ff:ff:ff:ff
testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst prefix 48
You are even allowed to change your mind:
testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix 00:00:00:00:09:00
All these will be properly documented in the v2 patchset. Note, this version
will replace the "fix" keyword with "is" ("fix" made no sense according to
feedback).
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16 9:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Neil Horman, dev, thomas.monjalon, vido, fiona.trahe,
adrien.mazarguil
In-Reply-To: <20161215092207.168ba141@xeon-e3>
Hi,
On Thu, 15 Dec 2016 09:22:07 -0800, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 15 Dec 2016 11:09:12 -0500
> Neil Horman <nhorman@tuxdriver.com> wrote:
>
> > On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:
> > > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver
> > > to declare the list of kernel modules required to run properly.
> > >
> > > Today, most PCI drivers require uio/vfio.
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > >
> > > v2 -> v3:
> > > - fix kmods deps advertised by mellanox drivers as pointed out
> > > by Adrien
> > >
> > > v1 ->
> > > v2:
> > > - do not advertise uio_pci_generic for vf drivers
> > > - rebase on top of head: use new driver names and prefix
> > > macro with
> > > RTE_
> > >
> > > rfc -> v1:
> > > - the kmod information can be per-device using a modalias-like
> > > pattern
> > > - change syntax to use '&' and '|' instead of ',' and ':'
> > > - remove useless prerequisites in kmod lis: no need to
> > > specify both uio and uio_pci_generic, only the latter is
> > > required
> > > - update kmod list in szedata2 driver
> > > - remove kmod list in qat driver: it requires more than just
> > > loading a kmod, which is described in documentation
> > >
> > > buildtools/pmdinfogen/pmdinfogen.c | 1 +
> > > buildtools/pmdinfogen/pmdinfogen.h | 1 +
> > > drivers/net/bnx2x/bnx2x_ethdev.c | 2 ++
> > > drivers/net/bnxt/bnxt_ethdev.c | 1 +
> > > drivers/net/cxgbe/cxgbe_ethdev.c | 1 +
> > > drivers/net/e1000/em_ethdev.c | 1 +
> > > drivers/net/e1000/igb_ethdev.c | 2 ++
> > > drivers/net/ena/ena_ethdev.c | 1 +
> > > drivers/net/enic/enic_ethdev.c | 1 +
> > > drivers/net/fm10k/fm10k_ethdev.c | 1 +
> > > drivers/net/i40e/i40e_ethdev.c | 1 +
> > > drivers/net/i40e/i40e_ethdev_vf.c | 1 +
> > > drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
> > > drivers/net/mlx4/mlx4.c | 2 ++
> > > drivers/net/mlx5/mlx5.c | 1 +
> > > drivers/net/nfp/nfp_net.c | 1 +
> > > drivers/net/qede/qede_ethdev.c | 2 ++
> > > drivers/net/szedata2/rte_eth_szedata2.c | 2 ++
> > > drivers/net/thunderx/nicvf_ethdev.c | 1 +
> > > drivers/net/virtio/virtio_ethdev.c | 1 +
> > > drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 +
> > > lib/librte_eal/common/include/rte_dev.h | 25
> > > +++++++++++++++++++++++++ tools/dpdk-pmdinfo.py
> > > | 5 ++++- 23 files changed, 56 insertions(+), 1 deletion(-)
> > >
> > Its odd that all devices, regardless of vendor should depend on the
> > igb_uio module. It seems to me that depending on uio_pci_generic
> > or vfio is sufficient.
igb_uio is the historical uio module of dpdk. Although it is called
igb_uio, it is not specific to Intel drivers.
Most drivers declare "igb_uio | uio_pci_generic | vfio", which means
that any of the 3 kernel modules can be used.
I think there are some cases where people will prefer using igb_uio,
for instance to use a vf pmd in a vm where there is no iommu,
and where the kernel vfio module does not support the no-iommu mode.
>
> Yes it seems just a special case extension for Mellanox drivers.
Kmod deps are different whether it's a vf driver or not.
Mellanox drivers are not the only drivers that do not require uio,
there is also szedata2.
Is it an argument for not including this patch?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16 9:23 UTC (permalink / raw)
To: Jan Blunck
Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02=wKpjadYrA6JUHaoG5WH=MG7Y581fFeTry7K0mbBL1Q@mail.gmail.com>
On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >>
> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> wrote:
> >>>
> >>> From: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >>> type of the passed pointer against the type of the member.
> >>>
> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >>> [jblunck@infradead.org: add type checking and __extension__]
> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> --
> >>> v2:
> >>> - fix checkpatch error
> >>> ---
> >>> lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >>> 1 file changed, 21 insertions(+)
> >>>
> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >>> b/lib/librte_eal/common/include/rte_common.h
> >>> index db5ac91..3eb8d11 100644
> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
> >>> #endif
> >>>
> >>> +/**
> >>> + * Return pointer to the wrapping struct instance.
> >>> + *
> >>> + * Example:
> >>> + *
> >>> + * struct wrapper {
> >>> + * ...
> >>> + * struct child c;
> >>> + * ...
> >>> + * };
> >>> + *
> >>> + * struct child *x = obtain(...);
> >>> + * struct wrapper *w = container_of(x, struct wrapper, c);
> >>> + */
> >>> +#ifndef container_of
> >>> +#define container_of(ptr, type, member) (__extension__ ({
> >>> \
> >>> + typeof(((type *)0)->member) * _ptr = (ptr); \
> >>> + (type *)(((char *)_ptr) - offsetof(type,
> >>> member));\
> >>> + }))
> >>
> >>
> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> IIRC we already discussed this before.
> >
> >
> > I too thought something similar was discussed. I tried searching the
> > archives but couldn't find anything - thus, I thought probably I was
> > hallucinating :P
> >
> > So, you want me to revert back the '()' change? Does it impact the expansion
> > of this macro?
>
> We haven't added this on any other usage of the __extension__ keyword
> in the existing code. From my perspective it is more consistent to
> revert it.
>
> Anyone else with an opinion here? David? Thomas?
As an exported header, rte_common.h must pass check-includes.sh. Both
typeof() and the ({ ... }) construct are non-standard GCC extensions and
would fail to compile with pedantic options.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16 9:36 UTC (permalink / raw)
To: Ferruh Yigit
Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen,
adrien.mazarguil
In-Reply-To: <ba0a18d0-d4db-3c65-f11c-10465924082d@intel.com>
Hi Ferruh,
On Thu, 15 Dec 2016 14:52:02 +0000, Ferruh Yigit
<ferruh.yigit@intel.com> wrote:
> Hi Olivier, Thomas,
>
> On 12/15/2016 1:46 PM, Olivier Matz wrote:
> > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> > declare the list of kernel modules required to run properly.
> >
> > Today, most PCI drivers require uio/vfio.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
>
> This patch is for master branch, what do you think targeting it to
> next-net tree?
> So that new PMDs also can be included into patch?
That makes sense, I can rebase on next-net.
Thomas, do you agree?
From what I see, the 2 new PMDs are sfc (uio) and tap (nothing).
Regards,
Olivier
^ permalink raw reply
* [PATCH 1/2] ethdev: fix name index in xstats Api
From: Olivier Matz @ 2016-12-16 9:44 UTC (permalink / raw)
To: dev, thomas.monjalon; +Cc: remy.horton, stable
The function rte_eth_xstats_get() return an array of tuples (id,
value). The value is the statistic counter, while the id references a
name in the array returned by rte_eth_xstats_get_name().
Today, each 'id' returned by rte_eth_xstats_get() is equal to the index
in the returned array, making this value useless. It also prevents a
driver from having different indexes for names and value, like in the
example below:
rte_eth_xstats_get_name() returns:
0: "rx0_stat"
1: "rx1_stat"
2: ...
7: "rx7_stat"
8: "tx0_stat"
9: "tx1_stat"
...
15: "tx7_stat"
rte_eth_xstats_get() returns:
0: id=0, val=<stat> ("rx0_stat")
1: id=1, val=<stat> ("rx1_stat")
2: id=8, val=<stat> ("tx0_stat")
3: id=9, val=<stat> ("tx1_stat")
This patch fixes the drivers to set the 'id' in their ethdev->xstats_get()
(except e1000 which was already doing it), and fixes ethdev by not setting
the 'id' field to the index of the table for pmd-specific stats: instead,
they should just be shifted by the max number of generic statistics.
CC: stable@dpdk.org
Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/bnx2x/bnx2x_ethdev.c | 1 +
drivers/net/fm10k/fm10k_ethdev.c | 3 +++
drivers/net/i40e/i40e_ethdev.c | 4 ++++
drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
drivers/net/qede/qede_ethdev.c | 2 ++
drivers/net/vhost/rte_eth_vhost.c | 2 ++
drivers/net/virtio/virtio_ethdev.c | 2 ++
lib/librte_ether/rte_ethdev.c | 5 ++++-
8 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 0f1e4a2..f01047f 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -422,6 +422,7 @@ bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[num].value =
*(uint64_t *)((char *)&sc->eth_stats +
bnx2x_xstats_strings[num].offset_lo);
+ xstats[num].id = num;
}
return num;
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index fe74f6d..0d3098e 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1315,6 +1315,7 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < FM10K_NB_HW_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
fm10k_hw_stats_strings[count].offset);
+ xstats[count].id = count;
count++;
}
@@ -1324,12 +1325,14 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value =
*(uint64_t *)(((char *)&hw_stats->q[q]) +
fm10k_hw_stats_rx_q_strings[i].offset);
+ xstats[count].id = count;
count++;
}
for (i = 0; i < FM10K_NB_TX_Q_XSTATS; i++) {
xstats[count].value =
*(uint64_t *)(((char *)&hw_stats->q[q]) +
fm10k_hw_stats_tx_q_strings[i].offset);
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b0c0fbf..f01455e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2533,6 +2533,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)&hw_stats->eth) +
rte_i40e_stats_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2540,6 +2541,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < I40E_NB_HW_PORT_XSTATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_i40e_hw_port_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2549,6 +2551,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)hw_stats) +
rte_i40e_rxq_prio_strings[i].offset +
(sizeof(uint64_t) * prio));
+ xstats[count].id = count;
count++;
}
}
@@ -2559,6 +2562,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)hw_stats) +
rte_i40e_txq_prio_strings[i].offset +
(sizeof(uint64_t) * prio));
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index baffc71..cf68fa9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2902,6 +2902,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_stats_strings[i].offset);
+ xstats[count].id = count;
count++;
}
@@ -2911,6 +2912,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_rxq_strings[stat].offset +
(sizeof(uint64_t) * i));
+ xstats[count].id = count;
count++;
}
}
@@ -2921,6 +2923,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
rte_ixgbe_txq_strings[stat].offset +
(sizeof(uint64_t) * i));
+ xstats[count].id = count;
count++;
}
}
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 001166a..6f3b10c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -973,6 +973,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (i = 0; i < RTE_DIM(qede_xstats_strings); i++) {
xstats[stat_idx].value = *(uint64_t *)(((char *)&stats) +
qede_xstats_strings[i].offset);
+ xstats[stat_idx].id = stat_idx;
stat_idx++;
}
@@ -982,6 +983,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
xstats[stat_idx].value = *(uint64_t *)(
((char *)(qdev->fp_array[(qid)].rxq)) +
qede_rxq_xstats_strings[i].offset);
+ xstats[stat_idx].id = stat_idx;
stat_idx++;
}
}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 766d4ef..dd79ccf 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -324,6 +324,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)vq)
+ vhost_rxport_stat_strings[t].offset);
}
+ xstats[count].id = count;
count++;
}
for (t = 0; t < VHOST_NB_XSTATS_TXPORT; t++) {
@@ -336,6 +337,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
*(uint64_t *)(((char *)vq)
+ vhost_txport_stat_strings[t].offset);
}
+ xstats[count].id = count;
count++;
}
return count;
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1bd60e9..f6335aa 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -893,6 +893,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (t = 0; t < VIRTIO_NB_RXQ_XSTATS; t++) {
xstats[count].value = *(uint64_t *)(((char *)rxvq) +
rte_virtio_rxq_stat_strings[t].offset);
+ xstats[count].id = count;
count++;
}
}
@@ -908,6 +909,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
for (t = 0; t < VIRTIO_NB_TXQ_XSTATS; t++) {
xstats[count].value = *(uint64_t *)(((char *)txvq) +
rte_virtio_txq_stat_strings[t].offset);
+ xstats[count].id = count;
count++;
}
}
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 1e0f206..45d8286 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1491,8 +1491,11 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
}
}
- for (i = 0; i < count + xcount; i++)
+ for (i = 0; i < count; i++)
xstats[i].id = i;
+ /* add an offset to driver-specific stats */
+ for ( ; i < count + xcount; i++)
+ xstats[i].id += count;
return count + xcount;
}
--
2.8.1
^ permalink raw reply related
* [PATCH 2/2] ethdev: clarify xstats Api documentation
From: Olivier Matz @ 2016-12-16 9:44 UTC (permalink / raw)
To: dev, thomas.monjalon; +Cc: remy.horton, stable
In-Reply-To: <1481881454-17382-1-git-send-email-olivier.matz@6wind.com>
Reword the Api documentation of xstats ethdev.
CC: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..4479553 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -938,23 +938,26 @@ struct rte_eth_txq_info {
/**
* An Ethernet device extended statistic structure
*
- * This structure is used by ethdev->eth_xstats_get() to provide
- * statistics that are not provided in the generic rte_eth_stats
+ * This structure is used by rte_eth_xstats_get() to provide
+ * statistics that are not provided in the generic *rte_eth_stats*
* structure.
+ * It maps a name id, corresponding to an index in the array returned
+ * by rte_eth_xstats_get_names(), to a statistic value.
*/
struct rte_eth_xstat {
- uint64_t id;
- uint64_t value;
+ uint64_t id; /**< The index in xstats name array. */
+ uint64_t value; /**< The statistic counter value. */
};
/**
- * A name-key lookup element for extended statistics.
+ * A name element for extended statistics.
*
- * This structure is used to map between names and ID numbers
- * for extended ethernet statistics.
+ * An array of this structure is returned by rte_eth_xstats_get_names().
+ * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
+ * structure references these names by their array index.
*/
struct rte_eth_xstat_name {
- char name[RTE_ETH_XSTATS_NAME_SIZE];
+ char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
};
#define ETH_DCB_NUM_TCS 8
@@ -2272,18 +2275,19 @@ void rte_eth_stats_reset(uint8_t port_id);
* @param port_id
* The port identifier of the Ethernet device.
* @param xstats_names
- * Block of memory to insert names into. Must be at least size in capacity.
- * If set to NULL, function returns required capacity.
+ * An rte_eth_xstat_name array of at least *size* elements to
+ * be filled. If set to NULL, the function returns the required number
+ * of elements.
* @param size
- * Capacity of xstats_names (number of names).
+ * The size of the xstats_names array (number of elements).
* @return
- * - positive value lower or equal to size: success. The return value
+ * - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
- * - positive value higher than size: error, the given statistics table
+ * - A positive value higher than size: error, the given statistics table
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - negative value on error (invalid port id)
+ * - A negative value on error (invalid port id).
*/
int rte_eth_xstats_get_names(uint8_t port_id,
struct rte_eth_xstat_name *xstats_names,
@@ -2296,19 +2300,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
* The port identifier of the Ethernet device.
* @param xstats
* A pointer to a table of structure of type *rte_eth_xstat*
- * to be filled with device statistics ids and values.
+ * to be filled with device statistics ids and values: id is the
+ * index of the name string in xstats_names (@see rte_eth_xstats_get_names),
+ * and value is the statistic counter.
* This parameter can be set to NULL if n is 0.
* @param n
- * The size of the stats table, which should be large enough to store
- * all the statistics of the device.
+ * The size of the xstats array (number of elements).
* @return
- * - positive value lower or equal to n: success. The return value
+ * - A positive value lower or equal to n: success. The return value
* is the number of entries filled in the stats table.
- * - positive value higher than n: error, the given statistics table
+ * - A positive value higher than n: error, the given statistics table
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - negative value on error (invalid port id)
+ * - A negative value on error (invalid port id).
*/
int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
unsigned n);
--
2.8.1
^ permalink raw reply related
* Re: [PATCH v2 00/32] Support more features in Solarflare PMD
From: Ferruh Yigit @ 2016-12-16 9:57 UTC (permalink / raw)
To: Andrew Rybchenko, dev
In-Reply-To: <1481806283-10387-1-git-send-email-arybchenko@solarflare.com>
On 12/15/2016 12:50 PM, Andrew Rybchenko wrote:
> The patch series adds a number of features to Solarflare libefx-based
> PMD. Basically one patch per feature.
>
> The patches are grouped into one series since they touch nearby lines
> in either PMD feature list, or dev_ops structure, or documentation.
> So, patches cannot be applied in arbitrary order.
>
> ---
>
> v2:
> * Fix ICC and clang warnings
> * Slightly change sfc_tso_{alloc,free}_tsoh_objs() prototypes
>
>
> Andrew Rybchenko (17):
> net/sfc: implement MCDI logging callback
> net/sfc: support parameter to choose performance profile
> net/sfc: implement ethdev hook to get basic statistics
> net/sfc: support extended statistics
> net/sfc: support flow control settings get/set
> net/sfc: support link status change interrupt
> net/sfc: implement device operation to change MTU
> net/sfc: support link speed and duplex settings
> net/sfc: support checksum offloads on receive
> net/sfc: handle received packet type info provided by HW
> net/sfc: support callback to get receive queue information
> net/sfc: support Rx free threshold
> net/sfc: add callback to get RxQ pending descriptors count
> net/sfc: add RxQ descriptor done callback
> net/sfc: support scattered Rx DMA
> net/sfc: support deferred start of receive queues
> net/sfc/base: do not use enum type when values are bitmask
>
> Artem Andreev (1):
> net/sfc: support link up/down
>
> Ivan Malov (14):
> net/sfc: support promiscuous and all-multicast control
> net/sfc: support main (the first) MAC address change
> net/sfc: support multicast addresses list controls
> net/sfc: add callback to get transmit queue information
> net/sfc: support Tx free threshold
> net/sfc: support deferred start of transmit queues
> net/sfc: support VLAN offload on transmit path
> net/sfc: add basic stubs for RSS support on driver attach
> net/sfc: support RSS hash offload
> net/sfc: add callback to query RSS key and hash types config
> net/sfc: add callback to set RSS key and hash types config
> net/sfc: add callback to query RSS redirection table
> net/sfc: add callback to update RSS redirection table
> net/sfc: support firmware-assisted TSOv2
>
<...>
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH 1/4] rte_mbuf: add rte_pktmbuf_coalesce
From: Olivier Matz @ 2016-12-16 10:06 UTC (permalink / raw)
To: Tomasz Kulasek; +Cc: dev
In-Reply-To: <1480698466-17620-2-git-send-email-tomaszx.kulasek@intel.com>
Hi Tomasz,
On Fri, 2 Dec 2016 18:07:43 +0100, Tomasz Kulasek
<tomaszx.kulasek@intel.com> wrote:
> This patch adds function rte_pktmbuf_coalesce to let crypto PMD
> coalesce chained mbuf before crypto operation and extend their
> capabilities to support segmented mbufs when device cannot handle
> them natively.
>
>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
> lib/librte_mbuf/rte_mbuf.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ead7c6e..f048681 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1647,6 +1647,40 @@ static inline int rte_pktmbuf_chain(struct
> rte_mbuf *head, struct rte_mbuf *tail }
>
> /**
> + * Coalesce data from mbuf to the continuous buffer.
> + *
> + * @param mbuf_dst
> + * Contiguous destination mbuf
> + * @param mbuf_src
> + * Uncontiguous source mbuf
> + *
> + * @return
> + * - 0, on success
> + * - -EINVAL, on error
> + */
I think the API should be clarified. In your case, it is expected that the
destination mbuf is already filled with uninitialized data (i.e. that
rte_pktmbuf_append() has been called).
We could wonder if a better API wouldn't be to allocate the dst mbuf in
the function, call append()/prepend(), and do the copy.
Even better, we could have:
int rte_pktmbuf_linearize(struct rte_mbuf *m)
It will reuse the same mbuf (maybe moving the data).
> +
> +#include <rte_hexdump.h>
This should be removed.
> +
> +static inline int
> +rte_pktmbuf_coalesce(struct rte_mbuf *mbuf_dst, struct rte_mbuf *mbuf_src) {
Source mbuf should be const.
> + char *dst;
> +
> + if (!rte_pktmbuf_is_contiguous(mbuf_dst) ||
> + rte_pktmbuf_data_len(mbuf_dst) >=
> + rte_pktmbuf_pkt_len(mbuf_src))
> + return -EINVAL;
Why >= ?
> +
> + dst = rte_pktmbuf_mtod(mbuf_dst, char *);
> +
> + if (!__rte_pktmbuf_read(mbuf_src, 0, rte_pktmbuf_pkt_len(mbuf_src),
> + dst))
When a function returns a pointer, I think it is clearer to do:
if (func() == NULL)
than:
if (!func())
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> * Dump an mbuf structure to a file.
> *
> * Dump all fields for the given packet mbuf and all its associated
One more question, I don't see where this function is used in your
patchset. What is your use-case?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH 07/13] pcap: fix timestamps in output pcap file
From: Ferruh Yigit @ 2016-12-16 10:06 UTC (permalink / raw)
To: Michał Mirosław, dev; +Cc: "CC: stable"
In-Reply-To: <b8fb833e-634e-f559-12ce-1e71d7740ce5@intel.com>
On 12/14/2016 5:45 PM, Ferruh Yigit wrote:
> On 12/13/2016 1:08 AM, Michał Mirosław wrote:
>> From: Piotr Bartosiewicz <piotr.bartosiewicz@atendesoftware.pl>
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>> ---
>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
CC: stable@dpdk.org
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jianbo Liu @ 2016-12-16 10:12 UTC (permalink / raw)
To: Jerin Jacob
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin
In-Reply-To: <20161215110807.GA10881@localhost.localdomain>
On 15 December 2016 at 19:08, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
>> On 15 December 2016 at 18:04, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
>> >> On 14 December 2016 at 09:55, Jerin Jacob
>> >> <jerin.jacob@caviumnetworks.com> wrote:
>> >> > Override the generic I/O device memory read/write access and implement it
>> >> > using armv8 instructions for arm64.
>> >> >
>> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> >> > ---
>> >> > lib/librte_eal/common/include/arch/arm/rte_io.h | 4 +
>> >> > lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
>> >> > 2 files changed, 187 insertions(+)
>> >> > create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> >
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > index 74c1f2c..9593b42 100644
>> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > @@ -38,7 +38,11 @@
>> >> > extern "C" {
>> >> > #endif
>> >> >
>> >> > +#ifdef RTE_ARCH_64
>> >> > +#include "rte_io_64.h"
>> >> > +#else
>> >> > #include "generic/rte_io.h"
>> >> > +#endif
>> >> >
>> >> > #ifdef __cplusplus
>> >> > }
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > new file mode 100644
>> >> > index 0000000..09e7a89
>> >> > --- /dev/null
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > @@ -0,0 +1,183 @@
>> >> > +/*
>> >> > + * BSD LICENSE
>> >> > + *
>> >> > + * Copyright (C) Cavium networks Ltd. 2016.
>> >> > + *
>> >> > + * Redistribution and use in source and binary forms, with or without
>> >> > + * modification, are permitted provided that the following conditions
>> >> > + * are met:
>> >> > + *
>> >> > + * * Redistributions of source code must retain the above copyright
>> >> > + * notice, this list of conditions and the following disclaimer.
>> >> > + * * Redistributions in binary form must reproduce the above copyright
>> >> > + * notice, this list of conditions and the following disclaimer in
>> >> > + * the documentation and/or other materials provided with the
>> >> > + * distribution.
>> >> > + * * Neither the name of Cavium networks nor the names of its
>> >> > + * contributors may be used to endorse or promote products derived
>> >> > + * from this software without specific prior written permission.
>> >> > + *
>> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> >> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> >> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> >> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> >> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> >> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> >> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> >> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> >> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> >> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> >> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> >> > + */
>> >> > +
>> >> > +#ifndef _RTE_IO_ARM64_H_
>> >> > +#define _RTE_IO_ARM64_H_
>> >> > +
>> >> > +#ifdef __cplusplus
>> >> > +extern "C" {
>> >> > +#endif
>> >> > +
>> >> > +#include <stdint.h>
>> >> > +
>> >> > +#define RTE_OVERRIDE_IO_H
>> >> > +
>> >> > +#include "generic/rte_io.h"
>> >> > +#include "rte_atomic_64.h"
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint8_t
>> >> > +__rte_arm64_readb(const volatile void *addr)
>> >> > +{
>> >> > + uint8_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldrb %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint16_t
>> >> > +__rte_arm64_readw(const volatile void *addr)
>> >> > +{
>> >> > + uint16_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldrh %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint32_t
>> >> > +__rte_arm64_readl(const volatile void *addr)
>> >> > +{
>> >> > + uint32_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldr %w[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint64_t
>> >> > +__rte_arm64_readq(const volatile void *addr)
>> >> > +{
>> >> > + uint64_t val;
>> >> > +
>> >> > + asm volatile(
>> >> > + "ldr %x[val], [%x[addr]]"
>> >> > + : [val] "=r" (val)
>> >> > + : [addr] "r" (addr));
>> >> > + return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "strb %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "strh %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "str %w[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
>> >> > +{
>> >> > + asm volatile(
>> >> > + "str %x[val], [%x[addr]]"
>> >> > + :
>> >> > + : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >>
>> >> I'm not quite sure about these overridings. Can you explain the
>> >> benefit to do so?
>> >
>> > Better to be native if there is option. That all. Do you see any issue?
>> > or what is the real concern?
>> >
>>
>> I think it's the same as the generic c version after compiling. Am I right?
>
> I really don't that is the case for all the scenarios like compiler may
> combine two 16bit reads one 32bit read etc and which will impact on IO
I wonder which compiler will do that as armv8 is 32/64 bit system?
> register access.
>
> But, I am sure the proposed scheme generates correct instruction in all the cases.
^ permalink raw reply
* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Yang, Zhiyong @ 2016-12-16 10:19 UTC (permalink / raw)
To: Richardson, Bruce
Cc: Ananyev, Konstantin, Thomas Monjalon, dev@dpdk.org,
yuanhan.liu@linux.intel.com, De Lara Guarch, Pablo
In-Reply-To: <20161215101242.GA125588@bricha3-MOBL3.ger.corp.intel.com>
Hi, Bruce:
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Thursday, December 15, 2016 6:13 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> Monjalon <thomas.monjalon@6wind.com>; dev@dpdk.org;
> yuanhan.liu@linux.intel.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce rte_memset on
> IA platform
>
> On Thu, Dec 15, 2016 at 06:51:08AM +0000, Yang, Zhiyong wrote:
> > Hi, Thomas, Konstantin:
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
> > > Sent: Sunday, December 11, 2016 8:33 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > Monjalon <thomas.monjalon@6wind.com>
> > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> rte_memset
> > > on IA platform
> > >
> > > Hi, Konstantin, Bruce:
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Thursday, December 8, 2016 6:31 PM
> > > > To: Yang, Zhiyong <zhiyong.yang@intel.com>; Thomas Monjalon
> > > > <thomas.monjalon@6wind.com>
> > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > <pablo.de.lara.guarch@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > rte_memset on IA platform
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Yang, Zhiyong
> > > > > Sent: Thursday, December 8, 2016 9:53 AM
> > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > > > Monjalon <thomas.monjalon@6wind.com>
> > > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > > <pablo.de.lara.guarch@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > > rte_memset on IA platform
> > > > >
> > > > extern void *(*__rte_memset_vector)( (void *s, int c, size_t n);
> > > >
> > > > static inline void*
> > > > rte_memset_huge(void *s, int c, size_t n) {
> > > > return __rte_memset_vector(s, c, n); }
> > > >
> > > > static inline void *
> > > > rte_memset(void *s, int c, size_t n) {
> > > > If (n < XXX)
> > > > return rte_memset_scalar(s, c, n);
> > > > else
> > > > return rte_memset_huge(s, c, n); }
> > > >
> > > > XXX could be either a define, or could also be a variable, so it
> > > > can be setuped at startup, depending on the architecture.
> > > >
> > > > Would that work?
> > > > Konstantin
> > > >
> > I have implemented the code for choosing the functions at run time.
> > rte_memcpy is used more frequently, So I test it at run time.
> >
> > typedef void *(*rte_memcpy_vector_t)(void *dst, const void *src,
> > size_t n); extern rte_memcpy_vector_t rte_memcpy_vector; static inline
> > void * rte_memcpy(void *dst, const void *src, size_t n) {
> > return rte_memcpy_vector(dst, src, n); } In order to reduce
> > the overhead at run time, I assign the function address to var
> > rte_memcpy_vector before main() starts to init the var.
> >
> > static void __attribute__((constructor))
> > rte_memcpy_init(void)
> > {
> > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > {
> > rte_memcpy_vector = rte_memcpy_avx2;
> > }
> > else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
> > {
> > rte_memcpy_vector = rte_memcpy_sse;
> > }
> > else
> > {
> > rte_memcpy_vector = memcpy;
> > }
> >
> > }
> > I run the same virtio/vhost loopback tests without NIC.
> > I can see the throughput drop when running choosing functions at run
> > time compared to original code as following on the same platform(my
> machine is haswell)
> > Packet size perf drop
> > 64 -4%
> > 256 -5.4%
> > 1024 -5%
> > 1500 -2.5%
> > Another thing, I run the memcpy_perf_autotest, when N= <128, the
> > rte_memcpy perf gains almost disappears When choosing functions at run
> > time. For N=other numbers, the perf gains will become narrow.
> >
> How narrow. How significant is the improvement that we gain from having to
> maintain our own copy of memcpy. If the libc version is nearly as good we
> should just use that.
>
> /Bruce
Zhihong sent a patch about rte_memcpy, From the patch,
we can see the optimization job for memcpy will bring obvious perf improvements
than glibc for DPDK.
http://www.dpdk.org/dev/patchwork/patch/17753/
git log as following:
This patch is tested on Ivy Bridge, Haswell and Skylake, it provides
up to 20% gain for Virtio Vhost PVP traffic, with packet size ranging
from 64 to 1500 bytes.
thanks
Zhiyong
^ permalink raw reply
* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jerin Jacob @ 2016-12-16 10:25 UTC (permalink / raw)
To: Jianbo Liu
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin
In-Reply-To: <CAP4Qi38_5zKe6a=v3W5wzcDgeB9Un97O8UAybucgPjhucHsE8Q@mail.gmail.com>
On Fri, Dec 16, 2016 at 06:12:13PM +0800, Jianbo Liu wrote:
> On 15 December 2016 at 19:08, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
> >> On 15 December 2016 at 18:04, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> >> >> On 14 December 2016 at 09:55, Jerin Jacob
> >> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> >> > Override the generic I/O device memory read/write access and implement it
> >> >> > using armv8 instructions for arm64.
> >> >> >
> >> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> >> > ---
> >> >> > lib/librte_eal/common/include/arch/arm/rte_io.h | 4 +
> >> >> > lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
> >> >> > 2 files changed, 187 insertions(+)
> >> >> > create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> >
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > index 74c1f2c..9593b42 100644
> >> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > @@ -38,7 +38,11 @@
> >> >> > extern "C" {
> >> >> > #endif
> >> >> >
> >> >> > +#ifdef RTE_ARCH_64
> >> >> > +#include "rte_io_64.h"
> >> >> > +#else
> >> >> > #include "generic/rte_io.h"
> >> >> > +#endif
> >> >> >
> >> >> > #ifdef __cplusplus
> >> >> > }
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > new file mode 100644
> >> >> > index 0000000..09e7a89
> >> >> > --- /dev/null
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > @@ -0,0 +1,183 @@
> >> >> > +/*
> >> >> > + * BSD LICENSE
> >> >> > + *
> >> >> > + * Copyright (C) Cavium networks Ltd. 2016.
> >> >> > + *
> >> >> > + * Redistribution and use in source and binary forms, with or without
> >> >> > + * modification, are permitted provided that the following conditions
> >> >> > + * are met:
> >> >> > + *
> >> >> > + * * Redistributions of source code must retain the above copyright
> >> >> > + * notice, this list of conditions and the following disclaimer.
> >> >> > + * * Redistributions in binary form must reproduce the above copyright
> >> >> > + * notice, this list of conditions and the following disclaimer in
> >> >> > + * the documentation and/or other materials provided with the
> >> >> > + * distribution.
> >> >> > + * * Neither the name of Cavium networks nor the names of its
> >> >> > + * contributors may be used to endorse or promote products derived
> >> >> > + * from this software without specific prior written permission.
> >> >> > + *
> >> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >> >> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >> >> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >> >> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >> >> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >> >> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >> >> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >> >> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >> >> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >> >> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >> >> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> >> > + */
> >> >> > +
> >> >> > +#ifndef _RTE_IO_ARM64_H_
> >> >> > +#define _RTE_IO_ARM64_H_
> >> >> > +
> >> >> > +#ifdef __cplusplus
> >> >> > +extern "C" {
> >> >> > +#endif
> >> >> > +
> >> >> > +#include <stdint.h>
> >> >> > +
> >> >> > +#define RTE_OVERRIDE_IO_H
> >> >> > +
> >> >> > +#include "generic/rte_io.h"
> >> >> > +#include "rte_atomic_64.h"
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint8_t
> >> >> > +__rte_arm64_readb(const volatile void *addr)
> >> >> > +{
> >> >> > + uint8_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldrb %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint16_t
> >> >> > +__rte_arm64_readw(const volatile void *addr)
> >> >> > +{
> >> >> > + uint16_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldrh %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint32_t
> >> >> > +__rte_arm64_readl(const volatile void *addr)
> >> >> > +{
> >> >> > + uint32_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldr %w[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint64_t
> >> >> > +__rte_arm64_readq(const volatile void *addr)
> >> >> > +{
> >> >> > + uint64_t val;
> >> >> > +
> >> >> > + asm volatile(
> >> >> > + "ldr %x[val], [%x[addr]]"
> >> >> > + : [val] "=r" (val)
> >> >> > + : [addr] "r" (addr));
> >> >> > + return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "strb %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "strh %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "str %w[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
> >> >> > +{
> >> >> > + asm volatile(
> >> >> > + "str %x[val], [%x[addr]]"
> >> >> > + :
> >> >> > + : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >>
> >> >> I'm not quite sure about these overridings. Can you explain the
> >> >> benefit to do so?
> >> >
> >> > Better to be native if there is option. That all. Do you see any issue?
> >> > or what is the real concern?
> >> >
> >>
> >> I think it's the same as the generic c version after compiling. Am I right?
> >
> > I really don't that is the case for all the scenarios like compiler may
> > combine two 16bit reads one 32bit read etc and which will impact on IO
>
> I wonder which compiler will do that as armv8 is 32/64 bit system?
Not specific to armv8.
Two consecutive continues 16bits reads one 32bit read for optimization.
Any idea why Linux kernel doing explicit instructions for readl/writel?
obviously not for fun.
>
> > register access.
> >
> > But, I am sure the proposed scheme generates correct instruction in all the cases.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox