| Message ID | 20200318230102.36952-8-andre.guedes@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Jeff Kirsher |
| Headers | show |
| Series | igc: Refactor MAC address filtering code | expand |
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); > } > >
> 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
> -----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
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); }
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(-)