DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 29/29] net/i40e: set/clear VF stats from PF
From: Qi Zhang @ 2016-12-15 21:05 UTC (permalink / raw)
  To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In-Reply-To: <1481835919-36488-1-git-send-email-qi.z.zhang@intel.com>

This patch add support to get/clear VF statistics
from PF side.
Two APIs are added:
rte_pmd_i40e_get_vf_stats.
rte_pmd_i40e_reset_vf_stats.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c            | 76 +++++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           | 41 +++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |  2 +
 3 files changed, 119 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5ee010e..080eff2 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10462,3 +10462,79 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
 
 	return ret;
 }
+
+int
+rte_pmd_i40e_get_vf_stats(uint8_t port,
+			  uint16_t vf_id,
+			  struct rte_eth_stats *stats)
+{
+	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info;
+	struct i40e_pf *pf;
+	struct i40e_vsi *vsi;
+	int ret = 0;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	rte_eth_dev_info_get(port, &dev_info);
+
+	if (vf_id >= dev_info.max_vfs)
+		return -EINVAL;
+
+	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+		PMD_DRV_LOG(ERR, "Invalid argument.");
+		return -EINVAL;
+	}
+
+	vsi = pf->vfs[vf_id].vsi;
+	i40e_update_vsi_stats(vsi);
+
+	stats->ipackets = vsi->eth_stats.rx_unicast +
+			vsi->eth_stats.rx_multicast +
+			vsi->eth_stats.rx_broadcast;
+	stats->opackets = vsi->eth_stats.tx_unicast +
+			vsi->eth_stats.tx_multicast +
+			vsi->eth_stats.tx_broadcast;
+	stats->ibytes   = vsi->eth_stats.rx_bytes;
+	stats->obytes   = vsi->eth_stats.tx_bytes;
+	stats->ierrors  = vsi->eth_stats.rx_discards;
+	stats->oerrors  = vsi->eth_stats.tx_errors + vsi->eth_stats.tx_discards;
+
+	return ret;
+}
+
+int
+rte_pmd_i40e_reset_vf_stats(uint8_t port,
+			    uint16_t vf_id)
+{
+	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info;
+	struct i40e_pf *pf;
+	struct i40e_vsi *vsi;
+	int ret = 0;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	rte_eth_dev_info_get(port, &dev_info);
+
+	if (vf_id >= dev_info.max_vfs)
+		return -EINVAL;
+
+	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+		PMD_DRV_LOG(ERR, "Invalid argument.");
+		return -EINVAL;
+	}
+
+	vsi = pf->vfs[vf_id].vsi;
+
+	vsi->offset_loaded = false;
+	i40e_update_vsi_stats(vsi);
+
+	return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 96cda89..2bda36d 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -284,4 +284,45 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t port, uint16_t vf_id, uint8_t on);
 int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
 				    uint64_t vf_mask, uint8_t on);
 
+/**
+ * Get VF's statistics
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param vf_id
+ *    VF on which to get.
+ * @param stats
+ *    A pointer to a structure of type *rte_eth_stats* to be filled with
+ *    the values of device counters for the following set of statistics:
+ *   - *ipackets* with the total of successfully received packets.
+ *   - *opackets* with the total of successfully transmitted packets.
+ *   - *ibytes*   with the total of successfully received bytes.
+ *   - *obytes*   with the total of successfully transmitted bytes.
+ *   - *ierrors*  with the total of erroneous received packets.
+ *   - *oerrors*  with the total of failed transmitted packets.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+
+int rte_pmd_i40e_get_vf_stats(uint8_t port,
+			      uint16_t vf_id,
+			      struct rte_eth_stats *stats);
+
+/**
+ * Clear VF's statistics
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param vf_id
+ *    VF on which to get.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_pmd_i40e_reset_vf_stats(uint8_t port,
+				uint16_t vf_id);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 818ff9c..e4dff0b 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -18,5 +18,7 @@ DPDK_17.02 {
 	rte_pmd_i40e_set_vf_broadcast;
 	rte_pmd_i40e_set_vf_vlan_tag;
 	rte_pmd_i40e_set_vf_vlan_filter;
+	rte_pmd_i40e_get_vf_stats;
+	rte_pmd_i40e_reset_vf_stats;
 
 } DPDK_2.0;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 23/28] net/ixgbe: use eal I/O device memory read/write API
From: Santosh Shukla @ 2016-12-16  4:40 UTC (permalink / raw)
  To: Jianbo Liu
  Cc: Jerin Jacob, dev, Ananyev, Konstantin, Thomas Monjalon,
	Bruce Richardson, Jan Viktorin, Helin Zhang
In-Reply-To: <CAP4Qi38Bp6oL7uQcOoUqFHk53ARS3Yd+RKw4w1XeHUC+aveEMw@mail.gmail.com>

On Thu, Dec 15, 2016 at 04:37:12PM +0800, Jianbo Liu wrote:
> On 14 December 2016 at 09:55, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > From: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >
> > Replace the raw I/O device memory read/write access with eal
> > abstraction for I/O device memory read/write access to fix
> > portability issues across different architectures.
> >
> > Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: Helin Zhang <helin.zhang@intel.com>
> > CC: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_osdep.h | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_osdep.h b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > index 77f0af5..9d16c21 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_osdep.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_osdep.h
> > @@ -44,6 +44,7 @@
> >  #include <rte_cycles.h>
> >  #include <rte_log.h>
> >  #include <rte_byteorder.h>
> > +#include <rte_io.h>
> >
> >  #include "../ixgbe_logs.h"
> >  #include "../ixgbe_bypass_defines.h"
> > @@ -121,16 +122,20 @@ typedef int               bool;
> >
> >  #define prefetch(x) rte_prefetch0(x)
> >
> > -#define IXGBE_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
> > +#define IXGBE_PCI_REG(reg) ({                  \
> > +       uint32_t __val;                         \
> > +       __val = rte_readl(reg);                 \
> > +       __val;                                  \
> > +})
> >
> >  static inline uint32_t ixgbe_read_addr(volatile void* addr)
> >  {
> >         return rte_le_to_cpu_32(IXGBE_PCI_REG(addr));
> >  }
> >
> > -#define IXGBE_PCI_REG_WRITE(reg, value) do { \
> > -       IXGBE_PCI_REG((reg)) = (rte_cpu_to_le_32(value)); \
> > -} while(0)
> > +#define IXGBE_PCI_REG_WRITE(reg, value) ({             \
> > +       rte_writel(rte_cpu_to_le_32(value), reg);       \
> > +})
> >
> 
> memory barrier operation is put inside IXGBE_PCI_REG_READ/WRITE in
> your change, but I found rte_*mb is called before these macros in some
> places.
> Can you remove all these redundant calls? And please do the same
> checking for other drivers.
>

Ok.

Thinking of adding _relaxed_rd/wr style macro agnostic to arch for ixgbe case 
in particular. Such that for those code incident:
x86 case> first default barrier + relaxed call.
arm case> first default barrier + relaxed call.

Does that make sense to you? If so then will take care in v2.

Santosh.


> >  #define IXGBE_PCI_REG_ADDR(hw, reg) \
> >         ((volatile uint32_t *)((char *)(hw)->hw_addr + (reg)))
> > --
> > 2.5.5
> >

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Jan Blunck @ 2016-12-16  8:14 UTC (permalink / raw)
  To: Shreyansh Jain
  Cc: dev, David Marchand, Thomas Monjalon, Ferruh Yigit, jianbo.liu,
	Jan Viktorin
In-Reply-To: <3310c320-fa39-cd8c-ab77-ced20daa5073@nxp.com>

On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>>
>> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> wrote:
>>>
>>> From: Jan Blunck <jblunck@infradead.org>
>>>
>>> This macro is based on Jan Viktorin's original patch but also checks the
>>> type of the passed pointer against the type of the member.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> [jblunck@infradead.org: add type checking and __extension__]
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>>
>>> --
>>> v2:
>>>  - fix checkpatch error
>>> ---
>>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/common/include/rte_common.h
>>> b/lib/librte_eal/common/include/rte_common.h
>>> index db5ac91..3eb8d11 100644
>>> --- a/lib/librte_eal/common/include/rte_common.h
>>> +++ b/lib/librte_eal/common/include/rte_common.h
>>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>>>  #endif
>>>
>>> +/**
>>> + * Return pointer to the wrapping struct instance.
>>> + *
>>> + * Example:
>>> + *
>>> + *  struct wrapper {
>>> + *      ...
>>> + *      struct child c;
>>> + *      ...
>>> + *  };
>>> + *
>>> + *  struct child *x = obtain(...);
>>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>>> + */
>>> +#ifndef container_of
>>> +#define container_of(ptr, type, member)        (__extension__  ({
>>> \
>>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
>>> +                       (type *)(((char *)_ptr) - offsetof(type,
>>> member));\
>>> +                       }))
>>
>>
>> This is a checkpatch false positive. It should be fine to ignore this.
>> IIRC we already discussed this before.
>
>
> I too thought something similar was discussed. I tried searching the
> archives but couldn't find anything - thus, I thought probably I was
> hallucinating :P
>
> So, you want me to revert back the '()' change? Does it impact the expansion
> of this macro?

We haven't added this on any other usage of the __extension__ keyword
in the existing code. From my perspective it is more consistent to
revert it.

Anyone else with an opinion here? David? Thomas?


>
>>
>>
>>> +#endif
>>> +
>>>  #define _RTE_STR(x) #x
>>>  /** Take a macro value and get a string version of it */
>>>  #define RTE_STR(x) _RTE_STR(x)
>>> --
>>> 2.7.4
>>>
>>
>

^ permalink raw reply

* Re: [PATCH 00/22] Generic flow API (rte_flow)
From: Adrien Mazarguil @ 2016-12-16  8:22 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, Thomas Monjalon, Pablo de Lara, Olivier Matz
In-Reply-To: <57fdc1d0-1d2b-da61-481b-358aed8d91a2@intel.com>

On Thu, Dec 15, 2016 at 12:20:36PM +0000, Ferruh Yigit wrote:
> On 12/8/2016 3:19 PM, Adrien Mazarguil wrote:
> > Hi Ferruh,
> > 
> > On Fri, Dec 02, 2016 at 04:58:53PM +0000, Ferruh Yigit wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 4:23 PM, Adrien Mazarguil wrote:
> >>> As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> >>> described in [3] (also pasted below), here is the first non-draft series
> >>> for this new API.
> >>>
> >>> Its capabilities are so generic that its name had to be vague, it may be
> >>> called "Generic flow API", "Generic flow interface" (possibly shortened
> >>> as "GFI") to refer to the name of the new filter type, or "rte_flow" from
> >>> the prefix used for its public symbols. I personally favor the latter.
> >>>
> >>> While it is currently meant to supersede existing filter types in order for
> >>> all PMDs to expose a common filtering/classification interface, it may
> >>> eventually evolve to cover the following ideas as well:
> >>>
> >>> - Rx/Tx offloads configuration through automatic offloads for specific
> >>>   packets, e.g. performing checksum on TCP packets could be expressed with
> >>>   an egress rule with a TCP pattern and a kind of checksum action.
> >>>
> >>> - RSS configuration (already defined actually). Could be global or per rule
> >>>   depending on hardware capabilities.
> >>>
> >>> - Switching configuration for devices with many physical ports; rules doing
> >>>   both ingress and egress could even be used to completely bypass software
> >>>   if supported by hardware.
> >>>
> >>>  [1] http://dpdk.org/ml/archives/dev/2016-July/043365.html
> >>>  [2] http://dpdk.org/ml/archives/dev/2016-August/045383.html
> >>>  [3] http://dpdk.org/ml/archives/dev/2016-November/050044.html
> >>>
> >>> Changes since RFC v2:
> >>>
> >>> - New separate VLAN pattern item (previously part of the ETH definition),
> >>>   found to be much more convenient.
> >>>
> >>> - Removed useless "any" field from VF pattern item, the same effect can be
> >>>   achieved by not providing a specification structure.
> >>>
> >>> - Replaced bit-fields from the VXLAN pattern item to avoid endianness
> >>>   conversion issues on 24-bit fields.
> >>>
> >>> - Updated struct rte_flow_item with a new "last" field to create inclusive
> >>>   ranges. They are defined as the interval between (spec & mask) and
> >>>   (last & mask). All three parameters are optional.
> >>>
> >>> - Renamed ID action MARK.
> >>>
> >>> - Renamed "queue" fields in actions QUEUE and DUP to "index".
> >>>
> >>> - "rss_conf" field in RSS action is now const.
> >>>
> >>> - VF action now uses a 32 bit ID like its pattern item counterpart.
> >>>
> >>> - Removed redundant struct rte_flow_pattern, API functions now expect
> >>>   struct
> >>>   rte_flow_item lists terminated by END items.
> >>>
> >>> - Replaced struct rte_flow_actions for the same reason, with struct
> >>>   rte_flow_action lists terminated by END actions.
> >>>
> >>> - Error types (enum rte_flow_error_type) have been updated and the cause
> >>>   pointer in struct rte_flow_error is now const.
> >>>
> >>> - Function prototypes (rte_flow_create, rte_flow_validate) have also been
> >>>   updated for clarity.
> >>>
> >>> Additions:
> >>>
> >>> - Public wrapper functions rte_flow_{validate|create|destroy|flush|query}
> >>>   are now implemented in rte_flow.c, with their symbols exported and
> >>>   versioned. Related filter type RTE_ETH_FILTER_GENERIC has been added.
> >>>
> >>> - A separate header (rte_flow_driver.h) has been added for driver-side
> >>>   functionality, in particular struct rte_flow_ops which contains PMD
> >>>   callbacks returned by RTE_ETH_FILTER_GENERIC query.
> >>>
> >>> - testpmd now exposes most of this API through the new "flow" command.
> >>>
> >>> What remains to be done:
> >>>
> >>> - Using endian-aware integer types (rte_beX_t) where necessary for clarity.
> >>>
> >>> - API documentation (based on RFC).
> >>>
> >>> - testpmd flow command documentation (although context-aware command
> >>>   completion should already help quite a bit in this regard).
> >>>
> >>> - A few pattern item / action properties cannot be configured yet
> >>>   (e.g. rss_conf parameter for RSS action) and a few completions
> >>>   (e.g. possible queue IDs) should be added.
> >>>
> >>
> >> <...>
> >>
> >> I was trying to check driver filter API patches, but hit a few compiler
> >> errors with this patchset.
> >>
> >> [1] clang complains about variable bitfield value changed from -1 to 1.
> >> Which is correct, but I guess that is intentional, but I don't know how
> >> to tell this to clang?
> >>
> >> [2] shred library compilation error, because of missing rte_flow_flush
> >> in rte_ether_version.map file
> >>
> >> [3] bunch of icc compilation errors, almost all are same type:
> >> error #188: enumerated type mixed with another type
> > 
> > Thanks for the report, I'll attempt to address them all in v2. 
> 
> Hi Adrien,
> 
> I would like to remind that there are driver patch sets depends to this
> patch.
> 
> New version of this patch should give some time to drivers to re-do (if
> required) the patchsets before integration deadline.

Hi Ferruh,

I intend to send v2 (including all the requested changes and fixes) shortly,
most likely today.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Adrien Mazarguil @ 2016-12-16  8:23 UTC (permalink / raw)
  To: Olivier Matz; +Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen
In-Reply-To: <1481809599-27896-1-git-send-email-olivier.matz@6wind.com>

On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:
> Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> declare the list of kernel modules required to run properly.
> 
> Today, most PCI drivers require uio/vfio.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
> 
> v2 -> v3:
> - fix kmods deps advertised by mellanox drivers as pointed out
>   by Adrien

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH 12/22] app/testpmd: add rte_flow item spec handler
From: Adrien Mazarguil @ 2016-12-16  9:17 UTC (permalink / raw)
  To: Pei, Yulong
  Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
	Olivier Matz, Xing, Beilei
In-Reply-To: <188971FCDA171749BED5DA74ABF3E6F03B6625B9@shsmsx102.ccr.corp.intel.com>

Hi Yulong,

On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> Hi Adrien,
> 
> I try to setup the following rule, but it seems that after set 'spec'  param, can not set 'mask' param,  is it an issue here or am I wrong to use it ?
> 
> testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
>  dst [TOKEN]: destination MAC
>  src [TOKEN]: source MAC
>  type [TOKEN]: EtherType
>  / [TOKEN]: specify next pattern item

You need to re-specify dst with "mask" instead of "spec". You can specify it
as many times you like to update each structure in turn, e.g.:

 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask 00:00:00:00:ff:ff

