diff mbox series

[net-next,6/6] net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum header

Message ID 20210304223431.15045-7-elder@linaro.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: qualcomm: rmnet: stop using C bit-fields | 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 success CCed 5 of 5 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 1 this patch: 7
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, 95 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 1 this patch: 7
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Alex Elder March 4, 2021, 10:34 p.m. UTC 
Replace the use of C bit-fields in the rmnet_map_ul_csum_header
structure with a single two-byte (big endian) structure member,
and use field masks to encode or get values within it.

Previously rmnet_map_ipv4_ul_csum_header() would update values in
the host byte-order fields, and then forcibly fix their byte order
using a combination of byte order operations and types.

Instead, just compute the value that needs to go into the new
structure member and save it with a simple byte-order conversion.

Make similar simplifications in rmnet_map_ipv6_ul_csum_header().

Finally, in rmnet_map_checksum_uplink_packet() a set of assignments
zeroes every field in the upload checksum header.  Replace that with
a single memset() operation.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 34 ++++++-------------
 include/linux/if_rmnet.h                      | 21 ++++++------
 2 files changed, 21 insertions(+), 34 deletions(-)

Comments

Bjorn Andersson March 5, 2021, 5:26 a.m. UTC | #1 
On Thu 04 Mar 16:34 CST 2021, Alex Elder wrote:

> Replace the use of C bit-fields in the rmnet_map_ul_csum_header
> structure with a single two-byte (big endian) structure member,
> and use field masks to encode or get values within it.
> 
> Previously rmnet_map_ipv4_ul_csum_header() would update values in
> the host byte-order fields, and then forcibly fix their byte order
> using a combination of byte order operations and types.
> 
> Instead, just compute the value that needs to go into the new
> structure member and save it with a simple byte-order conversion.
> 
> Make similar simplifications in rmnet_map_ipv6_ul_csum_header().
> 
> Finally, in rmnet_map_checksum_uplink_packet() a set of assignments
> zeroes every field in the upload checksum header.  Replace that with
> a single memset() operation.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>
> ---
>  .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 34 ++++++-------------
>  include/linux/if_rmnet.h                      | 21 ++++++------
>  2 files changed, 21 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> index 29d485b868a65..db76bbf000aa1 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> @@ -198,23 +198,19 @@ rmnet_map_ipv4_ul_csum_header(void *iphdr,
>  			      struct rmnet_map_ul_csum_header *ul_header,
>  			      struct sk_buff *skb)
>  {
> -	__be16 *hdr = (__be16 *)ul_header;
>  	struct iphdr *ip4h = iphdr;
>  	u16 offset;
> +	u16 val;
>  
>  	offset = skb_transport_header(skb) - (unsigned char *)iphdr;
>  	ul_header->csum_start_offset = htons(offset);
>  
> -	ul_header->csum_insert_offset = skb->csum_offset;
> -	ul_header->csum_enabled = 1;
> +	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);

Why are you using be16_ here? Won't that cancel the htons() below?

Regards,
Bjorn

