| Message ID | 20210304223431.15045-7-elder@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: qualcomm: rmnet: stop using C bit-fields | expand |
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 >
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
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 >
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 --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_) */
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(-) -- 2.20.1