diff mbox series

[net-next,v5,2/3] net: ethernet: rmnet: Support for ingress MAPv5 checksum offload

Message ID 1619180343-3943-3-git-send-email-sharathv@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: qualcomm: rmnet: Enable Mapv5 | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: stranche@codeaurora.org subashab@codeaurora.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 6720 this patch: 6720
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 180 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 6933 this patch: 6933
netdev/header_inline success Link

Commit Message

Sharath Chandra Vurukala April 23, 2021, 12:19 p.m. UTC 
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.

Acked-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Acked-by: Alex Elder <elder@linaro.org>
Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 17 ++++---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  3 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 58 +++++++++++++++++++++-
 include/linux/if_rmnet.h                           | 27 +++++++++-
 include/uapi/linux/if_link.h                       |  1 +
 5 files changed, 97 insertions(+), 9 deletions(-)

Comments

Alex Elder April 23, 2021, 12:48 p.m. UTC | #1 
On 4/23/21 7:19 AM, 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.
> 
> Acked-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> Acked-by: Alex Elder <elder@linaro.org>

I did not acknowledge this message.  I did for patch 1
only, which only updates the documentation.

I have a few other minor things I recommend you fix.

I don't see any bugs, but I'm not going to offer
"Reviewed-by" until you've had a chance to either
update your patch or explain why you won't.

> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
> ---
>   .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 17 ++++---
>   drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  3 +-
>   .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 58 +++++++++++++++++++++-
>   include/linux/if_rmnet.h                           | 27 +++++++++-
>   include/uapi/linux/if_link.h                       |  1 +
>   5 files changed, 97 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 0be5ac7..706a225 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> @@ -1,5 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
>    *
>    * RMNET Data ingress/egress handler
>    */
> @@ -82,11 +82,16 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
>   
>   	skb->dev = ep->egress_dev;
>   
> -	/* Subtract MAP header */
> -	skb_pull(skb, sizeof(struct rmnet_map_header));
> -	rmnet_set_skb_proto(skb);
> -
> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
> +	if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
> +	    (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
> +		if (rmnet_map_process_next_hdr_packet(skb, len))
> +			goto free_skb;
> +		skb_pull(skb, sizeof(*map_header));
> +		rmnet_set_skb_proto(skb);
> +	} else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
> +		/* Subtract MAP header */
> +		skb_pull(skb, sizeof(*map_header));
> +		rmnet_set_skb_proto(skb);
>   		if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
>   			skb->ip_summed = CHECKSUM_UNNECESSARY;
>   	}
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> index 2aea153..1a399bf 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
>    */
>   
>   #ifndef _RMNET_MAP_H_
> @@ -48,5 +48,6 @@ void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
>   int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
>   void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>   				      struct net_device *orig_dev);
> +int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
>   
>   #endif /* _RMNET_MAP_H_ */
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> index 0ac2ff8..43813cf 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -1,5 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
>    *
>    * RMNET Data MAP protocol
>    */
> @@ -8,6 +8,7 @@
>   #include <linux/ip.h>
>   #include <linux/ipv6.h>
>   #include <net/ip6_checksum.h>
> +#include <linux/bitfield.h>
>   #include "rmnet_config.h"
>   #include "rmnet_map.h"
>   #include "rmnet_private.h"
> @@ -300,8 +301,11 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
>   struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
>   				      struct rmnet_port *port)
>   {
> +	struct rmnet_map_v5_csum_header *next_hdr = NULL;
> +	unsigned char *data = skb->data;

If you define the data variable to have (void *) type you can
get rid of some casts below, and it won't need to use a cast
when assigned here either.

>   	struct rmnet_map_header *maph;
>   	struct sk_buff *skbn;
> +	u8 nexthdr_type;
>   	u32 packet_len;
>   
>   	if (skb->len == 0)
> @@ -312,6 +316,17 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
>   
>   	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
>   		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);

