* Re: [PATCH v2] app/testpmd: supported offload capabilities query
From: Wu, Jingjing @ 2016-12-20 5:50 UTC (permalink / raw)
To: Yang, Qiming, dev@dpdk.org; +Cc: Yang, Qiming
In-Reply-To: <1481008055-68096-1-git-send-email-qiming.yang@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang
> Sent: Tuesday, December 6, 2016 3:08 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>
> Subject: [dpdk-dev] [PATCH v2] app/testpmd: supported offload capabilities
> query
>
> Add two new commands "show port capa <port>" and "show port capa all"to
> diaplay what offload capabilities supported in ports. It will not only display all
> the capabilities of the port, but also the enabling condition for each capability
> in the running time.
>
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>
> ---
> v2 changes:
> * fixed the output style as Ferruh's patch show and add some
> descriptions in testpmd_funcs.rst for new functions.
> ---
> ---
> app/test-pmd/cmdline.c | 15 ++-
> app/test-pmd/config.c | 172 ++++++++++++++++++++++++++++
> app/test-pmd/testpmd.h | 1 +
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 12 +-
> 4 files changed, 191 insertions(+), 9 deletions(-)
>
Please add this new command for the print when "help display" in cmd_help_long_parsed.
Thanks
Jingjing
^ permalink raw reply
* Bug in virtqueue_dequeue_burst_rx()
From: Gopakumar Choorakkot Edakkunni @ 2016-12-20 5:59 UTC (permalink / raw)
To: huawei.xie, dev
While I was testing virtio with ubuntu 14.04 kvm as host and dpdk16.07
linux as guest, quite often I have seen that I get into a situation where
virtio_recv_mergeable_pkts() gets into a forever loop, after sending
traffic for a while. In the below API, I see that it clearly leads to a
while loop, I am not quite familiar with virtio or mergeable buffers, so
thought of checking with dpdk alias on the intent here.
I checked the linux kernel virtio_net.c file which does the similar
mergeable recieve, and the kernel code breaks out in case of error.
Shouldnt dpdk be breaking out of here on error instead of continue ?
virtio_recv_mergeable_pkts()
{
<snip>
while (i < nb_used) {
<snip>
* num = virtqueue_dequeue_burst_rx(rxvq, rcv_pkts, len, 1);*
* if (num != 1)*
* continue;*
<snip>
}
<snip?
}
Rgds,
Gopa.
^ permalink raw reply
* Re: [PATCHv2 01/34] lib/ether: add rte_device in rte_eth_dev
From: Hemant Agrawal @ 2016-12-20 6:12 UTC (permalink / raw)
To: Stephen Hemminger
Cc: dev, thomas.monjalon, bruce.richardson, shreyansh.jain,
john.mcnamara, ferruh.yigit, jerin.jacob
In-Reply-To: <20161219081651.6482f6b1@xeon-e3>
On 12/19/2016 9:46 PM, Stephen Hemminger wrote:
> On Tue, 20 Dec 2016 02:23:40 +0530
> Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> lib/librte_ether/rte_ethdev.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index 9678179..0b601e9 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -1626,6 +1626,7 @@ struct rte_eth_dev {
>> eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
>> eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
>> struct rte_eth_dev_data *data; /**< Pointer to device data */
>> + struct rte_device *device;
>> const struct eth_driver *driver;/**< Driver for this device */
>> const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
>> struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */
>
> NAK
> I would rather that rte_pci_device be eliminated from rte_eth_dev_data and
> replace by more generic rte_device. I am working on a patch set to do this,
> it is not fundamentally hard.
>
As discussed before, this patch is just an stop-gap arrangement till we
get a proper solution. That is the very reason, I put it as first patch
in my patchset - to be removed easily.
It is helpful that you are coming with the proper solution.
^ permalink raw reply
* Re: [PATCH 01/13] ethdev: increase length ethernet device internal name
From: Shreyansh Jain @ 2016-12-20 6:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20161219215944.17226-2-sthemmin@microsoft.com>
On Tuesday 20 December 2016 03:29 AM, Stephen Hemminger wrote:
> Allow sufficicent space for UUID in string form (36+1).
> Needed to use UUID with Hyper-V
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 3 +++
> lib/librte_ether/rte_ethdev.h | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 2d17bc6e..b83f23a1 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -58,6 +58,9 @@ Deprecation Notices
> ``port`` field, may be moved or removed as part of this mbuf work. A
> ``timestamp`` will also be added.
>
> +* ethdev: for 17.02 the size of internal device name will be increased
> + to 40 characters to allow for storing UUID.
> +
> * The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
> are respectively replaced by PKT_RX_VLAN_STRIPPED and
> PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 96781792..3c85e331 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1652,7 +1652,11 @@ struct rte_eth_dev_sriov {
> };
> #define RTE_ETH_DEV_SRIOV(dev) ((dev)->data->sriov)
>
> -#define RTE_ETH_NAME_MAX_LEN (32)
> +/*
> + * Internal identifier length
> + * Sufficiently large to allow for UUID or PCI address
> + */
> +#define RTE_ETH_NAME_MAX_LEN 40
Just to clarify my doubt: UUID is 36 byte long. So, 4 extra bytes are to
keep the name length 4 byte aligned (along with one byte of \0)?
Or, PCI based addressing can stretch 40 bytes?
>
> /**
> * @internal
>
^ permalink raw reply
* Re: [PATCH 04/13] eal: introduce driver type
From: Shreyansh Jain @ 2016-12-20 7:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20161219215944.17226-5-sthemmin@microsoft.com>
On Tuesday 20 December 2016 03:29 AM, Stephen Hemminger wrote:
> Since multiple buses and device types need to be supported.
> Provide type field in driver.
> ---
> lib/librte_eal/common/include/rte_dev.h | 15 ++++++++++++---
> lib/librte_eal/common/include/rte_pci.h | 1 +
> lib/librte_eal/common/include/rte_vdev.h | 1 +
> 3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index e5471a22..3f4e26e6 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -144,12 +144,21 @@ void rte_eal_device_insert(struct rte_device *dev);
> void rte_eal_device_remove(struct rte_device *dev);
>
> /**
> + * Type of device driver
> + */
> +enum rte_driver_type {
> + PMD_VIRTUAL,
> + PMD_PCI,
> +};
So the expectation is that if a new device type is introduced (anything
other than virtual or PCI), this enum is expanded?
Broadly, we have two possible ways being discussed on ML for device
relationship with PMD/APIs:
1. the way you have done: each device belongs to a particular type (enum
rte_driver_type) and based on its type, operations would be performed on
it (using conditional operators, for example). We continue to have a
common device list containing all type of devices.
2. disassociating the device (rte_device) completely from its type and
basing it on a Bus type. So, we have separate buses for each device type
and hence no need for separation of logic based on device type.
I think implementation similar to (1) existed before it was removed in
6751f6de.
I have an (obvious) inclination towards (2) because that helps plugging
in more drivers/device types without expecting the contributor to change
the EAL - however trivial the change is.
> +
> +/**
> * A structure describing a device driver.
> */
> struct rte_driver {
> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
> - const char *name; /**< Driver name. */
> - const char *alias; /**< Driver alias. */
> + const char *name; /**< Driver name. */
> + const char *alias; /**< Driver alias. */
> + enum rte_driver_type type; /**< Driver type. */
> };
>
> /**
> @@ -243,4 +252,4 @@ __attribute__((used)) = str
> }
> #endif
>
> -#endif /* _RTE_VDEV_H_ */
> +#endif /* _RTE_DEV_H_ */
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 9ce88472..d377d539 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -492,6 +492,7 @@ RTE_INIT(pciinitfn_ ##nm); \
> static void pciinitfn_ ##nm(void) \
> {\
> (pci_drv).driver.name = RTE_STR(nm);\
> + (pci_drv).driver.type = PMD_PCI; \
> rte_eal_pci_register(&pci_drv); \
> } \
> RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> diff --git a/lib/librte_eal/common/include/rte_vdev.h b/lib/librte_eal/common/include/rte_vdev.h
> index 784e837d..98fb5bb5 100644
> --- a/lib/librte_eal/common/include/rte_vdev.h
> +++ b/lib/librte_eal/common/include/rte_vdev.h
> @@ -88,6 +88,7 @@ static void vdrvinitfn_ ##vdrv(void)\
> {\
> (vdrv).driver.name = RTE_STR(nm);\
> (vdrv).driver.alias = vdrvinit_ ## nm ## _alias;\
> + (vdrv).driver.type = PMD_VIRTUAL;\
> rte_eal_vdrv_register(&vdrv);\
> } \
> RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>
^ permalink raw reply
* Re: [PATCH v3 10/25] app/testpmd: add flow flush command
From: Zhao1, Wei @ 2016-12-20 7:32 UTC (permalink / raw)
To: Adrien Mazarguil, dev@dpdk.org
In-Reply-To: <8a8d3efbf0258cc3afa5b334b180e049ce2f9ee3.1482168851.git.adrien.mazarguil@6wind.com>
Hi, Adrien
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Tuesday, December 20, 2016 1:49 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 10/25] app/testpmd: add flow flush
> command
>
> Syntax:
>
> flow flush {port_id}
>
> Destroy all flow rules on a port.
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Acked-by: Olga Shern <olgas@mellanox.com>
> ---
> app/test-pmd/cmdline.c | 3 +++
> app/test-pmd/cmdline_flow.c | 43
> +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 0dc6c63..6e2b289 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -811,6 +811,9 @@ static void cmd_help_long_parsed(void
> *parsed_result,
> " (select|add)\n"
> " Set the input set for FDir.\n\n"
>
> + "flow flush {port_id}\n"
> + " Destroy all flow rules.\n\n"
> +
> "flow list {port_id} [group {group_id}] [...]\n"
> " List existing flow rules sorted by priority,"
> " filtered by group identifiers.\n\n"
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index bd3da38..49578eb 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -63,6 +63,7 @@ enum index {
> FLOW,
>
> /* Sub-level commands. */
> + FLUSH,
> LIST,
>
> /* List arguments. */
> @@ -179,6 +180,9 @@ static const enum index next_list_attr[] = { static int
> parse_init(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> +static int parse_flush(struct context *, const struct token *,
> + const char *, unsigned int,
> + void *, unsigned int);
> static int parse_list(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> @@ -240,10 +244,19 @@ static const struct token token_list[] = {
> .name = "flow",
> .type = "{command} {port_id} [{arg} [...]]",
> .help = "manage ingress/egress flow rules",
> - .next = NEXT(NEXT_ENTRY(LIST)),
> + .next = NEXT(NEXT_ENTRY
> + (FLUSH,
> + LIST)),
> .call = parse_init,
> },
> /* Sub-level commands. */
> + [FLUSH] = {
> + .name = "flush",
> + .help = "destroy all flow rules",
> + .next = NEXT(NEXT_ENTRY(PORT_ID)),
> + .args = ARGS(ARGS_ENTRY(struct buffer, port)),
> + .call = parse_flush,
> + },
> [LIST] = {
> .name = "list",
> .help = "list existing flow rules",
> @@ -316,6 +329,31 @@ parse_init(struct context *ctx, const struct token
> *token,
> return len;
> }
>
> +/** Parse tokens for flush command. */
> +static int
> +parse_flush(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size)
> +{
> + struct buffer *out = buf;
> +
> + /* Token name must match. */
> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> + return -1;
> + /* Nothing else to do if there is no buffer. */
> + if (!out)
> + return len;
> + if (!out->command) {
> + if (ctx->curr != FLUSH)
> + return -1;
> + if (sizeof(*out) > size)
> + return -1;
> + out->command = ctx->curr;
> + ctx->object = out;
> + }
> + return len;
> +}
> +
> /** Parse tokens for list command. */
> static int
> parse_list(struct context *ctx, const struct token *token, @@ -698,6 +736,9
> @@ static void cmd_flow_parsed(const struct buffer *in) {
> switch (in->command) {
> + case FLUSH:
> + port_flow_flush(in->port);
> + break;
> case LIST:
> port_flow_list(in->port, in->args.list.group_n,
> in->args.list.group);
> --
> 2.1.4
When user flow flush cmd, PMD will flush all the rule on the specific port, and the memory of which rte_flow point to must be flushed.
This memory is returned when flow create, will rte layer flush this memory or PMD is responsible for that memory flush?
BTW, there is no argument about rte_flow in flush function pass into PMD layer.
^ permalink raw reply
* Re: [PATCH v2 05/32] net/i40e: set TX loopback from PF
From: Lu, Wenzhuo @ 2016-12-20 8:16 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org
In-Reply-To: <b5f7b037-0aa2-d3ef-190b-26f247a940da@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 9:05 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 05/32] net/i40e: set TX loopback from PF
>
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > Support enabling/disabling TX loopback from PF.
> > User can call the API on PF to enable/disable TX loopback for all the
> > PF and VFs.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 219
> ++++++++++++++++++++++++++++++
> > drivers/net/i40e/rte_pmd_i40e.h | 16 +++
> > drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> > 3 files changed, 236 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index ec863b9..8bd0d70 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -9938,3 +9938,222 @@ static void i40e_set_default_mac_addr(struct
> > rte_eth_dev *dev,
> >
> > return ret;
> > }
> > +
>
> <...>
>
> > +
> > +static int
> > +i40e_vsi_set_tx_loopback(struct i40e_vsi *vsi, uint8_t on) {
> > + struct i40e_vsi_context ctxt;
> > + struct i40e_hw *hw;
> > + int ret;
> > +
> > + hw = I40E_VSI_TO_HW(vsi);
> > +
> > + /* Use the FW API if FW >= v5.0 */
> > + if (hw->aq.fw_maj_ver < 5) {
> > + PMD_INIT_LOG(ERR, "FW < v5.0, cannot enable loopback");
> > + return -ENOTSUP;
> > + }
> > +
> > + /* Check if it has been already on or off */
> > + if (vsi->info.valid_sections &
> > + rte_cpu_to_le_16(I40E_AQ_VSI_PROP_SWITCH_VALID)) {
> > + if (on) {
> > + if ((vsi->info.switch_id &
> > + I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB) ==
> > + I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB)
> > + return 0; /* already on */
> > + } else {
> > + if ((vsi->info.switch_id &
> > + I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB) == 0)
> > + return 0; /* already off */
> > + }
> > + }
> > +
> > + /* remove all the MACs first */
> > + ret = i40e_vsi_rm_mac_filter(vsi);
> > + if (ret)
> > + return ret;
> > +
> > + vsi->info.valid_sections =
> cpu_to_le16(I40E_AQ_VSI_PROP_SWITCH_VALID);
> > + if (on)
> > + vsi->info.switch_id |= I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB;
> > + else
> > + vsi->info.switch_id &= ~I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB;
> > +
>
> ---->
>
> > + memset(&ctxt, 0, sizeof(ctxt));
> > + (void)rte_memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
> > + ctxt.seid = vsi->seid;
> > +
> > + ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL);
> > + if (ret != I40E_SUCCESS) {
> > + PMD_DRV_LOG(ERR, "Failed to update VSI params");
> > + return ret;
> > + }
>
> <----
>
> This part is now duplicated in a few functions, does it make sense to make it
> separate function, in the first patch it appeared 3/32 ?
There's already a function 'i40e_aq_update_vsi_params'. The duplicate code is for preparing the parameter for the functions. It looks bad to me if we add a function for that.
>
> > +
> > + /* add all the MACs back */
> > + ret = i40e_vsi_restore_mac_filter(vsi);
> > + if (ret)
> > + return ret;
> > +
> > + return ret;
> > +}
> > +
> > +int
> > +rte_pmd_i40e_set_tx_loopback(uint8_t port, uint8_t on) {
> > + struct rte_eth_dev *dev;
> > + struct i40e_pf *pf;
> > + struct i40e_pf_vf *vf;
> > + struct i40e_vsi *vsi;
> > + uint16_t vf_id;
> > + int ret;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port];
> > +
> > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > +
> > + /* setup PF TX loopback */
> > + vsi = pf->main_vsi;
> > + ret = i40e_vsi_set_tx_loopback(vsi, on);
> > + if (ret)
> > + return ret;
> > +
> > + /* setup TX loopback for all the VFs */
> > + if (!pf->vfs) {
> > + PMD_DRV_LOG(ERR, "Invalid argument.");
> > + return -EINVAL;
> > + }
> > +
> > + for (vf_id = 0; vf_id < pf->vf_num; vf_id++) {
> > + vf = &pf->vfs[vf_id];
> > + vsi = vf->vsi;
> > +
> > + ret = i40e_vsi_set_tx_loopback(vsi, on);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> > b/drivers/net/i40e/rte_pmd_i40e.h index c8736c8..3c65be4 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.h
> > +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > @@ -114,4 +114,20 @@ int rte_pmd_i40e_set_vf_vlan_anti_spoof(uint8_t
> port,
> > uint16_t vf_id,
> > uint8_t on);
> >
> > +/**
> > + * Enable/Disable TX loopback on all the PF and VFs.
> > + *
> > + * @param port
> > + * The port identifier of the Ethernet device.
> > + * @param on
> > + * 1 - Enable TX loopback.
> > + * 0 - Disable TX loopback.
> > + * @return
> > + * - (0) if successful.
> > + * - (-ENODEV) if *port* invalid.
> > + * - (-EINVAL) if bad parameter.
> > + */
> > +int rte_pmd_i40e_set_tx_loopback(uint8_t port,
> > + uint8_t on);
> > +
> > #endif /* _PMD_I40E_H_ */
> > diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
> > b/drivers/net/i40e/rte_pmd_i40e_version.map
> > index fff6cf9..3da04d3 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> > +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> > @@ -9,4 +9,5 @@ DPDK_17.02 {
> > rte_pmd_i40e_ping_vfs;
> > rte_pmd_i40e_set_vf_mac_anti_spoof;
> > rte_pmd_i40e_set_vf_vlan_anti_spoof;
> > + rte_pmd_i40e_set_tx_loopback;
>
> We can add these alphabetically.
> This may not worth to create a new version of the patch itself, but can be fixed
> if a new version already required..
Yes, will do it.
>
> > } DPDK_2.0;
> >
^ permalink raw reply
* [PATCH] net/ixgbe/base: clear redundant macro define
From: Chenghu Yao @ 2016-12-20 8:27 UTC (permalink / raw)
To: wenzhuo.lu, helin.zhang; +Cc: dev, Chenghu Yao
In head file "ixgbe_mbx.h", macro define IXGBE_VF_API_NEGOTIATE and
IXGBE_VF_GET_QUEUES appears two times with the same name，value,
and notes.
Version 2.0 VF requests can inherit the two macro defines
in previous version 1.0 and 1.1. Otherwise, may be cause confusion.
Signed-off-by: Chenghu Yao <yao.chenghu@zte.com.cn>
---
drivers/net/ixgbe/base/ixgbe_mbx.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ixgbe/base/ixgbe_mbx.h b/drivers/net/ixgbe/base/ixgbe_mbx.h
index 7556a81..483e9b3 100644
--- a/drivers/net/ixgbe/base/ixgbe_mbx.h
+++ b/drivers/net/ixgbe/base/ixgbe_mbx.h
@@ -128,8 +128,6 @@ enum ixgbe_pfvf_api_rev {
#define IXGBE_PF_CONTROL_MSG 0x0100 /* PF control message */
/* mailbox API, version 2.0 VF requests */
-#define IXGBE_VF_API_NEGOTIATE 0x08 /* negotiate API version */
-#define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
#define IXGBE_VF_ENABLE_MACADDR 0x0A /* enable MAC address */
#define IXGBE_VF_DISABLE_MACADDR 0x0B /* disable MAC address */
#define IXGBE_VF_GET_MACADDRS 0x0C /* get all configured MAC addrs */
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 02/32] net/i40e: add callback to user on VF to PF mbox msg
From: Lu, Wenzhuo @ 2016-12-20 8:49 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org
In-Reply-To: <a94e0b5d-dc76-4348-6f18-2f0a6e0a0ade@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 8:45 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 02/32] net/i40e: add callback to user on VF to
> PF mbox msg
>
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > The callback asks the user application if it is allowed to perform the
> > mailbox messages.
> >
> > If the return value from user is RTE_PMD_I40E_MB_EVENT_PROCEED then
> > continue. If ACK or NACK, do nothing and send not_supported to VF.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> > drivers/net/i40e/i40e_pf.c | 230
> ++++++++++++++++++++++++++++++++++------
> > drivers/net/i40e/rte_pmd_i40e.h | 21 ++++
> > 2 files changed, 216 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index f70712b..8b8a14f 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -55,6 +55,7 @@
> > #include "i40e_ethdev.h"
> > #include "i40e_rxtx.h"
> > #include "i40e_pf.h"
> > +#include "rte_pmd_i40e.h"
> >
> > #define I40E_CFG_CRCSTRIP_DEFAULT 1
> >
> > @@ -272,14 +273,23 @@
> > }
> >
> > static void
> > -i40e_pf_host_process_cmd_version(struct i40e_pf_vf *vf)
> > +i40e_pf_host_process_cmd_version(struct i40e_pf_vf *vf, bool b_op)
> > {
> > struct i40e_virtchnl_version_info info;
> >
> > info.major = I40E_DPDK_VERSION_MAJOR;
> > info.minor = I40E_DPDK_VERSION_MINOR;
> > - i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_VERSION,
> > - I40E_SUCCESS, (uint8_t *)&info, sizeof(info));
> > +
> > + if (b_op)
> > + i40e_pf_host_send_msg_to_vf(vf,
> I40E_VIRTCHNL_OP_VERSION,
> > + I40E_SUCCESS,
> > + (uint8_t *)&info,
> > + sizeof(info));
> > + else
> > + i40e_pf_host_send_msg_to_vf(vf,
> I40E_VIRTCHNL_OP_VERSION,
> > + I40E_NOT_SUPPORTED,
> > + (uint8_t *)&info,
> > + sizeof(info));
>
> Even I40E_NOT_SUPPORTED returned to the VF, it seems VF is ignoring this
> value, is it something should be fixed as part of this patch?
>
> This path is a little complex, I may be missing something but stack trace is:
>
> VF:
> i40evf_check_api_version()
> i40evf_execute_vf_cmd(vfd_cmd_info arg)
> i40e_aq_send_msg_to_pf(arg->op, retval, arg->in_msg)
> desc <- op, retval
> msg_in <- arg_in_msg
> i40e_asq_send_command(desc, msg_in)
>
> PF:
> i40e_pf_host_handle_vf_msg(op, msg_in)
> i40e_pf_host_process_cmd_version()
> i40e_pf_host_send_msg_to_vf(op, retval, msg_out)
> i40e_aq_send_msg_to_vf(op, retval, msg_out)
> desc <- op, retval
> i40e_asq_send_command(desc, msg_out)
>
> VF:
> data <- arg->out_xxx
> i40evf_read_pfmsg(data)
> event <- data->out_msg
> op <-
> retval <-
> i40e_clean_arq_element(event)
> event->desc <- desc
> event->msg <- msg_out
> data->result = retval <----------------
> return 0;
> ver = arg->out_msg
> return 0;
>
>
> So, as far as I can see I40E_NOT_SUPPORTED is somewhere in the stack but not
> reached to the final VF function, is this OK?
I think it's a bug in the existing code. It's forgotten to check the ' data->result ' . I will send a separate patch to fix it.
>
>
> > }
> >
> > static int
> > @@ -292,13 +302,20 @@
> > }
> >
> > static int
> > -i40e_pf_host_process_cmd_get_vf_resource(struct i40e_pf_vf *vf)
> > +i40e_pf_host_process_cmd_get_vf_resource(struct i40e_pf_vf *vf, bool
> > +b_op)
> > {
> > struct i40e_virtchnl_vf_resource *vf_res = NULL;
> > struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
> > uint32_t len = 0;
> > int ret = I40E_SUCCESS;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(vf,
> > +
> I40E_VIRTCHNL_OP_GET_VF_RESOURCES,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > /* only have 1 VSI by default */
> > len = sizeof(struct i40e_virtchnl_vf_resource) +
> > I40E_DEFAULT_VF_VSI_NUM *
> > @@ -423,7 +440,8 @@
> > static int
> > i40e_pf_host_process_cmd_config_vsi_queues(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
> > struct i40e_vsi *vsi = vf->vsi;
> > @@ -432,6 +450,13 @@
> > struct i40e_virtchnl_queue_pair_info *vc_qpi;
> > int i, ret = I40E_SUCCESS;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(vf,
> > +
> I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (!msg || vc_vqci->num_queue_pairs > vsi->nb_qps ||
> > vc_vqci->num_queue_pairs > I40E_MAX_VSI_QP ||
> > msglen < I40E_VIRTCHNL_CONFIG_VSI_QUEUES_SIZE(vc_vqci,
> > @@ -482,7 +507,8 @@
> > static int
> > i40e_pf_host_process_cmd_config_vsi_queues_ext(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
> > struct i40e_vsi *vsi = vf->vsi;
> > @@ -491,6 +517,14 @@
> > struct i40e_virtchnl_queue_pair_ext_info *vc_qpei;
> > int i, ret = I40E_SUCCESS;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EXT,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (!msg || vc_vqcei->num_queue_pairs > vsi->nb_qps ||
> > vc_vqcei->num_queue_pairs > I40E_MAX_VSI_QP ||
> > msglen < I40E_VIRTCHNL_CONFIG_VSI_QUEUES_SIZE(vc_vqcei,
> > @@ -539,12 +573,21 @@
> >
> > static int
> > i40e_pf_host_process_cmd_config_irq_map(struct i40e_pf_vf *vf,
> > - uint8_t *msg, uint16_t msglen)
> > + uint8_t *msg, uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_irq_map_info *irqmap =
> > (struct i40e_virtchnl_irq_map_info *)msg;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_CONFIG_IRQ_MAP,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen < sizeof(struct i40e_virtchnl_irq_map_info)) {
> > PMD_DRV_LOG(ERR, "buffer too short");
> > ret = I40E_ERR_PARAM;
> > @@ -646,12 +689,21 @@
> > static int
> > i40e_pf_host_process_cmd_disable_queues(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_queue_select *q_sel =
> > (struct i40e_virtchnl_queue_select *)msg;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_DISABLE_QUEUES,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen != sizeof(*q_sel)) {
> > ret = I40E_ERR_PARAM;
> > goto send_msg;
> > @@ -669,7 +721,8 @@
> > static int
> > i40e_pf_host_process_cmd_add_ether_address(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_ether_addr_list *addr_list = @@ -678,6 +731,14
> > @@
> > int i;
> > struct ether_addr *mac;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > memset(&filter, 0 , sizeof(struct i40e_mac_filter_info));
> >
> > if (msg == NULL || msglen <= sizeof(*addr_list)) { @@ -707,7 +768,8
> > @@ static int i40e_pf_host_process_cmd_del_ether_address(struct
> > i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_ether_addr_list *addr_list = @@ -715,6 +777,14
> > @@
> > int i;
> > struct ether_addr *mac;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen <= sizeof(*addr_list)) {
> > PMD_DRV_LOG(ERR, "delete_ether_address argument too
> short");
> > ret = I40E_ERR_PARAM;
> > @@ -739,7 +809,8 @@
> >
> > static int
> > i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
> > - uint8_t *msg, uint16_t msglen)
> > + uint8_t *msg, uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_vlan_filter_list *vlan_filter_list = @@ -747,6
> > +818,14 @@
> > int i;
> > uint16_t *vid;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_ADD_VLAN,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen <= sizeof(*vlan_filter_list)) {
> > PMD_DRV_LOG(ERR, "add_vlan argument too short");
> > ret = I40E_ERR_PARAM;
> > @@ -771,7 +850,8 @@
> > static int
> > i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_vlan_filter_list *vlan_filter_list = @@ -779,6
> > +859,14 @@
> > int i;
> > uint16_t *vid;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_DEL_VLAN,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen <= sizeof(*vlan_filter_list)) {
> > PMD_DRV_LOG(ERR, "delete_vlan argument too short");
> > ret = I40E_ERR_PARAM;
> > @@ -803,7 +891,8 @@
> > i40e_pf_host_process_cmd_config_promisc_mode(
> > struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_promisc_info *promisc = @@ -811,6 +900,14 @@
> > struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
> > bool unicast = FALSE, multicast = FALSE;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_CONFIG_PROMISCUOUS_MODE,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen != sizeof(*promisc)) {
> > ret = I40E_ERR_PARAM;
> > goto send_msg;
> > @@ -836,13 +933,20 @@
> > }
> >
> > static int
> > -i40e_pf_host_process_cmd_get_stats(struct i40e_pf_vf *vf)
> > +i40e_pf_host_process_cmd_get_stats(struct i40e_pf_vf *vf, bool b_op)
> > {
> > i40e_update_vsi_stats(vf->vsi);
> >
> > - i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_GET_STATS,
> > - I40E_SUCCESS, (uint8_t *)&vf->vsi->eth_stats,
> > - sizeof(vf->vsi->eth_stats));
> > + if (b_op)
> > + i40e_pf_host_send_msg_to_vf(vf,
> I40E_VIRTCHNL_OP_GET_STATS,
> > + I40E_SUCCESS,
> > + (uint8_t *)&vf->vsi->eth_stats,
> > + sizeof(vf->vsi->eth_stats));
> > + else
> > + i40e_pf_host_send_msg_to_vf(vf,
> I40E_VIRTCHNL_OP_GET_STATS,
> > + I40E_NOT_SUPPORTED,
> > + (uint8_t *)&vf->vsi->eth_stats,
> > + sizeof(vf->vsi->eth_stats));
> >
> > return I40E_SUCCESS;
> > }
> > @@ -851,12 +955,21 @@
> > i40e_pf_host_process_cmd_cfg_vlan_offload(
> > struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_vlan_offload_info *offload =
> > (struct i40e_virtchnl_vlan_offload_info *)msg;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen != sizeof(*offload)) {
> > ret = I40E_ERR_PARAM;
> > goto send_msg;
> > @@ -877,12 +990,21 @@
> > static int
> > i40e_pf_host_process_cmd_cfg_pvid(struct i40e_pf_vf *vf,
> > uint8_t *msg,
> > - uint16_t msglen)
> > + uint16_t msglen,
> > + bool b_op)
> > {
> > int ret = I40E_SUCCESS;
> > struct i40e_virtchnl_pvid_info *tpid_info =
> > (struct i40e_virtchnl_pvid_info *)msg;
> >
> > + if (!b_op) {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf,
> > + I40E_VIRTCHNL_OP_CFG_VLAN_PVID,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + return ret;
> > + }
> > +
> > if (msg == NULL || msglen != sizeof(*tpid_info)) {
> > ret = I40E_ERR_PARAM;
> > goto send_msg;
> > @@ -923,6 +1045,8 @@
> > struct i40e_pf_vf *vf;
> > /* AdminQ will pass absolute VF id, transfer to internal vf id */
> > uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > + struct rte_pmd_i40e_mb_event_param cb_param;
> > + bool b_op = TRUE;
> >
> > if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > PMD_DRV_LOG(ERR, "invalid argument"); @@ -937,10
> +1061,35 @@
> > return;
> > }
> >
> > + /**
> > + * initialise structure to send to user application
> > + * will return response from user in retval field
> > + */
> > + cb_param.retval = RTE_PMD_I40E_MB_EVENT_PROCEED;
> > + cb_param.vfid = vf_id;
> > + cb_param.msg_type = opcode;
> > + cb_param.msg = (void *)msg;
> > + cb_param.msglen = msglen;
> > +
> > + /**
> > + * Ask user application if we're allowed to perform those functions.
> > + * If we get cb_param.retval == RTE_PMD_I40E_MB_EVENT_PROCEED,
> > + * then business as usual.
> > + * If RTE_PMD_I40E_MB_EVENT_NOOP_ACK or
> RTE_PMD_I40E_MB_EVENT_NOOP_NACK,
> > + * do nothing and send not_supported to VF. As PF must send a response
> > + * to VF and ACK/NACK is not defined.
> > + */
> > + _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_VF_MBOX,
> &cb_param);
> > + if (cb_param.retval != RTE_PMD_I40E_MB_EVENT_PROCEED) {
> > + PMD_DRV_LOG(WARNING, "VF to PF message(%d) is not
> permitted!",
> > + opcode);
> > + b_op = FALSE;
> > + }
> > +
>
> Is the reason you just not do [1], because final function requires valid buffers?
> Can it be an option to fix them?
I thought about doing [1], but the problem is we cannot use NULL for all kinds of messages. For some messages NULL will break them, so have to handle them case by case.
>
> [1]
> if (!b_op) {
> i40e_pf_host_send_msg_to_vf(vf, opcode, I40E_NOT_SUPPORTED,
> NULL, 0); }
>
> > switch (opcode) {
> > case I40E_VIRTCHNL_OP_VERSION :
> > PMD_DRV_LOG(INFO, "OP_VERSION received");
> > - i40e_pf_host_process_cmd_version(vf);
> > + i40e_pf_host_process_cmd_version(vf, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_RESET_VF :
> > PMD_DRV_LOG(INFO, "OP_RESET_VF received"); @@ -948,61
> +1097,72 @@
> > break;
> > case I40E_VIRTCHNL_OP_GET_VF_RESOURCES:
> > PMD_DRV_LOG(INFO, "OP_GET_VF_RESOURCES received");
> > - i40e_pf_host_process_cmd_get_vf_resource(vf);
> > + i40e_pf_host_process_cmd_get_vf_resource(vf, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES:
> > PMD_DRV_LOG(INFO, "OP_CONFIG_VSI_QUEUES received");
> > - i40e_pf_host_process_cmd_config_vsi_queues(vf, msg,
> msglen);
> > + i40e_pf_host_process_cmd_config_vsi_queues(vf, msg,
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EXT:
> > PMD_DRV_LOG(INFO, "OP_CONFIG_VSI_QUEUES_EXT
> received");
> > i40e_pf_host_process_cmd_config_vsi_queues_ext(vf, msg,
> > - msglen);
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CONFIG_IRQ_MAP:
> > PMD_DRV_LOG(INFO, "OP_CONFIG_IRQ_MAP received");
> > - i40e_pf_host_process_cmd_config_irq_map(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_config_irq_map(vf, msg, msglen,
> b_op);
> > break;
> > case I40E_VIRTCHNL_OP_ENABLE_QUEUES:
> > PMD_DRV_LOG(INFO, "OP_ENABLE_QUEUES received");
> > - i40e_pf_host_process_cmd_enable_queues(vf, msg, msglen);
> > - i40e_notify_vf_link_status(dev, vf);
> > + if (b_op) {
> > + i40e_pf_host_process_cmd_enable_queues(vf, msg,
> msglen);
> > + i40e_notify_vf_link_status(dev, vf);
> > + } else {
> > + i40e_pf_host_send_msg_to_vf(
> > + vf, I40E_VIRTCHNL_OP_ENABLE_QUEUES,
> > + I40E_NOT_SUPPORTED, NULL, 0);
> > + }
> > break;
> > case I40E_VIRTCHNL_OP_DISABLE_QUEUES:
> > PMD_DRV_LOG(INFO, "OP_DISABLE_QUEUE received");
> > - i40e_pf_host_process_cmd_disable_queues(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_disable_queues(vf, msg, msglen,
> b_op);
> > break;
> > case I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS:
> > PMD_DRV_LOG(INFO, "OP_ADD_ETHER_ADDRESS received");
> > - i40e_pf_host_process_cmd_add_ether_address(vf, msg,
> msglen);
> > + i40e_pf_host_process_cmd_add_ether_address(vf, msg,
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS:
> > PMD_DRV_LOG(INFO, "OP_DEL_ETHER_ADDRESS received");
> > - i40e_pf_host_process_cmd_del_ether_address(vf, msg,
> msglen);
> > + i40e_pf_host_process_cmd_del_ether_address(vf, msg,
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_ADD_VLAN:
> > PMD_DRV_LOG(INFO, "OP_ADD_VLAN received");
> > - i40e_pf_host_process_cmd_add_vlan(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_add_vlan(vf, msg, msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_DEL_VLAN:
> > PMD_DRV_LOG(INFO, "OP_DEL_VLAN received");
> > - i40e_pf_host_process_cmd_del_vlan(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_del_vlan(vf, msg, msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CONFIG_PROMISCUOUS_MODE:
> > PMD_DRV_LOG(INFO, "OP_CONFIG_PROMISCUOUS_MODE
> received");
> > - i40e_pf_host_process_cmd_config_promisc_mode(vf, msg,
> msglen);
> > + i40e_pf_host_process_cmd_config_promisc_mode(vf, msg,
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_GET_STATS:
> > PMD_DRV_LOG(INFO, "OP_GET_STATS received");
> > - i40e_pf_host_process_cmd_get_stats(vf);
> > + i40e_pf_host_process_cmd_get_stats(vf, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD:
> > PMD_DRV_LOG(INFO, "OP_CFG_VLAN_OFFLOAD received");
> > - i40e_pf_host_process_cmd_cfg_vlan_offload(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_cfg_vlan_offload(vf, msg,
> > + msglen, b_op);
> > break;
> > case I40E_VIRTCHNL_OP_CFG_VLAN_PVID:
> > PMD_DRV_LOG(INFO, "OP_CFG_VLAN_PVID received");
> > - i40e_pf_host_process_cmd_cfg_pvid(vf, msg, msglen);
> > + i40e_pf_host_process_cmd_cfg_pvid(vf, msg, msglen, b_op);
> > break;
> > /* Don't add command supported below, which will
> > * return an error code.
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> > b/drivers/net/i40e/rte_pmd_i40e.h index 14852f2..eb7a72b 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.h
> > +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > @@ -42,6 +42,27 @@
> > #include <rte_ethdev.h>
> >
> > /**
> > + * Response sent back to i40e driver from user app after callback */
> > +enum rte_pmd_i40e_mb_event_rsp {
> > + RTE_PMD_I40E_MB_EVENT_NOOP_ACK, /**< skip mbox request and
> ACK */
> > + RTE_PMD_I40E_MB_EVENT_NOOP_NACK, /**< skip mbox request and
> NACK */
> > + RTE_PMD_I40E_MB_EVENT_PROCEED, /**< proceed with mbox
> request */
> > + RTE_PMD_I40E_MB_EVENT_MAX /**< max value of this enum */
> > +};
> > +
> > +/**
> > + * Data sent to the user application when the callback is executed.
> > + */
> > +struct rte_pmd_i40e_mb_event_param {
> > + uint16_t vfid; /**< Virtual Function number */
> > + uint16_t msg_type; /**< VF to PF message type, see i40e_virtchnl_ops
> */
> > + uint16_t retval; /**< return value */
> > + void *msg; /**< pointer to message */
> > + uint16_t msglen; /**< length of the message */
> > +};
> > +
> > +/**
> > * Notify VF when PF link status changes.
> > *
> > * @param port
> >
^ permalink raw reply
* Re: [PATCH v3 22/25] app/testpmd: add L4 items to flow command
From: Pei, Yulong @ 2016-12-20 9:14 UTC (permalink / raw)
To: Adrien Mazarguil, dev@dpdk.org; +Cc: Xing, Beilei
In-Reply-To: <11827edfa78272fe967e5a53451a3f48c6d7d17a.1482168851.git.adrien.mazarguil@6wind.com>
Hi Adrien,
For SCTP, it may need support to set 'tag' since in FDIR (--pkt-filter-mode=perfect) need set it when filter sctp flow.
struct sctp_hdr {
uint16_t src_port; /**< Source port. */
uint16_t dst_port; /**< Destin port. */
uint32_t tag; /**< Validation tag. */
uint32_t cksum; /**< Checksum. */
} __attribute__((__packed__));
testpmd> flow create 0 ingress pattern eth / ipv4 src is 192.168.0.1 dst is 192.168.0.2 / sctp
src [TOKEN]: SCTP source port
dst [TOKEN]: SCTP destination port
/ [TOKEN]: specify next pattern item
Best Regards
Yulong Pei
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
Sent: Tuesday, December 20, 2016 1:49 AM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v3 22/25] app/testpmd: add L4 items to flow command
Add the ability to match a few properties of common L4[.5] protocol
headers:
- ICMP: type and code.
- UDP: source and destination ports.
- TCP: source and destination ports.
- SCTP: source and destination ports.
- VXLAN: network identifier.
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Olga Shern <olgas@mellanox.com>
---
app/test-pmd/cmdline_flow.c | 163 +++++++++++++++++++++++++++++++++++++++
1 file changed, 163 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index c2725a5..a340a75 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -132,6 +132,20 @@ enum index {
ITEM_IPV6,
ITEM_IPV6_SRC,
ITEM_IPV6_DST,
+ ITEM_ICMP,
+ ITEM_ICMP_TYPE,
+ ITEM_ICMP_CODE,
+ ITEM_UDP,
+ ITEM_UDP_SRC,
+ ITEM_UDP_DST,
+ ITEM_TCP,
+ ITEM_TCP_SRC,
+ ITEM_TCP_DST,
+ ITEM_SCTP,
+ ITEM_SCTP_SRC,
+ ITEM_SCTP_DST,
+ ITEM_VXLAN,
+ ITEM_VXLAN_VNI,
/* Validate/create actions. */
ACTIONS,
@@ -359,6 +373,11 @@ static const enum index next_item[] = {
ITEM_VLAN,
ITEM_IPV4,
ITEM_IPV6,
+ ITEM_ICMP,
+ ITEM_UDP,
+ ITEM_TCP,
+ ITEM_SCTP,
+ ITEM_VXLAN,
ZERO,
};
@@ -419,6 +438,40 @@ static const enum index item_ipv6[] = {
ZERO,
};
+static const enum index item_icmp[] = {
+ ITEM_ICMP_TYPE,
+ ITEM_ICMP_CODE,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_udp[] = {
+ ITEM_UDP_SRC,
+ ITEM_UDP_DST,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_tcp[] = {
+ ITEM_TCP_SRC,
+ ITEM_TCP_DST,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_sctp[] = {
+ ITEM_SCTP_SRC,
+ ITEM_SCTP_DST,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_vxlan[] = {
+ ITEM_VXLAN_VNI,
+ ITEM_NEXT,
+ ZERO,
+};
+
static const enum index next_action[] = {
ACTION_END,
ACTION_VOID,
@@ -930,6 +983,103 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ipv6,
hdr.dst_addr)),
},
+ [ITEM_ICMP] = {
+ .name = "icmp",
+ .help = "match ICMP header",
+ .priv = PRIV_ITEM(ICMP, sizeof(struct rte_flow_item_icmp)),
+ .next = NEXT(item_icmp),
+ .call = parse_vc,
+ },
+ [ITEM_ICMP_TYPE] = {
+ .name = "type",
+ .help = "ICMP packet type",
+ .next = NEXT(item_icmp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_icmp,
+ hdr.icmp_type)),
+ },
+ [ITEM_ICMP_CODE] = {
+ .name = "code",
+ .help = "ICMP packet code",
+ .next = NEXT(item_icmp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_icmp,
+ hdr.icmp_code)),
+ },
+ [ITEM_UDP] = {
+ .name = "udp",
+ .help = "match UDP header",
+ .priv = PRIV_ITEM(UDP, sizeof(struct rte_flow_item_udp)),
+ .next = NEXT(item_udp),
+ .call = parse_vc,
+ },
+ [ITEM_UDP_SRC] = {
+ .name = "src",
+ .help = "UDP source port",
+ .next = NEXT(item_udp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_udp,
+ hdr.src_port)),
+ },
+ [ITEM_UDP_DST] = {
+ .name = "dst",
+ .help = "UDP destination port",
+ .next = NEXT(item_udp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_udp,
+ hdr.dst_port)),
+ },
+ [ITEM_TCP] = {
+ .name = "tcp",
+ .help = "match TCP header",
+ .priv = PRIV_ITEM(TCP, sizeof(struct rte_flow_item_tcp)),
+ .next = NEXT(item_tcp),
+ .call = parse_vc,
+ },
+ [ITEM_TCP_SRC] = {
+ .name = "src",
+ .help = "TCP source port",
+ .next = NEXT(item_tcp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_tcp,
+ hdr.src_port)),
+ },
+ [ITEM_TCP_DST] = {
+ .name = "dst",
+ .help = "TCP destination port",
+ .next = NEXT(item_tcp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_tcp,
+ hdr.dst_port)),
+ },
+ [ITEM_SCTP] = {
+ .name = "sctp",
+ .help = "match SCTP header",
+ .priv = PRIV_ITEM(SCTP, sizeof(struct rte_flow_item_sctp)),
+ .next = NEXT(item_sctp),
+ .call = parse_vc,
+ },
+ [ITEM_SCTP_SRC] = {
+ .name = "src",
+ .help = "SCTP source port",
+ .next = NEXT(item_sctp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_sctp,
+ hdr.src_port)),
+ },
+ [ITEM_SCTP_DST] = {
+ .name = "dst",
+ .help = "SCTP destination port",
+ .next = NEXT(item_sctp, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_sctp,
+ hdr.dst_port)),
+ },
+ [ITEM_VXLAN] = {
+ .name = "vxlan",
+ .help = "match VXLAN header",
+ .priv = PRIV_ITEM(VXLAN, sizeof(struct rte_flow_item_vxlan)),
+ .next = NEXT(item_vxlan),
+ .call = parse_vc,
+ },
+ [ITEM_VXLAN_VNI] = {
+ .name = "vni",
+ .help = "VXLAN identifier",
+ .next = NEXT(item_vxlan, NEXT_ENTRY(UNSIGNED), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan, vni)),
+ },
/* Validate/create actions. */
[ACTIONS] = {
.name = "actions",
@@ -1491,6 +1641,19 @@ parse_int(struct context *ctx, const struct token *token,
case sizeof(uint16_t):
*(uint16_t *)buf = arg->hton ? rte_cpu_to_be_16(u) : u;
break;
+ case sizeof(uint8_t [3]):
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+ if (!arg->hton) {
+ ((uint8_t *)buf)[0] = u;
+ ((uint8_t *)buf)[1] = u >> 8;
+ ((uint8_t *)buf)[2] = u >> 16;
+ break;
+ }
+#endif
+ ((uint8_t *)buf)[0] = u >> 16;
+ ((uint8_t *)buf)[1] = u >> 8;
+ ((uint8_t *)buf)[2] = u;
+ break;
case sizeof(uint32_t):
*(uint32_t *)buf = arg->hton ? rte_cpu_to_be_32(u) : u;
break;
--
2.1.4
^ permalink raw reply
* [dpdk-announce] Performance Report for DPDK 16.11 now available
From: Mcnamara, John @ 2016-12-20 9:20 UTC (permalink / raw)
To: announce@dpdk.org
The performance report for DPDK 16.11 on Intel Nics is now available. See:
http://dpdk.org/doc/
http://fast.dpdk.org/doc/perf/Intel_DPDK_R16_11_NIC_performance_report.pdf
We intend to have a similar report for each release and hope that some other vendors will also add reports.
We also hope that more dynamic performance reports will come out of the ongoing DPDK Continuous Integration effort.
^ permalink raw reply
* Re: [PATCH v3 21/25] app/testpmd: add items ipv4/ipv6 to flow command
From: Pei, Yulong @ 2016-12-20 9:21 UTC (permalink / raw)
To: Adrien Mazarguil, dev@dpdk.org; +Cc: Xing, Beilei
In-Reply-To: <f7837fb26514f3a58d594fc50fe9bc7a256d472e.1482168851.git.adrien.mazarguil@6wind.com>
Hi adrien,
Is it possible to support to set ipv4 TOS, ipv4 PROTO, ipv4 TTL and ipv6 tc, ipv6 next-header, ipv6 hop-limits since
previous FDIR for i40e already support it.
Best Regards
Yulong Pei
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
Sent: Tuesday, December 20, 2016 1:49 AM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v3 21/25] app/testpmd: add items ipv4/ipv6 to flow command
Add the ability to match basic fields from IPv4 and IPv6 headers (source and destination addresses only).
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Olga Shern <olgas@mellanox.com>
---
app/test-pmd/cmdline_flow.c | 177 +++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index 53709fe..c2725a5 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -38,6 +38,7 @@
#include <errno.h>
#include <ctype.h>
#include <string.h>
+#include <arpa/inet.h>
#include <rte_common.h>
#include <rte_ethdev.h>
@@ -61,6 +62,8 @@ enum index {
BOOLEAN,
STRING,
MAC_ADDR,
+ IPV4_ADDR,
+ IPV6_ADDR,
RULE_ID,
PORT_ID,
GROUP_ID,
@@ -123,6 +126,12 @@ enum index {
ITEM_VLAN,
ITEM_VLAN_TPID,
ITEM_VLAN_TCI,
+ ITEM_IPV4,
+ ITEM_IPV4_SRC,
+ ITEM_IPV4_DST,
+ ITEM_IPV6,
+ ITEM_IPV6_SRC,
+ ITEM_IPV6_DST,
/* Validate/create actions. */
ACTIONS,
@@ -348,6 +357,8 @@ static const enum index next_item[] = {
ITEM_RAW,
ITEM_ETH,
ITEM_VLAN,
+ ITEM_IPV4,
+ ITEM_IPV6,
ZERO,
};
@@ -394,6 +405,20 @@ static const enum index item_vlan[] = {
ZERO,
};
+static const enum index item_ipv4[] = {
+ ITEM_IPV4_SRC,
+ ITEM_IPV4_DST,
+ ITEM_NEXT,
+ ZERO,
+};
+
+static const enum index item_ipv6[] = {
+ ITEM_IPV6_SRC,
+ ITEM_IPV6_DST,
+ ITEM_NEXT,
+ ZERO,
+};
+
static const enum index next_action[] = {
ACTION_END,
ACTION_VOID,
@@ -439,6 +464,12 @@ static int parse_string(struct context *, const struct token *, static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_ipv4_addr(struct context *, const struct token *,
+ const char *, unsigned int,
+ void *, unsigned int);
+static int parse_ipv6_addr(struct context *, const struct token *,
+ const char *, unsigned int,
+ void *, unsigned int);
static int parse_port(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -509,6 +540,20 @@ static const struct token token_list[] = {
.call = parse_mac_addr,
.comp = comp_none,
},
+ [IPV4_ADDR] = {
+ .name = "{IPv4 address}",
+ .type = "IPV4 ADDRESS",
+ .help = "standard IPv4 address notation",
+ .call = parse_ipv4_addr,
+ .comp = comp_none,
+ },
+ [IPV6_ADDR] = {
+ .name = "{IPv6 address}",
+ .type = "IPV6 ADDRESS",
+ .help = "standard IPv6 address notation",
+ .call = parse_ipv6_addr,
+ .comp = comp_none,
+ },
[RULE_ID] = {
.name = "{rule id}",
.type = "RULE ID",
@@ -843,6 +888,48 @@ static const struct token token_list[] = {
.next = NEXT(item_vlan, NEXT_ENTRY(UNSIGNED), item_param),
.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vlan, tci)),
},
+ [ITEM_IPV4] = {
+ .name = "ipv4",
+ .help = "match IPv4 header",
+ .priv = PRIV_ITEM(IPV4, sizeof(struct rte_flow_item_ipv4)),
+ .next = NEXT(item_ipv4),
+ .call = parse_vc,
+ },
+ [ITEM_IPV4_SRC] = {
+ .name = "src",
+ .help = "source address",
+ .next = NEXT(item_ipv4, NEXT_ENTRY(IPV4_ADDR), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ipv4,
+ hdr.src_addr)),
+ },
+ [ITEM_IPV4_DST] = {
+ .name = "dst",
+ .help = "destination address",
+ .next = NEXT(item_ipv4, NEXT_ENTRY(IPV4_ADDR), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ipv4,
+ hdr.dst_addr)),
+ },
+ [ITEM_IPV6] = {
+ .name = "ipv6",
+ .help = "match IPv6 header",
+ .priv = PRIV_ITEM(IPV6, sizeof(struct rte_flow_item_ipv6)),
+ .next = NEXT(item_ipv6),
+ .call = parse_vc,
+ },
+ [ITEM_IPV6_SRC] = {
+ .name = "src",
+ .help = "source address",
+ .next = NEXT(item_ipv6, NEXT_ENTRY(IPV6_ADDR), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ipv6,
+ hdr.src_addr)),
+ },
+ [ITEM_IPV6_DST] = {
+ .name = "dst",
+ .help = "destination address",
+ .next = NEXT(item_ipv6, NEXT_ENTRY(IPV6_ADDR), item_param),
+ .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_ipv6,
+ hdr.dst_addr)),
+ },
/* Validate/create actions. */
[ACTIONS] = {
.name = "actions",
@@ -1514,6 +1601,96 @@ parse_mac_addr(struct context *ctx, const struct token *token,
return -1;
}
+/**
+ * Parse an IPv4 address.
+ *
+ * Last argument (ctx->args) is retrieved to determine storage size and
+ * location.
+ */
+static int
+parse_ipv4_addr(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg = pop_args(ctx);
+ char str2[len + 1];
+ struct in_addr tmp;
+ int ret;
+
+ /* Argument is expected. */
+ if (!arg)
+ return -1;
+ size = arg->size;
+ /* Bit-mask fill is not supported. */
+ if (arg->mask || size != sizeof(tmp))
+ goto error;
+ /* Only network endian is supported. */
+ if (!arg->hton)
+ goto error;
+ memcpy(str2, str, len);
+ str2[len] = '\0';
+ ret = inet_pton(AF_INET, str2, &tmp);
+ if (ret != 1) {
+ /* Attempt integer parsing. */
+ push_args(ctx, arg);
+ return parse_int(ctx, token, str, len, buf, size);
+ }
+ if (!ctx->object)
+ return len;
+ buf = (uint8_t *)ctx->object + arg->offset;
+ memcpy(buf, &tmp, size);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg->offset, 0xff, size);
+ return len;
+error:
+ push_args(ctx, arg);
+ return -1;
+}
+
+/**
+ * Parse an IPv6 address.
+ *
+ * Last argument (ctx->args) is retrieved to determine storage size and
+ * location.
+ */
+static int
+parse_ipv6_addr(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg = pop_args(ctx);
+ char str2[len + 1];
+ struct in6_addr tmp;
+ int ret;
+
+ (void)token;
+ /* Argument is expected. */
+ if (!arg)
+ return -1;
+ size = arg->size;
+ /* Bit-mask fill is not supported. */
+ if (arg->mask || size != sizeof(tmp))
+ goto error;
+ /* Only network endian is supported. */
+ if (!arg->hton)
+ goto error;
+ memcpy(str2, str, len);
+ str2[len] = '\0';
+ ret = inet_pton(AF_INET6, str2, &tmp);
+ if (ret != 1)
+ goto error;
+ if (!ctx->object)
+ return len;
+ buf = (uint8_t *)ctx->object + arg->offset;
+ memcpy(buf, &tmp, size);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg->offset, 0xff, size);
+ return len;
+error:
+ push_args(ctx, arg);
+ return -1;
+}
+
/** Boolean values (even indices stand for false). */ static const char *const boolean_name[] = {
"0", "1",
--
2.1.4
^ permalink raw reply
* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Yang, Zhiyong @ 2016-12-20 9:31 UTC (permalink / raw)
To: Ananyev, Konstantin, Thomas Monjalon
Cc: dev@dpdk.org, yuanhan.liu@linux.intel.com, Richardson, Bruce,
De Lara Guarch, Pablo
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0F069A@irsmsx105.ger.corp.intel.com>
Hi, Konstantin:
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, December 16, 2016 7:48 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce rte_memset on
> IA platform
>
> Hi Zhiyong,
>
> > > > > > >
> > > > > > extern void *(*__rte_memset_vector)( (void *s, int c, size_t
> > > > > > n);
> > > > > >
> > > > > > static inline void*
> > > > > > rte_memset_huge(void *s, int c, size_t n) {
> > > > > > return __rte_memset_vector(s, c, n); }
> > > > > >
> > > > > > static inline void *
> > > > > > rte_memset(void *s, int c, size_t n) {
> > > > > > If (n < XXX)
> > > > > > return rte_memset_scalar(s, c, n);
> > > > > > else
> > > > > > return rte_memset_huge(s, c, n); }
> > > > > >
> > > > > > XXX could be either a define, or could also be a variable, so
> > > > > > it can be setuped at startup, depending on the architecture.
> > > > > >
> > > > > > Would that work?
> > > > > > Konstantin
> > > > > >
> > > > I have implemented the code for choosing the functions at run time.
> > > > rte_memcpy is used more frequently, So I test it at run time.
> > > >
> > > > typedef void *(*rte_memcpy_vector_t)(void *dst, const void *src,
> > > > size_t n); extern rte_memcpy_vector_t rte_memcpy_vector; static
> > > > inline void * rte_memcpy(void *dst, const void *src, size_t n) {
> > > > return rte_memcpy_vector(dst, src, n); } In order to
> > > > reduce the overhead at run time, I assign the function address to
> > > > var rte_memcpy_vector before main() starts to init the var.
> > > >
> > > > static void __attribute__((constructor))
> > > > rte_memcpy_init(void)
> > > > {
> > > > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > > > {
> > > > rte_memcpy_vector = rte_memcpy_avx2;
> > > > }
> > > > else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
> > > > {
> > > > rte_memcpy_vector = rte_memcpy_sse;
> > > > }
> > > > else
> > > > {
> > > > rte_memcpy_vector = memcpy;
> > > > }
> > > >
> > > > }
> > >
> > > I thought we discussed a bit different approach.
> > > In which rte_memcpy_vector() (rte_memeset_vector) would be called
> > > only after some cutoff point, i.e:
> > >
> > > void
> > > rte_memcpy(void *dst, const void *src, size_t len) {
> > > if (len < N) memcpy(dst, src, len);
> > > else rte_memcpy_vector(dst, src, len); }
> > >
> > > If you just always call rte_memcpy_vector() for every len, then it
> > > means that compiler most likely has always to generate a proper call
> > > (not inlining happening).
> >
> > > For small length(s) price of extra function would probably
> > > overweight any potential gain with SSE/AVX2 implementation.
> > >
> > > Konstantin
> >
> > Yes, in fact, from my tests, For small length(s) rte_memset is far
> > better than glibc memset, For large lengths, rte_memset is only a bit better
> than memset.
> > because memset use the AVX2/SSE, too. Of course, it will use AVX512 on
> future machine.
>
> Ok, thanks for clarification.
> From previous mails I got a wrong impression that on big lengths
> rte_memset_vector() is significantly faster than memset().
>
> >
> > >For small length(s) price of extra function would probably overweight
> > >any
> > >potential gain.
> > This is the key point. I think it should include the scalar optimization, not
> only vector optimization.
> >
> > The value of rte_memset is always inlined and for small lengths it will be
> better.
> > when in some case We are not sure that memset is always inlined by
> compiler.
>
> Ok, so do you know in what cases memset() is not get inlined?
> Is it when len parameter can't be precomputed by the compiler (is not a
> constant)?
>
> So to me it sounds like:
> - We don't need to have an optimized verision of rte_memset() for big sizes.
> - Which probably means we don't need an arch specific versions of
> rte_memset_vector() at all -
> for small sizes (<= 32B) scalar version would be good enough.
> - For big sizes we can just rely on memset().
> Is that so?
Using memset has actually met some trouble in some case, such as
http://dpdk.org/ml/archives/dev/2016-October/048628.html
>
> > It seems that choosing function at run time will lose the gains.
> > The following is tested on haswell by patch code.
>
> Not sure what columns 2 and 3 in the table below mean?
> Konstantin
Column1 shows Size(bytes).
Column2 shows rte_memset Vs memset perf results in cache
Column3 shows rte_memset Vs memset perf results in memory.
The data is gotten using rte_rdtsc();
The test can be run using [PATCH 3/4] app/test: add performance autotest for rte_memset
Thanks
Zhiyong
>
> > ** rte_memset() - memset perf tests
> > (C = compile-time constant) ** ======== ======= ========
> > ======= ========
> > Size memset in cache memset in mem
> > (bytes) (ticks) (ticks)
> > ------- -------------- --------------- ============= 32B aligned
> > ================
> > 3 3 - 8 19 - 128
> > 4 4 - 8 13 - 128
> > 8 2 - 7 19 - 128
> > 9 2 - 7 19 - 127
> > 12 2 - 7 19 - 127
> > 17 3 - 8 19 - 132
> > 64 3 - 8 28 - 168
> > 128 7 - 13 54 - 200
> > 255 8 - 20 100 - 223
> > 511 14 - 20 187 - 314
> > 1024 24 - 29 328 - 379
> > 8192 198 - 225 1829 - 2193
> >
> > Thanks
> > Zhiyong
^ permalink raw reply
* Re: [PATCH v2 06/25] app/testpmd: implement basic support for rte_flow
From: Adrien Mazarguil @ 2016-12-20 9:38 UTC (permalink / raw)
To: Xing, Beilei; +Cc: dev@dpdk.org, Pei, Yulong
In-Reply-To: <94479800C636CB44BD422CB454846E0131574785@SHSMSX101.ccr.corp.intel.com>
On Tue, Dec 20, 2016 at 01:57:46AM +0000, Xing, Beilei wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Monday, December 19, 2016 6:20 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Pei, Yulong <yulong.pei@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v2 06/25] app/testpmd: implement basic
> > support for rte_flow
> >
> > Hi Beilei,
> >
> > On Mon, Dec 19, 2016 at 08:37:20AM +0000, Xing, Beilei wrote:
> > > Hi Adrien,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien
> > > > Mazarguil
> > > > Sent: Saturday, December 17, 2016 12:25 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH v2 06/25] app/testpmd: implement basic
> > > > support for rte_flow
> > > >
> > > > Add basic management functions for the generic flow API (validate,
> > > > create, destroy, flush, query and list). Flow rule objects and
> > > > properties are arranged in lists associated with each port.
> > > >
> > > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > +/** Create flow rule. */
> > > > +int
> > > > +port_flow_create(portid_t port_id,
> > > > + const struct rte_flow_attr *attr,
> > > > + const struct rte_flow_item *pattern,
> > > > + const struct rte_flow_action *actions) {
> > > > + struct rte_flow *flow;
> > > > + struct rte_port *port;
> > > > + struct port_flow *pf;
> > > > + uint32_t id;
> > > > + struct rte_flow_error error;
> > > > +
> > >
> > > I think there should be memset for error here, e.g. memset(&error, 0,
> > > sizeof(struct rte_flow_error)); Since both cause and message may be NULL
> > regardless of the error type, if there's no error.cause and error.message
> > returned from PMD, Segmentation fault will happen in port_flow_complain.
> > > PS: This issue doesn't happen if add "export EXTRA_CFLAGS=' -g O0'" when
> > compiling.
> >
> > Actually, PMDs must fill the error structure only in case of error if the
> > application provides one, it's not optional. I didn't initialize this structure for
> > this reason.
> >
> > I suggest we initialize it with a known poisoning value for debugging purposes
> > though, to make it fail every time. Does it sound reasonable?
Done for v3 by the way.
> OK, I see. Do you want PMD to allocate the memory for cause and message of error, and must fill the cause and message if error exists, right?
> So is it possible to set NULL for pointers of cause and message in application? then PMD can judge if it's need to allocate or overlap memory.
PMDs never allocate this structure, applications do and initialize it
however they want. They only provide a non-NULL pointer if they want
additional details in case of error.
It will likely be allocated on the stack in most cases (as in testpmd).
>From a PMD standpoint, the contents of this structure must be updated in
case of non-NULL pointer and error state.
Now the message pointer can be allocated dynamically but it's not
recommended, it's far easier to make it point to some constant
string. Applications won't free it anyway, so PMDs would have to do it
during dev_close(). Please see "Verbose error reporting" documentation
section.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v2] doc: fix required tools list layout
From: Mcnamara, John @ 2016-12-20 9:42 UTC (permalink / raw)
To: Baruch Siach, dev@dpdk.org
In-Reply-To: <48da34687fbcd3c2dc2b16e5630c0f1c1b4a904f.1482175720.git.baruch@tkos.co.il>
> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Monday, December 19, 2016 7:29 PM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Baruch Siach
> <baruch@tkos.co.il>
> Subject: [PATCH v2] doc: fix required tools list layout
>
> The Python requirement should appear in the bullet list.
>
> Also, indent the x32 note, since it is related to the previous bullet.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH v3 10/25] app/testpmd: add flow flush command
From: Adrien Mazarguil @ 2016-12-20 9:45 UTC (permalink / raw)
To: Zhao1, Wei; +Cc: dev@dpdk.org
In-Reply-To: <A2573D2ACFCADC41BB3BE09C6DE313CA0200C443@PGSMSX103.gar.corp.intel.com>
Hi Wei,
On Tue, Dec 20, 2016 at 07:32:29AM +0000, Zhao1, Wei wrote:
> Hi, Adrien
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Tuesday, December 20, 2016 1:49 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 10/25] app/testpmd: add flow flush
> > command
> >
> > Syntax:
> >
> > flow flush {port_id}
> >
> > Destroy all flow rules on a port.
> >
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Acked-by: Olga Shern <olgas@mellanox.com>
> > ---
> > app/test-pmd/cmdline.c | 3 +++
> > app/test-pmd/cmdline_flow.c | 43
> > +++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 0dc6c63..6e2b289 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -811,6 +811,9 @@ static void cmd_help_long_parsed(void
> > *parsed_result,
> > " (select|add)\n"
> > " Set the input set for FDir.\n\n"
> >
> > + "flow flush {port_id}\n"
> > + " Destroy all flow rules.\n\n"
> > +
> > "flow list {port_id} [group {group_id}] [...]\n"
> > " List existing flow rules sorted by priority,"
> > " filtered by group identifiers.\n\n"
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index bd3da38..49578eb 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -63,6 +63,7 @@ enum index {
> > FLOW,
> >
> > /* Sub-level commands. */
> > + FLUSH,
> > LIST,
> >
> > /* List arguments. */
> > @@ -179,6 +180,9 @@ static const enum index next_list_attr[] = { static int
> > parse_init(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > +static int parse_flush(struct context *, const struct token *,
> > + const char *, unsigned int,
> > + void *, unsigned int);
> > static int parse_list(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > @@ -240,10 +244,19 @@ static const struct token token_list[] = {
> > .name = "flow",
> > .type = "{command} {port_id} [{arg} [...]]",
> > .help = "manage ingress/egress flow rules",
> > - .next = NEXT(NEXT_ENTRY(LIST)),
> > + .next = NEXT(NEXT_ENTRY
> > + (FLUSH,
> > + LIST)),
> > .call = parse_init,
> > },
> > /* Sub-level commands. */
> > + [FLUSH] = {
> > + .name = "flush",
> > + .help = "destroy all flow rules",
> > + .next = NEXT(NEXT_ENTRY(PORT_ID)),
> > + .args = ARGS(ARGS_ENTRY(struct buffer, port)),
> > + .call = parse_flush,
> > + },
> > [LIST] = {
> > .name = "list",
> > .help = "list existing flow rules",
> > @@ -316,6 +329,31 @@ parse_init(struct context *ctx, const struct token
> > *token,
> > return len;
> > }
> >
> > +/** Parse tokens for flush command. */
> > +static int
> > +parse_flush(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size)
> > +{
> > + struct buffer *out = buf;
> > +
> > + /* Token name must match. */
> > + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> > + return -1;
> > + /* Nothing else to do if there is no buffer. */
> > + if (!out)
> > + return len;
> > + if (!out->command) {
> > + if (ctx->curr != FLUSH)
> > + return -1;
> > + if (sizeof(*out) > size)
> > + return -1;
> > + out->command = ctx->curr;
> > + ctx->object = out;
> > + }
> > + return len;
> > +}
> > +
> > /** Parse tokens for list command. */
> > static int
> > parse_list(struct context *ctx, const struct token *token, @@ -698,6 +736,9
> > @@ static void cmd_flow_parsed(const struct buffer *in) {
> > switch (in->command) {
> > + case FLUSH:
> > + port_flow_flush(in->port);
> > + break;
> > case LIST:
> > port_flow_list(in->port, in->args.list.group_n,
> > in->args.list.group);
> > --
> > 2.1.4
>
> When user flow flush cmd, PMD will flush all the rule on the specific port, and the memory of which rte_flow point to must be flushed.
Right.
> This memory is returned when flow create, will rte layer flush this memory or PMD is responsible for that memory flush?
All handles are considered destroyed and their memory freed, i.e. no
rte_flow object remains valid after flush. Applications still need to clean
up the memory they allocated to manage these objects, but that's their
problem.
> BTW, there is no argument about rte_flow in flush function pass into PMD layer.
Right, that's because flush does not request the destruction of a specific
rule. PMDs that allocate memory for rte_flow objects must link them together
somehow to retrieve them during a flush event.
Note this is likely already necessary to clean up the memory allocated for
flow rules during dev_close().
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3 22/25] app/testpmd: add L4 items to flow command
From: Adrien Mazarguil @ 2016-12-20 9:50 UTC (permalink / raw)
To: Pei, Yulong; +Cc: dev@dpdk.org, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03B66D499@shsmsx102.ccr.corp.intel.com>
Hi Yulong,
On Tue, Dec 20, 2016 at 09:14:55AM +0000, Pei, Yulong wrote:
> Hi Adrien,
>
> For SCTP, it may need support to set 'tag' since in FDIR (--pkt-filter-mode=perfect) need set it when filter sctp flow.
>
> struct sctp_hdr {
> uint16_t src_port; /**< Source port. */
> uint16_t dst_port; /**< Destin port. */
> uint32_t tag; /**< Validation tag. */
> uint32_t cksum; /**< Checksum. */
> } __attribute__((__packed__));
>
> testpmd> flow create 0 ingress pattern eth / ipv4 src is 192.168.0.1 dst is 192.168.0.2 / sctp
> src [TOKEN]: SCTP source port
> dst [TOKEN]: SCTP destination port
> / [TOKEN]: specify next pattern item
Sure, let's add it in a subsequent patch after this series is applied, it is
only a few lines of code in testpmd (basically like all missing protocol
fields).
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v3 21/25] app/testpmd: add items ipv4/ipv6 to flow command
From: Adrien Mazarguil @ 2016-12-20 10:02 UTC (permalink / raw)
To: Pei, Yulong; +Cc: dev@dpdk.org, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03B66D4AB@shsmsx102.ccr.corp.intel.com>
Hi Yulong,
On Tue, Dec 20, 2016 at 09:21:28AM +0000, Pei, Yulong wrote:
> Hi adrien,
>
> Is it possible to support to set ipv4 TOS, ipv4 PROTO, ipv4 TTL and ipv6 tc, ipv6 next-header, ipv6 hop-limits since
> previous FDIR for i40e already support it.
I suggest we add them later (like for SCTP tag, it's just a bunch of new
flow tokens to manage in testpmd), it is not blocking for the API itself.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v2] doc: introduce PVP reference benchmark
From: Thomas Monjalon @ 2016-12-20 10:03 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Mcnamara, John, yuanhan.liu@linux.intel.com, Yang, Zhiyong,
ktraynor@redhat.com, dev
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202671A59@IRSMSX103.ger.corp.intel.com>
> > >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > >
> > > There is one trailing whitespace warning but apart from that:
> > >
> > > Acked-by: John McNamara <john.mcnamara@intel.com>
> >
> > Thanks John,
> >
> > Do you want me to send a v3, fixing the trailing whitespace & collecting
> > the acks?
>
> No need (unless the tree maintainer says otherwise).
> It was one trailing whitespace. Just something to look out for in future.
Removed when applying.
Fixed some heading issues also:
--- a/doc/guides/howto/pvp_reference_benchmark.rst
+++ b/doc/guides/howto/pvp_reference_benchmark.rst
@@ -160,7 +160,7 @@ Testpmd launch
--portmask=f --disable-hw-vlan -i --rxq=1 --txq=1
--nb-cores=4 --forward-mode=io
-With this command, isolated CPUs 2 to 5 will be used as lcores for PMD threads.
+ With this command, isolated CPUs 2 to 5 will be used as lcores for PMD threads.
#. In testpmd interactive mode, set the portlist to obtain the correct port
chaining:
@@ -176,7 +176,8 @@ VM launch
The VM may be launched either by calling QEMU directly, or by using libvirt.
-#. Qemu way:
+Qemu way
+^^^^^^^^
Launch QEMU with two Virtio-net devices paired to the vhost-user sockets
created by testpmd. Below example uses default Virtio-net options, but options
@@ -210,7 +211,8 @@ where isolated CPUs 6 and 7 will be used as lcores for Virtio PMDs:
export PYTHONPATH=$PYTHONPATH:<QEMU path>/scripts/qmp
./qmp-vcpu-pin -s /tmp/qmp.socket 1 6 7
-#. Libvirt way:
+Libvirt way
+^^^^^^^^^^^
Some initial steps are required for libvirt to be able to connect to testpmd's
sockets.
Thanks
^ permalink raw reply
* [PATCH] nfp: add tso support
From: Alejandro Lucero @ 2016-12-20 10:18 UTC (permalink / raw)
To: dev
This patch implements NFP PMD support for TSO but it also requires
a firmware advertising the capability.
Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
doc/guides/nics/features/nfp.ini | 1 +
drivers/net/nfp/nfp_net.c | 40 ++++++++++++++++++++++++++++++++--------
2 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
index 7ac0d34..c04a738 100644
--- a/doc/guides/nics/features/nfp.ini
+++ b/doc/guides/nics/features/nfp.ini
@@ -11,6 +11,7 @@ Queue start/stop = Y
MTU update = Y
Jumbo frame = Y
Promiscuous mode = Y
+TSO = Y
RSS hash = Y
RSS key update = Y
RSS reta update = Y
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index ace9583..cee8f63 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1147,6 +1147,9 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
dev_info->speed_capa = ETH_SPEED_NUM_1G | ETH_LINK_SPEED_10G |
ETH_SPEED_NUM_25G | ETH_SPEED_NUM_40G |
ETH_SPEED_NUM_50G | ETH_LINK_SPEED_100G;
+
+ if (hw->cap & NFP_NET_CFG_CTRL_LSO)
+ dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
}
static const uint32_t *
@@ -1641,6 +1644,27 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
return 0;
}
+/* nfp_net_tx_tso - Set TX descriptor for TSO */
+static inline void
+nfp_net_tx_tso(struct nfp_net_txq *txq, struct nfp_net_tx_desc *txd,
+ struct rte_mbuf *mb)
+{
+ uint64_t ol_flags;
+ struct nfp_net_hw *hw = txq->hw;
+
+ if (!(hw->cap & NFP_NET_CFG_CTRL_LSO))
+ return;
+
+ ol_flags = mb->ol_flags;
+
+ if (!(ol_flags & PKT_TX_TCP_SEG))
+ return;
+
+ txd->l4_offset = mb->l2_len + mb->l3_len + mb->l4_len;
+ txd->lso = rte_cpu_to_le_16(mb->tso_segsz);
+ txd->flags |= PCIE_DESC_TX_LSO;
+}
+
/* nfp_net_tx_cksum - Set TX CSUM offload flags in TX descriptor */
static inline void
nfp_net_tx_cksum(struct nfp_net_txq *txq, struct nfp_net_tx_desc *txd,
@@ -2009,7 +2033,7 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
{
struct nfp_net_txq *txq;
struct nfp_net_hw *hw;
- struct nfp_net_tx_desc *txds;
+ struct nfp_net_tx_desc *txds, txd;
struct rte_mbuf *pkt;
uint64_t dma_addr;
int pkt_size, dma_size;
@@ -2058,19 +2082,17 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
/*
* Checksum and VLAN flags just in the first descriptor for a
- * multisegment packet
+ * multisegment packet, but TSO info needs to be in all of them.
*/
- nfp_net_tx_cksum(txq, txds, pkt);
+ nfp_net_tx_tso(txq, &txd, pkt);
+ nfp_net_tx_cksum(txq, &txd, pkt);
if ((pkt->ol_flags & PKT_TX_VLAN_PKT) &&
(hw->cap & NFP_NET_CFG_CTRL_TXVLAN)) {
- txds->flags |= PCIE_DESC_TX_VLAN;
- txds->vlan = pkt->vlan_tci;
+ txd.flags |= PCIE_DESC_TX_VLAN;
+ txd.vlan = pkt->vlan_tci;
}
- if (pkt->ol_flags & PKT_TX_TCP_SEG)
- rte_panic("TSO is not supported\n");
-
/*
* mbuf data_len is the data in one segment and pkt_len data
* in the whole packet. When the packet is just one segment,
@@ -2088,6 +2110,8 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
*lmbuf = pkt;
while (pkt_size) {
+ /* Copying TSO, VLAN and cksum info */
+ *txds = txd;
dma_size = pkt->data_len;
dma_addr = rte_mbuf_data_dma_addr(pkt);
PMD_TX_LOG(DEBUG, "Working with mbuf at dma address:"
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2] doc: add details of sub-trees and maintainers
From: Thomas Monjalon @ 2016-12-20 10:20 UTC (permalink / raw)
To: John McNamara; +Cc: dev
In-Reply-To: <1481910634-19445-1-git-send-email-john.mcnamara@intel.com>
2016-12-16 17:50, John McNamara:
> Add a new section to the Code Contributors Guide to outline
> the roles of tree and component maintainers and how they
> can be added and removed.
>
> Signed-off-by: John McNamara <john.mcnamara@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2] doc: fix mistakes in contribution guide
From: Thomas Monjalon @ 2016-12-20 10:22 UTC (permalink / raw)
To: Yong Wang; +Cc: dev, Mcnamara, John
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202671B27@IRSMSX103.ger.corp.intel.com>
> > Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
> > ---
> > v2:
> > * modify some redundant descriptions.
>
>
> Thanks,
>
> Acked-by: John McNamara <john.mcnamara@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] nfp: extend speed capabilities advertised
From: Ferruh Yigit @ 2016-12-20 10:25 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: dev
In-Reply-To: <CAD+H991y+h4T6OdCtYJNV4cq3wndGLxK9kkVqyGs5e_TN8Hiqg@mail.gmail.com>
On 12/19/2016 6:00 PM, Alejandro Lucero wrote:
> I forgot one thing: to update the features file with this new one.
>
> I will wait for your feedback regarding the discussed problem for
> sending another version.
I think it is good to go, please send updated version.
>
<...>
>
> Sorry, confused. Is it like following:
>
> "
> For new FW, there is no problem, it supports the range between
> 1G - 50G,
> and reports correct current speed.
>
> With old FW, device still can be set to speed range between 1G -
> 50G,
> but it doesn't report current speed correct, and DPDK driver reports
> back to application as device current speed is 40G, without really
> knowing the current speed.
> "
>
>
> Yes, that is. Should then I do anything else or the patch is right
> for you now?
So, this patch is correct.
But for the previous patch "net/nfp: report link speed using hardware
info", it would be nice to add above information into source as comment
and into commit log.
If you can, would you mind sending a new version of that patch?
Thanks,
ferruh
>
<...>
^ permalink raw reply
* Re: [PATCH v2] doc: fix required tools list layout
From: Thomas Monjalon @ 2016-12-20 10:26 UTC (permalink / raw)
To: Baruch Siach; +Cc: dev, Mcnamara, John
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202689C6D@IRSMSX103.ger.corp.intel.com>
> > The Python requirement should appear in the bullet list.
> >
> > Also, indent the x32 note, since it is related to the previous bullet.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>
> Acked-by: John McNamara <john.mcnamara@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] nfp: extend speed capabilities advertised
From: Alejandro Lucero @ 2016-12-20 10:29 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev
In-Reply-To: <d25cdbc7-b4c5-3709-2d1f-0054f5071fa8@intel.com>
On Tue, Dec 20, 2016 at 10:25 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
> On 12/19/2016 6:00 PM, Alejandro Lucero wrote:
> > I forgot one thing: to update the features file with this new one.
> >
> > I will wait for your feedback regarding the discussed problem for
> > sending another version.
>
> I think it is good to go, please send updated version.
>
>
OK
> >
>
> <...>
>
> >
> > Sorry, confused. Is it like following:
> >
> > "
> > For new FW, there is no problem, it supports the range between
> > 1G - 50G,
> > and reports correct current speed.
> >
> > With old FW, device still can be set to speed range between 1G -
> > 50G,
> > but it doesn't report current speed correct, and DPDK driver
> reports
> > back to application as device current speed is 40G, without
> really
> > knowing the current speed.
> > "
> >
> >
> > Yes, that is. Should then I do anything else or the patch is right
> > for you now?
>
> So, this patch is correct.
>
> But for the previous patch "net/nfp: report link speed using hardware
> info", it would be nice to add above information into source as comment
> and into commit log.
> If you can, would you mind sending a new version of that patch?
>
>
I'll do it.
Thanks!
> Thanks,
> ferruh
>
> >
> <...>
>
^ 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