DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
From: Vincent JARDIN @ 2016-12-19 13:45 UTC (permalink / raw)
  To: Chen, Jing D, Thomas Monjalon
  Cc: dev@dpdk.org, Yigit, Ferruh, Wu, Jingjing, Zhang, Helin
In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D3C5B8459@shsmsx102.ccr.corp.intel.com>

Le 19/12/2016 à 14:39, Chen, Jing D a écrit :
> They will
> have concern why VM can have such privilege (like promisc mode). But I need
> to check as I know there is some mechanism now to make a VM privileged.

 From iproute2's man:
<--
trust on|off - trust the specified VF user. This enables that VF user 
can set a specific feature which may impact security and/or performance. 
(e.g. VF multicast promiscuous mode)
-->

So yes, it is possible to get PF features upstream'd into the kernel 
without having a specific PF into DPDK.

best regards,
   Vincent

^ permalink raw reply

* Re: [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
From: Chen, Jing D @ 2016-12-19 13:39 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev@dpdk.org, Vincent JARDIN, Yigit, Ferruh, Wu, Jingjing,
	Zhang, Helin
In-Reply-To: <7801511.7yxptAly8J@xps13>

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, December 19, 2016 9:21 PM
> To: Chen, Jing D <jing.d.chen@intel.com>
> Cc: dev@dpdk.org; Vincent JARDIN <vincent.jardin@6wind.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5 00/29] Support VFD and DPDK PF + kernel
> VF on i40e
> 
> 2016-12-19 09:01, Chen, Jing D:
> > Since then, both Linux and DPDK keep developing code. Then, we found
> > it's necessary to extend VF capability (Like promiscuous mode). It
> > will be hard to ask Linux PF to support that service considering upstream
> effort in Linux world.
> 
> Please, could you clarify this?
> Do you mean you cannot change the Linux driver?

There are 2 things here. One is to add extended functionality into Linux driver,
Another is to have it upstream into kernel world.
The first one can be achieved easier, while later one may be difficult. They will
have concern why VM can have such privilege (like promisc mode). But I need
to check as I know there is some mechanism now to make a VM privileged.

^ permalink raw reply

* Re: [PATCH 6/7] net/qede: fix maximum VF count to 0
From: Thomas Monjalon @ 2016-12-19 13:38 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Harish Patil, dev, Dept-Eng DPDK Dev
In-Reply-To: <eabebd19-fd7e-4c1e-e921-13c809f79f6e@intel.com>

2016-12-12 18:13, Ferruh Yigit:
> On 12/12/2016 5:47 PM, Harish Patil wrote:
> >> btw, while checking feature list, I have seen qede_vf supports SR-IOV,
> >> is that correct?
> > 
> > Yes. The supported combination for SR-IOV is VF driver (qede PMD) with PF
> > driver (qede linux driver).
> 
> So you are using SR-IOV feature set in VF driver, as meaning VF driver
> support exists. I don't know what SR-IOV feature mean for VF drivers.
> Some other VF drivers not has this feature flag set.
> 
> CC'ed Thomas for help, if this is the intention of the feature flag, it
> is OK.

Good question.
I wonder where better describe the meaning of this row in the features table.
Maybe it would be clearer by splitting in 2 rows:
- SR-IOV VF
- SR-IOV PF

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Thomas Monjalon @ 2016-12-19 13:30 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Ferruh Yigit, dev, nhorman, vido, fiona.trahe, stephen,
	adrien.mazarguil
In-Reply-To: <20161216103602.481449e4@platinum>

2016-12-16 10:36, Olivier Matz:
> Hi Ferruh,
> 
> On Thu, 15 Dec 2016 14:52:02 +0000, Ferruh Yigit
> <ferruh.yigit@intel.com> wrote:
> > 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?

Yes I agree, this patch is mainly about drivers/net.

^ permalink raw reply

* Re: No packets received if burst is too small in rte_eth_rx_burst
From: Thomas Monjalon @ 2016-12-19 13:24 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, tom.barbette
In-Reply-To: <20161219102504.GA170848@bricha3-MOBL3.ger.corp.intel.com>

