| Message ID | 20231201075043.7822-2-kurt@linutronix.de (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | igc: ethtool: Check VLAN TCI mask | expand |
Hi Kurt, >+ if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_ETYPE) { >+ fsp->flow_type |= FLOW_EXT; >+ fsp->h_ext.vlan_etype = rule->filter.vlan_etype; >+ fsp->m_ext.vlan_etype = ETHER_TYPE_FULL_MASK; [Suman] User can provide mask for vlan-etype as well. Why not take that rather than hard coding it? I checked you are already doing the same for TCI in the other patch. >+ } >+ > if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_TCI) { > fsp->flow_type |= FLOW_EXT; > fsp->h_ext.vlan_tci = htons(rule->filter.vlan_tci); >-- >2.39.2 >
Hi Suman, On Fri Dec 01 2023, Suman Ghosh wrote: > Hi Kurt, > > >>+ if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_ETYPE) { >>+ fsp->flow_type |= FLOW_EXT; >>+ fsp->h_ext.vlan_etype = rule->filter.vlan_etype; >>+ fsp->m_ext.vlan_etype = ETHER_TYPE_FULL_MASK; > [Suman] User can provide mask for vlan-etype as well. Why not take > that rather than hard coding it? I checked you are already doing the > same for TCI in the other patch. Currently the driver allows/supports to match the VLAN EtherType only by full mask. That is different from TCI, where two different mask values are possible. Thanks, Kurt
>Hi Kurt, > > >>+ if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_ETYPE) { >>+ fsp->flow_type |= FLOW_EXT; >>+ fsp->h_ext.vlan_etype = rule->filter.vlan_etype; >>+ fsp->m_ext.vlan_etype = ETHER_TYPE_FULL_MASK; >[Suman] User can provide mask for vlan-etype as well. Why not take that >rather than hard coding it? I checked you are already doing the same for >TCI in the other patch. Currently the driver allows/supports to match the VLAN EtherType only by full mask. That is different from TCI, where two different mask values are possible. Thanks, Kurt >>+ } [Suman] It is up to you. But I feel, in that case we should have some checks to reject the ntuple rule if user is providing some valid mask value. Otherwise, there will be mismatch between user expectation and driver functionality. >>+ >> if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_TCI) { >> fsp->flow_type |= FLOW_EXT; >> fsp->h_ext.vlan_tci = htons(rule->filter.vlan_tci); >>-- >>2.39.2 >>
> [Suman] It is up to you. But I feel, in that case we should have some > checks to reject the ntuple rule if user is providing some valid mask > value. Sure. That's another issue and deserves a separate patch. > Otherwise, there will be mismatch between user expectation and > driver functionality. Yeah, it's the whole point of series to improve the user experience. Thanks, Kurt
On Fri, Dec 01, 2023 at 08:50:42AM +0100, Kurt Kanzenbach wrote: > Currently the driver allows to configure matching by VLAN EtherType. > However, the retrieval function does not report it back to the user. Add > it. > > Before: > |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 > |Added rule with ID 63 > |root@host:~# ethtool --show-ntuple enp3s0 > |4 RX rings available > |Total 1 rules > | > |Filter: 63 > | Flow Type: Raw Ethernet > | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Ethertype: 0x0 mask: 0xFFFF > | Action: Direct to queue 0 > > After: > |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 > |Added rule with ID 63 > |root@host:~# ethtool --show-ntuple enp3s0 > |4 RX rings available > |Total 1 rules > | > |Filter: 63 > | Flow Type: Raw Ethernet > | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Ethertype: 0x0 mask: 0xFFFF > | VLAN EtherType: 0x8100 mask: 0x0 > | VLAN: 0x0 mask: 0xffff > | User-defined: 0x0 mask: 0xffffffffffffffff > | Action: Direct to queue 0 > > Fixes: 2b477d057e33 ("igc: Integrate flex filter into ethtool ops") > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
On 12/1/2023 09:50, Kurt Kanzenbach wrote: > Currently the driver allows to configure matching by VLAN EtherType. > However, the retrieval function does not report it back to the user. Add > it. > > Before: > |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 > |Added rule with ID 63 > |root@host:~# ethtool --show-ntuple enp3s0 > |4 RX rings available > |Total 1 rules > | > |Filter: 63 > | Flow Type: Raw Ethernet > | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Ethertype: 0x0 mask: 0xFFFF > | Action: Direct to queue 0 > > After: > |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 > |Added rule with ID 63 > |root@host:~# ethtool --show-ntuple enp3s0 > |4 RX rings available > |Total 1 rules > | > |Filter: 63 > | Flow Type: Raw Ethernet > | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF > | Ethertype: 0x0 mask: 0xFFFF > | VLAN EtherType: 0x8100 mask: 0x0 > | VLAN: 0x0 mask: 0xffff > | User-defined: 0x0 mask: 0xffffffffffffffff > | Action: Direct to queue 0 > > Fixes: 2b477d057e33 ("igc: Integrate flex filter into ethtool ops") > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> > --- > drivers/net/ethernet/intel/igc/igc_ethtool.c | 6 ++++++ > 1 file changed, 6 insertions(+) Tested-by: Naama Meir <naamax.meir@linux.intel.com>
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c index 785eaa8e0ba8..69b2fd006293 100644 --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c @@ -980,6 +980,12 @@ static int igc_ethtool_get_nfc_rule(struct igc_adapter *adapter, fsp->m_u.ether_spec.h_proto = ETHER_TYPE_FULL_MASK; } + if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_ETYPE) { + fsp->flow_type |= FLOW_EXT; + fsp->h_ext.vlan_etype = rule->filter.vlan_etype; + fsp->m_ext.vlan_etype = ETHER_TYPE_FULL_MASK; + } + if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_TCI) { fsp->flow_type |= FLOW_EXT; fsp->h_ext.vlan_tci = htons(rule->filter.vlan_tci);
Currently the driver allows to configure matching by VLAN EtherType. However, the retrieval function does not report it back to the user. Add it. Before: |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 |Added rule with ID 63 |root@host:~# ethtool --show-ntuple enp3s0 |4 RX rings available |Total 1 rules | |Filter: 63 | Flow Type: Raw Ethernet | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF | Ethertype: 0x0 mask: 0xFFFF | Action: Direct to queue 0 After: |root@host:~# ethtool -N enp3s0 flow-type ether vlan-etype 0x8100 action 0 |Added rule with ID 63 |root@host:~# ethtool --show-ntuple enp3s0 |4 RX rings available |Total 1 rules | |Filter: 63 | Flow Type: Raw Ethernet | Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF | Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF | Ethertype: 0x0 mask: 0xFFFF | VLAN EtherType: 0x8100 mask: 0x0 | VLAN: 0x0 mask: 0xffff | User-defined: 0x0 mask: 0xffffffffffffffff | Action: Direct to queue 0 Fixes: 2b477d057e33 ("igc: Integrate flex filter into ethtool ops") Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> --- drivers/net/ethernet/intel/igc/igc_ethtool.c | 6 ++++++ 1 file changed, 6 insertions(+)