2021-02-23 20:59:22

by Sharath Chandra Vurukala

[permalink] [raw]
Subject: [PATCH net-next v3 0/3] net: qualcomm: rmnet: Enable Mapv5

This series introduces the MAPv5 packet format.

Patch 0 documents the MAPv5.
Patch 1 introduces the Mapv5 and the Inline checksum offload for RX.
Patch 2 introduces the Mapv5 and the Inline checksum offload for TX.

A new checksum header format is used as part of MAPv5.
For RX checksum offload, the checksum is verified by the HW and the
validity is marked in the checksum header of MAPv5.
for TX, the required metadata is filled up so hardware can compute the
checksum.

v1->v2:
- Fixed the compilation errors, warnings reported by kernel test robot.
- Checksum header definition is expanded to support big, little endian
formats as mentioned by Jakub.

v2->v3:
- Fixed compilation errors reported by kernel bot for big endian flavor.

Sharath Chandra Vurukala (3):
docs: networking: Add documentation for MAPv5
net: ethernet: rmnet: Support for downlink MAPv5 checksum offload
net: ethernet: rmnet: Add support for Mapv5 uplink packet

.../device_drivers/cellular/qualcomm/rmnet.rst | 53 ++++++-
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 34 +++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 22 ++-
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 153 ++++++++++++++++++++-
include/linux/if_rmnet.h | 24 +++-
include/uapi/linux/if_link.h | 2 +
7 files changed, 263 insertions(+), 29 deletions(-)

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


2021-02-23 21:03:10

by Sharath Chandra Vurukala

[permalink] [raw]
Subject: [PATCH net-next v3 2/3] net: ethernet: rmnet: Support for downlink MAPv5 checksum offload

Adding support for processing of Mapv5 downlink packets.
It involves parsing the Mapv5 packet and checking the csum header
to know whether the hardware has validated the checksum and is
valid or not.

Based on the checksum valid bit the corresponding stats are
incremented and skb->ip_summed is marked either CHECKSUM_UNNECESSARY
or left as CHEKSUM_NONE to let network stack revalidated the checksum
and update the respective snmp stats.

Current MapV1 header has been modified, the reserved field in the
Mapv1 header is now used for next header indication.

Signed-off-by: Sharath Chandra Vurukala <[email protected]>
---
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 19 ++++---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 14 ++++-
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 60 +++++++++++++++++++++-
include/linux/if_rmnet.h | 24 +++++++--
include/uapi/linux/if_link.h | 1 +
5 files changed, 106 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 3d7d3ab..70ad6a7 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
*
* RMNET Data ingress/egress handler
*/
@@ -57,8 +57,8 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
struct rmnet_port *port)
{
struct rmnet_endpoint *ep;
+ u8 mux_id, next_hdr;
u16 len, pad;
- u8 mux_id;

if (RMNET_MAP_GET_CD_BIT(skb)) {
if (port->data_format & RMNET_FLAGS_INGRESS_MAP_COMMANDS)
@@ -70,6 +70,7 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
mux_id = RMNET_MAP_GET_MUX_ID(skb);
pad = RMNET_MAP_GET_PAD(skb);
len = RMNET_MAP_GET_LENGTH(skb) - pad;
+ next_hdr = RMNET_MAP_GET_NH_BIT(skb);

if (mux_id >= RMNET_MAX_LOGICAL_EP)
goto free_skb;
@@ -80,15 +81,19 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,

skb->dev = ep->egress_dev;

- /* Subtract MAP header */
- skb_pull(skb, sizeof(struct rmnet_map_header));
- rmnet_set_skb_proto(skb);
-
- if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
+ if (next_hdr &&
+ (port->data_format & (RMNET_FLAGS_INGRESS_MAP_CKSUMV5))) {
+ if (rmnet_map_process_next_hdr_packet(skb, len))
+ goto free_skb;
+ } else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
skb->ip_summed = CHECKSUM_UNNECESSARY;
}

+ /* Subtract MAP header */
+ skb_pull(skb, sizeof(struct rmnet_map_header));
+ rmnet_set_skb_proto(skb);
+
skb_trim(skb, len);
rmnet_deliver_skb(skb);
return;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 576501d..2ee1ce2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
*/

#ifndef _RMNET_MAP_H_
@@ -23,6 +23,12 @@ struct rmnet_map_control_command {
};
} __aligned(1);

+enum rmnet_map_v5_header_type {
+ RMNET_MAP_HEADER_TYPE_UNKNOWN,
+ RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD = 0x2,
+ RMNET_MAP_HEADER_TYPE_ENUM_LENGTH
+};
+
enum rmnet_map_commands {
RMNET_MAP_COMMAND_NONE,
RMNET_MAP_COMMAND_FLOW_DISABLE,
@@ -44,6 +50,9 @@ enum rmnet_map_commands {
#define RMNET_MAP_GET_LENGTH(Y) (ntohs(((struct rmnet_map_header *) \
(Y)->data)->pkt_len))

+#define RMNET_MAP_GET_NH_BIT(Y) (((struct rmnet_map_header *) \
+ (Y)->data)->next_hdr)
+
#define RMNET_MAP_COMMAND_REQUEST 0
#define RMNET_MAP_COMMAND_ACK 1
#define RMNET_MAP_COMMAND_UNSUPPORTED 2
@@ -60,5 +69,8 @@ void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
struct net_device *orig_dev);
+int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
+u8 rmnet_map_get_next_hdr_type(struct sk_buff *skb);
+bool rmnet_map_get_csum_valid(struct sk_buff *skb);

#endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 21d3816..a3dc220 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
*
* RMNET Data MAP protocol
*/
@@ -311,6 +311,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
struct rmnet_port *port)
{
+ unsigned char *data = skb->data, *next_hdr = NULL;
struct rmnet_map_header *maph;
struct sk_buff *skbn;
u32 packet_len;
@@ -323,6 +324,12 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,

if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
+ else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
+ if (!maph->cd_bit) {
+ packet_len += sizeof(struct rmnet_map_v5_csum_header);
+ next_hdr = data + sizeof(*maph);
+ }
+ }

if (((int)skb->len - (int)packet_len) < 0)
return NULL;
@@ -331,6 +338,11 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
if (ntohs(maph->pkt_len) == 0)
return NULL;

