* Re: [PATCH v7 03/27] net/i40e: set VF MAC anti-spoofing from PF
From: Wu, Jingjing @ 2017-01-05 6:42 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1483426488-117332-4-git-send-email-wenzhuo.lu@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:54 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH v7 03/27] net/i40e: set VF MAC anti-spoofing from
> PF
>
> Support enabling/disabling VF MAC anti-spoofing from PF.
> User can call the API on PF to enable/disable a specific VF's MAC anti-spoofing.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 63
> +++++++++++++++++++++++++++++++
> drivers/net/i40e/rte_pmd_i40e.h | 19 ++++++++++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 83 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index fc7e987..68c07de 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -9723,3 +9723,66 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>
> return 0;
> }
> +
> +int
> +rte_pmd_i40e_set_vf_mac_anti_spoof(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_vsi *vsi;
> + struct i40e_hw *hw;
> + 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);
> +
Why need to call rte_eth_dev_info_get in driver?
> + if (vf_id >= dev_info.max_vfs)
> + return -EINVAL;
> +
Vf_id is already be checked by below, even I prefer :
if (vf_id > pf->vf_num - 1 || !pf->vfs)
to be
if (!pf->vfs || vf_id > pf->vf_num - 1)
or if (!pf->vfs || vf_id >= pf->vf_num)
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
^ permalink raw reply
* Re: [PATCH v2 09/29] eal/arm64: define I/O device memory barriers for arm64
From: Jerin Jacob @ 2017-01-05 6:24 UTC (permalink / raw)
To: Jianbo Liu
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin, Santosh Shukla, david.marchand
In-Reply-To: <CAP4Qi3-r80hfY2Sj5sHVWJFe+ubYDQ4=91OMDme8zGzBibjT9w@mail.gmail.com>
On Thu, Jan 05, 2017 at 01:31:44PM +0800, Jianbo Liu wrote:
> On 4 January 2017 at 18:01, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:
> > On Tue, Jan 03, 2017 at 03:48:32PM +0800, Jianbo Liu wrote:
> >> On 27 December 2016 at 17:49, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > CC: Jianbo Liu <jianbo.liu@linaro.org>
> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> > ---
> >> > lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 6 ++++++
> >> > 1 file changed, 6 insertions(+)
> >> >
> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
> >> > index 78ebea2..ef0efc7 100644
> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
> >> > @@ -88,6 +88,12 @@ static inline void rte_rmb(void)
> >> >
> >> > #define rte_smp_rmb() dmb(ishld)
> >> >
> >> > +#define rte_io_mb() rte_mb()
> >> > +
> >> > +#define rte_io_wmb() rte_wmb()
> >> > +
> >> > +#define rte_io_rmb() rte_rmb()
> >> > +
> >>
> >> I think it's better to use outer shareable dmb for io barrier, instead of dsb.
> >
> > Its is difficult to generalize. AFAIK, from the IO barrier perspective
> > dsb would be the right candidate. But just for the DMA barrier between IO may
> > be outer sharable dmb is enough. In-terms of performance implication, the
> > fastpath code(door bell write) has been changed to relaxed write in all
> > the drivers in this patchset and rte_io_* will be only
> > used by rte_[read/write]8/16/32/64 which will be in slow-path.
> > So, IMO, it better stick with dsb and its safe from the complete IO barrier
> > perspective.
>
> If so, why not use *mb() directly?
Adding David Marchand, EAL Maintainer.
Instead of rte_io_?. I thought, IO specific constraints can be abstracted
here in rte_io_*. Apart from arm, there other arch like "arc" has similar
constraints. IMHO, no harm in keeping that abstraction.
Thoughts ?
http://lxr.free-electrons.com/ident?i=__iormb
>
> >
> > At least on ThunderX, I couldn't see any performance difference between
> > using dsb(st) and dmb(oshst) for dma write barrier before the doorbell register
> > write in fastpath. In case there are platforms which has such performance difference,
> > may be could add rte_dma_wmb() and rte_dma_rmb() in future like Linux kernel
> > dma_wmb() and dma_rmb().(But i couldn't see all the driver are using it,
> > though)
> >
>
> But there is no io_*mb() in the kernel, so you want to be different?
It is their for arm,arm64,arc architectures in Linux kernel. Please check writel
implementation for arm64
http://lxr.free-electrons.com/source/arch/arm64/include/asm/io.h#L143
^ permalink raw reply
* Re: [PATCH v2 1/6] ethdev: fix port data mismatched in multiple process model
From: Yuanhan Liu @ 2017-01-05 6:25 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev, Bruce Richardson, Ferruh Yigit, Gonzalez Monroy, Sergio
In-Reply-To: <1506472.SIAYXrUjSr@xps13>
On Wed, Jan 04, 2017 at 06:34:40PM +0100, Thomas Monjalon wrote:
> +Cc Sergio (maintainer of the secondary process thing)
>
> 2016-12-28 19:02, Yuanhan Liu:
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -201,9 +201,6 @@ rte_eth_dev_allocate(const char *name)
> > return NULL;
> > }
> >
> > - if (rte_eth_dev_data == NULL)
> > - rte_eth_dev_data_alloc();
> > -
>
> It is dangerous to move this to rte_eth_dev_pci_probe.
> Please keep it here and duplicate it in eth_dev_attach.
Oh, right, I missed the fact that it could be invoked from other places.
> [...]
> > +/*
> > + * Attach to a port already registered by the primary process, which
> > + * makes sure that the same device would both have the same port id
> > + * in the primary and secondary process.
> > + */
> > +static struct rte_eth_dev *
> > +eth_dev_attach(const char *name)
>
> Maybe that the word "secondary" could help to differentiate of
> the function rte_eth_dev_attach().
Yes, it's better. How about "_attach_secondary", or "_attach_to_primary"?
> > +{
> > + uint8_t i;
> > + struct rte_eth_dev *eth_dev;
> > +
> > + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> > + if (strcmp(rte_eth_dev_data[i].name, name) == 0)
> > + break;
> > + }
> > + if (i == RTE_MAX_ETHPORTS) {
> > + RTE_PMD_DEBUG_TRACE(
> > + "device %s is not driven by the primary process\n",
> > + name);
> > + return NULL;
> > + }
> > +
> > + RTE_ASSERT(eth_dev->data->port_id == i);
> > +
> > + eth_dev = &rte_eth_devices[i];
> > + eth_dev->data = &rte_eth_dev_data[i];
> > + eth_dev->attached = DEV_ATTACHED;
> > + nb_ports++;
>
> I am a bit nervous when I see these lines duplicated from rte_eth_dev_allocate.
> Not sure whether it deserves a common function or not.
I don't think so, they do share some common assignments, but the assignments
are actually not the same. The primary one has few more: notably, they are:
- eth_dev->data
- eth_dev->data->name
- eth_dev->data->port_id
>
> [...]
> > @@ -246,9 +275,26 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
> > - eth_dev = rte_eth_dev_allocate(ethdev_name);
> > - if (eth_dev == NULL)
> > - return -ENOMEM;
> > + if (rte_eth_dev_data == NULL)
> > + rte_eth_dev_data_alloc();
> > +
> > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > + eth_dev = rte_eth_dev_allocate(ethdev_name);
> > + if (eth_dev == NULL)
> > + return -ENOMEM;
> > + } else {
> > + /*
> > + * if we failed to attach a device, it means that
> > + * device is skipped, due to some errors. Take
> > + * virtio-net device as example, it could be the
> > + * device is managed by virtio-net kernel driver.
> > + * For such case, we return a positive value, to
> > + * let EAL skip it as well.
> > + */
>
> This comment (a bit too long) should be placed between "if" and "return".
Okay.
--yliu
^ permalink raw reply
* Re: [PATCH v7 01/27] net/i40e: support link status notification
From: Lu, Wenzhuo @ 2017-01-05 6:23 UTC (permalink / raw)
To: Wu, Jingjing, dev@dpdk.org
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC342A@SHSMSX103.ccr.corp.intel.com>
Hi Jingjing,
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, January 5, 2017 1:58 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH v7 01/27] net/i40e: support link status
> notification
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > Sent: Tuesday, January 3, 2017 2:54 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > Subject: [dpdk-dev] [PATCH v7 01/27] net/i40e: support link status
> > notification
> >
> > Add an API to expose the ability, that PF can notify VF when link
> > status changes, to APP.
> > So if PF APP doesn't want to enable interruption but check link status
> > by itself, PF APP can let VF know link status changed.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> > drivers/net/i40e/Makefile | 4 ++-
> > drivers/net/i40e/i40e_ethdev.c | 28 +++++++++++++++
> > drivers/net/i40e/i40e_pf.c | 4 +--
> > drivers/net/i40e/i40e_pf.h | 4 ++-
> > drivers/net/i40e/rte_pmd_i40e.h | 58
> > +++++++++++++++++++++++++++++++
> > drivers/net/i40e/rte_pmd_i40e_version.map | 7 ++++
> > 6 files changed, 101 insertions(+), 4 deletions(-) create mode
> > 100644 drivers/net/i40e/rte_pmd_i40e.h
> >
> > diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
> > index 66997b6..a2ef53c 100644
> > --- a/drivers/net/i40e/Makefile
> > +++ b/drivers/net/i40e/Makefile
> > @@ -1,6 +1,6 @@
> > # BSD LICENSE
> > #
> > -# Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> > +# Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
> > # All rights reserved.
> > #
> > # Redistribution and use in source and binary forms, with or without
> > @@ -111,6 +111,8 @@ ifeq ($(findstring
> > RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
> > CFLAGS_i40e_rxtx_vec_sse.o += -msse4.1 endif
> >
> > +# install this header file
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_I40E_PMD)-include := rte_pmd_i40e.h
> >
> > # this lib depends upon:
> > DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_eal
> > lib/librte_ether diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index f42f4ba..fc7e987 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -62,6 +62,7 @@
> > #include "i40e_rxtx.h"
> > #include "i40e_pf.h"
> > #include "i40e_regs.h"
> > +#include "rte_pmd_i40e.h"
> >
> > #define ETH_I40E_FLOATING_VEB_ARG "enable_floating_veb"
> > #define ETH_I40E_FLOATING_VEB_LIST_ARG "floating_veb_list"
> > @@ -9695,3 +9696,30 @@ static void i40e_set_default_mac_addr(struct
> > rte_eth_dev *dev,
> >
> > return ret;
> > }
> > +
> > +int
> > +rte_pmd_i40e_ping_vfs(uint8_t port, uint16_t vf) {
> > + struct rte_eth_dev *dev;
> > + struct rte_eth_dev_info dev_info;
> > + struct i40e_pf *pf;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port];
> > + rte_eth_dev_info_get(port, &dev_info);
> > +
> > + if (vf >= dev_info.max_vfs)
> > + return -EINVAL;
> > +
> > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > +
> > + if (vf > pf->vf_num - 1 || !pf->vfs) {
> How about
> if (!pf->vfs || vf > pf->vf_num - 1) {
> because if vf_num is 0, the pf->vfs will be NULL.
Thanks for the comments.
As the number can be 0, how about change the code to?
if (vf >= pf->vf_num || !pf->vfs) {
>
> Thanks
> Jingjing
^ permalink raw reply
* Re: [PATCH v7 02/27] net/i40e: add callback to user on VF to PF mbox msg
From: Wu, Jingjing @ 2017-01-05 6:18 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1483426488-117332-3-git-send-email-wenzhuo.lu@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:54 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH v7 02/27] net/i40e: add callback to user on VF to PF
> mbox msg
>
> 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>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
^ permalink raw reply
* Re: [PATCH v5 07/17] net/i40e: add flow validate function
From: Xing, Beilei @ 2017-01-05 6:08 UTC (permalink / raw)
To: Yigit, Ferruh, Wu, Jingjing, Zhang, Helin; +Cc: dev@dpdk.org, Zhao1, Wei
In-Reply-To: <1bf8ab46-2ac1-a565-a965-38e68afe60cd@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, January 5, 2017 2:57 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 07/17] net/i40e: add flow validate
> function
>
> On 1/4/2017 3:22 AM, Beilei Xing wrote:
> > This patch adds i40e_flow_validation function to check if a flow is
> > valid according to the flow pattern.
> > i40e_parse_ethertype_filter is added first, it also gets the ethertype
> > info.
> > i40e_flow.c is added to handle all generic filter events.
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
>
> <...>
>
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 153322a..edfd52b 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -8426,6 +8426,8 @@ i40e_ethertype_filter_handle(struct rte_eth_dev
> *dev,
> > return ret;
> > }
> >
> > +const struct rte_flow_ops i40e_flow_ops;
>
> Is this intentional (instead of using extern) ?
> Because i40e_flow.c has a global variable definition with same name, it looks
> like this is not causing a build error, but I think confusing.
>
Actually it's the global variable definition in i40e_flow.c. I thought gcc would add extern automatically during compiling, as I checked the address of the variable is the same in different files.
To avoid confusion, I will add extern in next version.
> <...>
>
> > +static int i40e_parse_ethertype_act(struct rte_eth_dev *dev,
> > + const struct rte_flow_action *actions,
> > + struct rte_flow_error *error,
> > + struct rte_eth_ethertype_filter *filter);
>
> In API naming, I would prefer full "action" instead of shorten "act", but it is
> your call.
I will change the API name in next version. Thanks.
>
> <...>
>
> > +
> > +union i40e_filter_t cons_filter;
>
> Why this cons_filter is required. I can see this is saving some state related
> rule during validate function.
> If the plan is to use this during rule creation, is user has to call validate before
> each create?
You are right, cons_filter will get filter info during validation, and it's for flow_create function.
User needn't to call the flow_validate function, as validate function will be called in i40e_flow_create.
>
> <...>
>
> > +
> > +static int
> > +i40e_parse_ethertype_filter(struct rte_eth_dev *dev,
> > + const struct rte_flow_attr *attr,
> > + const struct rte_flow_item pattern[],
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error,
> > + union i40e_filter_t *filter)
> > +{
> > + struct rte_eth_ethertype_filter *ethertype_filter =
> > + &filter->ethertype_filter;
> > + int ret;
> > +
> > + ret = i40e_parse_ethertype_pattern(dev, pattern, error,
> > + ethertype_filter);
> > + if (ret)
> > + return ret;
> > +
> > + ret = i40e_parse_ethertype_act(dev, actions, error,
> > + ethertype_filter);
> > + if (ret)
> > + return ret;
> > +
> > + ret = i40e_parse_attr(attr, error);
>
> It is your call, but I would suggest using a specific namespace for all rte_flow
> related functions, something like "i40e_flow_".
> In this context it is clear what this function is, but in whole driver code, the
> function name is too generic to understand what it does.
Make sense. I'll update the function names.
>
> > + if (ret)
> > + return ret;
> > +
> > + return ret;
> > +}
> > +
>
> <...>
>
> > +
> > +static int
> > +i40e_parse_ethertype_pattern(__rte_unused struct rte_eth_dev *dev,
> > + const struct rte_flow_item *pattern,
> > + struct rte_flow_error *error,
> > + struct rte_eth_ethertype_filter *filter)
>
> I think it is good idea to comment what pattern is recognized in to function
> comment, instead of reading code every time to figure out.
In fact, the array of i40e_supported_patterns has listed all supported patterns for each filter type.
i40e_supported_patterns is also defined in this patch.
>
> > +{
> > + const struct rte_flow_item *item = pattern;
> > + const struct rte_flow_item_eth *eth_spec;
> > + const struct rte_flow_item_eth *eth_mask;
> > + enum rte_flow_item_type item_type;
> > +
> > + for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> > + if (item->last) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Not support range");
> > + return -rte_errno;
> > + }
> > + item_type = item->type;
> > + switch (item_type) {
> > + case RTE_FLOW_ITEM_TYPE_ETH:
> > + eth_spec = (const struct rte_flow_item_eth *)item-
> >spec;
> > + eth_mask = (const struct rte_flow_item_eth *)item-
> >mask;
> > + /* Get the MAC info. */
> > + if (!eth_spec || !eth_mask) {
>
> Why an eth_mask is required?
Yes, since eth_type mask in eth_mask should be UINT16_MAX.
> Can't driver support drop/queue packets from specific src to specific dst with
> specific eth_type?
No, we support specific dst with specific eth_type, or only specific eth_type. Perfect match.
>
> > + rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "NULL ETH spec/mask");
> > + return -rte_errno;
> > + }
> > +
> > + /* Mask bits of source MAC address must be full of 0.
> > + * Mask bits of destination MAC address must be full
> > + * of 1 or full of 0.
> > + */
> > + if (!is_zero_ether_addr(ð_mask->src) ||
> > + (!is_zero_ether_addr(ð_mask->dst) &&
> > + !is_broadcast_ether_addr(ð_mask->dst))) {
> > + rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Invalid MAC_addr mask");
> > + return -rte_errno;
> > + }
> > +
> > + if ((eth_mask->type & UINT16_MAX) !=
> UINT16_MAX) {
> > + rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Invalid ethertype mask");
>
> Why returning error here?
> Can't we say drop packets to specific MAC address, independent from the
> ether_type?
No. as I said above, we support specific dst with specific eth_type, or only specific eth_type for ethertype_filter.
>
> > + return -rte_errno;
> > + }
> > +
> > + /* If mask bits of destination MAC address
> > + * are full of 1, set RTE_ETHTYPE_FLAGS_MAC.
> > + */
> > + if (is_broadcast_ether_addr(ð_mask->dst)) {
> > + filter->mac_addr = eth_spec->dst;
> > + filter->flags |= RTE_ETHTYPE_FLAGS_MAC;
> > + } else {
> > + filter->flags &= ~RTE_ETHTYPE_FLAGS_MAC;
> > + }
> > + filter->ether_type = rte_be_to_cpu_16(eth_spec-
> >type);
> > +
> > + if (filter->ether_type == ETHER_TYPE_IPv4 ||
> > + filter->ether_type == ETHER_TYPE_IPv6) {
> > + rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Unsupported ether_type
> in"
> > + " control packet filter.");
>
> Can't we create a drop rule based on dst MAC address if eth_type is ip ?
No, we don't support drop MAC_addr + eth_type_IP for ethertype filter.
>
> > + return -rte_errno;
> > + }
> > + if (filter->ether_type == ETHER_TYPE_VLAN)
> > + PMD_DRV_LOG(WARNING, "filter vlan
> ether_type in"
> > + " first tag is not supported.");
>
> Who is the target of this message?
> To the caller, this API is responding as this is supported.
> The end user, the user of the application, can see this message, how this
> message will help to end user?
Actually I add this warning according to the original processing in i40e_dev_eythertype_filter_set.
After checing datasheet, "The ethertype programmed by this command should not be one of the L2 tags ethertype (VLAN, E-tag, S-tag, etc.) and should not be IP or IPv6" is descripted.
But if QinQ is disabled, and inner vlan is ETHER_TYPE_VLAN, the filter works. So the message is "vlan ether_type in outer tag is not supported".
I want to simplify it in next version, don't support the situation above, and return error if (filter->ether_type == ETHER_TYPE_VLAN), because HW only recognizes ETH when QinQ is diabled. What do you think?
>
> > +
> > + break;
> > + default:
> > + break;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +i40e_parse_ethertype_act(struct rte_eth_dev *dev,
> > + const struct rte_flow_action *actions,
> > + struct rte_flow_error *error,
> > + struct rte_eth_ethertype_filter *filter)
>
> I think it would be good to comment this functions to say only DROP and
> QUEUE actions are supported.
Yes, will update in next version.
>
> <...>
>
> > +
> > +static int
> > +i40e_flow_validate(struct rte_eth_dev *dev,
> > + const struct rte_flow_attr *attr,
> > + const struct rte_flow_item pattern[],
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error)
> > +{
> > + struct rte_flow_item *items; /* internal pattern w/o VOID items */
> > + parse_filter_t parse_filter;
> > + uint32_t item_num = 0; /* non-void item number of pattern*/
> > + uint32_t i = 0;
> > + int ret;
> > +
> > + if (!pattern) {
> > + rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM_NUM,
> > + NULL, "NULL pattern.");
> > + return -rte_errno;
> > + }
> > +
> > + if (!actions) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ACTION_NUM,
> > + NULL, "NULL action.");
> > + return -rte_errno;
> > + }
>
> It may be good to validate attr too, if it is NULL or not. It is accessed without
> check in later stages of the call stack.
Yes. Thanks for reminder.
Best Regards,
Beilei
>
> <...>
>
^ permalink raw reply
* Re: [PATCH v7 01/27] net/i40e: support link status notification
From: Wu, Jingjing @ 2017-01-05 5:58 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: Lu, Wenzhuo
In-Reply-To: <1483426488-117332-2-git-send-email-wenzhuo.lu@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:54 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH v7 01/27] net/i40e: support link status notification
>
> Add an API to expose the ability, that PF can notify VF when link status changes,
> to APP.
> So if PF APP doesn't want to enable interruption but check link status by itself, PF
> APP can let VF know link status changed.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> drivers/net/i40e/Makefile | 4 ++-
> drivers/net/i40e/i40e_ethdev.c | 28 +++++++++++++++
> drivers/net/i40e/i40e_pf.c | 4 +--
> drivers/net/i40e/i40e_pf.h | 4 ++-
> drivers/net/i40e/rte_pmd_i40e.h | 58
> +++++++++++++++++++++++++++++++
> drivers/net/i40e/rte_pmd_i40e_version.map | 7 ++++
> 6 files changed, 101 insertions(+), 4 deletions(-) create mode 100644
> drivers/net/i40e/rte_pmd_i40e.h
>
> diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile index
> 66997b6..a2ef53c 100644
> --- a/drivers/net/i40e/Makefile
> +++ b/drivers/net/i40e/Makefile
> @@ -1,6 +1,6 @@
> # BSD LICENSE
> #
> -# Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> +# Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
> # All rights reserved.
> #
> # Redistribution and use in source and binary forms, with or without
> @@ -111,6 +111,8 @@ ifeq ($(findstring
> RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
> CFLAGS_i40e_rxtx_vec_sse.o += -msse4.1
> endif
>
> +# install this header file
> +SYMLINK-$(CONFIG_RTE_LIBRTE_I40E_PMD)-include := rte_pmd_i40e.h
>
> # this lib depends upon:
> DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_eal lib/librte_ether
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index f42f4ba..fc7e987 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -62,6 +62,7 @@
> #include "i40e_rxtx.h"
> #include "i40e_pf.h"
> #include "i40e_regs.h"
> +#include "rte_pmd_i40e.h"
>
> #define ETH_I40E_FLOATING_VEB_ARG "enable_floating_veb"
> #define ETH_I40E_FLOATING_VEB_LIST_ARG "floating_veb_list"
> @@ -9695,3 +9696,30 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>
> return ret;
> }
> +
> +int
> +rte_pmd_i40e_ping_vfs(uint8_t port, uint16_t vf) {
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + if (vf >= dev_info.max_vfs)
> + return -EINVAL;
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> + if (vf > pf->vf_num - 1 || !pf->vfs) {
How about
if (!pf->vfs || vf > pf->vf_num - 1) {
because if vf_num is 0, the pf->vfs will be NULL.
Thanks
Jingjing
^ permalink raw reply
* Re: [PATCH v2 09/29] eal/arm64: define I/O device memory barriers for arm64
From: Jianbo Liu @ 2017-01-05 5:31 UTC (permalink / raw)
To: Jerin Jacob
Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
Jan Viktorin, Santosh Shukla
In-Reply-To: <20170104100104.GA6578@localhost.localdomain>
On 4 January 2017 at 18:01, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:
> On Tue, Jan 03, 2017 at 03:48:32PM +0800, Jianbo Liu wrote:
>> On 27 December 2016 at 17:49, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > CC: Jianbo Liu <jianbo.liu@linaro.org>
>> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> > ---
>> > lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
>> > index 78ebea2..ef0efc7 100644
>> > --- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
>> > +++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
>> > @@ -88,6 +88,12 @@ static inline void rte_rmb(void)
>> >
>> > #define rte_smp_rmb() dmb(ishld)
>> >
>> > +#define rte_io_mb() rte_mb()
>> > +
>> > +#define rte_io_wmb() rte_wmb()
>> > +
>> > +#define rte_io_rmb() rte_rmb()
>> > +
>>
>> I think it's better to use outer shareable dmb for io barrier, instead of dsb.
>
> Its is difficult to generalize. AFAIK, from the IO barrier perspective
> dsb would be the right candidate. But just for the DMA barrier between IO may
> be outer sharable dmb is enough. In-terms of performance implication, the
> fastpath code(door bell write) has been changed to relaxed write in all
> the drivers in this patchset and rte_io_* will be only
> used by rte_[read/write]8/16/32/64 which will be in slow-path.
> So, IMO, it better stick with dsb and its safe from the complete IO barrier
> perspective.
If so, why not use *mb() directly?
>
> At least on ThunderX, I couldn't see any performance difference between
> using dsb(st) and dmb(oshst) for dma write barrier before the doorbell register
> write in fastpath. In case there are platforms which has such performance difference,
> may be could add rte_dma_wmb() and rte_dma_rmb() in future like Linux kernel
> dma_wmb() and dma_rmb().(But i couldn't see all the driver are using it,
> though)
>
But there is no io_*mb() in the kernel, so you want to be different?
^ permalink raw reply
* Re: [PATCH v2 14/18] net/ixgbe: parse L2 tunnel filter
From: Zhao1, Wei @ 2017-01-05 3:12 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev@dpdk.org, Lu, Wenzhuo
In-Reply-To: <20170103140759.GA12822@6wind.com>
Hi, adrien
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, January 3, 2017 10:08 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 14/18] net/ixgbe: parse L2 tunnel filter
>
> Hi Wei,
>
> On Fri, Dec 30, 2016 at 03:53:06PM +0800, Wei Zhao wrote:
> > check if the rule is a L2 tunnel rule, and get the L2 tunnel info.
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >
> > ---
> >
> > v2:
> > --add new error set function
> > --change return value type of parser function
> > ---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 269
> +++++++++++++++++++++++++++++++++++----
> > lib/librte_ether/rte_flow.h | 32 +++++
> > 2 files changed, 273 insertions(+), 28 deletions(-)
> [...]
> > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> > index 98084ac..e9e6220 100644
> > --- a/lib/librte_ether/rte_flow.h
> > +++ b/lib/librte_ether/rte_flow.h
> > @@ -268,6 +268,13 @@ enum rte_flow_item_type {
> > * See struct rte_flow_item_vxlan.
> > */
> > RTE_FLOW_ITEM_TYPE_VXLAN,
> > +
> > + /**
> > + * Matches a E_TAG header.
> > + *
> > + * See struct rte_flow_item_e_tag.
> > + */
> > + RTE_FLOW_ITEM_TYPE_E_TAG,
> > };
> >
> > /**
> > @@ -454,6 +461,31 @@ struct rte_flow_item_vxlan { };
> >
> > /**
> > + * RTE_FLOW_ITEM_TYPE_E_TAG.
> > + *
> > + * Matches a E-tag header.
> > + */
> > +struct rte_flow_item_e_tag {
> > + struct ether_addr dst; /**< Destination MAC. */
> > + struct ether_addr src; /**< Source MAC. */
> > + uint16_t e_tag_ethertype; /**< E-tag EtherType, 0x893F. */
> > + uint16_t e_pcp:3; /**< E-PCP */
> > + uint16_t dei:1; /**< DEI */
> > + uint16_t in_e_cid_base:12; /**< Ingress E-CID base */
> > + uint16_t rsv:2; /**< reserved */
> > + uint16_t grp:2; /**< GRP */
> > + uint16_t e_cid_base:12; /**< E-CID base */
> > + uint16_t in_e_cid_ext:8; /**< Ingress E-CID extend */
> > + uint16_t e_cid_ext:8; /**< E-CID extend */
> > + uint16_t type; /**< MAC type. */
> > + unsigned int tags; /**< Number of 802.1Q/ad tags defined. */
> > + struct {
> > + uint16_t tpid; /**< Tag protocol identifier. */
> > + uint16_t tci; /**< Tag control information. */
> > + } tag[]; /**< 802.1Q/ad tag definitions, outermost first. */ };
> [...]
>
> See my previous reply [1], this definition is not endian-safe and comprises
> protocols defined as independent items (namely ETH and VLAN). Here is an
> untested suggestion:
>
> struct rte_flow_item_e_tag {
> uint16_t tpid; /**< Tag protocol identifier (0x893F). */
> /** E-Tag control information (E-TCI). */
> uint16_t epcp_edei_in_ecid_b; /**< E-PCP (3b), E-DEI (1b), ingress E-CID
> base (12b). */
> uint16_t rsvd_grp_ecid_b; /**< Reserved (2b), GRP (2b), E-CID base (12b).
> */
> uint8_t in_ecid_e; /**< Ingress E-CID ext. */
> uint8_t ecid_e; /**< E-CID ext. */
> };
>
> Applications are responsibile for breaking down and filling individual fields
> properly. Ethernet header would be provided as its own item as shown in
> this testpmd flow command example:
>
> flow create 0 ingress pattern eth / e_tag in_ecid_base is 42 / end actions
> drop / end
>
In this case , is eth an option or mandatory?
I think it is optional, because user may do not have any parameter in ETH config.
> Note, all multibyte values are in network order like other protocol header
> definitions.
>
> [1] http://dpdk.org/ml/archives/dev/2016-December/053181.html
> Message ID: 20161223081310.GH10340@6wind.com
>
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply
* Re: [PATCH v2 0/5] example/ethtool: add bus info and fw version get
From: Zhang, Helin @ 2017-01-05 3:04 UTC (permalink / raw)
To: Thomas Monjalon, Yigit, Ferruh; +Cc: Yang, Qiming, dev@dpdk.org, Horton, Remy
In-Reply-To: <4171707.MgFeI6QLbH@xps13>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, December 22, 2016 11:31 PM
> To: Yigit, Ferruh
> Cc: Yang, Qiming; dev@dpdk.org; Horton, Remy
> Subject: Re: [dpdk-dev] [PATCH v2 0/5] example/ethtool: add bus info and fw
> version get
>
> 2016-12-22 15:05, Ferruh Yigit:
> > On 12/22/2016 2:47 PM, Thomas Monjalon wrote:
> > > 2016-12-22 14:36, Ferruh Yigit:
> > >> On 12/22/2016 11:07 AM, Thomas Monjalon wrote:
> > >>> I think it is OK to add a new dev_ops and a new API function for
> > >>> firmware query. Generally speaking, it is a good thing to avoid
> > >>> putting all informations in the same structure (e.g. rte_eth_dev_info).
> > >>
> > >> OK.
> > >>
> > >>> However, there
> > >>> is a balance to find. Could we plan to add more info to this new query?
> > >>> Instead of
> > >>> rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int
> > >>> fw_length)
> > > [...]
> > >>> could it fill a struct?
> > >>> rte_eth_dev_fw_info_get(uint8_t port_id, struct
> > >>> rte_eth_dev_fw_info *fw_info)
> > >>
> > >> I believe this is better. But the problem we are having with this
> > >> usage
> > >> is: ABI breakage.
> > >>
> > >> Since this struct will be a public structure, in the future if we
> > >> want to add a new field to the struct, it will break the ABI, and
> > >> just this change will cause a new version for whole ethdev library!
> > >>
> > >> When all required fields received via arguments, one by one,
> > >> instead of struct, at least ABI versioning can be done on the API
> > >> when new field added, and can be possible to escape from ABI
> > >> breakage. But this will be ugly when number of arguments increased.
> > >>
> > >> Or any other opinion on how to define API to reduce ABI breakage?
> > >
> > > You're right.
> > > But I don't think we should have a function per data. Just because
> > > it would be ugly :)
> >
> > I am no suggesting function per data, instead something like:
> >
> > rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min);
> >
> > And in the future if we need etrack_id too, we can have both in
> > versioned manner:
> >
> > rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min);
> >
> > rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t maj, uint32_t min,
> > uint32_t etrack_id);
>
> Oh I see. So it can be versioned with compat macros.
>
> > So my concern was if the number of the arguments becomes too many by
> time.
>
> It looks to be a good proposal. We should not have a dozen of arguments.
I'd suggest to do that the similar way of kernel driver/ethtool (Linux or FreeBSD) does,
which should be well discussed.
In addition, for future extention, and avoid breaking any ABI in a strcuture,
we can just pre-define a lot of bytes as reserved, e.g. 64 bytes. Inside DPDK,
there are several strucutres defined like this, e.g. mbuf.
Thanks,
Helin
^ permalink raw reply
* [PATCH] net/mlx5: add support for ConnectX-5 NICs
From: Yongseok Koh @ 2017-01-05 2:32 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, adrien.mazarguil, Yongseok Koh
Add PCI device ID for ConnectX-5 and enable multi-packet send for PF and
VF.
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
ConnectX-5 is a newly announced NIC of Mellanox. This patch includes basic
enablement of ConnectX-5.
drivers/net/mlx5/mlx5.c | 42 +++++++++++++++++++++++++++++++++++++-----
drivers/net/mlx5/mlx5.h | 4 ++++
drivers/net/mlx5/mlx5_ethdev.c | 7 ++-----
drivers/net/mlx5/mlx5_txq.c | 2 +-
4 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index b97b6d16a..6293c1fda 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -286,7 +286,7 @@ mlx5_args_check(const char *key, const char *val, void *opaque)
} else if (strcmp(MLX5_TXQS_MIN_INLINE, key) == 0) {
priv->txqs_inline = tmp;
} else if (strcmp(MLX5_TXQ_MPW_EN, key) == 0) {
- priv->mps = !!tmp;
+ priv->mps &= !!tmp; /* Enable MPW only if HW supports */
} else {
WARN("%s: unknown parameter", key);
return -EINVAL;
@@ -408,10 +408,26 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
sriov = ((pci_dev->id.device_id ==
PCI_DEVICE_ID_MELLANOX_CONNECTX4VF) ||
(pci_dev->id.device_id ==
- PCI_DEVICE_ID_MELLANOX_CONNECTX4LXVF));
- /* Multi-packet send is only supported by ConnectX-4 Lx PF. */
- mps = (pci_dev->id.device_id ==
- PCI_DEVICE_ID_MELLANOX_CONNECTX4LX);
+ PCI_DEVICE_ID_MELLANOX_CONNECTX4LXVF) ||
+ (pci_dev->id.device_id ==
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5VF) ||
+ (pci_dev->id.device_id ==
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF));
+ /*
+ * Multi-packet send is supported by ConnectX-4 Lx PF as well
+ * as all ConnectX-5 devices.
+ */
+ switch (pci_dev->id.device_id) {
+ case PCI_DEVICE_ID_MELLANOX_CONNECTX4LX:
+ case PCI_DEVICE_ID_MELLANOX_CONNECTX5:
+ case PCI_DEVICE_ID_MELLANOX_CONNECTX5VF:
+ case PCI_DEVICE_ID_MELLANOX_CONNECTX5EX:
+ case PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF:
+ mps = 1;
+ break;
+ default:
+ mps = 0;
+ }
INFO("PCI information matches, using device \"%s\""
" (SR-IOV: %s, MPS: %s)",
list[i]->name,
@@ -719,6 +735,22 @@ static const struct rte_pci_id mlx5_pci_id_map[] = {
PCI_DEVICE_ID_MELLANOX_CONNECTX4LXVF)
},
{
+ RTE_PCI_DEVICE(PCI_VENDOR_ID_MELLANOX,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5)
+ },
+ {
+ RTE_PCI_DEVICE(PCI_VENDOR_ID_MELLANOX,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5VF)
+ },
+ {
+ RTE_PCI_DEVICE(PCI_VENDOR_ID_MELLANOX,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5EX)
+ },
+ {
+ RTE_PCI_DEVICE(PCI_VENDOR_ID_MELLANOX,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF)
+ },
+ {
.vendor_id = 0
}
};
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index c415ce32c..ee62e044e 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -83,6 +83,10 @@ enum {
PCI_DEVICE_ID_MELLANOX_CONNECTX4VF = 0x1014,
PCI_DEVICE_ID_MELLANOX_CONNECTX4LX = 0x1015,
PCI_DEVICE_ID_MELLANOX_CONNECTX4LXVF = 0x1016,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5 = 0x1017,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5VF = 0x1018,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5EX = 0x1019,
+ PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF = 0x101a,
};
struct priv {
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 65228d5f9..fbb1b6566 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1517,14 +1517,11 @@ void
priv_select_tx_function(struct priv *priv)
{
priv->dev->tx_pkt_burst = mlx5_tx_burst;
- /* Display warning for unsupported configurations. */
- if (priv->sriov && priv->mps)
- WARN("multi-packet send WQE cannot be used on a SR-IOV setup");
/* Select appropriate TX function. */
- if ((priv->sriov == 0) && priv->mps && priv->txq_inline) {
+ if (priv->mps && priv->txq_inline) {
priv->dev->tx_pkt_burst = mlx5_tx_burst_mpw_inline;
DEBUG("selected MPW inline TX function");
- } else if ((priv->sriov == 0) && priv->mps) {
+ } else if (priv->mps) {
priv->dev->tx_pkt_burst = mlx5_tx_burst_mpw;
DEBUG("selected MPW TX function");
}
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index 053665d55..4f36402eb 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -412,7 +412,7 @@ txq_ctrl_setup(struct rte_eth_dev *dev, struct txq_ctrl *txq_ctrl,
.obj = tmpl.qp,
/* Enable multi-packet send if supported. */
.family_flags =
- ((priv->mps && !priv->sriov) ?
+ (priv->mps ?
IBV_EXP_QP_BURST_CREATE_ENABLE_MULTI_PACKET_SEND_WR :
0),
};
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v4] ethtool: dispaly bus information
From: Yang, Qiming @ 2017-01-05 1:51 UTC (permalink / raw)
To: Mcnamara, John, dev@dpdk.org; +Cc: Yigit, Ferruh, Zhang, Helin, Horton, Remy
In-Reply-To: <B27915DBBA3421428155699D51E4CFE2026A6CA2@IRSMSX103.ger.corp.intel.com>
-----Original Message-----
From: Mcnamara, John
Sent: Wednesday, January 4, 2017 10:49 PM
To: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>; Yang, Qiming <qiming.yang@intel.com>
Subject: RE: [dpdk-dev] [PATCH v4] ethtool: dispaly bus information
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang
> Sent: Wednesday, January 4, 2017 12:18 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>
> Subject: [dpdk-dev] [PATCH v4] ethtool: dispaly bus information
Typo: s/dispaly/display in this and other patches.
Qiming: Will correct them, thank you.
^ permalink raw reply
* Re: [PATCH v4 2/5] net/e1000: add firmware version get
From: Yang, Qiming @ 2017-01-05 1:50 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Zhang, Helin, Horton, Remy
In-Reply-To: <cc171fa6-7a8b-6580-b258-92854af6eedf@intel.com>
-----Original Message-----
From: Yigit, Ferruh
Sent: Wednesday, January 4, 2017 10:00 PM
To: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org
Cc: Zhang, Helin <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>
Subject: Re: [PATCH v4 2/5] net/e1000: add firmware version get
On 1/4/2017 12:03 PM, Qiming Yang wrote:
> This patch adds a new function eth_igb_fw_version_get.
>
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>
> ---
<...>
>
> static void
> +eth_igb_fw_version_get(struct rte_eth_dev *dev, u32 *fw_major, u32 *fw_minor,
> + u32 *fw_patch, u32 *etrack_id)
> +{
<...>
> + default:
> + /* if option rom is valid, display its version too*/
> + if (fw.or_valid) {
> + *fw_major = fw.eep_major;
> + *fw_minor = fw.eep_minor;
> + *etrack_id = fw.etrack_id;
> + *fw_patch = fw.or_patch;
> + /* no option rom */
> + } else {
> + if (fw.etrack_id != 0X0000) {
> + *fw_major = fw.eep_major;
> + *fw_minor = fw.eep_minor;
> + *etrack_id = fw.etrack_id;
indentation is wrong here. Also it looks like major, minor assignment is common and can be moved from if statement.
Qiming: will correct them
> + } else {
> + *fw_major = fw.eep_major;
> + *fw_minor = fw.eep_minor;
> + }
> + }
> + break;
> + }
> +}
<...>
^ permalink raw reply
* Re: [PATCH v4 5/5] ethtool: dispaly firmware version
From: Yang, Qiming @ 2017-01-05 1:31 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Zhang, Helin, Horton, Remy
In-Reply-To: <416747ed-2df3-10d9-5f01-39e77c84b66b@intel.com>
-----Original Message-----
From: Yigit, Ferruh
Sent: Wednesday, January 4, 2017 10:01 PM
To: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org
Cc: Zhang, Helin <helin.zhang@intel.com>; Horton, Remy <remy.horton@intel.com>
Subject: Re: [PATCH v4 5/5] ethtool: dispaly firmware version
On 1/4/2017 12:03 PM, Qiming Yang wrote:
> This patch enhances the ethtool example to support to show firmware
> version, in the same way that the Linux kernel ethtool does.
>
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>
<...>
>
> @@ -61,6 +67,12 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
> dev_info.driver_name);
> snprintf(drvinfo->version, sizeof(drvinfo->version), "%s",
> rte_version());
> + if (strcmp(drvinfo->driver, "net_ixgbe") == 0)
Do you need this check. I think it is not good idea to add this kind of checks into ethtool app. Why not just print "%d.%d.%d %#X, major, minor, patch, etrack" for all cases ?
Qiming: because I want to keep the format same with kernel version ethtool.
> + snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
> + "0x%08x", etrack);
> + else
> + snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
> + "%d.%02d 0x%08x", fw_major, fw_minor, etrack);
> if (dev_info.pci_dev)
> snprintf(drvinfo->bus_info, sizeof(drvinfo->bus_info),
> "%04x:%02x:%02x.%x",
>
^ permalink raw reply
* Re: [PATCH v3 1/4] ethdev: add firmware information get
From: Wu, Jingjing @ 2017-01-05 1:04 UTC (permalink / raw)
To: Yigit, Ferruh, Yang, Qiming; +Cc: dev@dpdk.org, Horton, Remy, Thomas Monjalon
In-Reply-To: <d7f082cb-0aa0-2648-1a17-f7e6bc236160@intel.com>
> > Different HW may have different version format, so it is better to use string.
> >
> > And I prefer the API definition in your v2 patch like
> >
> > rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int
> > fw_length);
>
> The problem with this is the format and content of the string is not defined, as
> you said different HW has different version format. This is no problem if you will
> only print the string.
>
> But this is a public API, if an application wants to call this API and do something
> useful according the FW version information, it will need to parse the string, and
> it will not able to parse it because format of the string is not defined. By making
> API fill some defined variables, app won't need to parse them, and API output
> won't be HW dependent.
>
As my understand, the firmware version is specific things to each HW. The format
cannot be generic at all, or at least we have no standard to follow. I think
the API should not be HW dependent, but about the output, application should know
What the string's meaning. Otherwise why we need to provide the firmware info?
Thanks
Jingjing
^ permalink raw reply
* Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
From: Tiwei Bie @ 2017-01-04 23:56 UTC (permalink / raw)
To: Ananyev, Konstantin
Cc: dev@dpdk.org, adrien.mazarguil@6wind.com, Lu, Wenzhuo,
Mcnamara, John, olivier.matz@6wind.com, thomas.monjalon@6wind.com,
Zhang, Helin, Dai, Wei, Wang, Xiao W
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F100375@irsmsx105.ger.corp.intel.com>
On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
[...]
> > >
> > > I understand that.
> > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > One of them is ixgbe, while other would be some other type.
> > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > But it would mean completely different thing.
> > > How bonded device would know that to deal properly?
> > >
> > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > >
> > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > What does it really mean if there are different NIC types in the system?
> > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > Which I suppose, is not always convenient.
> > >
> >
> > The main purpose is to put the PMD-specific APIs in a separate
> > namespace instead of mixing the PMD-specific APIs and global APIs
> > up, and also save the bits in mbuf.ol_flags.
> >
> > There are other ways to achieve this goal, such as introducing
> > the PMD specific ol_flags in mbuf second cache line as you said.
> > I just thought defining some reserved bits seems to be the most
> > simple way which won't introduce many changes.
> >
> > What's your suggestions? Should I just revert the changes and
> > define the generic flags directly?
>
> Yes, that would be my preference.
> As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> In return there would be no need to consider how to handle all that confusing scenarios in future.
Okay. I'll update my patches. Thanks a lot for your comments.
Thanks & regards,
Tiwei Bie
^ permalink raw reply
* Re: Running DPDK as an unprivileged user
From: Walker, Benjamin @ 2017-01-04 21:35 UTC (permalink / raw)
To: thomas.monjalon@6wind.com; +Cc: stephen@networkplumber.org, dev@dpdk.org
In-Reply-To: <3821624.b18tgR1uvW@xps13>
On Wed, 2017-01-04 at 11:11 +0100, Thomas Monjalon wrote:
> 2017-01-03 22:50, Walker, Benjamin:
> > 1) Physical addresses cannot be exposed to unprivileged users due to
> > security
> > concerns (the fallout of rowhammer). Therefore, systems without an IOMMU can
> > only support privileged users. I think this is probably fine.
> > 2) The IOCTL from vfio to pin the memory is tied to specifying the DMA
> > address
> > and programming the IOMMU. This is unfortunate - systems without an IOMMU
> > still
> > want to do the pinning, but they need to be given the physical address
> > instead
> > of specifying a DMA address.
> > 3) Not all device types, particularly in virtualization environments,
> > support
> > vfio today. These devices have no way to explicitly pin memory.
>
> In VM we can use VFIO-noiommu. Is it helping for mapping?
There does not appear to be a vfio IOCTL that pins memory without also
programming the IOMMU, so vfio-noiommu is broken in the same way that uio is for
drivers that require physical memory.
^ permalink raw reply
* Re: Running DPDK as an unprivileged user
From: Walker, Benjamin @ 2017-01-04 21:34 UTC (permalink / raw)
To: Tan, Jianfeng, dev@dpdk.org
In-Reply-To: <685186b4-e50e-c122-459b-e4635404c3f8@intel.com>
On Wed, 2017-01-04 at 19:39 +0800, Tan, Jianfeng wrote:
> Hi Benjamin,
>
>
> On 12/30/2016 4:41 AM, Walker, Benjamin wrote:
> > DPDK today begins by allocating all of the required
> > hugepages, then finds all of the physical addresses for
> > those hugepages using /proc/self/pagemap, sorts the
> > hugepages by physical address, then remaps the pages to
> > contiguous virtual addresses. Later on and if vfio is
> > enabled, it asks vfio to pin the hugepages and to set their
> > DMA addresses in the IOMMU to be the physical addresses
> > discovered earlier. Of course, running as an unprivileged
> > user means all of the physical addresses in
> > /proc/self/pagemap are just 0, so this doesn't end up
> > working. Further, there is no real reason to choose the
> > physical address as the DMA address in the IOMMU - it would
> > be better to just count up starting at 0.
>
> Why not just using virtual address as the DMA address in this case to
> avoid maintaining another kind of addresses?
That's a valid choice, although I'm just storing the DMA address in the
physical address field that already exists. You either have a physical
address or a DMA address and never both.
>
> > Also, because the
> > pages are pinned after the virtual to physical mapping is
> > looked up, there is a window where a page could be moved.
> > Hugepage mappings can be moved on more recent kernels (at
> > least 4.x), and the reliability of hugepages having static
> > mappings decreases with every kernel release.
>
> Do you mean kernel might take back a physical page after mapping it to a
> virtual page (maybe copy the data to another physical page)? Could you
> please show some links or kernel commits?
Yes - the kernel can move a physical page to another physical page
and change the virtual mapping at any time. For a concise example
see 'man migrate_pages(2)', or for a more serious example the code
that performs memory page compaction in the kernel which was
recently extended to support hugepages.
Before we go down the path of me proving that the mapping isn't static,
let me turn that line of thinking around. Do you have any documentation
demonstrating that the mapping is static? It's not static for 4k pages, so
why are we assuming that it is static for 2MB pages? I understand that
it happened to be static for some versions of the kernel, but my understanding
is that this was purely by coincidence and never by intention.
>
> > Note that this
> > probably means that using uio on recent kernels is subtly
> > broken and cannot be supported going forward because there
> > is no uio mechanism to pin the memory.
> >
> > The first open question I have is whether DPDK should allow
> > uio at all on recent (4.x) kernels. My current understanding
> > is that there is no way to pin memory and hugepages can now
> > be moved around, so uio would be unsafe. What does the
> > community think here?
> >
> > My second question is whether the user should be allowed to
> > mix uio and vfio usage simultaneously. For vfio, the
> > physical addresses are really DMA addresses and are best
> > when arbitrarily chosen to appear sequential relative to
> > their virtual addresses.
>
> Why "sequential relative to their virtual addresses"? IOMMU table is for
> DMA addr -> physical addr mapping. So we need to DMA addresses
> "sequential relative to their physical addresses"? Based on your above
> analysis on how hugepages are initialized, virtual addresses is a good
> candidate for DMA address?
The code already goes through a separate organizational step on all of
the pages that remaps the virtual addresses such that they're sequential
relative to the physical backing pages, so this mostly ends up as the same
thing.
Choosing to use the virtual address is a totally valid choice, but I worry it
may lead to confusion during debugging or in a multi-process scenario.
I'm open to making this choice instead of starting from zero, though.
>
> Thanks,
> Jianfeng
^ permalink raw reply
* Re: [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
From: Scott Daniels @ 2017-01-04 21:09 UTC (permalink / raw)
To: dev; +Cc: "jing.d.chen at intel.com,vincent.jardin", kaustubh,
az5157
In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D3C5CA3DA@shsmsx102.ccr.corp.intel.com>
> Vincent,
>
> Sorry, I missed this reply.
>
>>
>> Le 22/12/2016 à 09:10, Chen, Jing D a écrit :
>> > In the meanwhile, we have some test models ongoing to validate
>> > combination of Linux and DPDK drivers for VF and PF. We'll fully
support
>> below 4 cases going forward.
>> > 1. DPDK PF + DPDK VF
>> > 2. DPDK PF + Linux VF
>>
>> + DPDK PF + FreeBSD VF
>> + DPDK PF + Windows VF
>> + DPDK PF + OS xyz VF
>>
>
> If all drivers follow same API spec, what's the problem here?
> What extra DPDK PF effort you observed?
>
>> > 3. Linux PF + DPDK VF
>> > 4. Linux PF + Linux VF (it's not our scope)
>>
>> So, you confirm the issue: having DPDK becoming a PF, even if SRIOV
protocol
>> includes version-ing, it doubles the combinatory cases.
>
> If extended functions are needed, the answer is yes.
> That's not a big problem, right? I have several workarounds/approaches to
> support extended funcs while following original API spec. I can fix
it in this
> release. In order to have a mature solution, I left it here for
further implementation.
>
>>
>> >
>> > After applied this patch, i've done below test without observing
>> compatibility issue.
>> > 1. DPDK PF + DPDK VF (middle of 16.11 and 17.02 code base). PF to
support
>> API 1.0 while VF
>> > to support API 1.1/1.0
>> > 2. DPDK PF + Linux VF 1.5.14. PF to support 1.0, while Linux to
>> > support 1.1/1.0
>> >
>> > Linux PF + DPDK VF has been tested with 1.0 API long time ago. There
>> > is some test activities ongoing.
>> >
>> > Finally, please give strong reasons to support your NAC.
>>
>> I feel bad because I do recognize the strong and hard work that you
have done
>> on this PF development, but I feel we need first to assess if DPDK
should
>> become a PF or not. I know ixgbe did open the path and that they are
some
>> historical DPDK PF supports in Intel NICs, but before we generalize
it, we have
>> to make sure we are not turning this DataPlane development Kit into a
>> ControlPlane Driver Kit that we are scared to upstream into Linux
kernel. Even
>> if "DPDK is not Linux", it does not mean that Linux should be
ignored. In case
>> of DPDK on other OS, same, their PF could be extended too.
>>
>
> Thanks for the recognition of our work on PF driver. :)
>
>> So currently, yes, I do keep a nack't
>>
>> Since DPDK PF features can be into Linux PF features too and since
Linux (and
>> other hypervisors) has already some tools to manage PF (see
iproute2, etc.),
>> why should we have an other management path with DPDK?
>> DPDK is aimed to be a Dataplane Development kit, not a
management/control
>> plane driver kit.
>
> Before we debated on Dataplane and ControPlane, can you answer me a
question,
> why we have generic filter API? Is it a API for dataplane?
>
> I can't imagine that we'll have to say 'you need to use Linux PF'
driver when users
> want to deploy PF + VF cases. Why we can't provide an alternative
option. they are not
> exclusive and users can decide which combination is better for them.
> The reason why we developed DPDK PF host driver is we have
requirements from
> users. Our motivation is simple, there are requirements, we satisfy them.
>
> Sorry, you NACK can't convince me.
>
>>
>> Assuming you want to use DPDK PF for dataplane feature, that could be OK
>> then, using:
>> - configure one VF on the hypervisor from Linux's PF, let's name if
>> VF_forPFtraffic, see
http://dpdk.org/doc/guides/howto/flow_bifurcation.html
>> - have no (or few IO)s to the PF's queue
>> - assign the traffic to all VF_forPFtraffic's queues of the
hypervisor,
>> - run DPDK into the hypervisor's VF_forPFtraffic
>>
>> Doing so, we get the same benefit of running DPDK over PF or running
DPDK
>> over VF_forPFtraffic, don't we? It is a benefit of:
>> http://dpdk.org/doc/guides/howto/flow_bifurcation.html
>>
>> Thank you,
>> Vincent
>>
With holidays we are a bit late with our thoughts, but would like to
toss them into the mix.
The original NAK is understandable, however having the ability to
configure the PF via DPDK is advantageous for several reasons:
1) While some functions may be duplicated and/or available from the kernel
driver, it is often not possible to introduce new kernel drivers into
production without a large amount of additional testing of the entire
platform which can cause a significant delay when introducing a DPDK based
product. If the PF control is a part of the DPDK environment, then only
the application needs to pass the operational testing before deployment; a
much more simple task.
2) If the driver changes are upstreamed into the kernel proper, the
difficulty of operational readiness testing increases as a new kernel is
introduced, and further undermines the ability to quickly and easily
release a DPDK based application into production. While the application
may eventually fall back on driver and/or kernel support, this could be
years away.
3) As DPDK is being used to configure the NIC, it just seems to make
sense, for consistency, that the configuration capabilities should include
the ability to configure the PF as is proposed.
We are currently supporting/enhancing one such DPDK application to
manage PF and VFs of which the VFs are exposed as SR-IOV devices to
guests:
https://github.com/att/vfd/wiki. As new NICs become available the ability
to transition to them is important to DPDK users.
Collectively,
Scott Daniels,
Alex Zelezniak,
Kaustubh Joshi
------------------------------------------------------------------------
E. Scott Daniels
Cloud Software Research
AT&T Labs
daniels at research.att.com
^ permalink raw reply
* Re: [PATCH] eal: define generic vector types
From: Thomas Monjalon @ 2017-01-04 20:34 UTC (permalink / raw)
To: 'Nelio Laranjeiro'
Cc: Chao Zhu, dev, 'Jianbo Liu', 'Jerin Jacob',
'Zhigang Lu', 'Liming Sun',
'Bruce Richardson', 'Konstantin Ananyev',
'Adrien Mazarguil'
In-Reply-To: <000301d24a18$5aedf360$10c9da20$@linux.vnet.ibm.com>
> > Add common vector type definitions to all CPU architectures.
> >
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>
> Acked-by: Chao Zhu < chaozhu@linux.vnet.ibm.com>
Applied, thanks
It is a first important step to use vector instructions with generic C code
when possible.
^ permalink raw reply
* Re: [PATCH 0/3] buildtools/devtools/usertools
From: Thomas Monjalon @ 2017-01-04 20:22 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit
In-Reply-To: <e7162286-743f-41d1-f67d-245cbf0415bd@intel.com>
2016-12-20 15:24, Ferruh Yigit:
> On 12/15/2016 9:59 PM, Thomas Monjalon wrote:
> > The current tools/ and scripts/ directory names are not
> > self describing.
> > These patches create devtools/ and usertools/ directories.
>
> I think classifying scripts is a good idea.
>
> >
> > Thomas Monjalon (3):
> > scripts: move to buildtools
> > scripts: move to devtools
> > tools: move to usertools
> >
> <...>
>
> Series Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied
^ permalink raw reply
* Re: [PATCH v4 0/3] app: make python apps python2/3 compliant
From: Thomas Monjalon @ 2017-01-04 20:15 UTC (permalink / raw)
To: John McNamara; +Cc: dev, mkletzan, nhorman
In-Reply-To: <1482332629-11783-1-git-send-email-john.mcnamara@intel.com>
2016-12-21 15:03, John McNamara:
> These patches refactor the DPDK Python applications to make them Python 2/3
> compatible.
>
> In order to do this the patchset starts by making the apps PEP8 compliant in
> accordance with the DPDK Coding guidelines:
>
> http://dpdk.org/doc/guides/contributing/coding_style.html#python-code
>
> Implementing PEP8 and Python 2/3 compliance means that we can check all future
> Python patches for consistency. Python 2/3 support also makes downstream
> packaging easier as more distros move to Python 3 as the system python.
Applied, thanks
^ permalink raw reply
* Re: [PATCH] mk: disable ICC warning 188
From: Thomas Monjalon @ 2017-01-04 19:57 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Adrien Mazarguil, dev
In-Reply-To: <20170103171907.GD12822@6wind.com>
2017-01-03 18:19, Adrien Mazarguil:
> On Tue, Jan 03, 2017 at 04:15:42PM +0000, Ferruh Yigit wrote:
> > error #188: enumerated type mixed with another type
> >
> > This is get when an integer assigned to an enum variable.
> >
> > Since this usage is common and causing many ICC compilation errors, and
> > other compilers accept this usage. Disabling the warning.
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
> I also think this warning may be useful but is not worth the trouble in many
> cases, thus:
>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Applied, thanks
I think we should mark ICC support as best effort to make clear
that no ICC check is required from contributors.
^ permalink raw reply
* Re: [PATCH v15 0/8] add Tx preparation
From: Thomas Monjalon @ 2017-01-04 19:41 UTC (permalink / raw)
To: Tomasz Kulasek; +Cc: dev
In-Reply-To: <1482518454-7592-1-git-send-email-tomaszx.kulasek@intel.com>
2016-12-23 19:40, Tomasz Kulasek:
> v15 changes:
> - marked rte_eth_tx_prepare api as experimental
> - improved doxygen comments for nb_seg_max and nb_mtu_seg_max fields
> - removed unused "uint8_t tx_prepare" declaration from testpmd
No you didn't remove this useless declaration. I did it for you.
This feature is now applied! Thanks and congratulations :)
^ permalink raw reply
* Re: [PATCH v2 00/25] Generic flow API (rte_flow)
From: John Fastabend @ 2017-01-04 19:34 UTC (permalink / raw)
To: Adrien Mazarguil, Simon Horman; +Cc: dev
In-Reply-To: <20170104181219.GH12822@6wind.com>
[...]
>>> Well, it should be easier to answer if you have a specific use-case in mind
>>> you would like to support but that cannot be expressed with the API as
>>> defined in [1], in which case please share it with the community.
>>
>> A use-case would be implementing OvS DPIF flow offload using this API.
>
> OK, OVS has been mentioned several times in this thread and my understanding
> is that rte_flow seems to accommodate most of its needs according to people
> familiar with it. Perhaps ML archives can answer the remaining questions you
> may have about combining rte_flow with OVS.
For what its worth. I reviewed this and believe it should be sufficient
to support the OVS SR-IOV offload use case with action/classifier extensions
but without any fundamental changes to the design. We built a prototype
OVS offload on top of another API we dubbed Flow-API a year+ ago and there
seems to be a 1:1 mapping between that older API and the one now in DPDK
so I'm happy. And the missing things seem to fit nicely into extensions.
Also I believe the partial pre-classify use cases should be easily handled
as well although I'm not as familiar with the bit-level details of that
implementation.
At some point capability discovery will be useful but we certainly don't need
those in first iteration and rte_flow doesn't preclude these type of extensions
so that is good.
By the way thanks for doing this work Adrien, glad to see it being accepted
and drivers picking it up.
Thanks,
John
>
>>> [1] http://dpdk.org/ml/archives/dev/2016-December/052954.html
>>> [2] http://dpdk.org/ml/archives/dev/2016-December/052975.html
>
> [3] http://dpdk.org/doc/guides/prog_guide/rte_flow.html
>
^ 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