Re: [dpdk-dev] [PATCH 1/6] vhost: fix desc access in relay helpers

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Descs in desc table should be indexed using the desc idx
instead of the idx of avail ring and used ring.

Fixes: b13ad2decc83 ("vhost: provide helpers for virtio ring relay")

Signed-off-by: Tiwei Bie
---
  lib/librte_vhost/vdpa.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)


Good catch!

Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in multicast

[Zhao1, Wei]

> -Original Message-
> From: Zhang, Qi Z
> Sent: Thursday, January 3, 2019 9:47 PM
> To: Zhao1, Wei ; dev@dpdk.org
> Cc: sta...@dpdk.org; Wu, Jingjing ; Zhao1, Wei
> 
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in
> multicast
> 
> Hi Wei
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Wednesday, January 2, 2019 2:33 PM
> > To: dev@dpdk.org
> > Cc: sta...@dpdk.org; Wu, Jingjing ; Zhao1, Wei
> > 
> > Subject: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in
> > multicast
> 
> What is MAT means ?
> >
> > In ixgbe PMD code, all vf ars set with bit IXGBE_VMOLR_ROMPE, which
> > make vf accept packets that match the MTA table, if some vf update
> > IXGBE_MTA in function ixgbe_vf_set_multicast, then all vf will receive
> > packets from these address.
> > So thhere is need to set VMOLR register bit ROPE onlty after this vf
> > has been set multicast address. If this bit is when pf host doing
> > initialization, this vf will receive multicast packets with address
> > written in MTA table. Align to ixgbe pf kernel 5.3.7 code to fix this bug.
> 
> There are some typo in you commit log.

Sorry, v2 will commit.

> 
> >
> > Fixes: 00e30184daa0 ("ixgbe: add PF support")
> >
> > Signed-off-by: Wei Zhao 
> > ---
> >  drivers/net/ixgbe/ixgbe_pf.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> > b/drivers/net/ixgbe/ixgbe_pf.c index 4b833ff..0f4b96b 100644
> > --- a/drivers/net/ixgbe/ixgbe_pf.c
> > +++ b/drivers/net/ixgbe/ixgbe_pf.c
> > @@ -351,7 +351,7 @@ ixgbe_vf_reset_event(struct rte_eth_dev *dev,
> > uint16_t vf)
> > int rar_entry = hw->mac.num_rar_entries - (vf + 1);
> > uint32_t vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> >
> > -   vmolr |= (IXGBE_VMOLR_ROPE | IXGBE_VMOLR_ROMPE |
> > +   vmolr |= (IXGBE_VMOLR_ROPE |
> > IXGBE_VMOLR_BAM | IXGBE_VMOLR_AUPE);
> > IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> >
> > @@ -503,6 +503,7 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev,
> > uint32_t vf, uint32_t *msgbuf)
> > const uint32_t IXGBE_MTA_BIT_MASK = (0x1 <<
> IXGBE_MTA_BIT_SHIFT) -
> > 1;
> > uint32_t reg_val;
> > int i;
> > +   u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> >
> > /* Disable multicast promiscuous first */
> > ixgbe_disable_vf_mc_promisc(dev, vf); @@ -525,6 +526,9 @@
> > ixgbe_vf_set_multicast(struct rte_eth_dev *dev, uint32_t vf, uint32_t
> > *msgbuf)
> > IXGBE_WRITE_REG(hw, IXGBE_MTA(mta_idx), reg_val);
> > }
> >
> > +   vmolr |= IXGBE_VMOLR_ROMPE;
> > +   IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> 
> 
> Please correct me if I'm wrong
> 
> My understand is MTA table is shared by all VFs (I guess also pf), but what if

Yes, vf share it but not pf, it is used in vf pool switch

> two VFs both enable multi-cast but with different address filter?
> Should we prevent the second one to enable multi-cast if any conflict be
> detected? Otherwise there still will be unexpected behavior.

Sorry, I do not known what is the confict.
Because IXGBE_VMOLR is vf specific, that is to say, each vf control itself for 
enable ROMPE.


> 
> > +
> > return 0;
> >  }
> >
> > --
> > 2.7.5

Re: [dpdk-dev] [PATCH 2/6] vhost: fix possible out of bound access in relay helpers

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Fixes: b13ad2decc83 ("vhost: provide helpers for virtio ring relay")

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/vdpa.c | 27 ++-
  1 file changed, 22 insertions(+), 5 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 3/6] vhost: fix possible dead loop in relay helpers

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Fix a possible dead loop which may happen, e.g. when
driver created a loop in the desc list.

Fixes: b13ad2decc83 ("vhost: provide helpers for virtio ring relay")

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/vdpa.c | 16 
  1 file changed, 16 insertions(+)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 4/6] vhost: fix possible out of bound access in vector filling

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Fixes: 7f74b95c444f ("vhost: pre update used ring for Tx and Rx")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/virtio_net.c | 3 +++
  1 file changed, 3 insertions(+)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 5/6] vhost: fix possible dead loop in vector filling

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Fix a possible dead loop which may happen, e.g. when driver
created a loop in the desc list and lens in descs are zero.

Fixes: fd68b4739d2c ("vhost: use buffer vectors in dequeue path")
Fixes: 2f3225a7d69b ("vhost: add vector filling support for packed ring")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/virtio_net.c | 13 +
  1 file changed, 13 insertions(+)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 6/6] vhost: ensure event idx is mapped when negotiated

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Fixes: 30920b1e2b15 ("vhost: ensure all range is mapped when translating QVAs")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/vhost_user.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

[dpdk-dev] [PATCH v2] net/ixgbe: fix multicast table enable error for VF

[Wei Zhao]

In ixgbe PMD code, all vf ars set with bit IXGBE_VMOLR_ROMPE,
which make vf accept packets that match the MTA table,
if some vf update IXGBE_MTA in function ixgbe_vf_set_multicast,
then all vf will receive packets from these address.
So thhere is need to set VMOLR register bit ROPE only after this
vf has been set multicast address. If this bit is set when pf host doing
initialization, this vf will receive multicast packets with address
written in MTA table. Align to ixgbe pf kernel 5.3.7 code to fix this
bug.

Fixes: 00e30184daa0 ("ixgbe: add PF support")

Signed-off-by: Wei Zhao 

---

v2:
change patch name and fix typo in log.
---
 drivers/net/ixgbe/ixgbe_pf.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 4b833ff..0f4b96b 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -351,7 +351,7 @@ ixgbe_vf_reset_event(struct rte_eth_dev *dev, uint16_t vf)
int rar_entry = hw->mac.num_rar_entries - (vf + 1);
uint32_t vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
 
-   vmolr |= (IXGBE_VMOLR_ROPE | IXGBE_VMOLR_ROMPE |
+   vmolr |= (IXGBE_VMOLR_ROPE |
IXGBE_VMOLR_BAM | IXGBE_VMOLR_AUPE);
IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
 
@@ -503,6 +503,7 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev, uint32_t 
vf, uint32_t *msgbuf)
const uint32_t IXGBE_MTA_BIT_MASK = (0x1 << IXGBE_MTA_BIT_SHIFT) - 1;
uint32_t reg_val;
int i;
+   u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
 
/* Disable multicast promiscuous first */
ixgbe_disable_vf_mc_promisc(dev, vf);
@@ -525,6 +526,9 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev, uint32_t 
vf, uint32_t *msgbuf)
IXGBE_WRITE_REG(hw, IXGBE_MTA(mta_idx), reg_val);
}
 
+   vmolr |= IXGBE_VMOLR_ROMPE;
+   IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
+
return 0;
 }
 
-- 
2.7.5

[dpdk-dev] [PATCH v2] doc: fix a parameter name in testpmd guide

[Rami Rosen]

There is no parameter called "eth-peer-configfile" in testpmd. It should
be "eth-peers-configfile". See the usage() method in 
app/test-pmd/parameters.c.

v2: Fix line too long in the patch commit log.

Fixes: a67857e97ba8 ("doc: clarify usage of testpmd MAC forward mode")
Cc: sta...@dpdk.org

Signed-off-by: Rami Rosen 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index cbf23e928..1ceb000da 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -304,7 +304,7 @@ The available information categories are:
 * ``mac``: Changes the source and the destination Ethernet addresses of 
packets before forwarding them.
   Default application behaviour is to set source Ethernet address to that of 
the transmitting interface, and destination
   address to a dummy value (set during init). The user may specify a target 
destination Ethernet address via the 'eth-peer' or
-  'eth-peer-configfile' command-line options. It is not currently possible to 
specify a specific source Ethernet address.
+  'eth-peers-configfile' command-line options. It is not currently possible to 
specify a specific source Ethernet address.
 
 * ``macswap``: MAC swap forwarding mode.
   Swaps the source and the destination Ethernet addresses of packets before 
forwarding them.
-- 
2.19.2

[dpdk-dev] [PATCH v2] doc: fix a parameter name in testpmd guide

[Rami Rosen]

There is no parameter called "eth-peer-configfile" in testpmd. It should
be "eth-peers-configfile". See the usage() method in 
app/test-pmd/parameters.c.

v2: Fix line too long in the patch commit log.

Fixes: a67857e97ba8 ("doc: clarify usage of testpmd MAC forward mode")
Cc: sta...@dpdk.org

Signed-off-by: Rami Rosen 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index cbf23e928..1ceb000da 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -304,7 +304,7 @@ The available information categories are:
 * ``mac``: Changes the source and the destination Ethernet addresses of 
packets before forwarding them.
   Default application behaviour is to set source Ethernet address to that of 
the transmitting interface, and destination
   address to a dummy value (set during init). The user may specify a target 
destination Ethernet address via the 'eth-peer' or
-  'eth-peer-configfile' command-line options. It is not currently possible to 
specify a specific source Ethernet address.
+  'eth-peers-configfile' command-line options. It is not currently possible to 
specify a specific source Ethernet address.
 
 * ``macswap``: MAC swap forwarding mode.
   Swaps the source and the destination Ethernet addresses of packets before 
forwarding them.
-- 
2.19.2

Re: [dpdk-dev] [PATCH v6 01/10] cryptodev: add opaque userdata pointer into crypto sym session

[Ananyev, Konstantin]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, January 4, 2019 12:26 AM
> To: Ananyev, Konstantin 
> Cc: dev@dpdk.org; akhil.go...@nxp.com
> Subject: Re: [dpdk-dev] [PATCH v6 01/10] cryptodev: add opaque userdata 
> pointer into crypto sym session
> 
> On Thu,  3 Jan 2019 20:16:17 +
> Konstantin Ananyev  wrote:
> 
> > Add 'uint64_t opaque_data' inside struct rte_cryptodev_sym_session.
> > That allows upper layer to easily associate some user defined
> > data with the session.
> >
> > Signed-off-by: Konstantin Ananyev 
> > Acked-by: Fiona Trahe 
> > Acked-by: Mohammad Abdul Awal 
> > Acked-by: Declan Doherty 
> > Acked-by: Akhil Goyal 
> > ---
> >  lib/librte_cryptodev/rte_cryptodev.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
> > b/lib/librte_cryptodev/rte_cryptodev.h
> > index 4099823f1..009860e7b 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.h
> > +++ b/lib/librte_cryptodev/rte_cryptodev.h
> > @@ -954,6 +954,8 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t 
> > qp_id,
> >   * has a fixed algo, key, op-type, digest_len etc.
> >   */
> >  struct rte_cryptodev_sym_session {
> > +   uint64_t opaque_data;
> > +   /**< Opaque user defined data */
> > __extension__ void *sess_private_data[0];
> > /**< Private symmetric session material */
> >  };
> 
> This will cause ABI breakage.

Yes, it surely would.
That's why we submitted deprecation notice in 18.11 and got 3 acks for it.
Konstantin

Re: [dpdk-dev] [PATCH v2] doc: fix a parameter name in testpmd guide

[Iremonger, Bernard]

> -Original Message-
> From: Rami Rosen [mailto:ramir...@gmail.com]
> Sent: Friday, January 4, 2019 9:11 AM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Iremonger, Bernard ;
> Rami Rosen 
> Subject: [PATCH v2] doc: fix a parameter name in testpmd guide
> 
> There is no parameter called "eth-peer-configfile" in testpmd. It should be 
> "eth-
> peers-configfile". See the usage() method in app/test-pmd/parameters.c.
> 
> v2: Fix line too long in the patch commit log.
> 
> Fixes: a67857e97ba8 ("doc: clarify usage of testpmd MAC forward mode")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Rami Rosen 

Acked-by: Bernard Iremonger 

Re: [dpdk-dev] [PATCH] app/test-pmd: stop all ports before any close

[Iremonger, Bernard]

Hi Cristian,

> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Thursday, January 3, 2019 4:37 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo ; Wu, Jingjing
> ; Iremonger, Bernard 
> Subject: [PATCH] app/test-pmd: stop all ports before any close

./devtools/check-git-log.sh -1
Wrong headline label:
app/test-pmd: stop all ports before any close

Should be app/testpmd

The commit message should also have the "fix" keyword

> 
> This patch proposes a slightly different test-pmd quit operation: stop all 
> devices
> before starting to close any device. Basically, stop all moving parts before
> beginning to remove them. The current test-pmd quit is stoping and closing 
> each
> device before moving to the next device.
> 
> If all devices in the system are independent of each other, this difference is
> usually not important. In case of Soft NIC devices, any such virtual device
> typically depends on one or more physical devices being alive, as it accesses
> their queues, so this difference becomes important.
> 
> Without this straightforward fix, all the Soft NIC devices need to be manually
> stopped before the quit command is issued, otherwise the quit command can
> sometimes crash the test-pmd application.

There should be a fixes line before the sign-off, and maybe a " Cc: 
sta...@dpdk.org" line
 
> Signed-off-by: Cristian Dumitrescu 
> ---
>  app/test-pmd/testpmd.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 8d584b008..15a948828 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2391,9 +2391,13 @@ pmd_test_exit(void)
>   if (ports != NULL) {
>   no_link_check = 1;
>   RTE_ETH_FOREACH_DEV(pt_id) {
> - printf("\nShutting down port %d...\n", pt_id);
> + printf("\nStopping port %d...\n", pt_id);
>   fflush(stdout);
>   stop_port(pt_id);
> + }
> + RTE_ETH_FOREACH_DEV(pt_id) {
> + printf("\nShutting down port %d...\n", pt_id);
> + fflush(stdout);
>   close_port(pt_id);
> 
>   /*
> --
> 2.17.1

Regards,

Bernard.

Re: [dpdk-dev] [PATCH v3] mk: fix scope of disabling AVX512F support

[Ferruh Yigit]

On 1/4/2019 2:40 AM, Varghese, Vipin wrote:
> Hi Ferruh,
> 
> Should not be there a documentation update in 'Known Issues and Limitations 
> in Legacy Releases' for the limitation. If it is already added can you please 
> add the patchwork link for the same.

It is good idea to document this limitation.
Previous limitation was documented in 18.11 release notes, I can move it into
known_issues.rst, and put updated limitation into 19.02 release notes.

> 
> Thanks
> Vipin Varghese

Re: [dpdk-dev] [PATCH v3] mk: fix scope of disabling AVX512F support

[Varghese, Vipin]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Friday, January 4, 2019 3:58 PM
> To: Varghese, Vipin ; Yongseok Koh
> 
> Cc: Thomas Monjalon ; dev ; dpdk
> stable ; Tom Barbette ; Ananyev,
> Konstantin ; Richardson, Bruce
> 
> Subject: Re: [PATCH v3] mk: fix scope of disabling AVX512F support
> 
> On 1/4/2019 2:40 AM, Varghese, Vipin wrote:
> > Hi Ferruh,
> >
> > Should not be there a documentation update in 'Known Issues and
> Limitations in Legacy Releases' for the limitation. If it is already added 
> can you
> please add the patchwork link for the same.
> 
> It is good idea to document this limitation.
> Previous limitation was documented in 18.11 release notes, I can move it into
> known_issues.rst, and put updated limitation into 19.02 release notes.
Thanks Ferruh for understanding

> 
> >
> > Thanks
> > Vipin Varghese
> 

[dpdk-dev] [Bug 179] mp_socket is not well handled when secondary process is exited

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=179

Bug ID: 179
   Summary: mp_socket is not well handled when secondary process
is exited
   Product: DPDK
   Version: 18.11
  Hardware: x86
OS: Linux
Status: CONFIRMED
  Severity: critical
  Priority: Normal
 Component: other
  Assignee: dev@dpdk.org
  Reporter: danielbenl...@gmail.com
  Target Milestone: ---

I am running dpdk-pdump and found the mp socket (see below output) it created
is not deleted when dpdk-pdump is exited. Now it can work because when
dpdk-dump call rte_eal_init(), rte_bus_scan() will be called and the file will
be removed. The path rte_bus_scan() -> bus->scan() -> vdev_scan() ->
rte_mp_request_sync() -> mp_request_sync() -> send_msg() -> sendmsg() will
eventually delete the mp_socket_* which is retained by last run.


root@n14-045-219:~# ls /var/run/dpdk/rte/mp_socket_*
/var/run/dpdk/rte/mp_socket_21695_4467ff77b19ec2


I do think this is not expected. When a secondary process is exited, the mp
socket file should be cleared in some way.

You can have the following way to reproduce it.
1. You need to change dpdk-pdump to call rte_eal_hotplug_remove("vdev",
vdev_name) when dpdk-pdump is exited. Otherwise dpdk-pdump can only run once as
vdev is not freed.
2. Run dpdk-pdump twice with testpmd, you will see the second run will fail
because of it send request to the mp_socket which is retained by last run.
testpmd command:  testpmd -l 0-1 -n 4 --master-lcore 0 -- -a
pdump comman: dpdk-pdump -- --pdump 'port=0,queue=*,rx-dev=/tmp/rx.pcap'

Please take a look at this and fix it.

Thanks,
Daniel

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [dpdk-dev] [PATCH] app/test-pmd: stop all ports before any close

[Dumitrescu, Cristian]

Hi Bernard,

Thanks for reviewing this patch.

> -Original Message-
> From: Iremonger, Bernard
> Sent: Friday, January 4, 2019 10:16 AM
> To: Dumitrescu, Cristian ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Wu, Jingjing
> 
> Subject: RE: [PATCH] app/test-pmd: stop all ports before any close
> 
> Hi Cristian,
> 
> > -Original Message-
> > From: Dumitrescu, Cristian
> > Sent: Thursday, January 3, 2019 4:37 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo ; Wu, Jingjing
> > ; Iremonger, Bernard
> 
> > Subject: [PATCH] app/test-pmd: stop all ports before any close
> 
> ./devtools/check-git-log.sh -1
> Wrong headline label:
> app/test-pmd: stop all ports before any close
> 
> Should be app/testpmd

Will fix.

> 
> The commit message should also have the "fix" keyword
> 

We can treat this as a fix or as an improvement, I was not sure about what 
people think. I don't mind either way. Will treat this as a fix then in v2.

> >
> > This patch proposes a slightly different test-pmd quit operation: stop all
> devices
> > before starting to close any device. Basically, stop all moving parts before
> > beginning to remove them. The current test-pmd quit is stoping and closing
> each
> > device before moving to the next device.
> >
> > If all devices in the system are independent of each other, this difference 
> > is
> > usually not important. In case of Soft NIC devices, any such virtual device
> > typically depends on one or more physical devices being alive, as it 
> > accesses
> > their queues, so this difference becomes important.
> >
> > Without this straightforward fix, all the Soft NIC devices need to be
> manually
> > stopped before the quit command is issued, otherwise the quit command
> can
> > sometimes crash the test-pmd application.
> 
> There should be a fixes line before the sign-off, and maybe a " Cc:
> sta...@dpdk.org" line
> 
> > Signed-off-by: Cristian Dumitrescu 
> > ---
> >  app/test-pmd/testpmd.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 8d584b008..15a948828 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2391,9 +2391,13 @@ pmd_test_exit(void)
> > if (ports != NULL) {
> > no_link_check = 1;
> > RTE_ETH_FOREACH_DEV(pt_id) {
> > -   printf("\nShutting down port %d...\n", pt_id);
> > +   printf("\nStopping port %d...\n", pt_id);
> > fflush(stdout);
> > stop_port(pt_id);
> > +   }
> > +   RTE_ETH_FOREACH_DEV(pt_id) {
> > +   printf("\nShutting down port %d...\n", pt_id);
> > +   fflush(stdout);
> > close_port(pt_id);
> >
> > /*
> > --
> > 2.17.1
> 
> Regards,
> 
> Bernard.

Regards,
Cristian

[dpdk-dev] [PATCH 0/2] vhost/crypto: some fixes

[Fan Zhang]

This patchset fixes two possible issues in vhost crypto. They are:
- The possible dead loop issue
- The out of bound access issue.

The fixes are in line with the following patchset
"[dpdk-dev] [PATCH 0/6] Some fixes for vhost"
(https://mails.dpdk.org/archives/dev/2019-January/122372.html)

Fan Zhang (2):
  vhost/crypto: fix possible dead loop
  vhost/crypto: fix possible out of bound access

 lib/librte_vhost/vhost_crypto.c | 140 
 1 file changed, 100 insertions(+), 40 deletions(-)

-- 
2.13.6

[dpdk-dev] [PATCH 1/2] vhost/crypto: fix possible dead loop

[Fan Zhang]

This patch fixes a possible infinite loop caused by incorrect
descriptor chain created by the driver.

Cc: sta...@dpdk.org
Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")

Signed-off-by: Fan Zhang 
---
 lib/librte_vhost/vhost_crypto.c | 121 +++-
 1 file changed, 82 insertions(+), 39 deletions(-)

diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index dd01afc08..80b83ef77 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -466,12 +466,17 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 }
 
 static __rte_always_inline struct vring_desc *
-find_write_desc(struct vring_desc *head, struct vring_desc *desc)
+find_write_desc(struct vring_desc *head, struct vring_desc *desc,
+   uint32_t *nb_descs)
 {
if (desc->flags & VRING_DESC_F_WRITE)
return desc;
 
while (desc->flags & VRING_DESC_F_NEXT) {
+   if (unlikely(*nb_descs == 0))
+   return NULL;
+   (*nb_descs)--;
+
desc = &head[desc->next];
if (desc->flags & VRING_DESC_F_WRITE)
return desc;
@@ -481,13 +486,18 @@ find_write_desc(struct vring_desc *head, struct 
vring_desc *desc)
 }
 
 static struct virtio_crypto_inhdr *
-reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vring_desc *desc)
+reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vring_desc *desc,
+   uint32_t *nb_descs)
 {
uint64_t dlen;
struct virtio_crypto_inhdr *inhdr;
 
-   while (desc->flags & VRING_DESC_F_NEXT)
+   while (desc->flags & VRING_DESC_F_NEXT) {
+   if (unlikely(*nb_descs == 0))
+   return NULL;
+   (*nb_descs)--;
desc = &vc_req->head[desc->next];
+   }
 
dlen = desc->len;
inhdr = IOVA_TO_VVA(struct virtio_crypto_inhdr *, vc_req, desc->addr,
@@ -500,15 +510,16 @@ reach_inhdr(struct vhost_crypto_data_req *vc_req, struct 
vring_desc *desc)
 
 static __rte_always_inline int
 move_desc(struct vring_desc *head, struct vring_desc **cur_desc,
-   uint32_t size)
+   uint32_t size, uint32_t *nb_descs)
 {
struct vring_desc *desc = *cur_desc;
-   int left = size;
-
-   rte_prefetch0(&head[desc->next]);
-   left -= desc->len;
+   int left = size - desc->len;
 
while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
+   (*nb_descs)--;
+   if (unlikely(*nb_descs == 0))
+   return -1;
+
desc = &head[desc->next];
rte_prefetch0(&head[desc->next]);
left -= desc->len;
@@ -517,7 +528,10 @@ move_desc(struct vring_desc *head, struct vring_desc 
**cur_desc,
if (unlikely(left > 0))
return -1;
 
-   *cur_desc = &head[desc->next];
+   if (unlikely(*nb_descs == 0))
+   *cur_desc = NULL;
+   else
+   *cur_desc = &head[desc->next];
return 0;
 }
 
@@ -539,7 +553,7 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req, struct 
vring_desc *cur_desc,
 
 static int
 copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
-   struct vring_desc **cur_desc, uint32_t size)
+   struct vring_desc **cur_desc, uint32_t size, uint32_t *nb_descs)
 {
struct vring_desc *desc = *cur_desc;
uint64_t remain, addr, dlen, len;
@@ -548,7 +562,6 @@ copy_data(void *dst_data, struct vhost_crypto_data_req 
*vc_req,
uint8_t *src;
int left = size;
 
-   rte_prefetch0(&vc_req->head[desc->next]);
to_copy = RTE_MIN(desc->len, (uint32_t)left);
dlen = to_copy;
src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
@@ -582,6 +595,12 @@ copy_data(void *dst_data, struct vhost_crypto_data_req 
*vc_req,
left -= to_copy;
 
while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
+   if (unlikely(*nb_descs == 0)) {
+   VC_LOG_ERR("Invalid descriptors");
+   return -1;
+   }
+   (*nb_descs)--;
+
desc = &vc_req->head[desc->next];
rte_prefetch0(&vc_req->head[desc->next]);
to_copy = RTE_MIN(desc->len, (uint32_t)left);
@@ -624,7 +643,10 @@ copy_data(void *dst_data, struct vhost_crypto_data_req 
*vc_req,
return -1;
}
 
-   *cur_desc = &vc_req->head[desc->next];
+   if (unlikely(*nb_descs == 0))
+   *cur_desc = NULL;
+   else
+   *cur_desc = &vc_req->head[desc->next];
 
return 0;
 }
@@ -684,7 +706,8 @@ prepare_write_back_data(struct vhost_crypto_data_req 
*vc_req,
struct vhost_crypto_writeback_data **end_wb_data,
uint8_t *src,
uint32_t offset,
-   uint64_t write_back_len)
+   uint64_t write_back_len,
+  

[dpdk-dev] [PATCH 2/2] vhost/crypto: fix possible out of bound access

[Fan Zhang]

This patch fixes a out of bound access possbility in vhost
crypto. Originally the incorrect next descriptor index may
cause the library read invalid memory content and crash
the application.

Cc: sta...@dpdk.org
Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")

Signed-off-by: Fan Zhang 
---
 lib/librte_vhost/vhost_crypto.c | 89 -
 1 file changed, 53 insertions(+), 36 deletions(-)

diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index 80b83ef77..0694c0a74 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -467,13 +467,13 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 
 static __rte_always_inline struct vring_desc *
 find_write_desc(struct vring_desc *head, struct vring_desc *desc,
-   uint32_t *nb_descs)
+   uint32_t *nb_descs, uint32_t vq_size)
 {
if (desc->flags & VRING_DESC_F_WRITE)
return desc;
 
while (desc->flags & VRING_DESC_F_NEXT) {
-   if (unlikely(*nb_descs == 0))
+   if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
return NULL;
(*nb_descs)--;
 
@@ -487,13 +487,13 @@ find_write_desc(struct vring_desc *head, struct 
vring_desc *desc,
 
 static struct virtio_crypto_inhdr *
 reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vring_desc *desc,
-   uint32_t *nb_descs)
+   uint32_t *nb_descs, uint32_t vq_size)
 {
uint64_t dlen;
struct virtio_crypto_inhdr *inhdr;
 
while (desc->flags & VRING_DESC_F_NEXT) {
-   if (unlikely(*nb_descs == 0))
+   if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
return NULL;
(*nb_descs)--;
desc = &vc_req->head[desc->next];
@@ -510,14 +510,14 @@ reach_inhdr(struct vhost_crypto_data_req *vc_req, struct 
vring_desc *desc,
 
 static __rte_always_inline int
 move_desc(struct vring_desc *head, struct vring_desc **cur_desc,
-   uint32_t size, uint32_t *nb_descs)
+   uint32_t size, uint32_t *nb_descs, uint32_t vq_size)
 {
struct vring_desc *desc = *cur_desc;
int left = size - desc->len;
 
while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
(*nb_descs)--;
-   if (unlikely(*nb_descs == 0))
+   if (unlikely(*nb_descs == 0 || desc->next >= vq_size))
return -1;
 
desc = &head[desc->next];
@@ -530,8 +530,12 @@ move_desc(struct vring_desc *head, struct vring_desc 
**cur_desc,
 
if (unlikely(*nb_descs == 0))
*cur_desc = NULL;
-   else
+   else {
+   if (unlikely(desc->next >= vq_size))
+   return -1;
*cur_desc = &head[desc->next];
+   }
+
return 0;
 }
 
@@ -553,7 +557,8 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req, struct 
vring_desc *cur_desc,
 
 static int
 copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
-   struct vring_desc **cur_desc, uint32_t size, uint32_t *nb_descs)
+   struct vring_desc **cur_desc, uint32_t size,
+   uint32_t *nb_descs, uint32_t vq_size)
 {
struct vring_desc *desc = *cur_desc;
uint64_t remain, addr, dlen, len;
@@ -595,7 +600,7 @@ copy_data(void *dst_data, struct vhost_crypto_data_req 
*vc_req,
left -= to_copy;
 
while ((desc->flags & VRING_DESC_F_NEXT) && left > 0) {
-   if (unlikely(*nb_descs == 0)) {
+   if (unlikely(*nb_descs == 0 || desc->next >= vq_size)) {
VC_LOG_ERR("Invalid descriptors");
return -1;
}
@@ -645,8 +650,11 @@ copy_data(void *dst_data, struct vhost_crypto_data_req 
*vc_req,
 
if (unlikely(*nb_descs == 0))
*cur_desc = NULL;
-   else
+   else {
+   if (unlikely(desc->next >= vq_size))
+   return -1;
*cur_desc = &vc_req->head[desc->next];
+   }
 
return 0;
 }
@@ -657,7 +665,6 @@ write_back_data(struct vhost_crypto_data_req *vc_req)
struct vhost_crypto_writeback_data *wb_data = vc_req->wb, *wb_last;
 
while (wb_data) {
-   rte_prefetch0(wb_data->next);
rte_memcpy(wb_data->dst, wb_data->src, wb_data->len);
wb_last = wb_data;
wb_data = wb_data->next;
@@ -707,7 +714,7 @@ prepare_write_back_data(struct vhost_crypto_data_req 
*vc_req,
uint8_t *src,
uint32_t offset,
uint64_t write_back_len,
-   uint32_t *nb_descs)
+   uint32_t *nb_descs, uint32_t vq_size)
 {
struct vhost_crypto_writeback_data *wb_data, *head;
struct vring_desc *desc = *cur_desc;
@@ -754,7 +761,7 @@ prepare_write_back_data(struct vhost_crypto_data_req 
*vc_req,

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Akhil Goyal]

Hi Konstantin,

With this patchset, I am seeing a 3% drop in legacy mode lookaside and 
12% drop with -l option.
I am debugging this. Will let you know if I find something.

-Akhil

On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
> This patch series depends on the patch series:
>
> ipsec: new library for IPsec data-path processing
> http://patches.dpdk.org/patch/49410/
> http://patches.dpdk.org/patch/49411/
> http://patches.dpdk.org/patch/49412/
> http://patches.dpdk.org/patch/49413/
> http://patches.dpdk.org/patch/49414/
> http://patches.dpdk.org/patch/49415/
> http://patches.dpdk.org/patch/49416/
> http://patches.dpdk.org/patch/49417/
> http://patches.dpdk.org/patch/49418/
> http://patches.dpdk.org/patch/49419/
>
> to be applied first.
>
> v5 -> v6
>Address issues reported by Akhil:
>   segfault when using lookaside-proto device
>   HW IPv4 cksum offload not enabled by default
>   crypto-dev dequeue() is called to often
>
> v4 -> v5
> - Address Akhil comments:
>   documentation update
>   spell checks spacing etc.
>   introduce rxoffload/txoffload parameters
>   single SA for ipv6
>   update Makefile
>
> v3 -> v4
>   - fix few issues with the test scripts
>   - update docs
>
> v2 -> v3
>   - add IPv6 cases into test scripts
>   - fixes for IPv6 support
>   - fixes for inline-crypto support
>   - some code restructuring
>
> v1 -> v2
>   - Several bug fixes
>
> That series contians few bug-fixes and changes to make ipsec-secgw
> to utilize librte_ipsec library:
>   - changes in the related data structures.
>   - changes in the initialization code.
>   - changes in the data-path code.
>   - new command-line parameters to enable librte_ipsec codepath
> and related features.
>   - test scripts to help automate ipsec-secgw functional testing.
>
> Note that right now by default current (non-librte_ipsec) code-path
> will be used. User has to run application with new command-line option
> ('-l')
> to enable new codepath.
> The main reason for that:
>- current librte_ipsec doesn't support all ipsec algorithms
>  and features that the app does.
>- allow users to run both versions in parallel for some time
>  to figure out any functional or performance degradation with the
>  new code.
>
> Test scripts were run with the following crypto devices:
>   - aesni_mb
>   - aesni_gcm
>   - qat
>
> Konstantin Ananyev (10):
>examples/ipsec-secgw: allow user to disable some RX/TX offloads
>examples/ipsec-secgw: allow to specify neighbour mac address
>examples/ipsec-secgw: fix crypto-op might never get dequeued
>examples/ipsec-secgw: fix outbound codepath for single SA
>examples/ipsec-secgw: make local variables static
>examples/ipsec-secgw: fix inbound SA checking
>examples/ipsec-secgw: make app to use ipsec library
>examples/ipsec-secgw: make data-path to use ipsec library
>examples/ipsec-secgw: add scripts for functional test
>doc: update ipsec-secgw guide and relelase notes
>
>   doc/guides/rel_notes/release_19_02.rst|  14 +
>   doc/guides/sample_app_ug/ipsec_secgw.rst  | 159 +-
>   examples/ipsec-secgw/Makefile |   5 +-
>   examples/ipsec-secgw/ipsec-secgw.c| 480 ++
>   examples/ipsec-secgw/ipsec.c  | 101 ++--
>   examples/ipsec-secgw/ipsec.h  |  67 +++
>   examples/ipsec-secgw/ipsec_process.c  | 357 +
>   examples/ipsec-secgw/meson.build  |   6 +-
>   examples/ipsec-secgw/parser.c |  91 
>   examples/ipsec-secgw/parser.h |   8 +-
>   examples/ipsec-secgw/sa.c | 263 +-
>   examples/ipsec-secgw/sp4.c|  35 +-
>   examples/ipsec-secgw/sp6.c|  35 +-
>   examples/ipsec-secgw/test/common_defs.sh  | 153 ++
>   examples/ipsec-secgw/test/data_rxtx.sh|  62 +++
>   examples/ipsec-secgw/test/linux_test4.sh  |  63 +++
>   examples/ipsec-secgw/test/linux_test6.sh  |  64 +++
>   examples/ipsec-secgw/test/run_test.sh |  80 +++
>   .../test/trs_aescbc_sha1_common_defs.sh   |  69 +++
>   .../ipsec-secgw/test/trs_aescbc_sha1_defs.sh  |  67 +++
>   .../test/trs_aescbc_sha1_esn_atom_defs.sh |   5 +
>   .../test/trs_aescbc_sha1_esn_defs.sh  |  66 +++
>   .../test/trs_aescbc_sha1_old_defs.sh  |   5 +
>   .../test/trs_aesgcm_common_defs.sh|  60 +++
>   examples/ipsec-secgw/test/trs_aesgcm_defs.sh  |  66 +++
>   .../test/trs_aesgcm_esn_atom_defs.sh  |   5 +
>   .../ipsec-secgw/test/trs_aesgcm_esn_defs.sh   |  66 +++
>   .../ipsec-secgw/test/trs_aesgcm_old_defs.sh   |   5 +
>   .../test/tun_aescbc_sha1_common_defs.sh   |  68 +++
>   .../ipsec-secgw/test/tun_aescbc_sha1_defs.sh  |  70 +++
>   .../test/tun_aescbc_sha1_esn_atom_defs.sh |   5 +
>   .../test/tun_aescbc_sha1_esn_defs.sh  |  70 +++
>   .../test/tun_aesc

Re: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in multicast

[Zhang, Qi Z]

> -Original Message-
> From: Zhao1, Wei
> Sent: Friday, January 4, 2019 4:35 PM
> To: Zhang, Qi Z ; dev@dpdk.org
> Cc: sta...@dpdk.org; Wu, Jingjing 
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in multicast
> 
> 
> 
> > -Original Message-
> > From: Zhang, Qi Z
> > Sent: Thursday, January 3, 2019 9:47 PM
> > To: Zhao1, Wei ; dev@dpdk.org
> > Cc: sta...@dpdk.org; Wu, Jingjing ; Zhao1, Wei
> > 
> > Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in
> > multicast
> >
> > Hi Wei
> >
> > > -Original Message-
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> > > Sent: Wednesday, January 2, 2019 2:33 PM
> > > To: dev@dpdk.org
> > > Cc: sta...@dpdk.org; Wu, Jingjing ; Zhao1,
> > > Wei 
> > > Subject: [dpdk-dev] [PATCH] net/ixgbe: fix MAT enable for VF in
> > > multicast
> >
> > What is MAT means ?
> > >
> > > In ixgbe PMD code, all vf ars set with bit IXGBE_VMOLR_ROMPE, which
> > > make vf accept packets that match the MTA table, if some vf update
> > > IXGBE_MTA in function ixgbe_vf_set_multicast, then all vf will
> > > receive packets from these address.
> > > So thhere is need to set VMOLR register bit ROPE onlty after this vf
> > > has been set multicast address. If this bit is when pf host doing
> > > initialization, this vf will receive multicast packets with address
> > > written in MTA table. Align to ixgbe pf kernel 5.3.7 code to fix this bug.
> >
> > There are some typo in you commit log.
> 
> Sorry, v2 will commit.
> 
> >
> > >
> > > Fixes: 00e30184daa0 ("ixgbe: add PF support")
> > >
> > > Signed-off-by: Wei Zhao 
> > > ---
> > >  drivers/net/ixgbe/ixgbe_pf.c | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> > > b/drivers/net/ixgbe/ixgbe_pf.c index 4b833ff..0f4b96b 100644
> > > --- a/drivers/net/ixgbe/ixgbe_pf.c
> > > +++ b/drivers/net/ixgbe/ixgbe_pf.c
> > > @@ -351,7 +351,7 @@ ixgbe_vf_reset_event(struct rte_eth_dev *dev,
> > > uint16_t vf)
> > >   int rar_entry = hw->mac.num_rar_entries - (vf + 1);
> > >   uint32_t vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> > >
> > > - vmolr |= (IXGBE_VMOLR_ROPE | IXGBE_VMOLR_ROMPE |
> > > + vmolr |= (IXGBE_VMOLR_ROPE |
> > >   IXGBE_VMOLR_BAM | IXGBE_VMOLR_AUPE);
> > >   IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> > >
> > > @@ -503,6 +503,7 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev,
> > > uint32_t vf, uint32_t *msgbuf)
> > >   const uint32_t IXGBE_MTA_BIT_MASK = (0x1 <<
> > IXGBE_MTA_BIT_SHIFT) -
> > > 1;
> > >   uint32_t reg_val;
> > >   int i;
> > > + u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> > >
> > >   /* Disable multicast promiscuous first */
> > >   ixgbe_disable_vf_mc_promisc(dev, vf); @@ -525,6 +526,9 @@
> > > ixgbe_vf_set_multicast(struct rte_eth_dev *dev, uint32_t vf,
> > > uint32_t
> > > *msgbuf)
> > >   IXGBE_WRITE_REG(hw, IXGBE_MTA(mta_idx), reg_val);
> > >   }
> > >
> > > + vmolr |= IXGBE_VMOLR_ROMPE;
> > > + IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> >
> >
> > Please correct me if I'm wrong
> >
> > My understand is MTA table is shared by all VFs (I guess also pf), but
> > what if
> 
> Yes, vf share it but not pf, it is used in vf pool switch
> 
> > two VFs both enable multi-cast but with different address filter?
> > Should we prevent the second one to enable multi-cast if any conflict
> > be detected? Otherwise there still will be unexpected behavior.
> 
> Sorry, I do not known what is the confict.
> Because IXGBE_VMOLR is vf specific, that is to say, each vf control itself for
> enable ROMPE.

The conflict what I mean is for example, vf1 set filter for address A while vf2 
set filter for address B, then both VFs will receive A and B which is not 
expected.
But if the only issue for receive unexpected multi-cast packet is just a 
performance impact then I agree with most of patch, 
one more comment is we may need to reset IXGBE_VMOLR_ROMPE if nb_entries = 0, 
which is the case to clear all multi-cast filter.

> 
> 
> >
> > > +
> > >   return 0;
> > >  }
> > >
> > > --
> > > 2.7.5

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix multicast table enable error for VF

[Zhang, Qi Z]

> -Original Message-
> From: Zhao1, Wei
> Sent: Friday, January 4, 2019 4:34 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Zhang, Qi Z ; Wu, Jingjing
> ; Zhao1, Wei 
> Subject: [PATCH v2] net/ixgbe: fix multicast table enable error for VF
> 
> In ixgbe PMD code, all vf ars set with bit IXGBE_VMOLR_ROMPE, which make vf
> accept packets that match the MTA table, if some vf update IXGBE_MTA in
> function ixgbe_vf_set_multicast, then all vf will receive packets from these
> address.
> So thhere is need to set VMOLR register bit ROPE only after this vf has been 
> set

s/thhere/there

> multicast address. If this bit is set when pf host doing initialization, this 
> vf will
> receive multicast packets with address written in MTA table. Align to ixgbe pf
> kernel 5.3.7 code to fix this bug.

Please check my last comment in v1, we need to consider the case to clean all 
multicast filter.
 
> 
> Fixes: 00e30184daa0 ("ixgbe: add PF support")
> 
> Signed-off-by: Wei Zhao 
> 
> ---
> 
> v2:
> change patch name and fix typo in log.
> ---
>  drivers/net/ixgbe/ixgbe_pf.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c index
> 4b833ff..0f4b96b 100644
> --- a/drivers/net/ixgbe/ixgbe_pf.c
> +++ b/drivers/net/ixgbe/ixgbe_pf.c
> @@ -351,7 +351,7 @@ ixgbe_vf_reset_event(struct rte_eth_dev *dev,
> uint16_t vf)
>   int rar_entry = hw->mac.num_rar_entries - (vf + 1);
>   uint32_t vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> 
> - vmolr |= (IXGBE_VMOLR_ROPE | IXGBE_VMOLR_ROMPE |
> + vmolr |= (IXGBE_VMOLR_ROPE |
>   IXGBE_VMOLR_BAM | IXGBE_VMOLR_AUPE);
>   IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> 
> @@ -503,6 +503,7 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev,
> uint32_t vf, uint32_t *msgbuf)
>   const uint32_t IXGBE_MTA_BIT_MASK = (0x1 << IXGBE_MTA_BIT_SHIFT) -
> 1;
>   uint32_t reg_val;
>   int i;
> + u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> 
>   /* Disable multicast promiscuous first */
>   ixgbe_disable_vf_mc_promisc(dev, vf);
> @@ -525,6 +526,9 @@ ixgbe_vf_set_multicast(struct rte_eth_dev *dev,
> uint32_t vf, uint32_t *msgbuf)
>   IXGBE_WRITE_REG(hw, IXGBE_MTA(mta_idx), reg_val);
>   }
> 
> + vmolr |= IXGBE_VMOLR_ROMPE;
> + IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> +
>   return 0;
>  }
> 
> --
> 2.7.5

Re: [dpdk-dev] [PATCH] examples/ip_fragmentation: support big packets

[Ananyev, Konstantin]

Hi ,

> 
> In some vendors the RX and TX configuration must be the same, therefore
> the MTU size need to be equal to max_rx_pkt_len.
> The MTU is the largest size packet in bytes that can be sent on the
> network, therefore before changing this parameter, the NIC could not
> receive packets larger than 1500 bytes, which is the default MTU size.
> In addition, scatter-gather need to be enabled in order to receive
> frames bigger than mbuf size.
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Noa Ezra 
> ---
>  examples/ip_fragmentation/main.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/examples/ip_fragmentation/main.c 
> b/examples/ip_fragmentation/main.c
> index 17a877d..d294f5f 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -141,6 +141,7 @@ struct lcore_queue_conf {
>   .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
>   .split_hdr_size = 0,
>   .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> +  DEV_RX_OFFLOAD_SCATTER |
>DEV_RX_OFFLOAD_JUMBO_FRAME),
>   },
>   .txmode = {
> @@ -892,6 +893,16 @@ struct rte_lpm6_config lpm6_config = {
>   dev_info.max_rx_pktlen,
>   local_port_conf.rxmode.max_rx_pkt_len);
> 
> + /* set the mtu to the maximum received packet size */
> + ret = rte_eth_dev_set_mtu(portid,
> + local_port_conf.rxmode.max_rx_pkt_len);
> + if (ret < 0) {
> + printf("\n");
> + rte_exit(EXIT_FAILURE, "Set MTU failed: "
> + "err=%d, port=%d\n",
> + ret, portid);
> + }
> +

It probably doesn't matter for that particular case, but
shouldn't set_mtu() be done after dev_configure()?
To avoid situation when max_rx_pkt_len will be set by set_mtu()
and then overwritten by dev_configure().
Konstantin 


>   /* get the lcore_id for this port */
>   while (rte_lcore_is_enabled(rx_lcore_id) == 0 ||
>  qconf->n_rx_queue == (unsigned)rx_queue_per_lcore) {
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH 0/4] NXP DPAA fixes and enhancements

[Shreyansh Jain]

On Wednesday 26 December 2018 05:03 PM, Hemant Agrawal wrote:
> Few fixes and minor enhancements in NXP DPAA driver
> 
> Hemant Agrawal (4):
>bus/dpaa: fix the logical to physical core affine logic
>net/dpaa: fix the secondary process
>net/dpaa: update supported ptypes
>net/dpaa: update RSS offload types
> 
>   drivers/bus/dpaa/dpaa_bus.c| 41 +-
>   drivers/net/dpaa/dpaa_ethdev.c | 19 +++-
>   drivers/net/dpaa/dpaa_ethdev.h | 12 ++
>   3 files changed, 42 insertions(+), 30 deletions(-)
> 

Series-Acked-by: Shreyansh Jain 

[dpdk-dev] [PATCH v2] app/testpmd: fix quit to stop all ports before close

[Cristian Dumitrescu]

This patch proposes a slightly different test-pmd quit operation: stop
all devices before starting to close any device. Basically, stop all
moving parts before beginning to remove them. The current test-pmd quit
is stopping and closing each device before moving to the next device.

If all devices in the system are independent of each other, this
difference is usually not important. In case of Soft NIC devices, any
such virtual device typically depends on one or more physical devices
being alive, as it accesses their queues, so this difference becomes
important.

Without this straightforward fix, all the Soft NIC devices need to be
manually stopped before the quit command is issued, otherwise the quit
command can sometimes crash the test-pmd application.

Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
Cc: sta...@dpdk.org

Signed-off-by: Cristian Dumitrescu 
---
Changes in V2:
-reworded title
-added Fixes and Cc lines

 app/test-pmd/testpmd.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 8d584b008..15a948828 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2391,9 +2391,13 @@ pmd_test_exit(void)
if (ports != NULL) {
no_link_check = 1;
RTE_ETH_FOREACH_DEV(pt_id) {
-   printf("\nShutting down port %d...\n", pt_id);
+   printf("\nStopping port %d...\n", pt_id);
fflush(stdout);
stop_port(pt_id);
+   }
+   RTE_ETH_FOREACH_DEV(pt_id) {
+   printf("\nShutting down port %d...\n", pt_id);
+   fflush(stdout);
close_port(pt_id);
 
/*
-- 
2.17.1

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Ananyev, Konstantin]

Hi Akhil,

> 
> Hi Konstantin,
> 
> With this patchset, I am seeing a 3% drop in legacy mode lookaside and
> 12% drop with -l option.
> I am debugging this. Will let you know if I find something.

Ok, thanks.
For legacy mode do you know which patch in the series causing 3% drop?
Is it still: " fix crypto-op might never get dequeued" or a different one?
Konstantin 


> 
> -Akhil
> 
> On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
> > This patch series depends on the patch series:
> >
> > ipsec: new library for IPsec data-path processing
> > http://patches.dpdk.org/patch/49410/
> > http://patches.dpdk.org/patch/49411/
> > http://patches.dpdk.org/patch/49412/
> > http://patches.dpdk.org/patch/49413/
> > http://patches.dpdk.org/patch/49414/
> > http://patches.dpdk.org/patch/49415/
> > http://patches.dpdk.org/patch/49416/
> > http://patches.dpdk.org/patch/49417/
> > http://patches.dpdk.org/patch/49418/
> > http://patches.dpdk.org/patch/49419/
> >
> > to be applied first.
> >
> > v5 -> v6
> >Address issues reported by Akhil:
> >   segfault when using lookaside-proto device
> >   HW IPv4 cksum offload not enabled by default
> >   crypto-dev dequeue() is called to often
> >
> > v4 -> v5
> > - Address Akhil comments:
> >   documentation update
> >   spell checks spacing etc.
> >   introduce rxoffload/txoffload parameters
> >   single SA for ipv6
> >   update Makefile
> >
> > v3 -> v4
> >   - fix few issues with the test scripts
> >   - update docs
> >
> > v2 -> v3
> >   - add IPv6 cases into test scripts
> >   - fixes for IPv6 support
> >   - fixes for inline-crypto support
> >   - some code restructuring
> >
> > v1 -> v2
> >   - Several bug fixes
> >
> > That series contians few bug-fixes and changes to make ipsec-secgw
> > to utilize librte_ipsec library:
> >   - changes in the related data structures.
> >   - changes in the initialization code.
> >   - changes in the data-path code.
> >   - new command-line parameters to enable librte_ipsec codepath
> > and related features.
> >   - test scripts to help automate ipsec-secgw functional testing.
> >
> > Note that right now by default current (non-librte_ipsec) code-path
> > will be used. User has to run application with new command-line option
> > ('-l')
> > to enable new codepath.
> > The main reason for that:
> >- current librte_ipsec doesn't support all ipsec algorithms
> >  and features that the app does.
> >- allow users to run both versions in parallel for some time
> >  to figure out any functional or performance degradation with the
> >  new code.
> >
> > Test scripts were run with the following crypto devices:
> >   - aesni_mb
> >   - aesni_gcm
> >   - qat
> >
> > Konstantin Ananyev (10):
> >examples/ipsec-secgw: allow user to disable some RX/TX offloads
> >examples/ipsec-secgw: allow to specify neighbour mac address
> >examples/ipsec-secgw: fix crypto-op might never get dequeued
> >examples/ipsec-secgw: fix outbound codepath for single SA
> >examples/ipsec-secgw: make local variables static
> >examples/ipsec-secgw: fix inbound SA checking
> >examples/ipsec-secgw: make app to use ipsec library
> >examples/ipsec-secgw: make data-path to use ipsec library
> >examples/ipsec-secgw: add scripts for functional test
> >doc: update ipsec-secgw guide and relelase notes
> >
> >   doc/guides/rel_notes/release_19_02.rst|  14 +
> >   doc/guides/sample_app_ug/ipsec_secgw.rst  | 159 +-
> >   examples/ipsec-secgw/Makefile |   5 +-
> >   examples/ipsec-secgw/ipsec-secgw.c| 480 ++
> >   examples/ipsec-secgw/ipsec.c  | 101 ++--
> >   examples/ipsec-secgw/ipsec.h  |  67 +++
> >   examples/ipsec-secgw/ipsec_process.c  | 357 +
> >   examples/ipsec-secgw/meson.build  |   6 +-
> >   examples/ipsec-secgw/parser.c |  91 
> >   examples/ipsec-secgw/parser.h |   8 +-
> >   examples/ipsec-secgw/sa.c | 263 +-
> >   examples/ipsec-secgw/sp4.c|  35 +-
> >   examples/ipsec-secgw/sp6.c|  35 +-
> >   examples/ipsec-secgw/test/common_defs.sh  | 153 ++
> >   examples/ipsec-secgw/test/data_rxtx.sh|  62 +++
> >   examples/ipsec-secgw/test/linux_test4.sh  |  63 +++
> >   examples/ipsec-secgw/test/linux_test6.sh  |  64 +++
> >   examples/ipsec-secgw/test/run_test.sh |  80 +++
> >   .../test/trs_aescbc_sha1_common_defs.sh   |  69 +++
> >   .../ipsec-secgw/test/trs_aescbc_sha1_defs.sh  |  67 +++
> >   .../test/trs_aescbc_sha1_esn_atom_defs.sh |   5 +
> >   .../test/trs_aescbc_sha1_esn_defs.sh  |  66 +++
> >   .../test/trs_aescbc_sha1_old_defs.sh  |   5 +
> >   .../test/trs_aesgcm_common_defs.sh|  60 +++
> >   examples/ipsec-secgw/test/trs_aesgcm_defs.sh  |  66 +++
> >   .../test/trs_a

Re: [dpdk-dev] [PATCH] net/fm10k: initialize sm_down variable

[Zhang, Qi Z]

> -Original Message-
> From: Wang, Xiao W
> Sent: Thursday, January 3, 2019 10:10 AM
> To: Julien Meunier ; Zhang, Qi Z
> 
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: RE: [PATCH] net/fm10k: initialize sm_down variable
> 
> Hi Julien,
> 
> > -Original Message-
> > From: Julien Meunier [mailto:julien.meun...@nokia.com]
> > Sent: Wednesday, January 2, 2019 11:58 PM
> > To: Zhang, Qi Z ; Wang, Xiao W
> > 
> > Cc: dev@dpdk.org; sta...@dpdk.org
> > Subject: [PATCH] net/fm10k: initialize sm_down variable
> >
> > Fixes: 6f22f2f67268 ("net/fm10k: redefine link status semantics")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Julien Meunier 
> > ---
> >  drivers/net/fm10k/fm10k_ethdev.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> > b/drivers/net/fm10k/fm10k_ethdev.c
> > index 85fb6c5..caf4d1b 100644
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -3003,6 +3003,7 @@ fm10k_params_init(struct rte_eth_dev *dev)
> > hw->bus.payload = fm10k_bus_payload_256;
> >
> > info->rx_vec_allowed = true;
> > +   info->sm_down = false;
> >  }
> >
> >  static int
> > --
> > 2.10.2
> 
> Acked-by: Xiao Wang 

Applied to dpdk-next-net-intel.

Thanks
Qi

> 
> BRs,
> Xiao

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix quit to stop all ports before close

[Iremonger, Bernard]

> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Friday, January 4, 2019 12:29 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo ; Wu, Jingjing
> ; Iremonger, Bernard ;
> sta...@dpdk.org
> Subject: [PATCH v2] app/testpmd: fix quit to stop all ports before close
> 
> This patch proposes a slightly different test-pmd quit operation: stop all 
> devices
> before starting to close any device. Basically, stop all moving parts before
> beginning to remove them. The current test-pmd quit is stopping and closing
> each device before moving to the next device.
> 
> If all devices in the system are independent of each other, this difference is
> usually not important. In case of Soft NIC devices, any such virtual device
> typically depends on one or more physical devices being alive, as it accesses
> their queues, so this difference becomes important.
> 
> Without this straightforward fix, all the Soft NIC devices need to be manually
> stopped before the quit command is issued, otherwise the quit command can
> sometimes crash the test-pmd application.
> 
> Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Cristian Dumitrescu 

Acked-by: Bernard Iremonger 

[dpdk-dev] [PATCH v4 0/2] lib/librte_meter: add RFC4115 trTCM meter support

[Eelco Chaudron]

This patch adds support for RFC4115 trTCM meters.


Signed-off-by: Eelco Chaudron 

v4:
 - Fixed typo
 - Cleaned up code to be aligned with RFC diagram

v3:
 - Gave the rfc4115 meter it's own data structures

v2:
 - Marked all functions with __rte_experimental, and added
   "EXPERIMENTAL:..." to doxygen comments.
 - Removed library version change, and merged it with first patch
 - Do not call rte_meter_trtcm_rfc4115_color_aware_check in
   rte_meter_trtcm_rfc4115_color_blind_check to avoid error with
   it being marked as experimental

Eelco Chaudron (2):
  lib/librte_meter: add RFC4115 trTCM meter support
  test/test_meter: update meter test to include RFC4115 meters


 lib/librte_meter/rte_meter.c   |   42 ++
 lib/librte_meter/rte_meter.h   |  233 
 lib/librte_meter/rte_meter_version.map |9 +
 test/test/test_meter.c |  212 +
 4 files changed, 494 insertions(+), 2 deletions(-)

[dpdk-dev] [PATCH v4 1/2] lib/librte_meter: add RFC4115 trTCM meter support

[Eelco Chaudron]

This patch adds support for RFC4115 trTCM meters.

Signed-off-by: Eelco Chaudron 
---
 lib/librte_meter/rte_meter.c   |   42 ++
 lib/librte_meter/rte_meter.h   |  233 
 lib/librte_meter/rte_meter_version.map |9 +
 3 files changed, 282 insertions(+), 2 deletions(-)

diff --git a/lib/librte_meter/rte_meter.c b/lib/librte_meter/rte_meter.c
index 473f69aba..e55f9be65 100644
--- a/lib/librte_meter/rte_meter.c
+++ b/lib/librte_meter/rte_meter.c
@@ -110,3 +110,45 @@ rte_meter_trtcm_config(struct rte_meter_trtcm *m,
 
return 0;
 }
+
+int __rte_experimental
+rte_meter_trtcm_rfc4115_profile_config(
+   struct rte_meter_trtcm_rfc4115_profile *p,
+   struct rte_meter_trtcm_rfc4115_params *params)
+{
+   uint64_t hz = rte_get_tsc_hz();
+
+   /* Check input parameters */
+   if ((p == NULL) ||
+   (params == NULL) ||
+   (params->cir != 0 && params->cbs == 0) ||
+   (params->eir != 0 && params->ebs == 0))
+   return -EINVAL;
+
+   /* Initialize trTCM run-time structure */
+   p->cbs = params->cbs;
+   p->ebs = params->ebs;
+   rte_meter_get_tb_params(hz, params->cir, &p->cir_period,
+   &p->cir_bytes_per_period);
+   rte_meter_get_tb_params(hz, params->eir, &p->eir_period,
+   &p->eir_bytes_per_period);
+
+   return 0;
+}
+
+int __rte_experimental
+rte_meter_trtcm_rfc4115_config(
+   struct rte_meter_trtcm_rfc4115 *m,
+   struct rte_meter_trtcm_rfc4115_profile *p)
+{
+   /* Check input parameters */
+   if ((m == NULL) || (p == NULL))
+   return -EINVAL;
+
+   /* Initialize trTCM run-time structure */
+   m->time_tc = m->time_te = rte_get_tsc_cycles();
+   m->tc = p->cbs;
+   m->te = p->ebs;
+
+   return 0;
+}
diff --git a/lib/librte_meter/rte_meter.h b/lib/librte_meter/rte_meter.h
index 58a051583..c7eee896a 100644
--- a/lib/librte_meter/rte_meter.h
+++ b/lib/librte_meter/rte_meter.h
@@ -1,3 +1,4 @@
+
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2014 Intel Corporation
  */
@@ -16,6 +17,7 @@ extern "C" {
  * Traffic metering algorithms:
  *1. Single Rate Three Color Marker (srTCM): defined by IETF RFC 2697
  *2. Two Rate Three Color Marker (trTCM): defined by IETF RFC 2698
+ *3. Two Rate Three Color Marker (trTCM): defined by IETF RFC 4115
  *
  ***/
 
@@ -49,10 +51,21 @@ be greater than or equal to CIR. Both CBS or EBS have to be 
greater than zero. *
 struct rte_meter_trtcm_params {
uint64_t cir; /**< Committed Information Rate (CIR). Measured in bytes 
per second. */
uint64_t pir; /**< Peak Information Rate (PIR). Measured in bytes per 
second. */
-   uint64_t cbs; /**< Committed Burst Size (CBS). Measured in byes. */
+   uint64_t cbs; /**< Committed Burst Size (CBS). Measured in bytes. */
uint64_t pbs; /**< Peak Burst Size (PBS). Measured in bytes. */
 };
 
+/** trTCM parameters per metered traffic flow. The CIR, EIR, CBS and EBS
+parameters only count bytes of IP packets and do not include link specific
+headers. The CBS and EBS need to be greater than zero if CIR and EIR are
+none-zero respectively.*/
+struct rte_meter_trtcm_rfc4115_params {
+   uint64_t cir; /**< Committed Information Rate (CIR). Measured in bytes 
per second. */
+   uint64_t eir; /**< Excess Information Rate (EIR). Measured in bytes per 
second. */
+   uint64_t cbs; /**< Committed Burst Size (CBS). Measured in bytes. */
+   uint64_t ebs; /**< Excess Burst Size (EBS). Measured in bytes. */
+};
+
 /**
  * Internal data structure storing the srTCM configuration profile. Typically
  * shared by multiple srTCM objects.
@@ -65,12 +78,24 @@ struct rte_meter_srtcm_profile;
  */
 struct rte_meter_trtcm_profile;
 
+/**
+ * Internal data structure storing the trTCM RFC4115 configuration profile.
+ * Typically shared by multiple trTCM objects.
+ */
+struct rte_meter_trtcm_rfc4115_profile;
+
 /** Internal data structure storing the srTCM run-time context per metered 
traffic flow. */
 struct rte_meter_srtcm;
 
 /** Internal data structure storing the trTCM run-time context per metered 
traffic flow. */
 struct rte_meter_trtcm;
 
+/**
+ * Internal data structure storing the trTCM RFC4115 run-time context per
+ * metered traffic flow.
+ */
+struct rte_meter_trtcm_rfc4115;
+
 /**
  * srTCM profile configuration
  *
@@ -98,6 +123,23 @@ rte_meter_srtcm_profile_config(struct 
rte_meter_srtcm_profile *p,
 int
 rte_meter_trtcm_profile_config(struct rte_meter_trtcm_profile *p,
struct rte_meter_trtcm_params *params);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * trTCM RFC 4115 profile configuration
+ *
+ * @param p
+ *Pointer to pre-allocated trTCM profile data structure
+ * @param params
+ *trTCM profile parameters
+ * @return
+ *0 upon success, error code otherwise
+ */
+int __rte_experimental
+rte_meter_

[dpdk-dev] [PATCH v4 2/2] test/test_meter: update meter test to include RFC4115 meters

[Eelco Chaudron]

Add test cases for RFC4115 meters

Signed-off-by: Eelco Chaudron 
---
 test/test/test_meter.c |  212 
 1 file changed, 212 insertions(+)

diff --git a/test/test/test_meter.c b/test/test/test_meter.c
index 8bb47e75c..f935faa53 100644
--- a/test/test/test_meter.c
+++ b/test/test/test_meter.c
@@ -32,8 +32,10 @@
 
 #define TM_TEST_TRTCM_CIR_DF 4600
 #define TM_TEST_TRTCM_PIR_DF 6900
+#define TM_TEST_TRTCM_EIR_DF 6900
 #define TM_TEST_TRTCM_CBS_DF 2048
 #define TM_TEST_TRTCM_PBS_DF 4096
+#define TM_TEST_TRTCM_EBS_DF 4096
 
 static struct rte_meter_srtcm_params sparams =
{.cir = TM_TEST_SRTCM_CIR_DF,
@@ -46,6 +48,12 @@ static structrte_meter_trtcm_params tparams=
 .cbs = TM_TEST_TRTCM_CBS_DF,
 .pbs = TM_TEST_TRTCM_PBS_DF,};
 
+static struct rte_meter_trtcm_rfc4115_params rfc4115params =
+   {.cir = TM_TEST_TRTCM_CIR_DF,
+.eir = TM_TEST_TRTCM_EIR_DF,
+.cbs = TM_TEST_TRTCM_CBS_DF,
+.ebs = TM_TEST_TRTCM_EBS_DF,};
+
 /**
  * functional test for rte_meter_srtcm_config
  */
@@ -148,6 +156,45 @@ tm_test_trtcm_config(void)
return 0;
 }
 
+/**
+ * functional test for rte_meter_trtcm_rfc4115_config
+ */
+static inline int
+tm_test_trtcm_rfc4115_config(void)
+{
+   struct rte_meter_trtcm_rfc4115_profile tp;
+   struct  rte_meter_trtcm_rfc4115_params rfc4115params1;
+#define TRTCM_RFC4115_CFG_MSG "trtcm_rfc4115_config"
+
+   /* invalid parameter test */
+   if (rte_meter_trtcm_rfc4115_profile_config(NULL, NULL) == 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, NULL) == 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+   if (rte_meter_trtcm_rfc4115_profile_config(NULL, &rfc4115params) == 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+
+   /*
+* cbs and pbs should be none-zero if cir and eir are none-zero
+* respectively
+*/
+   rfc4115params1 = rfc4115params;
+   rfc4115params1.cbs = 0;
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params1) == 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+
+   rfc4115params1 = rfc4115params;
+   rfc4115params1.ebs = 0;
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params1) == 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+
+   /* usual parameter, should be successful */
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params) != 0)
+   melog(TRTCM_RFC4115_CFG_MSG);
+
+   return 0;
+}
+
 /**
  * functional test for rte_meter_srtcm_color_blind_check
  */
@@ -265,6 +312,65 @@ tm_test_trtcm_color_blind_check(void)
return 0;
 }
 
+/**
+ * functional test for rte_meter_trtcm_rfc4115_color_blind_check
+ */
+static inline int
+tm_test_trtcm_rfc4115_color_blind_check(void)
+{
+#define TRTCM_RFC4115_BLIND_CHECK_MSG "trtcm_rfc4115_blind_check"
+
+   uint64_t time;
+   struct rte_meter_trtcm_rfc4115_profile tp;
+   struct rte_meter_trtcm_rfc4115 tm;
+   uint64_t hz = rte_get_tsc_hz();
+
+   /* Test green */
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   if (rte_meter_trtcm_rfc4115_config(&tm, &tp) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   time = rte_get_tsc_cycles() + hz;
+   if (rte_meter_trtcm_rfc4115_color_blind_check(
+   &tm, &tp, time, TM_TEST_TRTCM_CBS_DF - 1)
+   != e_RTE_METER_GREEN)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG" GREEN");
+
+   /* Test yellow */
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   if (rte_meter_trtcm_rfc4115_config(&tm, &tp) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   time = rte_get_tsc_cycles() + hz;
+   if (rte_meter_trtcm_rfc4115_color_blind_check(
+   &tm, &tp, time, TM_TEST_TRTCM_CBS_DF + 1)
+   != e_RTE_METER_YELLOW)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG" YELLOW");
+
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   if (rte_meter_trtcm_rfc4115_config(&tm, &tp) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   time = rte_get_tsc_cycles() + hz;
+   if (rte_meter_trtcm_rfc4115_color_blind_check(
+   &tm, &tp, time, TM_TEST_TRTCM_EBS_DF - 1)
+   != e_RTE_METER_YELLOW)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG" YELLOW");
+
+   /* Test red */
+   if (rte_meter_trtcm_rfc4115_profile_config(&tp, &rfc4115params) != 0)
+   melog(TRTCM_RFC4115_BLIND_CHECK_MSG);
+   if (rte_mete

Re: [dpdk-dev] [PATCH v3 1/2] lib/librte_meter: add RFC4115 trTCM meter support

[Eelco Chaudron]

On 21 Dec 2018, at 17:15, Dumitrescu, Cristian wrote:


Hi Eelco,





+/** trTCM parameters per metered traffic flow. The CIR, EIT, CBS and 
EBS


Small typo here: EIT to be replaced by EIR.

+parameters only count bytes of IP packets and do not include link 
specific
+headers. The CBS and EBS need to be greater than zero if CIR and EIR 
are

+none-zero respectively.*/
+struct rte_meter_trtcm_rfc4115_params {
+   uint64_t cir; /**< Committed Information Rate (CIR). Measured in
bytes per second. */
+   uint64_t eir; /**< Excess Information Rate (EIR). Measured in bytes
per second. */
+   uint64_t cbs; /**< Committed Burst Size (CBS). Measured in bytes.
*/
+   uint64_t ebs; /**< Excess Burst Size (EBS). Measured in bytes. */
+};
+





+static inline enum rte_meter_color __rte_experimental
+rte_meter_trtcm_rfc4115_color_blind_check(
+   struct rte_meter_trtcm_rfc4115 *m,
+   struct rte_meter_trtcm_rfc4115_profile *p,
+   uint64_t time,
+   uint32_t pkt_len)
+{
+	uint64_t time_diff_tc, time_diff_te, n_periods_tc, n_periods_te, 
tc,

te;
+
+   /* Bucket update */
+   time_diff_tc = time - m->time_tc;
+   time_diff_te = time - m->time_te;
+   n_periods_tc = time_diff_tc / p->cir_period;
+   n_periods_te = time_diff_te / p->eir_period;
+   m->time_tc += n_periods_tc * p->cir_period;
+   m->time_te += n_periods_te * p->eir_period;
+
+   tc = m->tc + n_periods_tc * p->cir_bytes_per_period;
+   if (tc > p->cbs)
+   tc = p->cbs;
+
+   te = m->te + n_periods_te * p->eir_bytes_per_period;
+   if (te > p->ebs)
+   te = p->ebs;
+
+   /* Color logic */
+   if (tc >= pkt_len) {
+   m->tc = tc - pkt_len;
+   m->te = te;
+   return e_RTE_METER_GREEN;
+   } else if (te >= pkt_len) {
+   m->tc = tc;
+   m->te = te - pkt_len;
+   return e_RTE_METER_YELLOW;
+   }
+
+   /* If we end up here the color is RED */
+   m->tc = tc;
+   m->te = te;
+   return e_RTE_METER_RED;
+}
+


Since the branch (tc >= pkt_len) == TRUE always returns, I suggest we 
remove the following "else", as it is redundant:


/* Color logic */
if (tc >= pkt_len) {
m->tc = tc - pkt_len;
m->te = te;
return e_RTE_METER_GREEN;
}

if (te >= pkt_len) {
m->tc = tc;
m->te = te - pkt_len;
return e_RTE_METER_YELLOW;
}

/* If we end up here the color is RED */
m->tc = tc;
m->te = te;
return e_RTE_METER_RED;



+static inline enum rte_meter_color __rte_experimental
+rte_meter_trtcm_rfc4115_color_aware_check(
+   struct rte_meter_trtcm_rfc4115 *m,
+   struct rte_meter_trtcm_rfc4115_profile *p,
+   uint64_t time,
+   uint32_t pkt_len,
+   enum rte_meter_color pkt_color)
+{
+	uint64_t time_diff_tc, time_diff_te, n_periods_tc, n_periods_te, 
tc,

te;
+
+   /* Bucket update */
+   time_diff_tc = time - m->time_tc;
+   time_diff_te = time - m->time_te;
+   n_periods_tc = time_diff_tc / p->cir_period;
+   n_periods_te = time_diff_te / p->eir_period;
+   m->time_tc += n_periods_tc * p->cir_period;
+   m->time_te += n_periods_te * p->eir_period;
+
+   tc = m->tc + n_periods_tc * p->cir_bytes_per_period;
+   if (tc > p->cbs)
+   tc = p->cbs;
+
+   te = m->te + n_periods_te * p->eir_bytes_per_period;
+   if (te > p->ebs)
+   te = p->ebs;
+
+   /* Color logic */
+   if (pkt_color == e_RTE_METER_GREEN) {
+   if (tc >= pkt_len) {
+   m->tc = tc - pkt_len;
+   m->te = te;
+   return e_RTE_METER_GREEN;
+   } else if (te >= pkt_len) {
+   m->tc = tc;
+   m->te = te - pkt_len;
+   return e_RTE_METER_YELLOW;
+   }
+   } else if (pkt_color == e_RTE_METER_YELLOW && te >= pkt_len) {
+   m->tc = tc;
+   m->te = te - pkt_len;
+   return e_RTE_METER_YELLOW;
+   }
+
+   /* If we end up here the color is RED */
+   m->tc = tc;
+   m->te = te;
+   return e_RTE_METER_RED;
+}
+
+


I suggest we follow the logic from the diagram in the RFC rather than 
the logic in the text preceding the diagram. Although the two 
descriptions are equivalent (after a bit of thinking), the diagram 
seems more optimal to me:


/* Color logic */
if ((pkt_color == e_RTE_METER_GREEN) && (tc >= pkt_len)) {
m->tc = tc - pkt_len;
m->te = te;
return e_RTE_METER_GREEN;
}

if ((pkt_color != e_RTE_METER_RED) && (te >= pkt_len)) {
m->tc = tc;
m->te = te - pkt_len;
return e_RTE_METER_YELLOW;
}

/* If we end up here

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Akhil Goyal]

On 1/4/2019 5:59 PM, Ananyev, Konstantin wrote:
> Hi Akhil,
>
>> Hi Konstantin,
>>
>> With this patchset, I am seeing a 3% drop in legacy mode lookaside and
>> 12% drop with -l option.
>> I am debugging this. Will let you know if I find something.
> Ok, thanks.
> For legacy mode do you know which patch in the series causing 3% drop?
> Is it still: " fix crypto-op might never get dequeued" or a different one?
> Konstantin
>
for legacy mode you may consider this diff in 3/10

diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
b/examples/ipsec-secgw/ipsec-secgw.c
index 4f7a77d8d..d183862b8 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1015,10 +1015,13 @@ main_loop(__attribute__((unused)) void *dummy)

     if (nb_rx > 0)
     process_pkts(qconf, pkts, nb_rx, portid);
-   }
+// }

-   drain_inbound_crypto_queues(qconf, &qconf->inbound);
-   drain_outbound_crypto_queues(qconf, &qconf->outbound);
+   if (UNPROTECTED_PORT(portid))
+   drain_inbound_crypto_queues(qconf, 
&qconf->inbound);
+   else
+   drain_outbound_crypto_queues(qconf, 
&qconf->outbound);
+   }
     }
  }

The 3% gap was on single core performance. But on multi cores scenario, 
there is no significant drop.

But I see a bigger functional problem in case of non-legacy mode.
I am trying a multi tunnel scenario with 32 kind of flows on one side 
each with different tunnels.
The flows are kind of dest ip: 192.168.101.1 ... 192.168.x.1  
.  192.168.132.1.
Each IP has a different tunnel.
All are pumped from same port. I can see that some of the IPs are not 
getting forwarded.
like 192.168.103.1, 192.168.104.1, 192.168.105.1, 192.168.114.1, 
192.168.115.1, 192.168.116.1, 192.168.122.1, 192.168.125.1, 
192.168.126.1, 192.168.130.1

The same setup/flows works perfectly fine with legacy mode.


>> -Akhil
>>
>> On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
>>> This patch series depends on the patch series:
>>>
>>> ipsec: new library for IPsec data-path processing
>>> http://patches.dpdk.org/patch/49410/
>>> http://patches.dpdk.org/patch/49411/
>>> http://patches.dpdk.org/patch/49412/
>>> http://patches.dpdk.org/patch/49413/
>>> http://patches.dpdk.org/patch/49414/
>>> http://patches.dpdk.org/patch/49415/
>>> http://patches.dpdk.org/patch/49416/
>>> http://patches.dpdk.org/patch/49417/
>>> http://patches.dpdk.org/patch/49418/
>>> http://patches.dpdk.org/patch/49419/
>>>
>>> to be applied first.
>>>
>>> v5 -> v6
>>> Address issues reported by Akhil:
>>>segfault when using lookaside-proto device
>>>HW IPv4 cksum offload not enabled by default
>>>crypto-dev dequeue() is called to often
>>>
>>> v4 -> v5
>>> - Address Akhil comments:
>>>documentation update
>>>spell checks spacing etc.
>>>introduce rxoffload/txoffload parameters
>>>single SA for ipv6
>>>update Makefile
>>>
>>> v3 -> v4
>>>- fix few issues with the test scripts
>>>- update docs
>>>
>>> v2 -> v3
>>>- add IPv6 cases into test scripts
>>>- fixes for IPv6 support
>>>- fixes for inline-crypto support
>>>- some code restructuring
>>>
>>> v1 -> v2
>>>- Several bug fixes
>>>
>>> That series contians few bug-fixes and changes to make ipsec-secgw
>>> to utilize librte_ipsec library:
>>>- changes in the related data structures.
>>>- changes in the initialization code.
>>>- changes in the data-path code.
>>>- new command-line parameters to enable librte_ipsec codepath
>>>  and related features.
>>>- test scripts to help automate ipsec-secgw functional testing.
>>>
>>> Note that right now by default current (non-librte_ipsec) code-path
>>> will be used. User has to run application with new command-line option
>>> ('-l')
>>> to enable new codepath.
>>> The main reason for that:
>>> - current librte_ipsec doesn't support all ipsec algorithms
>>>   and features that the app does.
>>> - allow users to run both versions in parallel for some time
>>>   to figure out any functional or performance degradation with the
>>>   new code.
>>>
>>> Test scripts were run with the following crypto devices:
>>>- aesni_mb
>>>- aesni_gcm
>>>- qat
>>>
>>> Konstantin Ananyev (10):
>>> examples/ipsec-secgw: allow user to disable some RX/TX offloads
>>> examples/ipsec-secgw: allow to specify neighbour mac address
>>> examples/ipsec-secgw: fix crypto-op might never get dequeued
>>> examples/ipsec-secgw: fix outbound codepath for single SA
>>> examples/ipsec-secgw: make local variables static
>>> examples/ipsec-secgw: fix inbound SA checking
>>> examples/ipsec-secgw: make app to use ipsec library
>>> examples/ipsec-secgw: make data-path to use ipsec library
>>>

Re: [dpdk-dev] [PATCH v6 08/10] examples/ipsec-secgw: make data-path to use ipsec library

[Akhil Goyal]

On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
>   /* process ipv4 packets */
> - inbound_sp_sa(ctx->sp4_ctx, ctx->sa_ctx, &trf.ip4, 0);
> - route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
> + if (trf.ip4.num != 0) {
> + inbound_sp_sa(ctx->sp4_ctx, ctx->sa_ctx, &trf.ip4, 0);
> + route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
> + }
>   
>   /* process ipv6 packets */
> - inbound_sp_sa(ctx->sp6_ctx, ctx->sa_ctx, &trf.ip6, 0);
> - route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
> + if (trf.ip6.num != 0) {
> + inbound_sp_sa(ctx->sp6_ctx, ctx->sa_ctx, &trf.ip6, 0);
> + route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
> + }
One more thing.
Why do you need these changes. There are multiple places where this 
change is done.

route6_pkts and route4_pkts already checks that internally for 0 pkts.
inbound_sp_sa also does that.
These are getting called from multiple places. So it is better we add this 
check inside the function itself instead of outside it.

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Akhil Goyal]

On 1/4/2019 8:10 PM, Akhil Goyal wrote:
>
> On 1/4/2019 5:59 PM, Ananyev, Konstantin wrote:
>> Hi Akhil,
>>
>>> Hi Konstantin,
>>>
>>> With this patchset, I am seeing a 3% drop in legacy mode lookaside and
>>> 12% drop with -l option.
>>> I am debugging this. Will let you know if I find something.
>> Ok, thanks.
>> For legacy mode do you know which patch in the series causing 3% drop?
>> Is it still: " fix crypto-op might never get dequeued" or a different one?
>> Konstantin
>>
> for legacy mode you may consider this diff in 3/10
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> b/examples/ipsec-secgw/ipsec-secgw.c
> index 4f7a77d8d..d183862b8 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1015,10 +1015,13 @@ main_loop(__attribute__((unused)) void *dummy)
>
>       if (nb_rx > 0)
>       process_pkts(qconf, pkts, nb_rx, portid);
> -   }
> +// }
>
> -   drain_inbound_crypto_queues(qconf, &qconf->inbound);
> -   drain_outbound_crypto_queues(qconf, &qconf->outbound);
> +   if (UNPROTECTED_PORT(portid))
> +   drain_inbound_crypto_queues(qconf,
> &qconf->inbound);
> +   else
> +   drain_outbound_crypto_queues(qconf,
> &qconf->outbound);
> +   }
>       }
>    }
>
> The 3% gap was on single core performance. But on multi cores scenario,
> there is no significant drop.
>
> But I see a bigger functional problem in case of non-legacy mode.
> I am trying a multi tunnel scenario with 32 kind of flows on one side
> each with different tunnels.
> The flows are kind of dest ip: 192.168.101.1 ... 192.168.x.1
> .  192.168.132.1.
> Each IP has a different tunnel.
> All are pumped from same port. I can see that some of the IPs are not
> getting forwarded.
> like 192.168.103.1, 192.168.104.1, 192.168.105.1, 192.168.114.1,
> 192.168.115.1, 192.168.116.1, 192.168.122.1, 192.168.125.1,
> 192.168.126.1, 192.168.130.1
>
> The same setup/flows works perfectly fine with legacy mode.

