linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5.
@ 2021-02-11 21:35 Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 1/3] docs:networking: Add documentation for MAP v5 Sharath Chandra Vurukala
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Sharath Chandra Vurukala @ 2021-02-11 21:35 UTC (permalink / raw)
  To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
  Cc: Sharath Chandra Vurukala

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.

Sharath Chandra Vurukala (3):
  docs:networking: Add documentation for MAP v5
  net:ethernet:rmnet:Support for downlink MAPv5 csum 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   |  36 ++++--
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  36 +++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 136 +++++++++++++++++++--
 include/linux/if_rmnet.h                           |  17 ++-
 include/uapi/linux/if_link.h                       |   2 +
 7 files changed, 255 insertions(+), 29 deletions(-)

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] docs:networking: Add documentation for MAP v5
  2021-02-11 21:35 [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5 Sharath Chandra Vurukala
@ 2021-02-11 21:35 ` Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet Sharath Chandra Vurukala
  2 siblings, 0 replies; 12+ messages in thread
From: Sharath Chandra Vurukala @ 2021-02-11 21:35 UTC (permalink / raw)
  To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
  Cc: Sharath Chandra Vurukala

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

Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
 .../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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-11 21:35 [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5 Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 1/3] docs:networking: Add documentation for MAP v5 Sharath Chandra Vurukala
@ 2021-02-11 21:35 ` Sharath Chandra Vurukala
  2021-02-12  2:04   ` Jakub Kicinski
  2021-02-12  4:53   ` kernel test robot
  2021-02-11 21:35 ` [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet Sharath Chandra Vurukala
  2 siblings, 2 replies; 12+ messages in thread
From: Sharath Chandra Vurukala @ 2021-02-11 21:35 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 revalidated the checksum
and update the respective snmp stats.

Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h |  3 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 19 ++++++----
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    | 32 +++++++++++++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 44 +++++++++++++++++++++-
 include/linux/if_rmnet.h                           | 17 +++++++--
 include/uapi/linux/if_link.h                       |  1 +
 6 files changed, 102 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..d4d61471 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 The Linux Foundation.
+ * All rights reserved.
  *
  * RMNET Data configuration engine
  */
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..55d293c 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
@@ -55,10 +64,29 @@ 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);
+int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
+
+static 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;
+}
+
+static inline 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;
+}
 
 #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..3d7e03f 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,33 @@ 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);
+	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..81acd0b 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,15 @@ struct rmnet_map_ul_csum_header {
 #endif
 } __aligned(1);
 
+/* MAP CSUM headers */
+struct rmnet_map_v5_csum_header {
+	u8  next_hdr:1;
+	u8  header_type:7;
+	u8  hw_reserved:5;
+	u8  priority:1;
+	u8  hw_reserved_bit:1;
+	u8  csum_valid_required:1;
+	__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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet
  2021-02-11 21:35 [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5 Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 1/3] docs:networking: Add documentation for MAP v5 Sharath Chandra Vurukala
  2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
@ 2021-02-11 21:35 ` Sharath Chandra Vurukala
  2021-02-12  2:25   ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Sharath Chandra Vurukala @ 2021-02-11 21:35 UTC (permalink / raw)
  To: davem, kuba, elder, cpratapa, subashab, netdev, linux-kernel
  Cc: Sharath Chandra Vurukala

Adding Support for Mapv5 uplink packet.
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 |  3 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 17 +++-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  4 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 92 ++++++++++++++++++++--
 include/uapi/linux/if_link.h                       |  1 +
 5 files changed, 105 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index d4d61471..8e64ca9 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2013-2014, 2016-2018 The Linux Foundation.
+/* Copyright (c) 2013-2014, 2016-2018, 2021 The Linux Foundation.
  * All rights reserved.
  *
  * RMNET Data configuration engine
@@ -57,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 70ad6a7..870be09 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -131,8 +131,9 @@ 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;
 	struct rmnet_map_header *map_header;
+	csum_type = 0;
 
 	additional_header_len = 0;
 	required_headroom = sizeof(struct rmnet_map_header);
@@ -140,17 +141,25 @@ 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;
 	}
 
+	required_headroom += additional_header_len;
+
 	if (skb_headroom(skb) < required_headroom) {
 		if (pskb_expand_head(skb, required_headroom, 0, GFP_ATOMIC))
 			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 55d293c..9b2aef0 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -70,7 +70,9 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 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);
 
 static u8 rmnet_map_get_next_hdr_type(struct sk_buff *skb)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 3d7e03f..600b9a2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -263,12 +263,68 @@ rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
 }
 #endif
 
+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 = (struct rmnet_map_v5_csum_header *)
+		    skb_push(skb, sizeof(*ul_header));
+	memset(ul_header, 0, sizeof(*ul_header));
+	ul_header->header_type = RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD;
+
+	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;
+		}
+#if IS_ENABLED(CONFIG_IPV6)
+		else if (skb->protocol == htons(ETH_P_IPV6)) {
+			u16 ip_len = sizeof(struct ipv6hdr);
+
+			proto = ((struct ipv6hdr *)iph)->nexthdr;
+			trans = iph + ip_len;
+		}
+#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_valid_required = 1;
+			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;
@@ -279,6 +335,11 @@ 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->next_hdr = 1;
+	}
+
 	if (pad == RMNET_MAP_NO_PAD_BYTES) {
 		map_header->pkt_len = htons(map_datalen);
 		return map_header;
@@ -395,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)
+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;
@@ -418,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++;
@@ -441,6 +501,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/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 838bd29..319865f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1234,6 +1234,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] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
@ 2021-02-12  2:04   ` Jakub Kicinski
  2021-02-12 14:01     ` Alex Elder
  2021-02-12  4:53   ` kernel test robot
  1 sibling, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2021-02-12  2:04 UTC (permalink / raw)
  To: Sharath Chandra Vurukala
  Cc: davem, elder, cpratapa, subashab, netdev, linux-kernel

On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:
> +/* MAP CSUM headers */
> +struct rmnet_map_v5_csum_header {
> +	u8  next_hdr:1;
> +	u8  header_type:7;
> +	u8  hw_reserved:5;
> +	u8  priority:1;
> +	u8  hw_reserved_bit:1;
> +	u8  csum_valid_required:1;
> +	__be16 reserved;
> +} __aligned(1);

Will this work on big endian?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet
  2021-02-11 21:35 ` [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet Sharath Chandra Vurukala
@ 2021-02-12  2:25   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-02-12  2:25 UTC (permalink / raw)
  To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
	netdev, linux-kernel
  Cc: kbuild-all, clang-built-linux, Sharath Chandra Vurukala

[-- Attachment #1: Type: text/plain, Size: 10905 bytes --]

Hi Sharath,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ipvs/master]
[also build test WARNING on linus/master sparc-next/master v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sharath-Chandra-Vurukala/docs-networking-Add-documentation-for-MAP-v5/20210212-063547
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
config: x86_64-randconfig-a012-20210209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/7f0a1e35c1d1c17de5873aded88d5dadfedce2fb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sharath-Chandra-Vurukala/docs-networking-Add-documentation-for-MAP-v5/20210212-063547
        git checkout 7f0a1e35c1d1c17de5873aded88d5dadfedce2fb
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:266:6: warning: no previous prototype for function 'rmnet_map_v5_checksum_uplink_packet' [-Wmissing-prototypes]
   void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
        ^
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:266:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
   ^
   static 
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:459:6: warning: no previous prototype for function 'rmnet_map_v4_checksum_uplink_packet' [-Wmissing-prototypes]
   void rmnet_map_v4_checksum_uplink_packet(struct sk_buff *skb,
        ^
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:459:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void rmnet_map_v4_checksum_uplink_packet(struct sk_buff *skb,
   ^
   static 
   2 warnings generated.


vim +/rmnet_map_v5_checksum_uplink_packet +266 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c

   265	
 > 266	void rmnet_map_v5_checksum_uplink_packet(struct sk_buff *skb,
   267						 struct rmnet_port *port,
   268						 struct net_device *orig_dev)
   269	{
   270		struct rmnet_priv *priv = netdev_priv(orig_dev);
   271		struct rmnet_map_v5_csum_header *ul_header;
   272	
   273		if (!(port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5))
   274			return;
   275	
   276		ul_header = (struct rmnet_map_v5_csum_header *)
   277			    skb_push(skb, sizeof(*ul_header));
   278		memset(ul_header, 0, sizeof(*ul_header));
   279		ul_header->header_type = RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD;
   280	
   281		if (skb->ip_summed == CHECKSUM_PARTIAL) {
   282			void *iph = (char *)ul_header + sizeof(*ul_header);
   283			__sum16 *check;
   284			void *trans;
   285			u8 proto;
   286	
   287			if (skb->protocol == htons(ETH_P_IP)) {
   288				u16 ip_len = ((struct iphdr *)iph)->ihl * 4;
   289	
   290				proto = ((struct iphdr *)iph)->protocol;
   291				trans = iph + ip_len;
   292			}
   293	#if IS_ENABLED(CONFIG_IPV6)
   294			else if (skb->protocol == htons(ETH_P_IPV6)) {
   295				u16 ip_len = sizeof(struct ipv6hdr);
   296	
   297				proto = ((struct ipv6hdr *)iph)->nexthdr;
   298				trans = iph + ip_len;
   299			}
   300	#endif /* CONFIG_IPV6 */
   301			else {
   302				priv->stats.csum_err_invalid_ip_version++;
   303				goto sw_csum;
   304			}
   305	
   306			check = rmnet_map_get_csum_field(proto, trans);
   307			if (check) {
   308				skb->ip_summed = CHECKSUM_NONE;
   309				/* Ask for checksum offloading */
   310				ul_header->csum_valid_required = 1;
   311				priv->stats.csum_hw++;
   312				return;
   313			}
   314		}
   315	
   316	sw_csum:
   317		priv->stats.csum_sw++;
   318	}
   319	
   320	/* Adds MAP header to front of skb->data
   321	 * Padding is calculated and set appropriately in MAP header. Mux ID is
   322	 * initialized to 0.
   323	 */
   324	struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
   325							  int hdrlen,
   326							  struct rmnet_port *port,
   327							  int pad)
   328	{
   329		struct rmnet_map_header *map_header;
   330		u32 padding, map_datalen;
   331		u8 *padbytes;
   332	
   333		map_datalen = skb->len - hdrlen;
   334		map_header = (struct rmnet_map_header *)
   335				skb_push(skb, sizeof(struct rmnet_map_header));
   336		memset(map_header, 0, sizeof(struct rmnet_map_header));
   337	
   338		/* Set next_hdr bit for csum offload packets */
   339		if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV5) {
   340			map_header->next_hdr = 1;
   341		}
   342	
   343		if (pad == RMNET_MAP_NO_PAD_BYTES) {
   344			map_header->pkt_len = htons(map_datalen);
   345			return map_header;
   346		}
   347	
   348		padding = ALIGN(map_datalen, 4) - map_datalen;
   349	
   350		if (padding == 0)
   351			goto done;
   352	
   353		if (skb_tailroom(skb) < padding)
   354			return NULL;
   355	
   356		padbytes = (u8 *)skb_put(skb, padding);
   357		memset(padbytes, 0, padding);
   358	
   359	done:
   360		map_header->pkt_len = htons(map_datalen + padding);
   361		map_header->pad_len = padding & 0x3F;
   362	
   363		return map_header;
   364	}
   365	
   366	/* Deaggregates a single packet
   367	 * A whole new buffer is allocated for each portion of an aggregated frame.
   368	 * Caller should keep calling deaggregate() on the source skb until 0 is
   369	 * returned, indicating that there are no more packets to deaggregate. Caller
   370	 * is responsible for freeing the original skb.
   371	 */
   372	struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
   373					      struct rmnet_port *port)
   374	{
   375		unsigned char *data = skb->data, *next_hdr = NULL;
   376		struct rmnet_map_header *maph;
   377		struct sk_buff *skbn;
   378		u32 packet_len;
   379	
   380		if (skb->len == 0)
   381			return NULL;
   382	
   383		maph = (struct rmnet_map_header *)skb->data;
   384		packet_len = ntohs(maph->pkt_len) + sizeof(struct rmnet_map_header);
   385	
   386		if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
   387			packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
   388		else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
   389			if (!maph->cd_bit) {
   390				packet_len += sizeof(struct rmnet_map_v5_csum_header);
   391				next_hdr = data + sizeof(*maph);
   392			}
   393		}
   394	
   395		if (((int)skb->len - (int)packet_len) < 0)
   396			return NULL;
   397	
   398		/* Some hardware can send us empty frames. Catch them */
   399		if (ntohs(maph->pkt_len) == 0)
   400			return NULL;
   401	
   402		if (next_hdr &&
   403		    ((struct rmnet_map_v5_csum_header *)next_hdr)->header_type !=
   404		     RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
   405			return NULL;
   406	
   407		skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
   408		if (!skbn)
   409			return NULL;
   410	
   411		skb_reserve(skbn, RMNET_MAP_DEAGGR_HEADROOM);
   412		skb_put(skbn, packet_len);
   413		memcpy(skbn->data, skb->data, packet_len);
   414		skb_pull(skb, packet_len);
   415	
   416		return skbn;
   417	}
   418	
   419	/* Validates packet checksums. Function takes a pointer to
   420	 * the beginning of a buffer which contains the IP payload +
   421	 * padding + checksum trailer.
   422	 * Only IPv4 and IPv6 are supported along with TCP & UDP.
   423	 * Fragmented or tunneled packets are not supported.
   424	 */
   425	int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
   426	{
   427		struct rmnet_priv *priv = netdev_priv(skb->dev);
   428		struct rmnet_map_dl_csum_trailer *csum_trailer;
   429	
   430		if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
   431			priv->stats.csum_sw++;
   432			return -EOPNOTSUPP;
   433		}
   434	
   435		csum_trailer = (struct rmnet_map_dl_csum_trailer *)(skb->data + len);
   436	
   437		if (!csum_trailer->valid) {
   438			priv->stats.csum_valid_unset++;
   439			return -EINVAL;
   440		}
   441	
   442		if (skb->protocol == htons(ETH_P_IP)) {
   443			return rmnet_map_ipv4_dl_csum_trailer(skb, csum_trailer, priv);
   444		} else if (skb->protocol == htons(ETH_P_IPV6)) {
   445	#if IS_ENABLED(CONFIG_IPV6)
   446			return rmnet_map_ipv6_dl_csum_trailer(skb, csum_trailer, priv);
   447	#else
   448			priv->stats.csum_err_invalid_ip_version++;
   449			return -EPROTONOSUPPORT;
   450	#endif
   451		} else {
   452			priv->stats.csum_err_invalid_ip_version++;
   453			return -EPROTONOSUPPORT;
   454		}
   455	
   456		return 0;
   457	}
   458	
 > 459	void rmnet_map_v4_checksum_uplink_packet(struct sk_buff *skb,
   460						 struct net_device *orig_dev)
   461	{
   462		struct rmnet_priv *priv = netdev_priv(orig_dev);
   463		struct rmnet_map_ul_csum_header *ul_header;
   464		void *iphdr;
   465	
   466		ul_header = (struct rmnet_map_ul_csum_header *)
   467			    skb_push(skb, sizeof(struct rmnet_map_ul_csum_header));
   468	
   469		if (unlikely(!(orig_dev->features &
   470			     (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))))
   471			goto sw_csum;
   472	
   473		if (skb->ip_summed == CHECKSUM_PARTIAL) {
   474			iphdr = (char *)ul_header +
   475				sizeof(struct rmnet_map_ul_csum_header);
   476	
   477			if (skb->protocol == htons(ETH_P_IP)) {
   478				rmnet_map_ipv4_ul_csum_header(iphdr, ul_header, skb);
   479				priv->stats.csum_hw++;
   480				return;
   481			} else if (skb->protocol == htons(ETH_P_IPV6)) {
   482	#if IS_ENABLED(CONFIG_IPV6)
   483				rmnet_map_ipv6_ul_csum_header(iphdr, ul_header, skb);
   484				priv->stats.csum_hw++;
   485				return;
   486	#else
   487				priv->stats.csum_err_invalid_ip_version++;
   488				goto sw_csum;
   489	#endif
   490			} else {
   491				priv->stats.csum_err_invalid_ip_version++;
   492			}
   493		}
   494	
   495	sw_csum:
   496		ul_header->csum_start_offset = 0;
   497		ul_header->csum_insert_offset = 0;
   498		ul_header->csum_enabled = 0;
   499		ul_header->udp_ind = 0;
   500	
   501		priv->stats.csum_sw++;
   502	}
   503	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34209 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
  2021-02-12  2:04   ` Jakub Kicinski
@ 2021-02-12  4:53   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-02-12  4:53 UTC (permalink / raw)
  To: Sharath Chandra Vurukala, davem, kuba, elder, cpratapa, subashab,
	netdev, linux-kernel
  Cc: kbuild-all, Sharath Chandra Vurukala

[-- Attachment #1: Type: text/plain, Size: 6566 bytes --]

Hi Sharath,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ipvs/master]
[also build test ERROR on linus/master sparc-next/master v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sharath-Chandra-Vurukala/docs-networking-Add-documentation-for-MAP-v5/20210212-063547
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/293142d706c02bf2e6ce7acb4e04ebb6cf4a2a63
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sharath-Chandra-Vurukala/docs-networking-Add-documentation-for-MAP-v5/20210212-063547
        git checkout 293142d706c02bf2e6ce7acb4e04ebb6cf4a2a63
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Sharath-Chandra-Vurukala/docs-networking-Add-documentation-for-MAP-v5/20210212-063547 HEAD 7f0a1e35c1d1c17de5873aded88d5dadfedce2fb builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c: In function 'rmnet_map_egress_handler':
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c:153:15: error: too few arguments to function 'rmnet_map_add_map_header'
     153 |  map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c:14:
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h:66:26: note: declared here
      66 | struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~
   At top level:
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h:76:11: warning: 'rmnet_map_get_next_hdr_type' defined but not used [-Wunused-function]
      76 | static u8 rmnet_map_get_next_hdr_type(struct sk_buff *skb)
         |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
--
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:270:26: error: conflicting types for 'rmnet_map_add_map_header'
     270 | struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:12:
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h:66:26: note: previous declaration of 'rmnet_map_add_map_header' was here
      66 | struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/rmnet_map_add_map_header +153 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c

ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  129  
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  130  static int rmnet_map_egress_handler(struct sk_buff *skb,
56470c927f1ba1 Subash Abhinov Kasiviswanathan 2017-10-11  131  				    struct rmnet_port *port, u8 mux_id,
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  132  				    struct net_device *orig_dev)
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  133  {
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  134  	int required_headroom, additional_header_len;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  135  	struct rmnet_map_header *map_header;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  136  
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  137  	additional_header_len = 0;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  138  	required_headroom = sizeof(struct rmnet_map_header);
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  139  
14452ca3b5ce30 Subash Abhinov Kasiviswanathan 2018-03-21  140  	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4) {
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  141  		additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  142  		required_headroom += additional_header_len;
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  143  	}
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  144  
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  145  	if (skb_headroom(skb) < required_headroom) {
6392ff3c8e4c23 Subash Abhinov Kasiviswanathan 2018-10-02  146  		if (pskb_expand_head(skb, required_headroom, 0, GFP_ATOMIC))
1eece799d3f611 Subash Abhinov Kasiviswanathan 2018-05-15  147  			return -ENOMEM;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  148  	}
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  149  
14452ca3b5ce30 Subash Abhinov Kasiviswanathan 2018-03-21  150  	if (port->data_format & RMNET_FLAGS_EGRESS_MAP_CKSUMV4)
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  151  		rmnet_map_checksum_uplink_packet(skb, orig_dev);
5eb5f8608ef118 Subash Abhinov Kasiviswanathan 2018-01-07  152  
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29 @153  	map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  154  	if (!map_header)
1eece799d3f611 Subash Abhinov Kasiviswanathan 2018-05-15  155  		return -ENOMEM;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  156  
56470c927f1ba1 Subash Abhinov Kasiviswanathan 2017-10-11  157  	map_header->mux_id = mux_id;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  158  
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  159  	skb->protocol = htons(ETH_P_MAP);
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  160  
cf2fe57b0cc220 Subash Abhinov Kasiviswanathan 2017-12-11  161  	return 0;
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  162  }
ceed73a2cf4aff Subash Abhinov Kasiviswanathan 2017-08-29  163  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 76425 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-12  2:04   ` Jakub Kicinski
@ 2021-02-12 14:01     ` Alex Elder
  2021-02-12 17:49       ` subashab
  2021-02-12 18:51       ` Jakub Kicinski
  0 siblings, 2 replies; 12+ messages in thread
From: Alex Elder @ 2021-02-12 14:01 UTC (permalink / raw)
  To: Jakub Kicinski, Sharath Chandra Vurukala
  Cc: davem, elder, cpratapa, subashab, netdev, linux-kernel

On 2/11/21 8:04 PM, Jakub Kicinski wrote:
> On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:
>> +/* MAP CSUM headers */
>> +struct rmnet_map_v5_csum_header {
>> +	u8  next_hdr:1;
>> +	u8  header_type:7;
>> +	u8  hw_reserved:5;
>> +	u8  priority:1;
>> +	u8  hw_reserved_bit:1;
>> +	u8  csum_valid_required:1;
>> +	__be16 reserved;
>> +} __aligned(1);
> 
> Will this work on big endian?

Sort of related to this point...

I'm sure the response to this will be to add two versions
of the definition, surrounded __LITTLE_ENDIAN_BITFIELD
and __BIG_ENDIAN_BITFIELD tests.

I really find this non-intuitive, and every time I
look at it I have to think about it a bit to figure
out where the bits actually lie in the word.

I know this pattern is used elsewhere in the networking
code, but that doesn't make it any easier for me to
understand...

Can we used mask, defined in host byte order, to
specify the positions of these fields?

I proposed a change at one time that did this and
this *_ENDIAN_BITFIELD thing was used instead.

I will gladly implement this change (completely
separate from what's being done here), but thought
it might be best to see what people think about it
before doing that work.

					-Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-12 14:01     ` Alex Elder
@ 2021-02-12 17:49       ` subashab
  2021-02-12 18:51       ` Jakub Kicinski
  1 sibling, 0 replies; 12+ messages in thread
From: subashab @ 2021-02-12 17:49 UTC (permalink / raw)
  To: Alex Elder
  Cc: Jakub Kicinski, Sharath Chandra Vurukala, davem, elder, cpratapa,
	netdev, linux-kernel

On 2021-02-12 07:01, Alex Elder wrote:
> On 2/11/21 8:04 PM, Jakub Kicinski wrote:
>> On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:
>>> +/* MAP CSUM headers */
>>> +struct rmnet_map_v5_csum_header {
>>> +	u8  next_hdr:1;
>>> +	u8  header_type:7;
>>> +	u8  hw_reserved:5;
>>> +	u8  priority:1;
>>> +	u8  hw_reserved_bit:1;
>>> +	u8  csum_valid_required:1;
>>> +	__be16 reserved;
>>> +} __aligned(1);
>> 
>> Will this work on big endian?
> 
> Sort of related to this point...
> 
> I'm sure the response to this will be to add two versions
> of the definition, surrounded __LITTLE_ENDIAN_BITFIELD
> and __BIG_ENDIAN_BITFIELD tests.
> 
> I really find this non-intuitive, and every time I
> look at it I have to think about it a bit to figure
> out where the bits actually lie in the word.
> 
> I know this pattern is used elsewhere in the networking
> code, but that doesn't make it any easier for me to
> understand...
> 
> Can we used mask, defined in host byte order, to
> specify the positions of these fields?
> 
> I proposed a change at one time that did this and
> this *_ENDIAN_BITFIELD thing was used instead.
> 
> I will gladly implement this change (completely
> separate from what's being done here), but thought
> it might be best to see what people think about it
> before doing that work.
> 
> 					-Alex

Our preference is to stick with __LITTLE_ENDIAN_BITFIELD
& __BIG_ENDIAN_BITFIELD definitions similar to other
networking definitions.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-12 14:01     ` Alex Elder
  2021-02-12 17:49       ` subashab
@ 2021-02-12 18:51       ` Jakub Kicinski
  2021-02-12 19:06         ` Alex Elder
  1 sibling, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2021-02-12 18:51 UTC (permalink / raw)
  To: Alex Elder
  Cc: Sharath Chandra Vurukala, davem, elder, cpratapa, subashab,
	netdev, linux-kernel

On Fri, 12 Feb 2021 08:01:15 -0600 Alex Elder wrote:
> On 2/11/21 8:04 PM, Jakub Kicinski wrote:
> > On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:  
> >> +/* MAP CSUM headers */
> >> +struct rmnet_map_v5_csum_header {
> >> +	u8  next_hdr:1;
> >> +	u8  header_type:7;
> >> +	u8  hw_reserved:5;
> >> +	u8  priority:1;
> >> +	u8  hw_reserved_bit:1;
> >> +	u8  csum_valid_required:1;
> >> +	__be16 reserved;
> >> +} __aligned(1);  
> > 
> > Will this work on big endian?  
> 
> Sort of related to this point...
> 
> I'm sure the response to this will be to add two versions
> of the definition, surrounded __LITTLE_ENDIAN_BITFIELD
> and __BIG_ENDIAN_BITFIELD tests.
> 
> I really find this non-intuitive, and every time I
> look at it I have to think about it a bit to figure
> out where the bits actually lie in the word.
> 
> I know this pattern is used elsewhere in the networking
> code, but that doesn't make it any easier for me to
> understand...
> 
> Can we used mask, defined in host byte order, to
> specify the positions of these fields?
> 
> I proposed a change at one time that did this and
> this *_ENDIAN_BITFIELD thing was used instead.
> 
> I will gladly implement this change (completely
> separate from what's being done here), but thought
> it might be best to see what people think about it
> before doing that work.

Most definitely agree, please convert.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-12 18:51       ` Jakub Kicinski
@ 2021-02-12 19:06         ` Alex Elder
  2021-02-12 20:11           ` subashab
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-02-12 19:06 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Sharath Chandra Vurukala, davem, elder, cpratapa, subashab,
	netdev, linux-kernel

On 2/12/21 12:51 PM, Jakub Kicinski wrote:
> On Fri, 12 Feb 2021 08:01:15 -0600 Alex Elder wrote:
>> On 2/11/21 8:04 PM, Jakub Kicinski wrote:
>>> On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:
>>>> +/* MAP CSUM headers */
>>>> +struct rmnet_map_v5_csum_header {
>>>> +	u8  next_hdr:1;
>>>> +	u8  header_type:7;
>>>> +	u8  hw_reserved:5;
>>>> +	u8  priority:1;
>>>> +	u8  hw_reserved_bit:1;
>>>> +	u8  csum_valid_required:1;
>>>> +	__be16 reserved;
>>>> +} __aligned(1);
>>>
>>> Will this work on big endian?
>>
>> Sort of related to this point...
>>
>> I'm sure the response to this will be to add two versions
>> of the definition, surrounded __LITTLE_ENDIAN_BITFIELD
>> and __BIG_ENDIAN_BITFIELD tests.
>>
>> I really find this non-intuitive, and every time I
>> look at it I have to think about it a bit to figure
>> out where the bits actually lie in the word.
>>
>> I know this pattern is used elsewhere in the networking
>> code, but that doesn't make it any easier for me to
>> understand...
>>
>> Can we used mask, defined in host byte order, to
>> specify the positions of these fields?
>>
>> I proposed a change at one time that did this and
>> this *_ENDIAN_BITFIELD thing was used instead.
>>
>> I will gladly implement this change (completely
>> separate from what's being done here), but thought
>> it might be best to see what people think about it
>> before doing that work.
> 
> Most definitely agree, please convert.

KS, would you like me to do this to the existing code
first?

I don't think it will take me very long.  If it were
a priority I could probably get it done by the end of
today, but I'd want to ensure the result worked for
the testing you do.

					-Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
  2021-02-12 19:06         ` Alex Elder
@ 2021-02-12 20:11           ` subashab
  0 siblings, 0 replies; 12+ messages in thread
From: subashab @ 2021-02-12 20:11 UTC (permalink / raw)
  To: Alex Elder
  Cc: Jakub Kicinski, Sharath Chandra Vurukala, davem, elder, cpratapa,
	netdev, linux-kernel

On 2021-02-12 12:06, Alex Elder wrote:
> On 2/12/21 12:51 PM, Jakub Kicinski wrote:
>> On Fri, 12 Feb 2021 08:01:15 -0600 Alex Elder wrote:
>>> On 2/11/21 8:04 PM, Jakub Kicinski wrote:
>>>> On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote:
>>>>> +/* MAP CSUM headers */
>>>>> +struct rmnet_map_v5_csum_header {
>>>>> +	u8  next_hdr:1;
>>>>> +	u8  header_type:7;
>>>>> +	u8  hw_reserved:5;
>>>>> +	u8  priority:1;
>>>>> +	u8  hw_reserved_bit:1;
>>>>> +	u8  csum_valid_required:1;
>>>>> +	__be16 reserved;
>>>>> +} __aligned(1);
>>>> 
>>>> Will this work on big endian?
>>> 
>>> Sort of related to this point...
>>> 
>>> I'm sure the response to this will be to add two versions
>>> of the definition, surrounded __LITTLE_ENDIAN_BITFIELD
>>> and __BIG_ENDIAN_BITFIELD tests.
>>> 
>>> I really find this non-intuitive, and every time I
>>> look at it I have to think about it a bit to figure
>>> out where the bits actually lie in the word.
>>> 
>>> I know this pattern is used elsewhere in the networking
>>> code, but that doesn't make it any easier for me to
>>> understand...
>>> 
>>> Can we used mask, defined in host byte order, to
>>> specify the positions of these fields?
>>> 
>>> I proposed a change at one time that did this and
>>> this *_ENDIAN_BITFIELD thing was used instead.
>>> 
>>> I will gladly implement this change (completely
>>> separate from what's being done here), but thought
>>> it might be best to see what people think about it
>>> before doing that work.
>> 
>> Most definitely agree, please convert.
> 
> KS, would you like me to do this to the existing code
> first?
> 
> I don't think it will take me very long.  If it were
> a priority I could probably get it done by the end of
> today, but I'd want to ensure the result worked for
> the testing you do.
> 
> 					-Alex

Sorry, I am not convinced that it is helping
to improve anything. It just adds a big
overhead of testing everything again without any
apparent improvement of performance or readablity
of code.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-02-12 20:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 21:35 [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5 Sharath Chandra Vurukala
2021-02-11 21:35 ` [PATCH 1/3] docs:networking: Add documentation for MAP v5 Sharath Chandra Vurukala
2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
2021-02-12  2:04   ` Jakub Kicinski
2021-02-12 14:01     ` Alex Elder
2021-02-12 17:49       ` subashab
2021-02-12 18:51       ` Jakub Kicinski
2021-02-12 19:06         ` Alex Elder
2021-02-12 20:11           ` subashab
2021-02-12  4:53   ` kernel test robot
2021-02-11 21:35 ` [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet Sharath Chandra Vurukala
2021-02-12  2:25   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).