The IPA driver requires some (but not all) symbols defined in
"drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h". Create a new
public header file "include/soc/qcom/rmnet.h" and move the needed
definitions there.
Signed-off-by: Alex Elder <[email protected]>
---
.../ethernet/qualcomm/rmnet/rmnet_handlers.c | 1 +
.../net/ethernet/qualcomm/rmnet/rmnet_map.h | 24 ------------
.../qualcomm/rmnet/rmnet_map_command.c | 1 +
.../ethernet/qualcomm/rmnet/rmnet_map_data.c | 1 +
.../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
include/soc/qcom/rmnet.h | 38 +++++++++++++++++++
6 files changed, 42 insertions(+), 24 deletions(-)
create mode 100644 include/soc/qcom/rmnet.h
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 11167abe5934..3aa79b7ed539 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -17,6 +17,7 @@
#include <linux/netdev_features.h>
#include <linux/if_arp.h>
#include <net/sock.h>
+#include <soc/qcom/rmnet.h>
#include "rmnet_private.h"
#include "rmnet_config.h"
#include "rmnet_vnd.h"
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 884f1f52dcc2..39d0be99a771 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -39,30 +39,6 @@ enum rmnet_map_commands {
RMNET_MAP_COMMAND_ENUM_LENGTH
};
-struct rmnet_map_header {
- u8 pad_len:6;
- u8 reserved_bit:1;
- u8 cd_bit:1;
- u8 mux_id;
- __be16 pkt_len;
-} __aligned(1);
-
-struct rmnet_map_dl_csum_trailer {
- u8 reserved1;
- u8 valid:1;
- u8 reserved2:7;
- u16 csum_start_offset;
- u16 csum_length;
- __be16 csum_value;
-} __aligned(1);
-
-struct rmnet_map_ul_csum_header {
- __be16 csum_start_offset;
- u16 csum_insert_offset:14;
- u16 udp_ip4_ind:1;
- u16 csum_enabled:1;
-} __aligned(1);
-
#define RMNET_MAP_GET_MUX_ID(Y) (((struct rmnet_map_header *) \
(Y)->data)->mux_id)
#define RMNET_MAP_GET_CD_BIT(Y) (((struct rmnet_map_header *) \
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
index f6cf59aee212..54b86a8be570 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
@@ -11,6 +11,7 @@
*/
#include <linux/netdevice.h>
+#include <soc/qcom/rmnet.h>
#include "rmnet_config.h"
#include "rmnet_map.h"
#include "rmnet_private.h"
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 57a9c314a665..e3fb4035820c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -17,6 +17,7 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <net/ip6_checksum.h>
+#include <soc/qcom/rmnet.h>
#include "rmnet_config.h"
#include "rmnet_map.h"
#include "rmnet_private.h"
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index d11c16aeb19a..b8df36e827d4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -17,6 +17,7 @@
#include <linux/etherdevice.h>
#include <linux/if_arp.h>
#include <net/pkt_sched.h>
+#include <soc/qcom/rmnet.h>
#include "rmnet_config.h"
#include "rmnet_handlers.h"
#include "rmnet_private.h"
diff --git a/include/soc/qcom/rmnet.h b/include/soc/qcom/rmnet.h
new file mode 100644
index 000000000000..80dcd6e68c3d
--- /dev/null
+++ b/include/soc/qcom/rmnet.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2018-2019 Linaro Ltd.
+ */
+#ifndef _SOC_QCOM_RMNET_H_
+#define _SOC_QCOM_RMNET_H_
+
+#include <linux/types.h>
+
+/* Header structure that precedes packets in ETH_P_MAP protocol */
+struct rmnet_map_header {
+ u8 pad_len : 6;
+ u8 reserved_bit : 1;
+ u8 cd_bit : 1;
+ u8 mux_id;
+ __be16 pkt_len;
+} __aligned(1);
+
+/* Checksum offload metadata header for outbound packets*/
+struct rmnet_map_ul_csum_header {
+ __be16 csum_start_offset;
+ u16 csum_insert_offset : 14;
+ u16 udp_ip4_ind : 1;
+ u16 csum_enabled : 1;
+} __aligned(1);
+
+/* Checksum offload metadata trailer for inbound packets */
+struct rmnet_map_dl_csum_trailer {
+ u8 reserved1;
+ u8 valid : 1;
+ u8 reserved2 : 7;
+ u16 csum_start_offset;
+ u16 csum_length;
+ __be16 csum_value;
+} __aligned(1);
+
+#endif /* _SOC_QCOM_RMNET_H_ */
--
2.20.1
On Sat, 2019-05-11 at 20:24 -0500, Alex Elder wrote:
> include/soc/qcom/rmnet.h
Should this file be added to the MAINTAINERS file
update in patch 16/18 ?
On 5/11/19 9:34 PM, Joe Perches wrote:
> On Sat, 2019-05-11 at 20:24 -0500, Alex Elder wrote:
>> include/soc/qcom/rmnet.h
>
> Should this file be added to the MAINTAINERS file
> update in patch 16/18 ?
Sure, that's a good point. I'll add it when I submit a v2.
Thank you.
-Alex
On Sun, May 12, 2019 at 3:25 AM Alex Elder <[email protected]> wrote:
> diff --git a/include/soc/qcom/rmnet.h b/include/soc/qcom/rmnet.h
> new file mode 100644
> index 000000000000..80dcd6e68c3d
> --- /dev/null
> +++ b/include/soc/qcom/rmnet.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2018-2019 Linaro Ltd.
> + */
> +#ifndef _SOC_QCOM_RMNET_H_
> +#define _SOC_QCOM_RMNET_H_
> +
> +#include <linux/types.h>
> +
> +/* Header structure that precedes packets in ETH_P_MAP protocol */
> +struct rmnet_map_header {
> + u8 pad_len : 6;
> + u8 reserved_bit : 1;
> + u8 cd_bit : 1;
> + u8 mux_id;
> + __be16 pkt_len;
> +} __aligned(1);
If we move this into include/soc/, I want the structure to be portable,
and avoid the bit fields. Please use mask/shift operations or the
include/linux/bits.h macros instead to make this work with big-endian
kernels.
Arnd
On 5/15/19 1:59 AM, Arnd Bergmann wrote:
> On Sun, May 12, 2019 at 3:25 AM Alex Elder <[email protected]> wrote:
>
>> diff --git a/include/soc/qcom/rmnet.h b/include/soc/qcom/rmnet.h
>> new file mode 100644
>> index 000000000000..80dcd6e68c3d
>> --- /dev/null
>> +++ b/include/soc/qcom/rmnet.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
>> + * Copyright (C) 2018-2019 Linaro Ltd.
>> + */
>> +#ifndef _SOC_QCOM_RMNET_H_
>> +#define _SOC_QCOM_RMNET_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* Header structure that precedes packets in ETH_P_MAP protocol */
>> +struct rmnet_map_header {
>> + u8 pad_len : 6;
>> + u8 reserved_bit : 1;
>> + u8 cd_bit : 1;
>> + u8 mux_id;
>> + __be16 pkt_len;
>> +} __aligned(1);
>
> If we move this into include/soc/, I want the structure to be portable,
> and avoid the bit fields. Please use mask/shift operations or the
> include/linux/bits.h macros instead to make this work with big-endian
> kernels.
Sure, I'll do that. I did that everywhere else in the driver,
but here I just tried to preserve the original code as I moved
it. I will update at least these structures, and all existing
code (plus the IPA code) to use fields masks.
-Alex
>
> Arnd
>
>>> +#ifndef _SOC_QCOM_RMNET_H_
>>> +#define _SOC_QCOM_RMNET_H_
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +/* Header structure that precedes packets in ETH_P_MAP protocol */
>>> +struct rmnet_map_header {
>>> + u8 pad_len : 6;
>>> + u8 reserved_bit : 1;
>>> + u8 cd_bit : 1;
>>> + u8 mux_id;
>>> + __be16 pkt_len;
>>> +} __aligned(1);
>>
>> If we move this into include/soc/, I want the structure to be
>> portable,
>> and avoid the bit fields. Please use mask/shift operations or the
>> include/linux/bits.h macros instead to make this work with big-endian
>> kernels.
>
> Sure, I'll do that. I did that everywhere else in the driver,
> but here I just tried to preserve the original code as I moved
> it. I will update at least these structures, and all existing
> code (plus the IPA code) to use fields masks.
>
> -Alex
>>
>> Arnd
>>
Hi Alex
Could we instead have the rmnet header definition in
include/linux/if_rmnet.h
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 5/15/19 8:09 PM, Subash Abhinov Kasiviswanathan wrote:
. . .
> Hi Alex
>
> Could we instead have the rmnet header definition in
> include/linux/if_rmnet.h
I have no objection to that, but I don't actually know what
the criteria are for putting a file in that directory.
Glancing at other "if_*" files there it seems sensible, but
because I don't know, I'd like to have a little better
justification.
Can you provide a good explanation about why these
definitions belong in "include/linux/if_rmnet.h" instead
of "include/soc/qcom/rmnet.h"?
Thanks.
-Alex
On 2019-05-17 11:27, Alex Elder wrote:
> On 5/15/19 8:09 PM, Subash Abhinov Kasiviswanathan wrote:
> . . .
>> Hi Alex
>>
>> Could we instead have the rmnet header definition in
>> include/linux/if_rmnet.h
>
> I have no objection to that, but I don't actually know what
> the criteria are for putting a file in that directory.
>
> Glancing at other "if_*" files there it seems sensible, but
> because I don't know, I'd like to have a little better
> justification.
>
> Can you provide a good explanation about why these
> definitions belong in "include/linux/if_rmnet.h" instead
> of "include/soc/qcom/rmnet.h"?
>
> Thanks.
>
> -Alex
rmnet was designed similar to vlan / macvlan / ipvlan / bridge.
These drivers support creation of virtual netdevices,
define custom rtnl_link_ops, expose netlink attributes to
uapi via if_link.h and register rx_handlers.
They expose some common structs and helpers via if_vlan.h /
if_macvlan.h / if_bridge.h. I would prefer rmnet to use if_rmnet.h
similar to them.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 5/17/19 1:08 PM, Subash Abhinov Kasiviswanathan wrote:
> On 2019-05-17 11:27, Alex Elder wrote:
. . .
>> Can you provide a good explanation about why these
>> definitions belong in "include/linux/if_rmnet.h" instead
>> of "include/soc/qcom/rmnet.h"?
>>
>> Thanks.
>>
>> -Alex
>
> rmnet was designed similar to vlan / macvlan / ipvlan / bridge.
> These drivers support creation of virtual netdevices,
> define custom rtnl_link_ops, expose netlink attributes to
> uapi via if_link.h and register rx_handlers.
>
> They expose some common structs and helpers via if_vlan.h /
> if_macvlan.h / if_bridge.h. I would prefer rmnet to use if_rmnet.h
> similar to them.
OK, I will name the file "include/linux/if_rmnet.h" as you suggest.
It will still only define the three structures that I need in the
IPA driver; I won't expose anything else from the rmnet_data driver.
I will mention now that, to facilitate addressing Arnd's concerns
about the portability of using C bit-fields in these structures,
I made a set of other changes (including a bug fix in one of
the structure definitions). As a preview, here are the subject
lines for that series:
net: qualcomm: rmnet: fix struct rmnet_map_header
net: qualcomm: rmnet: kill RMNET_MAP_GET_*() accessor macros
net: qualcomm: rmnet: use field masks instead of C bit-fields
net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum header
net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum trailer
soc: qcom: ipa: get rid of a variable in rmnet_map_ipv4_ul_csum_header()
net: create "include/linux/if_rmnet.h"
I will be posting that as a separate series now and will have
the IPA driver series mention a dependence on that.
-Alex