Re: [Intel-wired-lan] [PATCH iwl-next 1/2] ice: Remove unnecessary argument from ice_fdir_comp_rules()
On 12/5/2023 10:23 PM, Tony Nguyen wrote:
On 11/29/2023 9:55 PM, Lukasz Plachno wrote:
Passing v6 argument is unnecessary as flow_type is still
analyzed inside the function.
Reviewed-by: Przemek Kitszel
Signed-off-by: Lukasz Plachno
This doesn't build cleanly. >
../drivers/net/ethernet/intel/ice/ice_fdir.c: In function
‘ice_fdir_comp_rules’:
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_NONE’ not handled in switch
[-Wswitch]
switch (flow_type) {
^~
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_UDP’ not handled
in switch [-Wswitch]
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_TCP’ not handled
in switch [-Wswitch]
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_ICMP’ not handled
in switch [-Wswitch]
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER’ not handled
in switch [-Wswitch]
../drivers/net/ethernet/intel/ice/ice_fdir.c:1203:2: warning:
enumeration value ‘ICE_FLTR_PTYPE_NONF_IPV6_GTPU_IPV6_OTHER’ not handled
in switch [-Wswitch]
...
I will fix that in v2
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v2 2/2] ice: Implement 'flow-type ether' rules
On 12/12/2023 11:29 AM, Simon Horman wrote:
On Thu, Dec 07, 2023 at 01:48:40PM +0100, Lukasz Plachno wrote:
From: Jakub Buchocki
Add support for 'flow-type ether' Flow Director rules via ethtool.
Rules not containing masks are processed by the Flow Director,
and support the following set of input parameters in all combinations:
src, dst, proto, vlan-etype, vlan, action.
It is possible to specify address mask in ethtool parameters but only
00:00:00:00:00 and FF:FF:FF:FF:FF are valid.
The same applies to proto, vlan-etype and vlan masks:
only 0x and 0x masks are valid.
Signed-off-by: Jakub Buchocki
Co-developed-by: Mateusz Pacuszka
Signed-off-by: Mateusz Pacuszka
Reviewed-by: Przemek Kitszel
Signed-off-by: Lukasz Plachno
...
@@ -1268,6 +1374,16 @@ ice_cfg_fdir_xtrct_seq(struct ice_pf *pf, struct
ethtool_rx_flow_spec *fsp,
ret = ice_set_fdir_ip6_usr_seg(seg, &fsp->m_u.usr_ip6_spec,
&perfect_filter);
break;
+ case ETHER_FLOW:
+ ret = ice_set_ether_flow_seg(seg, &fsp->m_u.ether_spec);
+ if (!ret && (fsp->m_ext.vlan_etype || fsp->m_ext.vlan_tci)) {
+ if (!ice_fdir_vlan_valid(fsp)) {
+ ret = -EINVAL;
+ break;
+ }
+ ret = ice_set_fdir_vlan_seg(seg, &fsp->m_ext);
+ }
+ break;
default:
ret = -EINVAL;
}
Hi Jakub,
A bit further down this function, perfect_filter is used as follows.
...
if (user && user->flex_fltr) {
perfect_filter = false;
...
}
...
assign_bit(fltr_idx, hw->fdir_perfect_fltr, perfect_filter);
And unlike other non-error cases handled in the switch statement,
the new ETHER_FLOW case does not set perfect_filter.
It's unclear to me if this is actually the case or not,
but Smatch flags that perfect_filter may now be used uninitialised
in the assign_bit() call above.
...
Hi Simon,
Thank you for pointing that out, perfect_filter should be initialized to
false, I will fix that in V3.
Thanks,
Łukasz
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v3 2/2] ice: Implement 'flow-type ether' rules
On 12/16/2023 11:03 AM, Simon Horman wrote:
On Thu, Dec 14, 2023 at 05:34:49AM +0100, Lukasz Plachno wrote:
...
@@ -1199,6 +1212,99 @@ ice_set_fdir_ip6_usr_seg(struct ice_flow_seg_info *seg,
return 0;
}
+/**
+ * ice_fdir_vlan_valid - validate VLAN data for Flow Director rule
+ * @fsp: pointer to ethtool Rx flow specification
+ *
+ * Return: true if vlan data is valid, false otherwise
+ */
+static bool ice_fdir_vlan_valid(struct ethtool_rx_flow_spec *fsp)
+{
+ if (fsp->m_ext.vlan_etype &&
+ ntohs(fsp->h_ext.vlan_etype) & ~(ETH_P_8021Q | ETH_P_8021AD))
+ return false;
Hi Jakub and Lukasz,
It is not obvious to me that a bitwise comparison of the vlan_ethtype is
correct. Possibly naively I expected something more like
(completely untested!):
if (!eth_type_vlan(sp->m_ext.vlan_etype))
return false:
+
+ if (fsp->m_ext.vlan_tci &&
+ ntohs(fsp->h_ext.vlan_tci) >= VLAN_N_VID)
+ return false;
+
+ return true;
+}
eth_type_vlan() does what is needed here and is much more readable, I
will switch to it in V4
Thanks,
Łukasz
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v3 2/2] ice: Implement 'flow-type ether' rules
On 12/19/2023 6:35 PM, Brett Creeley wrote:
On 12/13/2023 8:34 PM, Lukasz Plachno wrote:
Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.
From: Jakub Buchocki
Add support for 'flow-type ether' Flow Director rules via ethtool.
Rules not containing masks are processed by the Flow Director,
and support the following set of input parameters in all combinations:
src, dst, proto, vlan-etype, vlan, action.
It is possible to specify address mask in ethtool parameters but only
00:00:00:00:00 and FF:FF:FF:FF:FF are valid.
The same applies to proto, vlan-etype and vlan masks:
only 0x and 0x masks are valid.
Would it be useful to have user facing error messages for invalid masks
stating what the valid masks are?
Do you think about something like:
dev_warn("Driver only supports masks 00:00:00:00:00:00 and
FF:FF:FF:FF:FF:FF"),
or there is a way to pass custom message to ethtool to print it to user?
Signed-off-by: Jakub Buchocki
Co-developed-by: Mateusz Pacuszka
Signed-off-by: Mateusz Pacuszka
Reviewed-by: Przemek Kitszel
Signed-off-by: Lukasz Plachno
---
.../net/ethernet/intel/ice/ice_ethtool_fdir.c | 128 +-
drivers/net/ethernet/intel/ice/ice_fdir.c | 27
drivers/net/ethernet/intel/ice/ice_fdir.h | 11 ++
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
4 files changed, 166 insertions(+), 1 deletion(-)
[...]
___
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Re: [Intel-wired-lan] [PATCH iwl-next v4 2/2] ice: Implement 'flow-type ether' rules
On 1/25/2024 12:09 AM, Paul Menzel wrote:
Dear Lukasz, dear Jakub, dear Mateusz,
Thank you for your patch.
Am 24.01.24 um 16:21 schrieb Lukasz Plachno:
From: Jakub Buchocki
Add support for 'flow-type ether' Flow Director rules via ethtool.
Can you please elaborate on the implementation?
New commit message below
Rules not containing masks are processed by the Flow Director,
and support the following set of input parameters in all combinations:
src, dst, proto, vlan-etype, vlan, action.
It is possible to specify address mask in ethtool parameters but only
00:00:00:00:00 and FF:FF:FF:FF:FF are valid.
The same applies to proto, vlan-etype and vlan masks:
only 0x and 0x masks are valid.
It’d be great, if you gave an example, how you tested this.
Example at the bottom, let me know if such commit message is OK for you.
ice: Implement 'flow-type ether' rules
Add support for 'flow-type ether' Flow Director rules via ethtool.
Create packet segment info for filter configuration based on ethtool
command parameters.
Reuse infrastructure already created for ipv4 and ipv6 flows to convert
packet segment into extraction sequence, which is later used to program
the filter inside Flow Director block of the Rx pipeline.
Rules not containing masks are processed by the Flow Director,
and support the following set of input parameters in all
combinations: src, dst, proto, vlan-etype, vlan, action.
It is possible to specify address mask in ethtool parameters but
only 00:00:00:00:00 and FF:FF:FF:FF:FF are valid.
The same applies to proto, vlan-etype and vlan masks:
only 0x and 0x masks are valid.
Testing:
# (DUT) iperf3 -s
# (DUT) ethtool -U ens785f0np0 flow-type ether dst 00:00:00:00:01:00
action 10
# (DUT) watch 'ethtool -S ens785f0np0 | grep rx_queue'
# (LP) iperf3 -c ${DUT_IP}
Counters increase only for:
'rx_queue_10_packets'
'rx_queue_10_bytes'
Thanks,
Łukasz
Signed-off-by: Jakub Buchocki
Co-developed-by: Mateusz Pacuszka
Signed-off-by: Mateusz Pacuszka
Reviewed-by: Przemek Kitszel
Signed-off-by: Lukasz Plachno
---
.../net/ethernet/intel/ice/ice_ethtool_fdir.c | 140 +-
drivers/net/ethernet/intel/ice/ice_fdir.c | 27
drivers/net/ethernet/intel/ice/ice_fdir.h | 11 ++
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
4 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
index 9a1a04f5f146..d6fadc65a7a6 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
@@ -41,6 +41,8 @@ static struct in6_addr zero_ipv6_addr_mask = {
static int ice_fltr_to_ethtool_flow(enum ice_fltr_ptype flow)
{
switch (flow) {
+ case ICE_FLTR_PTYPE_NONF_ETH:
+ return ETHER_FLOW;
case ICE_FLTR_PTYPE_NONF_IPV4_TCP:
return TCP_V4_FLOW;
case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
@@ -72,6 +74,8 @@ static int ice_fltr_to_ethtool_flow(enum
ice_fltr_ptype flow)
static enum ice_fltr_ptype ice_ethtool_flow_to_fltr(int eth)
{
switch (eth) {
+ case ETHER_FLOW:
+ return ICE_FLTR_PTYPE_NONF_ETH;
case TCP_V4_FLOW:
return ICE_FLTR_PTYPE_NONF_IPV4_TCP;
case UDP_V4_FLOW:
@@ -137,6 +141,15 @@ int ice_get_ethtool_fdir_entry(struct ice_hw *hw,
struct ethtool_rxnfc *cmd)
memset(&fsp->m_ext, 0, sizeof(fsp->m_ext));
switch (fsp->flow_type) {
+ case ETHER_FLOW:
+ fsp->h_u.ether_spec.h_proto = rule->eth.type;
+ fsp->m_u.ether_spec.h_proto = rule->eth_mask.type;
+ ether_addr_copy(fsp->h_u.ether_spec.h_dest, rule->eth.dst);
+ ether_addr_copy(fsp->m_u.ether_spec.h_dest, rule->eth_mask.dst);
+ ether_addr_copy(fsp->h_u.ether_spec.h_source, rule->eth.src);
+ ether_addr_copy(fsp->m_u.ether_spec.h_source,
+ rule->eth_mask.src);
+ break;
case IPV4_USER_FLOW:
fsp->h_u.usr_ip4_spec.ip_ver = ETH_RX_NFC_IP4;
fsp->h_u.usr_ip4_spec.proto = 0;
@@ -1193,6 +1206,111 @@ ice_set_fdir_ip6_usr_seg(struct
ice_flow_seg_info *seg,
return 0;
}
+/**
+ * ice_fdir_vlan_valid - validate VLAN data for Flow Director rule
+ * @fsp: pointer to ethtool Rx flow specification
+ *
+ * Return: true if vlan data is valid, false otherwise
+ */
+static bool ice_fdir_vlan_valid(struct ethtool_rx_flow_spec *fsp)
+{
+ if (fsp->m_ext.vlan_etype && !eth_type_vlan(fsp->h_ext.vlan_etype))
+ return false;
+
+ if (fsp->m_ext.vlan_tci &&
+ ntohs(fsp->h_ext.vlan_tci) >= VLAN_N_VID)
+ return false;
+
+ return true;
+}
+
+/**
+ * ice_set_ether_flow_seg
+ * @seg: flow segment for programming
+ * @eth_spec: mask data from ethtool
+ *
+ * Return: 0 on success and errno in case of error.
+ */
+static int ice_set_ether_flow_seg(struct device *dev,
+ struct ice_flow_seg_info *seg,
+ struct ethhdr *eth_spec)
+{
Re: [Intel-wired-lan] [PATCH iwl-next v5 2/2] ice: Implement 'flow-type ether' rules
On 2/7/2024 3:07 PM, Alexander Lobakin wrote:
From: Lukasz Plachno
Date: Tue, 6 Feb 2024 17:33:37 +0100
From: Jakub Buchocki
Add support for 'flow-type ether' Flow Director rules via ethtool.
Create packet segment info for filter configuration based on ethtool
command parameters. Reuse infrastructure already created for
ipv4 and ipv6 flows to convert packet segment into
extraction sequence, which is later used to program the filter
inside Flow Director block of the Rx pipeline.
[...]
diff --git a/drivers/net/ethernet/intel/ice/ice_fdir.c
b/drivers/net/ethernet/intel/ice/ice_fdir.c
index 1f7b26f38818..5fe0bad00fd7 100644
--- a/drivers/net/ethernet/intel/ice/ice_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_fdir.c
@@ -4,6 +4,8 @@
#include "ice_common.h"
/* These are training packet headers used to program flow director filters. */
+static const u8 ice_fdir_eth_pkt[22] = {0};
I believe this zeroing is not needed, just declare it and the compiler
will zero it automatically.
{0}; will be removed in V6
[...]
@@ -97,6 +100,12 @@ struct ice_rx_flow_userdef {
u16 flex_fltr;
};
+struct ice_fdir_eth {
+ u8 dst[ETH_ALEN];
+ u8 src[ETH_ALEN];
+ __be16 type;
+};
This is clearly `struct ethhdr`, please remove this duplicating
definition and just use the generic structure.
I will fix that in V6, thank you for catching that.
Regards,
Łukasz
Re: [Intel-wired-lan] [PATCH iwl-next v6 2/2] ice: Implement 'flow-type ether' rules
On 2/9/2024 6:00 PM, Simon Horman wrote:
On Fri, Feb 09, 2024 at 11:18:23AM +0100, Lukasz Plachno wrote:
...
diff --git a/drivers/net/ethernet/intel/ice/ice_fdir.c
b/drivers/net/ethernet/intel/ice/ice_fdir.c
index 1f7b26f38818..ec8a84b80a73 100644
--- a/drivers/net/ethernet/intel/ice/ice_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_fdir.c
@@ -4,6 +4,8 @@
#include "ice_common.h"
/* These are training packet headers used to program flow director filters. */
+static const u8 ice_fdir_eth_pkt[22] = {0};
+
I think it was agreed to drop the "{0}" in the review of v5 as it is
unnecessary.
Yes, I haven't included it by my error. Will send V8 with "= {0}"
properly removed.
Thanks,
Łukasz
Re: [Intel-wired-lan] [PATCH iwl-next v7 2/2] ice: Implement 'flow-type ether' rules
On 2/12/2024 12:03 PM, Lukasz Plachno wrote:
From: Jakub Buchocki
Add support for 'flow-type ether' Flow Director rules via ethtool.
Create packet segment info for filter configuration based on ethtool
command parameters. Reuse infrastructure already created for
ipv4 and ipv6 flows to convert packet segment into
extraction sequence, which is later used to program the filter
inside Flow Director block of the Rx pipeline.
Rules not containing masks are processed by the Flow Director,
and support the following set of input parameters in all combinations:
src, dst, proto, vlan-etype, vlan, action.
It is possible to specify address mask in ethtool parameters but only
00:00:00:00:00 and FF:FF:FF:FF:FF are valid.
The same applies to proto, vlan-etype and vlan masks:
only 0x and 0x masks are valid.
Testing:
(DUT) iperf3 -s
(DUT) ethtool -U ens785f0np0 flow-type ether dst \
action 10
(DUT) watch 'ethtool -S ens785f0np0 | grep rx_queue'
(LP) iperf3 -c ${DUT_IP}
Counters increase only for:
'rx_queue_10_packets'
'rx_queue_10_bytes'
Signed-off-by: Jakub Buchocki
Co-developed-by: Mateusz Pacuszka
Signed-off-by: Mateusz Pacuszka
Signed-off-by: Lukasz Plachno
---
.../net/ethernet/intel/ice/ice_ethtool_fdir.c | 130 +-
drivers/net/ethernet/intel/ice/ice_fdir.c | 27
drivers/net/ethernet/intel/ice/ice_fdir.h | 5 +
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
4 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
index 9a1a04f5f146..6963e0da6518 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
@@ -41,6 +41,8 @@ static struct in6_addr zero_ipv6_addr_mask = {
static int ice_fltr_to_ethtool_flow(enum ice_fltr_ptype flow)
{
switch (flow) {
+ case ICE_FLTR_PTYPE_NONF_ETH:
+ return ETHER_FLOW;
case ICE_FLTR_PTYPE_NONF_IPV4_TCP:
return TCP_V4_FLOW;
case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
@@ -72,6 +74,8 @@ static int ice_fltr_to_ethtool_flow(enum ice_fltr_ptype flow)
static enum ice_fltr_ptype ice_ethtool_flow_to_fltr(int eth)
{
switch (eth) {
+ case ETHER_FLOW:
+ return ICE_FLTR_PTYPE_NONF_ETH;
case TCP_V4_FLOW:
return ICE_FLTR_PTYPE_NONF_IPV4_TCP;
case UDP_V4_FLOW:
@@ -137,6 +141,10 @@ int ice_get_ethtool_fdir_entry(struct ice_hw *hw, struct
ethtool_rxnfc *cmd)
memset(&fsp->m_ext, 0, sizeof(fsp->m_ext));
switch (fsp->flow_type) {
+ case ETHER_FLOW:
+ fsp->h_u.ether_spec = rule->eth;
+ fsp->m_u.ether_spec = rule->eth_mask;
+ break;
case IPV4_USER_FLOW:
fsp->h_u.usr_ip4_spec.ip_ver = ETH_RX_NFC_IP4;
fsp->h_u.usr_ip4_spec.proto = 0;
@@ -1193,6 +1201,112 @@ ice_set_fdir_ip6_usr_seg(struct ice_flow_seg_info *seg,
return 0;
}
+/**
+ * ice_fdir_vlan_valid - validate VLAN data for Flow Director rule
+ * @fsp: pointer to ethtool Rx flow specification
+ *
+ * Return: true if vlan data is valid, false otherwise
+ */
+static bool ice_fdir_vlan_valid(struct ethtool_rx_flow_spec *fsp)
+{
+ if (fsp->m_ext.vlan_etype && !eth_type_vlan(fsp->h_ext.vlan_etype))
+ return false;
+
+ if (fsp->m_ext.vlan_tci &&
+ ntohs(fsp->h_ext.vlan_tci) >= VLAN_N_VID)
+ return false;
+
+ return true;
+}
+
+/**
+ * ice_set_ether_flow_seg
+ * @dev: network interface device structure
+ * @seg: flow segment for programming
+ * @eth_spec: mask data from ethtool
+ *
+ * Return: 0 on success and errno in case of error.
+ */
+static int ice_set_ether_flow_seg(struct device *dev,
+ struct ice_flow_seg_info *seg,
+ struct ethhdr *eth_spec)
+{
+ ICE_FLOW_SET_HDRS(seg, ICE_FLOW_SEG_HDR_ETH);
+
+ /* empty rules are not valid */
+ if (is_zero_ether_addr(eth_spec->h_source) &&
+ is_zero_ether_addr(eth_spec->h_dest) &&
+ !eth_spec->h_proto)
+ return -EINVAL;
+
+ /* Ethertype */
+ if (eth_spec->h_proto == htons(0x)) {
+ ice_flow_set_fld(seg, ICE_FLOW_FIELD_IDX_ETH_TYPE,
+ICE_FLOW_FLD_OFF_INVAL,
+ICE_FLOW_FLD_OFF_INVAL,
+ICE_FLOW_FLD_OFF_INVAL, false);
+ } else if (eth_spec->h_proto) {
+ dev_warn(dev, "Only 0x or 0x proto mask is allowed for
flow-type ether");
+ return -EOPNOTSUPP;
+ }
+
+ /* Source MAC address */
+ if (is_broadcast_ether_addr(eth_spec->h_source))
+ ice_flow_set_fld(seg, ICE_FLOW_FIELD_IDX_ETH_SA,
+ICE_FLOW_FLD_OFF_INVAL, ICE_FLOW_FLD_OFF_INVAL,
+