If you want to specify both spec and mask at once assuming you want it full,
these commands yield the same result:

 testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00
 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask ff:ff:ff:ff:ff:ff
 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst prefix 48

You are even allowed to change your mind:

 testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix 00:00:00:00:09:00

All these will be properly documented in the v2 patchset. Note, this version
will replace the "fix" keyword with "is" ("fix" made no sense according to
feedback).

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16  9:22 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Neil Horman, dev, thomas.monjalon, vido, fiona.trahe,
	adrien.mazarguil
In-Reply-To: <20161215092207.168ba141@xeon-e3>

Hi,

On Thu, 15 Dec 2016 09:22:07 -0800, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 15 Dec 2016 11:09:12 -0500
> Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:  
> > > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver
> > > to declare the list of kernel modules required to run properly.
> > > 
> > > Today, most PCI drivers require uio/vfio.
> > > 
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> > > ---
> > > 
> > > v2 -> v3:
> > > - fix kmods deps advertised by mellanox drivers as pointed out
> > >   by Adrien
> > > 
> > > v1 ->
> > > v2:                                                                                                
> > > - do not advertise uio_pci_generic for vf drivers
> > > - rebase on top of head: use new driver names and prefix
> > >   macro with
> > > RTE_                                                                                       
> > > 
> > > rfc -> v1:
> > > - the kmod information can be per-device using a modalias-like
> > >   pattern
> > > - change syntax to use '&' and '|' instead of ',' and ':'
> > > - remove useless prerequisites in kmod lis: no need to
> > >   specify both uio and uio_pci_generic, only the latter is
> > >   required
> > > - update kmod list in szedata2 driver
> > > - remove kmod list in qat driver: it requires more than just
> > > loading a kmod, which is described in documentation
> > > 
> > >  buildtools/pmdinfogen/pmdinfogen.c      |  1 +
> > >  buildtools/pmdinfogen/pmdinfogen.h      |  1 +
> > >  drivers/net/bnx2x/bnx2x_ethdev.c        |  2 ++
> > >  drivers/net/bnxt/bnxt_ethdev.c          |  1 +
> > >  drivers/net/cxgbe/cxgbe_ethdev.c        |  1 +
> > >  drivers/net/e1000/em_ethdev.c           |  1 +
> > >  drivers/net/e1000/igb_ethdev.c          |  2 ++
> > >  drivers/net/ena/ena_ethdev.c            |  1 +
> > >  drivers/net/enic/enic_ethdev.c          |  1 +
> > >  drivers/net/fm10k/fm10k_ethdev.c        |  1 +
> > >  drivers/net/i40e/i40e_ethdev.c          |  1 +
> > >  drivers/net/i40e/i40e_ethdev_vf.c       |  1 +
> > >  drivers/net/ixgbe/ixgbe_ethdev.c        |  2 ++
> > >  drivers/net/mlx4/mlx4.c                 |  2 ++
> > >  drivers/net/mlx5/mlx5.c                 |  1 +
> > >  drivers/net/nfp/nfp_net.c               |  1 +
> > >  drivers/net/qede/qede_ethdev.c          |  2 ++
> > >  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > >  drivers/net/thunderx/nicvf_ethdev.c     |  1 +
> > >  drivers/net/virtio/virtio_ethdev.c      |  1 +
> > >  drivers/net/vmxnet3/vmxnet3_ethdev.c    |  1 +
> > >  lib/librte_eal/common/include/rte_dev.h | 25
> > > +++++++++++++++++++++++++ tools/dpdk-pmdinfo.py
> > > |  5 ++++- 23 files changed, 56 insertions(+), 1 deletion(-)
> > >     
> > Its odd that all devices, regardless of vendor should depend on the
> > igb_uio module.  It seems to me that depending on uio_pci_generic
> > or vfio is sufficient.

igb_uio is the historical uio module of dpdk. Although it is called
igb_uio, it is not specific to Intel drivers.

Most drivers declare "igb_uio | uio_pci_generic | vfio", which means
that any of the 3 kernel modules can be used.

I think there are some cases where people will prefer using igb_uio,
for instance to use a vf pmd in a vm where there is no iommu,
and where the kernel vfio module does not support the no-iommu mode.


> 
> Yes it seems just a special case extension for Mellanox drivers.

Kmod deps are different whether it's a vf driver or not.
Mellanox drivers are not the only drivers that do not require uio,
there is also szedata2.

Is it an argument for not including this patch?


Regards,
Olivier

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16  9:23 UTC (permalink / raw)
  To: Jan Blunck
  Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
	Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02=wKpjadYrA6JUHaoG5WH=MG7Y581fFeTry7K0mbBL1Q@mail.gmail.com>

On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >>
> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> wrote:
> >>>
> >>> From: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >>> type of the passed pointer against the type of the member.
> >>>
> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >>> [jblunck@infradead.org: add type checking and __extension__]
> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >>>
> >>> --
> >>> v2:
> >>>  - fix checkpatch error
> >>> ---
> >>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >>>  1 file changed, 21 insertions(+)
> >>>
> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >>> b/lib/librte_eal/common/include/rte_common.h
> >>> index db5ac91..3eb8d11 100644
> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
> >>>  #endif
> >>>
> >>> +/**
> >>> + * Return pointer to the wrapping struct instance.
> >>> + *
> >>> + * Example:
> >>> + *
> >>> + *  struct wrapper {
> >>> + *      ...
> >>> + *      struct child c;
> >>> + *      ...
> >>> + *  };
> >>> + *
> >>> + *  struct child *x = obtain(...);
> >>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
> >>> + */
> >>> +#ifndef container_of
> >>> +#define container_of(ptr, type, member)        (__extension__  ({
> >>> \
> >>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
> >>> +                       (type *)(((char *)_ptr) - offsetof(type,
> >>> member));\
> >>> +                       }))
> >>
> >>
> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> IIRC we already discussed this before.
> >
> >
> > I too thought something similar was discussed. I tried searching the
> > archives but couldn't find anything - thus, I thought probably I was
> > hallucinating :P
> >
> > So, you want me to revert back the '()' change? Does it impact the expansion
> > of this macro?
> 
> We haven't added this on any other usage of the __extension__ keyword
> in the existing code. From my perspective it is more consistent to
> revert it.
> 
> Anyone else with an opinion here? David? Thomas?

As an exported header, rte_common.h must pass check-includes.sh. Both
typeof() and the ({ ... }) construct are non-standard GCC extensions and
would fail to compile with pedantic options.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Olivier Matz @ 2016-12-16  9:36 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: dev, nhorman, thomas.monjalon, vido, fiona.trahe, stephen,
	adrien.mazarguil
In-Reply-To: <ba0a18d0-d4db-3c65-f11c-10465924082d@intel.com>

Hi Ferruh,

On Thu, 15 Dec 2016 14:52:02 +0000, Ferruh Yigit
<ferruh.yigit@intel.com> wrote:
> Hi Olivier, Thomas,
> 
> On 12/15/2016 1:46 PM, Olivier Matz wrote:
> > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver to
> > declare the list of kernel modules required to run properly.
> > 
> > Today, most PCI drivers require uio/vfio.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > Acked-by: Fiona Trahe <fiona.trahe@intel.com>  
> 
> This patch is for master branch, what do you think targeting it to
> next-net tree?
> So that new PMDs also can be included into patch?

That makes sense, I can rebase on next-net.
Thomas, do you agree?

From what I see, the 2 new PMDs are sfc (uio) and tap (nothing).


Regards,
Olivier

^ permalink raw reply

* [PATCH 1/2] ethdev: fix name index in xstats Api
From: Olivier Matz @ 2016-12-16  9:44 UTC (permalink / raw)
  To: dev, thomas.monjalon; +Cc: remy.horton, stable

The function rte_eth_xstats_get() return an array of tuples (id,
value). The value is the statistic counter, while the id references a
name in the array returned by rte_eth_xstats_get_name().

Today, each 'id' returned by rte_eth_xstats_get() is equal to the index
in the returned array, making this value useless. It also prevents a
driver from having different indexes for names and value, like in the
example below:

  rte_eth_xstats_get_name() returns:
    0: "rx0_stat"
    1: "rx1_stat"
    2: ...
    7: "rx7_stat"
    8: "tx0_stat"
    9: "tx1_stat"
    ...
    15: "tx7_stat"

  rte_eth_xstats_get() returns:
    0: id=0, val=<stat>    ("rx0_stat")
    1: id=1, val=<stat>    ("rx1_stat")
    2: id=8, val=<stat>    ("tx0_stat")
    3: id=9, val=<stat>    ("tx1_stat")

