* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Yuanhan Liu @ 2016-10-10 2:44 UTC (permalink / raw)
To: Wang, Zhihong; +Cc: Jianbo Liu, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7BBE7D@SHSMSX103.ccr.corp.intel.com>
On Sun, Oct 09, 2016 at 12:09:07PM +0000, Wang, Zhihong wrote:
> > > > Tested with testpmd, host: txonly, guest: rxonly
> > > > size (bytes) improvement (%)
> > > > 64 4.12
> > > > 128 6
> > > > 256 2.65
> > > > 512 -1.12
> > > > 1024 -7.02
> > >
> > > There is a difference between Zhihong's code and the old I spotted in
> > > the first time: Zhihong removed the avail_idx prefetch. I understand
> > > the prefetch becomes a bit tricky when mrg-rx code path is considered;
> > > thus, I didn't comment on that.
> > >
> > > That's one of the difference that, IMO, could drop a regression. I then
> > > finally got a chance to add it back.
> > >
> > > A rough test shows it improves the performance of 1400B packet size
> > greatly
> > > in the "txonly in host and rxonly in guest" case: +33% is the number I get
> > > with my test server (Ivybridge).
> >
> > Thanks Yuanhan! I'll validate this on x86.
>
> Hi Yuanhan,
>
> Seems your code doesn't perform correctly. I write a new version
> of avail idx prefetch but didn't see any perf benefit.
>
> To be honest I doubt the benefit of this idea. The previous mrg_off
> code has this method but doesn't give any benefits.
Good point. I thought of that before, too. But you know that I made it
in rush, that I didn't think further and test more.
I looked the code a bit closer this time, and spotted a bug: the prefetch
actually didn't happen, due to following code piece:
if (vq->next_avail_idx >= NR_AVAIL_IDX_PREFETCH) {
prefetch_avail_idx(vq);
...
}
Since vq->next_avail_idx is set to 0 at the entrance of enqueue path,
prefetch_avail_idx() will be called. The fix is easy though: just put
prefetch_avail_idx before invoking enqueue_packet.
In summary, Zhihong is right, I see no more gains with that fix :(
However, as stated, that's kind of the only difference I found between
yours and the old code, that maybe it's still worthwhile to have a
test on ARM, Jianbo?
--yliu
> Even if this is useful, the benefits should be more significant for
> small packets, it's unlikely this simple idx prefetch could bring
> over 30% perf gain for large packets like 1400B ones.
>
> But if you really do work it out like that I'll be very glad to see.
>
> Thanks
> Zhihong
>
> >
> > >
> > > I guess this might/would help your case as well. Mind to have a test
> > > and tell me the results?
> > >
> > > BTW, I made it in rush; I haven't tested the mrg-rx code path yet.
> > >
> > > Thanks.
> > >
> > > --yliu
^ permalink raw reply
* [PATCH] app/testpmd: fix pf/vf check of flow director
From: Wenzhuo Lu @ 2016-10-10 2:47 UTC (permalink / raw)
To: dev; +Cc: Wenzhuo Lu
Parameters pf & vf are added into most of flow director
filter CLIs.
But mac-valn and tunnel filters don't have these parameters,
the parameters should not be checked for mac-vlan and tunnel
filters.
Fixes: e6a68c013353 ("app/testpmd: extend commands for flow director in VF")
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
app/test-pmd/cmdline.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f90befc..2580f27 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8502,24 +8502,28 @@ cmd_flow_director_filter_parsed(void *parsed_result,
else
entry.action.behavior = RTE_ETH_FDIR_ACCEPT;
- if (!strcmp(res->pf_vf, "pf"))
- entry.input.flow_ext.is_vf = 0;
- else if (!strncmp(res->pf_vf, "vf", 2)) {
- struct rte_eth_dev_info dev_info;
-
- memset(&dev_info, 0, sizeof(dev_info));
- rte_eth_dev_info_get(res->port_id, &dev_info);
- errno = 0;
- vf_id = strtoul(res->pf_vf + 2, &end, 10);
- if (errno != 0 || *end != '\0' || vf_id >= dev_info.max_vfs) {
+ if (fdir_conf.mode != RTE_FDIR_MODE_PERFECT_MAC_VLAN &&
+ fdir_conf.mode != RTE_FDIR_MODE_PERFECT_TUNNEL) {
+ if (!strcmp(res->pf_vf, "pf"))
+ entry.input.flow_ext.is_vf = 0;
+ else if (!strncmp(res->pf_vf, "vf", 2)) {
+ struct rte_eth_dev_info dev_info;
+
+ memset(&dev_info, 0, sizeof(dev_info));
+ rte_eth_dev_info_get(res->port_id, &dev_info);
+ errno = 0;
+ vf_id = strtoul(res->pf_vf + 2, &end, 10);
+ if (errno != 0 || *end != '\0' ||
+ vf_id >= dev_info.max_vfs) {
+ printf("invalid parameter %s.\n", res->pf_vf);
+ return;
+ }
+ entry.input.flow_ext.is_vf = 1;
+ entry.input.flow_ext.dst_id = (uint16_t)vf_id;
+ } else {
printf("invalid parameter %s.\n", res->pf_vf);
return;
}
- entry.input.flow_ext.is_vf = 1;
- entry.input.flow_ext.dst_id = (uint16_t)vf_id;
- } else {
- printf("invalid parameter %s.\n", res->pf_vf);
- return;
}
/* set to report FD ID by default */
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 3:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Stephen Hemminger, Maxime Coquelin, dev, qemu-devel
In-Reply-To: <20161010014900-mutt-send-email-mst@kernel.org>
On Mon, Oct 10, 2016 at 02:20:22AM +0300, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2016 at 10:28:48AM +0800, Yuanhan Liu wrote:
> > On Tue, Sep 27, 2016 at 10:56:40PM +0300, Michael S. Tsirkin wrote:
> > > On Tue, Sep 27, 2016 at 11:11:58AM +0800, Yuanhan Liu wrote:
> > > > On Mon, Sep 26, 2016 at 10:24:55PM +0300, Michael S. Tsirkin wrote:
> > > > > On Mon, Sep 26, 2016 at 11:01:58AM -0700, Stephen Hemminger wrote:
> > > > > > I assume that if using Version 1 that the bit will be ignored
> > > >
> > > > Yes, but I will just quote what you just said: what if the guest
> > > > virtio device is a legacy device? I also gave my reasons in another
> > > > email why I consistently set this flag:
> > > >
> > > > - we have to return all features we support to the guest.
> > > >
> > > > We don't know the guest is a modern or legacy device. That means
> > > > we should claim we support both: VERSION_1 and ANY_LAYOUT.
> > > >
> > > > Assume guest is a legacy device and we just set VERSION_1 (the current
> > > > case), ANY_LAYOUT will never be negotiated.
> > > >
> > > > - I'm following the way Linux kernel takes: it also set both features.
> > > >
> > > > Maybe, we could unset ANY_LAYOUT when VERSION_1 is _negotiated_?
> > > >
> > > > The unset after negotiation I proposed turned out it won't work: the
> > > > feature is already negotiated; unsetting it only in vhost side doesn't
> > > > change anything. Besides, it may break the migration as Michael stated
> > > > below.
> > >
> > > I think the reverse. Teach vhost user that for future machine types
> > > only VERSION_1 implies ANY_LAYOUT.
>
> So I guess at this point, we can teach vhost-user in qemu
> that version 1 implies any_layout but only for machine types
> qemu 2.8 and up. It sets a bad precedent but oh well.
It should work.
--yliu
^ permalink raw reply
* Re: [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 3:04 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Stephen Hemminger, Maxime Coquelin, dev, qemu-devel
In-Reply-To: <20161010030333.GU1597@yliu-dev.sh.intel.com>
On Mon, Oct 10, 2016 at 11:03:33AM +0800, Yuanhan Liu wrote:
> On Mon, Oct 10, 2016 at 02:20:22AM +0300, Michael S. Tsirkin wrote:
> > On Wed, Sep 28, 2016 at 10:28:48AM +0800, Yuanhan Liu wrote:
> > > On Tue, Sep 27, 2016 at 10:56:40PM +0300, Michael S. Tsirkin wrote:
> > > > On Tue, Sep 27, 2016 at 11:11:58AM +0800, Yuanhan Liu wrote:
> > > > > On Mon, Sep 26, 2016 at 10:24:55PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Mon, Sep 26, 2016 at 11:01:58AM -0700, Stephen Hemminger wrote:
> > > > > > > I assume that if using Version 1 that the bit will be ignored
> > > > >
> > > > > Yes, but I will just quote what you just said: what if the guest
> > > > > virtio device is a legacy device? I also gave my reasons in another
> > > > > email why I consistently set this flag:
> > > > >
> > > > > - we have to return all features we support to the guest.
> > > > >
> > > > > We don't know the guest is a modern or legacy device. That means
> > > > > we should claim we support both: VERSION_1 and ANY_LAYOUT.
> > > > >
> > > > > Assume guest is a legacy device and we just set VERSION_1 (the current
> > > > > case), ANY_LAYOUT will never be negotiated.
> > > > >
> > > > > - I'm following the way Linux kernel takes: it also set both features.
> > > > >
> > > > > Maybe, we could unset ANY_LAYOUT when VERSION_1 is _negotiated_?
> > > > >
> > > > > The unset after negotiation I proposed turned out it won't work: the
> > > > > feature is already negotiated; unsetting it only in vhost side doesn't
> > > > > change anything. Besides, it may break the migration as Michael stated
> > > > > below.
> > > >
> > > > I think the reverse. Teach vhost user that for future machine types
> > > > only VERSION_1 implies ANY_LAYOUT.
> >
> > So I guess at this point, we can teach vhost-user in qemu
> > that version 1 implies any_layout but only for machine types
> > qemu 2.8 and up. It sets a bad precedent but oh well.
>
> It should work.
>
> --yliu
Cool. Want to post a patch?
--
MST
^ permalink raw reply
* Re: [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 3:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Stephen Hemminger, Maxime Coquelin, dev, qemu-devel
In-Reply-To: <20161010060420-mutt-send-email-mst@kernel.org>
On Mon, Oct 10, 2016 at 06:04:32AM +0300, Michael S. Tsirkin wrote:
> > > So I guess at this point, we can teach vhost-user in qemu
> > > that version 1 implies any_layout but only for machine types
> > > qemu 2.8 and up. It sets a bad precedent but oh well.
> >
> > It should work.
> >
> > --yliu
>
> Cool. Want to post a patch?
I could have a try this week (Well, it's very unlikely though).
If not, it will be postponed for a while: I am traveling next week.
--yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 3:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel,
Wang, Zhihong
In-Reply-To: <20160929231252-mutt-send-email-mst@kernel.org>
On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> >
> >
> > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> Yes but two points.
>
> 1. why is this memset expensive?
I don't have the exact answer, but just some rough thoughts:
It's an external clib function: there is a call stack and the
IP register will bounch back and forth. BTW, It's kind of an
overkill to use that for resetting 14 bytes structure.
Some trick like
*(struct virtio_net_hdr *)hdr = {0, };
Or even
hdr->xxx = 0;
hdr->yyy = 0;
should behaviour better.
There was an example: the vhost enqueue optmization patchset from
Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
on my Ivybridge server: it has no such issue on his server though.
[0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
--yliu
> Is the test completely skipping looking
> at the packet otherwise?
>
> 2. As long as we are doing this, see
> Alignment vs. Networking
> ========================
> in Documentation/unaligned-memory-access.txt
>
>
> > From the micro-benchmarks results, we can expect +10% compared to
> > indirect descriptors, and + 5% compared to using 2 descs in the
> > virtqueue.
> > Also, it should have the same benefits as indirect descriptors for 0%
> > pkt loss (as we can fill 2x more packets in the virtqueue).
> >
> > What do you think?
> >
> > Thanks,
> > Maxime
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 3:46 UTC (permalink / raw)
To: Yuanhan Liu
Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel,
Wang, Zhihong
In-Reply-To: <20161010033744.GW1597@yliu-dev.sh.intel.com>
On Mon, Oct 10, 2016 at 11:37:44AM +0800, Yuanhan Liu wrote:
> On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> > >
> > >
> > > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> > Yes but two points.
> >
> > 1. why is this memset expensive?
>
> I don't have the exact answer, but just some rough thoughts:
>
> It's an external clib function: there is a call stack and the
> IP register will bounch back and forth.
for memset 0? gcc 5.3.1 on fedora happily inlines it.
> BTW, It's kind of an
> overkill to use that for resetting 14 bytes structure.
>
> Some trick like
> *(struct virtio_net_hdr *)hdr = {0, };
>
> Or even
> hdr->xxx = 0;
> hdr->yyy = 0;
>
> should behaviour better.
>
> There was an example: the vhost enqueue optmization patchset from
> Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
> on my Ivybridge server: it has no such issue on his server though.
>
> [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
>
> --yliu
I'd say that's weird. what's your config? any chance you
are using an old compiler?
> > Is the test completely skipping looking
> > at the packet otherwise?
> >
> > 2. As long as we are doing this, see
> > Alignment vs. Networking
> > ========================
> > in Documentation/unaligned-memory-access.txt
> >
> >
> > > From the micro-benchmarks results, we can expect +10% compared to
> > > indirect descriptors, and + 5% compared to using 2 descs in the
> > > virtqueue.
> > > Also, it should have the same benefits as indirect descriptors for 0%
> > > pkt loss (as we can fill 2x more packets in the virtqueue).
> > >
> > > What do you think?
> > >
> > > Thanks,
> > > Maxime
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 3:50 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: Michael S. Tsirkin, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <2889e609-f750-a4e1-66f8-768bb07a2339@redhat.com>
On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> >so doing this unconditionally would be a spec violation, but if you see
> >value in this, we can add a feature bit.
> Right it would be a spec violation, so it should be done conditionally.
> If a feature bit is to be added, what about VIRTIO_NET_F_NO_TX_HEADER?
> It would imply VIRTIO_NET_F_CSUM not set, and no GSO features set.
> If negotiated, we wouldn't need to prepend a header.
If we could skip Tx header, I think we could also skip Rx header, in the
case when mrg-rx is aslo turned off?
> From the micro-benchmarks results, we can expect +10% compared to
> indirect descriptors, and + 5% compared to using 2 descs in the
> virtqueue.
> Also, it should have the same benefits as indirect descriptors for 0%
> pkt loss (as we can fill 2x more packets in the virtqueue).
>
> What do you think?
I would vote for this. It should yield maximum performance for the case
that it's guaranteed that packet size will always fit in a typical MTU
(1500).
--yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 3:59 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel,
Wang, Zhihong
In-Reply-To: <20161010064113-mutt-send-email-mst@kernel.org>
On Mon, Oct 10, 2016 at 06:46:44AM +0300, Michael S. Tsirkin wrote:
> On Mon, Oct 10, 2016 at 11:37:44AM +0800, Yuanhan Liu wrote:
> > On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> > > >
> > > >
> > > > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> > > Yes but two points.
> > >
> > > 1. why is this memset expensive?
> >
> > I don't have the exact answer, but just some rough thoughts:
> >
> > It's an external clib function: there is a call stack and the
> > IP register will bounch back and forth.
>
> for memset 0? gcc 5.3.1 on fedora happily inlines it.
Good to know!
> > overkill to use that for resetting 14 bytes structure.
> >
> > Some trick like
> > *(struct virtio_net_hdr *)hdr = {0, };
> >
> > Or even
> > hdr->xxx = 0;
> > hdr->yyy = 0;
> >
> > should behaviour better.
> >
> > There was an example: the vhost enqueue optmization patchset from
> > Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
> > on my Ivybridge server: it has no such issue on his server though.
> >
> > [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
> >
> > --yliu
>
> I'd say that's weird. what's your config? any chance you
> are using an old compiler?
Not really, it's gcc 5.3.1. Maybe Zhihong could explain more. IIRC,
he said the memset is not well optimized for Ivybridge server.
--yliu
^ permalink raw reply
* Re: [PATCH v2] i40: fix the VXLAN TSO issue
From: Wu, Jingjing @ 2016-10-10 3:58 UTC (permalink / raw)
To: Zhe Tao, dev@dpdk.org, Yigit, Ferruh
In-Reply-To: <1467865627-23524-1-git-send-email-zhe.tao@intel.com>
NACK.
This fix has been done by a new one which is already merged: http://dpdk.org/dev/patchwork/patch/15059/
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhe Tao
> Sent: Thursday, July 7, 2016 12:27 PM
> To: dev@dpdk.org
> Cc: zhe.tao@intel.com; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [dpdk-dev] [PATCH v2] i40: fix the VXLAN TSO issue
>
> Problem:
> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> the multiple UDP segments which are TSOed by the i40e will have the wrong
> value.
>
> Fix this problem by adding the tunnel type field in the i40e descriptor which
> was missed before.
>
> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
>
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> V2: Edited some comments for mbuf structure and i40e driver.
>
> app/test-pmd/csumonly.c | 26 +++++++++++++++++++-------
> drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> lib/librte_mbuf/rte_mbuf.h | 16 +++++++++++++++-
> 3 files changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> ac4bd8f..d423c20 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct
> testpmd_offload_info *info) static void parse_vxlan(struct udp_hdr
> *udp_hdr,
> struct testpmd_offload_info *info,
> - uint32_t pkt_type)
> + uint32_t pkt_type,
> + uint64_t *ol_flags)
> {
> struct ether_hdr *eth_hdr;
>
> @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> return;
>
> + *ol_flags |= PKT_TX_TUNNEL_VXLAN;
> info->is_tunnel = 1;
> info->outer_ethertype = info->ethertype;
> info->outer_l2_len = info->l2_len;
> @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>
> /* Parse a gre header */
> static void
> -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info
> *info)
> +parse_gre(struct simple_gre_hdr *gre_hdr,
> + struct testpmd_offload_info *info,
> + uint64_t *ol_flags)
> {
> struct ether_hdr *eth_hdr;
> struct ipv4_hdr *ipv4_hdr;
> @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct
> testpmd_offload_info *info)
> if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
> return;
>
> + *ol_flags |= PKT_TX_TUNNEL_GRE;
> +
> gre_len += sizeof(struct simple_gre_hdr);
>
> if (gre_hdr->flags & _htons(GRE_KEY_PRESENT)) @@ -417,7 +423,7
> @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info
> *info,
> * packet */
> static uint64_t
> process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
> - uint16_t testpmd_ol_flags)
> + uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
> {
> struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> struct ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -442,6 +448,9 @@
> process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
> * hardware supporting it today, and no API for it. */
>
> udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info-
> >outer_l3_len);
> + if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> + ((orig_ol_flags & PKT_TX_TUNNEL_MASK) ==
> PKT_TX_TUNNEL_VXLAN))
> + udp_hdr->dgram_cksum = 0;
> /* do not recalculate udp cksum if it was 0 */
> if (udp_hdr->dgram_cksum != 0) {
> udp_hdr->dgram_cksum = 0;
> @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream
> *fs)
> if (info.l4_proto == IPPROTO_UDP) {
> struct udp_hdr *udp_hdr;
> udp_hdr = (struct udp_hdr *)((char *)l3_hdr
> +
> - info.l3_len);
> - parse_vxlan(udp_hdr, &info, m-
> >packet_type);
> + info.l3_len);
> + parse_vxlan(udp_hdr, &info, m-
> >packet_type,
> + &ol_flags);
> } else if (info.l4_proto == IPPROTO_GRE) {
> struct simple_gre_hdr *gre_hdr;
> gre_hdr = (struct simple_gre_hdr *)
> ((char *)l3_hdr + info.l3_len);
> - parse_gre(gre_hdr, &info);
> + parse_gre(gre_hdr, &info, &ol_flags);
> } else if (info.l4_proto == IPPROTO_IPIP) {
> void *encap_ip_hdr;
> +
> + ol_flags |= PKT_TX_TUNNEL_IPIP;
> encap_ip_hdr = (char *)l3_hdr + info.l3_len;
> parse_encap_ip(encap_ip_hdr, &info);
> }
> @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> * processed in hardware. */
> if (info.is_tunnel == 1) {
> ol_flags |= process_outer_cksums(outer_l3_hdr,
> &info,
> - testpmd_ol_flags);
> + testpmd_ol_flags, ol_flags);
> }
>
> /* step 4: fill the mbuf meta data (flags and header lengths)
> */ diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index 049a813..4c987f2 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
> union i40e_tx_offload tx_offload,
> uint32_t *cd_tunneling)
> {
> + /* Tx pkts tunnel type*/
> + if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> + *cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> + else if ((ol_flags & PKT_TX_TUNNEL_MASK) ==
> PKT_TX_TUNNEL_GRE)
> + *cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> +
> /* UDP tunneling packet TX checksum offload */
> if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
>
> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
>
> /* set i40e TSO context descriptor */
> static inline uint64_t
> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> + union i40e_tx_offload tx_offload)
> {
> uint64_t ctx_desc = 0;
> uint32_t cd_cmd, hdr_len, cd_tso_len;
> @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union
> i40e_tx_offload tx_offload)
> }
>
> /**
> - * in case of tunneling packet, the outer_l2_len and
> + * in case of non tunneling packet, the outer_l2_len and
> * outer_l3_len must be 0.
> */
> hdr_len = tx_offload.outer_l2_len +
> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union
> i40e_tx_offload tx_offload)
> I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
> ((uint64_t)mbuf->tso_segsz <<
> I40E_TXD_CTX_QW1_MSS_SHIFT);
> -
> return ctx_desc;
> }
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index
> 15e3a10..8eb0d33 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -133,6 +133,17 @@ extern "C" {
> /* add new TX flags here */
>
> /**
> + * Bits 45:48 used for the tunnel type.
> + * When doing Tx offload like TSO or checksum, the HW needs to
> +configure the
> + * tunnel type into the HW descriptors.
> + */
> +#define PKT_TX_TUNNEL_VXLAN (1ULL << 45)
> +#define PKT_TX_TUNNEL_GRE (2ULL << 45)
> +#define PKT_TX_TUNNEL_IPIP (3ULL << 45)
> +/* add new TX TUNNEL type here */
> +#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
> +
> +/**
> * Second VLAN insertion (QinQ) flag.
> */
> #define PKT_TX_QINQ_PKT (1ULL << 49) /**< TX packet with double
> VLAN inserted. */
> @@ -867,7 +878,10 @@ struct rte_mbuf {
> union {
> uint64_t tx_offload; /**< combined for easy fetch */
> struct {
> - uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> + /* L2 (MAC) Header Length if it is not a tunneling pkt.
> + * for tunnel it is outer L4 len+tunnel len+inner L2 len
> + */
> + uint64_t l2_len:7;
> uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length.
> */
> uint64_t tso_segsz:16; /**< TCP TSO segment size */
> --
> 2.1.4
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 4:05 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <20160930221241-mutt-send-email-mst@kernel.org>
On Fri, Sep 30, 2016 at 10:16:43PM +0300, Michael S. Tsirkin wrote:
> > > And the same is done is done in DPDK:
> > >
> > > static inline int __attribute__((always_inline))
> > > copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
> > > uint16_t max_desc, struct rte_mbuf *m, uint16_t desc_idx,
> > > struct rte_mempool *mbuf_pool)
> > > {
> > > ...
> > > /*
> > > * A virtio driver normally uses at least 2 desc buffers
> > > * for Tx: the first for storing the header, and others
> > > * for storing the data.
> > > */
> > > if (likely((desc->len == dev->vhost_hlen) &&
> > > (desc->flags & VRING_DESC_F_NEXT) != 0)) {
> > > desc = &descs[desc->next];
> > > if (unlikely(desc->flags & VRING_DESC_F_INDIRECT))
> > > return -1;
> > >
> > > desc_addr = gpa_to_vva(dev, desc->addr);
> > > if (unlikely(!desc_addr))
> > > return -1;
> > >
> > > rte_prefetch0((void *)(uintptr_t)desc_addr);
> > >
> > > desc_offset = 0;
> > > desc_avail = desc->len;
> > > nr_desc += 1;
> > >
> > > PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0);
> > > } else {
> > > desc_avail = desc->len - dev->vhost_hlen;
> > > desc_offset = dev->vhost_hlen;
> > > }
> >
> > Actually, the header is parsed in DPDK vhost implementation.
> > But as Virtio PMD provides a zero'ed header, we could just parse
> > the header only if VIRTIO_NET_F_NO_TX_HEADER is not negotiated.
>
> host can always skip the header parse if it wants to.
> It didn't seem worth it to add branches there but
> if I'm wrong, by all means code it up.
It's added by following commit, which yields about 10% performance
boosts for PVP case (with 64B packet size).
At that time, a packet always use 2 descs. Since indirect desc is
enabled (by default) now, the assumption is not true then. What's
worse, it might even slow things a bit down. That should also be
part of the reason why performance is slightly worse than before.
--yliu
commit 1d41d77cf81c448c1b09e1e859bfd300e2054a98
Author: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Date: Mon May 2 17:46:17 2016 -0700
vhost: optimize dequeue for small packets
A virtio driver normally uses at least 2 desc buffers for Tx: the
first for storing the header, and the others for storing the data.
Therefore, we could fetch the first data desc buf before the main
loop, and do the copy first before the check of "are we done yet?".
This could save one check for small packets that just have one data
desc buffer and need one mbuf to store it.
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Acked-by: Huawei Xie <huawei.xie@intel.com>
Tested-by: Rich Lane <rich.lane@bigswitch.com>
^ permalink raw reply
* Re: [PATCH v2] i40: fix the VXLAN TSO issue
From: Yuanhan Liu @ 2016-10-10 4:14 UTC (permalink / raw)
To: Wu, Jingjing; +Cc: dev@dpdk.org, Yigit, Ferruh
In-Reply-To: <9BB6961774997848B5B42BEC655768F80E2779DA@SHSMSX103.ccr.corp.intel.com>
On Mon, Oct 10, 2016 at 03:58:57AM +0000, Wu, Jingjing wrote:
> NACK.
>
> This fix has been done by a new one which is already merged: http://dpdk.org/dev/patchwork/patch/15059/
Just want to let you know, that you don't have to NACK an old patchset. In
the process of the patchwork, ideally, the patch author should mark the old
one as "Superseded" once he send out a new version.
Meanwhile, the committers might/could also do the mark while reviewing the
patchwork. I have done that a lot, since I didn't see any one submitted
virtio/vhost patches have done that before :/
--yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Wang, Zhihong @ 2016-10-10 4:16 UTC (permalink / raw)
To: Yuanhan Liu, Michael S. Tsirkin
Cc: Maxime Coquelin, Stephen Hemminger, dev@dpdk.org,
qemu-devel@nongnu.org
In-Reply-To: <20161010035910.GY1597@yliu-dev.sh.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, October 10, 2016 11:59 AM
> To: Michael S. Tsirkin <mst@redhat.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev@dpdk.org; qemu-
> devel@nongnu.org; Wang, Zhihong <zhihong.wang@intel.com>
> Subject: Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
>
> On Mon, Oct 10, 2016 at 06:46:44AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Oct 10, 2016 at 11:37:44AM +0800, Yuanhan Liu wrote:
> > > On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> > > > >
> > > > >
> > > > > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> > > > Yes but two points.
> > > >
> > > > 1. why is this memset expensive?
> > >
> > > I don't have the exact answer, but just some rough thoughts:
> > >
> > > It's an external clib function: there is a call stack and the
> > > IP register will bounch back and forth.
> >
> > for memset 0? gcc 5.3.1 on fedora happily inlines it.
>
> Good to know!
>
> > > overkill to use that for resetting 14 bytes structure.
> > >
> > > Some trick like
> > > *(struct virtio_net_hdr *)hdr = {0, };
> > >
> > > Or even
> > > hdr->xxx = 0;
> > > hdr->yyy = 0;
> > >
> > > should behaviour better.
> > >
> > > There was an example: the vhost enqueue optmization patchset from
> > > Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
> > > on my Ivybridge server: it has no such issue on his server though.
> > >
> > > [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
> > >
> > > --yliu
> >
> > I'd say that's weird. what's your config? any chance you
> > are using an old compiler?
>
> Not really, it's gcc 5.3.1. Maybe Zhihong could explain more. IIRC,
> he said the memset is not well optimized for Ivybridge server.
The dst is remote in that case. It's fine on Haswell but has complication
in Ivy Bridge which (wasn't supposed to but) causes serious frontend issue.
I don't think gcc inlined it there. I'm using fc24 gcc 6.1.1.
>
> --yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 4:17 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <20161010040531.GZ1597@yliu-dev.sh.intel.com>
On Mon, Oct 10, 2016 at 12:05:31PM +0800, Yuanhan Liu wrote:
> On Fri, Sep 30, 2016 at 10:16:43PM +0300, Michael S. Tsirkin wrote:
> > > > And the same is done is done in DPDK:
> > > >
> > > > static inline int __attribute__((always_inline))
> > > > copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
> > > > uint16_t max_desc, struct rte_mbuf *m, uint16_t desc_idx,
> > > > struct rte_mempool *mbuf_pool)
> > > > {
> > > > ...
> > > > /*
> > > > * A virtio driver normally uses at least 2 desc buffers
> > > > * for Tx: the first for storing the header, and others
> > > > * for storing the data.
> > > > */
> > > > if (likely((desc->len == dev->vhost_hlen) &&
> > > > (desc->flags & VRING_DESC_F_NEXT) != 0)) {
> > > > desc = &descs[desc->next];
> > > > if (unlikely(desc->flags & VRING_DESC_F_INDIRECT))
> > > > return -1;
> > > >
> > > > desc_addr = gpa_to_vva(dev, desc->addr);
> > > > if (unlikely(!desc_addr))
> > > > return -1;
> > > >
> > > > rte_prefetch0((void *)(uintptr_t)desc_addr);
> > > >
> > > > desc_offset = 0;
> > > > desc_avail = desc->len;
> > > > nr_desc += 1;
> > > >
> > > > PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0);
> > > > } else {
> > > > desc_avail = desc->len - dev->vhost_hlen;
> > > > desc_offset = dev->vhost_hlen;
> > > > }
> > >
> > > Actually, the header is parsed in DPDK vhost implementation.
> > > But as Virtio PMD provides a zero'ed header, we could just parse
> > > the header only if VIRTIO_NET_F_NO_TX_HEADER is not negotiated.
> >
> > host can always skip the header parse if it wants to.
> > It didn't seem worth it to add branches there but
> > if I'm wrong, by all means code it up.
>
> It's added by following commit, which yields about 10% performance
> boosts for PVP case (with 64B packet size).
>
> At that time, a packet always use 2 descs. Since indirect desc is
> enabled (by default) now, the assumption is not true then. What's
> worse, it might even slow things a bit down. That should also be
> part of the reason why performance is slightly worse than before.
>
> --yliu
I'm not sure I get what you are saying
> commit 1d41d77cf81c448c1b09e1e859bfd300e2054a98
> Author: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Date: Mon May 2 17:46:17 2016 -0700
>
> vhost: optimize dequeue for small packets
>
> A virtio driver normally uses at least 2 desc buffers for Tx: the
> first for storing the header, and the others for storing the data.
>
> Therefore, we could fetch the first data desc buf before the main
> loop, and do the copy first before the check of "are we done yet?".
> This could save one check for small packets that just have one data
> desc buffer and need one mbuf to store it.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Acked-by: Huawei Xie <huawei.xie@intel.com>
> Tested-by: Rich Lane <rich.lane@bigswitch.com>
This fast-paths the 2-descriptors format but it's not active
for indirect descriptors. Is this what you mean?
Should be a simple matter to apply this optimization for indirect.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yuanhan Liu @ 2016-10-10 4:22 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <20161010070800-mutt-send-email-mst@kernel.org>
On Mon, Oct 10, 2016 at 07:17:06AM +0300, Michael S. Tsirkin wrote:
> On Mon, Oct 10, 2016 at 12:05:31PM +0800, Yuanhan Liu wrote:
> > On Fri, Sep 30, 2016 at 10:16:43PM +0300, Michael S. Tsirkin wrote:
> > > > > And the same is done is done in DPDK:
> > > > >
> > > > > static inline int __attribute__((always_inline))
> > > > > copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
> > > > > uint16_t max_desc, struct rte_mbuf *m, uint16_t desc_idx,
> > > > > struct rte_mempool *mbuf_pool)
> > > > > {
> > > > > ...
> > > > > /*
> > > > > * A virtio driver normally uses at least 2 desc buffers
> > > > > * for Tx: the first for storing the header, and others
> > > > > * for storing the data.
> > > > > */
> > > > > if (likely((desc->len == dev->vhost_hlen) &&
> > > > > (desc->flags & VRING_DESC_F_NEXT) != 0)) {
> > > > > desc = &descs[desc->next];
> > > > > if (unlikely(desc->flags & VRING_DESC_F_INDIRECT))
> > > > > return -1;
> > > > >
> > > > > desc_addr = gpa_to_vva(dev, desc->addr);
> > > > > if (unlikely(!desc_addr))
> > > > > return -1;
> > > > >
> > > > > rte_prefetch0((void *)(uintptr_t)desc_addr);
> > > > >
> > > > > desc_offset = 0;
> > > > > desc_avail = desc->len;
> > > > > nr_desc += 1;
> > > > >
> > > > > PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0);
> > > > > } else {
> > > > > desc_avail = desc->len - dev->vhost_hlen;
> > > > > desc_offset = dev->vhost_hlen;
> > > > > }
> > > >
> > > > Actually, the header is parsed in DPDK vhost implementation.
> > > > But as Virtio PMD provides a zero'ed header, we could just parse
> > > > the header only if VIRTIO_NET_F_NO_TX_HEADER is not negotiated.
> > >
> > > host can always skip the header parse if it wants to.
> > > It didn't seem worth it to add branches there but
> > > if I'm wrong, by all means code it up.
> >
> > It's added by following commit, which yields about 10% performance
> > boosts for PVP case (with 64B packet size).
> >
> > At that time, a packet always use 2 descs. Since indirect desc is
> > enabled (by default) now, the assumption is not true then. What's
> > worse, it might even slow things a bit down. That should also be
> > part of the reason why performance is slightly worse than before.
> >
> > --yliu
>
> I'm not sure I get what you are saying
>
> > commit 1d41d77cf81c448c1b09e1e859bfd300e2054a98
> > Author: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > Date: Mon May 2 17:46:17 2016 -0700
> >
> > vhost: optimize dequeue for small packets
> >
> > A virtio driver normally uses at least 2 desc buffers for Tx: the
> > first for storing the header, and the others for storing the data.
> >
> > Therefore, we could fetch the first data desc buf before the main
> > loop, and do the copy first before the check of "are we done yet?".
> > This could save one check for small packets that just have one data
> > desc buffer and need one mbuf to store it.
> >
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > Acked-by: Huawei Xie <huawei.xie@intel.com>
> > Tested-by: Rich Lane <rich.lane@bigswitch.com>
>
> This fast-paths the 2-descriptors format but it's not active
> for indirect descriptors. Is this what you mean?
Yes. It's also not active when ANY_LAYOUT is actually turned on.
> Should be a simple matter to apply this optimization for indirect.
Might be.
--yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 4:24 UTC (permalink / raw)
To: Wang, Zhihong
Cc: Yuanhan Liu, Maxime Coquelin, Stephen Hemminger, dev@dpdk.org,
qemu-devel@nongnu.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7BC050@SHSMSX103.ccr.corp.intel.com>
On Mon, Oct 10, 2016 at 04:16:19AM +0000, Wang, Zhihong wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Monday, October 10, 2016 11:59 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Stephen Hemminger
> > <stephen@networkplumber.org>; dev@dpdk.org; qemu-
> > devel@nongnu.org; Wang, Zhihong <zhihong.wang@intel.com>
> > Subject: Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
> >
> > On Mon, Oct 10, 2016 at 06:46:44AM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Oct 10, 2016 at 11:37:44AM +0800, Yuanhan Liu wrote:
> > > > On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> > > > > >
> > > > > >
> > > > > > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> > > > > Yes but two points.
> > > > >
> > > > > 1. why is this memset expensive?
> > > >
> > > > I don't have the exact answer, but just some rough thoughts:
> > > >
> > > > It's an external clib function: there is a call stack and the
> > > > IP register will bounch back and forth.
> > >
> > > for memset 0? gcc 5.3.1 on fedora happily inlines it.
> >
> > Good to know!
> >
> > > > overkill to use that for resetting 14 bytes structure.
> > > >
> > > > Some trick like
> > > > *(struct virtio_net_hdr *)hdr = {0, };
> > > >
> > > > Or even
> > > > hdr->xxx = 0;
> > > > hdr->yyy = 0;
> > > >
> > > > should behaviour better.
> > > >
> > > > There was an example: the vhost enqueue optmization patchset from
> > > > Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
> > > > on my Ivybridge server: it has no such issue on his server though.
> > > >
> > > > [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
> > > >
> > > > --yliu
> > >
> > > I'd say that's weird. what's your config? any chance you
> > > are using an old compiler?
> >
> > Not really, it's gcc 5.3.1. Maybe Zhihong could explain more. IIRC,
> > he said the memset is not well optimized for Ivybridge server.
>
> The dst is remote in that case. It's fine on Haswell but has complication
> in Ivy Bridge which (wasn't supposed to but) causes serious frontend issue.
>
> I don't think gcc inlined it there. I'm using fc24 gcc 6.1.1.
I just wrote some test code and compiled with gcc -O2,
it did get inlined.
It's probably this:
uint16_t head_size = vq->hw->vtnet_hdr_size;
...
memset(hdr, 0, head_size);
IOW head_size is not known to compiler.
Try sticking a bool there instead of vtnet_hdr_size, and
memset(hdr, 0, bigheader ? 10 : 12);
> >
> > --yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 4:25 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Maxime Coquelin, Stephen Hemminger, dev, qemu-devel
In-Reply-To: <20161010042209.GB1597@yliu-dev.sh.intel.com>
On Mon, Oct 10, 2016 at 12:22:09PM +0800, Yuanhan Liu wrote:
> On Mon, Oct 10, 2016 at 07:17:06AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Oct 10, 2016 at 12:05:31PM +0800, Yuanhan Liu wrote:
> > > On Fri, Sep 30, 2016 at 10:16:43PM +0300, Michael S. Tsirkin wrote:
> > > > > > And the same is done is done in DPDK:
> > > > > >
> > > > > > static inline int __attribute__((always_inline))
> > > > > > copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
> > > > > > uint16_t max_desc, struct rte_mbuf *m, uint16_t desc_idx,
> > > > > > struct rte_mempool *mbuf_pool)
> > > > > > {
> > > > > > ...
> > > > > > /*
> > > > > > * A virtio driver normally uses at least 2 desc buffers
> > > > > > * for Tx: the first for storing the header, and others
> > > > > > * for storing the data.
> > > > > > */
> > > > > > if (likely((desc->len == dev->vhost_hlen) &&
> > > > > > (desc->flags & VRING_DESC_F_NEXT) != 0)) {
> > > > > > desc = &descs[desc->next];
> > > > > > if (unlikely(desc->flags & VRING_DESC_F_INDIRECT))
> > > > > > return -1;
> > > > > >
> > > > > > desc_addr = gpa_to_vva(dev, desc->addr);
> > > > > > if (unlikely(!desc_addr))
> > > > > > return -1;
> > > > > >
> > > > > > rte_prefetch0((void *)(uintptr_t)desc_addr);
> > > > > >
> > > > > > desc_offset = 0;
> > > > > > desc_avail = desc->len;
> > > > > > nr_desc += 1;
> > > > > >
> > > > > > PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0);
> > > > > > } else {
> > > > > > desc_avail = desc->len - dev->vhost_hlen;
> > > > > > desc_offset = dev->vhost_hlen;
> > > > > > }
> > > > >
> > > > > Actually, the header is parsed in DPDK vhost implementation.
> > > > > But as Virtio PMD provides a zero'ed header, we could just parse
> > > > > the header only if VIRTIO_NET_F_NO_TX_HEADER is not negotiated.
> > > >
> > > > host can always skip the header parse if it wants to.
> > > > It didn't seem worth it to add branches there but
> > > > if I'm wrong, by all means code it up.
> > >
> > > It's added by following commit, which yields about 10% performance
> > > boosts for PVP case (with 64B packet size).
> > >
> > > At that time, a packet always use 2 descs. Since indirect desc is
> > > enabled (by default) now, the assumption is not true then. What's
> > > worse, it might even slow things a bit down. That should also be
> > > part of the reason why performance is slightly worse than before.
> > >
> > > --yliu
> >
> > I'm not sure I get what you are saying
> >
> > > commit 1d41d77cf81c448c1b09e1e859bfd300e2054a98
> > > Author: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > Date: Mon May 2 17:46:17 2016 -0700
> > >
> > > vhost: optimize dequeue for small packets
> > >
> > > A virtio driver normally uses at least 2 desc buffers for Tx: the
> > > first for storing the header, and the others for storing the data.
> > >
> > > Therefore, we could fetch the first data desc buf before the main
> > > loop, and do the copy first before the check of "are we done yet?".
> > > This could save one check for small packets that just have one data
> > > desc buffer and need one mbuf to store it.
> > >
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > Acked-by: Huawei Xie <huawei.xie@intel.com>
> > > Tested-by: Rich Lane <rich.lane@bigswitch.com>
> >
> > This fast-paths the 2-descriptors format but it's not active
> > for indirect descriptors. Is this what you mean?
>
> Yes. It's also not active when ANY_LAYOUT is actually turned on.
It's not needed there though - you only use 1 desc, no point in
fetching the next one.
> > Should be a simple matter to apply this optimization for indirect.
>
> Might be.
>
> --yliu
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Michael S. Tsirkin @ 2016-10-10 4:39 UTC (permalink / raw)
To: Wang, Zhihong
Cc: Yuanhan Liu, Maxime Coquelin, Stephen Hemminger, dev@dpdk.org,
qemu-devel@nongnu.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7BC050@SHSMSX103.ccr.corp.intel.com>
On Mon, Oct 10, 2016 at 04:16:19AM +0000, Wang, Zhihong wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Monday, October 10, 2016 11:59 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Stephen Hemminger
> > <stephen@networkplumber.org>; dev@dpdk.org; qemu-
> > devel@nongnu.org; Wang, Zhihong <zhihong.wang@intel.com>
> > Subject: Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
> >
> > On Mon, Oct 10, 2016 at 06:46:44AM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Oct 10, 2016 at 11:37:44AM +0800, Yuanhan Liu wrote:
> > > > On Thu, Sep 29, 2016 at 11:21:48PM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Sep 29, 2016 at 10:05:22PM +0200, Maxime Coquelin wrote:
> > > > > >
> > > > > >
> > > > > > On 09/29/2016 07:57 PM, Michael S. Tsirkin wrote:
> > > > > Yes but two points.
> > > > >
> > > > > 1. why is this memset expensive?
> > > >
> > > > I don't have the exact answer, but just some rough thoughts:
> > > >
> > > > It's an external clib function: there is a call stack and the
> > > > IP register will bounch back and forth.
> > >
> > > for memset 0? gcc 5.3.1 on fedora happily inlines it.
> >
> > Good to know!
> >
> > > > overkill to use that for resetting 14 bytes structure.
> > > >
> > > > Some trick like
> > > > *(struct virtio_net_hdr *)hdr = {0, };
> > > >
> > > > Or even
> > > > hdr->xxx = 0;
> > > > hdr->yyy = 0;
> > > >
> > > > should behaviour better.
> > > >
> > > > There was an example: the vhost enqueue optmization patchset from
> > > > Zhihong [0] uses memset, and it introduces more than 15% drop (IIRC)
> > > > on my Ivybridge server: it has no such issue on his server though.
> > > >
> > > > [0]: http://dpdk.org/ml/archives/dev/2016-August/045272.html
> > > >
> > > > --yliu
> > >
> > > I'd say that's weird. what's your config? any chance you
> > > are using an old compiler?
> >
> > Not really, it's gcc 5.3.1. Maybe Zhihong could explain more. IIRC,
> > he said the memset is not well optimized for Ivybridge server.
>
> The dst is remote in that case. It's fine on Haswell but has complication
> in Ivy Bridge which (wasn't supposed to but) causes serious frontend issue.
>
> I don't think gcc inlined it there. I'm using fc24 gcc 6.1.1.
So try something like this then:
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index dd7693f..7a3f88e 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -292,6 +292,16 @@ vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
return (hw->guest_features & (1ULL << bit)) != 0;
}
+static inline int
+vtnet_hdr_size(struct virtio_hw *hw)
+{
+ if (vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF) ||
+ vtpci_with_feature(hw, VIRTIO_F_VERSION_1))
+ return sizeof(struct virtio_net_hdr_mrg_rxbuf);
+ else
+ return sizeof(struct virtio_net_hdr);
+}
+
/*
* Function declaration from virtio_pci.c
*/
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index a27208e..21a45e1 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -216,7 +216,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
struct vring_desc *start_dp;
uint16_t seg_num = cookie->nb_segs;
uint16_t head_idx, idx;
- uint16_t head_size = vq->hw->vtnet_hdr_size;
+ uint16_t head_size = vtnet_hdr_size(vq->hw);
unsigned long offs;
head_idx = vq->vq_desc_head_idx;
Generally pointer chasing in vq->hw->vtnet_hdr_size can't be good
for performance. Move fields used on data path into vq
and use from there to avoid indirections?
^ permalink raw reply related
* Re: [PATCH v2] drivers: prefix driver REGISTER macro with RTE PMD
From: Shreyansh Jain @ 2016-10-10 5:11 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand@6wind.com, dev, nhorman@tuxdriver.com
In-Reply-To: <4525292.xovB97rfbE@xps13>
On Monday 10 October 2016 01:20 AM, Thomas Monjalon wrote:
> 2016-10-09 15:12, Shreyansh Jain:
>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>>> 2016-10-08 23:35, Shreyansh Jain:
>>>> +PMDINFO_TO_O = if grep -E 'RTE_PMD_REGISTER_PCI\([0-9a-zA-Z,_\.
>>> ]+\)|RTE_PMD_REGISTER_VDEV\([0-9a-zA-Z,_\. ]+\)' $<;\
>>>> + then \
>>>
>>> I don't understand why you don't simply grep 'RTE_PMD_REGISTER_.*(' ?
>>
>> Because I want to make sure that the grep matches only the DRIVER registration functions.
>> In case a new macro (or driver type) is added in future, this macro can be updated. This way we can reduce the probability of a faulty match.
>>
>> Is there a problem with closest possible match?
>
> It is just long and useless. A macro starting with RTE_PMD_REGISTER_ must
> be called from a PMD. What else?
Long, yes. But I don't know why you state it as useless. Reducing
probability of a false positive is what it does.
Anyways, I will send a v3 with "grep -Eq 'DRIVER_REGISTER_.*\(.*\)'".
-
Shreyansh
^ permalink raw reply
* Re: [PATCH v2] drivers: prefix driver REGISTER macro with RTE PMD
From: Shreyansh Jain @ 2016-10-10 5:16 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand@6wind.com, dev, nhorman@tuxdriver.com
In-Reply-To: <de52ed62-59e5-6f14-6e58-866f04da43db@nxp.com>
On Monday 10 October 2016 10:41 AM, Shreyansh Jain wrote:
> On Monday 10 October 2016 01:20 AM, Thomas Monjalon wrote:
>> 2016-10-09 15:12, Shreyansh Jain:
>>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>>>> 2016-10-08 23:35, Shreyansh Jain:
>>>>> +PMDINFO_TO_O = if grep -E 'RTE_PMD_REGISTER_PCI\([0-9a-zA-Z,_\.
>>>> ]+\)|RTE_PMD_REGISTER_VDEV\([0-9a-zA-Z,_\. ]+\)' $<;\
>>>>> + then \
>>>>
>>>> I don't understand why you don't simply grep 'RTE_PMD_REGISTER_.*(' ?
>>>
>>> Because I want to make sure that the grep matches only the DRIVER
>>> registration functions.
>>> In case a new macro (or driver type) is added in future, this macro
>>> can be updated. This way we can reduce the probability of a faulty
>>> match.
>>>
>>> Is there a problem with closest possible match?
>>
>> It is just long and useless. A macro starting with RTE_PMD_REGISTER_ must
>> be called from a PMD. What else?
>
> Long, yes. But I don't know why you state it as useless. Reducing
> probability of a false positive is what it does.
>
> Anyways, I will send a v3 with "grep -Eq 'DRIVER_REGISTER_.*\(.*\)'".
I meant "grep -q 'RTE_PMD_REGISTER_.*(.*)'".
Minimal changes from existing.
-
Shreyansh
^ permalink raw reply
* Re: [PATCH v2] test_cryptodev_perf: IV and digest should be stored at a DMAeble address
From: Akhil Goyal @ 2016-10-10 5:22 UTC (permalink / raw)
To: De Lara Guarch, Pablo, Kusztal, ArkadiuszX, Doherty, Declan
Cc: Griffin, John, Trahe, Fiona, Jain, Deepak K, dev@dpdk.org
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA05E10@IRSMSX108.ger.corp.intel.com>
On 10/8/2016 3:06 AM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
>> akhil.goyal@nxp.com
>> Sent: Friday, October 07, 2016 10:06 AM
>> To: Kusztal, ArkadiuszX; Doherty, Declan
>> Cc: Griffin, John; Trahe, Fiona; Jain, Deepak K; dev@dpdk.org; Akhil Goyal
>> Subject: [dpdk-dev] [PATCH v2] test_cryptodev_perf: IV and digest should be
>> stored at a DMAeble address
>>
>> From: Akhil Goyal <akhil.goyal@nxp.com>
>>
>> For physical crypto devices, IV and digest are processed by the crypto
>> device which need the contents to be written on some DMA able address.
>>
>> So in order to do that, IV and digest are accomodated in the packet.
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>> v2: patch rebased
>
> You need to rebase against the HEAD of the dpdk-next-crypto subtree:
> (http://dpdk.org/browse/next/dpdk-next-crypto/).
>
> Thanks!
> Pablo
>
>
>
>
>
Hi Pablo,
I have already rebased this patch to dpdk-next-crypto subtree. Please
let me know if there is any issue.
Here is my git log
779f1301a382b6b9e2877fd0357bba33d1242d65 test_cryptodev_perf: IV and
digest should be stored at a DMAeble address
8c9fdf4568768b7765fc2176e400a860dc758020 app/test: remove hard-coding of
crypto num qps
c1876c1cb90f0882ada0acd9e430be7cf63bc765 app/test: cleanup unnecessary
ring size setup
136592c3a350ded56438b59cc4921a243f08e1d0 app/test: remove pointless for loop
fca4f966b42adc0c8f3e1d43a94ddddd93ea4fcb crypto/aesni_mb: free ring
memory on qp release in PMD
Regards
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Jianbo Liu @ 2016-10-10 5:31 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Wang, Zhihong, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <20161010024428.GT1597@yliu-dev.sh.intel.com>
On 10 October 2016 at 10:44, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Sun, Oct 09, 2016 at 12:09:07PM +0000, Wang, Zhihong wrote:
>> > > > Tested with testpmd, host: txonly, guest: rxonly
>> > > > size (bytes) improvement (%)
>> > > > 64 4.12
>> > > > 128 6
>> > > > 256 2.65
>> > > > 512 -1.12
>> > > > 1024 -7.02
>> > >
>> > > There is a difference between Zhihong's code and the old I spotted in
>> > > the first time: Zhihong removed the avail_idx prefetch. I understand
>> > > the prefetch becomes a bit tricky when mrg-rx code path is considered;
>> > > thus, I didn't comment on that.
>> > >
>> > > That's one of the difference that, IMO, could drop a regression. I then
>> > > finally got a chance to add it back.
>> > >
>> > > A rough test shows it improves the performance of 1400B packet size
>> > greatly
>> > > in the "txonly in host and rxonly in guest" case: +33% is the number I get
>> > > with my test server (Ivybridge).
>> >
>> > Thanks Yuanhan! I'll validate this on x86.
>>
>> Hi Yuanhan,
>>
>> Seems your code doesn't perform correctly. I write a new version
>> of avail idx prefetch but didn't see any perf benefit.
>>
>> To be honest I doubt the benefit of this idea. The previous mrg_off
>> code has this method but doesn't give any benefits.
>
> Good point. I thought of that before, too. But you know that I made it
> in rush, that I didn't think further and test more.
>
> I looked the code a bit closer this time, and spotted a bug: the prefetch
> actually didn't happen, due to following code piece:
>
> if (vq->next_avail_idx >= NR_AVAIL_IDX_PREFETCH) {
> prefetch_avail_idx(vq);
> ...
> }
>
> Since vq->next_avail_idx is set to 0 at the entrance of enqueue path,
> prefetch_avail_idx() will be called. The fix is easy though: just put
> prefetch_avail_idx before invoking enqueue_packet.
>
> In summary, Zhihong is right, I see no more gains with that fix :(
>
> However, as stated, that's kind of the only difference I found between
> yours and the old code, that maybe it's still worthwhile to have a
> test on ARM, Jianbo?
>
I haven't tested it, but I think it could be no improvement for ARM either.
A smalll suggestion for enqueue_packet:
.....
+ /* start copy from mbuf to desc */
+ while (mbuf_avail || mbuf->next) {
.....
Considering pkt_len is in the first cache line (same as data_len),
while next pointer is in the second cache line,
is it better to check the total packet len, instead of the last mbuf's
next pointer to jump out of while loop and avoid possible cache miss?
^ permalink raw reply
* [PATCH v3] drivers: prefix driver REGISTER macro with RTE PMD
From: Shreyansh Jain @ 2016-10-10 5:43 UTC (permalink / raw)
To: david.marchand; +Cc: dev, thomas.monjalon, nhorman, Shreyansh Jain
In-Reply-To: <1475949668-26829-1-git-send-email-shreyansh.jain@nxp.com>
All macros related to driver registeration renamed from DRIVER_*
to RTE_PMD_*
This includes:
DRIVER_REGISTER_PCI -> RTE_PMD_REGISTER_PCI
DRIVER_REGISTER_PCI_TABLE -> RTE_PMD_REGISTER_PCI_TABLE
DRIVER_REGISTER_VDEV -> RTE_PMD_REGISTER_VDEV
DRIVER_REGISTER_PARAM_STRING -> RTE_PMD_REGISTER_PARAM_STRING
DRIVER_EXPORT_* -> RTE_PMD_EXPORT_*
Fix PMDINFOGEN tool to look for matches of RTE_PMD_REGISTER_*.
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
--
Changes since v2:
- Rebase over master
- revert PMDINFO grep pattern
Changes since v1:
- Fix patch headline
Changes since v0:
- expand replacement to DRIVER_EXPORT_*
- merge all changes into single commit
- checkpatch fixes
---
doc/guides/prog_guide/dev_kit_build_system.rst | 2 +-
drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 ++--
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 4 ++--
drivers/crypto/kasumi/rte_kasumi_pmd.c | 4 ++--
drivers/crypto/libcrypto/rte_libcrypto_pmd.c | 5 +++--
drivers/crypto/null/null_crypto_pmd.c | 4 ++--
drivers/crypto/qat/rte_qat_cryptodev.c | 4 ++--
drivers/crypto/snow3g/rte_snow3g_pmd.c | 4 ++--
drivers/crypto/zuc/rte_zuc_pmd.c | 4 ++--
drivers/net/af_packet/rte_eth_af_packet.c | 4 ++--
drivers/net/bnx2x/bnx2x_ethdev.c | 8 ++++----
drivers/net/bnxt/bnxt_ethdev.c | 4 ++--
drivers/net/bonding/rte_eth_bond_pmd.c | 4 ++--
drivers/net/cxgbe/cxgbe_ethdev.c | 4 ++--
drivers/net/e1000/em_ethdev.c | 4 ++--
drivers/net/e1000/igb_ethdev.c | 8 ++++----
drivers/net/ena/ena_ethdev.c | 4 ++--
drivers/net/enic/enic_ethdev.c | 4 ++--
drivers/net/fm10k/fm10k_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 4 ++--
drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++++----
drivers/net/mlx4/mlx4.c | 4 ++--
drivers/net/mlx5/mlx5.c | 4 ++--
drivers/net/mpipe/mpipe_tilegx.c | 4 ++--
drivers/net/nfp/nfp_net.c | 4 ++--
drivers/net/null/rte_eth_null.c | 4 ++--
drivers/net/pcap/rte_eth_pcap.c | 4 ++--
drivers/net/qede/qede_ethdev.c | 8 ++++----
drivers/net/ring/rte_eth_ring.c | 4 ++--
drivers/net/szedata2/rte_eth_szedata2.c | 4 ++--
drivers/net/thunderx/nicvf_ethdev.c | 4 ++--
drivers/net/vhost/rte_eth_vhost.c | 4 ++--
drivers/net/virtio/virtio_ethdev.c | 4 ++--
drivers/net/virtio/virtio_user_ethdev.c | 4 ++--
drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 ++--
drivers/net/xenvirt/rte_eth_xenvirt.c | 4 ++--
lib/librte_eal/common/eal_common_dev.c | 2 +-
lib/librte_eal/common/include/rte_dev.h | 10 +++++-----
lib/librte_eal/common/include/rte_pci.h | 4 ++--
lib/librte_eal/common/include/rte_vdev.h | 4 ++--
mk/internal/rte.compile-pre.mk | 2 +-
42 files changed, 93 insertions(+), 92 deletions(-)
diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst
index 05358d0..19de156 100644
--- a/doc/guides/prog_guide/dev_kit_build_system.rst
+++ b/doc/guides/prog_guide/dev_kit_build_system.rst
@@ -264,7 +264,7 @@ instance the macro:
.. code-block:: c
- DRIVER_REGISTER_PCI(name, drv)
+ RTE_PMD_REGISTER_PCI(name, drv)
Creates the following symbol:
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index 76dce9c..0b3fd09 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -528,8 +528,8 @@ static struct rte_vdev_driver aesni_gcm_pmd_drv = {
.remove = aesni_gcm_remove
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_AESNI_GCM_PMD, aesni_gcm_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_GCM_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_AESNI_GCM_PMD, aesni_gcm_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_GCM_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index ff2fc25..b936735 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -719,8 +719,8 @@ static struct rte_vdev_driver cryptodev_aesni_mb_pmd_drv = {
.remove = cryptodev_aesni_mb_remove
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_AESNI_MB_PMD, cryptodev_aesni_mb_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_AESNI_MB_PMD, cryptodev_aesni_mb_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index 03e7272..11bbf80 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -655,8 +655,8 @@ static struct rte_vdev_driver cryptodev_kasumi_pmd_drv = {
.remove = cryptodev_kasumi_remove
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_KASUMI_PMD, cryptodev_kasumi_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_KASUMI_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_KASUMI_PMD, cryptodev_kasumi_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_KASUMI_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/libcrypto/rte_libcrypto_pmd.c b/drivers/crypto/libcrypto/rte_libcrypto_pmd.c
index 535fb57..e991c57 100644
--- a/drivers/crypto/libcrypto/rte_libcrypto_pmd.c
+++ b/drivers/crypto/libcrypto/rte_libcrypto_pmd.c
@@ -1054,8 +1054,9 @@ static struct rte_vdev_driver cryptodev_libcrypto_pmd_drv = {
.remove = cryptodev_libcrypto_uninit
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_LIBCRYPTO_PMD, cryptodev_libcrypto_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_LIBCRYPTO_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_LIBCRYPTO_PMD,
+ cryptodev_libcrypto_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_LIBCRYPTO_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/null/null_crypto_pmd.c b/drivers/crypto/null/null_crypto_pmd.c
index e41e819..a7d3600 100644
--- a/drivers/crypto/null/null_crypto_pmd.c
+++ b/drivers/crypto/null/null_crypto_pmd.c
@@ -273,8 +273,8 @@ static struct rte_vdev_driver cryptodev_null_pmd_drv = {
.remove = cryptodev_null_remove
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_NULL_PMD, cryptodev_null_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_NULL_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_NULL_PMD, cryptodev_null_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_NULL_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c b/drivers/crypto/qat/rte_qat_cryptodev.c
index 4846b30..1e7ee61 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -129,6 +129,6 @@ static struct rte_cryptodev_driver rte_qat_pmd = {
.dev_private_size = sizeof(struct qat_pmd_private),
};
-DRIVER_REGISTER_PCI(CRYPTODEV_NAME_QAT_SYM_PMD, rte_qat_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(CRYPTODEV_NAME_QAT_SYM_PMD, pci_id_qat_map);
+RTE_PMD_REGISTER_PCI(CRYPTODEV_NAME_QAT_SYM_PMD, rte_qat_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(CRYPTODEV_NAME_QAT_SYM_PMD, pci_id_qat_map);
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index 60d8969..a794251 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -643,8 +643,8 @@ static struct rte_vdev_driver cryptodev_snow3g_pmd_drv = {
.remove = cryptodev_snow3g_remove
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_SNOW3G_PMD, cryptodev_snow3g_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_SNOW3G_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_SNOW3G_PMD, cryptodev_snow3g_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_SNOW3G_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/crypto/zuc/rte_zuc_pmd.c b/drivers/crypto/zuc/rte_zuc_pmd.c
index 457f3ca..f46a16c 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd.c
+++ b/drivers/crypto/zuc/rte_zuc_pmd.c
@@ -543,8 +543,8 @@ static struct rte_vdev_driver cryptodev_zuc_pmd_drv = {
.remove = cryptodev_zuc_uninit
};
-DRIVER_REGISTER_VDEV(CRYPTODEV_NAME_ZUC_PMD, cryptodev_zuc_pmd_drv);
-DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_ZUC_PMD,
+RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_ZUC_PMD, cryptodev_zuc_pmd_drv);
+RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_ZUC_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 6777c41..201c1be 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -894,8 +894,8 @@ static struct rte_vdev_driver pmd_af_packet_drv = {
.remove = rte_pmd_af_packet_remove,
};
-DRIVER_REGISTER_VDEV(net_af_packet, pmd_af_packet_drv);
-DRIVER_REGISTER_PARAM_STRING(net_af_packet,
+RTE_PMD_REGISTER_VDEV(net_af_packet, pmd_af_packet_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_af_packet,
"iface=<string> "
"qpairs=<int> "
"blocksz=<int> "
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index e38c238..416bcbb 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -641,7 +641,7 @@ static struct eth_driver rte_bnx2xvf_pmd = {
.dev_private_size = sizeof(struct bnx2x_softc),
};
-DRIVER_REGISTER_PCI(net_bnx2x, rte_bnx2x_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_bnx2x, pci_id_bnx2x_map);
-DRIVER_REGISTER_PCI(net_bnx2xvf, rte_bnx2xvf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_bnx2xvf, pci_id_bnx2xvf_map);
+RTE_PMD_REGISTER_PCI(net_bnx2x, rte_bnx2x_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_bnx2x, pci_id_bnx2x_map);
+RTE_PMD_REGISTER_PCI(net_bnx2xvf, rte_bnx2xvf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_bnx2xvf, pci_id_bnx2xvf_map);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 427aa69..24e668d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1121,5 +1121,5 @@ static struct eth_driver bnxt_rte_pmd = {
.dev_private_size = sizeof(struct bnxt),
};
-DRIVER_REGISTER_PCI(net_bnxt, bnxt_rte_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_bnxt, bnxt_pci_id_map);
+RTE_PMD_REGISTER_PCI(net_bnxt, bnxt_rte_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_bnxt, bnxt_pci_id_map);
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 970fe0c..d3c3e14 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2513,9 +2513,9 @@ static struct rte_vdev_driver bond_drv = {
.remove = bond_remove,
};
-DRIVER_REGISTER_VDEV(net_bonding, bond_drv);
+RTE_PMD_REGISTER_VDEV(net_bonding, bond_drv);
-DRIVER_REGISTER_PARAM_STRING(net_bonding,
+RTE_PMD_REGISTER_PARAM_STRING(net_bonding,
"slave=<ifc> "
"primary=<ifc> "
"mode=[0-6] "
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index d3ff94a..b7f28eb 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -1048,5 +1048,5 @@ static struct eth_driver rte_cxgbe_pmd = {
.dev_private_size = sizeof(struct port_info),
};
-DRIVER_REGISTER_PCI(net_cxgbe, rte_cxgbe_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_cxgbe, cxgb4_pci_tbl);
+RTE_PMD_REGISTER_PCI(net_cxgbe, rte_cxgbe_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_cxgbe, cxgb4_pci_tbl);
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index f767e1c..99d07f0 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1793,5 +1793,5 @@ eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
return 0;
}
-DRIVER_REGISTER_PCI(net_e1000_em, rte_em_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_e1000_em, pci_id_em_map);
+RTE_PMD_REGISTER_PCI(net_e1000_em, rte_em_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_e1000_em, pci_id_em_map);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 5a1a83e..1a7d9b8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -5226,7 +5226,7 @@ eth_igb_configure_msix_intr(struct rte_eth_dev *dev)
E1000_WRITE_FLUSH(hw);
}
-DRIVER_REGISTER_PCI(net_e1000_igb, rte_igb_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_e1000_igb, pci_id_igb_map);
-DRIVER_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_e1000_igb_vf, pci_id_igbvf_map);
+RTE_PMD_REGISTER_PCI(net_e1000_igb, rte_igb_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb, pci_id_igb_map);
+RTE_PMD_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb_vf, pci_id_igbvf_map);
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 6efd801..2227265 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1695,5 +1695,5 @@ static struct eth_driver rte_ena_pmd = {
.dev_private_size = sizeof(struct ena_adapter),
};
-DRIVER_REGISTER_PCI(net_ena, rte_ena_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_ena, pci_id_ena_map);
+RTE_PMD_REGISTER_PCI(net_ena, rte_ena_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_ena, pci_id_ena_map);
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 82dc265..45756d4 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -643,5 +643,5 @@ static struct eth_driver rte_enic_pmd = {
.dev_private_size = sizeof(struct enic),
};
-DRIVER_REGISTER_PCI(net_enic, rte_enic_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_enic, pci_id_enic_map);
+RTE_PMD_REGISTER_PCI(net_enic, rte_enic_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_enic, pci_id_enic_map);
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 372564b..c804436 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -3071,5 +3071,5 @@ static struct eth_driver rte_pmd_fm10k = {
.dev_private_size = sizeof(struct fm10k_adapter),
};
-DRIVER_REGISTER_PCI(net_fm10k, rte_pmd_fm10k.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_fm10k, pci_id_fm10k_map);
+RTE_PMD_REGISTER_PCI(net_fm10k, rte_pmd_fm10k.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_fm10k, pci_id_fm10k_map);
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d0640b9..f213de5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -702,8 +702,8 @@ rte_i40e_dev_atomic_write_link_status(struct rte_eth_dev *dev,
return 0;
}
-DRIVER_REGISTER_PCI(net_i40e, rte_i40e_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_i40e, pci_id_i40e_map);
+RTE_PMD_REGISTER_PCI(net_i40e, rte_i40e_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_i40e, pci_id_i40e_map);
#ifndef I40E_GLQF_ORT
#define I40E_GLQF_ORT(_i) (0x00268900 + ((_i) * 4))
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 34eb274..635a170 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1564,8 +1564,8 @@ static struct eth_driver rte_i40evf_pmd = {
.dev_private_size = sizeof(struct i40e_adapter),
};
-DRIVER_REGISTER_PCI(net_i40e_vf, rte_i40evf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_i40e_vf, pci_id_i40evf_map);
+RTE_PMD_REGISTER_PCI(net_i40e_vf, rte_i40evf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_i40e_vf, pci_id_i40evf_map);
static int
i40evf_dev_configure(struct rte_eth_dev *dev)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 6b3d4fa..685f0f2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -7348,7 +7348,7 @@ ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
ixgbevf_dev_interrupt_action(dev);
}
-DRIVER_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
-DRIVER_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
+RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
+RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 1553b2e..12ab7da 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -5940,5 +5940,5 @@ rte_mlx4_pmd_init(void)
rte_eal_pci_register(&mlx4_driver.pci_drv);
}
-DRIVER_EXPORT_NAME(net_mlx4, __COUNTER__);
-DRIVER_REGISTER_PCI_TABLE(net_mlx4, mlx4_pci_id_map);
+RTE_PMD_EXPORT_NAME(net_mlx4, __COUNTER__);
+RTE_PMD_REGISTER_PCI_TABLE(net_mlx4, mlx4_pci_id_map);
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 758df6e..3678aad 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -756,5 +756,5 @@ rte_mlx5_pmd_init(void)
rte_eal_pci_register(&mlx5_driver.pci_drv);
}
-DRIVER_EXPORT_NAME(net_mlx5, __COUNTER__);
-DRIVER_REGISTER_PCI_TABLE(net_mlx5, mlx5_pci_id_map);
+RTE_PMD_EXPORT_NAME(net_mlx5, __COUNTER__);
+RTE_PMD_REGISTER_PCI_TABLE(net_mlx5, mlx5_pci_id_map);
diff --git a/drivers/net/mpipe/mpipe_tilegx.c b/drivers/net/mpipe/mpipe_tilegx.c
index b9eefdf..adf299b 100644
--- a/drivers/net/mpipe/mpipe_tilegx.c
+++ b/drivers/net/mpipe/mpipe_tilegx.c
@@ -1631,8 +1631,8 @@ static struct rte_vdev_driver pmd_mpipe_gbe_drv = {
.probe = rte_pmd_mpipe_probe,
};
-DRIVER_REGISTER_VDEV(net_mpipe_xgbe, pmd_mpipe_xgbe_drv);
-DRIVER_REGISTER_VDEV(net_mpipe_gbe, pmd_mpipe_gbe_drv);
+RTE_PMD_REGISTER_VDEV(net_mpipe_xgbe, pmd_mpipe_xgbe_drv);
+RTE_PMD_REGISTER_VDEV(net_mpipe_gbe, pmd_mpipe_gbe_drv);
static void __attribute__((constructor, used))
mpipe_init_contexts(void)
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index d526f34..faf725c 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2479,8 +2479,8 @@ static struct eth_driver rte_nfp_net_pmd = {
.dev_private_size = sizeof(struct nfp_net_adapter),
};
-DRIVER_REGISTER_PCI(net_nfp, rte_nfp_net_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_nfp, pci_id_nfp_net_map);
+RTE_PMD_REGISTER_PCI(net_nfp, rte_nfp_net_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_nfp, pci_id_nfp_net_map);
/*
* Local variables:
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index c2e10a8..0b7cc37 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -691,7 +691,7 @@ static struct rte_vdev_driver pmd_null_drv = {
.remove = rte_pmd_null_remove,
};
-DRIVER_REGISTER_VDEV(net_null, pmd_null_drv);
-DRIVER_REGISTER_PARAM_STRING(net_null,
+RTE_PMD_REGISTER_VDEV(net_null, pmd_null_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_null,
"size=<int> "
"copy=<int>");
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 965c999..0c4711d 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1064,8 +1064,8 @@ static struct rte_vdev_driver pmd_pcap_drv = {
.remove = pmd_pcap_remove,
};
-DRIVER_REGISTER_VDEV(net_pcap, pmd_pcap_drv);
-DRIVER_REGISTER_PARAM_STRING(net_pcap,
+RTE_PMD_REGISTER_VDEV(net_pcap, pmd_pcap_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
ETH_PCAP_RX_PCAP_ARG "=<string> "
ETH_PCAP_TX_PCAP_ARG "=<string> "
ETH_PCAP_RX_IFACE_ARG "=<ifc> "
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index d00c1d9..452139d 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1502,7 +1502,7 @@ static struct eth_driver rte_qede_pmd = {
.dev_private_size = sizeof(struct qede_dev),
};
-DRIVER_REGISTER_PCI(net_qede, rte_qede_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_qede, pci_id_qede_map);
-DRIVER_REGISTER_PCI(net_qede_vf, rte_qedevf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_qede_vf, pci_id_qedevf_map);
+RTE_PMD_REGISTER_PCI(net_qede, rte_qede_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_qede, pci_id_qede_map);
+RTE_PMD_REGISTER_PCI(net_qede_vf, rte_qedevf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_qede_vf, pci_id_qedevf_map);
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index b91f222..fb423fa 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -628,6 +628,6 @@ static struct rte_vdev_driver pmd_ring_drv = {
.remove = rte_pmd_ring_remove,
};
-DRIVER_REGISTER_VDEV(net_ring, pmd_ring_drv);
-DRIVER_REGISTER_PARAM_STRING(net_ring,
+RTE_PMD_REGISTER_VDEV(net_ring, pmd_ring_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_ring,
"nodeaction=[attach|detach]");
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index f4ec5ea..f3cd52d 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1581,5 +1581,5 @@ static struct eth_driver szedata2_eth_driver = {
.dev_private_size = sizeof(struct pmd_internals),
};
-DRIVER_REGISTER_PCI(RTE_SZEDATA2_DRIVER_NAME, szedata2_eth_driver.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(RTE_SZEDATA2_DRIVER_NAME, rte_szedata2_pci_id_table);
+RTE_PMD_REGISTER_PCI(RTE_SZEDATA2_DRIVER_NAME, szedata2_eth_driver.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(RTE_SZEDATA2_DRIVER_NAME, rte_szedata2_pci_id_table);
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index b758c9f..44afdbe 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -1780,5 +1780,5 @@ static struct eth_driver rte_nicvf_pmd = {
.dev_private_size = sizeof(struct nicvf),
};
-DRIVER_REGISTER_PCI(net_thunderx, rte_nicvf_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_thunderx, pci_id_nicvf_map);
+RTE_PMD_REGISTER_PCI(net_thunderx, rte_nicvf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_thunderx, pci_id_nicvf_map);
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index c1d09a0..036cbd1 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1230,7 +1230,7 @@ static struct rte_vdev_driver pmd_vhost_drv = {
.remove = rte_pmd_vhost_remove,
};
-DRIVER_REGISTER_VDEV(net_vhost, pmd_vhost_drv);
-DRIVER_REGISTER_PARAM_STRING(net_vhost,
+RTE_PMD_REGISTER_VDEV(net_vhost, pmd_vhost_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_vhost,
"iface=<ifc> "
"queues=<int>");
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b4dfc0a..09355fe 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1557,5 +1557,5 @@ __rte_unused uint8_t is_rx)
return 0;
}
-DRIVER_EXPORT_NAME(net_virtio, __COUNTER__);
-DRIVER_REGISTER_PCI_TABLE(net_virtio, pci_id_virtio_map);
+RTE_PMD_EXPORT_NAME(net_virtio, __COUNTER__);
+RTE_PMD_REGISTER_PCI_TABLE(net_virtio, pci_id_virtio_map);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 6fcedd9..bfdc3d0 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -478,8 +478,8 @@ static struct rte_vdev_driver virtio_user_driver = {
.remove = virtio_user_pmd_remove,
};
-DRIVER_REGISTER_VDEV(net_virtio_user, virtio_user_driver);
-DRIVER_REGISTER_PARAM_STRING(net_virtio_user,
+RTE_PMD_REGISTER_VDEV(net_virtio_user, virtio_user_driver);
+RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
"path=<path> "
"mac=<mac addr> "
"cq=<int> "
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 51e2d4c..8bb13e5 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -960,5 +960,5 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
}
#endif
-DRIVER_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd.pci_drv);
-DRIVER_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map);
+RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map);
diff --git a/drivers/net/xenvirt/rte_eth_xenvirt.c b/drivers/net/xenvirt/rte_eth_xenvirt.c
index fa01ba0..5a897b9 100644
--- a/drivers/net/xenvirt/rte_eth_xenvirt.c
+++ b/drivers/net/xenvirt/rte_eth_xenvirt.c
@@ -764,6 +764,6 @@ static struct rte_vdev_driver pmd_xenvirt_drv = {
.remove = rte_pmd_xenvirt_remove,
};
-DRIVER_REGISTER_VDEV(net_xenvirt, pmd_xenvirt_drv);
-DRIVER_REGISTER_PARAM_STRING(net_xenvirt,
+RTE_PMD_REGISTER_VDEV(net_xenvirt, pmd_xenvirt_drv);
+RTE_PMD_REGISTER_PARAM_STRING(net_xenvirt,
"mac=<mac addr>");
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index d1f0ad8..4f3b493 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -84,7 +84,7 @@ rte_eal_dev_init(void)
/*
* Note that the dev_driver_list is populated here
* from calls made to rte_eal_driver_register from constructor functions
- * embedded into PMD modules via the DRIVER_REGISTER_VDEV macro
+ * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro
*/
/* call the init function for each virtual device */
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d159991..b3873bd 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -222,19 +222,19 @@ int rte_eal_dev_attach(const char *name, const char *devargs);
*/
int rte_eal_dev_detach(const char *name);
-#define DRIVER_EXPORT_NAME_ARRAY(n, idx) n##idx[]
+#define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
-#define DRIVER_EXPORT_NAME(name, idx) \
-static const char DRIVER_EXPORT_NAME_ARRAY(this_pmd_name, idx) \
+#define RTE_PMD_EXPORT_NAME(name, idx) \
+static const char RTE_PMD_EXPORT_NAME_ARRAY(this_pmd_name, idx) \
__attribute__((used)) = RTE_STR(name)
#define DRV_EXP_TAG(name, tag) __##name##_##tag
-#define DRIVER_REGISTER_PCI_TABLE(name, table) \
+#define RTE_PMD_REGISTER_PCI_TABLE(name, table) \
static const char DRV_EXP_TAG(name, pci_tbl_export)[] __attribute__((used)) = \
RTE_STR(table)
-#define DRIVER_REGISTER_PARAM_STRING(name, str) \
+#define RTE_PMD_REGISTER_PARAM_STRING(name, str) \
static const char DRV_EXP_TAG(name, param_string_export)[] \
__attribute__((used)) = str
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 3a8e8c8..9ce8847 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -487,14 +487,14 @@ void rte_eal_pci_dump(FILE *f);
void rte_eal_pci_register(struct rte_pci_driver *driver);
/** Helper for PCI device registration from driver (eth, crypto) instance */
-#define DRIVER_REGISTER_PCI(nm, pci_drv) \
+#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
RTE_INIT(pciinitfn_ ##nm); \
static void pciinitfn_ ##nm(void) \
{\
(pci_drv).driver.name = RTE_STR(nm);\
rte_eal_pci_register(&pci_drv); \
} \
-DRIVER_EXPORT_NAME(nm, __COUNTER__)
+RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
/**
* Unregister a PCI driver.
diff --git a/lib/librte_eal/common/include/rte_vdev.h b/lib/librte_eal/common/include/rte_vdev.h
index 0dec8eb..97260b2 100644
--- a/lib/librte_eal/common/include/rte_vdev.h
+++ b/lib/librte_eal/common/include/rte_vdev.h
@@ -81,14 +81,14 @@ void rte_eal_vdrv_register(struct rte_vdev_driver *driver);
*/
void rte_eal_vdrv_unregister(struct rte_vdev_driver *driver);
-#define DRIVER_REGISTER_VDEV(nm, vdrv)\
+#define RTE_PMD_REGISTER_VDEV(nm, vdrv)\
RTE_INIT(vdrvinitfn_ ##vdrv);\
static void vdrvinitfn_ ##vdrv(void)\
{\
(vdrv).driver.name = RTE_STR(nm);\
rte_eal_vdrv_register(&vdrv);\
} \
-DRIVER_EXPORT_NAME(nm, __COUNTER__)
+RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
#ifdef __cplusplus
}
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index a42ef03..da8dda4 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -87,7 +87,7 @@ endif
PMDINFO_GEN = $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c
PMDINFO_CC = $(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@.pmd.o $@.pmd.c
PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
-PMDINFO_TO_O = if grep -q 'DRIVER_REGISTER_.*(.*)' $<; then \
+PMDINFO_TO_O = if grep -q 'RTE_PMD_REGISTER_.*(.*)' $<; then \
echo "$(if $V,$(PMDINFO_GEN), PMDINFO $@.pmd.c)" && \
$(PMDINFO_GEN) && \
echo "$(if $V,$(PMDINFO_CC), CC $@.pmd.o)" && \
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Wang, Zhihong @ 2016-10-10 6:22 UTC (permalink / raw)
To: Jianbo Liu, Yuanhan Liu; +Cc: Maxime Coquelin, dev@dpdk.org
In-Reply-To: <CAP4Qi3_jk5+=diZhq8V7FecUkZge1_riEq-bopY35n71Q_pANQ@mail.gmail.com>
> -----Original Message-----
> From: Jianbo Liu [mailto:jianbo.liu@linaro.org]
> Sent: Monday, October 10, 2016 1:32 PM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Cc: Wang, Zhihong <zhihong.wang@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] vhost: optimize enqueue
>
> On 10 October 2016 at 10:44, Yuanhan Liu <yuanhan.liu@linux.intel.com>
> wrote:
> > On Sun, Oct 09, 2016 at 12:09:07PM +0000, Wang, Zhihong wrote:
> >> > > > Tested with testpmd, host: txonly, guest: rxonly
> >> > > > size (bytes) improvement (%)
> >> > > > 64 4.12
> >> > > > 128 6
> >> > > > 256 2.65
> >> > > > 512 -1.12
> >> > > > 1024 -7.02
> >> > >
> >> > > There is a difference between Zhihong's code and the old I spotted in
> >> > > the first time: Zhihong removed the avail_idx prefetch. I understand
> >> > > the prefetch becomes a bit tricky when mrg-rx code path is
> considered;
> >> > > thus, I didn't comment on that.
> >> > >
> >> > > That's one of the difference that, IMO, could drop a regression. I then
> >> > > finally got a chance to add it back.
> >> > >
> >> > > A rough test shows it improves the performance of 1400B packet size
> >> > greatly
> >> > > in the "txonly in host and rxonly in guest" case: +33% is the number I
> get
> >> > > with my test server (Ivybridge).
> >> >
> >> > Thanks Yuanhan! I'll validate this on x86.
> >>
> >> Hi Yuanhan,
> >>
> >> Seems your code doesn't perform correctly. I write a new version
> >> of avail idx prefetch but didn't see any perf benefit.
> >>
> >> To be honest I doubt the benefit of this idea. The previous mrg_off
> >> code has this method but doesn't give any benefits.
> >
> > Good point. I thought of that before, too. But you know that I made it
> > in rush, that I didn't think further and test more.
> >
> > I looked the code a bit closer this time, and spotted a bug: the prefetch
> > actually didn't happen, due to following code piece:
> >
> > if (vq->next_avail_idx >= NR_AVAIL_IDX_PREFETCH) {
> > prefetch_avail_idx(vq);
> > ...
> > }
> >
> > Since vq->next_avail_idx is set to 0 at the entrance of enqueue path,
> > prefetch_avail_idx() will be called. The fix is easy though: just put
> > prefetch_avail_idx before invoking enqueue_packet.
> >
> > In summary, Zhihong is right, I see no more gains with that fix :(
> >
> > However, as stated, that's kind of the only difference I found between
> > yours and the old code, that maybe it's still worthwhile to have a
> > test on ARM, Jianbo?
> >
> I haven't tested it, but I think it could be no improvement for ARM either.
>
> A smalll suggestion for enqueue_packet:
>
> .....
> + /* start copy from mbuf to desc */
> + while (mbuf_avail || mbuf->next) {
> .....
>
> Considering pkt_len is in the first cache line (same as data_len),
> while next pointer is in the second cache line,
> is it better to check the total packet len, instead of the last mbuf's
> next pointer to jump out of while loop and avoid possible cache miss?
Jianbo,
Thanks for the reply!
This idea sounds good, but it won't help the general perf in my
opinion, since the 2nd cache line is accessed anyway prior in
virtio_enqueue_offload.
Also this would bring a NULL check when actually access mbuf->next.
BTW, could you please publish the number of:
1. mrg_rxbuf=on, comparison between original and original + this patch
2. mrg_rxbuf=off, comparison between original and original + this patch
So we can have a whole picture of how this patch impact on ARM platform.
Thanks
Zhihong
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Jianbo Liu @ 2016-10-10 6:57 UTC (permalink / raw)
To: Wang, Zhihong; +Cc: Yuanhan Liu, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7BC1BD@SHSMSX103.ccr.corp.intel.com>
On 10 October 2016 at 14:22, Wang, Zhihong <zhihong.wang@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Jianbo Liu [mailto:jianbo.liu@linaro.org]
>> Sent: Monday, October 10, 2016 1:32 PM
>> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> Cc: Wang, Zhihong <zhihong.wang@intel.com>; Maxime Coquelin
>> <maxime.coquelin@redhat.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3 0/5] vhost: optimize enqueue
>>
>> On 10 October 2016 at 10:44, Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> wrote:
>> > On Sun, Oct 09, 2016 at 12:09:07PM +0000, Wang, Zhihong wrote:
>> >> > > > Tested with testpmd, host: txonly, guest: rxonly
>> >> > > > size (bytes) improvement (%)
>> >> > > > 64 4.12
>> >> > > > 128 6
>> >> > > > 256 2.65
>> >> > > > 512 -1.12
>> >> > > > 1024 -7.02
>> >> > >
>> >> > > There is a difference between Zhihong's code and the old I spotted in
>> >> > > the first time: Zhihong removed the avail_idx prefetch. I understand
>> >> > > the prefetch becomes a bit tricky when mrg-rx code path is
>> considered;
>> >> > > thus, I didn't comment on that.
>> >> > >
>> >> > > That's one of the difference that, IMO, could drop a regression. I then
>> >> > > finally got a chance to add it back.
>> >> > >
>> >> > > A rough test shows it improves the performance of 1400B packet size
>> >> > greatly
>> >> > > in the "txonly in host and rxonly in guest" case: +33% is the number I
>> get
>> >> > > with my test server (Ivybridge).
>> >> >
>> >> > Thanks Yuanhan! I'll validate this on x86.
>> >>
>> >> Hi Yuanhan,
>> >>
>> >> Seems your code doesn't perform correctly. I write a new version
>> >> of avail idx prefetch but didn't see any perf benefit.
>> >>
>> >> To be honest I doubt the benefit of this idea. The previous mrg_off
>> >> code has this method but doesn't give any benefits.
>> >
>> > Good point. I thought of that before, too. But you know that I made it
>> > in rush, that I didn't think further and test more.
>> >
>> > I looked the code a bit closer this time, and spotted a bug: the prefetch
>> > actually didn't happen, due to following code piece:
>> >
>> > if (vq->next_avail_idx >= NR_AVAIL_IDX_PREFETCH) {
>> > prefetch_avail_idx(vq);
>> > ...
>> > }
>> >
>> > Since vq->next_avail_idx is set to 0 at the entrance of enqueue path,
>> > prefetch_avail_idx() will be called. The fix is easy though: just put
>> > prefetch_avail_idx before invoking enqueue_packet.
>> >
>> > In summary, Zhihong is right, I see no more gains with that fix :(
>> >
>> > However, as stated, that's kind of the only difference I found between
>> > yours and the old code, that maybe it's still worthwhile to have a
>> > test on ARM, Jianbo?
>> >
>> I haven't tested it, but I think it could be no improvement for ARM either.
>>
>> A smalll suggestion for enqueue_packet:
>>
>> .....
>> + /* start copy from mbuf to desc */
>> + while (mbuf_avail || mbuf->next) {
>> .....
>>
>> Considering pkt_len is in the first cache line (same as data_len),
>> while next pointer is in the second cache line,
>> is it better to check the total packet len, instead of the last mbuf's
>> next pointer to jump out of while loop and avoid possible cache miss?
>
> Jianbo,
>
> Thanks for the reply!
>
> This idea sounds good, but it won't help the general perf in my
> opinion, since the 2nd cache line is accessed anyway prior in
> virtio_enqueue_offload.
>
Yes, you are right. I'm thinking of prefetching beforehand.
And if it's a chained mbuf, virtio_enqueue_offload will not be called
in next loop.
> Also this would bring a NULL check when actually access mbuf->next.
>
> BTW, could you please publish the number of:
>
> 1. mrg_rxbuf=on, comparison between original and original + this patch
>
> 2. mrg_rxbuf=off, comparison between original and original + this patch
>
> So we can have a whole picture of how this patch impact on ARM platform.
>
I think you already have got many results in my previous emails.
Sorry I can't test right now and busy with other things.
^ 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