Thanks, Quick check on that function - very similar to unclone which we know ameliorates the issue, just has the headroom parameter rather than priority. So clearly the right way to go. Just one more question I hope - where is the appropriate place for the skb_cow_head call in the driver - at the main entry point, or closer or in the function(s) that may alter the header itself?
James (Apologies to Eric for originally sending him this message rather than to netdev - was doing it all from a phone and screwed it up) On 17 April 2017 at 17:07, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Mon, 2017-04-17 at 16:02 +0100, James Hughes wrote: >> Netdevs, >> >> We have recently got to the bottom of an issue which we have been >> encountering on a Raspberry Pi being used as an access point, and we >> need a bit of advice on the correct way of fixing the issue. >> >> The set up is a Raspberry Pi 3 running hostapd on its inbuilt wireless >> adaptor (Brcm43438 ), bridged to the built in ethernet adaptor >> (smsc9514). Using the standard drivers for these devices as of 4.9 >> (looking at 4.11, no changes noted that would affect the issue. I will >> be trying the latest kernel once I get back in to the office). >> >> We were encountering an error in the Brcm Wireless driver that after >> investigation was a skb_buff being corrupted by an action in the smsc >> Ethernet driver. Further digging shows that the bridge was cloning an >> incoming skb from the Ethernet, then sending it out to both the >> Ethernet and wlan ports. This mean that both the Ethernet driver and >> the wireless driver were looking at the same physical data, and one or >> both were altering that data in different ways (varied headers) , >> which mean that headers were corrupted. This was only happening on >> broadcast packets. >> >> We can fix the issue by, in the drivers, using skb_unclone in >> appropriate places in the driver. >> >> So now to the questions. Is adding the unclone to the drivers the >> correct way of dealing with this issue? Examining other drivers shows >> that unclone is not particularly common, presumably in many cases, >> where the driver does not alter the skb, it doesn't matter. However, >> in any case where a driver may add header information to the skb (as >> is the case with the Brcm wireless driver), when the incoming skb has >> been cloned, the results must surely be undetermined unless an unclone >> is performed. >> >> Or, is the bridging code making a mistake by cloning the skb and >> passing it to multiple recipients? >> > > > bridge code is fine. > > Problem is that some drivers lack calls to skb_cow_head() before they > mess with headers. > > > > > > -- James Hughes Principal Software Engineer, Raspberry Pi (Trading) Ltd
