* Re: [PATCH v2 1/8] eal: define container_of macro
From: Shreyansh Jain @ 2016-12-09 7:36 UTC (permalink / raw)
To: Jan Blunck; +Cc: dev, david.marchand, Jan Viktorin
In-Reply-To: <df2a131d-dae7-abc9-bbf4-0ae36caafe5e@nxp.com>
On Thursday 08 December 2016 12:31 PM, Shreyansh Jain wrote:
> Hello Jan,
>
> Need your help in resolving the checkpatch issue for this patch.
> I have used this in EAL Bus series [1].
>
> [1] http://dpdk.org/ml/archives/dev/2016-December/051350.html
>
> On Monday 21 November 2016 10:25 PM, Jan Blunck wrote:
>> This macro is based on Jan Viktorin's original patch but also checks the
>> type of the passed pointer against the type of the member.
>>
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> [jblunck@infradead.org: add type checking and __extension__]
>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>> ---
>> lib/librte_eal/common/include/rte_common.h | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/rte_common.h
>> b/lib/librte_eal/common/include/rte_common.h
>> index db5ac91..8dda3e2 100644
>> --- a/lib/librte_eal/common/include/rte_common.h
>> +++ b/lib/librte_eal/common/include/rte_common.h
>> @@ -331,6 +331,26 @@ rte_bsf32(uint32_t v)
>> #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
>> #endif
>>
>> +/**
>> + * Return pointer to the wrapping struct instance.
>> + *
>> + * Example:
>> + *
>> + * struct wrapper {
>> + * ...
>> + * struct child c;
>> + * ...
>> + * };
>> + *
>> + * struct child *x = obtain(...);
>> + * struct wrapper *w = container_of(x, struct wrapper, c);
>> + */
>> +#ifndef container_of
>> +#define container_of(ptr, type, member) __extension__ ({ \
>> + typeof(((type *)0)->member) *_ptr = (ptr); \
>> + (type *)(((char *)_ptr) - offsetof(type, member)); })
>> +#endif
>
> (I think there was discussion in ML about this but where, I couldn't
> find it).
>
> Above code snippet doesn't go down well with the checkpatch script. It
> reports:
>
> ---->8----
> ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
> parentheses
> #40: FILE: lib/librte_eal/common/include/rte_common.h:349:
> +#define container_of(ptr, type, member) __extension__ ({ \
> + typeof(((type *)0)->member) *_ptr = (ptr); \
> + (type *)(((char *)_ptr) - offsetof(type,
> member)); })
>
> ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
> #41: FILE: lib/librte_eal/common/include/rte_common.h:350:
> + typeof(((type *)0)->member) *_ptr = (ptr); \
> ---->8----
>
> Second error is primarily because of '*<space>_ptr' rather than '*_ptr'.
>
> Is do{ ... }while(0) the fix for the first one?
>
> This:
>
> ---->8----
> #ifndef container_of
> #define container_of(ptr, type, member) do { \
> __extension__ \
> typeof(((type *)0)->member) * _ptr = (ptr); \
> (type *)(((char *)_ptr) - offsetof(type, member));\
> } while (0)
> #endif
> ---->8----
>
> Seems to be ok with checkpatch. Do you see any technical/compiler issue
> with this?
My bad.
obviously a 'do{ .. } while(0)' will not work in case of assignment.
Any other hints as to how to remove this checkpatch issue?
>
>> +
>> #define _RTE_STR(x) #x
>> /** Take a macro value and get a string version of it */
>> #define RTE_STR(x) _RTE_STR(x)
>>
>
> -
> Shreyansh
>
>
>
^ permalink raw reply
* Re: [PATCH v2] doc: introduce PVP reference benchmark
From: Maxime Coquelin @ 2016-12-09 6:29 UTC (permalink / raw)
To: Mcnamara, John, yuanhan.liu@linux.intel.com,
thomas.monjalon@6wind.com, Yang, Zhiyong, ktraynor@redhat.com,
dev@dpdk.org
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202670D72@IRSMSX103.ger.corp.intel.com>
On 12/07/2016 10:07 PM, Mcnamara, John wrote:
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Tuesday, December 6, 2016 12:25 PM
>> To: yuanhan.liu@linux.intel.com; thomas.monjalon@6wind.com; Mcnamara, John
>> <john.mcnamara@intel.com>; Yang, Zhiyong <zhiyong.yang@intel.com>;
>> ktraynor@redhat.com; dev@dpdk.org
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v2] doc: introduce PVP reference benchmark
>>
>> Having reference benchmarks is important in order to obtain reproducible
>> performance figures.
>>
>> This patch describes required steps to configure a PVP setup using testpmd
>> in both host and guest.
>>
>> Not relying on external vSwitch ease integration in a CI loop by not being
>> impacted by DPDK API changes.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> There is one trailing whitespace warning but apart from that:
>
> Acked-by: John McNamara <john.mcnamara@intel.com>
Thanks John,
Do you want me to send a v3, fixing the trailing whitespace &
collecting the acks?
-- Maxime
>
>
>
>> ---
>>
>> Thanks to all the reviewers, this v2 should take all remarks into account.
>>
>> -- Maxime
>>
>> doc/guides/howto/img/pvp_2nics.svg | 556
>> +++++++++++++++++++++++++++
>> doc/guides/howto/index.rst | 1 +
>> doc/guides/howto/pvp_reference_benchmark.rst | 395 +++++++++++++++++++
>> 3 files changed, 952 insertions(+)
>> create mode 100644 doc/guides/howto/img/pvp_2nics.svg
>> create mode 100644 doc/guides/howto/pvp_reference_benchmark.rst
>>
>> diff --git a/doc/guides/howto/img/pvp_2nics.svg
>> b/doc/guides/howto/img/pvp_2nics.svg
>> new file mode 100644
>> index 0000000..517a800
>> --- /dev/null
>> +++ b/doc/guides/howto/img/pvp_2nics.svg
>> @@ -0,0 +1,556 @@
>> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
>> +<!-- Created with Inkscape (http://www.inkscape.org/) -->
>> +
>> +<svg
>> + xmlns:dc="http://purl.org/dc/elements/1.1/"
>> + xmlns:cc="http://creativecommons.org/ns#"
>> + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>> + xmlns:svg="http://www.w3.org/2000/svg"
>> + xmlns="http://www.w3.org/2000/svg"
>> + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
>> + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
>> + width="127.46428mm"
>> + height="139.41411mm"
>> + viewBox="0 0 451.64508 493.987"
>> + id="svg2"
>> + version="1.1"
>> + inkscape:version="0.92pre2 r"
>> + sodipodi:docname="pvp_2nics.svg"
>> + inkscape:export-filename="/home/max/Pictures/dpdk/pvp/pvp.png"
>> + inkscape:export-xdpi="90"
>> + inkscape:export-ydpi="90">
>> + <defs
>> + id="defs4">
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="marker4760"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4762"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="marker4642"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4644"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="marker10370"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + id="path10372"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:isstock="true"
>> + style="overflow:visible"
>> + id="marker10306"
>> + refX="0"
>> + refY="0"
>> + orient="auto"
>> + inkscape:stockid="Arrow1Lend">
>> + <path
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + id="path10308"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:isstock="true"
>> + style="overflow:visible"
>> + id="marker9757"
>> + refX="0"
>> + refY="0"
>> + orient="auto"
>> + inkscape:stockid="Arrow1Lend"
>> + inkscape:collect="always">
>> + <path
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + id="path9759"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lstart"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + id="path4224"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lend"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + id="path4227"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lend-1"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4227-27"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lstart-9"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4224-3"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:isstock="true"
>> + style="overflow:visible"
>> + id="marker9757-0"
>> + refX="0"
>> + refY="0"
>> + orient="auto"
>> + inkscape:stockid="Arrow1Lend">
>> + <path
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + id="path9759-6"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lstart-6"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4224-0"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lend-62"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4227-6"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="marker10370-7"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path10372-9"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:isstock="true"
>> + style="overflow:visible"
>> + id="marker9757-2"
>> + refX="0"
>> + refY="0"
>> + orient="auto"
>> + inkscape:stockid="Arrow1Lend">
>> + <path
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)"
>> + style="fill:#ff0000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#ff0000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + id="path9759-0"
>> + inkscape:connector-curvature="0" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lstart"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lstart-9-2"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4224-3-3"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(0.8,0,0,0.8,10,0)" />
>> + </marker>
>> + <marker
>> + inkscape:stockid="Arrow1Lend"
>> + orient="auto"
>> + refY="0"
>> + refX="0"
>> + id="Arrow1Lend-1-7"
>> + style="overflow:visible"
>> + inkscape:isstock="true">
>> + <path
>> + inkscape:connector-curvature="0"
>> + id="path4227-27-5"
>> + d="M 0,0 5,-5 -12.5,0 5,5 Z"
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:1.00000003pt;stroke-opacity:1"
>> + transform="matrix(-0.8,0,0,-0.8,-10,0)" />
>> + </marker>
>> + </defs>
>> + <sodipodi:namedview
>> + id="base"
>> + pagecolor="#ffffff"
>> + bordercolor="#666666"
>> + borderopacity="1.0"
>> + inkscape:pageopacity="0.0"
>> + inkscape:pageshadow="2"
>> + inkscape:zoom="0.49497475"
>> + inkscape:cx="206.7485"
>> + inkscape:cy="227.93958"
>> + inkscape:document-units="px"
>> + inkscape:current-layer="layer1"
>> + showgrid="false"
>> + inkscape:object-nodes="true"
>> + inkscape:window-width="1916"
>> + inkscape:window-height="1040"
>> + inkscape:window-x="0"
>> + inkscape:window-y="38"
>> + inkscape:window-maximized="0"
>> + inkscape:snap-grids="true"
>> + inkscape:snap-to-guides="true"
>> + inkscape:snap-others="false"
>> + inkscape:snap-nodes="false"
>> + inkscape:snap-global="false" />
>> + <metadata
>> + id="metadata7">
>> + <rdf:RDF>
>> + <cc:Work
>> + rdf:about="">
>> + <dc:format>image/svg+xml</dc:format>
>> + <dc:type
>> + rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
>> + <dc:title></dc:title>
>> + </cc:Work>
>> + </rdf:RDF>
>> + </metadata>
>> + <g
>> + inkscape:label="Layer 1"
>> + inkscape:groupmode="layer"
>> + id="layer1"
>> + transform="translate(-5.3301459,-7.348317)">
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.78969002;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4140"
>> + width="434.38919"
>> + height="75.295639"
>> + x="21.691195"
>> + y="404.59354" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="421.47873"
>> + y="501.3353"
>> + id="text4142"><tspan
>> + sodipodi:role="line"
>> + id="tspan4144"
>> + x="421.47873"
>> + y="501.3353"
>> + style="font-size:25px;line-height:125%">TE</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.46599996;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4146"
>> + width="92.934036"
>> + height="32.324883"
>> + x="182.57764"
>> + y="372.03574" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="183.5878"
>> + y="397.28958"
>> + id="text4148"><tspan
>> + sodipodi:role="line"
>> + id="tspan4150"
>> + x="183.5878"
>> + y="397.28958"
>> + style="font-size:25px">10G NIC</tspan></text>
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="166.92024"
>> + y="451.33276"
>> + id="text4152"><tspan
>> + sodipodi:role="line"
>> + id="tspan4154"
>> + x="166.92024"
>> + y="451.33276">Moongen</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.39882457;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4156"
>> + width="449.73071"
>> + height="244.32167"
>> + x="6.0295582"
>> + y="29.046324" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="405.31628"
>> + y="25.048317"
>> + id="text4158"><tspan
>> + sodipodi:role="line"
>> + id="tspan4160"
>> + x="405.31628"
>> + y="25.048317"
>> + style="font-size:25px">DUT</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.14168489;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4162"
>> + width="418.69415"
>> + height="107.50462"
>> + x="19.038134"
>> + y="41.044758" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="395.38812"
>> + y="66.496857"
>> + id="text4164"><tspan
>> + sodipodi:role="line"
>> + id="tspan4166"
>> + x="395.38812"
>> + y="66.496857"
>> + style="font-size:25px">VM</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.46599996;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4146-3"
>> + width="92.934036"
>> + height="32.324883"
>> + x="183.0827"
>> + y="274.05093" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="184.09286"
>> + y="299.30475"
>> + id="text4148-6"><tspan
>> + sodipodi:role="line"
>> + id="tspan4150-7"
>> + x="184.09286"
>> + y="299.30475"
>> + style="font-size:25px">10G NIC</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:2.4804399;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4189"
>> + width="398.00476"
>> + height="65.451302"
>> + x="26.901583"
>> + y="82.647781" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:25px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="30.683046"
>> + y="108.31288"
>> + id="text4191"><tspan
>> + sodipodi:role="line"
>> + id="tspan4193"
>> + x="30.683046"
>> + y="108.31288">TestPMD</tspan><tspan
>> + sodipodi:role="line"
>> + x="30.683046"
>> + y="139.56288"
>> + id="tspan10476">(macswap)</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:2.49124122;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4189-5"
>> + width="397.22263"
>> + height="66.152573"
>> + x="29.743357"
>> + y="207.6543" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:25px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="42.720772"
>> + y="231.14902"
>> + id="text4191-3"><tspan
>> + sodipodi:role="line"
>> + id="tspan4193-5"
>> + x="42.720772"
>> + y="231.14902">TestPMD </tspan><tspan
>> + sodipodi:role="line"
>> + x="42.720772"
>> + y="262.39902"
>> + id="tspan9747">(io)</tspan></text>
>> + <path
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:0.97838062px;stroke-
>> linecap:butt;stroke-linejoin:miter;stroke-opacity:1;marker-
>> end:url(#Arrow1Lend)"
>> + d="M 202.56669,371.44487 V 308.37034"
>> + id="path4218"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#000000;stroke-
>> width:0.97297633px;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> opacity:1;marker-start:url(#Arrow1Lstart)"
>> + d="M 252.03098,369.63533 V 307.25568"
>> + id="path4218-9"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:0.92982113px;stroke-
>> linecap:butt;stroke-linejoin:miter;stroke-opacity:1;marker-
>> end:url(#Arrow1Lend-1)"
>> + d="M 198.63811,207.44389 V 150.47507"
>> + id="path4218-0"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#000000;stroke-
>> width:0.95360273px;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> opacity:1;marker-start:url(#Arrow1Lstart-9)"
>> + d="M 255.56859,206.9303 V 147.01008"
>> + id="path4218-9-6"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:3, 1;stroke-dashoffset:0;stroke-
>> opacity:1;marker-end:url(#marker9757)"
>> + d="M 199.50513,271.00921 V 207.3696"
>> + id="path9749"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:3, 1;stroke-dashoffset:0;stroke-
>> opacity:1;marker-start:url(#marker10370)"
>> + d="M 255.56859,270.56991 V 206.9303"
>> + id="path9749-2"
>> + inkscape:connector-curvature="0" />
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.46599996;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4146-36"
>> + width="92.934036"
>> + height="32.324883"
>> + x="304.05591"
>> + y="372.52954" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="305.06607"
>> + y="397.78339"
>> + id="text4148-7"><tspan
>> + sodipodi:role="line"
>> + id="tspan4150-5"
>> + x="305.06607"
>> + y="397.78339"
>> + style="font-size:25px">10G NIC</tspan></text>
>> + <rect
>> + style="fill:none;fill-opacity:1;stroke:#000000;stroke-
>> width:1.46599996;stroke-miterlimit:4;stroke-dasharray:none;stroke-
>> dashoffset:0;stroke-opacity:1"
>> + id="rect4146-3-3"
>> + width="92.934036"
>> + height="32.324883"
>> + x="306.07623"
>> + y="273.53461" />
>> + <text
>> + xml:space="preserve"
>> + style="font-style:normal;font-weight:normal;font-size:40px;line-
>> height:125%;font-family:sans-serif;letter-spacing:0px;word-
>> spacing:0px;fill:#000000;fill-opacity:1;stroke:none;stroke-
>> width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
>> + x="307.0864"
>> + y="298.78842"
>> + id="text4148-6-5"><tspan
>> + sodipodi:role="line"
>> + id="tspan4150-7-6"
>> + x="307.0864"
>> + y="298.78842"
>> + style="font-size:25px">10G NIC</tspan></text>
>> + <path
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:0.97838062px;stroke-
>> linecap:butt;stroke-linejoin:miter;stroke-opacity:1;marker-
>> end:url(#Arrow1Lend-62)"
>> + d="M 323.7504,370.24835 V 307.17382"
>> + id="path4218-1"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#000000;stroke-
>> width:0.97297633px;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> opacity:1;marker-start:url(#Arrow1Lstart-6)"
>> + d="M 373.21469,368.43881 V 306.05916"
>> + id="path4218-9-8"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:#000000;fill-opacity:1;fill-
>> rule:evenodd;stroke:#000000;stroke-width:0.92982113px;stroke-
>> linecap:butt;stroke-linejoin:miter;stroke-opacity:1;marker-
>> end:url(#Arrow1Lend-1-7)"
>> + d="M 324.93036,207.24894 V 150.28012"
>> + id="path4218-0-9"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#000000;stroke-
>> width:0.95360273px;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> opacity:1;marker-start:url(#Arrow1Lstart-9-2)"
>> + d="M 381.86084,206.73535 V 146.81513"
>> + id="path4218-9-6-2"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:3, 1;stroke-dashoffset:0;stroke-
>> opacity:1;marker-end:url(#marker9757-2)"
>> + d="M 325.79738,270.81426 V 207.17465"
>> + id="path9749-28"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:3, 1;stroke-dashoffset:0;stroke-
>> opacity:1;marker-start:url(#marker10370-7)"
>> + d="M 381.86084,270.37496 V 206.73535"
>> + id="path9749-2-9"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:2, 1;stroke-dashoffset:0;stroke-
>> opacity:1;marker-end:url(#marker4642)"
>> + d="M 198.57143,148.79077 V 95.93363 h 182.85714 v 50"
>> + id="path3748"
>> + inkscape:connector-curvature="0" />
>> + <path
>> + style="fill:none;fill-rule:evenodd;stroke:#ff0000;stroke-
>> width:1.01005316;stroke-linecap:butt;stroke-linejoin:miter;stroke-
>> miterlimit:4;stroke-dasharray:2.02010632, 1.01005316;stroke-
>> dashoffset:0;stroke-opacity:1;marker-end:url(#marker4760)"
>> + d="m 325.70774,148.78714 v -32.84999 h -70.7012 v 30.70761"
>> + id="path4634"
>> + inkscape:connector-curvature="0" />
>> + </g>
>> +</svg>
>> diff --git a/doc/guides/howto/index.rst b/doc/guides/howto/index.rst index
>> 5575b27..712a9f3 100644
>> --- a/doc/guides/howto/index.rst
>> +++ b/doc/guides/howto/index.rst
>> @@ -38,3 +38,4 @@ HowTo Guides
>> lm_bond_virtio_sriov
>> lm_virtio_vhost_user
>> flow_bifurcation
>> + pvp_reference_benchmark
>> diff --git a/doc/guides/howto/pvp_reference_benchmark.rst
>> b/doc/guides/howto/pvp_reference_benchmark.rst
>> new file mode 100644
>> index 0000000..c5bbd32
>> --- /dev/null
>> +++ b/doc/guides/howto/pvp_reference_benchmark.rst
>> @@ -0,0 +1,395 @@
>> +.. BSD LICENSE
>> + Copyright(c) 2016 Red Hat, Inc. All rights reserved.
>> + All rights reserved.
>> +
>> + Redistribution and use in source and binary forms, with or without
>> + modification, are permitted provided that the following conditions
>> + are met:
>> +
>> + * Redistributions of source code must retain the above copyright
>> + notice, this list of conditions and the following disclaimer.
>> + * Redistributions in binary form must reproduce the above copyright
>> + notice, this list of conditions and the following disclaimer in
>> + the documentation and/or other materials provided with the
>> + distribution.
>> + * Neither the name of Intel Corporation nor the names of its
>> + contributors may be used to endorse or promote products derived
>> + from this software without specific prior written permission.
>> +
>> + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> + "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> + A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> + OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> + DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> + THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> +
>> +
>> +PVP reference benchmark setup using testpmd
>> +===========================================
>> +
>> +This guide lists the steps required to setup a PVP benchmark using
>> +testpmd as a simple forwarder between NICs and Vhost interfaces. The
>> +goal of this setup is to have a reference PVP benchmark without using
>> +external vSwitches (OVS, VPP, ...) to make it easier to obtain
>> +reproducible results and to facilitate continuous integration testing.
>> +
>> +The guide covers two ways of launching the VM, either by directly
>> +calling the QEMU command line, or by relying on libvirt. It has been
>> +tested with DPDK
>> +v16.11 using RHEL7 for both host and guest.
>> +
>> +
>> +Setup overview
>> +--------------
>> +
>> +.. _figure_pvp_2nics:
>> +
>> +.. figure:: img/pvp_2nics.*
>> +
>> + PVP setup using 2 NICs
>> +
>> +In this diagram, each red arrow represents one logical core. This
>> +use-case requires 6 dedicated logical cores. A forwarding configuration
>> +with a single NIC is also possible, requiring 3 logical cores.
>> +
>> +
>> +Host setup
>> +----------
>> +
>> +In this setup, we isolate 6 cores (from CPU2 to CPU7) on the same NUMA
>> +node. Two cores are assigned to the VM vCPUs running testpmd and four
>> +are assigned to testpmd on the host.
>> +
>> +
>> +Host tuning
>> +~~~~~~~~~~~
>> +
>> +#. On BIOS, disable turbo-boost and hyper-threads.
>> +
>> +#. Append these options to Kernel command line:
>> +
>> + .. code-block:: console
>> +
>> + intel_pstate=disable mce=ignore_ce default_hugepagesz=1G
>> + hugepagesz=1G hugepages=6 isolcpus=2-7 rcu_nocbs=2-7 nohz_full=2-7
>> + iommu=pt intel_iommu=on
>> +
>> +#. Disable hyper-threads at runtime if necessary or if BIOS is not
>> accessible:
>> +
>> + .. code-block:: console
>> +
>> + cat /sys/devices/system/cpu/cpu*[0-9]/topology/thread_siblings_list
>> \
>> + | sort | uniq \
>> + | awk -F, '{system("echo 0 >
>> /sys/devices/system/cpu/cpu"$2"/online")}'
>> +
>> +#. Disable NMIs:
>> +
>> + .. code-block:: console
>> +
>> + echo 0 > /proc/sys/kernel/nmi_watchdog
>> +
>> +#. Exclude isolated CPUs from the writeback cpumask:
>> +
>> + .. code-block:: console
>> +
>> + echo ffffff03 > /sys/bus/workqueue/devices/writeback/cpumask
>> +
>> +#. Isolate CPUs from IRQs:
>> +
>> + .. code-block:: console
>> +
>> + clear_mask=0xfc #Isolate CPU2 to CPU7 from IRQs
>> + for i in /proc/irq/*/smp_affinity
>> + do
>> + echo "obase=16;$(( 0x$(cat $i) & ~$clear_mask ))" | bc > $i
>> + done
>> +
>> +
>> +Qemu build
>> +~~~~~~~~~~
>> +
>> +Build Qemu:
>> +
>> + .. code-block:: console
>> +
>> + git clone git://git.qemu.org/qemu.git
>> + cd qemu
>> + mkdir bin
>> + cd bin
>> + ../configure --target-list=x86_64-softmmu
>> +
>> +
>> +DPDK build
>> +~~~~~~~~~~
>> +
>> +Build DPDK:
>> +
>> + .. code-block:: console
>> +
>> + git clone git://dpdk.org/dpdk
>> + cd dpdk
>> + export RTE_SDK=$PWD
>> + make install T=x86_64-native-linuxapp-gcc DESTDIR=install
>> +
>> +
>> +Testpmd launch
>> +~~~~~~~~~~~~~~
>> +
>> +#. Assign NICs to DPDK:
>> +
>> + .. code-block:: console
>> +
>> + modprobe vfio-pci
>> + $RTE_SDK/install/sbin/dpdk-devbind -b vfio-pci 0000:11:00.0
>> + 0000:11:00.1
>> +
>> + .. Note::
>> +
>> + The Sandy Bridge family seems to have some IOMMU limitations giving
>> poor
>> + performance results. To achieve good performance on these machines
>> + consider using UIO instead.
>> +
>> +#. Launch the testpmd application:
>> +
>> + .. code-block:: console
>> +
>> + $RTE_SDK/install/bin/testpmd -l 0,2,3,4,5 --socket-mem=1024 -n 4 \
>> + --vdev 'net_vhost0,iface=/tmp/vhost-user1' \
>> + --vdev 'net_vhost1,iface=/tmp/vhost-user2' -- \
>> + --portmask=f --disable-hw-vlan -i --rxq=1 --txq=1
>> + --nb-cores=4 --forward-mode=io
>> +
>> +With this command, isolated CPUs 2 to 5 will be used as lcores for PMD
>> threads.
>> +
>> +#. In testpmd interactive mode, set the portlist to obtain the correct
>> port
>> + chaining:
>> +
>> + .. code-block:: console
>> +
>> + set portlist 0,2,1,3
>> + start
>> +
>> +
>> +VM launch
>> +~~~~~~~~~
>> +
>> +The VM may be launched either by calling QEMU directly, or by using
>> libvirt.
>> +
>> +#. Qemu way:
>> +
>> +Launch QEMU with two Virtio-net devices paired to the vhost-user
>> +sockets created by testpmd. Below example uses default Virtio-net
>> +options, but options may be specified, for example to disable mergeable
>> +buffers or indirect descriptors.
>> +
>> + .. code-block:: console
>> +
>> + <QEMU path>/bin/x86_64-softmmu/qemu-system-x86_64 \
>> + -enable-kvm -cpu host -m 3072 -smp 3 \
>> + -chardev socket,id=char0,path=/tmp/vhost-user1 \
>> + -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce \
>> + -device virtio-net-
>> pci,netdev=mynet1,mac=52:54:00:02:d9:01,addr=0x10 \
>> + -chardev socket,id=char1,path=/tmp/vhost-user2 \
>> + -netdev type=vhost-user,id=mynet2,chardev=char1,vhostforce \
>> + -device virtio-net-
>> pci,netdev=mynet2,mac=52:54:00:02:d9:02,addr=0x11 \
>> + -object memory-backend-file,id=mem,size=3072M,mem-
>> path=/dev/hugepages,share=on \
>> + -numa node,memdev=mem -mem-prealloc \
>> + -net user,hostfwd=tcp::1002$1-:22 -net nic \
>> + -qmp unix:/tmp/qmp.socket,server,nowait \
>> + -monitor stdio <vm_image>.qcow2
>> +
>> +You can use this `qmp-vcpu-pin
>> +<https://patchwork.kernel.org/patch/9361617/>`_
>> +script to pin vCPUs.
>> +
>> +It can be used as follows, for example to pin 3 vCPUs to CPUs 1, 6 and
>> +7, where isolated CPUs 6 and 7 will be used as lcores for Virtio PMDs:
>> +
>> + .. code-block:: console
>> +
>> + export PYTHONPATH=$PYTHONPATH:<QEMU path>/scripts/qmp
>> + ./qmp-vcpu-pin -s /tmp/qmp.socket 1 6 7
>> +
>> +#. Libvirt way:
>> +
>> +Some initial steps are required for libvirt to be able to connect to
>> +testpmd's sockets.
>> +
>> +First, SELinux policy needs to be set to permissive, since testpmd is
>> +generally run as root (note, as reboot is required):
>> +
>> + .. code-block:: console
>> +
>> + cat /etc/selinux/config
>> +
>> + # This file controls the state of SELinux on the system.
>> + # SELINUX= can take one of these three values:
>> + # enforcing - SELinux security policy is enforced.
>> + # permissive - SELinux prints warnings instead of enforcing.
>> + # disabled - No SELinux policy is loaded.
>> + SELINUX=permissive
>> +
>> + # SELINUXTYPE= can take one of three two values:
>> + # targeted - Targeted processes are protected,
>> + # minimum - Modification of targeted policy.
>> + # Only selected processes are protected.
>> + # mls - Multi Level Security protection.
>> + SELINUXTYPE=targeted
>> +
>> +
>> +Also, Qemu needs to be run as root, which has to be specified in
>> +``/etc/libvirt/qemu.conf``:
>> +
>> + .. code-block:: console
>> +
>> + user = "root"
>> +
>> +Once the domain created, the following snippet is an extract of he most
>> +important information (hugepages, vCPU pinning, Virtio PCI devices):
>> +
>> + .. code-block:: xml
>> +
>> + <domain type='kvm'>
>> + <memory unit='KiB'>3145728</memory>
>> + <currentMemory unit='KiB'>3145728</currentMemory>
>> + <memoryBacking>
>> + <hugepages>
>> + <page size='1048576' unit='KiB' nodeset='0'/>
>> + </hugepages>
>> + <locked/>
>> + </memoryBacking>
>> + <vcpu placement='static'>3</vcpu>
>> + <cputune>
>> + <vcpupin vcpu='0' cpuset='1'/>
>> + <vcpupin vcpu='1' cpuset='6'/>
>> + <vcpupin vcpu='2' cpuset='7'/>
>> + <emulatorpin cpuset='0'/>
>> + </cputune>
>> + <numatune>
>> + <memory mode='strict' nodeset='0'/>
>> + </numatune>
>> + <os>
>> + <type arch='x86_64' machine='pc-i440fx-rhel7.0.0'>hvm</type>
>> + <boot dev='hd'/>
>> + </os>
>> + <cpu mode='host-passthrough'>
>> + <topology sockets='1' cores='3' threads='1'/>
>> + <numa>
>> + <cell id='0' cpus='0-2' memory='3145728' unit='KiB'
>> memAccess='shared'/>
>> + </numa>
>> + </cpu>
>> + <devices>
>> + <interface type='vhostuser'>
>> + <mac address='56:48:4f:53:54:01'/>
>> + <source type='unix' path='/tmp/vhost-user1' mode='client'/>
>> + <model type='virtio'/>
>> + <driver name='vhost' rx_queue_size='256' />
>> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10'
>> function='0x0'/>
>> + </interface>
>> + <interface type='vhostuser'>
>> + <mac address='56:48:4f:53:54:02'/>
>> + <source type='unix' path='/tmp/vhost-user2' mode='client'/>
>> + <model type='virtio'/>
>> + <driver name='vhost' rx_queue_size='256' />
>> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11'
>> function='0x0'/>
>> + </interface>
>> + </devices>
>> + </domain>
>> +
>> +
>> +Guest setup
>> +-----------
>> +
>> +
>> +Guest tuning
>> +~~~~~~~~~~~~
>> +
>> +#. Append these options to the Kernel command line:
>> +
>> + .. code-block:: console
>> +
>> + default_hugepagesz=1G hugepagesz=1G hugepages=1 intel_iommu=on
>> + iommu=pt isolcpus=1,2 rcu_nocbs=1,2 nohz_full=1,2
>> +
>> +#. Disable NMIs:
>> +
>> + .. code-block:: console
>> +
>> + echo 0 > /proc/sys/kernel/nmi_watchdog
>> +
>> +#. Exclude isolated CPU1 and CPU2 from the writeback cpumask:
>> +
>> + .. code-block:: console
>> +
>> + echo 1 > /sys/bus/workqueue/devices/writeback/cpumask
>> +
>> +#. Isolate CPUs from IRQs:
>> +
>> + .. code-block:: console
>> +
>> + clear_mask=0x6 #Isolate CPU1 and CPU2 from IRQs
>> + for i in /proc/irq/*/smp_affinity
>> + do
>> + echo "obase=16;$(( 0x$(cat $i) & ~$clear_mask ))" | bc > $i
>> + done
>> +
>> +
>> +DPDK build
>> +~~~~~~~~~~
>> +
>> +Build DPDK:
>> +
>> + .. code-block:: console
>> +
>> + git clone git://dpdk.org/dpdk
>> + cd dpdk
>> + export RTE_SDK=$PWD
>> + make install T=x86_64-native-linuxapp-gcc DESTDIR=install
>> +
>> +
>> +Testpmd launch
>> +~~~~~~~~~~~~~~
>> +
>> +Probe vfio module without iommu:
>> +
>> + .. code-block:: console
>> +
>> + modprobe -r vfio_iommu_type1
>> + modprobe -r vfio
>> + modprobe vfio enable_unsafe_noiommu_mode=1
>> + cat /sys/module/vfio/parameters/enable_unsafe_noiommu_mode
>> + modprobe vfio-pci
>> +
>> +Bind the virtio-net devices to DPDK:
>> +
>> + .. code-block:: console
>> +
>> + $RTE_SDK/tools/dpdk-devbind.py -b vfio-pci 0000:00:10.0
>> + 0000:00:11.0
>> +
>> +Start testpmd:
>> +
>> + .. code-block:: console
>> +
>> + $RTE_SDK/install/bin/testpmd -l 0,1,2 --socket-mem 1024 -n 4 \
>> + --proc-type auto --file-prefix pg -- \
>> + --portmask=3 --forward-mode=macswap --port-topology=chained \
>> + --disable-hw-vlan --disable-rss -i --rxq=1 --txq=1 \
>> + --rxd=256 --txd=256 --nb-cores=2 --auto-start
>> +
>> +Results template
>> +----------------
>> +
>> +Below template should be used when sharing results:
>> +
>> + .. code-block:: none
>> +
>> + Traffic Generator: <Test equipment (e.g. IXIA, Moongen, ...)>
>> + Acceptable Loss: <n>%
>> + Validation run time: <n>min
>> + Host DPDK version/commit: <version, SHA-1>
>> + Guest DPDK version/commit: <version, SHA-1>
>> + Patches applied: <link to patchwork>
>> + QEMU version/commit: <version>
>> + Virtio features: <features (e.g. mrg_rxbuf='off', leave empty if
>> default)>
>> + CPU: <CPU model>, <CPU frequency>
>> + NIC: <NIC model>
>> + Result: <n> Mpps
>> --
>> 2.9.3
>
^ permalink raw reply
* Re: [PATCH 07/13] pci: replace probe and remove handlers with rte_driver
From: Shreyansh Jain @ 2016-12-09 4:59 UTC (permalink / raw)
To: Ferruh Yigit, dev@dpdk.org
Cc: thomas.monjalon@6wind.com, david.marchand@6wind.com,
Adrien Mazarguil
In-Reply-To: <74db5496-6348-1f18-d4b8-457260cd6e97@intel.com>
Hello Ferruh,
As always, thanks for your checks.
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, December 08, 2016 11:21 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>; dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; david.marchand@6wind.com; Adrien Mazarguil
> <adrien.mazarguil@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 07/13] pci: replace probe and remove handlers
> with rte_driver
>
> Hi Shreyansh,
>
> On 12/4/2016 10:11 AM, Shreyansh Jain wrote:
> > rte_pci_driver probe/remove callback are replaced with the rte_driver
> > based probe/remove. This patch changes all the PCI drivers which reference
> > rte_pci_driver->probe/remove.
> >
> > rte_pci_driver continues to have probe/remove callback which would be used
> > once eth_driver is removed in later patches.
> >
> > Only changes to PCI are done. VDEV changes require a different
> > model because of arguments being passed to vdev->probe/remove.
> >
> > Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > ---
> > drivers/net/bnx2x/bnx2x_ethdev.c | 12 ++++++++----
> > drivers/net/bnxt/bnxt_ethdev.c | 6 ++++--
> > drivers/net/cxgbe/cxgbe_ethdev.c | 6 ++++--
> > drivers/net/e1000/em_ethdev.c | 6 ++++--
> > drivers/net/e1000/igb_ethdev.c | 12 ++++++++----
> > drivers/net/ena/ena_ethdev.c | 6 ++++--
> > drivers/net/enic/enic_ethdev.c | 6 ++++--
> > drivers/net/fm10k/fm10k_ethdev.c | 6 ++++--
> > drivers/net/i40e/i40e_ethdev.c | 6 ++++--
> > drivers/net/i40e/i40e_ethdev_vf.c | 6 ++++--
> > drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++++++++----
> > drivers/net/mlx4/mlx4.c | 6 ++++--
>
> Generating compile error:
> .../net/mlx4/mlx4.c:5913:13: error: subobject initialization overrides
> initialization of other fields within its enclosing subobject
> [-Werror,-Winitializer-overrides]
> .driver = {
> ^
> .../net/mlx4/mlx4.c:5909:13: note: previous initialization is here
> .driver = {
> ^
>
>
> > drivers/net/mlx5/mlx5.c | 4 +++-
>
> .../net/mlx5/mlx5.c:736:13: error: subobject initialization overrides
> initialization of other fields within its enclosing subobject
> [-Werror,-Winitializer-overrides]
> .driver = {
> ^
> .../net/mlx5/mlx5.c:732:13: note: previous initialization is here
> .driver = {
> ^
>
Yes, David Marchand also highlighted this another comment. Somehow I messed up the MLX changes. And just like the last time, I forgot to change my environment configuration to enable it before sending the patches.
>
> > drivers/net/nfp/nfp_net.c | 6 ++++--
> > drivers/net/qede/qede_ethdev.c | 16 ++++++++++------
> > drivers/net/szedata2/rte_eth_szedata2.c | 6 ++++--
> > drivers/net/virtio/virtio_ethdev.c | 4 ++--
> > drivers/net/vmxnet3/vmxnet3_ethdev.c | 6 ++++--
>
> Following drivers seems missing:
> drivers/net/thunderx/nicvf_ethdev.c
I will fix this as well in v2.
>
> Also new added drivers/net/sfc/sfc_ethdev.c, but since sfc is not merged
> to main branch yet, perhaps fix may wait your patches to be applied first.
>
> > lib/librte_eal/common/eal_common_pci.c | 10 ++++++++--
> > lib/librte_eal/common/include/rte_dev.h | 12 ++++++++++++
> > lib/librte_ether/rte_ethdev.c | 16 ++++++++++++----
> > lib/librte_ether/rte_ethdev.h | 6 +++---
> > 22 files changed, 122 insertions(+), 54 deletions(-)
> >
>
> Also getting following with mlx5:
>
> In file included from
> .../x86_64-native-linuxapp-gcc/include/rte_mbuf.h:57:0,
> from .../x86_64-native-linuxapp-gcc/include/rte_ether.h:52,
> from .../drivers/net/mlx5/mlx5_trigger.c:38:
> /usr/include/infiniband/verbs.h: In function 'verbs_get_device':
> .../x86_64-native-linuxapp-gcc/include/rte_common.h:350:40: error:
> initialization discards 'const' qualifier from pointer target type
> [-Werror=discarded-qualifiers]
> typeof(((type *)0)->member) *_ptr = (ptr); \
>
>
> <...>
I will check MLX* again.
Thanks for your inputs.
-
Shreyansh
^ permalink raw reply
* Re: [PATCH v2 15/32] net/i40e: add VF vlan strip func
From: Chen, Jing D @ 2016-12-09 3:07 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <8d2aeee4-f751-c312-7a08-6a5393f20c75@intel.com>
Hi, Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, December 08, 2016 6:43 PM
> To: Chen, Jing D <jing.d.chen@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
>
> On 12/8/2016 9:10 AM, Chen, Jing D wrote:
> > HI, Ferruh,
> >
> > Best Regards,
> > Mark
> >
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, December 07, 2016 10:18 PM
> >> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> >> Cc: Chen, Jing D <jing.d.chen@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
> >>
> >> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> >>> Add a function to configure vlan strip enable/disable for specific
> >>> SRIOV VF device.
> >>>
> >>> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> >>> ---
> >>
> >> <...>
> >>
> >>> +
> >>> +/* Set vlan strip on/off for specific VF from host */
> >>> +int
> >>> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> >>> +{
> >>> + struct rte_eth_dev *dev;
> >>> + struct i40e_pf *pf;
> >>> + struct i40e_vsi *vsi;
> >>> +
> >>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> >>> +
> >>> + dev = &rte_eth_devices[port];
> >>> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> >>> +
> >>> + if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> >>> + PMD_DRV_LOG(ERR, "Invalid argument.");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + vsi = pf->vfs[vf_id].vsi;
> >>> +
> >>> + if (vsi)
> >>> + return i40e_vsi_config_vlan_stripping(vsi, !!on);
> >>> + else
> >>
> >> if vd_if is valid, can vsi be NULL? If so this check may be required in
> >> some prev patches too.
> >
> > It's a little impossible. This sanity check just make the code stronger.
> >
>
> If it is impossible, do you agree to remove this? And if this can be
> possible we must update other patches, almost all other patches assume
> this can't be NULL.
I'll recommend other patches to add it, too.
The reason is we can't image if there is some code change have impact in
future, the necessary sanity check in slow patch make code stronger.
^ permalink raw reply
* Re: ixgbevf: support multicast packets from PF to VF
From: Dey, Souvik @ 2016-12-08 22:07 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Dai, Wei
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC0909393561EC@shsmsx102.ccr.corp.intel.com>
Hi Wenzhuo,
Now I am using rte_eth_dev_set_mc_addr_list to configure the PF with the list of mc_addr supported by the VF, but interestingly only 1 of the mc_addr works and for other mc_addr packets does not even come up to the VF/DPDK PMD.
PMD: ixgbe_update_mc_addr_list_vf(): ixgbe_update_mc_addr_list_vf
PMD: ixgbe_update_mc_addr_list_vf(): MC Addr Count = 5
PMD: ixgbe_mta_vector(): MC Addr0 = 0,MC Addr1 = 1,mc_filter_type =0
PMD: ixgbe_update_mc_addr_list_vf(): Hash value = 0x010
PMD: ixgbe_mta_vector(): MC Addr0 = 0,MC Addr1 = 33,mc_filter_type =0
PMD: ixgbe_update_mc_addr_list_vf(): Hash value = 0x210
PMD: ixgbe_mta_vector(): MC Addr0 = 0,MC Addr1 = 213,mc_filter_type =0
PMD: ixgbe_update_mc_addr_list_vf(): Hash value = 0xD50
PMD: ixgbe_mta_vector(): MC Addr0 = 0,MC Addr1 = 14,mc_filter_type =0
PMD: ixgbe_update_mc_addr_list_vf(): Hash value = 0x0E0
PMD: ixgbe_mta_vector(): MC Addr0 = 0,MC Addr1 = 78,mc_filter_type =0
PMD: ixgbe_update_mc_addr_list_vf(): Hash value = 0x4E0
In the above list only the mc_aadr hash D50 works and the other 2 in RED fails to receive any multicast packets. Is there any other setting or reconfiguration that is required to be done (both in DPDK or on PF) to get this working all the configured MC_ADDRs ?
I am using 2.1 DPDK on the guest and kernel 4.4 ixgbe PF drivers on the host .
--
Regards,
Souvik
From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
Sent: Monday, December 5, 2016 6:35 PM
To: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
Cc: Dai, Wei <wei.dai@intel.com>
Subject: RE: ixgbevf: support multicast packets from PF to VF
Hi Dey,
I'm confused.
rte_eth_allmulticast_enable means the port can receive all the multicast packets. In another word, it's multicast promiscuous mode.
rte_eth_dev_set_mc_addr_list means adding a series of multicast addresses to the filter, so the port can receive these specific multicast packets. It's not promiscuous.
During your test, I think only rte_eth_dev_set_mc_addr_list is working. rte_eth_allmulticast_enable has no effect.
As you mentioned the PF driver version, I'm afraid the problem is the PF. When you call rte_eth_allmulticast_enable on VF, VF only sends a message to PF. PF need to take action. So you must have a PF which can support this feature.
From: Dey, Souvik [mailto:sodey@sonusnet.com]
Sent: Tuesday, December 6, 2016 3:01 AM
To: Lu, Wenzhuo <wenzhuo.lu@intel.com<mailto:wenzhuo.lu@intel.com>>; dev@dpdk.org<mailto:dev@dpdk.org>
Subject: RE: ixgbevf: support multicast packets from PF to VF
Hi Wenzhuo,
There is nothing set with the rte_eth_dev_set_mc_addr_list and we are trying to receive the NS packet which has the destination MAC set as 33 33 ff 00 00 14. Also what I saw is that the handling of allmulticast_enable message in the kernel has happened after 4.0 version and the PF drivers which earlier kernel version will not support this. How should handle those scenarios ?
In my case too I tried 2 experiments :
1. Only set the rte_eth_allmulticast_enable from the DPDK app and I patched the ixgbevf_pmd with our patch. The function was returning SUCCESS but the NS packets were received in the application.
2. Then along with rte_eth_allmulticast_enable, I used the rte_eth_dev_set_mc_addr_list to set the MAC 33 33 ff 00 00 14 from my app to the pmd. After this I was successfully receiving the NS packets. But then the bigger question is how to automate the addition of mc_addr in rte_eth_dev_set_mc_addr_list as in the kni we are currently not using the kni_net_set_rx_mode() function which is called by the net_device whenever the new mc_addr is assigned to the net_device.
--
Regards,
Souvik
From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
Sent: Sunday, December 4, 2016 9:02 PM
To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>; dev@dpdk.org<mailto:dev@dpdk.org>
Subject: RE: ixgbevf: support multicast packets from PF to VF
Hi Souvik,
To my opinion, rte_eth_dev_set_mc_addr_list has nothing to do with rte_eth_allmulticast_enable. rte_eth_allmulticast_enable is enough for the multicast packets.
I'm curious about the 1, what MAC addresses are set by rte_eth_dev_set_mc_addr_list? 2, What multicast packets are sent?
Thanks.
Best regards
Wenzhuo Lu
From: Dey, Souvik [mailto:sodey@sonusnet.com]
Sent: Saturday, December 3, 2016 1:28 AM
To: dev@dpdk.org<mailto:dev@dpdk.org>; Lu, Wenzhuo
Subject: RE: ixgbevf: support multicast packets from PF to VF
Adding wenzhuo.lu@intel.com<mailto:wenzhuo.lu@intel.com>
From: Dey, Souvik
Sent: Friday, December 2, 2016 12:27 PM
To: 'dev@dpdk.org' <dev@dpdk.org<mailto:dev@dpdk.org>>
Subject: ixgbevf: support multicast packets from PF to VF
Hi All,
I am trying to support multicast packet over SRIOV using kernel PF + DPDK VF(ixgbevf) drivers for ipv6. I am currently using 2.1 DPDK and found that there was a patch in 16.04 for "ixgbe: support multicast promiscuous mode on VF". So I have backported the patch to the 2.1 DPDK but still multicast packets were not coming up to the DPDK app. Then I tried to enable the rte_eth_dev_set_mc_addr_list and with the the packets were coming up properly. Now I have some doubts :
1. Do we have to use both rte_eth_dev_set_mc_addr_list and rte_eth_allmulticast_enable to get the multicast packets.
2. How do we get the mc_addr_list dynamically as I don't see we are using the kni_net_set_rx_mode in rte_kni. Without this the DPDK app will not have any idea to update the mc_addr_list in the PF.
3. Is there any other patches which I should be using to get this functionality working.
I am using : DPDK -2.1
Host kernel - 4.4 ( ubuntu)
Guest kernel - 3.2 (Debian)
Drivers - ixgbe ( for both pf and vf).
Thanks in advance for the help and support.
--
Regards,
Souvik
^ permalink raw reply
* Re: [PATCH v2 3/4] app: give python apps a consistent shebang line
From: Mcnamara, John @ 2016-12-08 20:44 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org, mkletzan@redhat.com
In-Reply-To: <3898678.2vNOkO5FOW@xps13>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, December 8, 2016 4:21 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; mkletzan@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v2 3/4] app: give python apps a consistent
> shebang line
>
> 2016-12-08 16:03, John McNamara:
> > Add a consistent "env python" shebang line to the DPDK Python apps so
> > that they can call the default system python.
>
> The shebang is only useful for executable scripts.
> Those included by other python scripts should not have this line.
> Please could you remove the shebang for conf.py and data files?
Good point. In that case I'll squash 3/4 into 2/4 since the shebang change
only affects one executable file, even though it isn't strictly a Python 3
change.
John
^ permalink raw reply
* Re: [PATCH v2 1/6] eventdev: introduce event driven programming model
From: Jerin Jacob @ 2016-12-08 20:41 UTC (permalink / raw)
To: Bruce Richardson
Cc: dev, thomas.monjalon, hemant.agrawal, gage.eads, harry.van.haaren
In-Reply-To: <20161208093048.GA55440@bricha3-MOBL3.ger.corp.intel.com>
On Thu, Dec 08, 2016 at 09:30:49AM +0000, Bruce Richardson wrote:
> On Thu, Dec 08, 2016 at 12:23:03AM +0530, Jerin Jacob wrote:
> > On Tue, Dec 06, 2016 at 04:51:19PM +0000, Bruce Richardson wrote:
> > > On Tue, Dec 06, 2016 at 09:22:15AM +0530, Jerin Jacob wrote:
> > > > In a polling model, lcores poll ethdev ports and associated
> > > > rx queues directly to look for packet. In an event driven model,
> > > > by contrast, lcores call the scheduler that selects packets for
> > > > them based on programmer-specified criteria. Eventdev library
> > > > adds support for event driven programming model, which offer
> > > > applications automatic multicore scaling, dynamic load balancing,
> > > > pipelining, packet ingress order maintenance and
> > > > synchronization services to simplify application packet processing.
> > > >
> > > > By introducing event driven programming model, DPDK can support
> > > > both polling and event driven programming models for packet processing,
> > > > and applications are free to choose whatever model
> > > > (or combination of the two) that best suits their needs.
> > > >
> > > > This patch adds the eventdev specification header file.
> > > >
> > > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > ---
> > > > + /** WORD1 */
> > > > + RTE_STD_C11
> > > > + union {
> > > > + uint64_t u64;
> > > > + /**< Opaque 64-bit value */
> > > > + uintptr_t event_ptr;
> > > > + /**< Opaque event pointer */
> > >
> > > Since we have a uint64_t member of the union, might this be better as a
> > > void * rather than uintptr_t?
> >
> > No strong opinion here. For me, uintptr_t looks clean.
> > But, It is OK to change to void* as per your input.
> >
> > >
> > > > + struct rte_mbuf *mbuf;
> > > > + /**< mbuf pointer if dequeued event is associated with mbuf */
> > > > + };
> > > > +};
> > > > +
> > > <snip>
> > > > +/**
> > > > + * Link multiple source event queues supplied in *rte_event_queue_link*
> > > > + * structure as *queue_id* to the destination event port designated by its
> > > > + * *port_id* on the event device designated by its *dev_id*.
> > > > + *
> > > > + * The link establishment shall enable the event port *port_id* from
> > > > + * receiving events from the specified event queue *queue_id*
> > > > + *
> > > > + * An event queue may link to one or more event ports.
> > > > + * The number of links can be established from an event queue to event port is
> > > > + * implementation defined.
> > > > + *
> > > > + * Event queue(s) to event port link establishment can be changed at runtime
> > > > + * without re-configuring the device to support scaling and to reduce the
> > > > + * latency of critical work by establishing the link with more event ports
> > > > + * at runtime.
> > >
> > > I think this might need to be clarified. The device doesn't need to be
> > > reconfigured, but does it need to be stopped? In SW implementation, this
> > > affects how much we have to make things thread-safe. At minimum I think
> > > we should limit this to having only one thread call the function at a
> > > time, but we may allow enqueue dequeue ops from the data plane to run
> > > in parallel.
> >
> > Cavium implementation can change it at runtime without re-configuring or stopping
> > the device to support runtime load balancing from the application perspective.
> >
> > AFAIK, link establishment is _NOT_ fast path API. But the application
> > can invoke it from worker thread whenever there is a need for re-wiring
> > the queue to port connection for better explicit load balancing. IMO, A
> > software implementation with lock is fine here as we don't use this in
> > fastpath.
> >
> > Thoughts?
> > >
>
> I agree that it's obviously not fast-path. Therefore I suggest that we
> document that this API should be safe to call while the data path is in
> operation, but that it should not be called by multiple cores
> simultaneously i.e. single-writer, multi-reader safe, but not
> multi-writer safe. Does that seem reasonable to you?
If I understand it correctly, per "event port" their will be ONLY ONE
writer at time.
i.e, In the valid case, Following two can be invoked in parallel
rte_event_port_link(dev_id, 0 /*port_id*/,..)
rte_event_port_link(dev_id, 1 /*port_id*/,..)
But, not invoking rte_event_port_link() on the _same_ event port in parallel
Are we on same page?
Jerin
>
> /Bruce
^ permalink raw reply
* Re: Does DPDK 1.7.1 support jumboframes for VMXNET3
From: Yong Wang @ 2016-12-08 18:46 UTC (permalink / raw)
To: Kanika Singhal, dev@dpdk.org
In-Reply-To: <CACrtOHpiTCK4C5NBW5fYfcG3n=Qgjnk4TOXFR3SEJK-D1MyYTA@mail.gmail.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Kanika Singhal
> Sent: Wednesday, December 7, 2016 12:18 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] Does DPDK 1.7.1 support jumboframes for VMXNET3
>
> Hi,
>
> I am a new to DPDK.
>
> My product is using DPDK 1.6 and it does not support jumbo frames for
> VMXNET3.
>
> I have to upgrade to 1.7.1, but i am not able to find info on whether the
> jumbo frame support is being added for VMXNET3 or not.
Jumbo frame for vmxnet3 was introduced in commit
commit 2fdd835f992c2cdf55fc528fea9312c73313103c
Author: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri Mar 4 10:08:00 2016 -0800
This is in release 16.04 and later. Either you need to upgrade to a version 16.04 or onwards or you need to backport that change to your release.
> I can see some jumbo frame related patches being posted for vmxnet3
>
> Do I need to apply the patches in 1.7.1 to make jumbo work/Is there any
> DPDK version which supports jumbo?
>
> Thanks,
> Kanika
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Yong Wang @ 2016-12-08 18:20 UTC (permalink / raw)
To: Ananyev, Konstantin, Yigit, Ferruh, Thomas Monjalon
Cc: Harish Patil, dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd,
Jan Medala, Jakub Palider, John Daley, Adrien Mazarguil,
Alejandro Lucero, Rasesh Mody, Jacob, Jerin, Yuanhan Liu,
Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E4E05@irsmsx105.ger.corp.intel.com>
> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Wednesday, December 7, 2016 2:03 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Yong Wang
> <yongwang@vmware.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Cc: Harish Patil <harish.patil@qlogic.com>; dev@dpdk.org; Rahul Lakkireddy
> <rahul.lakkireddy@chelsio.com>; Stephen Hurd
> <stephen.hurd@broadcom.com>; Jan Medala <jan@semihalf.com>; Jakub
> Palider <jpa@semihalf.com>; John Daley <johndale@cisco.com>; Adrien
> Mazarguil <adrien.mazarguil@6wind.com>; Alejandro Lucero
> <alejandro.lucero@netronome.com>; Rasesh Mody
> <rasesh.mody@qlogic.com>; Jacob, Jerin <Jerin.Jacob@cavium.com>;
> Yuanhan Liu <yuanhan.liu@linux.intel.com>; Kulasek, TomaszX
> <tomaszx.kulasek@intel.com>; olivier.matz@6wind.com
> Subject: RE: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
>
>
> Hi Ferruh,
>
> >
> > On 12/6/2016 6:25 PM, Yong Wang wrote:
> > >> -----Original Message-----
> > >> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> > >> Sent: Sunday, December 4, 2016 4:11 AM
> > >> To: Yong Wang <yongwang@vmware.com>; Thomas Monjalon
> > >> <thomas.monjalon@6wind.com>
> > >> Cc: Harish Patil <harish.patil@qlogic.com>; dev@dpdk.org; Rahul
> Lakkireddy
> > >> <rahul.lakkireddy@chelsio.com>; Stephen Hurd
> > >> <stephen.hurd@broadcom.com>; Jan Medala <jan@semihalf.com>;
> Jakub
> > >> Palider <jpa@semihalf.com>; John Daley <johndale@cisco.com>;
> Adrien
> > >> Mazarguil <adrien.mazarguil@6wind.com>; Alejandro Lucero
> > >> <alejandro.lucero@netronome.com>; Rasesh Mody
> > >> <rasesh.mody@qlogic.com>; Jacob, Jerin <Jerin.Jacob@cavium.com>;
> > >> Yuanhan Liu <yuanhan.liu@linux.intel.com>; Kulasek, TomaszX
> > >> <tomaszx.kulasek@intel.com>; olivier.matz@6wind.com
> > >> Subject: RE: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
> > >>
> > >> Hi
> > >>
> > >>
> > >>
> > >>>>
> > >>
> > >>>> 2016-11-30 17:42, Ananyev, Konstantin:
> > >>
> > >>>>>>> Please, we need a comment for each driver saying
> > >>
> > >>>>>>> "it is OK, we do not need any checksum preparation for TSO"
> > >>
> > >>>>>>> or
> > >>
> > >>>>>>> "yes we have to implement tx_prepare or TSO will not work in
> this
> > >>
> > >>>> mode"
> > >>
> > >>>>>>>
> > >>
> > >>>>>>
> > >>
> > >>>>>> qede PMD doesn’t currently support TSO yet, it only supports Tx
> > >>
> > >>>> TCP/UDP/IP
> > >>
> > >>>>>> csum offloads.
> > >>
> > >>>>>> So Tx preparation isn’t applicable. So as of now -
> > >>
> > >>>>>> "it is OK, we do not need any checksum preparation for TSO"
> > >>
> > >>>>>
> > >>
> > >>>>> Thanks for the answer.
> > >>
> > >>>>> Though please note that it not only for TSO.
> > >>
> > >>>>
> > >>
> > >>>> Oh yes, sorry, my wording was incorrect.
> > >>
> > >>>> We need to know if any checksum preparation is needed prior
> > >>
> > >>>> offloading its final computation to the hardware or driver.
> > >>
> > >>>> So the question applies to TSO and simple checksum offload.
> > >>
> > >>>>
> > >>
> > >>>> We are still waiting answers for
> > >>
> > >>>> bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.
> > >>
> > >>>
> > >>
> > >>> The case for a virtual device is a little bit more complicated as packets
> > >> offloaded from a virtual device can eventually be delivered to
> > >>
> > >>> another virtual NIC or different physical NICs that have different
> offload
> > >> requirements. In ESX, the hypervisor will enforce that the packets
> > >>
> > >>> offloaded will be something that the hardware expects. The contract
> for
> > >> vmxnet3 is that the guest needs to fill in pseudo header checksum
> > >>
> > >>> for both l4 checksum only and TSO + l4 checksum offload cases.
> > >>
> > >>
> > >>
> > >> Ok, so at first glance that looks to me very similar to Intel HW
> requirements.
> > >>
> > >> Could you confirm would rte_net_intel_cksum_prepare()
> > >>
> > >> also work for vmxnet3 or some extra modifications are required?
> > >>
> > >> You can look at it here:
> https://urldefense.proofpoint.com/v2/url?u=http-
> > >>
> 3A__dpdk.org_dev_patchwork_patch_17184_&d=DgIGaQ&c=uilaK90D4TOV
> > >>
> oH58JNXRgQ&r=v4BBYIqiDq552fkYnKKFBFyqvMXOR3UXSdFO2plFD1s&m=NS
> > >> 4zOl2je_tyGhnOJMSnu37HmJxOZf-1KLYcVsu8iYY&s=dL-NOC-
> > >> 18HclXUURQzuyW5Udw4NY13pKMndYvfgCfbA&e= .
> > >>
> > >> Note that for Intel HW the rules for pseudo-header csum calculation
> > >>
> > >> differ for TSO and non-TSO case.
> > >>
> > >> For TSO length inside pseudo-header are set to 0, while for non-tso case
> > >>
> > >> It should be set to L3 payload length.
> > >>
> > >> Is it the same for vmxnet3 or no?
> > >>
> > >> Thanks
> > >>
> > >> Konstantin
> > >>
> > >
> > > Yes and this is the same for vmxnet3.
> > >
> >
> > This means vmxnet3 PMD also should be updated, right?
>
> Yes, that's right.
>
> >Should that update
> > be part of tx_prep patchset? Or separate patch?
>
> Another question I suppose is who will do the actual patch for vmxnet3.
> Yong, are you ok to do the patch for vmxnet3, or prefer us to do that?
> Please note, that in both cases will need your help in testing/reviewing it.
> Konstantin
It will be great if you can put together a patch as part of the entire patchset on tx_prep() for vmxnet3 and I will definitely help review it.
Regarding testing, I can definitely help but I don't have a testing harness to cover the entire matrix (different ESX version, different vmxnet3 device version, VM-VM, VM-physical over different uplinks, etc.) so it will be limited. Related to this, I have the impression that Intel has some existing coverage for vmxnet3 as well as other NICs. Do we know if that will cover this use case as well?
> >
> > >>>
> > >>
> > >>>>> This is for any TX offload for which the upper layer SW would have
> > >>
> > >>>>> to modify the contents of the packet.
> > >>
> > >>>>> Though as I can see for qede neither PKT_TX_IP_CKSUM or
> > >>
> > >>>> PKT_TX_TCP_CKSUM
> > >>
> > >>>>> exhibits any extra requirements for the user.
> > >>
> > >>>>> Is that correct?
> > >>
> > >>
> > >
^ permalink raw reply
* Re: [PATCH v4] eal: restrict cores auto detection
From: Thomas Monjalon @ 2016-12-08 18:19 UTC (permalink / raw)
To: Jianfeng Tan; +Cc: dev, david.marchand, pmatilai, bruce.richardson
In-Reply-To: <1480700936-8450-1-git-send-email-jianfeng.tan@intel.com>
2016-12-02 17:48, Jianfeng Tan:
> This patch uses pthread_getaffinity_np() to narrow down used
> cores when none of below options is specified:
> * coremask (-c)
> * corelist (-l)
> * and coremap (--lcores)
>
> The purpose of this patch is to leave out these core related options
> when DPDK applications are deployed under container env, so that
> users do not need decide the core related parameters when developing
> applications. Instead, when applications are deployed in containers,
> use cpu-set to constrain which cores can be used inside this container
> instance. And DPDK application inside containers just rely on this
> auto detect mechanism to start polling threads.
>
> Note: previously, some users are using isolated CPUs, which could
> be excluded by default. Please add commands like taskset to use
> those cores.
>
> Test example:
> $ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024
Bruce, what do you think of this version?
It requires taskset only if -c, -l and --lcores are not used.
> static int master_lcore_parsed;
> static int mem_parsed;
> +static int core_specified;
I think it's better to keep the word "parsed" as others.
^ permalink raw reply
* Re: [PATCH 00/22] Generic flow API (rte_flow)
From: Ferruh Yigit @ 2016-12-08 17:56 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz
In-Reply-To: <20161208151935.GK10340@6wind.com>
On 12/8/2016 3:19 PM, Adrien Mazarguil wrote:
> Hi Ferruh,
>
> On Fri, Dec 02, 2016 at 04:58:53PM +0000, Ferruh Yigit wrote:
>> Hi Adrien,
>>
>> On 11/16/2016 4:23 PM, Adrien Mazarguil wrote:
>>> As previously discussed in RFC v1 [1], RFC v2 [2], with changes
>>> described in [3] (also pasted below), here is the first non-draft series
>>> for this new API.
>>>
>>> Its capabilities are so generic that its name had to be vague, it may be
>>> called "Generic flow API", "Generic flow interface" (possibly shortened
>>> as "GFI") to refer to the name of the new filter type, or "rte_flow" from
>>> the prefix used for its public symbols. I personally favor the latter.
>>>
>>> While it is currently meant to supersede existing filter types in order for
>>> all PMDs to expose a common filtering/classification interface, it may
>>> eventually evolve to cover the following ideas as well:
>>>
>>> - Rx/Tx offloads configuration through automatic offloads for specific
>>> packets, e.g. performing checksum on TCP packets could be expressed with
>>> an egress rule with a TCP pattern and a kind of checksum action.
>>>
>>> - RSS configuration (already defined actually). Could be global or per rule
>>> depending on hardware capabilities.
>>>
>>> - Switching configuration for devices with many physical ports; rules doing
>>> both ingress and egress could even be used to completely bypass software
>>> if supported by hardware.
>>>
>>> [1] http://dpdk.org/ml/archives/dev/2016-July/043365.html
>>> [2] http://dpdk.org/ml/archives/dev/2016-August/045383.html
>>> [3] http://dpdk.org/ml/archives/dev/2016-November/050044.html
>>>
>>> Changes since RFC v2:
>>>
>>> - New separate VLAN pattern item (previously part of the ETH definition),
>>> found to be much more convenient.
>>>
>>> - Removed useless "any" field from VF pattern item, the same effect can be
>>> achieved by not providing a specification structure.
>>>
>>> - Replaced bit-fields from the VXLAN pattern item to avoid endianness
>>> conversion issues on 24-bit fields.
>>>
>>> - Updated struct rte_flow_item with a new "last" field to create inclusive
>>> ranges. They are defined as the interval between (spec & mask) and
>>> (last & mask). All three parameters are optional.
>>>
>>> - Renamed ID action MARK.
>>>
>>> - Renamed "queue" fields in actions QUEUE and DUP to "index".
>>>
>>> - "rss_conf" field in RSS action is now const.
>>>
>>> - VF action now uses a 32 bit ID like its pattern item counterpart.
>>>
>>> - Removed redundant struct rte_flow_pattern, API functions now expect
>>> struct
>>> rte_flow_item lists terminated by END items.
>>>
>>> - Replaced struct rte_flow_actions for the same reason, with struct
>>> rte_flow_action lists terminated by END actions.
>>>
>>> - Error types (enum rte_flow_error_type) have been updated and the cause
>>> pointer in struct rte_flow_error is now const.
>>>
>>> - Function prototypes (rte_flow_create, rte_flow_validate) have also been
>>> updated for clarity.
>>>
>>> Additions:
>>>
>>> - Public wrapper functions rte_flow_{validate|create|destroy|flush|query}
>>> are now implemented in rte_flow.c, with their symbols exported and
>>> versioned. Related filter type RTE_ETH_FILTER_GENERIC has been added.
>>>
>>> - A separate header (rte_flow_driver.h) has been added for driver-side
>>> functionality, in particular struct rte_flow_ops which contains PMD
>>> callbacks returned by RTE_ETH_FILTER_GENERIC query.
>>>
>>> - testpmd now exposes most of this API through the new "flow" command.
>>>
>>> What remains to be done:
>>>
>>> - Using endian-aware integer types (rte_beX_t) where necessary for clarity.
>>>
>>> - API documentation (based on RFC).
>>>
>>> - testpmd flow command documentation (although context-aware command
>>> completion should already help quite a bit in this regard).
>>>
>>> - A few pattern item / action properties cannot be configured yet
>>> (e.g. rss_conf parameter for RSS action) and a few completions
>>> (e.g. possible queue IDs) should be added.
>>>
>>
>> <...>
>>
>> I was trying to check driver filter API patches, but hit a few compiler
>> errors with this patchset.
>>
>> [1] clang complains about variable bitfield value changed from -1 to 1.
>> Which is correct, but I guess that is intentional, but I don't know how
>> to tell this to clang?
>>
>> [2] shred library compilation error, because of missing rte_flow_flush
>> in rte_ether_version.map file
>>
>> [3] bunch of icc compilation errors, almost all are same type:
>> error #188: enumerated type mixed with another type
>
> Thanks for the report, I'll attempt to address them all in v2. However icc
> error #188 looks like a pain, I think I can work around it but do we really
> not tolerate the use of normal integers inside enum fields in DPDK?
If this warning is not improving the code, and community agree on it, it
is possible to disable warning by adding "-wd188" to test-pmd Makefile
for ICC compiler.
^ permalink raw reply
* Re: [PATCH 07/13] pci: replace probe and remove handlers with rte_driver
From: Ferruh Yigit @ 2016-12-08 17:50 UTC (permalink / raw)
To: Shreyansh Jain, dev; +Cc: thomas.monjalon, david.marchand, Adrien Mazarguil
In-Reply-To: <1480846288-2517-8-git-send-email-shreyansh.jain@nxp.com>
Hi Shreyansh,
On 12/4/2016 10:11 AM, Shreyansh Jain wrote:
> rte_pci_driver probe/remove callback are replaced with the rte_driver
> based probe/remove. This patch changes all the PCI drivers which reference
> rte_pci_driver->probe/remove.
>
> rte_pci_driver continues to have probe/remove callback which would be used
> once eth_driver is removed in later patches.
>
> Only changes to PCI are done. VDEV changes require a different
> model because of arguments being passed to vdev->probe/remove.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
> drivers/net/bnx2x/bnx2x_ethdev.c | 12 ++++++++----
> drivers/net/bnxt/bnxt_ethdev.c | 6 ++++--
> drivers/net/cxgbe/cxgbe_ethdev.c | 6 ++++--
> drivers/net/e1000/em_ethdev.c | 6 ++++--
> drivers/net/e1000/igb_ethdev.c | 12 ++++++++----
> drivers/net/ena/ena_ethdev.c | 6 ++++--
> drivers/net/enic/enic_ethdev.c | 6 ++++--
> drivers/net/fm10k/fm10k_ethdev.c | 6 ++++--
> drivers/net/i40e/i40e_ethdev.c | 6 ++++--
> drivers/net/i40e/i40e_ethdev_vf.c | 6 ++++--
> drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++++++++----
> drivers/net/mlx4/mlx4.c | 6 ++++--
Generating compile error:
.../net/mlx4/mlx4.c:5913:13: error: subobject initialization overrides
initialization of other fields within its enclosing subobject
[-Werror,-Winitializer-overrides]
.driver = {
^
.../net/mlx4/mlx4.c:5909:13: note: previous initialization is here
.driver = {
^
> drivers/net/mlx5/mlx5.c | 4 +++-
.../net/mlx5/mlx5.c:736:13: error: subobject initialization overrides
initialization of other fields within its enclosing subobject
[-Werror,-Winitializer-overrides]
.driver = {
^
.../net/mlx5/mlx5.c:732:13: note: previous initialization is here
.driver = {
^
> drivers/net/nfp/nfp_net.c | 6 ++++--
> drivers/net/qede/qede_ethdev.c | 16 ++++++++++------
> drivers/net/szedata2/rte_eth_szedata2.c | 6 ++++--
> drivers/net/virtio/virtio_ethdev.c | 4 ++--
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 6 ++++--
Following drivers seems missing:
drivers/net/thunderx/nicvf_ethdev.c
Also new added drivers/net/sfc/sfc_ethdev.c, but since sfc is not merged
to main branch yet, perhaps fix may wait your patches to be applied first.
> lib/librte_eal/common/eal_common_pci.c | 10 ++++++++--
> lib/librte_eal/common/include/rte_dev.h | 12 ++++++++++++
> lib/librte_ether/rte_ethdev.c | 16 ++++++++++++----
> lib/librte_ether/rte_ethdev.h | 6 +++---
> 22 files changed, 122 insertions(+), 54 deletions(-)
>
Also getting following with mlx5:
In file included from
.../x86_64-native-linuxapp-gcc/include/rte_mbuf.h:57:0,
from .../x86_64-native-linuxapp-gcc/include/rte_ether.h:52,
from .../drivers/net/mlx5/mlx5_trigger.c:38:
/usr/include/infiniband/verbs.h: In function ‘verbs_get_device’:
.../x86_64-native-linuxapp-gcc/include/rte_common.h:350:40: error:
initialization discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
typeof(((type *)0)->member) *_ptr = (ptr); \
<...>
^ permalink raw reply
* Re: [PATCH v2 00/12] Add crypto PMD optimized for ARMv8
From: Jerin Jacob @ 2016-12-08 17:45 UTC (permalink / raw)
To: Zbigniew Bodek; +Cc: Bruce Richardson, pablo.de.lara.guarch, dev
In-Reply-To: <cffdbc51-2732-2917-f8f3-52328a534d1f@caviumnetworks.com>
On Thu, Dec 08, 2016 at 12:32:52PM +0100, Zbigniew Bodek wrote:
> On 08.12.2016 11:24, Bruce Richardson wrote:
> > On Tue, Dec 06, 2016 at 06:32:53PM -0800, zbigniew.bodek@caviumnetworks.com wrote:
> > > From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
> > >
> > > Introduce crypto poll mode driver using ARMv8
> > > cryptographic extensions. This PMD is optimized
> > > to provide performance boost for chained
> > > crypto operations processing, such as:
> > > * encryption + HMAC generation
> > > * decryption + HMAC validation.
> > > In particular, cipher only or hash only
> > > operations are not provided.
> > > Performance gain can be observed in tests
> > > against OpenSSL PMD which also uses ARM
> > > crypto extensions for packets processing.
> > >
> > Hi,
> >
> > great to see more crypto drivers coming into DPDK, thanks.
> >
> > Question: do you know if this code would have any export compliance
> > implications for DPDK - or for those repackaging DPDK? Up till now, all
> > the crypto code used by DPDK was actually packaged in separate libraries
> > that were re-used, meaning that DPDK didn't contain any crypto
> > algorithms itself.
> >
>
> Hello Bruce,
>
> I don't know to be honest. I didn't know the reasoning behind not including
> crypto code for Intel for example. I thought it was due to licensing and
> code control rather than export compliance.
>
> Maybe someone from the DPDK community will know what are the constraints
> related to including crypto algorithms to DPDK.
One of the primary reason why we thought of going with this approach is
for out of the box "distribution" enablement. We thought, if the core crypto
algorithm sits in some git-hub code or public hosted tarball then the
PMD will never be added to standard distributions and which is a setback
for armv8 server ecosystem.
Having said that and as Zbigniew mentioned, We are open for revisiting
the crypto core algorithm and PMD split if there are community concerns
about export compliance. Let us know.
Jerin
>
> Kind regards
> Zbigniew
^ permalink raw reply
* Re: [PATCH v12 1/6] ethdev: add Tx preparation
From: Olivier Matz @ 2016-12-08 17:24 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: Thomas Monjalon, Kulasek, TomaszX, dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E3405@irsmsx105.ger.corp.intel.com>
Hi Konstantin,
On Fri, 2 Dec 2016 16:17:51 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> Hi Olivier,
>
> > -----Original Message-----
> > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > Sent: Friday, December 2, 2016 8:24 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Kulasek, TomaszX
> > <tomaszx.kulasek@intel.com>; dev@dpdk.org Subject: Re: [dpdk-dev]
> > [PATCH v12 1/6] ethdev: add Tx preparation
> >
> > Hi Konstantin,
> >
> > On Fri, 2 Dec 2016 01:06:30 +0000, "Ananyev, Konstantin"
> > <konstantin.ananyev@intel.com> wrote:
> > > >
> > > > 2016-11-23 18:36, Tomasz Kulasek:
> > > > > +/**
> > > > > + * Process a burst of output packets on a transmit queue of
> > > > > an Ethernet device.
> > > > > + *
> > > > > + * The rte_eth_tx_prepare() function is invoked to prepare
> > > > > output packets to be
> > > > > + * transmitted on the output queue *queue_id* of the Ethernet
> > > > > device designated
> > > > > + * by its *port_id*.
> > > > > + * The *nb_pkts* parameter is the number of packets to be
> > > > > prepared which are
> > > > > + * supplied in the *tx_pkts* array of *rte_mbuf* structures,
> > > > > each of them
> > > > > + * allocated from a pool created with
> > > > > rte_pktmbuf_pool_create().
> > > > > + * For each packet to send, the rte_eth_tx_prepare() function
> > > > > performs
> > > > > + * the following operations:
> > > > > + *
> > > > > + * - Check if packet meets devices requirements for tx
> > > > > offloads.
> > > > > + *
> > > > > + * - Check limitations about number of segments.
> > > > > + *
> > > > > + * - Check additional requirements when debug is enabled.
> > > > > + *
> > > > > + * - Update and/or reset required checksums when tx offload
> > > > > is set for packet.
> > > > > + *
> > > > > + * Since this function can modify packet data, provided mbufs
> > > > > must be safely
> > > > > + * writable (e.g. modified data cannot be in shared
> > > > > segment).
> > > >
> > > > I think we will have to remove this limitation in next releases.
> > > > As we don't know how it could affect the API, I suggest to
> > > > declare this API EXPERIMENTAL.
> > >
> > > While I don't really mind to mart it as experimental, I don't
> > > really understand the reasoning: Why " this function can modify
> > > packet data, provided mbufs must be safely writable" suddenly
> > > becomes a problem? That seems like and obvious limitation to me
> > > and let say tx_burst() has the same one. Second, I don't see how
> > > you are going to remove it without introducing a heavy
> > > performance impact. Konstantin
> >
> > About tx_burst(), I don't think we should force the user to provide
> > a writable mbuf. There are many use cases where passing a clone
> > already works as of today and it avoids duplicating the mbuf data.
> > For instance: traffic generator, multicast, bridging/tap, etc...
> >
> > Moreover, this requirement would be inconsistent with the model you
> > are proposing in case of pipeline:
> > - tx_prepare() on core X, may update the data
> > - tx_burst() on core Y, should not touch the data to avoid cache
> > misses
>
> Probably I wasn't very clear in my previous mail.
> I am not saying that we should force the user to pass a writable mbuf.
> What I am saying that for tx_burst() current expectation is that
> after mbuf is handled to tx_burst() user shouldn't try to modify its
> buffer contents till TX engine is done with the buffer (mbuf_free()
> is called by TX func for it). For tx_prep(), I think, it is the same
> though restrictions are a bit more strict: user should not try to
> read/write to the mbuf while tx_prep() is not finished with it. What
> puzzles me is that why that should be the reason to mark tx_prep() as
> experimental. Konstantin
To be sure we're on the same page, let me reword:
- mbufs passed to tx_prepare() by the application must have their
headers (l2_len + l3_len + l4_len) writable because the phdr checksum
can be replaced. It could be precised in the api comment.
- mbufs passed to tx_burst() must not be modified by the driver/hw, nor
by the application.
About the API itself, I have one more question. I know you've
already discussed this a bit with Adrien, I don't want to spawn a new
big thread from here ;)
The API provides tx_prepare() to check the packets have the proper
format, and possibly modify them (ex: csum offload / tso) to match hw
requirements. So it does both checks (number of segments) and fixes
(csum/tso). What determines things that should be checked and things
that should be fixed?
The application gets few information from tx_prepare() about what should
be done to make the packet accepted by the hw, and the actions will
probably be different depending on hardware. So could we imagine that
in the future the function also tries to fix the packet? I've seen your
comment saying that it has to be an application decision, so what about
having a parameter saying "fix the packet" or "don't fix it"?
About rte_eth_desc_lim->nb_seg_max and
rte_eth_desc_lim->nb_mtu_seg_max, I'm still quite reserved, especially
for the 2nd one, because I don't see how it can be used by the
application. Well, it does not hurt to have them, but for me it looks a
bit useless.
Last thing, I think this API should become the default in the future.
For instance, it would prevent the application to calculate a phdr csum
that will not be used by the hw. Not calling tx_prepare() would require
the user/application to exactly knows the underlying hw and the kind of
packet that are generated. So for me it means we'll need to also update
other examples (other testpmd engines, l2fwd, ...). Do you agree?
Regards,
Olivier
^ permalink raw reply
* Re: [PATCH] net/ixgbe: fix typo in comment
From: Ferruh Yigit @ 2016-12-08 17:19 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org
Cc: Zhang, Helin, Ananyev, Konstantin, stable@dpdk.org
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093936147D@shsmsx102.ccr.corp.intel.com>
On 12/8/2016 12:49 AM, Lu, Wenzhuo wrote:
> Hi,
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Thursday, December 8, 2016 1:20 AM
>> To: dev@dpdk.org
>> Cc: Zhang, Helin; Ananyev, Konstantin; stable@dpdk.org
>> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix typo in comment
>>
>> Fixes: c03fcee9abbd ("ixgbe: remove CRC size from byte counters")
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [RFC] ethdev: abstraction layer for QoS hierarchical scheduler
From: Dumitrescu, Cristian @ 2016-12-08 17:18 UTC (permalink / raw)
To: Alan Robertson; +Cc: dev@dpdk.org, Thomas Monjalon
In-Reply-To: <ed5a51a8bda648d2a92504c42ada9381@EMEAWP-EXMB11.corp.brocade.com>
> Hi Cristian,
>
> The way qos works just now should be feasible for dynamic targets. That is
> similar functions
> to rte_sched_port_enqueue() and rte_sched_port_dequeue() would be
> called. The first to
> enqueue the mbufs onto the queues the second to dequeue. The qos
> structures and scheduler
> don't need to be as functionally rich though. I would have thought a simple
> pipe with child
> nodes should suffice for most. That would allow each tunnel/session to be
> shaped and the
> queueing and drop logic inherited from what is there just now.
>
> Thanks,
> Alan.
Hi Alan,
So just to make sure I get this right: you suggest that tunnels/sessions could simply be mapped as one of the layers under the port hierarchy?
Thanks,
Cristian
^ permalink raw reply
* Re: [PATCH] net/bonding: improve non-ip packets RSS
From: Ferruh Yigit @ 2016-12-08 17:13 UTC (permalink / raw)
To: Haifeng Lin, dev; +Cc: declan.doherty, jerry.lilijun
In-Reply-To: <1479460122-18780-1-git-send-email-haifeng.lin@huawei.com>
On 11/18/2016 9:08 AM, Haifeng Lin wrote:
> Most ethernet not support non-ip packets RSS and only first
> queue can used to receive. In this scenario lacp bond can
> only use one queue even if multi queue configured.
>
> We use below formula to change the map between bond_qid and
> slave_qid to let at least slave_num queues to receive packets:
>
> slave_qid = (bond_qid + slave_id) % queue_num
>
> Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
> ---
Reminder for the patch ...
<..>
^ permalink raw reply
* Re: [PATCH] net/ixgbe: ensure link status is updated
From: Ferruh Yigit @ 2016-12-08 17:10 UTC (permalink / raw)
To: Laurent Hardy, helin.zhang, konstantin.ananyev
Cc: dev, Wenzhuo Lu, Zhang Qi, Xiao Wang
In-Reply-To: <583C68FF.5070907@6wind.com>
On 11/28/2016 5:27 PM, Laurent Hardy wrote:
> Hello,
> Is there anyone available to review this patch ?
Adding more developers into cc.
>
> regards,
> Laurent
>
<...>
^ permalink raw reply
* Re: [PATCH 01/22] ethdev: introduce generic flow API
From: Adrien Mazarguil @ 2016-12-08 17:07 UTC (permalink / raw)
To: Kevin Traynor
Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz,
sugesh.chandran
In-Reply-To: <7f65ba09-e6fe-d97a-6ab5-97e84a828a81@redhat.com>
On Fri, Dec 02, 2016 at 09:06:42PM +0000, Kevin Traynor wrote:
> On 12/01/2016 08:36 AM, Adrien Mazarguil wrote:
> > Hi Kevin,
> >
> > On Wed, Nov 30, 2016 at 05:47:17PM +0000, Kevin Traynor wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 04:23 PM, Adrien Mazarguil wrote:
> >>> This new API supersedes all the legacy filter types described in
> >>> rte_eth_ctrl.h. It is slightly higher level and as a result relies more on
> >>> PMDs to process and validate flow rules.
> >>>
> >>> Benefits:
> >>>
> >>> - A unified API is easier to program for, applications do not have to be
> >>> written for a specific filter type which may or may not be supported by
> >>> the underlying device.
> >>>
> >>> - The behavior of a flow rule is the same regardless of the underlying
> >>> device, applications do not need to be aware of hardware quirks.
> >>>
> >>> - Extensible by design, API/ABI breakage should rarely occur if at all.
> >>>
> >>> - Documentation is self-standing, no need to look up elsewhere.
> >>>
> >>> Existing filter types will be deprecated and removed in the near future.
> >>
> >> I'd suggest to add a deprecation notice to deprecation.rst, ideally with
> >> a target release.
> >
> > Will do, not a sure about the target release though. It seems a bit early
> > since no PMD really supports this API yet.
> >
> > [...]
> >>> diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
> >>> new file mode 100644
> >>> index 0000000..064963d
> >>> --- /dev/null
> >>> +++ b/lib/librte_ether/rte_flow.c
> >>> @@ -0,0 +1,159 @@
> >>> +/*-
> >>> + * BSD LICENSE
> >>> + *
> >>> + * Copyright 2016 6WIND S.A.
> >>> + * Copyright 2016 Mellanox.
> >>
> >> There's Mellanox copyright but you are the only signed-off-by - is that
> >> right?
> >
> > Yes, I'm the primary maintainer for Mellanox PMDs and this API was designed
> > on their behalf to expose several features from mlx4/mlx5 as the existing
> > filter types had too many limitations.
> >
> > [...]
> >>> +/* Get generic flow operations structure from a port. */
> >>> +const struct rte_flow_ops *
> >>> +rte_flow_ops_get(uint8_t port_id, struct rte_flow_error *error)
> >>> +{
> >>> + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> >>> + const struct rte_flow_ops *ops;
> >>> + int code;
> >>> +
> >>> + if (unlikely(!rte_eth_dev_is_valid_port(port_id)))
> >>> + code = ENODEV;
> >>> + else if (unlikely(!dev->dev_ops->filter_ctrl ||
> >>> + dev->dev_ops->filter_ctrl(dev,
> >>> + RTE_ETH_FILTER_GENERIC,
> >>> + RTE_ETH_FILTER_GET,
> >>> + &ops) ||
> >>> + !ops))
> >>> + code = ENOTSUP;
> >>> + else
> >>> + return ops;
> >>> + rte_flow_error_set(error, code, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> >>> + NULL, rte_strerror(code));
> >>> + return NULL;
> >>> +}
> >>> +
> >>
> >> Is it expected that the application or pmd will provide locking between
> >> these functions if required? I think it's going to have to be the app.
> >
> > Locking is indeed expected to be performed by applications. This API only
> > documents places where locking would make sense if necessary and expected
> > behavior.
> >
> > Like all control path APIs, this one assumes a single control thread.
> > Applications must take the necessary precautions.
>
> If you look at OVS now it's quite possible that you have 2 rx queues
> serviced by different threads, that would also install the flow rules in
> the software flow caches - possibly that could extend to adding hardware
> flows. There could also be another thread that is querying for stats. So
> anything that can be done to minimise the locking would be helpful -
> maybe query() could be atomic and not require any locking?
I think we need basic functions with as few constraints as possible on PMDs
first, this API being somewhat complex to implement on their side. That
covers the common use case where applications have a single control thread
or otherwise perform locking on their own.
Once the basics are there for most PMDs, we may add new functions, items,
properties and actions that provide additional constraints (timing,
multi-threading and so on), which remain to be defined according to
feedback. It is designed to be extended without causing ABI breakage.
As for query(), let's see how PMDs handle it first. A race between query()
and create() on a given device is almost unavoidable without locking, same
for queries that reset counters in a given flow rule. Basic parallel queries
should not cause any harm otherwise, although this cannot be guaranteed yet.
> > [...]
> >>> +/**
> >>> + * Flow rule attributes.
> >>> + *
> >>> + * Priorities are set on two levels: per group and per rule within groups.
> >>> + *
> >>> + * Lower values denote higher priority, the highest priority for both levels
> >>> + * is 0, so that a rule with priority 0 in group 8 is always matched after a
> >>> + * rule with priority 8 in group 0.
> >>> + *
> >>> + * Although optional, applications are encouraged to group similar rules as
> >>> + * much as possible to fully take advantage of hardware capabilities
> >>> + * (e.g. optimized matching) and work around limitations (e.g. a single
> >>> + * pattern type possibly allowed in a given group).
> >>> + *
> >>> + * Group and priority levels are arbitrary and up to the application, they
> >>> + * do not need to be contiguous nor start from 0, however the maximum number
> >>> + * varies between devices and may be affected by existing flow rules.
> >>> + *
> >>> + * If a packet is matched by several rules of a given group for a given
> >>> + * priority level, the outcome is undefined. It can take any path, may be
> >>> + * duplicated or even cause unrecoverable errors.
> >>
> >> I get what you are trying to do here wrt supporting multiple
> >> pmds/hardware implementations and it's a good idea to keep it flexible.
> >>
> >> Given that the outcome is undefined, it would be nice that the
> >> application has a way of finding the specific effects for verification
> >> and debugging.
> >
> > Right, however it was deemed a bit difficult to manage in many cases hence
> > the vagueness.
> >
> > For example, suppose two rules with the same group and priority, one
> > matching any IPv4 header, the other one any UDP header:
> >
> > - TCPv4 packets => rule #1.
> > - UDPv6 packets => rule #2.
> > - UDPv4 packets => both?
> >
> > That last one is perhaps invalid, checking that some unspecified protocol
> > combination does not overlap is expensive and may miss corner cases, even
> > assuming this is not an issue, what if the application guarantees that no
> > UDPv4 packets can ever hit that rule?
>
> that's fine - I don't expect the software to be able to know what the
> hardware will do with those rules. It's more about trying to get a dump
> from the hardware if something goes wrong. Anyway covered in comment later.
>
> >
> > Suggestions are welcome though, perhaps we can refine the description
> >
> >>> + *
> >>> + * Note that support for more than a single group and priority level is not
> >>> + * guaranteed.
> >>> + *
> >>> + * Flow rules can apply to inbound and/or outbound traffic (ingress/egress).
> >>> + *
> >>> + * Several pattern items and actions are valid and can be used in both
> >>> + * directions. Those valid for only one direction are described as such.
> >>> + *
> >>> + * Specifying both directions at once is not recommended but may be valid in
> >>> + * some cases, such as incrementing the same counter twice.
> >>> + *
> >>> + * Not specifying any direction is currently an error.
> >>> + */
> >>> +struct rte_flow_attr {
> >>> + uint32_t group; /**< Priority group. */
> >>> + uint32_t priority; /**< Priority level within group. */
> >>> + uint32_t ingress:1; /**< Rule applies to ingress traffic. */
> >>> + uint32_t egress:1; /**< Rule applies to egress traffic. */
> >>> + uint32_t reserved:30; /**< Reserved, must be zero. */
> >>> +};
> > [...]
> >>> +/**
> >>> + * RTE_FLOW_ITEM_TYPE_VF
> >>> + *
> >>> + * Matches packets addressed to a virtual function ID of the device.
> >>> + *
> >>> + * If the underlying device function differs from the one that would
> >>> + * normally receive the matched traffic, specifying this item prevents it
> >>> + * from reaching that device unless the flow rule contains a VF
> >>> + * action. Packets are not duplicated between device instances by default.
> >>> + *
> >>> + * - Likely to return an error or never match any traffic if this causes a
> >>> + * VF device to match traffic addressed to a different VF.
> >>> + * - Can be specified multiple times to match traffic addressed to several
> >>> + * specific VFs.
> >>> + * - Can be combined with a PF item to match both PF and VF traffic.
> >>> + *
> >>> + * A zeroed mask can be used to match any VF.
> >>
> >> can you refer explicitly to id
> >
> > If you mean "VF" to "VF ID" then yes, will do it for v2.
> >
> >>> + */
> >>> +struct rte_flow_item_vf {
> >>> + uint32_t id; /**< Destination VF ID. */
> >>> +};
> > [...]
> >>> +/**
> >>> + * Matching pattern item definition.
> >>> + *
> >>> + * A pattern is formed by stacking items starting from the lowest protocol
> >>> + * layer to match. This stacking restriction does not apply to meta items
> >>> + * which can be placed anywhere in the stack with no effect on the meaning
> >>> + * of the resulting pattern.
> >>> + *
> >>> + * A stack is terminated by a END item.
> >>> + *
> >>> + * The spec field should be a valid pointer to a structure of the related
> >>> + * item type. It may be set to NULL in many cases to use default values.
> >>> + *
> >>> + * Optionally, last can point to a structure of the same type to define an
> >>> + * inclusive range. This is mostly supported by integer and address fields,
> >>> + * may cause errors otherwise. Fields that do not support ranges must be set
> >>> + * to the same value as their spec counterparts.
> >>> + *
> >>> + * By default all fields present in spec are considered relevant.* This
> >>
> >> typo "*"
> >
> > No, that's an asterisk for a footnote below. Perhaps it is a bit unusual,
> > would something like "[1]" look better?
>
> oh, I thought it was the start of a comment line gone astray. Maybe "See
> note below", no big deal though.
OK, will change it anyway for clarity.
> >>> + * behavior can be altered by providing a mask structure of the same type
> >>> + * with applicable bits set to one. It can also be used to partially filter
> >>> + * out specific fields (e.g. as an alternate mean to match ranges of IP
> >>> + * addresses).
> >>> + *
> >>> + * Note this is a simple bit-mask applied before interpreting the contents
> >>> + * of spec and last, which may yield unexpected results if not used
> >>> + * carefully. For example, if for an IPv4 address field, spec provides
> >>> + * 10.1.2.3, last provides 10.3.4.5 and mask provides 255.255.0.0, the
> >>> + * effective range is 10.1.0.0 to 10.3.255.255.
> >>> + *
> >
> > See footnote below:
> >
> >>> + * * The defaults for data-matching items such as IPv4 when mask is not
> >>> + * specified actually depend on the underlying implementation since only
> >>> + * recognized fields can be taken into account.
> >>> + */
> >>> +struct rte_flow_item {
> >>> + enum rte_flow_item_type type; /**< Item type. */
> >>> + const void *spec; /**< Pointer to item specification structure. */
> >>> + const void *last; /**< Defines an inclusive range (spec to last). */
> >>> + const void *mask; /**< Bit-mask applied to spec and last. */
> >>> +};
> >>> +
> >>> +/**
> >>> + * Action types.
> >>> + *
> >>> + * Each possible action is represented by a type. Some have associated
> >>> + * configuration structures. Several actions combined in a list can be
> >>> + * affected to a flow rule. That list is not ordered.
> >>> + *
> >>> + * They fall in three categories:
> >>> + *
> >>> + * - Terminating actions (such as QUEUE, DROP, RSS, PF, VF) that prevent
> >>> + * processing matched packets by subsequent flow rules, unless overridden
> >>> + * with PASSTHRU.
> >>> + *
> >>> + * - Non terminating actions (PASSTHRU, DUP) that leave matched packets up
> >>> + * for additional processing by subsequent flow rules.
> >>> + *
> >>> + * - Other non terminating meta actions that do not affect the fate of
> >>> + * packets (END, VOID, MARK, FLAG, COUNT).
> >>> + *
> >>> + * When several actions are combined in a flow rule, they should all have
> >>> + * different types (e.g. dropping a packet twice is not possible). The
> >>> + * defined behavior is for PMDs to only take into account the last action of
> >>> + * a given type found in the list. PMDs still perform error checking on the
> >>> + * entire list.
> >>
> >> why do you define that the pmd will interpret multiple same type rules
> >> in this way...would it not make more sense for the pmd to just return
> >> EINVAL for an invalid set of rules? It seems more transparent for the
> >> application.
> >
> > Well, I had to define something as a default. The reason is that any number
> > of VOID actions may specified and did not want that to be a special case in
> > order to keep PMD parsers as simple as possible. I'll settle for EINVAL (or
> > some other error) if at least one PMD maintainer other than Nelio who
> > intends to implement this API is not convinced by this explanation, all
> > right?
>
> From an API perspective I think it's cleaner to pass or fail with the
> input rather than change it. But yes, please take pmd maintainers input
> as to what is reasonable to check also.
>
> >
> > [...]
> >>> +/**
> >>> + * RTE_FLOW_ACTION_TYPE_MARK
> >>> + *
> >>> + * Attaches a 32 bit value to packets.
> >>> + *
> >>> + * This value is arbitrary and application-defined. For compatibility with
> >>> + * FDIR it is returned in the hash.fdir.hi mbuf field. PKT_RX_FDIR_ID is
> >>> + * also set in ol_flags.
> >>> + */
> >>> +struct rte_flow_action_mark {
> >>> + uint32_t id; /**< 32 bit value to return with packets. */
> >>> +};
> >>
> >> One use case I thought we would be able to do for OVS is classification
> >> in hardware and the unique flow id is sent with the packet to software.
> >> But in OVS the ufid is 128 bits, so it means we can't and there is still
> >> the miniflow extract overhead. I'm not sure if there is a practical way
> >> around this.
> >>
> >> Sugesh (cc'd) has looked at this before and may be able to comment or
> >> correct me.
> >
> > Yes, we settled on 32 bit because currently no known hardware implementation
> > supports more than this. If that changes, another action with a larger type
> > shall be provided (no ABI breakage).
> >
> > Also since even 64 bit would not be enough for the use case you mention,
> > there is no choice but use this as an indirect value (such as an array or
> > hash table index/value).
>
> ok, cool. I think Sugesh has other ideas anyway!
>
> >
> > [...]
> >>> +/**
> >>> + * RTE_FLOW_ACTION_TYPE_RSS
> >>> + *
> >>> + * Similar to QUEUE, except RSS is additionally performed on packets to
> >>> + * spread them among several queues according to the provided parameters.
> >>> + *
> >>> + * Note: RSS hash result is normally stored in the hash.rss mbuf field,
> >>> + * however it conflicts with the MARK action as they share the same
> >>> + * space. When both actions are specified, the RSS hash is discarded and
> >>> + * PKT_RX_RSS_HASH is not set in ol_flags. MARK has priority. The mbuf
> >>> + * structure should eventually evolve to store both.
> >>> + *
> >>> + * Terminating by default.
> >>> + */
> >>> +struct rte_flow_action_rss {
> >>> + const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> >>> + uint16_t queues; /**< Number of entries in queue[]. */
> >>> + uint16_t queue[]; /**< Queues indices to use. */
> >>
> >> I'd try and avoid queue and queues - someone will say "huh?" when
> >> reading code. s/queues/num ?
> >
> > Agreed, will update for v2.
> >
> >>> +};
> >>> +
> >>> +/**
> >>> + * RTE_FLOW_ACTION_TYPE_VF
> >>> + *
> >>> + * Redirects packets to a virtual function (VF) of the current device.
> >>> + *
> >>> + * Packets matched by a VF pattern item can be redirected to their original
> >>> + * VF ID instead of the specified one. This parameter may not be available
> >>> + * and is not guaranteed to work properly if the VF part is matched by a
> >>> + * prior flow rule or if packets are not addressed to a VF in the first
> >>> + * place.
> >>
> >> Not clear what you mean by "not guaranteed to work if...". Please return
> >> fail when this action is used if this is not going to work.
> >
> > Again, this is a case where it is difficult for a PMD to determine if the
> > entire list of flow rules makes sense. Perhaps it does, perhaps whatever
> > goes through has already been filtered out of possible issues.
> >
> > Here the documentation states the precautions an application should take to
> > guarantee it will work as intended. Perhaps it can be reworded (any
> > suggestion?), but a PMD can certainly not provide any strong guarantee.
>
> I see your point. Maybe for easy check things the pmd would return fail,
> but for more complex I agree it's too difficult.
>
> >
> >>> + *
> >>> + * Terminating by default.
> >>> + */
> >>> +struct rte_flow_action_vf {
> >>> + uint32_t original:1; /**< Use original VF ID if possible. */
> >>> + uint32_t reserved:31; /**< Reserved, must be zero. */
> >>> + uint32_t id; /**< VF ID to redirect packets to. */
> >>> +};
> > [...]
> >>> +/**
> >>> + * Check whether a flow rule can be created on a given port.
> >>> + *
> >>> + * While this function has no effect on the target device, the flow rule is
> >>> + * validated against its current configuration state and the returned value
> >>> + * should be considered valid by the caller for that state only.
> >>> + *
> >>> + * The returned value is guaranteed to remain valid only as long as no
> >>> + * successful calls to rte_flow_create() or rte_flow_destroy() are made in
> >>> + * the meantime and no device parameter affecting flow rules in any way are
> >>> + * modified, due to possible collisions or resource limitations (although in
> >>> + * such cases EINVAL should not be returned).
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param[in] attr
> >>> + * Flow rule attributes.
> >>> + * @param[in] pattern
> >>> + * Pattern specification (list terminated by the END pattern item).
> >>> + * @param[in] actions
> >>> + * Associated actions (list terminated by the END action).
> >>> + * @param[out] error
> >>> + * Perform verbose error reporting if not NULL.
> >>> + *
> >>> + * @return
> >>> + * 0 if flow rule is valid and can be created. A negative errno value
> >>> + * otherwise (rte_errno is also set), the following errors are defined:
> >>> + *
> >>> + * -ENOSYS: underlying device does not support this functionality.
> >>> + *
> >>> + * -EINVAL: unknown or invalid rule specification.
> >>> + *
> >>> + * -ENOTSUP: valid but unsupported rule specification (e.g. partial
> >>> + * bit-masks are unsupported).
> >>> + *
> >>> + * -EEXIST: collision with an existing rule.
> >>> + *
> >>> + * -ENOMEM: not enough resources.
> >>> + *
> >>> + * -EBUSY: action cannot be performed due to busy device resources, may
> >>> + * succeed if the affected queues or even the entire port are in a stopped
> >>> + * state (see rte_eth_dev_rx_queue_stop() and rte_eth_dev_stop()).
> >>> + */
> >>> +int
> >>> +rte_flow_validate(uint8_t port_id,
> >>> + const struct rte_flow_attr *attr,
> >>> + const struct rte_flow_item pattern[],
> >>> + const struct rte_flow_action actions[],
> >>> + struct rte_flow_error *error);
> >>
> >> Why not just use rte_flow_create() and get an error? Is it less
> >> disruptive to do a validate and find the rule cannot be created, than
> >> using a create directly?
> >
> > The rationale can be found in the original RFC, which I'll convert to actual
> > documentation in v2. In short:
> >
> > - Calling rte_flow_validate() before rte_flow_create() is useless since
> > rte_flow_create() also performs validation.
> >
> > - We cannot possibly express a full static set of allowed flow rules, even
> > if we could, it usually depends on the current hardware configuration
> > therefore would not be static.
> >
> > - rte_flow_validate() is thus provided as a replacement for capability
> > flags. It can be used to determine during initialization if the underlying
> > device can support the typical flow rules an application might want to
> > provide later and do something useful with that information (e.g. always
> > use software fallback due to HW limitations).
> >
> > - rte_flow_validate() being a subset of rte_flow_create(), it is essentially
> > free to expose.
>
> make sense now, thanks.
>
> >
> >>> +
> >>> +/**
> >>> + * Create a flow rule on a given port.
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param[in] attr
> >>> + * Flow rule attributes.
> >>> + * @param[in] pattern
> >>> + * Pattern specification (list terminated by the END pattern item).
> >>> + * @param[in] actions
> >>> + * Associated actions (list terminated by the END action).
> >>> + * @param[out] error
> >>> + * Perform verbose error reporting if not NULL.
> >>> + *
> >>> + * @return
> >>> + * A valid handle in case of success, NULL otherwise and rte_errno is set
> >>> + * to the positive version of one of the error codes defined for
> >>> + * rte_flow_validate().
> >>> + */
> >>> +struct rte_flow *
> >>> +rte_flow_create(uint8_t port_id,
> >>> + const struct rte_flow_attr *attr,
> >>> + const struct rte_flow_item pattern[],
> >>> + const struct rte_flow_action actions[],
> >>> + struct rte_flow_error *error);
> >>
> >> General question - are these functions threadsafe? In the OVS example
> >> you could have several threads wanting to create flow rules at the same
> >> time for same or different ports.
> >
> > No they aren't, applications have to perform their own locking. The RFC (to
> > be converted to actual documentation in v2) says that:
> >
> > - API operations are synchronous and blocking (``EAGAIN`` cannot be
> > returned).
> >
> > - There is no provision for reentrancy/multi-thread safety, although nothing
> > should prevent different devices from being configured at the same
> > time. PMDs may protect their control path functions accordingly.
>
> other comment above wrt locking.
>
> >
> >>> +
> >>> +/**
> >>> + * Destroy a flow rule on a given port.
> >>> + *
> >>> + * Failure to destroy a flow rule handle may occur when other flow rules
> >>> + * depend on it, and destroying it would result in an inconsistent state.
> >>> + *
> >>> + * This function is only guaranteed to succeed if handles are destroyed in
> >>> + * reverse order of their creation.
> >>
> >> How can the application find this information out on error?
> >
> > Without maintaining a list, they cannot. The specified case is the only
> > possible guarantee. That does not mean PMDs should not do their best to
> > destroy flow rules, only that ordering must remain consistent in case of
> > inability to destroy one.
> >
> > What do you suggest?
>
> I think if the app cannot remove a specific rule it may want to remove
> all rules and deal with flows in software for a time. So once the app
> knows it fails that should be enough.
OK, then since destruction may return an error already, is it fine?
Applications may call rte_flow_flush() (not supposed to fail unless there is
a serious issue, abort() in that case) and switch to SW fallback.
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param flow
> >>> + * Flow rule handle to destroy.
> >>> + * @param[out] error
> >>> + * Perform verbose error reporting if not NULL.
> >>> + *
> >>> + * @return
> >>> + * 0 on success, a negative errno value otherwise and rte_errno is set.
> >>> + */
> >>> +int
> >>> +rte_flow_destroy(uint8_t port_id,
> >>> + struct rte_flow *flow,
> >>> + struct rte_flow_error *error);
> >>> +
> >>> +/**
> >>> + * Destroy all flow rules associated with a port.
> >>> + *
> >>> + * In the unlikely event of failure, handles are still considered destroyed
> >>> + * and no longer valid but the port must be assumed to be in an inconsistent
> >>> + * state.
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param[out] error
> >>> + * Perform verbose error reporting if not NULL.
> >>> + *
> >>> + * @return
> >>> + * 0 on success, a negative errno value otherwise and rte_errno is set.
> >>> + */
> >>> +int
> >>> +rte_flow_flush(uint8_t port_id,
> >>> + struct rte_flow_error *error);
> >>
> >> rte_flow_destroy_all() would be more descriptive (but breaks your style)
> >
> > There are enough underscores as it is. I like flush, if enough people
> > complain we'll change it but it has to occur before the first public
> > release.
> >
> >>> +
> >>> +/**
> >>> + * Query an existing flow rule.
> >>> + *
> >>> + * This function allows retrieving flow-specific data such as counters.
> >>> + * Data is gathered by special actions which must be present in the flow
> >>> + * rule definition.
> >>
> >> re last sentence, it would be good if you can put a link to
> >> RTE_FLOW_ACTION_TYPE_COUNT
> >
> > Will do, I did not know how until very recently.
> >
> >>> + *
> >>> + * @param port_id
> >>> + * Port identifier of Ethernet device.
> >>> + * @param flow
> >>> + * Flow rule handle to query.
> >>> + * @param action
> >>> + * Action type to query.
> >>> + * @param[in, out] data
> >>> + * Pointer to storage for the associated query data type.
> >>
> >> can this be anything other than rte_flow_query_count?
> >
> > Likely in the future. I've only defined this one as a counterpart for
> > existing API functionality and because we wanted to expose it in mlx5.
> >
> >>> + * @param[out] error
> >>> + * Perform verbose error reporting if not NULL.
> >>> + *
> >>> + * @return
> >>> + * 0 on success, a negative errno value otherwise and rte_errno is set.
> >>> + */
> >>> +int
> >>> +rte_flow_query(uint8_t port_id,
> >>> + struct rte_flow *flow,
> >>> + enum rte_flow_action_type action,
> >>> + void *data,
> >>> + struct rte_flow_error *error);
> >>> +
> >>> +#ifdef __cplusplus
> >>> +}
> >>> +#endif
> >>
> >> I don't see a way to dump all the rules for a port out. I think this is
> >> neccessary for degbugging. You could have a look through dpif.h in OVS
> >> and see how dpif_flow_dump_next() is used, it might be a good reference.
> >
> > DPDK does not maintain flow rules and, depending on hardware capabilities
> > and level of compliance, PMDs do not necessarily do it either, particularly
> > since it requires space and application probably have a better method to
> > store these pointers for their own needs.
>
> understood
>
> >
> > What you see here is only a PMD interface. Depending on applications needs,
> > generic helper functions built on top of these may be added to manage flow
> > rules in the future.
>
> I'm thinking of the case where something goes wrong and I want to get a
> dump of all the flow rules from hardware, not query the rules I think I
> have. I don't see a way to do it or something to build a helper on top of?
Generic helper functions would exist on top of this API and would likely
maintain a list of flow rules themselves. The dump in that case would be
entirely implemented in software. I think that recovering flow rules from HW
may be complicated in many cases (even without taking storage allocation and
rules conversion issues into account), therefore if there is really a need
for it, we could perhaps add a dump() function that PMDs are free to
implement later.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH] net/qede: fix resource leak
From: Ferruh Yigit @ 2016-12-08 16:52 UTC (permalink / raw)
To: Yong Wang, harish.patil; +Cc: dev, Harish Patil
In-Reply-To: <1480509129-27623-1-git-send-email-wang.yong19@zte.com.cn>
On 11/30/2016 12:32 PM, Yong Wang wrote:
> Current code does not close 'fd' on function exit, leaking resources.
>
> Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
> ---
Add new mail address of the maintainer.
CC: Harish Patil <harish.patil@cavium.com>
<...>
^ permalink raw reply
* Re: [PATCH 1/7] net/qede: reduce noise in debug logs
From: Ferruh Yigit @ 2016-12-08 16:48 UTC (permalink / raw)
To: Harish Patil, dev; +Cc: Dept-EngDPDKDev, Rasesh Mody
In-Reply-To: <1480733039-13046-1-git-send-email-harish.patil@qlogic.com>
On 12/3/2016 2:43 AM, Harish Patil wrote:
> From: Rasesh Mody <Rasesh.Mody@cavium.com>
>
> Replace CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER with
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL which is a 32-bit bitmapped value
> where each bit represent a particular submodule to debug. Also move
> notice messages under CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO.
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
> ---
Is 32bit supported by driver?
If so it is throwing a compilation error for it [1], if not can you
please document it?
[1]
In file included from .../drivers/net/qede/base/ecore.h:35:0,
from .../drivers/net/qede/qede_ethdev.h:22,
from .../drivers/net/qede/qede_ethdev.c:9:
.../drivers/net/qede/qede_ethdev.c: In function ‘qede_rss_hash_update’:
.../drivers/net/qede/base/../qede_logs.h:33:3: error: format ‘%lx’
expects argument of type ‘long unsigned int’, but argument 7 has type
‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
"[%s:%d(%s)]" fmt, \
^
.../drivers/net/qede/qede_ethdev.c:1472:2: note: in expansion of macro
‘DP_INFO’
DP_INFO(edev, "RSS hf = 0x%lx len = %u key = %p\n", hf, len, key);
^~~~~~~
cc1: all warnings being treated as errors
<...>
^ permalink raw reply
* Re: [PATCH 7/7] net/qede: restrict maximum queues for PF/VF
From: Ferruh Yigit @ 2016-12-08 16:45 UTC (permalink / raw)
To: Harish Patil, dev; +Cc: Dept-EngDPDKDev
In-Reply-To: <1480733039-13046-7-git-send-email-harish.patil@qlogic.com>
On 12/3/2016 2:43 AM, Harish Patil wrote:
> Fix to adverstise max_rx_queues by taking into account the number
s/adverstise/advertise
> of PF connections instead of returning max_queues supported by the
> HW.
Can you please describe what is the effect, what happens if this is not
fixed, and driver keeps reporting max_queues supported by the HW?
>
> Fixes: 2ea6f76a ("qede: add core driver")
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> ---
<...>
^ permalink raw reply
* Re: [PATCH 6/7] net/qede: fix maximum VF count to 0
From: Ferruh Yigit @ 2016-12-08 16:45 UTC (permalink / raw)
To: Harish Patil, dev; +Cc: Dept-EngDPDKDev
In-Reply-To: <1480733039-13046-6-git-send-email-harish.patil@qlogic.com>
On 12/3/2016 2:43 AM, Harish Patil wrote:
> Set max_vfs to 0 since it is relevant only to SR-IOV PF
> which is not supported yet.
>
> Fixes: 2ea6f76a ("qede: add core driver")
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
Can you please update patch title to indicate what has been fixed
instead of what has been done in the patch.
btw, while checking feature list, I have seen qede_vf supports SR-IOV,
is that correct?
> ---
> drivers/net/qede/qede_ethdev.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index ee8fb43..10abb8b 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -976,10 +976,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
> dev_info->max_rx_queues = (uint16_t)QEDE_MAX_RSS_CNT(qdev);
> dev_info->max_tx_queues = dev_info->max_rx_queues;
> dev_info->max_mac_addrs = qdev->dev_info.num_mac_addrs;
> - if (IS_VF(edev))
> - dev_info->max_vfs = 0;
> - else
> - dev_info->max_vfs = (uint16_t)NUM_OF_VFS(&qdev->edev);
> + dev_info->max_vfs = 0;
> dev_info->reta_size = ECORE_RSS_IND_TABLE_SIZE;
> dev_info->hash_key_size = ECORE_RSS_KEY_SIZE * sizeof(uint32_t);
> dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;
>
^ permalink raw reply
* Re: [PATCH 2/7] net/qede: refactor filtering code
From: Ferruh Yigit @ 2016-12-08 16:45 UTC (permalink / raw)
To: Harish Patil, dev; +Cc: Dept-EngDPDKDev
In-Reply-To: <1480733039-13046-2-git-send-email-harish.patil@qlogic.com>
On 12/3/2016 2:43 AM, Harish Patil wrote:
> The filter_config in qed_eth_ops_pass is a wrapper call driving all the
> filtering configuration. This requires defining multiple structures and
> passing different function arguments for each filter type which is
> unnecessary. So as part of this refactoring we remove filter_config from
> qed_eth_ops_pass and invoke base apis directly. Another change is to
> use a singly list for unicast/multicast macs and thereby prevent duplicate
singly linked list?
> entries.
>
> This change is primarily intended to support future tunneling support
> which makes use of existing L2 filtering/classifications.
>
> Fixes: 2ea6f76a ("qede: add core driver")
What is fixed in this patch, isn't it a refactor?
btw, all Fixes formats are wrong in the patchset, can you please use the
git alias provided:
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> ---
<...>
^ permalink raw reply
* Re: [PATCH 1/7] net/qede: reduce noise in debug logs
From: Ferruh Yigit @ 2016-12-08 16:44 UTC (permalink / raw)
To: Harish Patil, dev; +Cc: Dept-EngDPDKDev, Rasesh Mody
In-Reply-To: <1480733039-13046-1-git-send-email-harish.patil@qlogic.com>
On 12/3/2016 2:43 AM, Harish Patil wrote:
> From: Rasesh Mody <Rasesh.Mody@cavium.com>
>
> Replace CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER with
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL which is a 32-bit bitmapped value
> where each bit represent a particular submodule to debug. Also move
> notice messages under CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO.
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
> ---
> config/common_base | 2 +-
> doc/guides/nics/qede.rst | 4 ++--
> drivers/net/qede/qede_ethdev.c | 4 ++--
> drivers/net/qede/qede_logs.h | 21 +++++----------------
> 4 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/config/common_base b/config/common_base
> index 4bff83a..2ffd557 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -320,7 +320,7 @@ CONFIG_RTE_LIBRTE_BOND_DEBUG_ALB_L1=n
> CONFIG_RTE_LIBRTE_QEDE_PMD=y
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_INIT=n
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO=n
> -CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER=n
> +CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL=0
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX=n
> CONFIG_RTE_LIBRTE_QEDE_DEBUG_RX=n
> #Provides abs path/name of the firmware file.
> diff --git a/doc/guides/nics/qede.rst b/doc/guides/nics/qede.rst
> index d22ecdd..ddf4248 100644
> --- a/doc/guides/nics/qede.rst
> +++ b/doc/guides/nics/qede.rst
> @@ -103,9 +103,9 @@ enabling debugging options may affect system performance.
>
> Toggle display of generic debugging messages.
>
> -- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER`` (default **n**)
> +- ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_VAL`` (default **0**)
Does it make sense to document how DEBUG_VAL used?
Also commit log says bitmapped value to enable/disable a particular
submodule, you may want to document here which value enable/disable
which submodule.
>
> - Toggle display of ecore related messages.
> + Control driver debug verbosity using 32-bit bitmap flags.
>
> - ``CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX`` (default **n**)
>
<...>
> diff --git a/drivers/net/qede/qede_logs.h b/drivers/net/qede/qede_logs.h
> index 45c4af0..08fdf04 100644
> --- a/drivers/net/qede/qede_logs.h
> +++ b/drivers/net/qede/qede_logs.h
> @@ -16,15 +16,18 @@
> (p_dev)->name ? (p_dev)->name : "", \
> ##__VA_ARGS__)
>
> +#ifdef RTE_LIBRTE_QEDE_DEBUG_INFO
Is "_INFO" carries any meaning in this config option, why not just
RTE_LIBRTE_QEDE_DEBUG?
> #define DP_NOTICE(p_dev, is_assert, fmt, ...) \
> rte_log(RTE_LOG_NOTICE, RTE_LOGTYPE_PMD,\
> "[QEDE PMD: (%s)]%s:" fmt, \
> (p_dev)->name ? (p_dev)->name : "", \
> __func__, \
> ##__VA_ARGS__)
> +#else
> +#define DP_NOTICE(p_dev, fmt, ...) do { } while (0)
> +#endif
>
> #ifdef RTE_LIBRTE_QEDE_DEBUG_INFO
> -
> #define DP_INFO(p_dev, fmt, ...) \
> rte_log(RTE_LOG_INFO, RTE_LOGTYPE_PMD, \
> "[%s:%d(%s)]" fmt, \
> @@ -33,10 +36,8 @@
> ##__VA_ARGS__)
> #else
> #define DP_INFO(p_dev, fmt, ...) do { } while (0)
> -
> #endif
>
> -#ifdef RTE_LIBRTE_QEDE_DEBUG_DRIVER
Are you sure you want to enable DP_VERBOSE, I guess most verbose log
macro, by default? Perhaps may want to control it via
RTE_LIBRTE_QEDE_DEBUG_INFO?
> #define DP_VERBOSE(p_dev, module, fmt, ...) \
> do { \
> if ((p_dev)->dp_module & module) \
> @@ -46,9 +47,7 @@
> (p_dev)->name ? (p_dev)->name : "", \
> ##__VA_ARGS__); \
> } while (0)
> -#else
> -#define DP_VERBOSE(p_dev, fmt, ...) do { } while (0)
> -#endif
> +
>
<...>
^ 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