* Re: [PATCH] net/mlx5: support extended statistics
From: Hanoch Haim (hhaim) @ 2017-01-12 9:56 UTC (permalink / raw)
To: Adrien Mazarguil, Shahaf Shuler; +Cc: dev@dpdk.org, Elad Persiko
In-Reply-To: <20170111165446.GP12822@6wind.com>
Hi Shahaf,
1) I would add *all* the hw counters to PF
$ethtool -S enp135s0f0
NIC statistics:
rx_packets: 54
rx_bytes: 3240
tx_packets: 138
tx_bytes: 8280
tx_tso_packets: 0
tx_tso_bytes: 0
tx_tso_inner_packets: 0
tx_tso_inner_bytes: 0
rx_lro_packets: 0
rx_lro_bytes: 0
rx_csum_unnecessary: 0
rx_csum_none: 54
rx_csum_complete: 0
rx_csum_unnecessary_inner: 0
tx_csum_partial: 0
tx_csum_partial_inner: 0
tx_csum_none: 138
tx_queue_stopped: 0
tx_queue_wake: 0
tx_queue_dropped: 0
rx_sw_lro_aggregated: 0
rx_sw_lro_flushed: 0
rx_sw_lro_no_desc: 0
rx_wqe_err: 0
rx_cqe_compress_pkts: 0
rx_cqe_compress_blks: 0
rx_mpwqe_filler: 0
rx_mpwqe_frag: 0
link_down_events_phy: 0
rx_out_of_buffer: 0
rx_vport_unicast_packets: 26126070627
rx_vport_unicast_bytes: 1328543705864986
tx_vport_unicast_packets: 687072959078
tx_vport_unicast_bytes: 3339188884659844
rx_vport_multicast_packets: 0
rx_vport_multicast_bytes: 0
tx_vport_multicast_packets: 0
tx_vport_multicast_bytes: 0
rx_vport_broadcast_packets: 4543553
rx_vport_broadcast_bytes: 272614872
tx_vport_broadcast_packets: 4543637
tx_vport_broadcast_bytes: 272619912
rx_vport_rdma_unicast_packets: 430134
rx_vport_rdma_unicast_bytes: 32690184
tx_vport_rdma_unicast_packets: 0
tx_vport_rdma_unicast_bytes: 0
rx_vport_rdma_multicast_packets: 0
rx_vport_rdma_multicast_bytes: 0
tx_vport_rdma_multicast_packets: 0
tx_vport_rdma_multicast_bytes: 0
tx_packets_phy: 11756775169850
rx_packets_phy: 2246825269635
rx_crc_errors_phy: 0
tx_bytes_phy: 3404211507236837
rx_bytes_phy: 1342896754459327
tx_multicast_phy: 0
tx_broadcast_phy: 4552688
rx_multicast_phy: 0
rx_broadcast_phy: 4552604
rx_in_range_len_errors_phy: 0
rx_out_of_range_len_phy: 0
rx_oversize_pkts_phy: 1135778701
rx_symbol_err_phy: 0
tx_mac_control_phy: 16579953
rx_mac_control_phy: 16670966
rx_unsupported_op_phy: 0
rx_pause_ctrl_phy: 16670966
tx_pause_ctrl_phy: 16579953
rx_discards_phy: 12244164648
tx_discards_phy: 0
tx_errors_phy: 0
rx_undersize_pkts_phy: 0
rx_fragments_phy: 0
rx_jabbers_phy: 0
rx_64_bytes_phy: 911948813404
rx_65_to_127_bytes_phy: 335422614022
rx_128_to_255_bytes_phy: 153383903056
rx_256_to_511_bytes_phy: 80464779917
rx_512_to_1023_bytes_phy: 184356224678
rx_1024_to_1518_bytes_phy: 313733619509
rx_1519_to_2047_bytes_phy: 235669614881
rx_2048_to_4095_bytes_phy: 4051420173
rx_4096_to_8191_bytes_phy: 1011852034
rx_8192_to_10239_bytes_phy: 27710327250
time_since_last_clear_phy: 2998621833
symbol_errors_phy: 0
sync_headers_errors_phy: 0
edpl/bip_errors_lane0_phy: 0
edpl/bip_errors_lane1_phy: 0
edpl/bip_errors_lane2_phy: 0
edpl/bip_errors_lane3_phy: 0
fc_corrected_blocks_lane0_phy: 0
fc_corrected_blocks_lane1_phy: 0
fc_corrected_blocks_lane2_phy: 0
fc_corrected_blocks_lane3_phy: 0
fc_uncorrectable_lane0_phy: 0
fc_uncorrectable_lane1_phy: 0
fc_uncorrectable_lane2_phy: 0
fc_uncorrectable_lane3_phy: 0
rs_corrected_blocks_phy: 0
rs_uncorrectable_blocks_phy: 0
rs_no_errors_blocks_phy: 58566832662545
rs_single_error_blocks_phy: 0
rs_corrected_symbols_total_phy: 0
rs_corrected_symbols_lane0_phy: 0
rs_corrected_symbols_lane1_phy: 0
rs_corrected_symbols_lane2_phy: 0
rs_corrected_symbols_lane3_phy: 0
rx_prio0_bytes: 1342896754459327
rx_prio0_packets: 2234564434021
tx_prio0_bytes: 3404210446119845
tx_prio0_packets: 11756758589897
rx_prio1_bytes: 0
rx_prio1_packets: 0
tx_prio1_bytes: 0
tx_prio1_packets: 0
rx_prio2_bytes: 0
rx_prio2_packets: 0
tx_prio2_bytes: 0
tx_prio2_packets: 0
rx_prio3_bytes: 0
rx_prio3_packets: 0
tx_prio3_bytes: 0
tx_prio3_packets: 0
rx_prio4_bytes: 0
rx_prio4_packets: 0
tx_prio4_bytes: 0
tx_prio4_packets: 0
rx_prio5_bytes: 0
rx_prio5_packets: 0
tx_prio5_bytes: 0
tx_prio5_packets: 0
rx_prio6_bytes: 0
rx_prio6_packets: 0
tx_prio6_bytes: 0
tx_prio6_packets: 0
rx_prio7_bytes: 0
rx_prio7_packets: 0
tx_prio7_bytes: 0
tx_prio7_packets: 0
rx0_packets: 54
rx0_bytes: 3240
rx0_csum_complete
2) Add the right HW counters to the VF
tx28_nop: 0
tx28_queue_stopped: 0
tx28_queue_wake: 0
tx28_queue_dropped: 0
tx29_packets: 0
tx29_bytes: 0
tx29_tso_packets: 0
tx29_tso_bytes: 0
tx29_tso_inner_packets: 0
tx29_tso_inner_bytes: 0
tx29_csum_none: 0
tx29_csum_partial: 0
tx29_csum_partial_inner: 0
tx29_nop: 0
tx29_queue_stopped: 0
3) Implement the stats_get/reset using HW counters
.stats_get = mlx5_stats_get,
.stats_reset = mlx5_stats_reset,
Thanks,
Hanoh
-----Original Message-----
From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
Sent: Wednesday, January 11, 2017 6:55 PM
To: Shahaf Shuler
Cc: dev@dpdk.org; Elad Persiko; Hanoch Haim (hhaim)
Subject: Re: [PATCH] net/mlx5: support extended statistics
Hi Shahaf,
Thanks, I have a few comments, most of them relatively minor. Please see below.
On Wed, Jan 11, 2017 at 10:55:42AM +0200, Shahaf Shuler wrote:
> Implement xstats_*() DPDK callbacks
>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Elad Persiko <eladpe@mellanox.com>
> Signed-off-by: Hanoch Haim <hhaim@cisco.com>
> ---
> drivers/net/mlx5/mlx5.c | 3 +
> drivers/net/mlx5/mlx5.h | 12 ++
> drivers/net/mlx5/mlx5_defs.h | 3 +
> drivers/net/mlx5/mlx5_stats.c | 342 ++++++++++++++++++++++++++++++++++++++++
> drivers/net/mlx5/mlx5_trigger.c | 1 +
> 5 files changed, 361 insertions(+)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 6293c1f..3359d3c 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -199,6 +199,9 @@
> .link_update = mlx5_link_update,
> .stats_get = mlx5_stats_get,
> .stats_reset = mlx5_stats_reset,
> + .xstats_get = mlx5_xstats_get,
> + .xstats_reset = mlx5_xstats_reset,
> + .xstats_get_names = mlx5_xstats_get_names,
> .dev_infos_get = mlx5_dev_infos_get,
> .dev_supported_ptypes_get = mlx5_dev_supported_ptypes_get,
> .vlan_filter_set = mlx5_vlan_filter_set, diff --git
> a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> ee62e04..034a05e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -89,6 +89,11 @@ enum {
> PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF = 0x101a, };
>
> +struct mlx5_xstats_ctrl {
> + uint64_t shadow[MLX5_MAX_XSTATS];
> + uint16_t stats_n; /* Number of device stats. */ };
> +
> struct priv {
> struct rte_eth_dev *dev; /* Ethernet device. */
> struct ibv_context *ctx; /* Verbs context. */ @@ -142,6 +147,7 @@
> struct priv {
> struct fdir_queue *fdir_drop_queue; /* Flow director drop queue. */
> LIST_HEAD(mlx5_flows, rte_flow) flows; /* RTE Flow rules. */
> uint32_t link_speed_capa; /* Link speed capabilities. */
> + struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */
> rte_spinlock_t lock; /* Lock for control functions. */ };
>
> @@ -250,8 +256,14 @@ int mlx5_dev_rss_reta_update(struct rte_eth_dev
> *,
>
> /* mlx5_stats.c */
>
> +void priv_xstats_init(struct priv *);
> void mlx5_stats_get(struct rte_eth_dev *, struct rte_eth_stats *);
> void mlx5_stats_reset(struct rte_eth_dev *);
> +int mlx5_xstats_get(struct rte_eth_dev *,
> + struct rte_eth_xstat *, unsigned int); void
> +mlx5_xstats_reset(struct rte_eth_dev *); int
> +mlx5_xstats_get_names(struct rte_eth_dev *,
> + struct rte_eth_xstat_name *, unsigned int);
>
> /* mlx5_vlan.c */
>
> diff --git a/drivers/net/mlx5/mlx5_defs.h
> b/drivers/net/mlx5/mlx5_defs.h index b32816e..beabb70 100644
> --- a/drivers/net/mlx5/mlx5_defs.h
> +++ b/drivers/net/mlx5/mlx5_defs.h
> @@ -79,4 +79,7 @@
> /* Alarm timeout. */
> #define MLX5_ALARM_TIMEOUT_US 100000
>
> +/* Maximum number of extended statistics counters. */ #define
> +MLX5_MAX_XSTATS 32
> +
> #endif /* RTE_PMD_MLX5_DEFS_H_ */
> diff --git a/drivers/net/mlx5/mlx5_stats.c
> b/drivers/net/mlx5/mlx5_stats.c index f2b5781..08a7656 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -31,11 +31,16 @@
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#include <linux/sockios.h>
> +#include <linux/ethtool.h>
> +
> /* DPDK headers don't like -pedantic. */ #ifdef PEDANTIC #pragma
> GCC diagnostic ignored "-Wpedantic"
> #endif
> #include <rte_ethdev.h>
> +#include <rte_common.h>
> +#include <rte_malloc.h>
> #ifdef PEDANTIC
> #pragma GCC diagnostic error "-Wpedantic"
> #endif
> @@ -44,6 +49,269 @@
> #include "mlx5_rxtx.h"
> #include "mlx5_defs.h"
>
> +struct mlx5_counter_ctrl {
> + /* Name of the counter for dpdk user. */
"for dpdk user" is redundant given the purpose of this API.
> + char dpdk_name[RTE_ETH_XSTATS_NAME_SIZE];
> + /* Name of the counter on the device table. */
> + char ctr_name[RTE_ETH_XSTATS_NAME_SIZE];
> + /* Index in the device counters table. */
> + uint16_t dev_table_idx;
> +};
> +
> +static struct mlx5_counter_ctrl mlx5_counters_init[] = {
> + {
> + .dpdk_name = "rx_port_unicast_bytes",
> + .ctr_name = "rx_vport_unicast_bytes",
> + .dev_table_idx = 0
Please add comma after ".dev_table_idx = 0" (same comment applies to the following definitions). Note you may leave it out entirely since unspecified fields are automatically initialized to zero.
> + },
> + {
> + .dpdk_name = "rx_port_multicast_bytes",
> + .ctr_name = "rx_vport_multicast_bytes",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_port_broadcast_bytes",
> + .ctr_name = "rx_vport_broadcast_bytes",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_port_unicast_packets",
> + .ctr_name = "rx_vport_unicast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_port_multicast_packets",
> + .ctr_name = "rx_vport_multicast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_port_broadcast_packets",
> + .ctr_name = "rx_vport_broadcast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_unicast_bytes",
> + .ctr_name = "tx_vport_unicast_bytes",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_multicast_bytes",
> + .ctr_name = "tx_vport_multicast_bytes",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_broadcast_bytes",
> + .ctr_name = "tx_vport_broadcast_bytes",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_unicast_packets",
> + .ctr_name = "tx_vport_unicast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_multicast_packets",
> + .ctr_name = "tx_vport_multicast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_port_broadcast_packets",
> + .ctr_name = "tx_vport_broadcast_packets",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_wqe_err",
> + .ctr_name = "rx_wqe_err",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_crc_errors_phy",
> + .ctr_name = "rx_crc_errors_phy",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_in_range_len_errors_phy",
> + .ctr_name = "rx_in_range_len_errors_phy",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "rx_symbol_err_phy",
> + .ctr_name = "rx_symbol_err_phy",
> + .dev_table_idx = 0
> + },
> + {
> + .dpdk_name = "tx_errors_phy",
> + .ctr_name = "tx_errors_phy",
> + .dev_table_idx = 0
> + },
> +};
> +
> +const unsigned int xstats_n = RTE_DIM(mlx5_counters_init);
Considering this global depends on mlx5_counters_init[] and is only used in this file, it should be static.
> +
> +/**
> + * Read device counters table.
> + *
> + * @param priv
> + * Pointer to private structure.
> + * @param[out] stats
> + * Counters table output buffer.
> + *
> + * @return
> + * 0 on success and stats is filled, negative on error.
> + */
> +static int
> +priv_read_dev_counters(struct priv *priv, uint64_t *stats) {
> + struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
> + unsigned int i;
> + struct ifreq ifr;
> + unsigned int stats_sz = (xstats_ctrl->stats_n * sizeof(uint64_t)) +
> + sizeof(struct ethtool_stats);
> + struct ethtool_stats et_stats[(stats_sz + (
> + sizeof(struct ethtool_stats) - 1)) /
> + sizeof(struct ethtool_stats)];
> +
> + et_stats->cmd = ETHTOOL_GSTATS;
> + et_stats->n_stats = xstats_ctrl->stats_n;
> + ifr.ifr_data = (caddr_t)et_stats;
> + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) != 0) {
> + WARN("unable to get statistic values");
> + return -1;
> + }
> + for (i = 0; (i != xstats_n) ; ++i)
> + stats[i] = (uint64_t)
> + et_stats->data[mlx5_counters_init[i].dev_table_idx];
> + return 0;
> +}
> +
> +/**
> + * Init the strcutures to read device counters.
Typo, "strcutures".
> + *
> + * @param priv
> + * Pointer to private structure.
> + */
> +void
> +priv_xstats_init(struct priv *priv)
> +{
> + struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
> + unsigned int i;
> + unsigned int j;
> + char ifname[IF_NAMESIZE];
> + struct ifreq ifr;
> + struct ethtool_drvinfo drvinfo;
> + struct ethtool_gstrings *strings = NULL;
> + unsigned int dev_stats_n;
> + unsigned int str_sz;
> +
> + if (priv_get_ifname(priv, &ifname)) {
> + WARN("unable to get interface name");
> + return;
> + }
> + /* How many statistics are available. */
> + drvinfo.cmd = ETHTOOL_GDRVINFO;
> + ifr.ifr_data = (caddr_t)&drvinfo;
> + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) != 0) {
> + WARN("unable to get driver info");
> + return;
> + }
> + dev_stats_n = drvinfo.n_stats;
> + if (dev_stats_n < 1) {
> + WARN("no extended statistics available");
> + return;
> + }
> + xstats_ctrl->stats_n = dev_stats_n;
> + /* Allocate memory to grab stat names and values. */
> + str_sz = dev_stats_n * ETH_GSTRING_LEN;
> + strings = (struct ethtool_gstrings *)
> + rte_malloc("xstats_strings",
> + str_sz + sizeof(struct ethtool_gstrings), 0);
> + if (!strings) {
> + WARN("unable to allocate memory for xstats");
> + return;
> + }
> + strings->cmd = ETHTOOL_GSTRINGS;
> + strings->string_set = ETH_SS_STATS;
> + strings->len = dev_stats_n;
> + ifr.ifr_data = (caddr_t)strings;
> + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) != 0) {
> + WARN("unable to get statistic names");
> + goto free;
> + }
> + for (j = 0; (j != xstats_n); ++j)
> + mlx5_counters_init[j].dev_table_idx = dev_stats_n;
As a global, mlx5_counters_init[] affects all DPDK ports that use the mlx5 PMD simultaneously. Are you sure calling priv_xstats_init() from
mlx5_dev_start() is safe?
I think this global should be const and initialized only once.
> + for (i = 0; (i != dev_stats_n); ++i) {
> + const char *curr_string = (const char *)
> + &strings->data[i * ETH_GSTRING_LEN];
> +
> + for (j = 0; (j != xstats_n); ++j) {
> + if (!strcmp(mlx5_counters_init[j].ctr_name,
> + curr_string)) {
> + mlx5_counters_init[j].dev_table_idx = i;
The above comment also applies here. The PMD instance should allocate its own mapping as a priv field if there is no other choice. You could perhaps make it part of the mlx5_xstats_ctrl structure.
> + break;
> + }
> + }
> + }
> + for (j = 0; (j != xstats_n); ++j) {
> + if (mlx5_counters_init[j].dev_table_idx >= dev_stats_n) {
> + WARN("Counters are not recognized");
Displaying the name of the counter that is not recognized could help users determine what's doing on. Please make sure all info/warning/error messages are helpful enough, e.g.:
testpmd> show port xstats 0
###### NIC extended statistics for port 0
mlx5: Counters are not recognized
testpmd> # what?
> + goto free;
> + }
> + }
> + /* Copy to shadow at first time. */
> + assert(xstats_n <= MLX5_MAX_XSTATS);
> + priv_read_dev_counters(priv, xstats_ctrl->shadow);
> +free:
> + rte_free(strings);
> +}
> +
> +/**
> + * Get device extended statistics.
> + *
> + * @param priv
> + * Pointer to private structure.
> + * @param[out] stats
> + * Pointer to rte extended stats table.
> + *
> + * @return
> + * Number of extended stats on success and stats is filled,
> + * nagative on error.
Typo, "nagative".
> + */
> +static int
> +priv_xstats_get(struct priv *priv, struct rte_eth_xstat *stats) {
> + struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
> + unsigned int i;
> + uint64_t counters[xstats_n];
> +
> + if (priv_read_dev_counters(priv, counters) < 0)
> + return -1;
> + for (i = 0; (i != xstats_n) ; ++i) {
> + stats[i].id = i;
> + stats[i].value = (uint64_t)
> + (counters[i] - xstats_ctrl->shadow[i]);
I understand the purpose of the shadow array in this context, however to me "shadow" implies some sort of caching is taking place. I think "init" or "base" (as the base value or something) would make its purpose clearer.
> + }
> + return xstats_n;
> +}
> +
> +/**
> + * Reset device extended statistics.
> + *
> + * @param priv
> + * Pointer to private structure.
> + */
> +static void
> +priv_xstats_reset(struct priv *priv)
> +{
> + struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
> + unsigned int i;
> + uint64_t counters[xstats_n];
> +
> + if (priv_read_dev_counters(priv, counters) < 0)
> + return;
> + for (i = 0; (i != xstats_n) ; ++i)
> + xstats_ctrl->shadow[i] = counters[i]; }
> +
> /**
> * DPDK callback to get device statistics.
> *
> @@ -142,3 +410,77 @@
> #endif
> priv_unlock(priv);
> }
> +
> +/**
> + * DPDK callback to get extended device statistics.
> + *
> + * @param dev
> + * Pointer to Ethernet device structure.
> + * @param[out] stats
> + * Stats table output buffer.
> + * @param n
> + * The size of the stats table.
> + *
> + * @return
> + * Number of xstats on success, negative on failure.
> + */
> +int
> +mlx5_xstats_get(struct rte_eth_dev *dev,
> + struct rte_eth_xstat *stats, unsigned int n) {
> + struct priv *priv = mlx5_get_priv(dev);
> + int ret = xstats_n;
> +
> + if (n >= xstats_n && stats) {
> + priv_lock(priv);
> + ret = priv_xstats_get(priv, stats);
> + priv_unlock(priv);
> + }
> + return ret;
> +}
> +
> +/**
> + * DPDK callback to clear device extended statistics.
> + *
> + * @param dev
> + * Pointer to Ethernet device structure.
> + */
> +void
> +mlx5_xstats_reset(struct rte_eth_dev *dev) {
> + struct priv *priv = mlx5_get_priv(dev);
> +
> + priv_lock(priv);
> + priv_xstats_reset(priv);
> + priv_unlock(priv);
> +}
> +
> +/**
> + * DPDK callback to retrieve names of extended device statistics
> + *
> + * @param dev
> + * Pointer to Ethernet device structure.
> + * @param[out] xstats_names
> + * Block of memory to insert names into
Let's call "block of memory" a "buffer"? There is also a missing period at the end of this sentence.
> + * @param size
> + * Number of names.
"num" (or "n" as in the API) would make more sense than "size" here.
> + *
> + * @return
> + * Number of xstats names.
> + */
> +int
> +mlx5_xstats_get_names(struct rte_eth_dev *dev,
> + struct rte_eth_xstat_name *xstats_names, unsigned int size) {
> + struct priv *priv = mlx5_get_priv(dev);
> + unsigned int i;
> +
> + if (size >= xstats_n && xstats_names) {
> + priv_lock(priv);
> + for (i = 0; (i != xstats_n) ; ++i)
> + strcpy(xstats_names[i].name,
> + mlx5_counters_init[i].dpdk_name);
> + priv_unlock(priv);
> + }
> + return xstats_n;
> +}
> diff --git a/drivers/net/mlx5/mlx5_trigger.c
> b/drivers/net/mlx5/mlx5_trigger.c index 2399243..30addd2 100644
> --- a/drivers/net/mlx5/mlx5_trigger.c
> +++ b/drivers/net/mlx5/mlx5_trigger.c
> @@ -91,6 +91,7 @@
> priv_fdir_enable(priv);
> priv_dev_interrupt_handler_install(priv, dev);
> err = priv_flow_start(priv);
> + priv_xstats_init(priv);
> priv_unlock(priv);
> return -err;
> }
> --
> 1.8.3.1
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH] app/testpmd: fix ixgbe private API calling
From: Iremonger, Bernard @ 2017-01-12 10:08 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Wu, Jingjing, stable@dpdk.org
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B55864B@shsmsx102.ccr.corp.intel.com>
Hi Wenzhuo,
> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Thursday, January 12, 2017 1:09 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API calling
>
> Hi Bernard,
>
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Wednesday, January 11, 2017 6:27 PM
> > To: Lu, Wenzhuo; dev@dpdk.org
> > Cc: Wu, Jingjing; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API
> > calling
> >
> > Hi Wenzhuo,
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > > Sent: Wednesday, January 11, 2017 2:48 AM
> > > To: dev@dpdk.org
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>; stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API
> > > calling
> > >
> > > Some ixgbe private APIs are added to expose ixgbe specific functions.
> > > When they're used by testpmd, there's no check for if the NICs are ixgbe.
> > > Other NICs also have chance to call these APIs.
> > > This patch add the check and the feedback print.
> >
> > I am not sure that testpmd is the right place to do this.
> > The rte_pmd_ixgbe_* functions are public API's which can be called by
> > other applications.
> > The checks should be in the rte_pmd_ixgbe_* API's
> To be safer, it's better to add a check in the APIs.
I have already sent a patch for ixgbe which Ferruh has reviewed and applied to dpdk-next-net
http://dpdk.org/dev/patchwork/patch/19151/
We should consider doing something similar for the i40e.
> But the APIs is so called private API, not really public. Considering if we have
> the same function on different NICs, for example we have rte_pmd_ixgbe_a
> and rte_pmd_i40e_a.
> APP still need to call them one by one, like ret = rte_pmd_ixgbe_a; ret =
> rte_pmd_i40e_a;
>
> then, why not add the check, like
> If (NIC is ixgbe)
> ret = rte_pmd_ixgbe_a;
> if (NIC is i40e)
> ret = rte_pmd_i40e_a;
There is already code like the above in testpmd in cases where there is a choice of rte_pmd_* functions to call.
> testpmd is an example to let the users to know how to use the APIs. They
> should follow the example.
> How about your opinion?
In the case where there is no choice of function to make. There is no need to check the NIC in testpmd (as it is now done in the rte_pmd_ixgbe_* API's).
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test vectors
From: De Lara Guarch, Pablo @ 2017-01-12 10:48 UTC (permalink / raw)
To: zbigniew.bodek@caviumnetworks.com, dev@dpdk.org
Cc: Doherty, Declan, jerin.jacob@caviumnetworks.com
In-Reply-To: <1483551207-18236-9-git-send-email-zbigniew.bodek@caviumnetworks.com>
Hi Bodek,
> -----Original Message-----
> From: zbigniew.bodek@caviumnetworks.com
> [mailto:zbigniew.bodek@caviumnetworks.com]
> Sent: Wednesday, January 04, 2017 5:33 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Doherty, Declan;
> jerin.jacob@caviumnetworks.com; Zbigniew Bodek
> Subject: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test vectors
>
> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>
> Introduce unit tests for ARMv8 crypto PMD.
> Add test vectors for short cases such as 160 bytes.
> These test cases are ARMv8 specific since the code provides
> different processing paths for different input data sizes.
>
> User can validate correctness of algorithms' implementation using:
> * cryptodev_sw_armv8_autotest
> For performance test one can use:
> * cryptodev_sw_armv8_perftest
>
> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
Could you rebase this patchset with the dpdk-next-crypto tree?
There is a compilation error due to a missing parameter in a function that has recently changed.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v3 25/29] net/nfp: use eal I/O device memory read/write API
From: Alejandro Lucero @ 2017-01-12 10:53 UTC (permalink / raw)
To: Jerin Jacob
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
jianbo.liu, Jan Viktorin, santosh.shukla
In-Reply-To: <1484212646-10338-26-git-send-email-jerin.jacob@caviumnetworks.com>
Hi,
I've tried to find out which dpdk repo should I use for testing this change
with NFP PMD.
It seems rte_read/write functions are not with last dpdk main repo, nor
with dpdk-net-next.
Can someone tell me which repo should I use?
On Thu, Jan 12, 2017 at 9:17 AM, 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.
>
> CC: Alejandro Lucero <alejandro.lucero@netronome.com>
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
> drivers/net/nfp/nfp_net_pmd.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_net_pmd.h b/drivers/net/nfp/nfp_net_pmd.h
> index c180972..f11b32e 100644
> --- a/drivers/net/nfp/nfp_net_pmd.h
> +++ b/drivers/net/nfp/nfp_net_pmd.h
> @@ -121,25 +121,26 @@ struct nfp_net_adapter;
> #define NFD_CFG_MINOR_VERSION_of(x) (((x) >> 0) & 0xff)
>
> #include <linux/types.h>
> +#include <rte_io.h>
>
> static inline uint8_t nn_readb(volatile const void *addr)
> {
> - return *((volatile const uint8_t *)(addr));
> + return rte_read8(addr);
> }
>
> static inline void nn_writeb(uint8_t val, volatile void *addr)
> {
> - *((volatile uint8_t *)(addr)) = val;
> + rte_write8(val, addr);
> }
>
> static inline uint32_t nn_readl(volatile const void *addr)
> {
> - return *((volatile const uint32_t *)(addr));
> + return rte_read32(addr);
> }
>
> static inline void nn_writel(uint32_t val, volatile void *addr)
> {
> - *((volatile uint32_t *)(addr)) = val;
> + rte_write32(val, addr);
> }
>
> static inline uint64_t nn_readq(volatile void *addr)
> --
> 2.5.5
>
>
^ permalink raw reply
* Re: [PATCH v5 00/18] net/ixgbe: Consistent filter API
From: Xing, Beilei @ 2017-01-12 11:38 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org
In-Reply-To: <1484212665-1635-1-git-send-email-wei.zhao1@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Thursday, January 12, 2017 5:17 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v5 00/18] net/ixgbe: Consistent filter API
>
> The patches mainly finish following functions:
> 1) Store and restore all kinds of filters.
> 2) Parse all kinds of filters.
> 3) Add flow validate function.
> 4) Add flow create function.
> 5) Add flow destroy function.
> 6) Add flow flush function.
>
> v2 changes:
> fix git log error
> Modify some function call relationship
> Change return value type of all parse flow functions Update error info for all
> flow ops Add ixgbe_filterlist_flush to flush flows and rules created
>
> v3 change:
> add new file ixgbe_flow.c to store generic API parser related functions add
> more comment about pattern and action rules add attr check in parser
> functions change struct name ixgbe_flow to rte_flow change SYN to TCP
> SYN change to use memset initizlize struct ixgbe_filter_info break down
> filter uninit process to 3 indepedent functions in eth_ixgbe_dev_uninit()
> change struct rte_flow_item_nvgre definition change struct
> rte_flow_item_e_tag definition fix one bug in function ixgbe_dev_filter_ctrl
> add goto in function ixgbe_flow_create delete some useless initialization
> eliminate some git log check warning
>
> v4 change:
> fix some check patch warning
>
> v5 change:
> fix some git log warning
>
> zhao wei (18):
> net/ixgbe: store TCP SYN filter
> net/ixgbe: store flow director filter
> net/ixgbe: store L2 tunnel filter
> net/ixgbe: restore n-tuple filter
> net/ixgbe: restore ether type filter
> net/ixgbe: restore TCP SYN filter
> net/ixgbe: restore flow director filter
> net/ixgbe: restore L2 tunnel filter
> net/ixgbe: store and restore L2 tunnel configuration
> net/ixgbe: flush all the filters
> net/ixgbe: parse n-tuple filter
> net/ixgbe: parse ethertype filter
> net/ixgbe: parse TCP SYN filter
> net/ixgbe: parse L2 tunnel filter
> net/ixgbe: parse flow director filter
> net/ixgbe: create consistent filter
> net/ixgbe: destroy consistent filter
> net/ixgbe: flush all the filter list
>
> drivers/net/ixgbe/Makefile | 2 +
> drivers/net/ixgbe/ixgbe_ethdev.c | 667 +++++++--
> drivers/net/ixgbe/ixgbe_ethdev.h | 203 ++-
> drivers/net/ixgbe/ixgbe_fdir.c | 407 ++++--
> drivers/net/ixgbe/ixgbe_flow.c | 2808
> ++++++++++++++++++++++++++++++++++++++
> drivers/net/ixgbe/ixgbe_pf.c | 26 +-
> lib/librte_ether/rte_flow.h | 48 +
> 7 files changed, 3950 insertions(+), 211 deletions(-) create mode 100644
> drivers/net/ixgbe/ixgbe_flow.c
>
Acked-by: Beilei Xing <beilei.xing@intel.com>
^ permalink raw reply
* Re: [PATCH v3 00/31] net/i40e: base code update
From: Ferruh Yigit @ 2017-01-12 11:48 UTC (permalink / raw)
To: Gregory Etelson, dev; +Cc: Jingjing Wu, helin.zhang
In-Reply-To: <4364070.FlkcxLpqix@polaris>
Hi Gregory,
On 12/20/2016 3:40 PM, Gregory Etelson wrote:
> Hello,
>
> I have several XL710-Q2 interfaces that fail with
> `PMD: eth_i40e_dev_init(): Failed to init adminq: -54'
> Rebase to the latest dpdk-next-net/master did not fix the fault
If I understand correctly, this was existing issue right? Not introduced
with base driver update?
> Firmware version is 5.04 0x80002505 0.0.0
> lspci output looks the same for working and failed interfaces
>
> Full traces with all I40*DEBUG options enabled follow.
>
> Is there a way to extract more info to investigate this fault ?
>
> Regards,
> Gregory
>
<...>
^ permalink raw reply
* Re: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test vectors
From: Zbigniew Bodek @ 2017-01-12 11:50 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org
Cc: Doherty, Declan, jerin.jacob@caviumnetworks.com
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D897476C3029@IRSMSX108.ger.corp.intel.com>
Hello Pablo,
On 12.01.2017 11:48, De Lara Guarch, Pablo wrote:
> Hi Bodek,
>
>> -----Original Message-----
>> From: zbigniew.bodek@caviumnetworks.com
>> [mailto:zbigniew.bodek@caviumnetworks.com]
>> Sent: Wednesday, January 04, 2017 5:33 PM
>> To: dev@dpdk.org
>> Cc: De Lara Guarch, Pablo; Doherty, Declan;
>> jerin.jacob@caviumnetworks.com; Zbigniew Bodek
>> Subject: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test vectors
>>
>> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>>
>> Introduce unit tests for ARMv8 crypto PMD.
>> Add test vectors for short cases such as 160 bytes.
>> These test cases are ARMv8 specific since the code provides
>> different processing paths for different input data sizes.
>>
>> User can validate correctness of algorithms' implementation using:
>> * cryptodev_sw_armv8_autotest
>> For performance test one can use:
>> * cryptodev_sw_armv8_perftest
>>
>> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>
> Could you rebase this patchset with the dpdk-next-crypto tree?
> There is a compilation error due to a missing parameter in a function that has recently changed.
I see. The rebase is done. Should I send full v4 patchset now?
Kind regards
Zbigniew
>
> Thanks,
> Pablo
>
^ permalink raw reply
* Re: [dpdk-users] IGB_UIO: PCI Resources Management
From: Ferruh Yigit @ 2017-01-12 11:55 UTC (permalink / raw)
To: Gregory Etelson, dev, users
In-Reply-To: <3355891.l3I590SjcV@polaris>
On 12/9/2016 8:54 AM, Gregory Etelson wrote:
> Hello,
>
> IGB_UIO driver does not close port PCI activities after DPDK process exits.
> DPDK API provides rte_eth_dev_close() to manage port PCI,
> but it can be skipped if process receives SIGKILL signal
I guess I understand the problem.
> The patches below provide IGB_UIO release callback and IXGBEVF release function
But adding ixgbe specific code into igb_uio may not be good idea.
Can be anything done one upper layer, pci layer, generic to all drivers?
> With the patches, each time DPDK process terminates,
> UIO release callback will trigger port PCI close.
> On the down side, patched IGB_UIO can be bound to a single adapter type
>
> Regards,
> Gregory
<...>
^ permalink raw reply
* Re: fm10k pmd limitations
From: Ferruh Yigit @ 2017-01-12 11:56 UTC (permalink / raw)
To: Shaham Fridenberg, dev@dpdk.org; +Cc: Jing Chen
In-Reply-To: <2E654B490240B7449C846A96A8D8FE0C0116617B08@ILMB1.corp.radware.com>
On 12/13/2016 1:49 PM, Shaham Fridenberg wrote:
> Hey guys,
>
> I'm using dpdk 16.4 and fm10k card. According to the code, there's no support for disabling vlan stripping and VLAN QinQ in pmd fm10k.
> Does anybody know why? If there's any way to work-around it, or when is a behavior change expected?
> I need my VF to receive the packets with the VLAN headers.
> Even if it's possible to change this configurations through the linux kernel fm10k driver?
>
> Thanks!
>
CC fm10k maintainer: Jing Chen <jing.d.chen@intel.com>
^ permalink raw reply
* Re: Does DPDK i40e driver support 'Toeplitz hash'
From: Ferruh Yigit @ 2017-01-12 11:58 UTC (permalink / raw)
To: ALeX Wang, dev; +Cc: Keith Amidon, Jingjing Wu, Helin Zhang
In-Reply-To: <CANmyKO7s+7-O_ynHFRxrXnhpCX4-WhUyYASA-30XDeQnyGEnpg@mail.gmail.com>
On 12/12/2016 6:31 AM, ALeX Wang wrote:
> Hi,
>
> We want to use 'Toeplitz hash' for RSS hash on our server equipped with
> 'Intel X710-DA4' card.
>
> However, seemed that we hit the exact same issue as this:
>
> Why only rx queue "0" can receive network packet by i40e NIC
> http://dpdk.org/ml/archives/dev/2015-July/022453.html
>
> ...
> We are using the v16.11 release and would like to confirm if the issue above
> has been address. If so, could anyone post here how to configure?
CC'ing i40e maintainers:
Jingjing Wu <jingjing.wu@intel.com>; Helin Zhang <helin.zhang@intel.com>
>
> Appreciate,
> Alex Wang,
>
^ permalink raw reply
* Re: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test vectors
From: De Lara Guarch, Pablo @ 2017-01-12 12:07 UTC (permalink / raw)
To: Zbigniew Bodek, dev@dpdk.org
Cc: Doherty, Declan, jerin.jacob@caviumnetworks.com
In-Reply-To: <5d33a057-2114-0c82-bc72-68c6220febad@caviumnetworks.com>
> -----Original Message-----
> From: Zbigniew Bodek [mailto:zbigniew.bodek@caviumnetworks.com]
> Sent: Thursday, January 12, 2017 11:51 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Doherty, Declan; jerin.jacob@caviumnetworks.com
> Subject: Re: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test
> vectors
>
> Hello Pablo,
>
> On 12.01.2017 11:48, De Lara Guarch, Pablo wrote:
> > Hi Bodek,
> >
> >> -----Original Message-----
> >> From: zbigniew.bodek@caviumnetworks.com
> >> [mailto:zbigniew.bodek@caviumnetworks.com]
> >> Sent: Wednesday, January 04, 2017 5:33 PM
> >> To: dev@dpdk.org
> >> Cc: De Lara Guarch, Pablo; Doherty, Declan;
> >> jerin.jacob@caviumnetworks.com; Zbigniew Bodek
> >> Subject: [PATCH v3 8/8] app/test: add ARMv8 crypto tests and test
> vectors
> >>
> >> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
> >>
> >> Introduce unit tests for ARMv8 crypto PMD.
> >> Add test vectors for short cases such as 160 bytes.
> >> These test cases are ARMv8 specific since the code provides
> >> different processing paths for different input data sizes.
> >>
> >> User can validate correctness of algorithms' implementation using:
> >> * cryptodev_sw_armv8_autotest
> >> For performance test one can use:
> >> * cryptodev_sw_armv8_perftest
> >>
> >> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
> >
> > Could you rebase this patchset with the dpdk-next-crypto tree?
> > There is a compilation error due to a missing parameter in a function that
> has recently changed.
>
> I see. The rebase is done. Should I send full v4 patchset now?
>
There are some comments from Jianbo Liu. Take a look at them in case
you have something to change there.
Also, since you are sending a v4 patchset, make the commit name changes too, please.
Thanks,
Pablo
> Kind regards
> Zbigniew
>
> >
> > Thanks,
> > Pablo
> >
^ permalink raw reply
* Re: [dpdk-users] IGB_UIO: PCI Resources Management
From: Alejandro Lucero @ 2017-01-12 12:12 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Gregory Etelson, dev, users
In-Reply-To: <608e7dfd-5226-3e30-f43b-0fbe01aee16a@intel.com>
On Thu, Jan 12, 2017 at 11:55 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
> On 12/9/2016 8:54 AM, Gregory Etelson wrote:
> > Hello,
> >
> > IGB_UIO driver does not close port PCI activities after DPDK process
> exits.
> > DPDK API provides rte_eth_dev_close() to manage port PCI,
> > but it can be skipped if process receives SIGKILL signal
>
> I guess I understand the problem.
>
This is a known problem, but it is not just a UIO problem, and this patch
does not solve it, maybe it just solves part of it.
In fact, a DPDK program crashing could imply the NIC DMAing after that and
after that memory was assigned to another program.
>
> > The patches below provide IGB_UIO release callback and IXGBEVF release
> function
>
> But adding ixgbe specific code into igb_uio may not be good idea.
> Can be anything done one upper layer, pci layer, generic to all drivers?
>
>
This module is not just being used for Intel cards, so this addition will
break, at least, the NFP PMD support.
I was told to use igb_uio instead of adding a new NFP uio driver, so I
guess that implies this igb_uio driver should be considered not only a igb
driver.
> > With the patches, each time DPDK process terminates,
> > UIO release callback will trigger port PCI close.
> > On the down side, patched IGB_UIO can be bound to a single adapter type
> >
> > Regards,
> > Gregory
>
> <...>
>
^ permalink raw reply
* Re: After a port is bound to dpdk, when 'ip addr' or 'ip link add ~vlan' command against non-dpdk port , Linux kernel hangs
From: Ferruh Yigit @ 2017-01-12 12:16 UTC (permalink / raw)
To: Joo Kim, dev
In-Reply-To: <CAFj4wcKhyjE+uW95ELR-oXPB7gEo5TenN21mQFOUgXkyBCtqkg@mail.gmail.com>
On 12/9/2016 11:27 PM, Joo Kim wrote:
> Hello,
> I am using dpdk 2.2.0 in this VM where several NIC ports are available.
>
> Also, uio, kni drivers are installed.
> [root@mylinux]# lsmod | grep uio
> igb_uio 13224 0
> uio 19259 1 igb_uio
> [root@mylinux]# lsmod | grep kni
> rte_kni 294375 0
> i2c_core 40325 2 i2c_dev,rte_kni
Why kni module depends on i2c?
> [root@mylinux]# uname -r
> 3.10.0-229.20.1.el7.x86_64
>
> Weirdly, once I bind a network port to dpdk, after that, when I run 'ip
> addr' or 'ip link add ~vlan' command against non-dpdk port , this VM's
> Linux kernel hangs.
Is KNI module used at all in this point?
>
> For example, when I run 'ip link add ~vlan' command, then following dmesg
> is seen and Linux hangs.
>
> Any idea on root cause/how to fix?
>
<...>
> [ 4884.158003] Call Trace:
> [ 4884.158003] [<ffffffff815bb3ae>] ? fib6_clean_all+0xae/0x100
> [ 4884.158003] [<ffffffffa006f3f0>] virtnet_vlan_rx_kill_vid+0x50/0x90
> [virtio_net]
> [ 4884.158003] [<ffffffff815e67e3>] vlan_vid_del+0x113/0x170
> [ 4884.158003] [<ffffffffa02b681b>] vlan_device_event+0x65b/0x690 [8021q]
> [ 4884.158003] [<ffffffff8160fdec>] notifier_call_chain+0x4c/0x70
> [ 4884.158003] [<ffffffff8109d456>] raw_notifier_call_chain+0x16/0x20
> [ 4884.158003] [<ffffffff814f78fd>] call_netdevice_notifiers+0x2d/0x60
> [ 4884.158003] [<ffffffff814f7bac>] dev_close_many+0xcc/0x120
> [ 4884.158003] [<ffffffff814f8ee8>] rollback_registered_many+0xa8/0x220
> [ 4884.158003] [<ffffffff814f90a0>] rollback_registered+0x40/0x70
> [ 4884.158003] [<ffffffff814fa388>] unregister_netdevice_queue+0x48/0x80
> [ 4884.158003] [<ffffffff814fa3dc>] unregister_netdev+0x1c/0x30
> [ 4884.158003] [<ffffffffa006e0e3>] virtnet_remove+0x43/0x80 [virtio_net]
> [ 4884.158003] [<ffffffffa002f119>] virtio_dev_remove+0x49/0xd0 [virtio]
This looks like crash occurred when virtio unbind.
Are you using custom modules?
> [ 4884.158003] [<ffffffff813d2cbf>] __device_release_driver+0x7f/0xf0
> [ 4884.158003] [<ffffffff813d2d53>] device_release_driver+0x23/0x30
> [ 4884.158003] [<ffffffff813d24c8>] bus_remove_device+0x108/0x180
> [ 4884.158003] [<ffffffff813ce9e5>] device_del+0x135/0x1f0
> [ 4884.158003] [<ffffffff813ceabe>] device_unregister+0x1e/0x60
> [ 4884.158003] [<ffffffffa002f426>] unregister_virtio_device+0x16/0x30
> [virtio]
> [ 4884.158003] [<ffffffffa01c852b>] virtio_pci_remove+0x2b/0x70
> [virtio_pci]
> [ 4884.158003] [<ffffffff813097bb>] pci_device_remove+0x3b/0xb0
> [ 4884.158003] [<ffffffff813d2cbf>] __device_release_driver+0x7f/0xf0
> [ 4884.158003] [<ffffffff813d2d53>] device_release_driver+0x23/0x30
> [ 4884.158003] [<ffffffff813d16bd>] driver_unbind+0xbd/0xe0
> [ 4884.158003] [<ffffffff813d0bf4>] drv_attr_store+0x24/0x40
> [ 4884.158003] [<ffffffff8123cfc6>] sysfs_write_file+0xc6/0x140
> [ 4884.158003] [<ffffffff811c6a2d>] vfs_write+0xbd/0x1e0
> [ 4884.158003] [<ffffffff8160fce3>] ? trace_do_page_fault+0x43/0x100
> [ 4884.158003] [<ffffffff811c7478>] SyS_write+0x58/0xb0
> [ 4884.158003] [<ffffffff81614409>] system_call_fastpath+0x16/0x1b
> [ 4884.158003] Code: c5 70 ff ff ff e8 2e 11 fe ff 48 8b 7b 08 e8 c5 02 fe
> ff eb 12 0f 1f 00 48 8b 7b 08 e8 77 ff fd ff 84 c0 75 17 f3 90 48 8b 7b 08
> <48> 8d b5 6c ff ff ff e8 01 01 fe ff 48 85 c0 74 dc 80 bd 69 ff
> {noformat}
>
^ permalink raw reply
* Re: [dpdk-users] IGB_UIO: PCI Resources Management
From: Ferruh Yigit @ 2017-01-12 12:22 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: Gregory Etelson, dev, users
In-Reply-To: <CAD+H991Zvaa56O6s_KareaH7mxYxJrn1pW6gG4cLDDKdDA=b0A@mail.gmail.com>
On 1/12/2017 12:12 PM, Alejandro Lucero wrote:
>
>
> On Thu, Jan 12, 2017 at 11:55 AM, Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
>
> On 12/9/2016 8:54 AM, Gregory Etelson wrote:
> > Hello,
> >
> > IGB_UIO driver does not close port PCI activities after DPDK process exits.
> > DPDK API provides rte_eth_dev_close() to manage port PCI,
> > but it can be skipped if process receives SIGKILL signal
>
> I guess I understand the problem.
>
>
> This is a known problem, but it is not just a UIO problem, and this
> patch does not solve it, maybe it just solves part of it.
>
> In fact, a DPDK program crashing could imply the NIC DMAing after that
> and after that memory was assigned to another program.
Yes.
Can there be a way to stop NIC DMA, (or prevent it access to mem
anymore) when app crashes?
I think that is what this patch is looking for.
>
>
>
>
> > The patches below provide IGB_UIO release callback and IXGBEVF release function
>
> But adding ixgbe specific code into igb_uio may not be good idea.
> Can be anything done one upper layer, pci layer, generic to all drivers?
>
>
> This module is not just being used for Intel cards, so this addition
> will break, at least, the NFP PMD support.
>
> I was told to use igb_uio instead of adding a new NFP uio driver, so I
> guess that implies this igb_uio driver should be considered not only a
> igb driver.
No it is generic, I think names has igb_ just for historical reasons.
>
>
> > With the patches, each time DPDK process terminates,
> > UIO release callback will trigger port PCI close.
> > On the down side, patched IGB_UIO can be bound to a single adapter type
> >
> > Regards,
> > Gregory
>
> <...>
>
>
^ permalink raw reply
* Re: [PATCH v2 01/11] librte_cryptodev: Add rte_device pointer in cryptodevice
From: Akhil Goyal @ 2017-01-12 12:26 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org
Cc: thomas.monjalon@6wind.com, Doherty, Declan,
hemant.agrawal@nxp.com, Mcnamara, John, nhorman@tuxdriver.com
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D897476C075F@IRSMSX108.ger.corp.intel.com>
On 1/9/2017 7:04 PM, De Lara Guarch, Pablo wrote:
> Hi,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
>> Sent: Thursday, December 22, 2016 8:17 PM
>> To: dev@dpdk.org
>> Cc: thomas.monjalon@6wind.com; Doherty, Declan; De Lara Guarch, Pablo;
>> hemant.agrawal@nxp.com; Mcnamara, John; nhorman@tuxdriver.com;
>> Akhil Goyal
>> Subject: [dpdk-dev] [PATCH v2 01/11] librte_cryptodev: Add rte_device
>> pointer in cryptodevice
>>
>> This patch will not be required as some parallel work is going
>> on to add it across all crypto devices.
>>
> Could you tell me the patch that is going to add this?
> In that case, you can drop this and just say that your patchset depends on it.
>
> Also, the title should be "cryptodev: add rte_device pointer in crypto device".
> Note that for libraries, you just need the name of the library (i.e. cryptodev).
> The other thing is that the first letter should be lowercase. Could you change this in the other patches too?
>
>
> Thanks,
> Pablo
>
Thanks for your comments Pablo.
There is some discussion ongoing regarding bus model for DPAA2 platform.
This change is already there for ethdev and we would be replicating the
same for cryptodev, once the bus model patches are completed. This is
added here so that compilation is not broken for subsequent patches.
For title, I will correct that in next version.
Thanks,
Akhil
^ permalink raw reply
* Re: [PATCH v2 02/11] crypto/dpaa2_sec: Run time assembler for Descriptor formation
From: Akhil Goyal @ 2017-01-12 12:28 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org
Cc: thomas.monjalon@6wind.com, Doherty, Declan,
hemant.agrawal@nxp.com, Mcnamara, John, nhorman@tuxdriver.com,
Horia Geanta Neag
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D897476C07F1@IRSMSX108.ger.corp.intel.com>
On 1/9/2017 7:25 PM, De Lara Guarch, Pablo wrote:
>
>
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Thursday, December 22, 2016 8:17 PM
>> To: dev@dpdk.org
>> Cc: thomas.monjalon@6wind.com; Doherty, Declan; De Lara Guarch, Pablo;
>> hemant.agrawal@nxp.com; Mcnamara, John; nhorman@tuxdriver.com;
>> Akhil Goyal; Horia Geanta Neag
>> Subject: [PATCH v2 02/11] crypto/dpaa2_sec: Run time assembler for
>> Descriptor formation
>>
>> A set of header files(hw) which helps in making the descriptors
>> that are understood by NXP's SEC hardware.
>> This patch provides header files for command words which can be used
>> for descriptor formation.
>>
>> Signed-off-by: Horia Geanta Neag <horia.geanta@nxp.com>
>> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
>> ---
>
> ...
>
>> diff --git a/drivers/crypto/dpaa2_sec/hw/rta.h
>> b/drivers/crypto/dpaa2_sec/hw/rta.h
>> new file mode 100644
>> index 0000000..7eb0455
>> --- /dev/null
>> +++ b/drivers/crypto/dpaa2_sec/hw/rta.h
>
> ...
>
>> +extern enum rta_sec_era rta_sec_era;
>> +
>> +/**
>> + * rta_set_sec_era - Set SEC Era HW block revision for which the RTA
>> library
>> + * will generate the descriptors.
>> + * @era: SEC Era (enum rta_sec_era)
>> + *
>> + * Return: 0 if the ERA was set successfully, -1 otherwise (int)
>> + *
>> + * Warning 1: Must be called *only once*, *before* using any other RTA
>> API
>> + * routine.
>> + *
>> + * Warning 2: *Not thread safe*.
>> + */
>
>> +static inline int rta_set_sec_era(enum rta_sec_era era)
>> +{
>
> "static inline int" should go in a different line than the function name and parameters.
> So it should be:
>
> static inline int
> rta_set_sec_era(enum rta_sec_era era)
> {
>
> Could you make this change here and in the rest of the functions?
>
> Thanks,
> Pablo
>
>
Ok, I will correct in the next version.
Thanks,
Akhil
^ permalink raw reply
* Re: [PATCH v2 07/11] crypto/dpaa2_sec: Add DPAA2_SEC PMD into build system
From: Akhil Goyal @ 2017-01-12 12:35 UTC (permalink / raw)
To: Thomas Monjalon, hemant.agrawal
Cc: dev, declan.doherty, pablo.de.lara.guarch, john.mcnamara, nhorman
In-Reply-To: <1544143.B2rDg7MdQ1@xps13>
On 1/9/2017 9:03 PM, Thomas Monjalon wrote:
> 2016-12-23 01:46, Akhil Goyal:
>> +ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_COMMON),y)
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += -lrte_pmd_dpaa2_sec
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += -lrte_pmd_dpaa2_qbman
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += -lrte_pmd_dpaa2_dpio
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += -lrte_pmd_dpaa2_pool
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += -lrte_pmd_fslmcbus
>> +endif
>
> There are so much libs!
> We do not have even one commit per library in this patchset.
> Splitting patches would allow to better introduce them one by one
> with an explanation of the design and the role of each library.
>
Thanks for your comments Thomas.
The libraries that are referred here are not added in this patchset.
They were introduced in the base patches for DPAA2 platform.
[1] http://dpdk.org/ml/archives/dev/2016-December/051364.html
This patch set only uses those libraries. The design and role of each
library is introduced in doc/guides/cryptodevs/dpaa2_sec.rst.
Please let me know if something is not clear in that.
Thanks,
Akhil
^ permalink raw reply
* Re: [dpdk-users] IGB_UIO: PCI Resources Management
From: Alejandro Lucero @ 2017-01-12 12:58 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Gregory Etelson, dev, users
In-Reply-To: <e1507eab-1604-b300-8f86-5d5ac2c98ee5@intel.com>
On Thu, Jan 12, 2017 at 12:22 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
> On 1/12/2017 12:12 PM, Alejandro Lucero wrote:
> >
> >
> > On Thu, Jan 12, 2017 at 11:55 AM, Ferruh Yigit <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> > On 12/9/2016 8:54 AM, Gregory Etelson wrote:
> > > Hello,
> > >
> > > IGB_UIO driver does not close port PCI activities after DPDK
> process exits.
> > > DPDK API provides rte_eth_dev_close() to manage port PCI,
> > > but it can be skipped if process receives SIGKILL signal
> >
> > I guess I understand the problem.
> >
> >
> > This is a known problem, but it is not just a UIO problem, and this
> > patch does not solve it, maybe it just solves part of it.
> >
> > In fact, a DPDK program crashing could imply the NIC DMAing after that
> > and after that memory was assigned to another program.
>
> Yes.
> Can there be a way to stop NIC DMA, (or prevent it access to mem
> anymore) when app crashes?
> I think that is what this patch is looking for.
>
>
But with the patch, it just happens when igb_uio module is removed. I guess
this is good for then loading or binding the device to another module, but
that does not solve the problem about stopping the NIC asap.
As I see it, the EAL should catch signals forcing always to close ports,
even when no signal, because it could be just the app exiting without error
but the port/NIC able to receive packets. But for SIGKILL, that would not
be enough. So we need something else for always calling a destructor before
fully exiting.
> >
> >
> >
> >
> > > The patches below provide IGB_UIO release callback and IXGBEVF
> release function
> >
> > But adding ixgbe specific code into igb_uio may not be good idea.
> > Can be anything done one upper layer, pci layer, generic to all
> drivers?
> >
> >
> > This module is not just being used for Intel cards, so this addition
> > will break, at least, the NFP PMD support.
> >
> > I was told to use igb_uio instead of adding a new NFP uio driver, so I
> > guess that implies this igb_uio driver should be considered not only a
> > igb driver.
>
> No it is generic, I think names has igb_ just for historical reasons.
>
Great.
>
> >
> >
> > > With the patches, each time DPDK process terminates,
> > > UIO release callback will trigger port PCI close.
> > > On the down side, patched IGB_UIO can be bound to a single adapter
> type
> > >
> > > Regards,
> > > Gregory
> >
> > <...>
> >
> >
>
>
^ permalink raw reply
* [PATCH] net/nfp: fix Vlan offload flags check
From: Olivier Matz @ 2017-01-12 13:04 UTC (permalink / raw)
To: dev, alejandro.lucero
Fix typo when checking that no Vlan offload flags are passed at port
initialization.
By the way, also fix a typo in the log.
Fixes: d4a27a3b092a ("nfp: add basic features")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/nfp/nfp_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 65ba09f..ed3c9b8 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2174,8 +2174,8 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
new_ctrl = 0;
if ((mask & ETH_VLAN_FILTER_OFFLOAD) ||
- (mask & ETH_VLAN_FILTER_OFFLOAD))
- RTE_LOG(INFO, PMD, "Not support for ETH_VLAN_FILTER_OFFLOAD or"
+ (mask & ETH_VLAN_FILTER_EXTEND))
+ RTE_LOG(INFO, PMD, "No support for ETH_VLAN_FILTER_OFFLOAD or"
" ETH_VLAN_FILTER_EXTEND");
/* Enable vlan strip if it is not configured yet */
--
2.8.1
^ permalink raw reply related
* Re: [PATCH v3 3/8] crypto/armv8: add PMD optimized for ARMv8 processors
From: Zbigniew Bodek @ 2017-01-12 13:12 UTC (permalink / raw)
To: Jianbo Liu; +Cc: dev, pablo.de.lara.guarch, Declan Doherty, Jerin Jacob
In-Reply-To: <CAP4Qi3-An1H2d2B3AZxMNn2WcTa=M2Kp8nN0nPz+z9+DtoTDBA@mail.gmail.com>
Hello Jianbo Liu,
Thanks for the review. Please check my answers in-line.
Kind regards
Zbigniew
On 06.01.2017 03:45, Jianbo Liu wrote:
> On 5 January 2017 at 01:33, <zbigniew.bodek@caviumnetworks.com> wrote:
>> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>>
>> This patch introduces crypto poll mode driver
>> using ARMv8 cryptographic extensions.
>> CPU compatibility with this driver is detected in
>> run-time and virtual crypto device will not be
>> created if CPU doesn't provide:
>> AES, SHA1, SHA2 and NEON.
>>
>> This PMD is optimized to provide performance boost
>> for chained crypto operations processing,
>> such as encryption + HMAC generation,
>> decryption + HMAC validation. In particular,
>> cipher only or hash only operations are
>> not provided.
>>
>> The driver currently supports AES-128-CBC
>> in combination with: SHA256 HMAC and SHA1 HMAC
>> and relies on the external armv8_crypto library:
>> https://github.com/caviumnetworks/armv8_crypto
>>
>
> It's standalone lib. I think you should change the following line in
> its Makefile, so not depend on DPDK.
> "include $(RTE_SDK)/mk/rte.lib.mk"
>
>> This patch adds driver's code only and does
>> not include it in the build system.
>>
>> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>> ---
>> drivers/crypto/armv8/Makefile | 73 ++
>> drivers/crypto/armv8/rte_armv8_pmd.c | 926 +++++++++++++++++++++++++
>> drivers/crypto/armv8/rte_armv8_pmd_ops.c | 369 ++++++++++
>> drivers/crypto/armv8/rte_armv8_pmd_private.h | 211 ++++++
>> drivers/crypto/armv8/rte_armv8_pmd_version.map | 3 +
>> 5 files changed, 1582 insertions(+)
>> create mode 100644 drivers/crypto/armv8/Makefile
>> create mode 100644 drivers/crypto/armv8/rte_armv8_pmd.c
>> create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_ops.c
>> create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_private.h
>> create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_version.map
>>
>> diff --git a/drivers/crypto/armv8/Makefile b/drivers/crypto/armv8/Makefile
>> new file mode 100644
>> index 0000000..dc5ea02
>> --- /dev/null
>> +++ b/drivers/crypto/armv8/Makefile
>> @@ -0,0 +1,73 @@
>> +#
>> +# BSD LICENSE
>> +#
>> +# Copyright (C) Cavium networks Ltd. 2017.
>> +#
>> +# 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.
>> +#
>> +
>> +include $(RTE_SDK)/mk/rte.vars.mk
>> +
>> +ifneq ($(MAKECMDGOALS),clean)
>> +ifneq ($(MAKECMDGOALS),config)
>> +ifeq ($(ARMV8_CRYPTO_LIB_PATH),)
>> +$(error "Please define ARMV8_CRYPTO_LIB_PATH environment variable")
>> +endif
>> +endif
>> +endif
>> +
>> +# library name
>> +LIB = librte_pmd_armv8.a
>> +
>> +# build flags
>> +CFLAGS += -O3
>> +CFLAGS += $(WERROR_FLAGS)
>> +CFLAGS += -L$(RTE_SDK)/../openssl -I$(RTE_SDK)/../openssl/include
>
> Is it really needed?
No. It is removed now.
>
>> +
>> +# library version
>> +LIBABIVER := 1
>> +
>> +# versioning export map
>> +EXPORT_MAP := rte_armv8_pmd_version.map
>> +
>> +# external library dependencies
>> +CFLAGS += -I$(ARMV8_CRYPTO_LIB_PATH)
>> +CFLAGS += -I$(ARMV8_CRYPTO_LIB_PATH)/asm/include
>> +LDLIBS += -L$(ARMV8_CRYPTO_LIB_PATH) -larmv8_crypto
>> +
>> +# library source files
>> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += rte_armv8_pmd.c
>> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += rte_armv8_pmd_ops.c
>> +
>> +# library dependencies
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += lib/librte_eal
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += lib/librte_mbuf
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += lib/librte_mempool
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += lib/librte_ring
>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += lib/librte_cryptodev
>> +
>> +include $(RTE_SDK)/mk/rte.lib.mk
>> diff --git a/drivers/crypto/armv8/rte_armv8_pmd.c b/drivers/crypto/armv8/rte_armv8_pmd.c
>> new file mode 100644
>> index 0000000..39433bb
>> --- /dev/null
>> +++ b/drivers/crypto/armv8/rte_armv8_pmd.c
>> @@ -0,0 +1,926 @@
>> +/*
>> + * BSD LICENSE
>> + *
>> + * Copyright (C) Cavium networks Ltd. 2017.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <stdbool.h>
>> +
>> +#include <rte_common.h>
>> +#include <rte_hexdump.h>
>> +#include <rte_cryptodev.h>
>> +#include <rte_cryptodev_pmd.h>
>> +#include <rte_vdev.h>
>> +#include <rte_malloc.h>
>> +#include <rte_cpuflags.h>
>> +
>> +#include "armv8_crypto_defs.h"
>> +
>> +#include "rte_armv8_pmd_private.h"
>> +
>> +static int cryptodev_armv8_crypto_uninit(const char *name);
>> +
>> +/**
>> + * Pointers to the supported combined mode crypto functions are stored
>> + * in the static tables. Each combined (chained) cryptographic operation
>> + * can be decribed by a set of numbers:
>> + * - order: order of operations (cipher, auth) or (auth, cipher)
>> + * - direction: encryption or decryption
>> + * - calg: cipher algorithm such as AES_CBC, AES_CTR, etc.
>> + * - aalg: authentication algorithm such as SHA1, SHA256, etc.
>> + * - keyl: cipher key length, for example 128, 192, 256 bits
>> + *
>> + * In order to quickly acquire each function pointer based on those numbers,
>> + * a hierarchy of arrays is maintained. The final level, 3D array is indexed
>> + * by the combined mode function parameters only (cipher algorithm,
>> + * authentication algorithm and key length).
>> + *
>> + * This gives 3 memory accesses to obtain a function pointer instead of
>> + * traversing the array manually and comparing function parameters on each loop.
>> + *
>> + * +--+CRYPTO_FUNC
>> + * +--+ENC|
>> + * +--+CA|
>> + * | +--+DEC
>> + * ORDER|
>> + * | +--+ENC
>> + * +--+AC|
>> + * +--+DEC
>> + *
>> + */
>> +
>> +/**
>> + * 3D array type for ARM Combined Mode crypto functions pointers.
>> + * CRYPTO_CIPHER_MAX: max cipher ID number
>> + * CRYPTO_AUTH_MAX: max auth ID number
>> + * CRYPTO_CIPHER_KEYLEN_MAX: max key length ID number
>> + */
>> +typedef const crypto_func_t
>> +crypto_func_tbl_t[CRYPTO_CIPHER_MAX][CRYPTO_AUTH_MAX][CRYPTO_CIPHER_KEYLEN_MAX];
>> +
>> +/* Evaluate to key length definition */
>> +#define KEYL(keyl) (ARMV8_CRYPTO_CIPHER_KEYLEN_ ## keyl)
>> +
>> +/* Local aliases for supported ciphers */
>> +#define CIPH_AES_CBC RTE_CRYPTO_CIPHER_AES_CBC
>> +/* Local aliases for supported hashes */
>> +#define AUTH_SHA1_HMAC RTE_CRYPTO_AUTH_SHA1_HMAC
>> +#define AUTH_SHA256 RTE_CRYPTO_AUTH_SHA256
>> +#define AUTH_SHA256_HMAC RTE_CRYPTO_AUTH_SHA256_HMAC
>> +
>> +/**
>> + * Arrays containing pointers to particular cryptographic,
>> + * combined mode functions.
>> + * crypto_op_ca_encrypt: cipher (encrypt), authenticate
>> + * crypto_op_ca_decrypt: cipher (decrypt), authenticate
>> + * crypto_op_ac_encrypt: authenticate, cipher (encrypt)
>> + * crypto_op_ac_decrypt: authenticate, cipher (decrypt)
>> + */
>> +static const crypto_func_tbl_t
>> +crypto_op_ca_encrypt = {
>> + /* [cipher alg][auth alg][key length] = crypto_function, */
>> + [CIPH_AES_CBC][AUTH_SHA1_HMAC][KEYL(128)] = aes128cbc_sha1_hmac,
>> + [CIPH_AES_CBC][AUTH_SHA256_HMAC][KEYL(128)] = aes128cbc_sha256_hmac,
>> +};
>> +
>> +static const crypto_func_tbl_t
>> +crypto_op_ca_decrypt = {
>> + NULL
>> +};
>> +
>> +static const crypto_func_tbl_t
>> +crypto_op_ac_encrypt = {
>> + NULL
>> +};
>> +
>> +static const crypto_func_tbl_t
>> +crypto_op_ac_decrypt = {
>> + /* [cipher alg][auth alg][key length] = crypto_function, */
>> + [CIPH_AES_CBC][AUTH_SHA1_HMAC][KEYL(128)] = sha1_hmac_aes128cbc_dec,
>> + [CIPH_AES_CBC][AUTH_SHA256_HMAC][KEYL(128)] = sha256_hmac_aes128cbc_dec,
>> +};
>> +
>> +/**
>> + * Arrays containing pointers to particular cryptographic function sets,
>> + * covering given cipher operation directions (encrypt, decrypt)
>> + * for each order of cipher and authentication pairs.
>> + */
>> +static const crypto_func_tbl_t *
>> +crypto_cipher_auth[] = {
>> + &crypto_op_ca_encrypt,
>> + &crypto_op_ca_decrypt,
>> + NULL
>> +};
>> +
>> +static const crypto_func_tbl_t *
>> +crypto_auth_cipher[] = {
>> + &crypto_op_ac_encrypt,
>> + &crypto_op_ac_decrypt,
>> + NULL
>> +};
>> +
>> +/**
>> + * Top level array containing pointers to particular cryptographic
>> + * function sets, covering given order of chained operations.
>> + * crypto_cipher_auth: cipher first, authenticate after
>> + * crypto_auth_cipher: authenticate first, cipher after
>> + */
>> +static const crypto_func_tbl_t **
>> +crypto_chain_order[] = {
>> + crypto_cipher_auth,
>> + crypto_auth_cipher,
>> + NULL
>> +};
>> +
>> +/**
>> + * Extract particular combined mode crypto function from the 3D array.
>> + */
>> +#define CRYPTO_GET_ALGO(order, cop, calg, aalg, keyl) \
>> +({ \
>> + crypto_func_tbl_t *func_tbl = \
>> + (crypto_chain_order[(order)])[(cop)]; \
>> + \
>> + ((*func_tbl)[(calg)][(aalg)][KEYL(keyl)]); \
>> +})
>> +
>> +/*----------------------------------------------------------------------------*/
>> +
>> +/**
>> + * 2D array type for ARM key schedule functions pointers.
>> + * CRYPTO_CIPHER_MAX: max cipher ID number
>> + * CRYPTO_CIPHER_KEYLEN_MAX: max key length ID number
>> + */
>> +typedef const crypto_key_sched_t
>> +crypto_key_sched_tbl_t[CRYPTO_CIPHER_MAX][CRYPTO_CIPHER_KEYLEN_MAX];
>> +
>> +static const crypto_key_sched_tbl_t
>> +crypto_key_sched_encrypt = {
>> + /* [cipher alg][key length] = key_expand_func, */
>> + [CIPH_AES_CBC][KEYL(128)] = aes128_key_sched_enc,
>> +};
>> +
>> +static const crypto_key_sched_tbl_t
>> +crypto_key_sched_decrypt = {
>> + /* [cipher alg][key length] = key_expand_func, */
>> + [CIPH_AES_CBC][KEYL(128)] = aes128_key_sched_dec,
>> +};
>> +
>> +/**
>> + * Top level array containing pointers to particular key generation
>> + * function sets, covering given operation direction.
>> + * crypto_key_sched_encrypt: keys for encryption
>> + * crypto_key_sched_decrypt: keys for decryption
>> + */
>> +static const crypto_key_sched_tbl_t *
>> +crypto_key_sched_dir[] = {
>> + &crypto_key_sched_encrypt,
>> + &crypto_key_sched_decrypt,
>> + NULL
>> +};
>> +
>> +/**
>> + * Extract particular combined mode crypto function from the 3D array.
>> + */
>> +#define CRYPTO_GET_KEY_SCHED(cop, calg, keyl) \
>> +({ \
>> + crypto_key_sched_tbl_t *ks_tbl = crypto_key_sched_dir[(cop)]; \
>> + \
>> + ((*ks_tbl)[(calg)][KEYL(keyl)]); \
>> +})
>> +
>> +/*----------------------------------------------------------------------------*/
>> +
>> +/**
>> + * Global static parameter used to create a unique name for each
>> + * ARMV8 crypto device.
>> + */
>> +static unsigned int unique_name_id;
>> +
>> +static inline int
>> +create_unique_device_name(char *name, size_t size)
>> +{
>> + int ret;
>> +
>> + if (name == NULL)
>> + return -EINVAL;
>> +
>> + ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_ARMV8_PMD),
>> + unique_name_id++);
>> + if (ret < 0)
>> + return ret;
>> + return 0;
>> +}
>> +
>> +/*
>> + *------------------------------------------------------------------------------
>> + * Session Prepare
>> + *------------------------------------------------------------------------------
>> + */
>> +
>> +/** Get xform chain order */
>> +static enum armv8_crypto_chain_order
>> +armv8_crypto_get_chain_order(const struct rte_crypto_sym_xform *xform)
>> +{
>> +
>> + /*
>> + * This driver currently covers only chained operations.
>> + * Ignore only cipher or only authentication operations
>> + * or chains longer than 2 xform structures.
>> + */
>> + if (xform->next == NULL || xform->next->next != NULL)
>> + return ARMV8_CRYPTO_CHAIN_NOT_SUPPORTED;
>> +
>> + if (xform->type == RTE_CRYPTO_SYM_XFORM_AUTH) {
>> + if (xform->next->type == RTE_CRYPTO_SYM_XFORM_CIPHER)
>> + return ARMV8_CRYPTO_CHAIN_AUTH_CIPHER;
>> + }
>> +
>> + if (xform->type == RTE_CRYPTO_SYM_XFORM_CIPHER) {
>> + if (xform->next->type == RTE_CRYPTO_SYM_XFORM_AUTH)
>> + return ARMV8_CRYPTO_CHAIN_CIPHER_AUTH;
>> + }
>> +
>> + return ARMV8_CRYPTO_CHAIN_NOT_SUPPORTED;
>> +}
>> +
>> +static inline void
>> +auth_hmac_pad_prepare(struct armv8_crypto_session *sess,
>> + const struct rte_crypto_sym_xform *xform)
>> +{
>> + size_t i;
>> +
>> + /* Generate i_key_pad and o_key_pad */
>> + memset(sess->auth.hmac.i_key_pad, 0, sizeof(sess->auth.hmac.i_key_pad));
>> + rte_memcpy(sess->auth.hmac.i_key_pad, sess->auth.hmac.key,
>> + xform->auth.key.length);
>> + memset(sess->auth.hmac.o_key_pad, 0, sizeof(sess->auth.hmac.o_key_pad));
>> + rte_memcpy(sess->auth.hmac.o_key_pad, sess->auth.hmac.key,
>> + xform->auth.key.length);
>> + /*
>> + * XOR key with IPAD/OPAD values to obtain i_key_pad
>> + * and o_key_pad.
>> + * Byte-by-byte operation may seem to be the less efficient
>> + * here but in fact it's the opposite.
>> + * The result ASM code is likely operate on NEON registers
>> + * (load auth key to Qx, load IPAD/OPAD to multiple
>> + * elements of Qy, eor 128 bits at once).
>> + */
>> + for (i = 0; i < SHA_BLOCK_MAX; i++) {
>> + sess->auth.hmac.i_key_pad[i] ^= HMAC_IPAD_VALUE;
>> + sess->auth.hmac.o_key_pad[i] ^= HMAC_OPAD_VALUE;
>> + }
>> +}
>> +
>> +static inline int
>> +auth_set_prerequisites(struct armv8_crypto_session *sess,
>> + const struct rte_crypto_sym_xform *xform)
>> +{
>> + uint8_t partial[64] = { 0 };
>> + int error;
>> +
>> + switch (xform->auth.algo) {
>> + case RTE_CRYPTO_AUTH_SHA1_HMAC:
>> + /*
>> + * Generate authentication key, i_key_pad and o_key_pad.
>> + */
>> + /* Zero memory under key */
>> + memset(sess->auth.hmac.key, 0, SHA1_AUTH_KEY_LENGTH);
>> +
>> + if (xform->auth.key.length > SHA1_AUTH_KEY_LENGTH) {
>> + /*
>> + * In case the key is longer than 160 bits
>> + * the algorithm will use SHA1(key) instead.
>> + */
>> + error = sha1_block(NULL, xform->auth.key.data,
>> + sess->auth.hmac.key, xform->auth.key.length);
>> + if (error != 0)
>> + return -1;
>> + } else {
>> + /*
>> + * Now copy the given authentication key to the session
>> + * key assuming that the session key is zeroed there is
>> + * no need for additional zero padding if the key is
>> + * shorter than SHA1_AUTH_KEY_LENGTH.
>> + */
>> + rte_memcpy(sess->auth.hmac.key, xform->auth.key.data,
>> + xform->auth.key.length);
>> + }
>> +
>> + /* Prepare HMAC padding: key|pattern */
>> + auth_hmac_pad_prepare(sess, xform);
>> + /*
>> + * Calculate partial hash values for i_key_pad and o_key_pad.
>> + * Will be used as initialization state for final HMAC.
>> + */
>> + error = sha1_block_partial(NULL, sess->auth.hmac.i_key_pad,
>> + partial, SHA1_BLOCK_SIZE);
>> + if (error != 0)
>> + return -1;
>> + memcpy(sess->auth.hmac.i_key_pad, partial, SHA1_BLOCK_SIZE);
>> +
>> + error = sha1_block_partial(NULL, sess->auth.hmac.o_key_pad,
>> + partial, SHA1_BLOCK_SIZE);
>> + if (error != 0)
>> + return -1;
>> + memcpy(sess->auth.hmac.o_key_pad, partial, SHA1_BLOCK_SIZE);
>> +
>> + break;
>> + case RTE_CRYPTO_AUTH_SHA256_HMAC:
>> + /*
>> + * Generate authentication key, i_key_pad and o_key_pad.
>> + */
>> + /* Zero memory under key */
>> + memset(sess->auth.hmac.key, 0, SHA256_AUTH_KEY_LENGTH);
>> +
>> + if (xform->auth.key.length > SHA256_AUTH_KEY_LENGTH) {
>> + /*
>> + * In case the key is longer than 256 bits
>> + * the algorithm will use SHA256(key) instead.
>> + */
>> + error = sha256_block(NULL, xform->auth.key.data,
>> + sess->auth.hmac.key, xform->auth.key.length);
>> + if (error != 0)
>> + return -1;
>> + } else {
>> + /*
>> + * Now copy the given authentication key to the session
>> + * key assuming that the session key is zeroed there is
>> + * no need for additional zero padding if the key is
>> + * shorter than SHA256_AUTH_KEY_LENGTH.
>> + */
>> + rte_memcpy(sess->auth.hmac.key, xform->auth.key.data,
>> + xform->auth.key.length);
>> + }
>> +
>> + /* Prepare HMAC padding: key|pattern */
>> + auth_hmac_pad_prepare(sess, xform);
>> + /*
>> + * Calculate partial hash values for i_key_pad and o_key_pad.
>> + * Will be used as initialization state for final HMAC.
>> + */
>> + error = sha256_block_partial(NULL, sess->auth.hmac.i_key_pad,
>> + partial, SHA256_BLOCK_SIZE);
>> + if (error != 0)
>> + return -1;
>> + memcpy(sess->auth.hmac.i_key_pad, partial, SHA256_BLOCK_SIZE);
>> +
>> + error = sha256_block_partial(NULL, sess->auth.hmac.o_key_pad,
>> + partial, SHA256_BLOCK_SIZE);
>> + if (error != 0)
>> + return -1;
>> + memcpy(sess->auth.hmac.o_key_pad, partial, SHA256_BLOCK_SIZE);
>> +
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static inline int
>> +cipher_set_prerequisites(struct armv8_crypto_session *sess,
>> + const struct rte_crypto_sym_xform *xform)
>> +{
>> + crypto_key_sched_t cipher_key_sched;
>> +
>> + cipher_key_sched = sess->cipher.key_sched;
>> + if (likely(cipher_key_sched != NULL)) {
>> + /* Set up cipher session key */
>> + cipher_key_sched(sess->cipher.key.data, xform->cipher.key.data);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +armv8_crypto_set_session_chained_parameters(struct armv8_crypto_session *sess,
>> + const struct rte_crypto_sym_xform *cipher_xform,
>> + const struct rte_crypto_sym_xform *auth_xform)
>> +{
>> + enum armv8_crypto_chain_order order;
>> + enum armv8_crypto_cipher_operation cop;
>> + enum rte_crypto_cipher_algorithm calg;
>> + enum rte_crypto_auth_algorithm aalg;
>> +
>> + /* Validate and prepare scratch order of combined operations */
>> + switch (sess->chain_order) {
>> + case ARMV8_CRYPTO_CHAIN_CIPHER_AUTH:
>> + case ARMV8_CRYPTO_CHAIN_AUTH_CIPHER:
>> + order = sess->chain_order;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + /* Select cipher direction */
>> + sess->cipher.direction = cipher_xform->cipher.op;
>> + /* Select cipher key */
>> + sess->cipher.key.length = cipher_xform->cipher.key.length;
>> + /* Set cipher direction */
>> + cop = sess->cipher.direction;
>> + /* Set cipher algorithm */
>> + calg = cipher_xform->cipher.algo;
>> +
>> + /* Select cipher algo */
>> + switch (calg) {
>> + /* Cover supported cipher algorithms */
>> + case RTE_CRYPTO_CIPHER_AES_CBC:
>> + sess->cipher.algo = calg;
>> + /* IV len is always 16 bytes (block size) for AES CBC */
>> + sess->cipher.iv_len = 16;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + /* Select auth generate/verify */
>> + sess->auth.operation = auth_xform->auth.op;
>> +
>> + /* Select auth algo */
>> + switch (auth_xform->auth.algo) {
>> + /* Cover supported hash algorithms */
>> + case RTE_CRYPTO_AUTH_SHA256:
>> + aalg = auth_xform->auth.algo;
>> + sess->auth.mode = ARMV8_CRYPTO_AUTH_AS_AUTH;
>> + break;
>> + case RTE_CRYPTO_AUTH_SHA1_HMAC:
>> + case RTE_CRYPTO_AUTH_SHA256_HMAC: /* Fall through */
>> + aalg = auth_xform->auth.algo;
>> + sess->auth.mode = ARMV8_CRYPTO_AUTH_AS_HMAC;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + /* Verify supported key lengths and extract proper algorithm */
>> + switch (cipher_xform->cipher.key.length << 3) {
>> + case 128:
>> + sess->crypto_func =
>> + CRYPTO_GET_ALGO(order, cop, calg, aalg, 128);
>> + sess->cipher.key_sched =
>> + CRYPTO_GET_KEY_SCHED(cop, calg, 128);
>> + break;
>> + case 192:
>> + sess->crypto_func =
>> + CRYPTO_GET_ALGO(order, cop, calg, aalg, 192);
>> + sess->cipher.key_sched =
>> + CRYPTO_GET_KEY_SCHED(cop, calg, 192);
>> + break;
>> + case 256:
>> + sess->crypto_func =
>> + CRYPTO_GET_ALGO(order, cop, calg, aalg, 256);
>> + sess->cipher.key_sched =
>> + CRYPTO_GET_KEY_SCHED(cop, calg, 256);
>> + break;
>> + default:
>> + sess->crypto_func = NULL;
>> + sess->cipher.key_sched = NULL;
>> + return -EINVAL;
>> + }
>> +
>> + if (unlikely(sess->crypto_func == NULL)) {
>> + /*
>> + * If we got here that means that there must be a bug
>
> Since AES-128-CBC is only supported in your patch. It means that
> crypto_func could be NULL according to the switch above if
> cipher.key.length > 128?
Yes. Instead of checking for key lengths in a similar way that we check
for algorithms, etc. we just fail when we don't find appropriate
function. Do you suggest that this should be changed?
>
>> + * in the algorithms selection above. Nevertheless keep
>> + * it here to catch bug immediately and avoid NULL pointer
>> + * dereference in OPs processing.
>> + */
>> + ARMV8_CRYPTO_LOG_ERR(
>> + "No appropriate crypto function for given parameters");
>> + return -EINVAL;
>> + }
>> +
>> + /* Set up cipher session prerequisites */
>> + if (cipher_set_prerequisites(sess, cipher_xform) != 0)
>> + return -EINVAL;
>> +
>> + /* Set up authentication session prerequisites */
>> + if (auth_set_prerequisites(sess, auth_xform) != 0)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>
> ....
>
>> diff --git a/drivers/crypto/armv8/rte_armv8_pmd_ops.c b/drivers/crypto/armv8/rte_armv8_pmd_ops.c
>> new file mode 100644
>> index 0000000..2bf6475
>> --- /dev/null
>> +++ b/drivers/crypto/armv8/rte_armv8_pmd_ops.c
>> @@ -0,0 +1,369 @@
>> +/*
>> + * BSD LICENSE
>> + *
>> + * Copyright (C) Cavium networks Ltd. 2017.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <string.h>
>> +
>> +#include <rte_common.h>
>> +#include <rte_malloc.h>
>> +#include <rte_cryptodev_pmd.h>
>> +
>> +#include "armv8_crypto_defs.h"
>> +
>> +#include "rte_armv8_pmd_private.h"
>> +
>> +static const struct rte_cryptodev_capabilities
>> + armv8_crypto_pmd_capabilities[] = {
>> + { /* SHA1 HMAC */
>> + .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>> + {.sym = {
>> + .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
>> + {.auth = {
>> + .algo = RTE_CRYPTO_AUTH_SHA1_HMAC,
>> + .block_size = 64,
>> + .key_size = {
>> + .min = 16,
>> + .max = 128,
>> + .increment = 0
>> + },
>> + .digest_size = {
>> + .min = 20,
>> + .max = 20,
>> + .increment = 0
>> + },
>> + .aad_size = { 0 }
>> + }, }
>> + }, }
>> + },
>> + { /* SHA256 HMAC */
>> + .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>> + {.sym = {
>> + .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
>> + {.auth = {
>> + .algo = RTE_CRYPTO_AUTH_SHA256_HMAC,
>> + .block_size = 64,
>> + .key_size = {
>> + .min = 16,
>> + .max = 128,
>> + .increment = 0
>> + },
>> + .digest_size = {
>> + .min = 32,
>> + .max = 32,
>> + .increment = 0
>> + },
>> + .aad_size = { 0 }
>> + }, }
>> + }, }
>> + },
>> + { /* AES CBC */
>> + .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>> + {.sym = {
>> + .xform_type = RTE_CRYPTO_SYM_XFORM_CIPHER,
>> + {.cipher = {
>> + .algo = RTE_CRYPTO_CIPHER_AES_CBC,
>> + .block_size = 16,
>> + .key_size = {
>> + .min = 16,
>> + .max = 16,
>> + .increment = 0
>> + },
>> + .iv_size = {
>> + .min = 16,
>> + .max = 16,
>> + .increment = 0
>> + }
>> + }, }
>> + }, }
>> + },
>> +
>
> It's strange that you defined aes and hmac here, but not implemented
> them, though their combinations are implemented.
> Will you add later?
We may add standalone algorithms in the future but those ops here are
not for that purpose. I thought that since there is no chained
operations capability we should export what we can do even though that
it will work (mean not return error) only if the operations are chained.
Do you have some other suggestion?
>
>> + RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
>> +};
>> +
>> +
^ permalink raw reply
* Re: [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI backplane
From: Dai, Wei @ 2017-01-12 13:17 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Zhang, Helin, Ananyev, Konstantin
In-Reply-To: <d1379ab4-d9a9-ba94-c6bb-aa66112c0652@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 11:38 PM
> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI
> backplane
>
> On 1/10/2017 3:45 PM, Wei Dai wrote:
> > This patch adds initial support for a Braodwell-DE XFI backplane
> > interface. The XFI backplane requires a custom tuned link.
> > Hardware/Firmware owns the link config for XF backplane and software
> > must not interfere with it.
>
> Does it make sense to announce this support in release notes?
This is just an initial support and need to be further completed and optimized,
So I don't think it is suitable to announce in this release note.
Maybe we can do that in next release 17.05 .
>
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> <...>
^ permalink raw reply
* Re: [PATCH v6 1/4] lib: add information metrics library
From: Thomas Monjalon @ 2017-01-12 13:22 UTC (permalink / raw)
To: Remy Horton; +Cc: dev
In-Reply-To: <1484150594-3758-2-git-send-email-remy.horton@intel.com>
2017-01-12 00:03, Remy Horton:
> This patch adds a new information metric library that allows other
> modules to register named metrics and update their values. It is
> intended to be independent of ethdev, rather than mixing ethdev
> and non-ethdev information in xstats.
[...]
> --- a/doc/api/doxy-api.conf
> +++ b/doc/api/doxy-api.conf
> @@ -58,6 +58,7 @@ INPUT = doc/api/doxy-api-index.md \
> lib/librte_reorder \
> lib/librte_ring \
> lib/librte_sched \
> + lib/librte_metrics \
> lib/librte_table \
> lib/librte_timer \
> lib/librte_vhost
It is not in the right order.
Tip: when you add an item to a list, you should ask yourself what is the
order. There are 3 types of order for the lists in DPDK:
- chronological (add at the end)
- alphabetical
- logical/semantic
The game is to find the right one :)
[...]
> @@ -171,6 +177,7 @@ The libraries prepended with a plus sign were incremented in this version.
> librte_mbuf.so.2
> librte_mempool.so.2
> librte_meter.so.1
> + + librte_metrics.so.1
> librte_net.so.1
> librte_pdump.so.1
> librte_pipeline.so.3
Right order here ;)
[...]
> --- /dev/null
> +++ b/lib/librte_metrics/rte_metrics.h
> +/** Used to indicate port-independent information */
> +#define RTE_METRICS_NONPORT -1
I do not understand this constant.
Why using the word "port" to name any device?
What means independent?
> +/**
> + * Metric name
> + */
> +struct rte_metric_name {
> + /** String describing metric */
> + char name[RTE_METRICS_MAX_NAME_LEN];
> +};
Why a struct for a simple string?
> +/**
> + * Metric name.
Copy/paste typo?
> + */
> +struct rte_metric_value {
> + /** Numeric identifier of metric */
> + uint16_t key;
How the key is bound to the name?
Remember how the xstats comments were improved:
http://dpdk.org/commit/6d52d1d
> + /** Value for metric */
> + uint64_t value;
> +};
> +
> +
> +/**
> + * Initializes metric module. This only has to be explicitly called if you
> + * intend to use rte_metrics_reg_metric() or rte_metrics_reg_metrics() from a
> + * secondary process. This function must be called from a primary process.
> + */
> +void rte_metrics_init(void);
> +
> +
> +/**
> + * Register a metric
You need to explain what is implied in registering.
I have the same comment for registering a set of metrics.
[...]
> +int rte_metrics_reg_metric(const char *name);
> +
> +/**
> + * Register a set of metrics
[...]
> +int rte_metrics_reg_metrics(const char **names, uint16_t cnt_names);
> +
> +/**
> + * Get metric name-key lookup table.
> + *
> + * @param names
> + * Array of names to receive key names
> + *
> + * @param capacity
> + * Space available in names
What happens if there is not enough space?
> + * @return
> + * - Non-negative: Success (number of names)
> + * - Negative: Failure
> + */
> +int rte_metrics_get_names(
> + struct rte_metric_name *names,
> + uint16_t capacity);
^ permalink raw reply
* Re: fm10k pmd limitations
From: Chen, Jing D @ 2017-01-12 13:30 UTC (permalink / raw)
To: Yigit, Ferruh, Shaham Fridenberg, dev@dpdk.org
In-Reply-To: <c0c1f059-5a7a-de91-7f9a-fc085503e639@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, January 12, 2017 7:57 PM
> To: Shaham Fridenberg <ShahamF@Radware.com>; dev@dpdk.org
> Cc: Chen, Jing D <jing.d.chen@intel.com>
> Subject: Re: [dpdk-dev] fm10k pmd limitations
>
> On 12/13/2016 1:49 PM, Shaham Fridenberg wrote:
> > Hey guys,
> >
> > I'm using dpdk 16.4 and fm10k card. According to the code, there's no
> support for disabling vlan stripping and VLAN QinQ in pmd fm10k.
> > Does anybody know why? If there's any way to work-around it, or when is a
> behavior change expected?
Yes, HW will always strip it and driver will copy it into
(struct rte_mbuf *)mbuf->vlan_tci.
> > I need my VF to receive the packets with the VLAN headers.
VF can read from rte_mbuf.
> > Even if it's possible to change this configurations through the linux kernel
> fm10k driver?
> >
> > Thanks!
> >
>
> CC fm10k maintainer: Jing Chen <jing.d.chen@intel.com>
^ permalink raw reply
* Re: [PATCH v6 3/4] app/test-pmd: add support for bitrate statistics
From: Thomas Monjalon @ 2017-01-12 13:32 UTC (permalink / raw)
To: Remy Horton; +Cc: dev
In-Reply-To: <1484150594-3758-4-git-send-email-remy.horton@intel.com>
I do not understand clearly this library and the output from testpmd.
It seems you have a need but you do not explain why it is not done in
the application.
Bitrate is specific to ethdev, right?
Why not put it directly in testpmd first?
^ permalink raw reply
* Re: [PATCH v3 25/29] net/nfp: use eal I/O device memory read/write API
From: Jerin Jacob @ 2017-01-12 13:40 UTC (permalink / raw)
To: Alejandro Lucero
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
jianbo.liu, Jan Viktorin, santosh.shukla
In-Reply-To: <CAD+H992WfM6goFehpOuGRkN=ANXLYrDF9FMGS23MZACzFWG8TQ@mail.gmail.com>
On Thu, Jan 12, 2017 at 10:53:17AM +0000, Alejandro Lucero wrote:
> Hi,
>
> I've tried to find out which dpdk repo should I use for testing this change
> with NFP PMD.
>
> It seems rte_read/write functions are not with last dpdk main repo, nor
> with dpdk-net-next.
>
> Can someone tell me which repo should I use?
It is based on the dpdk main repo. This patchset has 29 patches. The initial
patches in the series contains the definition of rte_read/write.
I have created a temporary branch in public repo to make other users to
test the changes without applying all patches.
https://github.com/jerinjacobk/dpdk.git
branch: rte_io_v3
It is based on today's dpdk master and this patchset.
^ 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