* Re: [PATCH v1 0/4] Generalize PCI specific EAL function/structures
From: Thomas Monjalon @ 2016-10-03 13:36 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: David Marchand, dev, Jan Viktorin
In-Reply-To: <4e8b181f-8d46-f4b4-3821-a19f5c622ec1@nxp.com>
2016-10-03 11:07, Shreyansh Jain:
> Hi David,
>
> On Friday 30 September 2016 09:01 PM, David Marchand wrote:
> > On Tue, Sep 27, 2016 at 4:12 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >> (I rebased these over HEAD 7b3c4f3)
> >>
> >> These patches were initially part of Jan's original series on SoC
> >> Framework ([1],[2]). An update to that series, without these patches,
> >> was posted here [3].
> >>
> >> Main motivation for these is aim of introducing a non-PCI centric
> >> subsystem in EAL. As of now the first usecase is SoC, but not limited to
> >> it.
> >>
> >> 4 patches in this series are independent of each other, as well as SoC
> >> framework. All these focus on generalizing some structure or functions
> >> present with the PCI specific code to EAL Common area (or splitting a
> >> function to be more userful).
> >
> > Those patches move linux specifics (binding pci devices using sysfs)
> > to common infrastucture.
> > We have no proper hotplug support on bsd, but if we had some common
> > code we should at least try to make the apis generic.
> >
>
> I am not sure if I understood your point well. Just to confirm - you are
> stating that the movement done in the patches might not suit BSD.
> Probably you are talking about (Patch 3/4 and 4/4).
> Is my understanding correct?
>
> So, movement to just Linux area is not enough?
> I am not well versed with BSD way of doing something similar so if
> someone can point it out, I can integrate that. (I will investigate it
> at my end as well).
>
> This patchset makes the PCI->EAL movement *only* for Linux for sysfs
> bind/unbind. (I should add this to cover letter, at the least).
The concern is about function declarations in
lib/librte_eal/common/eal_private.h
We cannot be sure it can be applicable to something else than Linux.
As it is implemented in Linux only, it should not be in a common header.
^ permalink raw reply
* Re: [PATCH] eal: fix crash on mmap error in rte_eal_hugepage_attach()
From: Thomas Monjalon @ 2016-10-03 14:06 UTC (permalink / raw)
To: maciej.czekaj; +Cc: dev, Sergio Gonzalez Monroy, david.marchand
In-Reply-To: <6c2618d5-3f54-7c84-aa92-498461842f6a@intel.com>
2016-10-03 14:04, Sergio Gonzalez Monroy:
> On 28/09/2016 11:52, maciej.czekaj@caviumnetworks.com wrote:
> > From: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> >
> > In ASLR-enabled system, it is possible that selected
> > virtual space is occupied by program segments. Therefore,
> > error path should not blindly unmap all memmory segments
> > but only those already mapped.
> >
> > Steps that lead to crash:
> > 1. memeseg 0 in secondary process overlaps
> > with libc.so
> > 2. mmap of /dev/zero fails for virtual space of memseg 0
> > 3. munmap of memseg 0 leads to unmapping libc.so itself
> > 4. app gets SIGSEGV after returning from syscall to libc
> >
> > Fixes: ea329d7f8e34 ("mem: fix leak after mapping failure")
> >
> > Signed-off-by: Maciej Czekaj <maciej.czekaj@caviumnetworks.com>
> > ---
> > lib/librte_eal/linuxapp/eal/eal_memory.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
>
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] eal: remove single file segments related code
From: Thomas Monjalon @ 2016-10-03 14:08 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Sergio Gonzalez Monroy, David Marchand, dev, Tan, Jianfeng
In-Reply-To: <65601617-b4a2-4971-6482-af4810dbb34a@intel.com>
2016-09-30 15:48, Sergio Gonzalez Monroy:
> On 30/09/2016 15:32, David Marchand wrote:
> > On Fri, Sep 23, 2016 at 12:08 PM, Tan, Jianfeng <jianfeng.tan@intel.com> wrote:
> >>> -----Original Message-----
> >>> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> >>> Sent: Friday, September 23, 2016 5:15 PM
> >>> To: Thomas Monjalon
> >>> Cc: dev@dpdk.org; David Marchand; Tan, Jianfeng
> >>> Subject: Re: [PATCH] eal: remove single file segments related code
> >>>
> >>> On Fri, Sep 23, 2016 at 10:50:06AM +0200, Thomas Monjalon wrote:
> >>>> 2016-09-23 15:10, Yuanhan Liu:
> >>>>> Commit c711ccb30987 ("ivshmem: remove library and its EAL integration")
> >>>>> removed ivshmem support, but seems David forgot to remove the another
> >>>>> piece of code: code for RTE_EAL_SINGLE_FILE_SEGMENTS, which is
> >>> introduced
> >>>>> when ivshmem was firstly added.
> >>>> It is not a mistake. We thought it is used by container use case.
> >>> I think no. It would help the container case a bit, but not too much I
> >>> would think, especially when the memory goes fragement.
> >>>
> >>> Jianfeng, IIRC, you don't use that option for container case, right?
> >>>
> >>> --yliu
> >> No, I don't use this option for container case. As yuanhan said, it cannot provide much help for virtio_user memory region number limitation.
> > Ok, as said, since this feature had been introduced with ivshmem
> > 40b966a211ab ("ivshmem: library changes for mmaping using ivshmem"),
> > if Sergio has nothing against this removal, I am all for removing
> > unused code.
>
> I certainly do not have anything against this removal :)
>
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] eal: check cpu flags at init
From: Thomas Monjalon @ 2016-10-03 14:13 UTC (permalink / raw)
To: Flavio Leitner; +Cc: dev, Aaron Conole
In-Reply-To: <f7ttwcyv3ol.fsf@redhat.com>
2016-09-29 16:42, Aaron Conole:
> Flavio Leitner <fbl@sysclose.org> writes:
>
> > On Mon, Sep 26, 2016 at 11:43:37AM -0400, Aaron Conole wrote:
> >> My only concern is whether this change would be considered ABI
> >> breaking. I wouldn't think so, since it doesn't seem as though an
> >> application would want to call this explicitly (and is spelled out as
> >> such), but I can't be sure that it isn't already included in the
> >> standard application API, and therefore needs to go through the change
> >> process.
> >
> > I didn't want to change the original behavior more than needed.
> >
> > I think another patch would be necessary to change the whole EAL
> > initialization because there's a bunch of rte_panic() there which
> > aren't friendly with callers either.
Yes please, we need to remove all those panic/exit calls.
> Okay makes sense.
>
> Acked-by: Aaron Conole <aconole@redhat.com>
Applied, thanks
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Maxime Coquelin @ 2016-10-03 14:20 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Yuanhan Liu, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <20160929231252-mutt-send-email-mst@kernel.org>
On 09/29/2016 10:21 PM, Michael S. Tsirkin wrote:
> On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
>> >
>> >
>> > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
>>> > > On Thu, Sep 29, 2016 at 05:30:53PM +0200, Maxime Coquelin wrote:
>> > ...
>>>> > > >
>>>> > > > Before enabling anything by default, we should first optimize the 1 slot
>>>> > > > case. Indeed, micro-benchmark using testpmd in txonly[0] shows ~17%
>>>> > > > perf regression for 64 bytes case:
>>>> > > > - 2 descs per packet: 11.6Mpps
>>>> > > > - 1 desc per packet: 9.6Mpps
>>>> > > >
>>>> > > > This is due to the virtio header clearing in virtqueue_enqueue_xmit().
>>>> > > > Removing it, we get better results than with 2 descs (1.20Mpps).
>>>> > > > Since the Virtio PMD doesn't support offloads, I wonder whether we can
>>>> > > > just drop the memset?
>>> > >
>>> > > What will happen? Will the header be uninitialized?
>> > Yes..
>> > I didn't look closely at the spec, but just looked at DPDK's and Linux
>> > vhost implementations. IIUC, the header is just skipped in the two
>> > implementations.
> In linux guest skbs are initialized AFAIK. See virtio_net_hdr_from_skb
> first thing it does is
> memset(hdr, 0, sizeof(*hdr));
>
>
>
>>> > >
>>> > > The spec says:
>>> > > The driver can send a completely checksummed packet. In this case, flags
>>> > > will be zero, and gso_type
>>> > > will be VIRTIO_NET_HDR_GSO_NONE.
>>> > >
>>> > > and
>>> > > The driver MUST set num_buffers to zero.
>>> > > If VIRTIO_NET_F_CSUM is not negotiated, the driver MUST set flags to
>>> > > zero and SHOULD supply a fully
>>> > > checksummed packet to the device.
>>> > >
>>> > > and
>>> > > If none of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have been
>>> > > negotiated, the driver MUST
>>> > > set gso_type to VIRTIO_NET_HDR_GSO_NONE.
>>> > >
>>> > > so doing this unconditionally would be a spec violation, but if you see
>>> > > value in this, we can add a feature bit.
>> > Right it would be a spec violation, so it should be done conditionally.
>> > If a feature bit is to be added, what about VIRTIO_NET_F_NO_TX_HEADER?
>> > It would imply VIRTIO_NET_F_CSUM not set, and no GSO features set.
>> > If negotiated, we wouldn't need to prepend a header.
> Yes but two points.
>
> 1. why is this memset expensive? Is the test completely skipping looking
> at the packet otherwise?
>
> 2. As long as we are doing this, see
> Alignment vs. Networking
> ========================
> in Documentation/unaligned-memory-access.txt
This change will not have an impact on the IP header alignment,
as is offset in the mbuf will not change.
Regards,
Maxime
^ permalink raw reply
* Re: [PATCH v11 07/24] driver: probe/remove common wrappers for PCI drivers
From: Thomas Monjalon @ 2016-10-03 14:21 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev, viktorin, David Marchand, hemant.agrawal
In-Reply-To: <1474375296-26078-8-git-send-email-shreyansh.jain@nxp.com>
2016-09-20 18:11, Shreyansh Jain:
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -4372,6 +4372,19 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id);
> int
> rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
>
> +/**
> + * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
> + * interface.
> + */
> +int rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
> + struct rte_pci_device *pci_dev);
> +
> +/**
> + * Wrapper for use by pci drivers as a .remove function to detach a ethdev
> + * interface.
> + */
> +int rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev);
These functions are used by the drivers only (as helpers).
So they should be marked @internal (added after applying the patch).
^ permalink raw reply
* Re: [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
From: Thomas Monjalon @ 2016-10-03 14:28 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev, viktorin, David Marchand, hemant.agrawal
In-Reply-To: <1474375296-26078-1-git-send-email-shreyansh.jain@nxp.com>
Applied, thanks everybody for the great (re)work!
2016-09-20 18:11, Shreyansh Jain:
> Future Work/Pending:
> ===================
> - Presently eth_driver, rte_eth_dev are not aligned to the rte_driver/
> rte_device model. eth_driver still is a PCI specific entity. This
> has been highlighted by comments from Ferruh in [9].
> - Some variables, like drv_name (as highlighted by Ferruh), are getting
> duplicated across rte_xxx_driver/device and rte_driver/device.
What about those pending work?
I would add more remaining issues:
- probe/remove naming could be applied to vdev for consistency
- rte_eal_device_insert must be called in vdev
- REGISTER macros should be prefixed with RTE_
- Some functions in EAL does not need eal_ in their prefix:
rte_eal_pci_ -> rte_pci_
rte_eal_dev_ -> rte_dev_
rte_eal_vdev_ -> rte_vdev_
rte_eal_driver -> rte_drv_
rte_eal_vdrv -> rte_vdrv_
^ permalink raw reply
* Re: [PATCH] log: do not drop debug logs at compile time
From: Olivier Matz @ 2016-10-03 15:02 UTC (permalink / raw)
To: Wiles, Keith, Thomas Monjalon; +Cc: dev@dpdk.org, david.marchand@6wind.com
In-Reply-To: <D9218931-1F74-4467-9C72-D6F989762383@intel.com>
Hi Keith,
On 09/30/2016 05:48 PM, Wiles, Keith wrote:
>> On Sep 30, 2016, at 4:33 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
>>
>> 2016-09-16 09:43, Olivier Matz:
>>> Today, all logs whose level is lower than INFO are dropped at
>>> compile-time. This prevents from enabling debug logs at runtime using
>>> --log-level=8.
>>>
>>> The rationale was to remove debug logs from the data path at
>>> compile-time, avoiding a test at run-time.
>>>
>>> This patch changes the behavior of RTE_LOG() to avoid the compile-time
>>> optimization, and introduces the RTE_LOG_DP() macro that has the same
>>> behavior than the previous RTE_LOG(), for the rare cases where debug
>>> logs are in the data path.
>>>
>>> So it is now possible to enable debug logs at run-time by just
>>> specifying --log-level=8. Some drivers still have special compile-time
>>> options to enable more debug log. Maintainers may consider to
>>> remove/reduce them.
>>>
>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>
>> I think it is a good change.
>> However I'm not sure we should take it for 16.11 as it was sent late and
>> there is no review comment.
>> It is neither really a fix nor really a feature.
>> If there are some +1, and no opinions against, it will go in 16.11.
>> Note that some drivers would need some changes to fully benefit of
>> debug logs enabled at run-time.
>
> Would this be easier to add a new LOG level instead say DEBUG_DATAPATH and then change the RTE_LOG to exclude the new log level?
>
>
The log levels are quite standard, I don't feel it would be very clear
to have a new level for that. It would also prevent to have different
log level inside data path.
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH v2 1/2] mempool: fix comments for mempool create functions
From: Olivier Matz @ 2016-10-03 15:06 UTC (permalink / raw)
To: Ferruh Yigit, dev
In-Reply-To: <20160928135929.4917-1-ferruh.yigit@intel.com>
On 09/28/2016 03:59 PM, Ferruh Yigit wrote:
> Fixes: 85226f9c526b ("mempool: introduce a function to create an empty pool")
> Fixes: d1d914ebbc25 ("mempool: allocate in several memory chunks by default")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
Series:
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Thanks
^ permalink raw reply
* Re: [PATCH] log: do not drop debug logs at compile time
From: Wiles, Keith @ 2016-10-03 15:27 UTC (permalink / raw)
To: Olivier Matz; +Cc: Thomas Monjalon, dev@dpdk.org, david.marchand@6wind.com
In-Reply-To: <f6563886-b2f5-112b-2264-0e1a6c2be001@6wind.com>
Regards,
Keith
> On Oct 3, 2016, at 10:02 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Hi Keith,
>
> On 09/30/2016 05:48 PM, Wiles, Keith wrote:
>>> On Sep 30, 2016, at 4:33 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
>>>
>>> 2016-09-16 09:43, Olivier Matz:
>>>> Today, all logs whose level is lower than INFO are dropped at
>>>> compile-time. This prevents from enabling debug logs at runtime using
>>>> --log-level=8.
>>>>
>>>> The rationale was to remove debug logs from the data path at
>>>> compile-time, avoiding a test at run-time.
>>>>
>>>> This patch changes the behavior of RTE_LOG() to avoid the compile-time
>>>> optimization, and introduces the RTE_LOG_DP() macro that has the same
>>>> behavior than the previous RTE_LOG(), for the rare cases where debug
>>>> logs are in the data path.
>>>>
>>>> So it is now possible to enable debug logs at run-time by just
>>>> specifying --log-level=8. Some drivers still have special compile-time
>>>> options to enable more debug log. Maintainers may consider to
>>>> remove/reduce them.
>>>>
>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>
>>> I think it is a good change.
>>> However I'm not sure we should take it for 16.11 as it was sent late and
>>> there is no review comment.
>>> It is neither really a fix nor really a feature.
>>> If there are some +1, and no opinions against, it will go in 16.11.
>>> Note that some drivers would need some changes to fully benefit of
>>> debug logs enabled at run-time.
>>
>> Would this be easier to add a new LOG level instead say DEBUG_DATAPATH and then change the RTE_LOG to exclude the new log level?
>>
>>
>
> The log levels are quite standard, I don't feel it would be very clear
> to have a new level for that. It would also prevent to have different
> log level inside data path.
I am not following you here. Having one more log level for DEBUG in the data path is not a big change and you can still have any other log level in the data or anyplace else for that matter.
>
> Regards,
> Olivier
^ permalink raw reply
* Re: [PATCH v3 1/2] librte_ether: add internal callback functions
From: Iremonger, Bernard @ 2016-10-03 15:29 UTC (permalink / raw)
To: Stephen Hemminger
Cc: dev@dpdk.org, Lu, Wenzhuo, jerin.jacob@caviumnetworks.com,
az5157@att.com, Shah, Rahul R
In-Reply-To: <CAOaVG14duWoFS6JC6LNkmpgr9_CRHpd=MmqZ_c1BvP=rwzz-RA@mail.gmail.com>
Hi Stephen,
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Sunday, October 2, 2016 10:13 AM
To: Iremonger, Bernard <bernard.iremonger@intel.com>
Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; jerin.jacob@caviumnetworks.com; az5157@att.com; Shah, Rahul R <rahul.r.shah@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] librte_ether: add internal callback functions
I know callbacks are needed, in fact even more are necessary. That is why I don't like this design. It expands the API for each event. I think something like the Linux kernel netlink callback mechanism that passes an event and device handle.
The current rte_eth_dev_callback_register() function takes a parameter void *cb_arg. This allows the passing of a parameter to the callback. For the events RTE_ETH_EVENT_QUEUE_STATE, RTE_ETH_EVENT_INTR_RESET the callback parameter is not used. In some cases for the RTE_ETH_EVENT_INTR_LSC the call back parameter is used.
This patch adds a new event RTE_ETH_VF_MBOX and a parameter for this event struct rte_eth_mb_event_param{}. This parameter is only used with the RTE_ETH_VF_MBOX event and does not affect the other events.
The struct rte_eth_mb_event_param{} should probably not be in rte_eth_dev.h, I will send a v4.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH] log: do not drop debug logs at compile time
From: Olivier Matz @ 2016-10-03 15:37 UTC (permalink / raw)
To: Wiles, Keith; +Cc: Thomas Monjalon, dev@dpdk.org, david.marchand@6wind.com
In-Reply-To: <D18F01AF-30AB-4433-B769-651ACF75F004@intel.com>
On 10/03/2016 05:27 PM, Wiles, Keith wrote:
>
> Regards,
> Keith
>
>> On Oct 3, 2016, at 10:02 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>>
>> Hi Keith,
>>
>> On 09/30/2016 05:48 PM, Wiles, Keith wrote:
>>>> On Sep 30, 2016, at 4:33 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
>>>>
>>>> 2016-09-16 09:43, Olivier Matz:
>>>>> Today, all logs whose level is lower than INFO are dropped at
>>>>> compile-time. This prevents from enabling debug logs at runtime using
>>>>> --log-level=8.
>>>>>
>>>>> The rationale was to remove debug logs from the data path at
>>>>> compile-time, avoiding a test at run-time.
>>>>>
>>>>> This patch changes the behavior of RTE_LOG() to avoid the compile-time
>>>>> optimization, and introduces the RTE_LOG_DP() macro that has the same
>>>>> behavior than the previous RTE_LOG(), for the rare cases where debug
>>>>> logs are in the data path.
>>>>>
>>>>> So it is now possible to enable debug logs at run-time by just
>>>>> specifying --log-level=8. Some drivers still have special compile-time
>>>>> options to enable more debug log. Maintainers may consider to
>>>>> remove/reduce them.
>>>>>
>>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>>
>>>> I think it is a good change.
>>>> However I'm not sure we should take it for 16.11 as it was sent late and
>>>> there is no review comment.
>>>> It is neither really a fix nor really a feature.
>>>> If there are some +1, and no opinions against, it will go in 16.11.
>>>> Note that some drivers would need some changes to fully benefit of
>>>> debug logs enabled at run-time.
>>>
>>> Would this be easier to add a new LOG level instead say DEBUG_DATAPATH and then change the RTE_LOG to exclude the new log level?
>>>
>>>
>>
>> The log levels are quite standard, I don't feel it would be very clear
>> to have a new level for that. It would also prevent to have different
>> log level inside data path.
>
> I am not following you here. Having one more log level for DEBUG in the data path is not a big change and you can still have any other log level in the data or anyplace else for that matter.
Adding a new log level is not a big change, you are right.
But to me it looks confusing to have DEBUG, INFO, ..., WARNING, ERROR,
plus a DEBUG_DATAPATH. For instance, how do you compare levels? Or if
your log stream forwards logs to syslog, you cannot do a 1:1 mapping
with standard syslog levels.
What makes you feel it's easier to add a log level instead of adding a
new RTE_LOG_DP() function?
Regards,
Olivier
^ permalink raw reply
* Re: [RFC 0/7] changing mbuf pool handler
From: Olivier Matz @ 2016-10-03 15:49 UTC (permalink / raw)
To: Hemant Agrawal, dev; +Cc: jerin.jacob, david.hunt
In-Reply-To: <d11f3842-d715-3afc-9d5d-9e02f2767c2d@nxp.com>
Hi Hemant,
Thank you for your feedback.
On 09/22/2016 01:52 PM, Hemant Agrawal wrote:
> Hi Olivier
>
> On 9/19/2016 7:12 PM, Olivier Matz wrote:
>> Hello,
>>
>> Following discussion from [1] ("usages issue with external mempool").
>>
>> This is a tentative to make the mempool_ops feature introduced
>> by David Hunt [2] more widely used by applications.
>>
>> It applies on top of a minor fix in mbuf lib [3].
>>
>> To sumarize the needs (please comment if I did not got it properly):
>>
>> - new hw-assisted mempool handlers will soon be introduced
>> - to make use of it, the new mempool API [4] (rte_mempool_create_empty,
>> rte_mempool_populate, ...) has to be used
>> - the legacy mempool API (rte_mempool_create) does not allow to change
>> the mempool ops. The default is "ring_<s|m>p_<s|m>c" depending on
>> flags.
>> - the mbuf helper (rte_pktmbuf_pool_create) does not allow to change
>> them either, and the default is RTE_MBUF_DEFAULT_MEMPOOL_OPS
>> ("ring_mp_mc")
>> - today, most (if not all) applications and examples use either
>> rte_pktmbuf_pool_create or rte_mempool_create to create the mbuf
>> pool, making it difficult to take advantage of this feature with
>> existing apps.
>>
>> My initial idea was to deprecate both rte_pktmbuf_pool_create() and
>> rte_mempool_create(), forcing the applications to use the new API, which
>> is more flexible. But after digging a bit, it appeared that
>> rte_mempool_create() is widely used, and not only for mbufs. Deprecating
>> it would have a big impact on applications, and replacing it with the
>> new API would be overkill in many use-cases.
>
> I agree with the proposal.
>
>>
>> So I finally tried the following approach (inspired from a suggestion
>> Jerin [5]):
>>
>> - add a new mempool_ops parameter to rte_pktmbuf_pool_create(). This
>> unfortunatelly breaks the API, but I implemented an ABI compat layer.
>> If the patch is accepted, we could discuss how to announce/schedule
>> the API change.
>> - update the applications and documentation to prefer
>> rte_pktmbuf_pool_create() as much as possible
>> - update most used examples (testpmd, l2fwd, l3fwd) to add a new command
>> line argument to select the mempool handler
>>
>> I hope the external applications would then switch to
>> rte_pktmbuf_pool_create(), since it supports most of the use-cases (even
>> priv_size != 0, since we can call rte_mempool_obj_iter() after) .
>>
>
> I will still prefer if you can add the "rte_mempool_obj_cb_t *obj_cb,
> void *obj_cb_arg" into "rte_pktmbuf_pool_create". This single
> consolidated wrapper will almost make it certain that applications will
> not try to use rte_mempool_create for packet buffers.
The patch changes the example applications. I'm not sure I understand
why adding these arguments would force application to not use
rte_mempool_create() for packet buffers. Do you have a application in mind?
For the mempool_ops parameter, we must pass it at init because we need
to know the mempool handler before populating the pool. For object
initialization, it can be done after, so I thought it was better to
reduce the number of arguments to avoid to fall in the mempool_create()
syndrom :)
Any other opinions?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.
From: Jean Tourrilhes @ 2016-10-03 15:55 UTC (permalink / raw)
To: Sergio Gonzalez Monroy; +Cc: dev
In-Reply-To: <4a84de40-04b9-da6d-aeb9-ce661a9ebad5@intel.com>
On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> Hi Jean,
>
> There are some format issues with the patch:
>
> You can run scripts/check-git-log.sh to check them:
> Wrong headline format:
> eal: Fix misleading error messages, errno can't be trusted.
> Wrong headline uppercase:
> eal: Fix misleading error messages, errno can't be trusted.
> Missing 'Fixes' tag:
> eal: Fix misleading error messages, errno can't be trusted.
>
> The script's output highlights the different issues.
SOrry about that, I casually read the page on
http://dpdk.org/dev, but obviously I need to look at it again.
> On 21/09/2016 22:10, Jean Tourrilhes wrote:
> >@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
> > mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
> > sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
> > mem_cfg_fd, 0);
> >+ if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> >+ if (mem_config != MAP_FAILED)
> >+ /* errno is stale, don't use */
> >+ rte_panic("Cannot mmap memory for rte_config at [%p], got [%p] - please use '--base-virtaddr' option\n",
> >+ rte_mem_cfg_addr, mem_config);
> >+ else
> >+ rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
> >+ errno, strerror(errno));
> >+ }
> > close(mem_cfg_fd);
> >- if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> >- rte_panic("Cannot mmap memory for rte_config\n");
>
> NIT but any reason you moved the check before closing the file descriptor?
> (not that it matters with current code as we panic anyway)
"close()" may change "errno" according to its man page.
> Thanks,
> Sergio
Thanks for the review !
Jean
^ permalink raw reply
* Re: [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.
From: Jean Tourrilhes @ 2016-10-03 16:07 UTC (permalink / raw)
To: Sergio Gonzalez Monroy; +Cc: dev
In-Reply-To: <4a84de40-04b9-da6d-aeb9-ce661a9ebad5@intel.com>
On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> Hi Jean,
>
> NIT but any reason you moved the check before closing the file descriptor?
> (not that it matters with current code as we panic anyway)
>
> Thanks,
> Sergio
More details, as I admit I was terse
Running secondary is tricky due to the need to map the memory
region at the right place, which is whatever primary has chosen. If
the base address for primary happens to by already mapped in the
secondary, we will hit precisely this error message (well, in a few
case we might hit the other one). This is why there is already
a comment about ASLR.
A colleague of mine hit that message and was misled by errno
claiming "permission denied", which sent him down the wrong
track. It's such a common error for secondary that I feel this error
message should be unambiguous and helpful.
Regards,
Jean
^ permalink raw reply
* Re: [PATCH 1/2] mbuf: add rte_pktmbuff_reset_headroom function
From: Olivier Matz @ 2016-10-03 16:11 UTC (permalink / raw)
To: Maxime Coquelin, dev; +Cc: stephen, mst, yuanhan.liu
In-Reply-To: <1475151633-6785-1-git-send-email-maxime.coquelin@redhat.com>
Hi Maxime,
On 09/29/2016 02:20 PM, Maxime Coquelin wrote:
> Some application use rte_mbuf_raw_alloc() function to improve
> performance by not resetting mbuf's fields to their default state.
>
> This can be however problematic for mbuf consumers that need some
> headroom, meaning that data_off field gets decremented after
> allocation. When the mbuf is re-used afterwards, there might not
> be enough room for the consumer to prepend anything, if the data_off
> field is not reset to its default value.
>
> This patch adds a new rte_pktmbuf_reset_headroom() function that
> applications can call to reset the data_off field.
> This patch also replaces current data_off affectations in the mbuf
> lib with a call to this function.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Sounds like a good idea. Just one small comment below.
>
> /**
> + * Reset the data_off field of a packet mbuf to its default value.
> + *
> + * The given mbuf must have only one segment.
> + *
> + * @param m
> + * The packet mbuf's data_off field has to be reset.
> + */
> +static inline void rte_pktmbuf_reset_headroom(struct rte_mbuf *m)
> +{
> + m->data_off = RTE_MIN(RTE_PKTMBUF_HEADROOM, (uint16_t)m->buf_len);
> +}
Maybe we should also highlight in the API comment that the segment
should be empty.
Thanks,
Olivier
^ permalink raw reply
* Re: [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.
From: Mcnamara, John @ 2016-10-03 16:15 UTC (permalink / raw)
To: jean.tourrilhes@hpe.com, Gonzalez Monroy, Sergio; +Cc: dev@dpdk.org
In-Reply-To: <20161003155556.GA23662@labs.hpe.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jean Tourrilhes
> Sent: Monday, October 3, 2016 4:56 PM
> To: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/1 v2] eal: Fix misleading error messages,
> errno can't be trusted.
>
> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> > Hi Jean,
> >
> > There are some format issues with the patch:
> >
> > You can run scripts/check-git-log.sh to check them:
> > Wrong headline format:
> > eal: Fix misleading error messages, errno can't be trusted.
> > Wrong headline uppercase:
> > eal: Fix misleading error messages, errno can't be trusted.
> > Missing 'Fixes' tag:
> > eal: Fix misleading error messages, errno can't be trusted.
> >
> > The script's output highlights the different issues.
>
> SOrry about that, I casually read the page on http://dpdk.org/dev,
> but obviously I need to look at it again.
The longer more detailed version is here: "Contributing Code to DPDK":
http://dpdk.org/doc/guides/contributing/patches.html
John
^ permalink raw reply
* Re: [PATCH] log: do not drop debug logs at compile time
From: Wiles, Keith @ 2016-10-03 16:27 UTC (permalink / raw)
To: Olivier Matz; +Cc: Thomas Monjalon, dev@dpdk.org, david.marchand@6wind.com
In-Reply-To: <104cb490-b39a-58e6-2031-d99e023474f2@6wind.com>
Regards,
Keith
> On Oct 3, 2016, at 10:37 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>
>
>
> On 10/03/2016 05:27 PM, Wiles, Keith wrote:
>>
>> Regards,
>> Keith
>>
>>> On Oct 3, 2016, at 10:02 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>>>
>>> Hi Keith,
>>>
>>> On 09/30/2016 05:48 PM, Wiles, Keith wrote:
>>>>> On Sep 30, 2016, at 4:33 AM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
>>>>>
>>>>> 2016-09-16 09:43, Olivier Matz:
>>>>>> Today, all logs whose level is lower than INFO are dropped at
>>>>>> compile-time. This prevents from enabling debug logs at runtime using
>>>>>> --log-level=8.
>>>>>>
>>>>>> The rationale was to remove debug logs from the data path at
>>>>>> compile-time, avoiding a test at run-time.
>>>>>>
>>>>>> This patch changes the behavior of RTE_LOG() to avoid the compile-time
>>>>>> optimization, and introduces the RTE_LOG_DP() macro that has the same
>>>>>> behavior than the previous RTE_LOG(), for the rare cases where debug
>>>>>> logs are in the data path.
>>>>>>
>>>>>> So it is now possible to enable debug logs at run-time by just
>>>>>> specifying --log-level=8. Some drivers still have special compile-time
>>>>>> options to enable more debug log. Maintainers may consider to
>>>>>> remove/reduce them.
>>>>>>
>>>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>>>
>>>>> I think it is a good change.
>>>>> However I'm not sure we should take it for 16.11 as it was sent late and
>>>>> there is no review comment.
>>>>> It is neither really a fix nor really a feature.
>>>>> If there are some +1, and no opinions against, it will go in 16.11.
>>>>> Note that some drivers would need some changes to fully benefit of
>>>>> debug logs enabled at run-time.
>>>>
>>>> Would this be easier to add a new LOG level instead say DEBUG_DATAPATH and then change the RTE_LOG to exclude the new log level?
>>>>
>>>>
>>>
>>> The log levels are quite standard, I don't feel it would be very clear
>>> to have a new level for that. It would also prevent to have different
>>> log level inside data path.
>>
>> I am not following you here. Having one more log level for DEBUG in the data path is not a big change and you can still have any other log level in the data or anyplace else for that matter.
>
> Adding a new log level is not a big change, you are right.
> But to me it looks confusing to have DEBUG, INFO, ..., WARNING, ERROR,
> plus a DEBUG_DATAPATH. For instance, how do you compare levels? Or if
> your log stream forwards logs to syslog, you cannot do a 1:1 mapping
> with standard syslog levels.
Doing 1:1 mapping is not a big problem unless you are trying to compare to old logs, which maybe the case the first time and after that it should not be a problem.
>
> What makes you feel it's easier to add a log level instead of adding a
> new RTE_LOG_DP() function?
It seems to me the log levels are for displaying logs at different levels adding a new macro to not log is just a hack because we do not have a log level for data path. This is why I would like to see a log level added and not a new macro.
It also appears the new RTE_LOG() will always be in the code as you moved the test to the RTE_LOG_DP() macro. This would mean all RTE_LOG() in the code will always call rte_log(), correct?
If using a new DEBUG_DP (maybe DATAPATH is a better log level name) level we can use the same macro as before and modify the level only. This way we can remove via the compiler any log that is below the default RTE_LOG_LEVEL. I see keeping the rte_log() could be a performance problem or code blot when you really want to remove them all.
The DATAPATH log level would be above (smaller number) then DEBUG in the enum list. To remove all debug logs just set the RTE_LOG_LEVEL to RTE_LOG_DATAPATH.
>
>
> Regards,
> Olivier
^ permalink raw reply
* Re: [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.
From: Jean Tourrilhes @ 2016-10-03 16:34 UTC (permalink / raw)
To: Mcnamara, John; +Cc: Gonzalez Monroy, Sergio, dev@dpdk.org
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202627191@IRSMSX103.ger.corp.intel.com>
On Mon, Oct 03, 2016 at 04:15:11PM +0000, Mcnamara, John wrote:
>
> The longer more detailed version is here: "Contributing Code to DPDK":
>
> http://dpdk.org/doc/guides/contributing/patches.html
>
> John
Thanks a lot. I'll try to find time to look at it.
Jean
^ permalink raw reply
* [PATCH] eal: fix c++ compilation issue with rte_delay_us()
From: Konstantin Ananyev @ 2016-10-03 17:27 UTC (permalink / raw)
To: dev, dev; +Cc: Konstantin Ananyev
When compiling with C++, it treats
void (*rte_delay_us)(unsigned int us);
as definition of the global variable.
So further linking with librte_eal fails.
Fixes: b4d63fb62240 ("eal: customize delay function")
Steps to reproduce:
$ cat rttm1.cpp
#include <iostream>
#include <rte_eal.h>
#include <rte_cycles.h>
using namespace std;
int main(int argc, char *argv[])
{
int ret = rte_eal_init(argc, argv);
rte_delay_us(1);
cout << "return code ";
cout << ret;
return ret;
}
$ g++ -m64 -I/${RTE_SDK}/${RTE_TARGET}/include -c -o rttm1.o rttm1.cpp
$ gcc -m64 -pthread -o rttm1 rttm1.o -ldl -Wl,-lstdc++ \
-L/${RTE_SDK}/${RTE_TARGET}/lib -Wl,-lrte_eal
.../librte_eal.a(eal_common_timer.o):
(.bss+0x0): multiple definition of `rte_delay_us'
rttm1.o:(.bss+0x0): first defined here
collect2: error: ld returned 1 exit status
$ nm rttm1.o | grep rte_delay_us
0000000000000092 t _GLOBAL__sub_I_rte_delay_us
0000000000000000 B rte_delay_us
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
lib/librte_eal/common/include/generic/rte_cycles.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/include/generic/rte_cycles.h b/lib/librte_eal/common/include/generic/rte_cycles.h
index 96a2da9..00103ca 100644
--- a/lib/librte_eal/common/include/generic/rte_cycles.h
+++ b/lib/librte_eal/common/include/generic/rte_cycles.h
@@ -188,7 +188,7 @@ rte_get_timer_hz(void)
* @param us
* The number of microseconds to wait.
*/
-void
+extern void
(*rte_delay_us)(unsigned int us);
/**
--
2.4.3
^ permalink raw reply related
* Re: qos: traffic shaping at queue level
From: Dumitrescu, Cristian @ 2016-10-03 18:12 UTC (permalink / raw)
To: Nikhil Jagtap, dev@dpdk.org, users@dpdk.org
In-Reply-To: <CANKBMf1jFJ_1kft6v0UTWrJxh9idsPzj03qoqY-5oLM3a1tFbA@mail.gmail.com>
From: Nikhil Jagtap [mailto:nikhil.jagtap@gmail.com]
Sent: Friday, September 30, 2016 7:12 AM
To: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; users@dpdk.org
Subject: Re: qos: traffic shaping at queue level
Hi,
Can someone please answer my queries?
I tried using queue weights to distribute traffic-class bandwidth among the child queues, but did not get the desired results.
[Cristian] Can you please describe what issues you see?
Regards,
Nikhil
On 27 September 2016 at 15:34, Nikhil Jagtap <nikhil.jagtap@gmail.com<mailto:nikhil.jagtap@gmail.com>> wrote:
Hi,
I have a few questions about the hierarchical scheduler. I am taking a simple example here to get a better understanding.
Reference example:
pipe rate = 30 mbps
tc 0 rate = 30 mbps
traffic-type 0 being queued to queue 0, tc 0.
traffic-type 1 being queued to queue 1, tc 0.
Assume traffic-type 0 is being received at the rate of 25 mbps.
Assume traffic-type 1 is also being received at the rate of 25 mbps.
Requirement:
To limit traffic-type 0 to (CIR = 5 mbps, PIR = 30 mbps), AND
limit traffic-type 1 to (CIR = 25 mbps, PIR = 30 mbps).
The questions:
1) I understand that with the scheduler, it is possible to do rate limiting only at the sub-port and pipe levels and not at the individual queue level.
[Cristian] Yes, correct, only subports and pipes own token buckets, with all the pipe traffic classes and queues sharing their pipe token bucket.
Is it possible to achieve rate limiting using the notion of queue weights? For the above example, will assigning weights in 1:5 ratio to the two queues help achieve shaping the two traffic-types at the two different rates?
[Cristian] Yes. However, getting the weight observed accurately relies on all the queues being backlogged (always having packets to dequeue). When a pipe and certain TC is examined for dequeuing, the relative weights are enforced between the queues that have packets at that precise moment in time, with the empty queues being ignored. The fully backlogged scenario is not taking place in practice, and the set of non-empty queues changes over time. As said it the past, having big relative weight ratios between queues helps (1:5 should be good).
2) In continuation to previous question: if queue weights don't help, would it be possible to use metering to achieve rate limiting? Assume we meter individual traffic-types (using CIR-PIR config mentioned above) before queuing it to the scheduler queues. So to achieve the respective queue rates, the dequeuer would be expected to prioritise green packets over yellow.
Looking into the code, the packet color is used as an input to the dropper block, but does not seem to be used anywhere in the scheduler. So I guess it is not possible to prioritise green packets when dequeing?
[Cristian] Packet color is used by Weighted RED (WRED) congestion management scheme on the enqueue side, not on the dequeue side. Once the packet has been enqueued, it cannot be dropped (i.e. every enqueued packet will eventually be dequeued), so rate limiting cannot be enforced on the dequeue side.
Regards,
Nikhil
^ permalink raw reply
* Re: [PATCH 1/1 v2] eal: Fix misleading error messages, errno can't be trusted.
From: Thomas Monjalon @ 2016-10-03 20:46 UTC (permalink / raw)
To: jean.tourrilhes, Sergio Gonzalez Monroy; +Cc: dev
In-Reply-To: <20161003155556.GA23662@labs.hpe.com>
2016-10-03 08:55, Jean Tourrilhes:
> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> > Hi Jean,
> >
> > There are some format issues with the patch:
> >
> > You can run scripts/check-git-log.sh to check them:
> > Wrong headline format:
> > eal: Fix misleading error messages, errno can't be trusted.
> > Wrong headline uppercase:
> > eal: Fix misleading error messages, errno can't be trusted.
> > Missing 'Fixes' tag:
> > eal: Fix misleading error messages, errno can't be trusted.
> >
> > The script's output highlights the different issues.
>
> SOrry about that, I casually read the page on
> http://dpdk.org/dev, but obviously I need to look at it again.
No problem. This guide is more oriented towards regular contributors.
You come with a bug and its fix, we can make some effort to format
the patch :)
The title could be "mem: fix hugepage mapping error messages"
> > On 21/09/2016 22:10, Jean Tourrilhes wrote:
> > >@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
> > > mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
> > > sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
> > > mem_cfg_fd, 0);
> > >+ if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> > >+ if (mem_config != MAP_FAILED)
> > >+ /* errno is stale, don't use */
> > >+ rte_panic("Cannot mmap memory for rte_config at [%p], got [%p] - please use '--base-virtaddr' option\n",
> > >+ rte_mem_cfg_addr, mem_config);
> > >+ else
> > >+ rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
> > >+ errno, strerror(errno));
> > >+ }
> > > close(mem_cfg_fd);
> > >- if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> > >- rte_panic("Cannot mmap memory for rte_config\n");
> >
> > NIT but any reason you moved the check before closing the file descriptor?
> > (not that it matters with current code as we panic anyway)
>
> "close()" may change "errno" according to its man page.
Sergio, do you have more comments?
Should we wait another version or is it OK?
Maybe you'd prefer to rework it yourself?
^ permalink raw reply
* [PATCH] app/test: add mempool walk
From: Thomas Monjalon @ 2016-10-03 20:58 UTC (permalink / raw)
To: olivier.matz; +Cc: dev
The mempool function rte_mempool_walk was not tested.
It will print the name of all mempools.
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
app/test/test_mempool.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index fffbf8d..b9880b3 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -500,6 +500,12 @@ test_mempool_xmem_misc(void)
return 0;
}
+static void
+walk_cb(struct rte_mempool *mp, void *userdata __rte_unused)
+{
+ printf("\t%s\n", mp->name);
+}
+
static int
test_mempool(void)
{
@@ -561,6 +567,9 @@ test_mempool(void)
goto err;
}
+ printf("Walk into mempools:\n");
+ rte_mempool_walk(walk_cb, NULL);
+
rte_mempool_list_dump(stdout);
/* basic tests without cache */
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v5 0/4] new crypto software based device
From: De Lara Guarch, Pablo @ 2016-10-03 22:36 UTC (permalink / raw)
To: Mrozowicz, SlawomirX, dev@dpdk.org; +Cc: Mrozowicz, SlawomirX
In-Reply-To: <1475504792-31557-1-git-send-email-slawomirx.mrozowicz@intel.com>
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Slawomir Mrozowicz
> Sent: Monday, October 03, 2016 7:26 AM
> To: dev@dpdk.org
> Cc: Mrozowicz, SlawomirX
> Subject: [dpdk-dev] [PATCH v5 0/4] new crypto software based device
>
> This code provides the initial implementation of the libcrypto poll mode
> driver.
> All cryptography operations are using Openssl library crypto API.
> Each algorithm uses EVP_ interface from openssl API - which is recommended
> by
> Openssl maintainers.
>
> For more information about how to use this driver, go to:
> doc/guides/cryptodevs/libcrypto.rst
>
> Changes in V5:
> - reduce source of big data test
>
> Changes in V4:
> - move aes test rework to another patch
> - move big data test to another patch
> - checking if libcrypto pmd is available
>
> Changes in V3:
> - add nagative verification tests
> - add big data test
> - fix pmd according to negative verification tests
> - change gmac aad max size
> - update documentation and commits comments
>
> Changes in V2:
> - add gcm/gmac algorithm correction
> - unit test rework
>
> Slawomir Mrozowicz (1):
> libcrypto_pmd: initial implementation of SW crypto device
>
> Piotr Azarewicz (2)
> app/test: cryptodev AES tests rework
> app/test: added tests for libcrypto PMD
>
> Daniel Mrzyglod (1)
> examples/l2fwd-crypto: updated example for libcrypto PMD
>
> MAINTAINERS | 4 +
> app/test/Makefile | 2 +-
> app/test/test_cryptodev.c | 1581 ++++++++++++++++++--
> app/test/test_cryptodev.h | 1 +
> app/test/test_cryptodev_aes.c | 687 ---------
> app/test/test_cryptodev_aes.h | 1124 --------------
> app/test/test_cryptodev_aes_test_vectors.h | 1095 ++++++++++++++
> app/test/test_cryptodev_blockcipher.c | 531 +++++++
> app/test/test_cryptodev_blockcipher.h | 125 ++
> app/test/test_cryptodev_des_test_vectors.h | 952 ++++++++++++
> app/test/test_cryptodev_gcm_test_vectors.h | 36 +-
> app/test/test_cryptodev_hash_test_vectors.h | 491 ++++++
> app/test/test_cryptodev_perf.c | 689 ++++++++-
> config/common_base | 6 +
> doc/guides/cryptodevs/index.rst | 1 +
> doc/guides/cryptodevs/libcrypto.rst | 116 ++
> doc/guides/rel_notes/release_16_11.rst | 23 +-
> drivers/crypto/Makefile | 1 +
> drivers/crypto/libcrypto/Makefile | 60 +
> drivers/crypto/libcrypto/rte_libcrypto_pmd.c | 1051 +++++++++++++
> drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c | 708 +++++++++
> .../crypto/libcrypto/rte_libcrypto_pmd_private.h | 174 +++
> .../crypto/libcrypto/rte_pmd_libcrypto_version.map | 3 +
> examples/l2fwd-crypto/main.c | 9 +
> lib/librte_cryptodev/rte_cryptodev.h | 5 +-
> mk/rte.app.mk | 23 +-
> 26 files changed, 7563 insertions(+), 1935 deletions(-)
> delete mode 100644 app/test/test_cryptodev_aes.c
> delete mode 100644 app/test/test_cryptodev_aes.h
> create mode 100644 app/test/test_cryptodev_aes_test_vectors.h
> create mode 100644 app/test/test_cryptodev_blockcipher.c
> create mode 100644 app/test/test_cryptodev_blockcipher.h
> create mode 100644 app/test/test_cryptodev_des_test_vectors.h
> create mode 100644 app/test/test_cryptodev_hash_test_vectors.h
> create mode 100644 doc/guides/cryptodevs/libcrypto.rst
> create mode 100644 drivers/crypto/libcrypto/Makefile
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd.c
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_private.h
> create mode 100644
> drivers/crypto/libcrypto/rte_pmd_libcrypto_version.map
>
> --
> 2.5.0
There are still some checkpatch errors, mainly related to exceeding maximum line length.
Some of these lines are more than 90 character long, so at least these ones should be fixed.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v11 07/24] driver: probe/remove common wrappers for PCI drivers
From: Shreyansh Jain @ 2016-10-04 6:16 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, viktorin, David Marchand, hemant.agrawal
In-Reply-To: <1868462.9szsyjFR7X@xps13>
On Monday 03 October 2016 07:51 PM, Thomas Monjalon wrote:
> 2016-09-20 18:11, Shreyansh Jain:
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -4372,6 +4372,19 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id);
>> int
>> rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
>>
>> +/**
>> + * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
>> + * interface.
>> + */
>> +int rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>> + struct rte_pci_device *pci_dev);
>> +
>> +/**
>> + * Wrapper for use by pci drivers as a .remove function to detach a ethdev
>> + * interface.
>> + */
>> +int rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev);
>
> These functions are used by the drivers only (as helpers).
> So they should be marked @internal (added after applying the patch).
>
Ok.
Thanks!
-
Shreyansh
^ 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