* Re: [PATCH 1/8] drivers/common/dpaa2: Run time assembler for Descriptor formation
From: Thomas Monjalon @ 2016-12-07 13:13 UTC (permalink / raw)
To: Akhil Goyal
Cc: dev, declan.doherty, pablo.de.lara.guarch, Hemant Agrawal,
Horia Geantă
In-Reply-To: <DB5PR0401MB1768345589F936AA5C794C37E6850@DB5PR0401MB1768.eurprd04.prod.outlook.com>
2016-12-07 11:44, Akhil Goyal:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> 2016-12-07 06:24, Akhil Goyal:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-12-05 18:25, Akhil Goyal:
> > > FLib is a library which helps in making the descriptors which is
> > > understood by NXP's SEC hardware.
> > > This patch provides header files for command words which can be used
> > > for descritptor formation.
> >
> > It seems this code is old. Does it exist as a standalone library somewhere?
> [Akhil] Let me correct here. This is not a library. This is a set of header files.
Use the name you want but please be consistent. You said above:
"FLib is a library"
and now
"This is not a library"
Funny :)
> Yes this is an old code. This is generally shipped with NXP SDK.
>
> > Where was it hosted before duplicating it in DPDK?
> [Akhil] This is part of NXP SDK and also available at git.freescale.com.
So it is not a DPDK code.
It should be a standalone library (or set of headers) and packaged as well.
> > Why do you want to have a common directory drivers/common/dpaa2/flib instead of a sub-directory in the crypto driver?
> [Akhil] I agree with your suggestion. This can be maintained within drivers/crypto as a common header files set for different NXP Architecture crypto drivers.
Yes it can be in drivers/crypto/ or not in DPDK at all.
^ permalink raw reply
* Re: [PATCH 00/13] Introducing EAL Bus-Device-Driver Model
From: Thomas Monjalon @ 2016-12-07 13:24 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: David Marchand, dev
In-Reply-To: <1697fe66-962d-0848-5e68-615249b52dad@nxp.com>
2016-12-07 18:40, Shreyansh Jain:
> Is there anything specific that you are looking for in patchset v2?
> I was thinking of:
> 0. fixing BSD compilation issue reported by CI
> 1. improving the test_pci.c
> 2. hotplugging
> 3. trying to move PCI to drives/bus/pci/linux/* and resolving how
> drivers link to it, and how EAL resources like devargs are consumed.
I am concerned about the time needed for all these changes.
Please let's make sure that the basic parts are well done and pushed, first.
That's why I suggest to postpone 1, 2 and 3 to next release if possible.
The priority is to have a clean bus model,
and if time permits, integrate the NXP driver.
^ permalink raw reply
* Re: [PATCH] vhost: allow for many vhost user ports
From: Jan Wickbom @ 2016-12-07 13:23 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: dev@dpdk.org, Patrik Andersson R
In-Reply-To: <20161207101257.GK31182@yliu-dev.sh.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: den 7 december 2016 11:13
> To: Jan Wickbom <jan.wickbom@ericsson.com>
> Cc: dev@dpdk.org; Patrik Andersson R <patrik.r.andersson@ericsson.com>
> Subject: Re: [PATCH] vhost: allow for many vhost user ports
>
> On Thu, Dec 01, 2016 at 04:26:50PM +0100, Jan Wickbom wrote:
> > static int
> > -fdset_fill(fd_set *rfset, fd_set *wfset, struct fdset *pfdset)
> > +fdset_fill(struct pollfd *rwfds, struct fdset *pfdset)
> > {
> > struct fdentry *pfdentry;
> > - int i, maxfds = -1;
> > - int num = MAX_FDS;
> > -
> > - if (pfdset == NULL)
> > - return -1;
> > + int i;
> > + int num;
> >
> > - for (i = 0; i < num; i++) {
> > + for (i = 0, num = pfdset->num; i < num; i++) {
> > pfdentry = &pfdset->fd[i];
> > - if (pfdentry->fd != -1) {
> > - int added = 0;
> > - if (pfdentry->rcb && rfset) {
> > - FD_SET(pfdentry-
> >fd, rfset);
> > - added = 1;
> > - }
> > - if (pfdentry->wcb && wfset) {
> > - FD_SET(pfdentry-
> >fd, wfset);
> > - added = 1;
> > - }
> > - if (added)
> > - maxfds = pfdentry-
> >fd < maxfds ?
> > -
> maxfds : pfdentry->fd;
> > +
> > + if (pfdentry->fd < 0) {
> > + /* Hole in the list. Move the last
> one here */
> > +
> > + *pfdentry = pfdset->fd[num - 1];
> > + pfdset->fd[num - 1].fd = -1;
> > + num =
> fdset_adjust_num(pfdset);
> > }
> > + rwfds[i].fd = pfdentry->fd;
> > + rwfds[i].events = pfdentry->rcb ? POLLIN : 0;
> > + rwfds[i].events |= pfdentry->wcb ? POLLOUT :
> 0;
>
> Another thing is we don't have to re-init this rwfds array again
> and again. Instead, we could
>
> - set it up correctly when fdset_add is invoked: set the fd and
> events.
>
> - reset revents when it's been handled at fdset_event_dispatch().
>
> - swap with the last one and shrink the array on fd delete
>
> Could you make a follow up patch for that?
I don't see how that could easily be done. The loop index, i, is a direct reference between
an entry in the rwfds array and an entry in the pfdset array. It should stay like that while we are
hanging in the poll(). If an entry in the pfdset array is removed while we are hanging in the poll()
and we then immediately replaces it with the last entry in the array we will end up in trouble if the
revent gets set for the "replaced" index. The direct reference is gone.
Or am I missing something?
/jaw
>
> Thanks.
>
> --yliu
^ permalink raw reply
* Re: [PATCH v2 09/32] rte: add APIs for VF stats get/reset
From: Ferruh Yigit @ 2016-12-07 13:52 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Qi Zhang, Thomas Monjalon
In-Reply-To: <1481081535-37448-10-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> This patch add below two APIs so that VF statistics
> can be get/clear from PF side.
> rte_eth_vf_stats_get.
> rte_eth_vf_stats_reset.
patch subject can have " ... from PF" both to be consistent with other
patches and to clarify what it does: add APIS to get/reset VF stats from PF?
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
<...>
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 9678179..8b564ee 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1271,6 +1271,15 @@ typedef int (*eth_set_vf_vlan_filter_t)(struct rte_eth_dev *dev,
> uint8_t vlan_on);
> /**< @internal Set VF VLAN pool filter */
>
> +typedef int (*eth_vf_stats_get)(struct rte_eth_dev *dev,
> + uint16_t vf,
> + struct rte_eth_stats *stats);
> +/**< @internal Get VF statistics */
> +
> +typedef int (*eth_vf_stats_reset)(struct rte_eth_dev *dev,
> + uint16_t vf);
> +/**< @internal Clear VF statistics */
> +
> typedef int (*eth_set_queue_rate_limit_t)(struct rte_eth_dev *dev,
> uint16_t queue_idx,
> uint16_t tx_rate);
> @@ -1483,6 +1492,8 @@ struct eth_dev_ops {
> eth_set_vf_rx_t set_vf_rx; /**< enable/disable a VF receive */
> eth_set_vf_tx_t set_vf_tx; /**< enable/disable a VF transmit */
> eth_set_vf_vlan_filter_t set_vf_vlan_filter; /**< Set VF VLAN filter */
> + eth_vf_stats_get vf_stats_get; /**< Get VF's statistics */
> + eth_vf_stats_reset vf_stats_reset; /**< Reset VF's statistics */
Do we really want to add more ops to the eth_dev_ops?
Although vf_stats_get & vf_stats_reset sounds generic, why not implement
these first in PMD specific manner, and more PMDs implement these, move
to the generic eth_dev_ops layer?
CC: Thomas
<...>
^ permalink raw reply
* Re: [PATCH v2 10/32] net/i40e: implement ops for VF stats get/reset
From: Ferruh Yigit @ 2016-12-07 13:59 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Qi Zhang
In-Reply-To: <1481081535-37448-11-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> This patch implement vf_stats_get and vf_stats_reset ops for i40e.
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
<...>
> +static int
> +i40e_vf_stats_get(struct rte_eth_dev *dev,
> + uint16_t vf,
> + struct rte_eth_stats *stats)
> +{
> + struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + struct i40e_vsi *vsi;
> +
> + int ret = 0;
> +
> + if (pf->vf_num <= vf) {
> + PMD_DRV_LOG(ERR, "Invalid VF id %d\n", vf);
> + return -EINVAL;
> + }
Do we need following check as it has been done in prev patches:
+ rte_eth_dev_info_get(port, &dev_info);
+
+ if (vf_id >= dev_info.max_vfs)
+ return -EINVAL;
^ permalink raw reply
* Re: [PATCH] Scheduler: add driver for scheduler crypto pmd
From: Neil Horman @ 2016-12-07 14:16 UTC (permalink / raw)
To: Declan Doherty; +Cc: Bruce Richardson, Thomas Monjalon, Fan Zhang, dev
In-Reply-To: <558a1817-9c81-5e5f-b1e2-b71934772631@intel.com>
On Wed, Dec 07, 2016 at 12:42:15PM +0000, Declan Doherty wrote:
> On 05/12/16 15:12, Neil Horman wrote:
> > On Fri, Dec 02, 2016 at 04:22:16PM +0000, Declan Doherty wrote:
> > > On 02/12/16 14:57, Bruce Richardson wrote:
> > > > On Fri, Dec 02, 2016 at 03:31:24PM +0100, Thomas Monjalon wrote:
> > > > > 2016-12-02 14:15, Fan Zhang:
> > > > > > This patch provides the initial implementation of the scheduler poll mode
> > > > > > driver using DPDK cryptodev framework.
> > > > > >
> > > > > > Scheduler PMD is used to schedule and enqueue the crypto ops to the
> > > > > > hardware and/or software crypto devices attached to it (slaves). The
> > > > > > dequeue operation from the slave(s), and the possible dequeued crypto op
> > > > > > reordering, are then carried out by the scheduler.
> > > > > >
> > > > > > The scheduler PMD can be used to fill the throughput gap between the
> > > > > > physical core and the existing cryptodevs to increase the overall
> > > > > > performance. For example, if a physical core has higher crypto op
> > > > > > processing rate than a cryptodev, the scheduler PMD can be introduced to
> > > > > > attach more than one cryptodevs.
> > > > > >
> > > > > > This initial implementation is limited to supporting the following
> > > > > > scheduling modes:
> > > > > >
> > > > > > - CRYPTO_SCHED_SW_ROUND_ROBIN_MODE (round robin amongst attached software
> > > > > > slave cryptodevs, to set this mode, the scheduler should have been
> > > > > > attached 1 or more software cryptodevs.
> > > > > >
> > > > > > - CRYPTO_SCHED_HW_ROUND_ROBIN_MODE (round robin amongst attached hardware
> > > > > > slave cryptodevs (QAT), to set this mode, the scheduler should have
> > > > > > been attached 1 or more QATs.
> > > > >
> > > > > Could it be implemented on top of the eventdev API?
> > > > >
> > > > Not really. The eventdev API is for different types of scheduling
> > > > between multiple sources that are all polling for packets, compared to
> > > > this, which is more analgous - as I understand it - to the bonding PMD
> > > > for ethdev.
> > > >
> > > > To make something like this work with an eventdev API you would need to
> > > > use one of the following models:
> > > > * have worker cores for offloading packets to the different crypto
> > > > blocks pulling from the eventdev APIs. This would make it difficult to
> > > > do any "smart" scheduling of crypto operations between the blocks,
> > > > e.g. that one crypto instance may be better at certain types of
> > > > operations than another.
> > > > * move the logic in this driver into an existing eventdev instance,
> > > > which uses the eventdev api rather than the crypto APIs and so has an
> > > > extra level of "structure abstraction" that has to be worked though.
> > > > It's just not really a good fit.
> > > >
> > > > So for this workload, I believe the pseudo-cryptodev instance is the
> > > > best way to go.
> > > >
> > > > /Bruce
> > > >
> > >
> > >
> > > As Bruce says this is much more analogous to the ethdev bonding driver, the
> > > main idea is to allow different crypto op scheduling mechanisms to be
> > > defined transparently to an application. This could be load-balancing across
> > > multiple hw crypto devices, or having a software crypto device to act as a
> > > backup device for a hw accelerator if it becomes oversubscribed. I think the
> > > main advantage of a crypto-scheduler approach means that the data path of
> > > the application doesn't need to have any knowledge that scheduling is
> > > happening at all, it is just using a different crypto device id, which is
> > > then manages the distribution of crypto work.
> > >
> > >
> > >
> > This is a good deal like the bonding pmd, and so from a certain standpoint it
> > makes sense to do this, but whereas the bonding pmd is meant to create a single
> > path to a logical network over several physical networks, this pmd really only
> > focuses on maximizing througput, and for that we already have tools. As Thomas
> > mentions, there is the eventdev library, but from my view the distributor
> > library already fits this bill. It already is a basic framework to process
> > mbufs in parallel according to whatever policy you want to implement, which
> > sounds like exactly what the goal of this pmd is.
> >
> > Neil
> >
> >
>
> Hey Neil,
>
> this is actually intended to act and look a good deal like the ethernet
> bonding device but to handling the crypto scheduling use cases.
>
> For example, take the case where multiple hw accelerators may be available.
> We want to provide user applications with a mechanism to transparently
> balance work across all devices without having to manage the load balancing
> details or the guaranteeing of ordering of the processed ops on the
> dequeue_burst side. In this case the application would just use the crypto
> dev_id of the scheduler and it would look after balancing the workload
> across the available hw accelerators.
>
>
> +-------------------+
> | Crypto Sch PMD |
> | |
> | ORDERING / RR SCH |
> +-------------------+
> ^ ^ ^
> | | |
> +-+ | +-------------------------------+
> | +---------------+ |
> | | |
> V V V
> +---------------+ +---------------+ +---------------+
> | Crypto HW PMD | | Crypto HW PMD | | Crypto HW PMD |
> +---------------+ +---------------+ +---------------+
>
> Another use case we hope to support is migration of processing from one
> device to another where a hw and sw crypto pmd can be bound to the same
> crypto scheduler and the crypto processing could be transparently migrated
> from the hw to sw pmd. This would allow for hw accelerators to be
> hot-plugged attached/detached in a Guess VM
>
> +----------------+
> | Crypto Sch PMD |
> | |
> | MIGRATION SCH |
> +----------------+
> | |
> | +-----------------+
> | |
> V V
> +---------------+ +---------------+
> | Crypto HW PMD | | Crypto SW PMD |
> | (Active) | | (Inactive) |
> +---------------+ +---------------+
>
> The main point is that isn't envisaged as just a mechanism for scheduling
> crypto work loads across multiple cores, but a framework for allowing
> different scheduling mechanisms to be introduced, to handle different crypto
> scheduling problems, and done so in a way which is completely transparent
> to the data path of an application. Like the eth bonding driver we want to
> support creating the crypto scheduler from EAL options, which allow
> specification of the scheduling mode and the crypto pmds which are to be
> bound to that crypto scheduler.
>
>
I get what its for, that much is pretty clear. But whereas the bonding driver
benefits from creating a single device interface for the purposes of properly
routing traffic through the network stack without exposing that complexity to
the using application, this pmd provides only aggregation accoring to various
policies. This is exactly what the distributor library was built for, and it
seems like a re-invention of the wheel to ignore that. At the very least, you
should implement this pmd on top of the distributor library. If that is
impracitcal, then I somewhat question why we have the distributor library at
all.
Neil
^ permalink raw reply
* Re: [PATCH v2 15/32] net/i40e: add VF vlan strip func
From: Ferruh Yigit @ 2016-12-07 14:18 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark)
In-Reply-To: <1481081535-37448-16-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> Add a function to configure vlan strip enable/disable for specific
> SRIOV VF device.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
<...>
> +
> +/* Set vlan strip on/off for specific VF from host */
> +int
> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> +{
> + struct rte_eth_dev *dev;
> + struct i40e_pf *pf;
> + struct i40e_vsi *vsi;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> + if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> + PMD_DRV_LOG(ERR, "Invalid argument.");
> + return -EINVAL;
> + }
> +
> + vsi = pf->vfs[vf_id].vsi;
> +
> + if (vsi)
> + return i40e_vsi_config_vlan_stripping(vsi, !!on);
> + else
if vd_if is valid, can vsi be NULL? If so this check may be required in
some prev patches too.
> + return -EINVAL;
> +}
> diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
> index ca5e05a..043ae62 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.h
> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> @@ -187,4 +187,24 @@ int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
> int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
> struct ether_addr *mac_addr);
>
> +/**
> + * Enable/Disable vf vlan strip for all queues in a pool
> + *
> + * @param port
> + * The port identifier of the Ethernet device.
> + * @param vf
> + * ID specifying VF.
> + * @param on
> + * 1 - Enable VF's vlan strip on RX queues.
> + * 0 - Disable VF's vlan strip on RX queues.
> + *
> + * @return
> + * - (0) if successful.
> + * - (-ENOTSUP) if hardware doesn't support this feature.
Is this error type returned?
> + * - (-ENODEV) if *port* invalid.
> + * - (-EINVAL) if bad parameter.
> + */
<...>
^ permalink raw reply
* Re: [PATCH v2 16/32] net/i40e: add set VF VLAN insert function
From: Ferruh Yigit @ 2016-12-07 14:26 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <1481081535-37448-17-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> Support inserting VF VLAN id from PF.
> User can call the API on PF to insert a VLAN id to a
> specific VF.
Same comment with prev patch, does it make sense to insert " from PF" to
patch title?
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 53 +++++++++++++++++++++++++++++++
> drivers/net/i40e/rte_pmd_i40e.h | 19 +++++++++++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 73 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 253209b..c571d8b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10361,3 +10361,56 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> else
> return -EINVAL;
> }
> +
> +int rte_pmd_i40e_set_vf_vlan_insert(uint8_t port, uint16_t vf_id,
> + uint16_t vlan_id)
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> + struct i40e_pf_vf *vf;
> + struct i40e_hw *hw;
> + struct i40e_vsi *vsi;
> + struct i40e_vsi_context ctxt;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + hw = I40E_PF_TO_HW(pf);
> +
> + /**
> + * return -ENODEV if SRIOV not enabled, VF number not configured
> + * or no queue assigned.
> + */
> + if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 ||
> + pf->vf_nb_qps == 0)
> + return -ENODEV;
> +
> + if (vf_id > pf->vf_num)
This check was [1] in prev patches:
[1]
if (vf_id > pf->vf_num - 1 || !pf->vfs)
> + return -EINVAL;
> +
> + if (vlan_id > 4095)
Can there be any define in base driver for this? Or ETH_VLAN_ID_MAX perhaps?
> + return -EINVAL;
> +
> + vf = &pf->vfs[vf_id];
> + vsi = vf->vsi;
> +
> + vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
> + vsi->info.port_vlan_flags |= I40E_AQ_VSI_PVLAN_INSERT_PVID;
> + vsi->info.pvid = vlan_id;
> +
--->
> + memset(&ctxt, 0, sizeof(ctxt));
> + (void)rte_memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
> + ctxt.seid = vsi->seid;
> +
> + hw = I40E_VSI_TO_HW(vsi);
> + ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL);
> + if (ret != I40E_SUCCESS)
> + PMD_DRV_LOG(ERR, "Failed to update VSI params");
<-----
If Wenzhuo prefers to extract this part into a function, it can be
re-used here too.
<...>
^ permalink raw reply
* Re: [PATCH v2 12/32] net/i40e: set VF MAC from PF support
From: Ferruh Yigit @ 2016-12-07 14:29 UTC (permalink / raw)
To: Wenzhuo Lu, dev
In-Reply-To: <1481081535-37448-13-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> Support setting VF MAC address from PF.
> User can call the API on PF to set a specific
> VF's MAC address.
>
> This will reset the VF.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
<...>
>
> +/**
> + * Set the VF MAC address.
> + *
> + * This will reset the vf.
It may be good if I add a comment that this also will remove all
existing mac filters. Same to commit log perhaps.
> + *
> + * @param port
> + * The port identifier of the Ethernet device.
> + * @param vf_id
> + * VF id.
> + * @param mac_addr
> + * VF MAC address.
> + * @return
> + * - (0) if successful.
> + * - (-ENODEV) if *port* invalid.
> + * - (-EINVAL) if *vf* or *mac_addr* is invalid.
> + */
> +int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
> + struct ether_addr *mac_addr);
> +
<...>
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Alejandro Lucero @ 2016-12-07 14:31 UTC (permalink / raw)
To: Ananyev, Konstantin
Cc: Yigit, Ferruh, Yong Wang, Thomas Monjalon, Harish Patil,
dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd, Jan Medala,
Jakub Palider, John Daley, Adrien Mazarguil, Rasesh Mody,
Jacob, Jerin, Yuanhan Liu, Kulasek, TomaszX,
olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E4E05@irsmsx105.ger.corp.intel.com>
For NFP, we do not have TSO support yet, although it is coming and
hopefully it will be within next release.
Regarding this email thread, it is "it is OK, we do not need any checksum
preparation for TSO"
On Wed, Dec 7, 2016 at 10:03 AM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:
>
> Hi Ferruh,
>
> >
> > On 12/6/2016 6:25 PM, Yong Wang wrote:
> > >> -----Original Message-----
> > >> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> > >> Sent: Sunday, December 4, 2016 4:11 AM
> > >> To: Yong Wang <yongwang@vmware.com>; Thomas Monjalon
> > >> <thomas.monjalon@6wind.com>
> > >> Cc: Harish Patil <harish.patil@qlogic.com>; dev@dpdk.org; Rahul
> Lakkireddy
> > >> <rahul.lakkireddy@chelsio.com>; Stephen Hurd
> > >> <stephen.hurd@broadcom.com>; Jan Medala <jan@semihalf.com>; Jakub
> > >> Palider <jpa@semihalf.com>; John Daley <johndale@cisco.com>; Adrien
> > >> Mazarguil <adrien.mazarguil@6wind.com>; Alejandro Lucero
> > >> <alejandro.lucero@netronome.com>; Rasesh Mody
> > >> <rasesh.mody@qlogic.com>; Jacob, Jerin <Jerin.Jacob@cavium.com>;
> > >> Yuanhan Liu <yuanhan.liu@linux.intel.com>; Kulasek, TomaszX
> > >> <tomaszx.kulasek@intel.com>; olivier.matz@6wind.com
> > >> Subject: RE: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
> > >>
> > >> Hi
> > >>
> > >>
> > >>
> > >>>>
> > >>
> > >>>> 2016-11-30 17:42, Ananyev, Konstantin:
> > >>
> > >>>>>>> Please, we need a comment for each driver saying
> > >>
> > >>>>>>> "it is OK, we do not need any checksum preparation for TSO"
> > >>
> > >>>>>>> or
> > >>
> > >>>>>>> "yes we have to implement tx_prepare or TSO will not work in this
> > >>
> > >>>> mode"
> > >>
> > >>>>>>>
> > >>
> > >>>>>>
> > >>
> > >>>>>> qede PMD doesn’t currently support TSO yet, it only supports Tx
> > >>
> > >>>> TCP/UDP/IP
> > >>
> > >>>>>> csum offloads.
> > >>
> > >>>>>> So Tx preparation isn’t applicable. So as of now -
> > >>
> > >>>>>> "it is OK, we do not need any checksum preparation for TSO"
> > >>
> > >>>>>
> > >>
> > >>>>> Thanks for the answer.
> > >>
> > >>>>> Though please note that it not only for TSO.
> > >>
> > >>>>
> > >>
> > >>>> Oh yes, sorry, my wording was incorrect.
> > >>
> > >>>> We need to know if any checksum preparation is needed prior
> > >>
> > >>>> offloading its final computation to the hardware or driver.
> > >>
> > >>>> So the question applies to TSO and simple checksum offload.
> > >>
> > >>>>
> > >>
> > >>>> We are still waiting answers for
> > >>
> > >>>> bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.
> > >>
> > >>>
> > >>
> > >>> The case for a virtual device is a little bit more complicated as
> packets
> > >> offloaded from a virtual device can eventually be delivered to
> > >>
> > >>> another virtual NIC or different physical NICs that have different
> offload
> > >> requirements. In ESX, the hypervisor will enforce that the packets
> > >>
> > >>> offloaded will be something that the hardware expects. The contract
> for
> > >> vmxnet3 is that the guest needs to fill in pseudo header checksum
> > >>
> > >>> for both l4 checksum only and TSO + l4 checksum offload cases.
> > >>
> > >>
> > >>
> > >> Ok, so at first glance that looks to me very similar to Intel HW
> requirements.
> > >>
> > >> Could you confirm would rte_net_intel_cksum_prepare()
> > >>
> > >> also work for vmxnet3 or some extra modifications are required?
> > >>
> > >> You can look at it here: https://urldefense.proofpoint.
> com/v2/url?u=http-
> > >> 3A__dpdk.org_dev_patchwork_patch_17184_&d=DgIGaQ&c=uilaK90D4TOV
> > >> oH58JNXRgQ&r=v4BBYIqiDq552fkYnKKFBFyqvMXOR3UXSdFO2plFD1s&m=NS
> > >> 4zOl2je_tyGhnOJMSnu37HmJxOZf-1KLYcVsu8iYY&s=dL-NOC-
> > >> 18HclXUURQzuyW5Udw4NY13pKMndYvfgCfbA&e= .
> > >>
> > >> Note that for Intel HW the rules for pseudo-header csum calculation
> > >>
> > >> differ for TSO and non-TSO case.
> > >>
> > >> For TSO length inside pseudo-header are set to 0, while for non-tso
> case
> > >>
> > >> It should be set to L3 payload length.
> > >>
> > >> Is it the same for vmxnet3 or no?
> > >>
> > >> Thanks
> > >>
> > >> Konstantin
> > >>
> > >
> > > Yes and this is the same for vmxnet3.
> > >
> >
> > This means vmxnet3 PMD also should be updated, right?
>
> Yes, that's right.
>
> >Should that update
> > be part of tx_prep patchset? Or separate patch?
>
> Another question I suppose is who will do the actual patch for vmxnet3.
> Yong, are you ok to do the patch for vmxnet3, or prefer us to do that?
> Please note, that in both cases will need your help in testing/reviewing
> it.
> Konstantin
>
> >
> > >>>
> > >>
> > >>>>> This is for any TX offload for which the upper layer SW would have
> > >>
> > >>>>> to modify the contents of the packet.
> > >>
> > >>>>> Though as I can see for qede neither PKT_TX_IP_CKSUM or
> > >>
> > >>>> PKT_TX_TCP_CKSUM
> > >>
> > >>>>> exhibits any extra requirements for the user.
> > >>
> > >>>>> Is that correct?
> > >>
> > >>
> > >
>
>
^ permalink raw reply
* Re: [PATCH v2 17/32] net/i40e: set VF broadcast mode from PF
From: Ferruh Yigit @ 2016-12-07 14:32 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <1481081535-37448-18-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> Support enabling/disabling VF broadcast mode from PF.
> User can call the API on PF to enable/disable a specific
> VF's broadcast mode.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
<...>
> +int rte_pmd_i40e_set_vf_broadcast(uint8_t port, uint16_t vf_id, uint8_t on)
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> + struct i40e_pf_vf *vf;
> + struct i40e_hw *hw;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + if (vf_id >= dev_info.max_vfs)
> + return -EINVAL;
> +
> + if (on > 1)
> + return -EINVAL;
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + hw = I40E_PF_TO_HW(pf);
> +
> + if (vf_id > pf->vf_num)
if (vf_id > pf->vf_num - 1 || !pf->vfs)
> + return -EINVAL;
> +
<...>
^ permalink raw reply
* Re: [PATCH v2 16/32] net/i40e: add set VF VLAN insert function
From: Ferruh Yigit @ 2016-12-07 14:42 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <a0331cec-719d-ac15-c833-eaab7f09ba90@intel.com>
On 12/7/2016 2:26 PM, Ferruh Yigit wrote:
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
>> Support inserting VF VLAN id from PF.
>> User can call the API on PF to insert a VLAN id to a
>> specific VF.
>
> Same comment with prev patch, does it make sense to insert " from PF" to
> patch title?
>
>>
>> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>> ---
>> drivers/net/i40e/i40e_ethdev.c | 53 +++++++++++++++++++++++++++++++
>> drivers/net/i40e/rte_pmd_i40e.h | 19 +++++++++++
>> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
>> 3 files changed, 73 insertions(+)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index 253209b..c571d8b 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -10361,3 +10361,56 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
>> else
>> return -EINVAL;
>> }
>> +
>> +int rte_pmd_i40e_set_vf_vlan_insert(uint8_t port, uint16_t vf_id,
>> + uint16_t vlan_id)
>> +{
>> + struct rte_eth_dev *dev;
>> + struct rte_eth_dev_info dev_info;
>> + struct i40e_pf *pf;
>> + struct i40e_pf_vf *vf;
>> + struct i40e_hw *hw;
>> + struct i40e_vsi *vsi;
>> + struct i40e_vsi_context ctxt;
>> + int ret;
>> +
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>> +
>> + dev = &rte_eth_devices[port];
>> + rte_eth_dev_info_get(port, &dev_info);
>> +
>> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>> + hw = I40E_PF_TO_HW(pf);
>> +
>> + /**
>> + * return -ENODEV if SRIOV not enabled, VF number not configured
>> + * or no queue assigned.
>> + */
>> + if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 ||
>> + pf->vf_nb_qps == 0)
>> + return -ENODEV;
>> +
>> + if (vf_id > pf->vf_num)
>
> This check was [1] in prev patches:
> [1]
> if (vf_id > pf->vf_num - 1 || !pf->vfs)
>
>> + return -EINVAL;
>> +
>> + if (vlan_id > 4095)
>
> Can there be any define in base driver for this? Or ETH_VLAN_ID_MAX perhaps?
Answer was in next patches, it seems we have two options (which is bad)
lib/librte_net/rte_ether.h
#define ETHER_MAX_VLAN_ID 4095 /**< Maximum VLAN ID. */
lib/librte_ether/rte_ethdev.h
#define ETH_VLAN_ID_MAX 0x0FFF /**< VLAN ID is in lower 12 bits*/
>
>> + return -EINVAL;
>> +
>> + vf = &pf->vfs[vf_id];
>> + vsi = vf->vsi;
>> +
>> + vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
>> + vsi->info.port_vlan_flags |= I40E_AQ_VSI_PVLAN_INSERT_PVID;
>> + vsi->info.pvid = vlan_id;
>> +
>
> --->
>> + memset(&ctxt, 0, sizeof(ctxt));
>> + (void)rte_memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
>> + ctxt.seid = vsi->seid;
>> +
>> + hw = I40E_VSI_TO_HW(vsi);
>> + ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL);
>> + if (ret != I40E_SUCCESS)
>> + PMD_DRV_LOG(ERR, "Failed to update VSI params");
> <-----
>
> If Wenzhuo prefers to extract this part into a function, it can be
> re-used here too.
>
> <...>
>
^ permalink raw reply
* Re: [PATCH] Scheduler: add driver for scheduler crypto pmd
From: Richardson, Bruce @ 2016-12-07 14:46 UTC (permalink / raw)
To: Neil Horman, Doherty, Declan
Cc: Thomas Monjalon, Zhang, Roy Fan, dev@dpdk.org
In-Reply-To: <20161207141656.GA31938@neilslaptop.think-freely.org>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Wednesday, December 7, 2016 2:17 PM
> To: Doherty, Declan <declan.doherty@intel.com>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Scheduler: add driver for scheduler crypto
> pmd
>
> On Wed, Dec 07, 2016 at 12:42:15PM +0000, Declan Doherty wrote:
> > On 05/12/16 15:12, Neil Horman wrote:
> > > On Fri, Dec 02, 2016 at 04:22:16PM +0000, Declan Doherty wrote:
> > > > On 02/12/16 14:57, Bruce Richardson wrote:
> > > > > On Fri, Dec 02, 2016 at 03:31:24PM +0100, Thomas Monjalon wrote:
> > > > > > 2016-12-02 14:15, Fan Zhang:
> > > > > > > This patch provides the initial implementation of the
> > > > > > > scheduler poll mode driver using DPDK cryptodev framework.
> > > > > > >
> > > > > > > Scheduler PMD is used to schedule and enqueue the crypto ops
> > > > > > > to the hardware and/or software crypto devices attached to
> > > > > > > it (slaves). The dequeue operation from the slave(s), and
> > > > > > > the possible dequeued crypto op reordering, are then carried
> out by the scheduler.
> > > > > > >
> > > > > > > The scheduler PMD can be used to fill the throughput gap
> > > > > > > between the physical core and the existing cryptodevs to
> > > > > > > increase the overall performance. For example, if a physical
> > > > > > > core has higher crypto op processing rate than a cryptodev,
> > > > > > > the scheduler PMD can be introduced to attach more than one
> cryptodevs.
> > > > > > >
> > > > > > > This initial implementation is limited to supporting the
> > > > > > > following scheduling modes:
> > > > > > >
> > > > > > > - CRYPTO_SCHED_SW_ROUND_ROBIN_MODE (round robin amongst
> attached software
> > > > > > > slave cryptodevs, to set this mode, the scheduler should
> have been
> > > > > > > attached 1 or more software cryptodevs.
> > > > > > >
> > > > > > > - CRYPTO_SCHED_HW_ROUND_ROBIN_MODE (round robin amongst
> attached hardware
> > > > > > > slave cryptodevs (QAT), to set this mode, the scheduler
> should have
> > > > > > > been attached 1 or more QATs.
> > > > > >
> > > > > > Could it be implemented on top of the eventdev API?
> > > > > >
> > > > > Not really. The eventdev API is for different types of
> > > > > scheduling between multiple sources that are all polling for
> > > > > packets, compared to this, which is more analgous - as I
> > > > > understand it - to the bonding PMD for ethdev.
> > > > >
> > > > > To make something like this work with an eventdev API you would
> > > > > need to use one of the following models:
> > > > > * have worker cores for offloading packets to the different crypto
> > > > > blocks pulling from the eventdev APIs. This would make it
> difficult to
> > > > > do any "smart" scheduling of crypto operations between the
> blocks,
> > > > > e.g. that one crypto instance may be better at certain types of
> > > > > operations than another.
> > > > > * move the logic in this driver into an existing eventdev
> instance,
> > > > > which uses the eventdev api rather than the crypto APIs and so
> has an
> > > > > extra level of "structure abstraction" that has to be worked
> though.
> > > > > It's just not really a good fit.
> > > > >
> > > > > So for this workload, I believe the pseudo-cryptodev instance is
> > > > > the best way to go.
> > > > >
> > > > > /Bruce
> > > > >
> > > >
> > > >
> > > > As Bruce says this is much more analogous to the ethdev bonding
> > > > driver, the main idea is to allow different crypto op scheduling
> > > > mechanisms to be defined transparently to an application. This
> > > > could be load-balancing across multiple hw crypto devices, or
> > > > having a software crypto device to act as a backup device for a hw
> > > > accelerator if it becomes oversubscribed. I think the main
> > > > advantage of a crypto-scheduler approach means that the data path
> > > > of the application doesn't need to have any knowledge that
> > > > scheduling is happening at all, it is just using a different crypto
> device id, which is then manages the distribution of crypto work.
> > > >
> > > >
> > > >
> > > This is a good deal like the bonding pmd, and so from a certain
> > > standpoint it makes sense to do this, but whereas the bonding pmd is
> > > meant to create a single path to a logical network over several
> > > physical networks, this pmd really only focuses on maximizing
> > > througput, and for that we already have tools. As Thomas mentions,
> > > there is the eventdev library, but from my view the distributor
> > > library already fits this bill. It already is a basic framework to
> > > process mbufs in parallel according to whatever policy you want to
> implement, which sounds like exactly what the goal of this pmd is.
> > >
> > > Neil
> > >
> > >
> >
> > Hey Neil,
> >
> > this is actually intended to act and look a good deal like the
> > ethernet bonding device but to handling the crypto scheduling use cases.
> >
> > For example, take the case where multiple hw accelerators may be
> available.
> > We want to provide user applications with a mechanism to transparently
> > balance work across all devices without having to manage the load
> > balancing details or the guaranteeing of ordering of the processed ops
> > on the dequeue_burst side. In this case the application would just use
> > the crypto dev_id of the scheduler and it would look after balancing
> > the workload across the available hw accelerators.
> >
> >
> > +-------------------+
> > | Crypto Sch PMD |
> > | |
> > | ORDERING / RR SCH |
> > +-------------------+
> > ^ ^ ^
> > | | |
> > +-+ | +-------------------------------+
> > | +---------------+ |
> > | | |
> > V V V
> > +---------------+ +---------------+ +---------------+
> > | Crypto HW PMD | | Crypto HW PMD | | Crypto HW PMD |
> > +---------------+ +---------------+ +---------------+
> >
> > Another use case we hope to support is migration of processing from
> > one device to another where a hw and sw crypto pmd can be bound to the
> > same crypto scheduler and the crypto processing could be
> > transparently migrated from the hw to sw pmd. This would allow for hw
> > accelerators to be hot-plugged attached/detached in a Guess VM
> >
> > +----------------+
> > | Crypto Sch PMD |
> > | |
> > | MIGRATION SCH |
> > +----------------+
> > | |
> > | +-----------------+
> > | |
> > V V
> > +---------------+ +---------------+
> > | Crypto HW PMD | | Crypto SW PMD |
> > | (Active) | | (Inactive) |
> > +---------------+ +---------------+
> >
> > The main point is that isn't envisaged as just a mechanism for
> > scheduling crypto work loads across multiple cores, but a framework
> > for allowing different scheduling mechanisms to be introduced, to
> > handle different crypto scheduling problems, and done so in a way
> > which is completely transparent to the data path of an application.
> > Like the eth bonding driver we want to support creating the crypto
> > scheduler from EAL options, which allow specification of the
> > scheduling mode and the crypto pmds which are to be bound to that crypto
> scheduler.
> >
> >
> I get what its for, that much is pretty clear. But whereas the bonding
> driver benefits from creating a single device interface for the purposes
> of properly routing traffic through the network stack without exposing
> that complexity to the using application, this pmd provides only
> aggregation accoring to various policies. This is exactly what the
> distributor library was built for, and it seems like a re-invention of the
> wheel to ignore that. At the very least, you should implement this pmd on
> top of the distributor library. If that is impracitcal, then I somewhat
> question why we have the distributor library at all.
>
> Neil
>
Hi Neil,
The distributor library, and the eventdev framework are not the solution here, as, firstly, the crypto devices are not cores, in the same way that ethdev's are not cores, and the distributor library is for evenly distributing work among cores. Sure, some crypto implementations may be software only, but many aren't, and those that are software still appear as a device to software that must be used like they were a HW device. In the same way that to use distributor to load balance traffic between various TX ports is not a suitable solution - because you need to use cores to do the work "bridging" between the distributor/eventdev and the ethdev device, similarly here, if we distribute traffic using the distributor, you need cores to pull those packets from the distributor and offload them to the crypto devices. To use the distributor library in place of this vpmd, we'd need crypto devices which are aware of how to talk to the distributor, and use it's protocols for pushing/pulling packets, or else we are pulling in extra core cycles to do bridging work.
Secondly, the distributor and eventdev libraries are designed for doing flow based (generally atomic) packet distribution. Load balancing between crypto devices is not generally based on flows, but rather on other factors like packet size, offload cost per device, etc. To distributor/eventdev, all workers are equal, but for working with devices, for crypto offload or nic transmission, that is plainly not the case. In short the distribution problems that are being solved by distributor and eventdev libraries are fundamentally different than those being solved by this vpmd. They would be the wrong tool for the job.
I would agree with the previous statements that this driver is far closer in functionality to the bonded ethdev driver than anything else. It makes multiple devices appear as a single one while hiding the complexity of the multiple devices to the using application. In the same way as the bonded ethdev driver has different modes for active-backup, and for active-active for increased throughput, this vpmd for crypto can have the exact same modes - multiple active bonded devices for higher performance operation, or two devices in active backup to enable migration when using SR-IOV as described by Declan above.
Regards,
/Bruce
^ permalink raw reply
* Re: [PATCH v2 19/32] net/i40e: set VF VLAN filter from PF
From: Ferruh Yigit @ 2016-12-07 14:46 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <1481081535-37448-20-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> add rte_pmd_i40e_set_vf_vlan_filter API.
> User can call the API on PF to enable/disable
> a set of VF's VLAN filters.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 50 +++++++++++++++++++++++++++++++
> drivers/net/i40e/rte_pmd_i40e.h | 22 ++++++++++++++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 73 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 601e933..bc96698 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10516,3 +10516,53 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t port, uint16_t vf_id, uint8_t on)
>
> return ret;
> }
> +
> +int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
> + uint64_t vf_mask, uint8_t on)
> +{
<...>
> +
> + for (pool_idx = 0;
> + pool_idx < ETH_64_POOLS && pool_idx < pf->nb_cfg_vmdq_vsi;
> + pool_idx++) {
> + if (vf_mask & ((uint64_t)(1ULL << pool_idx))) {
> + if (on)
> + ret = i40e_vsi_add_vlan(pf->vmdq[pool_idx].vsi,
> + vlan_id);
> + else
> + ret = i40e_vsi_delete_vlan(
> + pf->vmdq[pool_idx].vsi, vlan_id);
> + }
> + }
> +
> + if (ret != I40E_SUCCESS)
> + PMD_DRV_LOG(ERR, "Failed to set VF VLAN filter, on = %d", on);
Since loop not break on error, this will only log the last one, if the
error is in the middle, it is missed.
<...>
^ permalink raw reply
* Re: [PATCH] Force python scripts to run with python2
From: Thomas Monjalon @ 2016-12-07 14:48 UTC (permalink / raw)
To: Martin Kletzander; +Cc: dev, Mcnamara, John
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20267038C@IRSMSX103.ger.corp.intel.com>
2016-12-07 12:04, Mcnamara, John:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Martin Kletzander
> > Sent: Wednesday, December 7, 2016 10:17 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] Force python scripts to run with python2
> >
> > With python3 being the default in some distributions/installations,
> > shebang with just "python" will make the script run under version of
> > python that the scripts are not written for. In order to fix that and
> > mitigate future errors, use shebang properly and specify the python
> > version. That way the scripts will run in any distro/install, no matter
> > what python version is set as the default.
>
> I think a better approach would be to make the scripts Python 2 and Python 3 compatible.
>
> Some of the new ones already are.
Yes
The "solution" using python2 in the shebang does not work everywhere
because python2 can be an unknown command.
^ permalink raw reply
* Re: [PATCH v2 03/12] crypto/armv8: Add core crypto operations for ARMv8
From: Thomas Monjalon @ 2016-12-07 15:00 UTC (permalink / raw)
To: Jerin Jacob; +Cc: zbigniew.bodek, dev, pablo.de.lara.guarch, Emery Davis
In-Reply-To: <20161206232437.GA26779@localhost.localdomain>
2016-12-07 04:54, Jerin Jacob:
> On Tue, Dec 06, 2016 at 02:41:01PM -0800, Thomas Monjalon wrote:
> > 2016-12-07 03:35, Jerin Jacob:
> > > On Tue, Dec 06, 2016 at 10:42:51PM +0100, Thomas Monjalon wrote:
> > > > 2016-12-07 02:48, Jerin Jacob:
> > > > > On Tue, Dec 06, 2016 at 09:29:25PM +0100, Thomas Monjalon wrote:
> > > > > > 2016-12-06 18:32, zbigniew.bodek@caviumnetworks.com:
> > > > > > > From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
> > > > > > >
> > > > > > > This patch adds core low-level crypto operations
> > > > > > > for ARMv8 processors. The assembly code is a base
> > > > > > > for an optimized PMD and is currently excluded
> > > > > > > from the build.
> > > > > >
> > > > > > It's a bit sad that you cannot achieve the same performance with
> > > > > > C code and a good compiler.
> > > > > > Have you tried it? How much is the difference?
> > > > >
> > > > > Like AES-NI on IA side(exposed as separate PMD in dpdk),
> > > > > armv8 has special dedicated instructions for crypto operation using SIMD.
> > > > > This patch is using the "dedicated" armv8 crypto instructions and SIMD
> > > > > operation to achieve better performance.
> > > >
> > > > It does not justify to have all the code in asm.
> > >
> > > Why ? if we can have separate dpdk pmd for AES-NI on IA . Why not for ARM?
> >
> > Jerin, you or me is not understanding the other.
> > It is perfectly fine to have a separate PMD.
> > I am just talking about the language C vs ASM.
>
> Hmm. Both are bit connected topic :-)
>
> If you check the AES-NI PMD installation guide, We need to download the
> "ASM" optimized AES-NI library and build with yasm.
> We all uses fine grained ASM code such work.
> So AES-NI case those are still ASM code but reside in some other
> library.
Yes
> http://dpdk.org/doc/guides/cryptodevs/aesni_mb.html(Check Installation section)
> https://downloadcenter.intel.com/download/22972
>
> Even linux kernel use, hardcore ASM for crypto work.
> https://github.com/torvalds/linux/blob/master/arch/arm/crypto/aes-ce-core.S
Yes
> > > > > We had compared with openssl implementation.Here is the performance
> > > > > improvement for chained crypto operations case WRT openssl pmd
> > > > >
> > > > > Buffer
> > > > > Size(B) OPS(M) Throughput(Gbps)
> > > > > 64 729 % 742 %
> > > > > 128 577 % 592 %
> > > > > 256 483 % 476 %
> > > > > 512 336 % 351 %
> > > > > 768 300 % 286 %
> > > > > 1024 263 % 250 %
> > > > > 1280 225 % 229 %
> > > > > 1536 214 % 213 %
> > > > > 1792 186 % 203 %
> > > > > 2048 200 % 193 %
> > > >
> > > > OK but what is the performance difference between this asm code
> > > > and a C equivalent?
> > >
> > > Do you you want compare against the scalar version of C code? its not
> > > even worth to think about it. The vector version will use
> > > dedicated armv8 instruction for crypto so its not portable anyway.
> > > We would like to asm code so that we can have better control on what we do
> > > and we cant rely compiler for that.
> >
> > No I'm talking about comparing a PMD written in C vs this one in ASM.
>
> Only fast stuff written in ASM. Remaining pmd is written in C.
> Look "crypto/armv8: add PMD optimized for ARMv8 processors"
>
> > It"s just harder to read ASM. Most of DPDK code is in C.
> > And only some small functions are written in ASM.
> > The vector instructions use some C intrinsics.
> > Do you mean that the instructions that you are using have no intrinsics
> > equivalent? Nobody made it into GCC?
>
> There is intrinsic equivalent for crypto but that will work only on
> armv8. If we start using the arch specific intrinsic then it better to
> plain ASM code, it is clean and we all do similar scheme for core crypto
> work(like AES-NI library, linux etc)
>
> We did a lot of effort to make clean armv8 ASM code _optimized_ for DPDK workload.
> Just because someone doesn't familiar with armv8 Assembly its not fair to
> say write it in C.
I'm just saying it is sad, as it is sad for AES-NI or Linux code.
Please read again my questions:
Have you tried it? How much is the difference?
I'm not saying it should not enter in DPDK, I'm just asking some basic
questions to better understand the motivations and the status of ARM crypto
in general.
You did not answer for comparing with a C implementation, so I guess you
have implemented it in ASM without even trying to do it in C.
The conclusion: we will never know what is the real gain of coding this in ASM.
^ permalink raw reply
* Re: [PATCH v2 21/32] app/testpmd: use unicast promiscuous mode on i40e
From: Ferruh Yigit @ 2016-12-07 14:59 UTC (permalink / raw)
To: Wenzhuo Lu, dev
In-Reply-To: <1481081535-37448-22-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> Add testpmd CLI to set VF unicast promiscuous mode on i40e.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> app/test-pmd/cmdline.c | 92 +++++++++++++++++++++++++++++
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 +++
> 2 files changed, 100 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 12126ce..d39712e 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -404,6 +404,11 @@ static void cmd_help_long_parsed(void *parsed_result,
> "set allmulti (port_id|all) (on|off)\n"
> " Set the allmulti mode on port_id, or all.\n\n"
>
> +#ifdef RTE_LIBRTE_I40E_PMD
> + "set vf unicast-promisc (port_id) (vf_id) (on|off)\n"
Previous usages are all "promisc" instead of "unicals-promisc". Is this
to promisc mode for multicast packets? If so testpmd calls them
"allmulti" I guess, so they won't cause trouble.
Can we keep using command: "promisc"?
<...>
> +
> +cmdline_parse_inst_t cmd_set_vf_unicast_promisc = {
> + .f = cmd_set_vf_unicast_promisc_parsed,
> + .data = NULL,
> + .help_str = "set vf unicast promiscuous port_id vf_id on|off",
Can you please differentiate the keyword and variable by wrapping
variables with <>? Like:
"set vf unicast-promiscuous <port_id> <vf_id> on|off"
<...>
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index f1c269a..e17e3d5 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -820,6 +820,14 @@ Set the allmulti mode for a port or for all ports::
>
> Same as the ifconfig (8) option. Controls how multicast packets are handled.
>
> +set unicast promisc (for VF)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Should we mention this is PMD specific feature and only enabled with
some PMDs?
> +
> +Set the unicast promiscuous mode for a VF from PF.
> +In promiscuous mode packets are not dropped if they aren't for the specified MAC address::
> +
> + testpmd> set vf unicast-promisc (port_id) (vf_id) (on|off)
> +
> set flow_ctrl rx
> ~~~~~~~~~~~~~~~~
>
>
^ permalink raw reply
* Re: [PATCH v2 22/32] app/testpmd: use multicast promiscuous mode on i40e
From: Ferruh Yigit @ 2016-12-07 15:01 UTC (permalink / raw)
To: Wenzhuo Lu, dev
In-Reply-To: <1481081535-37448-23-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> Add testpmd CLI to set VF multicast promiscuous mode on i40e.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> app/test-pmd/cmdline.c | 86 +++++++++++++++++++++++++++++
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 +++
> 2 files changed, 94 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index d39712e..7e7a016 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -407,6 +407,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> #ifdef RTE_LIBRTE_I40E_PMD
> "set vf unicast-promisc (port_id) (vf_id) (on|off)\n"
> " Set unicast promiscuous mode for a VF from the PF.\n\n"
> +
> + "set vf multicast-promisc (port_id) (vf_id) (on|off)\n"
> + " Set multicast promiscuous mode for a VF from the PF.\n\n"
Why not "allmulti" instead of multicast-promisc?
Also same comments as previous patch for help_str and documentation.
<...>
^ permalink raw reply
* Re: [PATCH v2 26/32] app/testpmd: initialize receive mode for VMDq
From: Ferruh Yigit @ 2016-12-07 15:06 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <1481081535-37448-27-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> Initialise VMDq in the init_port_config function in a similar
> way to how it is done in the VMDq sample application.
What is the effect of doing existing initialization?
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
<...>
^ permalink raw reply
* Re: [PATCH v2 27/32] net/i40e: change version number to support Linux VF
From: Ferruh Yigit @ 2016-12-07 15:13 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark)
In-Reply-To: <1481081535-37448-28-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> i40e PF host only support to work with DPDK VF driver, Linux
> VF driver is not supported. This change will enhance in version
> number returned.
>
> Current version info returned won't be able to be recognized
> by Linux VF driver, change to values that both DPDK VF and Linux
> driver can recognize.
>
> The expense is original DPDK host specific feature like
> CFG_VLAN_PVID and CONFIG_VSI_QUEUES_EXT will not available.
>
> DPDK VF also can't identify host driver by version number returned.
> It always assume talking with Linux PF.
I guess you mention from following code [1], should it be also updated
to prevent it giving wrong information:
[1] i40e_ethdev_vf.c
if (vf->version_major == I40E_DPDK_VERSION_MAJOR)
PMD_DRV_LOG(INFO, "Peer is DPDK PF host");
else if ((vf->version_major == I40E_VIRTCHNL_VERSION_MAJOR) &&
(vf->version_minor <= I40E_VIRTCHNL_VERSION_MINOR))
PMD_DRV_LOG(INFO, "Peer is Linux PF host");
else {
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
<...>
^ permalink raw reply
* Re: [PATCH v2 28/32] net/i40e: return correct vsi_id
From: Ferruh Yigit @ 2016-12-07 15:16 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark)
In-Reply-To: <1481081535-37448-29-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> PF host didn't return correct VSI id to VF.
> This change fix it.
This looks like a fix for current code,
can you please update commit title and log to reflect the fix?
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
> drivers/net/i40e/i40e_pf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index 0f582ed..8319c2c 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -351,8 +351,7 @@
>
> /* Change below setting if PF host can support more VSIs for VF */
> vf_res->vsi_res[0].vsi_type = I40E_VSI_SRIOV;
> - /* As assume Vf only has single VSI now, always return 0 */
> - vf_res->vsi_res[0].vsi_id = 0;
> + vf_res->vsi_res[0].vsi_id = vf->vsi->vsi_id;
> vf_res->vsi_res[0].num_queue_pairs = vf->vsi->nb_qps;
> ether_addr_copy(&vf->mac_addr,
> (struct ether_addr *)vf_res->vsi_res[0].default_mac_addr);
>
^ permalink raw reply
* Re: [PATCH v2 29/32] net/i40e: parse more VF parameter and configure
From: Ferruh Yigit @ 2016-12-07 15:18 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark)
In-Reply-To: <1481081535-37448-30-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> When VF requested to configure TX queue, a few parameters are
> missed to be configured in PF host. This change have more
> fields parsed and configured for TX context.
What is the effect of missing Tx paramters configured?
If this cause a bug, this patch should be a fix.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
> drivers/net/i40e/i40e_pf.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index 8319c2c..1ad5ed1 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -422,10 +422,12 @@
>
> /* clear the context structure first */
> memset(&tx_ctx, 0, sizeof(tx_ctx));
> - tx_ctx.new_context = 1;
> tx_ctx.base = txq->dma_ring_addr / I40E_QUEUE_BASE_ADDR_UNIT;
> tx_ctx.qlen = txq->ring_len;
> tx_ctx.rdylist = rte_le_to_cpu_16(vf->vsi->info.qs_handle[0]);
> + tx_ctx.head_wb_ena = txq->headwb_enabled;
> + tx_ctx.head_wb_addr = txq->dma_headwb_addr;
> +
> err = i40e_clear_lan_tx_queue_context(hw, abs_queue_id);
> if (err != I40E_SUCCESS)
> return err;
>
^ permalink raw reply
* Re: [PATCH v2 29/32] net/i40e: parse more VF parameter and configure
From: Chen, Jing D @ 2016-12-07 15:32 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <f1fc596a-0bbb-5911-fa97-c751ab2b03e0@intel.com>
Hi, Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 11:19 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Chen, Jing D <jing.d.chen@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 29/32] net/i40e: parse more VF parameter
> and configure
>
> On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> > When VF requested to configure TX queue, a few parameters are missed
> > to be configured in PF host. This change have more fields parsed and
> > configured for TX context.
>
> What is the effect of missing Tx paramters configured?
>
> If this cause a bug, this patch should be a fix.
>
This need to analyze case by case. If PF driver is serving a DPDK VF client,
the missing part is OK. If serving a Linux VF client, the missing part will
cause some unexpected TX queue behaviors.
So, this is an enhancement to support Linux VF client.
> >
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
> > drivers/net/i40e/i40e_pf.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index 8319c2c..1ad5ed1 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -422,10 +422,12 @@
> >
> > /* clear the context structure first */
> > memset(&tx_ctx, 0, sizeof(tx_ctx));
> > - tx_ctx.new_context = 1;
> > tx_ctx.base = txq->dma_ring_addr / I40E_QUEUE_BASE_ADDR_UNIT;
> > tx_ctx.qlen = txq->ring_len;
> > tx_ctx.rdylist = rte_le_to_cpu_16(vf->vsi->info.qs_handle[0]);
> > + tx_ctx.head_wb_ena = txq->headwb_enabled;
> > + tx_ctx.head_wb_addr = txq->dma_headwb_addr;
> > +
> > err = i40e_clear_lan_tx_queue_context(hw, abs_queue_id);
> > if (err != I40E_SUCCESS)
> > return err;
> >
^ permalink raw reply
* Re: [PATCH v2 28/32] net/i40e: return correct vsi_id
From: Chen, Jing D @ 2016-12-07 15:34 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <63a680b6-6a5e-cfe9-6050-ac348334ba67@intel.com>
Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 11:16 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Chen, Jing D <jing.d.chen@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 28/32] net/i40e: return correct vsi_id
>
> On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> > PF host didn't return correct VSI id to VF.
> > This change fix it.
>
> This looks like a fix for current code,
> can you please update commit title and log to reflect the fix?
>
This is similar patch to support Linux VF client. DPDK VF client needn't
Vsi ID.
> >
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
> > drivers/net/i40e/i40e_pf.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index 0f582ed..8319c2c 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -351,8 +351,7 @@
> >
> > /* Change below setting if PF host can support more VSIs for VF */
> > vf_res->vsi_res[0].vsi_type = I40E_VSI_SRIOV;
> > - /* As assume Vf only has single VSI now, always return 0 */
> > - vf_res->vsi_res[0].vsi_id = 0;
> > + vf_res->vsi_res[0].vsi_id = vf->vsi->vsi_id;
> > vf_res->vsi_res[0].num_queue_pairs = vf->vsi->nb_qps;
> > ether_addr_copy(&vf->mac_addr,
> > (struct ether_addr *)vf_res->vsi_res[0].default_mac_addr);
> >
^ permalink raw reply
* Re: [PATCH v2 32/32] app/testpmd: fix invalid port ID
From: Ferruh Yigit @ 2016-12-07 15:48 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark)
In-Reply-To: <1481081535-37448-33-git-send-email-wenzhuo.lu@intel.com>
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> Some CLIs don't check the input port ID, it
> may cause segmentation fault (core dumped).
Are these functions (that we are adding extra check) added with this
patchset? If so why not add these checks where function implemented.
If these functions are already implemented before this patchset, this
patch can be standalone patch, instead of being part of this patchset,
and can be before this patchset so that it can be easily backported to
stable trees.
>
> Fixes: 425781ff5afe ("app/testpmd: add ixgbe VF management")
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
<...>
^ permalink raw reply
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