I see that the IPs are not fixed and changes on every run. Probably, the 
inbound /outbound queues gets flushed when they do not intend to on that 
particular core and packets get discarded as they come from non-destined 
port.
Also the issue does not come if I add the above suggested change.

>
>
>>> -Akhil
>>>
>>> On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
 This patch series depends on the patch series:

 ipsec: new library for IPsec data-path processing
 http://patches.dpdk.org/patch/49410/
 http://patches.dpdk.org/patch/49411/
 http://patches.dpdk.org/patch/49412/
 http://patches.dpdk.org/patch/49413/
 http://patches.dpdk.org/patch/49414/
 http://patches.dpdk.org/patch/49415/
 http://patches.dpdk.org/patch/49416/
 http://patches.dpdk.org/patch/49417/
 http://patches.dpdk.org/patch/49418/
 http://patches.dpdk.org/patch/49419/

 to be applied first.

 v5 -> v6
  Address issues reported by Akhil:
 segfault when using lookaside-proto device
 HW IPv4 cksum offload not enabled by default
 crypto-dev dequeue() is called to often

 v4 -> v5
 - Address Akhil comments:
 documentation update
 spell checks spacing etc.
 introduce rxoffload/txoffload parameters
 single SA for ipv6
 update Makefile

 v3 -> v4
 - fix few issues with the test scripts
 - update docs

 v2 -> v3
 - add IPv6 cases into test scripts
 - fixes for IPv6 support
 - fixes for inline-crypto support
 - some code restructuring

 v1 -> v2
 - Several bug fixes

 That series contians few bug-fixes and changes to make ipsec-secgw
 to utilize librte_ipsec library:
 - changes in the related data structures.
 - changes in the initialization code.
 - changes in the data-path code.
 - new command-line parameters to enable librte_ipsec codepath
   and related features.
 - test scripts to help automate ipsec-secgw functional testing.

 Note that right now by default current (non-librte_ipsec) code-path
 will be used. User has to run application with new command-line option
 ('-l')
 to enable new codepath.
 The main reason for that:
  - current librte_ipsec doesn't support all ipsec algorithms