This patch fixes the drivers to set the 'id' in their ethdev->xstats_get()
(except e1000 which was already doing it), and fixes ethdev by not setting
the 'id' field to the index of the table for pmd-specific stats: instead,
they should just be shifted by the max number of generic statistics.

CC: stable@dpdk.org
Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/bnx2x/bnx2x_ethdev.c   | 1 +
 drivers/net/fm10k/fm10k_ethdev.c   | 3 +++
 drivers/net/i40e/i40e_ethdev.c     | 4 ++++
 drivers/net/ixgbe/ixgbe_ethdev.c   | 3 +++
 drivers/net/qede/qede_ethdev.c     | 2 ++
 drivers/net/vhost/rte_eth_vhost.c  | 2 ++
 drivers/net/virtio/virtio_ethdev.c | 2 ++
 lib/librte_ether/rte_ethdev.c      | 5 ++++-
 8 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 0f1e4a2..f01047f 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -422,6 +422,7 @@ bnx2x_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[num].value =
 					  *(uint64_t *)((char *)&sc->eth_stats +
 					  bnx2x_xstats_strings[num].offset_lo);
+		xstats[num].id = num;
 	}
 
 	return num;
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index fe74f6d..0d3098e 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1315,6 +1315,7 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < FM10K_NB_HW_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			fm10k_hw_stats_strings[count].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -1324,12 +1325,14 @@ fm10k_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value =
 				*(uint64_t *)(((char *)&hw_stats->q[q]) +
 				fm10k_hw_stats_rx_q_strings[i].offset);
+			xstats[count].id = count;
 			count++;
 		}
 		for (i = 0; i < FM10K_NB_TX_Q_XSTATS; i++) {
 			xstats[count].value =
 				*(uint64_t *)(((char *)&hw_stats->q[q]) +
 				fm10k_hw_stats_tx_q_strings[i].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index b0c0fbf..f01455e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2533,6 +2533,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < I40E_NB_ETH_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)&hw_stats->eth) +
 			rte_i40e_stats_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2540,6 +2541,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < I40E_NB_HW_PORT_XSTATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_i40e_hw_port_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2549,6 +2551,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_rxq_prio_strings[i].offset +
 				(sizeof(uint64_t) * prio));
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -2559,6 +2562,7 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)hw_stats) +
 				rte_i40e_txq_prio_strings[i].offset +
 				(sizeof(uint64_t) * prio));
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index baffc71..cf68fa9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2902,6 +2902,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
 		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 				rte_ixgbe_stats_strings[i].offset);
+		xstats[count].id = count;
 		count++;
 	}
 
@@ -2911,6 +2912,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_rxq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -2921,6 +2923,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 					rte_ixgbe_txq_strings[stat].offset +
 					(sizeof(uint64_t) * i));
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 001166a..6f3b10c 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -973,6 +973,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	for (i = 0; i < RTE_DIM(qede_xstats_strings); i++) {
 		xstats[stat_idx].value = *(uint64_t *)(((char *)&stats) +
 					     qede_xstats_strings[i].offset);
+		xstats[stat_idx].id = stat_idx;
 		stat_idx++;
 	}
 
@@ -982,6 +983,7 @@ qede_get_xstats(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				xstats[stat_idx].value = *(uint64_t *)(
 					((char *)(qdev->fp_array[(qid)].rxq)) +
 					 qede_rxq_xstats_strings[i].offset);
+				xstats[stat_idx].id = stat_idx;
 				stat_idx++;
 			}
 		}
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 766d4ef..dd79ccf 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -324,6 +324,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)vq)
 				+ vhost_rxport_stat_strings[t].offset);
 		}
+		xstats[count].id = count;
 		count++;
 	}
 	for (t = 0; t < VHOST_NB_XSTATS_TXPORT; t++) {
@@ -336,6 +337,7 @@ vhost_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 				*(uint64_t *)(((char *)vq)
 				+ vhost_txport_stat_strings[t].offset);
 		}
+		xstats[count].id = count;
 		count++;
 	}
 	return count;
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1bd60e9..f6335aa 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -893,6 +893,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		for (t = 0; t < VIRTIO_NB_RXQ_XSTATS; t++) {
 			xstats[count].value = *(uint64_t *)(((char *)rxvq) +
 				rte_virtio_rxq_stat_strings[t].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
@@ -908,6 +909,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		for (t = 0; t < VIRTIO_NB_TXQ_XSTATS; t++) {
 			xstats[count].value = *(uint64_t *)(((char *)txvq) +
 				rte_virtio_txq_stat_strings[t].offset);
+			xstats[count].id = count;
 			count++;
 		}
 	}
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 1e0f206..45d8286 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1491,8 +1491,11 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		}
 	}
 
-	for (i = 0; i < count + xcount; i++)
+	for (i = 0; i < count; i++)
 		xstats[i].id = i;
+	/* add an offset to driver-specific stats */
+	for ( ; i < count + xcount; i++)
+		xstats[i].id += count;
 
 	return count + xcount;
 }
-- 
2.8.1

^ permalink raw reply related

* [PATCH 2/2] ethdev: clarify xstats Api documentation
From: Olivier Matz @ 2016-12-16  9:44 UTC (permalink / raw)
  To: dev, thomas.monjalon; +Cc: remy.horton, stable
In-Reply-To: <1481881454-17382-1-git-send-email-olivier.matz@6wind.com>

Reword the Api documentation of xstats ethdev.

