mbox series

[net-next,v7,0/3] net: qualcomm: rmnet: Enable Mapv5

Message ID 1622105322-2975-1-git-send-email-sharathv@codeaurora.org
Headers show
Series net: qualcomm: rmnet: Enable Mapv5 | expand

Message

Sharath Chandra Vurukala May 27, 2021, 8:48 a.m. UTC
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(-)

Comments

Subash Abhinov Kasiviswanathan May 28, 2021, 8 a.m. UTC | #1
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 <subashab@codeaurora.org>
Jakub Kicinski May 28, 2021, 10:58 p.m. UTC | #2
On Thu, 27 May 2021 14:18:41 +0530 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>


> @@ -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;

> +	void *data = skb->data;

>  	struct rmnet_map_header *maph;


Please maintain reverse xmas tree ordering

>  	struct sk_buff *skbn;

> +	u8 nexthdr_type;

>  	u32 packet_len;

>  

>  	if (skb->len == 0)

> @@ -310,8 +314,18 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,

>  	maph = (struct rmnet_map_header *)skb->data;

>  	packet_len = ntohs(maph->pkt_len) + sizeof(*maph);

>  

> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)

> +	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(*next_hdr);

> +			if (maph->flags & MAP_NEXT_HEADER_FLAG)

> +				next_hdr = (data + sizeof(*maph));


brackets unnecessary

> +			else

> +				/* Mapv5 data pkt without csum hdr is invalid */

> +				return NULL;

> +		}

> +	}

>  

>  	if (((int)skb->len - (int)packet_len) < 0)

>  		return NULL;

> @@ -320,6 +334,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 +435,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;


rc is not meaningfully used

> +	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(*next_hdr));

> +	} else {

> +		return -EINVAL;


flip condition, return early

> +	}

> +

> +	return rc;

> +}

> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h

> index 4efb537..8502ccc 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


Colon missing?

>   */

>  #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,26 @@ 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);


__aligned() seems rather pointless here but ok.

> +/* v5 header_info field

> + * NEXT_HEADER:  Represents whether there is any other header


double space

> + * HEADER TYPE: represents the type of this header


On previous line you used _ for a space, and started from capital
letter. Please be consistent.

> + *

> + * 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_FMASK	GENMASK(7, 1)

> +#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)

> +

> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2

>  #endif /* !(_LINUX_IF_RMNET_H_) */
Alex Elder May 31, 2021, 2:34 p.m. UTC | #3
On 5/28/21 5:58 PM, Jakub Kicinski wrote:
> On Thu, 27 May 2021 14:18:41 +0530 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.


Nice review Jakub.  I will wait for version 8 and will review that.

					-Alex

>>

>> 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>

> 

>> @@ -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;

>> +	void *data = skb->data;

>>  	struct rmnet_map_header *maph;

> 

> Please maintain reverse xmas tree ordering

> 

>>  	struct sk_buff *skbn;

>> +	u8 nexthdr_type;

>>  	u32 packet_len;

>>  

>>  	if (skb->len == 0)

>> @@ -310,8 +314,18 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,

>>  	maph = (struct rmnet_map_header *)skb->data;

>>  	packet_len = ntohs(maph->pkt_len) + sizeof(*maph);

>>  

>> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)

>> +	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(*next_hdr);

>> +			if (maph->flags & MAP_NEXT_HEADER_FLAG)

>> +				next_hdr = (data + sizeof(*maph));

> 

> brackets unnecessary

> 

>> +			else

>> +				/* Mapv5 data pkt without csum hdr is invalid */

>> +				return NULL;

>> +		}

>> +	}

>>  

>>  	if (((int)skb->len - (int)packet_len) < 0)

>>  		return NULL;

>> @@ -320,6 +334,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 +435,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;

> 

> rc is not meaningfully used

> 

>> +	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(*next_hdr));

>> +	} else {

>> +		return -EINVAL;

> 

> flip condition, return early

> 

>> +	}

>> +

>> +	return rc;

>> +}

>> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h

>> index 4efb537..8502ccc 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

> 

> Colon missing?

> 

>>   */

>>  #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,26 @@ 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);

> 

> __aligned() seems rather pointless here but ok.

> 

>> +/* v5 header_info field

>> + * NEXT_HEADER:  Represents whether there is any other header

> 

> double space

> 

>> + * HEADER TYPE: represents the type of this header

> 

> On previous line you used _ for a space, and started from capital

> letter. Please be consistent.

> 

>> + *

>> + * 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_FMASK	GENMASK(7, 1)

>> +#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)

>> +

>> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2

>>  #endif /* !(_LINUX_IF_RMNET_H_) */
Sharath Chandra Vurukala June 1, 2021, 7:06 p.m. UTC | #4
On 2021-05-29 04:28, Jakub Kicinski wrote:
> On Thu, 27 May 2021 14:18:41 +0530 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>

> 

>> @@ -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;

>> +	void *data = skb->data;

>>  	struct rmnet_map_header *maph;

> 

> Please maintain reverse xmas tree ordering


Thanks Jakub for the review, I will address all the comments that you 
gave in the next subsequent patch.
> 

>>  	struct sk_buff *skbn;

>> +	u8 nexthdr_type;

>>  	u32 packet_len;

>> 

>>  	if (skb->len == 0)

>> @@ -310,8 +314,18 @@ struct sk_buff *rmnet_map_deaggregate(struct 

>> sk_buff *skb,

>>  	maph = (struct rmnet_map_header *)skb->data;

>>  	packet_len = ntohs(maph->pkt_len) + sizeof(*maph);

>> 

>> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)

>> +	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(*next_hdr);

>> +			if (maph->flags & MAP_NEXT_HEADER_FLAG)

>> +				next_hdr = (data + sizeof(*maph));

> 

> brackets unnecessary

> 

Will take care in next patch.
>> +			else

>> +				/* Mapv5 data pkt without csum hdr is invalid */

>> +				return NULL;

>> +		}

>> +	}

>> 

>>  	if (((int)skb->len - (int)packet_len) < 0)

>>  		return NULL;

>> @@ -320,6 +334,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 +435,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;

> 

> rc is not meaningfully used

> 

>> +	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(*next_hdr));

>> +	} else {

>> +		return -EINVAL;

> 

> flip condition, return early

> 

Sure will take care in next patch.
>> +	}

>> +

>> +	return rc;

>> +}

>> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h

>> index 4efb537..8502ccc 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

> 

> Colon missing?

> 

>>   */

>>  #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,26 @@ 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);

> 

> __aligned() seems rather pointless here but ok.

> 

>> +/* v5 header_info field

>> + * NEXT_HEADER:  Represents whether there is any other header

> 

> double space

> 

>> + * HEADER TYPE: represents the type of this header

> 

> On previous line you used _ for a space, and started from capital

> letter. Please be consistent.

> 


Sure, will take care in next patch.

>> + *

>> + * 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_FMASK	GENMASK(7, 1)

>> +#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)

>> +

>> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2

>>  #endif /* !(_LINUX_IF_RMNET_H_) */