RE: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211

[Vinicius Costa Gomes]

Hi,

"Brown, Aaron F"  writes:

>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
>> *adapter, u32 index)
>>  if (is_valid_ether_addr(addr))
>>  rar_high |= E1000_RAH_AV;
>> 
>> -if (hw->mac.type == e1000_82575)
>> +switch (hw->mac.type) {
>> +case e1000_82575:
>> +case e1000_i210:
>> +case e1000_i211:
>> +rar_high |= E1000_RAH_QSEL_ENABLE;
>>  rar_high |= E1000_RAH_POOL_1 *
>> -adapter->mac_table[index].queue;
>> -else
>> +  adapter->mac_table[index].queue;
>> +break;
>> +default:
>>  rar_high |= E1000_RAH_POOL_1 <<
>> -adapter->mac_table[index].queue;
>> +adapter->mac_table[index].queue;
>
> Small nit.  Shouldn't this line be indented more to be a few spaces
> past the "|=" operator as above?

I don't know why my editor seemed to disagree, I will fix.


Thank you,
--
Vinicius

RE: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection on MAC filters on i210 and i211

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Vinicius Costa Gomes
> Sent: Wednesday, March 7, 2018 4:37 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus  palen...@intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v4 2/8] igb: Fix queue selection
> on MAC filters on i210 and i211
> 
> On the RAH registers there are semantic differences on the meaning of
> the "queue" parameter for traffic steering depending on the controller
> model: there is the 82575 meaning, which "queue" means a RX Hardware
> Queue, and the i350 meaning, where it is a reception pool.
> 
> The previous behaviour was having no effect for i210 and i211 based
> controllers because the QSEL bit of the RAH register wasn't being set.
> 
> This patch separates the condition in discrete cases, so the different
> handling is clearer.
> 
> Fixes: 83c21335c876 ("igb: improve MAC filter handling")
> Signed-off-by: Vinicius Costa Gomes 
> ---
>  drivers/net/ethernet/intel/igb/e1000_defines.h |  1 +
>  drivers/net/ethernet/intel/igb/igb_main.c  | 15 +++
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h
> b/drivers/net/ethernet/intel/igb/e1000_defines.h
> index 83cabff1e0ab..573bf177fd08 100644
> --- a/drivers/net/ethernet/intel/igb/e1000_defines.h
> +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
> @@ -490,6 +490,7 @@
>   * manageability enabled, allowing us room for 15 multicast addresses.
>   */
>  #define E1000_RAH_AV  0x8000/* Receive descriptor valid */
> +#define E1000_RAH_QSEL_ENABLE 0x1000
>  #define E1000_RAL_MAC_ADDR_LEN 4
>  #define E1000_RAH_MAC_ADDR_LEN 2
>  #define E1000_RAH_POOL_MASK 0x03FC
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index 715bb32e6901..eabedc6b6518 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -8747,12 +8747,19 @@ static void igb_rar_set_index(struct igb_adapter
> *adapter, u32 index)
>   if (is_valid_ether_addr(addr))
>   rar_high |= E1000_RAH_AV;
> 
> - if (hw->mac.type == e1000_82575)
> + switch (hw->mac.type) {
> + case e1000_82575:
> + case e1000_i210:
> + case e1000_i211:
> + rar_high |= E1000_RAH_QSEL_ENABLE;
>   rar_high |= E1000_RAH_POOL_1 *
> - adapter->mac_table[index].queue;
> - else
> +   adapter->mac_table[index].queue;
> + break;
> + default:
>   rar_high |= E1000_RAH_POOL_1 <<
> - adapter->mac_table[index].queue;
> + adapter->mac_table[index].queue;

Small nit.  Shouldn't this line be indented more to be a few spaces past the 
"|=" operator as above?

> + break;
> + }
>   }
> 
>   wr32(E1000_RAL(index), rar_low);
> --
> 2.16.2
> 
> ___
> Intel-wired-lan mailing list
> intel-wired-...@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan