[PATCH] util: fix very old bug/typo in virNetDevParseVfInfo()

[Michal Privoznik]

On 10/21/20 4:38 AM, Laine Stump wrote:
> When this function was recently changed to add in parsing of
> IFLA_VF_STATS, I noticed that the checks for existence of IFLA_VF_MAC
> and IFLA_VF_VLAN were looking in the *wrong array*. The array that
> contains the results of parsing each IFLA_VFINFO in
> tb[IFLA_VFINFO_LIST] is tb_vf, but we were checking for these in tb
> (which is the array containing the results of the toplevel parsing of
> the netlink message, *not* the results of parsing one of the nested
> IFLA_VFINFO's.
> 
> This incorrect code has been here since the function was originally
> written in 2012. It has only worked all these years due to coincidence
> - the items at those indexes in tb are IFLA_ADDRESS and IFLA_BROADCAST
> (of the *PF*, not of any of its VFs), and those happen to always be
> present in the toplevel netlink message; since we are only looking in
> the incorrect place to check for mere existence of the attribute (but
> are doing the actual retrieval of the attribute from the correct
> place), this bug has no real consequences other than confusing anyone
> trying to understand the code.
> 
> Signed-off-by: Laine Stump <laine at redhat.com>
> ---
>   src/util/virnetdev.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index e284d62233..591a73cb45 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1691,7 +1691,7 @@ virNetDevParseVfInfo(struct nlattr **tb, int32_t vf, virMacAddrPtr mac,
>               return rc;
>           }
>   
> -        if (mac && tb[IFLA_VF_MAC]) {
> +        if (mac && tb_vf[IFLA_VF_MAC]) {
>               vf_mac = RTA_DATA(tb_vf[IFLA_VF_MAC]);
>               if (vf_mac && vf_mac->vf == vf)  {
>                   virMacAddrSetRaw(mac, vf_mac->mac);
> @@ -1699,7 +1699,7 @@ virNetDevParseVfInfo(struct nlattr **tb, int32_t vf, virMacAddrPtr mac,
>               }
>           }
>   
> -        if (vlanid && tb[IFLA_VF_VLAN]) {
> +        if (vlanid && tb_vf[IFLA_VF_VLAN]) {
>               vf_vlan = RTA_DATA(tb_vf[IFLA_VF_VLAN]);
>               if (vf_vlan && vf_vlan->vf == vf)  {
>                   *vlanid = vf_vlan->vlan;
> 

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Also, feel free to bring the @tb_vf declaration into the loop (in a 
separate trivial patch).

Michal