Re: [PATCH v2] marvell sky2 driver: fix so it works without unalignedaccesses

From: Chris Metcalf
Date: Wed Apr 04 2012 - 17:52:43 EST


On 4/4/2012 2:42 PM, Stephen Hemminger wrote:
> On Wed, 4 Apr 2012 10:13:32 -0400
> Chris Metcalf <cmetcalf@xxxxxxxxxx> wrote:
>
>> The driver uses a receive_new() routine that ends up requiring unaligned
>> accesses in IP header processing. If the architecture doesn't support
>> efficient unaligned accesses, and SKY2_HW_RAM_BUFFER is set,
>> just copy all ingress packets to the bounce buffers instead.
>> Thanks to Eric Dumazet for pointing out the SKY2_HW_RAM_BUFFER issue.
>>
>> This allows the driver to be used on the Tilera TILEmpower-Gx, since
>> the tile architecture doesn't currently handle kernel unaligned accesses,
>> just userspace.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf@xxxxxxxxxx>
>> ---
>> drivers/net/ethernet/marvell/sky2.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
> What about the following (compile tested only)
> --- a/drivers/net/ethernet/marvell/sky2.c 2012-04-04 08:49:05.954853108 -0700
> +++ b/drivers/net/ethernet/marvell/sky2.c 2012-04-04 11:30:54.201432707 -0700
> @@ -2468,6 +2468,17 @@ static int sky2_change_mtu(struct net_de
> return err;
> }
>
> +static inline bool needs_copy(const struct rx_ring_info *re,
> + unsigned length)
> +{
> +#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> + /* Some architectures need the IP header to be aligned */
> + if (!IS_ALIGNED(re->data_addr + ETH_HLEN, sizeof(u32)))
> + return 1;
> +#endif
> + return length < copybreak;
> +}
> +
> /* For small just reuse existing skb for next receive */
> static struct sk_buff *receive_copy(struct sky2_port *sky2,
> const struct rx_ring_info *re,
> @@ -2605,7 +2616,7 @@ static struct sk_buff *sky2_receive(stru
> goto error;
>
> okay:
> - if (length < copybreak)
> + if (needs_copy(re, length))
> skb = receive_copy(sky2, re, length);
> else
> skb = receive_new(sky2, re, length);

Acked-by: Chris Metcalf <cmetcalf@xxxxxxxxxx>

This works on our TILEmpower-Gx boxes and seems like it's likely more
efficient. I believe kernel style says you should "return true" rather
than "return 1" from a "bool" routine, though.

Thanks!

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/