[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Rahul, Please see below. On Thu, Jul 21, 2016 at 01:43:37PM +0530, Rahul Lakkireddy wrote: > Hi Adrien, > > The proposal looks very good. It satisfies most of the features > supported by Chelsio NICs. We are looking for suggestions on exposing > more additional features supported by Chelsio NICs via this API. > > Chelsio NICs have two regions in which filters can be placed - > Maskfull and Maskless regions. As their names imply, maskfull region > can accept masks to match a range of values; whereas, maskless region > don't accept any masks and hence perform a more strict exact-matches. > Filters without masks can also be placed in maskfull region. By > default, maskless region have higher priority over the maskfull region. > However, the priority between the two regions is configurable. I understand this configuration affects the entire device. Just to be clear, assuming some filters are already configured, are they affected by a change of region priority later? > Please suggest on how we can let the apps configure in which region > filters must be placed and set the corresponding priority accordingly > via this API. Okay. Applications, like customers, are always right. With this in mind, PMDs are not allowed to break existing flow rules, and face two options when applications provide a flow specification that would break an existing rule: - Refuse to create it (easiest). - Find a workaround to apply it anyway (possibly quite complicated). The reason you have these two regions is performance right? Otherwise I'd just say put everything in the maskfull region. PMDs are allowed to rearrange existing rules or change device parameters as long as existing constraints are satisfied. In my opinion it does not matter which region has the highest default priority. Applications always want the best performance so the first created rule should be in the most appropriate region. If a subsequent rule requires it to be in the other region but the application specified the wrong priority for this to work, then the PMD can either choose to swap region priorities on the device (assuming it does not affect other rules), or destroy and recreate the original rule in a way that satisfies all constraints (i.e. moving conflicting rules from the maskless region to the maskfull one). Going further, when subsequent rules get destroyed the PMD should ideally move back maskfull rules back into the maskless region for better performance. This is only a suggestion. PMDs have the right to say "no" at some point. More important in my opinion is to make sure applications can create a given set of flow rules in any order. If rules a/b/c can be created, then it won't make sense from an application point of view if c/a/b for some reason cannot and the PMD maintainers will rightfully get a bug report. > More comments below. > > On Tuesday, July 07/05/16, 2016 at 20:16:46 +0200, Adrien Mazarguil wrote: > > Hi All, > > > [...] > > > > > ``ETH`` > > ^^^ > > > > Matches an Ethernet header. > > > > - ``dst``: destination MAC. > > - ``src``: source MAC. > > - ``type``: EtherType. > > - ``tags``: number of 802.1Q/ad tags defined. > > - ``tag[]``: 802.1Q/ad tag definitions, innermost first. For each one: > > > > - ``tpid``: Tag protocol identifier. > > - ``tci``: Tag control information. > > > > ``IPV4`` > > > > > > Matches an IPv4 header. > > > > - ``src``: source IP address. > > - ``dst``: destination IP address. > > - ``tos``: ToS/DSCP field. > > - ``ttl``: TTL field. > > - ``proto``: protocol number for the next layer. > > > > ``IPV6`` > > > > > > Matches an IPv6 header. > > > > - ``src``: source IP address. > > - ``dst``: destination IP address. > > - ``tc``: traffic class field. > > - ``nh``: Next header field (protocol). > > - ``hop_limit``: hop limit field (TTL). > > > > ``ICMP`` > > > > > > Matches an ICMP header. > > > > - TBD. > > > > ``UDP`` > > ^^^ > > > > Matches a UDP header. > > > > - ``sport``: source port. > > - ``dport``: destination port. > > - ``length``: UDP length. > > - ``checksum``: UDP checksum. > > > > .. raw:: pdf > > > >PageBreak > > > > ``TCP`` > > ^^^ > > > > Matches a TCP header. > > > > - ``sport``: source port. > > - ``dport``: destination port. > > - All other TCP fields and bits. > > > > ``VXLAN`` > > ^ > > > > Matches a VXLAN header. > > > > - TBD. > > > > In addition to above matches, Chelsio NICs have some additional > features: > > - Match based on unicast DST-MAC, multicast DST-MAC, broadcast DST-MAC. > Also, there is a match criteria available called 'promisc' - which > matches packets that are not destined for the interface, but had > been received by the hardware due to interface being in promiscuous > mode. There is no unicast/multicast/broadcast distinction in the ETH pattern item, those are covered by the specified MAC address. Now about this "promisc" match criteria, it can be added as a
[dpdk-dev] [RFC 00/21] pcap pmd improvements
No new feature added, code refactored. This patch targetted for 16.11 release. Ferruh Yigit (21): net/pcap: create own configuration parameter net/pcap: use macros for param string net/pcap: reorganize private structs net/pcap: add checks for max queue number net/pcap: update function to reuse it net/pcap: don't carry numa_node argument net/pcap: don't carry kvlist argument net/pcap: move comment to correct place net/pcap: remove duplicated max queue number check net/pcap: use single_iface variable instead of hardcoded net/pcap: group stats related fields into a struct net/pcap: make const array static net/pcap: reorder header files net/pcap: reorder functions net/pcap: update how single iface handled net/pcap: remove unnecessary check net/pcap: remove redundant assignment net/pcap: simplify function net/pcap: fix missing tx iface assignment net/pcap: coding convention updates net/pcap: remove rte prefix from static functions config/common_base | 1 + drivers/net/pcap/rte_eth_pcap.c | 590 +++- 2 files changed, 280 insertions(+), 311 deletions(-) -- 2.7.4
[dpdk-dev] [RFC 01/21] net/pcap: create own configuration parameter
pcap pmd is using ring pmd configuration parameters to set max number of
queues. This creates an unnecessary dependency and confusion.
Create a new config parameter for pcap PMD:
CONFIG_RTE_PMD_PCAP_MAX_QUEUES
Default value of config is same as ring parameter default.
pcap pmd doesn't need to be configured in a detail to set rx and tx max
queue numbers separately, so using same config item for both queues.
Signed-off-by: Ferruh Yigit
---
config/common_base | 1 +
drivers/net/pcap/rte_eth_pcap.c | 18 +-
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/config/common_base b/config/common_base
index 7830535..e32bd61 100644
--- a/config/common_base
+++ b/config/common_base
@@ -307,6 +307,7 @@ CONFIG_RTE_PMD_RING_MAX_TX_RINGS=16
# Compile software PMD backed by PCAP files
#
CONFIG_RTE_LIBRTE_PMD_PCAP=n
+CONFIG_RTE_PMD_PCAP_MAX_QUEUES=16
#
# Compile link bonding PMD library
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 7e213eb..6343c0e 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -87,22 +87,22 @@ struct pcap_tx_queue {
struct rx_pcaps {
unsigned num_of_rx;
- pcap_t *pcaps[RTE_PMD_RING_MAX_RX_RINGS];
- const char *names[RTE_PMD_RING_MAX_RX_RINGS];
- const char *types[RTE_PMD_RING_MAX_RX_RINGS];
+ pcap_t *pcaps[RTE_PMD_PCAP_MAX_QUEUES];
+ const char *names[RTE_PMD_PCAP_MAX_QUEUES];
+ const char *types[RTE_PMD_PCAP_MAX_QUEUES];
};
struct tx_pcaps {
unsigned num_of_tx;
- pcap_dumper_t *dumpers[RTE_PMD_RING_MAX_TX_RINGS];
- pcap_t *pcaps[RTE_PMD_RING_MAX_RX_RINGS];
- const char *names[RTE_PMD_RING_MAX_RX_RINGS];
- const char *types[RTE_PMD_RING_MAX_RX_RINGS];
+ pcap_dumper_t *dumpers[RTE_PMD_PCAP_MAX_QUEUES];
+ pcap_t *pcaps[RTE_PMD_PCAP_MAX_QUEUES];
+ const char *names[RTE_PMD_PCAP_MAX_QUEUES];
+ const char *types[RTE_PMD_PCAP_MAX_QUEUES];
};
struct pmd_internals {
- struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS];
- struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS];
+ struct pcap_rx_queue rx_queue[RTE_PMD_PCAP_MAX_QUEUES];
+ struct pcap_tx_queue tx_queue[RTE_PMD_PCAP_MAX_QUEUES];
int if_index;
int single_iface;
};
--
2.7.4
[dpdk-dev] [RFC 02/21] net/pcap: use macros for param string
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6343c0e..877e4c2 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1091,8 +1091,8 @@ static struct rte_driver pmd_pcap_drv = {
PMD_REGISTER_DRIVER(pmd_pcap_drv, eth_pcap);
DRIVER_REGISTER_PARAM_STRING(eth_pcap,
- "rx_pcap= "
- "tx_pcap= "
- "rx_iface= "
- "tx_iface= "
- "iface=");
+ ETH_PCAP_RX_PCAP_ARG"= "
+ ETH_PCAP_TX_PCAP_ARG"= "
+ ETH_PCAP_RX_IFACE_ARG"= "
+ ETH_PCAP_TX_IFACE_ARG"= "
+ ETH_PCAP_IFACE_ARG"=");
--
2.7.4
[dpdk-dev] [RFC 03/21] net/pcap: reorganize private structs
struct rx_pcaps and tx_pcaps used to point parsed devargs, but it is not
clear with current names.
Merged both into single struct and modified struct name and field names.
Functionality not changed, only struct names.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 123 +++-
1 file changed, 59 insertions(+), 64 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 877e4c2..34c779a 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -85,21 +85,6 @@ struct pcap_tx_queue {
char type[ETH_PCAP_ARG_MAXLEN];
};
-struct rx_pcaps {
- unsigned num_of_rx;
- pcap_t *pcaps[RTE_PMD_PCAP_MAX_QUEUES];
- const char *names[RTE_PMD_PCAP_MAX_QUEUES];
- const char *types[RTE_PMD_PCAP_MAX_QUEUES];
-};
-
-struct tx_pcaps {
- unsigned num_of_tx;
- pcap_dumper_t *dumpers[RTE_PMD_PCAP_MAX_QUEUES];
- pcap_t *pcaps[RTE_PMD_PCAP_MAX_QUEUES];
- const char *names[RTE_PMD_PCAP_MAX_QUEUES];
- const char *types[RTE_PMD_PCAP_MAX_QUEUES];
-};
-
struct pmd_internals {
struct pcap_rx_queue rx_queue[RTE_PMD_PCAP_MAX_QUEUES];
struct pcap_tx_queue tx_queue[RTE_PMD_PCAP_MAX_QUEUES];
@@ -107,6 +92,16 @@ struct pmd_internals {
int single_iface;
};
+struct pmd_devargs {
+ unsigned num_of_queue;
+ struct devargs_queue {
+ pcap_dumper_t *dumper;
+ pcap_t *pcap;
+ const char *name;
+ const char *type;
+ } queue[RTE_PMD_PCAP_MAX_QUEUES];
+};
+
const char *valid_arguments[] = {
ETH_PCAP_RX_PCAP_ARG,
ETH_PCAP_TX_PCAP_ARG,
@@ -625,16 +620,16 @@ open_rx_pcap(const char *key, const char *value, void
*extra_args)
{
unsigned i;
const char *pcap_filename = value;
- struct rx_pcaps *pcaps = extra_args;
+ struct pmd_devargs *rx = extra_args;
pcap_t *pcap = NULL;
- for (i = 0; i < pcaps->num_of_rx; i++) {
+ for (i = 0; i < rx->num_of_queue; i++) {
if (open_single_rx_pcap(pcap_filename, &pcap) < 0)
return -1;
- pcaps->pcaps[i] = pcap;
- pcaps->names[i] = pcap_filename;
- pcaps->types[i] = key;
+ rx->queue[i].pcap = pcap;
+ rx->queue[i].name = pcap_filename;
+ rx->queue[i].type = key;
}
return 0;
@@ -659,16 +654,16 @@ open_tx_pcap(const char *key, const char *value, void
*extra_args)
{
unsigned i;
const char *pcap_filename = value;
- struct tx_pcaps *dumpers = extra_args;
+ struct pmd_devargs *dumpers = extra_args;
pcap_dumper_t *dumper;
- for (i = 0; i < dumpers->num_of_tx; i++) {
+ for (i = 0; i < dumpers->num_of_queue; i++) {
if (open_single_tx_pcap(pcap_filename, &dumper) < 0)
return -1;
- dumpers->dumpers[i] = dumper;
- dumpers->names[i] = pcap_filename;
- dumpers->types[i] = key;
+ dumpers->queue[i].dumper = dumper;
+ dumpers->queue[i].name = pcap_filename;
+ dumpers->queue[i].type = key;
}
return 0;
@@ -721,15 +716,15 @@ static inline int
open_rx_tx_iface(const char *key, const char *value, void *extra_args)
{
const char *iface = value;
- struct rx_pcaps *pcaps = extra_args;
+ struct pmd_devargs *tx = extra_args;
pcap_t *pcap = NULL;
if (open_single_iface(iface, &pcap) < 0)
return -1;
- pcaps->pcaps[0] = pcap;
- pcaps->names[0] = iface;
- pcaps->types[0] = key;
+ tx->queue[0].pcap = pcap;
+ tx->queue[0].name = iface;
+ tx->queue[0].type = key;
return 0;
}
@@ -742,15 +737,15 @@ open_rx_iface(const char *key, const char *value, void
*extra_args)
{
unsigned i;
const char *iface = value;
- struct rx_pcaps *pcaps = extra_args;
+ struct pmd_devargs *rx = extra_args;
pcap_t *pcap = NULL;
- for (i = 0; i < pcaps->num_of_rx; i++) {
+ for (i = 0; i < rx->num_of_queue; i++) {
if (open_single_iface(iface, &pcap) < 0)
return -1;
- pcaps->pcaps[i] = pcap;
- pcaps->names[i] = iface;
- pcaps->types[i] = key;
+ rx->queue[i].pcap = pcap;
+ rx->queue[i].name = iface;
+ rx->queue[i].type = key;
}
return 0;
@@ -764,15 +759,15 @@ open_tx_iface(const char *key, const char *value, void
*extra_args)
{
unsigned i;
const char *iface = value;
- struct tx_pcaps *pcaps = extra_args;
+ struct pmd_devargs *tx = extra_args;
pcap_t *pcap;
- for (i = 0; i < pcaps->num_of_tx; i++) {
+ for (i = 0; i < tx->num_of_queue; i++) {
if (open_single_iface(if
[dpdk-dev] [RFC 06/21] net/pcap: don't carry numa_node argument
Instead of defining numa_node variable upper level of call stack and
carry into sub function, set it where needs to be used.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 40 +---
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 14f770c..94d7c88 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -786,13 +786,11 @@ open_single_iface(const char *iface, pcap_t **pcap)
static int
rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
- const unsigned nb_tx_queues,
- const unsigned numa_node,
- struct pmd_internals **internals,
- struct rte_eth_dev **eth_dev,
- struct rte_kvargs *kvlist)
+ const unsigned nb_tx_queues, struct pmd_internals **internals,
+ struct rte_eth_dev **eth_dev, struct rte_kvargs *kvlist)
{
struct rte_eth_dev_data *data = NULL;
+ unsigned int numa_node = rte_socket_id();
unsigned k_idx;
struct rte_kvargs_pair *pair = NULL;
@@ -802,8 +800,8 @@ rte_pmd_init_internals(const char *name, const unsigned
nb_rx_queues,
break;
}
- RTE_LOG(INFO, PMD,
- "Creating pcap-backed ethdev on numa socket %u\n",
numa_node);
+ RTE_LOG(INFO, PMD, "Creating pcap-backed ethdev on numa socket %u\n",
+ numa_node);
/* now do all data allocation - for eth_dev structure
* and internal (private) data
@@ -812,7 +810,8 @@ rte_pmd_init_internals(const char *name, const unsigned
nb_rx_queues,
if (data == NULL)
goto error;
- *internals = rte_zmalloc_socket(name, sizeof(**internals), 0,
numa_node);
+ *internals = rte_zmalloc_socket(name, sizeof(**internals), 0,
+ numa_node);
if (*internals == NULL)
goto error;
@@ -869,9 +868,8 @@ error:
static int
rte_eth_from_pcaps_common(const char *name, struct pmd_devargs *rx_queues,
const unsigned nb_rx_queues, struct pmd_devargs *tx_queues,
- const unsigned nb_tx_queues, const unsigned numa_node,
- struct rte_kvargs *kvlist, struct pmd_internals **internals,
- struct rte_eth_dev **eth_dev)
+ const unsigned nb_tx_queues, struct rte_kvargs *kvlist,
+ struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
{
unsigned i;
@@ -881,8 +879,8 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
if (tx_queues == NULL && nb_tx_queues > 0)
return -1;
- if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, numa_node,
- internals, eth_dev, kvlist) < 0)
+ if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, internals,
+ eth_dev, kvlist) < 0)
return -1;
for (i = 0; i < nb_rx_queues; i++) {
@@ -910,17 +908,15 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
static int
rte_eth_from_pcaps(const char *name, struct pmd_devargs *rx_queues,
const unsigned nb_rx_queues, struct pmd_devargs *tx_queues,
- const unsigned nb_tx_queues, const unsigned numa_node,
- struct rte_kvargs *kvlist, int single_iface,
- unsigned int using_dumpers)
+ const unsigned nb_tx_queues, struct rte_kvargs *kvlist,
+ int single_iface, unsigned int using_dumpers)
{
struct pmd_internals *internals = NULL;
struct rte_eth_dev *eth_dev = NULL;
int ret;
ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
- tx_queues, nb_tx_queues, numa_node, kvlist,
- &internals, ð_dev);
+ tx_queues, nb_tx_queues, kvlist, &internals, ð_dev);
if (ret < 0)
return ret;
@@ -942,7 +938,7 @@ rte_eth_from_pcaps(const char *name, struct pmd_devargs
*rx_queues,
static int
rte_pmd_pcap_devinit(const char *name, const char *params)
{
- unsigned numa_node, using_dumpers = 0;
+ unsigned using_dumpers = 0;
int ret;
struct rte_kvargs *kvlist;
struct pmd_devargs pcaps = {0};
@@ -950,8 +946,6 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
- numa_node = rte_socket_id();
-
gettimeofday(&start_time, NULL);
start_cycles = rte_get_timer_cycles();
hz = rte_get_timer_hz();
@@ -974,7 +968,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
dumpers.queue[0].name = pcaps.queue[0].name;
dumpers.queue[0].type = pcaps.queue[0].type;
ret = rte_eth_from_pcaps(name, &pcaps, 1, &dumpers
[dpdk-dev] [RFC 04/21] net/pcap: add checks for max queue number
Number of queues defined by devargs, a check added to be sure this
number is not bigger than configured queue length.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 34c779a..308b8bc 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1010,12 +1010,21 @@ rte_pmd_pcap_devinit(const char *name, const char
*params)
* We check whether we want to open a RX stream from a real NIC or a
* pcap file
*/
- if ((pcaps.num_of_queue = rte_kvargs_count(kvlist,
ETH_PCAP_RX_PCAP_ARG))) {
+ pcaps.num_of_queue = rte_kvargs_count(kvlist, ETH_PCAP_RX_PCAP_ARG);
+
+ if (pcaps.num_of_queue) {
+ if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+
ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
&open_rx_pcap, &pcaps);
} else {
pcaps.num_of_queue = rte_kvargs_count(kvlist,
ETH_PCAP_RX_IFACE_ARG);
+
+ if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+
ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
&open_rx_iface, &pcaps);
}
@@ -1027,14 +1036,22 @@ rte_pmd_pcap_devinit(const char *name, const char
*params)
* We check whether we want to open a TX stream to a real NIC or a
* pcap file
*/
- if ((dumpers.num_of_queue = rte_kvargs_count(kvlist,
- ETH_PCAP_TX_PCAP_ARG))) {
+ dumpers.num_of_queue = rte_kvargs_count(kvlist, ETH_PCAP_TX_PCAP_ARG);
+
+ if (dumpers.num_of_queue) {
+ if (dumpers.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ dumpers.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+
ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_PCAP_ARG,
&open_tx_pcap, &dumpers);
using_dumpers = 1;
} else {
dumpers.num_of_queue = rte_kvargs_count(kvlist,
ETH_PCAP_TX_IFACE_ARG);
+
+ if (dumpers.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ dumpers.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+
ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_IFACE_ARG,
&open_tx_iface, &dumpers);
}
--
2.7.4
[dpdk-dev] [RFC 07/21] net/pcap: don't carry kvlist argument
Don't carry kvlist argument into sub function and used it, use kvlist
argument in upper level of call stack.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 94d7c88..3eacb82 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -787,18 +787,10 @@ open_single_iface(const char *iface, pcap_t **pcap)
static int
rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
const unsigned nb_tx_queues, struct pmd_internals **internals,
- struct rte_eth_dev **eth_dev, struct rte_kvargs *kvlist)
+ struct rte_eth_dev **eth_dev)
{
struct rte_eth_dev_data *data = NULL;
unsigned int numa_node = rte_socket_id();
- unsigned k_idx;
- struct rte_kvargs_pair *pair = NULL;
-
- for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
- pair = &kvlist->pairs[k_idx];
- if (strstr(pair->key, ETH_PCAP_IFACE_ARG) != NULL)
- break;
- }
RTE_LOG(INFO, PMD, "Creating pcap-backed ethdev on numa socket %u\n",
numa_node);
@@ -833,11 +825,6 @@ rte_pmd_init_internals(const char *name, const unsigned
nb_rx_queues,
/* NOTE: we'll replace the data element, of originally allocated eth_dev
* so the rings are local per-process */
- if (pair == NULL)
- (*internals)->if_index = 0;
- else
- (*internals)->if_index = if_nametoindex(pair->value);
-
data->dev_private = *internals;
data->port_id = (*eth_dev)->data->port_id;
snprintf(data->name, sizeof(data->name), "%s", (*eth_dev)->data->name);
@@ -871,6 +858,8 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
const unsigned nb_tx_queues, struct rte_kvargs *kvlist,
struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
{
+ struct rte_kvargs_pair *pair = NULL;
+ unsigned k_idx;
unsigned i;
/* do some parameter checking */
@@ -880,7 +869,7 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
return -1;
if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, internals,
- eth_dev, kvlist) < 0)
+ eth_dev) < 0)
return -1;
for (i = 0; i < nb_rx_queues; i++) {
@@ -902,6 +891,17 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
tx_queues->queue[i].type);
}
+ for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
+ pair = &kvlist->pairs[k_idx];
+ if (strstr(pair->key, ETH_PCAP_IFACE_ARG) != NULL)
+ break;
+ }
+
+ if (pair == NULL)
+ (*internals)->if_index = 0;
+ else
+ (*internals)->if_index = if_nametoindex(pair->value);
+
return 0;
}
--
2.7.4
[dpdk-dev] [RFC 05/21] net/pcap: update function to reuse it
rte_eth_from_pcaps and rte_eth_from_pcaps_n_dumpers functions are very
close, updated rte_eth_from_pcaps function and reused.
No functional update.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 58 +
1 file changed, 13 insertions(+), 45 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 308b8bc..14f770c 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -908,43 +908,11 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
}
static int
-rte_eth_from_pcaps_n_dumpers(const char *name,
- struct pmd_devargs *rx_queues,
- const unsigned nb_rx_queues,
- struct pmd_devargs *tx_queues,
- const unsigned nb_tx_queues,
- const unsigned numa_node,
- struct rte_kvargs *kvlist)
-{
- struct pmd_internals *internals = NULL;
- struct rte_eth_dev *eth_dev = NULL;
- int ret;
-
- ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
- tx_queues, nb_tx_queues, numa_node, kvlist,
- &internals, ð_dev);
-
- if (ret < 0)
- return ret;
-
- /* using multiple pcaps/interfaces */
- internals->single_iface = 0;
-
- eth_dev->rx_pkt_burst = eth_pcap_rx;
- eth_dev->tx_pkt_burst = eth_pcap_tx_dumper;
-
- return 0;
-}
-
-static int
-rte_eth_from_pcaps(const char *name,
- struct pmd_devargs *rx_queues,
- const unsigned nb_rx_queues,
- struct pmd_devargs *tx_queues,
- const unsigned nb_tx_queues,
- const unsigned numa_node,
- struct rte_kvargs *kvlist,
- int single_iface)
+rte_eth_from_pcaps(const char *name, struct pmd_devargs *rx_queues,
+ const unsigned nb_rx_queues, struct pmd_devargs *tx_queues,
+ const unsigned nb_tx_queues, const unsigned numa_node,
+ struct rte_kvargs *kvlist, int single_iface,
+ unsigned int using_dumpers)
{
struct pmd_internals *internals = NULL;
struct rte_eth_dev *eth_dev = NULL;
@@ -961,7 +929,11 @@ rte_eth_from_pcaps(const char *name,
internals->single_iface = single_iface;
eth_dev->rx_pkt_burst = eth_pcap_rx;
- eth_dev->tx_pkt_burst = eth_pcap_tx;
+
+ if (using_dumpers)
+ eth_dev->tx_pkt_burst = eth_pcap_tx_dumper;
+ else
+ eth_dev->tx_pkt_burst = eth_pcap_tx;
return 0;
}
@@ -1002,7 +974,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
dumpers.queue[0].name = pcaps.queue[0].name;
dumpers.queue[0].type = pcaps.queue[0].type;
ret = rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1,
- numa_node, kvlist, 1);
+ numa_node, kvlist, 1, using_dumpers);
goto free_kvlist;
}
@@ -1059,12 +1031,8 @@ rte_pmd_pcap_devinit(const char *name, const char
*params)
if (ret < 0)
goto free_kvlist;
- if (using_dumpers)
- ret = rte_eth_from_pcaps_n_dumpers(name, &pcaps,
pcaps.num_of_queue,
- &dumpers, dumpers.num_of_queue, numa_node,
kvlist);
- else
- ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue,
&dumpers,
- dumpers.num_of_queue, numa_node, kvlist, 0);
+ ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers,
+ dumpers.num_of_queue, numa_node, kvlist, 0, using_dumpers);
free_kvlist:
rte_kvargs_free(kvlist);
--
2.7.4
[dpdk-dev] [RFC 08/21] net/pcap: move comment to correct place
Signed-off-by: Ferruh Yigit --- drivers/net/pcap/rte_eth_pcap.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index 3eacb82..7524293 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -822,9 +822,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues, * - point eth_dev_data to internals * - and point eth_dev structure to new eth_dev_data structure */ - /* NOTE: we'll replace the data element, of originally allocated eth_dev -* so the rings are local per-process */ - data->dev_private = *internals; data->port_id = (*eth_dev)->data->port_id; snprintf(data->name, sizeof(data->name), "%s", (*eth_dev)->data->name); @@ -835,6 +832,10 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues, strncpy(data->name, (*eth_dev)->data->name, strlen((*eth_dev)->data->name)); + /* +* NOTE: we'll replace the data element, of originally allocated +* eth_dev so the rings are local per-process +*/ (*eth_dev)->data = data; (*eth_dev)->dev_ops = &ops; (*eth_dev)->driver = NULL; -- 2.7.4
[dpdk-dev] [RFC 09/21] net/pcap: remove duplicated max queue number check
Remove duplicated check by reorganizing the code, no functional change.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 49 ++---
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 7524293..0445c74 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -939,11 +939,11 @@ rte_eth_from_pcaps(const char *name, struct pmd_devargs
*rx_queues,
static int
rte_pmd_pcap_devinit(const char *name, const char *params)
{
- unsigned using_dumpers = 0;
- int ret;
+ unsigned int is_rx_pcap = 0, is_tx_pcap = 0;
struct rte_kvargs *kvlist;
struct pmd_devargs pcaps = {0};
struct pmd_devargs dumpers = {0};
+ int ret;
RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
@@ -969,7 +969,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
dumpers.queue[0].name = pcaps.queue[0].name;
dumpers.queue[0].type = pcaps.queue[0].type;
ret = rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1,
- kvlist, 1, using_dumpers);
+ kvlist, 1, is_tx_pcap);
goto free_kvlist;
}
@@ -978,23 +978,21 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
* pcap file
*/
pcaps.num_of_queue = rte_kvargs_count(kvlist, ETH_PCAP_RX_PCAP_ARG);
-
- if (pcaps.num_of_queue) {
- if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
- pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
-
- ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
- &open_rx_pcap, &pcaps);
- } else {
+ if (pcaps.num_of_queue)
+ is_rx_pcap = 1;
+ else
pcaps.num_of_queue = rte_kvargs_count(kvlist,
ETH_PCAP_RX_IFACE_ARG);
- if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
- pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+ if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+ if (is_rx_pcap)
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
+ &open_rx_pcap, &pcaps);
+ else
ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
&open_rx_iface, &pcaps);
- }
if (ret < 0)
goto free_kvlist;
@@ -1004,30 +1002,27 @@ rte_pmd_pcap_devinit(const char *name, const char
*params)
* pcap file
*/
dumpers.num_of_queue = rte_kvargs_count(kvlist, ETH_PCAP_TX_PCAP_ARG);
-
- if (dumpers.num_of_queue) {
- if (dumpers.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
- dumpers.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
-
- ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_PCAP_ARG,
- &open_tx_pcap, &dumpers);
- using_dumpers = 1;
- } else {
+ if (dumpers.num_of_queue)
+ is_tx_pcap = 1;
+ else
dumpers.num_of_queue = rte_kvargs_count(kvlist,
ETH_PCAP_TX_IFACE_ARG);
- if (dumpers.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
- dumpers.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+ if (dumpers.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
+ dumpers.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
+ if (is_tx_pcap)
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_PCAP_ARG,
+ &open_tx_pcap, &dumpers);
+ else
ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_IFACE_ARG,
&open_tx_iface, &dumpers);
- }
if (ret < 0)
goto free_kvlist;
ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers,
- dumpers.num_of_queue, kvlist, 0, using_dumpers);
+ dumpers.num_of_queue, kvlist, 0, is_tx_pcap);
free_kvlist:
rte_kvargs_free(kvlist);
--
2.7.4
[dpdk-dev] [RFC 10/21] net/pcap: use single_iface variable instead of hardcoded
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 0445c74..31eed58 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -943,6 +943,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
struct rte_kvargs *kvlist;
struct pmd_devargs pcaps = {0};
struct pmd_devargs dumpers = {0};
+ int single_iface = 0;
int ret;
RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
@@ -963,13 +964,18 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
&open_rx_tx_iface, &pcaps);
+
if (ret < 0)
goto free_kvlist;
dumpers.queue[0].pcap = pcaps.queue[0].pcap;
dumpers.queue[0].name = pcaps.queue[0].name;
dumpers.queue[0].type = pcaps.queue[0].type;
+
+ single_iface = 1;
+
ret = rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1,
- kvlist, 1, is_tx_pcap);
+ kvlist, single_iface, is_tx_pcap);
+
goto free_kvlist;
}
@@ -1022,7 +1028,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
goto free_kvlist;
ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers,
- dumpers.num_of_queue, kvlist, 0, is_tx_pcap);
+ dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
free_kvlist:
rte_kvargs_free(kvlist);
--
2.7.4
[dpdk-dev] [RFC 11/21] net/pcap: group stats related fields into a struct
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 72 +
1 file changed, 37 insertions(+), 35 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 31eed58..ec25912 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -64,13 +64,17 @@ static struct timeval start_time;
static uint64_t start_cycles;
static uint64_t hz;
+struct queue_stat {
+ volatile unsigned long pkts;
+ volatile unsigned long bytes;
+ volatile unsigned long err_pkts;
+};
+
struct pcap_rx_queue {
pcap_t *pcap;
uint8_t in_port;
struct rte_mempool *mb_pool;
- volatile unsigned long rx_pkts;
- volatile unsigned long rx_bytes;
- volatile unsigned long err_pkts;
+ struct queue_stat rx_stat;
char name[PATH_MAX];
char type[ETH_PCAP_ARG_MAXLEN];
};
@@ -78,9 +82,7 @@ struct pcap_rx_queue {
struct pcap_tx_queue {
pcap_dumper_t *dumper;
pcap_t *pcap;
- volatile unsigned long tx_pkts;
- volatile unsigned long tx_bytes;
- volatile unsigned long err_pkts;
+ struct queue_stat tx_stat;
char name[PATH_MAX];
char type[ETH_PCAP_ARG_MAXLEN];
};
@@ -234,8 +236,8 @@ eth_pcap_rx(void *queue,
num_rx++;
rx_bytes += header.caplen;
}
- pcap_q->rx_pkts += num_rx;
- pcap_q->rx_bytes += rx_bytes;
+ pcap_q->rx_stat.pkts += num_rx;
+ pcap_q->rx_stat.bytes += rx_bytes;
return num_rx;
}
@@ -306,9 +308,9 @@ eth_pcap_tx_dumper(void *queue,
* we flush the pcap dumper within each burst.
*/
pcap_dump_flush(dumper_q->dumper);
- dumper_q->tx_pkts += num_tx;
- dumper_q->tx_bytes += tx_bytes;
- dumper_q->err_pkts += nb_pkts - num_tx;
+ dumper_q->tx_stat.pkts += num_tx;
+ dumper_q->tx_stat.bytes += tx_bytes;
+ dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
return num_tx;
}
@@ -362,9 +364,9 @@ eth_pcap_tx(void *queue,
rte_pktmbuf_free(mbuf);
}
- tx_queue->tx_pkts += num_tx;
- tx_queue->tx_bytes += tx_bytes;
- tx_queue->err_pkts += nb_pkts - num_tx;
+ tx_queue->tx_stat.pkts += num_tx;
+ tx_queue->tx_stat.bytes += tx_bytes;
+ tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
return num_tx;
}
@@ -501,7 +503,7 @@ eth_dev_info(struct rte_eth_dev *dev,
static void
eth_stats_get(struct rte_eth_dev *dev,
- struct rte_eth_stats *igb_stats)
+ struct rte_eth_stats *stats)
{
unsigned i;
unsigned long rx_packets_total = 0, rx_bytes_total = 0;
@@ -511,27 +513,27 @@ eth_stats_get(struct rte_eth_dev *dev,
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
i < dev->data->nb_rx_queues; i++) {
- igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
- igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
- rx_packets_total += igb_stats->q_ipackets[i];
- rx_bytes_total += igb_stats->q_ibytes[i];
+ stats->q_ipackets[i] = internal->rx_queue[i].rx_stat.pkts;
+ stats->q_ibytes[i] = internal->rx_queue[i].rx_stat.bytes;
+ rx_packets_total += stats->q_ipackets[i];
+ rx_bytes_total += stats->q_ibytes[i];
}
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
i < dev->data->nb_tx_queues; i++) {
- igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
- igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
- igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
- tx_packets_total += igb_stats->q_opackets[i];
- tx_bytes_total += igb_stats->q_obytes[i];
- tx_packets_err_total += igb_stats->q_errors[i];
+ stats->q_opackets[i] = internal->tx_queue[i].tx_stat.pkts;
+ stats->q_obytes[i] = internal->tx_queue[i].tx_stat.bytes;
+ stats->q_errors[i] = internal->tx_queue[i].tx_stat.err_pkts;
+ tx_packets_total += stats->q_opackets[i];
+ tx_bytes_total += stats->q_obytes[i];
+ tx_packets_err_total += stats->q_errors[i];
}
- igb_stats->ipackets = rx_packets_total;
- igb_stats->ibytes = rx_bytes_total;
- igb_stats->opackets = tx_packets_total;
- igb_stats->obytes = tx_bytes_total;
- igb_stats->oerrors = tx_packets_err_total;
+ stats->ipackets = rx_packets_total;
+ stats->ibytes = rx_bytes_total;
+ stats->opackets = tx_packets_total;
+ stats->obytes = tx_bytes_total;
+ stats->oerrors = tx_packets_err_total;
}
static void
@@ -540,13 +542,13 @@ eth_stats_reset(struct rte_eth_dev *dev)
unsigned i;
struct pmd_internals *internal = dev->data->dev_private;
[dpdk-dev] [RFC 12/21] net/pcap: make const array static
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index ec25912..3d85a7f 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -104,7 +104,7 @@ struct pmd_devargs {
} queue[RTE_PMD_PCAP_MAX_QUEUES];
};
-const char *valid_arguments[] = {
+static const char *valid_arguments[] = {
ETH_PCAP_RX_PCAP_ARG,
ETH_PCAP_TX_PCAP_ARG,
ETH_PCAP_RX_IFACE_ARG,
--
2.7.4
[dpdk-dev] [RFC 13/21] net/pcap: reorder header files
Remove unused and sort remaining. Signed-off-by: Ferruh Yigit --- drivers/net/pcap/rte_eth_pcap.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index 3d85a7f..a142e38 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -33,19 +33,18 @@ */ #include -#include -#include -#include -#include -#include -#include -#include -#include #include #include +#include +#include +#include +#include +#include +#include + #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535 #define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN #define RTE_ETH_PCAP_PROMISC 1 -- 2.7.4
[dpdk-dev] [RFC 14/21] net/pcap: reorder functions
Reorder functions to be able to remove function declarations in .c file.
Function definitions not modified.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 129
1 file changed, 64 insertions(+), 65 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index a142e38..8e011ea 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -112,10 +112,6 @@ static const char *valid_arguments[] = {
NULL
};
-static int open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t
**dumper);
-static int open_single_rx_pcap(const char *pcap_filename, pcap_t **pcap);
-static int open_single_iface(const char *iface, pcap_t **pcap);
-
static struct ether_addr eth_addr = { .addr_bytes = { 0, 0, 0, 0x1, 0x2, 0x3 }
};
static const char *drivername = "Pcap PMD";
static struct rte_eth_link pmd_link = {
@@ -369,6 +365,70 @@ eth_pcap_tx(void *queue,
return num_tx;
}
+/*
+ * pcap_open_live wrapper function
+ */
+static inline int
+open_iface_live(const char *iface, pcap_t **pcap) {
+ *pcap = pcap_open_live(iface, RTE_ETH_PCAP_SNAPLEN,
+ RTE_ETH_PCAP_PROMISC, RTE_ETH_PCAP_TIMEOUT, errbuf);
+
+ if (*pcap == NULL) {
+ RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", iface, errbuf);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+open_single_iface(const char *iface, pcap_t **pcap)
+{
+ if (open_iface_live(iface, pcap) < 0) {
+ RTE_LOG(ERR, PMD, "Couldn't open interface %s\n", iface);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
+{
+ pcap_t *tx_pcap;
+
+ /*
+* We need to create a dummy empty pcap_t to use it
+* with pcap_dump_open(). We create big enough an Ethernet
+* pcap holder.
+*/
+ if ((tx_pcap = pcap_open_dead(DLT_EN10MB, RTE_ETH_PCAP_SNAPSHOT_LEN))
+ == NULL) {
+ RTE_LOG(ERR, PMD, "Couldn't create dead pcap\n");
+ return -1;
+ }
+
+ /* The dumper is created using the previous pcap_t reference */
+ if ((*dumper = pcap_dump_open(tx_pcap, pcap_filename)) == NULL) {
+ RTE_LOG(ERR, PMD, "Couldn't open %s for writing.\n",
+ pcap_filename);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+open_single_rx_pcap(const char *pcap_filename, pcap_t **pcap)
+{
+ if ((*pcap = pcap_open_offline(pcap_filename, errbuf)) == NULL) {
+ RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", pcap_filename,
errbuf);
+ return -1;
+ }
+
+ return 0;
+}
+
static int
eth_dev_start(struct rte_eth_dev *dev)
{
@@ -636,16 +696,6 @@ open_rx_pcap(const char *key, const char *value, void
*extra_args)
return 0;
}
-static int
-open_single_rx_pcap(const char *pcap_filename, pcap_t **pcap)
-{
- if ((*pcap = pcap_open_offline(pcap_filename, errbuf)) == NULL) {
- RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", pcap_filename,
errbuf);
- return -1;
- }
- return 0;
-}
-
/*
* Opens a pcap file for writing and stores a reference to it
* for use it later on.
@@ -670,46 +720,6 @@ open_tx_pcap(const char *key, const char *value, void
*extra_args)
return 0;
}
-static int
-open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
-{
- pcap_t *tx_pcap;
- /*
-* We need to create a dummy empty pcap_t to use it
-* with pcap_dump_open(). We create big enough an Ethernet
-* pcap holder.
-*/
-
- if ((tx_pcap = pcap_open_dead(DLT_EN10MB, RTE_ETH_PCAP_SNAPSHOT_LEN))
- == NULL) {
- RTE_LOG(ERR, PMD, "Couldn't create dead pcap\n");
- return -1;
- }
-
- /* The dumper is created using the previous pcap_t reference */
- if ((*dumper = pcap_dump_open(tx_pcap, pcap_filename)) == NULL) {
- RTE_LOG(ERR, PMD, "Couldn't open %s for writing.\n",
pcap_filename);
- return -1;
- }
-
- return 0;
-}
-
-/*
- * pcap_open_live wrapper function
- */
-static inline int
-open_iface_live(const char *iface, pcap_t **pcap) {
- *pcap = pcap_open_live(iface, RTE_ETH_PCAP_SNAPLEN,
- RTE_ETH_PCAP_PROMISC, RTE_ETH_PCAP_TIMEOUT, errbuf);
-
- if (*pcap == NULL) {
- RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", iface, errbuf);
- return -1;
- }
- return 0;
-}
-
/*
* Opens an interface for reading and writing
*/
@@ -775,17 +785,6 @@ open_tx_iface(const char *key, const char *value, void
*extra_args)
}
static int
-open_single_iface(const char *iface, pcap_t **pcap)
-{
- if (open_iface_live(iface, pcap) < 0) {
- RTE_LOG(ERR, PMD, "Could
[dpdk-dev] [RFC 15/21] net/pcap: update how single iface handled
Remove hardcoded single interface values, make it more obvious. Signed-off-by: Ferruh Yigit --- drivers/net/pcap/rte_eth_pcap.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index 8e011ea..fb170db 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -967,16 +967,14 @@ rte_pmd_pcap_devinit(const char *name, const char *params) if (ret < 0) goto free_kvlist; - dumpers.queue[0].pcap = pcaps.queue[0].pcap; - dumpers.queue[0].name = pcaps.queue[0].name; - dumpers.queue[0].type = pcaps.queue[0].type; - single_iface = 1; + dumpers.queue[0] = pcaps.queue[0]; - ret = rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1, - kvlist, single_iface, is_tx_pcap); + single_iface = 1; + pcaps.num_of_queue = 1; + dumpers.num_of_queue = 1; - goto free_kvlist; + goto create_eth; } /* @@ -1027,6 +1025,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params) if (ret < 0) goto free_kvlist; +create_eth: ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers, dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap); -- 2.7.4
[dpdk-dev] [RFC 16/21] net/pcap: remove unnecessary check
Both fields are fields of same type of struct, one's size can't be bigger than others. Signed-off-by: Ferruh Yigit --- drivers/net/pcap/rte_eth_pcap.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index fb170db..9cb9861 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -812,10 +812,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues, if (*eth_dev == NULL) goto error; - /* check length of device name */ - if ((strlen((*eth_dev)->data->name) + 1) > sizeof(data->name)) - goto error; - /* now put it all together * - store queue data in internals, * - store numa_node info in eth_dev -- 2.7.4
[dpdk-dev] [RFC 17/21] net/pcap: remove redundant assignment
data->name assigned twice. Signed-off-by: Ferruh Yigit --- drivers/net/pcap/rte_eth_pcap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index 9cb9861..83b05e2 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -825,8 +825,6 @@ rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues, data->nb_tx_queues = (uint16_t)nb_tx_queues; data->dev_link = pmd_link; data->mac_addrs = ð_addr; - strncpy(data->name, - (*eth_dev)->data->name, strlen((*eth_dev)->data->name)); /* * NOTE: we'll replace the data element, of originally allocated -- 2.7.4
[dpdk-dev] [RFC 18/21] net/pcap: simplify function
simplify function rte_eth_from_pcaps_common by using interim variables.
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 83b05e2..ece7ff0 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -868,22 +868,21 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
return -1;
for (i = 0; i < nb_rx_queues; i++) {
- (*internals)->rx_queue[i].pcap = rx_queues->queue[i].pcap;
- snprintf((*internals)->rx_queue[i].name,
- sizeof((*internals)->rx_queue[i].name), "%s",
- rx_queues->queue[i].name);
- snprintf((*internals)->rx_queue[i].type,
- sizeof((*internals)->rx_queue[i].type), "%s",
- rx_queues->queue[i].type);
+ struct pcap_rx_queue *rx = &(*internals)->rx_queue[i];
+ struct devargs_queue *queue = &rx_queues->queue[i];
+
+ rx->pcap = queue->pcap;
+ snprintf(rx->name, sizeof(rx->name), "%s", queue->name);
+ snprintf(rx->type, sizeof(rx->type), "%s", queue->type);
}
+
for (i = 0; i < nb_tx_queues; i++) {
- (*internals)->tx_queue[i].dumper = tx_queues->queue[i].dumper;
- snprintf((*internals)->tx_queue[i].name,
- sizeof((*internals)->tx_queue[i].name), "%s",
- tx_queues->queue[i].name);
- snprintf((*internals)->tx_queue[i].type,
- sizeof((*internals)->tx_queue[i].type), "%s",
- tx_queues->queue[i].type);
+ struct pcap_tx_queue *tx = &(*internals)->tx_queue[i];
+ struct devargs_queue *queue = &tx_queues->queue[i];
+
+ tx->dumper = queue->dumper;
+ snprintf(tx->name, sizeof(tx->name), "%s", queue->name);
+ snprintf(tx->type, sizeof(tx->type), "%s", queue->type);
}
for (k_idx = 0; k_idx < kvlist->count; k_idx++) {
--
2.7.4
[dpdk-dev] [RFC 19/21] net/pcap: fix missing tx iface assignment
Missing pcap assignment may cause pcap opened again, and previous one
not closed at all.
Fixes: 1e38a7c66923 ("pcap: fix storage of name and type in queues")
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index ece7ff0..f3df372 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -881,6 +881,7 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
struct devargs_queue *queue = &tx_queues->queue[i];
tx->dumper = queue->dumper;
+ tx->pcap = queue->pcap;
snprintf(tx->name, sizeof(tx->name), "%s", queue->name);
snprintf(tx->type, sizeof(tx->type), "%s", queue->type);
}
--
2.7.4
[dpdk-dev] [RFC 20/21] net/pcap: coding convention updates
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 63 -
1 file changed, 31 insertions(+), 32 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index f3df372..552a1de 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -49,6 +49,7 @@
#define RTE_ETH_PCAP_SNAPLEN ETHER_MAX_JUMBO_FRAME_LEN
#define RTE_ETH_PCAP_PROMISC 1
#define RTE_ETH_PCAP_TIMEOUT -1
+
#define ETH_PCAP_RX_PCAP_ARG "rx_pcap"
#define ETH_PCAP_TX_PCAP_ARG "tx_pcap"
#define ETH_PCAP_RX_IFACE_ARG "rx_iface"
@@ -112,7 +113,9 @@ static const char *valid_arguments[] = {
NULL
};
-static struct ether_addr eth_addr = { .addr_bytes = { 0, 0, 0, 0x1, 0x2, 0x3 }
};
+static struct ether_addr eth_addr = {
+ .addr_bytes = { 0, 0, 0, 0x1, 0x2, 0x3 }
+};
static const char *drivername = "Pcap PMD";
static struct rte_eth_link pmd_link = {
.link_speed = ETH_SPEED_NUM_10G,
@@ -122,15 +125,12 @@ static struct rte_eth_link pmd_link = {
};
static int
-eth_pcap_rx_jumbo(struct rte_mempool *mb_pool,
- struct rte_mbuf *mbuf,
- const u_char *data,
- uint16_t data_len)
+eth_pcap_rx_jumbo(struct rte_mempool *mb_pool, struct rte_mbuf *mbuf,
+ const u_char *data, uint16_t data_len)
{
- struct rte_mbuf *m = mbuf;
-
/* Copy the first segment. */
uint16_t len = rte_pktmbuf_tailroom(mbuf);
+ struct rte_mbuf *m = mbuf;
rte_memcpy(rte_pktmbuf_append(mbuf, len), data, len);
data_len -= len;
@@ -170,7 +170,7 @@ eth_pcap_gather_data(unsigned char *data, struct rte_mbuf
*mbuf)
while (mbuf) {
rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
- mbuf->data_len);
+ mbuf->data_len);
data_len += mbuf->data_len;
mbuf = mbuf->next;
@@ -178,9 +178,7 @@ eth_pcap_gather_data(unsigned char *data, struct rte_mbuf
*mbuf)
}
static uint16_t
-eth_pcap_rx(void *queue,
- struct rte_mbuf **bufs,
- uint16_t nb_pkts)
+eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
{
unsigned i;
struct pcap_pkthdr header;
@@ -233,6 +231,7 @@ eth_pcap_rx(void *queue,
}
pcap_q->rx_stat.pkts += num_rx;
pcap_q->rx_stat.bytes += rx_bytes;
+
return num_rx;
}
@@ -251,9 +250,7 @@ calculate_timestamp(struct timeval *ts) {
* Callback to handle writing packets to a pcap file.
*/
static uint16_t
-eth_pcap_tx_dumper(void *queue,
- struct rte_mbuf **bufs,
- uint16_t nb_pkts)
+eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
{
unsigned i;
struct rte_mbuf *mbuf;
@@ -306,6 +303,7 @@ eth_pcap_tx_dumper(void *queue,
dumper_q->tx_stat.pkts += num_tx;
dumper_q->tx_stat.bytes += tx_bytes;
dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
+
return num_tx;
}
@@ -313,9 +311,7 @@ eth_pcap_tx_dumper(void *queue,
* Callback to handle sending packets through a real NIC.
*/
static uint16_t
-eth_pcap_tx(void *queue,
- struct rte_mbuf **bufs,
- uint16_t nb_pkts)
+eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
{
unsigned i;
int ret;
@@ -332,14 +328,13 @@ eth_pcap_tx(void *queue,
if (likely(mbuf->nb_segs == 1)) {
ret = pcap_sendpacket(tx_queue->pcap,
- rte_pktmbuf_mtod(mbuf, u_char *),
- mbuf->pkt_len);
+ rte_pktmbuf_mtod(mbuf, u_char *),
+ mbuf->pkt_len);
} else {
if (mbuf->pkt_len <= ETHER_MAX_JUMBO_FRAME_LEN) {
eth_pcap_gather_data(tx_pcap_data, mbuf);
ret = pcap_sendpacket(tx_queue->pcap,
- tx_pcap_data,
- mbuf->pkt_len);
+ tx_pcap_data, mbuf->pkt_len);
} else {
RTE_LOG(ERR, PMD,
"Dropping PCAP packet. "
@@ -362,6 +357,7 @@ eth_pcap_tx(void *queue,
tx_queue->tx_stat.pkts += num_tx;
tx_queue->tx_stat.bytes += tx_bytes;
tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
+
return num_tx;
}
@@ -457,9 +453,7 @@ eth_dev_start(struct rte_eth_dev *dev)
if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0)
{
if (open_single_tx_pcap(tx->name, &tx->dumper) < 0)
return -1;
- }
-
- else if (!tx->pcap && strcmp(
[dpdk-dev] [RFC 21/21] net/pcap: remove rte prefix from static functions
Signed-off-by: Ferruh Yigit
---
drivers/net/pcap/rte_eth_pcap.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 552a1de..55ff2c0 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -783,7 +783,7 @@ open_tx_iface(const char *key, const char *value, void
*extra_args)
}
static int
-rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues,
+pmd_init_internals(const char *name, const unsigned nb_rx_queues,
const unsigned nb_tx_queues, struct pmd_internals **internals,
struct rte_eth_dev **eth_dev)
{
@@ -846,7 +846,7 @@ error:
}
static int
-rte_eth_from_pcaps_common(const char *name, struct pmd_devargs *rx_queues,
+eth_from_pcaps_common(const char *name, struct pmd_devargs *rx_queues,
const unsigned nb_rx_queues, struct pmd_devargs *tx_queues,
const unsigned nb_tx_queues, struct rte_kvargs *kvlist,
struct pmd_internals **internals, struct rte_eth_dev **eth_dev)
@@ -861,7 +861,7 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
if (tx_queues == NULL && nb_tx_queues > 0)
return -1;
- if (rte_pmd_init_internals(name, nb_rx_queues, nb_tx_queues, internals,
+ if (pmd_init_internals(name, nb_rx_queues, nb_tx_queues, internals,
eth_dev) < 0)
return -1;
@@ -899,7 +899,7 @@ rte_eth_from_pcaps_common(const char *name, struct
pmd_devargs *rx_queues,
}
static int
-rte_eth_from_pcaps(const char *name, struct pmd_devargs *rx_queues,
+eth_from_pcaps(const char *name, struct pmd_devargs *rx_queues,
const unsigned nb_rx_queues, struct pmd_devargs *tx_queues,
const unsigned nb_tx_queues, struct rte_kvargs *kvlist,
int single_iface, unsigned int using_dumpers)
@@ -908,7 +908,7 @@ rte_eth_from_pcaps(const char *name, struct pmd_devargs
*rx_queues,
struct rte_eth_dev *eth_dev = NULL;
int ret;
- ret = rte_eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
+ ret = eth_from_pcaps_common(name, rx_queues, nb_rx_queues,
tx_queues, nb_tx_queues, kvlist, &internals, ð_dev);
if (ret < 0)
@@ -929,7 +929,7 @@ rte_eth_from_pcaps(const char *name, struct pmd_devargs
*rx_queues,
static int
-rte_pmd_pcap_devinit(const char *name, const char *params)
+pmd_pcap_devinit(const char *name, const char *params)
{
unsigned int is_rx_pcap = 0, is_tx_pcap = 0;
struct rte_kvargs *kvlist;
@@ -1018,7 +1018,7 @@ rte_pmd_pcap_devinit(const char *name, const char *params)
goto free_kvlist;
create_eth:
- ret = rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers,
+ ret = eth_from_pcaps(name, &pcaps, pcaps.num_of_queue, &dumpers,
dumpers.num_of_queue, kvlist, single_iface, is_tx_pcap);
free_kvlist:
@@ -1028,7 +1028,7 @@ free_kvlist:
}
static int
-rte_pmd_pcap_devuninit(const char *name)
+pmd_pcap_devuninit(const char *name)
{
struct rte_eth_dev *eth_dev = NULL;
@@ -1053,8 +1053,8 @@ rte_pmd_pcap_devuninit(const char *name)
static struct rte_driver pmd_pcap_drv = {
.type = PMD_VDEV,
- .init = rte_pmd_pcap_devinit,
- .uninit = rte_pmd_pcap_devuninit,
+ .init = pmd_pcap_devinit,
+ .uninit = pmd_pcap_devuninit,
};
PMD_REGISTER_DRIVER(pmd_pcap_drv, eth_pcap);
--
2.7.4
[dpdk-dev] [PATCH] doc: announce KNI ethtool removal
On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit wrote: > On 7/20/2016 5:07 PM, Thomas Monjalon wrote: > > The out-of-tree kernel code must be avoided. > > Moreover there is no good reason to keep this legacy feature > > which is only partially supported. > > > > As described earlier in this plan: > > http://dpdk.org/ml/archives/dev/2016-July/043606.html > > it will help to keep PCI ids in PMD code. > > > > Signed-off-by: Thomas Monjalon > > --- > > doc/guides/rel_notes/deprecation.rst | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > > index f502f86..9cadf6a 100644 > > --- a/doc/guides/rel_notes/deprecation.rst > > +++ b/doc/guides/rel_notes/deprecation.rst > > @@ -41,3 +41,10 @@ Deprecation Notices > > * The mempool functions for single/multi producer/consumer are > deprecated and > >will be removed in 16.11. > >It is replaced by rte_mempool_generic_get/put functions. > > + > > +* The ethtool support will be removed from KNI in 16.11. > > + It is implemented only for igb and ixgbe. > > + It is really hard to maintain because it requires some out-of-tree > kernel > > + code to be duplicated in this kernel module. > > + Removing this partial support will help to restrict the PCI id > definitions > > + to the PMD code. > > > > KNI ethtool is functional and maintained, and it may have users! > > Why just removing it, specially without providing an alternative? > Is is good time to discuss KCP again? > Yes, my product uses it. Seems like we are back to the same discussion we had a few months ago about the KNI situation... It shouldn't be removed unless there is a replacement, ideally one that works with the normal Linux tools like every other network device. While the code wasn't ready at the time, it was a definite improvement over what we have with KNI today. Jay
[dpdk-dev] [PATCH] validate_abi: build faster by augmenting make with job count
On Thu, Jul 21, 2016 at 03:22:45PM +, Wiles, Keith wrote:
>
> > On Jul 21, 2016, at 10:06 AM, Neil Horman wrote:
> >
> > On Thu, Jul 21, 2016 at 02:09:19PM +, Wiles, Keith wrote:
> >>
> >>> On Jul 21, 2016, at 8:54 AM, Neil Horman wrote:
> >>>
> >>> On Wed, Jul 20, 2016 at 10:32:28PM +, Wiles, Keith wrote:
>
> > On Jul 20, 2016, at 3:16 PM, Neil Horman
> > wrote:
> >
> > On Wed, Jul 20, 2016 at 07:47:32PM +, Wiles, Keith wrote:
> >>
> >>> On Jul 20, 2016, at 12:48 PM, Neil Horman
> >>> wrote:
> >>>
> >>> On Wed, Jul 20, 2016 at 07:40:49PM +0200, Thomas Monjalon wrote:
> 2016-07-20 13:09, Neil Horman:
> > From: Neil Horman
> >
> > John Mcnamara and I were discussing enhacing the validate_abi
> > script to build
> > the dpdk tree faster with multiple jobs. Theres no reason not to
> > do it, so this
> > implements that requirement. It uses a MAKE_JOBS variable that can
> > be set by
> > the user to limit the job count. By default the job count is set
> > to the number
> > of online cpus.
>
> Please could you use the variable name DPDK_MAKE_JOBS?
> This name is already used in scripts/test-build.sh.
>
> >>> Sure
> >>>
> > +if [ -z "$MAKE_JOBS" ]
> > +then
> > + # This counts the number of cpus on the system
> > + MAKE_JOBS=`lscpu -p=cpu | grep -v "#" | wc -l`
> > +fi
>
> Is lscpu common enough?
>
> >>> I'm not sure how to answer that. lscpu is part of the util-linux
> >>> package, which
> >>> is part of any base install. Theres a variant for BSD, but I'm not
> >>> sure how
> >>> common it is there.
> >>> Neil
> >>>
> Another acceptable default would be just "-j" without any number.
> It would make the number of jobs unlimited.
> >>
> >> I think the best is just use -j as it tries to use the correct number
> >> of jobs based on the number of cores, right?
> >>
> > -j with no argument (or -j 0), is sort of, maybe what you want. With
> > either of
> > those options, make will just issue jobs as fast as it processes
> > dependencies.
> > Dependent on how parallel the build is, that can lead to tons of
> > waiting process
> > (i.e. more than your number of online cpus), which can actually hurt
> > your build
> > time.
>
> I read the manual and looked at the code, which supports your statement.
> (I think I had some statement on stack overflow and the last time I
> believe anything on the internet :-) I have not seen a lot of
> differences in compile times with -j on my system. Mostly I suspect it
> is the number of paths in the dependency, cores and memory on the system.
>
> I have 72 lcores or 2 sockets, 18 cores per socket. Xeon 2.3Ghz cores.
>
> $ export RTE_TARGET=x86_64-native-linuxapp-gcc
>
> $ time make install T=${RTE_TARGET}
> real 0m59.445s user 0m27.344s sys 0m7.040s
>
> $ time make install T=${RTE_TARGET} -j
> real 0m26.584s user 0m14.380s sys 0m5.120s
>
> # Remove the x86_64-native-linuxapp-gcc
>
> $ time make install T=${RTE_TARGET} -j 72
> real 0m23.454s user 0m10.832s sys 0m4.664s
>
> $ time make install T=${RTE_TARGET} -j 8
> real 0m23.812s user 0m10.672s sys 0m4.276s
>
> cd x86_64-native-linuxapp-gcc
> $ make clean
> $ time make
> real 0m28.539s user 0m9.820s sys0m3.620s
>
> # Do a make clean between each build.
>
> $ time make -j
> real 0m7.217s user 0m6.532s sys0m2.332s
>
> $ time make -j 8
> real 0m8.256s user 0m6.472s sys0m2.456s
>
> $ time make -j 72
> real 0m6.866s user 0m6.184s sys0m2.216s
>
> Just the real time numbers in the following table.
>
> processes real Time depdirs
> no -j 59.4sYes
> -j 8 23.8sYes
> -j 7223.5sYes
> -j 26.5sYes
>
> no -j 28.5s No
> -j 8 8.2s No
> -j 72 6.8s No
> -j 7.2s No
>
> Looks like the depdirs build time on my system:
> $ make clean -j
> $ rm .depdirs
> $ time make -j
> real 0m23.734s user 0m11.228s sys 0m4.844s
>
> About 16 seconds, which is not a lot of savings. Now the difference from
> no -j to -j is a lot, but the difference between -j and -j
> is not a huge saving. This leads me back to over engineering the problem
> when ?-j? would
[dpdk-dev] [PATCH v2] net/i40e: remove weak symbols
On Wed, Jul 20, 2016 at 06:11:16PM +0100, Zoltan Kiss wrote: Hi, Kiss > Using weak symbols have a few issues with static linking: > > - normally the linker searches the .o files already linked, if your weak > one is there, it won't check if there is a strong version > - unless the symbol is directly referred, but it's not the right thing to > rely on > - or --whole-archive specified in the command line, which pulls in a lot > of unwanted stuff --whole-archive on the other hand can ensure all the object files are linked, and the strong symbol will take precedence over weak symbol. So we don't need to take care of the sequence of the object files in the ar or between ar. > - whole-archive also makes libtool dropping the library from the command > line, which is what should happen with dynamic linking, but not with > static (observed on Ubuntu 14.04). This is an important bug if you > build a static library depending on DPDK you mean the libtool bug for --whole-archive? if it does, you shouldn't using the libtool, BTW what's the circumstance you must use the libtool. using you own makefile for the library or APP. > > This patch merges the two version and make the behaviour rely on the > config. > > If we can agree in the concept, I can send a series to fix this for the > other weak functions. > > Signed-off-by: Zoltan Kiss > --- > > Notes: > v2: fix commit message > > drivers/net/i40e/i40e_rxtx.c | 36 +++- > drivers/net/i40e/i40e_rxtx_vec.c | 36 > 2 files changed, 35 insertions(+), 37 deletions(-) > >From the original design, we follow the rule, we don't want the Macro in the >file to seperate the different Rx/Tx path for disabled/enabled vector configuration. Adam Bynes
[dpdk-dev] [PATCH] net/i40e: fix out-of-bounds writes during vector Rx
On Thu, Jul 21, 2016 at 02:03:38PM +0300, Ilya Maximets wrote:
> From: Sergey Dyasly
>
> Rx loop inside _recv_raw_pkts_vec() ignores nb_pkts argument and always
> tries to receive RTE_I40E_VPMD_RX_BURST (32) packets. This is a violation
> of rte_eth_rx_burst() API and can lead to memory corruption (out-of-bounds
> writes to struct rte_mbuf **rx_pkts) if nb_pkts is less than 32.
>
> Fix this by actually using nb_pkts inside the loop.
>
> Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
>
> Signed-off-by: Sergey Dyasly
> Acked-by: Ilya Maximets
> ---
> drivers/net/i40e/i40e_rxtx_vec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/i40e/i40e_rxtx_vec.c
> b/drivers/net/i40e/i40e_rxtx_vec.c
> index 05cb415..51fb282 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec.c
> @@ -269,7 +269,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct
> rte_mbuf **rx_pkts,
>* D. fill info. from desc to mbuf
>*/
>
> - for (pos = 0, nb_pkts_recd = 0; pos < RTE_I40E_VPMD_RX_BURST;
> + for (pos = 0, nb_pkts_recd = 0; pos < nb_pkts;
> pos += RTE_I40E_DESCS_PER_LOOP,
> rxdp += RTE_I40E_DESCS_PER_LOOP) {
> __m128i descs[RTE_I40E_DESCS_PER_LOOP];
> --
> 2.7.4
Acked-by: Adam Bynes
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Jerin, Sorry, looks like I missed your reply. Please see below. On Mon, Jul 11, 2016 at 04:11:43PM +0530, Jerin Jacob wrote: > On Tue, Jul 05, 2016 at 08:16:46PM +0200, Adrien Mazarguil wrote: > > Hi Adrien, > > Overall this proposal looks very good. I could easily map to the > classification hardware engines I am familiar with. Great, thanks. > > Priorities > > ~~ > > > > A priority can be assigned to a matching pattern. > > > > The default priority level is 0 and is also the highest. Support for more > > than a single priority level in hardware is not guaranteed. > > > > If a packet is matched by several filters at a given priority level, the > > outcome is undefined. It can take any path and can even be duplicated. > > In some cases fatal unrecoverable error too Right, do you think I need to elaborate regarding unrecoverable errors? How much unrecoverable by the way? Like not being able to receive any more packets? > > Matching pattern items for packet data must be naturally stacked (ordered > > from lowest to highest protocol layer), as in the following examples: > > > > +--+ > > | TCPv4 as L4 | > > +===+==+ > > | 0 | Ethernet | > > +---+--+ > > | 1 | IPv4 | > > +---+--+ > > | 2 | TCP | > > +---+--+ > > > > ++ > > | TCPv6 in VXLAN | > > +===++ > > | 0 | Ethernet | > > +---++ > > | 1 | IPv4 | > > +---++ > > | 2 | UDP| > > +---++ > > | 3 | VXLAN | > > +---++ > > | 4 | Ethernet | > > +---++ > > | 5 | IPv6 | > > +---++ > > How about enumerating as "Inner-IPV6" flow type to avoid any confusion. > Though spec > can be same for both IPv6 and Inner-IPV6. I'm not sure, if we have a more than two encapsulated IPv6 headers, knowing that one of them is "inner" is not really useful. This is why I choose to enforce the stack ordering instead, I think it makes more sense. > > | 6 | TCP| > > +---++ > > > > +-+ > > | TCPv4 as L4 with meta items | > > +===+=+ > > | 0 | VOID| > > +---+-+ > > | 1 | Ethernet| > > +---+-+ > > | 2 | VOID| > > +---+-+ > > | 3 | IPv4| > > +---+-+ > > | 4 | TCP | > > +---+-+ > > | 5 | VOID| > > +---+-+ > > | 6 | VOID| > > +---+-+ > > > > The above example shows how meta items do not affect packet data matching > > items, as long as those remain stacked properly. The resulting matching > > pattern is identical to "TCPv4 as L4". > > > > ++ > > | UDPv6 anywhere | > > +===++ > > | 0 | IPv6 | > > +---++ > > | 1 | UDP| > > +---++ > > > > If supported by the PMD, omitting one or several protocol layers at the > > bottom of the stack as in the above example (missing an Ethernet > > specification) enables hardware to look anywhere in packets. > > It would be good if the common code can give it as Ethernet, IPV6, UDP > to PMD(to avoid common code duplication across PMDs) I left this mostly at PMD's discretion for now. Applications must provide explicit rules if they need a consistent behavior. PMDs may not support this at all, I've just documented what applications should expect when attempting this kind of pattern. > > It is unspecified whether the payload of supported encapsulations > > (e.g. VXLAN inner packet) is matched by such a pattern, which may apply to > > inner, outer or both packets. > > a separate flow type enumeration may fix that problem. like "Inner-IPV6" > mentioned above. Not sure about that, for the same reason as above. Which "inner" level would be matched by such a pattern? Note that it could have started with VXLAN followed by ETH and then IPv6 if the application cared. This is basically the ability to remain vague about a rule. I didn't want to forbid it outright because I'm sure there are possible use cases: - PMD validation and debugging. - Rough filtering according to protocols a packet might contain somewhere (think of the network admins who cannot stand anything other than packets addressed to TCP port 80). > > +-+ > > | Invalid, missing L3 | > > +===+=+ > > | 0 | Ethernet| > > +---+-+ > > | 1 | UDP | > > +---+-+ > > > > The above pattern is invalid due to a missing L3 specification between L2 > > and L4. It is only allowed at the bottom and at the top of the stack. > > > > > ``SIGNATURE`` > > ^ > > > > Requests hash-based signature dispatching for this rule. > > > > Considering this is a global setting on devices
[dpdk-dev] [PATCH] doc: announce KNI ethtool removal
2016-07-21 13:20, Jay Rolette: > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit > wrote: > > KNI ethtool is functional and maintained, and it may have users! > > > > Why just removing it, specially without providing an alternative? > > Is is good time to discuss KCP again? > > Yes, my product uses it. Your product uses what? KCP? KNI? KNI ethtool? > Seems like we are back to the same discussion we > had a few months ago about the KNI situation... > > It shouldn't be removed unless there is a replacement, ideally one that > works with the normal Linux tools like every other network device. This ethtool module works only for igb and ixgbe! There is already no replacement for other drivers. Who works on a replacement? > While the code wasn't ready at the time, it was a definite improvement over > what > we have with KNI today.
[dpdk-dev] [PATCH] eal: fix parsing of argument of option --lcores
On Thu, Jul 21, 2016 at 02:03:38PM +0800, Wei Dai wrote:
Hi Wei,
> The '-' in lcores set overrides cpu set of following
> lcore set in the argument of EAL option --lcores.
>
> Fixes: 53e54bf81700 ("eal: new option --lcores for cpu assignment")
>
> Signed-off-by: Wei Dai
> ---
> lib/librte_eal/common/eal_common_options.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index 0a594d7..96eb1a9 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -563,6 +563,7 @@ convert_to_cpuset(rte_cpuset_t *cpusetp,
> * lcores, cpus could be a single digit/range or a group.
> * '(' and ')' are necessary if it's a group.
> * If not supply '@cpus', the value of cpus uses the same as lcores.
> + * The 'a-b' in lcores not within '(' and ')' means a,a+1,...,b-1,b .
this description is not very clear, a-b and (a-b) are both the same meaning.
may be need a table for comparison
a-b@(c-d)
a-b at c-d
(a-b)@c-d
(a-b)@(c-d)
all the above I believe are the same
only the following two cases:
a-b,
(a-b),
so the key point here is the @ and (), not only @
> * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6),7-8' means start 9 EAL thread as below
> * lcore 0 runs on cpuset 0x41 (cpu 0,6)
> * lcore 1 runs on cpuset 0x2 (cpu 1)
> @@ -571,6 +572,15 @@ convert_to_cpuset(rte_cpuset_t *cpusetp,
> * lcore 6 runs on cpuset 0x41 (cpu 0,6)
> * lcore 7 runs on cpuset 0x80 (cpu 7)
> * lcore 8 runs on cpuset 0x100 (cpu 8)
> + * e.g. '0-2,(3-5)@(3,4),6@(5,6),7@(5-7)'means start 8 EAL threads as below
> + * lcore 0 runs on cpuset 0x1 (cpu 0)
> + * lcore 1 runs on cpuset 0x2 (cpu 1)
> + * lcore 2 runs on cpuset ox4 (cpu 2)
> + * lcore 3,4,5 runs on cpuset 0x18 (cpu 3,4)
> + * lcore 6 runs on cpuset 0x60 (cpu 5,6)
> + * lcore 7 runs on cpuset 0xe0 (cpu 5,6,7)
> + * The second case is used to test bugfix for lflags not be cleared after use
you can put this sentance and description into the commit log
I don't think you should put bugfix description in comments here.
> + */
> */
> static int
> eal_parse_lcores(const char *lcores)
> @@ -679,6 +689,8 @@ eal_parse_lcores(const char *lcores)
> sizeof(rte_cpuset_t));
> }
>
> + lflags = 0;
> +
> lcores = end + 1;
> } while (*end != '\0');
>
> --
> 2.5.5
Adam Bynes
[dpdk-dev] [PATCH] eal: fix check number of bytes from read function
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, July 21, 2016 4:36 PM
> To: Jastrzebski, MichalX K
> Cc: dev at dpdk.org; Richardson, Bruce ;
> Kobylinski, MichalX ; Gonzalez Monroy,
> Sergio ; david.marchand at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH] eal: fix check number of bytes from read
> function
>
> Hi,
>
> 2016-07-20 16:24, Michal Jastrzebski:
> > - if (read(fd, &page, sizeof(uint64_t)) < 0) {
> > +
> > + retval = read(fd, &page, sizeof(uint64_t));
> > + if (retval < 0) {
> > RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap:
> %s\n",
> > __func__, strerror(errno));
> > close(fd);
> > return RTE_BAD_PHYS_ADDR;
> > + } else if (retval >= 0 && retval < (int)sizeof(uint64_t)) {
>
Hi Thomas,
> I have 4 comments about the above line:
That's too much for one line. I should improve next time:)
> - the check retval >= 0 is not needed because implied by else
> - why not checking retval != sizeof(uint64_t) as it is the exact expected
> value?
Yes, it is better solution,
> - (int)sizeof(uint64_t) can be replaced by 8 but it's shorter ;)
I didn't want to change all invokes of read() function here.
I can use some macro:
#define PFN_MASK_SIZE 8
How do You think?
> - only 1 space is required between } and else
>
> > + RTE_LOG(ERR, EAL, "%s(): read %d bytes from
> /proc/self/pagemap "
> > + "but expected %d: %s\n",
> > + __func__, retval, (int)sizeof(uint64_t),
> strerror(errno));
>
> Are you sure errno is meaningful here?
I think it is not. Will send v2.
>
> > + close(fd);
> > + return RTE_BAD_PHYS_ADDR;
> > }
Thanks for a review
Michal.
[dpdk-dev] [PATCH] doc: announce KNI ethtool removal
On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon wrote: > 2016-07-21 13:20, Jay Rolette: > > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit > > wrote: > > > KNI ethtool is functional and maintained, and it may have users! > > > > > > Why just removing it, specially without providing an alternative? > > > Is is good time to discuss KCP again? > > > > Yes, my product uses it. > > Your product uses what? KCP? KNI? KNI ethtool? > Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the device/interface in Linux. I'm assuming the "ethtool" bits under discussion are the same things that make ifconfig work with KNI to the limited extent it does. > Seems like we are back to the same discussion we > > had a few months ago about the KNI situation... > > > > It shouldn't be removed unless there is a replacement, ideally one that > > works with the normal Linux tools like every other network device. > > This ethtool module works only for igb and ixgbe! > There is already no replacement for other drivers. > Who works on a replacement? > Ferruh submitted KCP previously, but you guys didn't like the fact that it was a kernel module. IIRC, one of the gains from that was simplified maintenance because you didn't need driver specific support for KNI. Assuming he's still willing to beat it into shape, we have something that is already most of the way there. If people are going to continue to block it because it is a kernel module, then IMO, it's better to leave the existing support on igx / ixgbe in place instead of stepping backwards to zero support for ethtool. > While the code wasn't ready at the time, it was a definite improvement > over what > > we have with KNI today. >
[dpdk-dev] [PATCH] memzone: allow full length name
> > (strlen(name) == sizeof(mz->name) - 1) is a valid case, change the
> > condition to reflect that.
> > Move it earlier to avoid lookup with invalid name.
> > Change errno to ENAMETOOLONG.
> >
> > Fixes: 85cf0079 ("mem: avoid memzone/mempool/ring name truncation")
> >
> > Signed-off-by: Zoltan Kiss
>
> Acked-by: Olivier Matz
Applied, thanks
[dpdk-dev] [PATCH v2] mempool: fix lack of free registration
2016-07-20 18:14, Zoltan Kiss:
> The new mempool handler interface forgets to register the free() function
> of the ops. Introduced in this patch:
>
> Fixes: 449c49b93a6b ("mempool: support handler operations")
>
> Signed-off-by: Zoltan Kiss
> Acked-by: Olivier Matz
Applied, thanks
[dpdk-dev] [PATCH 02/12] virtio: setup and start cq in configure callback
On Thu, 21 Jul 2016 10:08:20 +0200 Olivier Matz wrote: > + dev_info->max_rx_queues = (uint16_t) > + ((VIRTIO_MAX_RX_QUEUES < hw->max_queue_pairs) ? > + VIRTIO_MAX_RX_QUEUES : hw->max_queue_pairs); > + dev_info->max_tx_queues = (uint16_t) > + ((VIRTIO_MAX_TX_QUEUES < hw->max_queue_pairs) ? > + VIRTIO_MAX_TX_QUEUES : hw->max_queue_pairs); cast here was always unnecessary. Why not use RTE_MIN()
[dpdk-dev] [PATCH v2] mempool: adjust name string size in related data types
2016-07-21 16:25, Olivier Matz:
> On 07/21/2016 03:47 PM, Zoltan Kiss wrote:
> > On 21/07/16 14:40, Olivier Matz wrote:
> >> On 07/20/2016 07:16 PM, Zoltan Kiss wrote:
> >>> A recent patch brought up an issue about the size of the 'name' fields:
> >>>
> >>> 85cf0079 mem: avoid memzone/mempool/ring name truncation
> >>>
> >>> These relations should be observed:
> >>>
> >>> 1. Each ring creates a memzone with a prefixed name:
> >>> RTE_RING_NAMESIZE <= RTE_MEMZONE_NAMESIZE - strlen(RTE_RING_MZ_PREFIX)
> >>>
> >>> 2. There are some mempool handlers which create a ring with a prefixed
> >>> name:
> >>> RTE_MEMPOOL_NAMESIZE <= RTE_RING_NAMESIZE -
> >>> strlen(RTE_MEMPOOL_MZ_PREFIX)
> >>>
> >>> 3. A mempool can create up to RTE_MAX_MEMZONE pre and postfixed
> >>> memzones:
> >>> sprintf(postfix, "_%d", RTE_MAX_MEMZONE)
> >>> RTE_MEMPOOL_NAMESIZE <= RTE_MEMZONE_NAMESIZE -
> >>> strlen(RTE_MEMPOOL_MZ_PREFIX) - strlen(postfix)
> >>>
> >>> Setting all of them to 32 hides this restriction from the application.
> >>> This patch decreases the mempool and ring string size to accommodate for
> >>> these prefixes, but it doesn't apply the 3rd constraint. Applications
> >>> relying on these constants need to be recompiled, otherwise they'll run
> >>> into ENAMETOOLONG issues.
> >>> The size of the arrays are kept 32 for ABI compatibility, it can be
> >>> decreased next time the ABI changes.
> >>>
> >>> Signed-off-by: Zoltan Kiss
> >>
> >> Looks like to be a good compromise for the 16.07 release. One question
> >> however: why not taking in account the 3rd constraint? Because it may
> >> not completly fix the issue if the mempool is fragmented.
> >>
> >> We could define RTE_MEMPOOL_NAMESIZE to 24
> >> = 32 - len('mp_') - len('_0123'))
> >
> > I was trying to figure out a compile time macro for strlen(postfix), but
> > I could not. Your suggestion would work only until someone increases
> > RTE_MAX_MEMZONE above . As the likelihood of fragmenting a pool over
> > 99 memzones seems small, I did not bother to fix this with an ugly hack,
> > but if you think we should include it, let me know!
>
> Ok, looks fair, thanks for the clarification.
>
> Acked-by: Olivier Matz
Applied, thanks
[dpdk-dev] [PATCH 05/12] mbuf: add new Rx checksum mbuf flags
On Thu, 21 Jul 2016 10:08:23 +0200 Olivier Matz wrote: > +/** > + * Deprecated. > + * Checking this flag alone is deprecated: check the 2 bits of > + * PKT_RX_L4_CKSUM_MASK. > + * This flag was set when the L4 checksum of a packet was detected as > + * wrong by the hardware. > + */ > +#define PKT_RX_L4_CKSUM_BAD (1ULL << 3) > + > +/** > + * Deprecated. > + * Checking this flag alone is deprecated: check the 2 bits of > + * PKT_RX_IP_CKSUM_MASK. > + * This flag was set when the IP checksum of a packet was detected as > + * wrong by the hardware. > + */ > +#define PKT_RX_IP_CKSUM_BAD (1ULL << 4) I think you should use the GCC deprecated attribute, not sure how though
[dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update
> > Consumer queue dequeuing must be guaranteed to be done fully before the > > tail is updated. This is not guaranteed with a read barrier, > > changed to a write barrier just before tail update which in practice > > guarantees correct order of reads and writes. > > > > Signed-off-by: Juhamatti Kuusisaari > > Acked-by: Konstantin Ananyev Applied, thanks
[dpdk-dev] [PATCH] app/test: fix refcnt_mbuf_ring size
> > rte_ring_create expects the size of the ring to
> > be a power of 2. REFCNT_RING_SIZE value is not
> > power of 2 in-case if RTE_MAX_LCORE == 96.
> > Fix it by aligning the size to next power of 2 value.
> >
> > Fixes: af75078f ("first public release")
> >
> > Signed-off-by: Jerin Jacob
>
> Acked-by: Olivier Matz
Applied, thanks
[dpdk-dev] [PATCH] test_mempool: remove unused mp_ext var
2016-07-21 15:35, Thomas Monjalon:
> 2016-07-21 15:28, Olivier Matz:
> > Hi Santosh,
> >
> > On 07/21/2016 01:49 PM, Santosh Shukla wrote:
> > > test_mempool func not using pointer variable 'mp_ext' and incorrectly
> > > freed. So
> > > removing ptr var. Now freeing mp_stack var.
> > >
> > > Signed-off-by: Santosh Shukla
> >
> > Strange, it seems these modifications were present in latest patch from
> > David Hunt (v6). Maybe a bad manipulation during the push?
>
> Yes sorry, it seems to be a wrong manipulation, the v5 was pushed.
> This patch seems to fix my error, thanks.
>
> > The "Fixes:" line should be added though.
Fixes: ea151eb48a04 ("app/test: migrate custom mempool handler to stack
handler")
> > Acked-by: Olivier Matz
Applied, thanks
[dpdk-dev] [PATCH] app/test: disable filtering with stripped binary
2016-07-21 11:07, Olivier Matz:
> On 07/19/2016 06:53 PM, Thomas Monjalon wrote:
> > The unavailable tests are filtered out by autotest by looking for
> > the symbols in the binary:
> >
> > PCI autotest: Skipped [Not Available] [00m 00s]
> > Malloc autotest: Success [00m 00s]
> >
> > It results to skip everything if the binary has no symbol (stripped):
> >
> > PCI autotest: Skipped [Not Available] [00m 00s]
> > Malloc autotest: Skipped [Not Available] [00m 00s]
> >
> > This case is handled by getting back to the old behaviour if the binary
> > has no symbol information:
> >
> > PCI autotest: Fail [Not found] [00m 00s]
> > Malloc autotest: Success [00m 00s]
> >
> > Fixes: d553c8f2b1a2 ("app/test: filter out unavailable tests")
> >
> > Signed-off-by: Thomas Monjalon
>
> Tested-by: Olivier Matz
Applied
[dpdk-dev] [PATCH v2] doc: announce ABI change for rte_eth_dev structure
>
> This is an ABI deprecation notice for DPDK 16.11 in librte_ether about
> changes in rte_eth_dev and rte_eth_desc_lim structures.
>
> As discussed in that thread:
>
> http://dpdk.org/ml/archives/dev/2015-September/023603.html
>
> Different NIC models depending on HW offload requested might impose
> different requirements on packets to be TX-ed in terms of:
>
> - Max number of fragments per packet allowed
> - Max number of fragments per TSO segments
> - The way pseudo-header checksum should be pre-calculated
> - L3/L4 header fields filling
> - etc.
>
>
> MOTIVATION:
> ---
>
> 1) Some work cannot (and didn't should) be done in rte_eth_tx_burst.
>However, this work is sometimes required, and now, it's an
>application issue.
>
> 2) Different hardware may have different requirements for TX offloads,
>other subset can be supported and so on.
>
> 3) Some parameters (eg. number of segments in ixgbe driver) may hung
>device. These parameters may be vary for different devices.
>
>For example i40e HW allows 8 fragments per packet, but that is after
>TSO segmentation. While ixgbe has a 38-fragment pre-TSO limit.
>
> 4) Fields in packet may require different initialization (like eg. will
>require pseudo-header checksum precalculation, sometimes in a
>different way depending on packet type, and so on). Now application
>needs to care about it.
>
> 5) Using additional API (rte_eth_tx_prep) before rte_eth_tx_burst let to
>prepare packet burst in acceptable form for specific device.
>
> 6) Some additional checks may be done in debug mode keeping tx_burst
>implementation clean.
>
>
> PROPOSAL:
> -
>
> To help user to deal with all these varieties we propose to:
>
> 1. Introduce rte_eth_tx_prep() function to do necessary preparations of
>packet burst to be safely transmitted on device for desired HW
>offloads (set/reset checksum field according to the hardware
>requirements) and check HW constraints (number of segments per
>packet, etc).
>
>While the limitations and requirements may differ for devices, it
>requires to extend rte_eth_dev structure with new function pointer
>"tx_pkt_prep" which can be implemented in the driver to prepare and
>verify packets, in devices specific way, before burst, what should to
>prevent application to send malformed packets.
>
> 2. Also new fields will be introduced in rte_eth_desc_lim:
>nb_seg_max and nb_mtu_seg_max, providing an information about max
>segments in TSO and non-TSO packets acceptable by device.
>
>This information is useful for application to not create/limit
>malicious packet.
>
>
> APPLICATION (CASE OF USE):
> --
>
> 1) Application should to initialize burst of packets to send, set
>required tx offload flags and required fields, like l2_len, l3_len,
>l4_len, and tso_segsz
>
> 2) Application passes burst to the rte_eth_tx_prep to check conditions
>required to send packets through the NIC.
>
> 3) The result of rte_eth_tx_prep can be used to send valid packets
>and/or restore invalid if function fails.
>
> eg.
>
> for (i = 0; i < nb_pkts; i++) {
>
> /* initialize or process packet */
>
> bufs[i]->tso_segsz = 800;
> bufs[i]->ol_flags = PKT_TX_TCP_SEG | PKT_TX_IPV4
> | PKT_TX_IP_CKSUM;
> bufs[i]->l2_len = sizeof(struct ether_hdr);
> bufs[i]->l3_len = sizeof(struct ipv4_hdr);
> bufs[i]->l4_len = sizeof(struct tcp_hdr);
> }
>
> /* Prepare burst of TX packets */
> nb_prep = rte_eth_tx_prep(port, 0, bufs, nb_pkts);
>
> if (nb_prep < nb_pkts) {
> printf("tx_prep failed\n");
>
> /* drop or restore invalid packets */
>
> }
>
> /* Send burst of TX packets */
> nb_tx = rte_eth_tx_burst(port, 0, bufs, nb_prep);
>
> /* Free any unsent packets. */
>
>
>
> Signed-off-by: Tomasz Kulasek
> ---
> doc/guides/rel_notes/deprecation.rst |7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index f502f86..485aacb 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,10 @@ Deprecation Notices
> * The mempool functions for single/multi producer/consumer are deprecated and
>will be removed in 16.11.
>It is replaced by rte_mempool_generic_get/put functions.
> +
> +* In 16.11 ABI changes are plained: the ``rte_eth_dev`` structure will be
> + extended with new function pointer ``tx_pkt_prep`` allowing verification
> + and processing of packet burst to meet HW specific requirements before
> + transmit. Also new fields will be added to the ``rte_eth_desc_lim``
> structure:
> + ``nb_seg_max`` and ``nb_mtu_seg_max`` provideing information about number
> of
[dpdk-dev] [PATCH v2] i40e:Fix for wrong publish of card speed (was mixed between 10G and 40G)
2016-07-18 09:56, Ido Barnea: > Signed-off-by: Ido Barnea Patch reconstructed (format was not applicable), and applied, thanks
