2007-11-23 20:33:31

by Johannes Berg

[permalink] [raw]
Subject: wireless vs. alignment requirements


> David Miller found a problem in a wireless driver where I was using
> compare_ether_addr() on potentially unaligned data. Document that
> compare_ether_addr() is not safe for use everywhere, and add an equivalent
> function that works regardless of alignment.

FWIW, as I've said in the other thread I cannot believe this to be true
since the IP header is required to be aligned anyway. So *iff* this
really was a problem then we'd have much more problems and better fixed
the alignment of the received USB urb in zd1211rw.

Let me explain... This is going to take a bit of text...

Marking optional fields with < > for better readability, you have this
802.11 header layout (including byte sizes):

| FC (2) | durid (2) | A1 (6) | A2 (6) | A3 (6) | seq (2)
... < optional A4 (6) > < optional qos (2) >
... < optional HT (4) > < optional mesh (4 or 16) >
(NOTE: not sure about the order of the last two, this will probably
depend on the order in which 11n/11s are accepted! logically I would
expect the order as I wrote it)

(which makes it 24, 26, 30 or 32 bytes long plus the optional 11n/11s
additions which are multiples of four)

After that comes the data which may contain a SNAP header or such and
then the rest. The SNAP header is six bytes plus two byte ethertype, or
there could be a bridge tunnel header with six bytes plus two byte
ethertype, or it could be missing completely, in all of these cases it's
evenly divisible by four.

Now, the IP stack actually assumes that its header is four-byte aligned
(see comment at NET_IP_ALIGN, although it is not said explicitly that
the alignment requirement for an IP header is four) so that is actually
something for the hardware/firmware (!) to do, for example Broadcom
firmware will insert two bytes padding before the PLCP header (6 bytes,
sitting before the 802.11 header) if either the QoS field or A4 was
present (and the header length thus wasn't a multiple of four). Atheros
hardware will insert two bytes padding after the QoS field in the same
cases (IIRC).

Of course, you need to consider the RX header that sits before the
802.11 stuff. Broadcom's is 30 bytes, but if you add the six byte PLCP
header you have 36 bytes which is divisible by four. In the QoS XOR A4
case they add two bytes which makes it 38 with two bytes to spare for
4-byte alignment which are eaten up by the QoS/A4 field.

I don't know about any other hardware but considering that things work
as well as they do I'd think they do similar things.

Hence, going back to the 802.11 header and the IP header alignment
requirement, if we get the IP header alignment requirement right now I
cannot possibly see any way we would use compare_ether_addr() on an
address that is not at least two-byte aligned as required.

Whew. Or so I think.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-24 08:33:51

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> > Now, the IP stack actually assumes that its header is four-byte aligned
> > (see comment at NET_IP_ALIGN, although it is not said explicitly that
> > the alignment requirement for an IP header is four) so that is actually
> > something for the hardware/firmware (!) to do, for example Broadcom
>
> Good point. In fact IIRC we've always had the policy that drivers
> should do their best to generate aligned packets but it is not a
> requirement since on some platforms it's more important for the DMA
> to be aligned.

We still require four-byte alignment, no?

> So it's up the platform code to fix up any exceptions should they
> show up.
>
> Daniel, what's the specific case that you had in mind with this
> patch?

Well. This goes back to a user reporting unaligned accesses on sparc64.
Davem thought this came from the ether addr comparisons but the user
later reported that the patch from davem didn't fix it, and I think
Daniel just made a sweep over all ether addr comparisons replacing them
with unaligned ones.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-29 13:13:18

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Tue, Nov 27, 2007 at 09:16:07AM -0800, H. Peter Anvin wrote:
>
> I wrote a patch for the IP stack to realign packets if necessary at one
> point. I should dredge it up again and submit it for collective flamage.

As long as it doesn't penalise Ethernet (e.g., the 10Gb crowd :) it would
be good to have.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-24 13:51:15

by David Miller

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

From: Johannes Berg <[email protected]>
Date: Sat, 24 Nov 2007 14:49:36 +0100

>
> On Sat, 2007-11-24 at 21:32 +0800, Herbert Xu wrote:
> > On Sat, Nov 24, 2007 at 09:33:36AM +0100, Johannes Berg wrote:
> > >
> > > We still require four-byte alignment, no?
> >
> > Not at all. If NET_IP_ALIGN is zero then it won't be four-byte
> > aligned (since the Ethernet header is 14 bytes long).
>
> Right. I just didn't think that would be a valid value for an
> architecture to set.

It is, and explicitly used by powerpc to get more of the
DMA transfers 64-byte aligned which is critical for
performance on some powerpc boxes.

2007-11-25 01:45:00

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sat, Nov 24, 2007 at 10:13:19PM +0100, Johannes Berg wrote:
>
> Eight bytes really sucks for wireless, many things are multiples of four
> and QoS vs. non-QoS frames have a multiple of four and common hardware
> only adds two padding bytes to get it aligned on four bytes so there's
> no easy way to get hardware to align it properly. Hmm.

Sorry I was wrong about the 8 bytes requirement. Although the
IPv6 protocol does try to maintain an 8-byte alignment the Linux
stack never does anything that requires that.

So 4 bytes is enough.

However, the wireless core is definitely not out of the woods.
It needs to support variable hardware header lengths that are
not always a multiple of 4.

So here's my suggestion. Modify the wireless core to fix up any
packets which aren't aligned correctly. That should make it
work albeit in a way that's less than optimal.

Then for each driver where you care about this performance
(seriously I wouldn't for the speeds these things run at :),
pick the most common wireless hardware header length and have
the IP (or any other upper-level protocol) header aligned to
at least 4 bytes. Or better if you know what hardware header
length that you're going to get (e.g., based on what mode you're
in) then do the skb_reserve accordingly.

It's a good thing these things aren't running at 10Gb :)

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-25 11:23:05

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sun, Nov 25, 2007 at 12:00:28PM +0100, Johannes Berg wrote:
>
> Not sure. On the one hand, yeah, that's something we should probably do,
> on the other hand this will suck because for most drivers either nothing
> needs to be done or the fixup is trivial. I suppose we should do this
> but stick in a WARN_ON_ONCE() or something, at least with mac80211 debug
> enabled.

I don't see how you can get a WARN_ON to work when you're expecting
there to be unaligned packets from time to time as the hardware
header changes.

> Also, we do plan to run these things on rather smallish embedded devices
> like APs that receive a lot of frames from many stations, and with 11n
> we're pushing speeds up by quite a bit. I'm wary of putting more code
> into the generic receive path.

Well you don't have a choice if the hardware header is really
unpredictable. It's either that or we go and modify the entire
IP stack which penalises all the high-speed Ethernet NICs that
already get the alignment correctly.

> Well, you can receive non-QoS frames even in QoS or WPS frames in any
> other mode etc. so you can't really make any promises as which will be
> more common. Bu in practice all (sane) hardware makes sure things are
> aligned properly.

Here's an idea. Even if you can't predict the header length of
all packets, can you at least predict the header length of the
majority of data (ones carrying IP etc.) packets?

If so then you can do the skb_reserve based on that and the fix-up
in the wireless core would be minimised.

Since I know next to nothing about the wireless transport layer,
one of you experts will need to tell me whether such a prediction
could work :)

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-27 17:17:31

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
>
> Luckily all sky2 users have been on x86 so far :)
>

Hardly so. My previous employer was MIPS and we used it there (with my
patch.)

-hpa

2007-11-26 01:38:43

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sun, Nov 25, 2007 at 01:21:44PM -0800, Stephen Hemminger wrote:
>
> No too wasteful. I'm working a patch to eth_type_trans to realign if
> needed for any device.

If you're going to do that you can probably compute the checksum
at the same time.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-30 00:27:27

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Thu, Nov 29, 2007 at 09:50:35AM -0800, H. Peter Anvin wrote:
>
> Uhm, most cards affected *ARE* Ethernet cards, due to the bloody 14-byte
> header.

Well most Ethernet drivers are using NET_IP_ALIGN which means that IP
stack gets aligned packets only.

sky2 is the exception here, not the rule.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-28 02:55:27

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Stephen Hemminger wrote:
>
> Is there any standard kernel config define for "this platform can't do
> unaligned accesses"?
>

Not that I know of, but there should be.

To be somewhat more complexicated, some platforms (e.g. MIPS) can do
unaligned references quite cheaply, but they need different opcodes to
do so.

-hpa

2007-11-24 13:41:22

by Ulrich Kunitz

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Johannes,

