* [PATCH V3] mac80211: fix paged defragmentation @ 2010-05-11 18:22 Abhijeet Kolekar 2010-05-11 18:14 ` John W. Linville 0 siblings, 1 reply; 7+ messages in thread From: Abhijeet Kolekar @ 2010-05-11 18:22 UTC (permalink / raw) To: linux-wireless; +Cc: yi.zhu, Abhijeet Kolekar Paged RX skb patch broke the defragmentation. We need to read hdr again after linearization. It fixes following bug http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 Signed-off-by: Zhu, Yi <yi.zhu@intel.com> Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> --- v2: Changed hdr reading. v3: Added more comments. net/mac80211/rx.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 9a08f2c..6e2a7bc 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) if (skb_linearize(rx->skb)) return RX_DROP_UNUSABLE; + /* + * skb_linearize() might change the skb->data and + * previously cached variables (in this case, hdr) need to + * be refreshed with the new data. + */ + hdr = (struct ieee80211_hdr *)rx->skb->data; seq = (sc & IEEE80211_SCTL_SEQ) >> 4; if (frag == 0) { -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 18:22 [PATCH V3] mac80211: fix paged defragmentation Abhijeet Kolekar @ 2010-05-11 18:14 ` John W. Linville 2010-05-11 18:16 ` Abhijeet Kolekar 0 siblings, 1 reply; 7+ messages in thread From: John W. Linville @ 2010-05-11 18:14 UTC (permalink / raw) To: Abhijeet Kolekar; +Cc: linux-wireless, yi.zhu On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > Paged RX skb patch broke the defragmentation. We need to read hdr again > after linearization. > > It fixes following bug > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > --- > v2: Changed hdr reading. > v3: Added more comments. > net/mac80211/rx.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > index 9a08f2c..6e2a7bc 100644 > --- a/net/mac80211/rx.c > +++ b/net/mac80211/rx.c > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > if (skb_linearize(rx->skb)) > return RX_DROP_UNUSABLE; > > + /* > + * skb_linearize() might change the skb->data and > + * previously cached variables (in this case, hdr) need to > + * be refreshed with the new data. > + */ > + hdr = (struct ieee80211_hdr *)rx->skb->data; > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > if (frag == 0) { And what about making sure the compiler doesn't optimize this away? John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 18:14 ` John W. Linville @ 2010-05-11 18:16 ` Abhijeet Kolekar 2010-05-11 18:24 ` John W. Linville 0 siblings, 1 reply; 7+ messages in thread From: Abhijeet Kolekar @ 2010-05-11 18:16 UTC (permalink / raw) To: John W. Linville; +Cc: linux-wireless@vger.kernel.org, Zhu, Yi Hello John, On Tue, 2010-05-11 at 11:14 -0700, John W. Linville wrote: > On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > > Paged RX skb patch broke the defragmentation. We need to read hdr again > > after linearization. > > > > It fixes following bug > > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > > --- > > v2: Changed hdr reading. > > v3: Added more comments. > > net/mac80211/rx.c | 6 ++++++ > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > index 9a08f2c..6e2a7bc 100644 > > --- a/net/mac80211/rx.c > > +++ b/net/mac80211/rx.c > > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > > if (skb_linearize(rx->skb)) > > return RX_DROP_UNUSABLE; > > > > + /* > > + * skb_linearize() might change the skb->data and > > + * previously cached variables (in this case, hdr) need to > > + * be refreshed with the new data. > > + */ > > + hdr = (struct ieee80211_hdr *)rx->skb->data; > > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > > > if (frag == 0) { > > And what about making sure the compiler doesn't optimize this away? > To avoid the double assignment, there is one more approach is to directly read fc and seq_ctrl using skb_data. I will send that in the next version. Abhijeet > John ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 18:16 ` Abhijeet Kolekar @ 2010-05-11 18:24 ` John W. Linville 2010-05-11 18:52 ` Abhijeet Kolekar 0 siblings, 1 reply; 7+ messages in thread From: John W. Linville @ 2010-05-11 18:24 UTC (permalink / raw) To: Abhijeet Kolekar; +Cc: linux-wireless@vger.kernel.org, Zhu, Yi On Tue, May 11, 2010 at 11:16:50AM -0700, Abhijeet Kolekar wrote: > Hello John, > On Tue, 2010-05-11 at 11:14 -0700, John W. Linville wrote: > > On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > > > Paged RX skb patch broke the defragmentation. We need to read hdr again > > > after linearization. > > > > > > It fixes following bug > > > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > > > > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > > > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > > > --- > > > v2: Changed hdr reading. > > > v3: Added more comments. > > > net/mac80211/rx.c | 6 ++++++ > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > > index 9a08f2c..6e2a7bc 100644 > > > --- a/net/mac80211/rx.c > > > +++ b/net/mac80211/rx.c > > > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > > > if (skb_linearize(rx->skb)) > > > return RX_DROP_UNUSABLE; > > > > > > + /* > > > + * skb_linearize() might change the skb->data and > > > + * previously cached variables (in this case, hdr) need to > > > + * be refreshed with the new data. > > > + */ > > > + hdr = (struct ieee80211_hdr *)rx->skb->data; > > > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > > > > > if (frag == 0) { > > > > And what about making sure the compiler doesn't optimize this away? > > > To avoid the double assignment, there is one more approach is to > directly read fc and seq_ctrl using skb_data. I will send that in the > next version. I don't think the double assignment is so bad, I just think that a compiler might decide to ignore the second assignment. Am I wrong? John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 18:24 ` John W. Linville @ 2010-05-11 18:52 ` Abhijeet Kolekar 2010-05-11 19:04 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Abhijeet Kolekar @ 2010-05-11 18:52 UTC (permalink / raw) To: John W. Linville; +Cc: linux-wireless@vger.kernel.org, Zhu, Yi Hello John, On Tue, 2010-05-11 at 11:24 -0700, John W. Linville wrote: > On Tue, May 11, 2010 at 11:16:50AM -0700, Abhijeet Kolekar wrote: > > Hello John, > > On Tue, 2010-05-11 at 11:14 -0700, John W. Linville wrote: > > > On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > > > > Paged RX skb patch broke the defragmentation. We need to read hdr again > > > > after linearization. > > > > > > > > It fixes following bug > > > > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > > > > > > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > > > > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > > > > --- > > > > v2: Changed hdr reading. > > > > v3: Added more comments. > > > > net/mac80211/rx.c | 6 ++++++ > > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > > > index 9a08f2c..6e2a7bc 100644 > > > > --- a/net/mac80211/rx.c > > > > +++ b/net/mac80211/rx.c > > > > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > > > > if (skb_linearize(rx->skb)) > > > > return RX_DROP_UNUSABLE; > > > > > > > > + /* > > > > + * skb_linearize() might change the skb->data and > > > > + * previously cached variables (in this case, hdr) need to > > > > + * be refreshed with the new data. > > > > + */ > > > > + hdr = (struct ieee80211_hdr *)rx->skb->data; > > > > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > > > > > > > if (frag == 0) { > > > > > > And what about making sure the compiler doesn't optimize this away? > > > > > To avoid the double assignment, there is one more approach is to > > directly read fc and seq_ctrl using skb_data. I will send that in the > > next version. > > I don't think the double assignment is so bad, I just think that a > compiler might decide to ignore the second assignment. Am I wrong? > I don't understand why compiler will ignore the second assignment other than the above reason. What will be the solution in this case? Abhijeet > John ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 18:52 ` Abhijeet Kolekar @ 2010-05-11 19:04 ` Johannes Berg 2010-05-11 19:43 ` John W. Linville 0 siblings, 1 reply; 7+ messages in thread From: Johannes Berg @ 2010-05-11 19:04 UTC (permalink / raw) To: Abhijeet Kolekar Cc: John W. Linville, linux-wireless@vger.kernel.org, Zhu, Yi On Tue, 2010-05-11 at 11:52 -0700, Abhijeet Kolekar wrote: > Hello John, > On Tue, 2010-05-11 at 11:24 -0700, John W. Linville wrote: > > On Tue, May 11, 2010 at 11:16:50AM -0700, Abhijeet Kolekar wrote: > > > Hello John, > > > On Tue, 2010-05-11 at 11:14 -0700, John W. Linville wrote: > > > > On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > > > > > Paged RX skb patch broke the defragmentation. We need to read hdr again > > > > > after linearization. > > > > > > > > > > It fixes following bug > > > > > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > > > > > > > > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > > > > > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > > > > > --- > > > > > v2: Changed hdr reading. > > > > > v3: Added more comments. > > > > > net/mac80211/rx.c | 6 ++++++ > > > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > > > > index 9a08f2c..6e2a7bc 100644 > > > > > --- a/net/mac80211/rx.c > > > > > +++ b/net/mac80211/rx.c > > > > > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > > > > > if (skb_linearize(rx->skb)) > > > > > return RX_DROP_UNUSABLE; > > > > > > > > > > + /* > > > > > + * skb_linearize() might change the skb->data and > > > > > + * previously cached variables (in this case, hdr) need to > > > > > + * be refreshed with the new data. > > > > > + */ > > > > > + hdr = (struct ieee80211_hdr *)rx->skb->data; > > > > > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > > > > > > > > > if (frag == 0) { > > > > > > > > And what about making sure the compiler doesn't optimize this away? > > > > > > > To avoid the double assignment, there is one more approach is to > > > directly read fc and seq_ctrl using skb_data. I will send that in the > > > next version. > > > > I don't think the double assignment is so bad, I just think that a > > compiler might decide to ignore the second assignment. Am I wrong? > > > I don't understand why compiler will ignore the second assignment other > than the above reason. What will be the solution in this case? ACCESS_ONCE()? I have no idea why/if the compiler would actually do this though. johannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] mac80211: fix paged defragmentation 2010-05-11 19:04 ` Johannes Berg @ 2010-05-11 19:43 ` John W. Linville 0 siblings, 0 replies; 7+ messages in thread From: John W. Linville @ 2010-05-11 19:43 UTC (permalink / raw) To: Johannes Berg; +Cc: Abhijeet Kolekar, linux-wireless@vger.kernel.org, Zhu, Yi On Tue, May 11, 2010 at 09:04:56PM +0200, Johannes Berg wrote: > On Tue, 2010-05-11 at 11:52 -0700, Abhijeet Kolekar wrote: > > Hello John, > > On Tue, 2010-05-11 at 11:24 -0700, John W. Linville wrote: > > > On Tue, May 11, 2010 at 11:16:50AM -0700, Abhijeet Kolekar wrote: > > > > Hello John, > > > > On Tue, 2010-05-11 at 11:14 -0700, John W. Linville wrote: > > > > > On Tue, May 11, 2010 at 11:22:11AM -0700, Abhijeet Kolekar wrote: > > > > > > Paged RX skb patch broke the defragmentation. We need to read hdr again > > > > > > after linearization. > > > > > > > > > > > > It fixes following bug > > > > > > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194 > > > > > > > > > > > > Signed-off-by: Zhu, Yi <yi.zhu@intel.com> > > > > > > Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> > > > > > > --- > > > > > > v2: Changed hdr reading. > > > > > > v3: Added more comments. > > > > > > net/mac80211/rx.c | 6 ++++++ > > > > > > 1 files changed, 6 insertions(+), 0 deletions(-) > > > > > > > > > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > > > > > index 9a08f2c..6e2a7bc 100644 > > > > > > --- a/net/mac80211/rx.c > > > > > > +++ b/net/mac80211/rx.c > > > > > > @@ -1253,6 +1253,12 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) > > > > > > if (skb_linearize(rx->skb)) > > > > > > return RX_DROP_UNUSABLE; > > > > > > > > > > > > + /* > > > > > > + * skb_linearize() might change the skb->data and > > > > > > + * previously cached variables (in this case, hdr) need to > > > > > > + * be refreshed with the new data. > > > > > > + */ > > > > > > + hdr = (struct ieee80211_hdr *)rx->skb->data; > > > > > > seq = (sc & IEEE80211_SCTL_SEQ) >> 4; > > > > > > > > > > > > if (frag == 0) { > > > > > > > > > > And what about making sure the compiler doesn't optimize this away? > > > > > > > > > To avoid the double assignment, there is one more approach is to > > > > directly read fc and seq_ctrl using skb_data. I will send that in the > > > > next version. > > > > > > I don't think the double assignment is so bad, I just think that a > > > compiler might decide to ignore the second assignment. Am I wrong? > > > > > I don't understand why compiler will ignore the second assignment other > > than the above reason. What will be the solution in this case? > > ACCESS_ONCE()? I have no idea why/if the compiler would actually do this > though. I don't know about "if", but "why" might be that as far as the compiler can see you have two identical assignments without an obvious change to the data source in between. But maybe passing rx->skb to skb_linearize is enough information to make the compiler aware that rx->skb->data might have changed? Any better language lawyers than me around? I think ACCESS_ONCE would be enough, but maybe it isn't necessary...? John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-05-11 19:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-11 18:22 [PATCH V3] mac80211: fix paged defragmentation Abhijeet Kolekar 2010-05-11 18:14 ` John W. Linville 2010-05-11 18:16 ` Abhijeet Kolekar 2010-05-11 18:24 ` John W. Linville 2010-05-11 18:52 ` Abhijeet Kolekar 2010-05-11 19:04 ` Johannes Berg 2010-05-11 19:43 ` John W. Linville
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).