* Re: [PATCH v1 2/2] Test cases for rte_memcmp functions
From: Thomas Monjalon @ 2017-01-02 20:41 UTC (permalink / raw)
To: Wang, Zhihong, Ravi Kerur; +Cc: dev
In-Reply-To: <8F6C2BD409508844A0EFC19955BE094110759EFE@SHSMSX103.ccr.corp.intel.com>
2016-06-07 11:09, Wang, Zhihong:
> From: Ravi Kerur [mailto:rkerur@gmail.com]
> > Zhilong, Thomas,
> >
> > If there is enough interest within DPDK community I can work on adding support
> > for 'unaligned access' and 'test cases' for it. Please let me know either way.
>
> Hi Ravi,
>
> This rte_memcmp is proved with better performance than glibc's in aligned
> cases, I think it has good value to DPDK lib.
>
> Though we don't have memcmp in critical pmd data path, it offers a better
> choice for applications who do.
Re-thinking about this series, could it be some values to have a rte_memcmp
implementation?
What is the value compared to glibc one? Why not working on glibc?
^ permalink raw reply
* Re: Running DPDK as an unprivileged user
From: Stephen Hemminger @ 2017-01-02 19:47 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Walker, Benjamin, dev
In-Reply-To: <30911426.SrBqgP6jBN@xps13>
On Mon, 02 Jan 2017 15:32:08 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2016-12-29 17:14, Stephen Hemminger:
> > On Thu, 29 Dec 2016 20:41:21 +0000
> > "Walker, Benjamin" <benjamin.walker@intel.com> wrote:
> > > 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. For uio, they are physical
> > > addresses and are not chosen at all. It seems that these two
> > > things are in conflict and that it will be difficult, ugly,
> > > and maybe impossible to resolve the simultaneous use of
> > > both.
> >
> > Unless application is running as privileged user (ie root), UIO
> > is not going to work. Therefore don't worry about mixed environment.
>
> Yes, mixing UIO and VFIO is possible only as root.
> However, what is the benefit of mixing them?
One possible case where this could be used, Hyper-V/Azure and SR-IOV.
The VF interface will show up on an isolated PCI bus and the virtual NIC
is on VMBUS. It is possible to use VFIO on the PCI to get MSI-X per queue
interrupts, but there is no support for VFIO on VMBUS.
^ permalink raw reply
* Re: [PATCH v2 3/5] test: add distributor_perf autotest
From: Hunt, David @ 2017-01-02 16:24 UTC (permalink / raw)
To: Jerin Jacob; +Cc: dev, bruce.richardson
In-Reply-To: <20161222121943.GA8778@localhost.localdomain>
On 22/12/2016 12:19 PM, Jerin Jacob wrote:
> On Thu, Dec 22, 2016 at 04:37:06AM +0000, David Hunt wrote:
>> + struct rte_distributor_burst *d = arg;
>> + unsigned int count = 0;
>> + unsigned int num = 0;
>> + unsigned int id = __sync_fetch_and_add(&worker_idx, 1);
> Use rte_atomic equivalent
Jerin,
I'm looking for an equivalent, but I can't seem to find one. Here
I'm assigning 'id' with the incremented value of worker_idx in one
statement.
However, rte_atomic32_add just increments the variable and returns void,
so I'd have to use two statements, losing the atomicity.
static inline void
rte_atomic32_add(rte_atomic32_t *v, int32_t inc)
There's a second reason why I can't use the rte_atomics, and that's
because worker_idx is a volatile.
Maybe we could add new atomic functions in the future to address this?
Thanks,
Dave.
^ permalink raw reply
* Re: DPDK Accelaration Enhancement
From: Thomas Monjalon @ 2017-01-02 15:54 UTC (permalink / raw)
To: Ant loves honey; +Cc: dev, Jain, Deepak K
In-Reply-To: <1528264043.1886486.1482733162163@mail.yahoo.com>
2016-12-26 06:19, Ant loves honey:
> I am trying to figure out what it takes to have compression
> support on DPDK such as new PMD driver, additional defines
> and/or API or chipset initialization since the Intel QAT can
> support compression.
You need to solve 2 new things in DPDK:
- introduce a generic compression API
- be able to use a QAT device with a compression driver while
being used with a crypto driver
Interesting work to do :)
^ permalink raw reply
* Re: [PATCH RFC 0/2] Allow vectorized Rx with 4096 desc ring size on Intel NICs.
From: Thomas Monjalon @ 2017-01-02 15:40 UTC (permalink / raw)
To: Ilya Maximets, Ferruh Yigit
Cc: dev, Helin Zhang, Konstantin Ananyev, Jingjing Wu, Heetae Ahn,
Bruce Richardson, Wenzhuo Lu
In-Reply-To: <1ed69cc6-4980-fd24-9db5-3ca1b99be70f@samsung.com>
2016-12-27 08:03, Ilya Maximets:
> Hello.
> Ferruh, Thomas, is there a chance for this to be accepted to 17.02?
> Maybe I should resend this patch-set without 'RFC' tag?
Yes it should be integrated in 17.02.
Ferruh, any news?
^ permalink raw reply
* Re: [PATCH v3 1/4] ethdev: add firmware information get
From: Thomas Monjalon @ 2017-01-02 15:38 UTC (permalink / raw)
To: Qiming Yang; +Cc: dev, remy.horton, ferruh.yigit
In-Reply-To: <1482841816-54143-2-git-send-email-qiming.yang@intel.com>
2016-12-27 20:30, Qiming Yang:
> /**
> + * Retrieve the firmware version of a device.
> + *
> + * @param port_id
> + * The port identifier of the device.
> + * @param fw_major
> + * A array pointer to store the major firmware version of a device.
> + * @param fw_minor
> + * A array pointer to store the minor firmware version of a device.
> + * @param fw_patch
> + * A array pointer to store the firmware patch number of a device.
> + * @param etrack_id
> + * A array pointer to store the nvm version of a device.
> + */
> +void rte_eth_dev_fw_info_get(uint8_t port_id, uint32_t *fw_major,
> + uint32_t *fw_minor, uint32_t *fw_patch, uint32_t *etrack_id);
I have a reserve about the naming etrack_id.
Please could you point to a document explaining this ID?
Is it known outside of Intel?
^ permalink raw reply
* Re: [PATCH v2 15/18] net/ixgbe: parse flow director filter
From: Xing, Beilei @ 2017-01-02 15:24 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Zhao1, Wei, Lu, Wenzhuo
In-Reply-To: <1483084390-53159-16-git-send-email-wei.zhao1@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH v2 15/18] net/ixgbe: parse flow director filter
>
> check if the rule is a flow director rule, and get the flow director 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
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 1467
> +++++++++++++++++++++++++++++++++-----
> drivers/net/ixgbe/ixgbe_ethdev.h | 16 +
> drivers/net/ixgbe/ixgbe_fdir.c | 247 ++++---
> lib/librte_ether/rte_flow.h | 23 +
> 4 files changed, 1495 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0a5ac4f..c98aa0d 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -438,6 +438,31 @@ ixgbe_validate_l2_tn_filter(struct rte_eth_dev *dev,
> struct rte_eth_l2_tunnel_conf *rule,
> struct rte_flow_error *error);
> static int
> +ixgbe_validate_fdir_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 ixgbe_fdir_rule *rule,
> + struct rte_flow_error *error);
> +static int
> +ixgbe_parse_fdir_filter_normal(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct ixgbe_fdir_rule *rule,
> + struct rte_flow_error *error);
> +static int
> +ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct ixgbe_fdir_rule *rule,
> + struct rte_flow_error *error);
> +static int
> +ixgbe_parse_fdir_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct ixgbe_fdir_rule *rule,
> + struct rte_flow_error *error);
> +static int
> ixgbe_flow_validate(__rte_unused struct rte_eth_dev *dev,
> const struct rte_flow_attr *attr,
> const struct rte_flow_item pattern[],
> @@ -1475,6 +1500,8 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev
> *eth_dev)
> "Failed to allocate memory for fdir hash map!");
> return -ENOMEM;
> }
> + fdir_info->mask_added = FALSE;
> +
> return 0;
> }
>
> @@ -8951,117 +8978,22 @@ ixgbe_parse_syn_filter(const struct
> rte_flow_attr *attr,
> return 0;
> }
>
> -/**
> - * Parse the rule to see if it is a L2 tunnel rule.
> - * And get the L2 tunnel filter info BTW.
> - * Only support E-tag now.
> - */
> +/* Parse to get the attr and action info of flow director rule. */
> static int
> -cons_parse_l2_tn_filter(const struct rte_flow_attr *attr,
> - const struct rte_flow_item pattern[],
> - const struct rte_flow_action actions[],
> - struct rte_eth_l2_tunnel_conf *filter,
> - struct rte_flow_error *error)
Why do u remove functions added in patch 14/18? If the functions don't be needed, how about changing patch 14/18?
^ permalink raw reply
* Re: DPDK Acceleartion Enhancement - compression
From: Thomas Monjalon @ 2017-01-02 15:19 UTC (permalink / raw)
To: Ant loves honey; +Cc: dev
In-Reply-To: <1815632810.2503307.1482854884965@mail.yahoo.com>
2016-12-27 16:08, Ant loves honey:
> I am interested in making DPDK able to to use the Intel QuickAssist
> Technology for data compression. I think this will help IP payload
> compression and save bandwidth.
We could add a new driver type for compression:
drivers/compress/qat/
^ permalink raw reply
* Re: DPDK Acceleartion Enhancement - compression
From: Thomas Monjalon @ 2017-01-02 14:45 UTC (permalink / raw)
To: Ant loves honey; +Cc: dev
In-Reply-To: <1815632810.2503307.1482854884965@mail.yahoo.com>
Hi,
2016-12-27 16:08, Ant loves honey:
> Is this the correct forum to ask question about adding code for DPDK Acceleration Enhancement? Or this is strictly for code review? If so, please direct me to the correct forum.
> I have already read these 2 documents:
>
> - http://dpdk.org/doc/guides/ cryptodevs/qat.html
> - http://dpdk.org/doc/guides/ sample_app_ug/intel_ quickassist.html
>
> I am new to DPDK and would like to do some work but is hitting a road block. I am interested in making DPDK able to to use the Intel QuickAssist Technology for data compression. I think this will help IP payload compression and save bandwidth.
> Any help to get me going is greatly appreciated.
> Anthony.
If you are going to contribute some code to DPDK, dev@dpdk.org is the right address.
You should ask precise questions with related title, e.g. "quickassist for compression".
^ permalink raw reply
* Re: Running DPDK as an unprivileged user
From: Thomas Monjalon @ 2017-01-02 14:32 UTC (permalink / raw)
To: Walker, Benjamin; +Cc: dev, Stephen Hemminger
In-Reply-To: <20161229171453.57a4326a@xeon-e3>
2016-12-29 17:14, Stephen Hemminger:
> On Thu, 29 Dec 2016 20:41:21 +0000
> "Walker, Benjamin" <benjamin.walker@intel.com> wrote:
> > 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. For uio, they are physical
> > addresses and are not chosen at all. It seems that these two
> > things are in conflict and that it will be difficult, ugly,
> > and maybe impossible to resolve the simultaneous use of
> > both.
>
> Unless application is running as privileged user (ie root), UIO
> is not going to work. Therefore don't worry about mixed environment.
Yes, mixing UIO and VFIO is possible only as root.
However, what is the benefit of mixing them?
^ permalink raw reply
* Re: [RFC 00/23] Refactor eal_init to remove panic() calls
From: Thomas Monjalon @ 2017-01-02 14:22 UTC (permalink / raw)
To: Aaron Conole; +Cc: dev
In-Reply-To: <1483111580-5397-1-git-send-email-aconole@redhat.com>
Hi Aaron,
2016-12-30 10:25, Aaron Conole:
> In many cases, it's enough to simply let the application know that the
> call to initialize DPDK has failed. A complete halt can then be
> decided by the application based on error returned (and the app could
> even attempt a possible re-attempt after some corrective action by the
> user or application).
>
> There is still some work left in this series.
Thanks for starting the work.
I think it is candidate for 17.05 and can be promoted in the roadmap:
http://dpdk.org/dev/roadmap
Have you checked wether these changes are modifying the API?
Some doxygen comments may need to be updated when a new error code
is used.
^ permalink raw reply
* Re: [PATCH v2 11/18] net/ixgbe: parse n-tuple filter
From: Xing, Beilei @ 2017-01-02 10:45 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Zhao1, Wei, Lu, Wenzhuo
In-Reply-To: <1483084390-53159-12-git-send-email-wei.zhao1@intel.com>
> +
> + filter->dst_port_mask = tcp_mask->hdr.dst_port;
> + filter->src_port_mask = tcp_mask->hdr.src_port;
> + if (tcp_mask->hdr.tcp_flags == 0xFF) {
It's better to use UINT8_MAX here.
> + filter->flags |= RTE_NTUPLE_FLAGS_TCP_FLAG;
> + } else if (!tcp_mask->hdr.tcp_flags) {
> + filter->flags &= ~RTE_NTUPLE_FLAGS_TCP_FLAG;
> + } else {
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item, "Not supported by ntuple filter");
> + return -rte_errno;
> + }
> +
> + if (attr->priority > 0xFFFF) {
How about UINT16_MAX?
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> + attr, "Error priority.");
> + return -rte_errno;
> + }
> + filter->priority = (uint16_t)attr->priority;
> +
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH v2 11/18] net/ixgbe: parse n-tuple filter
From: Xing, Beilei @ 2017-01-02 10:41 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Zhao1, Wei, Lu, Wenzhuo
In-Reply-To: <1483084390-53159-12-git-send-email-wei.zhao1@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH v2 11/18] net/ixgbe: parse n-tuple filter
>
> Add rule validate function and check if the rule is a n-tuple rule, and get the
> n-tuple 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
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 414
> ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 409 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0de1318..198cc4b 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -388,6 +388,24 @@ static int ixgbe_dev_udp_tunnel_port_del(struct
> rte_eth_dev *dev,
> struct rte_eth_udp_tunnel *udp_tunnel); static int
> ixgbe_filter_restore(struct rte_eth_dev *dev); static void
> ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
> +static int
> +cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error);
Why do you declare cons_parse_ntuple_filter here? And seems it doesn't align with the name rule.
> +static int
> +ixgbe_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error);
> +static int
> +ixgbe_flow_validate(__rte_unused 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);
> static int ixgbe_flow_flush(struct rte_eth_dev *dev,
> struct rte_flow_error *error);
> /*
> @@ -769,7 +787,7 @@ static const struct rte_ixgbe_xstats_name_off
> rte_ixgbevf_stats_strings[] = {
> #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) / \
> sizeof(rte_ixgbevf_stats_strings[0]))
> static const struct rte_flow_ops ixgbe_flow_ops = {
> - NULL,
> + ixgbe_flow_validate,
> NULL,
> NULL,
> ixgbe_flow_flush,
> @@ -8072,6 +8090,390 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev
> *dev)
> return 0;
> }
>
> +static inline uint32_t
> +rte_be_to_cpu_24(uint32_t x)
> +{
> + return ((x & 0x000000ffUL) << 16) |
> + (x & 0x0000ff00UL) |
> + ((x & 0x00ff0000UL) >> 16);
> +}
Why do you define the function in PMD with rte_ prefixed? Do you want to move it to rte library?
> +
> +
> +/**
> + * Parse the rule to see if it is a n-tuple rule.
> + * And get the n-tuple filter info BTW.
> + */
> +static int
> +cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error)
How about splitting the function into three functions? Including parse pattern/parse actions/parse attr.
> +
> +/* a specific function for ixgbe because the flags is specific */
> +static int ixgbe_parse_ntuple_filter(const struct rte_flow_attr *attr,
> + const struct rte_flow_item pattern[],
> + const struct rte_flow_action actions[],
> + struct rte_eth_ntuple_filter *filter,
> + struct rte_flow_error *error)
> +{
> + int ret;
> +
> + ret = cons_parse_ntuple_filter(attr, pattern, actions, filter, error);
> +
> + if (ret)
> + return ret;
> +
> + /* Ixgbe doesn't support tcp flags. */
> + if (filter->flags & RTE_NTUPLE_FLAGS_TCP_FLAG) {
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL, "Not supported by ntuple filter");
> + return -rte_errno;
> + }
> +
> + /* Ixgbe doesn't support many priorities. */
> + if (filter->priority < IXGBE_MIN_N_TUPLE_PRIO ||
> + filter->priority > IXGBE_MAX_N_TUPLE_PRIO) {
> + memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + NULL, "Priority not supported by ntuple filter");
> + return -rte_errno;
> + }
> +
> + if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM ||
> + filter->priority > IXGBE_5TUPLE_MAX_PRI ||
> + filter->priority < IXGBE_5TUPLE_MIN_PRI)
> + return -rte_errno;
> +
> + /* fixed value for ixgbe */
> + filter->flags = RTE_5TUPLE_FLAGS;
> + return 0;
> +}
> +
> +/**
> + * Check if the flow rule is supported by ixgbe.
> + * It only checkes the format. Don't guarantee the rule can be
Typo: checkes -> checks
> +programmed into
> + * the HW. Because there can be no enough room for the rule.
> + */
> +static int
> +ixgbe_flow_validate(__rte_unused 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_eth_ntuple_filter ntuple_filter;
> + int ret;
> +
> + memset(&ntuple_filter, 0, sizeof(struct rte_eth_ntuple_filter));
> + ret = ixgbe_parse_ntuple_filter(attr, pattern,
> + actions, &ntuple_filter, error);
> + if (!ret)
> + return 0;
> +
> + return ret;
> +}
> +
> /* Destroy all flow rules associated with a port on ixgbe. */ static int
> ixgbe_flow_flush(struct rte_eth_dev *dev, @@ -8085,15 +8487,17 @@
> ixgbe_flow_flush(struct rte_eth_dev *dev,
>
> ret = ixgbe_clear_all_fdir_filter(dev);
> if (ret < 0) {
> - rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "Failed to flush rule");
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_HANDLE,
> + NULL, "Failed to flush rule");
> return ret;
> }
>
> ret = ixgbe_clear_all_l2_tn_filter(dev);
> if (ret < 0) {
> - rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_HANDLE,
> - NULL, "Failed to flush rule");
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_HANDLE,
> + NULL, "Failed to flush rule");
> return ret;
> }
>
> --
> 2.5.5
^ permalink raw reply
* Re: [PATCH v2 09/18] net/ixgbe: store and restore L2 tunnel configuration
From: Xing, Beilei @ 2017-01-02 10:18 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Lu, Wenzhuo, Zhao1, Wei
In-Reply-To: <1483084390-53159-10-git-send-email-wei.zhao1@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2 09/18] net/ixgbe: store and restore L2 tunnel
> configuration
>
> Add support for store and restore L2 tunnel filter in SW.
The whole patch set is related to filter, so do you think it's better to move the patch from his patch set since it's related to configuration? Please help to check.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 36
> ++++++++++++++++++++++++++++++++++++
> drivers/net/ixgbe/ixgbe_ethdev.h | 3 +++
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 5c39ffa..d68de65 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -385,6 +385,7 @@ static int ixgbe_dev_udp_tunnel_port_add(struct
> rte_eth_dev *dev, static int ixgbe_dev_udp_tunnel_port_del(struct
> rte_eth_dev *dev,
> struct rte_eth_udp_tunnel *udp_tunnel); static int
> ixgbe_filter_restore(struct rte_eth_dev *dev);
> +static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
>
> /*
> * Define VF Stats MACRO for Non "cleared on read" register @@ -1444,6
> +1445,9 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
> "Failed to allocate memory for L2 TN hash map!");
> return -ENOMEM;
> }
> + l2_tn_info->e_tag_en = FALSE;
> + l2_tn_info->e_tag_fwd_en = FALSE;
> + l2_tn_info->e_tag_ether_type = DEFAULT_ETAG_ETYPE;
>
> return 0;
> }
> @@ -2502,6 +2506,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>
> /* resume enabled intr since hw reset */
> ixgbe_enable_intr(dev);
> + ixgbe_l2_tunnel_conf(dev);
> ixgbe_filter_restore(dev);
>
> return 0;
> @@ -7038,12 +7043,15 @@ ixgbe_dev_l2_tunnel_eth_type_conf(struct
> rte_eth_dev *dev, {
> int ret = 0;
> struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
>
> if (l2_tunnel == NULL)
> return -EINVAL;
>
> switch (l2_tunnel->l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> + l2_tn_info->e_tag_ether_type = l2_tunnel->ether_type;
> ret = ixgbe_update_e_tag_eth_type(hw, l2_tunnel->ether_type);
> break;
> default:
> @@ -7082,9 +7090,12 @@ ixgbe_dev_l2_tunnel_enable(struct rte_eth_dev
> *dev, {
> int ret = 0;
> struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
>
> switch (l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> + l2_tn_info->e_tag_en = TRUE;
> ret = ixgbe_e_tag_enable(hw);
> break;
> default:
> @@ -7123,9 +7134,12 @@ ixgbe_dev_l2_tunnel_disable(struct rte_eth_dev
> *dev, {
> int ret = 0;
> struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
>
> switch (l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> + l2_tn_info->e_tag_en = FALSE;
> ret = ixgbe_e_tag_disable(hw);
> break;
> default:
> @@ -7432,10 +7446,13 @@ ixgbe_dev_l2_tunnel_forwarding_enable
> (struct rte_eth_dev *dev,
> enum rte_eth_tunnel_type l2_tunnel_type) {
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> int ret = 0;
>
> switch (l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> + l2_tn_info->e_tag_fwd_en = TRUE;
> ret = ixgbe_e_tag_forwarding_en_dis(dev, 1);
> break;
> default:
> @@ -7453,10 +7470,13 @@ ixgbe_dev_l2_tunnel_forwarding_disable
> (struct rte_eth_dev *dev,
> enum rte_eth_tunnel_type l2_tunnel_type) {
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> int ret = 0;
>
> switch (l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> + l2_tn_info->e_tag_fwd_en = FALSE;
> ret = ixgbe_e_tag_forwarding_en_dis(dev, 0);
> break;
> default:
> @@ -7950,6 +7970,22 @@ ixgbe_filter_restore(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static void
> +ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev) {
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> + struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> + if (l2_tn_info->e_tag_en)
> + (void)ixgbe_e_tag_enable(hw);
> +
> + if (l2_tn_info->e_tag_fwd_en)
> + (void)ixgbe_e_tag_forwarding_en_dis(dev, 1);
> +
> + (void)ixgbe_update_e_tag_eth_type(hw, l2_tn_info->e_tag_ether_type); }
> +
> RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
> RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
> RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic |
> vfio"); diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index d6253ad..6327962 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -307,6 +307,9 @@ struct ixgbe_l2_tn_info {
> struct ixgbe_l2_tn_filter_list l2_tn_list;
> struct ixgbe_l2_tn_filter **hash_map;
> struct rte_hash *hash_handle;
> + bool e_tag_en; /* e-tag enabled */
> + bool e_tag_fwd_en; /* e-tag based forwarding enabled */
> + bool e_tag_ether_type; /* ether type for e-tag */
> };
>
> /*
> --
> 2.5.5
^ permalink raw reply
* Re: [PATCH v2 03/18] net/ixgbe: store L2 tunnel filter
From: Xing, Beilei @ 2017-01-02 10:06 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Lu, Wenzhuo, Zhao1, Wei
In-Reply-To: <1483084390-53159-4-git-send-email-wei.zhao1@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2 03/18] net/ixgbe: store L2 tunnel filter
>
> Add support for storing L2 tunnel filter in SW.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>
>
> +static inline struct ixgbe_l2_tn_filter *
> +ixgbe_l2_tn_filter_lookup(struct ixgbe_l2_tn_info *l2_tn_info,
> + struct ixgbe_l2_tn_key *key)
> +{
> + int ret = 0;
Needles initialization since ret will be set bellow, it can be removed.
And this initialization can be removed in other filter lookup/insert/delete functions, either.
> +
> + ret = rte_hash_lookup(l2_tn_info->hash_handle, (const void *)key);
> + if (ret < 0)
> + return NULL;
> +
> + return l2_tn_info->hash_map[ret];
> +}
> +
> +static inline int
> +ixgbe_insert_l2_tn_filter(struct ixgbe_l2_tn_info *l2_tn_info,
> + struct ixgbe_l2_tn_filter *l2_tn_filter) {
> + int ret = 0;
> +
> + ret = rte_hash_add_key(l2_tn_info->hash_handle,
> + &l2_tn_filter->key);
> +
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR,
> + "Failed to insert L2 tunnel filter"
> + " to hash table %d!",
> + ret);
> + return ret;
> + }
> +
> + l2_tn_info->hash_map[ret] = l2_tn_filter;
> +
> + TAILQ_INSERT_TAIL(&l2_tn_info->l2_tn_list, l2_tn_filter, entries);
> +
> + return 0;
> +}
> /* Add l2 tunnel filter */
> static int
> ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
> struct rte_eth_l2_tunnel_conf *l2_tunnel) {
> int ret = 0;
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> + struct ixgbe_l2_tn_key key;
> + struct ixgbe_l2_tn_filter *node;
> +
> + key.l2_tn_type = l2_tunnel->l2_tunnel_type;
> + key.tn_id = l2_tunnel->tunnel_id;
> +
> + node = ixgbe_l2_tn_filter_lookup(l2_tn_info, &key);
> +
> + if (node) {
> + PMD_DRV_LOG(ERR, "The L2 tunnel filter already exists!");
> + return -EINVAL;
> + }
> +
> + node = rte_zmalloc("ixgbe_l2_tn",
> + sizeof(struct ixgbe_l2_tn_filter),
> + 0);
> + if (!node)
> + return -ENOMEM;
> +
> + (void)rte_memcpy(&node->key,
> + &key,
> + sizeof(struct ixgbe_l2_tn_key));
> + node->pool = l2_tunnel->pool;
> + ret = ixgbe_insert_l2_tn_filter(l2_tn_info, node);
> + if (ret < 0) {
> + rte_free(node);
> + return ret;
> + }
>
> switch (l2_tunnel->l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> @@ -7195,6 +7340,9 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev
> *dev,
> break;
> }
>
> + if (ret < 0)
> + (void)ixgbe_remove_l2_tn_filter(l2_tn_info, &key);
Some confusion, why remove l2_tn_filter here? Can this check be moved before ixgbe_insert_l2_tn_filter?
> +
> return ret;
> }
>
> @@ -7204,6 +7352,15 @@ ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev
> *dev,
> struct rte_eth_l2_tunnel_conf *l2_tunnel) {
> int ret = 0;
> + struct ixgbe_l2_tn_info *l2_tn_info =
> + IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
> + struct ixgbe_l2_tn_key key;
> +
> + key.l2_tn_type = l2_tunnel->l2_tunnel_type;
> + key.tn_id = l2_tunnel->tunnel_id;
> + ret = ixgbe_remove_l2_tn_filter(l2_tn_info, &key);
> + if (ret < 0)
> + return ret;
>
> switch (l2_tunnel->l2_tunnel_type) {
> case RTE_L2_TUNNEL_TYPE_E_TAG:
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 8310220..6663fc9 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -132,6 +132,7 @@
> #define IXGBE_RX_VEC_START
> RTE_INTR_VEC_RXTX_OFFSET
>
> #define IXGBE_MAX_FDIR_FILTER_NUM (1024 * 32)
> +#define IXGBE_MAX_L2_TN_FILTER_NUM 128
>
> /*
> * Information about the fdir mode.
> @@ -283,6 +284,25 @@ struct ixgbe_filter_info {
> uint32_t syn_info;
> };
>
> +struct ixgbe_l2_tn_key {
> + enum rte_eth_tunnel_type l2_tn_type;
> + uint32_t tn_id;
> +};
> +
> +struct ixgbe_l2_tn_filter {
> + TAILQ_ENTRY(ixgbe_l2_tn_filter) entries;
> + struct ixgbe_l2_tn_key key;
> + uint32_t pool;
Maybe more comments here will be helpful.
> +};
> +
^ permalink raw reply
* Re: [PATCH v2 02/18] net/ixgbe: store flow director filter
From: Xing, Beilei @ 2017-01-02 9:59 UTC (permalink / raw)
To: Zhao1, Wei, dev@dpdk.org; +Cc: Lu, Wenzhuo, Zhao1, Wei
In-Reply-To: <1483084390-53159-3-git-send-email-wei.zhao1@intel.com>
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 30, 2016 3:53 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2 02/18] net/ixgbe: store flow director filter
>
> Add support for storing flow director filter in SW.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>
> v2:
> --add a fdir initialization function in device start process
> ---
>
> +static inline struct ixgbe_fdir_filter *
> +ixgbe_fdir_filter_lookup(struct ixgbe_hw_fdir_info *fdir_info,
> + union ixgbe_atr_input *key)
> +{
> + int ret = 0;
Needless initialization here since ret will be set below.
> + ret = rte_hash_lookup(fdir_info->hash_handle, (const void *)key);
> + if (ret < 0)
> + return NULL;
> +
> + return fdir_info->hash_map[ret];
> +}
> +
> +static inline int
> +ixgbe_insert_fdir_filter(struct ixgbe_hw_fdir_info *fdir_info,
> + struct ixgbe_fdir_filter *fdir_filter) {
> + int ret = 0;
Same above.
> +
> + ret = rte_hash_add_key(fdir_info->hash_handle,
> + &fdir_filter->ixgbe_fdir);
> +
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR,
> + "Failed to insert fdir filter to hash table %d!",
> + ret);
> + return ret;
> + }
> +
> + fdir_info->hash_map[ret] = fdir_filter;
> +
> + TAILQ_INSERT_TAIL(&fdir_info->fdir_list, fdir_filter, entries);
> +
> + return 0;
> +}
> +
> +static inline int
> +ixgbe_remove_fdir_filter(struct ixgbe_hw_fdir_info *fdir_info,
> + union ixgbe_atr_input *key)
> +{
> + int ret = 0;
Same above.
> + struct ixgbe_fdir_filter *fdir_filter;
> +
> + ret = rte_hash_del_key(fdir_info->hash_handle, key);
> +
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "No such fdir filter to delete %d!", ret);
> + return ret;
> + }
> +
> + fdir_filter = fdir_info->hash_map[ret];
> + fdir_info->hash_map[ret] = NULL;
> +
> + TAILQ_REMOVE(&fdir_info->fdir_list, fdir_filter, entries);
> + rte_free(fdir_filter);
> +
> + return 0;
> +}
> +
> /*
> * ixgbe_add_del_fdir_filter - add or remove a flow diretor filter.
> * @dev: pointer to the structure rte_eth_dev @@ -1098,6 +1158,8 @@
> ixgbe_add_del_fdir_filter(struct rte_eth_dev *dev,
> struct ixgbe_hw_fdir_info *info =
> IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
> enum rte_fdir_mode fdir_mode = dev->data->dev_conf.fdir_conf.mode;
> + struct ixgbe_fdir_filter *node;
> + bool add_node = FALSE;
>
> if (fdir_mode == RTE_FDIR_MODE_NONE)
> return -ENOTSUP;
> @@ -1148,6 +1210,10 @@ ixgbe_add_del_fdir_filter(struct rte_eth_dev *dev,
> dev->data->dev_conf.fdir_conf.pballoc);
>
> if (del) {
> + err = ixgbe_remove_fdir_filter(info, &input);
> + if (err < 0)
> + return err;
> +
> err = fdir_erase_filter_82599(hw, fdirhash);
> if (err < 0)
> PMD_DRV_LOG(ERR, "Fail to delete FDIR filter!"); @@ -1172,6
> +1238,37 @@ ixgbe_add_del_fdir_filter(struct rte_eth_dev *dev,
> else
> return -EINVAL;
>
> + node = ixgbe_fdir_filter_lookup(info, &input);
> + if (node) {
> + if (update) {
> + node->fdirflags = fdircmd_flags;
> + node->fdirhash = fdirhash;
> + node->queue = queue;
> + } else {
> + PMD_DRV_LOG(ERR, "Conflict with existing fdir filter!");
> + return -EINVAL;
> + }
> + } else {
> + add_node = TRUE;
> + node = rte_zmalloc("ixgbe_fdir",
> + sizeof(struct ixgbe_fdir_filter),
> + 0);
> + if (!node)
> + return -ENOMEM;
> + (void)rte_memcpy(&node->ixgbe_fdir,
> + &input,
> + sizeof(union ixgbe_atr_input));
> + node->fdirflags = fdircmd_flags;
> + node->fdirhash = fdirhash;
> + node->queue = queue;
> +
> + err = ixgbe_insert_fdir_filter(info, node);
> + if (err < 0) {
> + rte_free(node);
> + return err;
> + }
> + }
> +
> if (is_perfect) {
> err = fdir_write_perfect_filter_82599(hw, &input, queue,
> fdircmd_flags, fdirhash,
> @@ -1180,10 +1277,14 @@ ixgbe_add_del_fdir_filter(struct rte_eth_dev
> *dev,
> err = fdir_add_signature_filter_82599(hw, &input, queue,
> fdircmd_flags, fdirhash);
> }
> - if (err < 0)
> + if (err < 0) {
> PMD_DRV_LOG(ERR, "Fail to add FDIR filter!");
> - else
> +
> + if (add_node)
> + (void)ixgbe_remove_fdir_filter(info, &input);
> + } else {
> PMD_DRV_LOG(DEBUG, "Success to add FDIR filter");
> + }
>
> return err;
> }
> --
> 2.5.5
^ permalink raw reply
* Re: [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD capabilities
From: Azarewicz, PiotrX T @ 2017-01-02 9:08 UTC (permalink / raw)
To: Azarewicz, PiotrX T, Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, De Lara Guarch, Pablo, Griffin, John,
Jain, Deepak K, Doherty, Declan, Kusztal, ArkadiuszX
In-Reply-To: <4837007523CC9A4B9414D20C13DE6E6413757B3D@IRSMSX102.ger.corp.intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD
> capabilities
>
> > Subject: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in
> > PMD capabilities
> >
> > This patch sets iv size in aesni gcm PMD to 12 bytes to be conformant
> > with nist SP800-38D.
> >
> > Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto
> > operations")
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > index e824d4b..c51f82a 100644
> > --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > @@ -77,8 +77,8 @@ static const struct rte_cryptodev_capabilities
> > aesni_gcm_pmd_capabilities[] = {
> > .increment = 0
> > },
> > .iv_size = {
> > - .min = 16,
> > - .max = 16,
> > + .min = 12,
> > + .max = 12,
> > .increment = 0
> > }
> > }, }
>
> I think that we should also remove 16 na 0 bytes allowed in
> process_gcm_crypto_op() function:
> if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
> op->cipher.iv.length != 0) {
> GCM_LOG_ERR("iv");
> return -1;
> }
I found this notice about IV in rte_crypto_sym.h :
* - For GCM mode, this is either the IV (if the length
* is 96 bits) or J0 (for other sizes), where J0 is as
* defined by NIST SP800-38D. Regardless of the IV
* length, a full 16 bytes needs to be allocated.
So it is fine to leave unchanged above code.
Acked-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
^ permalink raw reply
* Re: [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
From: Azarewicz, PiotrX T @ 2017-01-02 8:50 UTC (permalink / raw)
To: Azarewicz, PiotrX T, Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, De Lara Guarch, Pablo, Griffin, John,
Jain, Deepak K, Doherty, Declan, Kusztal, ArkadiuszX
In-Reply-To: <4837007523CC9A4B9414D20C13DE6E6413756B1A@IRSMSX102.ger.corp.intel.com>
Hi Arek,
> > Subject: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding
> > bytes for GCM
> >
> > This commit fixes pre-counter block (J0) padding by clearing four most
> > significant bytes before setting initial counter value.
> >
> > Fixes: b2bb3597470c ("crypto/aesni_gcm: move pre-counter block to
> > driver")
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > index dba5e15..af3d60f 100644
> > --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > @@ -40,6 +40,7 @@
> > #include <rte_vdev.h>
> > #include <rte_malloc.h>
> > #include <rte_cpuflags.h>
> > +#include <rte_byteorder.h>
> >
> > #include "aesni_gcm_pmd_private.h"
> >
> > @@ -241,7 +242,8 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> > struct rte_crypto_sym_op *op,
> > * to set BE LSB to 1, driver expects that 16B is allocated
>
> I think that 16B expected by driver while only 12B IV is supported is not clear
> from user perspective.
> I think that we should expect 12B only and allocate 16B locally.
I didn't notice that this exception is also described in rte_crypto_sym.h, so this is fine.
>
> > */
> > if (op->cipher.iv.length == 12) {
> > - op->cipher.iv.data[15] = 1;
> > + uint32_t *iv_padd = (uint32_t *)&op->cipher.iv.data[12];
> > + *iv_padd = rte_bswap32(1);
>
> Should not be that the last byte (number 15) always be set to 1?
I didn't notice that this code will always run in little-endian machine, so this is fine too.
>
> > }
> >
> > if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
> > --
> > 2.1.0
Acked-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
^ permalink raw reply
* [PATCH v3 2/2] net/vhost: emulate device start/stop behavior
From: Charles (Chas) Williams @ 2017-01-01 19:01 UTC (permalink / raw)
To: dev; +Cc: mtetsuyah, yuanhan.liu, Charles (Chas) Williams
In-Reply-To: <1483297317-20315-1-git-send-email-ciwillia@brocade.com>
.dev_start()/.dev_stop() roughly corresponds to the local device's
port being up or down. This is different from the remote client being
connected which is roughtly link up or down. Emulate the behavior by
separately tracking the local start/stop state to determine if we should
allow packets to be queued to the remote client.
Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
drivers/net/vhost/rte_eth_vhost.c | 65 ++++++++++++++++++++++++++++++++-------
1 file changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 6b11e40..d5a4540 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -100,7 +100,8 @@ struct vhost_stats {
struct vhost_queue {
int vid;
- rte_atomic32_t allow_queuing;
+ rte_atomic32_t connected;
+ rte_atomic32_t ready;
rte_atomic32_t while_queuing;
struct pmd_internal *internal;
struct rte_mempool *mb_pool;
@@ -383,18 +384,25 @@ vhost_update_packet_xstats(struct vhost_queue *vq,
}
}
+static inline bool
+queuing_stopped(struct vhost_queue *r)
+{
+ return unlikely(rte_atomic32_read(&r->connected) == 0 ||
+ rte_atomic32_read(&r->ready) == 0);
+}
+
static uint16_t
eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
{
struct vhost_queue *r = q;
uint16_t i, nb_rx = 0;
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
return 0;
rte_atomic32_set(&r->while_queuing, 1);
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
goto out;
/* Dequeue packets from guest TX queue */
@@ -422,12 +430,12 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
struct vhost_queue *r = q;
uint16_t i, nb_tx = 0;
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
return 0;
rte_atomic32_set(&r->while_queuing, 1);
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
goto out;
/* Enqueue packets to guest RX queue */
@@ -546,13 +554,13 @@ new_device(int vid)
vq = eth_dev->data->rx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 1);
+ rte_atomic32_set(&vq->connected, 1);
}
for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
vq = eth_dev->data->tx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 1);
+ rte_atomic32_set(&vq->connected, 1);
}
RTE_LOG(INFO, PMD, "New connection established\n");
@@ -585,7 +593,7 @@ destroy_device(int vid)
vq = eth_dev->data->rx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 0);
+ rte_atomic32_set(&vq->connected, 0);
while (rte_atomic32_read(&vq->while_queuing))
rte_pause();
}
@@ -593,7 +601,7 @@ destroy_device(int vid)
vq = eth_dev->data->tx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 0);
+ rte_atomic32_set(&vq->connected, 0);
while (rte_atomic32_read(&vq->while_queuing))
rte_pause();
}
@@ -770,14 +778,49 @@ vhost_driver_session_stop(void)
}
static int
-eth_dev_start(struct rte_eth_dev *dev __rte_unused)
+eth_dev_start(struct rte_eth_dev *dev)
{
+ struct vhost_queue *vq;
+ unsigned int i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ vq = dev->data->rx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 1);
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ vq = dev->data->tx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 1);
+ }
+
return 0;
}
static void
-eth_dev_stop(struct rte_eth_dev *dev __rte_unused)
+eth_dev_stop(struct rte_eth_dev *dev)
{
+ struct vhost_queue *vq;
+ unsigned int i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ vq = dev->data->rx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 0);
+ while (rte_atomic32_read(&vq->while_queuing))
+ rte_pause();
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ vq = dev->data->tx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 0);
+ while (rte_atomic32_read(&vq->while_queuing))
+ rte_pause();
+ }
}
static int
--
2.1.4
^ permalink raw reply related
* [PATCH v3 1/2] net/vhost: create datagram sockets immediately
From: Charles (Chas) Williams @ 2017-01-01 19:01 UTC (permalink / raw)
To: dev; +Cc: mtetsuyah, yuanhan.liu, Charles (Chas) Williams
If you create a vhost server device, it doesn't create the actual datagram
socket until you call .dev_start(). If you call .dev_stop() is also
deletes those sockets. For QEMU clients, this is a problem since QEMU
doesn't know how to re-attach to datagram sockets that have gone away.
To work around this, register and unregister the datagram sockets during
device creation and removal.
Fixes: ee584e9710b9 ("vhost: add driver on top of the library")
Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
drivers/net/vhost/rte_eth_vhost.c | 43 ++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 60b0f51..6b11e40 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -114,8 +114,6 @@ struct pmd_internal {
char *iface_name;
uint16_t max_queues;
uint64_t flags;
-
- volatile uint16_t once;
};
struct internal_list {
@@ -772,35 +770,14 @@ vhost_driver_session_stop(void)
}
static int
-eth_dev_start(struct rte_eth_dev *dev)
+eth_dev_start(struct rte_eth_dev *dev __rte_unused)
{
- struct pmd_internal *internal = dev->data->dev_private;
- int ret = 0;
-
- if (rte_atomic16_cmpset(&internal->once, 0, 1)) {
- ret = rte_vhost_driver_register(internal->iface_name,
- internal->flags);
- if (ret)
- return ret;
- }
-
- /* We need only one message handling thread */
- if (rte_atomic16_add_return(&nb_started_ports, 1) == 1)
- ret = vhost_driver_session_start();
-
- return ret;
+ return 0;
}
static void
-eth_dev_stop(struct rte_eth_dev *dev)
+eth_dev_stop(struct rte_eth_dev *dev __rte_unused)
{
- struct pmd_internal *internal = dev->data->dev_private;
-
- if (rte_atomic16_cmpset(&internal->once, 1, 0))
- rte_vhost_driver_unregister(internal->iface_name);
-
- if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
- vhost_driver_session_stop();
}
static int
@@ -1078,6 +1055,15 @@ eth_dev_vhost_create(const char *name, char *iface_name, int16_t queues,
eth_dev->rx_pkt_burst = eth_vhost_rx;
eth_dev->tx_pkt_burst = eth_vhost_tx;
+ if (rte_vhost_driver_register(iface_name, flags))
+ goto error;
+
+ /* We need only one message handling thread */
+ if (rte_atomic16_add_return(&nb_started_ports, 1) == 1) {
+ if (vhost_driver_session_start())
+ goto error;
+ }
+
return data->port_id;
error:
@@ -1215,6 +1201,11 @@ rte_pmd_vhost_remove(const char *name)
eth_dev_stop(eth_dev);
+ rte_vhost_driver_unregister(internal->iface_name);
+
+ if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
+ vhost_driver_session_stop();
+
rte_free(vring_states[eth_dev->data->port_id]);
vring_states[eth_dev->data->port_id] = NULL;
--
2.1.4
^ permalink raw reply related
* [PATCH v2 2/2] net/vhost: emulate device start/stop behavior
From: Charles (Chas) Williams @ 2017-01-01 18:53 UTC (permalink / raw)
To: dev; +Cc: mtetsuyah, yuanhan.liu, Charles (Chas) Williams
In-Reply-To: <1483296828-18544-1-git-send-email-ciwillia@brocade.com>
.dev_start()/.dev_stop() roughly corresponds to the local device's
port being up or down. This is different from the remote client being
connected which is roughtly link up or down. Emulate the behavior by
separately tracking the local start/stop state to determine if we should
allow packets to be queued to the remote client.
Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
drivers/net/vhost/rte_eth_vhost.c | 65 ++++++++++++++++++++++++++++++++-------
1 file changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 6b11e40..c9e45eb 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -100,7 +100,8 @@ struct vhost_stats {
struct vhost_queue {
int vid;
- rte_atomic32_t allow_queuing;
+ rte_atomic32_t connected;
+ rte_atomic32_t ready;
rte_atomic32_t while_queuing;
struct pmd_internal *internal;
struct rte_mempool *mb_pool;
@@ -383,18 +384,25 @@ vhost_update_packet_xstats(struct vhost_queue *vq,
}
}
+static inline bool
+queuing_stopped(struct vhost_queue *r)
+{
+ return unlikely(rte_atomic32_read(&r->connected) == 0 ||
+ rte_atomic32_read(&r->ready) == 0);
+}
+
static uint16_t
eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
{
struct vhost_queue *r = q;
uint16_t i, nb_rx = 0;
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
return 0;
rte_atomic32_set(&r->while_queuing, 1);
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
goto out;
/* Dequeue packets from guest TX queue */
@@ -422,12 +430,12 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
struct vhost_queue *r = q;
uint16_t i, nb_tx = 0;
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
return 0;
rte_atomic32_set(&r->while_queuing, 1);
- if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0))
+ if (queuing_stopped(r))
goto out;
/* Enqueue packets to guest RX queue */
@@ -546,13 +554,13 @@ new_device(int vid)
vq = eth_dev->data->rx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 1);
+ rte_atomic32_set(&vq->connected, 1);
}
for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
vq = eth_dev->data->tx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 1);
+ rte_atomic32_set(&vq->connected, 1);
}
RTE_LOG(INFO, PMD, "New connection established\n");
@@ -585,7 +593,7 @@ destroy_device(int vid)
vq = eth_dev->data->rx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 0);
+ rte_atomic32_set(&vq->connected, 0);
while (rte_atomic32_read(&vq->while_queuing))
rte_pause();
}
@@ -593,7 +601,7 @@ destroy_device(int vid)
vq = eth_dev->data->tx_queues[i];
if (vq == NULL)
continue;
- rte_atomic32_set(&vq->allow_queuing, 0);
+ rte_atomic32_set(&vq->connected, 0);
while (rte_atomic32_read(&vq->while_queuing))
rte_pause();
}
@@ -770,14 +778,49 @@ vhost_driver_session_stop(void)
}
static int
-eth_dev_start(struct rte_eth_dev *dev __rte_unused)
+eth_dev_start(struct rte_eth_dev *dev)
{
+ struct vhost_queue *vq;
+ unsigned i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ vq = dev->data->rx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 1);
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ vq = dev->data->tx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 1);
+ }
+
return 0;
}
static void
-eth_dev_stop(struct rte_eth_dev *dev __rte_unused)
+eth_dev_stop(struct rte_eth_dev *dev)
{
+ struct vhost_queue *vq;
+ unsigned i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ vq = dev->data->rx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 0);
+ while (rte_atomic32_read(&vq->while_queuing))
+ rte_pause();
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ vq = dev->data->tx_queues[i];
+ if (vq == NULL)
+ continue;
+ rte_atomic32_set(&vq->ready, 0);
+ while (rte_atomic32_read(&vq->while_queuing))
+ rte_pause();
+ }
}
static int
--
2.1.4
^ permalink raw reply related
* [PATCH v2 1/2] net/vhost: create datagram sockets immediately
From: Charles (Chas) Williams @ 2017-01-01 18:53 UTC (permalink / raw)
To: dev; +Cc: mtetsuyah, yuanhan.liu, Charles (Chas) Williams
If you create a vhost server device, it doesn't create the actual datagram
socket until you call .dev_start(). If you call .dev_stop() is also
deletes those sockets. For QEMU clients, this is a problem since QEMU
doesn't know how to re-attach to datagram sockets that have gone away.
To work around this, register and unregister the datagram sockets during
device creation and removal.
Fixes: ee584e9710b9 ("vhost: add driver on top of the library")
Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
drivers/net/vhost/rte_eth_vhost.c | 43 ++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 60b0f51..6b11e40 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -114,8 +114,6 @@ struct pmd_internal {
char *iface_name;
uint16_t max_queues;
uint64_t flags;
-
- volatile uint16_t once;
};
struct internal_list {
@@ -772,35 +770,14 @@ vhost_driver_session_stop(void)
}
static int
-eth_dev_start(struct rte_eth_dev *dev)
+eth_dev_start(struct rte_eth_dev *dev __rte_unused)
{
- struct pmd_internal *internal = dev->data->dev_private;
- int ret = 0;
-
- if (rte_atomic16_cmpset(&internal->once, 0, 1)) {
- ret = rte_vhost_driver_register(internal->iface_name,
- internal->flags);
- if (ret)
- return ret;
- }
-
- /* We need only one message handling thread */
- if (rte_atomic16_add_return(&nb_started_ports, 1) == 1)
- ret = vhost_driver_session_start();
-
- return ret;
+ return 0;
}
static void
-eth_dev_stop(struct rte_eth_dev *dev)
+eth_dev_stop(struct rte_eth_dev *dev __rte_unused)
{
- struct pmd_internal *internal = dev->data->dev_private;
-
- if (rte_atomic16_cmpset(&internal->once, 1, 0))
- rte_vhost_driver_unregister(internal->iface_name);
-
- if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
- vhost_driver_session_stop();
}
static int
@@ -1078,6 +1055,15 @@ eth_dev_vhost_create(const char *name, char *iface_name, int16_t queues,
eth_dev->rx_pkt_burst = eth_vhost_rx;
eth_dev->tx_pkt_burst = eth_vhost_tx;
+ if (rte_vhost_driver_register(iface_name, flags))
+ goto error;
+
+ /* We need only one message handling thread */
+ if (rte_atomic16_add_return(&nb_started_ports, 1) == 1) {
+ if (vhost_driver_session_start())
+ goto error;
+ }
+
return data->port_id;
error:
@@ -1215,6 +1201,11 @@ rte_pmd_vhost_remove(const char *name)
eth_dev_stop(eth_dev);
+ rte_vhost_driver_unregister(internal->iface_name);
+
+ if (rte_atomic16_sub_return(&nb_started_ports, 1) == 0)
+ vhost_driver_session_stop();
+
rte_free(vring_states[eth_dev->data->port_id]);
vring_states[eth_dev->data->port_id] = NULL;
--
2.1.4
^ permalink raw reply related
* [PATCH 4/5] net/qede: fix per queue stats/xstats
From: Rasesh Mody @ 2016-12-31 8:16 UTC (permalink / raw)
To: dev; +Cc: Rasesh Mody, stable, Dept-EngDPDKDev
In-Reply-To: <1483172217-30186-1-git-send-email-rasesh.mody@cavium.com>
From: Rasesh Mody <Rasesh.Mody@cavium.com>
If value of number of rxq/txq is diffrent than
RTE_ETHDEV_QUEUE_STAT_CNTRS, limit per queue
stats/xstats to minimum of the two.
Fixes: 7634c5f91569 ("net/qede: add queue statistics")
Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
drivers/net/qede/qede_ethdev.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index b7606c8..de8286c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1160,6 +1160,7 @@ qede_get_stats(struct rte_eth_dev *eth_dev, struct rte_eth_stats *eth_stats)
struct ecore_dev *edev = &qdev->edev;
struct ecore_eth_stats stats;
unsigned int i = 0, j = 0, qid;
+ unsigned int rxq_stat_cntrs, txq_stat_cntrs;
struct qede_tx_queue *txq;
qdev->ops->get_vport_stats(edev, &stats);
@@ -1193,6 +1194,17 @@ qede_get_stats(struct rte_eth_dev *eth_dev, struct rte_eth_stats *eth_stats)
eth_stats->oerrors = stats.tx_err_drop_pkts;
/* Queue stats */
+ rxq_stat_cntrs = RTE_MIN(QEDE_RSS_COUNT(qdev),
+ RTE_ETHDEV_QUEUE_STAT_CNTRS);
+ txq_stat_cntrs = RTE_MIN(QEDE_TSS_COUNT(qdev),
+ RTE_ETHDEV_QUEUE_STAT_CNTRS);
+ if ((rxq_stat_cntrs != QEDE_RSS_COUNT(qdev)) ||
+ (txq_stat_cntrs != QEDE_TSS_COUNT(qdev)))
+ DP_VERBOSE(edev, ECORE_MSG_DEBUG,
+ "Not all the queue stats will be displayed. Set"
+ " RTE_ETHDEV_QUEUE_STAT_CNTRS config param"
+ " appropriately and retry.\n");
+
for (qid = 0; qid < QEDE_QUEUE_CNT(qdev); qid++) {
if (qdev->fp_array[qid].type & QEDE_FASTPATH_RX) {
eth_stats->q_ipackets[i] =
@@ -1211,7 +1223,11 @@ qede_get_stats(struct rte_eth_dev *eth_dev, struct rte_eth_stats *eth_stats)
rx_alloc_errors));
i++;
}
+ if (i == rxq_stat_cntrs)
+ break;
+ }
+ for (qid = 0; qid < QEDE_QUEUE_CNT(qdev); qid++) {
if (qdev->fp_array[qid].type & QEDE_FASTPATH_TX) {
txq = qdev->fp_array[(qid)].txqs[0];
eth_stats->q_opackets[j] =
@@ -1221,13 +1237,17 @@ qede_get_stats(struct rte_eth_dev *eth_dev, struct rte_eth_stats *eth_stats)
xmit_pkts)));
j++;
}
+ if (j == txq_stat_cntrs)
+ break;
}
}
static unsigned
qede_get_xstats_count(struct qede_dev *qdev) {
return RTE_DIM(qede_xstats_strings) +
- (RTE_DIM(qede_rxq_xstats_strings) * QEDE_RSS_COUNT(qdev));
+ (RTE_DIM(qede_rxq_xstats_strings) *
+ RTE_MIN(QEDE_RSS_COUNT(qdev),
+ RTE_ETHDEV_QUEUE_STAT_CNTRS));
}
static int
@@ -1237,6 +1257,7 @@ qede_get_xstats_names(__rte_unused struct rte_eth_dev *dev,
struct qede_dev *qdev = dev->data->dev_private;
const unsigned int stat_cnt = qede_get_xstats_count(qdev);
unsigned int i, qid, stat_idx = 0;
+ unsigned int rxq_stat_cntrs;
if (xstats_names != NULL) {
for (i = 0; i < RTE_DIM(qede_xstats_strings); i++) {
@@ -1247,7 +1268,9 @@ qede_get_xstats_names(__rte_unused struct rte_eth_dev *dev,
stat_idx++;
}
- for (qid = 0; qid < QEDE_RSS_COUNT(qdev); qid++) {
+ rxq_stat_cntrs = RTE_MIN(QEDE_RSS_COUNT(qdev),
+ RTE_ETHDEV_QUEUE_STAT_CNTRS);
+ for (qid = 0; qid < rxq_stat_cntrs; qid++) {
for (i = 0; i < RTE_DIM(qede_rxq_xstats_strings); i++) {
snprintf(xstats_names[stat_idx].name,
sizeof(xstats_names[stat_idx].name),
@@ -1271,6 +1294,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
struct ecore_eth_stats stats;
const unsigned int num = qede_get_xstats_count(qdev);
unsigned int i, qid, stat_idx = 0;
+ unsigned int rxq_stat_cntrs;
if (n < num)
return num;
@@ -1283,7 +1307,9 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
stat_idx++;
}
- for (qid = 0; qid < QEDE_QUEUE_CNT(qdev); qid++) {
+ rxq_stat_cntrs = RTE_MIN(QEDE_RSS_COUNT(qdev),
+ RTE_ETHDEV_QUEUE_STAT_CNTRS);
+ for (qid = 0; qid < rxq_stat_cntrs; qid++) {
if (qdev->fp_array[qid].type & QEDE_FASTPATH_RX) {
for (i = 0; i < RTE_DIM(qede_rxq_xstats_strings); i++) {
xstats[stat_idx].value = *(uint64_t *)(
--
1.7.10.3
^ permalink raw reply related
* [PATCH 3/5] net/qede: fix PF fastpath status block index
From: Rasesh Mody @ 2016-12-31 8:16 UTC (permalink / raw)
To: dev; +Cc: Harish Patil, stable, Dept-EngDPDKDev
In-Reply-To: <1483172217-30186-1-git-send-email-rasesh.mody@cavium.com>
From: Harish Patil <harish.patil@qlogic.com>
Allocate double the number of fastpath status block index
since the PF RX/TX queues are not sharing the status block.
This is an interim solution till other parts of the code
is modified to handle the same.
Fixes: f1e4b6c0acee ("net/qede: fix status block index for VF queues")
Signed-off-by: Harish Patil <harish.patil@qlogic.com>
---
drivers/net/qede/qede_rxtx.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index aebe8cb..f20881c 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -431,13 +431,15 @@ int qede_alloc_fp_resc(struct qede_dev *qdev)
struct ecore_dev *edev = &qdev->edev;
struct qede_fastpath *fp;
uint32_t num_sbs;
- int rc, i;
+ uint16_t i;
+ uint16_t sb_idx;
+ int rc;
if (IS_VF(edev))
ecore_vf_get_num_sbs(ECORE_LEADING_HWFN(edev), &num_sbs);
else
- num_sbs = (ecore_cxt_get_proto_cid_count
- (ECORE_LEADING_HWFN(edev), PROTOCOLID_ETH, NULL)) / 2;
+ num_sbs = ecore_cxt_get_proto_cid_count
+ (ECORE_LEADING_HWFN(edev), PROTOCOLID_ETH, NULL);
if (num_sbs == 0) {
DP_ERR(edev, "No status blocks available\n");
@@ -455,7 +457,11 @@ int qede_alloc_fp_resc(struct qede_dev *qdev)
for (i = 0; i < QEDE_QUEUE_CNT(qdev); i++) {
fp = &qdev->fp_array[i];
- if (qede_alloc_mem_sb(qdev, fp->sb_info, i % num_sbs)) {
+ if (IS_VF(edev))
+ sb_idx = i % num_sbs;
+ else
+ sb_idx = i;
+ if (qede_alloc_mem_sb(qdev, fp->sb_info, sb_idx)) {
qede_free_fp_arrays(qdev);
return -ENOMEM;
}
--
1.7.10.3
^ permalink raw reply related
* [PATCH 2/5] net/qede: fix minimum buffer size and scatter Rx check
From: Rasesh Mody @ 2016-12-31 8:16 UTC (permalink / raw)
To: dev; +Cc: Harish Patil, stable, Dept-EngDPDKDev
In-Reply-To: <1483172217-30186-1-git-send-email-rasesh.mody@cavium.com>
From: Harish Patil <harish.patil@qlogic.com>
- Fix minimum RX buffer size to 1024B
- Force enable scatter/gather mode if given RX buf size is lesser than MTU
- Adjust RX buffer size to cache-line size with overhead included
Fixes: bec0228816c0 ("net/qede: support scatter gather")
Fixes: 2ea6f76aff40 ("qede: add core driver")
Signed-off-by: Harish Patil <harish.patil@qlogic.com>
---
drivers/net/qede/qede_ethdev.c | 3 +--
drivers/net/qede/qede_rxtx.c | 47 +++++++++++++++++-----------------------
drivers/net/qede/qede_rxtx.h | 11 ++++++++--
3 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index c8581d8..b7606c8 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -968,8 +968,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
PMD_INIT_FUNC_TRACE(edev);
- dev_info->min_rx_bufsize = (uint32_t)(ETHER_MIN_MTU +
- QEDE_ETH_OVERHEAD);
+ dev_info->min_rx_bufsize = (uint32_t)QEDE_MIN_RX_BUFF_SIZE;
dev_info->max_rx_pktlen = (uint32_t)ETH_TX_MAX_NON_LSO_PKT_LEN;
dev_info->rx_desc_lim = qede_rx_desc_lim;
dev_info->tx_desc_lim = qede_tx_desc_lim;
diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index ecff5bc..aebe8cb 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -89,11 +89,11 @@ qede_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
{
struct qede_dev *qdev = dev->data->dev_private;
struct ecore_dev *edev = &qdev->edev;
- struct rte_eth_dev_data *eth_data = dev->data;
+ struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
struct qede_rx_queue *rxq;
- uint16_t pkt_len = (uint16_t)dev->data->dev_conf.rxmode.max_rx_pkt_len;
+ uint16_t max_rx_pkt_len;
+ uint16_t bufsz;
size_t size;
- uint16_t data_size;
int rc;
int i;
@@ -127,34 +127,27 @@ qede_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
rxq->nb_rx_desc = nb_desc;
rxq->queue_id = queue_idx;
rxq->port_id = dev->data->port_id;
-
- /* Sanity check */
- data_size = (uint16_t)rte_pktmbuf_data_room_size(mp) -
- RTE_PKTMBUF_HEADROOM;
-
- if (pkt_len > data_size && !dev->data->scattered_rx) {
- DP_ERR(edev, "MTU %u should not exceed dataroom %u\n",
- pkt_len, data_size);
- rte_free(rxq);
- return -EINVAL;
+ max_rx_pkt_len = (uint16_t)rxmode->max_rx_pkt_len;
+ qdev->mtu = max_rx_pkt_len;
+
+ /* Fix up RX buffer size */
+ bufsz = (uint16_t)rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM;
+ if ((rxmode->enable_scatter) ||
+ (max_rx_pkt_len + QEDE_ETH_OVERHEAD) > bufsz) {
+ if (!dev->data->scattered_rx) {
+ DP_INFO(edev, "Forcing scatter-gather mode\n");
+ dev->data->scattered_rx = 1;
+ }
}
-
if (dev->data->scattered_rx)
- rxq->rx_buf_size = data_size;
+ rxq->rx_buf_size = bufsz + QEDE_ETH_OVERHEAD;
else
- rxq->rx_buf_size = pkt_len + QEDE_ETH_OVERHEAD;
-
- qdev->mtu = pkt_len;
+ rxq->rx_buf_size = qdev->mtu + QEDE_ETH_OVERHEAD;
+ /* Align to cache-line size if needed */
+ rxq->rx_buf_size = QEDE_CEIL_TO_CACHE_LINE_SIZE(rxq->rx_buf_size);
- DP_INFO(edev, "MTU = %u ; RX buffer = %u\n",
- qdev->mtu, rxq->rx_buf_size);
-
- if (pkt_len > ETHER_MAX_LEN) {
- dev->data->dev_conf.rxmode.jumbo_frame = 1;
- DP_NOTICE(edev, false, "jumbo frame enabled\n");
- } else {
- dev->data->dev_conf.rxmode.jumbo_frame = 0;
- }
+ DP_INFO(edev, "mtu %u mbufsz %u bd_max_bytes %u scatter_mode %d\n",
+ qdev->mtu, bufsz, rxq->rx_buf_size, dev->data->scattered_rx);
/* Allocate the parallel driver ring for Rx buffers */
size = sizeof(*rxq->sw_rx_ring) * rxq->nb_rx_desc;
diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h
index a95b4ab..9a393e9 100644
--- a/drivers/net/qede/qede_rxtx.h
+++ b/drivers/net/qede/qede_rxtx.h
@@ -51,14 +51,21 @@
((flags) & (PARSING_AND_ERR_FLAGS_TUNNEL8021QTAGEXIST_MASK \
<< PARSING_AND_ERR_FLAGS_TUNNEL8021QTAGEXIST_SHIFT))
+#define QEDE_MIN_RX_BUFF_SIZE (1024)
+#define QEDE_VLAN_TAG_SIZE (4)
+#define QEDE_LLC_SNAP_HDR_LEN (8)
+
/* Max supported alignment is 256 (8 shift)
* minimal alignment shift 6 is optimal for 57xxx HW performance
*/
#define QEDE_L1_CACHE_SHIFT 6
#define QEDE_RX_ALIGN_SHIFT (RTE_MAX(6, RTE_MIN(8, QEDE_L1_CACHE_SHIFT)))
#define QEDE_FW_RX_ALIGN_END (1UL << QEDE_RX_ALIGN_SHIFT)
-
-#define QEDE_ETH_OVERHEAD (ETHER_HDR_LEN + 8 + 8 + QEDE_FW_RX_ALIGN_END)
+#define QEDE_CEIL_TO_CACHE_LINE_SIZE(n) (((n) + (QEDE_FW_RX_ALIGN_END - 1)) & \
+ ~(QEDE_FW_RX_ALIGN_END - 1))
+/* Note: QEDE_LLC_SNAP_HDR_LEN is optional */
+#define QEDE_ETH_OVERHEAD ((ETHER_HDR_LEN) + ((2 * QEDE_VLAN_TAG_SIZE)) \
+ + (QEDE_LLC_SNAP_HDR_LEN))
#define QEDE_RSS_OFFLOAD_ALL (ETH_RSS_IPV4 |\
ETH_RSS_NONFRAG_IPV4_TCP |\
--
1.7.10.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox