Re: [PATCH net-next 1/1] i40e: Add support for VF to specify its primary MAC address

[Simon Horman <simon.horman@corigine.com>]

On Thu, Mar 30, 2023 at 10:00:22AM -0700, Tony Nguyen wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> 
> Currently in the i40e driver there is no implementation of different
> MAC address handling depending on whether it is a legacy or primary.
> Introduce new checks for VF to be able to specify its primary MAC
> address based on the VIRTCHNL_ETHER_ADDR_PRIMARY type.
> 
> Primary MAC address are treated differently compared to legacy
> ones in a scenario where:
> 1. If a unicast MAC is being added and it's specified as
> VIRTCHNL_ETHER_ADDR_PRIMARY, then replace the current
> default_lan_addr.addr.
> 2. If a unicast MAC is being deleted and it's type
> is specified as VIRTCHNL_ETHER_ADDR_PRIMARY, then zero the
> hw_lan_addr.addr.
> 
> Signed-off-by: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

> +/**
> + * i40e_update_vf_mac_addr
> + * @vf: VF to update
> + * @vc_ether_addr: structure from VIRTCHNL with MAC to add
> + *
> + * update the VF's cached hardware MAC if allowed
> + **/
> +static void
> +i40e_update_vf_mac_addr(struct i40e_vf *vf,
> +			struct virtchnl_ether_addr *vc_ether_addr)
> +{
> +	u8 *mac_addr = vc_ether_addr->addr;
> +
> +	if (!is_valid_ether_addr(mac_addr))
> +		return;
> +
> +	/* If request to add MAC filter is a primary request update its default
> +	 * MAC address with the requested one. If it is a legacy request then
> +	 * check if current default is empty if so update the default MAC
> +	 */
> +	if (i40e_is_vc_addr_primary(vc_ether_addr)) {
> +		ether_addr_copy(vf->default_lan_addr.addr, mac_addr);
> +	} else if (i40e_is_vc_addr_legacy(vc_ether_addr)) {
> +		if (is_zero_ether_addr(vf->default_lan_addr.addr))
> +			ether_addr_copy(vf->default_lan_addr.addr, mac_addr);
> +	}

FWIIW, I would have gone for something like this.
Though, TBH, I'm not sure it is any easier on the eyes.
(*Compile tested only!*)

        if (i40e_is_vc_addr_primary(vc_ether_addr) ||
            (i40e_is_vc_addr_legacy(vc_ether_addr) &&
             is_zero_ether_addr(vf->default_lan_addr.addr)))
                ether_addr_copy(vf->default_lan_addr.addr, mac_addr);

....