On Fri, 9 Aug 2019 18:49:50 +0000, Saeed Mahameed wrote: > On Thu, 2019-08-08 at 18:15 -0700, Jakub Kicinski wrote: > > On Thu, 8 Aug 2019 20:22:00 +0000, Saeed Mahameed wrote: > > > From: Maxim Mikityanskiy <maxi...@mellanox.com> > > > > > > The current ARFS code relies on certain fields to be set in the SKB > > > (e.g. transport_header) and extracts IP addresses and ports by > > > custom > > > code that parses the packet. The necessary SKB fields, however, are > > > not > > > always set at that point, which leads to an out-of-bounds access. > > > Use > > > skb_flow_dissect_flow_keys() to get the necessary information > > > reliably, > > > fix the out-of-bounds access and reuse the code. > > > > The whole series LGTM, FWIW. > > > > I'd be curious to hear which path does not have the skb fully > > set up, could you elaborate? (I'm certainly no aRFC expert this > > is pure curiosity). > > In our regression we found two use cases that might lead aRFS using un- > initialized values. > 1) GRO Disabled, Usually GRO fills the necessary fields. > 2) Raw socket type of tests. > > And i am sure there are many other use cases. So drivers must use > skb_flow_dissect_flow_keys() for aRFS parsing and eliminate all > uncertainties.
Looking at the code now it makes perfect sense. We could probably refactor to call the dissector in the core at some point. Thanks for the explanation!