and features that the app does.
  - allow users to run both versions in parallel for some time
to figure out any functional or performance degradation with the
new code.

 Test scripts were run with the following crypto devices:
 - aesni_mb
 - aesni_gcm
 - qat

 Konstantin Ananyev

Re: [dpdk-dev] [PATCH v6 08/10] examples/ipsec-secgw: make data-path to use ipsec library

[Ananyev, Konstantin]

> 
> On 1/4/2019 1:55 AM, Konstantin Ananyev wrote:
> > /* process ipv4 packets */
> > -   inbound_sp_sa(ctx->sp4_ctx, ctx->sa_ctx, &trf.ip4, 0);
> > -   route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
> > +   if (trf.ip4.num != 0) {
> > +   inbound_sp_sa(ctx->sp4_ctx, ctx->sa_ctx, &trf.ip4, 0);
> > +   route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
> > +   }
> >
> > /* process ipv6 packets */
> > -   inbound_sp_sa(ctx->sp6_ctx, ctx->sa_ctx, &trf.ip6, 0);
> > -   route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
> > +   if (trf.ip6.num != 0) {
> > +   inbound_sp_sa(ctx->sp6_ctx, ctx->sa_ctx, &trf.ip6, 0);
> > +   route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
> > +   }
> One more thing.
> Why do you need these changes. There are multiple places where this
> change is done.
> 
> route6_pkts and route4_pkts already checks that internally for 0 pkts.
> inbound_sp_sa also does that.
> These are getting called from multiple places. So it is better we add this 
> check inside the function itself instead of outside it.

You right, these checks are not necessary here.
I put these extra checks to avoid unnecessary calls -
there is no guarantee that both route_pkts() and inbound_sp_sa()
will always be inlined by the compiler (they are not that small).
Konstantin

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Ananyev, Konstantin]

> On 1/4/2019 5:59 PM, Ananyev, Konstantin wrote:
> > Hi Akhil,
> >
> >> Hi Konstantin,
> >>
> >> With this patchset, I am seeing a 3% drop in legacy mode lookaside and
> >> 12% drop with -l option.
> >> I am debugging this. Will let you know if I find something.
> > Ok, thanks.
> > For legacy mode do you know which patch in the series causing 3% drop?
> > Is it still: " fix crypto-op might never get dequeued" or a different one?
> > Konstantin
> >
> for legacy mode you may consider this diff in 3/10
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> b/examples/ipsec-secgw/ipsec-secgw.c
> index 4f7a77d8d..d183862b8 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1015,10 +1015,13 @@ main_loop(__attribute__((unused)) void *dummy)
> 
>      if (nb_rx > 0)
>      process_pkts(qconf, pkts, nb_rx, portid);
> -   }
> +// }
> 
> -   drain_inbound_crypto_queues(qconf, &qconf->inbound);
> -   drain_outbound_crypto_queues(qconf, &qconf->outbound);
> +   if (UNPROTECTED_PORT(portid))
> +   drain_inbound_crypto_queues(qconf,
> &qconf->inbound);
> +   else
> +   drain_outbound_crypto_queues(qconf,
> &qconf->outbound);
> +   }
>      }
>   }
> 
> The 3% gap was on single core performance. But on multi cores scenario,
> there is no significant drop.
> 
> But I see a bigger functional problem in case of non-legacy mode.
> I am trying a multi tunnel scenario with 32 kind of flows on one side
> each with different tunnels.
> The flows are kind of dest ip: 192.168.101.1 ... 192.168.x.1
> .  192.168.132.1.
> Each IP has a different tunnel.
> All are pumped from same port. I can see that some of the IPs are not
> getting forwarded.
> like 192.168.103.1, 192.168.104.1, 192.168.105.1, 192.168.114.1,
> 192.168.115.1, 192.168.116.1, 192.168.122.1, 192.168.125.1,
> 192.168.126.1, 192.168.130.1
> 
> The same setup/flows works perfectly fine with legacy mode.

I'll try to reproduce it on my box.
To clarify:
   you are using one core, one port.
   each dest ip assigned to different SA
  (i.e. for dst=192.168.103.1, spi=1,  dst=192.168.104.1, spi=2, ...)
  and for some SAs packets are not forwarded.
  I presume each rx burst contains packets from different SAs.
 correct?
 Is that failure for inbound or outbound packets or both?
 Any particular details about mode(tunnel/transport), algorithm? 
   
Thanks
Konstantin

Re: [dpdk-dev] [RFC v3] /net: memory interface (memif)

[Ferruh Yigit]

On 12/13/2018 1:30 PM, Jakub Grajciar wrote:
> Memory interface (memif), provides high performance
> packet transfer over shared memory.
> 
> Signed-off-by: Jakub Grajciar 

Hi Jakub,

I put some comments but overall it is a long patch, I am almost sure there are
some details missed, it can make easier to review if you can split the patch
into multiple patches.

> ---
>  config/common_base  |5 +
>  config/common_linuxapp  |1 +
>  doc/guides/nics/memif.rst   |   80 ++

"doc/guides/nics/index.rst" needs to be updated to add the new doc into index

Also there is a driver feature documentation, "doc/guides/nics/features/*.ini",
I admit it is not very informative for virtual devices, but having it will add
the driver to the overview table, so please add it too, with as much data as
possible.

>  drivers/net/Makefile|1 +
>  drivers/net/memif/Makefile  |   29 +
>  drivers/net/memif/memif.h   |  143 +++
>  drivers/net/memif/memif_socket.c| 1093 +
>  drivers/net/memif/memif_socket.h|  104 ++
>  drivers/net/memif/meson.build   |   10 +
>  drivers/net/memif/rte_eth_memif.c   | 1189 +++
>  drivers/net/memif/rte_eth_memif.h   |  207 
>  drivers/net/memif/rte_pmd_memif_version.map |4 +
>  drivers/net/meson.build |1 +
>  mk/rte.app.mk   |1 +

Can you please update "MAINTAINERS" file too, to add driver and its owner.

<...>

> @@ -0,0 +1,80 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +Copyright(c) 2018 Cisco Systems, Inc.
> +
> +Memif Poll Mode Driver
> +==
> +
> +Memif PMD allows for DPDK and any other client using memif (DPDK, VPP, 
> libmemif) to
> +communicate using shared memory.

It can be good to describe what 'memif' is briefly here and perhaps a link for
more detailed documentation.

And some more low level detail can be good, like message type, messaging
protocol, descriptor format etc.. A link also good enough if it is already
documented somewhere.

Also memif PMD is only for Linux, what do you think documenting that limitation
here?

<...>

> +Example: testpmd and VPP
> +
> +
> +For information on how to get and run VPP please see 
> ``_.

Can memif PMD used between two DPDK application, or as two interfaces to
testpmd? If so what do you think adding that as a main sample, which would be
easier for everyone to test?

<...>

> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2018 Cisco Systems, Inc.  All rights reserved.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_memif.a
> +
> +EXPORT_MAP := rte_pmd_memif_version.map
> +
> +LIBABIVER := 1
> +
> +CFLAGS += -O3
> +CFLAGS += -I$(SRCDIR)

Why need to include $(SRCDIR) ?

> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -Wno-pointer-arith

Is this warning disabled explicitly?

<...>

> @@ -0,0 +1,143 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2018 Cisco Systems, Inc.  All rights reserved.
> + */
> +
> +#ifndef _MEMIF_H_
> +#define _MEMIF_H_
> +
> +#ifndef MEMIF_CACHELINE_SIZE
> +#define MEMIF_CACHELINE_SIZE 64
> +#endif

How much this is related to processor cache size? If same thing it is better to
use RTE_CACHE_LINE_SIZE since it can be set properly for various architectures.

> +
> +#define MEMIF_COOKIE 0x3E31F20
> +#define MEMIF_VERSION_MAJOR  2
> +#define MEMIF_VERSION_MINOR  0
> +#define MEMIF_VERSION((MEMIF_VERSION_MAJOR << 8) | 
> MEMIF_VERSION_MINOR)
> +
> +/*
> + *  Type definitions
> + */
> +
> +typedef enum memif_msg_type {
> + MEMIF_MSG_TYPE_NONE = 0,
> + MEMIF_MSG_TYPE_ACK = 1,
> + MEMIF_MSG_TYPE_HELLO = 2,
> + MEMIF_MSG_TYPE_INIT = 3,
> + MEMIF_MSG_TYPE_ADD_REGION = 4,
> + MEMIF_MSG_TYPE_ADD_RING = 5,
> + MEMIF_MSG_TYPE_CONNECT = 6,
> + MEMIF_MSG_TYPE_CONNECTED = 7,
> + MEMIF_MSG_TYPE_DISCONNECT = 8,

No need to assign numbers to enums, they will be already provided by compiler.
Valid for all enums below.

> +} memif_msg_type_t;
> +
> +typedef enum {
> + MEMIF_RING_S2M = 0,
> + MEMIF_RING_M2S = 1
> +} memif_ring_type_t;
> +
> +typedef enum {
> + MEMIF_INTERFACE_MODE_ETHERNET = 0,
> + MEMIF_INTERFACE_MODE_IP = 1,
> + MEMIF_INTERFACE_MODE_PUNT_INJECT = 2,
> +} memif_interface_mode_t;
> +
> +typedef uint16_t memif_region_index_t;
> +typedef uint32_t memif_region_offset_t;
> +typedef uint64_t memif_region_size_t;
> +typedef uint16_t memif_ring_index_t;
> +typedef uint32_t memif_interface_id_t;
> +typedef uint16_t memif_version_t;
> +typedef uint8_t memif_log2_ring_size_t;
> +
> +/*
> + *  Socket messages
> + */
> +
> +typedef struct __attribute__ ((packed)) {

can use "__rte_packed" instead of "__attribute__ ((packed))"

> + uint8_t 

Re: [dpdk-dev] [PATCH 1/2] vhost/crypto: fix possible dead loop

[Maxime Coquelin]

On 1/4/19 12:22 PM, Fan Zhang wrote:

This patch fixes a possible infinite loop caused by incorrect
descriptor chain created by the driver.

Cc: sta...@dpdk.org
Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")

Signed-off-by: Fan Zhang 
---
  lib/librte_vhost/vhost_crypto.c | 121 +++-
  1 file changed, 82 insertions(+), 39 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 2/2] vhost/crypto: fix possible out of bound access

[Maxime Coquelin]

On 1/4/19 12:22 PM, Fan Zhang wrote:

This patch fixes a out of bound access possbility in vhost
crypto. Originally the incorrect next descriptor index may
cause the library read invalid memory content and crash
the application.

Cc: sta...@dpdk.org
Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")

Signed-off-by: Fan Zhang 
---
  lib/librte_vhost/vhost_crypto.c | 89 -
  1 file changed, 53 insertions(+), 36 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/fm10k: initialize sm_down variable

