* [PATCH net-next v4 1/3] docs: networking: Add documentation for MAPv5
2021-04-22 20:02 [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Sharath Chandra Vurukala
@ 2021-04-22 20:02 ` Sharath Chandra Vurukala
2021-04-23 12:32 ` Alex Elder
2021-04-22 20:02 ` [PATCH net-next v4 2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload Sharath Chandra Vurukala
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Sharath Chandra Vurukala @ 2021-04-22 20:02 UTC (permalink / raw)
To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
Cc: Sharath Chandra Vurukala
Adding documentation explaining the new MAPv4/v5 packet formats
and the corresponding checksum offload headers.
Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
.../device_drivers/cellular/qualcomm/rmnet.rst | 126 +++++++++++++++++++--
1 file changed, 114 insertions(+), 12 deletions(-)
diff --git a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
index 70643b5..4118384 100644
--- a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
+++ b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
@@ -27,34 +27,136 @@ 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.
+MAP header fields are in big endian format.
Packet format::
- Bit 0 1 2-7 8 - 15 16 - 31
+ 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
+or data packet. Command packet is used for transport level flow control. Data
packets are standard IP packets.
-Reserved bits are usually zeroed out and to be ignored by receiver.
+Reserved bits must be zero when sent and ignored when received.
-Padding is number of bytes to be added for 4 byte alignment if required by
-hardware.
+Padding is the number of bytes to be appended to the payload to
+ensure 4 byte alignment.
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 v4 (data / control)
+
+MAP header fields are in big endian format.
+
+Packet format::
+
+ Bit 0 1 2-7 8-15 16-31
+ Function Command / Data Reserved Pad Multiplexer ID Payload length
+
+ Bit 32-(x-33) (x-32)-x
+ Function Raw bytes Checksum offload header
+
+Command (1)/ Data (0) bit value is to indicate if the packet is a MAP command
+or data packet. Command packet is used for transport level flow control. Data
+packets are standard IP packets.
+
+Reserved bits must be zero when sent and ignored when received.
+
+Padding is the number of bytes to be appended to the payload to
+ensure 4 byte alignment.
+
+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.
+
+Checksum offload header, has the information about the checksum processing done
+by the hardware.Checksum offload header fields are in big endian format.
+
+Packet format::
+
+ Bit 0-14 15 16-31
+ Function Reserved Valid Checksum start offset
+
+ Bit 31-47 48-64
+ Function Checksum length Checksum value
+
+Reserved bits must be zero when sent and ignored when received.
+
+Valid bit indicates whether the partial checksum is calculated and is valid.
+Set to 1, if its is valid. Set to 0 otherwise.
+
+Padding is the number of bytes to be appended to the payload to
+ensure 4 byte alignment.
+
+Checksum start offset, Indicates the offset in bytes from the beginning of the
+IP header, from which modem computed checksum.
+
+Checksum length is the Length in bytes starting from CKSUM_START_OFFSET,
+over which checksum is computed.
+
+Checksum value, indicates the checksum computed.
+
+c. MAP packet v5 (data / control)
+
+MAP header fields are in big endian format.
+
+Packet format::
+
+ Bit 0 1 2-7 8-15 16-31
+ Function Command / Data Next header Pad Multiplexer ID Payload length
+
+ 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. Command 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 the number of bytes to be appended to the payload to
+ensure 4 byte alignment.
+
+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.
+
+d. Checksum offload header v5
+
+Checksum offload header fields are in big endian format.
+
+ Bit 0 - 6 7 8-15 16-31
+ Function Header Type Next Header Checksum Valid Reserved
+
+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 must be zero when sent and ignored when received.
+
+e. MAP packet v1/v5 (command specific)::
- Bit 0 1 2-7 8 - 15 16 - 31
+ Bit 0 1 2-7 8 - 15 16 - 31
Function Command Reserved Pad Multiplexer ID Payload length
Bit 32 - 39 40 - 45 46 - 47 48 - 63
Function Command name Reserved Command Type Reserved
@@ -74,7 +176,7 @@ Command types
3 is for error during processing of commands
= ==========================================
-c. Aggregation
+f. Aggregation
Aggregation is multiple MAP packets (can be data or command) delivered to
rmnet in a single linear skb. rmnet will process the individual
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next v4 1/3] docs: networking: Add documentation for MAPv5
2021-04-22 20:02 ` [PATCH net-next v4 1/3] docs: networking: Add documentation for MAPv5 Sharath Chandra Vurukala
@ 2021-04-23 12:32 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2021-04-23 12:32 UTC (permalink / raw)
To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
netdev, linux-kernel
On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
> Adding documentation explaining the new MAPv4/v5 packet formats
> and the corresponding checksum offload headers.
>
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
Thank you for updating this document, and in particular
for adding documentation for QMAPv4.
While I might suggest minor changes here or there to
wording, I think the document does a good enough job
of describing RMNet data structures.
My main interest right now is enabling inline checksum
offload functionality, so for now I'm content to just
acknowledge this patch.
Acked-by: Alex Elder <elder@linaro.org>
> ---
> .../device_drivers/cellular/qualcomm/rmnet.rst | 126 +++++++++++++++++++--
> 1 file changed, 114 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
> index 70643b5..4118384 100644
> --- a/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
> +++ b/Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
> @@ -27,34 +27,136 @@ 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.
> +MAP header fields are in big endian format.
>
> Packet format::
>
> - Bit 0 1 2-7 8 - 15 16 - 31
> + 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
> +or data packet. Command packet is used for transport level flow control. Data
> packets are standard IP packets.
>
> -Reserved bits are usually zeroed out and to be ignored by receiver.
> +Reserved bits must be zero when sent and ignored when received.
>
> -Padding is number of bytes to be added for 4 byte alignment if required by
> -hardware.
> +Padding is the number of bytes to be appended to the payload to
> +ensure 4 byte alignment.
>
> 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 v4 (data / control)
> +
> +MAP header fields are in big endian format.
> +
> +Packet format::
> +
> + Bit 0 1 2-7 8-15 16-31
> + Function Command / Data Reserved Pad Multiplexer ID Payload length
> +
> + Bit 32-(x-33) (x-32)-x
> + Function Raw bytes Checksum offload header
> +
> +Command (1)/ Data (0) bit value is to indicate if the packet is a MAP command
> +or data packet. Command packet is used for transport level flow control. Data
> +packets are standard IP packets.
> +
> +Reserved bits must be zero when sent and ignored when received.
> +
> +Padding is the number of bytes to be appended to the payload to
> +ensure 4 byte alignment.
> +
> +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.
> +
> +Checksum offload header, has the information about the checksum processing done
> +by the hardware.Checksum offload header fields are in big endian format.
> +
> +Packet format::
> +
> + Bit 0-14 15 16-31
> + Function Reserved Valid Checksum start offset
> +
> + Bit 31-47 48-64
> + Function Checksum length Checksum value
> +
> +Reserved bits must be zero when sent and ignored when received.
> +
> +Valid bit indicates whether the partial checksum is calculated and is valid.
> +Set to 1, if its is valid. Set to 0 otherwise.
> +
> +Padding is the number of bytes to be appended to the payload to
> +ensure 4 byte alignment.
> +
> +Checksum start offset, Indicates the offset in bytes from the beginning of the
> +IP header, from which modem computed checksum.
> +
> +Checksum length is the Length in bytes starting from CKSUM_START_OFFSET,
> +over which checksum is computed.
> +
> +Checksum value, indicates the checksum computed.
> +
> +c. MAP packet v5 (data / control)
> +
> +MAP header fields are in big endian format.
> +
> +Packet format::
> +
> + Bit 0 1 2-7 8-15 16-31
> + Function Command / Data Next header Pad Multiplexer ID Payload length
> +
> + 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. Command 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 the number of bytes to be appended to the payload to
> +ensure 4 byte alignment.
> +
> +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.
> +
> +d. Checksum offload header v5
> +
> +Checksum offload header fields are in big endian format.
> +
> + Bit 0 - 6 7 8-15 16-31
> + Function Header Type Next Header Checksum Valid Reserved
> +
> +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 must be zero when sent and ignored when received.
> +
> +e. MAP packet v1/v5 (command specific)::
>
> - Bit 0 1 2-7 8 - 15 16 - 31
> + Bit 0 1 2-7 8 - 15 16 - 31
> Function Command Reserved Pad Multiplexer ID Payload length
> Bit 32 - 39 40 - 45 46 - 47 48 - 63
> Function Command name Reserved Command Type Reserved
> @@ -74,7 +176,7 @@ Command types
> 3 is for error during processing of commands
> = ==========================================
>
> -c. Aggregation
> +f. Aggregation
>
> Aggregation is multiple MAP packets (can be data or command) delivered to
> rmnet in a single linear skb. rmnet will process the individual
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v4 2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload
2021-04-22 20:02 [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Sharath Chandra Vurukala
2021-04-22 20:02 ` [PATCH net-next v4 1/3] docs: networking: Add documentation for MAPv5 Sharath Chandra Vurukala
@ 2021-04-22 20:02 ` Sharath Chandra Vurukala
2021-04-23 12:32 ` Alex Elder
2021-04-22 20:02 ` [PATCH net-next v4 3/3] net: ethernet: rmnet: Add support for MAPv5 egress packets Sharath Chandra Vurukala
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Sharath Chandra Vurukala @ 2021-04-22 20:02 UTC (permalink / raw)
To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
Cc: Sharath Chandra Vurukala
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 revalidate 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 <sharathv@codeaurora.org>
---
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 17 ++++---
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 3 +-
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 58 +++++++++++++++++++++-
include/linux/if_rmnet.h | 27 +++++++++-
include/uapi/linux/if_link.h | 1 +
5 files changed, 97 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 0be5ac7..706a225 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
*/
@@ -82,11 +82,16 @@ __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 ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
+ (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
+ if (rmnet_map_process_next_hdr_packet(skb, len))
+ goto free_skb;
+ skb_pull(skb, sizeof(*map_header));
+ rmnet_set_skb_proto(skb);
+ } else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
+ /* Subtract MAP header */
+ skb_pull(skb, sizeof(*map_header));
+ rmnet_set_skb_proto(skb);
if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 2aea153..1a399bf 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_
@@ -48,5 +48,6 @@ 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);
#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 0ac2ff8..f427018 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
*/
@@ -8,6 +8,7 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <net/ip6_checksum.h>
+#include <linux/bitfield.h>
#include "rmnet_config.h"
#include "rmnet_map.h"
#include "rmnet_private.h"
@@ -300,8 +301,11 @@ 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)
{
+ struct rmnet_map_v5_csum_header *next_hdr = NULL;
+ unsigned char *data = skb->data;
struct rmnet_map_header *maph;
struct sk_buff *skbn;
+ u8 nexthdr_type;
u32 packet_len;
if (skb->len == 0)
@@ -312,6 +316,17 @@ 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->flags & MAP_CMD_FLAG)) {
+ packet_len += sizeof(struct rmnet_map_v5_csum_header);
+ if (maph->flags & MAP_NEXT_HEADER_FLAG)
+ next_hdr = (struct rmnet_map_v5_csum_header *)
+ (data + sizeof(*maph));
+ else
+ /* Mapv5 data pkt without csum hdr is invalid */
+ return NULL;
+ }
+ }
if (((int)skb->len - (int)packet_len) < 0)
return NULL;
@@ -320,6 +335,13 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
if (!maph->pkt_len)
return NULL;
+ if (next_hdr) {
+ nexthdr_type = u8_get_bits(next_hdr->header_info,
+ MAPV5_HDRINFO_HDR_TYPE_FMASK);
+ if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
+ return NULL;
+ }
+
skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
if (!skbn)
return NULL;
@@ -414,3 +436,37 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
priv->stats.csum_sw++;
}
+
+/* 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);
+ struct rmnet_map_v5_csum_header *next_hdr;
+ u8 nexthdr_type;
+ int rc = 0;
+
+ next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
+ sizeof(struct rmnet_map_header));
+
+ nexthdr_type = u8_get_bits(next_hdr->header_info,
+ MAPV5_HDRINFO_HDR_TYPE_FMASK);
+
+ if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
+ if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
+ priv->stats.csum_sw++;
+ } else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
+ 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));
+ } else
+ return -EINVAL;
+
+ return rc;
+}
+
diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
index 4efb537..f82e37e 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_
@@ -14,8 +14,10 @@ struct rmnet_map_header {
/* rmnet_map_header flags field:
* PAD_LEN: number of pad bytes following packet data
* CMD: 1 = packet contains a MAP command; 0 = packet contains data
+ * NEXT_HEADER 1 = packet contains V5 CSUM header 0 = no V5 CSUM header
*/
#define MAP_PAD_LEN_MASK GENMASK(5, 0)
+#define MAP_NEXT_HEADER_FLAG BIT(6)
#define MAP_CMD_FLAG BIT(7)
struct rmnet_map_dl_csum_trailer {
@@ -45,4 +47,27 @@ struct rmnet_map_ul_csum_header {
#define MAP_CSUM_UL_UDP_FLAG BIT(14)
#define MAP_CSUM_UL_ENABLED_FLAG BIT(15)
+/* MAP CSUM headers */
+struct rmnet_map_v5_csum_header {
+ u8 header_info;
+ u8 csum_info;
+ __be16 reserved;
+} __aligned(1);
+
+/* v5 header_info field
+ * NEXT_HEADER: Represents whether there is any other header
+ * HEADER TYPE: represents the type of this header
+ *
+ * csum_info field
+ * CSUM_VALID_OR_REQ:
+ * 1 = for UL, checksum computation is requested.
+ * 1 = for DL, validated the checksum and has found it valid
+ */
+
+#define MAPV5_HDRINFO_NXT_HDR_FLAG BIT(0)
+#define MAPV5_HDRINFO_HDR_TYPE_SHIFT 1
+#define MAPV5_HDRINFO_HDR_TYPE_FMASK GENMASK(7, MAPV5_HDRINFO_HDR_TYPE_SHIFT)
+#define MAPV5_CSUMINFO_VALID_FLAG BIT(7)
+
+#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
#endif /* !(_LINUX_IF_RMNET_H_) */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 91c8dda..21529b3 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1235,6 +1235,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next v4 2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload
2021-04-22 20:02 ` [PATCH net-next v4 2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload Sharath Chandra Vurukala
@ 2021-04-23 12:32 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2021-04-23 12:32 UTC (permalink / raw)
To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
netdev, linux-kernel
On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
> 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 revalidate 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 <sharathv@codeaurora.org>
I have a few minor things I recommend you fix.
I don't see any bugs, but I'm not going to offer
"Reviewed-by" until you've had a chance to either
update your patch or explain why you won't.
> ---
> .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 17 ++++---
> drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 3 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 58 +++++++++++++++++++++-
> include/linux/if_rmnet.h | 27 +++++++++-
> include/uapi/linux/if_link.h | 1 +
> 5 files changed, 97 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 0be5ac7..706a225 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
> */
> @@ -82,11 +82,16 @@ __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 ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
> + (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
> + if (rmnet_map_process_next_hdr_packet(skb, len))
> + goto free_skb;
> + skb_pull(skb, sizeof(*map_header));
> + rmnet_set_skb_proto(skb);
> + } else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
> + /* Subtract MAP header */
> + skb_pull(skb, sizeof(*map_header));
> + rmnet_set_skb_proto(skb);
> if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> }
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> index 2aea153..1a399bf 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_
> @@ -48,5 +48,6 @@ 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);
>
> #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 0ac2ff8..f427018 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
> */
> @@ -8,6 +8,7 @@
> #include <linux/ip.h>
> #include <linux/ipv6.h>
> #include <net/ip6_checksum.h>
> +#include <linux/bitfield.h>
> #include "rmnet_config.h"
> #include "rmnet_map.h"
> #include "rmnet_private.h"
> @@ -300,8 +301,11 @@ 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)
> {
> + struct rmnet_map_v5_csum_header *next_hdr = NULL;
> + unsigned char *data = skb->data;
If you define the data variable to have (void *) type you can
get rid of some casts below, and it won't need to use a cast
when assigned here either.
> struct rmnet_map_header *maph;
> struct sk_buff *skbn;
> + u8 nexthdr_type;
> u32 packet_len;
>
> if (skb->len == 0)
> @@ -312,6 +316,17 @@ 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);
This block should be surrounded by curly braces. If one block
in an if statement (or chain of them) has them, they all should.
> + else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
> + if (!(maph->flags & MAP_CMD_FLAG)) {
> + packet_len += sizeof(struct rmnet_map_v5_csum_header);
packet_len += sizeof(*next_hdr);
> + if (maph->flags & MAP_NEXT_HEADER_FLAG)
> + next_hdr = (struct rmnet_map_v5_csum_header *)
> + (data + sizeof(*maph));
If data is a void pointer, this could be:
next_hdr = data + sizeof(*maph);
> + else
> + /* Mapv5 data pkt without csum hdr is invalid */
> + return NULL;
> + }
> + }
>
> if (((int)skb->len - (int)packet_len) < 0)
> return NULL;
> @@ -320,6 +335,13 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
> if (!maph->pkt_len)
> return NULL;
>
> + if (next_hdr) {
> + nexthdr_type = u8_get_bits(next_hdr->header_info,
> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
Consider fixing the alignment of the second argument
above.
> + if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
> + return NULL;
> + }
> +
> skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
> if (!skbn)
> return NULL;
> @@ -414,3 +436,37 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>
> priv->stats.csum_sw++;
> }
> +
> +/* 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);
> + struct rmnet_map_v5_csum_header *next_hdr;
> + u8 nexthdr_type;
> + int rc = 0;
> +
> + next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
> + sizeof(struct rmnet_map_header));
> +
> + nexthdr_type = u8_get_bits(next_hdr->header_info,
> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
The arguments in the above two assignments don't look
aligned very well. Maybe it's just the way my mail
program is displaying it.
> +
> + if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
> + if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
> + priv->stats.csum_sw++;
> + } else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
> + 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));
skb_pull(skb, sizeof(*next_hdr);
> + } else
> + return -EINVAL;
Please add curly braces here too.
> +
> + return rc;
> +}
> +
> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
> index 4efb537..f82e37e 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_
> @@ -14,8 +14,10 @@ struct rmnet_map_header {
> /* rmnet_map_header flags field:
> * PAD_LEN: number of pad bytes following packet data
> * CMD: 1 = packet contains a MAP command; 0 = packet contains data
> + * NEXT_HEADER 1 = packet contains V5 CSUM header 0 = no V5 CSUM header
> */
> #define MAP_PAD_LEN_MASK GENMASK(5, 0)
> +#define MAP_NEXT_HEADER_FLAG BIT(6)
> #define MAP_CMD_FLAG BIT(7)
>
> struct rmnet_map_dl_csum_trailer {
> @@ -45,4 +47,27 @@ struct rmnet_map_ul_csum_header {
> #define MAP_CSUM_UL_UDP_FLAG BIT(14)
> #define MAP_CSUM_UL_ENABLED_FLAG BIT(15)
>
> +/* MAP CSUM headers */
> +struct rmnet_map_v5_csum_header {
> + u8 header_info;
> + u8 csum_info;
> + __be16 reserved;
> +} __aligned(1);
> +
> +/* v5 header_info field
> + * NEXT_HEADER: Represents whether there is any other header
> + * HEADER TYPE: represents the type of this header
> + *
> + * csum_info field
> + * CSUM_VALID_OR_REQ:
> + * 1 = for UL, checksum computation is requested.
> + * 1 = for DL, validated the checksum and has found it valid
> + */
> +
> +#define MAPV5_HDRINFO_NXT_HDR_FLAG BIT(0)
> +#define MAPV5_HDRINFO_HDR_TYPE_SHIFT 1
> +#define MAPV5_HDRINFO_HDR_TYPE_FMASK GENMASK(7, MAPV5_HDRINFO_HDR_TYPE_SHIFT)
This is the only place you use MAPV5_HDRINFO_TYPE_SHIFT.
Defining and using it here immediately after its definition
subtracts, rather than adds value. Just do:
#define MAPV5_HDRINFO_HDR_TYPE_FMASK GENMASK(7, 1)
-Alex
> +#define MAPV5_CSUMINFO_VALID_FLAG BIT(7)
> +
> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
> #endif /* !(_LINUX_IF_RMNET_H_) */
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 91c8dda..21529b3 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1235,6 +1235,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,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v4 3/3] net: ethernet: rmnet: Add support for MAPv5 egress packets
2021-04-22 20:02 [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Sharath Chandra Vurukala
2021-04-22 20:02 ` [PATCH net-next v4 1/3] docs: networking: Add documentation for MAPv5 Sharath Chandra Vurukala
2021-04-22 20:02 ` [PATCH net-next v4 2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload Sharath Chandra Vurukala
@ 2021-04-22 20:02 ` Sharath Chandra Vurukala
2021-04-23 12:32 ` Alex Elder
2021-04-22 20:14 ` [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Alex Elder
2021-04-23 12:31 ` Alex Elder
4 siblings, 1 reply; 11+ messages in thread
From: Sharath Chandra Vurukala @ 2021-04-22 20:02 UTC (permalink / raw)
To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
Cc: Sharath Chandra Vurukala
Adding Support for MAPv5 egress packets.
Based on the configuration Request HW for csum offload
by setting the csum_valid_required of Mapv5 packet.
Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 12 ++-
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 8 +-
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 93 ++++++++++++++++++++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +-
include/uapi/linux/if_link.h | 1 +
6 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 8d8d469..8e64ca9 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2013-2014, 2016-2018 The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2014, 2016-2018, 2021 The Linux Foundation.
+ * All rights reserved.
*
* RMNET Data configuration engine
*/
@@ -56,6 +57,7 @@ struct rmnet_priv_stats {
u64 csum_fragmented_pkt;
u64 csum_skipped;
u64 csum_sw;
+ u64 csum_hw;
};
struct rmnet_priv {
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 706a225..f8432e6 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -133,7 +133,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
struct rmnet_port *port, u8 mux_id,
struct net_device *orig_dev)
{
- int required_headroom, additional_header_len;
+ int required_headroom, additional_header_len, csum_type = 0;
struct rmnet_map_header *map_header;
additional_header_len = 0;
@@ -142,6 +142,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
required_headroom += additional_header_len;
+ csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+ } else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
+ additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
+ csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
}
if (skb_headroom(skb) < required_headroom) {
@@ -149,10 +153,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
return -ENOMEM;
}
- if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4)
- rmnet_map_checksum_uplink_packet(skb, orig_dev);
+ if (csum_type)
+ rmnet_map_checksum_uplink_packet(skb, port, orig_dev, csum_type);
- map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
+ map_header = rmnet_map_add_map_header(skb, additional_header_len, port, 0);
if (!map_header)
return -ENOMEM;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 1a399bf..e5a0b38 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -43,11 +43,15 @@ enum rmnet_map_commands {
struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
struct rmnet_port *port);
struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
- int hdrlen, int pad);
+ int hdrlen,
+ struct rmnet_port *port,
+ int pad);
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);
+ struct rmnet_port *port,
+ struct net_device *orig_dev,
+ int csum_type);
int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
#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 f427018..a8918d2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -12,6 +12,7 @@
#include "rmnet_config.h"
#include "rmnet_map.h"
#include "rmnet_private.h"
+#include <linux/bitfield.h>
#define RMNET_MAP_DEAGGR_SPACING 64
#define RMNET_MAP_DEAGGR_HEADROOM (RMNET_MAP_DEAGGR_SPACING / 2)
@@ -251,12 +252,69 @@ rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
}
#endif
+static void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
+ struct rmnet_port *port,
+ struct net_device *orig_dev)
+{
+ struct rmnet_priv *priv = netdev_priv(orig_dev);
+ struct rmnet_map_v5_csum_header *ul_header;
+
+ if (!(port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5))
+ return;
+
+ ul_header = skb_push(skb, sizeof(*ul_header));
+ memset(ul_header, 0, sizeof(*ul_header));
+ ul_header->header_info = RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD <<
+ MAPV5_HDRINFO_HDR_TYPE_SHIFT;
+
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ void *iph = (char *)ul_header + sizeof(*ul_header);
+ __sum16 *check;
+ void *trans;
+ u8 proto;
+
+ if (skb->protocol == htons(ETH_P_IP)) {
+ u16 ip_len = ((struct iphdr *)iph)->ihl * 4;
+
+ proto = ((struct iphdr *)iph)->protocol;
+ trans = iph + ip_len;
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+#if IS_ENABLED(CONFIG_IPV6)
+ u16 ip_len = sizeof(struct ipv6hdr);
+
+ proto = ((struct ipv6hdr *)iph)->nexthdr;
+ trans = iph + ip_len;
+#else
+ priv->stats.csum_err_invalid_ip_version++;
+ goto sw_csum;
+#endif /* CONFIG_IPV6 */
+ } else {
+ priv->stats.csum_err_invalid_ip_version++;
+ goto sw_csum;
+ }
+
+ check = rmnet_map_get_csum_field(proto, trans);
+ if (check) {
+ skb->ip_summed = CHECKSUM_NONE;
+ /* Ask for checksum offloading */
+ ul_header->csum_info |= MAPV5_CSUMINFO_VALID_FLAG;
+ priv->stats.csum_hw++;
+ return;
+ }
+ }
+
+sw_csum:
+ priv->stats.csum_sw++;
+}
+
/* Adds MAP header to front of skb->data
* Padding is calculated and set appropriately in MAP header. Mux ID is
* initialized to 0.
*/
struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
- int hdrlen, int pad)
+ int hdrlen,
+ struct rmnet_port *port,
+ int pad)
{
struct rmnet_map_header *map_header;
u32 padding, map_datalen;
@@ -267,6 +325,10 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
skb_push(skb, sizeof(struct rmnet_map_header));
memset(map_header, 0, sizeof(struct rmnet_map_header));
+ /* Set next_hdr bit for csum offload packets */
+ if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
+ map_header->flags |= MAP_NEXT_HEADER_FLAG;
+
if (pad == RMNET_MAP_NO_PAD_BYTES) {
map_header->pkt_len = htons(map_datalen);
return map_header;
@@ -394,11 +456,8 @@ int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
return 0;
}
-/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
- * packets that are supported for UL checksum offload.
- */
-void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
- struct net_device *orig_dev)
+static void rmnet_map_v4_checksum_uplink_packet(struct sk_buff *skb,
+ struct net_device *orig_dev)
{
struct rmnet_priv *priv = netdev_priv(orig_dev);
struct rmnet_map_ul_csum_header *ul_header;
@@ -417,10 +476,12 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
if (skb->protocol == htons(ETH_P_IP)) {
rmnet_map_ipv4_ul_csum_header(iphdr, ul_header, skb);
+ priv->stats.csum_hw++;
return;
} else if (skb->protocol == htons(ETH_P_IPV6)) {
#if IS_ENABLED(CONFIG_IPV6)
rmnet_map_ipv6_ul_csum_header(iphdr, ul_header, skb);
+ priv->stats.csum_hw++;
return;
#else
priv->stats.csum_err_invalid_ip_version++;
@@ -437,6 +498,26 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
priv->stats.csum_sw++;
}
+/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
+ * packets that are supported for UL checksum offload.
+ */
+void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
+ struct rmnet_port *port,
+ struct net_device *orig_dev,
+ int csum_type)
+{
+ switch (csum_type) {
+ case RMNET_FLAGS_EGRESS_MAP_CKSUMV4:
+ rmnet_map_v4_checksum_uplink_packet(skb, orig_dev);
+ break;
+ case RMNET_FLAGS_EGRESS_MAP_CKSUMV5:
+ rmnet_map_v5_checksum_uplink_packet(skb, port, orig_dev);
+ break;
+ default:
+ break;
+ }
+}
+
/* Process a MAPv5 packet header */
int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
u16 len)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 41fbd2c..bc6d6ac 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -174,6 +174,7 @@ static const char rmnet_gstrings_stats[][ETH_GSTRING_LEN] = {
"Checksum skipped on ip fragment",
"Checksum skipped",
"Checksum computed in software",
+ "Checksum computed in hardware",
};
static void rmnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
@@ -354,4 +355,4 @@ int rmnet_vnd_update_dev_mtu(struct rmnet_port *port,
}
return 0;
-}
\ No newline at end of file
+}
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 21529b3..1691f3a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1236,6 +1236,7 @@ enum {
#define RMNET_FLAGS_INGRESS_MAP_CKSUMV4 (1U << 2)
#define RMNET_FLAGS_EGRESS_MAP_CKSUMV4 (1U << 3)
#define RMNET_FLAGS_INGRESS_MAP_CKSUMV5 (1U << 4)
+#define RMNET_FLAGS_EGRESS_MAP_CKSUMV5 (1U << 5)
enum {
IFLA_RMNET_UNSPEC,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next v4 3/3] net: ethernet: rmnet: Add support for MAPv5 egress packets
2021-04-22 20:02 ` [PATCH net-next v4 3/3] net: ethernet: rmnet: Add support for MAPv5 egress packets Sharath Chandra Vurukala
@ 2021-04-23 12:32 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2021-04-23 12:32 UTC (permalink / raw)
To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
netdev, linux-kernel
On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
> Adding Support for MAPv5 egress packets.
> Based on the configuration Request HW for csum offload
> by setting the csum_valid_required of Mapv5 packet.
Please try to re-word this description. I'm not sure
I understand what it means.
I see what I think is a bug below. Please either
fix or explain.
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
> ---
> drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 12 ++-
> drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 8 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 93 ++++++++++++++++++++--
> drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +-
> include/uapi/linux/if_link.h | 1 +
> 6 files changed, 107 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
> index 8d8d469..8e64ca9 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> -/* Copyright (c) 2013-2014, 2016-2018 The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2013-2014, 2016-2018, 2021 The Linux Foundation.
> + * All rights reserved.
> *
> * RMNET Data configuration engine
> */
> @@ -56,6 +57,7 @@ struct rmnet_priv_stats {
> u64 csum_fragmented_pkt;
> u64 csum_skipped;
> u64 csum_sw;
> + u64 csum_hw;
Why is this new statistic type added? Would it be
meaningful to use before--with only QMAPv4? Or is
there something different about QMAPv5 (inline) checksum
offload that makes this necessary or desirable?
This is something new that ought to be at least
mentioned in the description at the top. And for
future reference, this could likely have been
defined in a separate patch, before this one.
> };
>
> struct rmnet_priv {
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 706a225..f8432e6 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> @@ -133,7 +133,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
> struct rmnet_port *port, u8 mux_id,
> struct net_device *orig_dev)
> {
> - int required_headroom, additional_header_len;
> + int required_headroom, additional_header_len, csum_type = 0;
> struct rmnet_map_header *map_header;
>
> additional_header_len = 0;
> @@ -142,6 +142,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
> if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
> additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
> required_headroom += additional_header_len;
> + csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
> + } else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
> + additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
Does additional_header_len need to be added to required_headroom,
as it is for QMAPv4 above?
If so, this is a bug and must be fixed.
What I tested last week (and verified work for IPA v3.5.1 and
IPA v4.2) looked like this:
if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
} else if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
additional_header_len = sizeof(struct rmnet_map_v5_csum_header);
csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
}
required_headroom += additional_header_len;
-Alex
> + csum_type = RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
> }
>
> if (skb_headroom(skb) < required_headroom) {
> @@ -149,10 +153,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
> return -ENOMEM;
> }
>
> - if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4)
> - rmnet_map_checksum_uplink_packet(skb, orig_dev);
> + if (csum_type)
> + rmnet_map_checksum_uplink_packet(skb, port, orig_dev, csum_type);
>
> - map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
> + map_header = rmnet_map_add_map_header(skb, additional_header_len, port, 0);
> if (!map_header)
> return -ENOMEM;
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> index 1a399bf..e5a0b38 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> @@ -43,11 +43,15 @@ enum rmnet_map_commands {
> struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
> struct rmnet_port *port);
> struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
> - int hdrlen, int pad);
> + int hdrlen,
> + struct rmnet_port *port,
> + int pad);
> 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);
> + struct rmnet_port *port,
> + struct net_device *orig_dev,
> + int csum_type);
> int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
>
> #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 f427018..a8918d2 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -12,6 +12,7 @@
> #include "rmnet_config.h"
> #include "rmnet_map.h"
> #include "rmnet_private.h"
> +#include <linux/bitfield.h>
>
> #define RMNET_MAP_DEAGGR_SPACING 64
> #define RMNET_MAP_DEAGGR_HEADROOM (RMNET_MAP_DEAGGR_SPACING / 2)
> @@ -251,12 +252,69 @@ rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
> }
> #endif
>
> +static void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
> + struct rmnet_port *port,
> + struct net_device *orig_dev)
> +{
> + struct rmnet_priv *priv = netdev_priv(orig_dev);
> + struct rmnet_map_v5_csum_header *ul_header;
> +
> + if (!(port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5))
> + return;
> +
> + ul_header = skb_push(skb, sizeof(*ul_header));
> + memset(ul_header, 0, sizeof(*ul_header));
> + ul_header->header_info = RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD <<
> + MAPV5_HDRINFO_HDR_TYPE_SHIFT;
> +
> + if (skb->ip_summed == CHECKSUM_PARTIAL) {
> + void *iph = (char *)ul_header + sizeof(*ul_header);
> + __sum16 *check;
> + void *trans;
> + u8 proto;
> +
> + if (skb->protocol == htons(ETH_P_IP)) {
> + u16 ip_len = ((struct iphdr *)iph)->ihl * 4;
> +
> + proto = ((struct iphdr *)iph)->protocol;
> + trans = iph + ip_len;
> + } else if (skb->protocol == htons(ETH_P_IPV6)) {
> +#if IS_ENABLED(CONFIG_IPV6)
> + u16 ip_len = sizeof(struct ipv6hdr);
> +
> + proto = ((struct ipv6hdr *)iph)->nexthdr;
> + trans = iph + ip_len;
> +#else
> + priv->stats.csum_err_invalid_ip_version++;
> + goto sw_csum;
> +#endif /* CONFIG_IPV6 */
> + } else {
> + priv->stats.csum_err_invalid_ip_version++;
> + goto sw_csum;
> + }
> +
> + check = rmnet_map_get_csum_field(proto, trans);
> + if (check) {
> + skb->ip_summed = CHECKSUM_NONE;
> + /* Ask for checksum offloading */
> + ul_header->csum_info |= MAPV5_CSUMINFO_VALID_FLAG;
> + priv->stats.csum_hw++;
> + return;
> + }
> + }
> +
> +sw_csum:
> + priv->stats.csum_sw++;
> +}
> +
> /* Adds MAP header to front of skb->data
> * Padding is calculated and set appropriately in MAP header. Mux ID is
> * initialized to 0.
> */
> struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
> - int hdrlen, int pad)
> + int hdrlen,
> + struct rmnet_port *port,
> + int pad)
> {
> struct rmnet_map_header *map_header;
> u32 padding, map_datalen;
> @@ -267,6 +325,10 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
> skb_push(skb, sizeof(struct rmnet_map_header));
> memset(map_header, 0, sizeof(struct rmnet_map_header));
>
> + /* Set next_hdr bit for csum offload packets */
> + if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5)
> + map_header->flags |= MAP_NEXT_HEADER_FLAG;
> +
> if (pad == RMNET_MAP_NO_PAD_BYTES) {
> map_header->pkt_len = htons(map_datalen);
> return map_header;
> @@ -394,11 +456,8 @@ int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
> return 0;
> }
>
> -/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
> - * packets that are supported for UL checksum offload.
> - */
> -void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
> - struct net_device *orig_dev)
> +static void rmnet_map_v4_checksum_uplink_packet(struct sk_buff *skb,
> + struct net_device *orig_dev)
> {
> struct rmnet_priv *priv = netdev_priv(orig_dev);
> struct rmnet_map_ul_csum_header *ul_header;
> @@ -417,10 +476,12 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>
> if (skb->protocol == htons(ETH_P_IP)) {
> rmnet_map_ipv4_ul_csum_header(iphdr, ul_header, skb);
> + priv->stats.csum_hw++;
> return;
> } else if (skb->protocol == htons(ETH_P_IPV6)) {
> #if IS_ENABLED(CONFIG_IPV6)
> rmnet_map_ipv6_ul_csum_header(iphdr, ul_header, skb);
> + priv->stats.csum_hw++;
> return;
> #else
> priv->stats.csum_err_invalid_ip_version++;
> @@ -437,6 +498,26 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
> priv->stats.csum_sw++;
> }
>
> +/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
> + * packets that are supported for UL checksum offload.
> + */
> +void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
> + struct rmnet_port *port,
> + struct net_device *orig_dev,
> + int csum_type)
> +{
> + switch (csum_type) {
> + case RMNET_FLAGS_EGRESS_MAP_CKSUMV4:
> + rmnet_map_v4_checksum_uplink_packet(skb, orig_dev);
> + break;
> + case RMNET_FLAGS_EGRESS_MAP_CKSUMV5:
> + rmnet_map_v5_checksum_uplink_packet(skb, port, orig_dev);
> + break;
> + default:
> + break;
> + }
> +}
> +
> /* Process a MAPv5 packet header */
> int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
> u16 len)
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> index 41fbd2c..bc6d6ac 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> @@ -174,6 +174,7 @@ static const char rmnet_gstrings_stats[][ETH_GSTRING_LEN] = {
> "Checksum skipped on ip fragment",
> "Checksum skipped",
> "Checksum computed in software",
> + "Checksum computed in hardware",
> };
>
> static void rmnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> @@ -354,4 +355,4 @@ int rmnet_vnd_update_dev_mtu(struct rmnet_port *port,
> }
>
> return 0;
> -}
> \ No newline at end of file
> +}
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 21529b3..1691f3a 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1236,6 +1236,7 @@ enum {
> #define RMNET_FLAGS_INGRESS_MAP_CKSUMV4 (1U << 2)
> #define RMNET_FLAGS_EGRESS_MAP_CKSUMV4 (1U << 3)
> #define RMNET_FLAGS_INGRESS_MAP_CKSUMV5 (1U << 4)
> +#define RMNET_FLAGS_EGRESS_MAP_CKSUMV5 (1U << 5)
>
> enum {
> IFLA_RMNET_UNSPEC,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5
2021-04-22 20:02 [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Sharath Chandra Vurukala
` (2 preceding siblings ...)
2021-04-22 20:02 ` [PATCH net-next v4 3/3] net: ethernet: rmnet: Add support for MAPv5 egress packets Sharath Chandra Vurukala
@ 2021-04-22 20:14 ` Alex Elder
2021-04-22 23:14 ` subashab
2021-04-23 12:31 ` Alex Elder
4 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2021-04-22 20:14 UTC (permalink / raw)
To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
netdev, linux-kernel
On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
> This series introduces the MAPv5 packet format.
>
> Patch 0 documents the MAPv4/v5.
> Patch 1 introduces the MAPv5 and the Inline checksum offload for RX/Ingress.
> Patch 2 introduces the MAPv5 and the Inline checksum offload for TX/Egress.
Was this supposed to be version 5?
I already reviewed version 4.
Please post version 5. I am going to ignore this series.
-Alex
>
> 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.
>
> v3->v4:
> - Made changes to use masks instead of C bit-fields as suggested by Jakub/Alex.
>
> Sharath Chandra Vurukala (3):
> docs: networking: Add documentation for MAPv5
> net: ethernet: rmnet: Support for ingress MAPv5 checksum offload
> net: ethernet: rmnet: Add support for MAPv5 egress packets
>
> .../device_drivers/cellular/qualcomm/rmnet.rst | 126 +++++++++++++++--
> drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 29 ++--
> drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 11 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 151 ++++++++++++++++++++-
> drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +-
> include/linux/if_rmnet.h | 27 +++-
> include/uapi/linux/if_link.h | 2 +
> 8 files changed, 318 insertions(+), 35 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5
2021-04-22 20:14 ` [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Alex Elder
@ 2021-04-22 23:14 ` subashab
2021-04-23 0:54 ` Alex Elder
0 siblings, 1 reply; 11+ messages in thread
From: subashab @ 2021-04-22 23:14 UTC (permalink / raw)
To: Alex Elder
Cc: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, netdev,
linux-kernel
On 2021-04-22 14:14, Alex Elder wrote:
> On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
>> This series introduces the MAPv5 packet format.
>>
>> Patch 0 documents the MAPv4/v5.
>> Patch 1 introduces the MAPv5 and the Inline checksum offload for
>> RX/Ingress.
>> Patch 2 introduces the MAPv5 and the Inline checksum offload for
>> TX/Egress.
>
> Was this supposed to be version 5?
>
> I already reviewed version 4.
>
> Please post version 5. I am going to ignore this series.
>
> -Alex
>
What are you talking about?
Patchwork shows that Sharath has posted upto v3 so far.
https://patchwork.kernel.org/project/netdevbpf/list/?submitter=197703&state=%2A&archive=both
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5
2021-04-22 23:14 ` subashab
@ 2021-04-23 0:54 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2021-04-23 0:54 UTC (permalink / raw)
To: subashab
Cc: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, netdev,
linux-kernel
On 4/22/21 6:14 PM, subashab@codeaurora.org wrote:
> On 2021-04-22 14:14, Alex Elder wrote:
>> On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
>>> This series introduces the MAPv5 packet format.
>>>
>>> Patch 0 documents the MAPv4/v5.
>>> Patch 1 introduces the MAPv5 and the Inline checksum offload for
>>> RX/Ingress.
>>> Patch 2 introduces the MAPv5 and the Inline checksum offload for
>>> TX/Egress.
>>
>> Was this supposed to be version 5?
>>
>> I already reviewed version 4.
>>
>> Please post version 5. I am going to ignore this series.
>>
>> -Alex
>>
>
> What are you talking about?
>
> Patchwork shows that Sharath has posted upto v3 so far.
Sorry about that. Sharath posted a series version 4 last week, which
I reviewed. I mistakenly thought he had posted it for upstream review
and didn't realize he sent it to me (and others) privately.
Still, I said he should tag the second patch "Acked-by:" me and
he did not, so I assumed it was a repost of the same code.
I'll review this after all.
-Alex
>
> https://patchwork.kernel.org/project/netdevbpf/list/?submitter=197703&state=%2A&archive=both
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5
2021-04-22 20:02 [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Sharath Chandra Vurukala
` (3 preceding siblings ...)
2021-04-22 20:14 ` [PATCH net-next v4 0/3] net: qualcomm: rmnet: Enable Mapv5 Alex Elder
@ 2021-04-23 12:31 ` Alex Elder
4 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2021-04-23 12:31 UTC (permalink / raw)
To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
netdev, linux-kernel
On 4/22/21 3:02 PM, Sharath Chandra Vurukala wrote:
> This series introduces the MAPv5 packet format.
>
> Patch 0 documents the MAPv4/v5.
> Patch 1 introduces the MAPv5 and the Inline checksum offload for RX/Ingress.
> Patch 2 introduces the MAPv5 and the Inline checksum offload for TX/Egress.
>
> 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.
It turns out many of review comments from last week were
ignored without explanation. So I will repeat some of them
this time.
I see one thing that I think might be a bug in the third
patch, but maybe I'm mistaken, and you can explain why.
I tested the code you supplied me last week, and with a
bug fix applied I found they worked for:
IPA v3.5.1, IPv4 in loopback, checksum enabled and not
IPA v4.2, IPv6 using LTE, checksum enabled and not
Both of the above tested ICMP, UDP, and TCP. I will retest
with this series.
I did not test with IPA v4.5+, which is unfortunately
the main user of this new code. I will try to do so
with your updated code, and if all testing passes I'll
send a message with "Tested-by" for you to add to your
patches.
-Alex
> 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.
>
> v3->v4:
> - Made changes to use masks instead of C bit-fields as suggested by Jakub/Alex.
>
> Sharath Chandra Vurukala (3):
> docs: networking: Add documentation for MAPv5
> net: ethernet: rmnet: Support for ingress MAPv5 checksum offload
> net: ethernet: rmnet: Add support for MAPv5 egress packets
>
> .../device_drivers/cellular/qualcomm/rmnet.rst | 126 +++++++++++++++--
> drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 29 ++--
> drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 11 +-
> .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 151 ++++++++++++++++++++-
> drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +-
> include/linux/if_rmnet.h | 27 +++-
> include/uapi/linux/if_link.h | 2 +
> 8 files changed, 318 insertions(+), 35 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread