[dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in comment
Hi, john
> -Original Message-
> From: Mcnamara, John
> Sent: Monday, November 14, 2016 6:30 PM
> To: Zhao1, Wei ; dev at dpdk.org
> Cc: olivier.matz at 6wind.com; Zhao1, Wei
> Subject: RE: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
> comment
>
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao
> > Sent: Monday, November 14, 2016 2:47 AM
> > To: dev at dpdk.org
> > Cc: olivier.matz at 6wind.com; Zhao1, Wei
> > Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
> > comment
> >
> > From: zhao wei
>
> I think you need to add your name to gitconfig file on the sending machine to
> avoid this "From:"
>
> >
> > There is a redundant repetition word "for" in commnet line the file
> > rte_mempool.h after the definition of RTE_MEMPOOL_OPS_NAMESIZE.
> > The word "for"appear twice in line 359 and 360.One of them is
> > redundant, so delete it.
> >
> > Fixes: 449c49b93a6b ("lib/librte_mempool: mempool: support handler
> > operations")
> >
> > Signed-off-by: zhao wei
>
> /commnet/comment/
>
> And same comment as before about the title. Apart from that:
>
> Acked-by: John McNamara
>
>
Thank you for your suggestion, I will change as your comment in following
patch!
[dpdk-dev] [RFC]Generic flow filtering API Sample Application
Hi All,
Now we are planning for an sample application for Generic flow
filtering API feature, and I have finished the RFC for this example app.
Now Adrien Mazarguil has send v2 version of Generic flow
filtering API, this sample application RFC is based on that.
Thank you.
Generic flow filtering API Sample Application
The application is a simple example of generic flow filtering API using the
DPDK.
The application performs flow director/filtering/classification in packet
processing.
Overview
The application demonstrates the use of generic flow
director/filtering/classification API
in the DPDK to implement packet forwarding.And this document focus on the guide
line of writing rules configuration
files and prompt commands usage. It also supply the definition of the available
EAL options arguments which is useful
in DPDK packet forwarding processing.
Compiling the Application
-
To compile the application:
#. Go to the sample application directory:
.. code-block:: console
export RTE_SDK=/path/to/rte_sdk
cd ${RTE_SDK}/examples/gen_filter
#. Set the target (a default target is used if not specified). For example:
.. code-block:: console
export RTE_TARGET=x86_64-native-linuxapp-gcc
See the *DPDK Getting Started Guide* for possible RTE_TARGET values.
#. Build the application:
.. code-block:: console
make
Running the Application
---
The application has a number of EAL options::
./gen_filter [EAL options] --
EAL options:
* -c
Codemask, set the hexadecimal bitmask of the cores to run on.
* -n
Num, set the number of memory channels to use.
APP PARAMS:
The following are the application options parameters, they must be
separated
from the EAL options with a "--" separator.
* -i
Interactive, run this app in interactive mode. In this mode, the app
starts with a prompt that can
be used to start and stop forwarding, then manage generic filters rule
configure in the application,
reference to the following description for more details.In
non-interactive mode, the application starts with the configuration specified
on the
command-line and immediately enters forwarding mode.
* --portmask=0xXX
Set the hexadecimal bitmask of the ports which can be used by the
generic flow director test in packet forwarding.
* --coremask=0xXX
Set the hexadecimal bitmask of the cores running the packet forwarding
test. The master
lcore is reserved for command line parsing only and cannot be masked on
for packet forwarding.
* --nb-ports=N
Set the number of forwarding ports, where 1 <= N <= "number of ports"
on the board
or CONFIG_RTE_MAX_ETHPORTS from the configuration file. The default
value is the number of ports on the board.
* --rxq=N
Set the number of RX queues per port to N, where 1 <= N <= 65535. The
default value is 1.
* --txq=N
Set the number of TX queues per port to N, where 1 <= N <= 65535. The
default value is 1.
###this part need to complete later after decision of which EAL commands
arguments need to be support in this application###
Interactive mode
* when the gen_filter application is started in interactive mode,
(-i|--interactive), it displays a prompt
that can be used to start and stop forwarding, and configure the
application to set the Flow Director,
display statistics, set the Flow Director and other tasks. The
application has a number of commands line options:
gen_filter>[Commands]
* There is a prompt "gen_filter> " before cursor, command can be enter
after that position,
also a space bar between configuration file name and command.
These are the commands that are currently working under the command line
interface:
* Control Commands
help: show the following commands which are currently available in this
application and their usage
gen_filter>help
quit: quits the application.
gen_filter>quit
start: start the application, start packet forwarding
gen_filter>start
stop: stop the application, stop packet forwarding
gen_filter>stop
showcfg: print configuration infomation about EAL parameters, for
example mapping of cores, rx queue, tx queues and so on.
gen_filter>showcfg
* General Commands to add/remove/query an filter rule:
App will print reminder message for user about whether this
rule command is SUCESS or FAIL after user type in the commmand.
add: add filter rules from configuration file
gen_filter>add port_id filename.txt
[dpdk-dev] [RFC v2] Generic flow director/filtering/classification API
Hi Adrien Mazarguil,
> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Tuesday, October 11, 2016 4:21 PM
> To: Zhao1, Wei
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC v2] Generic flow
> director/filtering/classification
> API
>
> Hi Wei,
>
> On Tue, Oct 11, 2016 at 01:47:53AM +, Zhao1, Wei wrote:
> > Hi Adrien Mazarguil,
> > There is a struct rte_flow_action_rss in rte_flow.txt, the member
> rss_conf is a pointer type, is there any convenience in using pointer?
> > Why not using struct rte_eth_rss_conf rss_conf type, as
> rte_flow_item_ipv4/ rte_flow_item_ipv6 struct member?
> >
> > Thank you.
> >
> > struct rte_flow_action_rss {
> > struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
> > uint16_t queues; /**< Number of entries in queue[]. */
> > uint16_t queue[]; /**< Queues indices to use. */ };
>
> Well I thought it made sharing flow RSS configuration with its counterpart in
> struct rte_eth_conf easier (this pointer should even be const). Also, while
> ABI breakage would still occur if rte_eth_rss_conf happened to be modified,
> the impact on this API would be limited as it would not cause a change in
> structure size. We'd ideally need some kind of version field to be completely
> safe but I guess that would be somewhat overkill.
>
> Now considering this API was written without an initial implementation, all
> structure definitions that do not make sense are still open to debate, we can
> adjust them as needed.
>
> --
> Adrien Mazarguil
> 6WIND
Your explanation seems very reasonable for me, structure pointer is an very
experienced usage in this situation.
Thank you!
[dpdk-dev] [RFC v2] Generic flow director/filtering/classification API
Hi Adrien Mazarguil,
There is a struct rte_flow_action_rss in rte_flow.txt, the member
rss_conf is a pointer type, is there any convenience in using pointer?
Why not using struct rte_eth_rss_conf rss_conf type, as rte_flow_item_ipv4/
rte_flow_item_ipv6 struct member?
Thank you.
struct rte_flow_action_rss {
struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */
uint16_t queues; /**< Number of entries in queue[]. */
uint16_t queue[]; /**< Queues indices to use. */
};
> -Original Message-
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Monday, October 10, 2016 9:19 PM
> To: Zhao1, Wei
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC v2] Generic flow
> director/filtering/classification
> API
>
> Hi Wei,
>
> On Mon, Oct 10, 2016 at 09:42:53AM +, Zhao1, Wei wrote:
> > Hi Adrien Mazarguil,
> >
> > In your v2 version of rte_flow.txt , there is an action type
> RTE_FLOW_ACTION_TYPE_MARK, but there is no definition of struct
> rte_flow_action_mark.
> > And there is an definition of struct rte_flow_action_id. Is it a typo or
> > other
> usage?
> >
> > Thank you.
> >
> > struct rte_flow_action_id {
> > uint32_t id; /**< 32 bit value to return with packets. */ };
>
> That is indeed a mistake, this struct should be named
> "rte_flow_action_mark". I'll fix it for the next update, thanks.
>
> --
> Adrien Mazarguil
> 6WIND
[dpdk-dev] [RFC v2] Generic flow director/filtering/classification API
Hi Adrien Mazarguil,
In your v2 version of rte_flow.txt , there is an action type
RTE_FLOW_ACTION_TYPE_MARK, but there is no definition of struct
rte_flow_action_mark.
And there is an definition of struct rte_flow_action_id. Is it a typo or other
usage?
Thank you.
struct rte_flow_action_id {
uint32_t id; /**< 32 bit value to return with packets. */
};
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Saturday, August 20, 2016 3:33 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [RFC v2] Generic flow director/filtering/classification
> API
>
> Hi All,
>
> Thanks to many for the positive and constructive feedback I've received so
> far. Here is the updated specification (v0.7) at last.
>
> I've attempted to address as many comments as possible but could not
> process them all just yet. A new section "Future evolutions" has been
> added for the remaining topics.
>
> This series adds rte_flow.h to the DPDK tree. Next time I will attempt to
> convert the specification as a documentation commit part of the patchset
> and actually implement API functions.
>
> I think including the entire document here makes it easier to annotate on
> the ML, apologies in advance for the resulting traffic.
>
> Finally I'm off for the next two weeks, do not expect replies from me in
> the meantime.
>
> Updates are also available online:
>
> HTML version:
> https://rawgit.com/6WIND/rte_flow/master/rte_flow.html
>
> PDF version:
> https://rawgit.com/6WIND/rte_flow/master/rte_flow.pdf
>
> Related draft header file (also in the next patch):
> https://raw.githubusercontent.com/6WIND/rte_flow/master/rte_flow.h
>
> Git tree:
> https://github.com/6WIND/rte_flow
>
> Changes from v1:
>
> Specification:
>
> - Settled on [generic] "flow interface" / "flow API" as the name of this
>framework, matches the rte_flow prefix better.
> - Minor wording changes in several places.
> - Partially added egress (TX) support.
> - Added "unrecoverable errors" as another consequence of overlapping
>rules.
> - Described flow rules groups and their interaction with flow rule
>priorities.
> - Fully described PF and VF meta pattern items so they are not open to
>interpretation anymore.
> - Removed the SIGNATURE meta pattern item as its description was too
>vague, may be re-added later if necessary.
> - Added the PORT pattern item to apply rules to non-default physical
>ports.
> - Entirely redefined the RAW pattern item.
> - Fixed tag error in the ETH item definition.
> - Updated protocol definitions (IPV4, IPV6, ICMP, UDP).
> - Added missing protocols (SCTP, VXLAN).
> - Converted ID action to MARK and FLAG actions, described interaction
>with the RSS hash result in mbufs.
> - Updated COUNT query structure to retrieve the number of bytes.
> - Updated VF action.
> - Documented negative item and action types, those will be used for
>dynamic types generated at run-time.
> - Added blurb about IPv4 options and IPv6 extension headers matching.
> - Updated function definitions.
> - Documented a flush method to remove all rules on a given port at once.
> - Documented the verbose error reporting interface.
> - Documented how the private interface for PMD use will work.
> - Documented expected behavior between successive port initializations.
> - Documented expected behavior for ports not under DPDK control.
> - Updated API migration section.
> - Added future evolutions section.
>
> Header file:
>
> - Not a draft anymore and can be used as-is for preliminary
>implementations.
> - Flow rule attributes (group, priority, etc) now have their own
>structure provided separately to API functions (struct rte_flow_attr).
> - Group and priority interactions have been documented.
> - Added PORT item.
> - Removed SIGNATURE item.
> - Defined ICMP, SCTP and VXLAN items.
> - Redefined PF, VF, RAW, IPV4, IPV6, UDP and TCP items.
> - Fixed tag error in the ETH item definition.
> - Converted ID action to MARK and FLAG actions.
>hash result in mbufs.
> - Updated COUNT query structure.
> - Updated VF action.
> - Added verbose errors interface.
> - Updated function prototypes according to the above.
> - Defined rte_flow_flush().
>
>
>
> ==
> Generic flow interface
> ==
>
> .. footer::
>
>v0.7
>
> .. contents::
> .. sectnum::
> .. raw:: pdf
>
>PageBreak
>
> Overview
>
>
> DPDK provides several competing interfaces added over time to perform
> packet
> matching and related actions such as filtering and classification.
>
> They must be extended to implement the features supported by newer
> devices
> in order to expose them to applications, however the current design has
> several drawbacks:
>
> - Complicated filter combinations which have not been hard-coded cannot be
> expressed.
> - Prone to API/ABI breakage when new features must be
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi ,Kyle Larose
> -Original Message-
> From: Kyle Larose [mailto:eomereadig at gmail.com]
> Sent: Wednesday, August 3, 2016 12:22 AM
> To: Zhao1, Wei
> Cc: Wu, Jingjing ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
>
> Hello Wei,
>
>
> On Tue, Aug 2, 2016 at 2:59 AM, Zhao1, Wei wrote:
> > Hi, Wujingjing and Kyle Larose
> >
> >
> >
> >> -Original Message-
> >> From: Zhao1, Wei
> >> Sent: Tuesday, August 2, 2016 11:27 AM
> >> To: Wu, Jingjing ; Lu, Wenzhuo
> >>
> >> Cc: dev at dpdk.org
> >> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> inconsistent
> >>
> >> Hi,Wu jingjing and wenzhuo
> >>
> >> > -Original Message-
> >> > From: Zhao1, Wei
> >> > Sent: Monday, August 1, 2016 4:58 PM
> >> > To: 'Kyle Larose'
> >> > Cc: dev at dpdk.org
> >> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> > inconsistent
> >> >
> >> > Hi, Kyle Larose
> >> >The core problem is i40e has no statistic of discard bytes, that
> >> > means even if when ports are not stopped, the statistic
> >> > rx_good_bytes is consist of discard
> >> > bytes?is that reasonable? In other words, I can just minus discard
> >> > bytes from rx_good_bytes if I can get discard bytes number, that is
> >> > much
> >> better.
> >> >
> >> > -Original Message-
> >> > From: Kyle Larose [mailto:eomereadig at gmail.com]
> >> > Sent: Saturday, July 30, 2016 1:17 AM
> >> > To: Zhao1, Wei
> >> > Cc: dev at dpdk.org
> >> > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> > inconsistent
> >> >
> >> > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1
> wrote:
> >> > > rx_good_bytes and rx_good_packets statistic is inconsistent when
> >> > > port stopped,ipackets statistic is minus the discard packets but
> >> > > rx_bytes statistic not.Also,i40e has no statistic of discard
> >> > > bytes, so we have to delete discard packets item from
> rx_good_packets statistic.
> >> > >
> >> > > Fixes: 9aace75fc82e ("i40e: fix statistics")
> >> > >
> >> > > Signed-off-by: Wei Zhao1
> >> > > ---
> >> > > drivers/net/i40e/i40e_ethdev.c | 3 +--
> >> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> >> > > --- a/drivers/net/i40e/i40e_ethdev.c
> >> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> >> > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev
> *dev,
> >> > > struct rte_eth_stats *stats)
> >> > >
> >> > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> >> > > pf->main_vsi->eth_stats.rx_multicast +
> >> > > - pf->main_vsi->eth_stats.rx_broadcast -
> >> > > - pf->main_vsi->eth_stats.rx_discards;
> >> > > + pf->main_vsi->eth_stats.rx_broadcast;
> >> > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> >> > > pf->main_vsi->eth_stats.tx_multicast +
> >> > > pf->main_vsi->eth_stats.tx_broadcast;
> >> > > --
> >> > > 2.5.5
> >> > >
> >> >
> >> > Is it not worse to report a received packet when no packet was
> >> > actually received by the upper layers under normal operations than
> >> > to ensure that packets and bytes are consistent when an interface
> >> > is stopped? It seems like the first case is much more likely to
> >> > occur than the
> >> second.
> >> >
> >> > Are we just introducing a new issue to fix another?
> >> >
> >> > How does this behaviour compare to other NICs? Does the ixgbe
> >> > report discarded packets in its ipackets? My reading of the driver is
> >> > that
> it does not.
> >> > In fact, it does something interesting to deal with the
> >> > problem:
> &
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi, Wujingjing and Kyle Larose
> -Original Message-
> From: Zhao1, Wei
> Sent: Tuesday, August 2, 2016 11:27 AM
> To: Wu, Jingjing ; Lu, Wenzhuo
>
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
>
> Hi,Wu jingjing and wenzhuo
>
> > -Original Message-
> > From: Zhao1, Wei
> > Sent: Monday, August 1, 2016 4:58 PM
> > To: 'Kyle Larose'
> > Cc: dev at dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> > inconsistent
> >
> > Hi, Kyle Larose
> >The core problem is i40e has no statistic of discard bytes, that
> > means even if when ports are not stopped, the statistic rx_good_bytes
> > is consist of discard
> > bytes?is that reasonable? In other words, I can just minus discard
> > bytes from rx_good_bytes if I can get discard bytes number, that is much
> better.
> >
> > -----Original Message-
> > From: Kyle Larose [mailto:eomereadig at gmail.com]
> > Sent: Saturday, July 30, 2016 1:17 AM
> > To: Zhao1, Wei
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> > inconsistent
> >
> > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote:
> > > rx_good_bytes and rx_good_packets statistic is inconsistent when
> > > port stopped,ipackets statistic is minus the discard packets but
> > > rx_bytes statistic not.Also,i40e has no statistic of discard bytes,
> > > so we have to delete discard packets item from rx_good_packets statistic.
> > >
> > > Fixes: 9aace75fc82e ("i40e: fix statistics")
> > >
> > > Signed-off-by: Wei Zhao1
> > > ---
> > > drivers/net/i40e/i40e_ethdev.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev,
> > > struct rte_eth_stats *stats)
> > >
> > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> > > pf->main_vsi->eth_stats.rx_multicast +
> > > - pf->main_vsi->eth_stats.rx_broadcast -
> > > - pf->main_vsi->eth_stats.rx_discards;
> > > + pf->main_vsi->eth_stats.rx_broadcast;
> > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> > > pf->main_vsi->eth_stats.tx_multicast +
> > > pf->main_vsi->eth_stats.tx_broadcast;
> > > --
> > > 2.5.5
> > >
> >
> > Is it not worse to report a received packet when no packet was
> > actually received by the upper layers under normal operations than to
> > ensure that packets and bytes are consistent when an interface is
> > stopped? It seems like the first case is much more likely to occur than the
> second.
> >
> > Are we just introducing a new issue to fix another?
> >
> > How does this behaviour compare to other NICs? Does the ixgbe report
> > discarded packets in its ipackets? My reading of the driver is that it does
> > not.
> > In fact, it does something interesting to deal with the
> > problem:
> >
> > from:
> > http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c
> >
> > /*
> > * An errata states that gprc actually counts good + missed packets:
> > * Workaround to set gprc to summated queue packet receives */
> > hw_stats-
> > >gprc = *total_qprc;
> >
> > total_gprc is equal to the sum of the qprc per queue. Can we do
> > something similar on the i40e instead of adding unicast, mulitcast and
> broadcast?
>
>
> I have checked ixgbe code about Rx statistic, in function
> ixgbe_read_stats_registers() we can find the rx_good_bytes and
> rx_good_packets statistic.
> It is listed below, we can see rx_good_packets is also just addition of Queue
> Packets Received Count and not minused discard packet number.
> Is there some wrong of understanding?
>
> for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) {
> ..
> *total_qprc += hw_stats->qprc[i];
> *total_qbrc += hw_stats->qbrc[i];
> ..
> }
The problem is i40e has no statistic of discard bytes, so it is impossible
to minus discard bytes from rx_good_bytes . If you think it's not reasonable to
Delete rx_discards iterm from rx_good_packets statistic, this patch will be
superseded. Because I didn't find other way to correct this problem at present.
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi,Wu jingjing and wenzhuo
> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, August 1, 2016 4:58 PM
> To: 'Kyle Larose'
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
>
> Hi, Kyle Larose
>The core problem is i40e has no statistic of discard bytes, that means
> even if
> when ports are not stopped, the statistic rx_good_bytes is consist of discard
> bytes?is that reasonable? In other words, I can just minus discard bytes
> from rx_good_bytes if I can get discard bytes number, that is much better.
>
> -Original Message-
> From: Kyle Larose [mailto:eomereadig at gmail.com]
> Sent: Saturday, July 30, 2016 1:17 AM
> To: Zhao1, Wei
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
>
> On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote:
> > rx_good_bytes and rx_good_packets statistic is inconsistent when port
> > stopped,ipackets statistic is minus the discard packets but rx_bytes
> > statistic not.Also,i40e has no statistic of discard bytes, so we have
> > to delete discard packets item from rx_good_packets statistic.
> >
> > Fixes: 9aace75fc82e ("i40e: fix statistics")
> >
> > Signed-off-by: Wei Zhao1
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev,
> > struct rte_eth_stats *stats)
> >
> > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> > pf->main_vsi->eth_stats.rx_multicast +
> > - pf->main_vsi->eth_stats.rx_broadcast -
> > - pf->main_vsi->eth_stats.rx_discards;
> > + pf->main_vsi->eth_stats.rx_broadcast;
> > stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> > pf->main_vsi->eth_stats.tx_multicast +
> > pf->main_vsi->eth_stats.tx_broadcast;
> > --
> > 2.5.5
> >
>
> Is it not worse to report a received packet when no packet was actually
> received by the upper layers under normal operations than to ensure that
> packets and bytes are consistent when an interface is stopped? It seems like
> the first case is much more likely to occur than the second.
>
> Are we just introducing a new issue to fix another?
>
> How does this behaviour compare to other NICs? Does the ixgbe report
> discarded packets in its ipackets? My reading of the driver is that it does
> not.
> In fact, it does something interesting to deal with the
> problem:
>
> from: http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c
>
> /*
> * An errata states that gprc actually counts good + missed packets:
> * Workaround to set gprc to summated queue packet receives */ hw_stats-
> >gprc = *total_qprc;
>
> total_gprc is equal to the sum of the qprc per queue. Can we do something
> similar on the i40e instead of adding unicast, mulitcast and broadcast?
I have checked ixgbe code about Rx statistic, in function
ixgbe_read_stats_registers() we can find the rx_good_bytes and rx_good_packets
statistic.
It is listed below, we can see rx_good_packets is also just addition of Queue
Packets Received Count and not minused discard packet number.
Is there some wrong of understanding?
for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++)
{
..
*total_qprc += hw_stats->qprc[i];
*total_qbrc += hw_stats->qbrc[i];
..
}
[dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Hi, Kyle Larose
The core problem is i40e has no statistic of discard bytes, that means even
if when ports are not stopped, the
statistic rx_good_bytes is consist of discard bytes?is that reasonable? In
other words, I can just minus discard bytes
from rx_good_bytes if I can get discard bytes number, that is much better.
-Original Message-
From: Kyle Larose [mailto:eomerea...@gmail.com]
Sent: Saturday, July 30, 2016 1:17 AM
To: Zhao1, Wei
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 wrote:
> rx_good_bytes and rx_good_packets statistic is inconsistent when port
> stopped,ipackets statistic is minus the discard packets but rx_bytes
> statistic not.Also,i40e has no statistic of discard bytes, so we have
> to delete discard packets item from rx_good_packets statistic.
>
> Fixes: 9aace75fc82e ("i40e: fix statistics")
>
> Signed-off-by: Wei Zhao1
> ---
> drivers/net/i40e/i40e_ethdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c
> b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev,
> struct rte_eth_stats *stats)
>
> stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> pf->main_vsi->eth_stats.rx_multicast +
> - pf->main_vsi->eth_stats.rx_broadcast -
> - pf->main_vsi->eth_stats.rx_discards;
> + pf->main_vsi->eth_stats.rx_broadcast;
> stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> pf->main_vsi->eth_stats.tx_multicast +
> pf->main_vsi->eth_stats.tx_broadcast;
> --
> 2.5.5
>
Is it not worse to report a received packet when no packet was actually
received by the upper layers under normal operations than to ensure that
packets and bytes are consistent when an interface is stopped? It seems like
the first case is much more likely to occur than the second.
Are we just introducing a new issue to fix another?
How does this behaviour compare to other NICs? Does the ixgbe report discarded
packets in its ipackets? My reading of the driver is that it does not. In fact,
it does something interesting to deal with the
problem:
from: http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c
/*
* An errata states that gprc actually counts good + missed packets:
* Workaround to set gprc to summated queue packet receives */ hw_stats->gprc =
*total_qprc;
total_gprc is equal to the sum of the qprc per queue. Can we do something
similar on the i40e instead of adding unicast, mulitcast and broadcast?
[dpdk-dev] [PATCH] net/i40e: fiX statstic inconsistent when port stopped
Hi, Wu Jingjing
Thanks for your feedback .I didn't find statistic of discard bytes i40e
data sheet,
so I have to delete discard packets item from rx_good_packets statistic.
In other words, we have no way to to minus the discard byte count from
rx_good_bytes.
Also I will make some change to meet requirements of commit log and subject.
-Original Message-
From: Wu, Jingjing
Sent: Friday, July 29, 2016 10:50 AM
To: Zhao1, Wei
Cc: dev at dpdk.org
Subject: RE: [dpdk-dev] [PATCH] net/i40e: fiX statstic inconsistent when port
stopped
Hi, zhaowei
Few comments below:
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao1
> Sent: Tuesday, July 26, 2016 10:06 AM
> To: dev at dpdk.org
> Cc: Zhao1, Wei
> Subject: [dpdk-dev] [PATCH] net/i40e: fiX statstic inconsistent when
> port stopped
>
Avoid typo:
fiX -> fix;
statstic -> statistic
And there are some requirements based on the subject and commit log in , such
as:
* The summary line should be around 50 characters.
* The text of the commit message should be wrapped at 72 characters.
Please check the doc " doc/guides/contributing/patches.rst" and use "
scripts/check-git-log.sh" to help you.
> rx_good_bytes and rx_good_packets statstic is inconsistent when port
> stopped,ipackets statistic is minus the discard packets but rx_bytes
> statistic not.
> Also,i40e has no statstic of discard bytes, so we have to delete
> discard packets item from rx_good_packets statstic.
>
> Fixes: 9aace75fc82e ("i40e: fix statistics")
>
> Signed-off-by: Wei Zhao1
> ---
> drivers/net/i40e/i40e_ethdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c
> b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev,
> struct rte_eth_stats *stats)
>
> stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> pf->main_vsi->eth_stats.rx_multicast +
> - pf->main_vsi->eth_stats.rx_broadcast -
> - pf->main_vsi->eth_stats.rx_discards;
> + pf->main_vsi->eth_stats.rx_broadcast;
> stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> pf->main_vsi->eth_stats.tx_multicast +
> pf->main_vsi->eth_stats.tx_broadcast;
rx_discards is included in imiss. So I think it's better to minus the discard
count.
/Jingjing
