> -----Original Message----- > From: Morten Brørup [mailto:m...@smartsharesystems.com] > Sent: Thursday, January 10, 2019 5:32 AM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; Stephen > Hemminger <step...@networkplumber.org>; Hu, Jiayu > <jiayu...@intel.com> > Cc: dev@dpdk.org; tho...@monjalon.net; Varghese, Vipin > <vipin.vargh...@intel.com>; sta...@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5] doc: add GRO API limitations in > prog_guide > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev, > Konstantin > > > From: Stephen Hemminger [mailto:step...@networkplumber.org] > > > > > > O be merged. > > > > + > > > > +GRO Library Limitations > > > > +----------------------- > > > > + > > > > +- GRO library uses the values of MBUF->l2_len/l3_len/l4_len/ > > > > + outer_l2_len/outer_l3_len to get protocol headers for the > > > > + input packet, rather than parsing the packet header. Therefore, > > > > + before call GRO APIs to merge packets, user applications > > > > + must set MBUF->l2_len/l3_len/l4_len/outer_l2_len/outer_l3_len > > > > + to the same values as the protocol headers of the packet. > > > > + > > > > > > Since these length values are critical to other functionality > > > why not require all poll mode drivers to set them. > > > > Most of current HW doesn't provide that functionality, > > so RX function would need to parse (touch) packet data. > > True... In an application where the first process step is to receive the > packet > from the PMD and put it directly into the appropriate queue (or drops it) > solely based on information provided in the "RX part" of the MBUF, two > cache misses can be avoided - reading the header in the packet data and > writing the "TX part" of the mbuf. > > This is a key selling point for NIC hardware with the ability to perform > classification, such as the Flow Director feature, so it can efficiently > identify > and discard packets related to DDOS attacks, as an example. > > > From other side not every rx_burst() consumer does use GRO library. > > > > Nonetheless, many applications need to touch the packet header on ingress > for classification purposes - either to identify a flow or to identify the > attributes used for routing and QoS classification. > > I expect that validating packet headers (i.e. identifying malformed packets) > somewhere early on the ingress fast path is a very common use case, which > is why I on another thread suggested extending rte_net_get_ptype() to > check packet validity and building a bulk function on top of that to set the > MBUF->l2_len/l3_len... fields, so they are ready for GRO, Fragment > Reassembly and other ingress path libraries requiring this information: > https://mails.dpdk.org/archives/dev/2019-January/122701.html > > > > > > > Many poll mode drivers call rte_net_get_ptype() on the received > > > mbuf and it already handles setting this. > > > > > > One could argue that GRO should just log and die if it > > > gets malformed data. > > This would be a good principle! If preconditions are not met, it is a bug and > should be treated as such. As I mentioned before, this specific function is > not taking foreign input; the application is in full control of passing > garbage > or not to this function.
Since we already have lots of discussions around GRO, I want to make a summary just in case that we go far away from the purpose of the two GRO patches. 1. To accelerate header processing, GRO is designed to use MBUF->l2_len/... to get protocol headers for input packets; user applications must set MBUF->l2_len/... to the same values as packet headers. As we discussed in the previous mails, in the real-world scenarios, many applications know the header information, and they can set MBUF->l2_len/... to the corresponding values. So I think the design of GRO makes sense. What we lack is to well explain it in the document, and this patch is to add the missing information. One thing to notice is that GRO shouldn't check if the values of MBUF->l2_len/... are the same as protocol headers in the packet. This is because that checking the values requires GRO to re-parse packet headers, which makes the design that uses MBUF->l2_len/... to get protocol headers meaningless. If you don't think using MBUF->l2_len/... is a reasonable design and you have better ideas, we can discuss in a new thread or in a RFC. Because it's a feature change rather than a bug fix. 2. The second patch is to forbid GRO to process invalid input packets (http://patches.dpdk.org/patch/49491/). Even if the application sets MBUF->l2_len.. to the same values as packet headers, the input packets of GRO may still be invalid. E.g., TCP header is less than 20 bytes. In current implementation, GRO will still process these invalid packets. In previous mails, you suggested to terminate applications. But it's too extreme. As a reassembly library, I think a better way is to add necessary checks to find invalid packets and return them to applications. Thanks, Jiayu > > > Med venlig hilsen / kind regards > - Morten Brørup