> Hence, going back to the 802.11 header and the IP header alignment
> requirement, if we get the IP header alignment requirement right now I
> cannot possibly see any way we would use compare_ether_addr() on an
> address that is not at least two-byte aligned as required.

ACK. I agree completely.

The problem with the zd1211rw driver is, that it copies the
complete frame received from the device into the SKB and pulls
later the five bytes ZD1211 uses for the PLCP information.
This causes the 802.11 header to be on an odd address. The
reported problems are caused by this.

The zd1211rw-mac80211 is not affected, because the PLCP header is
not copied into the skb and this way the 802.11 header becomes
correctly aligned.

Here is a patch, which should solve the zd1211rw alignment issues.
It compiles, but it is not tested right now, because I got the
idea while writing this e-mail. An official submission will
follow.

Uli

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index a903645..fb54cd7 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -1166,15 +1166,22 @@ static void do_rx(unsigned long mac_ptr)
int zd_mac_rx_irq(struct zd_mac *mac, const u8 *buffer, unsigned int length)
{
struct sk_buff *skb;
+ unsigned int length_to_reserve;

- skb = dev_alloc_skb(sizeof(struct zd_rt_hdr) + length);
+ /* This ensures that there is enough place for the radiotap header
+ * and the the 802.11 header is aligned by four following the
+ * five-byte ZD1211-specific PLCP header.
+ */
+ length_to_reserve = ((sizeof(struct zd_rt_hdr) + 3) & ~3) + 3;
+
+ skb = dev_alloc_skb(length_to_reserve + length);
if (!skb) {
struct ieee80211_device *ieee = zd_mac_to_ieee80211(mac);
dev_warn(zd_mac_dev(mac), "Could not allocate skb.\n");
ieee->stats.rx_dropped++;
return -ENOMEM;
}
- skb_reserve(skb, sizeof(struct zd_rt_hdr));
+ skb_reserve(skb, length_to_reserve);
memcpy(__skb_put(skb, length), buffer, length);
skb_queue_tail(&mac->rx_queue, skb);
tasklet_schedule(&mac->rx_tasklet);

--
Uli Kunitz

2007-11-24 21:33:31

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> Then what about hardware that can't dma ethernet to non-aligned address.
> Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack should
> handle any alignment, and do the appropriate memove if the CPU requires
> alignment.

Wouldn't that better be handled in the driver rather than having the
test in the generic RX path?

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-24 13:32:25

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sat, Nov 24, 2007 at 09:33:36AM +0100, Johannes Berg wrote:
>
> We still require four-byte alignment, no?

Not at all. If NET_IP_ALIGN is zero then it won't be four-byte
aligned (since the Ethernet header is 14 bytes long).

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-27 17:17:17

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Stephen Hemminger wrote:
> Herbert Xu wrote:
>> On Sat, Nov 24, 2007 at 02:49:36PM +0100, Johannes Berg wrote:
>>
>>> Right. I just didn't think that would be a valid value for an
>>> architecture to set.
>>>
>>
>> OK. Let me clarify this a bit more. We require at least one
>> of the following rules to be met:
>>
>> * the IPv4/IPv6 header is aligned by 8 bytes on reception;
>> * or the platform provides unaligned exception handlers.
>>
>> So if your platform violates both rules then it won't work with
>> the IP stack, simple as that. Fortunately I don't think such a
>> platform exists currently on Linux.
>>
>> Cheers,
>>
>
> Then what about hardware that can't dma ethernet to non-aligned address.
> Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack
> should handle any alignment, and do the appropriate memove if the CPU requires
> alignment.

I wrote a patch for the IP stack to realign packets if necessary at one
point. I should dredge it up again and submit it for collective flamage.

-hpa


2007-11-25 14:01:33

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sun, Nov 25, 2007 at 02:54:24PM +0100, Johannes Berg wrote:
>
> But I do have a choice where to fix it up and I'd prefer the drivers to
> do it where necessary. For that, the warning would work because it'd
> show driver authors that they need to fix something.

Fair enough.

> Hmm. I don't think so. Take an AP for example. It gets a lot of packets
> from stations. Now, if you're not QoS capable then all is well. But i
> you are and some stations are as well then all those stations send QoS
> packets (+2 bytes). Or take an AP connected via wireless (WPS), WPS has
> +6 bytes so I get all incoming upstream traffic with such unaligned
> headers.

The question is does this actually change all the time. Let's
say you took a random sample of a second worth of IP packets
over wireless, what proportion of them are going to have the
same hardware header length modulo 4?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-24 21:13:37

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> OK. Let me clarify this a bit more. We require at least one
> of the following rules to be met:
>
> * the IPv4/IPv6 header is aligned by 8 bytes on reception;
> * or the platform provides unaligned exception handlers.
>
> So if your platform violates both rules then it won't work with
> the IP stack, simple as that. Fortunately I don't think such a
> platform exists currently on Linux.

Ok, thanks for the clarification.

Eight bytes really sucks for wireless, many things are multiples of four
and QoS vs. non-QoS frames have a multiple of four and common hardware
only adds two padding bytes to get it aligned on four bytes so there's
no easy way to get hardware to align it properly. Hmm.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-24 20:13:03

by Stephen Hemminger

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
> On Sat, Nov 24, 2007 at 02:49:36PM +0100, Johannes Berg wrote:
>
>> Right. I just didn't think that would be a valid value for an
>> architecture to set.
>>
>
> OK. Let me clarify this a bit more. We require at least one
> of the following rules to be met:
>
> * the IPv4/IPv6 header is aligned by 8 bytes on reception;
> * or the platform provides unaligned exception handlers.
>
> So if your platform violates both rules then it won't work with
> the IP stack, simple as that. Fortunately I don't think such a
> platform exists currently on Linux.
>
> Cheers,
>

Then what about hardware that can't dma ethernet to non-aligned address.
Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack should
handle any alignment, and do the appropriate memove if the CPU requires
alignment.

2007-11-26 01:37:46

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sun, Nov 25, 2007 at 06:04:17PM +0100, Johannes Berg wrote:
>
> I'd think that totally depends on the traffic. If you have a non-QoS AP
> with WPS upstream connection, then the traffic to stations will be
> four-byte aligned while the WPS upstream will be at a 2-byte-mod-4
> boundary. And you'll have all packets from stations come in aligned and
> all response packets from wherever come in as WPS.

OK, sounds like you'll just have to fix them up after DMA.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-30 00:41:38

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
> On Thu, Nov 29, 2007 at 04:28:34PM -0800, H. Peter Anvin wrote:
>>> sky2 is the exception here, not the rule.
>> It is, but it's not unique. Several USB adapters have the same problem,
>> for example.
>
> Notice the common theme here that slow (or slower, i.e., certainly nowhere
> near 10Gb) NICs are the norm for violating alignment :)
>
> So I'd prefer something that only penalised them rather than everybody
> else.

Obviously, and this should be a configure option anyway.

-hpa

2007-11-25 21:23:12

by Stephen Hemminger

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
> On Sat, Nov 24, 2007 at 12:11:08PM -0800, Stephen Hemminger wrote:
>
>> Then what about hardware that can't dma ethernet to non-aligned address.
>> Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack should
>> handle any alignment, and do the appropriate memove if the CPU requires
>> alignment.
>>
>
> Luckily all sky2 users have been on x86 so far :)
>
> Here's an idea. Put the data of the packet into the page frags
> where alignment is not an issue but copy the header so that it
> is aligned.
>
> Would that work?
>
> Cheers,
>
No too wasteful. I'm working a patch to eth_type_trans to realign if
needed for any device.

2007-11-30 00:29:02

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
> On Thu, Nov 29, 2007 at 09:50:35AM -0800, H. Peter Anvin wrote:
>> Uhm, most cards affected *ARE* Ethernet cards, due to the bloody 14-byte
>> header.
>
> Well most Ethernet drivers are using NET_IP_ALIGN which means that IP
> stack gets aligned packets only.
>
> sky2 is the exception here, not the rule.
>

It is, but it's not unique. Several USB adapters have the same problem,
for example.

-hpa

2007-11-25 13:54:42

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> > Not sure. On the one hand, yeah, that's something we should probably do,
> > on the other hand this will suck because for most drivers either nothing
> > needs to be done or the fixup is trivial. I suppose we should do this
> > but stick in a WARN_ON_ONCE() or something, at least with mac80211 debug
> > enabled.
>
> I don't see how you can get a WARN_ON to work when you're expecting
> there to be unaligned packets from time to time as the hardware
> header changes.

Well if the hardware header changes and the hardware is dumb enough not
to do padding I'd expect the driver to fix that up so I don't penalise
hardware that gets it correct.