+ if (next_hdr &&
+ ((struct rmnet_map_v5_csum_header *)next_hdr)->header_type !=
+ RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
+ return NULL;
+
skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
if (!skbn)
return NULL;
@@ -428,3 +440,49 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,

priv->stats.csum_sw++;
}
+
+u8 rmnet_map_get_next_hdr_type(struct sk_buff *skb)
+{
+ unsigned char *data = skb->data;
+
+ data += sizeof(struct rmnet_map_header);
+ return ((struct rmnet_map_v5_csum_header *)data)->header_type;
+}
+
+bool rmnet_map_get_csum_valid(struct sk_buff *skb)
+{
+ unsigned char *data = skb->data;
+
+ data += sizeof(struct rmnet_map_header);
+ return ((struct rmnet_map_v5_csum_header *)data)->csum_valid_required;
+}
+
+/* Process a MAPv5 packet header */
+int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
+ u16 len)
+{
+ struct rmnet_priv *priv = netdev_priv(skb->dev);
+ int rc = 0;
+
+ switch (rmnet_map_get_next_hdr_type(skb)) {
+ case RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD:
+ if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
+ priv->stats.csum_sw++;
+ } else if (rmnet_map_get_csum_valid(skb)) {
+ priv->stats.csum_ok++;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ } else {
+ priv->stats.csum_valid_unset++;
+ }
+
+ /* Pull csum v5 header */
+ skb_pull(skb, sizeof(struct rmnet_map_v5_csum_header));
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
index 9661416..004773f 100644
--- a/include/linux/if_rmnet.h
+++ b/include/linux/if_rmnet.h
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only
- * Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013-2019, 2021 The Linux Foundation. All rights reserved.
*/

#ifndef _LINUX_IF_RMNET_H_
@@ -8,11 +8,11 @@
struct rmnet_map_header {
#if defined(__LITTLE_ENDIAN_BITFIELD)
u8 pad_len:6;
- u8 reserved_bit:1;
+ u8 next_hdr:1;
u8 cd_bit:1;
#elif defined (__BIG_ENDIAN_BITFIELD)
u8 cd_bit:1;
- u8 reserved_bit:1;
+ u8 next_hdr:1;
u8 pad_len:6;
#else
#error "Please fix <asm/byteorder.h>"
@@ -52,4 +52,22 @@ struct rmnet_map_ul_csum_header {
#endif
} __aligned(1);

+/* MAP CSUM headers */
+struct rmnet_map_v5_csum_header {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ u8 next_hdr:1;
+ u8 header_type:7;
+ u8 hw_reserved:7;
+ u8 csum_valid_required:1;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+ u8 header_type:7;
+ u8 next_hdr:1;
+ u8 csum_valid_required:1;
+ u8 hw_reserved:7;
+#else
+#error "Please fix <asm/byteorder.h>"
+#endif
+ __be16 reserved;
+} __aligned(1);
+
#endif /* !(_LINUX_IF_RMNET_H_) */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 82708c6..838bd29 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1233,6 +1233,7 @@ enum {
#define RMNET_FLAGS_INGRESS_MAP_COMMANDS (1U << 1)
#define RMNET_FLAGS_INGRESS_MAP_CKSUMV4 (1U << 2)
#define RMNET_FLAGS_EGRESS_MAP_CKSUMV4 (1U << 3)
+#define RMNET_FLAGS_INGRESS_MAP_CKSUMV5 (1U << 4)

enum {
IFLA_RMNET_UNSPEC,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-02-23 23:50:24

by Sharath Chandra Vurukala

[permalink] [raw]
Subject: [PATCH net-next v3 1/3] docs: networking: Add documentation for MAPv5

Adding documentation explaining the new MAPv5 packet format
and the corresponding checksum offload header.

Signed-off-by: Sharath Chandra Vurukala <[email protected]>
---
.../device_drivers/cellular/qualcomm/rmnet.rst | 53 ++++++++++++++++++++--
1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
index 70643b5..8c81f19 100644
--- a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
+++ b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
@@ -27,7 +27,7 @@ these MAP frames and send them to appropriate PDN's.
2. Packet format
================

-a. MAP packet (data / control)
+a. MAP packet v1 (data / control)

MAP header has the same endianness of the IP packet.

@@ -35,8 +35,9 @@ 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
+
+ Bit 32-x
+ Function Raw bytes

Command (1)/ Data (0) bit value is to indicate if the packet is a MAP command
or data packet. Control packet is used for transport level flow control. Data
@@ -52,7 +53,51 @@ Multiplexer ID is to indicate the PDN on which data has to be sent.
Payload length includes the padding length but does not include MAP header
length.

-b. MAP packet (command specific)::
+b. MAP packet v5 (data / control)
+
+MAP header has the same endianness of the IP packet.
+
+Packet format::
+
+ Bit 0 1 2-7 8 - 15 16 - 31
+ Function Command / Data Next Header Pad Multiplexer ID Payload length
+
+ Bit 32 - 38 1 40 41-63
+ Function Header Type Next Header Checksum Valid Reserved
+
+ Bit 64-x
+ Function Raw bytes
+
+Command (1)/ Data (0) bit value is to indicate if the packet is a MAP command
+or data packet. Control packet is used for transport level flow control. Data
+packets are standard IP packets.
+
+Next header is used to indicate the presence of another header, currently is
+limited to checksum header.
+
+Padding is number of bytes to be added for 4 byte alignment if required by
+hardware.
+
+Multiplexer ID is to indicate the PDN on which data has to be sent.
+
+Payload length includes the padding length but does not include MAP header
+length.
+
+Header Type is to indicate the type of header, this usually is set to CHECKSUM
+
+Header types
+= ==========================================
+0 Reserved
+1 Reserved
+2 checksum header
+
+Checksum Valid is to indicate whether the header checksum is valid. Value of 1
+implies that checksum is calculated on this packet and is valid, value of 0
+indicates that the calculated packet checksum is invalid.
+
+Reserved bits are usually zeroed out and to be ignored by receiver.
+
+c. MAP packet v1/v5 (command specific)::

Bit 0 1 2-7 8 - 15 16 - 31
Function Command Reserved Pad Multiplexer ID Payload length
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-02-25 03:03:31

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 2/3] net: ethernet: rmnet: Support for downlink MAPv5 checksum offload

On Wed, 24 Feb 2021 01:32:50 +0530 Sharath Chandra Vurukala wrote:
> +/* MAP CSUM headers */
> +struct rmnet_map_v5_csum_header {
> +#if defined(__LITTLE_ENDIAN_BITFIELD)
> + u8 next_hdr:1;
> + u8 header_type:7;
> + u8 hw_reserved:7;
> + u8 csum_valid_required:1;
> +#elif defined(__BIG_ENDIAN_BITFIELD)
> + u8 header_type:7;
> + u8 next_hdr:1;
> + u8 csum_valid_required:1;
> + u8 hw_reserved:7;
> +#else
> +#error "Please fix <asm/byteorder.h>"
> +#endif
> + __be16 reserved;
> +} __aligned(1);

This seems to be your first contribution so let me spell it out.

In Linux when maintainers ask you to do something you are expected
to do it.

You can leave the existing bitfields for later, but don't add another.

2021-02-25 15:30:58

by Alex Elder

[permalink] [raw]
Subject: Re: [PATCH net-next v3 2/3] net: ethernet: rmnet: Support for downlink MAPv5 checksum offload

On 2/24/21 12:23 PM, Jakub Kicinski wrote:
> On Wed, 24 Feb 2021 01:32:50 +0530 Sharath Chandra Vurukala wrote:
>> +/* MAP CSUM headers */
>> +struct rmnet_map_v5_csum_header {
>> +#if defined(__LITTLE_ENDIAN_BITFIELD)
>> + u8 next_hdr:1;
>> + u8 header_type:7;
>> + u8 hw_reserved:7;
>> + u8 csum_valid_required:1;
>> +#elif defined(__BIG_ENDIAN_BITFIELD)
>> + u8 header_type:7;
>> + u8 next_hdr:1;
>> + u8 csum_valid_required:1;
>> + u8 hw_reserved:7;
>> +#else
>> +#error "Please fix <asm/byteorder.h>"
>> +#endif
>> + __be16 reserved;
>> +} __aligned(1);
>
> This seems to be your first contribution so let me spell it out.
>
> In Linux when maintainers ask you to do something you are expected
> to do it.
>
> You can leave the existing bitfields for later, but don't add another.

As I offered, I will implement changes to the existing
code to use masks in place of these C bit-fields.

I will try complete this within the next week. If it
looks good it could serve as an example of how to go
about it.

-Alex