Re: [[RFC PATCH]] lib/ether: fix 16-bit unaligned access

[Jan Viktorin <viktorin@rehivetech.com>]

On Tue, 8 Dec 2015 12:02:54 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> On Tue,  8 Dec 2015 20:29:53 +0100
> Jan Viktorin <viktorin@rehivetech.com> wrote:
> 
> > Hello,
> > 	
> > I was looking at some warnings generated during ARM build. I can see
> > 53 warnings for my build based on v2.2.0-rc3, spread among:
> > 
> >  app/test-pmd/{flowgen,icmpecho,txonly}.c
> >  app/test/{packet_burst_generator,test_hash_functions,test_thash}.c
> >  lib/librte_ether/rte_ether.h
> >  drivers/net/bonding/rte_eth_bond_pmd.c
> >  lib/librte_acl/{acl_gen,acl_run}.c
> >  lib/librte_eal/linuxapp/eal/{eal_interrupts,eal_pci_vfio_mp_sync}.c
> >  lib/librte_hash/rte_cuckoo_hash.c
> >  lib/librte_ip_frag/rte_ipv4_reassembly.c
> >  lib/librte_sched/{rte_bitmap.h,rte_sched.c}
> > 
> > I think, some of them are false-positives. In this RFC patch I tried to fix
> > only the rte_ether.h which uses the unaligned_uint16_t data type. I didn't
> > test it as it is just the first kick to solve more of those warns.
> > 
> > Regards
> > Jan
> > 
> > (I considered to not add the cover-letter as this is just a single small patch.
> > I hope it does not matter a lot. Is there any convention how to do this?)
> > ---
> > This commit removes warning reported when building for ARMv7 target.
> > 
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> > ---
> >  lib/librte_ether/rte_ether.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/librte_ether/rte_ether.h b/lib/librte_ether/rte_ether.h
> > index 07c17d7..ba8a80a 100644
> > --- a/lib/librte_ether/rte_ether.h
> > +++ b/lib/librte_ether/rte_ether.h
> > @@ -175,10 +175,9 @@ static inline int is_multicast_ether_addr(const struct ether_addr *ea)
> >   */
> >  static inline int is_broadcast_ether_addr(const struct ether_addr *ea)
> >  {
> > -	const unaligned_uint16_t *ea_words = (const unaligned_uint16_t *)ea;
> > +	const uint32_t *ea_words = (const uint32_t *)ea;
> >  
> > -	return (ea_words[0] == 0xFFFF && ea_words[1] == 0xFFFF &&
> > -		ea_words[2] == 0xFFFF);
> > +	return ea_words[0] == 0xFFFFFFFF && (ea_words[1] & 0x0FFFF) == 0x0FFFF;  
> 
> The problem with that is that it assumes little-endian.

Well, true. But there is another issue that I considered just after
left for home today :). The ether_addr can be unaligned in memory easily
(as it comes in the Ethernet frame). Another false-positive... So the
original code is correct. The fix would be to mute the compiler here.

The solution is probably to enable RTE_ARCH_STRICT_ALIGN in the
defconfig_arm-armv7a-linuxapp-gcc. This will probably solve more unaligned false-positives.

Regards
Jan

-- 
  Jan Viktorin                E-mail: Viktorin@RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic