Re: [PATCH 1/8] net: qualcomm: rmnet: fix struct rmnet_map_header

From: Subash Abhinov Kasiviswanathan
Date: Mon May 20 2019 - 21:34:59 EST


If the above illustration is supposed to be in network byte order,
then it is wrong. The documentation has the definition for the MAP
packet.

Network *bit* order is irrelevant to the host. Host memory is
byte addressable only, and bit 0 is the low-order bit.

Packet format -

BitÂÂÂÂÂÂÂÂÂÂÂÂ 0ÂÂÂÂÂÂÂÂÂÂÂÂ 1ÂÂÂÂÂÂÂÂÂÂ 2-7ÂÂÂÂÂ 8 - 15ÂÂÂÂÂÂÂÂÂÂ 16 - 31
FunctionÂÂ Command / DataÂÂ ReservedÂÂÂÂ PadÂÂ Multiplexer IDÂÂÂ Payload length
BitÂÂÂÂÂÂÂÂÂÂÂ 32 - x
Function Raw Bytes

I don't know how to interpret this. Are you saying that the low-
order bit of the first byte is the command/data flag? Or is it
the high-order bit of the first byte?

I'm saying that what I observed when building the code was that
as originally defined, the cd_bit field was the high-order bit
(bit 7) of the first byte, which I understand to be wrong.

If you are telling me that the command/data flag resides at bit
7 of the first byte, I will update the field masks in a later
patch in this series to reflect that.


Higher order bit is Command / Data.

The driver was written assuming that the host was running ARM64, so
the structs are little endian. (I should have made it compatible
with big and little endian earlier so that is my fault).

Little endian and big endian have no bearing on the host's
interpretation of bits within a byte.

Please clarify. I want the patches to be correct, and what
you're explaining doesn't really straighten things out for me.

-Alex

Can't this bitfields just be used similar to how struct tcphdr &
iphdr are currently defined. That way, you dont have to make
these many changes.

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 884f1f5..302d1db 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -40,9 +40,17 @@ enum rmnet_map_commands {
};

struct rmnet_map_header {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
u8 pad_len:6;
u8 reserved_bit:1;
u8 cd_bit:1;
+#elif defined (__BIG_ENDIAN_BITFIELD)
+ u8 cd_bit:1;
+ u8 reserved_bit:1;
+ u8 pad_len:6;
+#else
+#error "Please fix <asm/byteorder.h>"
+#endif
u8 mux_id;
__be16 pkt_len;
} __aligned(1);


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project