[Ferruh Yigit]

On 1/4/2019 1:04 PM, Zhang, Qi Z wrote:
> 
> 
>> -Original Message-
>> From: Wang, Xiao W
>> Sent: Thursday, January 3, 2019 10:10 AM
>> To: Julien Meunier ; Zhang, Qi Z
>> 
>> Cc: dev@dpdk.org; sta...@dpdk.org
>> Subject: RE: [PATCH] net/fm10k: initialize sm_down variable
>>
>> Hi Julien,
>>
>>> -Original Message-
>>> From: Julien Meunier [mailto:julien.meun...@nokia.com]
>>> Sent: Wednesday, January 2, 2019 11:58 PM
>>> To: Zhang, Qi Z ; Wang, Xiao W
>>> 
>>> Cc: dev@dpdk.org; sta...@dpdk.org
>>> Subject: [PATCH] net/fm10k: initialize sm_down variable
>>>
>>> Fixes: 6f22f2f67268 ("net/fm10k: redefine link status semantics")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Julien Meunier 
>>> ---
>>>  drivers/net/fm10k/fm10k_ethdev.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
>>> b/drivers/net/fm10k/fm10k_ethdev.c
>>> index 85fb6c5..caf4d1b 100644
>>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>>> @@ -3003,6 +3003,7 @@ fm10k_params_init(struct rte_eth_dev *dev)
>>> hw->bus.payload = fm10k_bus_payload_256;
>>>
>>> info->rx_vec_allowed = true;
>>> +   info->sm_down = false;
>>>  }
>>>
>>>  static int
>>> --
>>> 2.10.2
>>
>> Acked-by: Xiao Wang 
> 
> Applied to dpdk-next-net-intel.

Is it possible to add more information to commit log?
What happens if "sm_down" is not initialized? What "sm_down" is for?
Should patch title be a 'fix' patch?

Re: [dpdk-dev] [PATCH v6 00/10] examples/ipsec-secgw: make app to use ipsec library

[Akhil Goyal]

On 1/4/2019 10:34 PM, Ananyev, Konstantin wrote:
>> On 1/4/2019 5:59 PM, Ananyev, Konstantin wrote:
>>> Hi Akhil,
>>>
 Hi Konstantin,

 With this patchset, I am seeing a 3% drop in legacy mode lookaside and
 12% drop with -l option.
 I am debugging this. Will let you know if I find something.
>>> Ok, thanks.
>>> For legacy mode do you know which patch in the series causing 3% drop?
>>> Is it still: " fix crypto-op might never get dequeued" or a different one?
>>> Konstantin
>>>
>> for legacy mode you may consider this diff in 3/10
>>
>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c
>> b/examples/ipsec-secgw/ipsec-secgw.c
>> index 4f7a77d8d..d183862b8 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>> @@ -1015,10 +1015,13 @@ main_loop(__attribute__((unused)) void *dummy)
>>
>>       if (nb_rx > 0)
>>       process_pkts(qconf, pkts, nb_rx, portid);
>> -   }
>> +// }
>>
>> -   drain_inbound_crypto_queues(qconf, &qconf->inbound);
>> -   drain_outbound_crypto_queues(qconf, &qconf->outbound);
>> +   if (UNPROTECTED_PORT(portid))
>> +   drain_inbound_crypto_queues(qconf,
>> &qconf->inbound);
>> +   else
>> +   drain_outbound_crypto_queues(qconf,
>> &qconf->outbound);
>> +   }
>>       }
>>    }
>>
>> The 3% gap was on single core performance. But on multi cores scenario,
>> there is no significant drop.
>>
>> But I see a bigger functional problem in case of non-legacy mode.
>> I am trying a multi tunnel scenario with 32 kind of flows on one side
>> each with different tunnels.
>> The flows are kind of dest ip: 192.168.101.1 ... 192.168.x.1
>> .  192.168.132.1.
>> Each IP has a different tunnel.
>> All are pumped from same port. I can see that some of the IPs are not
>> getting forwarded.
>> like 192.168.103.1, 192.168.104.1, 192.168.105.1, 192.168.114.1,
>> 192.168.115.1, 192.168.116.1, 192.168.122.1, 192.168.125.1,
>> 192.168.126.1, 192.168.130.1
>>
>> The same setup/flows works perfectly fine with legacy mode.
> I'll try to reproduce it on my box.
> To clarify:
> you are using one core, one port.
4 cores one port with 4 queues

./ipsec-secgw -c 0xff -n 1   -- -p 0x3 -P -u 0x2 
--config="(0,0,0),(0,1,1),(0,2,2),(0,3,3),(1,0,4),(1,1,5),(1,2,6),(1,3,7)" 
-f ep0_64X64_proto.cfg
> each dest ip assigned to different SA
>(i.e. for dst=192.168.103.1, spi=1,  dst=192.168.104.1, spi=2, ...)
yes
>and for some SAs packets are not forwarded.
>I presume each rx burst contains packets from different SAs.
>   correct?
yes
>   Is that failure for inbound or outbound packets or both?
I believe it can be anything, probably because unprotected port is 
trying to dequeue outbound queues.
>   Any particular details about mode(tunnel/transport), algorithm?
tunnel, aes-cbc-hmac-sha1
> 
> Thanks
> Konstantin

Re: [dpdk-dev] [PATCH 0/2] vhost/crypto: some fixes

[Maxime Coquelin]

On 1/4/19 12:22 PM, Fan Zhang wrote:

This patchset fixes two possible issues in vhost crypto. They are:
- The possible dead loop issue
- The out of bound access issue.

The fixes are in line with the following patchset
"[dpdk-dev] [PATCH 0/6] Some fixes for vhost"
(https://mails.dpdk.org/archives/dev/2019-January/122372.html)

Fan Zhang (2):
   vhost/crypto: fix possible dead loop
   vhost/crypto: fix possible out of bound access

  lib/librte_vhost/vhost_crypto.c | 140 
  1 file changed, 100 insertions(+), 40 deletions(-)



Applied to dpdk-next-virtio

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 0/6] Some fixes for vhost

[Maxime Coquelin]

On 1/4/19 5:06 AM, Tiwei Bie wrote:

Tiwei Bie (6):
   vhost: fix desc access in relay helpers
   vhost: fix possible out of bound access in relay helpers
   vhost: fix possible dead loop in relay helpers
   vhost: fix possible out of bound access in vector filling
   vhost: fix possible dead loop in vector filling
   vhost: ensure event idx is mapped when negotiated

  lib/librte_vhost/vdpa.c   | 55 ---
  lib/librte_vhost/vhost_user.c | 15 ++
  lib/librte_vhost/virtio_net.c | 16 ++
  3 files changed, 69 insertions(+), 17 deletions(-)


Applied to dpdk-next-virtio

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 0/2] Some fixes for virtio-user/packed-ring

[Maxime Coquelin]

On 1/3/19 3:40 AM, Tiwei Bie wrote:

Tiwei Bie (2):
   net/virtio-user: fix packed vq option parsing
   net/virtio-user: fix supported features list

  drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 +-
  drivers/net/virtio/virtio_user_ethdev.c  |  7 ---
  2 files changed, 9 insertions(+), 12 deletions(-)


Applied to dpdk-next-virtio

Thanks,
Maxime

Re: [dpdk-dev] [RFC v3] /net: memory interface (memif)

[Stephen Hemminger]

On Thu, 13 Dec 2018 14:30:51 +0100
Jakub Grajciar  wrote:

> +CFLAGS += -O3
> +CFLAGS += -I$(SRCDIR)
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -Wno-pointer-arith

Why this additional compiler flag?

Re: [dpdk-dev] [RFC v3] /net: memory interface (memif)

[Stephen Hemminger]

On Thu, 13 Dec 2018 14:30:51 +0100
Jakub Grajciar  wrote:

> +static ssize_t
> +memif_msg_send(int fd, memif_msg_t *msg, int afd)
> +{
> + struct msghdr mh = { 0 };
> + struct iovec iov[1];
> + char ctl[CMSG_SPACE(sizeof(int))];
> +
> + iov[0].iov_base = (void *)msg;

Since iov_base is of type void * the cast is unnecessary in C.

Re: [dpdk-dev] [RFC v3] /net: memory interface (memif)

[Stephen Hemminger]

On Thu, 13 Dec 2018 14:30:51 +0100
Jakub Grajciar  wrote:

> + role = MEMIF_ROLE_SLAVE;
> + flags = 0;
> + id = 0;
> + buffer_size = 2048;
> + log2_ring_size = 10;
> + nrxq = 1;
> + ntxq = 1;
> + socket_filename = ETH_MEMIF_DEFAULT_SOCKET_FILENAME;
> + secret = NULL;
> + eth_addr = NULL;

Minor nit, since you initialize some of the values when declaring them.
Why not move all these initializations up to where the declaration is.
Just makes everything more compact.


> + /* create interface */
> + ret =
> + memif_create(vdev, role, id, flags, socket_filename, log2_ring_size,
> +  nrxq, ntxq, buffer_size, secret, eth_addr);
> +

Another nit, why split the line like this? Instead of:
ret = memif_create(vdev, role, id, flags, socket_filename,
   log2_ring_size, nrxq, ntxq, buffer_size, secret, 
eth_addr);

Re: [dpdk-dev] [PATCH] doc: add GRO API requirement in prog_guide

[Stephen Hemminger]

On Tue, 18 Dec 2018 12:25:19 +0800
Jiayu Hu  wrote:

> +.. note::
> +To use the GRO library, applications must guarantee that
> +MBUF->l2_len/l3_len/l4_len/outer_l2_len/outer_l3_len and
> +MBUF->pkt_len/data_len of input packets are set to correct
> +values.
> +

This statement seems confusing to me since
this is Generic Receive Offload, and receive buffers come from
the poll mode driver.

Aren't you saying that the PMD must set the correct values.
If this is correct, then any PMD not doing so is buggy.
And should be reported and fixed. If it is really buggy PMD's
then the statement in the programmers guide is not the place to handle
this kind of issue.  Better to just shame the buggy drivers in the
release notes for that NIC.

Re: [dpdk-dev] [PATCH v4 1/2] lib/librte_meter: add RFC4115 trTCM meter support

[Stephen Hemminger]

On Fri,  4 Jan 2019 13:59:42 +
Eelco Chaudron  wrote:

> This patch adds support for RFC4115 trTCM meters.
> 
> Signed-off-by: Eelco Chaudron 



Fix comment formatting.

### [dpdk-dev] [PATCH v4 1/2] lib/librte_meter: add RFC4115 trTCM meter support

WARNING:BLOCK_COMMENT_STYLE: Block comments use * on subsequent lines
#172: FILE: lib/librte_meter/rte_meter.h:59:
+/** trTCM parameters per metered traffic flow. The CIR, EIR, CBS and EBS
+parameters only count bytes of IP packets and do not include link specific

WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#174: FILE: lib/librte_meter/rte_meter.h:61:
+none-zero respectively.*/

WARNING:LONG_LINE_COMMENT: line over 80 characters
#176: FILE: lib/librte_meter/rte_meter.h:63:
+   uint64_t cir; /**< Committed Information Rate (CIR). Measured in bytes 
per second. */

WARNING:LONG_LINE_COMMENT: line over 80 characters
#177: FILE: lib/librte_meter/rte_meter.h:64:
+   uint64_t eir; /**< Excess Information Rate (EIR). Measured in bytes per 
second. */

WARNING:LONG_LINE_COMMENT: line over 80 characters
#355: FILE: lib/librte_meter/rte_meter.h:406:
+   /**< Number of bytes currently available in the excess(E) token bucket 
*/

total: 0 errors, 5 warnings, 356 lines checked

[dpdk-dev] Default cacheline size for ARM

[Yongseok Koh]

Hi,

The cacheline size (RTE_CACHE_LINE_SIZE) for ARM CPUs is set to be 128B by
default. Mellanox's BlueField is an ARM CPU having Cortex-A72 and its CL size is
64B.

I can add config/defconfig_arm64-bluefield-linuxapp-gcc for legacy build anyway.

For the meson build, I know it parses the Main ID register to figure out
Implementor ID and Part Number. However, Mellanox doesn't program our own ID yet
but we set the Part Number as 0xd08 (A72).

According to my folks, ARM's A53, A57, A72, and A73 designs all have 64B CL. If
that's true, can I push a patch to make the change?

Please comment.


Thanks,
Yongseok

Re: [dpdk-dev] [EXT] Default cacheline size for ARM

[Jerin Jacob Kollanukkaran]

On Fri, 2019-01-04 at 19:59 +, Yongseok Koh wrote:
> ---
> ---
> Hi,
> 
> The cacheline size (RTE_CACHE_LINE_SIZE) for ARM CPUs is set to be
> 128B by
> default. Mellanox's BlueField is an ARM CPU having Cortex-A72 and its
> CL size is
> 64B.
> 
> I can add config/defconfig_arm64-bluefield-linuxapp-gcc for legacy
> build anyway.
> 

Makes sense.

> For the meson build, I know it parses the Main ID register to figure
> out
> Implementor ID and Part Number. However, Mellanox doesn't program our
> own ID yet
> but we set the Part Number as 0xd08 (A72).
> According to my folks, ARM's A53, A57, A72, and A73 designs all have
> 64B CL. If
> that's true, can I push a patch to make the change?

Yes. Broadcom Stingray has the same situation i.e Use flags_generic,
machine_args_generic

> 
> Please comment.
> 
> 
> Thanks,
> Yongseok
>