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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZCbuTjZKWHjVuoab@corigine.com>
Date:   Fri, 31 Mar 2023 16:29:34 +0200
From:   Simon Horman <simon.horman@...igine.com>
To:     Tony Nguyen <anthony.l.nguyen@...el.com>
Cc:     davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        edumazet@...gle.com, netdev@...r.kernel.org,
        Sylwester Dziedziuch <sylwesterx.dziedziuch@...el.com>,
        Mateusz Palczewski <mateusz.palczewski@...el.com>,
        Rafal Romanowski <rafal.romanowski@...el.com>
Subject: Re: [PATCH net-next 1/1] i40e: Add support for VF to specify its
 primary MAC address

On Thu, Mar 30, 2023 at 10:00:22AM -0700, Tony Nguyen wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch@...el.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@...el.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@...el.com>
> Tested-by: Rafal Romanowski <rafal.romanowski@...el.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>

Reviewed-by: Simon Horman <simon.horman@...igine.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);

....