2010-12-18 18:31:17

by Felix Fietkau

Subject: [PATCH 1/3] mac80211: fix initialization of skb->cb in ieee80211_subif_start_xmit

The change 'mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs'
added a check for copying the skb if it's shared, however the tx info variable
still points at the cb of the old skb

Cc: Helmut Schaa <[email protected]>
Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/tx.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 898e864..7d78aca 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1742,7 +1742,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
- struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_tx_info *info;
int ret = NETDEV_TX_BUSY, head_need;
u16 ethertype, hdrlen, meshhdrlen = 0;
__le16 fc;
@@ -2033,6 +2033,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
skb_set_network_header(skb, nh_pos);
skb_set_transport_header(skb, h_pos);

+ info = IEEE80211_SKB_CB(skb);
memset(info, 0, sizeof(*info));

dev->trans_start = jiffies;
--
1.7.3.2

2010-12-18 18:31:13

by Felix Fietkau

Subject: [PATCH 2/3] mac80211: skip unnecessary pskb_expand_head calls

If the skb is not cloned and we don't need any extra headroom, there
is no point in reallocating the skb head.

Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/tx.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7d78aca..56ae175 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1549,8 +1549,10 @@ static int ieee80211_skb_resize(struct ieee80211_local *local,

if (skb_header_cloned(skb))
I802_DEBUG_INC(local->tx_expand_skb_head_cloned);
- else
+ else if (head_need || tail_need)
I802_DEBUG_INC(local->tx_expand_skb_head);
+ else
+ return 0;

if (pskb_expand_head(skb, head_need, tail_need, GFP_ATOMIC)) {
wiphy_debug(local->hw.wiphy,
--
1.7.3.2

2010-12-18 18:31:15

by Felix Fietkau

Subject: [PATCH 3/3] mac80211: fix potentially redundant skb data copying

When an skb is shared, it needs to be duplicated, along with its data buffer.
If the skb does not have enough headroom, using skb_copy might cause the data
buffer to be copied twice (once by skb_copy and once by pskb_expand_head).
Fix this by using skb_clone initially and letting ieee80211_skb_resize sort
out the rest.

Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/tx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 56ae175..d484c07 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1939,7 +1939,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
*/
if (skb_shared(skb)) {
tmp_skb = skb;
- skb = skb_copy(skb, GFP_ATOMIC);
+ skb = skb_clone(skb, GFP_ATOMIC);
kfree_skb(tmp_skb);

if (!skb) {
--
1.7.3.2

2010-12-19 10:10:57

by Helmut Schaa

Subject: Re: [PATCH 1/3] mac80211: fix initialization of skb->cb in ieee80211_subif_start_xmit

Am Samstag, 18. Dezember 2010 schrieb Felix Fietkau:
> The change 'mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs'
> added a check for copying the skb if it's shared, however the tx info variable
> still points at the cb of the old skb
>
> Cc: Helmut Schaa <[email protected]>
> Signed-off-by: Felix Fietkau <[email protected]>

Ah, missed that. Thanks Felix.

Acked-by: Helmut Schaa <[email protected]>

> ---
> net/mac80211/tx.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 898e864..7d78aca 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1742,7 +1742,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> {
> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> struct ieee80211_local *local = sdata->local;
> - struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + struct ieee80211_tx_info *info;
> int ret = NETDEV_TX_BUSY, head_need;
> u16 ethertype, hdrlen, meshhdrlen = 0;
> __le16 fc;
> @@ -2033,6 +2033,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> skb_set_network_header(skb, nh_pos);
> skb_set_transport_header(skb, h_pos);
>
> + info = IEEE80211_SKB_CB(skb);
> memset(info, 0, sizeof(*info));
>
> dev->trans_start = jiffies;
>