Hi Cristian, Yes, I mostly agree with your suggestions: 1. We should fix the two obvious bugs (1a and 1b) right away. Jasvinder's patches look fine. 2. We should take no immediate action on the issue I raised about PMDs (vector IXGBE) not enqueuing more than 32 packets. We can discuss and debate; no patch for 16.04, perhaps something in 16.07.
Let's start the discussion now, regarding vector IXGBE. You state "Internally it handles packets in batches [of] 32 (as your code snippets suggest), but there is no drop of excess packets taking place." I guess it depends on your definition of "drop". If I pass 33 packets to ixgbe_xmit_pkts_vec(), it will enqueue 32 packets, and return a value of 32. Can we agree on that? -- Regards, Robert On 4/11/16 3:21 PM, "Dumitrescu, Cristian" <cristian.dumitrescu at intel.com> wrote: >Hi Robert, > >I am doing a quick summary below on the changes proposed by these patches: > >1. [PRIORITY 1] Bug fixing: >a) Fix buffer overflow issue in rte_port_ring.c (writer, writer_nodrop): >double the tx_buf buffer size (applicable for current code approach) >b) Fix issue with handling burst sizes bigger than 32: replace all >declarations of local variable bsz_size from uint32_t to uint64_t > >2. [PRIORITY 2] Treat burst size as a fixed/exact value for the TX burst >(Approach 2) instead of minimal value (current code, Approach 1) for >ethdev ports. Rationale is that some PMDs (like vector IXGBE) _might_ >drop the excess packets in the burst. > >Additional input: >1. Bruce and I looked together at the code, it looks that vector IXGBE is >not doing this (anymore). Internally it handles packets in batches on 32 >(as your code snippets suggest), but there is no drop of excess packets >taking place. > >2. Venky also suggested to keep a larger burst as a single burst >(Approach 1) rather than break the larger burst into a fixed/constant >size burst while buffering the excess packets until complete burst is met >in the future. > >Given this input and also the timing of the release, we think the best >option is: >- urgently send a quick patch to handle the bug fixes now >- keep the existing code (burst size used as minimal burst size >requirement, not constant) as is, at least for now, and if you feel it is >not the best choice, we can continue to debate it for 16.7 release. >What do you think? > >Jasvinder just send the bug fixing patches, hopefully they will make it >into the 16.4 release: >http://www.dpdk.org/ml/archives/dev/2016-April/037392.html >http://www.dpdk.org/ml/archives/dev/2016-April/037393.html > >Many thanks for your work on this, Robert! > >Regards, >Cristian
