netdev - Re: [PATCH v2 net-next 01/12] net: atlantic: changes for multi-TC support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20200522105831.4ab00ca5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 22 May 2020 10:58:31 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Igor Russkikh <irusskikh@...vell.com>
Cc:     <netdev@...r.kernel.org>, "David S . Miller" <davem@...emloft.net>,
        Mark Starovoytov <mstarovoitov@...vell.com>,
        Dmitry Bezrukov <dbezrukov@...vell.com>
Subject: Re: [PATCH v2 net-next 01/12] net: atlantic: changes for multi-TC
 support

On Fri, 22 May 2020 11:19:37 +0300 Igor Russkikh wrote:
> From: Dmitry Bezrukov <dbezrukov@...vell.com>
> 
> This patch contains the following changes:
> * access cfg via aq_nic_get_cfg() in aq_nic_start() and aq_nic_map_skb();
> * call aq_nic_get_dev() just once in aq_nic_map_skb();
> * move ring allocation/deallocation out of aq_vec_alloc()/aq_vec_free();
> * add the missing aq_nic_deinit() in atl_resume_common();
> * rename 'tcs' field to 'tcs_max' in aq_hw_caps_s to differentiate it from
>   the 'tcs' field in aq_nic_cfg_s, which is used for the current number of
>   TCs;
> * update _TC_MAX defines to the actual number of supported TCs;
> * move tx_tc_mode register defines slightly higher (just to keep the order
>   of definitions);
> * separate variables for TX/RX buff_size in hw_atl*_hw_qos_set();
> * use AQ_HW_*_TC instead of hardcoded magic numbers;
> * actually use the 'ret' value in aq_mdo_add_secy();

Whenever you do an enumeration like this - it's a strong indication that
those should all be separate patches. Please keep that in mind going
forward.

> Signed-off-by: Dmitry Bezrukov <dbezrukov@...vell.com>
> Co-developed-by: Mark Starovoytov <mstarovoitov@...vell.com>
> Signed-off-by: Mark Starovoytov <mstarovoitov@...vell.com>
> Signed-off-by: Igor Russkikh <irusskikh@...vell.com>

> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_macsec.c b/drivers/net/ethernet/aquantia/atlantic/aq_macsec.c
> index 91870ceaf3fe..4a6dfac857ca 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_macsec.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_macsec.c
> @@ -478,7 +478,7 @@ static int aq_mdo_add_secy(struct macsec_context *ctx)
>  
>  	set_bit(txsc_idx, &cfg->txsc_idx_busy);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int aq_mdo_upd_secy(struct macsec_context *ctx)

This should have been a separate fix for sure.