On Fri, Jan 29, 2016 at 3:58 PM, Sowmini Varadhan <sowmini.varad...@oracle.com> wrote: > On (01/29/16 15:31), Tom Herbert wrote: >> But even within flow dissector, to be completely correct, we need to >> replace all 32-bit accesses with the mempy (flow_label, mpls label, >> keyid) and be vigilant about new ones coming in. For that matter, .. > > well, one question that came to my mind when I was looking at this is: > why does eth_get_headlen try to compute the flow hash even before the > driver has had a chance to pull things into an aligned buffer? Isn't > that just risking even more unaligned accesses? > Hmm, thanks for pointing that out. It looks like this is called by a couple drivers as part of pulling up the data to get alignment. I am actually surprised they are doing this. Flow dissector is not the cheapest function in the world and it seems like a shame to be calling it this early and not even getting a hash for the skb. Also, this is another instance of touching the data so if we do get to the point of trying steering packets without cache miss (another thread); this undermines that. Seems like it might be just as well to pull up a fixed number of bytes (ixgbe want min of 60 bytes), or don't pull up anything avoid the cache miss here and let the stack pull up as needed.
Anyway, I think you do have a point that flow_dissector does not assume alignment or pull up data like the rest of the stack. Seems like we need a utility function like align_access (maybe there is one already) to get 32-bit fields and probably do need the sanity check for odd alignment. Tom
