DPDK patches and discussions
 help / color / mirror / Atom feed
From: Vladislav Zolotarov <vladz@cloudius-systems.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 for all NICs but 82598
Date: Fri, 11 Sep 2015 19:13:04 +0300	[thread overview]
Message-ID: <CAOYyTHZc=rN4L9qwTqpbW8E=ggmCLSo2hfOPruzaA0y30Avr+Q@mail.gmail.com> (raw)
In-Reply-To: <59AF69C657FD0841A61C55336867B5B0359263A3@IRSMSX103.ger.corp.intel.com>

On Sep 11, 2015 7:00 PM, "Richardson, Bruce" <bruce.richardson@intel.com>
wrote:
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov
> > Sent: Friday, September 11, 2015 4:13 PM
> > To: Thomas Monjalon
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above
1
> > for all NICs but 82598
> >
> > On Sep 11, 2015 5:55 PM, "Thomas Monjalon" <thomas.monjalon@6wind.com>
> > wrote:
> > >
> > > 2015-09-11 17:47, Avi Kivity:
> > > > On 09/11/2015 05:25 PM, didier.pallard wrote:
> > > > > On 08/25/2015 08:52 PM, Vlad Zolotarov wrote:
> > > > >>
> > > > >> Helin, the issue has been seen on x540 devices. Pls., see a
> > > > >> chapter
> > > > >> 7.2.1.1 of x540 devices spec:
> > > > >>
> > > > >> A packet (or multiple packets in transmit segmentation) can span
> > > > >> any number of buffers (and their descriptors) up to a limit of 40
> > > > >> minus WTHRESH minus 2 (see Section 7.2.3.3 for Tx Ring details
> > > > >> and section Section 7.2.3.5.1 for WTHRESH details). For best
> > > > >> performance it is recommended to minimize the number of buffers
> > > > >> as possible.
> > > > >>
> > > > >> Could u, pls., clarify why do u think that the maximum number of
> > > > >> data buffers is limited by 8?
> > > > >>
> > > > >> thanks,
> > > > >> vlad
> > > > >
> > > > > Hi vlad,
> > > > >
> > > > > Documentation states that a packet (or multiple packets in
> > > > > transmit
> > > > > segmentation) can span any number of buffers (and their
> > > > > descriptors) up to a limit of 40 minus WTHRESH minus 2.
> > > > >
> > > > > Shouldn't there be a test in transmit function that drops properly
> > > > > the mbufs with a too large number of segments, while incrementing
> > > > > a statistic; otherwise transmit function may be locked by the
> > > > > faulty packet without notification.
> > > > >
> > > >
> > > > What we proposed is that the pmd expose to dpdk, and dpdk expose to
> > > > the application, an mbuf check function.  This way applications that
> > > > can generate complex packets can verify that the device will be able
> > > > to process them, and applications that only generate simple mbufs
> > > > can avoid the overhead by not calling the function.
> > >
> > > More than a check, it should be exposed as a capability of the port.
> > > Anyway, if the application sends too much segments, the driver must
> > > drop it to avoid hang, and maintain a dedicated statistic counter to
> > > allow easy debugging.
> >
> > I agree with Thomas - this should not be optional. Malformed packets
> > should be dropped. In the icgbe case it's a very simple test - it's a
> > single branch per packet so i doubt that it could impose any measurable
> > performance degradation.
> >
> Actually, it could very well do - we'd have to test it. For the vector IO
> paths, every additional cycle in the RX or TX paths causes a noticeable
perf
> drop.

Well if your application is willing to know all different HW limitations
then u may not need it. However usually application doesn't want to know
the HW technical details. And it this case ignoring them may cause HW to
hang.

Of course, if your app always sends single fragment packets of less than
1500 bytes then u r right and u will most likely not hit any HW limitation,
however what i have in mind is a full featured case where packets are bit
more big and complicated and where a single branch per packet will change
nothing. This is regarding 40 segments case.

In regard to the RS bit - this is related to any packet and according to
spec it should be set in every packet.

>
> /Bruce

      reply	other threads:[~2015-09-11 16:13 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-13 18:06 Vlad Zolotarov
2015-08-13 20:28 ` Zhang, Helin
2015-08-14  5:37   ` Vlad Zolotarov
2015-08-19  0:42     ` Lu, Wenzhuo
2015-08-19  4:55       ` Vladislav Zolotarov
2015-08-19  7:43         ` Ananyev, Konstantin
2015-08-19 10:02           ` Vlad Zolotarov
2015-08-20  8:41             ` Ananyev, Konstantin
2015-08-20  8:56               ` Vlad Zolotarov
2015-08-20  9:05                 ` Vlad Zolotarov
2015-08-20  9:06                   ` Vlad Zolotarov
2015-08-25 17:33                     ` Ananyev, Konstantin
2015-08-25 17:39                       ` Avi Kivity
2015-08-19 17:29         ` Zhang, Helin
2015-08-25 18:13           ` Zhang, Helin
2015-08-25 18:33             ` Vladislav Zolotarov
2015-08-25 18:43               ` Zhang, Helin
2015-08-25 18:52                 ` Vlad Zolotarov
2015-08-25 19:16                   ` Zhang, Helin
2015-08-25 19:23                     ` Avi Kivity
2015-08-25 19:30                     ` Vladislav Zolotarov
2015-08-25 20:07                       ` Vlad Zolotarov
2015-08-25 20:13                       ` Zhang, Helin
2015-09-09 12:18                         ` Thomas Monjalon
2015-09-09 13:19                           ` Ananyev, Konstantin
2015-09-11 15:17                             ` Vladislav Zolotarov
2015-09-11 14:25                   ` didier.pallard
2015-09-11 14:47                     ` Avi Kivity
2015-09-11 14:55                       ` Thomas Monjalon
2015-09-11 15:12                         ` Vladislav Zolotarov
2015-09-11 15:43                           ` Avi Kivity
2015-09-11 16:04                             ` Vladislav Zolotarov
2015-09-11 16:07                               ` Richardson, Bruce
2015-09-11 16:14                                 ` Vladislav Zolotarov
2015-09-11 17:44                                 ` Avi Kivity
2015-09-11 16:08                             ` Thomas Monjalon
2015-09-11 16:18                               ` Vladislav Zolotarov
2015-09-11 17:17                                 ` Matthew Hall
2015-09-11 17:42                                   ` Ananyev, Konstantin
2015-09-11 17:58                                     ` Matthew Hall
2015-09-11 17:48                               ` Avi Kivity
2015-09-13 11:47                                 ` Ananyev, Konstantin
2015-09-13 12:24                                   ` Vlad Zolotarov
2015-09-13 12:32                                   ` Avi Kivity
2015-09-13 15:54                                     ` Ananyev, Konstantin
2015-09-13 16:01                                       ` Avi Kivity
2015-09-11 16:00                           ` Richardson, Bruce
2015-09-11 16:13                             ` Vladislav Zolotarov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOYyTHZc=rN4L9qwTqpbW8E=ggmCLSo2hfOPruzaA0y30Avr+Q@mail.gmail.com' \
    --to=vladz@cloudius-systems.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).