* Re: [PATCH v2] doc: introduce PVP reference benchmark
From: Mcnamara, John @ 2016-12-07 21:07 UTC (permalink / raw)
To: Maxime Coquelin, yuanhan.liu@linux.intel.com,
thomas.monjalon@6wind.com, Yang, Zhiyong, ktraynor@redhat.com,
dev@dpdk.org
In-Reply-To: <20161206122440.12039-1-maxime.coquelin@redhat.com>
> -----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 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 v2 09/12] doc/armv8: update documentation about crypto PMD
From: Mcnamara, John @ 2016-12-07 21:13 UTC (permalink / raw)
To: zbigniew.bodek@caviumnetworks.com, De Lara Guarch, Pablo,
jerin.jacob@caviumnetworks.com
Cc: dev@dpdk.org
In-Reply-To: <1481077985-4224-10-git-send-email-zbigniew.bodek@caviumnetworks.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> zbigniew.bodek@caviumnetworks.com
> Sent: Wednesday, December 7, 2016 2:33 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org; Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v2 09/12] doc/armv8: update documentation about
> crypto PMD
>
> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>
> Add documentation about the driver and update release notes.
>
> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH v2 32/32] app/testpmd: fix invalid port ID
From: Lu, Wenzhuo @ 2016-12-08 0:49 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Chen, Jing D
In-Reply-To: <49f75886-8149-4900-c948-ea75be694b11@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 11:49 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: Chen, Jing D
> Subject: Re: [dpdk-dev] [PATCH v2 32/32] app/testpmd: fix invalid port ID
>
> On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> > Some CLIs don't check the input port ID, it may cause segmentation
> > fault (core dumped).
>
> Are these functions (that we are adding extra check) added with this patchset? If
> so why not add these checks where function implemented.
>
> If these functions are already implemented before this patchset, this patch can
> be standalone patch, instead of being part of this patchset, and can be before
> this patchset so that it can be easily backported to stable trees.
This patch is for the existing code. I think you're right. I'll remove it from this patch set and send an isolate patch for it.
>
> >
> > Fixes: 425781ff5afe ("app/testpmd: add ixgbe VF management")
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
>
> <...>
^ permalink raw reply
* Re: [PATCH] net/ixgbe: fix typo in comment
From: Lu, Wenzhuo @ 2016-12-08 0:49 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org
Cc: Zhang, Helin, Ananyev, Konstantin, stable@dpdk.org
In-Reply-To: <20161207171948.19488-1-ferruh.yigit@intel.com>
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>
^ permalink raw reply
* Re: [PATCH v2] eal: optimize aligned rte_memcpy
From: Yao, Lei A @ 2016-12-08 0:55 UTC (permalink / raw)
To: Wang, Zhihong, dev@dpdk.org
Cc: yuanhan.liu@linux.intel.com, thomas.monjalon@6wind.com
In-Reply-To: <1481074266-4461-1-git-send-email-zhihong.wang@intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
- Apply patch to v16.11
I have tested the loopback performance for this patch on 3 following settings:
CPU: IVB
Ubutnu16.04
Kernal: 4.4.0
gcc : 5.4.0
CPU: HSW
Fedora 21
Kernal: 4.1.13
gcc: 4.9.2
CPU:BDW
Ubutnu16.04
Kernal: 4.4.0
gcc : 5.4.0
I can see 10%~20% performance gain for different packet size on mergeable path. Only on IVB + gcc5.4.0, slight performance drop(~4%) on vector path for packet size 128 ,260. It's may related to gcc version as this performance drop not see with gcc 6+.
-----Original Message-----
From: Wang, Zhihong
Sent: Wednesday, December 7, 2016 9:31 AM
To: dev@dpdk.org
Cc: yuanhan.liu@linux.intel.com; thomas.monjalon@6wind.com; Yao, Lei A <lei.a.yao@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
Subject: [PATCH v2] eal: optimize aligned rte_memcpy
This patch optimizes rte_memcpy for well aligned cases, where both
dst and src addr are aligned to maximum MOV width. It introduces a
dedicated function called rte_memcpy_aligned to handle the aligned
cases with simplified instruction stream. The existing rte_memcpy
is renamed as rte_memcpy_generic. The selection between them 2 is
done at the entry of rte_memcpy.
The existing rte_memcpy is for generic cases, it handles unaligned
copies and make store aligned, it even makes load aligned for micro
architectures like Ivy Bridge. However alignment handling comes at
a price: It adds extra load/store instructions, which can cause
complications sometime.
DPDK Vhost memcpy with Mergeable Rx Buffer feature as an example:
The copy is aligned, and remote, and there is header write along
which is also remote. In this case the memcpy instruction stream
should be simplified, to reduce extra load/store, therefore reduce
the probability of load/store buffer full caused pipeline stall, to
let the actual memcpy instructions be issued and let H/W prefetcher
goes to work as early as possible.
This patch is tested on Ivy Bridge, Haswell and Skylake, it provides
up to 20% gain for Virtio Vhost PVP traffic, with packet size ranging
from 64 to 1500 bytes.
The test can also be conducted without NIC, by setting loopback
traffic between Virtio and Vhost. For example, modify the macro
TXONLY_DEF_PACKET_LEN to the requested packet size in testpmd.h,
rebuild and start testpmd in both host and guest, then "start" on
one side and "start tx_first 32" on the other.
Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
.../common/include/arch/x86/rte_memcpy.h | 81 +++++++++++++++++++++-
1 file changed, 78 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index b3bfc23..b9785e8 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -69,6 +69,8 @@ rte_memcpy(void *dst, const void *src, size_t n) __attribute__((always_inline));
#ifdef RTE_MACHINE_CPUFLAG_AVX512F
+#define ALIGNMENT_MASK 0x3F
+
/**
* AVX512 implementation below
*/
@@ -189,7 +191,7 @@ rte_mov512blocks(uint8_t *dst, const uint8_t *src, size_t n)
}
static inline void *
-rte_memcpy(void *dst, const void *src, size_t n)
+rte_memcpy_generic(void *dst, const void *src, size_t n)
{
uintptr_t dstu = (uintptr_t)dst;
uintptr_t srcu = (uintptr_t)src;
@@ -308,6 +310,8 @@ COPY_BLOCK_128_BACK63:
#elif defined RTE_MACHINE_CPUFLAG_AVX2
+#define ALIGNMENT_MASK 0x1F
+
/**
* AVX2 implementation below
*/
@@ -387,7 +391,7 @@ rte_mov128blocks(uint8_t *dst, const uint8_t *src, size_t n)
}
static inline void *
-rte_memcpy(void *dst, const void *src, size_t n)
+rte_memcpy_generic(void *dst, const void *src, size_t n)
{
uintptr_t dstu = (uintptr_t)dst;
uintptr_t srcu = (uintptr_t)src;
@@ -499,6 +503,8 @@ COPY_BLOCK_128_BACK31:
#else /* RTE_MACHINE_CPUFLAG */
+#define ALIGNMENT_MASK 0x0F
+
/**
* SSE & AVX implementation below
*/
@@ -677,7 +683,7 @@ __extension__ ({ \
})
static inline void *
-rte_memcpy(void *dst, const void *src, size_t n)
+rte_memcpy_generic(void *dst, const void *src, size_t n)
{
__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
uintptr_t dstu = (uintptr_t)dst;
@@ -821,6 +827,75 @@ COPY_BLOCK_64_BACK15:
#endif /* RTE_MACHINE_CPUFLAG */
+static inline void *
+rte_memcpy_aligned(void *dst, const void *src, size_t n)
+{
+ void *ret = dst;
+
+ /* Copy size <= 16 bytes */
+ if (n < 16) {
+ if (n & 0x01) {
+ *(uint8_t *)dst = *(const uint8_t *)src;
+ src = (const uint8_t *)src + 1;
+ dst = (uint8_t *)dst + 1;
+ }
+ if (n & 0x02) {
+ *(uint16_t *)dst = *(const uint16_t *)src;
+ src = (const uint16_t *)src + 1;
+ dst = (uint16_t *)dst + 1;
+ }
+ if (n & 0x04) {
+ *(uint32_t *)dst = *(const uint32_t *)src;
+ src = (const uint32_t *)src + 1;
+ dst = (uint32_t *)dst + 1;
+ }
+ if (n & 0x08)
+ *(uint64_t *)dst = *(const uint64_t *)src;
+
+ return ret;
+ }
+
+ /* Copy 16 <= size <= 32 bytes */
+ if (n <= 32) {
+ rte_mov16((uint8_t *)dst, (const uint8_t *)src);
+ rte_mov16((uint8_t *)dst - 16 + n,
+ (const uint8_t *)src - 16 + n);
+
+ return ret;
+ }
+
+ /* Copy 32 < size <= 64 bytes */
+ if (n <= 64) {
+ rte_mov32((uint8_t *)dst, (const uint8_t *)src);
+ rte_mov32((uint8_t *)dst - 32 + n,
+ (const uint8_t *)src - 32 + n);
+
+ return ret;
+ }
+
+ /* Copy 64 bytes blocks */
+ for (; n >= 64; n -= 64) {
+ rte_mov64((uint8_t *)dst, (const uint8_t *)src);
+ dst = (uint8_t *)dst + 64;
+ src = (const uint8_t *)src + 64;
+ }
+
+ /* Copy whatever left */
+ rte_mov64((uint8_t *)dst - 64 + n,
+ (const uint8_t *)src - 64 + n);
+
+ return ret;
+}
+
+static inline void *
+rte_memcpy(void *dst, const void *src, size_t n)
+{
+ if (!(((uintptr_t)dst | (uintptr_t)src) & ALIGNMENT_MASK))
+ return rte_memcpy_aligned(dst, src, n);
+ else
+ return rte_memcpy_generic(dst, src, n);
+}
+
#ifdef __cplusplus
}
#endif
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 12/32] net/i40e: set VF MAC from PF support
From: Lu, Wenzhuo @ 2016-12-08 1:01 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org
In-Reply-To: <ade05c4f-1e55-f276-81ff-952ae768cc15@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 10:30 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: Re: [PATCH v2 12/32] net/i40e: set VF MAC from PF support
>
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > Support setting VF MAC address from PF.
> > User can call the API on PF to set a specific VF's MAC address.
> >
> > This will reset the VF.
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
>
> <...>
>
> >
> > +/**
> > + * Set the VF MAC address.
> > + *
> > + * This will reset the vf.
>
> It may be good if I add a comment that this also will remove all existing mac
> filters. Same to commit log perhaps.
Agree. Please:)
^ permalink raw reply
* Re: [PATCH v2 1/6] eventdev: introduce event driven programming model
From: Jerin Jacob @ 2016-12-08 1:24 UTC (permalink / raw)
To: Van Haaren, Harry
Cc: dev@dpdk.org, thomas.monjalon@6wind.com, Richardson, Bruce,
hemant.agrawal@nxp.com, Eads, Gage
In-Reply-To: <E923DB57A917B54B9182A2E928D00FA6129C9515@IRSMSX102.ger.corp.intel.com>
On Wed, Dec 07, 2016 at 10:57:13AM +0000, Van Haaren, Harry wrote:
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
>
> Hi Jerin,
Hi Harry,
>
> Re v2 rte_event struct, there seems to be some changes in the struct layout and field sizes. I've investigated them, and would like to propose some changes to balance the byte-alignment and accessing of the fields.
OK. Looks like balanced byte-alignment makes more sense on IA.We will go with that then.
Few comments below,
>
> These changes target only the first 64 bits of the rte_event struct. I've left the current v2 code for reference, please find my proposed changes below.
>
> > +struct rte_event {
> > + /** WORD0 */
> > + RTE_STD_C11
> > + union {
> > + uint64_t event;
> > + /** Event attributes for dequeue or enqueue operation */
> > + struct {
> > + uint64_t flow_id:20;
> > + /**< Targeted flow identifier for the enqueue and
> > + * dequeue operation.
> > + * The value must be in the range of
> > + * [0, nb_event_queue_flows - 1] which
> > + * previously supplied to rte_event_dev_configure().
> > + */
> > + uint64_t sub_event_type:8;
> > + /**< Sub-event types based on the event source.
> > + * @see RTE_EVENT_TYPE_CPU
> > + */
> > + uint64_t event_type:4;
> > + /**< Event type to classify the event source.
> > + * @see RTE_EVENT_TYPE_ETHDEV, (RTE_EVENT_TYPE_*)
> > + */
> > + uint64_t sched_type:2;
> > + /**< Scheduler synchronization type (RTE_SCHED_TYPE_*)
> > + * associated with flow id on a given event queue
> > + * for the enqueue and dequeue operation.
> > + */
> > + uint64_t queue_id:8;
> > + /**< Targeted event queue identifier for the enqueue or
> > + * dequeue operation.
> > + * The value must be in the range of
> > + * [0, nb_event_queues - 1] which previously supplied to
> > + * rte_event_dev_configure().
> > + */
> > + uint64_t priority:8;
> > + /**< Event priority relative to other events in the
> > + * event queue. The requested priority should in the
> > + * range of [RTE_EVENT_DEV_PRIORITY_HIGHEST,
> > + * RTE_EVENT_DEV_PRIORITY_LOWEST].
> > + * The implementation shall normalize the requested
> > + * priority to supported priority value.
> > + * Valid when the device has
> > + * RTE_EVENT_DEV_CAP_FLAG_EVENT_QOS capability.
> > + */
> > + uint64_t op:2;
> > + /**< The type of event enqueue operation - new/forward/
> > + * etc.This field is not preserved across an instance
> > + * and is undefined on dequeue.
> > + * @see RTE_EVENT_OP_NEW, (RTE_EVENT_OP_*)
> > + */
> > + uint64_t impl_opaque:12;
> > + /**< Implementation specific opaque value.
> > + * An implementation may use this field to hold
> > + * implementation specific value to share between
> > + * dequeue and enqueue operation.
> > + * The application should not modify this field.
> > + */
> > + };
> > + };
>
> struct rte_event {
> /** WORD0 */
> RTE_STD_C11
> union {
> uint64_t event;
> struct {
> uint32_t flow_id: 24;
> uint32_t impl_opaque : 8; /* not defined on deq */
>
> uint8_t queue_id;
> uint8_t priority;
>
> uint8_t operation : 4; /* new fwd drop */
> uint8_t sched_type : 4;
>
> uint8_t event_type : 4;
> uint8_t sub_event_type : 4;
> };
> };
> /** word 1 */
> <snip>
>
>
> The changes made are as follows:
> * Add impl_opaque to the remaining 8 bits of those 32 bits (previous size was 12 bits)
OK
>
> * QueueID and Priority remain 8 bit integers - but now accessible as 8 bit ints.
OK
>
> * Operation and sched_type *increased* to 4 bits each (from previous value of 2) to allow future expansion without ABI changes
Anyway it will break ABI if we add new operation. I would propose to keep 4bit
reserved and add it when required.
>
> * Event type remains constant at 4 bits
OK
> * Restore flow_id to 24 bits of a 32 bit int (previous size was 20 bits)
> * sub-event-type reduced to 4 bits (previous value was 8 bits). Can we think of situations where 16 values for application specified identifiers of each event-type is genuinely not enough?
One packet will not go beyond 16 stages but an application may have more stages and
each packet may go mutually exclusive stages. For example,
packet 0: stagex_0 ->stagex_1
packet 1: stagey_0 ->stagey_1
In that sense, IMO, more than 16 is required.(AFIAK, VPP has any much larger limit on number of stages)
>
> In my opinion this structure layout is more balanced, and will perform better due to less loads that will need masking to access the required value.
OK. Considering more balanced layout and above points. I propose following scheme(based on your input)
union {
uint64_t event;
struct {
uint32_t flow_id: 20;
uint32_t sub_event_type : 8;
uint32_t event_type : 4;
uint8_t rsvd: 4; /* for future additions */
uint8_t operation : 2; /* new fwd drop */
uint8_t sched_type : 2;
uint8_t queue_id;
uint8_t priority;
uint8_t impl_opaque;
};
};
Feedback and improvements welcomed,
^ permalink raw reply
* [PATCH 0/2] minor cleanups
From: Stephen Hemminger @ 2016-12-08 1:47 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
Saw this while reviewing other changes
Stephen Hemminger (2):
eth: get rid of goto's in rte_eth_dev_detach
sched/malloc: remove unnecesary return statements
drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
lib/librte_eal/common/rte_malloc.c | 1 -
lib/librte_ether/rte_ethdev.c | 21 +++++++--------------
lib/librte_sched/rte_bitmap.h | 2 --
4 files changed, 8 insertions(+), 18 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH 1/2] eth: get rid of goto's in rte_eth_dev_detach
From: Stephen Hemminger @ 2016-12-08 1:47 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
In-Reply-To: <20161208014751.24285-1-stephen@networkplumber.org>
Extra goto's to just a return are unnecessary.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/librte_ether/rte_ethdev.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 4209ad0..40c7cc6 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -466,27 +466,20 @@ rte_eth_dev_attach(const char *devargs, uint8_t *port_id)
int
rte_eth_dev_detach(uint8_t port_id, char *name)
{
- int ret = -1;
+ int ret;
- if (name == NULL) {
- ret = -EINVAL;
- goto err;
- }
+ if (name == NULL)
+ return -EINVAL;
/* FIXME: move this to eal, once device flags are relocated there */
- if (rte_eth_dev_is_detachable(port_id))
- goto err;
+ ret = rte_eth_dev_is_detachable(port_id);
+ if (ret < 0)
+ return ret;
snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
"%s", rte_eth_devices[port_id].data->name);
- ret = rte_eal_dev_detach(name);
- if (ret < 0)
- goto err;
- return 0;
-
-err:
- return ret;
+ return rte_eal_dev_detach(name);
}
static int
--
2.10.2
^ permalink raw reply related
* [PATCH 2/2] sched/malloc: remove unnecesary return statements
From: Stephen Hemminger @ 2016-12-08 1:47 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
In-Reply-To: <20161208014751.24285-1-stephen@networkplumber.org>
return statements at end of void functions are unnecessary and unwanted.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
lib/librte_eal/common/rte_malloc.c | 1 -
lib/librte_sched/rte_bitmap.h | 2 --
3 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index a80b6fa..05afcee 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -241,8 +241,8 @@ arp_op_name(uint16_t arp_op, char *buf)
default:
break;
}
+
snprintf(buf, sizeof("Unknown"), "%s", "Unknown");
- return;
}
#endif
#define MaxIPv4String 16
diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index f4a8835..09970b0 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -232,7 +232,6 @@ rte_malloc_dump_stats(FILE *f, __rte_unused const char *type)
fprintf(f, "\tAlloc_count:%u,\n",sock_stats.alloc_count);
fprintf(f, "\tFree_count:%u,\n", sock_stats.free_count);
}
- return;
}
/*
diff --git a/lib/librte_sched/rte_bitmap.h b/lib/librte_sched/rte_bitmap.h
index 010d752..d432fdf 100644
--- a/lib/librte_sched/rte_bitmap.h
+++ b/lib/librte_sched/rte_bitmap.h
@@ -453,8 +453,6 @@ rte_bitmap_clear(struct rte_bitmap *bmp, uint32_t pos)
offset1 = (pos >> RTE_BITMAP_CL_BIT_SIZE_LOG2) & RTE_BITMAP_SLAB_BIT_MASK;
slab1 = bmp->array1 + index1;
*slab1 &= ~(1lu << offset1);
-
- return;
}
static inline int
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v2 1/6] eventdev: introduce event driven programming model
From: Jerin Jacob @ 2016-12-08 1:48 UTC (permalink / raw)
To: Bruce Richardson
Cc: dev, thomas.monjalon, hemant.agrawal, gage.eads, harry.van.haaren
In-Reply-To: <20161207111251.GA22932@bricha3-MOBL3.ger.corp.intel.com>
On Wed, Dec 07, 2016 at 11:12:51AM +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>
> > ---
> > +
> > +/**
> > + * 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.
> > + *
> > + * @param dev_id
> > + * The identifier of the device.
> > + *
> > + * @param port_id
> > + * Event port identifier to select the destination port to link.
> > + *
> > + * @param link
> > + * Points to an array of *nb_links* objects of type *rte_event_queue_link*
> > + * structure which contain the event queue to event port link establishment
> > + * attributes.
> > + * NULL value is allowed, in which case this function links all the configured
> > + * event queues *nb_event_queues* which previously supplied to
> > + * rte_event_dev_configure() to the event port *port_id* with normal servicing
> > + * priority(RTE_EVENT_DEV_PRIORITY_NORMAL).
> > + *
> > + * @param nb_links
> > + * The number of links to establish
> > + *
> > + * @return
> > + * The number of links actually established. The return value can be less than
> > + * the value of the *nb_links* parameter when the implementation has the
> > + * limitation on specific queue to port link establishment or if invalid
> > + * parameters are specified in a *rte_event_queue_link*.
> > + * If the return value is less than *nb_links*, the remaining links at the end
> > + * of link[] are not established, and the caller has to take care of them.
> > + * If return value is less than *nb_links* then implementation shall update the
> > + * rte_errno accordingly, Possible rte_errno values are
> > + * (-EDQUOT) Quota exceeded(Application tried to link the queue configured with
> > + * RTE_EVENT_QUEUE_CFG_FLAG_SINGLE_LINK to more than one event ports)
> > + * (-EINVAL) Invalid parameter
> > + *
> > + */
> > +int
> > +rte_event_port_link(uint8_t dev_id, uint8_t port_id,
> > + const struct rte_event_queue_link link[],
> > + uint16_t nb_links);
> > +
>
> Hi again Jerin,
>
> another small suggestion here. I'm not a big fan of using small
> structures to pass parameters into functions, especially when not all
> fields are always going to be used. Rather than use the event queue link
> structure, can we just pass in two array parameters here - the list of
> QIDs, and the list of priorities. In cases where the eventdev
> implementation does not support link prioritization, or where the app
> does not want different priority mappings , then the second
> array can be null [implying NORMAL priority for the don't care case].
>
> int
> rte_event_port_link(uint8_t dev_id, uint8_t port_id,
> const uint8_t queues[], const uint8_t priorities[],
> uint16_t nb_queues);
>
> This just makes mapping an array of queues easier, as we can just pass
> an array of ints directly in, and it especially makes it easier to
> create a single link via:
>
> rte_event_port_link(dev_id, port_id, &queue_id, NULL, 1);
The reason why I thought of creating "struct rte_event_queue_link",
- Its easy to add new parameter in link attributes if required
- Its _easy_ to implement PAUSE and RESUME in application
PAUSE:
nr_links = rte_event_port_links_get(,,link)
rte_event_port_unlink_all
RESUME:
rte_event_port_link(,,link, nr_links);
No strong opinion here. I will go with your proposal then
>
> Regards,
> /Bruce
^ permalink raw reply
* Re: [PATCH v2] eal: optimize aligned rte_memcpy
From: Yuanhan Liu @ 2016-12-08 2:18 UTC (permalink / raw)
To: Zhihong Wang; +Cc: dev, thomas.monjalon, lei.a.yao
In-Reply-To: <1481074266-4461-1-git-send-email-zhihong.wang@intel.com>
On Tue, Dec 06, 2016 at 08:31:06PM -0500, Zhihong Wang wrote:
> This patch optimizes rte_memcpy for well aligned cases, where both
> dst and src addr are aligned to maximum MOV width. It introduces a
> dedicated function called rte_memcpy_aligned to handle the aligned
> cases with simplified instruction stream. The existing rte_memcpy
> is renamed as rte_memcpy_generic. The selection between them 2 is
> done at the entry of rte_memcpy.
>
> The existing rte_memcpy is for generic cases, it handles unaligned
> copies and make store aligned, it even makes load aligned for micro
> architectures like Ivy Bridge. However alignment handling comes at
> a price: It adds extra load/store instructions, which can cause
> complications sometime.
>
> DPDK Vhost memcpy with Mergeable Rx Buffer feature as an example:
> The copy is aligned, and remote, and there is header write along
> which is also remote. In this case the memcpy instruction stream
> should be simplified, to reduce extra load/store, therefore reduce
> the probability of load/store buffer full caused pipeline stall, to
> let the actual memcpy instructions be issued and let H/W prefetcher
> goes to work as early as possible.
>
> This patch is tested on Ivy Bridge, Haswell and Skylake, it provides
> up to 20% gain for Virtio Vhost PVP traffic, with packet size ranging
> from 64 to 1500 bytes.
>
> The test can also be conducted without NIC, by setting loopback
> traffic between Virtio and Vhost. For example, modify the macro
> TXONLY_DEF_PACKET_LEN to the requested packet size in testpmd.h,
> rebuild and start testpmd in both host and guest, then "start" on
> one side and "start tx_first 32" on the other.
>
>
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
--yliu
^ permalink raw reply
* [RFC] ethdev: expand size of eth_dev_name in next release
From: Stephen Hemminger @ 2016-12-08 2:27 UTC (permalink / raw)
To: dev
This came up while revising earlier work on Hyper-V.
The current versions of DPDK does not have enough space to support
a logical device name in VMBUS. The kernel exposes the VMBUS
devices by GUID in a manner similar to how PCI is expressed
with domain:host:function notation.
In order to support Hyper-V in a direct fashion, the eth_dev name
needs to be expanded. The standard format for text representation of GUID
is 36 bytes (plus null). See uuid_unparse(3).
The other alternative is to use base64 encoding, but this worse for
humans to read, and isn't directly handled by lib uuid.
---
doc/guides/rel_notes/release_17_02.rst | 4 +++-
lib/librte_ether/rte_ethdev.h | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 3b65038..52c97c6 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -116,7 +116,9 @@ ABI Changes
Also, make sure to start the actual text at the margin.
=========================================================
-
+ * The macro ``RTE_ETH_NAME_MAX_LEN`` used in rte_eth_dev_data will be
+ increased from 32 to 40 characters to allow for longer values such
+ as GUID which is 36 characters long (plus null character).
Shared Library Versions
-----------------------
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..68cb956 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1652,7 +1652,11 @@ struct rte_eth_dev_sriov {
};
#define RTE_ETH_DEV_SRIOV(dev) ((dev)->data->sriov)
+#ifdef RTE_NEXT_ABI
+#define RTE_ETH_NAME_MAX_LEN 40
+#else
#define RTE_ETH_NAME_MAX_LEN (32)
+#endif
/**
* @internal
--
2.10.2
^ permalink raw reply related
* [PATCH] doc: fix mistakes in contribution guide
From: Yong Wang @ 2016-12-08 4:19 UTC (permalink / raw)
To: john.mcnamara; +Cc: dev, Yong Wang
Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
---
doc/guides/contributing/documentation.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
index 5ec0675..5f16a0c 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -381,9 +381,9 @@ Lists
#. Item one.
#. Item two is a long line that is wrapped and then indented
- to match the start of the e first line.
+ to match the start of the first line.
- #. Item two is a long line that is wrapped and then indented to match
+ #. Item three is a long line that is wrapped and then indented to match
the start of the previous line.
* Definition lists can be written with or without a bullet::
@@ -477,7 +477,7 @@ Images
* `Inkscape <http://inkscape.org>`_ is the recommended graphics editor for creating the images.
Use some of the older images in ``doc/guides/prog_guide/img/`` as a template, for example ``mbuf1.svg``
- or ``ring-enqueue.svg``.
+ or ``ring-enqueue1.svg``.
* The SVG images should include a copyright notice, as an XML comment.
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 09/32] rte: add APIs for VF stats get/reset
From: Zhang, Qi Z @ 2016-12-08 3:23 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, dev@dpdk.org; +Cc: Thomas Monjalon
In-Reply-To: <86fe08d5-6f14-8495-c26c-2f08a5468587@intel.com>
Hi Ferruh:
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 9:52 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH v2 09/32] rte: add APIs for VF stats get/reset
>
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > This patch add below two APIs so that VF statistics can be get/clear
> > from PF side.
> > rte_eth_vf_stats_get.
> > rte_eth_vf_stats_reset.
>
> patch subject can have " ... from PF" both to be consistent with other patches
> and to clarify what it does: add APIS to get/reset VF stats from PF?
>
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
>
> <...>
>
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 9678179..8b564ee 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1271,6 +1271,15 @@ typedef int (*eth_set_vf_vlan_filter_t)(struct
> rte_eth_dev *dev,
> > uint8_t vlan_on);
> > /**< @internal Set VF VLAN pool filter */
> >
> > +typedef int (*eth_vf_stats_get)(struct rte_eth_dev *dev,
> > + uint16_t vf,
> > + struct rte_eth_stats *stats);
> > +/**< @internal Get VF statistics */
> > +
> > +typedef int (*eth_vf_stats_reset)(struct rte_eth_dev *dev,
> > + uint16_t vf);
> > +/**< @internal Clear VF statistics */
> > +
> > typedef int (*eth_set_queue_rate_limit_t)(struct rte_eth_dev *dev,
> > uint16_t queue_idx,
> > uint16_t tx_rate);
> > @@ -1483,6 +1492,8 @@ struct eth_dev_ops {
> > eth_set_vf_rx_t set_vf_rx; /**< enable/disable a VF
> receive */
> > eth_set_vf_tx_t set_vf_tx; /**< enable/disable a VF
> transmit */
> > eth_set_vf_vlan_filter_t set_vf_vlan_filter; /**< Set VF VLAN filter */
> > + eth_vf_stats_get vf_stats_get; /**< Get VF's statistics */
> > + eth_vf_stats_reset vf_stats_reset; /**< Reset VF's statistics */
>
> Do we really want to add more ops to the eth_dev_ops?
>
> Although vf_stats_get & vf_stats_reset sounds generic, why not implement
> these first in PMD specific manner, and more PMDs implement these, move to
> the generic eth_dev_ops layer?
OK, will move to rte_pmd_i40 APIs.
> CC: Thomas
>
> <...>
Thanks
Qi
^ permalink raw reply
* Re: [RFC PATCH] eventdev: add buffered enqueue and flush APIs
From: Jerin Jacob @ 2016-12-08 4:41 UTC (permalink / raw)
To: Eads, Gage
Cc: dev@dpdk.org, Richardson, Bruce, Van Haaren, Harry,
hemant.agrawal@nxp.com
In-Reply-To: <60DABA4C-E3E8-4768-B2E4-BB97C6421A50@intel.com>
On Mon, Dec 05, 2016 at 11:30:46PM +0000, Eads, Gage wrote:
>
> > On Dec 3, 2016, at 5:18 AM, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:
> >
> >> On Fri, Dec 02, 2016 at 01:45:56PM -0600, Gage Eads wrote:
> >> This commit adds buffered enqueue functionality to the eventdev API.
> >> It is conceptually similar to the ethdev API's tx buffering, however
> >> with a smaller API surface and no dropping of events.
> >
> > Hello Gage,
> > Different implementation may have different strategies to hold the buffers.
>
> A benefit of inlining the buffering logic in the header is that we avoid the overhead of entering the PMD for what is a fairly simple operation (common case: add event to an array, increment counter). If we make this implementation-defined (i.e. use PMD callbacks), we lose that benefit.
In general, I agree from the system perspective. But, few general issues with
eventdev integration part,
1) What if the burst has ATOMIC flows and if we are NOT en-queuing to the
implementation then other event ports won't get the packets from the same ATOMIC
tag ? BAD. Right?
2) At least, In our HW implementation, The event buffer strategy is more like,
if you enqueue to HW then ONLY you get the events from dequeue provided
if op == RTE_EVENT_OP_FORWARD.So it will create deadlock.i.e application cannot
hold the events with RTE_EVENT_OP_FORWARD
3) So considering the above case there is nothing like flush for us
4) In real high throughput benchmark case, we will get the packets at
the rate of max burst and then we always needs to memcpy before we flush.
Otherwise there will be ordering issue as burst can get us the packet from
different flows(unlike polling mode)
>
> > and some does not need to hold the buffers if it is DDR backed.
>
> Though DDR-backed hardware doesn't need to buffer in software, doing so would reduce the software overhead of enqueueing. Compared to N individual calls to enqueue, buffering N events then calling enqueue burst once can benefit from amortized (or parallelized) PMD-specific bookkeeping and error-checking across the set of events, and will definitely benefit from the amortized function call overhead and better I-cache behavior. (Essentially this is VPP from the fd.io project.) This should result in higher overall event throughout (agnostic of the underlying device).
See above. I am not against burst processing in "application".
The flush does not make sense for us in HW perspective and
it is costly for us if we trying generalize it.
>
> I'm skeptical that other buffering strategies would emerge, but I can only speculate on Cavium/NXP/etc. NPU software.
i>
> > IHMO, This may not be the candidate for common code. I guess you can move this
> > to driver side and abstract under SW driver's enqueue_burst.
> >
>
> I don't think that will work without adding a flush API, otherwise we could have indefinitely buffered events. I see three ways forward:
I agree. More portable way is to move the "flush" to the implementation and "flush"
whenever it makes sense to PMD.
>
> - The proposed approach
> - Add the proposed functions but make them implementation-specific.
> - Require the application to write its own buffering logic (i.e. no API change)
I think, If the additional function call overhead cost is too much for SW
implementation then we can think of implementation-specific API or
custom application flow based on SW driver.
But I am not fan of that(but tempted do now a days), If we take that route, we have truckload of custom
implementation specific API and now we try to hide all black magic under enqueue/dequeue
to make it portable at some expense.
>
> Thanks,
> Gage
>
> >
> >>
> >> Signed-off-by: Gage Eads <gage.eads@intel.com>
> >> ---
> >> lib/librte_eventdev/rte_eventdev.c | 29 ++++++++++
> >> lib/librte_eventdev/rte_eventdev.h | 106 +++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 135 insertions(+)
> >>
> >> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> >> index 17ce5c3..564573f 100644
> >> --- a/lib/librte_eventdev/rte_eventdev.c
> >> +++ b/lib/librte_eventdev/rte_eventdev.c
> >> @@ -219,6 +219,7 @@
> >> uint16_t *links_map;
> >> uint8_t *ports_dequeue_depth;
> >> uint8_t *ports_enqueue_depth;
> >> + struct rte_eventdev_enqueue_buffer *port_buffers;
> >> unsigned int i;
> >>
> >> EDEV_LOG_DEBUG("Setup %d ports on device %u", nb_ports,
> >> @@ -272,6 +273,19 @@
> >> "nb_ports %u", nb_ports);
> >> return -(ENOMEM);
> >> }
> >> +
> >> + /* Allocate memory to store port enqueue buffers */
> >> + dev->data->port_buffers =
> >> + rte_zmalloc_socket("eventdev->port_buffers",
> >> + sizeof(dev->data->port_buffers[0]) * nb_ports,
> >> + RTE_CACHE_LINE_SIZE, dev->data->socket_id);
> >> + if (dev->data->port_buffers == NULL) {
> >> + dev->data->nb_ports = 0;
> >> + EDEV_LOG_ERR("failed to get memory for port enq"
> >> + " buffers, nb_ports %u", nb_ports);
> >> + return -(ENOMEM);
> >> + }
> >> +
> >> } else if (dev->data->ports != NULL && nb_ports != 0) {/* re-config */
> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_release, -ENOTSUP);
> >>
> >> @@ -279,6 +293,7 @@
> >> ports_dequeue_depth = dev->data->ports_dequeue_depth;
> >> ports_enqueue_depth = dev->data->ports_enqueue_depth;
> >> links_map = dev->data->links_map;
> >> + port_buffers = dev->data->port_buffers;
> >>
> >> for (i = nb_ports; i < old_nb_ports; i++)
> >> (*dev->dev_ops->port_release)(ports[i]);
> >> @@ -324,6 +339,17 @@
> >> return -(ENOMEM);
> >> }
> >>
> >> + /* Realloc memory to store port enqueue buffers */
> >> + port_buffers = rte_realloc(dev->data->port_buffers,
> >> + sizeof(dev->data->port_buffers[0]) * nb_ports,
> >> + RTE_CACHE_LINE_SIZE);
> >> + if (port_buffers == NULL) {
> >> + dev->data->nb_ports = 0;
> >> + EDEV_LOG_ERR("failed to realloc mem for port enq"
> >> + " buffers, nb_ports %u", nb_ports);
> >> + return -(ENOMEM);
> >> + }
> >> +
> >> if (nb_ports > old_nb_ports) {
> >> uint8_t new_ps = nb_ports - old_nb_ports;
> >>
> >> @@ -336,12 +362,15 @@
> >> memset(links_map +
> >> (old_nb_ports * RTE_EVENT_MAX_QUEUES_PER_DEV),
> >> 0, sizeof(ports_enqueue_depth[0]) * new_ps);
> >> + memset(port_buffers + old_nb_ports, 0,
> >> + sizeof(port_buffers[0]) * new_ps);
> >> }
> >>
> >> dev->data->ports = ports;
> >> dev->data->ports_dequeue_depth = ports_dequeue_depth;
> >> dev->data->ports_enqueue_depth = ports_enqueue_depth;
> >> dev->data->links_map = links_map;
> >> + dev->data->port_buffers = port_buffers;
> >> } else if (dev->data->ports != NULL && nb_ports == 0) {
> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->port_release, -ENOTSUP);
> >>
> >> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> >> index 778d6dc..3f24342 100644
> >> --- a/lib/librte_eventdev/rte_eventdev.h
> >> +++ b/lib/librte_eventdev/rte_eventdev.h
> >> @@ -246,6 +246,7 @@
> >> #include <rte_dev.h>
> >> #include <rte_memory.h>
> >> #include <rte_errno.h>
> >> +#include <rte_memcpy.h>
> >>
> >> #define EVENTDEV_NAME_SKELETON_PMD event_skeleton
> >> /**< Skeleton event device PMD name */
> >> @@ -965,6 +966,26 @@ typedef uint16_t (*event_dequeue_burst_t)(void *port, struct rte_event ev[],
> >> #define RTE_EVENTDEV_NAME_MAX_LEN (64)
> >> /**< @internal Max length of name of event PMD */
> >>
> >> +#define RTE_EVENT_BUF_MAX 16
> >> +/**< Maximum number of events in an enqueue buffer. */
> >> +
> >> +/**
> >> + * @internal
> >> + * An enqueue buffer for each port.
> >> + *
> >> + * The reason this struct is in the header is for inlining the function calls
> >> + * to enqueue, as doing a function call per packet would incur significant
> >> + * performance overhead.
> >> + *
> >> + * \see rte_event_enqueue_buffer(), rte_event_enqueue_buffer_flush()
> >> + */
> >> +struct rte_eventdev_enqueue_buffer {
> >> + /**> Count of events in this buffer */
> >> + uint16_t count;
> >> + /**> Array of events in this buffer */
> >> + struct rte_event events[RTE_EVENT_BUF_MAX];
> >> +} __rte_cache_aligned;
> >> +
> >> /**
> >> * @internal
> >> * The data part, with no function pointers, associated with each device.
> >> @@ -983,6 +1004,8 @@ struct rte_eventdev_data {
> >> /**< Number of event ports. */
> >> void **ports;
> >> /**< Array of pointers to ports. */
> >> + struct rte_eventdev_enqueue_buffer *port_buffers;
> >> + /**< Array of port enqueue buffers. */
> >> uint8_t *ports_dequeue_depth;
> >> /**< Array of port dequeue depth. */
> >> uint8_t *ports_enqueue_depth;
> >> @@ -1132,6 +1155,89 @@ struct rte_eventdev {
> >> }
> >>
> >> /**
> >> + * Flush the enqueue buffer of the event port specified by *port_id*, in the
> >> + * event device specified by *dev_id*.
> >> + *
> >> + * This function attempts to flush as many of the buffered events as possible,
> >> + * and returns the number of flushed events. Any unflushed events remain in
> >> + * the buffer.
> >> + *
> >> + * @param dev_id
> >> + * The identifier of the device.
> >> + * @param port_id
> >> + * The identifier of the event port.
> >> + *
> >> + * @return
> >> + * The number of event objects actually flushed to the event device.
> >> + *
> >> + * \see rte_event_enqueue_buffer(), rte_event_enqueue_burst()
> >> + * \see rte_event_port_enqueue_depth()
> >> + */
> >> +static inline int
> >> +rte_event_enqueue_buffer_flush(uint8_t dev_id, uint8_t port_id)
> >> +{
> >> + struct rte_eventdev *dev = &rte_eventdevs[dev_id];
> >> + struct rte_eventdev_enqueue_buffer *buf =
> >> + &dev->data->port_buffers[port_id];
> >> + int n;
> >> +
> >> + n = rte_event_enqueue_burst(dev_id, port_id, buf->events, buf->count);
> >> +
> >> + if (n != buf->count)
> >> + memmove(buf->events, &buf->events[n], buf->count - n);
> >> +
> >> + buf->count -= n;
> >> +
> >> + return n;
> >> +}
> >> +
> >> +/**
> >> + * Buffer an event object supplied in *rte_event* structure for future
> >> + * enqueueing on an event device designated by its *dev_id* through the event
> >> + * port specified by *port_id*.
> >> + *
> >> + * This function takes a single event and buffers it for later enqueuing to the
> >> + * queue specified in the event structure. If the buffer is full, the
> >> + * function will attempt to flush the buffer before buffering the event.
> >> + * If the flush operation fails, the previously buffered events remain in the
> >> + * buffer and an error is returned to the user to indicate that *ev* was not
> >> + * buffered.
> >> + *
> >> + * @param dev_id
> >> + * The identifier of the device.
> >> + * @param port_id
> >> + * The identifier of the event port.
> >> + * @param ev
> >> + * Pointer to struct rte_event
> >> + *
> >> + * @return
> >> + * - 0 on success
> >> + * - <0 on failure. Failure can occur if the event port's output queue is
> >> + * backpressured, for instance.
> >> + *
> >> + * \see rte_event_enqueue_buffer_flush(), rte_event_enqueue_burst()
> >> + * \see rte_event_port_enqueue_depth()
> >> + */
> >> +static inline int
> >> +rte_event_enqueue_buffer(uint8_t dev_id, uint8_t port_id, struct rte_event *ev)
> >> +{
> >> + struct rte_eventdev *dev = &rte_eventdevs[dev_id];
> >> + struct rte_eventdev_enqueue_buffer *buf =
> >> + &dev->data->port_buffers[port_id];
> >> + int ret;
> >> +
> >> + /* If necessary, flush the enqueue buffer to make space for ev. */
> >> + if (buf->count == RTE_EVENT_BUF_MAX) {
> >> + ret = rte_event_enqueue_buffer_flush(dev_id, port_id);
> >> + if (ret == 0)
> >> + return -ENOSPC;
> >> + }
> >> +
> >> + rte_memcpy(&buf->events[buf->count++], ev, sizeof(struct rte_event));
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> * Converts nanoseconds to *wait* value for rte_event_dequeue()
> >> *
> >> * If the device is configured with RTE_EVENT_DEV_CFG_PER_DEQUEUE_WAIT flag then
> >> --
> >> 1.9.1
> >>
^ permalink raw reply
* Re: [PATCH 00/13] Introducing EAL Bus-Device-Driver Model
From: Shreyansh Jain @ 2016-12-08 5:04 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: David Marchand, dev
In-Reply-To: <1524529.OpOKghfniY@xps13>
On Wednesday 07 December 2016 06:54 PM, Thomas Monjalon wrote:
> 2016-12-07 18:40, Shreyansh Jain:
>> Is there anything specific that you are looking for in patchset v2?
>> I was thinking of:
>> 0. fixing BSD compilation issue reported by CI
>> 1. improving the test_pci.c
>> 2. hotplugging
>> 3. trying to move PCI to drives/bus/pci/linux/* and resolving how
>> drivers link to it, and how EAL resources like devargs are consumed.
>
> I am concerned about the time needed for all these changes.
> Please let's make sure that the basic parts are well done and pushed, first.
> That's why I suggest to postpone 1, 2 and 3 to next release if possible.
> The priority is to have a clean bus model,
> and if time permits, integrate the NXP driver.
>
I am absolutely OK with this.
Just a confirmation, though. Is it OK to have a broken test_pci.c (1) in
a series? It is not getting compiled by default, but it would definitely
not work if compiled (symbols are missing).
^ permalink raw reply
* Re: [PATCH] vhost: allow for many vhost user ports
From: Yuanhan Liu @ 2016-12-08 5:50 UTC (permalink / raw)
To: Jan Wickbom; +Cc: dev@dpdk.org, Patrik Andersson R
In-Reply-To: <AM4PR0701MB2146020B030F882D09A24A4E94850@AM4PR0701MB2146.eurprd07.prod.outlook.com>
On Wed, Dec 07, 2016 at 01:23:48PM +0000, Jan Wickbom wrote:
> > On Thu, Dec 01, 2016 at 04:26:50PM +0100, Jan Wickbom wrote:
> > > static int
> > > -fdset_fill(fd_set *rfset, fd_set *wfset, struct fdset *pfdset)
> > > +fdset_fill(struct pollfd *rwfds, struct fdset *pfdset)
...
> > > + rwfds[i].fd = pfdentry->fd;
> > > + rwfds[i].events = pfdentry->rcb ? POLLIN : 0;
> > > + rwfds[i].events |= pfdentry->wcb ? POLLOUT :
> > 0;
> >
> > Another thing is we don't have to re-init this rwfds array again
> > and again. Instead, we could
> >
> > - set it up correctly when fdset_add is invoked: set the fd and
> > events.
> >
> > - reset revents when it's been handled at fdset_event_dispatch().
> >
> > - swap with the last one and shrink the array on fd delete
> >
> > Could you make a follow up patch for that?
>
> I don't see how that could easily be done. The loop index, i, is a direct reference between
> an entry in the rwfds array and an entry in the pfdset array. It should stay like that while we are
> hanging in the poll(). If an entry in the pfdset array is removed while we are hanging in the poll()
> and we then immediately replaces it with the last entry in the array we will end up in trouble if the
> revent gets set for the "replaced" index. The direct reference is gone.
> Or am I missing something?
Yes, we should not shrink the rwfds during the poll, but we could later, at
the end of the while() loop.
Talking about that, you should not invoke fdset_shrink() inside fdset_del(),
since it could be in the poll stage.
--yliu
^ permalink raw reply
* Re: [PATCH 5/5] examples/l3fwd: add parse-ptype option
From: Yuanhan Liu @ 2016-12-08 6:59 UTC (permalink / raw)
To: Jianfeng Tan; +Cc: dev, stephen
In-Reply-To: <1480810702-114815-6-git-send-email-jianfeng.tan@intel.com>
On Sun, Dec 04, 2016 at 12:18:22AM +0000, Jianfeng Tan wrote:
> To support those devices that do not provide packet type info when
> receiving packets, add a new option, --parse-ptype, to analyze
> packet type in the Rx callback.
A quick question: does this patch have anything to do with the virtio
interrupt enabling?
--yliu
^ permalink raw reply
* Re: [PATCH v2 1/8] eal: define container_of macro
From: Shreyansh Jain @ 2016-12-08 7:01 UTC (permalink / raw)
To: Jan Blunck, dev; +Cc: david.marchand, Jan Viktorin
In-Reply-To: <1479747322-5774-1-git-send-email-jblunck@infradead.org>
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?
> +
> #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 1/3] net/virtio_user: add vhost layer
From: Yuanhan Liu @ 2016-12-08 7:21 UTC (permalink / raw)
To: Jianfeng Tan; +Cc: dev, ferruh.yigit, cunming.liang
In-Reply-To: <1480689075-66977-2-git-send-email-jianfeng.tan@intel.com>
On Fri, Dec 02, 2016 at 02:31:13PM +0000, Jianfeng Tan wrote:
> To support vhost kernel as the backend of net_virtio_user in comming
> patches, we abstract a vhost layer to hide all vhost_user functions.
> - Move vhost_user specific structs and macros into vhost_user.c,
> and only keep common definitions in vhost.h;
Do that in another patch.
> - Add a struct vhost_internal, and an array to store vhost_user and
> vhost_kernel backends; in multiqueue case, vhost_user has only
> one vhost FD, but vhost_kernel would have multiple FDs (equal to
> # of queues), so we turn to use an id to index the backend info;
It's okay to add a struct for that, but what's the point of referencing
it by an id? Instead, you could directly allocate one and return it.
This would also remove the hardcoded limit: 8 devices at most.
> - Add a struct vhost_ops depending on different type of backends.
Firstly, the "control" method is not well named, IMO. Something like
"send_request" is better.
And there is another way to achieve that: instead of introducing one
callback (control, or send_request) for all requests, you could
define one callback for each request. QEMU switched to that way
few releases ago. In that way, you don't have to do the request
translation at least.
Last, please make one patch for each of 3 items. It helps review.
--yliu
^ permalink raw reply
* Re: [PATCH 00/13] Introducing EAL Bus-Device-Driver Model
From: Thomas Monjalon @ 2016-12-08 7:21 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: David Marchand, dev
In-Reply-To: <a0b14c5f-ad9d-dc08-90f3-284226ce714a@nxp.com>
2016-12-08 10:34, Shreyansh Jain:
> On Wednesday 07 December 2016 06:54 PM, Thomas Monjalon wrote:
> > 2016-12-07 18:40, Shreyansh Jain:
> >> Is there anything specific that you are looking for in patchset v2?
> >> I was thinking of:
> >> 0. fixing BSD compilation issue reported by CI
> >> 1. improving the test_pci.c
> >> 2. hotplugging
> >> 3. trying to move PCI to drives/bus/pci/linux/* and resolving how
> >> drivers link to it, and how EAL resources like devargs are consumed.
> >
> > I am concerned about the time needed for all these changes.
> > Please let's make sure that the basic parts are well done and pushed, first.
> > That's why I suggest to postpone 1, 2 and 3 to next release if possible.
> > The priority is to have a clean bus model,
> > and if time permits, integrate the NXP driver.
> >
>
> I am absolutely OK with this.
>
> Just a confirmation, though. Is it OK to have a broken test_pci.c (1) in
> a series? It is not getting compiled by default, but it would definitely
> not work if compiled (symbols are missing).
No, that's why I said "if possible" :).
I thought 1. was about improvements. But if it does not compile, it is
a must.
^ permalink raw reply
* Re: [PATCH 5/5] examples/l3fwd: add parse-ptype option
From: Tan, Jianfeng @ 2016-12-08 7:40 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: dev@dpdk.org, stephen@networkplumber.org
In-Reply-To: <20161208065928.GA30698@yliu-dev.sh.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Thursday, December 8, 2016 2:59 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; stephen@networkplumber.org
> Subject: Re: [PATCH 5/5] examples/l3fwd: add parse-ptype option
>
> On Sun, Dec 04, 2016 at 12:18:22AM +0000, Jianfeng Tan wrote:
> > To support those devices that do not provide packet type info when
> > receiving packets, add a new option, --parse-ptype, to analyze
> > packet type in the Rx callback.
>
> A quick question: does this patch have anything to do with the virtio
> interrupt enabling?
Yes, it does as this example is used to test the functionality. But like l3fwd example, this example depends on the ptypes to forward packets. So we need to add software parser to parse ptypes like l3fwd example.
Thanks,
Jianfeng
>
> --yliu
^ permalink raw reply
* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Yang, Zhiyong @ 2016-12-08 7:41 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, yuanhan.liu@linux.intel.com, Richardson, Bruce,
Ananyev, Konstantin, De Lara Guarch, Pablo
In-Reply-To: <7223515.9TZuZb6buy@xps13>
HI, Thomas:
Sorry for late reply. I have been being always considering your suggestion.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, December 2, 2016 6:25 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce rte_memset on
> IA platform
>
> 2016-12-05 16:26, Zhiyong Yang:
> > +#ifndef _RTE_MEMSET_X86_64_H_
>
> Is this implementation specific to 64-bit?
>
Yes.
> > +
> > +#define rte_memset memset
> > +
> > +#else
> > +
> > +static void *
> > +rte_memset(void *dst, int a, size_t n);
> > +
> > +#endif
>
> If I understand well, rte_memset (as rte_memcpy) is using the most recent
> instructions available (and enabled) when compiling.
> It is not adapting the instructions to the run-time CPU.
> There is no need to downgrade at run-time the instruction set as it is
> obviously not a supported case, but it would be nice to be able to upgrade a
> "default compilation" at run-time as it is done in rte_acl.
> I explain this case more clearly for reference:
>
> We can have AVX512 supported in the compiler but disable it when compiling
> (CONFIG_RTE_MACHINE=snb) in order to build a binary running almost
> everywhere.
> When running this binary on a CPU having AVX512 support, it will not benefit
> of the AVX512 improvement.
> Though, we can compile an AVX512 version of some functions and use them
> only if the running CPU is capable.
> This kind of miracle can be achieved in two ways:
>
> 1/ For generic C code compiled with a recent GCC, a function can be built for
> several CPUs thanks to the attribute target_clones.
>
> 2/ For manually optimized functions using CPU-specific intrinsics or asm, it is
> possible to build them with non-default flags thanks to the attribute target.
>
> 3/ For manually optimized files using CPU-specific intrinsics or asm, we use
> specifics flags in the makefile.
>
> The function clone in case 1/ is dynamically chosen at run-time through ifunc
> resolver.
> The specific functions in cases 2/ and 3/ must chosen at run-time by
> initializing a function pointer thanks to rte_cpu_get_flag_enabled().
>
> Note that rte_hash and software crypto PMDs have a run-time check with
> rte_cpu_get_flag_enabled() but do not override CFLAGS in the Makefile.
> Next step for these libraries?
>
> Back to rte_memset, I think you should try the solution 2/.
I have read the ACL code, if I understand well , for complex algo implementation,
it is good idea, but Choosing functions at run time will bring some overhead. For frequently called function
Which consumes small cycles, the overhead maybe is more than the gains optimizations brings
For example, for most applications in dpdk, memset only set N = 10 or 12bytes. It consumes fewer cycles.
Thanks
Zhiyong
^ permalink raw reply
* Re: [PATCH 00/13] Introducing EAL Bus-Device-Driver Model
From: Shreyansh Jain @ 2016-12-08 7:53 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: David Marchand, dev@dpdk.org
In-Reply-To: <1743313.gQvgAgeimG@xps13>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, December 08, 2016 12:51 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: David Marchand <david.marchand@6wind.com>; dev@dpdk.org
> Subject: Re: [PATCH 00/13] Introducing EAL Bus-Device-Driver Model
>
> 2016-12-08 10:34, Shreyansh Jain:
> > On Wednesday 07 December 2016 06:54 PM, Thomas Monjalon wrote:
> > > 2016-12-07 18:40, Shreyansh Jain:
> > >> Is there anything specific that you are looking for in patchset v2?
> > >> I was thinking of:
> > >> 0. fixing BSD compilation issue reported by CI
> > >> 1. improving the test_pci.c
> > >> 2. hotplugging
> > >> 3. trying to move PCI to drives/bus/pci/linux/* and resolving how
> > >> drivers link to it, and how EAL resources like devargs are consumed.
> > >
> > > I am concerned about the time needed for all these changes.
> > > Please let's make sure that the basic parts are well done and pushed,
> first.
> > > That's why I suggest to postpone 1, 2 and 3 to next release if possible.
> > > The priority is to have a clean bus model,
> > > and if time permits, integrate the NXP driver.
> > >
> >
> > I am absolutely OK with this.
> >
> > Just a confirmation, though. Is it OK to have a broken test_pci.c (1) in
> > a series? It is not getting compiled by default, but it would definitely
> > not work if compiled (symbols are missing).
>
> No, that's why I said "if possible" :).
> I thought 1. was about improvements. But if it does not compile, it is
> a must.
Agree.
I should have mentioned 'fixing the test_pci.c' rather than 'improvement'.
^ 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