CC: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_ether/rte_ethdev.h | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..4479553 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -938,23 +938,26 @@ struct rte_eth_txq_info {
 /**
  * An Ethernet device extended statistic structure
  *
- * This structure is used by ethdev->eth_xstats_get() to provide
- * statistics that are not provided in the generic rte_eth_stats
+ * This structure is used by rte_eth_xstats_get() to provide
+ * statistics that are not provided in the generic *rte_eth_stats*
  * structure.
+ * It maps a name id, corresponding to an index in the array returned
+ * by rte_eth_xstats_get_names(), to a statistic value.
  */
 struct rte_eth_xstat {
-	uint64_t id;
-	uint64_t value;
+	uint64_t id;        /**< The index in xstats name array. */
+	uint64_t value;     /**< The statistic counter value. */
 };
 
 /**
- * A name-key lookup element for extended statistics.
+ * A name element for extended statistics.
  *
- * This structure is used to map between names and ID numbers
- * for extended ethernet statistics.
+ * An array of this structure is returned by rte_eth_xstats_get_names().
+ * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
+ * structure references these names by their array index.
  */
 struct rte_eth_xstat_name {
-	char name[RTE_ETH_XSTATS_NAME_SIZE];
+	char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */
 };
 
 #define ETH_DCB_NUM_TCS    8
@@ -2272,18 +2275,19 @@ void rte_eth_stats_reset(uint8_t port_id);
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param xstats_names
- *  Block of memory to insert names into. Must be at least size in capacity.
- *  If set to NULL, function returns required capacity.
+ *   An rte_eth_xstat_name array of at least *size* elements to
+ *   be filled. If set to NULL, the function returns the required number
+ *   of elements.
  * @param size
- *  Capacity of xstats_names (number of names).
+ *   The size of the xstats_names array (number of elements).
  * @return
- *   - positive value lower or equal to size: success. The return value
+ *   - A positive value lower or equal to size: success. The return value
  *     is the number of entries filled in the stats table.
- *   - positive value higher than size: error, the given statistics table
+ *   - A positive value higher than size: error, the given statistics table
  *     is too small. The return value corresponds to the size that should
  *     be given to succeed. The entries in the table are not valid and
  *     shall not be used by the caller.
- *   - negative value on error (invalid port id)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get_names(uint8_t port_id,
 		struct rte_eth_xstat_name *xstats_names,
@@ -2296,19 +2300,20 @@ int rte_eth_xstats_get_names(uint8_t port_id,
  *   The port identifier of the Ethernet device.
  * @param xstats
  *   A pointer to a table of structure of type *rte_eth_xstat*
- *   to be filled with device statistics ids and values.
+ *   to be filled with device statistics ids and values: id is the
+ *   index of the name string in xstats_names (@see rte_eth_xstats_get_names),
+ *   and value is the statistic counter.
  *   This parameter can be set to NULL if n is 0.
  * @param n
- *   The size of the stats table, which should be large enough to store
- *   all the statistics of the device.
+ *   The size of the xstats array (number of elements).
  * @return
- *   - positive value lower or equal to n: success. The return value
+ *   - A positive value lower or equal to n: success. The return value
  *     is the number of entries filled in the stats table.
- *   - positive value higher than n: error, the given statistics table
+ *   - A positive value higher than n: error, the given statistics table
  *     is too small. The return value corresponds to the size that should
  *     be given to succeed. The entries in the table are not valid and
  *     shall not be used by the caller.
- *   - negative value on error (invalid port id)
+ *   - A negative value on error (invalid port id).
  */
 int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		unsigned n);
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH v2 00/32] Support more features in Solarflare PMD
From: Ferruh Yigit @ 2016-12-16  9:57 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
In-Reply-To: <1481806283-10387-1-git-send-email-arybchenko@solarflare.com>

On 12/15/2016 12:50 PM, Andrew Rybchenko wrote:
> The patch series adds a number of features to Solarflare libefx-based
> PMD. Basically one patch per feature.
> 
> The patches are grouped into one series since they touch nearby lines
> in either PMD feature list, or dev_ops structure, or documentation.
> So, patches cannot be applied in arbitrary order.
> 
> ---
> 
> v2:
> * Fix ICC and clang warnings
> * Slightly change sfc_tso_{alloc,free}_tsoh_objs() prototypes
> 
> 
> Andrew Rybchenko (17):
>   net/sfc: implement MCDI logging callback
>   net/sfc: support parameter to choose performance profile
>   net/sfc: implement ethdev hook to get basic statistics
>   net/sfc: support extended statistics
>   net/sfc: support flow control settings get/set
>   net/sfc: support link status change interrupt
>   net/sfc: implement device operation to change MTU
>   net/sfc: support link speed and duplex settings
>   net/sfc: support checksum offloads on receive
>   net/sfc: handle received packet type info provided by HW
>   net/sfc: support callback to get receive queue information
>   net/sfc: support Rx free threshold
>   net/sfc: add callback to get RxQ pending descriptors count
>   net/sfc: add RxQ descriptor done callback
>   net/sfc: support scattered Rx DMA
>   net/sfc: support deferred start of receive queues
>   net/sfc/base: do not use enum type when values are bitmask
> 
> Artem Andreev (1):
>   net/sfc: support link up/down
> 
> Ivan Malov (14):
>   net/sfc: support promiscuous and all-multicast control
>   net/sfc: support main (the first) MAC address change
>   net/sfc: support multicast addresses list controls
>   net/sfc: add callback to get transmit queue information
>   net/sfc: support Tx free threshold
>   net/sfc: support deferred start of transmit queues
>   net/sfc: support VLAN offload on transmit path
>   net/sfc: add basic stubs for RSS support on driver attach
>   net/sfc: support RSS hash offload
>   net/sfc: add callback to query RSS key and hash types config
>   net/sfc: add callback to set RSS key and hash types config
>   net/sfc: add callback to query RSS redirection table
>   net/sfc: add callback to update RSS redirection table
>   net/sfc: support firmware-assisted TSOv2
> 

<...>

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH 1/4] rte_mbuf: add rte_pktmbuf_coalesce
From: Olivier Matz @ 2016-12-16 10:06 UTC (permalink / raw)
  To: Tomasz Kulasek; +Cc: dev
In-Reply-To: <1480698466-17620-2-git-send-email-tomaszx.kulasek@intel.com>

Hi Tomasz,

On Fri,  2 Dec 2016 18:07:43 +0100, Tomasz Kulasek
<tomaszx.kulasek@intel.com> wrote:
> This patch adds function rte_pktmbuf_coalesce to let crypto PMD
> coalesce chained mbuf before crypto operation and extend their
> capabilities to support segmented mbufs when device cannot handle
> them natively.
> 
> 
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
>  lib/librte_mbuf/rte_mbuf.h |   34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ead7c6e..f048681 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1647,6 +1647,40 @@ static inline int rte_pktmbuf_chain(struct
> rte_mbuf *head, struct rte_mbuf *tail }
>  
>  /**
> + * Coalesce data from mbuf to the continuous buffer.
> + *
> + * @param mbuf_dst
> + *   Contiguous destination mbuf
> + * @param mbuf_src
> + *   Uncontiguous source mbuf
> + *
> + * @return
> + *   - 0, on success
> + *   - -EINVAL, on error
> + */

I think the API should be clarified. In your case, it is expected that the
destination mbuf is already filled with uninitialized data (i.e. that
rte_pktmbuf_append() has been called).

We could wonder if a better API wouldn't be to allocate the dst mbuf in
the function, call append()/prepend(), and do the copy.

Even better, we could have:

  int rte_pktmbuf_linearize(struct rte_mbuf *m)

It will reuse the same mbuf (maybe moving the data).


> +
> +#include <rte_hexdump.h>

This should be removed.

