[07/12] igc: Remove 'queue' check in igc_del_mac_filter()
diff mbox series

Message ID 20200318230102.36952-8-andre.guedes@intel.com
State Under Review
Delegated to: Jeff Kirsher
Headers show
Series
  • igc: Refactor MAC address filtering code
Related show

Commit Message

Andre Guedes March 18, 2020, 11 p.m. UTC
igc_add_mac_filter() doesn't allow us to have more than one entry with
the same address and address type in adapter->mac_table so checking if
'queue' matches in igc_del_mac_filter() isn't necessary. This patch
removes that check.

This patch also takes the opportunity to improve the igc_del_mac_filter
documentation and remove comment which is not applicable to this I225
controller.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 27 ++++++++++-------------
 1 file changed, 12 insertions(+), 15 deletions(-)

Comments

Sasha Neftin March 31, 2020, 3:53 p.m. UTC | #1
On 3/19/2020 01:00, Andre Guedes wrote:
> igc_add_mac_filter() doesn't allow us to have more than one entry with
> the same address and address type in adapter->mac_table so checking if
> 'queue' matches in igc_del_mac_filter() isn't necessary. This patch
> removes that check.
> 
> This patch also takes the opportunity to improve the igc_del_mac_filter
> documentation and remove comment which is not applicable to this I225
> controller.
> 
> Signed-off-by: Andre Guedes <andre.guedes@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 27 ++++++++++-------------
>   1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index a71f1a5ca27c..8a3cae2367d4 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -2234,14 +2234,17 @@ static int igc_add_mac_filter(struct igc_adapter *adapter, const u8 *addr,
>   	return -ENOSPC;
>   }
>   
> -/* Remove a MAC filter for 'addr' directing matching traffic to
> - * 'queue', 'flags' is used to indicate what kind of match need to be
> - * removed, match is by default for the destination address, if
> - * matching by source address is to be removed the flag
> - * IGC_MAC_STATE_SRC_ADDR can be used.
> - */
> +/* Delete MAC address filter from adapter.
> + *
> + * @adapter: Pointer to adapter where the filter should be deleted from.
> + * @addr: MAC address.
> + * @flags: Set IGC_MAC_STATE_SRC_ADDR bit to indicate @address is a source
> + * address.
> + *
> + * In case of success, returns 0. Otherwise, it returns a negative errno code.
Block comments should align the * on each line (please, remove one space 
before *)
> +  */ >   static int igc_del_mac_filter(struct igc_adapter *adapter, const u8 
*addr,
> -			      const u8 queue, const u8 flags)
> +			      const u8 flags)
>   {
>   	struct igc_hw *hw = &adapter->hw;
>   	int rar_entries = hw->mac.rar_entry_count;
> @@ -2250,17 +2253,11 @@ static int igc_del_mac_filter(struct igc_adapter *adapter, const u8 *addr,
>   	if (!is_valid_ether_addr(addr))
>   		return -EINVAL;
>   
> -	/* Search for matching entry in the MAC table based on given address
> -	 * and queue. Do not touch entries at the end of the table reserved
> -	 * for the VF MAC addresses.
> -	 */
>   	for (i = 0; i < rar_entries; i++) {
>   		if (!(adapter->mac_table[i].state & IGC_MAC_STATE_IN_USE))
>   			continue;
>   		if (flags && (adapter->mac_table[i].state & flags) != flags)
>   			continue;
> -		if (adapter->mac_table[i].queue != queue)
> -			continue;
>   		if (!ether_addr_equal(adapter->mac_table[i].addr, addr))
>   			continue;
>   
> @@ -2296,7 +2293,7 @@ static int igc_uc_unsync(struct net_device *netdev, const unsigned char *addr)
>   {
>   	struct igc_adapter *adapter = netdev_priv(netdev);
>   
> -	return igc_del_mac_filter(adapter, addr, adapter->num_rx_queues, 0);
> +	return igc_del_mac_filter(adapter, addr, 0);
>   }
>   
>   /**
> @@ -3741,7 +3738,7 @@ int igc_add_mac_steering_filter(struct igc_adapter *adapter,
>   int igc_del_mac_steering_filter(struct igc_adapter *adapter,
>   				const u8 *addr, u8 queue, u8 flags)
>   {
> -	return igc_del_mac_filter(adapter, addr, queue,
> +	return igc_del_mac_filter(adapter, addr,
>   				  IGC_MAC_STATE_QUEUE_STEERING | flags);
>   }
>   
>
Brown, Aaron F March 31, 2020, 8:48 p.m. UTC | #2
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Neftin, Sasha
> Sent: Tuesday, March 31, 2020 8:53 AM
> To: intel-wired-lan@osuosl.org; Lifshits, Vitaly <vitaly.lifshits@intel.com>; Avivi,
> Amir <amir.avivi@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH 07/12] igc: Remove 'queue' check in
> igc_del_mac_filter()
> 
> On 3/19/2020 01:00, Andre Guedes wrote:
> > igc_add_mac_filter() doesn't allow us to have more than one entry with
> > the same address and address type in adapter->mac_table so checking if
> > 'queue' matches in igc_del_mac_filter() isn't necessary. This patch
> > removes that check.
> >
> > This patch also takes the opportunity to improve the igc_del_mac_filter
> > documentation and remove comment which is not applicable to this I225
> > controller.
> >
> > Signed-off-by: Andre Guedes <andre.guedes@intel.com>
> > ---
> >   drivers/net/ethernet/intel/igc/igc_main.c | 27 ++++++++++-------------
> >   1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> > index a71f1a5ca27c..8a3cae2367d4 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > @@ -2234,14 +2234,17 @@ static int igc_add_mac_filter(struct igc_adapter
> *adapter, const u8 *addr,
> >   	return -ENOSPC;
> >   }
> >
> > -/* Remove a MAC filter for 'addr' directing matching traffic to
> > - * 'queue', 'flags' is used to indicate what kind of match need to be
> > - * removed, match is by default for the destination address, if
> > - * matching by source address is to be removed the flag
> > - * IGC_MAC_STATE_SRC_ADDR can be used.
> > - */
> > +/* Delete MAC address filter from adapter.
> > + *
> > + * @adapter: Pointer to adapter where the filter should be deleted from.
> > + * @addr: MAC address.
> > + * @flags: Set IGC_MAC_STATE_SRC_ADDR bit to indicate @address is a
> source
> > + * address.
> > + *
> > + * In case of success, returns 0. Otherwise, it returns a negative errno code.
> Block comments should align the * on each line (please, remove one space
> before *)
> > +  */ >   static int igc_del_mac_filter(struct igc_adapter *adapter, const u8
> *addr, 

Yes, that comment block throws a checkpatch warning:
-------------------
u1322:[1]/usr/src/kernels/next-queue> git format-patch $item -1 --stdout|./scripts/checkpatch.pl -
WARNING: Block comments should align the * on each line
#42: FILE: drivers/net/ethernet/intel/igc/igc_main.c:2245:
+ * In case of success, returns 0. Otherwise, it returns a negative errno code.
+  */

total: 0 errors, 1 warnings, 0 checks, 57 lines checked

Aside from that:
Tested-by: Aaron Brown <aaron.f.brown
Kirsher, Jeffrey T March 31, 2020, 8:50 p.m. UTC | #3
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Brown, Aaron F
> Sent: Tuesday, March 31, 2020 13:48
> To: Neftin, Sasha <sasha.neftin@intel.com>; intel-wired-lan@osuosl.org;
> Lifshits, Vitaly <vitaly.lifshits@intel.com>; Avivi, Amir <amir.avivi@intel.com>;
> Guedes, Andre <andre.guedes@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH 07/12] igc: Remove 'queue' check in
> igc_del_mac_filter()
> 
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Neftin, Sasha
> > Sent: Tuesday, March 31, 2020 8:53 AM
> > To: intel-wired-lan@osuosl.org; Lifshits, Vitaly
> > <vitaly.lifshits@intel.com>; Avivi, Amir <amir.avivi@intel.com>
> > Subject: Re: [Intel-wired-lan] [PATCH 07/12] igc: Remove 'queue' check
> > in
> > igc_del_mac_filter()
> >
> > On 3/19/2020 01:00, Andre Guedes wrote:
> > > igc_add_mac_filter() doesn't allow us to have more than one entry
> > > with the same address and address type in adapter->mac_table so
> > > checking if 'queue' matches in igc_del_mac_filter() isn't necessary.
> > > This patch removes that check.
> > >
> > > This patch also takes the opportunity to improve the
> > > igc_del_mac_filter documentation and remove comment which is not
> > > applicable to this I225 controller.
> > >
> > > Signed-off-by: Andre Guedes <andre.guedes@intel.com>
> > > ---
> > >   drivers/net/ethernet/intel/igc/igc_main.c | 27 ++++++++++-------------
> > >   1 file changed, 12 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > b/drivers/net/ethernet/intel/igc/igc_main.c
> > > index a71f1a5ca27c..8a3cae2367d4 100644
> > > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > > @@ -2234,14 +2234,17 @@ static int igc_add_mac_filter(struct
> > > igc_adapter
> > *adapter, const u8 *addr,
> > >   	return -ENOSPC;
> > >   }
> > >
> > > -/* Remove a MAC filter for 'addr' directing matching traffic to
> > > - * 'queue', 'flags' is used to indicate what kind of match need to
> > > be
> > > - * removed, match is by default for the destination address, if
> > > - * matching by source address is to be removed the flag
> > > - * IGC_MAC_STATE_SRC_ADDR can be used.
> > > - */
> > > +/* Delete MAC address filter from adapter.
> > > + *
> > > + * @adapter: Pointer to adapter where the filter should be deleted from.
> > > + * @addr: MAC address.
> > > + * @flags: Set IGC_MAC_STATE_SRC_ADDR bit to indicate @address is a
> > source
> > > + * address.
> > > + *
> > > + * In case of success, returns 0. Otherwise, it returns a negative errno
> code.
> > Block comments should align the * on each line (please, remove one
> > space before *)
> > > +  */ >   static int igc_del_mac_filter(struct igc_adapter *adapter, const u8
> > *addr,
> 
> Yes, that comment block throws a checkpatch warning:
> -------------------
> u1322:[1]/usr/src/kernels/next-queue> git format-patch $item -1 --
> stdout|./scripts/checkpatch.pl -
> WARNING: Block comments should align the * on each line
> #42: FILE: drivers/net/ethernet/intel/igc/igc_main.c:2245:
> + * In case of success, returns 0. Otherwise, it returns a negative errno code.
> +  */
> 
> total: 0 errors, 1 warnings, 0 checks, 57 lines checked
>

[Kirsher, Jeffrey T] 
Yeah, Andre let me know and I will fix it when I go to push the patch upstream.
 
> Aside from that:
> Tested-by: Aaron Brown <aaron.f.brown
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Patch
diff mbox series

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index a71f1a5ca27c..8a3cae2367d4 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2234,14 +2234,17 @@  static int igc_add_mac_filter(struct igc_adapter *adapter, const u8 *addr,
 	return -ENOSPC;
 }
 
-/* Remove a MAC filter for 'addr' directing matching traffic to
- * 'queue', 'flags' is used to indicate what kind of match need to be
- * removed, match is by default for the destination address, if
- * matching by source address is to be removed the flag
- * IGC_MAC_STATE_SRC_ADDR can be used.
- */
+/* Delete MAC address filter from adapter.
+ *
+ * @adapter: Pointer to adapter where the filter should be deleted from.
+ * @addr: MAC address.
+ * @flags: Set IGC_MAC_STATE_SRC_ADDR bit to indicate @address is a source
+ * address.
+ *
+ * In case of success, returns 0. Otherwise, it returns a negative errno code.
+  */
 static int igc_del_mac_filter(struct igc_adapter *adapter, const u8 *addr,
-			      const u8 queue, const u8 flags)
+			      const u8 flags)
 {
 	struct igc_hw *hw = &adapter->hw;
 	int rar_entries = hw->mac.rar_entry_count;
@@ -2250,17 +2253,11 @@  static int igc_del_mac_filter(struct igc_adapter *adapter, const u8 *addr,
 	if (!is_valid_ether_addr(addr))
 		return -EINVAL;
 
-	/* Search for matching entry in the MAC table based on given address
-	 * and queue. Do not touch entries at the end of the table reserved
-	 * for the VF MAC addresses.
-	 */
 	for (i = 0; i < rar_entries; i++) {
 		if (!(adapter->mac_table[i].state & IGC_MAC_STATE_IN_USE))
 			continue;
 		if (flags && (adapter->mac_table[i].state & flags) != flags)
 			continue;
-		if (adapter->mac_table[i].queue != queue)
-			continue;
 		if (!ether_addr_equal(adapter->mac_table[i].addr, addr))
 			continue;
 
@@ -2296,7 +2293,7 @@  static int igc_uc_unsync(struct net_device *netdev, const unsigned char *addr)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
 
-	return igc_del_mac_filter(adapter, addr, adapter->num_rx_queues, 0);
+	return igc_del_mac_filter(adapter, addr, 0);
 }
 
 /**
@@ -3741,7 +3738,7 @@  int igc_add_mac_steering_filter(struct igc_adapter *adapter,
 int igc_del_mac_steering_filter(struct igc_adapter *adapter,
 				const u8 *addr, u8 queue, u8 flags)
 {
-	return igc_del_mac_filter(adapter, addr, queue,
+	return igc_del_mac_filter(adapter, addr,
 				  IGC_MAC_STATE_QUEUE_STEERING | flags);
 }