* Re: [PATCH v5 1/2] librte_ether: add protection against overwrite device data
From: Kerlin, MarcinX @ 2016-10-06 13:57 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, De Lara Guarch, Pablo, Gonzalez Monroy, Sergio
In-Reply-To: <1538060.S3DiiuebUu@xps13>
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, October 06, 2016 11:41 AM
> To: Kerlin, MarcinX <marcinx.kerlin@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>
> Subject: Re: [PATCH v5 1/2] librte_ether: add protection against overwrite
> device data
>
> Hi Marcin,
>
> 2016-09-30 16:00, Marcin Kerlin:
> > Added protection against overwrite device data in array
> > rte_eth_dev_data[] for the next secondary applications. Secondary
> > process appends in the first free place rather than at the beginning.
> > This behavior prevents overwriting devices data of primary process by
> secondary process.
>
> I've just realized that you are trying to fix an useless code.
> Why not just remove the array rte_eth_dev_data[] at first?
because pointer to rte_eth_dev_data in rte_eth_devices[] is
just to array rte_eth_dev_data[].
rte_ethdev.c:214
eth_dev->data = &rte_eth_dev_data[port_id];
> We already have the array rte_eth_devices[] and there is a pointer to
> rte_eth_dev_data in rte_eth_dev.
As you write above there is a pointer, but after run secondary testpmd this pointer
will indicate data which hold from now data for secondary testpmd.
1) run testpmd [primary]
rte_eth_devices[0].data.name = 3:0.1
2) run testpmd [secondary]
This testpmd overwrite first index in rte_eth_dev_data[]
3:0.1 -> eth_pcap0
3) back to testpmd [primary]
rte_eth_devices[0].data.name = eth_pcap0
from now in primary testpmd our device name is eth_pcap0 but should be 3:0.1
>
> Is it just a workaround to be able to lookup the rte_eth_dev_data memzone in
> the secondary process?
No it is not workaround, it is protection against overwrite device data.
I think that my cover letter good explain what is wrong. I did there
short debug log.
> So wouldn't it be saner to have rte_eth_devices[] in a memzone?
So you mean that move rte_eth_devices[] to memzone + remove rte_eth_dev_data[].
What will indicate pointer inside rte_eth_dev rte_eth_devices[]:
(struct rte_eth_dev_data *data; /**< Pointer to device data */)
If I did not understand you please clarify more.
Regards,
Marcin
>
> Sergio, as the multi-process maintainer, I guess you have an idea :)
^ permalink raw reply
* Re: [PATCH v7] net/virtio: add set_mtu in virtio
From: Kavanagh, Mark B @ 2016-10-06 13:58 UTC (permalink / raw)
To: Dey, Souvik, yuanhan.liu@linux.intel.com,
stephen@networkplumber.org
Cc: dev@dpdk.org
In-Reply-To: <BN3PR03MB14946676AD33FC565680FF74DAC70@BN3PR03MB1494.namprd03.prod.outlook.com>
>
>Hi Stephen/Liu,
> Any other comments or suggestions for this. If the below patch looks fine then please
>do suggest the next steps .
Hi Souvik,
Just to let you know that Yuanhan is out of office on account of public holidays in PRC.
AFAIA he should be back online from next week.
Thanks,
Mark
>
>--
>Regards,
>Souvik
>
>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dey, Souvik
>Sent: Wednesday, October 5, 2016 10:05 AM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; yuanhan.liu@linux.intel.com;
>stephen@networkplumber.org
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH v7] net/virtio: add set_mtu in virtio
>
>Yes Mark, I have modified the patch with the below comments.
>
>drivers/net/virtio/virtio_ethdev.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>index 423c597..1dbfea6 100644
>--- a/drivers/net/virtio/virtio_ethdev.c
>+++ b/drivers/net/virtio/virtio_ethdev.c
>@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); }
>
>+#define VLAN_TAG_LEN 4 /* 802.3ac tag (not DMA'd) */
>+
>+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>+ struct virtio_hw *hw = dev->data->dev_private;
>+ uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
>+ hw->vtnet_hdr_size;
>+ uint32_t frame_size = mtu + ether_hdr_len;
>+
>+ if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
>+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>+ ETHER_MIN_MTU, (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len));
>+ return -EINVAL;
>+ }
>+ return 0;
>+}
>
>Let mem know if this looks good or we have few more comments.
>
>--
>Regards,
>Souvik
>
>-----Original Message-----
>From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
>Sent: Wednesday, October 5, 2016 4:16 AM
>To: Dey, Souvik <sodey@sonusnet.com>; yuanhan.liu@linux.intel.com; stephen@networkplumber.org
>Cc: dev@dpdk.org
>Subject: RE: [PATCH v7] net/virtio: add set_mtu in virtio
>
>>Hi All,
>> Is there any further comments or modifications required for this
>>patch, or what next steps do you guys suggest here ?
>
>Hi Souvik,
>
>Some minor comments inline.
>
>Thanks,
>Mark
>
>>
>>--
>>Regards,
>>Souvik
>>
>>-----Original Message-----
>>From: Dey, Souvik
>>Sent: Saturday, October 1, 2016 10:09 AM
>>To: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;
>>stephen@networkplumber.org; dev@dpdk.org
>>Subject: RE: [PATCH v7] net/virtio: add set_mtu in virtio
>>
>>Hi Liu/Stephen/Mark,
>>
>> I have submitted Version 7 of this patch. Do let me know if this looks proper.
>>
>>--
>>Regards,
>>Souvik
>>
>>-----Original Message-----
>>From: Dey, Souvik
>>Sent: Thursday, September 29, 2016 4:32 PM
>>To: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;
>>stephen@networkplumber.org; dev@dpdk.org
>>Cc: Dey, Souvik <sodey@sonusnet.com>
>>Subject: [PATCH v7] net/virtio: add set_mtu in virtio
>>
>>
>>Virtio interfaces do not currently allow the user to specify a
>>particular Maximum Transmission Unit (MTU).Consequently, the MTU of
>>Virtio interfaces is typically set to the Ethernet default value of 1500.
>>This is problematic in the case of cloud deployments, in which a
>>specific (and potentially non-standard) MTU needs to be set by a DHCP
>>server, which needs to be honored by all interfaces across the traffic
>>path.To achieve this Virtio interfaces should support setting of MTU.
>>In case when GRE/VXLAN tunneling is used for internal communication,
>>there will be an overhead added by the infrastructure in the packet
>>over and above the ETHER MTU of 1518. So to take care of this overhead
>>in these cases the DHCP server corrects the L3 MTU to 1454. But since
>>virtio interfaces was not having the MTU set functionality that MTU
>>sent by the DHCP server was ignored and the instance will still send
>>packets with 1500 MTU which after encapsulation will become more than
>>1518 and eventually gets dropped in the infrastructure.
>>By adding an additional 'set_mtu' function to the Virtio driver, we can
>>honor the MTU sent by the DHCP server. The dhcp server/controller can
>>then leverage this 'set_mtu' functionality to resolve the above
>>mentioned issue of packets getting dropped due to incorrect size.
>>
>>
>>Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>>
>>---
>>Changes in v7:
>>- Replaced the CRC_LEN with the merge rx buf header length.
>>- Changed the frame_len max validation to VIRTIO_MAX_RX_PKTLEN.
>>Changes in v6:
>>- Description of change corrected
>>- Corrected the identations
>>- Corrected the subject line too
>>- The From line was also not correct
>>- Re-submitting as the below patch was not proper Changes in v5:
>>- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set
>>- Calculate frame size, based on 'mtu' parameter
>>- Corrected the upper bound and lower bound checks in virtio_mtu_set
>>Changes in v4: Incorporated review comments.
>>Changes in v3: Corrected few style errors as reported by sys-stv.
>>Changes in v2: Incorporated review comments.
>>
>> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>>diff --git a/drivers/net/virtio/virtio_ethdev.c
>>b/drivers/net/virtio/virtio_ethdev.c
>>index 423c597..1dbfea6 100644
>>--- a/drivers/net/virtio/virtio_ethdev.c
>>+++ b/drivers/net/virtio/virtio_ethdev.c
>>@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
>> PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); }
>>
>>+#define VLAN_TAG_LEN 4 /* 802.3ac tag (not DMA'd) */
>
>There should be a blank line between the #define and the function prototype beneath.
>
>>+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>>+ struct virtio_hw *hw = dev->data->dev_private;
>>+ uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
>>+ hw->vtnet_hdr_size;
>
>I'll rely on Stephen and Yuanhan's judgment for this.
>
>>+ uint32_t frame_size = mtu + ether_hdr_len;
>>+
>>+ if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
>>+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>>+ ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN);
>
>Shouldn't last format print parameter should be (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len)?
>i.e PMD_INIT_LOG(ERR, "MTU should........%d\n",
> ETHER_MIN_MTU, (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len));
>
>>+ return -EINVAL;
>>+ }
>>+ return 0;
>>+}
>>
>> /*
>> * dev_ops for virtio, bare necessities for basic operation
>> */
>>@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
>> .allmulticast_enable = virtio_dev_allmulticast_enable,
>> .allmulticast_disable = virtio_dev_allmulticast_disable,
>>+ .mtu_set = virtio_mtu_set,
>> .dev_infos_get = virtio_dev_info_get,
>> .stats_get = virtio_dev_stats_get,
>> .xstats_get = virtio_dev_xstats_get,
>>--
>>2.9.3.windows.1
^ permalink raw reply
* Re: ixgbe: support checksum flags in sse vector Rx function
From: Remy Horton @ 2016-10-06 14:00 UTC (permalink / raw)
To: Olivier Matz, dev, konstantin.ananyev, helin.zhang
Cc: bruce.richardson, Maxime Leroy
In-Reply-To: <1467893942-4048-1-git-send-email-olivier.matz@6wind.com>
On 07/07/2016 13:19, Olivier Matz wrote:
[..]
> Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 8 ++---
> drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c | 6 ++++
> drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 50 +++++++++++++++++++++----------
> 3 files changed, 42 insertions(+), 22 deletions(-)
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply
* Re: [PATCH] eal/vdev: rename vdev driver init/uninit to probe/remove
From: Thomas Monjalon @ 2016-10-06 14:04 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: david.marchand, dev
In-Reply-To: <1475762043-24913-1-git-send-email-shreyansh.jain@nxp.com>
2016-10-06 19:24, Shreyansh Jain:
> Inline with PCI probe and remove, VDEV probe and remove hooks provide
> a uniform naming.
> PCI probe represents scan and driver initialization. For VDEV, it will
> represent argument parsing and initialization.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v5 1/2] librte_ether: add protection against overwrite device data
From: Thomas Monjalon @ 2016-10-06 14:20 UTC (permalink / raw)
To: Kerlin, MarcinX; +Cc: dev, De Lara Guarch, Pablo, Gonzalez Monroy, Sergio
In-Reply-To: <68D830D942438745AD09BAFA99E33E812BE4BC@IRSMSX102.ger.corp.intel.com>
2016-10-06 13:57, Kerlin, MarcinX:
> Hi Thomas,
>
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> >
> > Hi Marcin,
> >
> > 2016-09-30 16:00, Marcin Kerlin:
> > > Added protection against overwrite device data in array
> > > rte_eth_dev_data[] for the next secondary applications. Secondary
> > > process appends in the first free place rather than at the beginning.
> > > This behavior prevents overwriting devices data of primary process by
> > secondary process.
> >
> > I've just realized that you are trying to fix an useless code.
> > Why not just remove the array rte_eth_dev_data[] at first?
>
> because pointer to rte_eth_dev_data in rte_eth_devices[] is
> just to array rte_eth_dev_data[].
>
> rte_ethdev.c:214
> eth_dev->data = &rte_eth_dev_data[port_id];
This line indicates that the pointer data is to the struct rte_eth_dev_data
of the port_id, not the array of every devices.
> > We already have the array rte_eth_devices[] and there is a pointer to
> > rte_eth_dev_data in rte_eth_dev.
>
> As you write above there is a pointer, but after run secondary testpmd this pointer
> will indicate data which hold from now data for secondary testpmd.
[...]
I think I've understood the bug.
I'm just saying you are fixing a weird design (rte_eth_dev_data[]).
> > Is it just a workaround to be able to lookup the rte_eth_dev_data memzone in
> > the secondary process?
>
> No it is not workaround, it is protection against overwrite device data.
> I think that my cover letter good explain what is wrong. I did there
> short debug log.
I'm talking about the initial introduction of rte_eth_dev_data[]
which seems to be a workaround for multi-process without touching
rte_eth_devices[] allocated as a global variable (not a memzone).
> > So wouldn't it be saner to have rte_eth_devices[] in a memzone?
>
> So you mean that move rte_eth_devices[] to memzone + remove rte_eth_dev_data[].
Yes
> What will indicate pointer inside rte_eth_dev rte_eth_devices[]:
> (struct rte_eth_dev_data *data; /**< Pointer to device data */)
After thinking more about it, I realize that rte_eth_devices cannot be
in a shared memzone because of its local pointers.
Sorry for the noise, I'll reconsider your patch.
^ permalink raw reply
* Re: [PATCH] Revert "bonding: use existing enslaved device queues"
From: Declan Doherty @ 2016-10-06 14:32 UTC (permalink / raw)
To: Ilya Maximets, dev
Cc: Heetae Ahn, Yuanhan Liu, Eric Kinzie, Bernard Iremonger, stable
In-Reply-To: <1473251290-22053-1-git-send-email-i.maximets@samsung.com>
On 07/09/16 13:28, Ilya Maximets wrote:
> This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.
>
> It is necessary to reconfigure all queues every time because configuration
> can be changed.
>
Hey Ilya, this makes sense. I guess this was my original intention but I
missed this case in my review of the change.
> For example, if we're reconfiguring bonding device with new memory pool,
> already configured queues will still use the old one. And if the old
> mempool be freed, application likely will panic in attempt to use
> freed mempool.
>
> This happens when we use the bonding device with OVS 2.6 while MTU
> reconfiguration:
>
> PANIC in rte_mempool_get_ops():
> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)" failed
>
> Cc: <stable@dpdk.org>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b20a272..eb5b6d1 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> struct bond_rx_queue *bd_rx_q;
> struct bond_tx_queue *bd_tx_q;
>
> - uint16_t old_nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
> - uint16_t old_nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
> int errval;
> uint16_t q_id;
>
> @@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> }
>
> /* Setup Rx Queues */
> - /* Use existing queues, if any */
> - for (q_id = old_nb_rx_queues;
> - q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
> bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id];
>
> errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
> @@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> }
>
> /* Setup Tx Queues */
> - /* Use existing queues, if any */
> - for (q_id = old_nb_tx_queues;
> - q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
> bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id];
>
> errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>
Acked-by: Declan Doherty <declan.doherty@intel.com>
^ permalink raw reply
* Re: [PATCH v5 01/13] librte_ether: modify internal callback function
From: Iremonger, Bernard @ 2016-10-06 14:33 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, Shah, Rahul R, Lu, Wenzhuo, az5157@att.com,
jerin.jacob@caviumnetworks.com
In-Reply-To: <8021250.9Wl94qBdaq@xps13>
Hi Thomas,
> Subject: Re: [dpdk-dev] [PATCH v5 01/13] librte_ether: modify internal
> callback function
>
> 2016-10-06 12:26, Bernard Iremonger:
> > void
> > _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > - enum rte_eth_event_type event)
> > + enum rte_eth_event_type event, void *param)
>
> You need to squash the patches updating the calls to this function.
> Otherwise, this patch does not compile.
I will have to squash everything into one patch, separate patches will not compile.
> [...]
> > + if (param != NULL)
> > + dev_cb.cb_arg = (void *) param;
>
> You are overriding the user parameter.
Yes, we want to update the user parameter for the RTE_ETH_EVENT_VF_MBOX
> As it is only for a new event, it can be described in the register API that the
> user param won't be returned for this event.
I will add a description in the rte_eth_dev_callback_register function.
> But a better design would be to add a new parameter to the callback.
> However it will be an API breakage.
I do not want to break the ABI at this point.
>
> > + RTE_ETH_EVENT_VF_MBOX, /**< PF mailbox processing callback */
>
> Sorry I do not parse well this line.
> The event name is VF_MBOX and the comment is about the callback
> processing this event on PF side?
> I would suggest this kind of comment: "message from VF received by PF"
Ok.
>
> [...]
> > * Pointer to struct rte_eth_dev.
> > * @param event
> > * Eth device interrupt event type.
> > + * @param param
> > + * Parameter to pass back to user application.
> > + * Allows the user application to decide if a particular function
> > + * is permitted.
>
> In a more generic case, the parameter gives some details about the event.
^ permalink raw reply
* Re: [PATCH v2 4/7] vhost: add dequeue zero copy
From: Xu, Qian Q @ 2016-10-06 14:37 UTC (permalink / raw)
To: Yuanhan Liu, dev@dpdk.org; +Cc: Maxime Coquelin
In-Reply-To: <1474604007-5221-5-git-send-email-yuanhan.liu@linux.intel.com>
See the bottom.
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yuanhan Liu
Sent: Friday, September 23, 2016 5:13 AM
To: dev@dpdk.org
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: [dpdk-dev] [PATCH v2 4/7] vhost: add dequeue zero copy
The basic idea of dequeue zero copy is, instead of copying data from the desc buf, here we let the mbuf reference the desc buf addr directly.
Doing so, however, has one major issue: we can't update the used ring at the end of rte_vhost_dequeue_burst. Because we don't do the copy here, an update of the used ring would let the driver to reclaim the desc buf. As a result, DPDK might reference a stale memory region.
To update the used ring properly, this patch does several tricks:
- when mbuf references a desc buf, refcnt is added by 1.
This is to pin lock the mbuf, so that a mbuf free from the DPDK
won't actually free it, instead, refcnt is subtracted by 1.
- We chain all those mbuf together (by tailq)
And we check it every time on the rte_vhost_dequeue_burst entrance,
to see if the mbuf is freed (when refcnt equals to 1). If that
happens, it means we are the last user of this mbuf and we are
safe to update the used ring.
- "struct zcopy_mbuf" is introduced, to associate an mbuf with the
right desc idx.
Dequeue zero copy is introduced for performance reason, and some rough tests show about 50% perfomance boost for packet size 1500B. For small packets, (e.g. 64B), it actually slows a bit down (well, it could up to 15%). That is expected because this patch introduces some extra works, and it outweighs the benefit from saving few bytes copy.
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
v2: - use unlikely/likely for dequeue_zero_copy check, as it's not enabled
by default, as well as it has some limitations in vm2nic case.
- handle the case that a desc buf might across 2 host phys pages
- reset nr_zmbuf to 0 at set_vring_num
- set the zmbuf_size to vq->size, but not the double of it.
---
lib/librte_vhost/vhost.c | 2 +
lib/librte_vhost/vhost.h | 22 +++++-
lib/librte_vhost/vhost_user.c | 42 +++++++++- lib/librte_vhost/virtio_net.c | 173 +++++++++++++++++++++++++++++++++++++-----
4 files changed, 219 insertions(+), 20 deletions(-)
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index 46095c3..ab25649 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -141,6 +141,8 @@ init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
/* always set the default vq pair to enabled */
if (qp_idx == 0)
vq->enabled = 1;
+
+ TAILQ_INIT(&vq->zmbuf_list);
}
static void
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index 8565fa1..be8a398 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -36,6 +36,7 @@
#include <stdint.h>
#include <stdio.h>
#include <sys/types.h>
+#include <sys/queue.h>
#include <unistd.h>
#include <linux/vhost.h>
@@ -61,6 +62,19 @@ struct buf_vector {
uint32_t desc_idx;
};
+/*
+ * A structure to hold some fields needed in zero copy code path,
+ * mainly for associating an mbuf with the right desc_idx.
+ */
+struct zcopy_mbuf {
+ struct rte_mbuf *mbuf;
+ uint32_t desc_idx;
+ uint16_t in_use;
+
+ TAILQ_ENTRY(zcopy_mbuf) next;
+};
+TAILQ_HEAD(zcopy_mbuf_list, zcopy_mbuf);
+
/**
* Structure contains variables relevant to RX/TX virtqueues.
*/
@@ -85,6 +99,12 @@ struct vhost_virtqueue {
/* Physical address of used ring, for logging */
uint64_t log_guest_addr;
+
+ uint16_t nr_zmbuf;
+ uint16_t zmbuf_size;
+ uint16_t last_zmbuf_idx;
+ struct zcopy_mbuf *zmbufs;
+ struct zcopy_mbuf_list zmbuf_list;
} __rte_cache_aligned;
/* Old kernels have no such macro defined */ @@ -135,6 +155,7 @@ struct virtio_net {
/* to tell if we need broadcast rarp packet */
rte_atomic16_t broadcast_rarp;
uint32_t virt_qp_nb;
+ int dequeue_zero_copy;
struct vhost_virtqueue *virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];
#define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
char ifname[IF_NAME_SZ];
@@ -146,7 +167,6 @@ struct virtio_net {
uint32_t nr_guest_pages;
uint32_t max_guest_pages;
struct guest_page *guest_pages;
-
} __rte_cache_aligned;
/**
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index a92377a..ac40408 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -180,7 +180,23 @@ static int
vhost_user_set_vring_num(struct virtio_net *dev,
struct vhost_vring_state *state)
{
- dev->virtqueue[state->index]->size = state->num;
+ struct vhost_virtqueue *vq = dev->virtqueue[state->index];
+
+ vq->size = state->num;
+
+ if (dev->dequeue_zero_copy) {
+ vq->nr_zmbuf = 0;
+ vq->last_zmbuf_idx = 0;
+ vq->zmbuf_size = vq->size;
+ vq->zmbufs = rte_zmalloc(NULL, vq->zmbuf_size *
+ sizeof(struct zcopy_mbuf), 0);
+ if (vq->zmbufs == NULL) {
+ RTE_LOG(WARNING, VHOST_CONFIG,
+ "failed to allocate mem for zero copy; "
+ "zero copy is force disabled\n");
+ dev->dequeue_zero_copy = 0;
+ }
+ }
return 0;
}
@@ -662,11 +678,32 @@ vhost_user_set_vring_kick(struct virtio_net *dev, struct VhostUserMsg *pmsg)
vq->kickfd = file.fd;
if (virtio_is_ready(dev) && !(dev->flags & VIRTIO_DEV_RUNNING)) {
+ if (dev->dequeue_zero_copy) {
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "Tx zero copy is enabled\n");
+ }
+
if (notify_ops->new_device(dev->vid) == 0)
dev->flags |= VIRTIO_DEV_RUNNING;
}
}
+static void
+free_zmbufs(struct vhost_virtqueue *vq) {
+ struct zcopy_mbuf *zmbuf, *next;
+
+ for (zmbuf = TAILQ_FIRST(&vq->zmbuf_list);
+ zmbuf != NULL; zmbuf = next) {
+ next = TAILQ_NEXT(zmbuf, next);
+
+ rte_pktmbuf_free(zmbuf->mbuf);
+ TAILQ_REMOVE(&vq->zmbuf_list, zmbuf, next);
+ }
+
+ rte_free(vq->zmbufs);
+}
+
/*
* when virtio is stopped, qemu will send us the GET_VRING_BASE message.
*/
@@ -695,6 +732,9 @@ vhost_user_get_vring_base(struct virtio_net *dev,
dev->virtqueue[state->index]->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
+ if (dev->dequeue_zero_copy)
+ free_zmbufs(dev->virtqueue[state->index]);
+
return 0;
}
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 1c2ee47..215542c 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -678,6 +678,43 @@ make_rarp_packet(struct rte_mbuf *rarp_mbuf, const struct ether_addr *mac)
return 0;
}
+static inline struct zcopy_mbuf *__attribute__((always_inline))
+get_zmbuf(struct vhost_virtqueue *vq) {
+ uint16_t i;
+ uint16_t last;
+ int tries = 0;
+
+ /* search [last_zmbuf_idx, zmbuf_size) */
+ i = vq->last_zmbuf_idx;
+ last = vq->zmbuf_size;
+
+again:
+ for (; i < last; i++) {
+ if (vq->zmbufs[i].in_use == 0) {
+ vq->last_zmbuf_idx = i + 1;
+ vq->zmbufs[i].in_use = 1;
+ return &vq->zmbufs[i];
+ }
+ }
+
+ tries++;
+ if (tries == 1) {
+ /* search [0, last_zmbuf_idx) */
+ i = 0;
+ last = vq->last_zmbuf_idx;
+ goto again;
+ }
+
+ return NULL;
+}
+
+static inline void __attribute__((always_inline)) put_zmbuf(struct
+zcopy_mbuf *zmbuf) {
+ zmbuf->in_use = 0;
+}
+
static inline int __attribute__((always_inline)) copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, uint16_t desc_idx, @@ -701,6 +738,27 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
if (unlikely(!desc_addr))
return -1;
+ if (unlikely(dev->dequeue_zero_copy)) {
+ struct zcopy_mbuf *zmbuf;
+
+ zmbuf = get_zmbuf(vq);
+ if (!zmbuf)
+ return -1;
+ zmbuf->mbuf = m;
+ zmbuf->desc_idx = desc_idx;
+
+ /*
+ * Pin lock the mbuf; we will check later to see whether
+ * the mbuf is freed (when we are the last user) or not.
+ * If that's the case, we then could update the used ring
+ * safely.
+ */
+ rte_mbuf_refcnt_update(m, 1);
+
+ vq->nr_zmbuf += 1;
+ TAILQ_INSERT_TAIL(&vq->zmbuf_list, zmbuf, next);
+ }
+
hdr = (struct virtio_net_hdr *)((uintptr_t)desc_addr);
rte_prefetch0(hdr);
this function copy_desc_to_mbuf has changed on the dpdk-next-virtio repo. Based on current dpdk-next-virtio repo, the commit ID is as below:
commit b4f7b43cd9d3b6413f41221051d03a23bc5f5fbe
Author: Zhiyong Yang <zhiyong.yang@intel.com>
Date: Thu Sep 29 20:35:49 2016 +0800
Then you will find the parameter "struct vhost_virtqueue *vq" is removed, so if apply your patch on that commit ID, the build will fail, since no vq definition but we used it in the function.
Could you check? Thx.
== Build lib/librte_table
/home/qxu10/dpdk-zero/lib/librte_vhost/virtio_net.c: In function 'copy_desc_to_mbuf':
/home/qxu10/dpdk-zero/lib/librte_vhost/virtio_net.c:745:21: error: 'vq' undeclared (first use in this function)
zmbuf = get_zmbuf(vq);
^
/home/qxu10/dpdk-zero/lib/librte_vhost/virtio_net.c:745:21: note: each undeclared identifier is reported only once for each function it appears in
/home/qxu10/dpdk-zero/mk/internal/rte.compile-pre.mk:138: recipe for target 'virtio_net.o' failed
make[5]: *** [virtio_net.o] Error 1
/home/qxu10/dpdk-zero/mk/rte.subdir.mk:61: recipe for target 'librte_vhost' failed
make[4]: *** [librte_vhost] Error 2
make[4]: *** Waiting for unfinished jobs....
^ permalink raw reply
* Re: [PATCH v5 1/2] librte_ether: add protection against overwrite device data
From: Thomas Monjalon @ 2016-10-06 14:52 UTC (permalink / raw)
To: Marcin Kerlin; +Cc: dev, pablo.de.lara.guarch
In-Reply-To: <1475244055-6309-2-git-send-email-marcinx.kerlin@intel.com>
2016-09-30 16:00, Marcin Kerlin:
> Added protection against overwrite device data in array rte_eth_dev_data[]
> for the next secondary applications. Secondary process appends in the
> first free place rather than at the beginning. This behavior prevents
> overwriting devices data of primary process by secondary process.
It would be good to state what is a secondary process.
You are trying to extend its capabilities to be able to initialize devices.
So primary and secondary processes are almost equivalent?
What happens if we do not create any device in the primary?
Answer from code review: "Cannot allocate memzone for ethernet port data\n"
The secondary process is a hack to me.
But it is fine to have such hack for debug or monitoring purpose.
I would like to understand what are the other use cases?
By the way, the code managing the shared data of a device should
be at the EAL level in order to be used by other interfaces like crypto.
> @@ -631,6 +692,8 @@ int
> rte_eth_dev_detach(uint8_t port_id, char *name)
> {
> struct rte_pci_addr addr;
> + struct rte_eth_dev_data *eth_dev_data = NULL;
> + char device[RTE_ETH_NAME_MAX_LEN];
> int ret = -1;
>
> if (name == NULL) {
> @@ -642,6 +705,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
> if (rte_eth_dev_is_detachable(port_id))
> goto err;
>
> + /* get device name by port id */
> + if (rte_eth_dev_get_name_by_port(port_id, device))
> + goto err;
> +
> + /* look for an entry in the shared device data */
> + eth_dev_data = rte_eth_dev_get_dev_data_by_name(device);
> + if (eth_dev_data == NULL)
> + goto err;
Why not getting eth_dev_data from rte_eth_devices[port_id].data ?
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> /**
> * @internal
> + * Release device data kept in shared memory for all processes.
> + *
> + * @param port_id
> + * The port identifier of the device to release device data.
> + * @return
> + * - 0 on success, negative on error
> + */
> +int rte_eth_dev_release_dev_data(uint8_t port_id);
Why this function? It is not used.
You already have done the job in the detach function.
^ permalink raw reply
* Re: [PATCH v2] app/test: remove hard-coding of crypto num qps
From: Trahe, Fiona @ 2016-10-06 14:55 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org; +Cc: Akhil Goyal
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA0465E@IRSMSX108.ger.corp.intel.com>
> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, October 5, 2016 1:50 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: RE: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto
> num qps
>
> Hi,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> > Sent: Thursday, September 29, 2016 10:18 AM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo; Trahe, Fiona; Akhil Goyal
> > Subject: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto
> > num qps
> >
> > ts_params->conf.nb_queue_pairs should not be hard coded with device
> > specific number. It should be retrieved from the device info.
> > Any test which changes it should restore it to orig value.
> >
> > Also related cleanup of test code setting number and size of
> > queue-pairs on a device, e.g.
> > * Removed irrelevant “for” loop – was hardcoded to only loop once.
> > * Removed obsolete comment re inability to free and re-allocate queu
> > memory
> > and obsolete workaround for it which used to create maximum size queues.
> >
> > And added freeing of ring memory on queue-pair release in aesni_mb
> > PMD, else releasing and setting up queue-pair of a different size fails.
> >
> > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> > ---
> >
> > v2:
> > Fix for broken QAT PMD unit tests exposed by v1
> > i.e. In test_device_configure_invalid_queue_pair_ids() after running tests
> > for invalid values restore original nb_queue_pairs.
> > Also cleanup of test code setting number and size of queue-pairs on a
> device
> > Also fix for aesni_mb PMD not freeing ring memory on qp release
>
> Sorry, I missed this patch. Could you split this patch into different patches?
> It looks like you are making (three?) changes in different places.
>
> Thanks,
> Pablo
Ok, will do,
Fiona
^ permalink raw reply
* Re: [PATCH v5 01/13] librte_ether: modify internal callback function
From: Thomas Monjalon @ 2016-10-06 14:56 UTC (permalink / raw)
To: Iremonger, Bernard
Cc: dev, Shah, Rahul R, Lu, Wenzhuo, az5157@att.com,
jerin.jacob@caviumnetworks.com
In-Reply-To: <8CEF83825BEC744B83065625E567D7C21A08F83F@IRSMSX108.ger.corp.intel.com>
2016-10-06 14:33, Iremonger, Bernard:
> Hi Thomas,
>
> > Subject: Re: [dpdk-dev] [PATCH v5 01/13] librte_ether: modify internal
> > callback function
> >
> > 2016-10-06 12:26, Bernard Iremonger:
> > > void
> > > _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > > - enum rte_eth_event_type event)
> > > + enum rte_eth_event_type event, void *param)
> >
> > You need to squash the patches updating the calls to this function.
> > Otherwise, this patch does not compile.
>
> I will have to squash everything into one patch, separate patches will not compile.
No you can keep a separate patch for the VF event in ixgbe.
> > [...]
> > > + if (param != NULL)
> > > + dev_cb.cb_arg = (void *) param;
> >
> > You are overriding the user parameter.
>
> Yes, we want to update the user parameter for the RTE_ETH_EVENT_VF_MBOX
>
> > As it is only for a new event, it can be described in the register API that the
> > user param won't be returned for this event.
>
> I will add a description in the rte_eth_dev_callback_register function.
>
> > But a better design would be to add a new parameter to the callback.
> > However it will be an API breakage.
>
> I do not want to break the ABI at this point.
Yes, but it can be considered for a later change.
> > > + RTE_ETH_EVENT_VF_MBOX, /**< PF mailbox processing callback */
> >
> > Sorry I do not parse well this line.
> > The event name is VF_MBOX and the comment is about the callback
> > processing this event on PF side?
> > I would suggest this kind of comment: "message from VF received by PF"
>
> Ok.
>
> >
> > [...]
> > > * Pointer to struct rte_eth_dev.
> > > * @param event
> > > * Eth device interrupt event type.
> > > + * @param param
> > > + * Parameter to pass back to user application.
> > > + * Allows the user application to decide if a particular function
> > > + * is permitted.
> >
> > In a more generic case, the parameter gives some details about the event.
Please consider a rewording here, thanks.
^ permalink raw reply
* Re: [PATCH v4 1/2] i40e: Add packet_type metadata in the i40e vPMD
From: Chen, Jing D @ 2016-10-06 15:28 UTC (permalink / raw)
To: Shaw, Jeffrey B, dev@dpdk.org
Cc: Zhang, Helin, Wu, Jingjing, damarion@cisco.com, Zhang, Qi Z
In-Reply-To: <1475735890-129160-1-git-send-email-jeffrey.b.shaw@intel.com>
> -----Original Message-----
> From: Shaw, Jeffrey B
> Sent: Wednesday, October 5, 2016 11:38 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; damarion@cisco.com; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> Subject: [PATCH v4 1/2] i40e: Add packet_type metadata in the i40e vPMD
>
> From: Damjan Marion <damarion@cisco.com>
>
> The ptype is decoded from the rx descriptor and stored in the packet type
> field in the mbuf using the same function as the non-vector driver.
>
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>
> Changes in v2:
> - Add missing reference to i40e_recv_scattered_pkts_vec() when
> querying supported packet types.
>
> Changes in v3:
> - None. (Please ignore this version).
>
> Changes in v4:
> - Fix rss/fdir status mask and shift to get accurate Flow Director Filter
> Match (FLM) indication.
>
> drivers/net/i40e/i40e_rxtx.c | 567 +--------------------------------------
> drivers/net/i40e/i40e_rxtx.h | 563
> ++++++++++++++++++++++++++++++++++++++
> drivers/net/i40e/i40e_rxtx_vec.c | 16 ++
> 3 files changed, 582 insertions(+), 564 deletions(-)
Acked-by : Jing Chen <jing.d.chen@intel.com>
^ permalink raw reply
* Re: [PATCH v4 2/2] i40e: Enable bad checksum flags in i40e vPMD
From: Chen, Jing D @ 2016-10-06 15:30 UTC (permalink / raw)
To: Shaw, Jeffrey B, dev@dpdk.org
Cc: Zhang, Helin, Wu, Jingjing, damarion@cisco.com, Zhang, Qi Z
In-Reply-To: <1475735890-129160-2-git-send-email-jeffrey.b.shaw@intel.com>
> -----Original Message-----
> From: Shaw, Jeffrey B
> Sent: Wednesday, October 5, 2016 11:38 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; damarion@cisco.com; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> Subject: [PATCH v4 2/2] i40e: Enable bad checksum flags in i40e vPMD
>
> From: Damjan Marion <damarion@cisco.com>
>
> Decode the checksum flags from the rx descriptor, setting the appropriate bit
> in the mbuf ol_flags field when the flag indicates a bad checksum.
>
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
^ permalink raw reply
* Re: [PATCH v5 01/13] librte_ether: modify internal callback function
From: Iremonger, Bernard @ 2016-10-06 15:32 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, Shah, Rahul R, Lu, Wenzhuo, az5157@att.com,
jerin.jacob@caviumnetworks.com
In-Reply-To: <1884200.vdWHEKFaxn@xps13>
Hi Thomas,
<snip>
> > > Subject: Re: [dpdk-dev] [PATCH v5 01/13] librte_ether: modify
> > > internal callback function
> > >
> > > 2016-10-06 12:26, Bernard Iremonger:
> > > > void
> > > > _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > > > - enum rte_eth_event_type event)
> > > > + enum rte_eth_event_type event, void *param)
> > >
> > > You need to squash the patches updating the calls to this function.
> > > Otherwise, this patch does not compile.
> >
> > I will have to squash everything into one patch, separate patches will not
> compile.
>
> No you can keep a separate patch for the VF event in ixgbe.
I have 4 patches at present
librte_ether
net/ixgbe
drivers/net
app/test
Would this be acceptable or do you just want everything squashed into librte_ether except for net/ixgbe?
> > > [...]
> > > > + if (param != NULL)
> > > > + dev_cb.cb_arg = (void *) param;
> > >
> > > You are overriding the user parameter.
> >
> > Yes, we want to update the user parameter for the
> > RTE_ETH_EVENT_VF_MBOX
I have renamed param to cb_arg to make it clearer what is happening.
> > > As it is only for a new event, it can be described in the register
> > > API that the user param won't be returned for this event.
> >
> > I will add a description in the rte_eth_dev_callback_register function.
> >
> > > But a better design would be to add a new parameter to the callback.
> > > However it will be an API breakage.
> >
> > I do not want to break the ABI at this point.
>
> Yes, but it can be considered for a later change.
Yes, ok
> > > > + RTE_ETH_EVENT_VF_MBOX, /**< PF mailbox processing callback */
> > >
> > > Sorry I do not parse well this line.
> > > The event name is VF_MBOX and the comment is about the callback
> > > processing this event on PF side?
> > > I would suggest this kind of comment: "message from VF received by PF"
> >
> > Ok.
> >
> > >
> > > [...]
> > > > * Pointer to struct rte_eth_dev.
> > > > * @param event
> > > > * Eth device interrupt event type.
> > > > + * @param param
> > > > + * Parameter to pass back to user application.
> > > > + * Allows the user application to decide if a particular
> > > > + function
> > > > + * is permitted.
> > >
> > > In a more generic case, the parameter gives some details about the
> event.
>
> Please consider a rewording here, thanks.
I have reworded here, I hope it is clearer.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v5 01/13] librte_ether: modify internal callback function
From: Thomas Monjalon @ 2016-10-06 15:41 UTC (permalink / raw)
To: Iremonger, Bernard
Cc: dev, Shah, Rahul R, Lu, Wenzhuo, az5157@att.com,
jerin.jacob@caviumnetworks.com
In-Reply-To: <8CEF83825BEC744B83065625E567D7C21A08F8EF@IRSMSX108.ger.corp.intel.com>
2016-10-06 15:32, Iremonger, Bernard:
> > > > 2016-10-06 12:26, Bernard Iremonger:
> > > > > void
> > > > > _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > > > > - enum rte_eth_event_type event)
> > > > > + enum rte_eth_event_type event, void *param)
> > > >
> > > > You need to squash the patches updating the calls to this function.
> > > > Otherwise, this patch does not compile.
> > >
> > > I will have to squash everything into one patch, separate patches will not
> > compile.
> >
> > No you can keep a separate patch for the VF event in ixgbe.
>
> I have 4 patches at present
>
> librte_ether
> net/ixgbe
> drivers/net
> app/test
>
> Would this be acceptable or do you just want everything squashed into librte_ether except for net/ixgbe?
You can test the compilation of each patch with this command:
git rebase -i origin/master -x scripts/test-build.sh
or just:
git rebase -i origin/master -x make
I think you need to squash drivers/net (adding just NULL param) and
app/test in ethdev patch.
^ permalink raw reply
* Re: [PATCH v5 01/13] librte_ether: modify internal callback function
From: Iremonger, Bernard @ 2016-10-06 15:45 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, Shah, Rahul R, Lu, Wenzhuo, az5157@att.com,
jerin.jacob@caviumnetworks.com
In-Reply-To: <4741428.MAME74GNI7@xps13>
Hi Thomas,
> Subject: Re: [dpdk-dev] [PATCH v5 01/13] librte_ether: modify internal
> callback function
>
> 2016-10-06 15:32, Iremonger, Bernard:
> > > > > 2016-10-06 12:26, Bernard Iremonger:
> > > > > > void
> > > > > > _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > > > > > - enum rte_eth_event_type event)
> > > > > > + enum rte_eth_event_type event, void *param)
> > > > >
> > > > > You need to squash the patches updating the calls to this function.
> > > > > Otherwise, this patch does not compile.
> > > >
> > > > I will have to squash everything into one patch, separate patches
> > > > will not
> > > compile.
> > >
> > > No you can keep a separate patch for the VF event in ixgbe.
> >
> > I have 4 patches at present
> >
> > librte_ether
> > net/ixgbe
> > drivers/net
> > app/test
> >
> > Would this be acceptable or do you just want everything squashed into
> librte_ether except for net/ixgbe?
>
> You can test the compilation of each patch with this command:
> git rebase -i origin/master -x scripts/test-build.sh or just:
> git rebase -i origin/master -x make
>
> I think you need to squash drivers/net (adding just NULL param) and
> app/test in ethdev patch.
They do not compile if applied one by one.
Ok, I will squash drivers/net and app/test into the librte_ether patch.
Regards,
Bernard.
^ permalink raw reply
* [PATCH v6 0/2] modify callback for VF management
From: Bernard Iremonger @ 2016-10-06 16:48 UTC (permalink / raw)
To: dev, rahul.r.shah, wenzhuo.lu, az5157, jerin.jacob; +Cc: Bernard Iremonger
In-Reply-To: <1475753191-17391-1-git-send-email-bernard.iremonger@intel.com>
This patchset modifies the callback function for VF management.
A third parameter has been added to the _rte_eth_dev_callback_process
function. All references to this function have been updated.
Changes have been made to the ixgbe_rcv_msg_from_vf function to
use the new callback parameter.
This patchset depends on the following patch.
http://dpdk.org/dev/patchwork/patch/16296/
[dpdk-dev,v6,1/2] net/ixgbe: add API's for VF management
These patches were part of the RFC PATCH v2 of the above patchset,
but were dropped from the v3 patchset.
Changes in v5:
Rebased to latest master.
Squashed patches down to two patches.
Renamed parameter to _rte_eth_dev_callback_process function.
Updated API descriptions.
Changes in v5:
Rebased to latest master.
Added parameter to the _rte_eth_dev_callback_process function
Removed two new callback functions which were added previously.
Updated all calls to the _rte_eth_dev_callback_process function.
Changes in v4:
Rebased to latest master.
Moved the callback parameter structure from the ethdev to the ixgbe PMD.
Changes in v3:
Rebased to latest master.
Submitted as a seperate patchset.
Moved the response enums from the ethdev to the ixgbe PMD.
Changes in v2:
Rebased to latest master.
Bernard Iremonger (2):
librte_ether: modify internal callback function
net/ixgbe: add callback to user app on VF to PF mbox msg
app/test/virtual_pmd.c | 2 +-
drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++---
drivers/net/e1000/em_ethdev.c | 2 +-
drivers/net/e1000/igb_ethdev.c | 4 ++--
drivers/net/enic/enic_main.c | 2 +-
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++--
drivers/net/ixgbe/ixgbe_pf.c | 42 +++++++++++++++++++++++++++++-----
drivers/net/ixgbe/rte_pmd_ixgbe.h | 17 ++++++++++++++
drivers/net/mlx4/mlx4.c | 4 ++--
drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
drivers/net/nfp/nfp_net.c | 2 +-
drivers/net/thunderx/nicvf_ethdev.c | 2 +-
drivers/net/vhost/rte_eth_vhost.c | 6 ++---
drivers/net/virtio/virtio_ethdev.c | 2 +-
lib/librte_ether/rte_ethdev.c | 5 +++-
lib/librte_ether/rte_ethdev.h | 12 +++++++++-
18 files changed, 91 insertions(+), 31 deletions(-)
--
2.9.0
^ permalink raw reply
* [PATCH v6 1/2] librte_ether: modify internal callback function
From: Bernard Iremonger @ 2016-10-06 16:48 UTC (permalink / raw)
To: dev, rahul.r.shah, wenzhuo.lu, az5157, jerin.jacob; +Cc: Bernard Iremonger
In-Reply-To: <1475753191-17391-1-git-send-email-bernard.iremonger@intel.com>
add cb_arg parameter to the _rte_eth_dev_callback_process function.
Adding a parameter to this function allows passing information
to the application when an eth device event occurs such as
a VF to PF message.
This allows the application to decide if a particular function
is permitted.
drivers/net: add parameter to callback process function
add parameter to call of _rte_eth_dev_callback_process function
in the following PMD's:
net/bonding
net/e1000
net/i40e
net/mlx4
net/mlx5
net/nfp
net/thunderx
net/vhost
net/virtio
net/enic
app/test: add parameter to callback process function
add parameter to call of _rte_eth_dev_callback_process function
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Signed-off-by: Alex Zelezniak <az5157@att.com>
---
app/test/virtual_pmd.c | 2 +-
drivers/net/bonding/rte_eth_bond_pmd.c | 6 +++---
drivers/net/e1000/em_ethdev.c | 2 +-
drivers/net/e1000/igb_ethdev.c | 4 ++--
drivers/net/enic/enic_main.c | 2 +-
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
drivers/net/mlx4/mlx4.c | 4 ++--
drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
drivers/net/nfp/nfp_net.c | 2 +-
drivers/net/thunderx/nicvf_ethdev.c | 2 +-
drivers/net/vhost/rte_eth_vhost.c | 6 +++---
drivers/net/virtio/virtio_ethdev.c | 2 +-
lib/librte_ether/rte_ethdev.c | 5 ++++-
lib/librte_ether/rte_ethdev.h | 12 +++++++++++-
15 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index 4831113..65b44c6 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -485,7 +485,7 @@ virtual_ethdev_simulate_link_status_interrupt(uint8_t port_id,
vrtl_eth_dev->data->dev_link.link_status = link_status;
- _rte_eth_dev_callback_process(vrtl_eth_dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(vrtl_eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
int
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index fb4050c..8d510e3 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1958,7 +1958,7 @@ bond_ethdev_delayed_lsc_propagation(void *arg)
return;
_rte_eth_dev_callback_process((struct rte_eth_dev *)arg,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
}
void
@@ -2079,7 +2079,7 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
(void *)bonded_eth_dev);
else
_rte_eth_dev_callback_process(bonded_eth_dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
} else {
if (internals->link_down_delay_ms > 0)
@@ -2088,7 +2088,7 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
(void *)bonded_eth_dev);
else
_rte_eth_dev_callback_process(bonded_eth_dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
}
}
}
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index f767e1c..e13e858 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1599,7 +1599,7 @@ eth_em_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
eth_em_interrupt_get_status(dev);
eth_em_interrupt_action(dev);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
static int
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 5a1a83e..d9ab2df 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2683,7 +2683,7 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev)
E1000_WRITE_REG(hw, E1000_TCTL, tctl);
E1000_WRITE_REG(hw, E1000_RCTL, rctl);
E1000_WRITE_FLUSH(hw);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
return 0;
@@ -2743,7 +2743,7 @@ void igbvf_mbx_process(struct rte_eth_dev *dev)
/* PF reset VF event */
if (in_msg == E1000_PF_CONTROL_MSG)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET, NULL);
}
static int
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 15a05b4..fb72491 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -436,7 +436,7 @@ enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
vnic_intr_return_all_credits(&enic->intr);
enic_link_update(enic);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
enic_log_q_error(enic);
}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 697800e..d947760 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -5510,7 +5510,7 @@ i40e_dev_interrupt_delayed_handler(void *param)
/* 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);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
i40e_pf_enable_irq0(hw);
rte_intr_enable(&(dev->pci_dev->intr_handle));
@@ -5594,7 +5594,7 @@ i40e_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
return;
else
_rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
}
done:
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 34eb274..bd89cd9 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1341,7 +1341,7 @@ i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
switch (pf_msg->event) {
case I40E_VIRTCHNL_EVENT_RESET_IMPENDING:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event\n");
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET, NULL);
break;
case I40E_VIRTCHNL_EVENT_LINK_CHANGE:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event\n");
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 1553b2e..3e57cca 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -5448,7 +5448,7 @@ mlx4_dev_link_status_handler(void *arg)
ret = priv_dev_link_status_handler(priv, dev);
priv_unlock(priv);
if (ret)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
/**
@@ -5471,7 +5471,7 @@ mlx4_dev_interrupt_handler(struct rte_intr_handle *intr_handle, void *cb_arg)
ret = priv_dev_link_status_handler(priv, dev);
priv_unlock(priv);
if (ret)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
/**
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index bf4232a..c76e754 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1069,7 +1069,7 @@ mlx5_dev_link_status_handler(void *arg)
ret = priv_dev_link_status_handler(priv, dev);
priv_unlock(priv);
if (ret)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
/**
@@ -1092,7 +1092,7 @@ mlx5_dev_interrupt_handler(struct rte_intr_handle *intr_handle, void *cb_arg)
ret = priv_dev_link_status_handler(priv, dev);
priv_unlock(priv);
if (ret)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
/**
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index d526f34..a2b9056 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1219,7 +1219,7 @@ nfp_net_dev_interrupt_delayed_handler(void *param)
struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
nfp_net_link_update(dev, 0);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
nfp_net_dev_link_status_print(dev);
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index b758c9f..c61e171 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -108,7 +108,7 @@ nicvf_interrupt(void *arg)
nicvf_set_eth_link_status(nic,
&nic->eth_dev->data->dev_link);
_rte_eth_dev_callback_process(nic->eth_dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
}
rte_eal_alarm_set(NICVF_INTR_POLL_INTERVAL_MS * 1000,
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index c1d09a0..18a0c10 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -559,7 +559,7 @@ new_device(int vid)
RTE_LOG(INFO, PMD, "New connection established\n");
- _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
return 0;
}
@@ -626,7 +626,7 @@ destroy_device(int vid)
RTE_LOG(INFO, PMD, "Connection closed\n");
- _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
static int
@@ -655,7 +655,7 @@ vring_state_changed(int vid, uint16_t vring, int enable)
RTE_LOG(INFO, PMD, "vring%u is %s\n",
vring, enable ? "enabled" : "disabled");
- _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_QUEUE_STATE);
+ _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_QUEUE_STATE, NULL);
return 0;
}
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b4dfc0a..2f0065a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1103,7 +1103,7 @@ virtio_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
if (isr & VIRTIO_PCI_ISR_CONFIG) {
if (virtio_dev_link_update(dev, 0) == 0)
_rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_LSC);
+ RTE_ETH_EVENT_INTR_LSC, NULL);
}
}
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index c517e88..13ba70d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2508,7 +2508,7 @@ rte_eth_dev_callback_unregister(uint8_t port_id,
void
_rte_eth_dev_callback_process(struct rte_eth_dev *dev,
- enum rte_eth_event_type event)
+ enum rte_eth_event_type event, void *cb_arg)
{
struct rte_eth_dev_callback *cb_lst;
struct rte_eth_dev_callback dev_cb;
@@ -2519,6 +2519,9 @@ _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
continue;
dev_cb = *cb_lst;
cb_lst->active = 1;
+ if (cb_arg != NULL)
+ dev_cb.cb_arg = (void *) cb_arg;
+
rte_spinlock_unlock(&rte_eth_dev_cb_lock);
dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
dev_cb.cb_arg);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 7218b6f..775e899 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3026,6 +3026,7 @@ enum rte_eth_event_type {
/**< queue state event (enabled/disabled) */
RTE_ETH_EVENT_INTR_RESET,
/**< reset interrupt event, sent to VF on PF reset */
+ RTE_ETH_EVENT_VF_MBOX, /**< message from the VF received by PF */
RTE_ETH_EVENT_MAX /**< max value of this enum */
};
@@ -3047,6 +3048,11 @@ typedef void (*rte_eth_dev_cb_fn)(uint8_t port_id, \
* @param cb_arg
* Pointer to the parameters for the registered callback.
*
+ * The cb_arg must not be NULL if the application requires
+ * data to be returned when the callback is processed.
+ * For the RTE_ETH_EVENT_VF_MBOX data is returned to the
+ * application.
+ *
* @return
* - On success, zero.
* - On failure, a negative value.
@@ -3085,12 +3091,16 @@ int rte_eth_dev_callback_unregister(uint8_t port_id,
* Pointer to struct rte_eth_dev.
* @param event
* Eth device interrupt event type.
+ * @param cb_arg
+ * Update callback parameter to pass data back to user application.
+ * This allows the user application to decide if a particular function
+ * is permitted or not.
*
* @return
* void
*/
void _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
- enum rte_eth_event_type event);
+ enum rte_eth_event_type event, void *cb_arg);
/**
* When there is no rx packet coming in Rx Queue for a long time, we can
--
2.9.0
^ permalink raw reply related
* [PATCH v6 2/2] net/ixgbe: add callback to user app on VF to PF mbox msg
From: Bernard Iremonger @ 2016-10-06 16:48 UTC (permalink / raw)
To: dev, rahul.r.shah, wenzhuo.lu, az5157, jerin.jacob; +Cc: Bernard Iremonger
In-Reply-To: <1475753191-17391-1-git-send-email-bernard.iremonger@intel.com>
call _rte_eth_dev_callback_process from ixgbe_rcv_msg_from_vf function.
The callback asks the user application if it is allowed to perform
the function.
If the cb_param.retval is RTE_PMD_IXGBE_MB_EVENT_PROCEED then continue,
if 0, do nothing and send ACK to VF
if > 1, do nothing and send NAK to VF.
Signed-off-by: Alex Zelezniak <az5157@att.com>
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++--
drivers/net/ixgbe/ixgbe_pf.c | 42 +++++++++++++++++++++++++++++++++------
drivers/net/ixgbe/rte_pmd_ixgbe.h | 17 ++++++++++++++++
3 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 55a82d3..91db2c3 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3559,7 +3559,7 @@ ixgbe_dev_interrupt_delayed_handler(void *param)
ixgbe_dev_link_update(dev, 0);
intr->flags &= ~IXGBE_FLAG_NEED_LINK_UPDATE;
ixgbe_dev_link_status_print(dev);
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
}
PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
@@ -7525,7 +7525,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
/* PF reset VF event */
if (in_msg == IXGBE_PF_CONTROL_MSG)
- _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET, NULL);
}
static int
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 56393ff..2a177b8 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -51,6 +51,7 @@
#include "base/ixgbe_common.h"
#include "ixgbe_ethdev.h"
+#include "rte_pmd_ixgbe.h"
#define IXGBE_MAX_VFTA (128)
#define IXGBE_VF_MSG_SIZE_DEFAULT 1
@@ -660,6 +661,7 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct ixgbe_vf_info *vfinfo =
*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
+ struct rte_pmd_ixgbe_mb_event_param cb_param;
retval = ixgbe_read_mbx(hw, msgbuf, mbx_size, vf);
if (retval) {
@@ -674,27 +676,54 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
/* flush the ack before we write any messages back */
IXGBE_WRITE_FLUSH(hw);
+ /**
+ * initialise structure to send to user application
+ * will return response from user in retval field
+ */
+ cb_param.retval = RTE_PMD_IXGBE_MB_EVENT_PROCEED;
+ cb_param.vfid = vf;
+ cb_param.msg_type = msgbuf[0] & 0xFFFF;
+ cb_param.userdata = (void *)msgbuf;
+
/* perform VF reset */
if (msgbuf[0] == IXGBE_VF_RESET) {
int ret = ixgbe_vf_reset(dev, vf, msgbuf);
vfinfo[vf].clear_to_send = true;
+
+ /* notify application about VF reset */
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_VF_MBOX, &cb_param);
return ret;
}
+ /**
+ * ask user application if we allowed to perform those functions
+ * if we get cb_param.retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED
+ * then business as usual,
+ * if 0, do nothing and send ACK to VF
+ * if cb_param.retval > 1, do nothing and send NAK to VF
+ */
+ _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_VF_MBOX, &cb_param);
+
+ retval = cb_param.retval;
+
/* check & process VF to PF mailbox message */
switch ((msgbuf[0] & 0xFFFF)) {
case IXGBE_VF_SET_MAC_ADDR:
- retval = ixgbe_vf_set_mac_addr(dev, vf, msgbuf);
+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+ retval = ixgbe_vf_set_mac_addr(dev, vf, msgbuf);
break;
case IXGBE_VF_SET_MULTICAST:
- retval = ixgbe_vf_set_multicast(dev, vf, msgbuf);
+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+ retval = ixgbe_vf_set_multicast(dev, vf, msgbuf);
break;
case IXGBE_VF_SET_LPE:
- retval = ixgbe_set_vf_lpe(dev, vf, msgbuf);
+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+ retval = ixgbe_set_vf_lpe(dev, vf, msgbuf);
break;
case IXGBE_VF_SET_VLAN:
- retval = ixgbe_vf_set_vlan(dev, vf, msgbuf);
+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+ retval = ixgbe_vf_set_vlan(dev, vf, msgbuf);
break;
case IXGBE_VF_API_NEGOTIATE:
retval = ixgbe_negotiate_vf_api(dev, vf, msgbuf);
@@ -704,7 +733,8 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
msg_size = IXGBE_VF_GET_QUEUE_MSG_SIZE;
break;
case IXGBE_VF_UPDATE_XCAST_MODE:
- retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
+ if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+ retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
break;
default:
PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x", (unsigned)msgbuf[0]);
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h b/drivers/net/ixgbe/rte_pmd_ixgbe.h
index 33b5b2d..2f6cf46 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
@@ -181,4 +181,21 @@ int rte_pmd_ixgbe_set_vf_split_drop_en(uint8_t port, uint16_t vf, uint8_t on);
*/
int
rte_pmd_ixgbe_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on);
+
+/**
+ * Response sent back to ixgbe driver from user app after callback
+ */
+enum rte_pmd_ixgbe_mb_event_rsp {
+ RTE_PMD_IXGBE_MB_EVENT_NOOP_ACK, /**< skip mbox request and ACK */
+ RTE_PMD_IXGBE_MB_EVENT_NOOP_NACK, /**< skip mbox request and NACK */
+ RTE_PMD_IXGBE_MB_EVENT_PROCEED, /**< proceed with mbox request */
+ RTE_PMD_IXGBE_MB_EVENT_MAX /**< max value of this enum */
+};
+
+struct rte_pmd_ixgbe_mb_event_param {
+ uint16_t vfid;
+ uint16_t msg_type;
+ uint16_t retval;
+ void *userdata;
+};
#endif /* _PMD_IXGBE_H_ */
--
2.9.0
^ permalink raw reply related
* [PATCH] vhost: Only access header if offloading is supported in dequeue path
From: Maxime Coquelin @ 2016-10-06 17:00 UTC (permalink / raw)
To: yuanhan.liu, dev
Cc: mst, jianfeng.tan, olivier.matz, stephen, Maxime Coquelin
If offloading features are not negotiated, parsing the virtio header
is not needed.
Micro-benchmark with testpmd shows that the gain is +4% with indirect
descriptors, +1% when using direct descriptors.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/librte_vhost/virtio_net.c | 47 ++++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index a59c39b..5d51693 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -548,6 +548,18 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id,
return virtio_dev_rx(dev, queue_id, pkts, count);
}
+static inline bool
+virtio_net_with_host_offload(struct virtio_net *dev)
+{
+ if (dev->features &
+ (VIRTIO_NET_F_CSUM | VIRTIO_NET_F_HOST_ECN |
+ VIRTIO_NET_F_HOST_TSO4 | VIRTIO_NET_F_HOST_TSO6 |
+ VIRTIO_NET_F_HOST_UFO))
+ return true;
+
+ return false;
+}
+
static void
parse_ethernet(struct rte_mbuf *m, uint16_t *l4_proto, void **l4_hdr)
{
@@ -600,6 +612,9 @@ vhost_dequeue_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *m)
void *l4_hdr = NULL;
struct tcp_hdr *tcp_hdr = NULL;
+ if (hdr->flags == 0 || hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
+ return;
+
parse_ethernet(m, &l4_proto, &l4_hdr);
if (hdr->flags == VIRTIO_NET_HDR_F_NEEDS_CSUM) {
if (hdr->csum_start == (m->l2_len + m->l3_len)) {
@@ -684,12 +699,12 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
struct rte_mempool *mbuf_pool)
{
struct vring_desc *desc;
- uint64_t desc_addr;
+ uint64_t desc_addr = 0;
uint32_t desc_avail, desc_offset;
uint32_t mbuf_avail, mbuf_offset;
uint32_t cpy_len;
struct rte_mbuf *cur = m, *prev = m;
- struct virtio_net_hdr *hdr;
+ struct virtio_net_hdr *hdr = NULL;
/* A counter to avoid desc dead loop chain */
uint32_t nr_desc = 1;
@@ -698,12 +713,14 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
(desc->flags & VRING_DESC_F_INDIRECT))
return -1;
- desc_addr = gpa_to_vva(dev, desc->addr);
- if (unlikely(!desc_addr))
- return -1;
+ if (virtio_net_with_host_offload(dev)) {
+ desc_addr = gpa_to_vva(dev, desc->addr);
+ if (unlikely(!desc_addr))
+ return -1;
- hdr = (struct virtio_net_hdr *)((uintptr_t)desc_addr);
- rte_prefetch0(hdr);
+ hdr = (struct virtio_net_hdr *)((uintptr_t)desc_addr);
+ rte_prefetch0(hdr);
+ }
/*
* A virtio driver normally uses at least 2 desc buffers
@@ -720,18 +737,24 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
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 {
+ if (!desc_addr) {
+ desc_addr = gpa_to_vva(dev, desc->addr);
+ if (unlikely(!desc_addr))
+ return -1;
+ }
+
desc_avail = desc->len - dev->vhost_hlen;
desc_offset = dev->vhost_hlen;
}
+ rte_prefetch0((void *)(uintptr_t)(desc_addr + desc_offset));
+
+ PRINT_PACKET(dev, (uintptr_t)(desc_addr + desc_offset), desc_avail, 0);
+
mbuf_offset = 0;
mbuf_avail = m->buf_len - RTE_PKTMBUF_HEADROOM;
while (1) {
@@ -795,7 +818,7 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vring_desc *descs,
prev->data_len = mbuf_offset;
m->pkt_len += mbuf_offset;
- if (hdr->flags != 0 || hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE)
+ if (virtio_net_with_host_offload(dev))
vhost_dequeue_offload(hdr, m);
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] vhost: Only access header if offloading is supported in dequeue path
From: Maxime Coquelin @ 2016-10-06 17:06 UTC (permalink / raw)
To: yuanhan.liu, dev; +Cc: mst, jianfeng.tan, olivier.matz, stephen
In-Reply-To: <1475773241-5714-1-git-send-email-maxime.coquelin@redhat.com>
On 10/06/2016 07:00 PM, Maxime Coquelin wrote:
> If offloading features are not negotiated, parsing the virtio header
> is not needed.
>
> Micro-benchmark with testpmd shows that the gain is +4% with indirect
> descriptors, +1% when using direct descriptors.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/librte_vhost/virtio_net.c | 47 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index a59c39b..5d51693 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -548,6 +548,18 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id,
> return virtio_dev_rx(dev, queue_id, pkts, count);
> }
>
> +static inline bool
> +virtio_net_with_host_offload(struct virtio_net *dev)
> +{
> + if (dev->features &
> + (VIRTIO_NET_F_CSUM | VIRTIO_NET_F_HOST_ECN |
> + VIRTIO_NET_F_HOST_TSO4 | VIRTIO_NET_F_HOST_TSO6 |
> + VIRTIO_NET_F_HOST_UFO))
> + return true;
> +
> + return false;
> +}
> +
> static void
> parse_ethernet(struct rte_mbuf *m, uint16_t *l4_proto, void **l4_hdr)
> {
> @@ -600,6 +612,9 @@ vhost_dequeue_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *m)
> void *l4_hdr = NULL;
> struct tcp_hdr *tcp_hdr = NULL;
>
> + if (hdr->flags == 0 || hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
> + return;
> +
Oops, just noticed I forgot to amend a fix I did.
Of course, the above test should be:
if (hdr->flags == 0 && hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
It will be fixed in the v2.
Regards,
Maxime
^ permalink raw reply
* [PATCH v3 2/4] app/test: remove pointless for loop
From: Fiona Trahe @ 2016-10-06 17:34 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, akhil.goyal
In-Reply-To: <20160926163300.22990-1-akhil.goyal@nxp.com>
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
app/test/test_cryptodev.c | 49 +++++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 8f40dea..db2f23c 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -357,37 +357,35 @@ testsuite_setup(void)
return TEST_FAILED;
/* Set up all the qps on the first of the valid devices found */
- for (i = 0; i < 1; i++) {
- dev_id = ts_params->valid_devs[i];
- rte_cryptodev_info_get(dev_id, &info);
+ dev_id = ts_params->valid_devs[0];
- /*
- * Since we can't free and re-allocate queue memory always set
- * the queues on this device up to max size first so enough
- * memory is allocated for any later re-configures needed by
- * other tests
- */
+ rte_cryptodev_info_get(dev_id, &info);
- ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
- ts_params->conf.socket_id = SOCKET_ID_ANY;
- ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
+ /*
+ * Since we can't free and re-allocate queue memory always set
+ * the queues on this device up to max size first so enough
+ * memory is allocated for any later re-configures needed by
+ * other tests
+ */
- TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id,
- &ts_params->conf),
- "Failed to configure cryptodev %u with %u qps",
- dev_id, ts_params->conf.nb_queue_pairs);
+ ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
+ ts_params->conf.socket_id = SOCKET_ID_ANY;
+ ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
- ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
+ TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id,
+ &ts_params->conf),
+ "Failed to configure cryptodev %u with %u qps",
+ dev_id, ts_params->conf.nb_queue_pairs);
- for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
- TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
- dev_id, qp_id, &ts_params->qp_conf,
- rte_cryptodev_socket_id(dev_id)),
- "Failed to setup queue pair %u on "
- "cryptodev %u",
- qp_id, dev_id);
- }
+ ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
+
+ for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
+ TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
+ dev_id, qp_id, &ts_params->qp_conf,
+ rte_cryptodev_socket_id(dev_id)),
+ "Failed to setup queue pair %u on cryptodev %u",
+ qp_id, dev_id);
}
return TEST_SUCCESS;
--
2.5.0
^ permalink raw reply related
* [PATCH v3 0/4] remove hard-coding of crypto num qps and cleanup
From: Fiona Trahe @ 2016-10-06 17:34 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, akhil.goyal
In-Reply-To: <20160926163300.22990-1-akhil.goyal@nxp.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]
ts_params->conf.nb_queue_pairs should not be hard coded with device
specific number. It should be retrieved from the device info.
Any test which changes it should restore it to orig value.
Also related cleanup of test code setting number and size of
queue-pairs on a device, e.g.
* Removed irrelevant “for” loop – was hardcoded to only loop once.
* Removed obsolete comment re inability to free and re-allocate queu memory
and obsolete workaround for it which used to create maximum size queues.
And added freeing of ring memory on queue-pair release in aesni_mb PMD,
else releasing and setting up queue-pair of a different size fails.
v3:
separate out into 4 patches
v2:
Fix for broken QAT PMD unit tests exposed by v1
i.e. In test_device_configure_invalid_queue_pair_ids() after running tests
for invalid values restore original nb_queue_pairs.
Also cleanup of test code setting number and size of queue-pairs on a device
Also fix for aesni_mb PMD not freeing ring memory on qp release
Fiona Trahe (4):
crypto/aesni_mb: free ring memory on qp release in PMD
app/test: remove pointless for loop
app/test: cleanup unnecessary ring size setup
app/test: remove hard-coding of crypto num qps
Akhil Goyal (1):
app/test: remove hard-coding of crypto num qps
app/test/test_cryptodev.c | 53 ++++++++++----------------
app/test/test_cryptodev_perf.c | 19 +--------
drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 10 ++++-
3 files changed, 31 insertions(+), 51 deletions(-)
--
2.5.0
^ permalink raw reply
* [PATCH v3 3/4] app/test: cleanup unnecessary ring size setup
From: Fiona Trahe @ 2016-10-06 17:34 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, akhil.goyal
In-Reply-To: <20160926163300.22990-1-akhil.goyal@nxp.com>
Removed obsolete comments re inability to free and re-allocate
queue memory and obsolete workaround for it
which used to create maximum size queues first, then later
create smaller queues.
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
app/test/test_cryptodev.c | 15 +--------------
app/test/test_cryptodev_perf.c | 17 +----------------
2 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index db2f23c..e0b0252 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -362,13 +362,6 @@ testsuite_setup(void)
rte_cryptodev_info_get(dev_id, &info);
- /*
- * Since we can't free and re-allocate queue memory always set
- * the queues on this device up to max size first so enough
- * memory is allocated for any later re-configures needed by
- * other tests
- */
-
ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
ts_params->conf.socket_id = SOCKET_ID_ANY;
ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
@@ -378,7 +371,7 @@ testsuite_setup(void)
"Failed to configure cryptodev %u with %u qps",
dev_id, ts_params->conf.nb_queue_pairs);
- ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
+ ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
@@ -430,12 +423,6 @@ ut_setup(void)
"Failed to configure cryptodev %u",
ts_params->valid_devs[0]);
- /*
- * Now reconfigure queues to size we actually want to use in this
- * test suite.
- */
- ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
-
for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs ; qp_id++) {
TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
ts_params->valid_devs[0], qp_id,
diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
index e8fc097..27d8cf8 100644
--- a/app/test/test_cryptodev_perf.c
+++ b/app/test/test_cryptodev_perf.c
@@ -426,9 +426,7 @@ testsuite_setup(void)
/*
* Using Crypto Device Id 0 by default.
- * Since we can't free and re-allocate queue memory always set the queues
- * on this device up to max size first so enough memory is allocated for
- * any later re-configures needed by other tests
+ * Set up all the qps on this device
*/
rte_cryptodev_info_get(ts_params->dev_id, &info);
@@ -442,19 +440,6 @@ testsuite_setup(void)
"Failed to configure cryptodev %u",
ts_params->dev_id);
-
- ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
-
- for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs ; qp_id++) {
- TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
- ts_params->dev_id, qp_id,
- &ts_params->qp_conf,
- rte_cryptodev_socket_id(ts_params->dev_id)),
- "Failed to setup queue pair %u on cryptodev %u",
- qp_id, ts_params->dev_id);
- }
-
- /*Now reconfigure queues to size we actually want to use in this testsuite.*/
ts_params->qp_conf.nb_descriptors = PERF_NUM_OPS_INFLIGHT;
for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs ; qp_id++) {
--
2.5.0
^ permalink raw reply related
* [PATCH v3 1/4] crypto/aesni_mb: free ring memory on qp release in PMD
From: Fiona Trahe @ 2016-10-06 17:34 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, akhil.goyal
In-Reply-To: <20160926163300.22990-1-akhil.goyal@nxp.com>
Free ring memory on queue_pair release, else
releasing and setting up queue-pair of a different size fails.
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index d3c46ac..3d49e2a 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -311,8 +311,14 @@ aesni_mb_pmd_info_get(struct rte_cryptodev *dev,
static int
aesni_mb_pmd_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
{
- if (dev->data->queue_pairs[qp_id] != NULL) {
- rte_free(dev->data->queue_pairs[qp_id]);
+ struct aesni_mb_qp *qp = dev->data->queue_pairs[qp_id];
+ struct rte_ring *r = NULL;
+
+ if (qp != NULL) {
+ r = rte_ring_lookup(qp->name);
+ if (r)
+ rte_ring_free(r);
+ rte_free(qp);
dev->data->queue_pairs[qp_id] = NULL;
}
return 0;
--
2.5.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