> +
> +static inline int
> +rte_pktmbuf_coalesce(struct rte_mbuf *mbuf_dst, struct rte_mbuf *mbuf_src) {

Source mbuf should be const.

> +	char *dst;
> +
> +	if (!rte_pktmbuf_is_contiguous(mbuf_dst) ||
> +			rte_pktmbuf_data_len(mbuf_dst) >=
> +			rte_pktmbuf_pkt_len(mbuf_src))
> +		return -EINVAL;

Why >= ?

> +
> +	dst = rte_pktmbuf_mtod(mbuf_dst, char *);
> +
> +	if (!__rte_pktmbuf_read(mbuf_src, 0, rte_pktmbuf_pkt_len(mbuf_src),
> +			dst))

When a function returns a pointer, I think it is clearer to do:
  if (func() == NULL)
than:
  if (!func())


> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/**
>   * Dump an mbuf structure to a file.
>   *
>   * Dump all fields for the given packet mbuf and all its associated


One more question, I don't see where this function is used in your
patchset. What is your use-case?

Regards,
Olivier

^ permalink raw reply

* Re: [PATCH 07/13] pcap: fix timestamps in output pcap file
From: Ferruh Yigit @ 2016-12-16 10:06 UTC (permalink / raw)
  To: Michał Mirosław, dev; +Cc: "CC: stable"
In-Reply-To: <b8fb833e-634e-f559-12ce-1e71d7740ce5@intel.com>

On 12/14/2016 5:45 PM, Ferruh Yigit wrote:
> On 12/13/2016 1:08 AM, Michał Mirosław wrote:
>> From: Piotr Bartosiewicz <piotr.bartosiewicz@atendesoftware.pl>
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>> ---
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

CC: stable@dpdk.org

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jianbo Liu @ 2016-12-16 10:12 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
	Jan Viktorin
In-Reply-To: <20161215110807.GA10881@localhost.localdomain>

On 15 December 2016 at 19:08, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
>> On 15 December 2016 at 18:04, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
>> >> On 14 December 2016 at 09:55, Jerin Jacob
>> >> <jerin.jacob@caviumnetworks.com> wrote:
>> >> > Override the generic I/O device memory read/write access and implement it
>> >> > using armv8 instructions for arm64.
>> >> >
>> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> >> > ---
>> >> >  lib/librte_eal/common/include/arch/arm/rte_io.h    |   4 +
>> >> >  lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
>> >> >  2 files changed, 187 insertions(+)
>> >> >  create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> >
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > index 74c1f2c..9593b42 100644
>> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
>> >> > @@ -38,7 +38,11 @@
>> >> >  extern "C" {
>> >> >  #endif
>> >> >
>> >> > +#ifdef RTE_ARCH_64
>> >> > +#include "rte_io_64.h"
>> >> > +#else
>> >> >  #include "generic/rte_io.h"
>> >> > +#endif
>> >> >
>> >> >  #ifdef __cplusplus
>> >> >  }
>> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > new file mode 100644
>> >> > index 0000000..09e7a89
>> >> > --- /dev/null
>> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
>> >> > @@ -0,0 +1,183 @@
>> >> > +/*
>> >> > + *   BSD LICENSE
>> >> > + *
>> >> > + *   Copyright (C) Cavium networks Ltd. 2016.
>> >> > + *
>> >> > + *   Redistribution and use in source and binary forms, with or without
>> >> > + *   modification, are permitted provided that the following conditions
>> >> > + *   are met:
>> >> > + *
>> >> > + *     * Redistributions of source code must retain the above copyright
>> >> > + *       notice, this list of conditions and the following disclaimer.
>> >> > + *     * Redistributions in binary form must reproduce the above copyright
>> >> > + *       notice, this list of conditions and the following disclaimer in
>> >> > + *       the documentation and/or other materials provided with the
>> >> > + *       distribution.
>> >> > + *     * Neither the name of Cavium networks nor the names of its
>> >> > + *       contributors may be used to endorse or promote products derived
>> >> > + *       from this software without specific prior written permission.
>> >> > + *
>> >> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> >> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> >> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> >> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> >> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> >> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> >> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> >> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> >> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> >> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> >> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> >> > + */
>> >> > +
>> >> > +#ifndef _RTE_IO_ARM64_H_
>> >> > +#define _RTE_IO_ARM64_H_
>> >> > +
>> >> > +#ifdef __cplusplus
>> >> > +extern "C" {
>> >> > +#endif
>> >> > +
>> >> > +#include <stdint.h>
>> >> > +
>> >> > +#define RTE_OVERRIDE_IO_H
>> >> > +
>> >> > +#include "generic/rte_io.h"
>> >> > +#include "rte_atomic_64.h"
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint8_t
>> >> > +__rte_arm64_readb(const volatile void *addr)
>> >> > +{
>> >> > +       uint8_t val;
>> >> > +
>> >> > +       asm volatile(
>> >> > +                   "ldrb %w[val], [%x[addr]]"
>> >> > +                   : [val] "=r" (val)
>> >> > +                   : [addr] "r" (addr));
>> >> > +       return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint16_t
>> >> > +__rte_arm64_readw(const volatile void *addr)
>> >> > +{
>> >> > +       uint16_t val;
>> >> > +
>> >> > +       asm volatile(
>> >> > +                   "ldrh %w[val], [%x[addr]]"
>> >> > +                   : [val] "=r" (val)
>> >> > +                   : [addr] "r" (addr));
>> >> > +       return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint32_t
>> >> > +__rte_arm64_readl(const volatile void *addr)
>> >> > +{
>> >> > +       uint32_t val;
>> >> > +
>> >> > +       asm volatile(
>> >> > +                   "ldr %w[val], [%x[addr]]"
>> >> > +                   : [val] "=r" (val)
>> >> > +                   : [addr] "r" (addr));
>> >> > +       return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) uint64_t
>> >> > +__rte_arm64_readq(const volatile void *addr)
>> >> > +{
>> >> > +       uint64_t val;
>> >> > +
>> >> > +       asm volatile(
>> >> > +                   "ldr %x[val], [%x[addr]]"
>> >> > +                   : [val] "=r" (val)
>> >> > +                   : [addr] "r" (addr));
>> >> > +       return val;
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
>> >> > +{
>> >> > +       asm volatile(
>> >> > +                   "strb %w[val], [%x[addr]]"
>> >> > +                   :
>> >> > +                   : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
>> >> > +{
>> >> > +       asm volatile(
>> >> > +                   "strh %w[val], [%x[addr]]"
>> >> > +                   :
>> >> > +                   : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
>> >> > +{
>> >> > +       asm volatile(
>> >> > +                   "str %w[val], [%x[addr]]"
>> >> > +                   :
>> >> > +                   : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >> > +
>> >> > +static inline __attribute__((always_inline)) void
>> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
>> >> > +{
>> >> > +       asm volatile(
>> >> > +                   "str %x[val], [%x[addr]]"
>> >> > +                   :
>> >> > +                   : [val] "r" (val), [addr] "r" (addr));
>> >> > +}
>> >>
>> >> I'm not quite sure about these overridings. Can you explain the
>> >> benefit to do so?
>> >
>> > Better to be native if there is option. That all. Do you see any issue?
>> > or what is the real concern?
>> >
>>
>> I think it's the same as the generic c version after compiling. Am I right?
>
> I really don't that is the case for all the scenarios like compiler may
> combine two 16bit reads one 32bit read etc and which will impact on IO

I wonder which compiler will do that as armv8 is 32/64 bit system?

> register access.
>
> But, I am sure the proposed scheme generates correct instruction in all the cases.

^ permalink raw reply

* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Yang, Zhiyong @ 2016-12-16 10:19 UTC (permalink / raw)
  To: Richardson, Bruce
  Cc: Ananyev, Konstantin, Thomas Monjalon, dev@dpdk.org,
	yuanhan.liu@linux.intel.com, De Lara Guarch, Pablo
In-Reply-To: <20161215101242.GA125588@bricha3-MOBL3.ger.corp.intel.com>

Hi, Bruce:

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Thursday, December 15, 2016 6:13 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> Monjalon <thomas.monjalon@6wind.com>; dev@dpdk.org;
> yuanhan.liu@linux.intel.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce rte_memset on
> IA platform
> 
> On Thu, Dec 15, 2016 at 06:51:08AM +0000, Yang, Zhiyong wrote:
> > Hi, Thomas, Konstantin:
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
> > > Sent: Sunday, December 11, 2016 8:33 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > Monjalon <thomas.monjalon@6wind.com>
> > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> rte_memset
> > > on IA platform
> > >
> > > Hi, Konstantin, Bruce:
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Thursday, December 8, 2016 6:31 PM
> > > > To: Yang, Zhiyong <zhiyong.yang@intel.com>; Thomas Monjalon
> > > > <thomas.monjalon@6wind.com>
> > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > <pablo.de.lara.guarch@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > rte_memset on IA platform
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Yang, Zhiyong
> > > > > Sent: Thursday, December 8, 2016 9:53 AM
> > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Thomas
> > > > > Monjalon <thomas.monjalon@6wind.com>
> > > > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; Richardson, Bruce
> > > > > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > > > > <pablo.de.lara.guarch@intel.com>
> > > > > Subject: RE: [dpdk-dev] [PATCH 1/4] eal/common: introduce
> > > > > rte_memset on IA platform
> > > > >
> > > > extern void *(*__rte_memset_vector)( (void *s, int c, size_t n);
> > > >
> > > > static inline void*
> > > > rte_memset_huge(void *s, int c, size_t n) {
> > > >    return __rte_memset_vector(s, c, n); }
> > > >
> > > > static inline void *
> > > > rte_memset(void *s, int c, size_t n) {
> > > > 	If (n < XXX)
> > > > 		return rte_memset_scalar(s, c, n);
> > > > 	else
> > > > 		return rte_memset_huge(s, c, n); }
> > > >
> > > > XXX could be either a define, or could also be a variable, so it
> > > > can be setuped at startup, depending on the architecture.
> > > >
> > > > Would that work?
> > > > Konstantin
> > > >
> > I have implemented the code for  choosing the functions at run time.
> > rte_memcpy is used more frequently, So I test it at run time.
> >
> > typedef void *(*rte_memcpy_vector_t)(void *dst, const void *src,
> > size_t n); extern rte_memcpy_vector_t rte_memcpy_vector; static inline
> > void * rte_memcpy(void *dst, const void *src, size_t n) {
> >         return rte_memcpy_vector(dst, src, n); } In order to reduce
> > the overhead at run time, I assign the function address to var
> > rte_memcpy_vector before main() starts to init the var.
> >
> > static void __attribute__((constructor))
> > rte_memcpy_init(void)
> > {
> > 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > 	{
> > 		rte_memcpy_vector = rte_memcpy_avx2;
> > 	}
> > 	else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
> > 	{
> > 		rte_memcpy_vector = rte_memcpy_sse;
> > 	}
> > 	else
> > 	{
> > 		rte_memcpy_vector = memcpy;
> > 	}
> >
> > }
> > I run the same virtio/vhost loopback tests without NIC.
> > I can see the  throughput drop  when running choosing functions at run
> > time compared to original code as following on the same platform(my
> machine is haswell)
> > 	Packet size	perf drop
> > 	64 		-4%
> > 	256 		-5.4%
> > 	1024		-5%
> > 	1500		-2.5%
> > Another thing, I run the memcpy_perf_autotest,  when N= <128, the
> > rte_memcpy perf gains almost disappears When choosing functions at run
> > time.  For N=other numbers, the perf gains will become narrow.
> >
> How narrow. How significant is the improvement that we gain from having to
> maintain our own copy of memcpy. If the libc version is nearly as good we
> should just use that.
> 
> /Bruce

Zhihong sent a patch about rte_memcpy,  From the patch,  
we can see the optimization job for memcpy will bring obvious perf improvements
than glibc for DPDK.
http://www.dpdk.org/dev/patchwork/patch/17753/
git log as following:
This patch is tested on Ivy Bridge, Haswell and Skylake, it provides
up to 20% gain for Virtio Vhost PVP traffic, with packet size ranging
from 64 to 1500 bytes.

thanks
Zhiyong

^ permalink raw reply

* Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64
From: Jerin Jacob @ 2016-12-16 10:25 UTC (permalink / raw)
  To: Jianbo Liu
  Cc: dev, Ananyev, Konstantin, Thomas Monjalon, Bruce Richardson,
	Jan Viktorin
In-Reply-To: <CAP4Qi38_5zKe6a=v3W5wzcDgeB9Un97O8UAybucgPjhucHsE8Q@mail.gmail.com>

On Fri, Dec 16, 2016 at 06:12:13PM +0800, Jianbo Liu wrote:
> On 15 December 2016 at 19:08, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
> >> On 15 December 2016 at 18:04, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> >> >> On 14 December 2016 at 09:55, Jerin Jacob
> >> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> >> > Override the generic I/O device memory read/write access and implement it
> >> >> > using armv8 instructions for arm64.
> >> >> >
> >> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> >> > ---
> >> >> >  lib/librte_eal/common/include/arch/arm/rte_io.h    |   4 +
> >> >> >  lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++
> >> >> >  2 files changed, 187 insertions(+)
> >> >> >  create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> >
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > index 74c1f2c..9593b42 100644
> >> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
> >> >> > @@ -38,7 +38,11 @@
> >> >> >  extern "C" {
> >> >> >  #endif
> >> >> >
> >> >> > +#ifdef RTE_ARCH_64
> >> >> > +#include "rte_io_64.h"
> >> >> > +#else
> >> >> >  #include "generic/rte_io.h"
> >> >> > +#endif
> >> >> >
> >> >> >  #ifdef __cplusplus
> >> >> >  }
> >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > new file mode 100644
> >> >> > index 0000000..09e7a89
> >> >> > --- /dev/null
> >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
> >> >> > @@ -0,0 +1,183 @@
> >> >> > +/*
> >> >> > + *   BSD LICENSE
> >> >> > + *
> >> >> > + *   Copyright (C) Cavium networks Ltd. 2016.
> >> >> > + *
> >> >> > + *   Redistribution and use in source and binary forms, with or without
> >> >> > + *   modification, are permitted provided that the following conditions
> >> >> > + *   are met:
> >> >> > + *
> >> >> > + *     * Redistributions of source code must retain the above copyright
> >> >> > + *       notice, this list of conditions and the following disclaimer.
> >> >> > + *     * Redistributions in binary form must reproduce the above copyright
> >> >> > + *       notice, this list of conditions and the following disclaimer in
> >> >> > + *       the documentation and/or other materials provided with the
> >> >> > + *       distribution.
> >> >> > + *     * Neither the name of Cavium networks nor the names of its
> >> >> > + *       contributors may be used to endorse or promote products derived
> >> >> > + *       from this software without specific prior written permission.
> >> >> > + *
> >> >> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >> >> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >> >> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >> >> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >> >> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >> >> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >> >> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >> >> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >> >> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >> >> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >> >> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >> >> > + */
> >> >> > +
> >> >> > +#ifndef _RTE_IO_ARM64_H_
> >> >> > +#define _RTE_IO_ARM64_H_
> >> >> > +
> >> >> > +#ifdef __cplusplus
> >> >> > +extern "C" {
> >> >> > +#endif
> >> >> > +
> >> >> > +#include <stdint.h>
> >> >> > +
> >> >> > +#define RTE_OVERRIDE_IO_H
> >> >> > +
> >> >> > +#include "generic/rte_io.h"
> >> >> > +#include "rte_atomic_64.h"
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint8_t
> >> >> > +__rte_arm64_readb(const volatile void *addr)
> >> >> > +{
> >> >> > +       uint8_t val;
> >> >> > +
> >> >> > +       asm volatile(
> >> >> > +                   "ldrb %w[val], [%x[addr]]"
> >> >> > +                   : [val] "=r" (val)
> >> >> > +                   : [addr] "r" (addr));
> >> >> > +       return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint16_t
> >> >> > +__rte_arm64_readw(const volatile void *addr)
> >> >> > +{
> >> >> > +       uint16_t val;
> >> >> > +
> >> >> > +       asm volatile(
> >> >> > +                   "ldrh %w[val], [%x[addr]]"
> >> >> > +                   : [val] "=r" (val)
> >> >> > +                   : [addr] "r" (addr));
> >> >> > +       return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint32_t
> >> >> > +__rte_arm64_readl(const volatile void *addr)
> >> >> > +{
> >> >> > +       uint32_t val;
> >> >> > +
> >> >> > +       asm volatile(
> >> >> > +                   "ldr %w[val], [%x[addr]]"
> >> >> > +                   : [val] "=r" (val)
> >> >> > +                   : [addr] "r" (addr));
> >> >> > +       return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) uint64_t
> >> >> > +__rte_arm64_readq(const volatile void *addr)
> >> >> > +{
> >> >> > +       uint64_t val;
> >> >> > +
> >> >> > +       asm volatile(
> >> >> > +                   "ldr %x[val], [%x[addr]]"
> >> >> > +                   : [val] "=r" (val)
> >> >> > +                   : [addr] "r" (addr));
> >> >> > +       return val;
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr)
> >> >> > +{
> >> >> > +       asm volatile(
> >> >> > +                   "strb %w[val], [%x[addr]]"
> >> >> > +                   :
> >> >> > +                   : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr)
> >> >> > +{
> >> >> > +       asm volatile(
> >> >> > +                   "strh %w[val], [%x[addr]]"
> >> >> > +                   :
> >> >> > +                   : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr)
> >> >> > +{
> >> >> > +       asm volatile(
> >> >> > +                   "str %w[val], [%x[addr]]"
> >> >> > +                   :
> >> >> > +                   : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >> > +
> >> >> > +static inline __attribute__((always_inline)) void
> >> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr)
> >> >> > +{
> >> >> > +       asm volatile(
> >> >> > +                   "str %x[val], [%x[addr]]"
> >> >> > +                   :
> >> >> > +                   : [val] "r" (val), [addr] "r" (addr));
> >> >> > +}
> >> >>
> >> >> I'm not quite sure about these overridings. Can you explain the
> >> >> benefit to do so?
> >> >
> >> > Better to be native if there is option. That all. Do you see any issue?
> >> > or what is the real concern?
> >> >
> >>
> >> I think it's the same as the generic c version after compiling. Am I right?
> >
> > I really don't that is the case for all the scenarios like compiler may
> > combine two 16bit reads one 32bit read etc and which will impact on IO
> 
> I wonder which compiler will do that as armv8 is 32/64 bit system?
Not specific to armv8.
Two consecutive continues 16bits reads one 32bit read for optimization.
Any idea why Linux kernel doing explicit instructions for readl/writel?
obviously not for fun.

> 
> > register access.
> >
> > But, I am sure the proposed scheme generates correct instruction in all the cases.

^ permalink raw reply

* Re: [PATCH] doc: Fix a typo in testpmd application user guide.
From: Mcnamara, John @ 2016-12-16 10:27 UTC (permalink / raw)
  To: Rosen, Rami, dev@dpdk.org; +Cc: Rosen, Rami
In-Reply-To: <1481835800-6241-1-git-send-email-rami.rosen@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rami Rosen
> Sent: Thursday, December 15, 2016 9:03 PM
> To: dev@dpdk.org
> Cc: Rosen, Rami <rami.rosen@intel.com>
> Subject: [dpdk-dev] [PATCH] doc: Fix a typo in testpmd application user
> guide.
> 
> This patch fixes a trivial typo in testpmd application user guide.
> 
> Signed-off-by: Rami Rosen <rami.rosen@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply

* Re: [PATCH v2 4/4] doc: add ixgbe specific APIs
From: Mcnamara, John @ 2016-12-16 10:29 UTC (permalink / raw)
  To: Bie, Tiwei, dev@dpdk.org
  Cc: Lu, Wenzhuo, Dai, Wei, Wang, Xiao W, Ananyev, Konstantin,
	Zhang, Helin
In-Reply-To: <1481852611-103254-5-git-send-email-tiwei.bie@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tiwei Bie
> Sent: Friday, December 16, 2016 1:44 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Wang, Xiao W <xiao.w.wang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 4/4] doc: add ixgbe specific APIs
> 
> Add information about the new ixgbe PMD APIs in the release note.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply

* Re: [PATCH] crypto/openssl: fix extra bytes being written at end of data
From: De Lara Guarch, Pablo @ 2016-12-16 10:30 UTC (permalink / raw)
  To: Azarewicz, PiotrX T; +Cc: dev@dpdk.org, stable@dpdk.org
In-Reply-To: <1481107554-206879-1-git-send-email-piotrx.t.azarewicz@intel.com>



> -----Original Message-----
> From: Azarewicz, PiotrX T
> Sent: Wednesday, December 07, 2016 10:46 AM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] crypto/openssl: fix extra bytes being written at end of
> data
> 
> Extra bytes are being written at end of data while process standard
> openssl cipher encryption. This behaviour is unexpected.
> 
> This patch disable the padding feature in openssl library, which is
> causing the problem.
> 
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH v2 08/13] PMD/af_packet: guard against buffer overruns in RX path
From: Ferruh Yigit @ 2016-12-16 10:32 UTC (permalink / raw)
  To: John W. Linville, Michał Mirosław; +Cc: dev, test-report
In-Reply-To: <20161213160550.GA27914@tuxdriver.com>

On 12/13/2016 4:05 PM, John W. Linville wrote:
> On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
>>
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> 
> Acked-by: John W. Linville <linville@tuxdriver.com>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH v2 09/13] PMD/af_packet: guard against buffer overruns in TX path
From: Ferruh Yigit @ 2016-12-16 10:32 UTC (permalink / raw)
  To: John W. Linville, Michał Mirosław; +Cc: dev
In-Reply-To: <20161213160605.GB27914@tuxdriver.com>

On 12/13/2016 4:06 PM, John W. Linville wrote:
> On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> 
> Acked-by: John W. Linville <linville@tuxdriver.com>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Jan Blunck @ 2016-12-16 10:47 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
	Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <20161216092306.GD10340@6wind.com>

On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
<adrien.mazarguil@6wind.com> wrote:
> On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
>> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>> >>
>> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> >> wrote:
>> >>>
>> >>> From: Jan Blunck <jblunck@infradead.org>
>> >>>
>> >>> This macro is based on Jan Viktorin's original patch but also checks the
>> >>> type of the passed pointer against the type of the member.
>> >>>
>> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> >>> [jblunck@infradead.org: add type checking and __extension__]
>> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>> >>>
>> >>> --
>> >>> v2:
>> >>>  - fix checkpatch error
>> >>> ---
>> >>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>> >>>  1 file changed, 21 insertions(+)
>> >>>
>> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
>> >>> b/lib/librte_eal/common/include/rte_common.h
>> >>> index db5ac91..3eb8d11 100644
>> >>> --- a/lib/librte_eal/common/include/rte_common.h
>> >>> +++ b/lib/librte_eal/common/include/rte_common.h
>> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>> >>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>> >>>  #endif
>> >>>
>> >>> +/**
>> >>> + * Return pointer to the wrapping struct instance.
>> >>> + *
>> >>> + * Example:
>> >>> + *
>> >>> + *  struct wrapper {
>> >>> + *      ...
>> >>> + *      struct child c;
>> >>> + *      ...
>> >>> + *  };
>> >>> + *
>> >>> + *  struct child *x = obtain(...);
>> >>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>> >>> + */
>> >>> +#ifndef container_of
>> >>> +#define container_of(ptr, type, member)        (__extension__  ({
>> >>> \
>> >>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
>> >>> +                       (type *)(((char *)_ptr) - offsetof(type,
>> >>> member));\
>> >>> +                       }))
>> >>
>> >>
>> >> This is a checkpatch false positive. It should be fine to ignore this.
>> >> IIRC we already discussed this before.
>> >
>> >
>> > I too thought something similar was discussed. I tried searching the
>> > archives but couldn't find anything - thus, I thought probably I was
>> > hallucinating :P
>> >
>> > So, you want me to revert back the '()' change? Does it impact the expansion
>> > of this macro?
>>
>> We haven't added this on any other usage of the __extension__ keyword
>> in the existing code. From my perspective it is more consistent to
>> revert it.
>>
>> Anyone else with an opinion here? David? Thomas?
>
> As an exported header, rte_common.h must pass check-includes.sh. Both
> typeof() and the ({ ... }) construct are non-standard GCC extensions and
> would fail to compile with pedantic options.
>

Thanks Adrien. These extensions are already in use by rte_common.h and
other headers. I don't believe we can remove the usage of typeof()
that easily without making the code really ugly.

^ permalink raw reply

* Re: [PATCH v2] crypto/qat: fix to avoid buffer overwrite in OOP case
From: Griffin, John @ 2016-12-16 11:20 UTC (permalink / raw)
  To: Trahe, Fiona, dev@dpdk.org
  Cc: De Lara Guarch, Pablo, Trahe, Fiona, stable@dpdk.org
In-Reply-To: <1481297945-20649-1-git-send-email-fiona.trahe@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Friday, December 9, 2016 3:39 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] crypto/qat: fix to avoid buffer overwrite in
> OOP case
> 
> In out-of-place operation, data is DMAed from source mbuf to destination
> mbuf. To avoid header data in dest mbuf being overwritten, the minimal data-
> set should be DMAed.
> 
> Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> performance")
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
Acked-by: John Griffin <john.griffin@intel.com>

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16 11:21 UTC (permalink / raw)
  To: Jan Blunck
  Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
	Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02WkXzbz+wA=ysvjf5SvU2bMCEY0vHTm0oGxXG=Jhs34Q@mail.gmail.com>

On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
> <adrien.mazarguil@6wind.com> wrote:
> > On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> >> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >> >>
> >> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >> wrote:
> >> >>>
> >> >>> From: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >> >>> type of the passed pointer against the type of the member.
> >> >>>
> >> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >>> [jblunck@infradead.org: add type checking and __extension__]
> >> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> --
> >> >>> v2:
> >> >>>  - fix checkpatch error
> >> >>> ---
> >> >>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >> >>>  1 file changed, 21 insertions(+)
> >> >>>
> >> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >> >>> b/lib/librte_eal/common/include/rte_common.h
> >> >>> index db5ac91..3eb8d11 100644
> >> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >> >>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
> >> >>>  #endif
> >> >>>
> >> >>> +/**
> >> >>> + * Return pointer to the wrapping struct instance.
> >> >>> + *
> >> >>> + * Example:
> >> >>> + *
> >> >>> + *  struct wrapper {
> >> >>> + *      ...
> >> >>> + *      struct child c;
> >> >>> + *      ...
> >> >>> + *  };
> >> >>> + *
> >> >>> + *  struct child *x = obtain(...);
> >> >>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
> >> >>> + */
> >> >>> +#ifndef container_of
> >> >>> +#define container_of(ptr, type, member)        (__extension__  ({
> >> >>> \
> >> >>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
> >> >>> +                       (type *)(((char *)_ptr) - offsetof(type,
> >> >>> member));\
> >> >>> +                       }))
> >> >>
> >> >>
> >> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> >> IIRC we already discussed this before.
> >> >
> >> >
> >> > I too thought something similar was discussed. I tried searching the
> >> > archives but couldn't find anything - thus, I thought probably I was
> >> > hallucinating :P
> >> >
> >> > So, you want me to revert back the '()' change? Does it impact the expansion
> >> > of this macro?
> >>
> >> We haven't added this on any other usage of the __extension__ keyword
> >> in the existing code. From my perspective it is more consistent to
> >> revert it.
> >>
> >> Anyone else with an opinion here? David? Thomas?
> >
> > As an exported header, rte_common.h must pass check-includes.sh. Both
> > typeof() and the ({ ... }) construct are non-standard GCC extensions and
> > would fail to compile with pedantic options.
> >
> 
> Thanks Adrien. These extensions are already in use by rte_common.h and
> other headers. I don't believe we can remove the usage of typeof()
> that easily without making the code really ugly.

Sure, no problem with that, I misread and thought you wanted to drop
__extension__ as well.

Parentheses may perhaps cause more accurate warnings in case of syntax
errors. That is not significant so either way is fine by me.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox