[dpdk-dev] [RFC PATCH] i40e: fix setting of default MAC address

[Igor Ryzhov]

Ping.

On Thu, Nov 24, 2016 at 3:34 PM, Igor Ryzhov  wrote:

> While testing X710 cards in our lab I found that setting of default MAC
> address
> doesn't work correctly for i40e driver. I compared DPDK driver
> implementation
> with Linux driver implementation and found that a lot of code is lost in
> DPDK.
> I tried to make DPDK implementation similar to Linux implementation and it
> worked for me ? now everything is working. But I'm not sure that my
> changes are
> correct so, please, maintainers, check the patch very careful.
>
> Signed-off-by: Igor Ryzhov 
> ---
>  drivers/net/i40e/i40e_ethdev.c | 30 --
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_
> ethdev.c
> index 67778ba..b73f9c8 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -9694,6 +9694,7 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev,
>  static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
>   struct ether_addr *mac_addr)
>  {
> +   struct i40e_vsi *vsi = I40E_DEV_PRIVATE_TO_MAIN_VSI(
> dev->data->dev_private);
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->
> data->dev_private);
>
> if (!is_valid_assigned_ether_addr(mac_addr)) {
> @@ -9701,8 +9702,33 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
> return;
> }
>
> -   /* Flags: 0x3 updates port address */
> -   i40e_aq_mac_address_write(hw, 0x3, mac_addr->addr_bytes, NULL);
> +   i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL,
> mac_addr->addr_bytes, NULL);
> +
> +   if (!memcmp(>data->mac_addrs[0].addr_bytes, hw->mac.addr,
> ETH_ADDR_LEN)) {
> +   struct i40e_aqc_remove_macvlan_element_data element;
> +
> +   memset(, 0, sizeof(element));
> +   memcpy(element.mac_addr, >data->mac_addrs[0].addr_bytes,
> ETH_ADDR_LEN);
> +   element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
> +   i40e_aq_remove_macvlan(hw, vsi->seid, , 1, NULL);
> +   } else {
> +   i40e_vsi_delete_mac(vsi, >data->mac_addrs[0]);
> +   }
> +
> +   if (!memcmp(mac_addr->addr_bytes, hw->mac.addr, ETH_ADDR_LEN)) {
> +   struct i40e_aqc_add_macvlan_element_data element;
> +
> +   memset(, 0, sizeof(element));
> +   memcpy(element.mac_addr, hw->mac.addr, ETH_ADDR_LEN);
> +   element.flags = CPU_TO_LE16(I40E_AQC_MACVLAN_
> ADD_PERFECT_MATCH);
> +   i40e_aq_add_macvlan(hw, vsi->seid, , 1, NULL);
> +   } else {
> +   struct i40e_mac_filter_info filter;
> +
> +   memcpy(_addr, mac_addr, ETH_ADDR_LEN);
> +   filter.filter_type = RTE_MAC_PERFECT_MATCH;
> +   i40e_vsi_add_mac(vsi, );
> +   }
>  }
>
>  static int
> --
> 2.6.4
>
>

[dpdk-dev] [RFC PATCH] i40e: fix setting of default MAC address

[Igor Ryzhov]

While testing X710 cards in our lab I found that setting of default MAC address
doesn't work correctly for i40e driver. I compared DPDK driver implementation
with Linux driver implementation and found that a lot of code is lost in DPDK.
I tried to make DPDK implementation similar to Linux implementation and it
worked for me ? now everything is working. But I'm not sure that my changes are
correct so, please, maintainers, check the patch very careful.

Signed-off-by: Igor Ryzhov 
---
 drivers/net/i40e/i40e_ethdev.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..b73f9c8 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -9694,6 +9694,7 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev,
 static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
  struct ether_addr *mac_addr)
 {
+   struct i40e_vsi *vsi = 
I40E_DEV_PRIVATE_TO_MAIN_VSI(dev->data->dev_private);
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

if (!is_valid_assigned_ether_addr(mac_addr)) {
@@ -9701,8 +9702,33 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev 
*dev,
return;
}

-   /* Flags: 0x3 updates port address */
-   i40e_aq_mac_address_write(hw, 0x3, mac_addr->addr_bytes, NULL);
+   i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL, 
mac_addr->addr_bytes, NULL);
+
+   if (!memcmp(>data->mac_addrs[0].addr_bytes, hw->mac.addr, 
ETH_ADDR_LEN)) {
+   struct i40e_aqc_remove_macvlan_element_data element;
+
+   memset(, 0, sizeof(element));
+   memcpy(element.mac_addr, >data->mac_addrs[0].addr_bytes, 
ETH_ADDR_LEN);
+   element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
+   i40e_aq_remove_macvlan(hw, vsi->seid, , 1, NULL);
+   } else {
+   i40e_vsi_delete_mac(vsi, >data->mac_addrs[0]);
+   }
+
+   if (!memcmp(mac_addr->addr_bytes, hw->mac.addr, ETH_ADDR_LEN)) {
+   struct i40e_aqc_add_macvlan_element_data element;
+
+   memset(, 0, sizeof(element));
+   memcpy(element.mac_addr, hw->mac.addr, ETH_ADDR_LEN);
+   element.flags = CPU_TO_LE16(I40E_AQC_MACVLAN_ADD_PERFECT_MATCH);
+   i40e_aq_add_macvlan(hw, vsi->seid, , 1, NULL);
+   } else {
+   struct i40e_mac_filter_info filter;
+
+   memcpy(_addr, mac_addr, ETH_ADDR_LEN);
+   filter.filter_type = RTE_MAC_PERFECT_MATCH;
+   i40e_vsi_add_mac(vsi, );
+   }
 }

 static int
-- 
2.6.4

[dpdk-dev] KNI discussion in userspace event

[Igor Ryzhov]

On Fri, Oct 28, 2016 at 9:40 PM, Thomas Monjalon 
wrote:

> 2016-10-28 20:29, Igor Ryzhov:
> > On Fri, Oct 28, 2016 Thomas Monjalon wrote:
> > > 2016-10-28 15:51, Richardson, Bruce:
> > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > > > 2016-10-28 15:31, Ferruh Yigit:
> > > > > > * Remove ethtool support ?
> > > > >
> > > > > That's the other part of KNI.
> > > > > It works only for e1000/ixgbe. That's a niche.
> > > >
> > > > Yes, it's something we need to remove, but again, we need an
> > > > alternative first.
> > > >
> > > > >
> > > > > > Still there is some interest, will keep it. But not able to
> extend it
> > > > > > to other drivers with current design.
> > > > >
> > > > > It should be removed one day.
> > > > > We must seriously think about a generic alternative.
> > > > > Either we add DPDK support in ethtool or we create a dpdk-ethtool.
> > > > > (or at least a library as the one in examples/).
> > > >
> > > > I don't view that as a great path forward. Sure, we can do our own
> > > > ethtool, but then people will look for ifconfig to work, and "ip" to
> work,
> > > > etc. I view having a kernel proxy module as the best path here as it
> is
> > > > tool agnostic on the userspace side, rather than trying to make every
> > > > tool for working with kernel netdevs also have support for dpdk
> ports.
> > >
> > > Yes that's the ultimately best solution.
> > > But:
> > > - we need some cooperation of the kernel team
> > > - ethtool manages a device (what DPDK provides) whereas iproute and
> others
> > > manage a TCP/IP stack so is out of control of DPDK.
> >
> > That's not true.
> > iproute can control a lot of things like MAC address, promiscuous, MTU,
> > etc. that cannot be controlled with ethtool.
> > Just compare net_device_ops and ethtool_ops to see the difference.
>
> Yes you're right. iproute was not a good example :)
>
> > And the question is not only about tools, it is also about how Linux
> kernel
> > works with network devices.
> > And it uses net_device_ops, not ethtool_ops.
>
> What do you mean exactly? I feel you have something in mind.
>

My main point is that if we want to control DPDK ports from Linux, it
should be done with standard utilities.
Every standard utility like iproute just uses existing Linux kernel
interfaces and kernel in its turn uses net_device_ops to control the device.

For example, you want to set MTU of the network device.
Regardless of the utility you use to do that (even if you write your own),
there are two options ? ioctl or netlink.
And regardless of the method you choose,  Linux kernel will then call
"ndo_change_mtu".

[dpdk-dev] KNI discussion in userspace event

[Igor Ryzhov]

On Fri, Oct 28, 2016 at 7:13 PM, Thomas Monjalon 
wrote:

> 2016-10-28 15:51, Richardson, Bruce:
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > 2016-10-28 15:31, Ferruh Yigit:
> > > > * virtio-user + vhost-net
> > > > This can be valid alternative, removes the out of tree kernel module
> > > > need. But missing control path. Proof of concept work will be done.
> > >
> > > That's probably a smart alternative for packet injection.
> > > What do you mean exactly by "missing control path"?
> >
> > We'll have to see how it performs - which is the key gap for data path
> that KNI fills. Until we get an alternative with (nearly) equivalent
> performance, there will be demand for KNI to stick around.
> > The "control path" is the ethtool part, to get stats and do operations
> on the NIC using command-line tools.
> >
> > >
> > > > * Remove ethtool support ?
> > >
> > > That's the other part of KNI.
> > > It works only for e1000/ixgbe. That's a niche.
> >
> > Yes, it's something we need to remove, but again, we need an alternative
> first.
> >
> > >
> > > > Still there is some interest, will keep it. But not able to extend it
> > > > to other drivers with current design.
> > >
> > > It should be removed one day.
> > > We must seriously think about a generic alternative.
> > > Either we add DPDK support in ethtool or we create a dpdk-ethtool.
> > > (or at least a library as the one in examples/).
> >
> > I don't view that as a great path forward. Sure, we can do our own
> ethtool, but then people will look for ifconfig to work, and "ip" to work,
> etc. I view having a kernel proxy module as the best path here as it is
> tool agnostic on the userspace side, rather than trying to make every tool
> for working with kernel netdevs also have support for dpdk ports.
>
> Yes that's the ultimately best solution.
> But:
> - we need some cooperation of the kernel team
> - ethtool manages a device (what DPDK provides) whereas iproute and others
> manage a TCP/IP stack so is out of control of DPDK.
>

That's not true.
iproute can control a lot of things like MAC address, promiscuous, MTU,
etc. that cannot be controlled with ethtool.
Just compare net_device_ops and ethtool_ops to see the difference.

And the question is not only about tools, it is also about how Linux kernel
works with network devices.
And it uses net_device_ops, not ethtool_ops.


>
> > > Or we do nothing and wait to have more hardware like Mellanox
> supporting a
> > > kernel bifurcated driver approach.
> >
> > Given the lack of other NICs supporting that, I think it could be quite
> a wait! Also, it doesn't work for virtio ports, for pcap ports, or any
> other ports which don't have physical hardware backing them. No reason you
> shouldn't be able to pull stats from all your dpdk ethdevs, not just the
> ones with physical hardware. The same ethdev APIs work for them, so should
> the same tools.
>
> Yes, very good point.
>
> > > > *KNI PMD
> > > > Patch is in the mail list, missing comments. If it gets some
> > > > interest/comments/acks it may go in to next release.
> > >
> > > I'm not against KNI PMD but it looks strange to add more support to an
> old
> > > dying approach.
> >
> > I think the main idea here is to clean up the API - at least for the
> data path. There is no reason why we need special KNI RX/TX functions, when
> ethdev RX/TX functions could do the job. However, at a higher level, the
> more basic requirement is that whatever solution for the data-path to
> kernel from dpdk is, it needs to appear as an ethdev, and not as a special
> library with different APIs, as KNI is now.
>
> Yes I agree to unifiy (and reduce) API.
> Why this PMD is not more commented?
> KNI users should be interested to review it.
> Please dear community, we need more reviews!
>

[dpdk-dev] KNI discussion in userspace event

[Igor Ryzhov]

Thank you, Ferruh.

As we are staying on the existing implementation, I think we can do some
improvements:
1. Implement more commands for net_device_ops.
2. Implement ethtool support the same way as net_device_ops are implemented
? send commands to application.
3. Add ability to set default MAC address for KNI interface. Now it is
random for all interfaces except those that work on igb or ixgbe.
4. Properly implement link state control feature. Now KNI interface is in
UNKNOWN state even after changing carrier flag to 1.

First two improvements are already done in KCP patches and can be easily
ported into the current code.
For the last two improvements I can send patches.

Best regards,
Igor

[dpdk-dev] rte_eth_dev_config_restore problem

[Igor Ryzhov]

Hello everyone,

I think there is a bug in rte_eth_dev_config_restore function.
During restoration of MAC address configuration, all MAC addresses are
restored with mac_addr_add function, but as I think MAC address with index
0 shouldn't be restored in such way, because it is a default MAC address.

This problem can be solved in two ways:
1. Just call mac_addr_set instead of mac_addr_add for index 0.
2. Don't restore address with index 0 at all and let driver do it.

I think the second option is the right one, because:
1. Some drivers don't support mac_addr_set at all, it means that we must
not touch it.
2. Some drivers already support restoration of default MAC address. For
example, look at the ixgbe "ixgbe_init_rx_addrs_generic" function. It
restores default MAC address if it was overridden by user. All that we have
to do is to rewrite hw->mac.addr in mac_addr_set function.

Best regards,
Igor

[dpdk-dev] rte_eth_dev_attach returns 0, although device is not attached

[Igor Ryzhov]

> 4 ???. 2016 ?., ? 16:21, Ferruh Yigit  ???(?):
> 
> On 8/4/2016 12:51 PM, Igor Ryzhov wrote:
>> Hello Ferruh,
>> 
>>> 4 ???. 2016 ?., ? 14:33, Ferruh Yigit  
>>> ???(?):
>>> 
>>> Hi Igor,
>>> 
>>> On 8/3/2016 5:58 PM, Igor Ryzhov wrote:
>>>> Hello.
>>>> 
>>>> Function rte_eth_dev_attach can return false positive result.
>>>> It happens because rte_eal_pci_probe_one returns zero if no driver is 
>>>> found for the device:
>>>> ret = pci_probe_all_drivers(dev);
>>>> if (ret < 0)
>>>>goto err_return;
>>>> return 0;
>>>> (pci_probe_all_drivers returns 1 in that case)
>>>> 
>>>> For example, it can be easily reproduced by trying to attach virtio 
>>>> device, managed by kernel driver.
>>> 
>>> You are right, and I did able to reproduce this issue with virtio as you
>>> suggest.
>>> 
>>> But I wonder why rte_eth_dev_get_port_by_addr() is not catching this.
>>> Perhaps a dev->attached check needs to be added into this function.
> 
> With a second check, rte_eth_dev_get_port_by_addr() catches it if the
> driver is missing.
> 
> But for virtio case, problem is not missing driver.
> Problem is eth_virtio_dev_init() is returning a positive value on fail.
> 
> Call stack is:
> rte_eal_pci_probe_one
>pci_probe_all_drivers
>rte_eal_pci_probe_one_driver
>rte_eth_dev_init
>   eth_virtio_dev_init
> 
> So rte_eal_pci_probe_one_driver() also returns positive value, as no
> driver found, and rte_eth_dev_get_port_by_addr() returns a valid
> port_id, since rte_eth_dev_init() allocated an eth_dev.
> 
> Briefly, this can be fixed in virtio pmd, instead of eal pci.
> 
>>> 
>>>> 
>>>> I think it should be:
>>>> ret = pci_probe_all_drivers(dev);
>>>> if (ret)
>>>>goto err_return;
>>>> return 0;
>>> 
>>> Your proposal looks good to me. Will you send a patch?
>> 
> 
> Original code silently ignores the if driver is missing for that dev,
> although it is still questionable, I think we can keep this as it is.
> 
>> Patch sent.
> 
> Sorry for this, but can you please test with following modification in
> virtio:
> index 07d6449..c74 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1156,7 +1156,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>if (pci_dev) {
>ret = vtpci_init(pci_dev, hw, _flags);
>if (ret)
> -   return ret;
> +   return -1;
>}
> 
>/* Reset the device although not necessary at startup */

I think it's not a good change, because it will break the idea of this patch - 
http://dpdk.org/browse/dpdk/commit/?id=ac5e1d83 
<http://dpdk.org/browse/dpdk/commit/?id=ac5e1d83>

Also, with your patch the application will not start, because rte_eal_pci_probe 
will fail:

if (ret < 0)
rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
 " cannot be used\n", dev->addr.domain, dev->addr.bus,
 dev->addr.devid, dev->addr.function);

And now I think that maybe we should change the way rte_eal_pci_probe works.
I think we shouldn't stop the application if just one of PCI devices is not 
probed successfully.

> 
> 
>> 
>>> 
>>>> Best regards,
>>>> Igor

[dpdk-dev] [PATCH] pci: fix one device probing

[Igor Ryzhov]

The rte_eal_pci_probe_one function could return false positive result if
no driver is found for the device.

Signed-off-by: Igor Ryzhov 
---
 lib/librte_eal/common/eal_common_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c 
b/lib/librte_eal/common/eal_common_pci.c
index 7248c38..bfb6fd2 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -344,7 +344,7 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
continue;

ret = pci_probe_all_drivers(dev);
-   if (ret < 0)
+   if (ret)
goto err_return;
return 0;
}
-- 
2.6.4

[dpdk-dev] rte_eth_dev_attach returns 0, although device is not attached

[Igor Ryzhov]

Hello.

Function rte_eth_dev_attach can return false positive result.
It happens because rte_eal_pci_probe_one returns zero if no driver is found for 
the device:
ret = pci_probe_all_drivers(dev);
if (ret < 0)
goto err_return;
return 0;
(pci_probe_all_drivers returns 1 in that case)

For example, it can be easily reproduced by trying to attach virtio device, 
managed by kernel driver.

I think it should be:
ret = pci_probe_all_drivers(dev);
if (ret)
goto err_return;
return 0;
Best regards,
Igor

[dpdk-dev] rte_ether: Driver-specific stats getting overwritten

[Igor Ryzhov]

Hello.

How about deleting rx_nombuf from rte_eth_stats?
Do you think this counter is necessary? It just shows enormous numbers in case 
of a lack of processing speed.
But we already have imissed counter which shows real number of packets, dropped 
for the same reason.

> 14  2016 ?., ? 16:37, Thomas Monjalon  
> ???(?):
> 
> 2016-07-14 14:29, Remy Horton:
>> 'noon,
>> 
>> In rte_eth_stats_get() after doing the driver callout to populate struct 
>> rte_eth_stats, the rx_nombuf member is overwritten with 
>> dev->data->rx_mbuf_alloc_failed even though some drivers will have 
>> filled rx_nombuf with a value from elsewhere. This makes assignment of 
>> rx_nombuf from within the driver callout redundant. Is this intentional?
> 
> Yes it is strange and has always been like that.
> Why not moving the assignment before calling the driver callback?

[dpdk-dev] [PATCH] lpm6: fix missing header dependency

[Igor Ryzhov]

Include stdint.h for the definition of uint*_t types.

Signed-off-by: Igor Ryzhov 
---
 lib/librte_lpm/rte_lpm6.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_lpm/rte_lpm6.h b/lib/librte_lpm/rte_lpm6.h
index cedcea8..13d027f 100644
--- a/lib/librte_lpm/rte_lpm6.h
+++ b/lib/librte_lpm/rte_lpm6.h
@@ -38,6 +38,8 @@
  * RTE Longest Prefix Match for IPv6 (LPM6)
  */

+#include 
+
 #ifdef __cplusplus
 extern "C" {
 #endif
-- 
2.6.4

[dpdk-dev] [PATCH] kni: set kni mac on ioctl_create

[Igor Ryzhov]

v2 is here, sorry: http://www.dpdk.org/dev/patchwork/patch/12190/.

> 22 ???. 2016 ?., ? 4:57, Zhang, Helin  ???(?):
> 
> 
> 
> From: Igor Ryzhov [mailto:iryzhov at nfware.com] 
> Sent: Thursday, April 21, 2016 11:16 PM
> To: Zhang, Helin
> Cc: Sergey Balabanov; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: set kni mac on ioctl_create
> 
> Hello.
> 
> I rebased a patch and added Suggested-by string.
> Check it, please: http://dpdk.org/dev/patchwork/patch/12188/.
> [Helin] is that the v2 version? It seems that I cannot find that.
> 
> Best regards,
> Igor
> 
> 18 ? 2016 ?., ? 5:14, Zhang, Helin  ???(?):
> 
> Hi Sergey
> 
> 
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Sergey Balabanov
> Sent: Friday, August 28, 2015 9:06 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] kni: set kni mac on ioctl_create
> 
> There is a situation when ioctl returns zero mac address (00:00:00:00:00:00)
> for just created kni. The situation happens because kni mac is set on 
> 'ipconfig
> up' event (kni_net_open callback) not on kni creation (kni_ioctl_create).
> Could you help to clarify a bit of the real issue? What's wrong there?
> 
> 
> 
> Signed-off-by: Sergey Balabanov 
> ---
> lib/librte_eal/linuxapp/kni/kni_misc.c | 10 ++
> lib/librte_eal/linuxapp/kni/kni_net.c  |  9 -
> 2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
> b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 2e9fa89..61f83a0 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -28,6 +28,7 @@
> #include 
> #include 
> #include 
> +#include  /* eth_type_trans */
> 
> #include 
> #include "kni_dev.h"
> @@ -465,6 +466,15 @@ kni_ioctl_create(unsigned int ioctl_num, unsigned
> long ioctl_param)
>if (pci)
>pci_dev_put(pci);
> 
> +if (kni->lad_dev)
> +memcpy(net_dev->dev_addr, kni->lad_dev->dev_addr,
> ETH_ALEN);
> +else
> +/*
> + * Generate random mac address. eth_random_addr() is the
> newer
> + * version of generating mac address in linux kernel.
> + */
> +random_ether_addr(net_dev->dev_addr);
> +
> A rebase is needed, as a lot of changes after that. Thanks!
> 
> Helin
> 
>ret = register_netdev(net_dev);
>if (ret) {
>KNI_ERR("error %i registering device \"%s\"\n", diff --git
> a/lib/librte_eal/linuxapp/kni/kni_net.c 
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index ab5add4..b50b4cf 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -70,15 +70,6 @@ kni_net_open(struct net_device *dev)
>struct rte_kni_request req;
>struct kni_dev *kni = netdev_priv(dev);
> 
> -if (kni->lad_dev)
> -memcpy(dev->dev_addr, kni->lad_dev->dev_addr,
> ETH_ALEN);
> -else
> -/*
> - * Generate random mac address. eth_random_addr() is the
> newer
> - * version of generating mac address in linux kernel.
> - */
> -random_ether_addr(dev->dev_addr);
> -
>netif_start_queue(dev);
> 
>memset(, 0, sizeof(req));
> --
> 2.1.4
> 

[dpdk-dev] [PATCH v2] kni: don't reassign ethernet address every time an interface goes up

[Igor Ryzhov]

Currently every time a KNI interface goes up, its ethernet address is 
reassigned.
After this patch ethernet address is assigned only once, at initialization time.

Suggested-by: Sergey Balabanov 
Signed-off-by: Igor Ryzhov 
---
v2:
- change variable name from dev to net_dev
- slightly rewrite commit message header and body

 lib/librte_eal/linuxapp/kni/kni_misc.c | 10 ++
 lib/librte_eal/linuxapp/kni/kni_net.c  |  9 -
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c 
b/lib/librte_eal/linuxapp/kni/kni_misc.c
index ae8133f..871437f 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -542,6 +543,15 @@ kni_ioctl_create(struct net *net,
if (pci)
pci_dev_put(pci);

+   if (kni->lad_dev)
+   memcpy(net_dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
+   else
+   /*
+* Generate random mac address. eth_random_addr() is the newer
+* version of generating mac address in linux kernel.
+*/
+   random_ether_addr(net_dev->dev_addr);
+
ret = register_netdev(net_dev);
if (ret) {
KNI_ERR("error %i registering device \"%s\"\n",
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c 
b/lib/librte_eal/linuxapp/kni/kni_net.c
index cfa8339..3d2d246 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -69,15 +69,6 @@ kni_net_open(struct net_device *dev)
struct rte_kni_request req;
struct kni_dev *kni = netdev_priv(dev);

-   if (kni->lad_dev)
-   memcpy(dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
-   else
-   /*
-* Generate random mac address. eth_random_addr() is the newer
-* version of generating mac address in linux kernel.
-*/
-   random_ether_addr(dev->dev_addr);
-
netif_start_queue(dev);

memset(, 0, sizeof(req));
-- 
2.6.4

[dpdk-dev] [PATCH] kni: don't rewrite ethernet address every time an interface goes up

[Igor Ryzhov]

Self nack. Forgot to change variable name from dev to net_dev. Will send v2.

> 21 ???. 2016 ?., ? 18:12, Igor Ryzhov  ???(?):
> 
> Now every time a KNI interface goes up, its ethernet address is overwritten.
> After this patch ethernet address is assigned only once, at initialization
> time.
> 
> Suggested-by: Sergey Balabanov 
> Signed-off-by: Igor Ryzhov 
> ---
> lib/librte_eal/linuxapp/kni/kni_misc.c | 10 ++
> lib/librte_eal/linuxapp/kni/kni_net.c  |  9 -
> 2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c 
> b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index ae8133f..45bcf32 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -26,6 +26,7 @@
> #include 
> #include 
> #include 
> +#include 
> #include 
> #include 
> #include 
> @@ -542,6 +543,15 @@ kni_ioctl_create(struct net *net,
>   if (pci)
>   pci_dev_put(pci);
> 
> + if (kni->lad_dev)
> + memcpy(dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
> + else
> + /*
> +  * Generate random mac address. eth_random_addr() is the newer
> +  * version of generating mac address in linux kernel.
> +  */
> + random_ether_addr(dev->dev_addr);
> +
>   ret = register_netdev(net_dev);
>   if (ret) {
>   KNI_ERR("error %i registering device \"%s\"\n",
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c 
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index cfa8339..3d2d246 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -69,15 +69,6 @@ kni_net_open(struct net_device *dev)
>   struct rte_kni_request req;
>   struct kni_dev *kni = netdev_priv(dev);
> 
> - if (kni->lad_dev)
> - memcpy(dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
> - else
> - /*
> -  * Generate random mac address. eth_random_addr() is the newer
> -  * version of generating mac address in linux kernel.
> -  */
> - random_ether_addr(dev->dev_addr);
> -
>   netif_start_queue(dev);
> 
>   memset(, 0, sizeof(req));
> -- 
> 2.6.4
> 

[dpdk-dev] [PATCH] kni: set kni mac on ioctl_create

[Igor Ryzhov]

Hello.

I rebased a patch and added Suggested-by string.
Check it, please: http://dpdk.org/dev/patchwork/patch/12188/ 
.

Best regards,
Igor

> 18 ? 2016 ?., ? 5:14, Zhang, Helin  ???(?):
> 
> Hi Sergey
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org ] 
>> On Behalf Of Sergey Balabanov
>> Sent: Friday, August 28, 2015 9:06 PM
>> To: dev at dpdk.org 
>> Subject: [dpdk-dev] [PATCH] kni: set kni mac on ioctl_create
>> 
>> There is a situation when ioctl returns zero mac address (00:00:00:00:00:00)
>> for just created kni. The situation happens because kni mac is set on 
>> 'ipconfig
>> up' event (kni_net_open callback) not on kni creation (kni_ioctl_create).
> Could you help to clarify a bit of the real issue? What's wrong there?
> 
>> 
>> Signed-off-by: Sergey Balabanov 
>> ---
>> lib/librte_eal/linuxapp/kni/kni_misc.c | 10 ++
>> lib/librte_eal/linuxapp/kni/kni_net.c  |  9 -
>> 2 files changed, 10 insertions(+), 9 deletions(-)
>> 
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> index 2e9fa89..61f83a0 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> @@ -28,6 +28,7 @@
>> #include 
>> #include 
>> #include 
>> +#include  /* eth_type_trans */
>> 
>> #include 
>> #include "kni_dev.h"
>> @@ -465,6 +466,15 @@ kni_ioctl_create(unsigned int ioctl_num, unsigned
>> long ioctl_param)
>>  if (pci)
>>  pci_dev_put(pci);
>> 
>> +if (kni->lad_dev)
>> +memcpy(net_dev->dev_addr, kni->lad_dev->dev_addr,
>> ETH_ALEN);
>> +else
>> +/*
>> + * Generate random mac address. eth_random_addr() is the
>> newer
>> + * version of generating mac address in linux kernel.
>> + */
>> +random_ether_addr(net_dev->dev_addr);
>> +
> A rebase is needed, as a lot of changes after that. Thanks!
> 
> Helin
>>  ret = register_netdev(net_dev);
>>  if (ret) {
>>  KNI_ERR("error %i registering device \"%s\"\n", diff --git
>> a/lib/librte_eal/linuxapp/kni/kni_net.c 
>> b/lib/librte_eal/linuxapp/kni/kni_net.c
>> index ab5add4..b50b4cf 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
>> @@ -70,15 +70,6 @@ kni_net_open(struct net_device *dev)
>>  struct rte_kni_request req;
>>  struct kni_dev *kni = netdev_priv(dev);
>> 
>> -if (kni->lad_dev)
>> -memcpy(dev->dev_addr, kni->lad_dev->dev_addr,
>> ETH_ALEN);
>> -else
>> -/*
>> - * Generate random mac address. eth_random_addr() is the
>> newer
>> - * version of generating mac address in linux kernel.
>> - */
>> -random_ether_addr(dev->dev_addr);
>> -
>>  netif_start_queue(dev);
>> 
>>  memset(, 0, sizeof(req));
>> --
>> 2.1.4

[dpdk-dev] [PATCH] kni: don't rewrite ethernet address every time an interface goes up

[Igor Ryzhov]

Now every time a KNI interface goes up, its ethernet address is overwritten.
After this patch ethernet address is assigned only once, at initialization
time.

Suggested-by: Sergey Balabanov 
Signed-off-by: Igor Ryzhov 
---
 lib/librte_eal/linuxapp/kni/kni_misc.c | 10 ++
 lib/librte_eal/linuxapp/kni/kni_net.c  |  9 -
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c 
b/lib/librte_eal/linuxapp/kni/kni_misc.c
index ae8133f..45bcf32 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -542,6 +543,15 @@ kni_ioctl_create(struct net *net,
if (pci)
pci_dev_put(pci);

+   if (kni->lad_dev)
+   memcpy(dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
+   else
+   /*
+* Generate random mac address. eth_random_addr() is the newer
+* version of generating mac address in linux kernel.
+*/
+   random_ether_addr(dev->dev_addr);
+
ret = register_netdev(net_dev);
if (ret) {
KNI_ERR("error %i registering device \"%s\"\n",
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c 
b/lib/librte_eal/linuxapp/kni/kni_net.c
index cfa8339..3d2d246 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -69,15 +69,6 @@ kni_net_open(struct net_device *dev)
struct rte_kni_request req;
struct kni_dev *kni = netdev_priv(dev);

-   if (kni->lad_dev)
-   memcpy(dev->dev_addr, kni->lad_dev->dev_addr, ETH_ALEN);
-   else
-   /*
-* Generate random mac address. eth_random_addr() is the newer
-* version of generating mac address in linux kernel.
-*/
-   random_ether_addr(dev->dev_addr);
-
netif_start_queue(dev);

memset(, 0, sizeof(req));
-- 
2.6.4

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

[Igor Ryzhov]

Ping.

CCing to maintainers of affected drivers.

> 10 ? 2016 ?., ? 16:03, Igor Ryzhov  ???(?):
> 
> Comment for "ierrors" counter says that it counts erroneous received packets. 
> But for some reason "imissed" counter is added to "ierrors" counter in most 
> drivers. It is a mistake, because missed packets are obviously not received. 
> This patch fixes it.
> 
> Signed-off-by: Igor Ryzhov 
> ---
> app/test-pmd/testpmd.c   | 4 ++--
> drivers/net/cxgbe/cxgbe_ethdev.c | 2 +-
> drivers/net/e1000/em_ethdev.c| 1 -
> drivers/net/e1000/igb_ethdev.c   | 1 -
> drivers/net/i40e/i40e_ethdev.c   | 3 +--
> drivers/net/ixgbe/ixgbe_ethdev.c | 1 -
> 6 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 269ef81..d3d733b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -753,7 +753,7 @@ fwd_port_stats_display(portid_t port_id, struct 
> rte_eth_stats *stats)
>   if (cur_fwd_eng == _fwd_engine)
>   printf("  Bad-ipcsum: %-14"PRIu64" Bad-l4csum: 
> %-14"PRIu64" \n",
>  port->rx_bad_ip_csum, port->rx_bad_l4_csum);
> - if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
> {
> + if ((stats->ierrors + stats->rx_nombuf) > 0) {
>   printf("  RX-error: %-"PRIu64"\n",  stats->ierrors);
>   printf("  RX-nombufs: %-14"PRIu64"\n", 
> stats->rx_nombuf);
>   }
> @@ -772,7 +772,7 @@ fwd_port_stats_display(portid_t port_id, struct 
> rte_eth_stats *stats)
>   if (cur_fwd_eng == _fwd_engine)
>   printf("  Bad-ipcsum:%14"PRIu64"
> Bad-l4csum:%14"PRIu64"\n",
>  port->rx_bad_ip_csum, port->rx_bad_l4_csum);
> - if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
> {
> + if ((stats->ierrors + stats->rx_nombuf) > 0) {
>   printf("  RX-error:%"PRIu64"\n", stats->ierrors);
>   printf("  RX-nombufs: %14"PRIu64"\n",
>  stats->rx_nombuf);
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c 
> b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 97ef152..0070e2a 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -662,7 +662,7 @@ static void cxgbe_dev_stats_get(struct rte_eth_dev 
> *eth_dev,
> ps.rx_trunc2 + ps.rx_trunc3;
>   eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
> ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
> -   ps.rx_len_err + eth_stats->imissed;
> +   ps.rx_len_err;
> 
>   /* TX Stats */
>   eth_stats->opackets = ps.tx_frames;
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 4a843fe..27ace6d 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -914,7 +914,6 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *rte_stats)
>   rte_stats->imissed = stats->mpc;
>   rte_stats->ierrors = stats->crcerrs +
>stats->rlec + stats->ruc + stats->roc +
> -  rte_stats->imissed +
>stats->rxerrc + stats->algnerrc + stats->cexterr;
> 
>   /* Tx Errors */
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 4ed5e95..6e93214 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1640,7 +1640,6 @@ eth_igb_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *rte_stats)
>   rte_stats->imissed = stats->mpc;
>   rte_stats->ierrors = stats->crcerrs +
>stats->rlec + stats->ruc + stats->roc +
> -  rte_stats->imissed +
>stats->rxerrc + stats->algnerrc + stats->cexterr;
> 
>   /* Tx Errors */
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7e68c61..7d68d4d 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2062,8 +2062,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct 
> rte_eth_stats *stats)
>   pf->main_vsi->eth_stats.rx_discards;
>   stats

[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter

[Igor Ryzhov]

Comment for "ierrors" counter says that it counts erroneous received packets. 
But for some reason "imissed" counter is added to "ierrors" counter in most 
drivers. It is a mistake, because missed packets are obviously not received. 
This patch fixes it.

Signed-off-by: Igor Ryzhov 
---
 app/test-pmd/testpmd.c   | 4 ++--
 drivers/net/cxgbe/cxgbe_ethdev.c | 2 +-
 drivers/net/e1000/em_ethdev.c| 1 -
 drivers/net/e1000/igb_ethdev.c   | 1 -
 drivers/net/i40e/i40e_ethdev.c   | 3 +--
 drivers/net/ixgbe/ixgbe_ethdev.c | 1 -
 6 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 269ef81..d3d733b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -753,7 +753,7 @@ fwd_port_stats_display(portid_t port_id, struct 
rte_eth_stats *stats)
if (cur_fwd_eng == _fwd_engine)
printf("  Bad-ipcsum: %-14"PRIu64" Bad-l4csum: 
%-14"PRIu64" \n",
   port->rx_bad_ip_csum, port->rx_bad_l4_csum);
-   if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
{
+   if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf("  RX-error: %-"PRIu64"\n",  stats->ierrors);
printf("  RX-nombufs: %-14"PRIu64"\n", 
stats->rx_nombuf);
}
@@ -772,7 +772,7 @@ fwd_port_stats_display(portid_t port_id, struct 
rte_eth_stats *stats)
if (cur_fwd_eng == _fwd_engine)
printf("  Bad-ipcsum:%14"PRIu64"
Bad-l4csum:%14"PRIu64"\n",
   port->rx_bad_ip_csum, port->rx_bad_l4_csum);
-   if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0) 
{
+   if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf("  RX-error:%"PRIu64"\n", stats->ierrors);
printf("  RX-nombufs: %14"PRIu64"\n",
   stats->rx_nombuf);
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 97ef152..0070e2a 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -662,7 +662,7 @@ static void cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
  ps.rx_trunc2 + ps.rx_trunc3;
eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
  ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
- ps.rx_len_err + eth_stats->imissed;
+ ps.rx_len_err;

/* TX Stats */
eth_stats->opackets = ps.tx_frames;
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 4a843fe..27ace6d 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -914,7 +914,6 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
rte_stats->imissed = stats->mpc;
rte_stats->ierrors = stats->crcerrs +
 stats->rlec + stats->ruc + stats->roc +
-rte_stats->imissed +
 stats->rxerrc + stats->algnerrc + stats->cexterr;

/* Tx Errors */
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 4ed5e95..6e93214 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1640,7 +1640,6 @@ eth_igb_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
rte_stats->imissed = stats->mpc;
rte_stats->ierrors = stats->crcerrs +
 stats->rlec + stats->ruc + stats->roc +
-rte_stats->imissed +
 stats->rxerrc + stats->algnerrc + stats->cexterr;

/* Tx Errors */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7e68c61..7d68d4d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2062,8 +2062,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
pf->main_vsi->eth_stats.rx_discards;
stats->ierrors  = ns->crc_errors +
ns->rx_length_errors + ns->rx_undersize +
-   ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
-   stats->imissed;
+   ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;

PMD_DRV_LOG(DEBUG, "* PF stats start 
***");
PMD_DRV_LOG(DEBUG, "rx_bytes:

[dpdk-dev] Virtio xstats problem

[Igor Ryzhov]

Sent a patch: http://dpdk.org/dev/patchwork/patch/10887/ 
<http://dpdk.org/dev/patchwork/patch/10887/>.

> 26 . 2016 ?., ? 17:35, Igor Ryzhov  ???(?):
> 
> Hello, Harry.
> 
> Understood about size of packets. It's a bit confusing, because in all other 
> drivers undersized packet is an error. Maybe we should add another one size 
> bin for virtio - 60 to 63 bytes?
> 
> I already checked about multicast/broadcast counters - broadcast packets are 
> counted twice:
> 
>   vq->multicast += is_multicast_ether_addr(ea);
>   vq->broadcast += is_broadcast_ether_addr(ea);
> 
> I think it should be something like:
> 
>   if (is_multicast_ether_addr(ea)) {
>   if (is_broadcast_ether_addr(ea)) {
>   vq->broadcast++;
>   } else {
>   vq->multicast++;
>   }
>   }
> 
> Best regards,
> Igor
> 
>> 26 . 2016 ?., ? 17:29, Van Haaren, Harry  
>> ???(?):
>> 
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
>>> I found some problem with virtio xstats counters.
>>> 
>>> Example:
>>> 
>>> rx_good_packets: 3
>>> rx_good_bytes: 180
>>> rx_errors: 0
>>> 
>>> rx_q0_good_packets: 3
>>> rx_q0_good_bytes: 180
>>> rx_q0_errors: 0
>>> 
>>> rx_q0_multicast_packets: 3
>>> rx_q0_broadcast_packets: 1
>>> rx_q0_undersize_packets: 3
>> 
>>> It means that undersize packets are counted as good packets instead of 
>>> errors.
>> 
>> Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
>> interfaces, so 60 bytes per packet is OK.
>> 
>>> Or maybe
>>> size of packet is calculated wrong.
>>> I don't have time now to check it more deeply - I can do it sometime later, 
>>> but maybe
>>> someone want to help.
>> 
>> Are the packets multicast or broadcast?
>> It looks like one of the counters there is wrong.
>> 
>>> PS. Is it a common practice to count broadcast packets twice - in broadcast 
>>> and multicast
>>> counters?
>> 
>> No packet should be counted twice - it must be put into one bucket of mutli, 
>> broad or unicast.
>> 
>> -Harry
> 

[dpdk-dev] [PATCH] virtio: don't count broadcast packets in multicast packets counter

[Igor Ryzhov]

Signed-off-by: Igor Ryzhov 
---
 drivers/net/virtio/virtio_rxtx.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 41a1366..fe18e1d 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -543,8 +543,13 @@ virtio_update_packet_stats(struct virtqueue *vq, struct 
rte_mbuf *mbuf)
}

ea = rte_pktmbuf_mtod(mbuf, struct ether_addr *);
-   vq->multicast += is_multicast_ether_addr(ea);
-   vq->broadcast += is_broadcast_ether_addr(ea);
+   if (is_multicast_ether_addr(ea)) {
+   if (is_broadcast_ether_addr(ea)) {
+   vq->broadcast++;
+   } else {
+   vq->multicast++;
+   }
+   }
 }

 #define VIRTIO_MBUF_BURST_SZ 64
-- 
2.6.4

[dpdk-dev] Virtio xstats problem

[Igor Ryzhov]

Hello, Harry.

Understood about size of packets. It's a bit confusing, because in all other 
drivers undersized packet is an error. Maybe we should add another one size bin 
for virtio - 60 to 63 bytes?

I already checked about multicast/broadcast counters - broadcast packets are 
counted twice:

vq->multicast += is_multicast_ether_addr(ea);
vq->broadcast += is_broadcast_ether_addr(ea);

I think it should be something like:

if (is_multicast_ether_addr(ea)) {
if (is_broadcast_ether_addr(ea)) {
vq->broadcast++;
} else {
vq->multicast++;
}
}

Best regards,
Igor

> 26 . 2016 ?., ? 17:29, Van Haaren, Harry  
> ???(?):
> 
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
>> I found some problem with virtio xstats counters.
>> 
>> Example:
>> 
>> rx_good_packets: 3
>> rx_good_bytes: 180
>> rx_errors: 0
>> 
>> rx_q0_good_packets: 3
>> rx_q0_good_bytes: 180
>> rx_q0_errors: 0
>> 
>> rx_q0_multicast_packets: 3
>> rx_q0_broadcast_packets: 1
>> rx_q0_undersize_packets: 3
> 
>> It means that undersize packets are counted as good packets instead of 
>> errors.
> 
> Are you sending 64 byte packets? There are no 4 bytes of CRC on virtual
> interfaces, so 60 bytes per packet is OK.
> 
>> Or maybe
>> size of packet is calculated wrong.
>> I don't have time now to check it more deeply - I can do it sometime later, 
>> but maybe
>> someone want to help.
> 
> Are the packets multicast or broadcast?
> It looks like one of the counters there is wrong.
> 
>> PS. Is it a common practice to count broadcast packets twice - in broadcast 
>> and multicast
>> counters?
> 
> No packet should be counted twice - it must be put into one bucket of mutli, 
> broad or unicast.
> 
> -Harry

[dpdk-dev] Virtio xstats problem

[Igor Ryzhov]

Hello.

I found some problem with virtio xstats counters.

Example:

rx_good_packets: 3
rx_good_bytes: 180
rx_errors: 0

rx_q0_good_packets: 3
rx_q0_good_bytes: 180
rx_q0_errors: 0

rx_q0_multicast_packets: 3
rx_q0_broadcast_packets: 1
rx_q0_undersize_packets: 3

It means that undersize packets are counted as good packets instead of errors. 
Or maybe size of packet is calculated wrong.
I don't have time now to check it more deeply - I can do it sometime later, but 
maybe someone want to help.

PS. Is it a common practice to count broadcast packets twice - in broadcast and 
multicast counters?

Best regards,
Igor

[dpdk-dev] Future Direction for rte_eth_stats_get()

[Igor Ryzhov]

Hello, everyone.

How about exposing stats according to IF-MIB?

Statistics to be exposed are - octets, unicast packets, multicast packets, 
broadcast packets, errors and discards for both TX and RX.
These counters are basic and implemented by most of drivers.

All other driver-specific counters can be exposed by xstats.

Best regards,
Igor

> 22 ???. 2016 ?., ? 17:44, Thomas Monjalon  
> ???(?):
> 
> 2016-01-22 14:18, Tahhan, Maryam:
>> So what can be enabled again in struct rte_eth_stats  from what was already 
>> there is the equivalent of: 
>> * rx_length_errors
>> * rx_crc_errors
>> * rx_missed_errors - the deprecation notice was removed for this field.
>> * multicast
>> 
>> What should be added in to distinguish between errors and drops. struct 
>> rte_eth_stats :
>> * rx_errors
>> * tx_errors
>> 
>> As for the detailed rx errors and tx errors I'm open to feedback from you 
>> folks as to what should go in and what is too detailed. These weren't in 
>> struct rte_eth_stats previously, they are available through xstats and are 
>> uniformly named across the drivers. Oliver + Harry any thoughts?
>> 
>> David I assume you are looking for all the missing fields to be added?
> 
> They are not missing. They just not exactly match ones having a
> long history in Linux kernel.
> Please let's avoid to blindly mimic others without thinking
> about modern needs.
> 
 From: David Harton
> Is there a reason the stats have been deprecated?  Why not keep
> the stats in line with the standard linux practices such as
> rtnl_link_stats64?
> 

[dpdk-dev] [PATCH 2/2] virtio: remove unnecessary rx_mbuf_alloc_failed counter clearing

[Igor Ryzhov]

This counter is cleared in rte_eth_stats_reset.

Signed-off-by: Igor Ryzhov 
---
 drivers/net/virtio/virtio_ethdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index 74c00ee..f5b72a3 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -788,8 +788,6 @@ virtio_dev_stats_reset(struct rte_eth_dev *dev)
rxvq->broadcast = 0;
memset(rxvq->size_bins, 0, sizeof(rxvq->size_bins[0]) * 8);
}
-
-   dev->data->rx_mbuf_alloc_failed = 0;
 }

 static void
-- 
2.4.9 (Apple Git-60)

[dpdk-dev] [PATCH 1/2] ethdev: clear rx_mbuf_alloc_failed counter on rte_eth_stats_reset

[Igor Ryzhov]

---
 lib/librte_ether/rte_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 3840775..41f5f0b 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1456,6 +1456,7 @@ rte_eth_stats_reset(uint8_t port_id)

RTE_FUNC_PTR_OR_RET(*dev->dev_ops->stats_reset);
(*dev->dev_ops->stats_reset)(dev);
+   dev->data->rx_mbuf_alloc_failed = 0;
 }

 /* retrieve ethdev extended statistics */
-- 
2.4.9 (Apple Git-60)

[dpdk-dev] [PATCH 0/2] Clear rx_mbuf_alloc_failed counter on rte_eth_stats_reset

[Igor Ryzhov]

The rx_mbuf_alloc_failed counter was only cleared by virtio driver.
Now it is cleared by common rte_eth_stats_clear function for all drivers at 
once.

Igor Ryzhov (2):
  ethdev: clear rx_mbuf_alloc_failed counter on rte_eth_stats_reset
  virtio: remove unnecessary rx_mbuf_alloc_failed counter clearing

 drivers/net/virtio/virtio_ethdev.c | 2 --
 lib/librte_ether/rte_ethdev.c  | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
2.4.9 (Apple Git-60)

[dpdk-dev] IXGBE error statistics

[Igor Ryzhov]

Hi, Harry.

Sorry, but minimum size of packets we can generate is 64 bytes long.

Best regards,
Igor

> 21 . 2015 ?., ? 16:45, Van Haaren, Harry  
> ???(?):
> 
>> From: Igor Ryzhov [mailto:iryzhov at arccn.ru]
>> Thank you, I'll wait for result of mspdc testing.
> 
> Hi Igor,
> 
> Regarding your original question:
> The datasheet says that a packet with total size < 32 bytes is
> discarded by MAC layer and counted in the mspdc register.
> 
> If possible, I would like to add a test for this type of packet when
> testing the accuracy of xstats. I've tried using a packet generator
> (48 bytes seems the smallest it is willing to generate for me), and
> using another NIC won't work, as smaller packets are padded to
> 64 bytes on TX.
> 
> Do you have a method of reproducing this < 32 byte mspdc error packet? 
> -Harry

[dpdk-dev] IXGBE error statistics

[Igor Ryzhov]

Hello, Harry.

Thank you, I'll wait for result of mspdc testing.

About rte_eth_stats - I found that not generic fields of the structure are all 
deprecated already. I will research xstats API, thank you.

Best regards,
Igor

> 18 . 2015 ?., ? 11:04, Van Haaren, Harry  
> ???(?):
> 
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
>> Hello everyone.
> 
> Hi Igor,
> 
>> Investigating IXGBE driver I found an mspdc counter (MAC Short Packet
>> Discard). And I am wondering why this counter is not used in the calculation
>> of total RX errors (ierrors field in rte_eth_stats structure). Is it already 
>> a part
>> of another counter, for example, rlec (Receive Length Error)? Or is it a bug?
> 
> There has been a discussion on list recently involving ixgbe stats, and 
> certain
> packets triggering multiple stats registers - the datasheet doesn't mention
> this could be the case for the mspdc register, I will research this issue and
> get back to you.
> 
>> Another one question is about incompleteness of rte_eth_stats structure.
>> IXGBE and other drivers have a lot of counters but only a part of them is
>> represented in rte_eth_stats. Is there any valuable reasons for that or it's
>> just not implemented?
> 
> The rte_eth_stats struct presents the most general statistics that every NIC 
> exposes.
> In 2.1, and extended statistics API was added which allows NICs to expose 
> stats
> that are unique to that NIC. Currently ixgbe is the only driver that has the 
> xstats API
> implemented, I am working on patches to implement the functionality for the 
> other
> Intel drivers.
> 
> As part of testing the xstats implementation for each driver, I can test the 
> exact
> behavior of the mspdc counter, and if it is mis-counted this should become 
> clear.
> 
> Cheers, -Harry

[dpdk-dev] IXGBE error statistics

[Igor Ryzhov]

Hello everyone.

Investigating IXGBE driver I found an mspdc counter (MAC Short Packet Discard). 
And I am wondering why this counter is not used in the calculation of total RX 
errors (ierrors field in rte_eth_stats structure). Is it already a part of 
another counter, for example, rlec (Receive Length Error)? Or is it a bug?

Another one question is about incompleteness of rte_eth_stats structure. IXGBE 
and other drivers have a lot of counters but only a part of them is represented 
in rte_eth_stats. Is there any valuable reasons for that or it's just not 
implemented?

Best regards,
Igor Ryzhov

[dpdk-dev] [PATCH v2 2/2] Filling speed capability bitmaps in the PMDs

[Igor Ryzhov]

Hello, Marc!

You swapped values for X710 and XL710 - you use 1G and 10G for XL710, 10G and 
40G for X710.

Best regards,
Igor

> 26 ??? 2015 ?., ? 22:50, Marc Sune  ???(?):
> 
> Added speed capabilities to all pmds supporting physical NICs:
> 
> * e1000
> * ixgbe
> * i40
> * mlx4
> * fm10k
> 
> Signed-off-by: Marc Sune 
> ---
> drivers/net/e1000/em_ethdev.c|  6 ++
> drivers/net/e1000/igb_ethdev.c   |  6 ++
> drivers/net/fm10k/fm10k_ethdev.c |  3 +++
> drivers/net/i40e/i40e_ethdev.c   |  9 +
> drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++
> drivers/net/mlx4/mlx4.c  |  6 ++
> 6 files changed, 40 insertions(+)
> 
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index d28030e..8e25cfa 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -883,6 +883,12 @@ eth_em_infos_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *dev_info)
> 
>   dev_info->max_rx_queues = 1;
>   dev_info->max_tx_queues = 1;
> +
> + dev_info->speed_capa = ETH_SPEED_CAP_10M_HD |
> + ETH_SPEED_CAP_10M_FD |
> + ETH_SPEED_CAP_100M_HD |
> + ETH_SPEED_CAP_100M_FD |
> + ETH_SPEED_CAP_1G;
> }
> 
> /* return 0 means link status changed, -1 means not changed */
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index e4b370d..424ad6f 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1398,6 +1398,12 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *dev_info)
>   },
>   .txq_flags = 0,
>   };
> +
> + dev_info->speed_capa = ETH_SPEED_CAP_10M_HD |
> + ETH_SPEED_CAP_10M_FD |
> + ETH_SPEED_CAP_100M_HD |
> + ETH_SPEED_CAP_100M_FD |
> + ETH_SPEED_CAP_1G;
> }
> 
> static void
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c 
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 275c19c..e97b857 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -791,6 +791,9 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
>   ETH_TXQ_FLAGS_NOOFFLOADS,
>   };
> 
> + dev_info->speed_capa = ETH_SPEED_CAP_1G | ETH_SPEED_CAP_2_5G |
> + ETH_SPEED_CAP_10G | ETH_SPEED_CAP_25G |
> + ETH_SPEED_CAP_40G | ETH_SPEED_CAP_100G;
> }
> 
> static int
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index fb64027..5e9db6b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1519,6 +1519,7 @@ static void
> i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> {
>   struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>   struct i40e_vsi *vsi = pf->main_vsi;
> 
>   dev_info->max_rx_queues = vsi->nb_qps;
> @@ -1574,6 +1575,14 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *dev_info)
>   dev_info->max_rx_queues += dev_info->vmdq_queue_num;
>   dev_info->max_tx_queues += dev_info->vmdq_queue_num;
>   }
> +
> + if (i40e_is_40G_device(hw->device_id))
> + /* For XL710 */
> + dev_info->speed_capa = ETH_SPEED_CAP_1G | ETH_SPEED_CAP_10G;
> + else
> + /* For X710 */
> + dev_info->speed_capa = ETH_SPEED_CAP_10G | ETH_SPEED_CAP_40G;
> +
> }
> 
> static int
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0d9f9b2..78b13a8 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2054,6 +2054,16 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *dev_info)
>   };
>   dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
>   dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
> +
> + dev_info->speed_capa = ETH_SPEED_CAP_1G | ETH_SPEED_CAP_10G;
> +
> + if (hw->mac.type == ixgbe_mac_X540 ||
> + hw->mac.type == ixgbe_mac_X540_vf ||
> + hw->mac.type == ixgbe_mac_X550 ||
> + hw->mac.type == ixgbe_mac_X550_vf)
> +
> + dev_info->speed_capa |= ETH_SPEED_CAP_100M_FD |
> + ETH_SPEED_CAP_100M_HD;
> }
> 
> static void
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index f915bc1..d4442fb 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -3482,6 +3482,12 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *info)
>   info->max_rx_queues = max;
>   info->max_tx_queues = max;
>   

[dpdk-dev] [PATCH] doc: fix vhost guide

[Igor Ryzhov]

Sorry, I used wrong email address to reply from. This one is correct.

On Mon, Apr 13, 2015 at 10:11 AM, Igor Ryzhov  wrote:

> Hello, Changchun.
>
> Previous paragraph says ?To enable vhost, turn on vhost library in the
> configure file config/common_linuxapp?, but string in a code-block is
> ?CONFIG_RTE_LIBRTE_VHOST=n?. I thought that idea is to use the default
> string from the config file that user have to change, not already changed
> string. So I used the same style.
>
> Regards,
> Igor
>
> 13 ???. 2015 ?., ? 7:52, Ouyang, Changchun 
> ???(?):
>
> Hi Igor,
>
> Good catch, comments as below.
>
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org ] On Behalf
> Of Igor Ryzhov
> Sent: Thursday, April 9, 2015 12:31 AM
> To: dev at dpdk.org
> Cc: Igor Ryzhov
> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>
> Guide says that a configure parameter to choose between vhost cuse and
> vhost user will be introduced in the future, but it?s already added by
> commit
> 28a1ccca41bf.
>
> Signed-off-by: Igor Ryzhov 
> ---
> doc/guides/sample_app_ug/vhost.rst | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/vhost.rst
> b/doc/guides/sample_app_ug/vhost.rst
> index 8a7eb3b..df8cd8c 100644
> --- a/doc/guides/sample_app_ug/vhost.rst
> +++ b/doc/guides/sample_app_ug/vhost.rst
> @@ -309,13 +309,12 @@ Compiling the Sample Code
>
> CONFIG_RTE_LIBRTE_VHOST=n
>
> -vhost user is turned on by default in the lib/librte_vhost/Makefile.
> -To enable vhost cuse, uncomment vhost cuse and comment vhost user
> manually. In future, a configure will be created for switch between two
> implementations.
> +vhost user is turned on by default in the configure file
> config/common_linuxapp.
> +To enable vhost cuse, disable vhost user.
>
> .. code-block:: console
>
> -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
> -#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
> vhost_user/virtio-net-user.c vhost_user/fd_man.c
> +CONFIG_RTE_LIBRTE_VHOST_USER=y
>
>
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
>
> Thanks
> Changchun
>
>
>

[dpdk-dev] [PATCH] doc: fix vhost guide

[Igor Ryzhov]

Hello, Changchun.

Previous paragraph says ?To enable vhost, turn on vhost library in the 
configure file config/common_linuxapp?, but string in a code-block is 
?CONFIG_RTE_LIBRTE_VHOST=n?. I thought that idea is to use the default string 
from the config file that user have to change, not already changed string. So I 
used the same style.

Regards,
Igor

> 13 ???. 2015 ?., ? 7:52, Ouyang, Changchun  
> ???(?):
> 
> Hi Igor,
> 
> Good catch, comments as below.
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Igor Ryzhov
>> Sent: Thursday, April 9, 2015 12:31 AM
>> To: dev at dpdk.org
>> Cc: Igor Ryzhov
>> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>> 
>> Guide says that a configure parameter to choose between vhost cuse and
>> vhost user will be introduced in the future, but it?s already added by commit
>> 28a1ccca41bf.
>> 
>> Signed-off-by: Igor Ryzhov 
>> ---
>> doc/guides/sample_app_ug/vhost.rst | 7 +++
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>> 
>> diff --git a/doc/guides/sample_app_ug/vhost.rst
>> b/doc/guides/sample_app_ug/vhost.rst
>> index 8a7eb3b..df8cd8c 100644
>> --- a/doc/guides/sample_app_ug/vhost.rst
>> +++ b/doc/guides/sample_app_ug/vhost.rst
>> @@ -309,13 +309,12 @@ Compiling the Sample Code
>> 
>> CONFIG_RTE_LIBRTE_VHOST=n
>> 
>> -vhost user is turned on by default in the lib/librte_vhost/Makefile.
>> -To enable vhost cuse, uncomment vhost cuse and comment vhost user
>> manually. In future, a configure will be created for switch between two
>> implementations.
>> +vhost user is turned on by default in the configure file
>> config/common_linuxapp.
>> +To enable vhost cuse, disable vhost user.
>> 
>> .. code-block:: console
>> 
>> -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
>> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
>> -#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
>> vhost_user/virtio-net-user.c vhost_user/fd_man.c
>> +CONFIG_RTE_LIBRTE_VHOST_USER=y
> 
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
> 
> Thanks
> Changchun

[dpdk-dev] [PATCH] doc: fix vhost guide

[Igor Ryzhov]

Guide says that a configure parameter to choose between vhost cuse and vhost 
user will be introduced in the future, but it?s already added by commit 
28a1ccca41bf.

Signed-off-by: Igor Ryzhov 
---
 doc/guides/sample_app_ug/vhost.rst | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/doc/guides/sample_app_ug/vhost.rst 
b/doc/guides/sample_app_ug/vhost.rst
index 8a7eb3b..df8cd8c 100644
--- a/doc/guides/sample_app_ug/vhost.rst
+++ b/doc/guides/sample_app_ug/vhost.rst
@@ -309,13 +309,12 @@ Compiling the Sample Code

 CONFIG_RTE_LIBRTE_VHOST=n

-vhost user is turned on by default in the lib/librte_vhost/Makefile.
-To enable vhost cuse, uncomment vhost cuse and comment vhost user 
manually. In future, a configure will be created for switch between two 
implementations.
+vhost user is turned on by default in the configure file 
config/common_linuxapp.
+To enable vhost cuse, disable vhost user.

 .. code-block:: console

-SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c 
vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
-#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c 
vhost_user/virtio-net-user.c vhost_user/fd_man.c
+CONFIG_RTE_LIBRTE_VHOST_USER=y

  After vhost is enabled and the implementation is selected, build the 
vhost library.

-- 
1.9.5 (Apple Git-50.3)

[dpdk-dev] Fortville Firmware

[Igor Ryzhov]

Hello.

We are testing new X710/XL710 cards and having some issues - our cards drop
significant amount of packets without indicating the reason on any of it's
counters. We found that our cards' firmware version is 4.22 but 4.2.4 is a
suggested version in some guides. We want to try our cards with other
firmware version, but the problem is that we can't find it. Can anybody
help with getting firmware version 4.2.4? Thank you.

Regards,
Igor Ryzhov

[dpdk-dev] [PATCH] lpm: fix overflow issue

[Igor Ryzhov]

Great. The easiest way to reproduce the issue is to fill LPM table with
rules using only one depth and try to add another one rule with same depth.
Rule will be successfully added and memory will be corrupted.

???, 22 ??? 2015 ?.  Richardson, Bruce ???:

> Sorry I missed this Friday. I'll look at it  shortly.
>
>
>
> On 21 Feb 2015, at 22:56, Igor Ryzhov 
> <mailto:iryzhov at nfware.com <javascript:;>>> wrote:
>
> Hello again. Will anybody review this patch?
> This is really critical issue, because it can lead to memory corruption
> and break any program using LPM.
>
> CCing this to Bruce Richardson, because he is maintainer of LPM.
>
> Regards,
> Igor Ryzhov
>
> On Fri, Feb 20, 2015 at 4:16 PM, Igor Ryzhov  <javascript:;><mailto:iryzhov at nfware.com <javascript:;>>> wrote:
> LPM table overflow may occur if table is full and added rule has the
> biggest depth that already have some rules.
>
> Signed-off-by: Igor Ryzhov  iryzhov at nfware.com <javascript:;>>>
> ---
>  lib/librte_lpm/rte_lpm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
> index 983e04b..cc51210 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -298,6 +298,9 @@ rule_add(struct rte_lpm *lpm, uint32_t ip_masked,
> uint8_t depth,
> return rule_index;
> }
> }
> +
> +   if (rule_index == lpm->max_rules)
> +   return -ENOSPC;
> } else {
>     /* Calculate the position in which the rule will be
> stored. */
> rule_index = 0;
> --
> 1.9.3 (Apple Git-50)
>
>
>
>
> --
> Regards,
> Igor Ryzhov
>

[dpdk-dev] [PATCH] lpm: fix overflow issue

[Igor Ryzhov]

Hello again. Will anybody review this patch?
This is really critical issue, because it can lead to memory corruption and
break any program using LPM.

CCing this to Bruce Richardson, because he is maintainer of LPM.

Regards,
Igor Ryzhov

On Fri, Feb 20, 2015 at 4:16 PM, Igor Ryzhov  wrote:

> LPM table overflow may occur if table is full and added rule has the
> biggest depth that already have some rules.
>
> Signed-off-by: Igor Ryzhov 
> ---
>  lib/librte_lpm/rte_lpm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
> index 983e04b..cc51210 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -298,6 +298,9 @@ rule_add(struct rte_lpm *lpm, uint32_t ip_masked,
> uint8_t depth,
> return rule_index;
> }
> }
> +
> +   if (rule_index == lpm->max_rules)
> +   return -ENOSPC;
> } else {
> /* Calculate the position in which the rule will be
> stored. */
>     rule_index = 0;
> --
> 1.9.3 (Apple Git-50)
>
>


-- 
Regards,
Igor Ryzhov

[dpdk-dev] Question about librte_cmdline

[Igor Ryzhov]

Thank you.

That?s not a really big problem that we need it?s own structure for every 
command, but the lack of optional parameters is a problem.

For example:
object add IP [port]

it?s just one command, but we need two different structures - with and 
without ?port?

Maybe I?ll work on it sometime later.

> 14 . 2014 ?., ? 14:14, Olivier MATZ  
> ???(?):
> 
> Hi Igor,
> 
> Adding-back the list to the discussion, I removed it by mistake in my
> first answer.
> 
>>> 14 . 2014 ?., ? 12:20, Olivier MATZ  
>>> ???(?):
>>> 
>>> Hi Igor,
>>> 
>>> On 11/14/2014 09:52 AM, Igor Ryzhov wrote:
>>>> Are there any docs with detailed description of cmdline library?
>>>> I found only some information in ?DPDK Sample Apps? document, but it 
>>>> describes only a couple of features.
>>> 
>>> In my knowledge, there is no such documentation.
>>> You can also refer to testpmd that gives a lot of different commands.
>>> 
>>> If you have any question, you can ask on the list.
>>> 
>>> Regards,
>>> Olivier
>> 
>> Thank you, I?ll check testpmd.
>> 
>> At the moment I have a question - is there a possibility to have optional 
>> tokens in one command?
>> 
>> For example:
>> 
>> I have one command - ?object? and two subcommands - ?add? and ?del?:
>> 
>>  object add name IP
>>  object del name
>> 
>> And the question is - can I have just one context instruction for this? 
>> Something like that:
>> 
>> Result struct:
>> 
>> struct object_result {
>>  cmdline_fixed_string_t object;
>>  cmdline_fixed_string_t cmd;
>>  cmdline_fixed_string_t name;
>>  cmdline_ipaddr_t ip;// I need it optional - only 
>> for ?add? case
>> }
>> 
>> And tokens:
>> 
>> cmdline_parse_token_string_t object =
>>  TOKEN_STRING_INITIALIZER(struct object_result, object, "object");
>> cmdline_parse_token_string_t cmd =
>>  TOKEN_STRING_INITIALIZER(struct object_result, cmd, "add#del");
>> cmdline_parse_token_string_t name =
>>  TOKEN_STRING_INITIALIZER(struct object_result, name, NULL);
>> cmdline_parse_token_ipaddr_t ip =
>>  TOKEN_IPV4_INITIALIZER(struct object_result, ip, NULL);
>> 
>> As I understand investigating the code of sample application - all tokens 
>> are required (because there are two different instructions - for ?add? and 
>> for ?del/show?).
>> And in this example configuration there is no possibility for string ?object 
>> del name? without last IP token.
>> So I need to have two different context instructions - one for ?add? and one 
>> for ?del?.
>> Am I right?
> 
> Right, there is no way to declare an optional token in one instruction.
> But if there are few case (ex: a "set" and a "show" intructions), you
> can factorize the structure and the callback function. There is an
> example in my latest TSO patch:
> http://dpdk.org/ml/archives/dev/2014-November/007962.html
> 
> Regards,
> Olivier

[dpdk-dev] Question about librte_cmdline

[Igor Ryzhov]

Hello.

Are there any docs with detailed description of cmdline library?
I found only some information in ?DPDK Sample Apps? document, but it describes 
only a couple of features.

Best regards,
Igor Ryzhov

[dpdk-dev] one lightwight rte_eal_init() for SECONDARY processes which only use sharedmemory

[Igor Ryzhov]

This is really useful, thank you!

Best regards,
Igor Ryzhov

> 12 . 2014 ?., ? 6:22, Chi, Xiaobo (NSN - CN/Hangzhou)  nsn.com> ???(?):
> 
> Hi,
> Background:
> What we are doing now is port make telecom network element to be cloud based. 
>  For one of our product,  DPDK is applied not only for fastpath/dataplane 
> processing, but also for Distributed Message eXchange (DMX) between different 
> processes/applications which may located in different VM even different host. 
>  for such a DMX system, in one VM, we have one DPDK based dmxdemo (which acts 
> as the PRIMARY) which is in charge of distribute message between different 
> applications, and dozens of applications (act as SECONDARY) to use DPDK based 
> rte_tx_ring/rte_rx_ring/mempool/memzone to send receive messages to dmxdemo.
> 
> Problem:
> Here, these DPDK based SECONDARY processes need only the DPDK's hugepage 
> based sharememory mechanism and it's upper libs (such as ring, mempool, 
> etc.), they need not cpu core pinning, iopl privilege changing , pci device, 
> timer, alarm, interrupt, shared_driver_list,  core_info, threads for each 
> core, etc. Then, for such kind of SECONDARY processes, the current 
> rte_eal_init() is too heavy.
> I have seen some others also met similar troubles.
> 
> Solution:
> I write one light weight rte_eal_init(), called rte_eal_secondary_mem_init() 
> as following.  It only initializes shared memory and mandatory resources. I 
> expect your review and hope these code can be merged into DPDK main branch.
> 
> static void eal_secondary_mem_parse_args(int argc, char **argv)
> {
>static struct option lgopts[] = {
>{OPT_HUGE_DIR, 1, 0, 0},
>{OPT_FILE_PREFIX, 1, 0, 0},
>{0, 0, 0, 0}
>};
> 
>int opt;
>int option_index;
> 
>while ((opt = getopt_long(argc, argv, "", lgopts, _index)) != 
> EOF) {
> 
>if (!opt ) {
> if (!strcmp(lgopts[option_index].name, OPT_HUGE_DIR)) {
>internal_config.hugepage_dir = optarg;
> }
> else if (!strcmp(lgopts[option_index].name, OPT_FILE_PREFIX)) {
>internal_config.hugefile_prefix = optarg;
> }
>  }
>   }
> }
> 
> int rte_eal_secondary_mem_init( int argc, char **argv )
> {
>static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
>const char *logid;
> 
>if (!rte_atomic32_test_and_set(_once))
>return -1;
> 
>logid = strrchr(argv[0], '/');
>logid = strdup(logid ? logid + 1: argv[0]);
> 
>if (rte_eal_log_early_init() < 0)
>rte_panic("Cannot init early logs\n");
> 
>   memset( _config, 0, sizeof( struct internal_config ) );
>   /*this is only for secondary PRBs */
>   internal_config.process_type = RTE_PROC_SECONDARY;
>internal_config.hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;
>internal_config.hugepage_dir = NULL;
>   /* user can freely define the hugefile_prefix and hugepage_dir */
>   eal_secondary_mem_parse_args( argc, argv );
> 
>   RTE_LOG(INFO, EAL, "prefix=%s, dir=%s.\n",internal_config.hugefile_prefix, 
> internal_config.hugepage_dir );
> 
>   /* To share memory config with PRIMARY process */
>   internal_config.no_shconf = 0;
>   rte_config_init();
> 
>if (rte_eal_memory_init() < 0)
>rte_panic("Cannot init memory\n");
> 
>if (rte_eal_memzone_init() < 0)
>rte_panic("Cannot init memzone\n");
> 
>if (rte_eal_log_init(logid, LOG_DAEMON ) < 0)
>rte_panic("Cannot init logs\n");
> 
>return 0;
> }
> 
> brgs,
> chi xiaobo
> 
> 
> 

[dpdk-dev] RTE Ring removing

[Igor Ryzhov]

It seems to be a good idea, thank you, Olivier!

But a few questions:
1. Will this changes affect performance?
2. In PATCH 2/2 you have a small bug:

In file rte_ring.h, in comments describing rte_ring_init function you have:

+ * @param name
+ *   The size of the ring.

But it is name of the ring, not size.

Best regards,
Igor Ryzhov

07.05.2014 15:39, Olivier MATZ ?:
> Hi Igor,
>
> On 05/07/2014 09:54 AM, Igor Ryzhov wrote:
>> I noticed that in Memzone realization there is a special global variable
>> "free_memseg" containing pointers on free memory segments.
>> An memzone reserve function just finst the best segment for allocation
>> from this "free_memseg" variable.
>>
>> So I think there is a possibility to unreserve already reserved memory
>> back to "free_memseg", and impossibility of unreserving memory is just
>> because there is no function for that, not because it is impossible in
>> principle.
>> Am I right? Or there are any restrictions?
>
> I think that implementing a freeing of memory segment is feasible, but
> it would require some work to properly merge freed zones to avoid memory
> fragmentation.
>
> Another solution is to allocate/free rings in standard memory (malloc
> for instance) instead of rte_memzones. Let me know if the patches I've
> just sent on the mailing list solves your issue.
>
> By the way, I plan to do the same thing for mempools in the coming
> weeks but there is much more work.
>
> Regards,
> Olivier
>

[dpdk-dev] RTE Ring removing

[Igor Ryzhov]

Hello again.

I did some investigation on the code.
I learned that RTE Ring creation function uses functions related to RTE 
Memzone to reserve memory (rte_memzone_reserve).
Documentation states that once reserved memzone can not be unreserved. I 
decided to find out why it is so.

I noticed that in Memzone realization there is a special global variable 
"free_memseg" containing pointers on free memory segments.
An memzone reserve function just finst the best segment for allocation 
from this "free_memseg" variable.

So I think there is a possibility to unreserve already reserved memory 
back to "free_memseg", and impossibility of unreserving memory is just 
because there is no function for that, not because it is impossible in 
principle.
Am I right? Or there are any restrictions?

Best regards,
Igor Ryzhov

06.05.2014 13:05, Igor Ryzhov ?:
> Hello.
>
> For what reason RTE Rings can not be removed once created?
> In my application I want to use many rings with different names so I 
> think there may be a problem with memory because of many ring that 
> already not in use, but allocated.
> Or DPDK has a mechanism of reusing memory if rings are not in use?
>
> Best regards,
> Igor Ryzhov

[dpdk-dev] RTE Ring removing

[Igor Ryzhov]

Hello.

For what reason RTE Rings can not be removed once created?
In my application I want to use many rings with different names so I 
think there may be a problem with memory because of many ring that 
already not in use, but allocated.
Or DPDK has a mechanism of reusing memory if rings are not in use?

Best regards,
Igor Ryzhov