2016-12-19 10:25, Bruce Richardson:
> On Sat, Dec 17, 2016 at 11:43:25AM +0100, tom.barbette@ulg.ac.be wrote:
> > Hi,
> > 
> > Your comments made me saw the line "PMD: i40e_set_rx_function(): Vector rx enabled, please make sure RX burst size no less than 4 (port=0)."
> > 
> > The problem was probably that I was under this limit... Is there a way to get that limit through a function or something? 
> > 
> > With 16.04 I received sometimes 5 or 7 packets with a burst_size of 4 which respects this limit. I see that "[dpdk-dev] net/i40e: fix out-of-bounds writes during vector Rx" fixed that, as the limit was in fact 32 no matter the message.
> > 
> > At the end, what should be the minimal rx burst size? How to find it at runtime for any NIC? I imagine that vector rx will create a problem if I give a burst size of 1 even with a recent DPDK version, right?
> > 
> 
> Sadly, there doesn't appear to be any way to discover this, and the i40e
> driver requires at least a burst size of 4 even with the latest DPDK.
> From i40e_rxtx_vec_sse.c:
> 
> 243         /* nb_pkts has to be floor-aligned to RTE_I40E_DESCS_PER_LOOP */
> 244         nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_I40E_DESCS_PER_LOOP);
> 245
> 
> I think in this case the gap is not so much having a discovery mechanism
> to determine min burst size, but rather a driver gap so as to allow some
> form of slower-path fallback when we get below min-size bursts for the
> vector driver.

Yes this is a severe bug.

Please Tom, could you keep asking/monitoring this bug until it is fixed?
Thanks for reporting.

^ permalink raw reply

* Re: [PATCH v5 00/29] Support VFD and DPDK PF + kernel VF on i40e
From: Thomas Monjalon @ 2016-12-19 13:20 UTC (permalink / raw)
  To: Chen, Jing D
  Cc: dev, Vincent JARDIN, Yigit, Ferruh, Wu, Jingjing, Zhang, Helin
In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D3C5B7FE2@shsmsx102.ccr.corp.intel.com>

2016-12-19 09:01, Chen, Jing D:
> Since then, both Linux and DPDK keep developing code. Then, we found it's
> necessary to extend VF capability (Like promiscuous mode). It will be hard to
> ask Linux PF to support that service considering upstream effort in Linux world.

Please, could you clarify this?
Do you mean you cannot change the Linux driver?

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Neil Horman @ 2016-12-19 12:42 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Olivier Matz, Stephen Hemminger, dev, thomas.monjalon, vido,
	fiona.trahe, adrien.mazarguil
In-Reply-To: <ead3cf82-9108-69a4-0fd7-ee3babbb178c@intel.com>

On Fri, Dec 16, 2016 at 02:19:59PM +0000, Ferruh Yigit wrote:
> On 12/16/2016 12:37 PM, Neil Horman wrote:
> > On Fri, Dec 16, 2016 at 10:22:08AM +0100, Olivier Matz wrote:
> >> 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?
> >>
> > Speaking only for myself, I'm not suggesting the patch not be included, only
> > questioning the need to list igb_uio as an optional dependency.  From what I
> > understand uio_pci_generic is equaly capable of being used in a vf as igb_uio,
> > and so it seems like its sufficient to list in the deps alone, or am I missing
> > something?
> > 
> > Additionally, in regards to the comment about rebasing on net-next here, I don't
> > think thats needed.  This patch is built such that you will be able to apply
> > this tag to additional drivers later, as they get merged into thomas's tree, you
> > don't need to get them all in one shot.
> 
> Right, more drivers can be added later. But also I don't see any problem
> if this patch rebased on next-net and be a more complete patch. That is
> why it was a question to the author.
> 
Right, it certainly doesn't hurt anything to do so, but given that:

1) We're on v3 of this patch
2) Not including information in all drivers isn't catastrophic
3) Rebasing on net-next still leaves the crypto drivers unaccounted for

It seems to me that including the patch now, so that the infrastructure is in
place for driver maintainers to add the macro on their own in their respective
trees seems like the more expiedient course of action.

Best
Neil

^ permalink raw reply

* Re: [PATCH v4] vmxnet3: fix Rx deadlock
From: Ferruh Yigit @ 2016-12-19 12:26 UTC (permalink / raw)
  To: Stefan Puiu, dev; +Cc: yongwang, mac_leehk, dpdk stable
In-Reply-To: <8e9b361a-566c-2c25-5497-da0ee0e7c818@intel.com>

