* [PATCH v2] examples/ip_pipeline: check VLAN and MPLS params
From: Jyoti, Anand B @ 2017-01-08 21:55 UTC (permalink / raw)
To: dev; +Cc: stephen, cristian.dumitrescu, Anand B Jyoti
In-Reply-To: <A1F25702B3CE3F4F8D3936A55AD1FF37925DB5F9@BGSMSX108.gar.corp.intel.com>
This commit add to CLI command check for the following errors
1. SVLAN and CVLAN IDs greater than 12 bits
2. MPLS ID greater than 20 bits
3. max number of supported MPLS labels to avoid array overflow
It prevents running CLI commands with invalid parameters.
Signed-off-by: Anand B Jyoti <anand.b.jyoti@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
examples/ip_pipeline/pipeline/pipeline_routing.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/examples/ip_pipeline/pipeline/pipeline_routing.c b/examples/ip_pipeline/pipeline/pipeline_routing.c
index 3aadbf9..3deaff9 100644
--- a/examples/ip_pipeline/pipeline/pipeline_routing.c
+++ b/examples/ip_pipeline/pipeline/pipeline_routing.c
@@ -494,6 +494,26 @@ app_pipeline_routing_add_route(struct app_params *app,
/* data */
if (data->port_id >= p->n_ports_out)
return -1;
+
+ /* Valid range of VLAN tags 12 bits */
+ if (data->flags & PIPELINE_ROUTING_ROUTE_QINQ)
+ if ((data->l2.qinq.svlan & 0xF000) ||
+ (data->l2.qinq.cvlan & 0xF000))
+ return -1;
+
+ /* Max number of MPLS labels supported */
+ if (data->flags & PIPELINE_ROUTING_ROUTE_MPLS) {
+ uint32_t i;
+
+ if (data->l2.mpls.n_labels >
+ PIPELINE_ROUTING_MPLS_LABELS_MAX)
+ return -1;
+
+ /* Max MPLS label value 20 bits */
+ for (i = 0; i < data->l2.mpls.n_labels; i++)
+ if (data->l2.mpls.labels[i] & 0xFFF00000)
+ return -1;
+ }
}
break;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process
From: Yuanhan Liu @ 2017-01-09 5:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, stable, Xu, Qian Q, Liu, Yong
In-Reply-To: <20170108151500.3272484c@xeon-e3>
On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> On Fri, 6 Jan 2017 18:16:16 +0800
> Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>
> > If the primary enables the vector Rx/Tx path, the current code would
> > let the secondary always choose the non vector Rx/Tx path. This results
> > to a Rx/Tx method mismatch between primary and secondary process. Werid
> > errors then may happen, something like:
> >
> > PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> >
> > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > That is, use vector path if it's given.
> >
> > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> >
> > Cc: stable@dpdk.org
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> This is failing on intel compile tests.
>
>
> http://dpdk.org/patch/18975
Thanks, but it looks like a false alarm to me, for reasons below.
> Failed Build #2:
> OS: RHEL7.2_64
> Target: x86_64-native-linuxapp-gcc
> MKRES test_resource_c.res.o /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
> rte_ethdev.c:(.text+0x9c4): undefined reference to `eth_dev_attach_secondary'
> collect2: error: ld returned 1 exit status
- eth_dev_attach_secondary is not defined in this patch, it's defined
(and used) in the first patch.
- eth_dev_attach_secondary is actually defined; The report even shows
it fails to build with gcc: the gcc build passes on my dev box.
Honestly, I seldom trusted the build reports from STV.
--yliu
^ permalink raw reply
* Re: [PATCH v1 2/2] Test cases for rte_memcmp functions
From: Wang, Zhihong @ 2017-01-09 5:29 UTC (permalink / raw)
To: Thomas Monjalon, Ravi Kerur; +Cc: dev@dpdk.org
In-Reply-To: <1828046.HL686xpgu5@xps13>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, January 3, 2017 4:41 AM
> To: Wang, Zhihong <zhihong.wang@intel.com>; Ravi Kerur
> <rkerur@gmail.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 2/2] Test cases for rte_memcmp
> functions
>
> 2016-06-07 11:09, Wang, Zhihong:
> > From: Ravi Kerur [mailto:rkerur@gmail.com]
> > > Zhilong, Thomas,
> > >
> > > If there is enough interest within DPDK community I can work on adding
> support
> > > for 'unaligned access' and 'test cases' for it. Please let me know either
> way.
> >
> > Hi Ravi,
> >
> > This rte_memcmp is proved with better performance than glibc's in aligned
> > cases, I think it has good value to DPDK lib.
> >
> > Though we don't have memcmp in critical pmd data path, it offers a better
> > choice for applications who do.
>
> Re-thinking about this series, could it be some values to have a rte_memcmp
> implementation?
I think this series (rte_memcmp included) could help:
1. Potentially better performance in hot paths.
2. Agile for tuning.
3. Avoid performance complications -- unusual but possible,
like the glibc memset issue I met while working on vhost
enqueue.
> What is the value compared to glibc one? Why not working on glibc?
As to working on glibc, wider design consideration and test
coverage might be needed, and we'll face different release
cycles, can we have the same agility? Also working with old
glibc could be a problem.
^ permalink raw reply
* Re: [PATCH v5 01/12] eal/bus: introduce bus abstraction
From: Shreyansh Jain @ 2017-01-09 6:24 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <105987546.PAaz77144n@xps13>
On Friday 06 January 2017 08:25 PM, Thomas Monjalon wrote:
> 2017-01-06 16:01, Shreyansh Jain:
>> On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
>>> 2016-12-26 18:53, Shreyansh Jain:
>>>> +/**
>>>> + * A structure describing a generic bus.
>>>> + */
>>>> +struct rte_bus {
>>>> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
>>>> + struct rte_driver_list driver_list;
>>>> + /**< List of all drivers on bus */
>>>> + struct rte_device_list device_list;
>>>> + /**< List of all devices on bus */
>>>> + const char *name; /**< Name of the bus */
>>>> +};
>>>
>>> I am not convinced we should link a generic bus to drivers and devices.
>>> What do you think of having rte_pci_bus being a rte_bus and linking
>>> with rte_pci_device and rte_pci_driver lists?
>>
>> This is different from what I had in mind.
>> You are saying:
>>
>> Class: rte_bus
>> `-> No object instantiated for this class
>> Class: rte_pci_bus inheriting rte_bus
>> `-> object instantiated for this class.
>>
>> Here, rte_bus is being treated as an abstract class which is only
>> inherited and rte_pci_bus is the base class which is instantiated.
>>
>> And I was thinking:
>>
>> Class: rte_bus
>> `-> Object: pci_bus (defined in */eal/eal_pci.c)
>>
>> Here, rte_bus is that base class which is instantiated.
>>
>> I agree that what you are suggesting is inline with current model:
>> rte_device -> abstract class (no one instantiates it)
>> `-> rte_pci_device -> Base class which inherits rte_device and
>> is instantiated.
>
> Yes
>
>> When I choose not to create rte_pci_bus, it was because I didn't want
>> another indirection in form of rte_bus->rte_pci_bus->object.
>> There were no 'non-generic' bus functions which were only applicable for
>> rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
>> of rte_bus.
>>
>>> I'm thinking to something like that:
>>>
>>> struct rte_bus {
>>> TAILQ_ENTRY(rte_bus) next;
>>> const char *name;
>>> rte_bus_scan_t scan;
>>> rte_bus_match_t match;
>>> };
>>> struct rte_pci_bus {
>>> struct rte_bus bus;
>>> struct rte_pci_driver_list pci_drivers;
>>> struct rte_pci_device_list pci_devices;
>>> };
>>
>> if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
>> fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
>> because I don't see any 'non-generic' information in rte_pci_bus which
>> can't be put in rte_bus.
>
> The lists of drivers and devices are specific to the bus.
> Your proposal was to list them as generic rte_driver/rte_device and
> cast them. I'm just saying we can directly declare them with the right type,
> e.g. rte_pci_driver/rte_pci_device.
Ok. I get your point. Already changing the code to reflect this.
>
> In the same logic, the functions probe/remove are specifics for the bus and
> should be declared in rte_pci_driver instead of the generic rte_driver.
Yes, I agree with this after above argument.
>
>
>>>> +/** Helper for Bus registration. The constructor has higher priority than
>>>> + * PMD constructors
>>>> + */
>>>> +#define RTE_REGISTER_BUS(nm, bus) \
>>>> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
>>>> +{\
>>>> + (bus).name = RTE_STR(nm);\
>>>> + rte_eal_bus_register(&bus); \
>>>> +}
>>>
>>> By removing the lists from rte_bus as suggested above, do you still need
>>> a priority for this constructor?
>>
>> I think yes.
>> Even if we have rte_pci_bus as a class, object of rte_bus should be part
>> of Bus list *before* registration of a driver (because, driver
>> registration searches for bus by name).
>>
>> (This is assuming that no global PCI/VDEV/XXX bus object is created
>> which is directly used within all PCI specific bus operations).
>>
>> There was another suggestion on list which was to check for existence of
>> bus _within_ each driver registration and create/instantiate an object
>> in case no bus is registered. I didn't like the approach so I didn't use
>> it. From David [1], and me [2]
>>
>> [1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
>> [2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
>
> OK, we can keep your approach of prioritize bus registrations.
> If we see an issue later, we could switch to a bus registration done
> when a first driver is registered on the bus.
Thanks for confirmation.
>
>
>>>> struct rte_device {
>>>> TAILQ_ENTRY(rte_device) next; /**< Next device */
>>>> + struct rte_bus *bus; /**< Device connected to this bus */
>>>> const struct rte_driver *driver;/**< Associated driver */
>>>> int numa_node; /**< NUMA node connection */
>>>> struct rte_devargs *devargs; /**< Device user arguments */
>>>> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>>>> */
>>>> struct rte_driver {
>>>> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
>>>> + struct rte_bus *bus; /**< Bus serviced by this driver */
>>>> const char *name; /**< Driver name. */
>>>> const char *alias; /**< Driver alias. */
>>>> };
>>>
>>> Do we need to know the bus associated to a driver in rte_driver?
>>> Bus and driver are already associated in rte_device.
>>
>> Two reasons:
>> 1/ A driver should be associated with a bus so that if required, all bus
>> can be directly extracted - even when probing has not been done.
>
> I do not understand this need.
For example, Looping over all drivers for plugging them out. We need to
know which bus a driver is on so that we can unplug the devices
associated with the driver on that bus.
>
>> 2/ device->driver would only be updated after probe. device->driver->bus
>> would not be valid in such cases, if required.
>
> We can update device->driver on match.
Yes, we can.
>
> Please let's do not over-engineer if not needed.
> In this case, I think we can skip rte_driver->bus.
Hm, Ok. This was more of prospective step. We can avoid it without much
impact. I will change the code.
>
>
>> Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
>> compared to rte_bus=>PCI-object. But, I would still like to get a final
>> confirmation of a more preferred way.
>>
>> Meanwhile, I will make changes to accommodate this change to save time
>> in case rte_pci_bus class is final/preferred method.
>
> It looks more natural to me to avoid class casting and use specialized classes
> when possible. So yes I prefer instantiating rte_pci_bus.
> However, I could be wrong, and will consider any argument.
Ok. I will go with your argument - mostly because I am OK either way and
we can always come back if framework changes are stable.
>
> Thanks
>
^ permalink raw reply
* Re: [PATCH v5 05/12] eal: add probe and remove support for rte_driver
From: Shreyansh Jain @ 2017-01-09 6:28 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <2232398.sT6R6nRinv@xps13>
On Friday 06 January 2017 08:56 PM, Thomas Monjalon wrote:
> 2017-01-06 17:14, Shreyansh Jain:
>> On Wednesday 04 January 2017 03:35 AM, Thomas Monjalon wrote:
>>> 2016-12-26 18:53, Shreyansh Jain:
>>>> --- a/lib/librte_eal/common/include/rte_dev.h
>>>> +++ b/lib/librte_eal/common/include/rte_dev.h
>>>> @@ -152,6 +162,8 @@ struct rte_driver {
>>>> struct rte_bus *bus; /**< Bus serviced by this driver */
>>>> const char *name; /**< Driver name. */
>>>> const char *alias; /**< Driver alias. */
>>>> + driver_probe_t *probe; /**< Probe the device */
>>>> + driver_remove_t *remove; /**< Remove/hotplugging the device */
>>>> };
>>>
>>> If I understand well, this probe function does neither scan nor match.
>>> So it could be named init.
>>
>> Current model is:
>>
>> After scanning for devices and populating bus->device_list,
>> Bus probe does:
>> `-> bus->match()
>> `-> rte_driver->probe() for matched driver
>>
>> For PCI drivers, '.probe = rte_eal_pci_probe'.
>>
>> For example, igb_ethdev.c:
>>
>> --->8---
>> static struct eth_driver rte_igb_pmd = {
>> .pci_drv = {
>> .driver = {
>> .probe = rte_eal_pci_probe,
>> .remove = rte_eal_pci_remove,
>> },
>> ...
>> --->8---
>
> Yes
> I'm just having some doubts about the naming "probe" compared to "init".
> And yes I know I was advocating to unify naming to "probe" recently :)
> I would like to be sure it is not confusing for anyone.
> Do you agree that "init" refers to global driver initialization and
> "probe" refers to instantiating a device?
Ok. Makes sense as a standardized way of differentiating 'init' from
'probe'.
>
> If yes, the comment could be changed from "Probe the device" to
> "Check and instantiate a device".
Now that probe if removed from rte_driver, I think this would no longer
be valid. [1]
[1] http://dpdk.org/ml/archives/dev/2017-January/054140.html
>
>>> I think the probe (init) and remove ops must be specific to the bus.
>>> We can have them in rte_bus, and as an example, the pci implementation
>>> would call the pci probe and remove ops of rte_pci_driver.
>
> I do not understand clearly what I was saying here :/
:)
>
>> So,
>> ---
>> After scanning for devices (bus->scan()):
>> Bus probe (rte_eal_bus_probe()):
>> `-> bus->match()
>> `-> bus->init() - a new fn rte_bus_pci_init()
>
> I suggest the naming bus->probe().
> It is currently implemented in rte_eal_pci_probe_one_driver().
>
>> -> which calls rte_eal_pci_probe()
>
> Not needed here, this function is converted into the PCI match function.
>
>> -> and rte_pci_driver->probe()
>
> Yes, bus->probe() makes some processing and calls rte_pci_driver->probe().
I have made some changes on similar lines. Will share them soon. Then we
can discuss again.
>
>
>> and remove rte_driver probe and remove callbacks because they are now
>> redundant. (they were added in bus patches itself)
>> ---
>>
>> Is the above correct understanding of your statement?
>
> I think we just need to move probe/remove in rte_pci_driver.
>
>> Somehow I don't remember why I didn't do this in first place - it seems
>> to be better option than introducing a rte_driver->probe()/remove()
>> layer. I will change it (and think again why I rejected this idea in
>> first place). Thanks.
>
> Thanks
>
^ permalink raw reply
* Re: [PATCH v5 04/12] eal: integrate bus scan and probe with EAL
From: Shreyansh Jain @ 2017-01-09 6:34 UTC (permalink / raw)
To: Rosen, Rami, david.marchand@6wind.com
Cc: dev@dpdk.org, thomas.monjalon@6wind.com
In-Reply-To: <9B0331B6EBBD0E4684FBFAEDA55776F93D482B72@HASMSX110.ger.corp.intel.com>
Hello,
On Sunday 08 January 2017 05:51 PM, Rosen, Rami wrote:
> Hi,
> ....
> ....
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 2206277..2c223de 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> ....
>
> +/* Scan all the buses for registering devices */ int
> +rte_eal_bus_scan(void)
> +{
> + int ret;
> + struct rte_bus *bus = NULL;
> +
> + TAILQ_FOREACH(bus, &rte_bus_list, next) {
> + ret = bus->scan(bus);
> + if (ret) {
> + RTE_LOG(ERR, EAL, "Scan for (%s) bus failed.\n",
> + bus->name);
> + /* TODO: Should error on a particular bus block scan
> + * for all others?
> + */
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
>
> Nitpick - the return type of rte_eal_bus_scan() is int and not void:
Thanks for review. I will fix this before sending v6.
>
> * Scan all the buses attached to the framework.
> + *
> + * @param void
> + * @return void
> + */
> +int rte_eal_bus_scan(void);
>
>
> Rami Rosen
>
-
Shreyansh
^ permalink raw reply
* Re: [PATCH v5 04/12] eal: integrate bus scan and probe with EAL
From: Shreyansh Jain @ 2017-01-09 6:35 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <1912570.qtcMvjyK2L@xps13>
On Friday 06 January 2017 07:16 PM, Thomas Monjalon wrote:
> 2017-01-06 17:30, Shreyansh Jain:
>> On Friday 06 January 2017 04:08 PM, Shreyansh Jain wrote:
>>> On Wednesday 04 January 2017 03:16 AM, Thomas Monjalon wrote:
>>>> 2016-12-26 18:53, Shreyansh Jain:
>>>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>>>> @@ -844,6 +845,9 @@ rte_eal_init(int argc, char **argv)
>>>>> if (rte_eal_intr_init() < 0)
>>>>> rte_panic("Cannot init interrupt-handling thread\n");
>>>>>
>>>>> + if (rte_eal_bus_scan())
>>>>> + rte_panic("Cannot scan the buses for devices\n");
>>>>
>>>> Yes, definitely. Just one scan functions which scan registered bus.
>>>>
>>>>> @@ -884,6 +888,9 @@ rte_eal_init(int argc, char **argv)
>>>>> if (rte_eal_pci_probe())
>>>>> rte_panic("Cannot probe PCI\n");
>>>>>
>>>>> + if (rte_eal_bus_probe())
>>>>> + rte_panic("Cannot probe devices\n");
>>>>> +
>>>>> if (rte_eal_dev_init() < 0)
>>>>> rte_panic("Cannot init pmd devices\n");
>>>>
>>>> What is the benefit of initializing (probe) a device outside of the scan?
>>>> Currently, it is done in two steps, so you are keeping the same
>>>> behaviour.
>>>
>>> Yes, only for compatibility to existing model of two-step process.
>>> Ideally, only a single step is enough (init->probe).
>>>
>>> During the discussion in [1] also this point was raised - at that time
>>> for VDEV and applicability to PCI.
>>>
>>> [1] http://dpdk.org/ml/archives/dev/2016-December/051306.html
>>>
>>> If you want, I can merge these two. I postponed it because 1) it is an
>>> independent change and should really impact bus and 2) I was not sure
>>> of dependency of init *before* pthread_create for all workers.
>>
>> I forgot _not_ in above - rephrasing:
>>
>> If you want, I can merge these two. I postponed it because 1) it is an
>> independent change and should _not_ really impact bus and 2) I was not
>> sure of dependency of init *before* pthread_create for all workers.
>
> I'm OK with your approach.
>
>>>> I imagine a model where the scan function decide to initialize the
>>>> device and can require some help from a callback to make this decision.
>>>> So the whitelist/blacklist policy can be implemented with callbacks at
>>>> the scan level and possibly the responsibility of the application.
>>>> Note that the callback model would be a change for a next release.
>>>
>>> Agree. But, that is not really part of Bus patches - isn't it? Or, you
>>> want to change that with this series?
>
> No it is not the scope of this series.
> Please could you add it in the cover letter as a next step?
Yes, I will add to cover letter as Pending Item.
> Thanks
>
^ permalink raw reply
* Re: [PATCH] doc: add known issue for uio_pci_generic in XL710
From: Wu, Jingjing @ 2017-01-09 7:16 UTC (permalink / raw)
To: Guo, Jia, Zhang, Helin; +Cc: dev@dpdk.org
In-Reply-To: <1482460523-74460-1-git-send-email-jia.guo@intel.com>
> -----Original Message-----
> From: Guo, Jia
> Sent: Friday, December 23, 2016 10:35 AM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: [PATCH] doc: add known issue for uio_pci_generic in XL710
>
> When bind with uio_pci_generic in XL710, the result is failed.
> uio_pci_generic is not supported by XL710.
>
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
^ permalink raw reply
* Re: [PATCH v5 1/5] ethdev: add firmware version get
From: Yang, Qiming @ 2017-01-09 7:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev@dpdk.org, Yigit, Ferruh, Zhang, Helin, Horton, Remy
In-Reply-To: <20170108150513.6a57d4f0@xeon-e3>
-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Monday, January 9, 2017 7:05 AM
To: Yang, Qiming <qiming.yang@intel.com>
Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>
Subject: Re: [dpdk-dev] [PATCH v5 1/5] ethdev: add firmware version get
On Sun, 8 Jan 2017 12:11:31 +0800
Qiming Yang <qiming.yang@intel.com> wrote:
> void
> +rte_eth_dev_fw_version_get(uint8_t port_id, char *fw_version, int
> +fw_length) {
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_RET(port_id);
> + dev = &rte_eth_devices[port_id];
> +
> + RTE_FUNC_PTR_OR_RET(*dev->dev_ops->fw_version_get);
> + (*dev->dev_ops->fw_version_get)(dev, fw_version, fw_length); }
Maybe dev argument to fw_version_get should be:
const struct rte_eth_dev *dev
Qiming: do you means the argument to ops fw_version_get?
why should add 'const'? both two are OK, but we usually use struct rte_eth_dev *dev.
^ permalink raw reply
* [PATCH v4] ethdev: fix port data mismatched in multiple process model
From: Yuanhan Liu @ 2017-01-09 7:50 UTC (permalink / raw)
To: dev; +Cc: Yuanhan Liu, stable, Thomas Monjalon, Bruce Richardson,
Ferruh Yigit
In-Reply-To: <1483697780-12088-2-git-send-email-yuanhan.liu@linux.intel.com>
Assume we have two virtio ports, 00:03.0 and 00:04.0. The first one is
managed by the kernel driver, while the later one is managed by DPDK.
Now we start the primary process. 00:03.0 will be skipped by DPDK virtio
PMD driver (since it's being used by the kernel). 00:04.0 would be
successfully initiated by DPDK virtio PMD (if nothing abnormal happens).
After that, we would get a port id 0, and all the related info needed
by virtio (virtio_hw) is stored at rte_eth_dev_data[0].
Then we start the secondary process. As usual, 00:03.0 will be firstly
probed. It firstly tries to get a local eth_dev structure for it (by
rte_eth_dev_allocate):
port_id = rte_eth_dev_find_free_port();
...
eth_dev = &rte_eth_devices[port_id];
eth_dev->data = &rte_eth_dev_data[port_id];
...
return eth_dev;
Since it's a first PCI device, port_id will be 0. eth_dev->data would
then point to rte_eth_dev_data[0]. And here things start going wrong,
as rte_eth_dev_data[0] actually stores the virtio_hw for 00:04.0.
That said, in the secondary process, DPDK will continue to drive PCI
device 00.03.0 (despite the fact it's been managed by kernel), with
the info from PCI device 00:04.0. Which is wrong.
The fix is to attach the port already registered by the primary process:
iterate the rte_eth_dev_data[], and get the port id who's PCI ID matches
the current PCI device.
This would let us maintain same port ID for the same PCI device, keeping
the chance of referencing to wrong data minimal.
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
v4: - assign eth_dev in the common eth_dev init help function
it also renamed to eth_dev_get, to not confuse with the
eth_dev_init callback.
- move primoary process specific assignments to rte_eth_dev_allocate
- drop the virtio example in comment
- combine two code block for primary into one
v3: - do not move rte_eth_dev_data_alloc to pci_probe
- rename eth_dev_attach to eth_dev_attach_secondary
- introduce eth_dev_init() for common eth_dev struct initiation
- move comment block inside the "if" block
---
lib/librte_ether/rte_ethdev.c | 71 +++++++++++++++++++++++++++++++++++++------
1 file changed, 62 insertions(+), 9 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..1453df1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -189,6 +189,19 @@ struct rte_eth_dev *
return RTE_MAX_ETHPORTS;
}
+static struct rte_eth_dev *
+eth_dev_get(uint8_t port_id)
+{
+ struct rte_eth_dev *eth_dev = &rte_eth_devices[port_id];
+
+ eth_dev->data = &rte_eth_dev_data[port_id];
+ eth_dev->attached = DEV_ATTACHED;
+ eth_dev_last_created_port = port_id;
+ nb_ports++;
+
+ return eth_dev;
+}
+
struct rte_eth_dev *
rte_eth_dev_allocate(const char *name)
{
@@ -210,13 +223,41 @@ struct rte_eth_dev *
return NULL;
}
- eth_dev = &rte_eth_devices[port_id];
- eth_dev->data = &rte_eth_dev_data[port_id];
+ eth_dev = eth_dev_get(port_id);
snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
eth_dev->data->port_id = port_id;
- eth_dev->attached = DEV_ATTACHED;
- eth_dev_last_created_port = port_id;
- nb_ports++;
+
+ return eth_dev;
+}
+
+/*
+ * Attach to a port already registered by the primary process, which
+ * makes sure that the same device would have the same port id both
+ * in the primary and secondary process.
+ */
+static struct rte_eth_dev *
+eth_dev_attach_secondary(const char *name)
+{
+ uint8_t i;
+ struct rte_eth_dev *eth_dev;
+
+ if (rte_eth_dev_data == NULL)
+ rte_eth_dev_data_alloc();
+
+ for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ if (strcmp(rte_eth_dev_data[i].name, name) == 0)
+ break;
+ }
+ if (i == RTE_MAX_ETHPORTS) {
+ RTE_PMD_DEBUG_TRACE(
+ "device %s is not driven by the primary process\n",
+ name);
+ return NULL;
+ }
+
+ eth_dev = eth_dev_get(i);
+ RTE_ASSERT(eth_dev->data->port_id == i);
+
return eth_dev;
}
@@ -246,16 +287,28 @@ struct rte_eth_dev *
rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
sizeof(ethdev_name));
- eth_dev = rte_eth_dev_allocate(ethdev_name);
- if (eth_dev == NULL)
- return -ENOMEM;
-
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ eth_dev = rte_eth_dev_allocate(ethdev_name);
+ if (eth_dev == NULL)
+ return -ENOMEM;
+
eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
eth_drv->dev_private_size,
RTE_CACHE_LINE_SIZE);
if (eth_dev->data->dev_private == NULL)
rte_panic("Cannot allocate memzone for private port data\n");
+ } else {
+ eth_dev = eth_dev_attach_secondary(ethdev_name);
+ if (eth_dev == NULL) {
+ /*
+ * if we failed to attach a device, it means the
+ * device is skipped in primary process, due to
+ * some errors. If so, we return a positive value,
+ * to let EAL skip it for the secondary process
+ * as well.
+ */
+ return 1;
+ }
}
eth_dev->pci_dev = pci_dev;
eth_dev->driver = eth_drv;
--
1.9.0
^ permalink raw reply related
* Re: [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process
From: Xu, Qian Q @ 2017-01-09 8:02 UTC (permalink / raw)
To: Yuanhan Liu, Stephen Hemminger, Wei, FangfangX
Cc: dev@dpdk.org, stable@dpdk.org, Liu, Yong
In-Reply-To: <20170109051919.GB21228@yliu-dev.sh.intel.com>
Fangfang,
Could you help double confirm below error and the patchset ? Thanks.
http://dpdk.org/patch/18975
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, January 9, 2017 1:19 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; stable@dpdk.org; Xu, Qian Q <qian.q.xu@intel.com>; Liu,
> Yong <yong.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for
> secondary process
>
> On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> > On Fri, 6 Jan 2017 18:16:16 +0800
> > Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >
> > > If the primary enables the vector Rx/Tx path, the current code would
> > > let the secondary always choose the non vector Rx/Tx path. This
> > > results to a Rx/Tx method mismatch between primary and secondary
> > > process. Werid errors then may happen, something like:
> > >
> > > PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> > >
> > > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > > That is, use vector path if it's given.
> > >
> > > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> > >
> > > Cc: stable@dpdk.org
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > This is failing on intel compile tests.
> >
> >
> > http://dpdk.org/patch/18975
>
> Thanks, but it looks like a false alarm to me, for reasons below.
>
> > Failed Build #2:
> > OS: RHEL7.2_64
> > Target: x86_64-native-linuxapp-gcc
> > MKRES test_resource_c.res.o /home/patchWorkOrg/compilation/x86_64-
> native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function
> `rte_eth_dev_pci_probe':
> > rte_ethdev.c:(.text+0x9c4): undefined reference to
> `eth_dev_attach_secondary'
> > collect2: error: ld returned 1 exit status
>
> - eth_dev_attach_secondary is not defined in this patch, it's defined
> (and used) in the first patch.
>
> - eth_dev_attach_secondary is actually defined; The report even shows
> it fails to build with gcc: the gcc build passes on my dev box.
>
> Honestly, I seldom trusted the build reports from STV.
>
> --yliu
^ permalink raw reply
* Re: [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process
From: Wei, FangfangX @ 2017-01-09 8:05 UTC (permalink / raw)
To: Xu, Qian Q, Yuanhan Liu, Stephen Hemminger
Cc: dev@dpdk.org, stable@dpdk.org, Liu, Yong
In-Reply-To: <82F45D86ADE5454A95A89742C8D1410E3B4D643B@shsmsx102.ccr.corp.intel.com>
I've re-build it manually, it passed with this patch.
-----Original Message-----
From: Xu, Qian Q
Sent: Monday, January 9, 2017 4:02 PM
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger <stephen@networkplumber.org>; Wei, FangfangX <fangfangx.wei@intel.com>
Cc: dev@dpdk.org; stable@dpdk.org; Liu, Yong <yong.liu@intel.com>
Subject: RE: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process
Fangfang,
Could you help double confirm below error and the patchset ? Thanks.
http://dpdk.org/patch/18975
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, January 9, 2017 1:19 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; stable@dpdk.org; Xu, Qian Q <qian.q.xu@intel.com>;
> Liu, Yong <yong.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx
> method for secondary process
>
> On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> > On Fri, 6 Jan 2017 18:16:16 +0800
> > Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >
> > > If the primary enables the vector Rx/Tx path, the current code
> > > would let the secondary always choose the non vector Rx/Tx path.
> > > This results to a Rx/Tx method mismatch between primary and
> > > secondary process. Werid errors then may happen, something like:
> > >
> > > PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> > >
> > > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > > That is, use vector path if it's given.
> > >
> > > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> > >
> > > Cc: stable@dpdk.org
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > This is failing on intel compile tests.
> >
> >
> > http://dpdk.org/patch/18975
>
> Thanks, but it looks like a false alarm to me, for reasons below.
>
> > Failed Build #2:
> > OS: RHEL7.2_64
> > Target: x86_64-native-linuxapp-gcc
> > MKRES test_resource_c.res.o /home/patchWorkOrg/compilation/x86_64-
> native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function
> `rte_eth_dev_pci_probe':
> > rte_ethdev.c:(.text+0x9c4): undefined reference to
> `eth_dev_attach_secondary'
> > collect2: error: ld returned 1 exit status
>
> - eth_dev_attach_secondary is not defined in this patch, it's defined
> (and used) in the first patch.
>
> - eth_dev_attach_secondary is actually defined; The report even shows
> it fails to build with gcc: the gcc build passes on my dev box.
>
> Honestly, I seldom trusted the build reports from STV.
>
> --yliu
^ permalink raw reply
* Re: [PATCH v2] doc: fix a link in contribution guide
From: Mcnamara, John @ 2017-01-09 8:33 UTC (permalink / raw)
To: Yongseok Koh; +Cc: dev@dpdk.org
In-Reply-To: <20170106224010.24752-1-yskoh@mellanox.com>
> -----Original Message-----
> From: Yongseok Koh [mailto:yskoh@mellanox.com]
> Sent: Friday, January 6, 2017 10:40 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>
> Subject: [PATCH v2] doc: fix a link in contribution guide
>
> A referenced document in the Linux Kernel has been moved to a sub-
> directory. And kernel community has moved to RST/Sphinx. The links are
> replaced with HTML rendered links.
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH] doc: fix a typo in testpmd application guide.
From: Mcnamara, John @ 2017-01-09 8:35 UTC (permalink / raw)
To: Rosen, Rami, dev@dpdk.org; +Cc: Rosen, Rami
In-Reply-To: <1483798084-14071-1-git-send-email-rami.rosen@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rami Rosen
> Sent: Saturday, January 7, 2017 2:08 PM
> To: dev@dpdk.org
> Cc: Rosen, Rami <rami.rosen@intel.com>
> Subject: [dpdk-dev] [PATCH] doc: fix a typo in testpmd application guide.
>
> This patch fixes a trivial typo in testpmd application guide.
>
> Signed-off-by: Rami Rosen <rami.rosen@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH v2 2/3] lib/librte_cryptodev: functions for new performance test application
From: Mrozowicz, SlawomirX @ 2017-01-09 9:11 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org; +Cc: Doherty, Declan, Kerlin, Marcin
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D897476BF528@IRSMSX108.ger.corp.intel.com>
>-----Original Message-----
>From: De Lara Guarch, Pablo
>Sent: Friday, January 6, 2017 1:22 PM
>To: Mrozowicz, SlawomirX <slawomirx.mrozowicz@intel.com>;
>dev@dpdk.org
>Cc: Mrozowicz, SlawomirX <slawomirx.mrozowicz@intel.com>; Doherty,
>Declan <declan.doherty@intel.com>; Kerlin, Marcin
><marcin.kerlin@intel.com>
>Subject: RE: [dpdk-dev] [PATCH v2 2/3] lib/librte_cryptodev: functions for
>new performance test application
>
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Slawomir
>> Mrozowicz
>> Sent: Thursday, January 05, 2017 4:50 PM
>> To: dev@dpdk.org
>> Cc: Mrozowicz, SlawomirX; Doherty, Declan; Kerlin, Marcin
>> Subject: [dpdk-dev] [PATCH v2 2/3] lib/librte_cryptodev: functions for
>> new performance test application
>>
>> This patch adds helper functions for new performance application.
>> Application can be used to measute throughput and latency of
>> cryptography operation performed by crypto device.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
>> Signed-off-by: Marcin Kerlin <marcinx.kerlin@intel.com>
>
>Hi Slawomir,
>
>You should change the title of this patch to something like: "cryptodev: add
>functions for...".
>Also, maybe it would be better to say "add functions to retrieve device info",
>since you are not saying much with the current title.
>
>I have seen also that this is based on the SGL code, so this patch didn't apply
>cleanly.
>Next time, specify it in the email the dependencies of the patch.
>
>Thanks,
>Pablo
>
I am going to change the title of this patch to:
"cryptodev: add functions to retrieve device info"
Is it ok for you?
I will fix the problem with correct patch applying.
Sławomir
^ permalink raw reply
* Re: Bug in virtqueue_dequeue_burst_rx()
From: Gopakumar Choorakkot Edakkunni @ 2017-01-09 9:38 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: huawei.xie, dev
In-Reply-To: <20161222073834.GZ18991@yliu-dev.sh.intel.com>
Hi Yuanahn,
Thanks for the response. Because of my filters, I missed this email
completely, apologies for the late response ! I haven't tried breaking out
of the loop to see if it will go back to working state - I have two
testbeds, one using plain linux kvm + guest VM running 16.07 dpdk where I
see the problem. I have another testved where I use ovs-dpdk + the same
guest VM image running 16.07 dpdk. The issue happens only on the linux kvm
setup, so for now I just switched to using ovs-dpdk instead. I will go back
sometime to the linux kvm setup and try breaking of the loop and see what
happens.
Without hardly any knowledge of virtio myself, I am guessing this issue is
because the "host" said it has put one buffer into the virtio ring whereas
it really did not put the buffer, right ? So is the issue likely to be in
the host virtio layer rather than in guest dpdk ?
As for the linux-kvm setup, the trigger is very straightforward. Its just
sending traffic. The details of my setup are as below. If you have a
similar setup, I can possibly try and help you recreate the issue or maybe
even provide access to my testbed if you would like that.
1. The guest VM runs dpdk 16.07 and has 8 virtio interfaces mapping to 8
different bridges on the host. The VM has 8Gig memory with 1Gig hugepage
size and 1 hugepage (1gig memory) given to dpdk. And the VM is backed up by
16 x 1Gig hugepages on the host. I am attaching the guest XML file here
2. The host runs the below version. And all that I do is send iperf traffic
that comes in on one of the 8 interfaces in the guest VM and goes out of
another interface in the guest VM.
root@ubuntu:/home/vcadmin# uname -a
Linux ubuntu 3.16.0-77-generic #99~14.04.1-Ubuntu SMP Tue Jun 28 19:17:10
UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
root@ubuntu:/home/vcadmin# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 14.04.5 LTS
Release: 14.04
Codename: trusty
root@ubuntu:/home/vcadmin# cat /proc/meminfo | grep Huge
AnonHugePages: 141312 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd: 0
HugePages_Surp: 0
Hugepagesize: 1048576 kB
Rgds,
Gopa.
On Wed, Dec 21, 2016 at 11:38 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com>
wrote:
> On Mon, Dec 19, 2016 at 09:59:33PM -0800, Gopakumar Choorakkot Edakkunni
> wrote:
> > While I was testing virtio with ubuntu 14.04 kvm as host and dpdk16.07
> > linux as guest, quite often I have seen that I get into a situation where
> > virtio_recv_mergeable_pkts() gets into a forever loop, after sending
> > traffic for a while. In the below API, I see that it clearly leads to a
> > while loop, I am not quite familiar with virtio or mergeable buffers, so
> > thought of checking with dpdk alias on the intent here.
> >
> > I checked the linux kernel virtio_net.c file which does the similar
> > mergeable recieve, and the kernel code breaks out in case of error.
> > Shouldnt dpdk be breaking out of here on error instead of continue ?
>
> Yep, that looks buggy.
>
> >
> > virtio_recv_mergeable_pkts()
> > {
> > <snip>
> > while (i < nb_used) {
> > <snip>
> > * num = virtqueue_dequeue_burst_rx(rxvq, rcv_pkts, len, 1);*
> > * if (num != 1)*
> > * continue;*
>
> However, normally, virtqueue_dequeue_burst_rx() would be successful
> here: the outer check (i < nb_used) somehow ascertains it.
>
> Two options I can think of so far:
>
> - will it go back working once you do "break" here?
>
> - tell us how you managed to trigger this issue, so that I could
> also reproduce it that I can debug it.
>
> --yliu
>
^ permalink raw reply
* Re: [PATCH v7 26/27] net/i40e: fix segmentation fault in close
From: Wu, Jingjing @ 2017-01-09 9:44 UTC (permalink / raw)
To: Iremonger, Bernard, Lu, Wenzhuo, dev@dpdk.org; +Cc: stable@dpdk.org
In-Reply-To: <8CEF83825BEC744B83065625E567D7C224D1B4CF@IRSMSX108.ger.corp.intel.com>
>
> All of the VSI's are released in the call to i40e_vsi_release(pf->main_vsi) at line
> 1895.
> This function is recursive and release all the VSI's.
>
> There is still a VSI address in pf->vmdq[i].vsi but calling i40e_vsi_release(pf-
> >vmdq[i].vsi);
> Results in a segmentation fault.
>
Thanks for the clarification.
You are correct. What I prefer is to move the code to release vmdq vsis to
before the i40e_vsi_release(pf->main_vsi);
What do you think?
Thanks
Jingjing
^ permalink raw reply
* Re: [PATCH v2 0/4] eal/common: introduce rte_memset and related test
From: Yang, Zhiyong @ 2017-01-09 9:48 UTC (permalink / raw)
To: thomas.monjalon@6wind.com, Richardson, Bruce, Ananyev, Konstantin
Cc: yuanhan.liu@linux.intel.com, De Lara Guarch, Pablo, dev@dpdk.org
In-Reply-To: <1482833098-38096-1-git-send-email-zhiyong.yang@intel.com>
Hi, Thomas, Bruce, Konstantin:
Any comments about the patchset? Do I need to modify anything?
Thanks
Zhiyong
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhiyong Yang
> Sent: Tuesday, December 27, 2016 6:05 PM
> To: dev@dpdk.org
> Cc: yuanhan.liu@linux.intel.com; thomas.monjalon@6wind.com; Richardson,
> Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev] [PATCH v2 0/4] eal/common: introduce rte_memset and
> related test
>
> DPDK code has met performance drop badly in some case when calling glibc
> function memset. Reference to discussions about memset in
> http://dpdk.org/ml/archives/dev/2016-October/048628.html
> It is necessary to introduce more high efficient function to fix it.
> One important thing about rte_memset is that we can get clear control on
> what instruction flow is used.
>
> This patchset introduces rte_memset to bring more high efficient
> implementation, and will bring obvious perf improvement, especially for
> small N bytes in the most application scenarios.
>
> Patch 1 implements rte_memset in the file rte_memset.h on IA platform The
> file supports three types of instruction sets including sse & avx (128bits),
> avx2(256bits) and avx512(512bits). rte_memset makes use of vectorization
> and inline function to improve the perf on IA. In addition, cache line and
> memory alignment are fully taken into consideration.
>
> Patch 2 implements functional autotest to validates the function whether to
> work in a right way.
>
> Patch 3 implements performance autotest separately in cache and memory.
> We can see the perf of rte_memset is obviously better than glibc memset
> especially for small N bytes.
>
> Patch 4 Using rte_memset instead of copy_virtio_net_hdr can bring 3%~4%
> performance improvements on IA platform from virtio/vhost non-mergeable
> loopback testing.
>
> Changes in V2:
>
> Patch 1:
> Rename rte_memset.h -> rte_memset_64.h and create a file rte_memset.h
> for each arch.
>
> Patch 3:
> add the perf comparation data between rte_memset and memset on
> haswell.
>
> Patch 4:
> Modify release_17_02.rst description.
>
> Zhiyong Yang (4):
> eal/common: introduce rte_memset on IA platform
> app/test: add functional autotest for rte_memset
> app/test: add performance autotest for rte_memset
> lib/librte_vhost: improve vhost perf using rte_memset
>
> app/test/Makefile | 3 +
> app/test/test_memset.c | 158 +++++++++
> app/test/test_memset_perf.c | 348 +++++++++++++++++++
> doc/guides/rel_notes/release_17_02.rst | 7 +
> .../common/include/arch/arm/rte_memset.h | 36 ++
> .../common/include/arch/ppc_64/rte_memset.h | 36 ++
> .../common/include/arch/tile/rte_memset.h | 36 ++
> .../common/include/arch/x86/rte_memset.h | 51 +++
> .../common/include/arch/x86/rte_memset_64.h | 378
> +++++++++++++++++++++
> lib/librte_eal/common/include/generic/rte_memset.h | 52 +++
> lib/librte_vhost/virtio_net.c | 18 +-
> 11 files changed, 1116 insertions(+), 7 deletions(-) create mode 100644
> app/test/test_memset.c create mode 100644 app/test/test_memset_perf.c
> create mode 100644
> lib/librte_eal/common/include/arch/arm/rte_memset.h
> create mode 100644
> lib/librte_eal/common/include/arch/ppc_64/rte_memset.h
> create mode 100644 lib/librte_eal/common/include/arch/tile/rte_memset.h
> create mode 100644
> lib/librte_eal/common/include/arch/x86/rte_memset.h
> create mode 100644
> lib/librte_eal/common/include/arch/x86/rte_memset_64.h
> create mode 100644 lib/librte_eal/common/include/generic/rte_memset.h
>
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v7 26/27] net/i40e: fix segmentation fault in close
From: Iremonger, Bernard @ 2017-01-09 9:50 UTC (permalink / raw)
To: Wu, Jingjing, Lu, Wenzhuo, dev@dpdk.org; +Cc: stable@dpdk.org
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC5369@SHSMSX103.ccr.corp.intel.com>
Hi Jingjing,
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Monday, January 9, 2017 9:44 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v7 26/27] net/i40e: fix segmentation fault in
> close
>
> >
> > All of the VSI's are released in the call to
> > i40e_vsi_release(pf->main_vsi) at line 1895.
> > This function is recursive and release all the VSI's.
> >
> > There is still a VSI address in pf->vmdq[i].vsi but calling
> > i40e_vsi_release(pf-
> > >vmdq[i].vsi);
> > Results in a segmentation fault.
> >
> Thanks for the clarification.
>
> You are correct. What I prefer is to move the code to release vmdq vsis to
> before the i40e_vsi_release(pf->main_vsi);
>
> What do you think?
>
> Thanks
> Jingjing
I will test it and see what happens.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v5 1/5] ethdev: add firmware version get
From: Remy Horton @ 2017-01-09 10:01 UTC (permalink / raw)
To: Yang, Qiming, Stephen Hemminger; +Cc: dev@dpdk.org, Yigit, Ferruh, Zhang, Helin
In-Reply-To: <F5DF4F0E3AFEF648ADC1C3C33AD4DBF16EDCA9C1@SHSMSX101.ccr.corp.intel.com>
On 09/01/2017 07:16, Yang, Qiming wrote:
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
[..]
>> void
>> +rte_eth_dev_fw_version_get(uint8_t port_id, char *fw_version, int
>> +fw_length) {
>> + struct rte_eth_dev *dev;
>> +
>> + RTE_ETH_VALID_PORTID_OR_RET(port_id);
>> + dev = &rte_eth_devices[port_id];
>> +
>> + RTE_FUNC_PTR_OR_RET(*dev->dev_ops->fw_version_get);
>> + (*dev->dev_ops->fw_version_get)(dev, fw_version, fw_length); }
>
> Maybe dev argument to fw_version_get should be:
> const struct rte_eth_dev *dev
> Qiming: do you means the argument to ops fw_version_get?
> why should add 'const'? both two are OK, but we usually use struct rte_eth_dev *dev.
Does seem a bit odd to me as I don't think any of the other rte_dev_ops
entrypoints use const. Maybe they should but if that's now policy (I've
been under a rock recently) probably better to do them all in a seperate
cleanup patchset..
^ permalink raw reply
* [PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer
From: Wenfeng Liu @ 2017-01-09 10:24 UTC (permalink / raw)
To: olivier.matz; +Cc: dev
We put objects to cache when the mempool is multiple producer, however the cache will not be used when it is single consumer.
With this patch we can get objects from cache when the single consumer is happen to be one of the producers, and this improves performance.
Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
---
lib/librte_mempool/rte_mempool.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index d315d42..4ab5a95 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -1250,8 +1250,9 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
uint32_t index, len;
void **cache_objs;
- /* No cache provided or single consumer */
- if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
+ /* No cache provided or single consumer and single producer */
+ if (unlikely(cache == NULL ||
+ (flags & MEMPOOL_F_SC_GET) && (flags & MEMPOOL_F_SP_PUT) ||
n >= cache->size))
goto ring_dequeue;
--
2.7.4
^ permalink raw reply related
* test
From: Liu, Yu Y @ 2017-01-09 10:26 UTC (permalink / raw)
To: dev@dpdk.org
^ permalink raw reply
* Re: [PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer
From: Ananyev, Konstantin @ 2017-01-09 10:36 UTC (permalink / raw)
To: Wenfeng Liu, olivier.matz@6wind.com; +Cc: dev@dpdk.org
In-Reply-To: <1483957487-92635-1-git-send-email-liuwf@arraynetworks.com.cn>
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenfeng Liu
> Sent: Monday, January 9, 2017 10:25 AM
> To: olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] mempool: try to get objects from cache when the mempool is single consumer and multiple producer
>
> We put objects to cache when the mempool is multiple producer, however the cache will not be used when it is single consumer.
> With this patch we can get objects from cache when the single consumer is happen to be one of the producers, and this improves
> performance.
>
> Signed-off-by: Wenfeng Liu <liuwf@arraynetworks.com.cn>
> ---
> lib/librte_mempool/rte_mempool.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index d315d42..4ab5a95 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -1250,8 +1250,9 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table,
> uint32_t index, len;
> void **cache_objs;
>
> - /* No cache provided or single consumer */
> - if (unlikely(cache == NULL || flags & MEMPOOL_F_SC_GET ||
> + /* No cache provided or single consumer and single producer */
> + if (unlikely(cache == NULL ||
> + (flags & MEMPOOL_F_SC_GET) && (flags & MEMPOOL_F_SP_PUT) ||
I suppose that's a good thing to do...
Might be go one step further and don't check flags at all?
if (unlikely(cache == NULL || n >= cache->size)
goto ring_dequeue;
If people don't want to have a mempool with cache,
they can just specify it at mempool creation time.
Again cache might improve performance even for SC|SP case.
Konstantin
> n >= cache->size))
> goto ring_dequeue;
>
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH v2] doc: add tso capabilities feature for mlx5
From: Ferruh Yigit @ 2017-01-09 10:52 UTC (permalink / raw)
To: Elad Persiko, dev
In-Reply-To: <1483892601-6997-1-git-send-email-eladpe@mellanox.com>
On 1/8/2017 4:23 PM, Elad Persiko wrote:
> Feature implemented at:
> commit b007e98ccda9 ("net/mlx5: implement TSO data path")
> commit 085c4137280a ("net/mlx5: support TSO in control plane")
Hi Elad,
Instead of send this a separate patch, it is better if you can send
these updates with the patch that adds the relevant feature.
So, you won't have to reference the commit ids here..
>
> Signed-off-by: Elad Persiko <eladpe@mellanox.com>
> ---
> doc/guides/nics/features/mlx4.ini | 1 +
> doc/guides/nics/features/mlx5.ini | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/doc/guides/nics/features/mlx4.ini b/doc/guides/nics/features/mlx4.ini
> index c9828f7..d74b9dd 100644
> --- a/doc/guides/nics/features/mlx4.ini
> +++ b/doc/guides/nics/features/mlx4.ini
> @@ -10,6 +10,7 @@ Queue start/stop = Y
> MTU update = Y
> Jumbo frame = Y
> Scattered Rx = Y
> +TSO = Y
> Promiscuous mode = Y
> Allmulticast mode = Y
> Unicast MAC filter = Y
> diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini
> index f811e3f..f8a215e 100644
> --- a/doc/guides/nics/features/mlx5.ini
> +++ b/doc/guides/nics/features/mlx5.ini
> @@ -11,6 +11,7 @@ Queue start/stop = Y
> MTU update = Y
> Jumbo frame = Y
> Scattered Rx = Y
> +TSO = Y
> Promiscuous mode = Y
> Allmulticast mode = Y
> Unicast MAC filter = Y
>
^ permalink raw reply
* Re: [PATCH v1 2/2] Test cases for rte_memcmp functions
From: Thomas Monjalon @ 2017-01-09 11:08 UTC (permalink / raw)
To: Wang, Zhihong; +Cc: Ravi Kerur, dev
In-Reply-To: <8F6C2BD409508844A0EFC19955BE0941512003E4@SHSMSX103.ccr.corp.intel.com>
2017-01-09 05:29, Wang, Zhihong:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-06-07 11:09, Wang, Zhihong:
> > > From: Ravi Kerur [mailto:rkerur@gmail.com]
> > > > Zhilong, Thomas,
> > > >
> > > > If there is enough interest within DPDK community I can work on adding
> > support
> > > > for 'unaligned access' and 'test cases' for it. Please let me know either
> > way.
> > >
> > > Hi Ravi,
> > >
> > > This rte_memcmp is proved with better performance than glibc's in aligned
> > > cases, I think it has good value to DPDK lib.
> > >
> > > Though we don't have memcmp in critical pmd data path, it offers a better
> > > choice for applications who do.
> >
> > Re-thinking about this series, could it be some values to have a rte_memcmp
> > implementation?
>
> I think this series (rte_memcmp included) could help:
>
> 1. Potentially better performance in hot paths.
>
> 2. Agile for tuning.
>
> 3. Avoid performance complications -- unusual but possible,
> like the glibc memset issue I met while working on vhost
> enqueue.
>
> > What is the value compared to glibc one? Why not working on glibc?
>
> As to working on glibc, wider design consideration and test
> coverage might be needed, and we'll face different release
> cycles, can we have the same agility? Also working with old
> glibc could be a problem.
Probably we need both: add the optimized version in DPDK while working
on a glibc optimization.
This strategy could be applicable to memcpy, memcmp and memset.
^ 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