> > Also, we do plan to run these things on rather smallish embedded devices
> > like APs that receive a lot of frames from many stations, and with 11n
> > we're pushing speeds up by quite a bit. I'm wary of putting more code
> > into the generic receive path.
>
> Well you don't have a choice if the hardware header is really
> unpredictable. It's either that or we go and modify the entire
> IP stack which penalises all the high-speed Ethernet NICs that
> already get the alignment correctly.

But I do have a choice where to fix it up and I'd prefer the drivers to
do it where necessary. For that, the warning would work because it'd
show driver authors that they need to fix something.

> Here's an idea. Even if you can't predict the header length of
> all packets, can you at least predict the header length of the
> majority of data (ones carrying IP etc.) packets?
>
> If so then you can do the skb_reserve based on that and the fix-up
> in the wireless core would be minimised.
>
> Since I know next to nothing about the wireless transport layer,
> one of you experts will need to tell me whether such a prediction
> could work :)

Hmm. I don't think so. Take an AP for example. It gets a lot of packets
from stations. Now, if you're not QoS capable then all is well. But i
you are and some stations are as well then all those stations send QoS
packets (+2 bytes). Or take an AP connected via wireless (WPS), WPS has
+6 bytes so I get all incoming upstream traffic with such unaligned
headers.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-24 14:13:30

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sat, Nov 24, 2007 at 02:49:36PM +0100, Johannes Berg wrote:
>
> Right. I just didn't think that would be a valid value for an
> architecture to set.

OK. Let me clarify this a bit more. We require at least one
of the following rules to be met:

* the IPv4/IPv6 header is aligned by 8 bytes on reception;
* or the platform provides unaligned exception handlers.

So if your platform violates both rules then it won't work with
the IP stack, simple as that. Fortunately I don't think such a
platform exists currently on Linux.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-25 01:08:50

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Sat, Nov 24, 2007 at 12:11:08PM -0800, Stephen Hemminger wrote:
>
> Then what about hardware that can't dma ethernet to non-aligned address.
> Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack should
> handle any alignment, and do the appropriate memove if the CPU requires
> alignment.

Luckily all sky2 users have been on x86 so far :)

Here's an idea. Put the data of the packet into the page frags
where alignment is not an issue but copy the header so that it
is aligned.

Would that work?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-27 18:40:57

by Stephen Hemminger

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Tue, 27 Nov 2007 09:16:07 -0800
"H. Peter Anvin" <[email protected]> wrote:

> Stephen Hemminger wrote:
> > Herbert Xu wrote:
> >> On Sat, Nov 24, 2007 at 02:49:36PM +0100, Johannes Berg wrote:
> >>
> >>> Right. I just didn't think that would be a valid value for an
> >>> architecture to set.
> >>>
> >>
> >> OK. Let me clarify this a bit more. We require at least one
> >> of the following rules to be met:
> >>
> >> * the IPv4/IPv6 header is aligned by 8 bytes on reception;
> >> * or the platform provides unaligned exception handlers.
> >>
> >> So if your platform violates both rules then it won't work with
> >> the IP stack, simple as that. Fortunately I don't think such a
> >> platform exists currently on Linux.
> >>
> >> Cheers,
> >>
> >
> > Then what about hardware that can't dma ethernet to non-aligned address.
> > Sky2 hardware breaks if DMA is not 8 byte aligned. IMHO the IP stack
> > should handle any alignment, and do the appropriate memove if the CPU requires
> > alignment.
>
> I wrote a patch for the IP stack to realign packets if necessary at one
> point. I should dredge it up again and submit it for collective flamage.
>
> -hpa
>

Is there any standard kernel config define for "this platform can't do
unaligned accesses"?

--
Stephen Hemminger <[email protected]>

2007-11-24 13:49:47

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


On Sat, 2007-11-24 at 21:32 +0800, Herbert Xu wrote:
> On Sat, Nov 24, 2007 at 09:33:36AM +0100, Johannes Berg wrote:
> >
> > We still require four-byte alignment, no?
>
> Not at all. If NET_IP_ALIGN is zero then it won't be four-byte
> aligned (since the Ethernet header is 14 bytes long).

Right. I just didn't think that would be a valid value for an
architecture to set.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-29 17:51:37

