> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hu, Jiayu > > From: Morten Brørup [mailto:m...@smartsharesystems.com] > > > 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
+1 Let's close the GRO API sidetrack here, and continue the RFC discussion in the other thread: https://mails.dpdk.org/archives/dev/2019-January/122774.html > > > > > > Med venlig hilsen / kind regards > > - Morten Brørup
