| Message ID | 1360687038-17610-2-git-send-email-michael-dev@fami-braun.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2013-02-12 at 17:37 +0100, Michael Braun wrote: > When sending a broadcast while at least on of the connected stations is > sleeping, it gets queued and send after a DTIM beacon is sent. > If the packet was to be sent on a vlan interface, the vif used for dequeing > from the per-bss queue does not hold the per-vlan sdata. The correct sdata is > required to use the correct per-vlan broadcast/multicast key. > > This patch fixes this by restoring the per-vlan sdata using the skb->dev entry. > > Signed-off-by: Michael Braun <michael-dev@fami-braun.de> > > V2: fix compile warning That "V2" should be after the ---, but in any case: > --- a/net/mac80211/tx.c > +++ b/net/mac80211/tx.c > @@ -2728,6 +2728,7 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, > struct ps_data *ps; > struct ieee80211_tx_info *info; > struct ieee80211_chanctx_conf *chanctx_conf; > + struct ieee80211_sub_if_data *frame_sdata; There's already an sdata variable, why not write *sdata, *frame_sdata? > @@ -2770,7 +2771,8 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, > cpu_to_le16(IEEE80211_FCTL_MOREDATA); > } > > - if (!ieee80211_tx_prepare(sdata, &tx, skb)) > + frame_sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev); > + if (!ieee80211_tx_prepare(frame_sdata, &tx, skb)) > break; This can now crash the machine. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-02-12 at 17:38 +0100, Johannes Berg wrote: > On Tue, 2013-02-12 at 17:37 +0100, Michael Braun wrote: > > When sending a broadcast while at least on of the connected stations is > > sleeping, it gets queued and send after a DTIM beacon is sent. > > If the packet was to be sent on a vlan interface, the vif used for dequeing > > from the per-bss queue does not hold the per-vlan sdata. The correct sdata is > > required to use the correct per-vlan broadcast/multicast key. > > > > This patch fixes this by restoring the per-vlan sdata using the skb->dev entry. > > > > Signed-off-by: Michael Braun <michael-dev@fami-braun.de> > > > > V2: fix compile warning > > That "V2" should be after the ---, but in any case: > > > --- a/net/mac80211/tx.c > > +++ b/net/mac80211/tx.c > > @@ -2728,6 +2728,7 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, > > struct ps_data *ps; > > struct ieee80211_tx_info *info; > > struct ieee80211_chanctx_conf *chanctx_conf; > > + struct ieee80211_sub_if_data *frame_sdata; > > There's already an sdata variable, why not write *sdata, *frame_sdata? Actually you could even just use it since it's not needed after getting the "ps" pointer. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, thanks for reviewing. Am 12.02.2013 17:38, schrieb Johannes Berg: > [use sdata instead of frame_sdata] can be done. >> - if (!ieee80211_tx_prepare(sdata, &tx, skb)) >> + frame_sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev); >> + if (!ieee80211_tx_prepare(frame_sdata, &tx, skb)) >> break; > > This can now crash the machine. why? when? What would be a better approach? Regards, M. Braun -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-02-12 at 18:42 +0100, michael-dev wrote: > >> - if (!ieee80211_tx_prepare(sdata, &tx, skb)) > >> + frame_sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev); > >> + if (!ieee80211_tx_prepare(frame_sdata, &tx, skb)) > >> break; > > > > This can now crash the machine. > > why? when? There's no guarantee that the VLAN interface isn't destroyed while the frame is stuck on the buffer queue. > What would be a better approach? This is probably fine, but incomplete -- frames for a VLAN need to be removed from the PS queue when it goes down. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 2ef0e19..583c025 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -2728,6 +2728,7 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, struct ps_data *ps; struct ieee80211_tx_info *info; struct ieee80211_chanctx_conf *chanctx_conf; + struct ieee80211_sub_if_data *frame_sdata; sdata = vif_to_sdata(vif); @@ -2770,7 +2771,8 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, cpu_to_le16(IEEE80211_FCTL_MOREDATA); } - if (!ieee80211_tx_prepare(sdata, &tx, skb)) + frame_sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev); + if (!ieee80211_tx_prepare(frame_sdata, &tx, skb)) break; dev_kfree_skb_any(skb); }
When sending a broadcast while at least on of the connected stations is sleeping, it gets queued and send after a DTIM beacon is sent. If the packet was to be sent on a vlan interface, the vif used for dequeing from the per-bss queue does not hold the per-vlan sdata. The correct sdata is required to use the correct per-vlan broadcast/multicast key. This patch fixes this by restoring the per-vlan sdata using the skb->dev entry. Signed-off-by: Michael Braun <michael-dev@fami-braun.de> V2: fix compile warning --- net/mac80211/tx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)