From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eyal Birger Subject: Re: [PATCH net-next 4/7] net: packet: use skb->dev as storage for skb orig len instead of skb->cb[] Date: Thu, 26 Feb 2015 20:38:05 +0200 Message-ID: References: <1424916612-744-1-git-send-email-eyal.birger@gmail.com> <1424916612-744-5-git-send-email-eyal.birger@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: David Miller , Eric Dumazet , Shmulik Ladkani , Marcel Holtmann , Network Development To: Willem de Bruijn Return-path: Received: from mail-yh0-f41.google.com ([209.85.213.41]:36936 "EHLO mail-yh0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753722AbbBZSiH (ORCPT ); Thu, 26 Feb 2015 13:38:07 -0500 Received: by yhoa41 with SMTP id a41so5471725yho.4 for ; Thu, 26 Feb 2015 10:38:06 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Hi, On Thu, Feb 26, 2015 at 7:26 PM, Willem de Bruijn wrote: > On Wed, Feb 25, 2015 at 9:10 PM, Eyal Birger wrote: >> As part of an effort to move skb->dropcount to skb->cb[], 4 bytes >> of additional room are needed in skb->cb[] in packet sockets. >> >> Store the skb original length in skb->dev instead of skb->cb[] for >> this purpose. > > Another option is to delay preparation of the full sockaddr_ll struct until > when it's needed. It often is not used at all (if msg_name == NULL). > > sll_family, sll_protocol and sll_pkttype can be derived when needed > in packet_recvmsg. The first two are the head of sockaddr_ll, so > a shorter struct filled in in packet_rcv only from sll_ifindex onwards > would save the 4B needed for origlen. Actually, a union is simpler: > > struct __packet_cb_sockaddr_ll { > union { > struct { > unsigned short sll_family; > __be16 sll_protocol; > } > unsigned int sll_origlen; > } > /* etc.. */ > }; > > The family and protocol can just overwrite sll_origlen in packet_recvmsg > before the memcpy into msg->msg_name. > That's an interesting option. My personal inclination is not to inline sockaddr_ll in the packet cb structure. IMHO it would probably make sense to do it as part of your below suggestion. > Also interesting would be to avoid copying the address in the fast path > as it may never be used, especially for long addresses (INFINIBAND_ALEN > and FWNET_ALEN). From what I can tell, all but one header_ops.parse > implementations return bytes from inside the packet, so could take an > unsigned char ** as argument, store that pointer and postpone the > memcpy to packet_recvmsg (handling skb_trim correctly). The exception is > fwnet_header_parse, which would require some workaround. I think it may be interesting to pursue this option, though I don't think it should be part of this specific effort. Thanks! Eyal.