>  	if (ip4h->protocol == IPPROTO_UDP)
> -		ul_header->udp_ind = 1;
> -	else
> -		ul_header->udp_ind = 0;
> +		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
> +	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
>  
> -	/* Changing remaining fields to network order */
> -	hdr++;
> -	*hdr = htons((__force u16)*hdr);
> +	ul_header->csum_info = htons(val);
>  
>  	skb->ip_summed = CHECKSUM_NONE;
>  
> @@ -241,24 +237,19 @@ rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
>  			      struct rmnet_map_ul_csum_header *ul_header,
>  			      struct sk_buff *skb)
>  {
> -	__be16 *hdr = (__be16 *)ul_header;
>  	struct ipv6hdr *ip6h = ip6hdr;
>  	u16 offset;
> +	u16 val;
>  
>  	offset = skb_transport_header(skb) - (unsigned char *)ip6hdr;
>  	ul_header->csum_start_offset = htons(offset);
>  
> -	ul_header->csum_insert_offset = skb->csum_offset;
> -	ul_header->csum_enabled = 1;
> -
> +	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
>  	if (ip6h->nexthdr == IPPROTO_UDP)
> -		ul_header->udp_ind = 1;
> -	else
> -		ul_header->udp_ind = 0;
> +		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
> +	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
>  
> -	/* Changing remaining fields to network order */
> -	hdr++;
> -	*hdr = htons((__force u16)*hdr);
> +	ul_header->csum_info = htons(val);
>  
>  	skb->ip_summed = CHECKSUM_NONE;
>  
> @@ -425,10 +416,7 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>  	}
>  
>  sw_csum:
> -	ul_header->csum_start_offset = 0;
> -	ul_header->csum_insert_offset = 0;
> -	ul_header->csum_enabled = 0;
> -	ul_header->udp_ind = 0;
> +	memset(ul_header, 0, sizeof(*ul_header));
>  
>  	priv->stats.csum_sw++;
>  }
> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
> index 1fbb7531238b6..149d696feb520 100644
> --- a/include/linux/if_rmnet.h
> +++ b/include/linux/if_rmnet.h
> @@ -33,17 +33,16 @@ struct rmnet_map_dl_csum_trailer {
>  
>  struct rmnet_map_ul_csum_header {
>  	__be16 csum_start_offset;
> -#if defined(__LITTLE_ENDIAN_BITFIELD)
> -	u16 csum_insert_offset:14;
> -	u16 udp_ind:1;
> -	u16 csum_enabled:1;
> -#elif defined (__BIG_ENDIAN_BITFIELD)
> -	u16 csum_enabled:1;
> -	u16 udp_ind:1;
> -	u16 csum_insert_offset:14;
> -#else
> -#error	"Please fix <asm/byteorder.h>"
> -#endif
> +	__be16 csum_info;		/* MAP_CSUM_UL_*_FMASK */
>  } __aligned(1);
>  
> +/* csum_info field:
> + *  ENABLED:	1 = checksum computation requested
> + *  UDP:	1 = UDP checksum (zero checkum means no checksum)
> + *  OFFSET:	where (offset in bytes) to insert computed checksum
> + */
> +#define MAP_CSUM_UL_OFFSET_FMASK	GENMASK(13, 0)
> +#define MAP_CSUM_UL_UDP_FMASK		GENMASK(14, 14)
> +#define MAP_CSUM_UL_ENABLED_FMASK	GENMASK(15, 15)
> +
>  #endif /* !(_LINUX_IF_RMNET_H_) */
> -- 
> 2.20.1
>
kernel test robot March 5, 2021, 6:22 a.m. UTC | #2 
Hi Alex,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Alex-Elder/net-qualcomm-rmnet-stop-using-C-bit-fields/20210305-064128
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d310ec03a34e92a77302edb804f7d68ee4f01ba0
config: riscv-randconfig-s031-20210305 (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-245-gacc5c298-dirty
        # https://github.com/0day-ci/linux/commit/dba638b67dff001926855ed81e35e52bd54880ea
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Alex-Elder/net-qualcomm-rmnet-stop-using-C-bit-fields/20210305-064128
        git checkout dba638b67dff001926855ed81e35e52bd54880ea
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=riscv 

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


"sparse warnings: (new ones prefixed by >>)"
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 @@
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse:     expected unsigned short [usertype] val
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse:     got restricted __be16
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse: sparse: invalid assignment: |=
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse:    left side has type unsigned short
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse:    right side has type restricted __be16
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse: sparse: invalid assignment: |=
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse:    left side has type unsigned short
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse:    right side has type restricted __be16
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 @@
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse:     expected unsigned short [usertype] val
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse:     got restricted __be16
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse: sparse: invalid assignment: |=
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse:    left side has type unsigned short
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse:    right side has type restricted __be16
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse: sparse: invalid assignment: |=
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse:    left side has type unsigned short
   drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse:    right side has type restricted __be16

vim +208 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c

   195	
   196	static void
   197	rmnet_map_ipv4_ul_csum_header(void *iphdr,
   198				      struct rmnet_map_ul_csum_header *ul_header,
   199				      struct sk_buff *skb)
   200	{
   201		struct iphdr *ip4h = iphdr;
   202		u16 offset;
   203		u16 val;
   204	
   205		offset = skb_transport_header(skb) - (unsigned char *)iphdr;
   206		ul_header->csum_start_offset = htons(offset);
   207	
 > 208		val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
   209		if (ip4h->protocol == IPPROTO_UDP)
 > 210			val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
   211		val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
   212	
   213		ul_header->csum_info = htons(val);
   214	
   215		skb->ip_summed = CHECKSUM_NONE;
   216	
   217		rmnet_map_complement_ipv4_txporthdr_csum_field(iphdr);
   218	}
   219	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Alex Elder March 5, 2021, 12:59 p.m. UTC | #3 
On 3/5/21 12:22 AM, kernel test robot wrote:
> Hi Alex,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on net-next/master]

Well that's embarrassing.  It explains why I had to make
a change or two after testing though.

I will fix this so the bit fields are defined in host
byte order, and will fix the encoding calls to use
u32_encode_bits() instead of be32_encode_bits().

I will redo my testing (for all of the patches) and
will then submit version 2 of the series.

					-Alex

> 
> url:    https://github.com/0day-ci/linux/commits/Alex-Elder/net-qualcomm-rmnet-stop-using-C-bit-fields/20210305-064128
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d310ec03a34e92a77302edb804f7d68ee4f01ba0
> config: riscv-randconfig-s031-20210305 (attached as .config)
> compiler: riscv64-linux-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.3-245-gacc5c298-dirty
>         # https://github.com/0day-ci/linux/commit/dba638b67dff001926855ed81e35e52bd54880ea
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Alex-Elder/net-qualcomm-rmnet-stop-using-C-bit-fields/20210305-064128
>         git checkout dba638b67dff001926855ed81e35e52bd54880ea
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=riscv 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> "sparse warnings: (new ones prefixed by >>)"
>>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 @@
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse:     expected unsigned short [usertype] val
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:208:13: sparse:     got restricted __be16
>>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse: sparse: invalid assignment: |=
>>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse:    left side has type unsigned short
>>> drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:210:21: sparse:    right side has type restricted __be16
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse: sparse: invalid assignment: |=
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse:    left side has type unsigned short
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:211:13: sparse:    right side has type restricted __be16
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 @@
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse:     expected unsigned short [usertype] val
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:247:13: sparse:     got restricted __be16
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse: sparse: invalid assignment: |=
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse:    left side has type unsigned short
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:249:21: sparse:    right side has type restricted __be16
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse: sparse: invalid assignment: |=
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse:    left side has type unsigned short
>    drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:250:13: sparse:    right side has type restricted __be16
> 
> vim +208 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
> 
>    195	
>    196	static void
>    197	rmnet_map_ipv4_ul_csum_header(void *iphdr,
>    198				      struct rmnet_map_ul_csum_header *ul_header,
>    199				      struct sk_buff *skb)
>    200	{
>    201		struct iphdr *ip4h = iphdr;
>    202		u16 offset;
>    203		u16 val;
>    204	
>    205		offset = skb_transport_header(skb) - (unsigned char *)iphdr;
>    206		ul_header->csum_start_offset = htons(offset);
>    207	
>  > 208		val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
>    209		if (ip4h->protocol == IPPROTO_UDP)
>  > 210			val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
>    211		val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
>    212	
>    213		ul_header->csum_info = htons(val);
>    214	
>    215		skb->ip_summed = CHECKSUM_NONE;
>    216	
>    217		rmnet_map_complement_ipv4_txporthdr_csum_field(iphdr);
>    218	}
>    219	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
Alex Elder March 5, 2021, 8:48 p.m. UTC | #4 
On 3/4/21 11:26 PM, Bjorn Andersson wrote:
> On Thu 04 Mar 16:34 CST 2021, Alex Elder wrote:
> 
>> Replace the use of C bit-fields in the rmnet_map_ul_csum_header
>> structure with a single two-byte (big endian) structure member,
>> and use field masks to encode or get values within it.
>>
>> Previously rmnet_map_ipv4_ul_csum_header() would update values in
>> the host byte-order fields, and then forcibly fix their byte order
>> using a combination of byte order operations and types.
>>
>> Instead, just compute the value that needs to go into the new
>> structure member and save it with a simple byte-order conversion.
>>
>> Make similar simplifications in rmnet_map_ipv6_ul_csum_header().
>>
>> Finally, in rmnet_map_checksum_uplink_packet() a set of assignments
>> zeroes every field in the upload checksum header.  Replace that with
>> a single memset() operation.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
>> ---
>>   .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 34 ++++++-------------
>>   include/linux/if_rmnet.h                      | 21 ++++++------
>>   2 files changed, 21 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> index 29d485b868a65..db76bbf000aa1 100644
>> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
>> @@ -198,23 +198,19 @@ rmnet_map_ipv4_ul_csum_header(void *iphdr,
>>   			      struct rmnet_map_ul_csum_header *ul_header,
>>   			      struct sk_buff *skb)
>>   {
>> -	__be16 *hdr = (__be16 *)ul_header;
>>   	struct iphdr *ip4h = iphdr;
>>   	u16 offset;
>> +	u16 val;
>>   
>>   	offset = skb_transport_header(skb) - (unsigned char *)iphdr;
>>   	ul_header->csum_start_offset = htons(offset);
>>   
>> -	ul_header->csum_insert_offset = skb->csum_offset;
>> -	ul_header->csum_enabled = 1;
>> +	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
> 
> Why are you using be16_ here? Won't that cancel the htons() below?

Yes.  This was a mistake, and as you know, the kernel test
robot caught the problem with assigning a big endian value
to a host byte order variable.  This cannot have worked
before and I'm not sure what happened.

I've updated this series and am re-testing everything.  That
includes running the patches through sparse, but also includes
running with checksum offload enabled and verifying errors
are not reported when checksum offload active.

					-Alex

> Regards,
> Bjorn
> 
>>   	if (ip4h->protocol == IPPROTO_UDP)
>> -		ul_header->udp_ind = 1;
>> -	else
>> -		ul_header->udp_ind = 0;
>> +		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
>> +	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
>>   
>> -	/* Changing remaining fields to network order */
>> -	hdr++;
>> -	*hdr = htons((__force u16)*hdr);
>> +	ul_header->csum_info = htons(val);
>>   
>>   	skb->ip_summed = CHECKSUM_NONE;
>>   
>> @@ -241,24 +237,19 @@ rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
>>   			      struct rmnet_map_ul_csum_header *ul_header,
>>   			      struct sk_buff *skb)
>>   {
>> -	__be16 *hdr = (__be16 *)ul_header;
>>   	struct ipv6hdr *ip6h = ip6hdr;
>>   	u16 offset;
>> +	u16 val;
>>   
>>   	offset = skb_transport_header(skb) - (unsigned char *)ip6hdr;
>>   	ul_header->csum_start_offset = htons(offset);
>>   
>> -	ul_header->csum_insert_offset = skb->csum_offset;
>> -	ul_header->csum_enabled = 1;
>> -
>> +	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
>>   	if (ip6h->nexthdr == IPPROTO_UDP)
>> -		ul_header->udp_ind = 1;
>> -	else
>> -		ul_header->udp_ind = 0;
>> +		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
>> +	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
>>   
>> -	/* Changing remaining fields to network order */
>> -	hdr++;
>> -	*hdr = htons((__force u16)*hdr);
>> +	ul_header->csum_info = htons(val);
>>   
>>   	skb->ip_summed = CHECKSUM_NONE;
>>   
>> @@ -425,10 +416,7 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>>   	}
>>   
>>   sw_csum:
>> -	ul_header->csum_start_offset = 0;
>> -	ul_header->csum_insert_offset = 0;
>> -	ul_header->csum_enabled = 0;
>> -	ul_header->udp_ind = 0;
>> +	memset(ul_header, 0, sizeof(*ul_header));
>>   
>>   	priv->stats.csum_sw++;
>>   }
>> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
>> index 1fbb7531238b6..149d696feb520 100644
>> --- a/include/linux/if_rmnet.h
>> +++ b/include/linux/if_rmnet.h
>> @@ -33,17 +33,16 @@ struct rmnet_map_dl_csum_trailer {
>>   
>>   struct rmnet_map_ul_csum_header {
>>   	__be16 csum_start_offset;
>> -#if defined(__LITTLE_ENDIAN_BITFIELD)
>> -	u16 csum_insert_offset:14;
>> -	u16 udp_ind:1;
>> -	u16 csum_enabled:1;
>> -#elif defined (__BIG_ENDIAN_BITFIELD)
>> -	u16 csum_enabled:1;
>> -	u16 udp_ind:1;
>> -	u16 csum_insert_offset:14;
>> -#else
>> -#error	"Please fix <asm/byteorder.h>"
>> -#endif
>> +	__be16 csum_info;		/* MAP_CSUM_UL_*_FMASK */
>>   } __aligned(1);
>>   
>> +/* csum_info field:
>> + *  ENABLED:	1 = checksum computation requested
>> + *  UDP:	1 = UDP checksum (zero checkum means no checksum)
>> + *  OFFSET:	where (offset in bytes) to insert computed checksum
>> + */
>> +#define MAP_CSUM_UL_OFFSET_FMASK	GENMASK(13, 0)
>> +#define MAP_CSUM_UL_UDP_FMASK		GENMASK(14, 14)
>> +#define MAP_CSUM_UL_ENABLED_FMASK	GENMASK(15, 15)
>> +
>>   #endif /* !(_LINUX_IF_RMNET_H_) */
>> -- 
>> 2.20.1
>>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 29d485b868a65..db76bbf000aa1 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -198,23 +198,19 @@  rmnet_map_ipv4_ul_csum_header(void *iphdr,
 			      struct rmnet_map_ul_csum_header *ul_header,
 			      struct sk_buff *skb)
 {
-	__be16 *hdr = (__be16 *)ul_header;
 	struct iphdr *ip4h = iphdr;
 	u16 offset;
+	u16 val;
 
 	offset = skb_transport_header(skb) - (unsigned char *)iphdr;
 	ul_header->csum_start_offset = htons(offset);
 
-	ul_header->csum_insert_offset = skb->csum_offset;
-	ul_header->csum_enabled = 1;
+	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
 	if (ip4h->protocol == IPPROTO_UDP)
-		ul_header->udp_ind = 1;
-	else
-		ul_header->udp_ind = 0;
+		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
+	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
 
-	/* Changing remaining fields to network order */
-	hdr++;
-	*hdr = htons((__force u16)*hdr);
+	ul_header->csum_info = htons(val);
 
 	skb->ip_summed = CHECKSUM_NONE;
 
@@ -241,24 +237,19 @@  rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
 			      struct rmnet_map_ul_csum_header *ul_header,
 			      struct sk_buff *skb)
 {
-	__be16 *hdr = (__be16 *)ul_header;
 	struct ipv6hdr *ip6h = ip6hdr;
 	u16 offset;
+	u16 val;
 
 	offset = skb_transport_header(skb) - (unsigned char *)ip6hdr;
 	ul_header->csum_start_offset = htons(offset);
 
-	ul_header->csum_insert_offset = skb->csum_offset;
-	ul_header->csum_enabled = 1;
-
+	val = be16_encode_bits(1, MAP_CSUM_UL_ENABLED_FMASK);
 	if (ip6h->nexthdr == IPPROTO_UDP)
-		ul_header->udp_ind = 1;
-	else
-		ul_header->udp_ind = 0;
+		val |= be16_encode_bits(1, MAP_CSUM_UL_UDP_FMASK);
+	val |= be16_encode_bits(skb->csum_offset, MAP_CSUM_UL_OFFSET_FMASK);
 
-	/* Changing remaining fields to network order */
-	hdr++;
-	*hdr = htons((__force u16)*hdr);
+	ul_header->csum_info = htons(val);
 
 	skb->ip_summed = CHECKSUM_NONE;
 
@@ -425,10 +416,7 @@  void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
 	}
 
 sw_csum:
-	ul_header->csum_start_offset = 0;
-	ul_header->csum_insert_offset = 0;
-	ul_header->csum_enabled = 0;
-	ul_header->udp_ind = 0;
+	memset(ul_header, 0, sizeof(*ul_header));
 
 	priv->stats.csum_sw++;
 }
diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
index 1fbb7531238b6..149d696feb520 100644
--- a/include/linux/if_rmnet.h
+++ b/include/linux/if_rmnet.h
@@ -33,17 +33,16 @@  struct rmnet_map_dl_csum_trailer {
 
 struct rmnet_map_ul_csum_header {
 	__be16 csum_start_offset;
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-	u16 csum_insert_offset:14;
-	u16 udp_ind:1;
-	u16 csum_enabled:1;
-#elif defined (__BIG_ENDIAN_BITFIELD)
-	u16 csum_enabled:1;
-	u16 udp_ind:1;
-	u16 csum_insert_offset:14;
-#else
-#error	"Please fix <asm/byteorder.h>"
-#endif
+	__be16 csum_info;		/* MAP_CSUM_UL_*_FMASK */
 } __aligned(1);
 
+/* csum_info field:
+ *  ENABLED:	1 = checksum computation requested
+ *  UDP:	1 = UDP checksum (zero checkum means no checksum)
+ *  OFFSET:	where (offset in bytes) to insert computed checksum
+ */
+#define MAP_CSUM_UL_OFFSET_FMASK	GENMASK(13, 0)
+#define MAP_CSUM_UL_UDP_FMASK		GENMASK(14, 14)
+#define MAP_CSUM_UL_ENABLED_FMASK	GENMASK(15, 15)
+
 #endif /* !(_LINUX_IF_RMNET_H_) */