* [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs
@ 2014-12-21 18:18 Amir Vadai
2014-12-21 19:53 ` Sergei Shtylyov
2015-01-05 9:46 ` David Laight
0 siblings, 2 replies; 4+ messages in thread
From: Amir Vadai @ 2014-12-21 18:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Yevgeny Petrilin, Amir Vadai, Wei Yang,
David Laight
iowrite32() will byteswap it's argument on big endian archs.
iowrite32be() will byteswap on little endian archs.
Since we don't want to do this unnecessary byteswap on the fast path,
doorbell is stored in the NIC's native endianness. Using the right
iowrite() according to the arch endianness.
CC: Wei Yang <weiyang@linux.vnet.ibm.com>
CC: David Laight <david.laight@aculab.com>
Fixes: 6a4e812 ("net/mlx4_en: Avoid calling bswap in tx fast path")
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index a308d41..6477cc7 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -962,7 +962,16 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
tx_desc->ctrl.owner_opcode = op_own;
if (send_doorbell) {
wmb();
- iowrite32(ring->doorbell_qpn,
+ /* Since there is no iowrite*_native() that writes the value
+ * as is, without byteswapping - using the one the doesn't do
+ * byteswapping in the relevant arch endianness.
+ */
+#if defined(__LITTLE_ENDIAN)
+ iowrite32(
+#else
+ iowrite32be(
+#endif
+ ring->doorbell_qpn,
ring->bf.uar->map + MLX4_SEND_DOORBELL);
} else {
ring->xmit_more++;
--
1.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs
2014-12-21 18:18 [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs Amir Vadai
@ 2014-12-21 19:53 ` Sergei Shtylyov
2014-12-22 8:17 ` Amir Vadai
2015-01-05 9:46 ` David Laight
1 sibling, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2014-12-21 19:53 UTC (permalink / raw)
To: Amir Vadai, David S. Miller
Cc: netdev, Or Gerlitz, Yevgeny Petrilin, Wei Yang, David Laight
Hello.
On 12/21/2014 9:18 PM, Amir Vadai wrote:
> iowrite32() will byteswap it's argument on big endian archs.
> iowrite32be() will byteswap on little endian archs.
> Since we don't want to do this unnecessary byteswap on the fast path,
> doorbell is stored in the NIC's native endianness. Using the right
> iowrite() according to the arch endianness.
> CC: Wei Yang <weiyang@linux.vnet.ibm.com>
> CC: David Laight <david.laight@aculab.com>
> Fixes: 6a4e812 ("net/mlx4_en: Avoid calling bswap in tx fast path")
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index a308d41..6477cc7 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> @@ -962,7 +962,16 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
> tx_desc->ctrl.owner_opcode = op_own;
> if (send_doorbell) {
> wmb();
> - iowrite32(ring->doorbell_qpn,
> + /* Since there is no iowrite*_native() that writes the value
> + * as is, without byteswapping - using the one the doesn't do
> + * byteswapping in the relevant arch endianness.
> + */
Why the comment is not aligned with the code?
> +#if defined(__LITTLE_ENDIAN)
> + iowrite32(
> +#else
> + iowrite32be(
> +#endif
Ugh...
> + ring->doorbell_qpn,
> ring->bf.uar->map + MLX4_SEND_DOORBELL);
[...]
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs
2014-12-21 19:53 ` Sergei Shtylyov
@ 2014-12-22 8:17 ` Amir Vadai
0 siblings, 0 replies; 4+ messages in thread
From: Amir Vadai @ 2014-12-22 8:17 UTC (permalink / raw)
To: Sergei Shtylyov, David S. Miller
Cc: netdev@vger.kernel.org, Or Gerlitz, Yevgeny Petrilin, Wei Yang,
David Laight
On 12/21/2014 9:53 PM, Sergei Shtylyov wrote:
> Hello.
>
Hi,
> On 12/21/2014 9:18 PM, Amir Vadai wrote:
>
>> iowrite32() will byteswap it's argument on big endian archs.
>> iowrite32be() will byteswap on little endian archs.
>> Since we don't want to do this unnecessary byteswap on the fast path,
>> doorbell is stored in the NIC's native endianness. Using the right
>> iowrite() according to the arch endianness.
>
>> CC: Wei Yang <weiyang@linux.vnet.ibm.com>
>> CC: David Laight <david.laight@aculab.com>
>> Fixes: 6a4e812 ("net/mlx4_en: Avoid calling bswap in tx fast path")
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> index a308d41..6477cc7 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>> @@ -962,7 +962,16 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
>> tx_desc->ctrl.owner_opcode = op_own;
>> if (send_doorbell) {
>> wmb();
>> - iowrite32(ring->doorbell_qpn,
>> + /* Since there is no iowrite*_native() that writes the value
>> + * as is, without byteswapping - using the one the doesn't do
>> + * byteswapping in the relevant arch endianness.
>> + */
>
> Why the comment is not aligned with the code?
By mistake. Sending V1.
>
>> +#if defined(__LITTLE_ENDIAN)
>> + iowrite32(
>> +#else
>> + iowrite32be(
>> +#endif
>
> Ugh...
Yes I think so too, but there is no iowrite*_native(). I plan to send a
patch to add it, but meanwhile the driver is completely broken in little
endian archs - so it must be fixed now. And just reverting to the old
behavior of bswap in the fast path looks like a bad alternative too.
>
>> + ring->doorbell_qpn,
>> ring->bf.uar->map + MLX4_SEND_DOORBELL);
> [...]
>
> WBR, Sergei
>
Amir
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs
2014-12-21 18:18 [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs Amir Vadai
2014-12-21 19:53 ` Sergei Shtylyov
@ 2015-01-05 9:46 ` David Laight
1 sibling, 0 replies; 4+ messages in thread
From: David Laight @ 2015-01-05 9:46 UTC (permalink / raw)
To: 'Amir Vadai', David S. Miller
Cc: netdev@vger.kernel.org, Or Gerlitz, Yevgeny Petrilin, Wei Yang
> iowrite32() will byteswap it's argument on big endian archs.
> iowrite32be() will byteswap on little endian archs.
> Since we don't want to do this unnecessary byteswap on the fast path,
> doorbell is stored in the NIC's native endianness. Using the right
> iowrite() according to the arch endianness.
Someone seems to have forgotten that the byteswap within iowrite32()
on ppc (and more recent sparc) is almost certainly 'free'.
So the value ought to be stored in the byte order required by iowrite32().
David
> Fixes: 6a4e812 ("net/mlx4_en: Avoid calling bswap in tx fast path")
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index a308d41..6477cc7 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> @@ -962,7 +962,16 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
> tx_desc->ctrl.owner_opcode = op_own;
> if (send_doorbell) {
> wmb();
> - iowrite32(ring->doorbell_qpn,
> + /* Since there is no iowrite*_native() that writes the value
> + * as is, without byteswapping - using the one the doesn't do
> + * byteswapping in the relevant arch endianness.
> + */
> +#if defined(__LITTLE_ENDIAN)
> + iowrite32(
> +#else
> + iowrite32be(
> +#endif
> + ring->doorbell_qpn,
> ring->bf.uar->map + MLX4_SEND_DOORBELL);
> } else {
> ring->xmit_more++;
> --
> 1.9.3
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-05 11:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-21 18:18 [PATCH net] net/mlx4_en: Doorbell is byteswapped in Little Endian archs Amir Vadai
2014-12-21 19:53 ` Sergei Shtylyov
2014-12-22 8:17 ` Amir Vadai
2015-01-05 9:46 ` David Laight
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).