* [PATCH] vhost: fix add_guest_pages bug
From: Haifeng Lin @ 2016-12-01 10:22 UTC (permalink / raw)
To: dev, yuanhan.liu, maxime.coquelin; +Cc: Haifeng Lin
When reg_size < page_size the function read in
rte_mem_virt2phy would not return, becasue gpa
is invalid.
Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
---
lib/librte_vhost/vhost_user.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 6b83c15..ce55e85 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -447,14 +447,14 @@ add_guest_pages(struct virtio_net *dev, struct virtio_memory_region *reg,
reg_size -= size;
while (reg_size > 0) {
+ size = reg_size >= page_size ? page_size : reg_size;
host_phys_addr = rte_mem_virt2phy((void *)(uintptr_t)
host_user_addr);
- add_one_guest_page(dev, guest_phys_addr, host_phys_addr,
- page_size);
+ add_one_guest_page(dev, guest_phys_addr, host_phys_addr, size);
- host_user_addr += page_size;
- guest_phys_addr += page_size;
- reg_size -= page_size;
+ host_user_addr += size;
+ guest_phys_addr += size;
+ reg_size -= size;
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH] i40e: fix setting of default MAC address
From: Igor Ryzhov @ 2016-12-01 9:52 UTC (permalink / raw)
To: dev@dpdk.org; +Cc: Helin Zhang, jingjing.wu
In-Reply-To: <1479990879-26598-1-git-send-email-iryzhov@nfware.com>
Ping.
On Thu, Nov 24, 2016 at 3:34 PM, Igor Ryzhov <iryzhov@nfware.com> wrote:
> While testing X710 cards in our lab I found that setting of default MAC
> address
> doesn't work correctly for i40e driver. I compared DPDK driver
> implementation
> with Linux driver implementation and found that a lot of code is lost in
> DPDK.
> I tried to make DPDK implementation similar to Linux implementation and it
> worked for me – now everything is working. But I'm not sure that my
> changes are
> correct so, please, maintainers, check the patch very careful.
>
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_
> ethdev.c
> index 67778ba..b73f9c8 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -9694,6 +9694,7 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev,
> static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> struct ether_addr *mac_addr)
> {
> + struct i40e_vsi *vsi = I40E_DEV_PRIVATE_TO_MAIN_VSI(
> dev->data->dev_private);
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->
> data->dev_private);
>
> if (!is_valid_assigned_ether_addr(mac_addr)) {
> @@ -9701,8 +9702,33 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
> return;
> }
>
> - /* Flags: 0x3 updates port address */
> - i40e_aq_mac_address_write(hw, 0x3, mac_addr->addr_bytes, NULL);
> + i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL,
> mac_addr->addr_bytes, NULL);
> +
> + if (!memcmp(&dev->data->mac_addrs[0].addr_bytes, hw->mac.addr,
> ETH_ADDR_LEN)) {
> + struct i40e_aqc_remove_macvlan_element_data element;
> +
> + memset(&element, 0, sizeof(element));
> + memcpy(element.mac_addr, &dev->data->mac_addrs[0].addr_bytes,
> ETH_ADDR_LEN);
> + element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
> + i40e_aq_remove_macvlan(hw, vsi->seid, &element, 1, NULL);
> + } else {
> + i40e_vsi_delete_mac(vsi, &dev->data->mac_addrs[0]);
> + }
> +
> + if (!memcmp(mac_addr->addr_bytes, hw->mac.addr, ETH_ADDR_LEN)) {
> + struct i40e_aqc_add_macvlan_element_data element;
> +
> + memset(&element, 0, sizeof(element));
> + memcpy(element.mac_addr, hw->mac.addr, ETH_ADDR_LEN);
> + element.flags = CPU_TO_LE16(I40E_AQC_MACVLAN_
> ADD_PERFECT_MATCH);
> + i40e_aq_add_macvlan(hw, vsi->seid, &element, 1, NULL);
> + } else {
> + struct i40e_mac_filter_info filter;
> +
> + memcpy(&filter.mac_addr, mac_addr, ETH_ADDR_LEN);
> + filter.filter_type = RTE_MAC_PERFECT_MATCH;
> + i40e_vsi_add_mac(vsi, &filter);
> + }
> }
>
> static int
> --
> 2.6.4
>
>
^ permalink raw reply
* [PATCH 3/3] doc: add missing supported algos for AESNI MB PMD
From: Pablo de Lara @ 2016-12-01 9:36 UTC (permalink / raw)
To: declan.doherty; +Cc: dev, Pablo de Lara
In-Reply-To: <1480585017-63239-1-git-send-email-pablo.de.lara.guarch@intel.com>
AESNI MB PMD supports SHA224-HMAC and SHA384-HMAC,
but the documentation was not updated with this.
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
doc/guides/cryptodevs/aesni_mb.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/doc/guides/cryptodevs/aesni_mb.rst b/doc/guides/cryptodevs/aesni_mb.rst
index cb429d7..8b18eba 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -55,7 +55,9 @@ Cipher algorithms:
Hash algorithms:
* RTE_CRYPTO_HASH_SHA1_HMAC
+* RTE_CRYPTO_HASH_SHA224_HMAC
* RTE_CRYPTO_HASH_SHA256_HMAC
+* RTE_CRYPTO_HASH_SHA384_HMAC
* RTE_CRYPTO_HASH_SHA512_HMAC
Limitations
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] crypto/aesni_mb: add single operation functionality
From: Pablo de Lara @ 2016-12-01 9:36 UTC (permalink / raw)
To: declan.doherty; +Cc: dev, Pablo de Lara
In-Reply-To: <1480585017-63239-1-git-send-email-pablo.de.lara.guarch@intel.com>
Update driver to use new AESNI Multibuffer IPSec library single
operation functionality (cipher only and authentication only).
This patch also adds tests for this new feature.
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
app/test/test_cryptodev.c | 34 ++++++++++++++++++++
app/test/test_cryptodev_aes_test_vectors.h | 36 ++++++++++++++-------
app/test/test_cryptodev_hash_test_vectors.h | 36 ++++++++++++++-------
doc/guides/cryptodevs/aesni_mb.rst | 2 --
doc/guides/rel_notes/release_17_02.rst | 1 +
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 49 +++++++++++++++++++----------
6 files changed, 116 insertions(+), 42 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index f1f3542..5895d99 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -1466,6 +1466,38 @@ test_AES_CBC_HMAC_SHA512_decrypt_perform(struct rte_cryptodev_sym_session *sess,
}
static int
+test_AES_cipheronly_mb_all(void)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ int status;
+
+ status = test_blockcipher_all_tests(ts_params->mbuf_pool,
+ ts_params->op_mpool, ts_params->valid_devs[0],
+ RTE_CRYPTODEV_AESNI_MB_PMD,
+ BLKCIPHER_AES_CIPHERONLY_TYPE);
+
+ TEST_ASSERT_EQUAL(status, 0, "Test failed");
+
+ return TEST_SUCCESS;
+}
+
+static int
+test_authonly_mb_all(void)
+{
+ struct crypto_testsuite_params *ts_params = &testsuite_params;
+ int status;
+
+ status = test_blockcipher_all_tests(ts_params->mbuf_pool,
+ ts_params->op_mpool, ts_params->valid_devs[0],
+ RTE_CRYPTODEV_AESNI_MB_PMD,
+ BLKCIPHER_AUTHONLY_TYPE);
+
+ TEST_ASSERT_EQUAL(status, 0, "Test failed");
+
+ return TEST_SUCCESS;
+}
+
+static int
test_AES_chain_mb_all(void)
{
struct crypto_testsuite_params *ts_params = &testsuite_params;
@@ -6559,6 +6591,8 @@ static struct unit_test_suite cryptodev_aesni_mb_testsuite = {
.teardown = testsuite_teardown,
.unit_test_cases = {
TEST_CASE_ST(ut_setup, ut_teardown, test_AES_chain_mb_all),
+ TEST_CASE_ST(ut_setup, ut_teardown, test_AES_cipheronly_mb_all),
+ TEST_CASE_ST(ut_setup, ut_teardown, test_authonly_mb_all),
TEST_CASES_END() /**< NULL terminate unit test array */
}
diff --git a/app/test/test_cryptodev_aes_test_vectors.h b/app/test/test_cryptodev_aes_test_vectors.h
index efbe7da..898aae1 100644
--- a/app/test/test_cryptodev_aes_test_vectors.h
+++ b/app/test/test_cryptodev_aes_test_vectors.h
@@ -1025,84 +1025,96 @@ static const struct blockcipher_test_case aes_cipheronly_test_cases[] = {
.test_data = &aes_test_data_4,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-128-CBC Decryption",
.test_data = &aes_test_data_4,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-192-CBC Encryption",
.test_data = &aes_test_data_10,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-192-CBC Decryption",
.test_data = &aes_test_data_10,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-256-CBC Encryption",
.test_data = &aes_test_data_11,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-256-CBC Decryption",
.test_data = &aes_test_data_11,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-128-CTR Encryption",
.test_data = &aes_test_data_1,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-128-CTR Decryption",
.test_data = &aes_test_data_1,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-192-CTR Encryption",
.test_data = &aes_test_data_2,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-192-CTR Decryption",
.test_data = &aes_test_data_2,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-256-CTR Encryption",
.test_data = &aes_test_data_3,
.op_mask = BLOCKCIPHER_TEST_OP_ENCRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "AES-256-CTR Decryption",
.test_data = &aes_test_data_3,
.op_mask = BLOCKCIPHER_TEST_OP_DECRYPT,
.pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
- BLOCKCIPHER_TEST_TARGET_PMD_QAT
+ BLOCKCIPHER_TEST_TARGET_PMD_QAT |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
};
diff --git a/app/test/test_cryptodev_hash_test_vectors.h b/app/test/test_cryptodev_hash_test_vectors.h
index 9f095cf..a897de4 100644
--- a/app/test/test_cryptodev_hash_test_vectors.h
+++ b/app/test/test_cryptodev_hash_test_vectors.h
@@ -358,13 +358,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-MD5 Digest",
.test_data = &hmac_md5_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-MD5 Digest Verify",
.test_data = &hmac_md5_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "SHA1 Digest",
@@ -382,13 +384,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-SHA1 Digest",
.test_data = &hmac_sha1_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-SHA1 Digest Verify",
.test_data = &hmac_sha1_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "SHA224 Digest",
@@ -406,13 +410,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-SHA224 Digest",
.test_data = &hmac_sha224_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-SHA224 Digest Verify",
.test_data = &hmac_sha224_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "SHA256 Digest",
@@ -430,13 +436,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-SHA256 Digest",
.test_data = &hmac_sha256_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-SHA256 Digest Verify",
.test_data = &hmac_sha256_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "SHA384 Digest",
@@ -454,13 +462,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-SHA384 Digest",
.test_data = &hmac_sha384_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-SHA384 Digest Verify",
.test_data = &hmac_sha384_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "SHA512 Digest",
@@ -478,13 +488,15 @@ static const struct blockcipher_test_case hash_test_cases[] = {
.test_descr = "HMAC-SHA512 Digest",
.test_data = &hmac_sha512_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_GEN,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
{
.test_descr = "HMAC-SHA512 Digest Verify",
.test_data = &hmac_sha512_test_vector,
.op_mask = BLOCKCIPHER_TEST_OP_AUTH_VERIFY,
- .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL
+ .pmd_mask = BLOCKCIPHER_TEST_TARGET_PMD_OPENSSL |
+ BLOCKCIPHER_TEST_TARGET_PMD_MB
},
};
diff --git a/doc/guides/cryptodevs/aesni_mb.rst b/doc/guides/cryptodevs/aesni_mb.rst
index b47cb6a..cb429d7 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -62,8 +62,6 @@ Limitations
-----------
* Chained mbufs are not supported.
-* Hash only is not supported.
-* Cipher only is not supported.
* Only in-place is currently supported (destination address is the same as source address).
* Only supports session-oriented API implementation (session-less APIs are not supported).
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 4f666df..5aa8a94 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -49,6 +49,7 @@ New Features
* The Intel(R) Multi Buffer Crypto for IPsec library used in
AESNI MB PMD has been moved to a new repository, in github.
+ * Support for single operations (cipher only and authentication only).
Resolved Issues
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index f07cd07..7591cc5 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -110,21 +110,22 @@ calculate_auth_precomputes(hash_one_block_t one_block_hash,
static int
aesni_mb_get_chain_order(const struct rte_crypto_sym_xform *xform)
{
- /*
- * Multi-buffer only supports HASH_CIPHER or CIPHER_HASH chained
- * operations, all other options are invalid, so we must have exactly
- * 2 xform structs chained together
- */
- if (xform->next == NULL || xform->next->next != NULL)
+ if (xform == NULL)
return -1;
- if (xform->type == RTE_CRYPTO_SYM_XFORM_AUTH &&
- xform->next->type == RTE_CRYPTO_SYM_XFORM_CIPHER)
- return HASH_CIPHER;
-
- if (xform->type == RTE_CRYPTO_SYM_XFORM_CIPHER &&
- xform->next->type == RTE_CRYPTO_SYM_XFORM_AUTH)
+ if ((xform->type == RTE_CRYPTO_SYM_XFORM_CIPHER) &&
+ (xform->cipher.op == RTE_CRYPTO_CIPHER_OP_ENCRYPT) &&
+ ((xform->next == NULL) ||
+ (xform->next->type == RTE_CRYPTO_SYM_XFORM_AUTH)))
return CIPHER_HASH;
+ if ((xform->type == RTE_CRYPTO_SYM_XFORM_CIPHER) &&
+ (xform->cipher.op == RTE_CRYPTO_CIPHER_OP_DECRYPT) &&
+ (xform->next == NULL))
+ return HASH_CIPHER;
+ if ((xform->type == RTE_CRYPTO_SYM_XFORM_AUTH) &&
+ (xform->next == NULL ||
+ xform->next->type == RTE_CRYPTO_SYM_XFORM_CIPHER))
+ return HASH_CIPHER;
return -1;
}
@@ -137,6 +138,11 @@ aesni_mb_set_session_auth_parameters(const struct aesni_mb_ops *mb_ops,
{
hash_one_block_t hash_oneblock_fn;
+ if (xform == NULL) {
+ sess->auth.algo = NULL_HASH;
+ return 0;
+ }
+
if (xform->type != RTE_CRYPTO_SYM_XFORM_AUTH) {
MB_LOG_ERR("Crypto xform struct not of type auth");
return -1;
@@ -199,6 +205,11 @@ aesni_mb_set_session_cipher_parameters(const struct aesni_mb_ops *mb_ops,
{
aes_keyexp_t aes_keyexp_fn;
+ if (xform == NULL) {
+ sess->cipher.mode = NULL_CIPHER;
+ return 0;
+ }
+
if (xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) {
MB_LOG_ERR("Crypto xform struct not of type cipher");
return -1;
@@ -270,8 +281,13 @@ aesni_mb_set_session_parameters(const struct aesni_mb_ops *mb_ops,
switch (aesni_mb_get_chain_order(xform)) {
case HASH_CIPHER:
sess->chain_order = HASH_CIPHER;
- auth_xform = xform;
- cipher_xform = xform->next;
+ if (xform->type == RTE_CRYPTO_SYM_XFORM_AUTH) {
+ auth_xform = xform;
+ cipher_xform = xform->next;
+ } else {
+ cipher_xform = xform;
+ auth_xform = xform->next;
+ }
break;
case CIPHER_HASH:
sess->chain_order = CIPHER_HASH;
@@ -396,7 +412,7 @@ process_crypto_op(struct aesni_mb_qp *qp, struct rte_crypto_op *op,
}
/* Set digest output location */
- if (job->cipher_direction == DECRYPT) {
+ if (job->cipher_direction == DECRYPT && job->hash_alg != NULL_HASH) {
job->auth_tag_output = (uint8_t *)rte_pktmbuf_append(m_dst,
get_digest_byte_length(job->hash_alg));
@@ -471,7 +487,8 @@ post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job)
return op;
} else if (job->chain_order == HASH_CIPHER) {
/* Verify digest if required */
- if (memcmp(job->auth_tag_output, op->sym->auth.digest.data,
+ if (job->hash_alg != NULL_HASH && memcmp(job->auth_tag_output,
+ op->sym->auth.digest.data,
job->auth_tag_output_len_in_bytes) != 0)
op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] doc: update AESNI MB PMD guide
From: Pablo de Lara @ 2016-12-01 9:36 UTC (permalink / raw)
To: declan.doherty; +Cc: dev, Pablo de Lara
In-Reply-To: <1480585017-63239-1-git-send-email-pablo.de.lara.guarch@intel.com>
The Intel(R) Multi Buffer Crypto library used in the AESNI MB PMD
has been moved to a new repository, in github.
This patch updates the link where it can be downloaded.
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
doc/guides/cryptodevs/aesni_mb.rst | 10 +++-------
doc/guides/rel_notes/release_17_02.rst | 7 +++++++
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/doc/guides/cryptodevs/aesni_mb.rst b/doc/guides/cryptodevs/aesni_mb.rst
index e812e95..b47cb6a 100644
--- a/doc/guides/cryptodevs/aesni_mb.rst
+++ b/doc/guides/cryptodevs/aesni_mb.rst
@@ -66,21 +66,17 @@ Limitations
* Cipher only is not supported.
* Only in-place is currently supported (destination address is the same as source address).
* Only supports session-oriented API implementation (session-less APIs are not supported).
-* Not performance tuned.
Installation
------------
To build DPDK with the AESNI_MB_PMD the user is required to download the mult-
-buffer library from `here <https://downloadcenter.intel.com/download/22972>`_
-and compile it on their user system before building DPDK. When building the
-multi-buffer library it is necessary to have YASM package installed and also
-requires the overriding of YASM path when building, as a path is hard coded in
-the Makefile of the release package.
+buffer library from `here <https://github.com/01org/intel-ipsec-mb>`_
+and compile it on their user system before building DPDK.
.. code-block:: console
- make YASM=/usr/bin/yasm
+ make
Initialization
--------------
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 873333b..4f666df 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -44,6 +44,13 @@ New Features
* Scatter-gather list (SGL) support.
+
+* **Updated the AESNI MB PMD.**
+
+ * The Intel(R) Multi Buffer Crypto for IPsec library used in
+ AESNI MB PMD has been moved to a new repository, in github.
+
+
Resolved Issues
---------------
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] AESNI MB PMD updates
From: Pablo de Lara @ 2016-12-01 9:36 UTC (permalink / raw)
To: declan.doherty; +Cc: dev, Pablo de Lara
The library used in AESNI MB PMD, Intel Multi Buffer Crypto for IPsec,
has been migrated to a new location, in github (see documentation patch
for the link).
The library has also been updated, so single crypto operations
are supported (cipher and authentication only). Therefore, the PMD
has been updated to support these operations.
This patchset depends on patchset "Add scatter-gather list capability to
Intel QuickAssist Technology driver" (http://dpdk.org/ml/archives/dev/2016-November/050947.html)
Pablo de Lara (3):
doc: update AESNI MB PMD guide
crypto/aesni_mb: add single operation functionality
doc: add missing supported algos for AESNI MB PMD
app/test/test_cryptodev.c | 34 ++++++++++++++++++++
app/test/test_cryptodev_aes_test_vectors.h | 36 ++++++++++++++-------
app/test/test_cryptodev_hash_test_vectors.h | 36 ++++++++++++++-------
doc/guides/cryptodevs/aesni_mb.rst | 14 +++------
doc/guides/rel_notes/release_17_02.rst | 8 +++++
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 49 +++++++++++++++++++----------
6 files changed, 128 insertions(+), 49 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Thomas Monjalon @ 2016-12-01 8:58 UTC (permalink / raw)
To: Adrien Mazarguil
Cc: Ananyev, Konstantin, dev, 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
In-Reply-To: <20161201071518.GG10340@6wind.com>
2016-12-01 08:15, Adrien Mazarguil:
> 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.
I agree that tx_prepare() should become mandatory shortly.
> [...]
> > > 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?
>
> 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.
I like this idea!
^ permalink raw reply
* Re: [PATCH 00/22] Generic flow API (rte_flow)
From: Adrien Mazarguil @ 2016-12-01 8:39 UTC (permalink / raw)
To: Pei, Yulong
Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
Olivier Matz, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03946BD34@shsmsx102.ccr.corp.intel.com>
Hi Yulong,
On Mon, Nov 28, 2016 at 10:03:53AM +0000, Pei, Yulong wrote:
> Hi Adrien,
>
> I think that you already did test for your patchset, do you have any automated test scripts can be shared for validation since there did not have testpmd flow command documentation yet?
No automated script, at least not yet. I intend to submit v2 with extra
API documentation, testpmd commands with examples of expected behavior and
output, as well as fixes for the issues pointed out by Nelio.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH 01/22] ethdev: introduce generic flow API
From: Adrien Mazarguil @ 2016-12-01 8:36 UTC (permalink / raw)
To: Kevin Traynor
Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz, sugesh.chandra
In-Reply-To: <59393e58-6c85-d2e5-1aab-a721fe9c933e@redhat.com>
Hi Kevin,
On Wed, Nov 30, 2016 at 05:47:17PM +0000, Kevin Traynor wrote:
> Hi Adrien,
>
> On 11/16/2016 04:23 PM, Adrien Mazarguil wrote:
> > This new API supersedes all the legacy filter types described in
> > rte_eth_ctrl.h. It is slightly higher level and as a result relies more on
> > PMDs to process and validate flow rules.
> >
> > Benefits:
> >
> > - A unified API is easier to program for, applications do not have to be
> > written for a specific filter type which may or may not be supported by
> > the underlying device.
> >
> > - The behavior of a flow rule is the same regardless of the underlying
> > device, applications do not need to be aware of hardware quirks.
> >
> > - Extensible by design, API/ABI breakage should rarely occur if at all.
> >
> > - Documentation is self-standing, no need to look up elsewhere.
> >
> > Existing filter types will be deprecated and removed in the near future.
>
> I'd suggest to add a deprecation notice to deprecation.rst, ideally with
> a target release.
Will do, not a sure about the target release though. It seems a bit early
since no PMD really supports this API yet.
[...]
> > diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
> > new file mode 100644
> > index 0000000..064963d
> > --- /dev/null
> > +++ b/lib/librte_ether/rte_flow.c
> > @@ -0,0 +1,159 @@
> > +/*-
> > + * BSD LICENSE
> > + *
> > + * Copyright 2016 6WIND S.A.
> > + * Copyright 2016 Mellanox.
>
> There's Mellanox copyright but you are the only signed-off-by - is that
> right?
Yes, I'm the primary maintainer for Mellanox PMDs and this API was designed
on their behalf to expose several features from mlx4/mlx5 as the existing
filter types had too many limitations.
[...]
> > +/* Get generic flow operations structure from a port. */
> > +const struct rte_flow_ops *
> > +rte_flow_ops_get(uint8_t port_id, struct rte_flow_error *error)
> > +{
> > + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > + const struct rte_flow_ops *ops;
> > + int code;
> > +
> > + if (unlikely(!rte_eth_dev_is_valid_port(port_id)))
> > + code = ENODEV;
> > + else if (unlikely(!dev->dev_ops->filter_ctrl ||
> > + dev->dev_ops->filter_ctrl(dev,
> > + RTE_ETH_FILTER_GENERIC,
> > + RTE_ETH_FILTER_GET,
> > + &ops) ||
> > + !ops))
> > + code = ENOTSUP;
> > + else
> > + return ops;
> > + rte_flow_error_set(error, code, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(code));
> > + return NULL;
> > +}
> > +
>
> Is it expected that the application or pmd will provide locking between
> these functions if required? I think it's going to have to be the app.
Locking is indeed expected to be performed by applications. This API only
documents places where locking would make sense if necessary and expected
behavior.
Like all control path APIs, this one assumes a single control thread.
Applications must take the necessary precautions.
[...]
> > +/**
> > + * Flow rule attributes.
> > + *
> > + * Priorities are set on two levels: per group and per rule within groups.
> > + *
> > + * Lower values denote higher priority, the highest priority for both levels
> > + * is 0, so that a rule with priority 0 in group 8 is always matched after a
> > + * rule with priority 8 in group 0.
> > + *
> > + * Although optional, applications are encouraged to group similar rules as
> > + * much as possible to fully take advantage of hardware capabilities
> > + * (e.g. optimized matching) and work around limitations (e.g. a single
> > + * pattern type possibly allowed in a given group).
> > + *
> > + * Group and priority levels are arbitrary and up to the application, they
> > + * do not need to be contiguous nor start from 0, however the maximum number
> > + * varies between devices and may be affected by existing flow rules.
> > + *
> > + * If a packet is matched by several rules of a given group for a given
> > + * priority level, the outcome is undefined. It can take any path, may be
> > + * duplicated or even cause unrecoverable errors.
>
> I get what you are trying to do here wrt supporting multiple
> pmds/hardware implementations and it's a good idea to keep it flexible.
>
> Given that the outcome is undefined, it would be nice that the
> application has a way of finding the specific effects for verification
> and debugging.
Right, however it was deemed a bit difficult to manage in many cases hence
the vagueness.
For example, suppose two rules with the same group and priority, one
matching any IPv4 header, the other one any UDP header:
- TCPv4 packets => rule #1.
- UDPv6 packets => rule #2.
- UDPv4 packets => both?
That last one is perhaps invalid, checking that some unspecified protocol
combination does not overlap is expensive and may miss corner cases, even
assuming this is not an issue, what if the application guarantees that no
UDPv4 packets can ever hit that rule?
Suggestions are welcome though, perhaps we can refine the description.
> > + *
> > + * Note that support for more than a single group and priority level is not
> > + * guaranteed.
> > + *
> > + * Flow rules can apply to inbound and/or outbound traffic (ingress/egress).
> > + *
> > + * Several pattern items and actions are valid and can be used in both
> > + * directions. Those valid for only one direction are described as such.
> > + *
> > + * Specifying both directions at once is not recommended but may be valid in
> > + * some cases, such as incrementing the same counter twice.
> > + *
> > + * Not specifying any direction is currently an error.
> > + */
> > +struct rte_flow_attr {
> > + uint32_t group; /**< Priority group. */
> > + uint32_t priority; /**< Priority level within group. */
> > + uint32_t ingress:1; /**< Rule applies to ingress traffic. */
> > + uint32_t egress:1; /**< Rule applies to egress traffic. */
> > + uint32_t reserved:30; /**< Reserved, must be zero. */
> > +};
[...]
> > +/**
> > + * RTE_FLOW_ITEM_TYPE_VF
> > + *
> > + * Matches packets addressed to a virtual function ID of the device.
> > + *
> > + * If the underlying device function differs from the one that would
> > + * normally receive the matched traffic, specifying this item prevents it
> > + * from reaching that device unless the flow rule contains a VF
> > + * action. Packets are not duplicated between device instances by default.
> > + *
> > + * - Likely to return an error or never match any traffic if this causes a
> > + * VF device to match traffic addressed to a different VF.
> > + * - Can be specified multiple times to match traffic addressed to several
> > + * specific VFs.
> > + * - Can be combined with a PF item to match both PF and VF traffic.
> > + *
> > + * A zeroed mask can be used to match any VF.
>
> can you refer explicitly to id
If you mean "VF" to "VF ID" then yes, will do it for v2.
> > + */
> > +struct rte_flow_item_vf {
> > + uint32_t id; /**< Destination VF ID. */
> > +};
[...]
> > +/**
> > + * Matching pattern item definition.
> > + *
> > + * A pattern is formed by stacking items starting from the lowest protocol
> > + * layer to match. This stacking restriction does not apply to meta items
> > + * which can be placed anywhere in the stack with no effect on the meaning
> > + * of the resulting pattern.
> > + *
> > + * A stack is terminated by a END item.
> > + *
> > + * The spec field should be a valid pointer to a structure of the related
> > + * item type. It may be set to NULL in many cases to use default values.
> > + *
> > + * Optionally, last can point to a structure of the same type to define an
> > + * inclusive range. This is mostly supported by integer and address fields,
> > + * may cause errors otherwise. Fields that do not support ranges must be set
> > + * to the same value as their spec counterparts.
> > + *
> > + * By default all fields present in spec are considered relevant.* This
>
> typo "*"
No, that's an asterisk for a footnote below. Perhaps it is a bit unusual,
would something like "[1]" look better?
> > + * behavior can be altered by providing a mask structure of the same type
> > + * with applicable bits set to one. It can also be used to partially filter
> > + * out specific fields (e.g. as an alternate mean to match ranges of IP
> > + * addresses).
> > + *
> > + * Note this is a simple bit-mask applied before interpreting the contents
> > + * of spec and last, which may yield unexpected results if not used
> > + * carefully. For example, if for an IPv4 address field, spec provides
> > + * 10.1.2.3, last provides 10.3.4.5 and mask provides 255.255.0.0, the
> > + * effective range is 10.1.0.0 to 10.3.255.255.
> > + *
See footnote below:
> > + * * The defaults for data-matching items such as IPv4 when mask is not
> > + * specified actually depend on the underlying implementation since only
> > + * recognized fields can be taken into account.
> > + */
> > +struct rte_flow_item {
> > + enum rte_flow_item_type type; /**< Item type. */
> > + const void *spec; /**< Pointer to item specification structure. */
> > + const void *last; /**< Defines an inclusive range (spec to last). */
> > + const void *mask; /**< Bit-mask applied to spec and last. */
> > +};
> > +
> > +/**
> > + * Action types.
> > + *
> > + * Each possible action is represented by a type. Some have associated
> > + * configuration structures. Several actions combined in a list can be
> > + * affected to a flow rule. That list is not ordered.
> > + *
> > + * They fall in three categories:
> > + *
> > + * - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
> > + * processing matched packets by subsequent flow rules, unless overridden
> > + * with PASSTHRU.
> > + *
> > + * - Non terminating actions (PASSTHRU, DUP) that leave matched packets up
> > + * for additional processing by subsequent flow rules.
> > + *
> > + * - Other non terminating meta actions that do not affect the fate of
> > + * packets (END, VOID, MARK, FLAG, COUNT).
> > + *
> > + * When several actions are combined in a flow rule, they should all have
> > + * different types (e.g. dropping a packet twice is not possible). The
> > + * defined behavior is for PMDs to only take into account the last action of
> > + * a given type found in the list. PMDs still perform error checking on the
> > + * entire list.
>
> why do you define that the pmd will interpret multiple same type rules
> in this way...would it not make more sense for the pmd to just return
> EINVAL for an invalid set of rules? It seems more transparent for the
> application.
Well, I had to define something as a default. The reason is that any number
of VOID actions may specified and did not want that to be a special case in
order to keep PMD parsers as simple as possible. I'll settle for EINVAL (or
some other error) if at least one PMD maintainer other than Nelio who
intends to implement this API is not convinced by this explanation, all
right?
[...]
> > +/**
> > + * RTE_FLOW_ACTION_TYPE_MARK
> > + *
> > + * Attaches a 32 bit value to packets.
> > + *
> > + * This value is arbitrary and application-defined. For compatibility with
> > + * FDIR it is returned in the hash.fdir.hi mbuf field. PKT_RX_FDIR_ID is
> > + * also set in ol_flags.
> > + */
> > +struct rte_flow_action_mark {
> > + uint32_t id; /**< 32 bit value to return with packets. */
> > +};
>
> One use case I thought we would be able to do for OVS is classification
> in hardware and the unique flow id is sent with the packet to software.
> But in OVS the ufid is 128 bits, so it means we can't and there is still
> the miniflow extract overhead. I'm not sure if there is a practical way
> around this.
>
> Sugesh (cc'd) has looked at this before and may be able to comment or
> correct me.
Yes, we settled on 32 bit because currently no known hardware implementation
supports more than this. If that changes, another action with a larger type
shall be provided (no ABI breakage).
Also since even 64 bit would not be enough for the use case you mention,
there is no choice but use this as an indirect value (such as an array or
hash table index/value).
[...]
> > +/**
> > + * RTE_FLOW_ACTION_TYPE_RSS
> > + *
> > + * Similar to QUEUE, except RSS is additionally performed on packets to
> > + * spread them among several queues according to the provided parameters.
> > + *
> > + * Note: RSS hash result is normally stored in the hash.rss mbuf field,
> > + * however it conflicts with the MARK action as they share the same
> > + * space. When both actions are specified, the RSS hash is discarded and
> > + * PKT_RX_RSS_HASH is not set in ol_flags. MARK has priority. The mbuf
> > + * structure should eventually evolve to store both.
> > + *
> > + * Terminating by default.
> > + */
> > +struct rte_flow_action_rss {
> > + const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> > + uint16_t queues; /**< Number of entries in queue[]. */
> > + uint16_t queue[]; /**< Queues indices to use. */
>
> I'd try and avoid queue and queues - someone will say "huh?" when
> reading code. s/queues/num ?
Agreed, will update for v2.
> > +};
> > +
> > +/**
> > + * RTE_FLOW_ACTION_TYPE_VF
> > + *
> > + * Redirects packets to a virtual function (VF) of the current device.
> > + *
> > + * Packets matched by a VF pattern item can be redirected to their original
> > + * VF ID instead of the specified one. This parameter may not be available
> > + * and is not guaranteed to work properly if the VF part is matched by a
> > + * prior flow rule or if packets are not addressed to a VF in the first
> > + * place.
>
> Not clear what you mean by "not guaranteed to work if...". Please return
> fail when this action is used if this is not going to work.
Again, this is a case where it is difficult for a PMD to determine if the
entire list of flow rules makes sense. Perhaps it does, perhaps whatever
goes through has already been filtered out of possible issues.
Here the documentation states the precautions an application should take to
guarantee it will work as intended. Perhaps it can be reworded (any
suggestion?), but a PMD can certainly not provide any strong guarantee.
> > + *
> > + * Terminating by default.
> > + */
> > +struct rte_flow_action_vf {
> > + uint32_t original:1; /**< Use original VF ID if possible. */
> > + uint32_t reserved:31; /**< Reserved, must be zero. */
> > + uint32_t id; /**< VF ID to redirect packets to. */
> > +};
[...]
> > +/**
> > + * Check whether a flow rule can be created on a given port.
> > + *
> > + * While this function has no effect on the target device, the flow rule is
> > + * validated against its current configuration state and the returned value
> > + * should be considered valid by the caller for that state only.
> > + *
> > + * The returned value is guaranteed to remain valid only as long as no
> > + * successful calls to rte_flow_create() or rte_flow_destroy() are made in
> > + * the meantime and no device parameter affecting flow rules in any way are
> > + * modified, due to possible collisions or resource limitations (although in
> > + * such cases EINVAL should not be returned).
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] attr
> > + * Flow rule attributes.
> > + * @param[in] pattern
> > + * Pattern specification (list terminated by the END pattern item).
> > + * @param[in] actions
> > + * Associated actions (list terminated by the END action).
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + * 0 if flow rule is valid and can be created. A negative errno value
> > + * otherwise (rte_errno is also set), the following errors are defined:
> > + *
> > + * -ENOSYS: underlying device does not support this functionality.
> > + *
> > + * -EINVAL: unknown or invalid rule specification.
> > + *
> > + * -ENOTSUP: valid but unsupported rule specification (e.g. partial
> > + * bit-masks are unsupported).
> > + *
> > + * -EEXIST: collision with an existing rule.
> > + *
> > + * -ENOMEM: not enough resources.
> > + *
> > + * -EBUSY: action cannot be performed due to busy device resources, may
> > + * succeed if the affected queues or even the entire port are in a stopped
> > + * state (see rte_eth_dev_rx_queue_stop() and rte_eth_dev_stop()).
> > + */
> > +int
> > +rte_flow_validate(uint8_t port_id,
> > + const struct rte_flow_attr *attr,
> > + const struct rte_flow_item pattern[],
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error);
>
> Why not just use rte_flow_create() and get an error? Is it less
> disruptive to do a validate and find the rule cannot be created, than
> using a create directly?
The rationale can be found in the original RFC, which I'll convert to actual
documentation in v2. In short:
- Calling rte_flow_validate() before rte_flow_create() is useless since
rte_flow_create() also performs validation.
- We cannot possibly express a full static set of allowed flow rules, even
if we could, it usually depends on the current hardware configuration
therefore would not be static.
- rte_flow_validate() is thus provided as a replacement for capability
flags. It can be used to determine during initialization if the underlying
device can support the typical flow rules an application might want to
provide later and do something useful with that information (e.g. always
use software fallback due to HW limitations).
- rte_flow_validate() being a subset of rte_flow_create(), it is essentially
free to expose.
> > +
> > +/**
> > + * Create a flow rule on a given port.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] attr
> > + * Flow rule attributes.
> > + * @param[in] pattern
> > + * Pattern specification (list terminated by the END pattern item).
> > + * @param[in] actions
> > + * Associated actions (list terminated by the END action).
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + * A valid handle in case of success, NULL otherwise and rte_errno is set
> > + * to the positive version of one of the error codes defined for
> > + * rte_flow_validate().
> > + */
> > +struct rte_flow *
> > +rte_flow_create(uint8_t port_id,
> > + const struct rte_flow_attr *attr,
> > + const struct rte_flow_item pattern[],
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error);
>
> General question - are these functions threadsafe? In the OVS example
> you could have several threads wanting to create flow rules at the same
> time for same or different ports.
No they aren't, applications have to perform their own locking. The RFC (to
be converted to actual documentation in v2) says that:
- API operations are synchronous and blocking (``EAGAIN`` cannot be
returned).
- There is no provision for reentrancy/multi-thread safety, although nothing
should prevent different devices from being configured at the same
time. PMDs may protect their control path functions accordingly.
> > +
> > +/**
> > + * Destroy a flow rule on a given port.
> > + *
> > + * Failure to destroy a flow rule handle may occur when other flow rules
> > + * depend on it, and destroying it would result in an inconsistent state.
> > + *
> > + * This function is only guaranteed to succeed if handles are destroyed in
> > + * reverse order of their creation.
>
> How can the application find this information out on error?
Without maintaining a list, they cannot. The specified case is the only
possible guarantee. That does not mean PMDs should not do their best to
destroy flow rules, only that ordering must remain consistent in case of
inability to destroy one.
What do you suggest?
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param flow
> > + * Flow rule handle to destroy.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +int
> > +rte_flow_destroy(uint8_t port_id,
> > + struct rte_flow *flow,
> > + struct rte_flow_error *error);
> > +
> > +/**
> > + * Destroy all flow rules associated with a port.
> > + *
> > + * In the unlikely event of failure, handles are still considered destroyed
> > + * and no longer valid but the port must be assumed to be in an inconsistent
> > + * state.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +int
> > +rte_flow_flush(uint8_t port_id,
> > + struct rte_flow_error *error);
>
> rte_flow_destroy_all() would be more descriptive (but breaks your style)
There are enough underscores as it is. I like flush, if enough people
complain we'll change it but it has to occur before the first public
release.
> > +
> > +/**
> > + * Query an existing flow rule.
> > + *
> > + * This function allows retrieving flow-specific data such as counters.
> > + * Data is gathered by special actions which must be present in the flow
> > + * rule definition.
>
> re last sentence, it would be good if you can put a link to
> RTE_FLOW_ACTION_TYPE_COUNT
Will do, I did not know how until very recently.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param flow
> > + * Flow rule handle to query.
> > + * @param action
> > + * Action type to query.
> > + * @param[in, out] data
> > + * Pointer to storage for the associated query data type.
>
> can this be anything other than rte_flow_query_count?
Likely in the future. I've only defined this one as a counterpart for
existing API functionality and because we wanted to expose it in mlx5.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +int
> > +rte_flow_query(uint8_t port_id,
> > + struct rte_flow *flow,
> > + enum rte_flow_action_type action,
> > + void *data,
> > + struct rte_flow_error *error);
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
>
> I don't see a way to dump all the rules for a port out. I think this is
> neccessary for degbugging. You could have a look through dpif.h in OVS
> and see how dpif_flow_dump_next() is used, it might be a good reference.
DPDK does not maintain flow rules and, depending on hardware capabilities
and level of compliance, PMDs do not necessarily do it either, particularly
since it requires space and application probably have a better method to
store these pointers for their own needs.
What you see here is only a PMD interface. Depending on applications needs,
generic helper functions built on top of these may be added to manage flow
rules in the future.
> Also, it would be nice if there were an api that would allow a test
> packet to be injected and traced for debugging - although I'm not
> exactly sure how well it could be traced. For reference:
> http://developers.redhat.com/blog/2016/10/12/tracing-packets-inside-open-vswitch/
Thanks for the link, I'm not sure how you'd do this either. Remember, as
generic as it looks, this interface is only meant to configure the
underlying device. You need to see it as one big offload, everything else
is left to applications.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Rahul Lakkireddy @ 2016-12-01 8:24 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, Stephen Hurd, Jan Medala, Jakub Palider, John Daley,
Adrien Mazarguil, Alejandro Lucero, Harish Patil, Rasesh Mody,
Jerin Jacob, Yuanhan Liu, Yong Wang, Tomasz Kulasek,
konstantin.ananyev@intel.com, olivier.matz@6wind.com
In-Reply-To: <8317180.L80Qf11uiu@xps13>
Hi Thomas,
On Monday, November 11/28/16, 2016 at 16:33:06 +0530, Thomas Monjalon wrote:
> We need attention of every PMD developers on this thread.
>
> Reminder of what Konstantin suggested:
> "
> - if the PMD supports TX offloads AND
> - if to be able use any of these offloads the upper layer SW would have to:
> * modify the contents of the packet OR
> * obey HW specific restrictions
> then it is a PMD developer responsibility to provide tx_prep() that would implement
> expected modifications of the packet contents and restriction checks.
> Otherwise, tx_prep() implementation is not required and can be safely set to NULL.
> "
>
> I copy/paste also my previous conclusion:
>
> Before txprep, there is only one API: the application must prepare the
> packets checksum itself (get_psd_sum in testpmd).
> With txprep, the application have 2 choices: keep doing the job itself
> or call txprep which calls a PMD-specific function.
> The question is: does non-Intel drivers need a checksum preparation for TSO?
> Will it behave well if txprep does nothing in these drivers?
>
> When looking at the code, most of drivers handle the TSO flags.
> But it is hard to know whether they rely on the pseudo checksum or not.
>
> git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG' drivers/net/
>
> drivers/net/bnxt/bnxt_txr.c
> drivers/net/cxgbe/sge.c
> drivers/net/e1000/em_rxtx.c
> drivers/net/e1000/igb_rxtx.c
> drivers/net/ena/ena_ethdev.c
> drivers/net/enic/enic_rxtx.c
> drivers/net/fm10k/fm10k_rxtx.c
> drivers/net/i40e/i40e_rxtx.c
> drivers/net/ixgbe/ixgbe_rxtx.c
> drivers/net/mlx4/mlx4.c
> drivers/net/mlx5/mlx5_rxtx.c
> drivers/net/nfp/nfp_net.c
> drivers/net/qede/qede_rxtx.c
> drivers/net/thunderx/nicvf_rxtx.c
> drivers/net/virtio/virtio_rxtx.c
> drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> Please, we need a comment for each driver saying
> "it is OK, we do not need any checksum preparation for TSO"
> or
> "yes we have to implement tx_prepare or TSO will not work in this mode"
For CXGBE PMD, "it is OK, we do not need any checksum preparation for
TSO".
Thanks,
Rahul
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Adrien Mazarguil @ 2016-12-01 7:19 UTC (permalink / raw)
To: Kulasek, TomaszX
Cc: Thomas Monjalon, dev@dpdk.org, Ananyev, Konstantin,
olivier.matz@6wind.com
In-Reply-To: <3042915272161B4EB253DA4D77EB373A14F57A50@IRSMSX102.ger.corp.intel.com>
Hi Tomasz,
On Wed, Nov 30, 2016 at 10:30:54AM +0000, Kulasek, TomaszX wrote:
[...]
> > > In my opinion the second approach is both faster to applications and
> > > more friendly from a usability perspective, am I missing something
> > obvious?
> >
> > I think it was not clearly explained in this patchset, but this is my
> > understanding:
> > tx_prepare and tx_burst can be called at different stages of a pipeline,
> > on different cores.
>
> Yes, this API is intended to be used optionaly, not only just before tx_burst.
>
> 1. Separating both stages:
> a) We may have a control over burst (packet content, validation) when needed.
> b) For invalid packets we may restore them or do some another task if needed (even on early stage of processing).
> c) Tx burst keep as simple as it should be.
>
> 2. Joining the functionality of tx_prepare and tx_burst have some disadvantages:
> a) When packet is invalid it cannot be restored by application should be dropped.
> b) Tx burst needs to modify the content of the packet.
> c) We have no way to eliminate overhead of preparation (tx_prepare) for the application where performance is a key.
>
> 3. Using tx callbacks
> a) We still need to have different implementations for different devices.
> b) The overhead in performance (comparing to the pair tx_prepare/tx_burst) will not be better while both ways uses very similar mechanism.
>
> In addition, tx_prepare mechanism can be turned off by compilation flag (as discussed with Jerin in http://dpdk.org/dev/patchwork/patch/15770/) to provide real NOOP functionality (e.g. for low-end CPUs, where even unnecessary memory dereference and check can have significant impact on performance).
Thanks for the reminder, also I've missed v12 for some reason and still
thought rte_phdr_cksum_fix() was some generic function that applications had
to use directly regardless.
Although I agree with your description, I still think there is an issue,
please see my reply to Konstantin [1].
[1] http://dpdk.org/ml/archives/dev/2016-December/050970.html
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Adrien Mazarguil @ 2016-12-01 7:15 UTC (permalink / raw)
To: Ananyev, Konstantin
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: <2601191342CEEE43887BDE71AB9772583F0E20C8@irsmsx105.ger.corp.intel.com>
Hi Konstantin,
On Wed, Nov 30, 2016 at 10:54:50AM +0000, Ananyev, Konstantin wrote:
[...]
> > Something is definitely needed here, and only PMDs can provide it. I think
> > applications should not have to clear checksum fields or initialize them to
> > some magic value, same goes for any other offload or hardware limitation
> > that needs to be worked around.
> >
> > tx_prep() is one possible answer to this issue, however as mentioned in the
> > original patch it can be very expensive if exposed by the PMD.
> >
> > Another issue I'm more concerned about is the way limitations are managed
> > (struct rte_eth_desc_lim). While not officially tied to tx_prep(), this
> > structure contains new fields that are only relevant to a few devices, and I
> > fear it will keep growing with each new hardware quirk to manage, breaking
> > ABIs in the process.
>
> Well, if some new HW capability/limitation would arise and we'd like to support
> it in DPDK, then yes we probably would need to think how to incorporate it here.
> 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. 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.
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().
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?
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.
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.
> Though, if you or any other PMD developer/maintainer would prefer
> for particular PMD to combine both functionalities into tx_burst() and
> keep tx_prep() as NOP - this is still possible too.
Whether they implement it or not, this issue does not impact PMDs anyway, we
should probably ask DPDK application developers instead.
[...]
> > For both mlx4 and mlx5 then,
> > "it is OK, we do not need any checksum preparation for TSO".
> >
> > Actually I do not think we'll ever need tx_prep() unless we add our own
> > quirks to struct rte_eth_desc_lim (and friends) which are currently quietly
> > handled by TX burst functions.
>
> Ok, so MLX PMD is not affected by these changes and tx_prep for MLX can be safely
> set to NULL, correct?
Correct, actually the rest of this message should be in a separate
thread. From the MLX side, there is no issue with tx_prepare().
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* [PATCH 3/3] maintainers: add stable mailing list
From: Yuanhan Liu @ 2016-12-01 7:06 UTC (permalink / raw)
To: dev; +Cc: Thomas Monjalon, John McNamara, Yuanhan Liu
In-Reply-To: <1480575999-14453-1-git-send-email-yuanhan.liu@linux.intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3df1754..076e86c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -35,6 +35,7 @@ F: scripts/test-build.sh
Stable Branches
---------------
T: git://dpdk.org/dpdk-stable
+M: stable@dpdk.org
Security Issues
---------------
--
1.9.0
^ permalink raw reply related
* [PATCH 2/3] maintainers: update virtio section name
From: Yuanhan Liu @ 2016-12-01 7:06 UTC (permalink / raw)
To: dev; +Cc: Thomas Monjalon, John McNamara, Yuanhan Liu
In-Reply-To: <1480575999-14453-1-git-send-email-yuanhan.liu@linux.intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
hmm.., maybe we could seperate lib vhost and virtio pmd, into two
different sections?
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index cc1ab68..3df1754 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -364,7 +364,7 @@ M: Sony Chacko <sony.chacko@qlogic.com>
F: drivers/net/qede/
F: doc/guides/nics/qede.rst
-RedHat virtio
+Virtio PMD and vhost lib
M: Yuanhan Liu <yuanhan.liu@linux.intel.com>
T: git://dpdk.org/next/dpdk-next-virtio
F: drivers/net/virtio/
--
1.9.0
^ permalink raw reply related
* [PATCH 1/3] maintainers: update virtio maintainer
From: Yuanhan Liu @ 2016-12-01 7:06 UTC (permalink / raw)
To: dev; +Cc: Thomas Monjalon, John McNamara, Yuanhan Liu, Huawei Xie
In-Reply-To: <1480575999-14453-1-git-send-email-yuanhan.liu@linux.intel.com>
Huawei has left DPDK team for months, and he hasn't showed up since
then. Remove him.
Cc: Huawei Xie <huawei.xie@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 26d9590..cc1ab68 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -365,7 +365,6 @@ F: drivers/net/qede/
F: doc/guides/nics/qede.rst
RedHat virtio
-M: Huawei Xie <huawei.xie@intel.com>
M: Yuanhan Liu <yuanhan.liu@linux.intel.com>
T: git://dpdk.org/next/dpdk-next-virtio
F: drivers/net/virtio/
--
1.9.0
^ permalink raw reply related
* [PATCH 0/3] maintainers: minor updates to virtio and stable
From: Yuanhan Liu @ 2016-12-01 7:06 UTC (permalink / raw)
To: dev; +Cc: Thomas Monjalon, John McNamara, Yuanhan Liu
---
Yuanhan Liu (3):
maintainers: update virtio maintainer
maintainers: update virtio section name
maintainers: add stable mailing list
MAINTAINERS | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
1.9.0
^ permalink raw reply
* Re: [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
From: Shreyansh Jain @ 2016-12-01 6:26 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
In-Reply-To: <1479931644-78960-3-git-send-email-benjamin.walker@intel.com>
Hello Ben,
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> Instead of passing domain, bus, devid, func, just pass
> an rte_pci_addr.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 876ba38..073af5f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
>
> /* Scan one pci sysfs entry, and fill the devices list from it. */
> static int
> -pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> - uint8_t devid, uint8_t function)
> +pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
> {
> char filename[PATH_MAX];
> unsigned long tmp;
> @@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> return -1;
>
> memset(dev, 0, sizeof(*dev));
> - dev->addr.domain = domain;
> - dev->addr.bus = bus;
> - dev->addr.devid = devid;
> - dev->addr.function = function;
> + dev->addr = *addr;
>
> /* get vendor id */
> snprintf(filename, sizeof(filename), "%s/vendor", dirname);
> @@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
> pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> addr->function);
>
> - return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> - addr->function);
> + return pci_scan_one(filename, addr);
> }
>
> /*
> * split up a pci address into its constituent parts.
> */
> static int
> -parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
> - uint8_t *bus, uint8_t *devid, uint8_t *function)
> +parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
> {
> /* first split on ':' */
> union splitaddr {
> @@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
>
> /* now convert to int values */
> errno = 0;
> - *domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
> - *bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
> - *devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
> - *function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
> + addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
> + addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
> + addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
> + addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
> if (errno != 0)
> goto error;
>
> @@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
> struct dirent *e;
> DIR *dir;
> char dirname[PATH_MAX];
> - uint16_t domain;
> - uint8_t bus, devid, function;
> + struct rte_pci_addr addr;
>
> dir = opendir(pci_get_sysfs_path());
> if (dir == NULL) {
> @@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
> return -1;
> }
>
> +
> while ((e = readdir(dir)) != NULL) {
> if (e->d_name[0] == '.')
> continue;
>
> - if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
> - &bus, &devid, &function) != 0)
> + if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
> continue;
>
> snprintf(dirname, sizeof(dirname), "%s/%s",
> pci_get_sysfs_path(), e->d_name);
> - if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
> + if (pci_scan_one(dirname, &addr) < 0)
> goto error;
> }
> closedir(dir);
> +
> return 0;
>
> error:
>
Do you mind if I use this patch directly as part of my patchset for bus
Model? I was doing a similar change to make the pci_scan_one simpler
(and pass along a new argument)..
-
Shreyansh
^ permalink raw reply
* Hyper-v support
From: Varun @ 2016-11-30 22:34 UTC (permalink / raw)
To: dev
Hi,
I would like to know if the latest DPDK (16.11) supports hyper-v?
I couldn't find any conclusive evidence online or in dpdk roadmap. Is it
likely that we see it in 17.05?
--
Regards,
Varun
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Jerin Jacob @ 2016-11-30 21:01 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Ananyev, Konstantin, Harish Patil, dev, Rahul Lakkireddy,
Stephen Hurd, Jan Medala, Jakub Palider, John Daley,
Adrien Mazarguil, Alejandro Lucero, Rasesh Mody, Jacob, Jerin,
Yuanhan Liu, Yong Wang, Kulasek, TomaszX, olivier.matz
In-Reply-To: <3517413.XL3bTbAyaC@xps13>
On Wed, Nov 30, 2016 at 07:26:36PM +0100, Thomas Monjalon wrote:
> 2016-11-30 17:42, Ananyev, Konstantin:
> > > >Please, we need a comment for each driver saying
> > > >"it is OK, we do not need any checksum preparation for TSO"
> > > >or
> > > >"yes we have to implement tx_prepare or TSO will not work in this mode"
> > > >
> > >
> > > qede PMD doesn’t currently support TSO yet, it only supports Tx TCP/UDP/IP
> > > csum offloads.
> > > So Tx preparation isn’t applicable. So as of now -
> > > "it is OK, we do not need any checksum preparation for TSO"
> >
> > Thanks for the answer.
> > Though please note that it not only for TSO.
>
> Oh yes, sorry, my wording was incorrect.
> We need to know if any checksum preparation is needed prior
> offloading its final computation to the hardware or driver.
> So the question applies to TSO and simple checksum offload.
>
> We are still waiting answers for
> bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.
The thunderx devices don't need pseudo header checksum
in the packet for TSO or TX checksum offload. So..
"it is OK, we do not need any checksum preparation for TSO"
>
> > This is for any TX offload for which the upper layer SW would have
> > to modify the contents of the packet.
> > Though as I can see for qede neither PKT_TX_IP_CKSUM or PKT_TX_TCP_CKSUM
> > exhibits any extra requirements for the user.
> > Is that correct?
>
^ permalink raw reply
* Re: [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
From: Jerin Jacob @ 2016-11-30 20:54 UTC (permalink / raw)
To: Ananyev, Konstantin
Cc: Richardson, Bruce, Satha Rao, Zhang, Helin, Wu, Jingjing,
jianbo.liu@linaro.org, dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E2477@irsmsx105.ger.corp.intel.com>
On Wed, Nov 30, 2016 at 05:52:02PM +0000, Ananyev, Konstantin wrote:
> Hi Jerin,
Hi Konstantin,
>
> >
> > On Tue, Nov 22, 2016 at 01:46:54PM +0000, Bruce Richardson wrote:
> > > On Tue, Nov 22, 2016 at 03:46:38AM +0530, Jerin Jacob wrote:
> > > > On Sun, Nov 20, 2016 at 11:21:43PM +0000, Ananyev, Konstantin wrote:
> > > > Hi
> > > > > >
> > > > > > i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
> > > > > > results. To solve this include rte memory barriers
> > > > > >
> > > > > > Signed-off-by: Satha Rao <skoteshwar@caviumnetworks.com>
> > > > > > ---
> > > > > > drivers/net/i40e/base/i40e_osdep.h | 14 ++++++++++++++
> > > > > > 1 file changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
> > > > > > index 38e7ba5..ffa3160 100644
> > > > > > --- a/drivers/net/i40e/base/i40e_osdep.h
> > > > > > +++ b/drivers/net/i40e/base/i40e_osdep.h
> > > > > > @@ -158,7 +158,13 @@ do { \
> > > > > > ((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
> > > > > > static inline uint32_t i40e_read_addr(volatile void *addr)
> > > > > > {
> > > > > > +#if defined(RTE_ARCH_ARM64)
> > > > > > + uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
> > > > > > + rte_rmb();
> > > > > > + return val;
> > > > >
> > > > > If you really need an rmb/wmb with MMIO read/writes on ARM,
> > > > > I think you can avoid #ifdefs here and use rte_smp_rmb/rte_smp_wmb.
> > > > > BTW, I suppose if you need it for i40e, you would need it for other devices too.
> > > >
> > > > Yes. ARM would need for all devices(typically, the devices on external PCI bus).
> > > > I guess rte_smp_rmb may not be the correct abstraction. So we need more of
> > > > rte_rmb() as we need only non smp variant on IO side. I guess then it make sense to
> > > > create new abstraction in eal with following variants so that each arch
> > > > gets opportunity to make what it makes sense that specific platform
> > > >
> > > > rte_readb_relaxed
> > > > rte_readw_relaxed
> > > > rte_readl_relaxed
> > > > rte_readq_relaxed
> > > > rte_writeb_relaxed
> > > > rte_writew_relaxed
> > > > rte_writel_relaxed
> > > > rte_writeq_relaxed
> > > > rte_readb
> > > > rte_readw
> > > > rte_readl
> > > > rte_readq
> > > > rte_writeb
> > > > rte_writew
> > > > rte_writel
> > > > rte_writeq
> > > >
> > > > Thoughts ?
> > > >
> > >
> > > That seems like a lot of API calls!
> > > Perhaps you can clarify - why would the rte_smp_rmb() not work for you?
> >
> > Currently arm64 mapped DMB as rte_smp_rmb() for smp case.
> >
> > Ideally for io barrier and non smp case, we need to map it as DSB and it is
> > bit heavier than DMB
>
> Ok, so you need some new macro, like rte_io_(r|w)mb or so, that would expand into dmb
> for ARM, correct?
The io barrier expands to dsb.
http://lxr.free-electrons.com/source/arch/arm64/include/asm/io.h#L110
>
> >
> > The linux kernel arm64 mappings
> > http://lxr.free-electrons.com/source/arch/arm64/include/asm/io.h#L142
> >
> > DMB vs DSB
> > https://community.arm.com/thread/3833
> >
> > The relaxed one are without any barriers.(the use case like accessing on
> > chip peripherals may need only relaxed versions)
> >
> > Thoughts on new rte EAL abstraction?
>
> Looks like a lot of macros but if you guys think that would help - NP with that :)
I don't have strong opinion here. If there is concern on a lot of macros
then, I can introduce only "rte_io_(r|w)mb" instead of read[b|w|l|q]/write[b|w|l|q]/relaxed.
let me know?
> Again, in that case we probably can get rid of driver specific pci reg read/write defines.
Yes. But, That's going to have a lot of change :-(
If there is no objection then I will introduce
"read[b|w|l|q]/write[b|w|l|q]/relaxed" and then change all external pcie drivers
with new macros.
>
> Konstantin
>
> >
> > >
> > > /Bruce
^ permalink raw reply
* apply commit e30a0178d290a4e83dc01f9c2170d4859339c9cf "kni: support RHEL 7.3" to dpdk-stable?
From: Roberts, Lee A. @ 2016-11-30 20:54 UTC (permalink / raw)
To: dev@dpdk.org
Does it make sense to apply the commit for "kni: support RHEL 7.3" (http://www.dpdk.org/browse/dpdk/commit/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h?id=e30a0178d290a4e83dc01f9c2170d4859339c9cf)
to the stable tree to enable clean compilation on RHEL 7.3?
- Lee Roberts
^ permalink raw reply
* Re: [PATCH] cryptodev: fix crash on null dereference
From: Jerin Jacob @ 2016-11-30 20:36 UTC (permalink / raw)
To: De Lara Guarch, Pablo; +Cc: dev@dpdk.org, Doherty, Declan
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA3F577@IRSMSX108.ger.corp.intel.com>
On Wed, Nov 30, 2016 at 03:10:14PM +0000, De Lara Guarch, Pablo wrote:
> Hi Jerin,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> > Sent: Tuesday, November 15, 2016 7:12 PM
> > To: dev@dpdk.org
> > Cc: Doherty, Declan; Jerin Jacob
> > Subject: [dpdk-dev] [PATCH] cryptodev: fix crash on null dereference
> >
> > crypodev->data->name will be null when
> > rte_cryptodev_get_dev_id() invoked without a valid
> > crypto device instance.
> >
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
> Could you add a "Fixes" line?
Sure. I will send the v2 then
>
> Thanks,
> Pablo
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Ajit Khaparde @ 2016-11-30 19:37 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev, Rahul Lakkireddy, Stephen Hurd, Jan Medala, Jakub Palider,
John Daley, Adrien Mazarguil, Alejandro Lucero, Harish Patil,
Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang, Tomasz Kulasek,
konstantin.ananyev, olivier.matz
In-Reply-To: <8317180.L80Qf11uiu@xps13>
On Mon,
Nov 28, 2016 at 5:03 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> We need attention of every PMD developers on this thread.
>
> Reminder of what Konstantin suggested:
> "
> - if the PMD supports TX offloads AND
> - if to be able use any of these offloads the upper layer SW would have to:
> * modify the contents of the packet OR
> * obey HW specific restrictions
> then it is a PMD developer responsibility to provide tx_prep() that would
> implement
> expected modifications of the packet contents and restriction checks.
> Otherwise, tx_prep() implementation is not required and can be safely set
> to NULL.
> "
>
> I copy/paste also my previous conclusion:
>
> Before txprep, there is only one API: the application must prepare the
> packets checksum itself (get_psd_sum in testpmd).
> With txprep, the application have 2 choices: keep doing the job itself
> or call txprep which calls a PMD-specific function.
> The question is: does non-Intel drivers need a checksum preparation for
> TSO?
> Will it behave well if txprep does nothing in these drivers?
>
> When looking at the code, most of drivers handle the TSO flags.
> But it is hard to know whether they rely on the pseudo checksum or not.
>
> git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG'
> drivers/net/
>
> drivers/net/bnxt/bnxt_txr.c
>
::: snip:::
>
> Please, we need a comment for each driver saying
> "it is OK, we do not need any checksum preparation for TSO"
> or
> "yes we have to implement tx_prepare or TSO will not work in this mode"
>
The bnxt devices
don't need pse
udo header checksum in the packet for TSO or TX
checksum offload.
So..
"it is OK, we do not need any checksum preparation for TSO"
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Harish Patil @ 2016-11-30 18:39 UTC (permalink / raw)
To: Ananyev, Konstantin, Thomas Monjalon, dev@dpdk.org,
Rahul Lakkireddy, Stephen Hurd, Jan Medala, Jakub Palider,
John Daley, Adrien Mazarguil, Alejandro Lucero, Rasesh Mody,
Jacob, Jerin, Yuanhan Liu, Yong Wang
Cc: Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E2444@irsmsx105.ger.corp.intel.com>
>
>
>
>Hi Harish,
>>
>>
>> >We need attention of every PMD developers on this thread.
>> >
>> >Reminder of what Konstantin suggested:
>> >"
>> >- if the PMD supports TX offloads AND
>> >- if to be able use any of these offloads the upper layer SW would have
>> >to:
>> > * modify the contents of the packet OR
>> > * obey HW specific restrictions
>> >then it is a PMD developer responsibility to provide tx_prep() that
>>would
>> >implement
>> >expected modifications of the packet contents and restriction checks.
>> >Otherwise, tx_prep() implementation is not required and can be safely
>>set
>> >to NULL.
>> >"
>> >
>> >I copy/paste also my previous conclusion:
>> >
>> >Before txprep, there is only one API: the application must prepare the
>> >packets checksum itself (get_psd_sum in testpmd).
>> >With txprep, the application have 2 choices: keep doing the job itself
>> >or call txprep which calls a PMD-specific function.
>> >The question is: does non-Intel drivers need a checksum preparation for
>> >TSO?
>> >Will it behave well if txprep does nothing in these drivers?
>> >
>> >When looking at the code, most of drivers handle the TSO flags.
>> >But it is hard to know whether they rely on the pseudo checksum or not.
>> >
>> >git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG'
>> >drivers/net/
>> >
>> >drivers/net/bnxt/bnxt_txr.c
>> >drivers/net/cxgbe/sge.c
>> >drivers/net/e1000/em_rxtx.c
>> >drivers/net/e1000/igb_rxtx.c
>> >drivers/net/ena/ena_ethdev.c
>> >drivers/net/enic/enic_rxtx.c
>> >drivers/net/fm10k/fm10k_rxtx.c
>> >drivers/net/i40e/i40e_rxtx.c
>> >drivers/net/ixgbe/ixgbe_rxtx.c
>> >drivers/net/mlx4/mlx4.c
>> >drivers/net/mlx5/mlx5_rxtx.c
>> >drivers/net/nfp/nfp_net.c
>> >drivers/net/qede/qede_rxtx.c
>> >drivers/net/thunderx/nicvf_rxtx.c
>> >drivers/net/virtio/virtio_rxtx.c
>> >drivers/net/vmxnet3/vmxnet3_rxtx.c
>> >
>> >Please, we need a comment for each driver saying
>> >"it is OK, we do not need any checksum preparation for TSO"
>> >or
>> >"yes we have to implement tx_prepare or TSO will not work in this mode"
>> >
>>
>> qede PMD doesn’t currently support TSO yet, it only supports Tx
>>TCP/UDP/IP
>> csum offloads.
>> So Tx preparation isn’t applicable. So as of now -
>> "it is OK, we do not need any checksum preparation for TSO"
>
>Thanks for the answer.
>Though please note that it not only for TSO.
Okay. I initially thought so. But was not sure, so I explicitly indicated
that there is no TSO support.
>This is for any TX offload for which the upper layer SW would have
>to modify the contents of the packet.
>Though as I can see for qede neither PKT_TX_IP_CKSUM or PKT_TX_TCP_CKSUM
>exhibits any extra requirements for the user.
>Is that correct?
That’s right.
>
>Konstantin
>
>
>>
>>
>> Thanks,
>> Harish
>
>
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Thomas Monjalon @ 2016-11-30 18:26 UTC (permalink / raw)
To: Ananyev, Konstantin
Cc: Harish Patil, dev, Rahul Lakkireddy, Stephen Hurd, Jan Medala,
Jakub Palider, John Daley, Adrien Mazarguil, Alejandro Lucero,
Rasesh Mody, Jacob, Jerin, Yuanhan Liu, Yong Wang,
Kulasek, TomaszX, olivier.matz
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E2444@irsmsx105.ger.corp.intel.com>
2016-11-30 17:42, Ananyev, Konstantin:
> > >Please, we need a comment for each driver saying
> > >"it is OK, we do not need any checksum preparation for TSO"
> > >or
> > >"yes we have to implement tx_prepare or TSO will not work in this mode"
> > >
> >
> > qede PMD doesn’t currently support TSO yet, it only supports Tx TCP/UDP/IP
> > csum offloads.
> > So Tx preparation isn’t applicable. So as of now -
> > "it is OK, we do not need any checksum preparation for TSO"
>
> Thanks for the answer.
> Though please note that it not only for TSO.
Oh yes, sorry, my wording was incorrect.
We need to know if any checksum preparation is needed prior
offloading its final computation to the hardware or driver.
So the question applies to TSO and simple checksum offload.
We are still waiting answers for
bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.
> This is for any TX offload for which the upper layer SW would have
> to modify the contents of the packet.
> Though as I can see for qede neither PKT_TX_IP_CKSUM or PKT_TX_TCP_CKSUM
> exhibits any extra requirements for the user.
> Is that correct?
^ 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