This block should be surrounded by curly braces.  If one block
in an if statement (or chain of them) has them, they all should.

> +	else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
> +		if (!(maph->flags & MAP_CMD_FLAG)) {
> +			packet_len += sizeof(struct rmnet_map_v5_csum_header);

			packet_len += sizeof(*next_hdr);

> +			if (maph->flags & MAP_NEXT_HEADER_FLAG)
> +				next_hdr = (struct rmnet_map_v5_csum_header *)
> +						(data + sizeof(*maph));

If data is a void pointer, this could be:
				next_hdr = data + sizeof(*maph);

> +			else
> +				/* Mapv5 data pkt without csum hdr is invalid */
> +				return NULL;
> +		}
> +	}
>   
>   	if (((int)skb->len - (int)packet_len) < 0)
>   		return NULL;
> @@ -320,6 +335,13 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
>   	if (!maph->pkt_len)
>   		return NULL;
>   
> +	if (next_hdr) {
> +		nexthdr_type = u8_get_bits(next_hdr->header_info,
> +					   MAPV5_HDRINFO_HDR_TYPE_FMASK);
> +		if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
> +			return NULL;
> +	}
> +
>   	skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
>   	if (!skbn)
>   		return NULL;
> @@ -414,3 +436,37 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>   
>   	priv->stats.csum_sw++;
>   }
> +
> +/* Process a MAPv5 packet header */
> +int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
> +				      u16 len)
> +{
> +	struct rmnet_priv *priv = netdev_priv(skb->dev);
> +	struct rmnet_map_v5_csum_header *next_hdr;
> +	u8 nexthdr_type;
> +	int rc = 0;
> +
> +	next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
> +			sizeof(struct rmnet_map_header));
> +
> +	nexthdr_type = u8_get_bits(next_hdr->header_info,
> +				   MAPV5_HDRINFO_HDR_TYPE_FMASK);
> +
> +	if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
> +		if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
> +			priv->stats.csum_sw++;
> +		} else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
> +			priv->stats.csum_ok++;
> +			skb->ip_summed = CHECKSUM_UNNECESSARY;
> +		} else {
> +			priv->stats.csum_valid_unset++;
> +		}
> +
> +		/* Pull csum v5 header */
> +		skb_pull(skb, sizeof(struct rmnet_map_v5_csum_header));

         skb_pull(skb, sizeof(*next_hdr);

> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return rc;
> +}
> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
> index 4efb537..f82e37e 100644
> --- a/include/linux/if_rmnet.h
> +++ b/include/linux/if_rmnet.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: GPL-2.0-only
> - * Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2013-2019, 2021 The Linux Foundation. All rights reserved.
>    */
>   
>   #ifndef _LINUX_IF_RMNET_H_
> @@ -14,8 +14,10 @@ struct rmnet_map_header {
>   /* rmnet_map_header flags field:
>    *  PAD_LEN:	number of pad bytes following packet data
>    *  CMD:	1 = packet contains a MAP command; 0 = packet contains data
> + *  NEXT_HEADER	1 = packet contains V5 CSUM header 0 = no V5 CSUM header
>    */
>   #define MAP_PAD_LEN_MASK		GENMASK(5, 0)
> +#define MAP_NEXT_HEADER_FLAG		BIT(6)
>   #define MAP_CMD_FLAG			BIT(7)
>   
>   struct rmnet_map_dl_csum_trailer {
> @@ -45,4 +47,27 @@ struct rmnet_map_ul_csum_header {
>   #define MAP_CSUM_UL_UDP_FLAG		BIT(14)
>   #define MAP_CSUM_UL_ENABLED_FLAG	BIT(15)
>   
> +/* MAP CSUM headers */
> +struct rmnet_map_v5_csum_header {
> +	u8 header_info;
> +	u8 csum_info;
> +	__be16 reserved;
> +} __aligned(1);
> +
> +/* v5 header_info field
> + * NEXT_HEADER:  Represents whether there is any other header
> + * HEADER TYPE: represents the type of this header
> + *
> + * csum_info field
> + * CSUM_VALID_OR_REQ:
> + * 1 = for UL, checksum computation is requested.
> + * 1 = for DL, validated the checksum and has found it valid
> + */
> +
> +#define MAPV5_HDRINFO_NXT_HDR_FLAG	BIT(0)
> +#define MAPV5_HDRINFO_HDR_TYPE_SHIFT	1
> +#define MAPV5_HDRINFO_HDR_TYPE_FMASK	GENMASK(7, MAPV5_HDRINFO_HDR_TYPE_SHIFT)
> +#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)

This is the only place you use MAPV5_HDRINFO_TYPE_SHIFT.
Defining and using it here immediately after its definition
subtracts, rather than adds value.  Just do:

#define MAPV5_HDRINFO_HDR_TYPE_FMASK    GENMASK(7, 1)

					-Alex

> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
>   #endif /* !(_LINUX_IF_RMNET_H_) */
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 91c8dda..21529b3 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1235,6 +1235,7 @@ enum {
>   #define RMNET_FLAGS_INGRESS_MAP_COMMANDS          (1U << 1)
>   #define RMNET_FLAGS_INGRESS_MAP_CKSUMV4           (1U << 2)
>   #define RMNET_FLAGS_EGRESS_MAP_CKSUMV4            (1U << 3)
> +#define RMNET_FLAGS_INGRESS_MAP_CKSUMV5           (1U << 4)
>   
>   enum {
>   	IFLA_RMNET_UNSPEC,
>
Sharath Chandra Vurukala May 26, 2021, 2:28 p.m. UTC | #2 
On 2021-04-23 18:18, Alex Elder wrote:
> On 4/23/21 7:19 AM, 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.
>> 
>> Acked-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
>> Acked-by: Alex Elder <elder@linaro.org>
> 
> I did not acknowledge this message.  I did for patch 1
> only, which only updates the documentation.
> 
> I have a few other minor things I recommend you fix.
> 
> I don't see any bugs, but I'm not going to offer
> "Reviewed-by" until you've had a chance to either
> update your patch or explain why you won't.
> 
Sorry Alex for Adding Acked-by on all the patches, will remove in the 
subsequent patch.

>> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
>> ---
>>   .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 17 ++++---
>>   drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  3 +-
>>   .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 58 
>> +++++++++++++++++++++-
>>   include/linux/if_rmnet.h                           | 27 +++++++++-
>>   include/uapi/linux/if_link.h                       |  1 +
>>   5 files changed, 97 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c 
>> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
>> index 0be5ac7..706a225 100644
>> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
>> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
>> @@ -1,5 +1,5 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights 
>> reserved.
>> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights 
>> reserved.
>>    *
>>    * RMNET Data ingress/egress handler
>>    */
>> @@ -82,11 +82,16 @@ __rmnet_map_ingress_handler(struct sk_buff *skb,
>>     	skb->dev = ep->egress_dev;
>>   -	/* Subtract MAP header */
>> -	skb_pull(skb, sizeof(struct rmnet_map_header));
>> -	rmnet_set_skb_proto(skb);
>> -
>> -	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
>> +	if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
>> +	    (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
>> +		if (rmnet_map_process_next_hdr_packet(skb, len))
>> +			goto free_skb;
>> +		skb_pull(skb, sizeof(*map_header));
>> +		rmnet_set_skb_proto(skb);
>> +	} else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
>> +		/* Subtract MAP header */
>> +		skb_pull(skb, sizeof(*map_header));
>> +		rmnet_set_skb_proto(skb);
>>   		if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
>>   			skb->ip_summed = CHECKSUM_UNNECESSARY;
>>   	}
>> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h 
>> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
>> index 2aea153..1a399bf 100644
>> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
>> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
>> @@ -1,5 +1,5 @@
>>   /* SPDX-License-Identifier: GPL-2.0-only */
>> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights 
>> reserved.
>> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights 
>> reserved.
>>    */
>>     #ifndef _RMNET_MAP_H_
>> @@ -48,5 +48,6 @@ void rmnet_map_command(struct sk_buff *skb, struct 
>> rmnet_port *port);
>>   int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 
>> len);
>>   void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>>   				      struct net_device *orig_dev);
>> +int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
>>     #endif /* _RMNET_MAP_H_ */
>> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c 
>> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> index 0ac2ff8..43813cf 100644
>> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> @@ -1,5 +1,5 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>> -/* Copyright (c) 2013-2018, The Linux Foundation. All rights 
>> reserved.
>> +/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights 
>> reserved.
>>    *
>>    * RMNET Data MAP protocol
>>    */
>> @@ -8,6 +8,7 @@
>>   #include <linux/ip.h>
>>   #include <linux/ipv6.h>
>>   #include <net/ip6_checksum.h>
>> +#include <linux/bitfield.h>
>>   #include "rmnet_config.h"
>>   #include "rmnet_map.h"
>>   #include "rmnet_private.h"
>> @@ -300,8 +301,11 @@ struct rmnet_map_header 
>> *rmnet_map_add_map_header(struct sk_buff *skb,
>>   struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
>>   				      struct rmnet_port *port)
>>   {
>> +	struct rmnet_map_v5_csum_header *next_hdr = NULL;
>> +	unsigned char *data = skb->data;
> 
> If you define the data variable to have (void *) type you can
> get rid of some casts below, and it won't need to use a cast
> when assigned here either.
> 

I will correct this and the all the comments related to the "data" 
variable in the subsequent patch.


>>   	struct rmnet_map_header *maph;
>>   	struct sk_buff *skbn;
>> +	u8 nexthdr_type;
>>   	u32 packet_len;
>>     	if (skb->len == 0)
>> @@ -312,6 +316,17 @@ struct sk_buff *rmnet_map_deaggregate(struct 
>> sk_buff *skb,
>>     	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
>>   		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
> 
> This block should be surrounded by curly braces.  If one block
> in an if statement (or chain of them) has them, they all should.
> 
Will correct this in the next patch.

>> +	else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
>> +		if (!(maph->flags & MAP_CMD_FLAG)) {
>> +			packet_len += sizeof(struct rmnet_map_v5_csum_header);
> 
> 			packet_len += sizeof(*next_hdr);
> 
>> +			if (maph->flags & MAP_NEXT_HEADER_FLAG)
>> +				next_hdr = (struct rmnet_map_v5_csum_header *)
>> +						(data + sizeof(*maph));
> 
> If data is a void pointer, this could be:
> 				next_hdr = data + sizeof(*maph);
> 
>> +			else
>> +				/* Mapv5 data pkt without csum hdr is invalid */
>> +				return NULL;
>> +		}
>> +	}
>>     	if (((int)skb->len - (int)packet_len) < 0)
>>   		return NULL;
>> @@ -320,6 +335,13 @@ struct sk_buff *rmnet_map_deaggregate(struct 
>> sk_buff *skb,
>>   	if (!maph->pkt_len)
>>   		return NULL;
>>   +	if (next_hdr) {
>> +		nexthdr_type = u8_get_bits(next_hdr->header_info,
>> +					   MAPV5_HDRINFO_HDR_TYPE_FMASK);
>> +		if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
>> +			return NULL;
>> +	}
>> +
>>   	skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, 
>> GFP_ATOMIC);
>>   	if (!skbn)
>>   		return NULL;
>> @@ -414,3 +436,37 @@ void rmnet_map_checksum_uplink_packet(struct 
>> sk_buff *skb,
>>     	priv->stats.csum_sw++;
>>   }
>> +
>> +/* Process a MAPv5 packet header */
>> +int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
>> +				      u16 len)
>> +{
>> +	struct rmnet_priv *priv = netdev_priv(skb->dev);
>> +	struct rmnet_map_v5_csum_header *next_hdr;
>> +	u8 nexthdr_type;
>> +	int rc = 0;
>> +
>> +	next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
>> +			sizeof(struct rmnet_map_header));
>> +
>> +	nexthdr_type = u8_get_bits(next_hdr->header_info,
>> +				   MAPV5_HDRINFO_HDR_TYPE_FMASK);
>> +
>> +	if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
>> +		if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
>> +			priv->stats.csum_sw++;
>> +		} else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
>> +			priv->stats.csum_ok++;
>> +			skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +		} else {
>> +			priv->stats.csum_valid_unset++;
>> +		}
>> +
>> +		/* Pull csum v5 header */
>> +		skb_pull(skb, sizeof(struct rmnet_map_v5_csum_header));
> 
>         skb_pull(skb, sizeof(*next_hdr);
> 
>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>> +	return rc;
>> +}
>> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
>> index 4efb537..f82e37e 100644
>> --- a/include/linux/if_rmnet.h
>> +++ b/include/linux/if_rmnet.h
>> @@ -1,5 +1,5 @@
>>   /* SPDX-License-Identifier: GPL-2.0-only
>> - * Copyright (c) 2013-2019, The Linux Foundation. All rights 
>> reserved.
>> + * Copyright (c) 2013-2019, 2021 The Linux Foundation. All rights 
>> reserved.
>>    */
>>     #ifndef _LINUX_IF_RMNET_H_
>> @@ -14,8 +14,10 @@ struct rmnet_map_header {
>>   /* rmnet_map_header flags field:
>>    *  PAD_LEN:	number of pad bytes following packet data
>>    *  CMD:	1 = packet contains a MAP command; 0 = packet contains data
>> + *  NEXT_HEADER	1 = packet contains V5 CSUM header 0 = no V5 CSUM 
>> header
>>    */
>>   #define MAP_PAD_LEN_MASK		GENMASK(5, 0)
>> +#define MAP_NEXT_HEADER_FLAG		BIT(6)
>>   #define MAP_CMD_FLAG			BIT(7)
>>     struct rmnet_map_dl_csum_trailer {
>> @@ -45,4 +47,27 @@ struct rmnet_map_ul_csum_header {
>>   #define MAP_CSUM_UL_UDP_FLAG		BIT(14)
>>   #define MAP_CSUM_UL_ENABLED_FLAG	BIT(15)
>>   +/* MAP CSUM headers */
>> +struct rmnet_map_v5_csum_header {
>> +	u8 header_info;
>> +	u8 csum_info;
>> +	__be16 reserved;
>> +} __aligned(1);
>> +
>> +/* v5 header_info field
>> + * NEXT_HEADER:  Represents whether there is any other header
>> + * HEADER TYPE: represents the type of this header
>> + *
>> + * csum_info field
>> + * CSUM_VALID_OR_REQ:
>> + * 1 = for UL, checksum computation is requested.
>> + * 1 = for DL, validated the checksum and has found it valid
>> + */
>> +
>> +#define MAPV5_HDRINFO_NXT_HDR_FLAG	BIT(0)
>> +#define MAPV5_HDRINFO_HDR_TYPE_SHIFT	1
>> +#define MAPV5_HDRINFO_HDR_TYPE_FMASK	GENMASK(7, 
>> MAPV5_HDRINFO_HDR_TYPE_SHIFT)
>> +#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)
> 
> This is the only place you use MAPV5_HDRINFO_TYPE_SHIFT.
> Defining and using it here immediately after its definition
> subtracts, rather than adds value.  Just do:
> 
> #define MAPV5_HDRINFO_HDR_TYPE_FMASK    GENMASK(7, 1)
> 
> 					-Alex
> 

The MAPV5_HDRINFO_HDR_TYPE_SHIFT is used in the uplink to set the 
header_type in the
header_info field. Will use u8_encode_bits and get rid of this in next 
patch.

>> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
>>   #endif /* !(_LINUX_IF_RMNET_H_) */
>> diff --git a/include/uapi/linux/if_link.h 
>> b/include/uapi/linux/if_link.h
>> index 91c8dda..21529b3 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -1235,6 +1235,7 @@ enum {
>>   #define RMNET_FLAGS_INGRESS_MAP_COMMANDS          (1U << 1)
>>   #define RMNET_FLAGS_INGRESS_MAP_CKSUMV4           (1U << 2)
>>   #define RMNET_FLAGS_EGRESS_MAP_CKSUMV4            (1U << 3)
>> +#define RMNET_FLAGS_INGRESS_MAP_CKSUMV5           (1U << 4)
>>     enum {
>>   	IFLA_RMNET_UNSPEC,
>>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 0be5ac7..706a225 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -1,5 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
  *
  * RMNET Data ingress/egress handler
  */
@@ -82,11 +82,16 @@  __rmnet_map_ingress_handler(struct sk_buff *skb,
 
 	skb->dev = ep->egress_dev;
 
-	/* Subtract MAP header */
-	skb_pull(skb, sizeof(struct rmnet_map_header));
-	rmnet_set_skb_proto(skb);
-
-	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
+	if ((port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) &&
+	    (map_header->flags & MAP_NEXT_HEADER_FLAG)) {
+		if (rmnet_map_process_next_hdr_packet(skb, len))
+			goto free_skb;
+		skb_pull(skb, sizeof(*map_header));
+		rmnet_set_skb_proto(skb);
+	} else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
+		/* Subtract MAP header */
+		skb_pull(skb, sizeof(*map_header));
+		rmnet_set_skb_proto(skb);
 		if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 2aea153..1a399bf 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
  */
 
 #ifndef _RMNET_MAP_H_
@@ -48,5 +48,6 @@  void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
 int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
 void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
 				      struct net_device *orig_dev);
+int rmnet_map_process_next_hdr_packet(struct sk_buff *skb, u16 len);
 
 #endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 0ac2ff8..43813cf 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -1,5 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2013-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2013-2018, 2021, The Linux Foundation. All rights reserved.
  *
  * RMNET Data MAP protocol
  */
@@ -8,6 +8,7 @@ 
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <net/ip6_checksum.h>
+#include <linux/bitfield.h>
 #include "rmnet_config.h"
 #include "rmnet_map.h"
 #include "rmnet_private.h"
@@ -300,8 +301,11 @@  struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
 				      struct rmnet_port *port)
 {
+	struct rmnet_map_v5_csum_header *next_hdr = NULL;
+	unsigned char *data = skb->data;
 	struct rmnet_map_header *maph;
 	struct sk_buff *skbn;
+	u8 nexthdr_type;
 	u32 packet_len;
 
 	if (skb->len == 0)
@@ -312,6 +316,17 @@  struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
 
 	if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
 		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
+	else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
+		if (!(maph->flags & MAP_CMD_FLAG)) {
+			packet_len += sizeof(struct rmnet_map_v5_csum_header);
+			if (maph->flags & MAP_NEXT_HEADER_FLAG)
+				next_hdr = (struct rmnet_map_v5_csum_header *)
+						(data + sizeof(*maph));
+			else
+				/* Mapv5 data pkt without csum hdr is invalid */
+				return NULL;
+		}
+	}
 
 	if (((int)skb->len - (int)packet_len) < 0)
 		return NULL;
@@ -320,6 +335,13 @@  struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
 	if (!maph->pkt_len)
 		return NULL;
 
+	if (next_hdr) {
+		nexthdr_type = u8_get_bits(next_hdr->header_info,
+					   MAPV5_HDRINFO_HDR_TYPE_FMASK);
+		if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
+			return NULL;
+	}
+
 	skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
 	if (!skbn)
 		return NULL;
@@ -414,3 +436,37 @@  void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
 
 	priv->stats.csum_sw++;
 }
+
+/* Process a MAPv5 packet header */
+int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
+				      u16 len)
+{
+	struct rmnet_priv *priv = netdev_priv(skb->dev);
+	struct rmnet_map_v5_csum_header *next_hdr;
+	u8 nexthdr_type;
+	int rc = 0;
+
+	next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
+			sizeof(struct rmnet_map_header));
+
+	nexthdr_type = u8_get_bits(next_hdr->header_info,
+				   MAPV5_HDRINFO_HDR_TYPE_FMASK);
+
+	if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
+		if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
+			priv->stats.csum_sw++;
+		} else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
+			priv->stats.csum_ok++;
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		} else {
+			priv->stats.csum_valid_unset++;
+		}
+
+		/* Pull csum v5 header */
+		skb_pull(skb, sizeof(struct rmnet_map_v5_csum_header));
+	} else {
+		return -EINVAL;
+	}
+
+	return rc;
+}
diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
index 4efb537..f82e37e 100644
--- a/include/linux/if_rmnet.h
+++ b/include/linux/if_rmnet.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only
- * Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013-2019, 2021 The Linux Foundation. All rights reserved.
  */
 
 #ifndef _LINUX_IF_RMNET_H_
@@ -14,8 +14,10 @@  struct rmnet_map_header {
 /* rmnet_map_header flags field:
  *  PAD_LEN:	number of pad bytes following packet data
  *  CMD:	1 = packet contains a MAP command; 0 = packet contains data
+ *  NEXT_HEADER	1 = packet contains V5 CSUM header 0 = no V5 CSUM header
  */
 #define MAP_PAD_LEN_MASK		GENMASK(5, 0)
+#define MAP_NEXT_HEADER_FLAG		BIT(6)
 #define MAP_CMD_FLAG			BIT(7)
 
 struct rmnet_map_dl_csum_trailer {
@@ -45,4 +47,27 @@  struct rmnet_map_ul_csum_header {
 #define MAP_CSUM_UL_UDP_FLAG		BIT(14)
 #define MAP_CSUM_UL_ENABLED_FLAG	BIT(15)
 
+/* MAP CSUM headers */
+struct rmnet_map_v5_csum_header {
+	u8 header_info;
+	u8 csum_info;
+	__be16 reserved;
+} __aligned(1);
+
+/* v5 header_info field
+ * NEXT_HEADER:  Represents whether there is any other header
+ * HEADER TYPE: represents the type of this header
+ *
+ * csum_info field
+ * CSUM_VALID_OR_REQ:
+ * 1 = for UL, checksum computation is requested.
+ * 1 = for DL, validated the checksum and has found it valid
+ */
+
+#define MAPV5_HDRINFO_NXT_HDR_FLAG	BIT(0)
+#define MAPV5_HDRINFO_HDR_TYPE_SHIFT	1
+#define MAPV5_HDRINFO_HDR_TYPE_FMASK	GENMASK(7, MAPV5_HDRINFO_HDR_TYPE_SHIFT)
+#define MAPV5_CSUMINFO_VALID_FLAG	BIT(7)
+
+#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
 #endif /* !(_LINUX_IF_RMNET_H_) */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 91c8dda..21529b3 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1235,6 +1235,7 @@  enum {
 #define RMNET_FLAGS_INGRESS_MAP_COMMANDS          (1U << 1)
 #define RMNET_FLAGS_INGRESS_MAP_CKSUMV4           (1U << 2)
 #define RMNET_FLAGS_EGRESS_MAP_CKSUMV4            (1U << 3)
+#define RMNET_FLAGS_INGRESS_MAP_CKSUMV5           (1U << 4)
 
 enum {
 	IFLA_RMNET_UNSPEC,