Re: [PATCH net-next 3/3] net: usb: kaweth: remove eth_addr_t typedef and bcast_addr definition

[Oliver Neukum <oneukum@suse.com>]

Hi,

unfortunately I must NACK this patch. Let me explain:

On 25.01.26 09:34, Ethan Nelson-Moore wrote:

>   /****************************************************************
>    *     usb_eth_dev
>    ****************************************************************/
> @@ -185,7 +183,7 @@ struct kaweth_ethernet_configuration
>   	__u8 size;
>   	__u8 reserved1;
>   	__u8 reserved2;
> -	eth_addr_t hw_addr;
> +	u8 hw_addr[ETH_ALEN];

You may note that this data structure is declared __packed
and the member hw_addr is preceded by three members of type __u8.
That means that it is located on a _three_ byte alignment.

>   	__u32 statistics_mask;
>   	__le16 segment_size;
>   	__u16 max_multicast_filters;
> @@ -882,7 +880,6 @@ static int kaweth_probe(

[..]
   
> @@ -991,9 +988,7 @@ static int kaweth_probe(
>   	dev_info(dev, "MTU: %d\n", le16_to_cpu(kaweth->configuration.segment_size));
>   	dev_info(dev, "Read MAC address %pM\n", kaweth->configuration.hw_addr);
>   
> -	if(!memcmp(&kaweth->configuration.hw_addr,
> -                   &bcast_addr,
> -		   sizeof(bcast_addr))) {
> +	if (is_broadcast_ether_addr((const u8 *)&kaweth->configuration.hw_addr)) {

Here you are using is_broadcast_ether_addr(). Let me quote
from the documentation on this helper:

/**
  * is_broadcast_ether_addr - Determine if the Ethernet address is broadcast
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
  * Return: true if the address is the broadcast address.
  *
  * Please note: addr must be aligned to u16.
  */

That requirement is not met. This code will bomb on ARM for example.

	HTH
		Oliver

NACKED-BY: Oliver Neukum <oneukum@suse.com>