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.
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.
v4->v5:
- Corrected checkpatch errors and warnings reported by patchwork.
v5->v6:
- Corrected the bug identified by Alex and incorporated all his comments.
v6->v7:
- Removed duplicate inclusion of linux/bitfield.h in rmnet_map_data.c
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 | 34 +++--
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 | 26 +++-
include/uapi/linux/if_link.h | 2 +
8 files changed, 320 insertions(+), 37 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Adding support for MAPv5 egress packets.
This involves adding the MAPv5 header and setting the csum_valid_required
in the checksum header to request HW compute the checksum.
Corresponding stats are incremented based on whether the checksum is
computed in software or HW.
New stat has been added which represents the count of packets whose
checksum is calculated by the HW.
Signed-off-by: Sharath Chandra Vurukala <[email protected]>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 17 ++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 8 +-
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 92 ++++++++++++++++++++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +-
include/uapi/linux/if_link.h | 1 +
6 files changed, 110 insertions(+), 15 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..03b3321 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;
@@ -141,18 +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 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 2280703..a375a46 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -251,12 +251,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 = u8_encode_bits(RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD,
+ MAPV5_HDRINFO_HDR_TYPE_FMASK);
+
+ 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 +324,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;
@@ -393,11 +454,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;
@@ -416,10 +474,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++;
@@ -436,6 +496,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
Adding documentation explaining the new MAPv4/v5 packet formats
and the corresponding checksum offload headers.
Signed-off-by: Sharath Chandra Vurukala <[email protected]>
---
.../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
On 2021-05-27 02:48, 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.
>
> 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.
>
> v4->v5:
> - Corrected checkpatch errors and warnings reported by patchwork.
>
> v5->v6:
> - Corrected the bug identified by Alex and incorporated all his
> comments.
>
> v6->v7:
> - Removed duplicate inclusion of linux/bitfield.h in
> rmnet_map_data.c
>
> 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 | 34 +++--
> 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 | 26 +++-
> include/uapi/linux/if_link.h | 2 +
> 8 files changed, 320 insertions(+), 37 deletions(-)
For the series-
Reviewed-by: Subash Abhinov Kasiviswanathan <[email protected]>
On Thu, 27 May 2021 14:18:42 +0530 Sharath Chandra Vurukala wrote:
> Adding support for MAPv5 egress packets.
>
> This involves adding the MAPv5 header and setting the csum_valid_required
> in the checksum header to request HW compute the checksum.
>
> Corresponding stats are incremented based on whether the checksum is
> computed in software or HW.
>
> New stat has been added which represents the count of packets whose
> checksum is calculated by the HW.
>
> Signed-off-by: Sharath Chandra Vurukala <[email protected]>
> +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;
how can we get here if this condition is not met? Looks like defensive
programming.
> + ul_header = skb_push(skb, sizeof(*ul_header));
Are you making sure you can modify head? I only see a check if there is
enough headroom but not if head is writable (skb_cow_head()).
> + memset(ul_header, 0, sizeof(*ul_header));
> + ul_header->header_info = u8_encode_bits(RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD,
> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
Is prepending the header required even when packet doesn't need
checksuming?
> + if (skb->ip_summed == CHECKSUM_PARTIAL) {
> + void *iph = (char *)ul_header + sizeof(*ul_header);
ip_hdr(skb)
> + __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;
Please try to keep the success path unindented.
> + }
> + }
> +
> +sw_csum:
> + priv->stats.csum_sw++;
> +}
On 2021-05-29 04:41, Jakub Kicinski wrote:
> On Thu, 27 May 2021 14:18:42 +0530 Sharath Chandra Vurukala wrote:
>> Adding support for MAPv5 egress packets.
>>
>> This involves adding the MAPv5 header and setting the
>> csum_valid_required
>> in the checksum header to request HW compute the checksum.
>>
>> Corresponding stats are incremented based on whether the checksum is
>> computed in software or HW.
>>
>> New stat has been added which represents the count of packets whose
>> checksum is calculated by the HW.
>>
>> Signed-off-by: Sharath Chandra Vurukala <[email protected]>
>
>> +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;
>
> how can we get here if this condition is not met? Looks like defensive
> programming.
>
Yes we get here only for the MAPv5 case, as you think this is just a
defensive code.
will remove this in next patch.
>> + ul_header = skb_push(skb, sizeof(*ul_header));
>
> Are you making sure you can modify head? I only see a check if there is
> enough headroom but not if head is writable (skb_cow_head()).
>
TSkb_cow_head() changes will be done in the rmnet_map_egress_handler()
in the next patch.
>> + memset(ul_header, 0, sizeof(*ul_header));
>> + ul_header->header_info =
>> u8_encode_bits(RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD,
>> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
>
> Is prepending the header required even when packet doesn't need
> checksuming?
>
>> + if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> + void *iph = (char *)ul_header + sizeof(*ul_header);
>
> ip_hdr(skb)
>
>> + __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;
>
> Please try to keep the success path unindented.
>
Sure will take care of these comments in next patch.
>> + }
>> + }
>> +
>> +sw_csum:
>> + priv->stats.csum_sw++;
>> +}