* Re: [PATCH] examples/l3fwd: enabling 4m hash for all 64 bit archs
From: Thomas Monjalon @ 2016-10-13 8:22 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev
In-Reply-To: <1471964080-29958-2-git-send-email-hemant.agrawal@nxp.com>
2016-08-23 20:24, Hemant Agrawal:
> This patch enables the support for 4 million hash entries
> for all 64 bit architectures.
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2 12/12] virtio: add Tso support
From: Yuanhan Liu @ 2016-10-13 8:18 UTC (permalink / raw)
To: Olivier Matz
Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
xiao.w.wang
In-Reply-To: <1475485223-30566-13-git-send-email-olivier.matz@6wind.com>
On Mon, Oct 03, 2016 at 11:00:23AM +0200, Olivier Matz wrote:
> +/* When doing TSO, the IP length is not included in the pseudo header
> + * checksum of the packet given to the PMD, but for virtio it is
> + * expected.
> + */
> +static void
> +virtio_tso_fix_cksum(struct rte_mbuf *m)
> +{
> + /* common case: header is not fragmented */
> + if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len +
> + m->l4_len)) {
...
> + /* replace it in the packet */
> + th->cksum = new_cksum;
> + } else {
...
> + /* replace it in the packet */
> + *rte_pktmbuf_mtod_offset(m, uint8_t *,
> + m->l2_len + m->l3_len + 16) = new_cksum.u8[0];
> + *rte_pktmbuf_mtod_offset(m, uint8_t *,
> + m->l2_len + m->l3_len + 17) = new_cksum.u8[1];
> + }
The tcp header will always be in the mbuf, right? Otherwise, you can't
update the cksum field here. What's the point of introducing the "else
clause" then?
--yliu
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Maxime Coquelin @ 2016-10-13 7:54 UTC (permalink / raw)
To: Wang, Zhihong, Yuanhan Liu; +Cc: Jianbo Liu, Thomas Monjalon, dev@dpdk.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7CDDC7@SHSMSX103.ccr.corp.intel.com>
On 10/13/2016 08:02 AM, Wang, Zhihong wrote:
>> > Yes, that's great effort! With your hardwork, we know what the bottleneck
>> > is and how it could be improved.
>> >
>> > However, you don't have to do code refactor (merge two code path to one)
>> > to apply those improvements. From what I know, in this patchset, there
>> > are two factors could improve the performance:
>> >
>> > - copy hdr together with packet data
>> >
>> > - shadow used ring update and update at once
>> >
>> > The overall performance boost I got with your v6 patchset with mrg-Rx
>> > code path is about 27% (in PVP case). And I have just applied the 1st
>> > optimization, it yields about 20% boosts. The left could be covered if
>> > we apply the 2nd optimization (I guess).
>> >
>> > That would be a clean way to optimize vhost mergeable Rx path:
>> >
>> > - you don't touch non-mrg Rx path (well, you may could apply the
>> > shadow_used_ring trick to it as wel)
>> >
>> > This would at least make sure we will have no such performance
>> > regression issue reported by ARM guys.
>> >
>> > - you don't refactor the code
>> >
>> > The rewrite from scratch could introduce other issues, besides the
>> > performance regression. We may just don't know it yet.
>> >
>> >
>> > Make sense to you? If you agree, I think we could still make it in
>> > this release: they would be some small changes after all. For example,
>> > below is the patch applies the 1st optimization tip on top of
>> > dpdk-next-virtio
>
> Thanks for this great idea. I think it's a better way to do it.
> I'll start to make the patch then.
>
>
I personally find having two paths better for maintenance as it is
easier to understand (IMHO).
So if we can have the performance gain while keeping the two paths,
I definitely support the idea.
Thanks,
Maxime
^ permalink raw reply
* Re: [PATCH v2 03/12] virtio: reinitialize the device in configure callback
From: Yuanhan Liu @ 2016-10-13 7:54 UTC (permalink / raw)
To: Olivier MATZ
Cc: dev, konstantin.ananyev, sugesh.chandran, bruce.richardson,
jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
xiao.w.wang
In-Reply-To: <57FE5E55.4010809@6wind.com>
On Wed, Oct 12, 2016 at 06:01:25PM +0200, Olivier MATZ wrote:
> Hello Yuanhan,
>
> On 10/12/2016 04:41 PM, Yuanhan Liu wrote:
> >On Mon, Oct 03, 2016 at 11:00:14AM +0200, Olivier Matz wrote:
> >>@@ -1344,6 +1347,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> >> {
> >> const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> >> struct virtio_hw *hw = dev->data->dev_private;
> >>+ uint64_t req_features;
> >> int ret;
> >>
> >> PMD_INIT_LOG(DEBUG, "configure");
> >>@@ -1353,6 +1357,14 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> >> return -EINVAL;
> >> }
> >>
> >>+ req_features = VIRTIO_PMD_GUEST_FEATURES;
> >>+ /* if request features changed, reinit the device */
> >>+ if (req_features != hw->req_guest_features) {
> >>+ ret = virtio_init_device(dev, req_features);
> >>+ if (ret < 0)
> >>+ return ret;
> >>+ }
> >
> >Why do you have to reset virtio here? This doesn't make too much sense
> >to me.
> >
> >IIUC, you want to make sure those TSO related features being unset at
> >init time, and enable it (by doing reset) when it's asked to be enabled
> >(by rte_eth_dev_configure)?
> >
> >Why not always setting those features? We could do the actual offloads
> >when:
> >
> >- those features have been negoiated
> >
> >- they are enabled through rte_eth_dev_configure
> >
> >With that, I think we could avoid the reset here?
>
> It would work for TX, since you decide to use or not the feature. But I
> think this won't work for RX: if you negociate LRO at init, the host may
> send you large packets, even if LRO is disabled in dev_configure.
I see. Thanks.
Besides, I think you should return error when LRO is not negoiated
after the reset (say, when it's disabled through qemu command line)?
--yliu
^ permalink raw reply
* Re: [PATCH v2 0/2] examples/l2fwd: Add option to enable/disable MAC addresses updating
From: Thomas Monjalon @ 2016-10-13 7:26 UTC (permalink / raw)
To: Maxime Coquelin
Cc: dev, bruce.richardson, pablo.de.lara.guarch, john.mcnamara
In-Reply-To: <1474638654-26919-1-git-send-email-maxime.coquelin@redhat.com>
2016-09-23 15:50, Maxime Coquelin:
> This series adds a new option to enable/disable MAC addresses updating in
> l2fwd example.
>
> Doing that, we can enable basic VM 2 VM communication easily, without
> external projects dependencies, nor real NIC (as with vhost example).
>
> Example of cli with vhost-user:
>
> #l2fwd -c f --socket-mem=1024 \
> --vdev 'eth_vhost0,iface=/tmp/vhost-user1,queues=1' \
> --vdev 'eth_vhost1,iface=/tmp/vhost-user2,queues=1' \
> -- -p3 --no-mac-updating
>
> By default, MAC addresses updating remains enabled, but maybe we could
> consider having it disabled by default to be consistent with l2fwd-cat
> for example.
Applied (as a single patch), thanks
^ permalink raw reply
* Re: [PATCH v4 1/6] ethdev: add Tx preparation
From: Thomas Monjalon @ 2016-10-13 7:08 UTC (permalink / raw)
To: Tomasz Kulasek; +Cc: dev, konstantin.ananyev
In-Reply-To: <1995417.Vz0qMdTBBe@xps13>
Hi Tomasz,
Any news?
Sorry to speed up, we are very very late for RC1.
2016-10-10 16:08, Thomas Monjalon:
> Hi,
>
> Now that the feature seems to meet a consensus, I've looked at it more
> closely before integrating. Sorry if it appears like a late review.
>
> 2016-09-30 11:00, Tomasz Kulasek:
> > Added API for `rte_eth_tx_prep`
>
> I would love to read the usability and performance considerations here.
> No need for something as long as the cover letter. Just few lines
> about why it is needed and why it is a good choice for performance.
>
> > uint16_t rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id,
> > struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> >
> > Added fields to the `struct rte_eth_desc_lim`:
> >
> > uint16_t nb_seg_max;
> > /**< Max number of segments per whole packet. */
> >
> > uint16_t nb_mtu_seg_max;
> > /**< Max number of segments per one MTU */
> [...]
> > +#else
> > +
> > +static inline uint16_t
> > +rte_eth_tx_prep(uint8_t port_id __rte_unused, uint16_t queue_id __rte_unused,
> > + struct rte_mbuf **tx_pkts __rte_unused, uint16_t nb_pkts)
>
> Doxygen is failing here.
> Have you tried to move __rte_unused before the identifier?
>
> [...]
> > +#define PKT_TX_OFFLOAD_MASK ( \
> > + PKT_TX_IP_CKSUM | \
> > + PKT_TX_L4_MASK | \
> > + PKT_TX_OUTER_IP_CKSUM | \
> > + PKT_TX_TCP_SEG | \
> > + PKT_TX_QINQ_PKT | \
> > + PKT_TX_VLAN_PKT)
>
> We should really stop adding some public constants without the proper
> RTE prefix.
> And by the way, should not we move such flags into rte_net?
Do not worry with this comment. It was just a thought which could be
addressed in a separate patch by someone else.
> [...]
> > -SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h rte_pkt.h
>
> You can use the += operator on a new line for free :)
>
> No more comments, the rest seems OK. Thanks
^ permalink raw reply
* Re: [PATCH v1 0/4] Generalize PCI specific EAL function/structures
From: Shreyansh Jain @ 2016-10-13 6:47 UTC (permalink / raw)
To: David Marchand; +Cc: dev@dpdk.org, Jan Viktorin, Thomas Monjalon
In-Reply-To: <CALwxeUtrswaOEBirUetWO+KWemgzySEBfC0cWPPKcP2VuKbZ8A@mail.gmail.com>
Hi David,
On Friday 30 September 2016 09:01 PM, David Marchand wrote:
> On Tue, Sep 27, 2016 at 4:12 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> (I rebased these over HEAD 7b3c4f3)
>>
>> These patches were initially part of Jan's original series on SoC
>> Framework ([1],[2]). An update to that series, without these patches,
>> was posted here [3].
>>
>> Main motivation for these is aim of introducing a non-PCI centric
>> subsystem in EAL. As of now the first usecase is SoC, but not limited to
>> it.
>>
>> 4 patches in this series are independent of each other, as well as SoC
>> framework. All these focus on generalizing some structure or functions
>> present with the PCI specific code to EAL Common area (or splitting a
>> function to be more userful).
>
> Those patches move linux specifics (binding pci devices using sysfs)
> to common infrastucture.
> We have no proper hotplug support on bsd, but if we had some common
> code we should at least try to make the apis generic.
>
rte_eal_unbind_kernel_driver() defined in the patch is essentially a
wrapper which can be implemented for Linux as well as BSD. Just that in
this patch a ENOTSUP implementation of this was not given for BSD.
I can think of two options:
1. Implement a ENOTSUPP code for BSD and allow
rte_eal_unbind_kernel_driver() as part of common/eal_private header.
implement rte_eal_unbind_kernel_driver in BSD as not supported.
2. That we keep this contained within Linux area:
linux_unbind_kernel_driver <-- pci_unbind_kernel_driver
^
`-- xxx_unbind_kernel_driver
bsd_unbind_kernel_driver <-- pci_unbind_kernel_driver
`-> this returns ENOTSUPP
@Jan: any additions/suggestions?
-
Shreyansh
^ permalink raw reply
* Re: [dpdk-stable] [PATCH v6 1/6] vhost: fix windows vm hang
From: Yuanhan Liu @ 2016-10-13 6:18 UTC (permalink / raw)
To: Zhihong Wang; +Cc: dev, maxime.coquelin, thomas.monjalon, stable
In-Reply-To: <1474336817-22683-2-git-send-email-zhihong.wang@intel.com>
On Mon, Sep 19, 2016 at 10:00:12PM -0400, Zhihong Wang wrote:
> This patch fixes a Windows VM compatibility issue in DPDK 16.07 vhost code
> which causes the guest to hang once any packets are enqueued when mrg_rxbuf
> is turned on by setting the right id and len in the used ring.
>
> As defined in virtio spec 0.95 and 1.0, in each used ring element, id means
> index of start of used descriptor chain, and len means total length of the
> descriptor chain which was written to. While in 16.07 code, index of the
> last descriptor is assigned to id, and the length of the last descriptor is
> assigned to len.
>
> How to test?
>
> 1. Start testpmd in the host with a vhost port.
>
> 2. Start a Windows VM image with qemu and connect to the vhost port.
>
> 3. Start io forwarding with tx_first in host testpmd.
>
> For 16.07 code, the Windows VM will hang once any packets are enqueued.
>
> Cc: <stable@dpdk.org>
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
Applied to dpdk-next-virtio (this patch only).
Thanks.
--yliu
^ permalink raw reply
* [PATCH 2/2] net/i40e: fix VF bonded device link down
From: Qiming Yang @ 2016-10-13 6:07 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang
In-Reply-To: <1476338840-50003-1-git-send-email-qiming.yang@intel.com>
Originally, using DPDK as host driver, when VF bonded device
uses I40E_VIRTCHNL_OP_GET_LINK_STAT to query PF the link status,
VF will wait for the interrupt from PF to get this link status.
PF uses interrupt which already exists to communication with VF.
These two kinds of interrupt will be confusing and fail to converge.
This patch uses PF to notify link status instead of VF query.
Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 22 ++++++++++-
drivers/net/i40e/i40e_ethdev.h | 4 +-
drivers/net/i40e/i40e_ethdev_vf.c | 81 +++++++++++++--------------------------
drivers/net/i40e/i40e_pf.c | 33 ++++++++--------
drivers/net/i40e/i40e_pf.h | 3 +-
5 files changed, 67 insertions(+), 76 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 9c1f542..13060db 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -5418,6 +5418,24 @@ i40e_dev_handle_vfr_event(struct rte_eth_dev *dev)
}
static void
+i40e_notify_all_vfs_link_status(struct rte_eth_dev *dev)
+{
+ struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+ struct i40e_virtchnl_pf_event event;
+ int i;
+
+ event.event = I40E_VIRTCHNL_EVENT_LINK_CHANGE;
+ event.event_data.link_event.link_status =
+ dev->data->dev_link.link_status;
+ event.event_data.link_event.link_speed =
+ dev->data->dev_link.link_speed;
+
+ for (i = 0; i < pf->vf_num; i++)
+ i40e_pf_host_send_msg_to_vf(&pf->vfs[i], I40E_VIRTCHNL_OP_EVENT,
+ I40E_SUCCESS, (uint8_t *)&event, sizeof(event));
+}
+
+static void
i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
{
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -5455,9 +5473,11 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
break;
case i40e_aqc_opc_get_link_status:
ret = i40e_dev_link_update(dev, 0);
- if (!ret)
+ if (!ret) {
+ i40e_notify_all_vfs_link_status(dev);
_rte_eth_dev_callback_process(dev,
RTE_ETH_EVENT_INTR_LSC);
+ }
break;
default:
PMD_DRV_LOG(ERR, "Request %u is not supported yet",
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 92c8fad..61dfa93 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -599,7 +599,9 @@ int i40e_hash_filter_inset_select(struct i40e_hw *hw,
struct rte_eth_input_set_conf *conf);
int i40e_fdir_filter_inset_select(struct i40e_pf *pf,
struct rte_eth_input_set_conf *conf);
-
+int i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf, uint32_t opcode,
+ uint32_t retval, uint8_t *msg,
+ uint16_t msglen);
void i40e_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
struct rte_eth_rxq_info *qinfo);
void i40e_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index a616ae0..ba63a7f 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -126,8 +126,6 @@ static void i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev);
static void i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev);
static void i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev);
static void i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev);
-static int i40evf_get_link_status(struct rte_eth_dev *dev,
- struct rte_eth_link *link);
static int i40evf_init_vlan(struct rte_eth_dev *dev);
static int i40evf_dev_rx_queue_start(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
@@ -1084,31 +1082,6 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
return err;
}
-static int
-i40evf_get_link_status(struct rte_eth_dev *dev, struct rte_eth_link *link)
-{
- struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
- int err;
- struct vf_cmd_info args;
- struct rte_eth_link *new_link;
-
- args.ops = (enum i40e_virtchnl_ops)I40E_VIRTCHNL_OP_GET_LINK_STAT;
- args.in_args = NULL;
- args.in_args_size = 0;
- args.out_buffer = vf->aq_resp;
- args.out_size = I40E_AQ_BUF_SZ;
- err = i40evf_execute_vf_cmd(dev, &args);
- if (err) {
- PMD_DRV_LOG(ERR, "fail to execute command OP_GET_LINK_STAT");
- return err;
- }
-
- new_link = (struct rte_eth_link *)args.out_buffer;
- (void)rte_memcpy(link, new_link, sizeof(*link));
-
- return 0;
-}
-
static const struct rte_pci_id pci_id_i40evf_map[] = {
{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_VF) },
{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_VF_HV) },
@@ -2166,35 +2139,33 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
* DPDK pf host provide interfacet to acquire link status
* while Linux driver does not
*/
- if (vf->version_major == I40E_DPDK_VERSION_MAJOR)
- i40evf_get_link_status(dev, &new_link);
- else {
- /* Linux driver PF host */
- switch (vf->link_speed) {
- case I40E_LINK_SPEED_100MB:
- new_link.link_speed = ETH_SPEED_NUM_100M;
- break;
- case I40E_LINK_SPEED_1GB:
- new_link.link_speed = ETH_SPEED_NUM_1G;
- break;
- case I40E_LINK_SPEED_10GB:
- new_link.link_speed = ETH_SPEED_NUM_10G;
- break;
- case I40E_LINK_SPEED_20GB:
- new_link.link_speed = ETH_SPEED_NUM_20G;
- break;
- case I40E_LINK_SPEED_40GB:
- new_link.link_speed = ETH_SPEED_NUM_40G;
- break;
- default:
- new_link.link_speed = ETH_SPEED_NUM_100M;
- break;
- }
- /* full duplex only */
- new_link.link_duplex = ETH_LINK_FULL_DUPLEX;
- new_link.link_status = vf->link_up ? ETH_LINK_UP :
- ETH_LINK_DOWN;
+
+ /* Linux driver PF host */
+ switch (vf->link_speed) {
+ case I40E_LINK_SPEED_100MB:
+ new_link.link_speed = ETH_SPEED_NUM_100M;
+ break;
+ case I40E_LINK_SPEED_1GB:
+ new_link.link_speed = ETH_SPEED_NUM_1G;
+ break;
+ case I40E_LINK_SPEED_10GB:
+ new_link.link_speed = ETH_SPEED_NUM_10G;
+ break;
+ case I40E_LINK_SPEED_20GB:
+ new_link.link_speed = ETH_SPEED_NUM_20G;
+ break;
+ case I40E_LINK_SPEED_40GB:
+ new_link.link_speed = ETH_SPEED_NUM_40G;
+ break;
+ default:
+ new_link.link_speed = ETH_SPEED_NUM_100M;
+ break;
}
+ /* full duplex only */
+ new_link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ new_link.link_status = vf->link_up ? ETH_LINK_UP :
+ ETH_LINK_DOWN;
+
i40evf_dev_atomic_write_link_status(dev, &new_link);
return 0;
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index d5b2d45..350f6a0 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -250,7 +250,7 @@ i40e_pf_host_vf_reset(struct i40e_pf_vf *vf, bool do_hw_reset)
return ret;
}
-static int
+int
i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf,
uint32_t opcode,
uint32_t retval,
@@ -847,18 +847,6 @@ i40e_pf_host_process_cmd_get_stats(struct i40e_pf_vf *vf)
return I40E_SUCCESS;
}
-static void
-i40e_pf_host_process_cmd_get_link_status(struct i40e_pf_vf *vf)
-{
- struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(vf->pf->main_vsi);
-
- /* Update link status first to acquire latest link change */
- i40e_dev_link_update(dev, 1);
- i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_GET_LINK_STAT,
- I40E_SUCCESS, (uint8_t *)&dev->data->dev_link,
- sizeof(struct rte_eth_link));
-}
-
static int
i40e_pf_host_process_cmd_cfg_vlan_offload(
struct i40e_pf_vf *vf,
@@ -909,6 +897,20 @@ send_msg:
return ret;
}
+static void
+i40e_notify_vf_link_status(struct rte_eth_dev *dev, struct i40e_pf_vf *vf)
+{
+ struct i40e_virtchnl_pf_event event;
+
+ event.event = I40E_VIRTCHNL_EVENT_LINK_CHANGE;
+ event.event_data.link_event.link_status =
+ dev->data->dev_link.link_status;
+ event.event_data.link_event.link_speed =
+ dev->data->dev_link.link_speed;
+ i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_EVENT,
+ I40E_SUCCESS, (uint8_t *)&event, sizeof(event));
+}
+
void
i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
uint16_t abs_vf_id, uint32_t opcode,
@@ -964,6 +966,7 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
case I40E_VIRTCHNL_OP_ENABLE_QUEUES:
PMD_DRV_LOG(INFO, "OP_ENABLE_QUEUES received");
i40e_pf_host_process_cmd_enable_queues(vf, msg, msglen);
+ i40e_notify_vf_link_status(dev, vf);
break;
case I40E_VIRTCHNL_OP_DISABLE_QUEUES:
PMD_DRV_LOG(INFO, "OP_DISABLE_QUEUE received");
@@ -993,10 +996,6 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
PMD_DRV_LOG(INFO, "OP_GET_STATS received");
i40e_pf_host_process_cmd_get_stats(vf);
break;
- case I40E_VIRTCHNL_OP_GET_LINK_STAT:
- PMD_DRV_LOG(INFO, "OP_GET_LINK_STAT received");
- i40e_pf_host_process_cmd_get_link_status(vf);
- break;
case I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD:
PMD_DRV_LOG(INFO, "OP_CFG_VLAN_OFFLOAD received");
i40e_pf_host_process_cmd_cfg_vlan_offload(vf, msg, msglen);
diff --git a/drivers/net/i40e/i40e_pf.h b/drivers/net/i40e/i40e_pf.h
index 9c01829..cddc45c 100644
--- a/drivers/net/i40e/i40e_pf.h
+++ b/drivers/net/i40e/i40e_pf.h
@@ -59,9 +59,8 @@ enum i40e_virtchnl_ops_dpdk {
* Keep some gap between Linux PF commands and
* DPDK PF extended commands.
*/
- I40E_VIRTCHNL_OP_GET_LINK_STAT = I40E_VIRTCHNL_OP_VERSION +
+ I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD = I40E_VIRTCHNL_OP_VERSION +
I40E_DPDK_OFFSET,
- I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD,
I40E_VIRTCHNL_OP_CFG_VLAN_PVID,
I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EXT,
};
--
2.5.5
^ permalink raw reply related
* [PATCH 1/2] net/i40e: fix link status change interrupt
From: Qiming Yang @ 2016-10-13 6:07 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang
Previously, link status interrupt in i40e is achieved by checking
LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
for diagnostic use. Instead, drivers need to get the link status
change notification by using LSE (Link Status Event).
This patch enables LSE and calls LSC callback when the event is
received. This patch also removes the processing on
LINK_STAT_CHANGE_MASK.
Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 97 +++++++++---------------------------------
1 file changed, 19 insertions(+), 78 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d0aeb70..9c1f542 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -108,7 +108,6 @@
I40E_PFINT_ICR0_ENA_GRST_MASK | \
I40E_PFINT_ICR0_ENA_PCI_EXCEPTION_MASK | \
I40E_PFINT_ICR0_ENA_STORM_DETECT_MASK | \
- I40E_PFINT_ICR0_ENA_LINK_STAT_CHANGE_MASK | \
I40E_PFINT_ICR0_ENA_HMC_ERR_MASK | \
I40E_PFINT_ICR0_ENA_PE_CRITERR_MASK | \
I40E_PFINT_ICR0_ENA_VFLR_MASK | \
@@ -1768,6 +1767,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
if (dev->data->dev_conf.intr_conf.lsc != 0)
PMD_INIT_LOG(INFO, "lsc won't enable because of"
" no intr multiplex\n");
+ } else if (dev->data->dev_conf.intr_conf.lsc != 0) {
+ ret = i40e_aq_set_phy_int_mask(hw,
+ ~(I40E_AQ_EVENT_LINK_UPDOWN |
+ I40E_AQ_EVENT_MODULE_QUAL_FAIL |
+ I40E_AQ_EVENT_MEDIA_NA), NULL);
+ if (ret != I40E_SUCCESS)
+ PMD_DRV_LOG(WARNING, "Fail to set phy mask");
+
+ /* Call get_link_info aq commond to enable LSE */
+ i40e_dev_link_update(dev, 0);
}
/* enable uio intr after callback register */
@@ -1984,6 +1993,7 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
struct rte_eth_link link, old;
int status;
unsigned rep_cnt = MAX_REPEAT_TIME;
+ bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
memset(&link, 0, sizeof(link));
memset(&old, 0, sizeof(old));
@@ -1992,7 +2002,8 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
do {
/* Get link status information from hardware */
- status = i40e_aq_get_link_info(hw, false, &link_status, NULL);
+ status = i40e_aq_get_link_info(hw, enable_lse,
+ &link_status, NULL);
if (status != I40E_SUCCESS) {
link.link_speed = ETH_SPEED_NUM_100M;
link.link_duplex = ETH_LINK_FULL_DUPLEX;
@@ -5442,6 +5453,12 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
info.msg_buf,
info.msg_len);
break;
+ case i40e_aqc_opc_get_link_status:
+ ret = i40e_dev_link_update(dev, 0);
+ if (!ret)
+ _rte_eth_dev_callback_process(dev,
+ RTE_ETH_EVENT_INTR_LSC);
+ break;
default:
PMD_DRV_LOG(ERR, "Request %u is not supported yet",
opcode);
@@ -5451,57 +5468,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
rte_free(info.msg_buf);
}
-/*
- * Interrupt handler is registered as the alarm callback for handling LSC
- * interrupt in a definite of time, in order to wait the NIC into a stable
- * state. Currently it waits 1 sec in i40e for the link up interrupt, and
- * no need for link down interrupt.
- */
-static void
-i40e_dev_interrupt_delayed_handler(void *param)
-{
- struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- uint32_t icr0;
-
- /* read interrupt causes again */
- icr0 = I40E_READ_REG(hw, I40E_PFINT_ICR0);
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
- if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
- PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error\n");
- if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
- PMD_DRV_LOG(ERR, "ICR0: malicious programming detected\n");
- if (icr0 & I40E_PFINT_ICR0_GRST_MASK)
- PMD_DRV_LOG(INFO, "ICR0: global reset requested\n");
- if (icr0 & I40E_PFINT_ICR0_PCI_EXCEPTION_MASK)
- PMD_DRV_LOG(INFO, "ICR0: PCI exception\n activated\n");
- if (icr0 & I40E_PFINT_ICR0_STORM_DETECT_MASK)
- PMD_DRV_LOG(INFO, "ICR0: a change in the storm control "
- "state\n");
- if (icr0 & I40E_PFINT_ICR0_HMC_ERR_MASK)
- PMD_DRV_LOG(ERR, "ICR0: HMC error\n");
- if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
- PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error\n");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
-
- if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
- PMD_DRV_LOG(INFO, "INT:VF reset detected\n");
- i40e_dev_handle_vfr_event(dev);
- }
- if (icr0 & I40E_PFINT_ICR0_ADMINQ_MASK) {
- PMD_DRV_LOG(INFO, "INT:ADMINQ event\n");
- i40e_dev_handle_aq_msg(dev);
- }
-
- /* handle the link up interrupt in an alarm callback */
- i40e_dev_link_update(dev, 0);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
-
- i40e_pf_enable_irq0(hw);
- rte_intr_enable(&(dev->pci_dev->intr_handle));
-}
-
/**
* Interrupt handler triggered by NIC for handling
* specific interrupt.
@@ -5558,31 +5524,6 @@ i40e_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
PMD_DRV_LOG(INFO, "ICR0: adminq event");
i40e_dev_handle_aq_msg(dev);
}
-
- /* Link Status Change interrupt */
- if (icr0 & I40E_PFINT_ICR0_LINK_STAT_CHANGE_MASK) {
-#define I40E_US_PER_SECOND 1000000
- struct rte_eth_link link;
-
- PMD_DRV_LOG(INFO, "ICR0: link status changed\n");
- memset(&link, 0, sizeof(link));
- rte_i40e_dev_atomic_read_link_status(dev, &link);
- i40e_dev_link_update(dev, 0);
-
- /*
- * For link up interrupt, it needs to wait 1 second to let the
- * hardware be a stable state. Otherwise several consecutive
- * interrupts can be observed.
- * For link down interrupt, no need to wait.
- */
- if (!link.link_status && rte_eal_alarm_set(I40E_US_PER_SECOND,
- i40e_dev_interrupt_delayed_handler, (void *)dev) >= 0)
- return;
- else
- _rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_LSC);
- }
-
done:
/* Enable interrupt */
i40e_pf_enable_irq0(hw);
--
2.5.5
^ permalink raw reply related
* Re: [dpdk-users] Project Governance and Linux Foundation
From: Muhammad Zain-ul-Abideen @ 2016-10-13 6:10 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: qin.chunhua, dev, users
In-Reply-To: <3947933.oyJplKdvEH@xps13>
I agree with Mr. Thomas Monjalon. Any optimization in DPDK structure should
be made open.
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Wang, Zhihong @ 2016-10-13 6:02 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Jianbo Liu, Thomas Monjalon, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <20161013053324.GP16751@yliu-dev.sh.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Thursday, October 13, 2016 1:33 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>
> Cc: Jianbo Liu <jianbo.liu@linaro.org>; Thomas Monjalon
> <thomas.monjalon@6wind.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] vhost: optimize enqueue
>
> On Wed, Oct 12, 2016 at 12:22:08PM +0000, Wang, Zhihong wrote:
> > > > >> > 3. How many percentage of drop are you seeing?
> > > > The testing result:
> > > > size (bytes) improvement (%)
> > > > 64 3.92
> > > > 128 11.51
> > > > 256 24.16
> > > > 512 -13.79
> > > > 1024 -22.51
> > > > 1500 -12.22
> > > > A correction is that performance is dropping if byte size is larger than
> 512.
> > >
> > > I have thought of this twice. Unfortunately, I think I may need NACK this
> > > series.
> > >
> > > Merging two code path into one is really good: as you stated, it improves
> > > the maintainability. But only if we see no performance regression on both
> > > path after the refactor. Unfortunately, that's not the case here: it hurts
> > > the performance for one code path (non-mrg Rx).
> > >
> > > That makes me think we may should not do the code path merge at all. I
> think
> > > that also aligns with what you have said before (internally): we could do
> the
> > > merge if it gives comparable performance before and after that.
> > >
> > > Besides that, I don't quite like the way you did in patch 2 (rewrite
> enqueue):
> > > you made a lot of changes in one patch. That means if something wrong
> > > happened,
> > > it is hard to narrow down which change introduces that regression. Badly,
> > > that's exactly what we met here. Weeks have been passed, I see no
> progress.
> > >
> > > That's the reason we like the idea of "one patch only does one thing, an
> > > atomic thing".
> >
> >
> > Yuanhan, folks,
> >
> > Thanks for the analysis. I disagree here though.
> >
> > I analyze, develop, benchmark on x86 platforms, where this patch
> > works great.
>
> Yes, that's great effort! With your hardwork, we know what the bottleneck
> is and how it could be improved.
>
> However, you don't have to do code refactor (merge two code path to one)
> to apply those improvements. From what I know, in this patchset, there
> are two factors could improve the performance:
>
> - copy hdr together with packet data
>
> - shadow used ring update and update at once
>
> The overall performance boost I got with your v6 patchset with mrg-Rx
> code path is about 27% (in PVP case). And I have just applied the 1st
> optimization, it yields about 20% boosts. The left could be covered if
> we apply the 2nd optimization (I guess).
>
> That would be a clean way to optimize vhost mergeable Rx path:
>
> - you don't touch non-mrg Rx path (well, you may could apply the
> shadow_used_ring trick to it as wel)
>
> This would at least make sure we will have no such performance
> regression issue reported by ARM guys.
>
> - you don't refactor the code
>
> The rewrite from scratch could introduce other issues, besides the
> performance regression. We may just don't know it yet.
>
>
> Make sense to you? If you agree, I think we could still make it in
> this release: they would be some small changes after all. For example,
> below is the patch applies the 1st optimization tip on top of
> dpdk-next-virtio
Thanks for this great idea. I think it's a better way to do it.
I'll start to make the patch then.
>
> --yliu
>
> ---------------------------------------------------------------
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 8a151af..0ddb5af 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -379,7 +379,7 @@ copy_mbuf_to_desc_mergeable(struct virtio_net
> *dev, struct vhost_virtqueue *vq,
> uint16_t end_idx, struct rte_mbuf *m,
> struct buf_vector *buf_vec)
> {
> - struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, 0};
> + struct virtio_net_hdr_mrg_rxbuf *virtio_hdr;
> uint32_t vec_idx = 0;
> uint16_t start_idx = vq->last_used_idx;
> uint16_t cur_idx = start_idx;
> @@ -388,6 +388,8 @@ copy_mbuf_to_desc_mergeable(struct virtio_net
> *dev, struct vhost_virtqueue *vq,
> uint32_t desc_offset, desc_avail;
> uint32_t cpy_len;
> uint16_t desc_idx, used_idx;
> + uint16_t num_buffers = end_idx - start_idx;
> + int hdr_copied = 0;
>
> if (unlikely(m == NULL))
> return 0;
> @@ -399,16 +401,11 @@ copy_mbuf_to_desc_mergeable(struct virtio_net
> *dev, struct vhost_virtqueue *vq,
> if (buf_vec[vec_idx].buf_len < dev->vhost_hlen || !desc_addr)
> return 0;
>
> - rte_prefetch0((void *)(uintptr_t)desc_addr);
> + virtio_hdr = (struct virtio_net_hdr_mrg_rxbuf
> *)(uintptr_t)desc_addr;
> + rte_prefetch0(virtio_hdr);
>
> - virtio_hdr.num_buffers = end_idx - start_idx;
> LOG_DEBUG(VHOST_DATA, "(%d) RX: num merge buffers %d\n",
> - dev->vid, virtio_hdr.num_buffers);
> -
> - virtio_enqueue_offload(m, &virtio_hdr.hdr);
> - copy_virtio_net_hdr(dev, desc_addr, virtio_hdr);
> - vhost_log_write(dev, buf_vec[vec_idx].buf_addr, dev->vhost_hlen);
> - PRINT_PACKET(dev, (uintptr_t)desc_addr, dev->vhost_hlen, 0);
> + dev->vid, num_buffers);
>
> desc_avail = buf_vec[vec_idx].buf_len - dev->vhost_hlen;
> desc_offset = dev->vhost_hlen;
> @@ -450,6 +447,15 @@ copy_mbuf_to_desc_mergeable(struct virtio_net
> *dev, struct vhost_virtqueue *vq,
> mbuf_avail = rte_pktmbuf_data_len(m);
> }
>
> + if (hdr_copied == 0) {
> + virtio_hdr->num_buffers = num_buffers;
> + virtio_enqueue_offload(m, &virtio_hdr->hdr);
> + vhost_log_write(dev, buf_vec[vec_idx].buf_addr,
> dev->vhost_hlen);
> + PRINT_PACKET(dev, (uintptr_t)desc_addr, dev-
> >vhost_hlen, 0);
> +
> + hdr_copied = 1;
> + }
> +
> cpy_len = RTE_MIN(desc_avail, mbuf_avail);
> rte_memcpy((void *)((uintptr_t)(desc_addr + desc_offset)),
> rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Yuanhan Liu @ 2016-10-13 5:35 UTC (permalink / raw)
To: Wang, Zhihong; +Cc: Jianbo Liu, Thomas Monjalon, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <20161013053324.GP16751@yliu-dev.sh.intel.com>
On Thu, Oct 13, 2016 at 01:33:24PM +0800, Yuanhan Liu wrote:
> That would be a clean way to optimize vhost mergeable Rx path:
>
> - you don't touch non-mrg Rx path (well, you may could apply the
> shadow_used_ring trick to it as wel)
I meant "to non-mrg Rx as well".
--yliu
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Yuanhan Liu @ 2016-10-13 5:33 UTC (permalink / raw)
To: Wang, Zhihong; +Cc: Jianbo Liu, Thomas Monjalon, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <8F6C2BD409508844A0EFC19955BE09414E7CD78E@SHSMSX103.ccr.corp.intel.com>
On Wed, Oct 12, 2016 at 12:22:08PM +0000, Wang, Zhihong wrote:
> > > >> > 3. How many percentage of drop are you seeing?
> > > The testing result:
> > > size (bytes) improvement (%)
> > > 64 3.92
> > > 128 11.51
> > > 256 24.16
> > > 512 -13.79
> > > 1024 -22.51
> > > 1500 -12.22
> > > A correction is that performance is dropping if byte size is larger than 512.
> >
> > I have thought of this twice. Unfortunately, I think I may need NACK this
> > series.
> >
> > Merging two code path into one is really good: as you stated, it improves
> > the maintainability. But only if we see no performance regression on both
> > path after the refactor. Unfortunately, that's not the case here: it hurts
> > the performance for one code path (non-mrg Rx).
> >
> > That makes me think we may should not do the code path merge at all. I think
> > that also aligns with what you have said before (internally): we could do the
> > merge if it gives comparable performance before and after that.
> >
> > Besides that, I don't quite like the way you did in patch 2 (rewrite enqueue):
> > you made a lot of changes in one patch. That means if something wrong
> > happened,
> > it is hard to narrow down which change introduces that regression. Badly,
> > that's exactly what we met here. Weeks have been passed, I see no progress.
> >
> > That's the reason we like the idea of "one patch only does one thing, an
> > atomic thing".
>
>
> Yuanhan, folks,
>
> Thanks for the analysis. I disagree here though.
>
> I analyze, develop, benchmark on x86 platforms, where this patch
> works great.
Yes, that's great effort! With your hardwork, we know what the bottleneck
is and how it could be improved.
However, you don't have to do code refactor (merge two code path to one)
to apply those improvements. From what I know, in this patchset, there
are two factors could improve the performance:
- copy hdr together with packet data
- shadow used ring update and update at once
The overall performance boost I got with your v6 patchset with mrg-Rx
code path is about 27% (in PVP case). And I have just applied the 1st
optimization, it yields about 20% boosts. The left could be covered if
we apply the 2nd optimization (I guess).
That would be a clean way to optimize vhost mergeable Rx path:
- you don't touch non-mrg Rx path (well, you may could apply the
shadow_used_ring trick to it as wel)
This would at least make sure we will have no such performance
regression issue reported by ARM guys.
- you don't refactor the code
The rewrite from scratch could introduce other issues, besides the
performance regression. We may just don't know it yet.
Make sense to you? If you agree, I think we could still make it in
this release: they would be some small changes after all. For example,
below is the patch applies the 1st optimization tip on top of
dpdk-next-virtio
--yliu
---------------------------------------------------------------
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 8a151af..0ddb5af 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -379,7 +379,7 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint16_t end_idx, struct rte_mbuf *m,
struct buf_vector *buf_vec)
{
- struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, 0};
+ struct virtio_net_hdr_mrg_rxbuf *virtio_hdr;
uint32_t vec_idx = 0;
uint16_t start_idx = vq->last_used_idx;
uint16_t cur_idx = start_idx;
@@ -388,6 +388,8 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint32_t desc_offset, desc_avail;
uint32_t cpy_len;
uint16_t desc_idx, used_idx;
+ uint16_t num_buffers = end_idx - start_idx;
+ int hdr_copied = 0;
if (unlikely(m == NULL))
return 0;
@@ -399,16 +401,11 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq,
if (buf_vec[vec_idx].buf_len < dev->vhost_hlen || !desc_addr)
return 0;
- rte_prefetch0((void *)(uintptr_t)desc_addr);
+ virtio_hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc_addr;
+ rte_prefetch0(virtio_hdr);
- virtio_hdr.num_buffers = end_idx - start_idx;
LOG_DEBUG(VHOST_DATA, "(%d) RX: num merge buffers %d\n",
- dev->vid, virtio_hdr.num_buffers);
-
- virtio_enqueue_offload(m, &virtio_hdr.hdr);
- copy_virtio_net_hdr(dev, desc_addr, virtio_hdr);
- vhost_log_write(dev, buf_vec[vec_idx].buf_addr, dev->vhost_hlen);
- PRINT_PACKET(dev, (uintptr_t)desc_addr, dev->vhost_hlen, 0);
+ dev->vid, num_buffers);
desc_avail = buf_vec[vec_idx].buf_len - dev->vhost_hlen;
desc_offset = dev->vhost_hlen;
@@ -450,6 +447,15 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq,
mbuf_avail = rte_pktmbuf_data_len(m);
}
+ if (hdr_copied == 0) {
+ virtio_hdr->num_buffers = num_buffers;
+ virtio_enqueue_offload(m, &virtio_hdr->hdr);
+ vhost_log_write(dev, buf_vec[vec_idx].buf_addr, dev->vhost_hlen);
+ PRINT_PACKET(dev, (uintptr_t)desc_addr, dev->vhost_hlen, 0);
+
+ hdr_copied = 1;
+ }
+
cpy_len = RTE_MIN(desc_avail, mbuf_avail);
rte_memcpy((void *)((uintptr_t)(desc_addr + desc_offset)),
rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
^ permalink raw reply related
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Jianbo Liu @ 2016-10-13 3:51 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Wang, Zhihong, Yuanhan Liu, Maxime Coquelin, dev
In-Reply-To: <1707408.heiIBWjog8@xps13>
Hi Thomas,
On 12 October 2016 at 23:31, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> Sorry guys, you lost me in the discussion.
>
> Is there some regression only on ARM?
> Does it need some work specifically on memcpy for ARM,
I don't know if there is common way to improve memcpy on different ARM
hardware. Even there is, it could take times.
I have tried do that using neon (like sse) instructions, but without success.
> or vhost for ARM?
> Who can work on ARM optimization?
>
^ permalink raw reply
* Re: 17.02 Roadmap
From: Tan, Jianfeng @ 2016-10-13 3:15 UTC (permalink / raw)
To: Thomas Monjalon, O'Driscoll, Tim; +Cc: dev, stephen
In-Reply-To: <1998191.9HGrB6oKr3@xps13>
Hi Thomas,
On 10/11/2016 4:42 AM, Thomas Monjalon wrote:
> Thanks Tim for the interesting inputs.
> Some of them may require a dedicated thread to continue the discussion
> based on some preliminary specifications or drafts.
[...]
>> Interrupt Mode Support in Virtio PMD: Support for interrupt mode will be added to the virtio PMD.
> I guess you mean Rx interrupt in virtio PMD to avoid 100% polling in the VM?
Yes, it is. Previously, Stephen Hemminger has worked out a version which
requires MSI support in uio driver, which is not upstreamed. And as
Stephen said here
(http://dpdk.org/ml/archives/dev/2015-December/030913.html), we will go
with VFIO with no-IOMMU. And this dependence has been addressed in Linux
4.8,
033291eccbd ("vfio: Include No-IOMMU mode").
Hi Stephen,
Sorry that I did not sync up with you about this topic before it's sent
out. Have you already started the work on this topic? Or I'll work on
this and ask your review?
>
>> Virtio-User as an Alternative Exception Path: Investigate the use of virtio-user and vhost-net as an alternative exception path to KNI that does not require out of tree drivers. This work is still at an experimental stage, so it may not be included in 17.02.
> Interesting. Please share more details of the design you are thinking of.
>
Currently, virtio-user just has the vhost-user backend (and the
vhost-kernel path has been dropped before merge). The path of
vhost-kernel has been dropped for bad performance comparing to
vhost-user. And we want keep the code as simple as possible.
But after a second thought, virtio_user + vhost-kernel is a good
candidate for exceptional path.
1. maintenance: vhost-net (kernel) is upstreamed and extensively used
kernel module. We don't need any out-of-tree module like KNI.
2. performance: this solution would have at least similar performance
with KNI.
3. feature: multi queue, tso, multi-seg mbuf, etc.
Any suggestions?
Thanks,
Jianfeng
^ permalink raw reply
* Re: [PATCH 1/5] i40e: extract non-x86 specific code from vector driver
From: Jianbo Liu @ 2016-10-13 3:00 UTC (permalink / raw)
To: Zhang, Qi Z
Cc: Zhang, Helin, Wu, Jingjing, jerin.jacob@caviumnetworks.com,
dev@dpdk.org
In-Reply-To: <039ED4275CED7440929022BC67E706115065A1BD@SHSMSX103.ccr.corp.intel.com>
On 12 October 2016 at 10:55, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> Hi Jianbo
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianbo Liu
>> Sent: Wednesday, August 24, 2016 5:54 PM
>> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
>> <jingjing.wu@intel.com>; jerin.jacob@caviumnetworks.com; dev@dpdk.org
>> Cc: Jianbo Liu <jianbo.liu@linaro.org>
>> Subject: [dpdk-dev] [PATCH 1/5] i40e: extract non-x86 specific code from vector
>> driver
>>
>> move scalar code which does not use x86 intrinsic functions to new file
>> "i40e_rxtx_vec_common.h", while keeping x86 code in i40e_rxtx_vec.c.
>> This allows the scalar code to to be shared among vector drivers for different
>> platforms.
>>
>> Signed-off-by: Jianbo Liu <jianbo.liu@linaro.org>
>> ---
...
>
> Should we rename the function "_40e_rx_queue_release_mbufs_vec" to
> "i40e_rx_queue_release_mbufs_vec_default", so functions be wrapped can follow a consistent rule?
I think these two ways are different.
For func/_func, _func implements what func needs to do, they are same.
We needs _func inline, to be called in different ARCHs.
But for func/func_default, func_default is the default behavior, but
you can use or not-use it in func.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/2] vhost: enable any layout feature
From: Yang, Zhiyong @ 2016-10-13 2:52 UTC (permalink / raw)
To: Liu, Yuanhan
Cc: mst@redhat.com, thomas.monjalon@6wind.com, Wang, Zhihong,
stephen@networkplumber.org, dev@dpdk.org, qemu-devel@nongnu.org,
maxime.coquelin@redhat.com
In-Reply-To: <F5DF4F0E3AFEF648ADC1C3C33AD4DBF16C2409EB@SHSMSX101.ccr.corp.intel.com>
Hi, Yuanhan:
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yuanhan Liu
> Sent: Wednesday, October 12, 2016 11:22 AM
> To: Michael S. Tsirkin <mst@redhat.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Cc: Wang, Zhihong <zhihong.wang@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev@dpdk.org; qemu-
> devel@nongnu.org
> Subject: Re: [dpdk-dev] [Qemu-devel] [PATCH 1/2] vhost: enable any layout
> feature
>
> On Tue, Oct 11, 2016 at 02:57:49PM +0800, Yuanhan Liu wrote:
> > > > > > > There was an example: the vhost enqueue optmization patchset
> > > > > > > from Zhihong [0] uses memset, and it introduces more than
> > > > > > > 15% drop (IIRC)
>
> Though it doesn't matter now, but I have verified it yesterday (with and
> wihtout memset), the drop could be up to 30+%.
>
> This is to let you know that it could behaviour badly if memset is not inlined.
>
> > > > > > > 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:
> >
> > Yes, I saw memset is inlined when this diff is applied.
>
> I have another concern though: It's a trick could let gcc do the inline, I am not
> quite sure whether that's ture with other compilers (i.e. clang, icc, or even,
> older gcc).
>
> For this case, I think I still prefer some trick like
> *(struct ..*) = {0, }
>
> Or even, we may could introduce rte_memset(). IIRC, that has been
> proposed somehow before?
>
I'm trying to introduce rte_memset to have a prototype It have
Gotten some performance enhancement For small size, I'm optimize it further.
--Zhiyong
> --yliu
^ permalink raw reply
* Re: [PATCH 2/5] i40e: implement vector PMD for ARM architecture
From: Jianbo Liu @ 2016-10-13 2:19 UTC (permalink / raw)
To: Zhang, Qi Z
Cc: Zhang, Helin, Wu, Jingjing, jerin.jacob@caviumnetworks.com,
dev@dpdk.org
In-Reply-To: <039ED4275CED7440929022BC67E7061150659198@SHSMSX103.ccr.corp.intel.com>
On 12 October 2016 at 10:46, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> Hi Jianbo:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianbo Liu
>> Sent: Wednesday, August 24, 2016 5:54 PM
>> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
>> <jingjing.wu@intel.com>; jerin.jacob@caviumnetworks.com; dev@dpdk.org
>> Cc: Jianbo Liu <jianbo.liu@linaro.org>
>> Subject: [dpdk-dev] [PATCH 2/5] i40e: implement vector PMD for ARM
>> architecture
>>
>> Use ARM NEON intrinsic to implement i40e vPMD
>>
>> Signed-off-by: Jianbo Liu <jianbo.liu@linaro.org>
>> ---
>> drivers/net/i40e/Makefile | 4 +
>> drivers/net/i40e/i40e_rxtx_vec_neon.c | 581
>> ++++++++++++++++++++++++++++++++++
>> 2 files changed, 585 insertions(+)
>> create mode 100644 drivers/net/i40e/i40e_rxtx_vec_neon.c
......
>
> ptype and bad checksum offload is enabled with below patches
> http://dpdk.org/dev/patchwork/patch/16394
> http://dpdk.org/dev/patchwork/patch/16395
> You may take a look to see if it's necessary to enable them for ARM also.
>
Yes, I'll update in the next version. Thanks!
^ permalink raw reply
* Re: [PATCH v3 0/5] vhost: optimize enqueue
From: Wang, Zhihong @ 2016-10-13 1:21 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Yuanhan Liu, Jianbo Liu, Maxime Coquelin, dev@dpdk.org
In-Reply-To: <1707408.heiIBWjog8@xps13>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 12, 2016 11:31 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>
> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Jianbo Liu
> <jianbo.liu@linaro.org>; Maxime Coquelin <maxime.coquelin@redhat.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] vhost: optimize enqueue
>
> Sorry guys, you lost me in the discussion.
>
> Is there some regression only on ARM?
ARM is what we see, no info on ppc yet.
> Does it need some work specifically on memcpy for ARM,
> or vhost for ARM?
> Who can work on ARM optimization?
These are still open questions, Jiaobo who reported this doesn't
have time for more testing now according to the reply.
I'm trying to do some test in the hope to identify the root cause.
>
> More comments below.
>
> 2016-10-12 12:22, Wang, Zhihong:
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > > It's an ARM server.
> > > >
> > > > >> > 3. How many percentage of drop are you seeing?
> > > > The testing result:
> > > > size (bytes) improvement (%)
> > > > 64 3.92
> > > > 128 11.51
> > > > 256 24.16
> > > > 512 -13.79
> > > > 1024 -22.51
> > > > 1500 -12.22
> > > > A correction is that performance is dropping if byte size is larger than
> 512.
> > >
> > > I have thought of this twice. Unfortunately, I think I may need NACK this
> > > series.
> > >
> > > Merging two code path into one is really good: as you stated, it improves
> > > the maintainability. But only if we see no performance regression on both
> > > path after the refactor. Unfortunately, that's not the case here: it hurts
> > > the performance for one code path (non-mrg Rx).
>
> +1
>
> > > That makes me think we may should not do the code path merge at all. I
> think
> > > that also aligns with what you have said before (internally): we could do
> the
> > > merge if it gives comparable performance before and after that.
> > >
> > > Besides that, I don't quite like the way you did in patch 2 (rewrite
> enqueue):
> > > you made a lot of changes in one patch. That means if something wrong
> > > happened,
> > > it is hard to narrow down which change introduces that regression. Badly,
> > > that's exactly what we met here. Weeks have been passed, I see no
> progress.
>
> +1, it is important to have simple patches making changes step by step.
>
> > > That's the reason we like the idea of "one patch only does one thing, an
> > > atomic thing".
> >
> >
> > Yuanhan, folks,
> >
> > Thanks for the analysis. I disagree here though.
> >
> > I analyze, develop, benchmark on x86 platforms, where this patch
> > works great.
> >
> > I've been trying to analyze on ARM too but it takes time and I've
> > had a schedule. Also since the ARM perf issue comes when it's
> > v6 already, I might not be able to make it in time. However
> > that's what I have to do for this patch to be merged in this
> > or the next release.
> >
> > In the meantime, may I suggest we consider the possibility to
> > have dedicated codes for **perf critical paths** for different
> > kinds of architecture?
>
> Yes that's what we do in several parts of DPDK.
>
> > It can be hard for a person to have both the knowledge and the
> > development environment for multiple archs at the same time.
>
> Yes we do not expect you work on ARM.
> So if nobody work on the ARM issue, you could make 2 code paths
> in order to allow your optimization for x86 only.
> But that's not the preferred way.
> And you must split your rework to better identify which part is
> a regression on ARM.
>
> > Moreover, different optimization techniques might be required for
> > different archs, so it's hard and unnecessary to make a function
> > works for all archs, sometimes it's just not the right thing to do.
>
> Yes sometimes. Please help us to be convinced for this case.
>
> > > So I will apply the first patch (it's a bug fixing patch) and ask you to
> > > refactor the rest, without the code path merge.
> > >
> > > I think we could still have a good maintainability code base if we introduce
> > > more common helper functions that can be used on both Rx path, or
> even on
> > > Tx path (such as update_used_ring, or shadow_used_ring).
>
> Yes it is a good step.
> And the code path merge could be reconsidered later.
>
> > > It's a bit late for too many changes for v16.11. I think you could just
> > > grab patch 6 (vhost: optimize cache access) to the old mrg-Rx code path,
> > > if that also helps the performance? Let us handle the left in next release,
> > > such as shadow used ring.
>
> Thank you
^ permalink raw reply
* [PATCH] eal: avoid unnecessary conflicts over rte_config file
From: John Ousterhout @ 2016-10-12 21:29 UTC (permalink / raw)
To: thomas.monjalon; +Cc: dev, John Ousterhout
Before this patch, DPDK used the file ~/.rte_config as a lock to detect
potential interference between multiple DPDK applications running on the
same machine. However, if a single user ran DPDK applications concurrently
on several different machines, and if the user's home directory was shared
between the machines via NFS, DPDK would incorrectly detect conflicts
for all but the first application and abort them. This patch fixes the
problem by incorporating the machine name into the config file name (e.g.,
~/.rte_hostname_config).
Signed-off-by: John Ousterhout <ouster@cs.stanford.edu>
---
doc/guides/prog_guide/multi_proc_support.rst | 11 +++++++----
lib/librte_eal/common/eal_common_proc.c | 8 ++------
lib/librte_eal/common/eal_filesystem.h | 15 +++++++++++++--
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/doc/guides/prog_guide/multi_proc_support.rst b/doc/guides/prog_guide/multi_proc_support.rst
index badd102..a54fa1c 100644
--- a/doc/guides/prog_guide/multi_proc_support.rst
+++ b/doc/guides/prog_guide/multi_proc_support.rst
@@ -129,10 +129,13 @@ Support for this usage scenario is provided using the ``--file-prefix`` paramete
By default, the EAL creates hugepage files on each hugetlbfs filesystem using the rtemap_X filename,
where X is in the range 0 to the maximum number of hugepages -1.
-Similarly, it creates shared configuration files, memory mapped in each process, using the /var/run/.rte_config filename,
-when run as root (or $HOME/.rte_config when run as a non-root user;
-if filesystem and device permissions are set up to allow this).
-The rte part of the filenames of each of the above is configurable using the file-prefix parameter.
+Similarly, it creates shared configuration files, memory mapped in each process.
+When run as root, the name of the configuration file will be
+/var/run/.rte_*host*_config, where *host* is the name of the machine.
+When run as a non-root user, the the name of the configuration file will be
+$HOME/.rte_*host*_config (if filesystem and device permissions are set up to allow this).
+If the ``--file-prefix`` parameter has been specified, its value will be used
+in place of "rte" in the file names.
In addition to specifying the file-prefix parameter,
any DPDK applications that are to be run side-by-side must explicitly limit their memory use.
diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 12e0fca..517aa0c 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -45,12 +45,8 @@ rte_eal_primary_proc_alive(const char *config_file_path)
if (config_file_path)
config_fd = open(config_file_path, O_RDONLY);
- else {
- char default_path[PATH_MAX+1];
- snprintf(default_path, PATH_MAX, RUNTIME_CONFIG_FMT,
- default_config_dir, "rte");
- config_fd = open(default_path, O_RDONLY);
- }
+ else
+ config_fd = open(eal_runtime_config_path(), O_RDONLY);
if (config_fd < 0)
return 0;
diff --git a/lib/librte_eal/common/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h
index fdb4a70..4929aa3 100644
--- a/lib/librte_eal/common/eal_filesystem.h
+++ b/lib/librte_eal/common/eal_filesystem.h
@@ -41,7 +41,7 @@
#define EAL_FILESYSTEM_H
/** Path of rte config file. */
-#define RUNTIME_CONFIG_FMT "%s/.%s_config"
+#define RUNTIME_CONFIG_FMT "%s/.%s_%s_config"
#include <stdint.h>
#include <limits.h>
@@ -59,11 +59,22 @@ eal_runtime_config_path(void)
static char buffer[PATH_MAX]; /* static so auto-zeroed */
const char *directory = default_config_dir;
const char *home_dir = getenv("HOME");
+ static char nameBuffer[1000];
+ int result;
if (getuid() != 0 && home_dir != NULL)
directory = home_dir;
+
+ /*
+ * Include the name of the host in the config file path. Otherwise,
+ * if DPDK applications run on different hosts but share a home
+ * directory (e.g. via NFS), they will choose the same config
+ * file and conflict unnecessarily.
+ */
+ result = gethostname(nameBuffer, sizeof(nameBuffer)-1);
snprintf(buffer, sizeof(buffer) - 1, RUNTIME_CONFIG_FMT, directory,
- internal_config.hugefile_prefix);
+ internal_config.hugefile_prefix,
+ (result == 0) ? nameBuffer : "unknown-host");
return buffer;
}
--
2.8.3
^ permalink raw reply related
* [PATCH v2] app/testpmd: add support to configure 25G and 50G speeds
From: Ajit Khaparde @ 2016-10-12 21:26 UTC (permalink / raw)
To: dev; +Cc: Pablo de Lara
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA07C47@IRSMSX108.ger.corp.intel.com>
Support to configure 25G and 50G speeds is missing from testpmd.
This patch also updates the testpmd user guide accordingly.
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
--
v2: shorten lines > 80 character limit
---
app/test-pmd/cmdline.c | 16 +++++++++++-----
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a1da8b8..b8c7b8b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -545,7 +545,7 @@ static void cmd_help_long_parsed(void *parsed_result,
" Detach physical or virtual dev by port_id\n\n"
"port config (port_id|all)"
- " speed (10|100|1000|10000|40000|100000|auto)"
+ " speed (10|100|1000|10000|25000|40000|50000|100000|auto)"
" duplex (half|full|auto)\n"
" Set speed and duplex for all ports or port_id\n\n"
@@ -1016,8 +1016,12 @@ parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t *speed)
*speed = ETH_LINK_SPEED_1G;
} else if (!strcmp(speedstr, "10000")) {
*speed = ETH_LINK_SPEED_10G;
+ } else if (!strcmp(speedstr, "25000")) {
+ *speed = ETH_LINK_SPEED_25G;
} else if (!strcmp(speedstr, "40000")) {
*speed = ETH_LINK_SPEED_40G;
+ } else if (!strcmp(speedstr, "50000")) {
+ *speed = ETH_LINK_SPEED_50G;
} else if (!strcmp(speedstr, "100000")) {
*speed = ETH_LINK_SPEED_100G;
} else if (!strcmp(speedstr, "auto")) {
@@ -1067,7 +1071,7 @@ cmdline_parse_token_string_t cmd_config_speed_all_item1 =
TOKEN_STRING_INITIALIZER(struct cmd_config_speed_all, item1, "speed");
cmdline_parse_token_string_t cmd_config_speed_all_value1 =
TOKEN_STRING_INITIALIZER(struct cmd_config_speed_all, value1,
- "10#100#1000#10000#40000#100000#auto");
+ "10#100#1000#10000#25000#40000#50000#100000#auto");
cmdline_parse_token_string_t cmd_config_speed_all_item2 =
TOKEN_STRING_INITIALIZER(struct cmd_config_speed_all, item2, "duplex");
cmdline_parse_token_string_t cmd_config_speed_all_value2 =
@@ -1077,7 +1081,8 @@ cmdline_parse_token_string_t cmd_config_speed_all_value2 =
cmdline_parse_inst_t cmd_config_speed_all = {
.f = cmd_config_speed_all_parsed,
.data = NULL,
- .help_str = "port config all speed 10|100|1000|10000|40000|100000|auto duplex "
+ .help_str = "port config all speed "
+ "10|100|1000|10000|25000|40000|50000|100000|auto duplex "
"half|full|auto",
.tokens = {
(void *)&cmd_config_speed_all_port,
@@ -1141,7 +1146,7 @@ cmdline_parse_token_string_t cmd_config_speed_specific_item1 =
"speed");
cmdline_parse_token_string_t cmd_config_speed_specific_value1 =
TOKEN_STRING_INITIALIZER(struct cmd_config_speed_specific, value1,
- "10#100#1000#10000#40000#100000#auto");
+ "10#100#1000#10000#25000#40000#50000#100000#auto");
cmdline_parse_token_string_t cmd_config_speed_specific_item2 =
TOKEN_STRING_INITIALIZER(struct cmd_config_speed_specific, item2,
"duplex");
@@ -1152,7 +1157,8 @@ cmdline_parse_token_string_t cmd_config_speed_specific_value2 =
cmdline_parse_inst_t cmd_config_speed_specific = {
.f = cmd_config_speed_specific_parsed,
.data = NULL,
- .help_str = "port config X speed 10|100|1000|10000|40000|100000|auto duplex "
+ .help_str = "port config X speed "
+ "10|100|1000|10000|25000|40000|50000|100000|auto duplex "
"half|full|auto",
.tokens = {
(void *)&cmd_config_speed_specific_port,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f87e0c2..e1eea34 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1167,7 +1167,7 @@ port config - speed
Set the speed and duplex mode for all ports or a specific port::
- testpmd> port config (port_id|all) speed (10|100|1000|10000|40000|100000|auto) \
+ testpmd> port config (port_id|all) speed (10|100|1000|10000|25000|40000|50000|100000|auto) \
duplex (half|full|auto)
port config - queues/descriptors
--
2.8.4 (Apple Git-73)
^ permalink raw reply related
* [PATCH] net/enic: fix poor multi-queue Rx performance
From: John Daley @ 2016-10-12 21:22 UTC (permalink / raw)
To: bruce.richardson; +Cc: dev, John Daley
The rx_free_thresh was not being initialized and left at 0
on 1/2 of the RQs which could lead to poor multi-queue
performanace.
Fixes: 856d7ba7ed22 ("net/enic: support scattered Rx")
Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
---
drivers/net/enic/enic.h | 2 +-
drivers/net/enic/enic_ethdev.c | 7 ++-----
drivers/net/enic/enic_main.c | 8 ++++++--
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 4ea4e4a..0d7f379 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -256,7 +256,7 @@ extern int enic_stop_rq(struct enic *enic, uint16_t queue_idx);
extern void enic_free_rq(void *rxq);
extern int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
unsigned int socket_id, struct rte_mempool *mp,
- uint16_t nb_desc);
+ uint16_t nb_desc, uint16_t free_thresh);
extern int enic_set_rss_nic_cfg(struct enic *enic);
extern int enic_set_vnic_res(struct enic *enic);
extern void enic_set_hdr_split_size(struct enic *enic, u16 split_hdr_size);
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 4d24bbd..ef20714 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -315,16 +315,13 @@ static int enicpmd_dev_rx_queue_setup(struct rte_eth_dev *eth_dev,
eth_dev->data->rx_queues[queue_idx] =
(void *)&enic->rq[enic_sop_rq(queue_idx)];
- ret = enic_alloc_rq(enic, queue_idx, socket_id, mp, nb_desc);
+ ret = enic_alloc_rq(enic, queue_idx, socket_id, mp, nb_desc,
+ rx_conf->rx_free_thresh);
if (ret) {
dev_err(enic, "error in allocating rq\n");
return ret;
}
- enic->rq[queue_idx].rx_free_thresh = rx_conf->rx_free_thresh;
- dev_debug(enic, "Set queue_id:%u free thresh:%u\n", queue_idx,
- enic->rq[queue_idx].rx_free_thresh);
-
return enicpmd_dev_setup_intr(enic);
}
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 622b317..eb6ae08 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -603,7 +603,7 @@ int enic_stop_rq(struct enic *enic, uint16_t queue_idx)
int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
unsigned int socket_id, struct rte_mempool *mp,
- uint16_t nb_desc)
+ uint16_t nb_desc, uint16_t free_thresh)
{
int rc;
uint16_t sop_queue_idx = enic_sop_rq(queue_idx);
@@ -624,6 +624,10 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
rq_data->socket_id = socket_id;
rq_data->mp = mp;
rq_sop->in_use = 1;
+ rq_sop->rx_free_thresh = free_thresh;
+ rq_data->rx_free_thresh = free_thresh;
+ dev_debug(enic, "Set queue_id:%u free thresh:%u\n", queue_idx,
+ free_thresh);
mbuf_size = (uint16_t)(rte_pktmbuf_data_room_size(mp) -
RTE_PKTMBUF_HEADROOM);
@@ -1236,7 +1240,7 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
enic_free_rq(rq);
rc = enic_alloc_rq(enic, rq_idx, rq->socket_id, rq->mp,
- rq->tot_nb_desc);
+ rq->tot_nb_desc, rq->rx_free_thresh);
if (rc) {
dev_err(enic,
"Fatal MTU alloc error- No traffic will pass\n");
--
2.10.0
^ permalink raw reply related
* Re: [PATCH v4] log: respect rte_openlog_stream calls before rte_eal_init
From: John Ousterhout @ 2016-10-12 21:17 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
In-Reply-To: <2906824.LEp5vtMuTD@xps13>
Suppose an application starts up, calls rte_eal_init, then later on invokes
code like this:
fclose(stderr);
stderr = fopen("foo", "w");
This might happen if it is using stderr for its log, but decides to roll
the log over to a new file.
Now stderr has changed. However, if DPDK made a copy of it with a
statement like this:
FILE *default_log_stream = stderr;
then default_log_stream will continue to refer to the old log file, not the
new one.
Thus, it's better to grab the value of stderr at the last possible moment
before logging.
-John-
On Wed, Oct 12, 2016 at 12:47 PM, Thomas Monjalon <thomas.monjalon@6wind.com
> wrote:
> 2016-10-12 12:38, John Ousterhout:
> > @@ -127,6 +125,19 @@ rte_vlog(uint32_t level, uint32_t logtype, const
> char *format, va_list ap)
> > {
> > int ret;
> > FILE *f = rte_logs.file;
> > + if (f == NULL) {
> > + f = default_log_stream;
> > + if (f == NULL) {
> > + /*
> > + * Grab the current value of stderr here, rather
> than
> > + * just initializing default_log_stream to stderr.
> This
> > + * ensures that we will always use the current
> value
> > + * of stderr, even if the application closes and
> > + * reopens it.
> > + */
> > + f = stderr;
> > + }
> > + }
>
> I don't understand this big comment.
> What is the difference with initializing default_log_stream to stderr?
> What do you mean by "if the application closes and reopens it"?
>
^ permalink raw reply
* [PATCH 2/2] net/enic: trivial function name changes
From: John Daley @ 2016-10-12 21:12 UTC (permalink / raw)
To: bruce.richardson; +Cc: dev, John Daley
In-Reply-To: <20161012211203.23250-1-johndale@cisco.com>
The function names for converting between RQ indexes known to
the RTE code and intenal RQ indexes for primary Start of Packet
(SOP) queues and spill-over (Data) queues was unclear and
confusing.
Clarify with more explicit function names.
Signed-off-by: John Daley <johndale@cisco.com>
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
---
drivers/net/enic/enic.h | 16 ++++++++-----
drivers/net/enic/enic_clsf.c | 2 +-
drivers/net/enic/enic_ethdev.c | 6 ++---
drivers/net/enic/enic_main.c | 51 ++++++++++++++++++++++++------------------
4 files changed, 43 insertions(+), 32 deletions(-)
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 13a4b31..45b27a5 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -175,19 +175,23 @@ static inline unsigned int enic_sop_rq_idx_to_cq_idx(unsigned int sop_idx)
{
return sop_idx / 2;
}
-static inline unsigned int enic_rq_sop(unsigned int sop_rq)
+
+/* Get the RTE RQ index from a Start of Packet(SOP) RQ index */
+static inline unsigned int enic_sop_rq_idx_to_rte_idx(unsigned int sop_idx)
{
- return sop_rq / 2;
+ return sop_idx / 2;
}
-static inline unsigned int enic_sop_rq(unsigned int rq)
+/* Get the Start of Packet(SOP) RQ index from a RTE RQ index */
+static inline unsigned int enic_rte_rq_idx_to_sop_idx(unsigned int rte_idx)
{
- return rq * 2;
+ return rte_idx * 2;
}
-static inline unsigned int enic_data_rq(unsigned int rq)
+/* Get the Data RQ index from a RTE RQ index */
+static inline unsigned int enic_rte_rq_idx_to_data_idx(unsigned int rte_idx)
{
- return rq * 2 + 1;
+ return rte_idx * 2 + 1;
}
static inline unsigned int enic_vnic_rq_count(struct enic *enic)
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index c709be9..724957a 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -384,7 +384,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_eth_fdir_filter *params)
}
/* Get the enicpmd RQ from the DPDK Rx queue */
- queue = enic_sop_rq(params->action.rx_queue);
+ queue = enic_rte_rq_idx_to_sop_idx(params->action.rx_queue);
if (!enic->rq[queue].in_use)
return -EINVAL;
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 4d24bbd..18bb356 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -154,7 +154,7 @@ static int enicpmd_dev_setup_intr(struct enic *enic)
return 0;
/* check start of packet (SOP) RQs only in case scatter is disabled. */
for (index = 0; index < enic->rq_count; index++) {
- if (!enic->rq[enic_sop_rq(index)].ctrl)
+ if (!enic->rq[enic_rte_rq_idx_to_sop_idx(index)].ctrl)
break;
}
if (enic->rq_count != index)
@@ -277,7 +277,7 @@ static uint32_t enicpmd_dev_rx_queue_count(struct rte_eth_dev *dev,
return 0;
}
- rq_num = enic_sop_rq(rx_queue_id);
+ rq_num = enic_rte_rq_idx_to_sop_idx(rx_queue_id);
cq = &enic->cq[enic_cq_rq(enic, rq_num)];
cq_idx = cq->to_clean;
@@ -313,7 +313,7 @@ static int enicpmd_dev_rx_queue_setup(struct rte_eth_dev *eth_dev,
}
eth_dev->data->rx_queues[queue_idx] =
- (void *)&enic->rq[enic_sop_rq(queue_idx)];
+ (void *)&enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
ret = enic_alloc_rq(enic, queue_idx, socket_id, mp, nb_desc);
if (ret) {
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 65a8307..7689c3c 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -241,14 +241,14 @@ void enic_init_vnic_resources(struct enic *enic)
struct vnic_rq *data_rq;
for (index = 0; index < enic->rq_count; index++) {
- cq_idx = enic_cq_rq(enic, enic_sop_rq(index));
+ cq_idx = enic_cq_rq(enic, enic_rte_rq_idx_to_sop_idx(index));
- vnic_rq_init(&enic->rq[enic_sop_rq(index)],
+ vnic_rq_init(&enic->rq[enic_rte_rq_idx_to_sop_idx(index)],
cq_idx,
error_interrupt_enable,
error_interrupt_offset);
- data_rq = &enic->rq[enic_data_rq(index)];
+ data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(index)];
if (data_rq->in_use)
vnic_rq_init(data_rq,
cq_idx,
@@ -462,17 +462,17 @@ int enic_enable(struct enic *enic)
for (index = 0; index < enic->rq_count; index++) {
err = enic_alloc_rx_queue_mbufs(enic,
- &enic->rq[enic_sop_rq(index)]);
+ &enic->rq[enic_rte_rq_idx_to_sop_idx(index)]);
if (err) {
dev_err(enic, "Failed to alloc sop RX queue mbufs\n");
return err;
}
err = enic_alloc_rx_queue_mbufs(enic,
- &enic->rq[enic_data_rq(index)]);
+ &enic->rq[enic_rte_rq_idx_to_data_idx(index)]);
if (err) {
/* release the allocated mbufs for the sop rq*/
enic_rxmbuf_queue_release(enic,
- &enic->rq[enic_sop_rq(index)]);
+ &enic->rq[enic_rte_rq_idx_to_sop_idx(index)]);
dev_err(enic, "Failed to alloc data RX queue mbufs\n");
return err;
@@ -569,8 +569,10 @@ int enic_stop_wq(struct enic *enic, uint16_t queue_idx)
void enic_start_rq(struct enic *enic, uint16_t queue_idx)
{
- struct vnic_rq *rq_sop = &enic->rq[enic_sop_rq(queue_idx)];
- struct vnic_rq *rq_data = &enic->rq[rq_sop->data_queue_idx];
+ struct vnic_rq *rq_sop;
+ struct vnic_rq *rq_data;
+ rq_sop = &enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
+ rq_data = &enic->rq[rq_sop->data_queue_idx];
struct rte_eth_dev *eth_dev = enic->rte_dev;
if (rq_data->in_use)
@@ -584,8 +586,10 @@ int enic_stop_rq(struct enic *enic, uint16_t queue_idx)
{
int ret1 = 0, ret2 = 0;
struct rte_eth_dev *eth_dev = enic->rte_dev;
- struct vnic_rq *rq_sop = &enic->rq[enic_sop_rq(queue_idx)];
- struct vnic_rq *rq_data = &enic->rq[rq_sop->data_queue_idx];
+ struct vnic_rq *rq_sop;
+ struct vnic_rq *rq_data;
+ rq_sop = &enic->rq[enic_rte_rq_idx_to_sop_idx(queue_idx)];
+ rq_data = &enic->rq[rq_sop->data_queue_idx];
ret2 = vnic_rq_disable(rq_sop);
rte_mb();
@@ -606,8 +610,8 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
uint16_t nb_desc)
{
int rc;
- uint16_t sop_queue_idx = enic_sop_rq(queue_idx);
- uint16_t data_queue_idx = enic_data_rq(queue_idx);
+ uint16_t sop_queue_idx = enic_rte_rq_idx_to_sop_idx(queue_idx);
+ uint16_t data_queue_idx = enic_rte_rq_idx_to_data_idx(queue_idx);
struct vnic_rq *rq_sop = &enic->rq[sop_queue_idx];
struct vnic_rq *rq_data = &enic->rq[data_queue_idx];
unsigned int mbuf_size, mbufs_per_pkt;
@@ -963,7 +967,7 @@ static int enic_set_rsscpu(struct enic *enic, u8 rss_hash_bits)
for (i = 0; i < (1 << rss_hash_bits); i++)
(*rss_cpu_buf_va).cpu[i / 4].b[i % 4] =
- enic_sop_rq(i % enic->rq_count);
+ enic_rte_rq_idx_to_sop_idx(i % enic->rq_count);
err = enic_set_rss_cpu(enic,
rss_cpu_buf_pa,
@@ -1115,8 +1119,8 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
unsigned int cq_idx = enic_cq_rq(enic, rq_idx);
int rc = 0;
- sop_rq = &enic->rq[enic_sop_rq(rq_idx)];
- data_rq = &enic->rq[enic_data_rq(rq_idx)];
+ sop_rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
+ data_rq = &enic->rq[enic_rte_rq_idx_to_data_idx(rq_idx)];
vnic_cq_clean(&enic->cq[cq_idx]);
vnic_cq_init(&enic->cq[cq_idx],
@@ -1132,12 +1136,14 @@ enic_reinit_rq(struct enic *enic, unsigned int rq_idx)
0 /* cq_message_addr */);
- vnic_rq_init_start(sop_rq, enic_cq_rq(enic, enic_sop_rq(rq_idx)),
- 0, sop_rq->ring.desc_count - 1, 1, 0);
+ vnic_rq_init_start(sop_rq, enic_cq_rq(enic,
+ enic_rte_rq_idx_to_sop_idx(rq_idx)), 0,
+ sop_rq->ring.desc_count - 1, 1, 0);
if (data_rq->in_use) {
vnic_rq_init_start(data_rq,
- enic_cq_rq(enic, enic_data_rq(rq_idx)),
- 0, data_rq->ring.desc_count - 1, 1, 0);
+ enic_cq_rq(enic,
+ enic_rte_rq_idx_to_data_idx(rq_idx)), 0,
+ data_rq->ring.desc_count - 1, 1, 0);
}
rc = enic_alloc_rx_queue_mbufs(enic, sop_rq);
@@ -1210,7 +1216,8 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
for (rq_idx = 0; rq_idx < enic->rq_count * 2; rq_idx++) {
rq = &enic->rq[rq_idx];
if (rq->is_sop && rq->in_use) {
- rc = enic_stop_rq(enic, enic_rq_sop(rq_idx));
+ rc = enic_stop_rq(enic,
+ enic_sop_rq_idx_to_rte_idx(rq_idx));
if (rc) {
dev_err(enic, "Failed to stop Rq %u\n", rq_idx);
goto set_mtu_done;
@@ -1232,7 +1239,7 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
/* free and reallocate RQs with the new MTU */
for (rq_idx = 0; rq_idx < enic->rq_count; rq_idx++) {
- rq = &enic->rq[enic_sop_rq(rq_idx)];
+ rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
enic_free_rq(rq);
rc = enic_alloc_rq(enic, rq_idx, rq->socket_id, rq->mp,
@@ -1258,7 +1265,7 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu)
/* restart Rx traffic */
for (rq_idx = 0; rq_idx < enic->rq_count; rq_idx++) {
- rq = &enic->rq[enic_sop_rq(rq_idx)];
+ rq = &enic->rq[enic_rte_rq_idx_to_sop_idx(rq_idx)];
if (rq->is_sop && rq->in_use)
enic_start_rq(enic, rq_idx);
}
--
2.10.0
^ permalink raw reply related
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