From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [RFC] Kernel unaligned access at __skb_flow_dissect Date: Fri, 29 Jan 2016 10:33:48 -0800 Message-ID: <1454092428.7627.52.camel@edumazet-glaptop2.roam.corp.google.com> References: <20160129180651.GA17127@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org To: Sowmini Varadhan Return-path: Received: from mail-pa0-f51.google.com ([209.85.220.51]:35405 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751134AbcA2Sdu (ORCPT ); Fri, 29 Jan 2016 13:33:50 -0500 Received: by mail-pa0-f51.google.com with SMTP id ho8so45250372pac.2 for ; Fri, 29 Jan 2016 10:33:50 -0800 (PST) In-Reply-To: <20160129180651.GA17127@oracle.com> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 2016-01-29 at 13:06 -0500, Sowmini Varadhan wrote: > There is an unaligned access at __skb_flow_dissect when it calls > ip6_flowlabel() with the call stack > > __skb_flow_dissect+0x2a8/0x87c > eth_get_headlen+0x5c/0xaxa4 > ixgbe_clean_rx_irq+0x5cc/0xb20 [ixgbe] > ixgbe_poll+0x5a4/0x760 [ixgbe] > net_rx_action+0x13c/0x354 > : > > Essentially, ixgbe_pull_tail() is trying to figure out how much > to pull, in order to have an aligned buffer: > > pull_len = eth_get_headlen(va, IXGBE_RX_HDR_SIZE); > > /* align pull length to size of long to optimize memcpy performance */ > skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long))); > > and seems like the unaligned access is unavoidable here (see comments > in __skb_get_poff, for example). > > This (below) is what I came up with, to get rid of the unaligned access > errors on sparc, Is there a better solution? (Not having access > to struct ip6_hdr in this file made put_unaligned usage non-obvious) > > > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -102,6 +102,17 @@ __be32 __skb_flow_get_ports(const struct sk_buff *skb, int > } > EXPORT_SYMBOL(__skb_flow_get_ports); > > +static inline __be32 ip6_flowlabel_align(const u8 *hdr) > +{ > + union { > + __u8 w[4]; > + __u32 flow; > + } ip6_flow; > + > + memcpy(ip6_flow.w, hdr, 4); > + return (ip6_flow.flow & IPV6_FLOWLABEL_MASK); > +} > + > /** > * __skb_flow_dissect - extract the flow_keys struct and return it > * @skb: sk_buff to extract the flow from, can be NULL if the rest are specifie > @@ -230,7 +241,7 @@ ipv6: > key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS; > } > > - flow_label = ip6_flowlabel(iph); > + flow_label = ip6_flowlabel_align((const u8 *)iph); > if (flow_label) { > if (dissector_uses_key(flow_dissector, > FLOW_DISSECTOR_KEY_FLOW_LABEL)) { > > Why ipv6 stack itself does not trigger the issue ? Maybe the driver itself does not properly align IP headers on sparc ? Make sure NET_IP_ALIGN is 2 on your build. Note that x86 does not care, but a driver should always align Ethernet header to NET_IP_ALIGN, so that IP headers are aligned to 4 bytes boundaries.