netdev - Re: [RFC] move dma_head/dma_maps out of skb_shared_info and into sk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4AF3868B.7070409@gmail.com>
Date:	Fri, 06 Nov 2009 03:14:35 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Alexander Duyck <alexander.h.duyck@...el.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [RFC] move dma_head/dma_maps out of skb_shared_info and into
 sk_buff

Alexander Duyck a écrit :
> During testing we found issues with the use of skb_dma_map/unmap on
> systems that had iommu enabled and were configured to use a bridge.  The
> issue is that if two ports are members of the same bridge, and a
> broadcast packet is sent out on the bridge skb_clone will be used to
> send a copy to all ports, but the clones run into issues because the
> dma mappings for the cloned skbs all share the shared_info structure
> where the dma mappings are stored.
> 
> To resolve that this patch moves those dma mappings out of the
> shared_info structure and into the sk_buff itself.  This allows cloned
> skbs to be mapped separately without causing dma unmapping errors.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>

Hello Alexander

You probably know such a change is a major one ;)

1) a diffstat -p1 -w70 for this kind of patch would be nice.

2) Your patch is garbled (tabulations were replaced by spaces
 by your mailer)

3) Are you sure we need to clear dma_maps[] array and dma_head 
   in __alloc_skb() ? I guess not.
   MAX_SKB_FRAGS = 18 on x86  -> 152 bytes on x86_64.
   Previous implementation was not clearing them.
   Thats would be a major slow down.

4) 152 bytes more in skb -> 304 bytes more in skbuff_fclone_cache
   Do we really want two copies of dma_maps[] when skb are allocated
   from fclone cache ?

5) It seems to me this stuff is needed for xmit only and few drivers,
   could we find a way to not have it for RX path and drivers that dont
   need it ? Maybe drivers themselves should allocate storage for this
  stuff so we can remove it both from shared_info *and* skb 

Thanks

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html