[dpdk-dev] buf->hash.rss always empty with i40e
Hi all, No matter the number of queues or the ETH_RSS_X parameter the mbuf->hash.rss field is empty using XL710 NICs. The exact same configuration gives me good hash value values with ixgbe/82599 cards. Any idea? I checked it is the same problem on 2.2 and 16.11. FlowDirector is not used. Thanks, Tom
Re: [dpdk-dev] [PATCH 5/5] net/virtio: fix Tso when mbuf is shared
On Tue, Jan 24, 2017 at 11:51:20AM +0100, Olivier MATZ wrote:
> On Wed, 18 Jan 2017 13:03:48 +0800, Yuanhan Liu
> wrote:
> > On Tue, Jan 17, 2017 at 12:18:25PM +0100, Olivier Matz wrote:
> > > > I hope I could have time to dig this further, since, honestly, I
> > > > don't quite like this patch: it makes things un-maintainable.
> > >
> > > Well, I'm not that proud of the patch, but that's the best solution
> > > I've found. Nevertheless saying it makes things un-maintainable
> > > looks a bit excessive to me :)
> >
> > Aha... really sorry about that!
> >
> > But honestly, I'd say again, it makes thing more complex, just for
> > fixing a corner and rare issue. I'd try to avoid that.
> >
> > >
> > > The option of reallocating a mbuf, copy and fix network headers in
> > > it looks even more complex to me (that was my first approach).
> > >
> > > > Besides that, I think we have similar issue with nic drivers. See
> > > > the rte_net_intel_cksum_flags_prepare() function introduced at
> > > > commit 4fb7e803eb1a ("ethdev: add Tx preparation").
> > >
> > > Yes, that was discussed a bit. See [1] and the subsequent mails.
> > > http://dpdk.org/ml/archives/dev/2016-December/051014.html
> >
> > Thanks for the info, and I'm pretty Okay with that.
> >
> > > My opinion is that tx_burst() should not change the mbuf data, it's
> > > always been like this. For Intel NICs, there is no issue since the
> > > DPDK API is derived from Intel NICs API, so there is no fix to do
> > > in the mbuf data.
> > >
> > > For tx_prepare(), it's explicitly said that it can update the data.
> > > If tx_prepare() becomes mandatory, it will naturally fix this issue
> > > without modifying the driver, because the phdr csum calculation
> > > will be done in tx_prepare().
> > >
> > > An alternative is to mark this as a known issue for now, and wait
> > > until tx_prepare() is mandatory.
> >
> > I see no reason to wait. Though my understanding is it may not be a
> > mandatory so far, but user is supposed to calculate the pseudo-header
> > checksum by themself before. Now they have one more option:
> > tx_prepare.
> >
> > That means, in either way, user has to do some extra works to make
> > TSO work (either by themself or call tx_prepare). So I don't think
> > it'd be an issue?
>
> Yes sounds good. I'll check how a tx_prepare() could be implemented for
> virtio, in order to fix this issue.
Thanks.
> In the meanwhile, for the 17.02, I think it could be good to highlight
> the problem in the known issues, what do you think?
Yes, I think it's a good idea.
--yliu
Re: [dpdk-dev] [PATCH] ethdev: fix wrong memset
On Wed, Jan 25, 2017 at 12:16:58PM +0100, Thomas Monjalon wrote: > > As I understand, the main problem is that rte_eth_devices[] is local, > > while rte_eth_dev_data points to the shared memory array. > > And rte_eth_dev_allocate() assumes that if rte_eth_devices[x] is free, > > then rte_eth_dev_data[port_id] is also free. > > Which is wrong in case when primary/secondary processes have different > > devices attached. > > Another problem is that inside rte_ethdev.c we manipulate rte_eth_dev_data[] > > contents without grabbing any lock. > > Yes there are a lot of problems with the multiproc mode because it has > been implemented as a hack. Oh, right, "hacky" is the word I intended to say in my last email. > We are fixing some cases without figuring the whole picture. Agreed. > > I'll apply the patch from Remy which fixes a case where process creates I would ask you not to apply that. IMO, his patch may have "fixed" a crash he saw, but it doesn't looks like to me it really fixes anything: the driver may reference rte_eth_data belongs to another driver -- things can't be right afterwards. > vdev (local data) and, hopefully, primary does no hotplug of PCI dev. > > I'll restart this discussion with a bigger picture of what multiproc is, > and what are the issues. Great! And I'm keen to know how the multiproc is actually used in real life (and why they don't use multiple thread). Most importantly, does people really care about it? (I fixed few virtio multiproc issues that I know there are some people/companies using it, and I want to know if there are more). OTOH, an initial idea comes to my mind now is, make the port_id a global id among primary and secondary process: - same device will get same port id (my patch acertains that) - different device will get different port id --yliu
Re: [dpdk-dev] [PATCH] net/virtio-user: check value returned from malloc
On Thu, Jan 26, 2017 at 03:05:42AM +, Jianfeng Tan wrote:
> Value returned from malloc is not checked for errors before being used.
> This patch fixes following coverity issue.
>
> static struct vhost_memory_kernel *
> prepare_vhost_memory_kernel(void)
> {
> ...
> vm = malloc(sizeof(struct vhost_memory_kernel) +
> max_regions *
> sizeof(struct vhost_memory_region));
> ...
> >>> CID 140744:(NULL_RETURNS)
> >>> Dereferencing a null pointer "vm".
> mr = &vm->regions[k++];
>
> Fixes: e3b434818bbb ("net/virtio-user: support kernel vhost")
> Coverity issue: 140744
>
> Signed-off-by: Jianfeng Tan
Applied to dpdk-next-virtio.
--yliu
[dpdk-dev] [dpdk-announce] new members in DPDK technical board
There are three new members in the technical board. Welcome to Hemant, Jan and Yuanhan joining and increasing the board size to 9. New composition (as listed in http://dpdk.org/ml/roster/techboard): * Bruce Richardson * Hemant Agrawal * Jan Blunck * Jerin Jacob * Konstantin Ananyev * Olivier Matz * Stephen Hemminger * Thomas Monjalon * Yuanhan Liu Please keep in mind that the decision making process is primarily based on consensus. However in rare cases, the technical board can make a decision when consensus is not reached on the mailing list. The scope of this body is limited to the questions directly related to the development in the following repositories: - dpdk.git - dpdk-stable.git - dpdk-next-*.git - dpdk-ci.git - dpdk-web.git The web site will be updated to reflect the recent changes. You can use techbo...@dpdk.org to request the board discussing any specific topic, thus filling the agenda for the next board meeting.
[dpdk-dev] [PATCH] net/tap: driver closing tx interface on queue setup
The tap driver setup both rx and tx file descriptors when the
rte_eth_rx_queue_setup() causing the tx to be closed when tx setup
was called.
Signed-off-by: Keith Wiles
---
drivers/net/tap/rte_eth_tap.c | 48 ++-
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index c0afc2d..267b421 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -406,32 +406,52 @@ tap_link_update(struct rte_eth_dev *dev __rte_unused,
}
static int
-tap_setup_queue(struct rte_eth_dev *dev,
+rx_setup_queue(struct rte_eth_dev *dev,
struct pmd_internals *internals,
uint16_t qid)
{
struct rx_queue *rx = &internals->rxq[qid];
- struct tx_queue *tx = &internals->txq[qid];
int fd;
fd = rx->fd;
if (fd < 0) {
- fd = tx->fd;
+ RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
+ dev->data->name, qid);
+ fd = tun_alloc(dev->data->name);
if (fd < 0) {
- RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
- dev->data->name, qid);
- fd = tun_alloc(dev->data->name);
- if (fd < 0) {
- RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
- dev->data->name);
- return -1;
- }
+ RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
+ dev->data->name);
+ return -1;
}
}
dev->data->rx_queues[qid] = rx;
- dev->data->tx_queues[qid] = tx;
rx->fd = fd;
+
+ return fd;
+}
+
+static int
+tx_setup_queue(struct rte_eth_dev *dev,
+ struct pmd_internals *internals,
+ uint16_t qid)
+{
+ struct tx_queue *tx = &internals->txq[qid];
+ int fd;
+
+ fd = tx->fd;
+ if (fd < 0) {
+ RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
+ dev->data->name, qid);
+ fd = tun_alloc(dev->data->name);
+ if (fd < 0) {
+ RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n",
+ dev->data->name);
+ return -1;
+ }
+ }
+ dev->data->tx_queues[qid] = tx;
+
tx->fd = fd;
return fd;
@@ -469,7 +489,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
return -ENOMEM;
}
- fd = tap_setup_queue(dev, internals, rx_queue_id);
+ fd = rx_setup_queue(dev, internals, rx_queue_id);
if (fd == -1)
return -1;
@@ -493,7 +513,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
if (tx_queue_id >= internals->nb_queues)
return -1;
- ret = tap_setup_queue(dev, internals, tx_queue_id);
+ ret = tx_setup_queue(dev, internals, tx_queue_id);
if (ret == -1)
return -1;
--
2.8.0.GIT
