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
| ||
|
Date: Wed, 25 Mar 2009 16:43:53 +0800 From: Li Yang <leoli@...escale.com> To: David Miller <davem@...emloft.net> Cc: shemminger@...ux-foundation.org, bridge@...ts.linux-foundation.org, netdev@...r.kernel.org Subject: Re: [PATCH] net/bridge: use the maximum hard_header_len of ports for bridging device On Tue, Mar 24, 2009 at 6:20 AM, David Miller <davem@...emloft.net> wrote: > From: Stephen Hemminger <shemminger@...ux-foundation.org> > Date: Mon, 23 Mar 2009 08:51:22 -0700 > >> That ensures big enough header for locally generated packets, but >> any drivers that need bigger headroom still must handle bridged packets >> that come in with smaller space. When bridging packets, the skb comes >> from the allocation by the receiving driver. Almost all drivers will >> use dev_alloc_skb() which will allocate NET_SKB_PAD (16) bytes of >> additional headroom. This is used to hold copy of ethernet header for >> the bridge/netfilter code. >> >> So your patch is fine as an optimization but a driver can not safely >> depend on any additional headroom. The driver must check if there >> is space, and if no space is available, reallocate and copy. > > We had some plans to deal with this kind of issue for wireless > too. Let me see if I can find the RFC patch from that discussion... > > Here it is, similar code would be added to the ipv4/ipv6 forwarding > paths: > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 7c1d446..6c06fba 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -600,6 +600,7 @@ struct net_device > * Cache line mostly used on receive path (including eth_type_trans()) > */ > unsigned long last_rx; /* Time of last Rx */ > + unsigned int rx_alloc_extra; > /* Interface address info used in eth_type_trans() */ > unsigned char dev_addr[MAX_ADDR_LEN]; /* hw address, (before bcast > because most packets are unicast) */ > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > index bdd7c35..531e483 100644 > --- a/net/bridge/br_forward.c > +++ b/net/bridge/br_forward.c > @@ -42,6 +42,22 @@ int br_dev_queue_push_xmit(struct sk_buff *skb) > if (nf_bridge_maybe_copy_header(skb)) > kfree_skb(skb); > else { > + unsigned int headroom = skb_headroom(skb); > + unsigned int hh_len = LL_RESERVED_SPACE(skb->dev); > + > + if (headroom < hh_len) { > + struct net_device *in_dev; > + unsigned int extra; > + > + in_dev = __dev_get_by_index(dev_net(skb->dev), > + skb->iif); > + BUG_ON(!in_dev); > + > + extra = hh_len - headroom; > + if (extra >= in_dev->rx_alloc_extra) > + in_dev->rx_alloc_extra = extra; > + } > + > skb_push(skb, ETH_HLEN); > > dev_queue_xmit(skb); Dynamically adjusting is a good idea, but the rx_alloc_extra can only go up not the other way down in your code. Another thought is that if you re-allocate skb here the driver would be saved from checking the headroom in the fastpath, am I right? - Leo -- 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
Powered by blists - more mailing lists