by H. Peter Anvin

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Herbert Xu wrote:
> On Tue, Nov 27, 2007 at 09:16:07AM -0800, H. Peter Anvin wrote:
>> I wrote a patch for the IP stack to realign packets if necessary at one
>> point. I should dredge it up again and submit it for collective flamage.
>
> As long as it doesn't penalise Ethernet (e.g., the 10Gb crowd :) it would
> be good to have.
>
> Thanks,

Uhm, most cards affected *ARE* Ethernet cards, due to the bloody 14-byte
header.

But it doesn't affect architectures which have alignment, and the cost
of scanning a properly-aligned packet is minimal.

I'll try to find it some time today.

-hpa

2007-11-25 11:00:42

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> Sorry I was wrong about the 8 bytes requirement. Although the
> IPv6 protocol does try to maintain an 8-byte alignment the Linux
> stack never does anything that requires that.
>
> So 4 bytes is enough.

Whew. Good to hear.

> However, the wireless core is definitely not out of the woods.
> It needs to support variable hardware header lengths that are
> not always a multiple of 4.
>
> So here's my suggestion. Modify the wireless core to fix up any
> packets which aren't aligned correctly. That should make it
> work albeit in a way that's less than optimal.

Not sure. On the one hand, yeah, that's something we should probably do,
on the other hand this will suck because for most drivers either nothing
needs to be done or the fixup is trivial. I suppose we should do this
but stick in a WARN_ON_ONCE() or something, at least with mac80211 debug
enabled.

Also, we do plan to run these things on rather smallish embedded devices
like APs that receive a lot of frames from many stations, and with 11n
we're pushing speeds up by quite a bit. I'm wary of putting more code
into the generic receive path.

> Then for each driver where you care about this performance
> (seriously I wouldn't for the speeds these things run at :),
> pick the most common wireless hardware header length and have
> the IP (or any other upper-level protocol) header aligned to
> at least 4 bytes. Or better if you know what hardware header
> length that you're going to get (e.g., based on what mode you're
> in) then do the skb_reserve accordingly.

Well, you can receive non-QoS frames even in QoS or WPS frames in any
other mode etc. so you can't really make any promises as which will be
more common. Bu in practice all (sane) hardware makes sure things are
aligned properly.

> It's a good thing these things aren't running at 10Gb :)

For sure!

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-24 06:15:27

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

Johannes Berg <[email protected]> wrote:
>
> Now, the IP stack actually assumes that its header is four-byte aligned
> (see comment at NET_IP_ALIGN, although it is not said explicitly that
> the alignment requirement for an IP header is four) so that is actually
> something for the hardware/firmware (!) to do, for example Broadcom

Good point. In fact IIRC we've always had the policy that drivers
should do their best to generate aligned packets but it is not a
requirement since on some platforms it's more important for the DMA
to be aligned.

So it's up the platform code to fix up any exceptions should they
show up.

Daniel, what's the specific case that you had in mind with this
patch?

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2007-11-25 17:04:42

by Johannes Berg

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements


> > Hmm. I don't think so. Take an AP for example. It gets a lot of packets
> > from stations. Now, if you're not QoS capable then all is well. But i
> > you are and some stations are as well then all those stations send QoS
> > packets (+2 bytes). Or take an AP connected via wireless (WPS), WPS has
> > +6 bytes so I get all incoming upstream traffic with such unaligned
> > headers.
>
> The question is does this actually change all the time. Let's
> say you took a random sample of a second worth of IP packets
> over wireless, what proportion of them are going to have the
> same hardware header length modulo 4?

I'd think that totally depends on the traffic. If you have a non-QoS AP
with WPS upstream connection, then the traffic to stations will be
four-byte aligned while the WPS upstream will be at a 2-byte-mod-4
boundary. And you'll have all packets from stations come in aligned and
all response packets from wherever come in as WPS.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-11-30 00:35:30

by Herbert Xu

[permalink] [raw]
Subject: Re: wireless vs. alignment requirements

On Thu, Nov 29, 2007 at 04:28:34PM -0800, H. Peter Anvin wrote:
>
> >sky2 is the exception here, not the rule.
>
> It is, but it's not unique. Several USB adapters have the same problem,
> for example.

Notice the common theme here that slow (or slower, i.e., certainly nowhere
near 10Gb) NICs are the norm for violating alignment :)

So I'd prefer something that only penalised them rather than everybody
else.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt