* Re: [PATCH 3/3] net/mlx5: do not invalidate title CQE
From: Ferruh Yigit @ 2016-11-18 9:55 UTC (permalink / raw)
To: Adrien Mazarguil, Nelio Laranjeiro; +Cc: dev
In-Reply-To: <20161117103830.GC4729@6wind.com>
On 11/17/2016 10:38 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:56AM +0100, Nelio Laranjeiro wrote:
>> We can leave the title completion queue entry untouched since its contents
>> are not modified.
>>
>> Reported-by: Liming Sun <lsun@mellanox.com>
>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
<...>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH 01/22] ethdev: introduce generic flow API
From: Adrien Mazarguil @ 2016-11-18 10:28 UTC (permalink / raw)
To: Xing, Beilei
Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
Olivier Matz
In-Reply-To: <94479800C636CB44BD422CB454846E012EA1C37D@SHSMSX101.ccr.corp.intel.com>
Hi Beilei,
On Fri, Nov 18, 2016 at 06:36:31AM +0000, Xing, Beilei wrote:
> Hi Adrien,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Thursday, November 17, 2016 12:23 AM
> > To: dev@dpdk.org
> > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; De Lara Guarch,
> > Pablo <pablo.de.lara.guarch@intel.com>; Olivier Matz
> > <olivier.matz@6wind.com>
> > Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
> >
> > This new API supersedes all the legacy filter types described in rte_eth_ctrl.h.
> > It is slightly higher level and as a result relies more on PMDs to process and
> > validate flow rules.
> >
> > Benefits:
> >
> > - A unified API is easier to program for, applications do not have to be
> > written for a specific filter type which may or may not be supported by
> > the underlying device.
> >
> > - The behavior of a flow rule is the same regardless of the underlying
> > device, applications do not need to be aware of hardware quirks.
> >
> > - Extensible by design, API/ABI breakage should rarely occur if at all.
> >
> > - Documentation is self-standing, no need to look up elsewhere.
> >
> > Existing filter types will be deprecated and removed in the near future.
> >
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
>
> > +
> > +/**
> > + * Opaque type returned after successfully creating a flow.
> > + *
> > + * This handle can be used to manage and query the related flow (e.g.
> > +to
> > + * destroy it or retrieve counters).
> > + */
> > +struct rte_flow;
> > +
>
> As we talked before, we use attr/pattern/actions to create and destroy a flow in PMD,
> but I don't think it's easy to clone the user-provided parameters and return the result
> to the application as a rte_flow pointer. As you suggested:
> /* PMD-specific code. */
> struct rte_flow {
> struct rte_flow_attr attr;
> struct rte_flow_item *pattern;
> struct rte_flow_action *actions;
> };
Just to provide some context to the community since the above snippet comes
from private exchanges, I've suggested the above structure as a mean to
create and remove rules in the same fashion as FDIR, by providing the rule
used for creation to the destroy callback.
As an opaque type, each PMD currently needs to implement its own version of
struct rte_flow. The above definition may ease transition from FDIR to
rte_flow for some PMDs, however they need to clone the entire
application-provided rule to do so because there is no requirement for it to
be kept allocated.
I've implemented such a function in testpmd (port_flow_new() in commit [1])
as an example.
[1] http://dpdk.org/ml/archives/dev/2016-November/050266.html
However my suggestion is for PMDs to use their own HW-specific structure
that only contains relevant information instead of being forced to drag
large, non-native data around, missing useful context and that requires
parsing every time. This is one benefit of using an opaque type in the first
place, the other being ABI breakage avoidance.
> Because both pattern and actions are pointers, and there're also pointers in structure
> rte_flow_item and struct rte_flow_action. We need to iterate allocation during clone
> and iterate free during destroy, then seems that the code is something ugly, right?
Well since I wrote that code, I won't easily admit it's ugly. I think PMDs
should not require the duplication of generic rules actually, which are only
defined as a common language between applications and PMDs. Both are free to
store rules in their own preferred and efficient format internally.
> I think application saves info when creating a flow rule, so why not application provide
> attr/pattern/actions info to PMD before calling PMD API?
They have to do so temporarily (e.g. allocated on the stack) while calling
rte_flow_create() and rte_flow_validate(), that's it. Once a rule is
created, there's no requirement for applications to keep anything around.
For simple applications such as testpmd, the generic format is probably
enough. More complex and existing applications such as ovs-dpdk may rather
choose to keep using their internal format that already fits their needs,
partially duplicating this information in rte_flow_attr and
rte_flow_item/rte_flow_action lists would waste memory. The conversion in
this case should only be performed when creating/validating flow rules.
In short, I fail to see any downside with maintaining struct rte_flow opaque
to applications.
Best regards,
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: pmdinfogen issues: cross compilation for ARM fails with older host compiler
From: Hemant Agrawal @ 2016-11-18 12:03 UTC (permalink / raw)
To: Neil Horman
Cc: Jerin Jacob, dev@dpdk.org, users@dpdk.org, Jacob, Jerin,
Jan Viktorin
In-Reply-To: <20161115150800.GB11283@hmsreliant.think-freely.org>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote:
> > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote:
> > > > > Hi Neil,
> > > > > Pmdinfogen compiles with host compiler. It usages
> rte_byteorder.h
> > > of the target platform.
> > > > > However, if the host compiler is older than 4.8, it will be an issue during
> cross
> > > compilation for some platforms.
> > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not
> > > understand the arm asm instructions.
> > > > >
> > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > register uint16_t x = _x;
> > > > > asm volatile ("rev16 %0,%1"
> > > > > : "=r" (x)
> > > > > : "r" (x)
> > > > > );
> > > > > return x;
> > > > > }
> > > > > #endif
> > > > >
> > > > > One easy solution is that we add compiler platform check in this
> > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined
> > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else ….
> > > > >
> > > > > Is there a better way to fix it?
> > > >
> > > > IMO, It is a HOST build infrastructure issue. If a host app is using
> > > > the dpdk service then it should compile and link against HOST
> > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > think, introducing the HOSTTARGET kind of scheme is a clean solution.
> > > >
> > > > /Jerin
> > > >
> > > >
> > > That would be accurate. That is to say, pmdinfogen is a tool that should only
> be
> > > run on the host doing the build, by the host doing the build, and so should be
> > > compiled to run on the host, not on the target being built for.
> > >
> > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h
> > > when building in a cross environment
> > >
> > +1
> >
> > > Neil
> >
>
> Give this a try, I've tested it on linux, but not BSD. From what I read the
> functions are not posix compliant, though they should exist on all BSD and Linux
> systems in recent history. There may be some fiddling needed for Net and
> OpenBSD variants, but I think this is the right general direction.
+ 1
This patch works good for Linux.
>
>
> diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> b/buildtools/pmdinfogen/pmdinfogen.h
> index 1da2966..c5ef89d 100644
> --- a/buildtools/pmdinfogen/pmdinfogen.h
> +++ b/buildtools/pmdinfogen/pmdinfogen.h
> @@ -21,7 +21,6 @@
> #include <elf.h>
> #include <rte_config.h>
> #include <rte_pci.h>
> -#include <rte_byteorder.h>
>
> /* On BSD-alike OSes elf.h defines these according to host's word size */
> #undef ELF_ST_BIND
> @@ -75,9 +74,9 @@
> #define CONVERT_NATIVE(fend, width, x) ({ \
> typeof(x) ___x; \
> if ((fend) == ELFDATA2LSB) \
> - ___x = rte_le_to_cpu_##width(x); \
> + ___x = le##width##toh(x); \
> else \
> - ___x = rte_be_to_cpu_##width(x); \
> + ___x = be##width##toh(x); \
> ___x; \
> })
>
^ permalink raw reply
* Clarification for - SoC specific driver based common sub component placing
From: Hemant Agrawal @ 2016-11-18 12:44 UTC (permalink / raw)
To: dev@dpdk.org; +Cc: Thomas Monjalon
Hi all,
We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) Poll mode drivers into the DPDK.
We need some clarification w.r.t the right placing of some dependent components, which can be common across drivers. E.g. We have hardware queue and buffer manager driver. This will be used by both network driver and crypto driver. But it is specific to NXP platform only.
What is the right place for such common hardware specific components in DPDK?
1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For each soc configuration only the required components will be compiled-in.
2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. And link the network and crypto drivers to it.
3. Add it to main network driver and make the crypto driver dependent on it.
Regards,
Hemant
^ permalink raw reply
* [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
From: Satha Rao @ 2016-11-18 12:52 UTC (permalink / raw)
To: helin.zhang; +Cc: jingjing.wu, jerin.jacob, jianbo.liu, dev, Satha Rao
i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
results. To solve this include rte memory barriers
Signed-off-by: Satha Rao <skoteshwar@caviumnetworks.com>
---
drivers/net/i40e/base/i40e_osdep.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
index 38e7ba5..ffa3160 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -158,7 +158,13 @@ do { \
((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
static inline uint32_t i40e_read_addr(volatile void *addr)
{
+#if defined(RTE_ARCH_ARM64)
+ uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
+ rte_rmb();
+ return val;
+#else
return rte_le_to_cpu_32(I40E_PCI_REG(addr));
+#endif
}
#define I40E_PCI_REG_WRITE(reg, value) \
do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
@@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void *addr)
I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value))
#define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg)))
+#if defined(RTE_ARCH_ARM64)
+#define wr32(a, reg, value) \
+ do { \
+ I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \
+ rte_wmb(); \
+ } while (0)
+#else
#define wr32(a, reg, value) \
I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value))
+#endif
#define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT)))
#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH 6/6] eal: removing eth_driver
From: Shreyansh Jain @ 2016-11-18 13:05 UTC (permalink / raw)
To: Jan Blunck; +Cc: David Marchand, dev
In-Reply-To: <CALe+Z00y8VDfUFS2gkxkW=VJN0YofjdXpq9WcDJcxmuKFgVaQg@mail.gmail.com>
sorry for delay in responding; somehow I didn't notice this email.
On Thursday 17 November 2016 06:23 PM, Jan Blunck wrote:
> On Thu, Nov 17, 2016 at 6:30 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> This patch demonstrates how eth_driver can be replaced with appropriate
>> changes for rte_xxx_driver from the PMD itself. It uses ixgbe_ethernet as
>> an example.
>>
>> A large set of changes exists in the rte_ethdev.c - primarily because too
>> much PCI centric code (names, assumption of rte_pci_device) still exists
>> in it. Most, except symbol naming, has been changed in this patch.
>>
>> This proposes that:
>> - PMD would declare the rte_xxx_driver. In case of ixgbe, it would be
>> rte_pci_driver.
>> - Probe and remove continue to exists in rte_pci_driver. But, the
>> rte_driver has new hooks for init and uninit. The rationale is that
>> once a ethernet or cryto device is created, the rte_driver->init would
>> be responsible for initializing the device.
>> -- Eth_dev -> rte_driver -> rte_pci_driver
>> | `-> probe/remove
>> `--> init/uninit
>
> Hmm, from my perspective this moves struct rte_driver a step closer to
> struct rte_eth_dev instead of decoupling them. It is up to the
> rte_driver->probe if it wants to allocate a struct rte_eth_dev,
> rte_crypto_dev or the famous rte_foo_dev.
That 'closeness' was my intention - to make rte_eth_dev an
implementation of rte_device type.
rte_eth_dev == rte_cryptodev == rte_anyother_functional_device
- for the above context. All would include rte_device.
As for rte_driver->probe(), it still comes in the rte_driver->init()'s
role to initialize the 'generic' functional device associated with the
driver. And, allowing bus specific driver (like PCI) for its individual
initialization using rte_xxx_driver->probe.
>
> Instead of explicitly modelling rte_eth_dev specifics like init, unit
> or dev_private_size I think we should delegate this to the
> rte_driver->probe instead. Most of what is in rte_eth_dev_pci_probe()
> today is anyway a rte_eth_dev_allocate_priv() anyway. I already have
> some patches in this area in my patch stack.
Can be done - either way rte_pci_driver->probe() ends up calling
driver->init() (or erstwhile eth_driver->eth_dev_init()).
But, I still think it is better to keep them separate.
A PCI device is type of rte_device, physically.
A ethernet device is type of rte_device, logically.
They both should exist independently. It will help in splitting the
functionality from physical layout in future - if need be.
>
>
>> - necessary changes in the rte_eth_dev have also been done so that it
>> refers to the rte_device and rte_driver rather than rte_xxx_*. This
>> would imply, ethernet device is 'linked' to a rte_device/rte_driver
>> which in turn is a rte_xxx_device/rte_xxx_driver type.
>> - for all operations related to extraction relvant xxx type,
>> container_of would have to be used.
>>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>> drivers/net/ixgbe/ixgbe_ethdev.c | 49 +++++++++++++++++++++-------------------
>> lib/librte_ether/rte_ethdev.c | 36 +++++++++++++++++------------
>> lib/librte_ether/rte_ethdev.h | 6 ++---
>> 3 files changed, 51 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index edc9b22..acead31 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1419,7 +1419,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>> return 0;
>> }
>>
>> - pci_dev = eth_dev->pci_dev;
>> + pci_dev = container_of(eth_dev->device, struct rte_pci_device, device);
>>
>> rte_eth_copy_pci_info(eth_dev, pci_dev);
>>
>> @@ -1532,7 +1532,9 @@ static int
>> eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>> {
>> struct ixgbe_hw *hw;
>> - struct rte_pci_device *pci_dev = eth_dev->pci_dev;
>> + struct rte_pci_device *pci_dev;
>> +
>> + pci_dev = container_of(eth_dev->device, struct rte_pci_device, device);
>>
>> PMD_INIT_FUNC_TRACE();
>>
>> @@ -1562,32 +1564,33 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>> return 0;
>> }
>>
>> -static struct eth_driver rte_ixgbe_pmd = {
>> - .pci_drv = {
>> - .id_table = pci_id_ixgbe_map,
>> - .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
>> - RTE_PCI_DRV_DETACHABLE,
>> - .probe = rte_eth_dev_pci_probe,
>> - .remove = rte_eth_dev_pci_remove,
>> +static struct rte_pci_driver rte_ixgbe_pci_driver = {
>> + .id_table = pci_id_ixgbe_map,
>> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
>> + RTE_PCI_DRV_DETACHABLE,
>> + .probe = rte_eth_dev_pci_probe,
>> + .remove = rte_eth_dev_pci_remove,
>> + .driver = {
>> + .driver_init_t= eth_ixgbe_dev_init,
>> + .driver_uninit_t= eth_ixgbe_dev_uninit,
>> + .dev_private_size = sizeof(struct ixgbe_adapter),
>> },
>> - .eth_dev_init = eth_ixgbe_dev_init,
>> - .eth_dev_uninit = eth_ixgbe_dev_uninit,
>> - .dev_private_size = sizeof(struct ixgbe_adapter),
>> };
>>
>> /*
>> * virtual function driver struct
>> */
>> -static struct eth_driver rte_ixgbevf_pmd = {
>> - .pci_drv = {
>> - .id_table = pci_id_ixgbevf_map,
>> - .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>> - .probe = rte_eth_dev_pci_probe,
>> - .remove = rte_eth_dev_pci_remove,
>> +static struct rte_pci_driver rte_ixgbevf_pci_driver = {
>> + .id_table = pci_id_ixgbevf_map,
>> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>> + .probe = rte_eth_dev_pci_probe,
>> + .remove = rte_eth_dev_pci_remove,
>> + .driver = {
>> + /* rte_driver hooks */
>> + .init = eth_ixgbevf_dev_init,
>> + .uninit = eth_ixgbevf_dev_uninit,
>> + .dev_private_size = sizeof(struct ixgbe_adapter),
>> },
>> - .eth_dev_init = eth_ixgbevf_dev_init,
>> - .eth_dev_uninit = eth_ixgbevf_dev_uninit,
>> - .dev_private_size = sizeof(struct ixgbe_adapter),
>> };
>>
>> static int
>> @@ -7592,7 +7595,7 @@ ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
>> ixgbevf_dev_interrupt_action(dev);
>> }
>>
>> -RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
>> +RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pci_driver);
>> RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
>> -RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd.pci_drv);
>> +RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pci_driver);
>> RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>> index fde8112..3535ff4 100644
>> --- a/lib/librte_ether/rte_ethdev.c
>> +++ b/lib/librte_ether/rte_ethdev.c
>> @@ -235,13 +235,13 @@ int
>> rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>> struct rte_pci_device *pci_dev)
>> {
>> - struct eth_driver *eth_drv;
>> + struct rte_driver *drv;
>> struct rte_eth_dev *eth_dev;
>> char ethdev_name[RTE_ETH_NAME_MAX_LEN];
>>
>> int diag;
>>
>> - eth_drv = (struct eth_driver *)pci_drv;
>> + drv = pci_drv->driver;
>>
>> rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
>> sizeof(ethdev_name));
>> @@ -252,13 +252,13 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>>
>> if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>> eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
>> - eth_drv->dev_private_size,
>> + drv->dev_private_size,
>> RTE_CACHE_LINE_SIZE);
>> if (eth_dev->data->dev_private == NULL)
>> rte_panic("Cannot allocate memzone for private port data\n");
>> }
>> - eth_dev->pci_dev = pci_dev;
>> - eth_dev->driver = eth_drv;
>> + eth_dev->device = pci_dev->device;
>> + eth_dev->driver = drv;
>> eth_dev->data->rx_mbuf_alloc_failed = 0;
>>
>> /* init user callbacks */
>> @@ -270,7 +270,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>> eth_dev->data->mtu = ETHER_MTU;
>>
>> /* Invoke PMD device initialization function */
>> - diag = (*eth_drv->eth_dev_init)(eth_dev);
>> + diag = (*drv->init)(eth_dev);
>> if (diag == 0)
>> return 0;
>>
>> @@ -287,7 +287,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>> int
>> rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
>> {
>> - const struct eth_driver *eth_drv;
>> + const struct rte_driver *drv;
>> struct rte_eth_dev *eth_dev;
>> char ethdev_name[RTE_ETH_NAME_MAX_LEN];
>> int ret;
>> @@ -302,11 +302,11 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
>> if (eth_dev == NULL)
>> return -ENODEV;
>>
>> - eth_drv = (const struct eth_driver *)pci_dev->driver;
>> + drv = pci_dev->driver;
>>
>> /* Invoke PMD device uninit function */
>> - if (*eth_drv->eth_dev_uninit) {
>> - ret = (*eth_drv->eth_dev_uninit)(eth_dev);
>> + if (*drv->uninit) {
>> + ret = (*drv->uninit)(eth_dev);
>> if (ret)
>> return ret;
>> }
>> @@ -317,7 +317,7 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
>> if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>> rte_free(eth_dev->data->dev_private);
>>
>> - eth_dev->pci_dev = NULL;
>> + eth_dev->device = NULL;
>> eth_dev->driver = NULL;
>> eth_dev->data = NULL;
>>
>> @@ -1556,7 +1556,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
>>
>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>> - dev_info->pci_dev = dev->pci_dev;
>> + dev_info->device = dev->device;
>> dev_info->driver_name = dev->data->drv_name;
>> dev_info->nb_rx_queues = dev->data->nb_rx_queues;
>> dev_info->nb_tx_queues = dev->data->nb_tx_queues;
>> @@ -2537,6 +2537,7 @@ rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
>> {
>> uint32_t vec;
>> struct rte_eth_dev *dev;
>> + struct rte_pci_device *pci_dev;
>> struct rte_intr_handle *intr_handle;
>> uint16_t qid;
>> int rc;
>> @@ -2544,6 +2545,10 @@ rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>
>> dev = &rte_eth_devices[port_id];
>> + /* TODO intr_handle is currently in rte_pci_device;
>> + * Below is incorrect until that time
>> + */
>> + pci_dev = container_of(dev->device, struct rte_pci_device, device);
>> intr_handle = &dev->pci_dev->intr_handle;
>> if (!intr_handle->intr_vec) {
>> RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>> @@ -2572,7 +2577,7 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>> const struct rte_memzone *mz;
>>
>> snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>> - dev->driver->pci_drv.driver.name, ring_name,
>> + dev->driver->name, ring_name,
>> dev->data->port_id, queue_id);
>>
>> mz = rte_memzone_lookup(z_name);
>> @@ -2593,6 +2598,7 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
>> {
>> uint32_t vec;
>> struct rte_eth_dev *dev;
>> + struct rte_pci_device *pci_dev;
>> struct rte_intr_handle *intr_handle;
>> int rc;
>>
>> @@ -2604,7 +2610,9 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
>> return -EINVAL;
>> }
>>
>> - intr_handle = &dev->pci_dev->intr_handle;
>> + /* TODO; Until intr_handle is available in rte_device, below is incorrect */
>> + pci_dev = container_of(dev->device, struct rte_pci_device, device);
>> + intr_handle = &pci_dev->intr_handle;
>> if (!intr_handle->intr_vec) {
>> RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>> return -EPERM;
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index 38641e8..2b1d826 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -876,7 +876,7 @@ struct rte_eth_conf {
>> * Ethernet device information
>> */
>> struct rte_eth_dev_info {
>> - struct rte_pci_device *pci_dev; /**< Device PCI information. */
>> + struct rte_device *device; /**< Device PCI information. */
>
> We already the situation that virtual devices don't set the pci_dev
> field. I wonder if it really makes sense to replace it with a struct
> rte_device because that is not adding a lot of value (only numa_node).
Sorry, I couldn't understand which way you are pointing:
- continuing with 'rte_pci_device' in rte_eth_dev_info.
- completely removing both, rte_pci_device and rte_device
In either case, I am ok. I went through the code usage of
rte_eth_dev_info and it is mostly being used for getting information. I
couldn't point out a situation where based on available info
(rte_eth_dev_info), we need to extract back the device it is
representing. Is that understanding correct?
If yes, I can remove this (after checking that this member is not being
used).
> If we break ABI we might want to add numa_node and dev_flags as
> suggested by Stephen Hemminger already. If we choose to not break ABI
> we can delegate the population of the pci_dev field to dev_infos_get.
> I already have that patch in my patch stack too.
We can't avoid the ABI breakage - it is anyway going to happen.
As for 'dev_flags', I am assuming you are referring to moving
'drv_flags' from rte_pci_driver. And you are suggesting moving that to
'rte_driver' - is that correct understanding?
I don't know if drv_flags have any significance in rte_device. I thought
they are driver specific flags (mmap, etc). Or, maybe they are just
placed in driver for acting on all compatible devices.
>
> The problem with rte_eth_dev_info is that it doesn't support
> extensions. Maybe its time to add rte_eth_dev_info_ex() ... or
> rte_eth_dev_xinfo() if you don't like the IB verbs API.
I have no idea about IB verbs. And as for extensions, I will have to see
- I don't prefer mixing that with current set. Though, the idea is nice.
>
>
>> const char *driver_name; /**< Device Driver name. */
>> unsigned int if_index; /**< Index to bound host interface, or 0 if none.
>> Use if_indextoname() to translate into an interface name. */
>> @@ -1623,9 +1623,9 @@ struct rte_eth_dev {
>> eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
>> eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
>> struct rte_eth_dev_data *data; /**< Pointer to device data */
>> - const struct eth_driver *driver;/**< Driver for this device */
>> + const struct rte_driver *driver;/**< Driver for this device */
>> const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
>> - struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */
>> + struct rte_device *device; /**< Device instance */
>> /** User application callbacks for NIC interrupts */
>> struct rte_eth_dev_cb_list link_intr_cbs;
>> /**
>> --
>> 2.7.4
>>
>
-
Shreyansh
^ permalink raw reply
* Re: Clarification for - SoC specific driver based common sub component placing
From: Thomas Monjalon @ 2016-11-18 13:38 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev
In-Reply-To: <DB5PR04MB1605C793753BB0AF410C60D789B00@DB5PR04MB1605.eurprd04.prod.outlook.com>
2016-11-18 12:44, Hemant Agrawal:
> We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) Poll mode drivers into the DPDK.
>
> We need some clarification w.r.t the right placing of some dependent components, which can be common across drivers. E.g. We have hardware queue and buffer manager driver. This will be used by both network driver and crypto driver. But it is specific to NXP platform only.
>
> What is the right place for such common hardware specific components in DPDK?
> 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For each soc configuration only the required components will be compiled-in.
> 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. And link the network and crypto drivers to it.
> 3. Add it to main network driver and make the crypto driver dependent on it.
Your question is more generic than SoC context.
You just want to share some code between drivers, right?
What about building a library located in drivers/common/nxp/ ?
^ permalink raw reply
* Re: Clarification for - SoC specific driver based common sub component placing
From: Thomas Monjalon @ 2016-11-18 13:43 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev
In-Reply-To: <7765421.y4BZ5iUgMg@xps13>
2016-11-18 14:38, Thomas Monjalon:
> 2016-11-18 12:44, Hemant Agrawal:
> > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) Poll mode drivers into the DPDK.
> >
> > We need some clarification w.r.t the right placing of some dependent components, which can be common across drivers. E.g. We have hardware queue and buffer manager driver. This will be used by both network driver and crypto driver. But it is specific to NXP platform only.
> >
> > What is the right place for such common hardware specific components in DPDK?
> > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For each soc configuration only the required components will be compiled-in.
> > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. And link the network and crypto drivers to it.
> > 3. Add it to main network driver and make the crypto driver dependent on it.
>
> Your question is more generic than SoC context.
> You just want to share some code between drivers, right?
> What about building a library located in drivers/common/nxp/ ?
I'm a bit reluctant to have company name in file hierarchy,
as it not something stable. And especially for NXP/Qualcomm...
In this case would it be better to name the directory
drivers/common/dpaa2/ ?
^ permalink raw reply
* Re: pmdinfogen issues: cross compilation for ARM fails with older host compiler
From: Neil Horman @ 2016-11-18 13:50 UTC (permalink / raw)
To: Hemant Agrawal
Cc: Jerin Jacob, dev@dpdk.org, users@dpdk.org, Jacob, Jerin,
Jan Viktorin
In-Reply-To: <DB5PR04MB1605E3C25DF89568176D668C89B00@DB5PR04MB1605.eurprd04.prod.outlook.com>
On Fri, Nov 18, 2016 at 12:03:19PM +0000, Hemant Agrawal wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > On Tue, Nov 15, 2016 at 09:34:16AM +0000, Hemant Agrawal wrote:
> > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > > On Fri, Nov 11, 2016 at 10:34:39AM +0000, Hemant Agrawal wrote:
> > > > > > Hi Neil,
> > > > > > Pmdinfogen compiles with host compiler. It usages
> > rte_byteorder.h
> > > > of the target platform.
> > > > > > However, if the host compiler is older than 4.8, it will be an issue during
> > cross
> > > > compilation for some platforms.
> > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will not
> > > > understand the arm asm instructions.
> > > > > >
> > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > > register uint16_t x = _x;
> > > > > > asm volatile ("rev16 %0,%1"
> > > > > > : "=r" (x)
> > > > > > : "r" (x)
> > > > > > );
> > > > > > return x;
> > > > > > }
> > > > > > #endif
> > > > > >
> > > > > > One easy solution is that we add compiler platform check in this
> > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined
> > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > > return (_x >> 8) | ((_x << 8) & 0xff00); } #else ….
> > > > > >
> > > > > > Is there a better way to fix it?
> > > > >
> > > > > IMO, It is a HOST build infrastructure issue. If a host app is using
> > > > > the dpdk service then it should compile and link against HOST
> > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution.
> > > > >
> > > > > /Jerin
> > > > >
> > > > >
> > > > That would be accurate. That is to say, pmdinfogen is a tool that should only
> > be
> > > > run on the host doing the build, by the host doing the build, and so should be
> > > > compiled to run on the host, not on the target being built for.
> > > >
> > > > Yeah, so what we need is a way to get to the host version of rte_byteorder.h
> > > > when building in a cross environment
> > > >
> > > +1
> > >
> > > > Neil
> > >
> >
> > Give this a try, I've tested it on linux, but not BSD. From what I read the
> > functions are not posix compliant, though they should exist on all BSD and Linux
> > systems in recent history. There may be some fiddling needed for Net and
> > OpenBSD variants, but I think this is the right general direction.
>
> + 1
> This patch works good for Linux.
>
Can someone test it on BSD? I'd like to ensure we don't need to modify it for
that platform
Neil
> >
> >
> > diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> > b/buildtools/pmdinfogen/pmdinfogen.h
> > index 1da2966..c5ef89d 100644
> > --- a/buildtools/pmdinfogen/pmdinfogen.h
> > +++ b/buildtools/pmdinfogen/pmdinfogen.h
> > @@ -21,7 +21,6 @@
> > #include <elf.h>
> > #include <rte_config.h>
> > #include <rte_pci.h>
> > -#include <rte_byteorder.h>
> >
> > /* On BSD-alike OSes elf.h defines these according to host's word size */
> > #undef ELF_ST_BIND
> > @@ -75,9 +74,9 @@
> > #define CONVERT_NATIVE(fend, width, x) ({ \
> > typeof(x) ___x; \
> > if ((fend) == ELFDATA2LSB) \
> > - ___x = rte_le_to_cpu_##width(x); \
> > + ___x = le##width##toh(x); \
> > else \
> > - ___x = rte_be_to_cpu_##width(x); \
> > + ___x = be##width##toh(x); \
> > ___x; \
> > })
> >
^ permalink raw reply
* Re: [PATCH] lib/librte_mempool: a redundant of socket_id assignment
From: Olivier Matz @ 2016-11-18 14:07 UTC (permalink / raw)
To: Mcnamara, John, Zhao1, Wei, dev@dpdk.org
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20264E206@IRSMSX103.ger.corp.intel.com>
Hi Wei,
On 11/14/2016 11:25 AM, Mcnamara, John wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
>> Sent: Monday, November 14, 2016 2:16 AM
>> To: dev@dpdk.org
>> Cc: olivier.matz@6wind.com; Zhao1, Wei <wei.zhao1@intel.com>
>> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant of socket_id
>> assignment
>>
>> From: zhao wei <wei.zhao1@intel.com>
>>
>> There is a redundant repetition mempool socket_id assignment in the file
>> rte_mempool.c in function rte_mempool_create_empty.The statement "mp-
>>> socket_id = socket_id;"appear twice in line 821 and 824.One of them is
>> redundant, so delete it.
>>
>> Fixes: 85226f9c526b ("lib/librte_mempool: mempool:introduce a function to
>> create an empty pool")
>>
>> Signed-off-by: zhao wei <wei.zhao1@intel.com>
>
> Titles should generally start with a verb to indicate what is being done.
> Something like:
>
> lib/librte_mempool: remove redundant socket_id assignment
>
> Apart from that.
>
> Acked-by: John McNamara <john.mcnamara@intel.com>
I would even say:
mempool: remove redundant socket_id assignment
Acked-by: Olivier Matz <olivier.matz@6wind.com>
^ permalink raw reply
* Re: [PATCH] lib/librte_mempool: a redundant word in comment
From: Olivier Matz @ 2016-11-18 14:15 UTC (permalink / raw)
To: Zhao1, Wei, Mcnamara, John, dev@dpdk.org
In-Reply-To: <A2573D2ACFCADC41BB3BE09C6DE313CA01FECD4D@PGSMSX103.gar.corp.intel.com>
Hi Wei,
2lOn 11/15/2016 07:54 AM, Zhao1, Wei wrote:
> Hi, john
>
>> -----Original Message-----
>> From: Mcnamara, John
>> Sent: Monday, November 14, 2016 6:30 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Cc: olivier.matz@6wind.com; Zhao1, Wei <wei.zhao1@intel.com>
>> Subject: RE: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
>> comment
>>
>>
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
>>> Sent: Monday, November 14, 2016 2:47 AM
>>> To: dev@dpdk.org
>>> Cc: olivier.matz@6wind.com; Zhao1, Wei <wei.zhao1@intel.com>
>>> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
>>> comment
>>>
>>> From: zhao wei <wei.zhao1@intel.com>
>>
>> I think you need to add your name to gitconfig file on the sending machine to
>> avoid this "From:"
>>
>>>
>>> There is a redundant repetition word "for" in commnet line the file
>>> rte_mempool.h after the definition of RTE_MEMPOOL_OPS_NAMESIZE.
>>> The word "for"appear twice in line 359 and 360.One of them is
>>> redundant, so delete it.
>>>
>>> Fixes: 449c49b93a6b ("lib/librte_mempool: mempool: support handler
>>> operations")
The proper fixline should be:
Fixes: 449c49b93a6b ("mempool: support handler operations")
(no need to add "lib/librte_mempool:")
This comment also applies to the other patch, I missed it.
>>>
>>> Signed-off-by: zhao wei <wei.zhao1@intel.com>
>>
>> /commnet/comment/
>>
>> And same comment as before about the title. Apart from that:
>>
>> Acked-by: John McNamara <john.mcnamara@intel.com>
>>
>>
>
> Thank you for your suggestion, I will change as your comment in following patch!
>
Also same comment about "mempool:" instead of "lib/librte_mempool: mempool:"
Thanks,
Olivier
^ permalink raw reply
* Re: Clarification for - SoC specific driver based common sub component placing
From: Hemant Agrawal @ 2016-11-18 14:31 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org
In-Reply-To: <13846356.5jcoesaunv@xps13>
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, November 18, 2016 7:13 PM
> To: Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: dev@dpdk.org
> Subject: Re: Clarification for - SoC specific driver based common sub component
> placing
>
> 2016-11-18 14:38, Thomas Monjalon:
> > 2016-11-18 12:44, Hemant Agrawal:
> > > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2)
> Poll mode drivers into the DPDK.
> > >
> > > We need some clarification w.r.t the right placing of some dependent
> components, which can be common across drivers. E.g. We have hardware
> queue and buffer manager driver. This will be used by both network driver and
> crypto driver. But it is specific to NXP platform only.
> > >
> > > What is the right place for such common hardware specific components in
> DPDK?
> > > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For each
> soc configuration only the required components will be compiled-in.
> > > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs.
> And link the network and crypto drivers to it.
> > > 3. Add it to main network driver and make the crypto driver dependent
> on it.
> >
> > Your question is more generic than SoC context.
> > You just want to share some code between drivers, right?
> > What about building a library located in drivers/common/nxp/ ?
>
> I'm a bit reluctant to have company name in file hierarchy, as it not something
> stable. And especially for NXP/Qualcomm...
>
> In this case would it be better to name the directory drivers/common/dpaa2/ ?
[Hemant] Sounds good.
Thanks!
^ permalink raw reply
* Re: [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
From: Bruce Richardson @ 2016-11-18 15:00 UTC (permalink / raw)
To: Satha Rao; +Cc: helin.zhang, jingjing.wu, jerin.jacob, jianbo.liu, dev
In-Reply-To: <1479473533-9393-1-git-send-email-skoteshwar@caviumnetworks.com>
On Fri, Nov 18, 2016 at 04:52:13AM -0800, Satha Rao wrote:
> i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
> results. To solve this include rte memory barriers
>
> Signed-off-by: Satha Rao <skoteshwar@caviumnetworks.com>
> ---
> drivers/net/i40e/base/i40e_osdep.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
> index 38e7ba5..ffa3160 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -158,7 +158,13 @@ do { \
> ((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
> static inline uint32_t i40e_read_addr(volatile void *addr)
> {
> +#if defined(RTE_ARCH_ARM64)
> + uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
> + rte_rmb();
> + return val;
> +#else
> return rte_le_to_cpu_32(I40E_PCI_REG(addr));
> +#endif
> }
> #define I40E_PCI_REG_WRITE(reg, value) \
> do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
> @@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void *addr)
> I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value))
>
> #define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg)))
> +#if defined(RTE_ARCH_ARM64)
> +#define wr32(a, reg, value) \
> + do { \
> + I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \
> + rte_wmb(); \
> + } while (0)
> +#else
> #define wr32(a, reg, value) \
> I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value))
> +#endif
> #define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT)))
>
> #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
> --
Would rte_smp_*mb() functions allow you to get a similar result without
the need for #ifdefs? It should be a full barrier on weakly ordered
platforms which just a compiler barrier on IA.
/Bruce
^ permalink raw reply
* [PATCH] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 15:04 UTC (permalink / raw)
To: dev
Previous reported speed was hardcoded.
Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
drivers/net/nfp/nfp_net.c | 31 +++++++++++++++++++++++++++++--
drivers/net/nfp/nfp_net_ctrl.h | 13 +++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..d5ec0ff 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -803,6 +803,7 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
hw->ctrl = new_ctrl;
}
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
/*
* return 0 means link status changed, -1 means not changed
*
@@ -816,6 +817,18 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
struct rte_eth_link link, old;
uint32_t nn_link_status;
+ static const uint32_t ls_to_ethtool[] = {
+ [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
+ [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE,
+ [NFP_NET_CFG_STS_LINK_RATE_1G] = ETH_SPEED_NUM_1G,
+ [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G,
+ [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G,
+ [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G,
+ [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G,
+ [NFP_NET_CFG_STS_LINK_RATE_100G] = ETH_SPEED_NUM_100G,
+ };
+
+
PMD_DRV_LOG(DEBUG, "Link update\n");
hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -831,8 +844,22 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
link.link_status = ETH_LINK_UP;
link.link_duplex = ETH_LINK_FULL_DUPLEX;
- /* Other cards can limit the tx and rx rate per VF */
- link.link_speed = ETH_SPEED_NUM_40G;
+
+ nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
+ NFP_NET_CFG_STS_LINK_RATE_MASK;
+
+ if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
+ ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
+ (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
+ link.link_speed = ETH_SPEED_NUM_40G;
+ else {
+
+ if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN ||
+ nn_link_status >= ARRAY_SIZE(ls_to_ethtool))
+ link.link_speed = ETH_SPEED_NUM_NONE;
+ else
+ link.link_speed = ls_to_ethtool[nn_link_status];
+ }
if (old.link_status != link.link_status) {
nfp_net_dev_atomic_write_link_status(dev, &link);
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index fce8251..f9aaba3 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,19 @@
#define NFP_NET_CFG_VERSION_MINOR(x) (((x) & 0xff) << 0)
#define NFP_NET_CFG_STS 0x0034
#define NFP_NET_CFG_STS_LINK (0x1 << 0) /* Link up or down */
+/* Link rate */
+#define NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
+#define NFP_NET_CFG_STS_LINK_RATE_MASK 0xF
+#define NFP_NET_CFG_STS_LINK_RATE \
+ (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT)
+#define NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED 0
+#define NFP_NET_CFG_STS_LINK_RATE_UNKNOWN 1
+#define NFP_NET_CFG_STS_LINK_RATE_1G 2
+#define NFP_NET_CFG_STS_LINK_RATE_10G 3
+#define NFP_NET_CFG_STS_LINK_RATE_25G 4
+#define NFP_NET_CFG_STS_LINK_RATE_40G 5
+#define NFP_NET_CFG_STS_LINK_RATE_50G 6
+#define NFP_NET_CFG_STS_LINK_RATE_100G 7
#define NFP_NET_CFG_CAP 0x0038
#define NFP_NET_CFG_MAX_TXRINGS 0x003c
#define NFP_NET_CFG_MAX_RXRINGS 0x0040
--
1.9.1
^ permalink raw reply related
* Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 15:10 UTC (permalink / raw)
To: Thomas Monjalon, dev
In-Reply-To: <20161118150433.AD575558B@dpdk.org>
Hi Thomas,
I got this email when sending a patch some minutes ago.
The point is I trusted script/checkpatches.sh which did not report those
warnings.
Am I doing anything wrong when using checkpatches.sh?
---------- Forwarded message ----------
From: <checkpatch@dpdk.org>
Date: Fri, Nov 18, 2016 at 3:04 PM
Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
To: test-report@dpdk.org
Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
Test-Label: checkpatch
Test-Status: WARNING
http://dpdk.org/patch/17091
_coding style issues_
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
#53: FILE: drivers/net/nfp/nfp_net.c:806:
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#91: FILE: drivers/net/nfp/nfp_net.c:856:
+ else {
+
total: 0 errors, 0 warnings, 2 checks, 68 lines checked
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Ferruh Yigit @ 2016-11-18 15:24 UTC (permalink / raw)
To: Alejandro Lucero, Thomas Monjalon, dev
In-Reply-To: <CAD+H9919z+wvjdP31kivhNy+tkPCOHULfNBO=Ae0xSdCGAVA8g@mail.gmail.com>
On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
>
> I got this email when sending a patch some minutes ago.
>
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?
I am also getting same warnings as below, this can be related to the
checkpatch.pl version.
I have: Version: 0.32
(./scripts/checkpatch.pl --version)
>
>
> ---------- Forwarded message ----------
> From: <checkpatch@dpdk.org>
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report@dpdk.org
> Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
>
>
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
>
> _coding style issues_
>
>
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> + else {
> +
>
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
>
^ permalink raw reply
* Re: [PATCH 0/4] libeventdev API and northbound implementation
From: Bruce Richardson @ 2016-11-18 15:25 UTC (permalink / raw)
To: Jerin Jacob; +Cc: dev, harry.van.haaren, hemant.agrawal, gage.eads
In-Reply-To: <1479447902-3700-1-git-send-email-jerin.jacob@caviumnetworks.com>
On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob 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.
>
> [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
>
> Changes since RFC v2:
>
> - Updated the documentation to define the need for this library[Jerin]
> - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in
> struct rte_event_queue_conf to enable optimized sw implementation [Bruce]
> - Introduced RTE_EVENT_OP* ops [Bruce]
> - Added nb_event_queue_flows,nb_event_port_dequeue_depth, nb_event_port_enqueue_depth
> in rte_event_dev_configure() like ethdev and crypto library[Jerin]
> - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to
> reduce fast path APIs and it is redundant too[Jerin]
> - In the view of better application portability, Removed pin_event
> from rte_event_enqueue as it is just hint and Intel/NXP can not support it[Jerin]
> - Added rte_event_port_links_get()[Jerin]
> - Added rte_event_dev_dump[Harry]
>
> Notes:
>
> - This patch set is check-patch clean with an exception that
> 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL
> - Looking forward to getting additional maintainers for libeventdev
>
>
> Possible next steps:
> 1) Review this patch set
> 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/]
> 3) Review proposed examples/eventdev_pipeline application[http://dpdk.org/dev/patchwork/patch/17053/]
> 4) Review proposed functional tests[http://dpdk.org/dev/patchwork/patch/17051/]
> 5) Cavium's HW based eventdev driver
>
> I am planning to work on (3),(4) and (5)
>
Thanks Jerin,
we'll review and get back to you with any comments or feedback (1), and
obviously start working on item (2) also! :-)
I'm also wonder whether we should have a staging tree for this work to
make interaction between us easier. Although this may not be
finalised enough for 17.02 release, do you think having an
dpdk-eventdev-next tree would be a help? My thinking is that once we get
the eventdev library itself in reasonable shape following our review, we
could commit that and make any changes thereafter as new patches, rather
than constantly respinning the same set. It also gives us a clean git
tree to base the respective driver implementations on from our two sides.
Thomas, any thoughts here on your end - or from anyone else?
Regards,
/Bruce
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Ferruh Yigit @ 2016-11-18 15:26 UTC (permalink / raw)
To: Alejandro Lucero, Thomas Monjalon, dev
In-Reply-To: <CAD+H9919z+wvjdP31kivhNy+tkPCOHULfNBO=Ae0xSdCGAVA8g@mail.gmail.com>
On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
>
> I got this email when sending a patch some minutes ago.
>
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?
>
>
> ---------- Forwarded message ----------
> From: <checkpatch@dpdk.org>
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report@dpdk.org
> Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
>
>
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
>
> _coding style issues_
>
>
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
btw, you can benefit from RTE_DIM:
lib/librte_eal/common/include/rte_common.h:352:
#define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0]))
>
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> + else {
> +
>
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
>
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 15:31 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Thomas Monjalon, dev
In-Reply-To: <d5671474-fe84-7acb-6b64-7ddc1c63aeda@intel.com>
On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> >
> > I got this email when sending a patch some minutes ago.
> >
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
>
> I am also getting same warnings as below, this can be related to the
> checkpatch.pl version.
>
> I have: Version: 0.32
> (./scripts/checkpatch.pl --version)
>
>
Uhmm, I got same one.
> >
> >
> > ---------- Forwarded message ----------
> > From: <checkpatch@dpdk.org>
> > Date: Fri, Nov 18, 2016 at 3:04 PM
> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> > To: test-report@dpdk.org
> > Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
> >
> >
> > Test-Label: checkpatch
> > Test-Status: WARNING
> > http://dpdk.org/patch/17091
> >
> > _coding style issues_
> >
> >
> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
> side-effects?
> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> >
> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
> > + else {
> > +
> >
> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> >
>
>
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 15:33 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Thomas Monjalon, dev
In-Reply-To: <00696ef7-6fb5-9bb8-5d77-a04d3253f138@intel.com>
On Fri, Nov 18, 2016 at 3:26 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> >
> > I got this email when sending a patch some minutes ago.
> >
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
> >
> >
> > ---------- Forwarded message ----------
> > From: <checkpatch@dpdk.org>
> > Date: Fri, Nov 18, 2016 at 3:04 PM
> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> > To: test-report@dpdk.org
> > Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
> >
> >
> > Test-Label: checkpatch
> > Test-Status: WARNING
> > http://dpdk.org/patch/17091
> >
> > _coding style issues_
> >
> >
> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
> side-effects?
> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>
> btw, you can benefit from RTE_DIM:
>
> lib/librte_eal/common/include/rte_common.h:352:
> #define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0]))
>
>
Thanks!
I will use it in the next patch version.
> >
> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
> > + else {
> > +
> >
> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> >
>
>
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Thomas Monjalon @ 2016-11-18 15:34 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: Ferruh Yigit, dev
In-Reply-To: <d5671474-fe84-7acb-6b64-7ddc1c63aeda@intel.com>
2016-11-18 15:24, Ferruh Yigit:
> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> >
> > I got this email when sending a patch some minutes ago.
> >
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
>
> I am also getting same warnings as below, this can be related to the
> checkpatch.pl version.
>
> I have: Version: 0.32
> (./scripts/checkpatch.pl --version)
Yes checkpatch@dpdk.org uses the version 0.32.
I could try to add it in the mail report.
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 15:42 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Thomas Monjalon, dev
In-Reply-To: <CAD+H992NtFefKXxNQiJng7i+6AtJywSSuLCaU4D-TBw_Svj17g@mail.gmail.com>
On Fri, Nov 18, 2016 at 3:31 PM, Alejandro Lucero <
alejandro.lucero@netronome.com> wrote:
>
>
> On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
>
>> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
>> > Hi Thomas,
>> >
>> > I got this email when sending a patch some minutes ago.
>> >
>> > The point is I trusted script/checkpatches.sh which did not report those
>> > warnings.
>> > Am I doing anything wrong when using checkpatches.sh?
>>
>> I am also getting same warnings as below, this can be related to the
>> checkpatch.pl version.
>>
>> I have: Version: 0.32
>> (./scripts/checkpatch.pl --version)
>>
>>
> Uhmm, I got same one.
>
>
Ok. It seems I suffered a temporal blindness. I though the automatic report
was about warnings but it is about checks. But I got just one of the checks
messages. This is the output with -v and adding OPTIONS used:
### [PATCH] nfp: report link speed using hardware info
OPTIONS: --no-tree --max-line-length=80 --show-types
--ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,SPLIT_STRING,LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,NEW_TYPEDEFS,COMPARISON_TO_NULL
CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#60: FILE: drivers/net/nfp/nfp_net.c:856:
+ else {
+
total: 0 errors, 0 warnings, 1 checks, 68 lines checked
0/1 valid patch
> >
>> >
>> > ---------- Forwarded message ----------
>> > From: <checkpatch@dpdk.org>
>> > Date: Fri, Nov 18, 2016 at 3:04 PM
>> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
>> > To: test-report@dpdk.org
>> > Cc: Alejandro Lucero <alejandro.lucero@netronome.com>
>> >
>> >
>> > Test-Label: checkpatch
>> > Test-Status: WARNING
>> > http://dpdk.org/patch/17091
>> >
>> > _coding style issues_
>> >
>> >
>> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
>> side-effects?
>> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
>> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>> >
>> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
>> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
>> > + else {
>> > +
>> >
>> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
>> >
>>
>>
>
^ permalink raw reply
* Re: Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
From: Thomas Monjalon @ 2016-11-18 15:44 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: Ferruh Yigit, dev
In-Reply-To: <CAD+H992NtFefKXxNQiJng7i+6AtJywSSuLCaU4D-TBw_Svj17g@mail.gmail.com>
2016-11-18 15:31, Alejandro Lucero:
> On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
>
> > On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > > Hi Thomas,
> > >
> > > I got this email when sending a patch some minutes ago.
> > >
> > > The point is I trusted script/checkpatches.sh which did not report those
> > > warnings.
> > > Am I doing anything wrong when using checkpatches.sh?
> >
> > I am also getting same warnings as below, this can be related to the
> > checkpatch.pl version.
> >
> > I have: Version: 0.32
> > (./scripts/checkpatch.pl --version)
> >
> Uhmm, I got same one.
The last update of this version number is from 2011...
I guess we have to live without checkpatch versioning.
^ permalink raw reply
* Re: [PATCH 0/4] libeventdev API and northbound implementation
From: Bruce Richardson @ 2016-11-18 16:04 UTC (permalink / raw)
To: Jerin Jacob
Cc: dev, harry.van.haaren, hemant.agrawal, gage.eads, thomas.monjalon
In-Reply-To: <20161118152518.GA121080@bricha3-MOBL3.ger.corp.intel.com>
+Thomas
On Fri, Nov 18, 2016 at 03:25:18PM +0000, Bruce Richardson wrote:
> On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob 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.
> >
> > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
> >
> > Changes since RFC v2:
> >
> > - Updated the documentation to define the need for this library[Jerin]
> > - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in
> > struct rte_event_queue_conf to enable optimized sw implementation [Bruce]
> > - Introduced RTE_EVENT_OP* ops [Bruce]
> > - Added nb_event_queue_flows,nb_event_port_dequeue_depth, nb_event_port_enqueue_depth
> > in rte_event_dev_configure() like ethdev and crypto library[Jerin]
> > - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to
> > reduce fast path APIs and it is redundant too[Jerin]
> > - In the view of better application portability, Removed pin_event
> > from rte_event_enqueue as it is just hint and Intel/NXP can not support it[Jerin]
> > - Added rte_event_port_links_get()[Jerin]
> > - Added rte_event_dev_dump[Harry]
> >
> > Notes:
> >
> > - This patch set is check-patch clean with an exception that
> > 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL
> > - Looking forward to getting additional maintainers for libeventdev
> >
> >
> > Possible next steps:
> > 1) Review this patch set
> > 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/]
> > 3) Review proposed examples/eventdev_pipeline application[http://dpdk.org/dev/patchwork/patch/17053/]
> > 4) Review proposed functional tests[http://dpdk.org/dev/patchwork/patch/17051/]
> > 5) Cavium's HW based eventdev driver
> >
> > I am planning to work on (3),(4) and (5)
> >
> Thanks Jerin,
>
> we'll review and get back to you with any comments or feedback (1), and
> obviously start working on item (2) also! :-)
>
> I'm also wonder whether we should have a staging tree for this work to
> make interaction between us easier. Although this may not be
> finalised enough for 17.02 release, do you think having an
> dpdk-eventdev-next tree would be a help? My thinking is that once we get
> the eventdev library itself in reasonable shape following our review, we
> could commit that and make any changes thereafter as new patches, rather
> than constantly respinning the same set. It also gives us a clean git
> tree to base the respective driver implementations on from our two sides.
>
> Thomas, any thoughts here on your end - or from anyone else?
>
> Regards,
> /Bruce
>
^ permalink raw reply
* [PATCH v2] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-11-18 16:06 UTC (permalink / raw)
To: dev
Previous reported speed was hardcoded.
Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
drivers/net/nfp/nfp_net.c | 28 ++++++++++++++++++++++++++--
drivers/net/nfp/nfp_net_ctrl.h | 13 +++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..24f3164 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -816,6 +816,17 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
struct rte_eth_link link, old;
uint32_t nn_link_status;
+ static const uint32_t ls_to_ethtool[] = {
+ [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
+ [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE,
+ [NFP_NET_CFG_STS_LINK_RATE_1G] = ETH_SPEED_NUM_1G,
+ [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G,
+ [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G,
+ [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G,
+ [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G,
+ [NFP_NET_CFG_STS_LINK_RATE_100G] = ETH_SPEED_NUM_100G,
+ };
+
PMD_DRV_LOG(DEBUG, "Link update\n");
hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -831,8 +842,21 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
link.link_status = ETH_LINK_UP;
link.link_duplex = ETH_LINK_FULL_DUPLEX;
- /* Other cards can limit the tx and rx rate per VF */
- link.link_speed = ETH_SPEED_NUM_40G;
+
+ nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
+ NFP_NET_CFG_STS_LINK_RATE_MASK;
+
+ if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
+ ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
+ (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
+ link.link_speed = ETH_SPEED_NUM_40G;
+ else {
+ if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN ||
+ nn_link_status >= RTE_DIM(ls_to_ethtool))
+ link.link_speed = ETH_SPEED_NUM_NONE;
+ else
+ link.link_speed = ls_to_ethtool[nn_link_status];
+ }
if (old.link_status != link.link_status) {
nfp_net_dev_atomic_write_link_status(dev, &link);
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index fce8251..f9aaba3 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,19 @@
#define NFP_NET_CFG_VERSION_MINOR(x) (((x) & 0xff) << 0)
#define NFP_NET_CFG_STS 0x0034
#define NFP_NET_CFG_STS_LINK (0x1 << 0) /* Link up or down */
+/* Link rate */
+#define NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
+#define NFP_NET_CFG_STS_LINK_RATE_MASK 0xF
+#define NFP_NET_CFG_STS_LINK_RATE \
+ (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT)
+#define NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED 0
+#define NFP_NET_CFG_STS_LINK_RATE_UNKNOWN 1
+#define NFP_NET_CFG_STS_LINK_RATE_1G 2
+#define NFP_NET_CFG_STS_LINK_RATE_10G 3
+#define NFP_NET_CFG_STS_LINK_RATE_25G 4
+#define NFP_NET_CFG_STS_LINK_RATE_40G 5
+#define NFP_NET_CFG_STS_LINK_RATE_50G 6
+#define NFP_NET_CFG_STS_LINK_RATE_100G 7
#define NFP_NET_CFG_CAP 0x0038
#define NFP_NET_CFG_MAX_TXRINGS 0x003c
#define NFP_NET_CFG_MAX_RXRINGS 0x0040
--
1.9.1
^ 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