On 12/19/2016 10:41 AM, Ferruh Yigit wrote:
> On 12/19/2016 9:40 AM, Stefan Puiu wrote:
>> Our use case is that we have an app that needs to keep mbufs around
>> for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from
>> vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX
>> descriptors (where it returns -err). Since there are no mbufs that the
>> virtual hardware can use, no packets will be received after this; the
>> driver won't refill the mbuf after this so it gets stuck in this
>> state. I call this a deadlock for lack of a better term - the virtual
>> HW waits for free mbufs, while the app waits for the hardware to
>> notify it for data (by flipping the generation bit on the used Rx
>> descriptors). Note that after this, the app can't recover.
>>
>> This fix is a rework of this patch by Marco Lee:
>> http://dpdk.org/dev/patchwork/patch/6575/. I had to forward port
>> it, address review comments and also reverted the allocation
>> failure handling to the first version of the patch
>> (http://dpdk.org/ml/archives/dev/2015-July/022079.html), since
>> that's the only approach that seems to work, and seems to be what
>> other drivers are doing (I checked ixgbe and em). Reusing the mbuf
>> that's getting passed to the application doesn't seem to make
>> sense, and it was causing weird issues in our app. Also, reusing
>> rxm without checking if it's NULL could cause the code to crash.
>>
>> Signed-off-by: Stefan Puiu <stefan.puiu@gmail.com>
> 
> Applied to dpdk-next-net/master, thanks.
> 

CC:stable@dpdk.org

^ permalink raw reply

* [PATCH] nfp: fix typo in tx offload capabilities
From: Alejandro Lucero @ 2016-12-19 12:22 UTC (permalink / raw)
  To: dev

Because macros for TCP and UDP related to offload cksums have
same values, this was not a main problem. But better to use the
right ones.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c5bdd4c..be0fefa 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1049,8 +1049,8 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 
 	if (hw->cap & NFP_NET_CFG_CTRL_TXCSUM)
 		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_IPV4_CKSUM |
-					     DEV_RX_OFFLOAD_UDP_CKSUM |
-					     DEV_RX_OFFLOAD_TCP_CKSUM;
+					     DEV_TX_OFFLOAD_UDP_CKSUM |
+					     DEV_TX_OFFLOAD_TCP_CKSUM;
 
 	dev_info->default_rxconf = (struct rte_eth_rxconf) {
 		.rx_thresh = {
-- 
1.9.1

^ permalink raw reply related

* [PATCH] nfp: remove rx port metadata
From: Alejandro Lucero @ 2016-12-19 12:14 UTC (permalink / raw)
  To: dev

This was required for middlebox-like firmware which NFP does
not support anymore.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c     | 32 --------------------------------
 drivers/net/nfp/nfp_net_pmd.h |  2 +-
 2 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 77015c4..c5bdd4c 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1628,12 +1628,6 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
 	hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
 
-	/*
-	 * hash type is sharing the same word with input port info
-	 * 31-8: input port
-	 * 7:0: hash type
-	 */
-	hash_type &= 0xff;
 	mbuf->hash.rss = hash;
 	mbuf->ol_flags |= PKT_RX_RSS_HASH;
 
@@ -1652,29 +1646,6 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	}
 }
 
-/* nfp_net_check_port - Set mbuf in_port field */
-static void
-nfp_net_check_port(struct nfp_net_rx_desc *rxd, struct rte_mbuf *mbuf)
-{
-	uint32_t port;
-
-	if (!(rxd->rxd.flags & PCIE_DESC_RX_INGRESS_PORT)) {
-		mbuf->port = 0;
-		return;
-	}
-
-	port = rte_be_to_cpu_32(*(uint32_t *)((uint8_t *)mbuf->buf_addr +
-					      mbuf->data_off - 8));
-
-	/*
-	 * hash type is sharing the same word with input port info
-	 * 31-8: input port
-	 * 7:0: hash type
-	 */
-	port = (uint8_t)(port >> 8);
-	mbuf->port = port;
-}
-
 static inline void
 nfp_net_mbuf_alloc_failed(struct nfp_net_rxq *rxq)
 {
@@ -1824,9 +1795,6 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 		/* Checking the checksum flag */
 		nfp_net_rx_cksum(rxq, rxds, mb);
 
-		/* Checking the port flag */
-		nfp_net_check_port(rxds, mb);
-
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
 		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
diff --git a/drivers/net/nfp/nfp_net_pmd.h b/drivers/net/nfp/nfp_net_pmd.h
index c180972..dc70d57 100644
--- a/drivers/net/nfp/nfp_net_pmd.h
+++ b/drivers/net/nfp/nfp_net_pmd.h
@@ -269,7 +269,7 @@ struct nfp_net_txq {
 #define PCIE_DESC_RX_I_TCP_CSUM_OK      (1 << 11)
 #define PCIE_DESC_RX_I_UDP_CSUM         (1 << 10)
 #define PCIE_DESC_RX_I_UDP_CSUM_OK      (1 <<  9)
-#define PCIE_DESC_RX_INGRESS_PORT       (1 <<  8)
+#define PCIE_DESC_RX_SPARE              (1 <<  8)
 #define PCIE_DESC_RX_EOP                (1 <<  7)
 #define PCIE_DESC_RX_IP4_CSUM           (1 <<  6)
 #define PCIE_DESC_RX_IP4_CSUM_OK        (1 <<  5)
-- 
1.9.1

^ permalink raw reply related

* [PATCH] nfp: extend speed capabilities advertised
From: Alejandro Lucero @ 2016-12-19 12:05 UTC (permalink / raw)
  To: dev

NFP supports more speeds than just 40 and 100GB, which were
what was advertised before.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 27afbfd..77015c4 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1077,7 +1077,9 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	dev_info->reta_size = NFP_NET_CFG_RSS_ITBL_SZ;
 	dev_info->hash_key_size = NFP_NET_CFG_RSS_KEY_SZ;
 
-	dev_info->speed_capa = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_100G;
+	dev_info->speed_capa = ETH_SPEED_NUM_1G | ETH_LINK_SPEED_10G |
+			       ETH_SPEED_NUM_25G | ETH_SPEED_NUM_40G |
+			       ETH_SPEED_NUM_50G | ETH_LINK_SPEED_100G;
 }
 
 static const uint32_t *
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-12-19 12:01 UTC (permalink / raw)
  To: dev

Previous reported speed was hardcoded because there was not firmware
support for getting this information. This change needs to support old
firmware versions, keeping with the hardcoded report, and the new
versions, where the firmware makes that information available.

v5: Fix missing parenthesis
v4: Make conditional simple and more ellaborated commit comment.
v3: remove unsed macro
v2: use RTE_DIM instead of own macro

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c      | 27 +++++++++++++++++++++++++--
 drivers/net/nfp/nfp_net_ctrl.h | 11 +++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 2ae7f87..27afbfd 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,20 @@ 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 >= 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..426402b 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,17 @@
 #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_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

* Re: [PATCH v3 5/6] event/skeleton: add skeleton eventdev driver
From: Bruce Richardson @ 2016-12-19 11:58 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: dev, thomas.monjalon, hemant.agrawal, gage.eads, harry.van.haaren
In-Reply-To: <1482070895-32491-6-git-send-email-jerin.jacob@caviumnetworks.com>

On Sun, Dec 18, 2016 at 07:51:34PM +0530, Jerin Jacob wrote:
> The skeleton driver facilitates, bootstrapping the new
> eventdev driver and creates a platform to verify
> the northbound eventdev common code.
> 
> The driver supports both VDEV and PCI based eventdev
> devices.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
<snip>
> diff --git a/drivers/event/skeleton/Makefile b/drivers/event/skeleton/Makefile
> new file mode 100644
> index 0000000..e557f6d
> --- /dev/null
> +++ b/drivers/event/skeleton/Makefile
> @@ -0,0 +1,55 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2016 Cavium Networks. All rights reserved.
> +#   All rights reserved.
> +#
> +#   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.
> +#
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_skeleton_event.a
> +
> +CFLAGS += $(WERROR_FLAGS)
> +
> +EXPORT_MAP := rte_pmd_skeleton_event_version.map
> +
> +LIBABIVER := 1
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += skeleton_eventdev.c
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += lib/librte_eal
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_EVENTDEV) += lib/librte_event
> +

Minor typo: this lib should be "lib/librte_eventdev", otherwise we get
compile failures when doing shared library builds.

/Bruce

^ permalink raw reply

* Re: [PATCH] nfp: add doc about supported features
From: Alejandro Lucero @ 2016-12-19 11:58 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Mcnamara, John, dev@dpdk.org
In-Reply-To: <17e4540d-e11b-3854-1fe7-b4abfd95a40c@intel.com>

On Mon, Dec 19, 2016 at 11:45 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 12/16/2016 5:29 PM, Alejandro Lucero wrote:
> > Hi,
> >
> > One question about this patch. I will send another patch soon which will
> > require to modify the file created by this patch. So, should I use the
> > dpdk-next for sending the new patch or the dpdk stable branch?
>
> I guess by "dpdk stable branch" you mean dpdk main repo, because we also
> have stable sub-tree which is something else.
>
>
Yes. That's what I meant.


> > I
> > understand that using the latter will imply some integration later, but
> > I really do not know if I should facilitate things using dpdk-next in
> > this case.
>
> If the patch is driver patch, please send to the next-net sub-tree, as a
> PMD maintainer, I expect majority of your patches should target next-net.
>
>
OK


> If patch just touches the documentation of the driver, you can send it
> to the main tree, but both next-net sub-tree and main tree are OK since
> PMD documentation is not heavily modified, integration will be (mostly)
> easy.
>
> Specific to the this document (feature.ini), since this is directly
> correlated with PMD source code, to update this file, you need to update
> the source code. And it is better to update this document in next-net as
> part of the patchset that updates the PMD code.
>
>
Fine.


> >
> > By the way, it is not just about this specific patch, because I have
> > other almost ready which I want to push before the 16.02 deadline.
>
> Please push 17.02 patches as soon as possible, although there is
> technically still some time for the integration deadline, practically
> there is less because of holidays in between ...
>
>
I'm working on this. I have been busy doing other more priority things but
this is now top priority for me.

Thanks


> >
> >
> <...>
>

^ permalink raw reply

* Re: [PATCH] nfp: add doc about supported features
From: Ferruh Yigit @ 2016-12-19 11:45 UTC (permalink / raw)
  To: Alejandro Lucero; +Cc: Mcnamara, John, dev@dpdk.org
In-Reply-To: <CAD+H990fau92xF1yKc=W72=pcNuw_v8fwTXdCVQ_eQPKod2zsQ@mail.gmail.com>

On 12/16/2016 5:29 PM, Alejandro Lucero wrote:
> Hi,
> 
> One question about this patch. I will send another patch soon which will
> require to modify the file created by this patch. So, should I use the
> dpdk-next for sending the new patch or the dpdk stable branch? 

I guess by "dpdk stable branch" you mean dpdk main repo, because we also
have stable sub-tree which is something else.

> I
> understand that using the latter will imply some integration later, but
> I really do not know if I should facilitate things using dpdk-next in
> this case. 

If the patch is driver patch, please send to the next-net sub-tree, as a
PMD maintainer, I expect majority of your patches should target next-net.

If patch just touches the documentation of the driver, you can send it
to the main tree, but both next-net sub-tree and main tree are OK since
PMD documentation is not heavily modified, integration will be (mostly)
easy.

Specific to the this document (feature.ini), since this is directly
correlated with PMD source code, to update this file, you need to update
the source code. And it is better to update this document in next-net as
part of the patchset that updates the PMD code.

> 
> By the way, it is not just about this specific patch, because I have
> other almost ready which I want to push before the 16.02 deadline.

Please push 17.02 patches as soon as possible, although there is
technically still some time for the integration deadline, practically
there is less because of holidays in between ...

> 
> 
<...>

^ permalink raw reply

* Re: [PATCH] doc: fix a mistake in prog guide
From: Mcnamara, John @ 2016-12-19 11:35 UTC (permalink / raw)
  To: Yong Wang; +Cc: dev@dpdk.org
In-Reply-To: <1482149678-18363-1-git-send-email-wang.yong19@zte.com.cn>



> -----Original Message-----
> From: Yong Wang [mailto:wang.yong19@zte.com.cn]
> Sent: Monday, December 19, 2016 12:15 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Yong Wang <wang.yong19@zte.com.cn>
> Subject: [PATCH] doc: fix a mistake in prog guide
> 
> Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>

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

^ permalink raw reply

* Re: [PATCH v5 23/29] app/testpmd: handle i40e in VF VLAN filter command
From: Ferruh Yigit @ 2016-12-19 11:13 UTC (permalink / raw)
  To: Vincent JARDIN, dev; +Cc: Jingjing Wu, Helin Zhang, Bernard Iremonger
In-Reply-To: <4ab71f12-5352-fa36-3691-929c8a2b8bbf@6wind.com>

On 12/16/2016 8:31 PM, Vincent JARDIN wrote:
> Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
>> +		ret = rte_pmd_ixgbe_set_vf_vlan_filter(res->port_id,
>> +				res->vlan_id, res->vf_mask, is_add);
>> +#endif
>> +#ifdef RTE_LIBRTE_I40E_PMD
>> +	if (strstr(dev_info.driver_name, "i40e") != NULL)
>> +		ret = rte_pmd_i40e_set_vf_vlan_filter(res->port_id,
>> +				res->vlan_id, res->vf_mask, is_add);
>> +#endif
> 
> That's exactly what we need to avoid, it won't scale to many PMDs.
> 

For a generic PMD feature, completely agree with you. Application
shouldn't know/worry about underlying hardware. eth_dev layer should be
used.
But above usage is for an application that knows the hardware, and
knowing that it is losing all the benefits of the portability and
explicitly including the PMD header to use those PMD specific APIs.

Thanks,
ferruh

^ permalink raw reply

* [PATCH] doc: fix a mistake in prog guide
From: Yong Wang @ 2016-12-19 12:14 UTC (permalink / raw)
  To: john.mcnamara; +Cc: dev, Yong Wang

Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
---
 doc/guides/prog_guide/vhost_lib.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
index 4f997d4..f0862e6 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -95,7 +95,7 @@ The following is an overview of the Vhost API functions:
     * for VM2NIC case, the ``nb_tx_desc`` has to be small enough: <= 64 if virtio
       indirect feature is not enabled and <= 128 if it is enabled.
 
-      The is because when dequeue zero copy is enabled, guest Tx used vring will
+      This is because when dequeue zero copy is enabled, guest Tx used vring will
       be updated only when corresponding mbuf is freed. Thus, the nb_tx_desc
       has to be small enough so that the PMD driver will run out of available
       Tx descriptors and free mbufs timely. Otherwise, guest Tx vring would be
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v2 02/25] doc: add rte_flow prog guide
From: Adrien Mazarguil @ 2016-12-19 11:10 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev@dpdk.org
In-Reply-To: <B27915DBBA3421428155699D51E4CFE202688A53@IRSMSX103.ger.corp.intel.com>

Hi John,

On Mon, Dec 19, 2016 at 10:45:32AM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Friday, December 16, 2016 4:25 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 02/25] doc: add rte_flow prog guide
> > 
> > This documentation is based on the latest RFC submission, subsequently
> > updated according to feedback from the community.
> 
> Hi,
> 
> Thanks. That is a very good doc.
> 
> A few RST comments.
> 
> Section headers should use the use the following underline formats:
> 
>     Level 1 Heading
>     ===============
> 
> 
>     Level 2 Heading
>     ---------------
> 
> 
>     Level 3 Heading
>     ~~~~~~~~~~~~~~~
> 
> 
>     Level 4 Heading
>     ^^^^^^^^^^^^^^^
> 
> See: http://dpdk.org/doc/guides/contributing/documentation.html#rst-guidelines

OK, although I do not see any mistake regarding this?

> Also, some of the section headers for Attributes, Patterns, Match and Action
> are a bit short and it isn't clear what section you are in, especially in the
> PDF doc. It might be clearer to add the section name before each item like:
> 
> 
>     Attribute: Group
>     ~~~~~~~~~~~~~~~~
> 
>     Match: VOID
>     ~~~~~~~~~~~
> 
> 
> Tables should have a reference link and a caption, like this:
> 
>     .. _table_qos_pipes:
> 
>     .. table:: Sample configuration for QOS pipes.
> 
>        +----------+----------+----------+
>        | Header 1 | Header 2 | Header 3 |
>        |          |          |          |
>        +==========+==========+==========+
>        | Text     | Text     | Text     |
>        +----------+----------+----------+
>        | ...      | ...      | ...      |
>        +----------+----------+----------+
> 
> 
> See: http://dpdk.org/doc/guides/contributing/documentation.html#tables
> 
> This will make the tables clearer when there are several in a row and will allow
> the text to refer to them with :numref:.
> 
> Also, there is one typo:
> 
> s/unpractically/impractically/
> 
> 
> Otherwise, very good work. A good clear document.

Thanks, I will fix these and re-submit.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH v5 19/29] app/testpmd: use unicast promiscuous mode on i40e
From: Ferruh Yigit @ 2016-12-19 11:05 UTC (permalink / raw)
  To: Vincent JARDIN, dev; +Cc: Jingjing Wu, Helin Zhang, Wenzhuo Lu
In-Reply-To: <e50661f8-0909-1fd4-5cb4-ac30e23ffdd2@6wind.com>

On 12/16/2016 8:28 PM, Vincent JARDIN wrote:
> Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
>> +#ifdef RTE_LIBRTE_I40E_PMD
>> +	ret = rte_pmd_i40e_set_vf_unicast_promisc(res->port_id,
>> +			res->vf_id, is_on);
>> +#endif
> 
> Why is an ifdef used here? It won't scale to all PMDs.

As explained in previous e-mail, because these are PMD specific APIs.
And it is not expected to scale to all PMDs.

> 
> I means that you are missing an abstraction layer.
> 

Yes, intentionally.

^ permalink raw reply

* Re: [PATCH v5 18/29] app/testpmd: use VFD APIs on i40e
From: Ferruh Yigit @ 2016-12-19 11:03 UTC (permalink / raw)
  To: Vincent JARDIN, dev
  Cc: Jingjing Wu, Helin Zhang, Wenzhuo Lu, Chen Jing D,
	Bernard Iremonger
In-Reply-To: <716260c7-4120-9984-3c46-02ad50b11a94@6wind.com>

On 12/16/2016 8:30 PM, Vincent JARDIN wrote:
> Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>>  			"set all queues drop (port_id) (on|off)\n"
>>  			"    Set drop enable bit for all queues.\n\n"
>>
>>  			"set vf split drop (port_id) (vf_id) (on|off)\n"
>>  			"    Set split drop enable bit for a VF from the PF.\n\n"
>> +#endif
> 
> it is not related to i40e. This serie should only be for i40e.

Please check the patch a little wider, it doesn't introduce the ifdef,
instead patch narrows down the scope of ifdef [1], because now some
functions are not just for ixgbe, but used both with i40e and ixgbe, so
the ifdef moved into the function itself [2].


[1]
-#ifdef RTE_LIBRTE_IXGBE_PMD
 	"set tx loopback (port_id) (on|off)\n"
 	"    Enable or disable tx loopback.\n\n"

+#ifdef RTE_LIBRTE_IXGBE_PMD
 	"set all queues drop (port_id) (on|off)\n"
 	"    Set drop enable bit for all queues.\n\n"

 	"set vf split drop (port_id) (vf_id) (on|off)\n"
 	"    Set split drop enable bit for a VF from the PF.\n\n"
+#endif

 	"set vf mac antispoof (port_id) (vf_id) (on|off).\n"
 	"    Set MAC antispoof for a VF from the PF.\n\n"
-#endif



[2]
@@ -11187,10 +11242,21 @@ cmd_set_tx_loopback_parsed(
...
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
+		ret = rte_pmd_ixgbe_set_tx_loopback(res->port_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (strstr(dev_info.driver_name, "i40e") != NULL)
+		ret = rte_pmd_i40e_set_tx_loopback(res->port_id, is_on);
+#endif

> 
> Moreover, it is a strange logic: how will it scale for all PMDs?

It won't.
That is why they are in ifdef.

These are PMD specific APIs, as naming convention shows "rte_pmd_i40e_..."

For the PMD features, which are not generic enough, but hardware is
capable and user may want to benefit from, PMD specific APIs exists.
Instead of these functions bloat the eth_dev layer, and give a fake
sense of abstraction, these APIs moved into PMDs.
And it is always possible to move these into ethdev layer, when multiple
PMDs supports same feature.
I agree this is something that needs to keep an eye on it, and be sure
if an API is generic, move it into eth_dev layer.

The application that use the PMD specific API, needs to be conditionally
compiled because that API may not always exits.

Thanks,
ferruh

^ permalink raw reply

* Re: [PATCH v2 03/25] doc: announce depreciation of legacy filter types
From: Mcnamara, John @ 2016-12-19 10:47 UTC (permalink / raw)
  To: Adrien Mazarguil, dev@dpdk.org
In-Reply-To: <0bb1ee6fb4f4f4193658a71275ec1e2027282f05.1481903839.git.adrien.mazarguil@6wind.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Friday, December 16, 2016 4:25 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 03/25] doc: announce depreciation of legacy
> filter types
> 
> They are superseded by the generic flow API (rte_flow). Target release is
> not defined yet.
> 
> Suggested-by: Kevin Traynor <ktraynor@redhat.com>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 2d17bc6..4819078 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -71,3 +71,10 @@ Deprecation Notices
>  * mempool: The functions for single/multi producer/consumer are
> deprecated
>    and will be removed in 17.02.
>    It is replaced by ``rte_mempool_generic_get/put`` functions.
> +
> +* ethdev: the legacy filter API, including
> +rte_eth_dev_filter_supported(),
> +  rte_eth_dev_filter_ctrl() as well as filter types MACVLAN, ETHERTYPE,
> +  FLEXIBLE, SYN, NTUPLE, TUNNEL, FDIR, HASH and L2_TUNNEL, is
> +superseded by
> +  the generic flow API (rte_flow) in PMDs that implement the latter.
> +  Target release for removal of the legacy API will be defined once
> +most
> +  PMDs have switched to rte_flow.


All the function names and constants should be fixed width quoted:
``rte_eth_dev_filter_supported()``.

^ permalink raw reply

* Re: [PATCH v2 02/25] doc: add rte_flow prog guide
From: Mcnamara, John @ 2016-12-19 10:45 UTC (permalink / raw)
  To: Adrien Mazarguil, dev@dpdk.org
In-Reply-To: <049b57d5216d8703bc5f2cdd29eabe40c2f09138.1481903839.git.adrien.mazarguil@6wind.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Friday, December 16, 2016 4:25 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 02/25] doc: add rte_flow prog guide
> 
> This documentation is based on the latest RFC submission, subsequently
> updated according to feedback from the community.

Hi,

Thanks. That is a very good doc.

A few RST comments.

Section headers should use the use the following underline formats:

    Level 1 Heading
    ===============


    Level 2 Heading
    ---------------


    Level 3 Heading
    ~~~~~~~~~~~~~~~


    Level 4 Heading
    ^^^^^^^^^^^^^^^

See: http://dpdk.org/doc/guides/contributing/documentation.html#rst-guidelines


Also, some of the section headers for Attributes, Patterns, Match and Action
are a bit short and it isn't clear what section you are in, especially in the
PDF doc. It might be clearer to add the section name before each item like:


    Attribute: Group
    ~~~~~~~~~~~~~~~~

    Match: VOID
    ~~~~~~~~~~~


Tables should have a reference link and a caption, like this:

    .. _table_qos_pipes:

    .. table:: Sample configuration for QOS pipes.

       +----------+----------+----------+
       | Header 1 | Header 2 | Header 3 |
       |          |          |          |
       +==========+==========+==========+
       | Text     | Text     | Text     |
       +----------+----------+----------+
       | ...      | ...      | ...      |
       +----------+----------+----------+


See: http://dpdk.org/doc/guides/contributing/documentation.html#tables

This will make the tables clearer when there are several in a row and will allow
the text to refer to them with :numref:.

Also, there is one typo:

s/unpractically/impractically/


Otherwise, very good work. A good clear document.

John

^ permalink raw reply

* Re: [PATCH v4] vmxnet3: fix Rx deadlock
From: Ferruh Yigit @ 2016-12-19 10:41 UTC (permalink / raw)
  To: Stefan Puiu, dev; +Cc: yongwang, mac_leehk
In-Reply-To: <1482140453-49649-1-git-send-email-stefan.puiu@gmail.com>

On 12/19/2016 9:40 AM, Stefan Puiu wrote:
> Our use case is that we have an app that needs to keep mbufs around
> for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from
> vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX
> descriptors (where it returns -err). Since there are no mbufs that the
> virtual hardware can use, no packets will be received after this; the
> driver won't refill the mbuf after this so it gets stuck in this
> state. I call this a deadlock for lack of a better term - the virtual
> HW waits for free mbufs, while the app waits for the hardware to
> notify it for data (by flipping the generation bit on the used Rx
> descriptors). Note that after this, the app can't recover.
> 
> This fix is a rework of this patch by Marco Lee:
> http://dpdk.org/dev/patchwork/patch/6575/. I had to forward port
> it, address review comments and also reverted the allocation
> failure handling to the first version of the patch
> (http://dpdk.org/ml/archives/dev/2015-July/022079.html), since
> that's the only approach that seems to work, and seems to be what
> other drivers are doing (I checked ixgbe and em). Reusing the mbuf
> that's getting passed to the application doesn't seem to make
> sense, and it was causing weird issues in our app. Also, reusing
> rxm without checking if it's NULL could cause the code to crash.
> 
> Signed-off-by: Stefan Puiu <stefan.puiu@gmail.com>

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

^ permalink raw reply

* Re: [PATCH v4] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-12-19 10:32 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev
In-Reply-To: <724e9785-3499-ac5a-bf7a-8093a649780c@intel.com>

On Mon, Dec 19, 2016 at 10:24 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 12/16/2016 5:10 PM, Alejandro Lucero wrote:
> > Previous reported speed was hardcoded because there was not firmware
> > support for getting this information. This change needs to support old
> > firmware versions, keeping with the hardcoded report, and the new
> > versions, where the firmware makes that information available.
> >
> > v4: Make conditional simple and more ellaborated commit comment.
> > v3: remove unsed macro
> > v2: use RTE_DIM instead of own macro
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > ---
> >  drivers/net/nfp/nfp_net.c      | 27 +++++++++++++++++++++++++--
> >  drivers/net/nfp/nfp_net_ctrl.h | 11 +++++++++++
> >  2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> <...>
> > @@ -831,8 +842,20 @@ 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 >= RTE_DIM(ls_to_ethtool)
>
> This is not compiling fine, missing parenthesis.
>

Sorry about that. It was so simple the change I did not do any test.



>
> > +                     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);
> <...>
>
>

^ 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